Date: Tue, 9 Jul 2013 11:20:21 -0400 From: John Baldwin <jhb@freebsd.org> To: Garrett Cooper <yaneurabeya@gmail.com> Cc: svn-src-head <svn-src-head@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, src-committers <src-committers@freebsd.org>, shahark@mellanox.com Subject: Re: svn commit: r253048 - in head/sys/ofed: drivers/net/mlx4 include/linux Message-ID: <201307091120.22114.jhb@freebsd.org> In-Reply-To: <3FF894D3-ACDF-4796-A682-F9F9DD8C943D@gmail.com> References: <201307082125.r68LPDlY023493@svn.freebsd.org> <3FF894D3-ACDF-4796-A682-F9F9DD8C943D@gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday, July 08, 2013 6:13:22 pm Garrett Cooper wrote: > On Jul 8, 2013, at 2:25 PM, John Baldwin wrote: > > > Author: jhb > > Date: Mon Jul 8 21:25:12 2013 > > New Revision: 253048 > > URL: http://svnweb.freebsd.org/changeset/base/253048 > > > > Log: > > Allow mlx4 devices to switch from Ethernet to Infiniband (and vice versa): > > - Fix sysctl wrapper for sysfs attributes to properly handle new string > > values similar to sysctl_handle_string() (only copyin the user's > > supplied length and nul-terminate the string). > > - Don't check for a trailing newline when evaluating the desired operating > > mode of a mlx4 device. > > > > PR: kern/179999 > > Submitted by: Shahar Klein <shahark@mellanox.com> > > MFC after: 1 week > > Was there an issue with the patch I submitted via http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/174213 (basically solving the same problem, but only with the sysfs <-> sysctl(9) handler)? I was of the impression that sysfs on Linux always added on trailing newlines (but I could be wrong because I haven't used Linux at a dev level for ages). Thanks! I hadn't seen it. I had wondered if the '\n' issue was a generic sysfs thing. It sounds like it is and I'd be happy to revert the mlx4 change and alter the sysfs bits to manage the newline directly if that is more appropriate. I'd also like this to use sysctl_handle_string() if at all possible. Are you in a position to test patches still? If so, maybe give this a whirl. It's similar to yours except it uses sysctl_handle_string() and strlcat() rather than continuing to do things by hand. It also outputs an empty string to userland if the attribute doesn't have a show method (your version would never pass out an old string in that case unlike the original code). Index: drivers/net/mlx4/main.c =================================================================== --- drivers/net/mlx4/main.c (revision 253096) +++ drivers/net/mlx4/main.c (working copy) @@ -479,11 +479,11 @@ int i; int err = 0; - if (!strcmp(buf, "ib")) + if (!strcmp(buf, "ib\n")) info->tmp_type = MLX4_PORT_TYPE_IB; - else if (!strcmp(buf, "eth")) + else if (!strcmp(buf, "eth\n")) info->tmp_type = MLX4_PORT_TYPE_ETH; - else if (!strcmp(buf, "auto")) + else if (!strcmp(buf, "auto\n")) info->tmp_type = MLX4_PORT_TYPE_AUTO; else { mlx4_err(mdev, "%s is not supported port type\n", buf); Index: include/linux/sysfs.h =================================================================== --- include/linux/sysfs.h (revision 253096) +++ include/linux/sysfs.h (working copy) @@ -81,37 +81,35 @@ kobj = arg1; attr = (struct attribute *)arg2; - buf = (void *)get_zeroed_page(GFP_KERNEL); - len = 1; /* Copy out a NULL byte at least. */ if (kobj->ktype == NULL || kobj->ktype->sysfs_ops == NULL) return (ENODEV); - ops = kobj->ktype->sysfs_ops; + buf = (void *)get_zeroed_page(GFP_KERNEL); if (buf == NULL) return (ENOMEM); + ops = kobj->ktype->sysfs_ops; if (ops->show) { len = ops->show(kobj, attr, buf); /* - * It's valid not to have a 'show' so we just return 1 byte - * of NULL. + * It's valid to not have a 'show' so just return an + * empty string. */ if (len < 0) { error = -len; - len = 1; if (error != EIO) goto out; } + + /* Trim trailing newline. */ + len--; + buf[len] = '\0'; } - error = SYSCTL_OUT(req, buf, len); - if (error || !req->newptr || ops->store == NULL) + + /* Leave one trailing byte to append a newline. */ + error = sysctl_handle_string(oidp, buf, PAGE_SIZE - 1, req); + if (error != 0 || req->newptr == NULL || ops->store == NULL) goto out; - len = req->newlen - req->newidx; - if (len >= PAGE_SIZE) - error = EINVAL; - else - error = SYSCTL_IN(req, buf, len); - if (error) - goto out; - ((char *)buf)[len] = '\0'; + len = strlcat(buf, "\n", PAGE_SIZE); + KASSERT(len < PAGE_SIZE, ("new attribute truncated")); len = ops->store(kobj, attr, buf, len); if (len < 0) error = -len; -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201307091120.22114.jhb>