| Message ID | 20211012132525.457323-1-idosch@idosch.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | ethtool: Use memory maps for EEPROM parsing | expand |
On Tue, Oct 12, 2021 at 04:25:11PM +0300, Ido Schimmel wrote: > From: Ido Schimmel <idosch@nvidia.com> > > This patchset prepares ethtool(8) for retrieval and parsing of optional > and banked module EEPROM pages, such as the ones present in CMIS. This > is done by better integration of the recent 'MODULE_EEPROM_GET' netlink > interface into ethtool(8). I still need to take a closer look at some of the patches but just to be sure: the only reason to leave this series for 5.16 cycle is that it's rather big and intrusive change (i.e. it does not depend on any kernel functionality not present in 5.15), right? Michal
On Wed, Oct 27, 2021 at 10:30:45PM +0200, Michal Kubecek wrote: > On Tue, Oct 12, 2021 at 04:25:11PM +0300, Ido Schimmel wrote: > > From: Ido Schimmel <idosch@nvidia.com> > > > > This patchset prepares ethtool(8) for retrieval and parsing of optional > > and banked module EEPROM pages, such as the ones present in CMIS. This > > is done by better integration of the recent 'MODULE_EEPROM_GET' netlink > > interface into ethtool(8). > > I still need to take a closer look at some of the patches but just to be > sure: the only reason to leave this series for 5.16 cycle is that it's > rather big and intrusive change (i.e. it does not depend on any kernel > functionality not present in 5.15), right? Right, it does not depend on any kernel functionality not present in 5.15
On Thu, Oct 28, 2021 at 01:01:05AM +0300, Ido Schimmel wrote: > On Wed, Oct 27, 2021 at 10:30:45PM +0200, Michal Kubecek wrote: > > On Tue, Oct 12, 2021 at 04:25:11PM +0300, Ido Schimmel wrote: > > > From: Ido Schimmel <idosch@nvidia.com> > > > > > > This patchset prepares ethtool(8) for retrieval and parsing of optional > > > and banked module EEPROM pages, such as the ones present in CMIS. This > > > is done by better integration of the recent 'MODULE_EEPROM_GET' netlink > > > interface into ethtool(8). > > > > I still need to take a closer look at some of the patches but just to be > > sure: the only reason to leave this series for 5.16 cycle is that it's > > rather big and intrusive change (i.e. it does not depend on any kernel > > functionality not present in 5.15), right? > > Right, it does not depend on any kernel functionality not present in > 5.15 Can the patchset be applied to ethtool-next please? It is marked as "Under Review" for over a month now and I would like to make progress with other patches I have in the queue. Thanks
Hello: This series was applied to ethtool/ethtool.git (master) by Michal Kubecek <mkubecek@suse.cz>: On Tue, 12 Oct 2021 16:25:11 +0300 you wrote: > From: Ido Schimmel <idosch@nvidia.com> > > This patchset prepares ethtool(8) for retrieval and parsing of optional > and banked module EEPROM pages, such as the ones present in CMIS. This > is done by better integration of the recent 'MODULE_EEPROM_GET' netlink > interface into ethtool(8). > > [...] Here is the summary with links: - [ethtool-next,01/14] cmis: Rename CMIS parsing functions https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=795f42092f20 - [ethtool-next,02/14] cmis: Initialize CMIS memory map https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=369b43a1a066 - [ethtool-next,03/14] cmis: Use memory map during parsing https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=da1628840bd6 - [ethtool-next,04/14] cmis: Consolidate code between IOCTL and netlink paths https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=6acaeb94402a - [ethtool-next,05/14] sff-8636: Rename SFF-8636 parsing functions https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=d7d15f737ab7 - [ethtool-next,06/14] sff-8636: Initialize SFF-8636 memory map https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=4230597fe952 - [ethtool-next,07/14] sff-8636: Use memory map during parsing https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=b74c040256de - [ethtool-next,08/14] sff-8636: Consolidate code between IOCTL and netlink paths https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=799572f86647 - [ethtool-next,09/14] sff-8079: Split SFF-8079 parsing function https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=9fdf45ca1726 - [ethtool-next,10/14] netlink: eeprom: Export a function to request an EEPROM page https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=2ccda2570d65 - [ethtool-next,11/14] cmis: Request specific pages for parsing in netlink path https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=86792dbbebf3 - [ethtool-next,12/14] sff-8636: Request specific pages for parsing in netlink path https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=6e2b32a0d0ea - [ethtool-next,13/14] sff-8079: Request specific pages for parsing in netlink path https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=c2170d40b6a1 - [ethtool-next,14/14] netlink: eeprom: Defer page requests to individual parsers https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=9538f384b535 You are awesome, thank you!
From: Ido Schimmel <idosch@nvidia.com> This patchset prepares ethtool(8) for retrieval and parsing of optional and banked module EEPROM pages, such as the ones present in CMIS. This is done by better integration of the recent 'MODULE_EEPROM_GET' netlink interface into ethtool(8). Background ========== ethtool(8) contains parsers for various module EEPROM memory maps such as SFF-8079, SFF-8636 and CMIS. Using the legacy IOCTL interface, ethtool(8) can ask the kernel to provide a buffer with the EEPROM contents. The buffer is then passed to the parsers that parse and print the EEPROM contents. The major disadvantage of this method is that in addition to ethtool(8), the kernel also needs to be familiar with the layout of the various memory maps, as it should not report to user space optional pages that do not exist. In addition, with the emergence of more complex layouts (e.g., CMIS) that include both optional and banked pages, the layout of the linear buffer provided by the kernel is unclear. For these reasons, kernel 5.13 was extended with the 'MODULE_EEPROM_GET' netlink message that allows user space to request specific EEPROM pages. Motivation ========== Unfortunately, the current integration of 'MODULE_EEPROM_GET' into ethtool(8) is not ideal. In the IOCTL path, a single large buffer is passed to the parsers, whereas in the netlink path, individual pages are passed. This is problematic for several reasons. First, this approach is not very scalable as standards such as CMIS support a lot of optional and banked pages. Passing them as separate arguments is not going to work. Second, the knowledge of which optional and banked pages are available should be encapsulated in the individual parsers, not in the common netlink code (i.e., netlink/module-eeprom.c). Currently, the common code is blindly requesting from the kernel optional pages that might not exist. Third, the difference in the way information is passed to the parsers propagates all the way to the individual parsing functions. For example, cmis_show_link_len() vs. cmis_show_link_len_from_page(). Implementation ============== In order to solve above mentioned problems and make it easier to integrate retrieval and parsing of optional and banked pages, this patchset reworks the EEPROM parsing code to use memory maps. For each parser, a structure describing the layout of the memory map is initialized with pointers to individual pages. In the IOCTL path, this structure contains pointers to sections of the linear buffer that was retrieved from the kernel. In the netlink path, this structure contains pointers to individual pages requested from the kernel. Care is taken to ensure that pages that do not exist are not requested from the kernel. After the structure is initialized, it is passed to the parsing code that parses and prints the information. This approach can be easily extended to support more optional and banked pages and allows us to keep the parsing code common to both the IOCTL and netlink paths. The only difference lies in how the memory map is initialized when the parser is invoked. Testing ======= Build tested each patch with the following configuration options: netlink | pretty-dump --------|------------ v | v x | x v | x x | v No differences in output before and after the patchset (*). Tested with QSFP (PC/AOC), QSFP-DD (PC/AOC), SFP (PC) and both IOCTL and netlink. No reports from AddressSanitizer / valgrind. (*) The only difference is in a few registers in CMIS that were not parsed correctly to begin with. Patchset overview ================= Patches #1-#4 move CMIS to use a memory map and consolidate the code paths between the IOCTL and netlink paths. Patches #5-#8 do the same for SFF-8636. Patch #9 does the same for SFF-8079. Patch #10 exports a function to allow parsers to request a specific EEPROM page. Patches #11-#13 change parsers to request only specific and valid EEPROM pages instead of getting potentially invalid pages from the common netlink code (i.e., netlink/module-eeprom.c). Patch #14 converts the common netlink code to simply call into individual parsers based on their SFF-8024 Identifier Value. The command context is passed to these parsers instead of potentially invalid pages. Ido Schimmel (14): cmis: Rename CMIS parsing functions cmis: Initialize CMIS memory map cmis: Use memory map during parsing cmis: Consolidate code between IOCTL and netlink paths sff-8636: Rename SFF-8636 parsing functions sff-8636: Initialize SFF-8636 memory map sff-8636: Use memory map during parsing sff-8636: Consolidate code between IOCTL and netlink paths sff-8079: Split SFF-8079 parsing function netlink: eeprom: Export a function to request an EEPROM page cmis: Request specific pages for parsing in netlink path sff-8636: Request specific pages for parsing in netlink path sff-8079: Request specific pages for parsing in netlink path netlink: eeprom: Defer page requests to individual parsers cmis.c | 268 ++++++++++++++-------- cmis.h | 8 +- ethtool.c | 8 +- internal.h | 8 +- netlink/extapi.h | 11 + netlink/module-eeprom.c | 318 ++++++++------------------ qsfp.c | 484 +++++++++++++++++++++++++--------------- sfpid.c | 28 ++- 8 files changed, 635 insertions(+), 498 deletions(-)