Re: [RFC PATCH V4 net-next 1/5] ethtool: Allow network drivers to dump arbitrary EEPROM data

On 3/23/2021 2:27 AM, Andrew Lunn wrote:

quoted

quoted

+#define ETH_MODULE_EEPROM_PAGE_LEN 256

Sorry to keep raising issues, but I think you want to make this constant
128.

Yes, i also think the KAPI should be limited to returning a maximum of
a 1/2 page per call.

OK.

quoted

quoted

+#define MODULE_EEPROM_MAX_OFFSET (257 *
ETH_MODULE_EEPROM_PAGE_LEN)

The device actually has 257 addressable chunks of 128 bytes each.  With
ETH_MODULE_EEPROM_PAGE_LEN set to 256, your constant is 2X too big.

Note also, SFP devices (but not QSFP or CMIS) actually have another 256
bytes available at 0x50, in addition to the full 257*128 at 0x51.  So SFP is
actually 259*128 or (256 + 257 * 128).

Devices that don't support pages have much lower limits (256 bytes for
QSFP/CMIS and 512 for SFP).  Some SFP only support 256 bytes.  Most devices
will return nonsense for pages above 3.  So, this check is really only an
absolute limit.  The SFP driver that takes this request will probably check
against a more refined MAX length (eg modinfo->eeprom_len).

I suggest setting this constant to 259 * (ETH_MODULE_EEPROM_PAGE_LEN / 2).
Let the driver refine it from there.

I don't even see a need for this. The offset should be within one 1/2
page, of one bank. So offset >= 0 and <= 127. Length is also > 0 and
<- 127. And offset+length is <= 127.

For the moment, please forget about backwards compatibility with the
IOCTL interface. Lets get a new clean KAPI and a new clean internal
API between the ethtool core and the drivers. Once we have that agreed
on, we can work on the various compatibility shims we need to work
between old and new APIs in various places.

OK, so following the comments here, I will ignore backward compatibility 
of having global offset and length.

That means I assume in this KAPI I am asked to get data from specific 
page. Should I also require user space to send page number to this KAPI 
(I mean make page number mandatory) ?

       Andrew