RE: Submission for S2io 10GbE driver

["Leonid Grossman" <leonid.grossman@s2io.com>]

Hi Stephen,
Below are responses from our developer.

Thanks for the input, Leonid

> -----Original Message-----
> From: Stephen Hemminger [mailto:shemminger@osdl.org] 
> Sent: Friday, January 23, 2004 4:22 PM
> To: Andi Kleen
> Cc: Leonid Grossman; netdev@oss.sgi.com
> Subject: Re: Submission for S2io 10GbE driver
> 
> 
> Noticed the setup loopback test seems to register for a 
> packet type and then forget to unregister that type! 

The packet type gets unregistered at the end of the test in the
'reset_loopback'
function through the system call 'dev_remove_pack()'

> 
> Also nothing really restricts the packet type to only coming 
> in on the expected interface; therefore if someone sends the 
> same packet in over another interface, then sp->loop_pkt_cnt 
> will end up incrementing some other drivers private data 
> structure *bad*.

Correct, that's why in the s2io.c source where I define the
packet_type's protocol value
through the Macro 'ETH_LOOP_TEST_TYPE' there's a ToDo to obtain a
private protocol ID. 
This way no app in the real world can ever pass a frame with that T/L
field in the packet.
Also, the problem can only happen during the 3 second duration when this
test is in progress.

> 
> IMHO the whole loopback test frame stuff seems like something 
> in a test bed driver, not production code.

The loopback test is there as a part of the ethtool's diagnostic option.


There are pros and cons of having the test in there I guess, anyone else
has an opinion on this?

Do other net drivers normally support loopback and other diag tests as a
part of the ethtool support,
or they provide little/no support for the option and ship a standalone
diag program instead?

Thanks, Leonid

>