RE: [PATCH v2] eeprom/optoe: driver to read/write SFP/QSFP/CMIS EEPROMS

[Don Bollinger]

> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@xxxxxxx]
> Sent: Friday, March 26, 2021 2:54 PM
> To: Don Bollinger <don@xxxxxxxxxxxxxxxxx>
> Cc: 'Jakub Kicinski' <kuba@xxxxxxxxxx>; arndb@xxxxxxxx;
> gregkh@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> brandon_chuang@xxxxxxxxxxxxx; wally_wang@xxxxxxxxxx;
> aken_liu@xxxxxxxxxxxxx; gulv@xxxxxxxxxxxxx; jolevequ@xxxxxxxxxxxxx;
> xinxliu@xxxxxxxxxxxxx; 'netdev' <netdev@xxxxxxxxxxxxxxx>; 'Moshe
> Shemesh' <moshe@xxxxxxxxxx>
> Subject: Re: [PATCH v2] eeprom/optoe: driver to read/write SFP/QSFP/CMIS
> EEPROMS
> 
> > The only thing wrong I can see is that it doesn't use the kernel
> > network stack.  Here's where "you keep missing the point".  The kernel
> > network stack is not being used in these systems.  Implementing EEPROM
> > access through ethtool is of course possible, but it is a lot of work
> > with no benefit on these systems.  The simple approach works.  Let me
use
> it.
> >
> > >
> > > The optoe KAPI needs to handle these 'interesting' SFP modules. The
> > > KAPI design needs to be flexible enough that the driver underneath
> > > it can be extended to support these SFPs. The code does not need to
> > > be there, but the KAPI design needs to allow it. And i personally
> > > cannot see how the
> > optoe
> > > KAPI can efficiently support these SFPs.
> >
> > Help me understand.  Your KAPI specifies ethtool as the KAPI, and the
> > ethtool/netlink stack as the interface through which the data flows.
> 
> Nearly. It specifies netlink and the netlink messages definitions.
> They happen to be in the ethtool group, but there is no requirement to use
> ethtool(1).
> 
> But there is a bit more to it.
> 
> Either the MAC driver implements the needed code, or it defers to the
> generic sfp.c code. In both cases, you have access to the GPIO lines.
> 
> If you are using the generic sfp.c, the Device Tree definitions of the
GPIOs
> become part of the KAPI. DT is consider ABI.
> 
> So the low level code knows when there was been a hotplug. It then can
> determine what quirks to apply for all future reads until the module is
> unplugged.

Got it.  All good.  I agree, I would like optoe to have access to the GPIO
lines, but it is a "nice to have", not a requirement...

> 
> Your KAPI is missing how optoe gets access to the GPIOs. Without knowing
if

Right.  It doesn't get access to the GPIOs.  So, that is not part of its
KAPI.

> the module has been hotplugged, in a robust manor, you have problems
> with quirks. For every userspace read, you need to assume the module has
> been changed, read the ID information from the EEPROM a byte at a time,
> figure out what quirks to apply, and then do the user requested read. I
doubt

Again, optoe does not deal with quirks.  Your code filters them out before
calling optoe or sfp_i2c_read.  My stack does not deal with them.  In my
community, quirky devices are not tolerated.  In the 4 years this code has
been in use, nobody has ever asked me to accommodate a weird module.

I inherited a limitation of writing one byte at a time from the ancestor of
optoe, which I have kept.  I don't know if it is needed, but someone once
thought it was required.  I apply it universally, all devices of all types.

I do need one item of state from the EEPROM, the register that tells me
whether pages are supported by the device.  Due to the hotplug risk, I
actually do read that register once for each access to non-zero pages.
(Once per read or write call.)  Access to GPIOs would eliminate that.  It
turns out the vast majority of calls are to page 0 or lower memory, and the
performance penalty is at most 25% since there is also a page write, data
access and page write in the same call.  The penalty goes down as the number
of bytes read increases.  Overall, it has never come up as an issue.  People
don't expect these things to be fast.

> that is good for performance. The design which has been chosen is that
> userspace is monitoring the GPIO lines. So to make it efficient, your KAPI
> need some way to pass down that the module has/has not been hot-
> plugged since the last read.

Nope.  optoe does not need to know, it assumes a new device every time it is
accessed.

> 
> Or do you see some other way to implement these quirks?

What I have works.  Your consumers get quirk handling, mine don't need it.
No compromise.

> 
>        Andrew

Don