Re: [dpdk-stable] [PATCH 19.11 1/2] net/bnxt: support speed capabilities query

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

Luca,

On Wed, Aug 12, 2020 at 1:38 AM Luca Boccassi <bluca@debian.org> wrote:

> On Wed, 2020-08-12 at 09:22 +0530, Kalesh A P wrote:
> > From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> >
> > [ upstream commit 85cf7f2bba0f665fe91710cd1cf0ca852f73890a ]
> >
> > Added information about supported speeds for the port in the
> > "dev_infos_get". As other PMDs are returning the speed capabilities,
> > apps may expect this behavior from bnxt PMD.
> >
> > Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > ---
> >  drivers/net/bnxt/bnxt_ethdev.c | 36 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> >
> > diff --git a/drivers/net/bnxt/bnxt_ethdev.c
> b/drivers/net/bnxt/bnxt_ethdev.c
> > index fe240b6..90e1a33 100644
> > --- a/drivers/net/bnxt/bnxt_ethdev.c
> > +++ b/drivers/net/bnxt/bnxt_ethdev.c
> > @@ -490,6 +490,40 @@ static int bnxt_shutdown_nic(struct bnxt *bp)
> >   * Device configuration and status function
> >   */
> >
> > +static uint32_t bnxt_get_speed_capabilities(struct bnxt *bp)
> > +{
> > +     uint32_t link_speed = bp->link_info.support_speeds;
> > +     uint32_t speed_capa = 0;
> > +
> > +     if (link_speed & HWRM_PORT_PHY_QCFG_OUTPUT_LINK_SPEED_100MB)
> > +             speed_capa |= ETH_LINK_SPEED_100M;
> > +     if (link_speed & HWRM_PORT_PHY_QCFG_OUTPUT_SUPPORT_SPEEDS_100MBHD)
> > +             speed_capa |= ETH_LINK_SPEED_100M_HD;
> > +     if (link_speed & HWRM_PORT_PHY_QCFG_OUTPUT_SUPPORT_SPEEDS_1GB)
> > +             speed_capa |= ETH_LINK_SPEED_1G;
> > +     if (link_speed & HWRM_PORT_PHY_QCFG_OUTPUT_SUPPORT_SPEEDS_2_5GB)
> > +             speed_capa |= ETH_LINK_SPEED_2_5G;
> > +     if (link_speed & HWRM_PORT_PHY_QCFG_OUTPUT_SUPPORT_SPEEDS_10GB)
> > +             speed_capa |= ETH_LINK_SPEED_10G;
> > +     if (link_speed & HWRM_PORT_PHY_QCFG_OUTPUT_SUPPORT_SPEEDS_20GB)
> > +             speed_capa |= ETH_LINK_SPEED_20G;
> > +     if (link_speed & HWRM_PORT_PHY_QCFG_OUTPUT_SUPPORT_SPEEDS_25GB)
> > +             speed_capa |= ETH_LINK_SPEED_25G;
> > +     if (link_speed & HWRM_PORT_PHY_QCFG_OUTPUT_SUPPORT_SPEEDS_40GB)
> > +             speed_capa |= ETH_LINK_SPEED_40G;
> > +     if (link_speed & HWRM_PORT_PHY_QCFG_OUTPUT_SUPPORT_SPEEDS_50GB)
> > +             speed_capa |= ETH_LINK_SPEED_50G;
> > +     if (link_speed & HWRM_PORT_PHY_QCFG_OUTPUT_SUPPORT_SPEEDS_100GB)
> > +             speed_capa |= ETH_LINK_SPEED_100G;
> > +
> > +     if (bp->link_info.auto_mode ==
> HWRM_PORT_PHY_QCFG_OUTPUT_AUTO_MODE_NONE)
> > +             speed_capa |= ETH_LINK_SPEED_FIXED;
> > +     else
> > +             speed_capa |= ETH_LINK_SPEED_AUTONEG;
> > +
> > +     return speed_capa;
> > +}
> > +
> >  static int bnxt_dev_info_get_op(struct rte_eth_dev *eth_dev,
> >                               struct rte_eth_dev_info *dev_info)
> >  {
> > @@ -533,6 +567,8 @@ static int bnxt_dev_info_get_op(struct rte_eth_dev
> *eth_dev,
> >       dev_info->tx_offload_capa = BNXT_DEV_TX_OFFLOAD_SUPPORT;
> >       dev_info->flow_type_rss_offloads = BNXT_ETH_RSS_SUPPORT;
> >
> > +     dev_info->speed_capa = bnxt_get_speed_capabilities(bp);
> > +
> >       /* *INDENT-OFF* */
> >       dev_info->default_rxconf = (struct rte_eth_rxconf) {
> >               .rx_thresh = {
>
> Hi,
>
> This smells like a feature? 20.02 does not have it, so what happens if
> a user expects this to work and upgrades from 19.11.x to 20.02?
>
Well in this example, the user will see a regression.
A dumb question.. Does it ever happen that a user upgrades from an LTS
like 19.11.x to 20.02?

Thanks
Ajit

>
> --
> Kind regards,
> Luca Boccassi
>