From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5B0B5A0501; Fri, 8 Apr 2022 13:00:44 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 346A24067E; Fri, 8 Apr 2022 13:00:44 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mails.dpdk.org (Postfix) with ESMTP id 6945B4003F for ; Fri, 8 Apr 2022 13:00:42 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1649415642; x=1680951642; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=Mg1rCir7HKMFTtrLDbP9Scnb7tM1IoBkjXOfcCiNUO4=; b=HHrsloGOt0yLQYgT8F+Jkpi+ALBDS37RxvbtgGeDfoCokDfyx9x7/NSB EbsSi5h/HCJvU9qtR760paXJtPlwtgLktosFa78vteWeJHKvoTmt2qf0I SQ3BwJcKRS65C0jX6fQS3gH/0bx4tYbKXTg+I5a2IspJBBZxKXpSs4oku Q5mUk4Wb+lp5L8Tr+QAjX01JSDU0AkZZzOc4G86dFyor3oFIxbAuO56xy bdPfgRTG27mlpGtTC6YnnFQeOM2XTvjI4I8ay6d0ADZ86flKcA11TJR8a IJBlXtdnTbL+hyvm971AnuhyOKzaeLeSGcM6RjHeTm6JPmjc3oeUN/a0Q w==; X-IronPort-AV: E=McAfee;i="6400,9594,10310"; a="324729120" X-IronPort-AV: E=Sophos;i="5.90,244,1643702400"; d="scan'208";a="324729120" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Apr 2022 04:00:41 -0700 X-IronPort-AV: E=Sophos;i="5.90,244,1643702400"; d="scan'208";a="524739798" Received: from bricha3-mobl.ger.corp.intel.com ([10.213.224.53]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 08 Apr 2022 04:00:40 -0700 Date: Fri, 8 Apr 2022 12:00:37 +0100 From: Bruce Richardson To: "Zhang, RobinX" Cc: "dev@dpdk.org" , "Yang, Qiming" , "Zhang, Qi Z" , "Yang, SteveX" Subject: Re: [PATCH v2] common/sff_module: add telemetry command to dump module EEPROM Message-ID: References: <20220215101853.919735-1-robinx.zhang@intel.com> <20220408102330.774749-1-robinx.zhang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Fri, Apr 08, 2022 at 11:55:07AM +0100, Zhang, RobinX wrote: > Hi Bruce, > > > -----Original Message----- > > From: Richardson, Bruce > > Sent: Friday, April 8, 2022 6:33 PM > > To: Zhang, RobinX > > Cc: dev@dpdk.org; Yang, Qiming ; Zhang, Qi Z > > ; Yang, SteveX > > 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 > > > --- > > > > > > 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, > > > > > > 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. > Hi, I think some discussion is needed before you go preparing a new version of this patchset. Some initial questions: 1. Does SFF code apply only to Intel products/NICs or is it multi-vendor? 2. For the driver approach you previously took, how was the presence of hardware detected to load the driver? 3. Does this work on SFPs need to interact with the NIC drivers in any way? Thanks, /Bruce