svn commit: r253048 - in head/sys/ofed: drivers/net/mlx4 include/linux

[Garrett Cooper]

On Thu, Jul 11, 2013 at 5:26 AM, John Baldwin <jhb at freebsd.org> wrote:
> On Wednesday, July 10, 2013 7:58:30 pm Garrett Cooper wrote:
>> On Tue, Jul 9, 2013 at 8:20 AM, John Baldwin <jhb at freebsd.org> wrote:
>>
>> ...
>>
>> > 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'll doublecheck this, but basically I'm really adverse to diverging
>> from Linux in this area -- hence that's why I did what I did in the PR
>> I mentioned.
>
> I checked the other sysfs attributes under sys/ofed and all the ones that deal
> with strings expect the newline.  There might be some that are "blobs" that do
> not expect newlines, but it wasn't clear.  I do agree that it would be best to
> avoid diffs for this case if possible in all the sysfs handlers.
>
> (It would be even nicer if ofed used a more abstract notion for device attributes
> in its drivers that could map to sysctl on FreeBSD and sysfs device attributes on
> Linux, but that's a much larger change, and one OFED would have to want to do.)
>
>> > I'd also like this to use sysctl_handle_string() if at all possible.  Are you in
>> > a position to test patches still?
>>
>> Unfortunately I'm not right now :(. Anthony may or may not be able to
>> test this out (I used his machine when we were hacking around on IB
>> stuff).
>>
>> > 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).
>>
>> Can you please pass along a patch to Anthony and me so we can try and
>> apply it to test it out (Gmail's going to taint the inline patch, as
>> is the EMC Exchange server with the patch attachment)? If Anthony
>> still has the machine setup, then I'd be more than happy to test out
>> the patch :).
>
> Ok, I've put the patch at a URL.  Note that it is relative to what was just
> committed to HEAD, so to test on 9 you'd have to apply both patches.
>
> http://www.freebsd.org/~jhb/patches/ofed_sysfs.patch

Sorry, but it seems that neither Anthony nor I have time to work on
this... FWIW my patch might have been ok for text, but not for binary
blobs (I didn't test that out before because I didn't have an easy
testcase for it).