Re: [dpdk-dev] [PATCH v6 1/2] ethdev: add an API to get device configuration info

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

[-- Attachment #1: Type: text/plain, Size: 4427 bytes --]

On Wed, Aug 25, 2021 at 1:08 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 8/24/2021 7:19 PM, Jie Wang wrote:
> > This patch adds a new API "rte_eth_dev_conf_info_get()" to help testpmd get
> > device configuration info.
> >
> > Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> > ---
> >  lib/ethdev/rte_ethdev.c | 27 +++++++++++++++++++++++++++
> >  lib/ethdev/rte_ethdev.h | 26 ++++++++++++++++++++++++++
> >  lib/ethdev/version.map  |  3 +++
> >  3 files changed, 56 insertions(+)
> >
> > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> > index 9d95cd11e1..74184099a1 100644
> > --- a/lib/ethdev/rte_ethdev.c
> > +++ b/lib/ethdev/rte_ethdev.c
> > @@ -3458,6 +3458,33 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
> >       return 0;
> >  }
> >
> > +int
> > +rte_eth_dev_conf_info_get(uint16_t port_id,
> > +                             struct rte_eth_dev_conf_info *dev_conf_info)
> > +{
> > +     struct rte_eth_dev *dev;
> > +
> > +     RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> > +     dev = &rte_eth_devices[port_id];
> > +
> > +     if (dev_conf_info == NULL) {
> > +             RTE_ETHDEV_LOG(ERR, "Cannot get ethdev port %u config info to NULL\n",
> > +                     port_id);
> > +             return -EINVAL;
> > +     }
> > +
> > +     /*
> > +      * Init dev_conf_info before port_id check since caller does not have
> > +      * return status and does not know if get is successful or not.
> > +      */
> > +     memset(dev_conf_info, 0, sizeof(struct rte_eth_dev_conf_info));
> > +
> > +     dev_conf_info->rx_offloads = dev->data->dev_conf.rxmode.offloads;
> > +     dev_conf_info->tx_offloads = dev->data->dev_conf.txmode.offloads;
> > +
> > +     return 0;
> > +}
> > +
> >  int
> >  rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
> >                                uint32_t *ptypes, int num)
> > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> > index d2b27c351f..70a2db550f 100644
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > @@ -1587,6 +1587,15 @@ struct rte_eth_dev_info {
> >       void *reserved_ptrs[2];   /**< Reserved for future fields */
> >  };
> >
> > +/**
> > + * Ethernet device configuration information structure.
> > + * Used to retrieve information about configured device.
> > + */
> > +struct rte_eth_dev_conf_info {
> > +     uint64_t rx_offloads; /**rxmode offloads */
> > +     uint64_t tx_offloads; /**txmode offloads */
> > +};
>
> My concern is if we need to extend this struct later, when application wants to
> get more current config from the dpdk layer, it will cause ABI break and will
> need to wait next LTS.
>
> And as this struct grow, it will be kind of duplication of the 'struct
> rte_eth_conf'.
>
> What do you think to reuse 'struct rte_eth_conf' in this API, to cover future needs?
+1
rte_eth_conf gives all the information needed and more for future enhancements!

>
> > +
> >  /**
> >   * RX/TX queue states
> >   */
> > @@ -3058,6 +3067,23 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
> >   */
> >  int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
> >
> > +/**
> > + * Retrieve the contextual information of an Ethernet device.
> > + *
> > + * @param port_id
> > + *   The port identifier of the Ethernet device.
> > + * @param dev_conf_info
> > + *   A pointer to a structure of type *rte_eth_dev_conf_info* to be filled with
> > + *   the contextual information of the Ethernet device.
> > + * @return
> > + *   - (0) if successful.
> > + *   - (-ENOTSUP) if support for dev_infos_get() does not exist for the device.
> > + *   - (-ENODEV) if *port_id* invalid.
> > + *   - (-EINVAL) if bad parameter.
> > + */
> > +int rte_eth_dev_conf_info_get(uint16_t port_id,
> > +                             struct rte_eth_dev_conf_info *dev_conf_info);
> > +
> >  /**
> >   * Retrieve the firmware version of a device.
> >   *
> > diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> > index 44d30b05ae..40539f99f9 100644
> > --- a/lib/ethdev/version.map
> > +++ b/lib/ethdev/version.map
> > @@ -249,6 +249,9 @@ EXPERIMENTAL {
> >       rte_mtr_meter_policy_delete;
> >       rte_mtr_meter_policy_update;
> >       rte_mtr_meter_policy_validate;
> > +
> > +     # added in 21.11
> > +     rte_eth_dev_conf_info_get;
> >  };
> >
> >  INTERNAL {
> >
>