[Open-FCoE] [PATCH] libhbalinux: use ethtool to get driver name and version

Zou, Yi yi.zou at intel.com
Wed Jan 7 18:15:36 UTC 2009


>
>Hi Yi and all.
>
>I'm taking this opportunity to re-raise some higher-level questions
>about the
>role of this library, as well as to pick some nits about the patch.
>Feel free
>to ignore the nits.  The code looks fine as far as it goes.
>
>Yi Zou wrote:
>> I came across this issue after having my nic driver as built-in, so
>fix this
>> by instead of using sys/class/net/%ifname/device/driver/module,
>changing
>> to query by ethtool, o.w., if the driver is built-in, the above path
>is invalid.
>
>So what good is that path if it can't be used in all kernel
>configurations?
>Don't device modules get a link in the built-in cases?
I did not see the link for built-in ixgbe driver, I don't think the link
is guaranteed to be there all the time. I will take a closer look at
this.

>
>> also, since this info. is not critical, do not fail the query even if
>the
>> driver and version info. is not available, just return w/ dummy info.
>
>I agree.
The patch might be an overkill, for the sake of not failing foceadm,
We only need to make this this does not fail. I thought ethool would
Always in these cases, but forgot about other fnics... 

>
>> +	strncpy(rq.ifr_name, ifname, sizeof(rq.ifr_name)-1);
>
>Put a space around the '-'.  Didn't checkpatch catch this?
No, checkpatch did not catch this...thanks for catching all the style issues, I need double-check my checkpatch switches...

>> +	return -1;
>> +}
>
>The above routine to return void, since that's how it's
>used and how it's documented.
Yes, this should be void.

>
>Also, this won't work for devices that aren't Ethernet, like fnic.
>It seems you're designing this library to be just for fcoe.ko.
>Which would require other libfc LLDs to have their own, practically
>identical vendor-specific library, so I'd like this library to work
>for all libfc LLDs.
Yeah, you are absolutely right.
 
>
>Perhaps the driver name/version should reflect that of the LLD,
>not the underlying network device.  Since fcoe.ko is more like a
>protocol,
>not a device.
Well, the way I see it is that libfcoe is more the protocol and fcoe is
the sw hba.
 
>
>The driver name can be extracted from
>/sys/class/fc_host/host*/symbolic_name.
>We could add the version name there, too.
>
>Please see the patches I submitted on December 4.  Do those look OK?
>If so, when will they get committed?
>
I thought they were already in...let me go back to review it, hopefully
we don't need this patch at all...

Thanks.

Yi

>
>	Regards,
>	Joe Eykholt



More information about the devel mailing list