[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <MW4PR11MB57769C0E492F1252D58DEFB1FD799@MW4PR11MB5776.namprd11.prod.outlook.com>
Date: Tue, 16 May 2023 10:36:24 +0000
From: "Drewek, Wojciech" <wojciech.drewek@...el.com>
To: Íñigo Huguet <ihuguet@...hat.com>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Ertman, David M"
<david.m.ertman@...el.com>, mschmidt <mschmidt@...hat.com>
Subject: RE: [PATCH iwl-next] ice: Remove LAG+SRIOV mutual exclusion
> -----Original Message-----
> From: Íñigo Huguet <ihuguet@...hat.com>
> Sent: wtorek, 16 maja 2023 11:18
> To: Drewek, Wojciech <wojciech.drewek@...el.com>
> Cc: intel-wired-lan@...ts.osuosl.org; netdev@...r.kernel.org; Ertman, David M <david.m.ertman@...el.com>; mschmidt
> <mschmidt@...hat.com>
> Subject: Re: [PATCH iwl-next] ice: Remove LAG+SRIOV mutual exclusion
>
> On Fri, May 12, 2023 at 11:08 AM Wojciech Drewek
> <wojciech.drewek@...el.com> wrote:
> >
> > From: Dave Ertman <david.m.ertman@...el.com>
> >
> > There was a change previously to stop SR-IOV and LAG from existing on the
> > same interface. This was to prevent the violation of LACP (Link
> > Aggregation Control Protocol). The method to achieve this was to add a
> > no-op Rx handler onto the netdev when SR-IOV VFs were present, thus
> > blocking bonding, bridging, etc from claiming the interface by adding
> > its own Rx handler. Also, when an interface was added into a aggregate,
> > then the SR-IOV capability was set to false.
> >
> > There are some users that have in house solutions using both SR-IOV and
> > bridging/bonding that this method interferes with (e.g. creating duplicate
> > VFs on the bonded interfaces and failing between them when the interface
> > fails over).
> >
> > It makes more sense to provide the most functionality
> > possible, the restriction on co-existence of these features will be
> > removed. No additional functionality is currently being provided beyond
> > what existed before the co-existence restriction was put into place. It is
> > up to the end user to not implement a solution that would interfere with
> > existing network protocols.
> >
> > Reviewed-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> > Signed-off-by: Dave Ertman <david.m.ertman@...el.com>
> > Signed-off-by: Wojciech Drewek <wojciech.drewek@...el.com>
> > ---
> > .../device_drivers/ethernet/intel/ice.rst | 18 -------
> > drivers/net/ethernet/intel/ice/ice.h | 19 -------
> > drivers/net/ethernet/intel/ice/ice_lag.c | 12 -----
> > drivers/net/ethernet/intel/ice/ice_lag.h | 53 -------------------
> > drivers/net/ethernet/intel/ice/ice_lib.c | 2 -
> > drivers/net/ethernet/intel/ice/ice_sriov.c | 4 --
> > 6 files changed, 108 deletions(-)
> >
<...>
> > diff --git a/drivers/net/ethernet/intel/ice/ice_lag.h b/drivers/net/ethernet/intel/ice/ice_lag.h
> > index 51b5cf467ce2..54d6663fe586 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_lag.h
> > +++ b/drivers/net/ethernet/intel/ice/ice_lag.h
> > @@ -26,62 +26,9 @@ struct ice_lag {
> > u8 bonded:1; /* currently bonded */
> > u8 primary:1; /* this is primary */
> > u8 handler:1; /* did we register a rx_netdev_handler */
>
> "handler" field seems unused now, shouldn't it be removed?
You're right, I'll send v2
>
> > - /* each thing blocking bonding will increment this value by one.
> > - * If this value is zero, then bonding is allowed.
> > - */
> > - u16 dis_lag;
> > u8 role;
> > };
> >
> > int ice_init_lag(struct ice_pf *pf);
> > void ice_deinit_lag(struct ice_pf *pf);
> > -rx_handler_result_t ice_lag_nop_handler(struct sk_buff **pskb);
> > -
> > -/**
> > - * ice_disable_lag - increment LAG disable count
> > - * @lag: LAG struct
> > - */
> > -static inline void ice_disable_lag(struct ice_lag *lag)
> > -{
> > - /* If LAG this PF is not already disabled, disable it */
> > - rtnl_lock();
> > - if (!netdev_is_rx_handler_busy(lag->netdev)) {
> > - if (!netdev_rx_handler_register(lag->netdev,
> > - ice_lag_nop_handler,
> > - NULL))
> > - lag->handler = true;
> > - }
> > - rtnl_unlock();
> > - lag->dis_lag++;
> > -}
> > -
> > -/**
> > - * ice_enable_lag - decrement disable count for a PF
> > - * @lag: LAG struct
> > - *
> > - * Decrement the disable counter for a port, and if that count reaches
> > - * zero, then remove the no-op Rx handler from that netdev
> > - */
> > -static inline void ice_enable_lag(struct ice_lag *lag)
> > -{
> > - if (lag->dis_lag)
> > - lag->dis_lag--;
> > - if (!lag->dis_lag && lag->handler) {
> > - rtnl_lock();
> > - netdev_rx_handler_unregister(lag->netdev);
> > - rtnl_unlock();
> > - lag->handler = false;
> > - }
> > -}
> > -
> > -/**
> > - * ice_is_lag_dis - is LAG disabled
> > - * @lag: LAG struct
> > - *
> > - * Return true if bonding is disabled
> > - */
> > -static inline bool ice_is_lag_dis(struct ice_lag *lag)
> > -{
> > - return !!(lag->dis_lag);
> > -}
> > #endif /* _ICE_LAG_H_ */
> > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> > index d9731476cd7f..5ddb95d1073a 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> > @@ -2712,8 +2712,6 @@ ice_vsi_setup(struct ice_pf *pf, struct ice_vsi_cfg_params *params)
> > return vsi;
> >
> > err_vsi_cfg:
> > - if (params->type == ICE_VSI_VF)
> > - ice_enable_lag(pf->lag);
> > ice_vsi_free(vsi);
> >
> > return NULL;
> > diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
> > index 9788f363e9dc..a222cd702fd5 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> > @@ -960,8 +960,6 @@ int ice_sriov_configure(struct pci_dev *pdev, int num_vfs)
> > if (!num_vfs) {
> > if (!pci_vfs_assigned(pdev)) {
> > ice_free_vfs(pf);
> > - if (pf->lag)
> > - ice_enable_lag(pf->lag);
> > return 0;
> > }
> >
> > @@ -973,8 +971,6 @@ int ice_sriov_configure(struct pci_dev *pdev, int num_vfs)
> > if (err)
> > return err;
> >
> > - if (pf->lag)
> > - ice_disable_lag(pf->lag);
> > return num_vfs;
> > }
> >
> > --
> > 2.39.2
> >
> >
>
>
> --
> Íñigo Huguet
Powered by blists - more mailing lists