From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from nbfkord-smmo03.seg.att.com (nbfkord-smmo03.seg.att.com [209.65.160.84]) by dpdk.org (Postfix) with ESMTP id B525B108F for ; Mon, 16 Jan 2017 08:05:14 +0100 (CET) Received: from unknown [193.34.186.16] (EHLO webmail.solarflare.com) by nbfkord-smmo03.seg.att.com(mxl_mta-7.2.4-7) over TLS secured channel with ESMTP id 9a07c785.0.2739347.00-2257.5584331.nbfkord-smmo03.seg.att.com (envelope-from ); Mon, 16 Jan 2017 07:05:16 +0000 (UTC) X-MXL-Hash: 587c70ac2fbdd2cf-6b58632d48e46a956ee05c0817e8f575e6f4da54 Received: from [192.168.38.17] (84.52.89.52) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Mon, 16 Jan 2017 07:05:08 +0000 To: Qiming Yang , References: <1484202716-41669-2-git-send-email-qiming.yang@intel.com> <1484545498-33882-1-git-send-email-qiming.yang@intel.com> <1484545498-33882-2-git-send-email-qiming.yang@intel.com> CC: , From: Andrew Rybchenko Message-ID: Date: Mon, 16 Jan 2017 10:05:03 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <1484545498-33882-2-git-send-email-qiming.yang@intel.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [84.52.89.52] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-11.0.0.1191-8.000.1202-22826.003 X-TM-AS-Result: No--11.054700-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-AnalysisOut: [v=2.1 cv=HuVwbhnS c=1 sm=1 tr=0 a=8P+NB+fYZDP74ap4g4d9Kw==] X-AnalysisOut: [:17 a=RB3BGLmKESwA:10 a=N659UExz7-8A:10 a=IgFoBzBjUZAA:10 ] X-AnalysisOut: [a=QyXUC8HyAAAA:8 a=27oK_dOWqRncwZ88tkIA:9 a=pILNOxqGKmIA:1] X-AnalysisOut: [0 a=avl4LiGQNoF5OB0DmCJ7:22] X-Spam: [F=0.2000000000; CM=0.500; S=0.200(2015072901)] X-MAIL-FROM: X-SOURCE-IP: [193.34.186.16] Subject: Re: [dpdk-dev] [PATCH v9 1/5] ethdev: add firmware version get X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Jan 2017 07:05:16 -0000 On 01/16/2017 08:44 AM, Qiming Yang wrote: > This patch adds a new API 'rte_eth_dev_fw_version_get' for > fetching firmware version by a given device. > > Signed-off-by: Qiming Yang > Acked-by: Remy Horton > --- > doc/guides/nics/features/default.ini | 1 + > doc/guides/rel_notes/deprecation.rst | 4 ---- > doc/guides/rel_notes/release_17_02.rst | 5 +++++ > lib/librte_ether/rte_ethdev.c | 12 ++++++++++++ > lib/librte_ether/rte_ethdev.h | 26 ++++++++++++++++++++++++++ > lib/librte_ether/rte_ether_version.map | 1 + > 6 files changed, 45 insertions(+), 4 deletions(-) > > diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini > index f1bf9bf..ae40d57 100644 > --- a/doc/guides/nics/features/default.ini > +++ b/doc/guides/nics/features/default.ini > @@ -50,6 +50,7 @@ Timesync = > Basic stats = > Extended stats = > Stats per queue = > +FW version = > EEPROM dump = > Registers dump = > Multiprocess aware = > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst > index 054e2e7..755dc65 100644 > --- a/doc/guides/rel_notes/deprecation.rst > +++ b/doc/guides/rel_notes/deprecation.rst > @@ -30,10 +30,6 @@ Deprecation Notices > ``nb_seg_max`` and ``nb_mtu_seg_max`` providing information about number of > segments limit to be transmitted by device for TSO/non-TSO packets. > > -* In 17.02 ABI change is planned: the ``rte_eth_dev_info`` structure > - will be extended with a new member ``fw_version`` in order to store > - the NIC firmware version. > - > * ethdev: an API change is planned for 17.02 for the function > ``_rte_eth_dev_callback_process``. In 17.02 the function will return an ``int`` > instead of ``void`` and a fourth parameter ``void *ret_param`` will be added. > diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst > index 5762d3f..f9134bb 100644 > --- a/doc/guides/rel_notes/release_17_02.rst > +++ b/doc/guides/rel_notes/release_17_02.rst > @@ -66,6 +66,11 @@ New Features > Support for Mellanox ConnectX-5 family of 10/25/40/50/100 Gbps adapters > has been added to the existing mlx5 PMD. > > +* **Added firmware version get API.** > + > + Added a new function ``rte_eth_dev_fw_version_get()`` to fetch firmware > + version by a given device. > + > Resolved Issues > --------------- > > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > index 917557a..89cffcf 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -1588,6 +1588,18 @@ rte_eth_dev_set_rx_queue_stats_mapping(uint8_t port_id, uint16_t rx_queue_id, > STAT_QMAP_RX); > } > > +int > +rte_eth_dev_fw_version_get(uint8_t port_id, char *fw_version, size_t fw_size) > +{ > + struct rte_eth_dev *dev; > + > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > + dev = &rte_eth_devices[port_id]; > + > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fw_version_get, -ENOTSUP); > + return (*dev->dev_ops->fw_version_get)(dev, fw_version, fw_size); I think it would be good to handle difference from snprintf() behaviour here and specify that fw_version_get callback has exactly snprintf()-like return value. It would allow to avoid duplicated code in all drivers (adding 1 for terminating null, conversion of success value to 0). Also I think warning about insufficient space is not required. It could be intentional to call the first time with 0 (or some small) space to get required space to be (re)allocated. May be debug level message would be useful. > +} > + > void > rte_eth_dev_info_get(uint8_t port_id, struct rte_eth_dev_info *dev_info) > { > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h > index ded43d7..37a55ef 100644 > --- a/lib/librte_ether/rte_ethdev.h > +++ b/lib/librte_ether/rte_ethdev.h > @@ -1177,6 +1177,10 @@ typedef uint32_t (*eth_rx_queue_count_t)(struct rte_eth_dev *dev, > typedef int (*eth_rx_descriptor_done_t)(void *rxq, uint16_t offset); > /**< @internal Check DD bit of specific RX descriptor */ > > +typedef int (*eth_fw_version_get_t)(struct rte_eth_dev *dev, > + char *fw_version, size_t fw_size); > +/**< @internal Get firmware information of an Ethernet device. */ > + If we finally have different return value for rte_eth_dev_fw_version_get() and here, it would be useful to highlight it here. > typedef void (*eth_rxq_info_get_t)(struct rte_eth_dev *dev, > uint16_t rx_queue_id, struct rte_eth_rxq_info *qinfo); > > @@ -1459,6 +1463,7 @@ struct eth_dev_ops { > eth_dev_infos_get_t dev_infos_get; /**< Get device info. */ > eth_rxq_info_get_t rxq_info_get; /**< retrieve RX queue information. */ > eth_txq_info_get_t txq_info_get; /**< retrieve TX queue information. */ > + eth_fw_version_get_t fw_version_get; /**< Get firmware version. */ > eth_dev_supported_ptypes_get_t dev_supported_ptypes_get; > /**< Get packet types supported and identified by device. */ > > @@ -2396,6 +2401,27 @@ void rte_eth_macaddr_get(uint8_t port_id, struct ether_addr *mac_addr); > void rte_eth_dev_info_get(uint8_t port_id, struct rte_eth_dev_info *dev_info); > > /** > + * Retrieve the firmware version of a device. > + * > + * @param port_id > + * The port identifier of the device. > + * @param fw_version > + * A array pointer to store the firmware version of a device, > + * allocated by caller. > + * @param fw_size > + * The size of the array pointed by fw_version, which should be > + * large enough to store firmware version of the device. > + * @return > + * - (0) if successful. > + * - (-ENOTSUP) if operation is not supported. > + * - (-ENODEV) if *port_id* invalid. > + * - (>0) if *fw_size* is not enough to store firmware version, return > + * the size of the non truncated string. It is OK for me to keep 0 for success here and it is right in this case to include terminating null iin the return value if size is insufficient (to cover corner case with empty FW version). Please, highlight that terminating null is included here. It is the difference from snprintf() and it should be 100% clear. > + */ > +int rte_eth_dev_fw_version_get(uint8_t port_id, > + char *fw_version, size_t fw_size); > + > +/** > * Retrieve the supported packet types of an Ethernet device. > * > * When a packet type is announced as supported, it *must* be recognized by > diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map > index 0c2859e..c6c9d0d 100644 > --- a/lib/librte_ether/rte_ether_version.map > +++ b/lib/librte_ether/rte_ether_version.map > @@ -146,6 +146,7 @@ DPDK_17.02 { > global: > > _rte_eth_dev_reset; > + rte_eth_dev_fw_version_get; > rte_flow_create; > rte_flow_destroy; > rte_flow_flush;