netdev - Re: [PATCH] usbnet: Do not implement ethtool get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20091103.010129.100672838.davem@davemloft.net>
Date:	Tue, 03 Nov 2009 01:01:29 -0800 (PST)
From:	David Miller <davem@...emloft.net>
To:	david-b@...bell.net
Cc:	ben@...adent.org.uk, dbrownell@...rs.sourceforge.net,
	greg@...ah.com, jacmet@...site.dk, steve.glendinning@...c.com,
	netdev@...r.kernel.org
Subject: Re: [PATCH] usbnet: Do not implement ethtool get_link() if link
 state is unknown

From: David Brownell <david-b@...bell.net>
Date: Mon, 2 Nov 2009 23:01:30 -0700

> On Monday 02 November 2009, Ben Hutchings wrote:
>> @@ -854,7 +854,7 @@ void usbnet_set_msglevel (struct net_device *net, u32 level)
>>  EXPORT_SYMBOL_GPL(usbnet_set_msglevel);
>>  
>>  /* drivers may override default ethtool_ops in their bind() routine */
>> -static const struct ethtool_ops usbnet_ethtool_ops = {
>> +static const struct ethtool_ops usbnet_get_link_ethtool_ops = {
>>         .get_settings           = usbnet_get_settings,
>>         .set_settings           = usbnet_set_settings,
>>         .get_link               = usbnet_get_link,
>> @@ -864,6 +864,15 @@ static const struct ethtool_ops usbnet_ethtool_ops = {
>>         .set_msglevel           = usbnet_set_msglevel,
>>  };
>>  
>> +static const struct ethtool_ops usbnet_ethtool_ops = {
>> +       .get_settings           = usbnet_get_settings,
>> +       .set_settings           = usbnet_set_settings,
> 
> Surely there's a code that usbnet_get_link() could return
> to say "I can't really tell"?
> 
> And if there isn't, there should be one.

Having a NULL operations pointer for this function is how to
indicate this.  It's a static situation based upon the device
type, not a dynamic one which would be resolved at run time
when inspecting the device registers for example.

> Having two tables for this is needlessly ugly.

Yes, it's really cruddy how the USB network driver tries to share
so much state amongst such very different devices :-)

All kidding aside, I think the alternative is for the USB network
driver to call ethtool_op_get_link() if it cannot determine the
link state in hardware.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html