[PATCH v6 1/2] net/ixgbe: Limit SDP3 check of TX_DISABLE to appropriate devices

[Jeff Daly]

> -----Original Message-----
> From: Wang, Haiyue <haiyue.wang at intel.com>
> Sent: Tuesday, April 12, 2022 9:22 PM
> To: Jeff Daly <jeffd at silicom-usa.com>; dev at dpdk.org
> Cc: stable at dpdk.org; Xiaolong Ye <xiaolong.ye at intel.com>; Xiao Zhang
> <xiao.zhang at intel.com>; Zhao1, Wei <wei.zhao1 at intel.com>; Lunyuan Cui
> <lunyuanx.cui at intel.com>
> Subject: RE: [PATCH v6 1/2] net/ixgbe: Limit SDP3 check of TX_DISABLE to
> appropriate devices
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments.
> 
> 
> > -----Original Message-----
> > From: Jeff Daly <jeffd at silicom-usa.com>
> > Sent: Wednesday, April 13, 2022 01:42
> > To: dev at dpdk.org
> > Cc: stable at dpdk.org; Wang, Haiyue <haiyue.wang at intel.com>; Xiaolong Ye
> > <xiaolong.ye at intel.com>; Xiao Zhang <xiao.zhang at intel.com>; Zhao1, Wei
> > <wei.zhao1 at intel.com>; Lunyuan Cui <lunyuanx.cui at intel.com>
> > Subject: [PATCH v6 1/2] net/ixgbe: Limit SDP3 check of TX_DISABLE to
> > appropriate devices
> >
> > 1ca05831b9b added a check that SDP3 (used as a TX_DISABLE output to
> > the SFP cage on these cards) is not asserted to avoid incorrectly
> > reporting link up when the SFP's laser is turned off.
> >
> > ff8162cb957 limited this workaround to fiber ports
> >
> > This patch:
> > * Adds devarg 'fiber_sdp3_no_tx_disable' not all fiber ixgbe devs use
> >   SDP3 as TX_DISABLE
> >
> > Fixes: 1ca05831b9b ("net/ixgbe: fix link status")
> > Fixes: ff8162cb957 ("net/ixgbe: fix link status")
> > Cc: stable at dpdk.org
> 
> This is new for soc for BIG change, not cc to stable.
> 
> >
> > Signed-off-by: Jeff Daly <jeffd at silicom-usa.com>
> > ---
> >  drivers/net/ixgbe/ixgbe_ethdev.c | 39
> > +++++++++++++++++++++++++++++++-  drivers/net/ixgbe/ixgbe_ethdev.h |
> > 3 +++
> >  2 files changed, 41 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index 2da3f67bbc..f31bbb7895 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -128,6 +128,13 @@
> >  #define IXGBE_EXVET_VET_EXT_SHIFT              16
> >  #define IXGBE_DMATXCTL_VT_MASK                 0xFFFF0000
> >
> > +#define IXGBE_DEVARG_FIBER_SDP3_NOT_TX_DISABLE
> "fiber_sdp3_no_tx_disable"
> > +
> 
> 'platform' may be a good arg for the soc related change.
> 
> dpdk-testpmd -a af:10.0,platform=xxx - -i
> 
> enum ixgbe_platform_type {
>         ixgbe_platform_unknown = 0,
>         ixgbe_platform_soc_atom, ??? You can specify it.
> 
> 
> 
> enum ixgbe_media_type ixgbe_get_platform_type(xxx) {
>         return xxx;
> }
> 

This patchset is not explicitly for SoC platform support.  *Any* implementation may or may not use
SDP3 as TX_DISABLE.   The previous version of the patch added a check for the mac being an 82599 that
uses fiber SFP rather than just a fiber SFP.  Our platform specifically can be fiber SFP, but TX_DISABLE 
is not SDP3.  However, our platform may not be the only implementation that doesn't use SDP3 this way.
It does seem that *most* implementations out there do use SDP3 this way, so our platform would be
the setting this option to 1 to skip this check while any others would work the same as before.

> 
> > +static const char * const ixgbe_valid_arguments[] = {
> > +     IXGBE_DEVARG_FIBER_SDP3_NOT_TX_DISABLE,
> > +     NULL
> > +};
> > +
> >  #define IXGBEVF_DEVARG_PFLINK_FULLCHK                "pflink_fullchk"
> >
> >  static const char * const ixgbevf_valid_arguments[] = { @@ -348,6
> > +355,8 @@ static int ixgbe_dev_udp_tunnel_port_del(struct rte_eth_dev
> > *dev,  static int ixgbe_filter_restore(struct rte_eth_dev *dev);
> > static void ixgbe_l2_tunnel_conf(struct rte_eth_dev *dev);  static int
> > ixgbe_wait_for_link_up(struct ixgbe_hw *hw);
> > +static int devarg_handle_int(__rte_unused const char *key, const char
> *value,
> > +                          void *extra_args);
> >
> >  /*
> >   * Define VF Stats MACRO for Non "cleared on read" register @@
> > -1032,6 +1041,29 @@ ixgbe_swfw_lock_reset(struct ixgbe_hw *hw)
> >       ixgbe_release_swfw_semaphore(hw, mask);  }
> >
> > +static void
> > +ixgbe_parse_devargs(struct ixgbe_adapter *adapter,
> > +                   struct rte_devargs *devargs) {
> > +     struct rte_kvargs *kvlist;
> > +     uint16_t sdp3_no_tx_disable;
> > +
> > +     if (devargs == NULL)
> > +             return;
> > +
> > +     kvlist = rte_kvargs_parse(devargs->args, ixgbe_valid_arguments);
> > +     if (kvlist == NULL)
> > +             return;
> > +
> > +     if (rte_kvargs_count(kvlist,
> IXGBE_DEVARG_FIBER_SDP3_NOT_TX_DISABLE) == 1 &&
> > +         rte_kvargs_process(kvlist,
> IXGBE_DEVARG_FIBER_SDP3_NOT_TX_DISABLE,
> > +                            devarg_handle_int, &sdp3_no_tx_disable) == 0 &&
> > +         sdp3_no_tx_disable == 1)
> > +             adapter->sdp3_no_tx_disable = 1;
> > +
> > +     rte_kvargs_free(kvlist);
> > +}
> > +
> >  /*
> >   * This function is based on code in ixgbe_attach() in base/ixgbe.c.
> >   * It returns 0 on success.
> > @@ -1095,6 +1127,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev,
> void *init_params __rte_unused)
> >       }
> >
> >       rte_atomic32_clear(&ad->link_thread_running);
> > +     ixgbe_parse_devargs(eth_dev->data->dev_private,
> > +                         pci_dev->device.devargs);
> >       rte_eth_copy_pci_info(eth_dev, pci_dev);
> >       eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
> >
> > @@ -4261,7 +4295,8 @@ ixgbe_dev_link_update_share(struct rte_eth_dev
> *dev,
> >               return rte_eth_linkstatus_set(dev, &link);
> >       }
> >
> > -     if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber) {
> > +     if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber &&
> > +         !ad->sdp3_no_tx_disable) {
> >               esdp_reg = IXGBE_READ_REG(hw, IXGBE_ESDP);
> >               if ((esdp_reg & IXGBE_ESDP_SDP3))
> >                       link_up = 0;
> > @@ -8250,6 +8285,8 @@ ixgbe_dev_macsec_register_disable(struct
> > rte_eth_dev *dev)  RTE_PMD_REGISTER_PCI(net_ixgbe, rte_ixgbe_pmd);
> > RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe, pci_id_ixgbe_map);
> > RTE_PMD_REGISTER_KMOD_DEP(net_ixgbe, "* igb_uio | uio_pci_generic |
> > vfio-pci");
> > +RTE_PMD_REGISTER_PARAM_STRING(net_ixgbe,
> > +                           IXGBE_DEVARG_FIBER_SDP3_NOT_TX_DISABLE
> > +"=<0|1>");
> >  RTE_PMD_REGISTER_PCI(net_ixgbe_vf, rte_ixgbevf_pmd);
> > RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe_vf, pci_id_ixgbevf_map);
> > RTE_PMD_REGISTER_KMOD_DEP(net_ixgbe_vf, "* igb_uio | vfio-pci"); diff
> > --git a/drivers/net/ixgbe/ixgbe_ethdev.h
> > b/drivers/net/ixgbe/ixgbe_ethdev.h
> > index 69e0e82a5b..cc6049a66a 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> > @@ -501,6 +501,9 @@ struct ixgbe_adapter {
> >       /* For RSS reta table update */
> >       uint8_t rss_reta_updated;
> >
> > +     /* Used for limiting SDP3 TX_DISABLE checks */
> > +     uint8_t sdp3_no_tx_disable;
> > +
> >       /* Used for VF link sync with PF's physical and logical (by checking
> >        * mailbox status) link status.
> >        */
> > --
> > 2.25.1