Hi Bruce, > -----Original Message----- > From: Richardson, Bruce <bruce.richard...@intel.com> > Sent: Friday, April 8, 2022 6:33 PM > To: Zhang, RobinX <robinx.zh...@intel.com> > Cc: dev@dpdk.org; Yang, Qiming <qiming.y...@intel.com>; Zhang, Qi Z > <qi.z.zh...@intel.com>; Yang, SteveX <stevex.y...@intel.com> > Subject: Re: [PATCH v2] common/sff_module: add telemetry command to > dump module EEPROM > > On Fri, Apr 08, 2022 at 10:23:30AM +0000, Robin Zhang wrote: > > This patch introduce a new telemetry command '/sff_module/info' > > to dump format module EEPROM information. > > > > The format support for SFP(Small Formfactor Pluggable)/SFP+ > > /QSFP+(Quad Small Formfactor Pluggable)/QSFP28 modules based on > > SFF(Small Form Factor) Committee specifications > > SFF-8079/SFF-8472/SFF-8024/SFF-8636. > > > > Signed-off-by: Robin Zhang <robinx.zh...@intel.com> > > --- > > > > v2: > > - Redesign the dump function as a telemetry command, so that the > EEPROM > > information can be used by other app. > > > > - The usage like this: > > > > Launch the primary application with telemetry: > > Take testpmd as example: ./app/dpdk-testpmd > > > > Then launch the telemetry client script: > > ./usertools/dpdk-telemetry.py > > > > In telemetry client run command: > > --> /sff_module/info,<port number> > > > > Both primary application and telemetry client will show the formated > > module EEPROM information. > > > > drivers/common/meson.build | 1 + > > drivers/common/sff_module/meson.build | 16 + > > drivers/common/sff_module/sff_8079.c | 672 ++++++++++++++ > > drivers/common/sff_module/sff_8472.c | 301 ++++++ > > drivers/common/sff_module/sff_8636.c | 1004 > +++++++++++++++++++++ > > drivers/common/sff_module/sff_8636.h | 592 ++++++++++++ > > drivers/common/sff_module/sff_common.c | 415 +++++++++ > > drivers/common/sff_module/sff_common.h | 192 ++++ > > drivers/common/sff_module/sff_telemetry.c | 142 +++ > > drivers/common/sff_module/sff_telemetry.h | 41 + > > drivers/common/sff_module/version.map | 9 + > > 11 files changed, 3385 insertions(+) > > create mode 100644 drivers/common/sff_module/meson.build > > create mode 100644 drivers/common/sff_module/sff_8079.c > > create mode 100644 drivers/common/sff_module/sff_8472.c > > create mode 100644 drivers/common/sff_module/sff_8636.c > > create mode 100644 drivers/common/sff_module/sff_8636.h > > create mode 100644 drivers/common/sff_module/sff_common.c > > create mode 100644 drivers/common/sff_module/sff_common.h > > create mode 100644 drivers/common/sff_module/sff_telemetry.c > > create mode 100644 drivers/common/sff_module/sff_telemetry.h > > create mode 100644 drivers/common/sff_module/version.map > > > Is this is whole new driver just to provide telemetry dumps of SFP > information? I can understand the problem somewhat - though I am in some > doubt that telemetry is the best way to expose this information - but > creating a new driver seems the wrong approach here. SFPs are for NIC > devices, so why isn't this available in a common API such as ethdev? >
I have considered add this function as a new telemetry command of ethdev (like '/ethdev/sff_module_info') to dump these SFP information. But I'm not sure if it's acceptable to add all these production code (sff_8xxx.c) into lib/ethdev? If it's OK, I can make V3 patches to change it as a telemetry command of ethdev. > /Bruce
