[net-next,v5,1/2] ethtool: add speed/duplex validation functions

On Wed, Feb 03, 2016 at 03:49:04PM -0800, Rick Jones wrote:
> On 02/03/2016 03:32 PM, Stephen Hemminger wrote:
> 
> >But why check for valid value at all. At some point in the
> >future, there will be yet another speed adopted by some standard body
> >and the switch statement would need another value.
> >
> >Why not accept any value? This is a virtual device.
> >
> 
> And even for not-quite-virtual devices - such as a VC/FlexNIC in an HPE
> blade server there can be just about any speed set.  I think we went down a
> path of patching some things to address that many years ago.  It would be a
> shame to undo that.
> 
> rick

I'm not sure I understand. The question is in defining the UAPI.
We currently have:

 * @speed: Low bits of the speed
 * @speed_hi: Hi bits of the speed

with the assumption that all values come from the defines.

So if we allow any value here we need to define what it means.

If the following is acceptable, then we can drop
most of validation:


--->
ethtool: future-proof interface for speed extensions

Many virtual and not quite virtual devices allow any speed to be set
through ethtool. Document this fact to make sure people don't assume the
enum lists all possible values.  Reserve values greater than INT_MAX for
future extension and to avoid conflict with SPEED_UNKNOWN.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----

On 02/04/2016 04:47 AM, Michael S. Tsirkin wrote:
> On Wed, Feb 03, 2016 at 03:49:04PM -0800, Rick Jones wrote:
>> And even for not-quite-virtual devices - such as a VC/FlexNIC in an HPE
>> blade server there can be just about any speed set.  I think we went down a
>> path of patching some things to address that many years ago.  It would be a
>> shame to undo that.
>>
>> rick
>
> I'm not sure I understand. The question is in defining the UAPI.
> We currently have:
>
>   * @speed: Low bits of the speed
>   * @speed_hi: Hi bits of the speed
>
> with the assumption that all values come from the defines.
>
> So if we allow any value here we need to define what it means.

I may be mixing apples and kiwis.  Many years ago when HP came-out with 
their blades and VirtualConnect, they included the ability to create 
"flex NICs" - "sub-NICs" out of a given interface port on a blade, and 
to assign each a specific bitrate in increments (IIRC) of 100 Mbit/s. 
This was reported up through the driver and it became necessary to make 
ethtool (again, IIRC) not so picky about "valid" speed values.

rick