Re: [RFC v2] ethdev: introduce ethdev dump API

[Ajit Khaparde <ajit.khaparde@broadcom.com>]

On Fri, Jan 14, 2022 at 4:25 PM Min Hu (Connor) <humin29@huawei.com> wrote:
>
> Hi, Ajit,
>
> 在 2022/1/15 1:56, Ajit Khaparde 写道:
> > On Wed, Jan 12, 2022 at 3:15 AM Min Hu (Connor) <humin29@huawei.com> wrote:
> >>
> >> Thanks Morten, fixed in v3.
> >>
> >> 在 2022/1/12 15:20, Morten Brørup 写道:
> >>>> From: Min Hu (Connor) [mailto:humin29@huawei.com]
> >>>> Sent: Wednesday, 12 January 2022 03.40
> >>>>
> >>>> Added the ethdev dump API which provides functions for query private
> >>>> info
> >>>> from device. There exists many private properties in different PMD
> >>>> drivers,
> >>>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
> >>>> information of
> >>>> these properties is important for debug. As the information is private,
> >>>> the new API is introduced.
> > Do you have any changes to testpmd to use this API?
> >
> No changes to testpmd.
> BTW, I will add some options for proc-info to use this API,
> and this is my next-step plan.
Ok. proc-info is a good place for this.

>
> >>>>
> >>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> >>>> ---
> >>>> v2:
> >>>> * fix dump API name
> >>>> * adjust description in doc.
> >>>> ---
> >>>>    doc/guides/rel_notes/release_22_03.rst |  7 +++++++
> >>>>    lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
> >>>>    lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
> >>>>    lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
> >>>>    4 files changed, 55 insertions(+)
> >>>>
> >>>> diff --git a/doc/guides/rel_notes/release_22_03.rst
> >>>> b/doc/guides/rel_notes/release_22_03.rst
> >>>> index 6d99d1eaa9..4f97df942d 100644
> >>>> --- a/doc/guides/rel_notes/release_22_03.rst
> >>>> +++ b/doc/guides/rel_notes/release_22_03.rst
> >>>> @@ -55,6 +55,13 @@ New Features
> >>>>         Also, make sure to start the actual text at the margin.
> >>>>         =======================================================
> >>>>
> >>>> +* **Added the private ethdev dump API, for query private info of
> >>>> ethdev.**
> >>>> +
> >>>> +  Added the private ethdev dump API which provides functions for query
> >>>> +  private info from device. There exists many private properties in
> >>>> +  different PMD drivers. The information of these properties is
> >>>> important
> >>>> +  for debug. As the information is private, the new API is introduced.
> >>>> +
> >>>>
> >>>>    Removed Items
> >>>>    -------------
> >>>> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> >>>> index d95605a355..e75ff3f15b 100644
> >>>> --- a/lib/ethdev/ethdev_driver.h
> >>>> +++ b/lib/ethdev/ethdev_driver.h
> >>>> @@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct
> >>>> rte_eth_dev *dev,
> >>>>    typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
> >>>>                                      uint64_t *features);
> >>>>
> >>>> +/**
> >>>> + * @internal
> >>>> + * Get ethdev private info.
> >>>
> >>> Suggestion: Dump ethdev private info to a file.
> >>>
> >>>> + *
> >>>> + * @param file
> >>>> + *   A pointer to a file for output.
> >>>> + * @param dev
> >>>> + *   Port (ethdev) handle.
> >>>> + *
> >>>> + * @return
> >>>> + *   Negative errno value on error, positive value on success.
> >>>> + */
> >>>> +typedef int (*eth_dev_priv_dump_t)(FILE *file, struct rte_eth_dev
> >>>> *dev);
> >>>> +
> >>>>    /**
> >>>>     * @internal A structure containing the functions exported by an
> >>>> Ethernet driver.
> >>>>     */
> >>>> @@ -1186,6 +1200,9 @@ struct eth_dev_ops {
> >>>>        * kinds of metadata to the PMD
> >>>>        */
> >>>>       eth_rx_metadata_negotiate_t rx_metadata_negotiate;
> >>>> +
> >>>> +    /** Dump ethdev private info */
> >>>> +    eth_dev_priv_dump_t eth_dev_priv_dump;
> >>>>    };
> >>>>
> >>>>    /**
> >>>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> >>>> index a1d475a292..9fc6d91d76 100644
> >>>> --- a/lib/ethdev/rte_ethdev.c
> >>>> +++ b/lib/ethdev/rte_ethdev.c
> >>>> @@ -6472,6 +6472,21 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id,
> >>>> uint64_t *features)
> >>>>                      (*dev->dev_ops->rx_metadata_negotiate)(dev,
> >>>> features));
> >>>>    }
> >>>>
> >>>> +int
> >>>> +rte_eth_dev_priv_dump(FILE *file, uint16_t port_id)
> >>>> +{
> >>>> +    struct rte_eth_dev *dev;
> >>>> +    int ret;
> >>>> +
> >>>> +    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->eth_dev_priv_dump, -
> >>>> ENOTSUP);
> >>>> +    ret = (*dev->dev_ops->eth_dev_priv_dump)(file, dev);
> >>>> +
> >>>> +    return ret;
> >>>> +}
> >>>> +
> >>>>    RTE_LOG_REGISTER_DEFAULT(rte_eth_dev_logtype, INFO);
> >>>>
> >>>>    RTE_INIT(ethdev_init_telemetry)
> >>>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> >>>> index fa299c8ad7..8e33e6927f 100644
> >>>> --- a/lib/ethdev/rte_ethdev.h
> >>>> +++ b/lib/ethdev/rte_ethdev.h
> >>>> @@ -5888,6 +5888,22 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t
> >>>> queue_id,
> >>>>       return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
> >>>>    }
> >>>>
> >>>> +/**
> >>>> + * @warning
> >>>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> >>>> notice
> >>>> + *
> >>>> + * Get ethdev private info.
> >>>
> >>> Suggestion: Dump ethdev private info to a file.
> >>>
> >>>> + *
> >>>> + * @param file
> >>>> + *   A pointer to a file for output.
> >>>> + * @param port_id
> >>>> + *   The port identifier of the Ethernet device.
> >>>> + * @return
> >>>> + *   Negative errno value on error, positive value on success.
> >>>> + */
> >>>> +__rte_experimental
> >>>> +int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
> >>>> +
> >>>>    #ifdef __cplusplus
> >>>>    }
> >>>>    #endif
> >>>> --
> >>>> 2.33.0
> >>>>
> >>>
> >>> You should probably also add rte_eth_dev_priv_dump to the EXPERIMENTAL section of the version.map file.
> >>>
> >>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> >>>
> >>> .
> >>>
> > .
> >