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

On 3/23/2021 7:47 PM, Don Bollinger wrote:

quoted

quoted

quoted

I don't even see a need for this. The offset should be within one
1/2

page, of

quoted

one bank. So offset >= 0 and <= 127. Length is also > 0 and
<- 127. And offset+length is <= 127.

I like the clean approach, but...   How do you request low memory?

Duh!

I got my conditions wrong. Too focused on 1/2 pages to think that two of
them makes one page!

Lets try again:

offset < 256
0 < len < 128

Actually 0 < len <= 128.  Length of 128 is not only legal, but very common.
"Read the whole 1/2 page block".

Ack.

quoted

if (offset < 128)
    offset + len < 128

Again, offset + len <= 128

quoted

else
    offset + len < 256

offset + len <= 256

Ack.

quoted

Does that look better?

Reading bytes from the lower 1/2 of page 0 should give the same data as
reading data from the lower 1/2 of page 42. So we can allow that, but

don't

quoted

be too surprised when an SFP gets it wrong and gives you rubbish. I would

The spec is clear that the lower half is the same for all pages.  If the SFP
gives you rubbish you should throw the device in the rubbish.

quoted

suggest ethtool(1) never actually does read from the lower 1/2 of any page
other than 0.

I agree, despite my previous comment.  While the spec is clear that should
work, I believe virtually all such instances are bugs not yet discovered.

Agreed, so we will accept offset < 128 only on page zero.

And, note that the legacy API provides no way to access lower memory from
any page but 0.  There's just no syntax for it.  Not that we care about
legacy :-).

quoted

And i agree about documentation. I would suggest a comment in
ethtool_netlink.h, and the RST documentation.

Ack, will comment there on limiting new KAPI only to half pages reading.

We may address reading cross pages by user space (implementing multiple 
calls to KAPI to get such data).

quoted

                  Andrew

Don