[PATCH] libertas: Fix ethtool statistics

Sebastian Siewior liber+tas at ml.breakpoint.cc
Sun Apr 20 04:41:01 EDT 2008


* Holger Schurig | 2008-04-18 15:10:53 [+0200]:

>> > I wrote "if (ret) return ret", not "if (ret) return;".
>>
>> I know you did, but I don't know why. The
>> lbs_ethtool_get_stats() function returns void. Sebastian's
>> patch which memsets it to zero makes sense, but it can't
>> return an error.
>
>Ahh, I stand corrected.
The function before lbs_ethtool_get_stats() (in net code that
determinate the number of elements, dunno the name right now) is able to
return an error. This one could check if the firmware has mesh support.

>> They're separate problems, really. With or without the patch I
>> first posted, you're getting crap back when you ask for
>> statistics on a non-mesh device. The second patch fixes that,
>> and stands alone.

Do I understand this correct: If the firmware has support for mesh
devices than we have ethX and mshX and only mshX should return the
statistics?

>Yes, this is separate. That's why I wrote "The ultimate patch 
>would be".
>
>BTW, I like your two patches combined more than Sebastians memset 
>approach. Because with Sebastians patch on on ethX we would get 
>zeroed nonsense instead of uninitialized nonsense.
returning uninitialized nonsense is leaking kernel memory.

>The memset should possible there anyway in the case of a firmware 
>that can mshX, but not CMD_ACT_MESH_GET_STATS or which would 
>return some error for another reason.
What about changing lbs_ethtool_get_stats() from void to int? All other
drivers are reading memory to get this data, we have to go through
usb/cs/sdio layer and all of them may fail.

I will try to form a patch around Monday that fixes Dan's comments (if
nobody else is going to).

Sebastian



More information about the libertas-dev mailing list