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

[Luca Boccassi]

On Wed, 2020-08-12 at 21:29 -0700, Ajit Khaparde wrote:
> Luca,
> 
> On Wed, Aug 12, 2020 at 1:38 AM Luca Boccassi <bluca at debian.org> wrote:
> 
> > On Wed, 2020-08-12 at 09:22 +0530, Kalesh A P wrote:
> > > From: Kalesh AP <kalesh-anakkur.purayil at 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 at broadcom.com>
> > > Reviewed-by: Ajit Khaparde <ajit.khaparde at 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

Hi,

I do not think we impose a "release date" based upgrade path, so I
think it's something that needs to be considered.

Now of course any missing bug fix would cause a regression - the
question is, would this be a bug in a behaviour in some corner case, or
is it a public interface/API/configuration that breaks?

-- 
Kind regards,
Luca Boccassi