On Sat, Jun 27, 2020 at 09:42:10PM +0100, Adrian Pop wrote:
> >
> > Hi Adrian, Andrew,
> >
> > Not sure I understand... You want the kernel to always pass page 03h to
> > user space (potentially zeroed)? Page 03h is not mandatory according to
> > the standard and page 01h contains information if page 03h is present or
> 
> Hi Ido!
> 
> Andrew was thinking of having 03h after 02h (potentially zeroed) just
> for the purpose of having a similar layout for QSFP-DD the same way we
> do for QSFP. But as you said, it is not mandatory according to the
> standard and I also don't know the entire codebase for ethtool and
> where it might be actually needed. I think Andrew can argue for its
> presence better than me.
> 
> > not. So user space has the information it needs to determine if after
> > page 02h we have page 03h or page 10h. Why always pass page 03h then?
> >
> 
> If we decide to add 03h but only sometimes, I think we will add an
> extra layer of complexity. Sometimes after 02h we would have 03h and
> sometimes 10h. In qsfp-dd.h (following the convention from qsfp.h) in
> my patch there are a lot of different constants defined with respect
> to the offset of the parent page in the memory layout and "dynamic
> offsets" don't sound very good, at least for me. So even if there's a
> way of checking in the user space which page is after 02h, a more
> stable memory layout works better on the long run.

Adrian,

Thanks for the detailed response. I don't think the kernel should pass
fake pages only to make it easier for user space to parse the
information. What you are describing is basic dissection and it's done
all the time by wireshark / tcpdump.

Anyway, even we pass a fake page 03h, page 11h can still be at a
variable offset. See table 8-28 [1], bits 1-0 at offset 142 in page 01h
determine the size of pages 10h and 11h:

0 - each page is 128 bytes in size
1 - each page is 256 bytes in size
2 - each page is 512 bytes in size

So a completely stable layout (unless I missed something) will entail
the kernel sending 1664 bytes to user space each time. This looks
unnecessarily rigid to me. The people who wrote the standard obviously
took into account the fact that the page layout needs to be discoverable
from the data and I think we should embrace it and only pass valid
information to user space.

Regardless, can Andrew and you let us know if you have a problems with
current patch set which only exposes pages 00h-02h? I see it's marked as
"Changes Requested", so I will need to re-submit.

Thanks

[1] http://www.qsfp-dd.com/wp-content/uploads/2019/05/QSFP-DD-CMIS-rev4p0.pdf

Reply via email to