diff mbox series

[iwl-next] ice: Remove LAG+SRIOV mutual exclusion

Message ID 20230512090652.190058-1-wojciech.drewek@intel.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [iwl-next] ice: Remove LAG+SRIOV mutual exclusion | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Wojciech Drewek May 12, 2023, 9:06 a.m. UTC
From: Dave Ertman <david.m.ertman@intel.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@linux.intel.com>
Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.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(-)

Comments

Michal Schmidt May 13, 2023, 11:18 a.m. UTC | #1
On Fri, May 12, 2023 at 11:08 AM Wojciech Drewek
<wojciech.drewek@intel.com> wrote:
>
> From: Dave Ertman <david.m.ertman@intel.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@linux.intel.com>
> Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.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(-)

I see that both Alexander Lobakin's and my feedback about the previous
version has been accomodated.
And thanks for updating the Documentation too.

Reviewed-by: Michal Schmidt <mschmidt@redhat.com>

> diff --git a/Documentation/networking/device_drivers/ethernet/intel/ice.rst b/Documentation/networking/device_drivers/ethernet/intel/ice.rst
> index 69695e5511f4..e4d065c55ea8 100644
> --- a/Documentation/networking/device_drivers/ethernet/intel/ice.rst
> +++ b/Documentation/networking/device_drivers/ethernet/intel/ice.rst
> @@ -84,24 +84,6 @@ Once the VM shuts down, or otherwise releases the VF, the command will
>  complete.
>
>
> -Important notes for SR-IOV and Link Aggregation
> ------------------------------------------------
> -Link Aggregation is mutually exclusive with SR-IOV.
> -
> -- If Link Aggregation is active, SR-IOV VFs cannot be created on the PF.
> -- If SR-IOV is active, you cannot set up Link Aggregation on the interface.
> -
> -Bridging and MACVLAN are also affected by this. If you wish to use bridging or
> -MACVLAN with SR-IOV, you must set up bridging or MACVLAN before enabling
> -SR-IOV. If you are using bridging or MACVLAN in conjunction with SR-IOV, and
> -you want to remove the interface from the bridge or MACVLAN, you must follow
> -these steps:
> -
> -1. Destroy SR-IOV VFs if they exist
> -2. Remove the interface from the bridge or MACVLAN
> -3. Recreate SRIOV VFs as needed
> -
> -
>  Additional Features and Configurations
>  ======================================
>
> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
> index 8b016511561f..b4bca1d964a9 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -814,25 +814,6 @@ static inline bool ice_is_switchdev_running(struct ice_pf *pf)
>         return pf->switchdev.is_running;
>  }
>
> -/**
> - * ice_set_sriov_cap - enable SRIOV in PF flags
> - * @pf: PF struct
> - */
> -static inline void ice_set_sriov_cap(struct ice_pf *pf)
> -{
> -       if (pf->hw.func_caps.common_cap.sr_iov_1_1)
> -               set_bit(ICE_FLAG_SRIOV_CAPABLE, pf->flags);
> -}
> -
> -/**
> - * ice_clear_sriov_cap - disable SRIOV in PF flags
> - * @pf: PF struct
> - */
> -static inline void ice_clear_sriov_cap(struct ice_pf *pf)
> -{
> -       clear_bit(ICE_FLAG_SRIOV_CAPABLE, pf->flags);
> -}
> -
>  #define ICE_FD_STAT_CTR_BLOCK_COUNT    256
>  #define ICE_FD_STAT_PF_IDX(base_idx) \
>                         ((base_idx) * ICE_FD_STAT_CTR_BLOCK_COUNT)
> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c
> index ee5b36941ba3..5a7753bda324 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lag.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lag.c
> @@ -6,15 +6,6 @@
>  #include "ice.h"
>  #include "ice_lag.h"
>
> -/**
> - * ice_lag_nop_handler - no-op Rx handler to disable LAG
> - * @pskb: pointer to skb pointer
> - */
> -rx_handler_result_t ice_lag_nop_handler(struct sk_buff __always_unused **pskb)
> -{
> -       return RX_HANDLER_PASS;
> -}
> -
>  /**
>   * ice_lag_set_primary - set PF LAG state as Primary
>   * @lag: LAG info struct
> @@ -158,7 +149,6 @@ ice_lag_link(struct ice_lag *lag, struct netdev_notifier_changeupper_info *info)
>                 lag->upper_netdev = upper;
>         }
>
> -       ice_clear_sriov_cap(pf);
>         ice_clear_rdma_cap(pf);
>
>         lag->bonded = true;
> @@ -205,7 +195,6 @@ ice_lag_unlink(struct ice_lag *lag,
>         }
>
>         lag->peer_netdev = NULL;
> -       ice_set_sriov_cap(pf);
>         ice_set_rdma_cap(pf);
>         lag->bonded = false;
>         lag->role = ICE_LAG_NONE;
> @@ -229,7 +218,6 @@ static void ice_lag_unregister(struct ice_lag *lag, struct net_device *netdev)
>         if (lag->upper_netdev) {
>                 dev_put(lag->upper_netdev);
>                 lag->upper_netdev = NULL;
> -               ice_set_sriov_cap(pf);
>                 ice_set_rdma_cap(pf);
>         }
>         /* perform some cleanup in case we come back */
> 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 */
> -       /* 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
>
Pucha, HimasekharX Reddy May 15, 2023, 7:28 a.m. UTC | #2
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Wojciech Drewek
> Sent: Friday, May 12, 2023 2:37 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH iwl-next] ice: Remove LAG+SRIOV mutual exclusion
>
> From: Dave Ertman <david.m.ertman@intel.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@linux.intel.com>
> Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.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(-)
>

Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Íñigo Huguet May 16, 2023, 9:18 a.m. UTC | #3
On Fri, May 12, 2023 at 11:08 AM Wojciech Drewek
<wojciech.drewek@intel.com> wrote:
>
> From: Dave Ertman <david.m.ertman@intel.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@linux.intel.com>
> Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.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/Documentation/networking/device_drivers/ethernet/intel/ice.rst b/Documentation/networking/device_drivers/ethernet/intel/ice.rst
> index 69695e5511f4..e4d065c55ea8 100644
> --- a/Documentation/networking/device_drivers/ethernet/intel/ice.rst
> +++ b/Documentation/networking/device_drivers/ethernet/intel/ice.rst
> @@ -84,24 +84,6 @@ Once the VM shuts down, or otherwise releases the VF, the command will
>  complete.
>
>
> -Important notes for SR-IOV and Link Aggregation
> ------------------------------------------------
> -Link Aggregation is mutually exclusive with SR-IOV.
> -
> -- If Link Aggregation is active, SR-IOV VFs cannot be created on the PF.
> -- If SR-IOV is active, you cannot set up Link Aggregation on the interface.
> -
> -Bridging and MACVLAN are also affected by this. If you wish to use bridging or
> -MACVLAN with SR-IOV, you must set up bridging or MACVLAN before enabling
> -SR-IOV. If you are using bridging or MACVLAN in conjunction with SR-IOV, and
> -you want to remove the interface from the bridge or MACVLAN, you must follow
> -these steps:
> -
> -1. Destroy SR-IOV VFs if they exist
> -2. Remove the interface from the bridge or MACVLAN
> -3. Recreate SRIOV VFs as needed
> -
> -
>  Additional Features and Configurations
>  ======================================
>
> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
> index 8b016511561f..b4bca1d964a9 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -814,25 +814,6 @@ static inline bool ice_is_switchdev_running(struct ice_pf *pf)
>         return pf->switchdev.is_running;
>  }
>
> -/**
> - * ice_set_sriov_cap - enable SRIOV in PF flags
> - * @pf: PF struct
> - */
> -static inline void ice_set_sriov_cap(struct ice_pf *pf)
> -{
> -       if (pf->hw.func_caps.common_cap.sr_iov_1_1)
> -               set_bit(ICE_FLAG_SRIOV_CAPABLE, pf->flags);
> -}
> -
> -/**
> - * ice_clear_sriov_cap - disable SRIOV in PF flags
> - * @pf: PF struct
> - */
> -static inline void ice_clear_sriov_cap(struct ice_pf *pf)
> -{
> -       clear_bit(ICE_FLAG_SRIOV_CAPABLE, pf->flags);
> -}
> -
>  #define ICE_FD_STAT_CTR_BLOCK_COUNT    256
>  #define ICE_FD_STAT_PF_IDX(base_idx) \
>                         ((base_idx) * ICE_FD_STAT_CTR_BLOCK_COUNT)
> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c
> index ee5b36941ba3..5a7753bda324 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lag.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lag.c
> @@ -6,15 +6,6 @@
>  #include "ice.h"
>  #include "ice_lag.h"
>
> -/**
> - * ice_lag_nop_handler - no-op Rx handler to disable LAG
> - * @pskb: pointer to skb pointer
> - */
> -rx_handler_result_t ice_lag_nop_handler(struct sk_buff __always_unused **pskb)
> -{
> -       return RX_HANDLER_PASS;
> -}
> -
>  /**
>   * ice_lag_set_primary - set PF LAG state as Primary
>   * @lag: LAG info struct
> @@ -158,7 +149,6 @@ ice_lag_link(struct ice_lag *lag, struct netdev_notifier_changeupper_info *info)
>                 lag->upper_netdev = upper;
>         }
>
> -       ice_clear_sriov_cap(pf);
>         ice_clear_rdma_cap(pf);
>
>         lag->bonded = true;
> @@ -205,7 +195,6 @@ ice_lag_unlink(struct ice_lag *lag,
>         }
>
>         lag->peer_netdev = NULL;
> -       ice_set_sriov_cap(pf);
>         ice_set_rdma_cap(pf);
>         lag->bonded = false;
>         lag->role = ICE_LAG_NONE;
> @@ -229,7 +218,6 @@ static void ice_lag_unregister(struct ice_lag *lag, struct net_device *netdev)
>         if (lag->upper_netdev) {
>                 dev_put(lag->upper_netdev);
>                 lag->upper_netdev = NULL;
> -               ice_set_sriov_cap(pf);
>                 ice_set_rdma_cap(pf);
>         }
>         /* perform some cleanup in case we come back */
> 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?

> -       /* 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
>
>
Wojciech Drewek May 16, 2023, 10:36 a.m. UTC | #4
> -----Original Message-----
> From: Íñigo Huguet <ihuguet@redhat.com>
> Sent: wtorek, 16 maja 2023 11:18
> To: Drewek, Wojciech <wojciech.drewek@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Ertman, David M <david.m.ertman@intel.com>; mschmidt
> <mschmidt@redhat.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@intel.com> wrote:
> >
> > From: Dave Ertman <david.m.ertman@intel.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@linux.intel.com>
> > Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
> > Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.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
diff mbox series

Patch

diff --git a/Documentation/networking/device_drivers/ethernet/intel/ice.rst b/Documentation/networking/device_drivers/ethernet/intel/ice.rst
index 69695e5511f4..e4d065c55ea8 100644
--- a/Documentation/networking/device_drivers/ethernet/intel/ice.rst
+++ b/Documentation/networking/device_drivers/ethernet/intel/ice.rst
@@ -84,24 +84,6 @@  Once the VM shuts down, or otherwise releases the VF, the command will
 complete.
 
 
-Important notes for SR-IOV and Link Aggregation
------------------------------------------------
-Link Aggregation is mutually exclusive with SR-IOV.
-
-- If Link Aggregation is active, SR-IOV VFs cannot be created on the PF.
-- If SR-IOV is active, you cannot set up Link Aggregation on the interface.
-
-Bridging and MACVLAN are also affected by this. If you wish to use bridging or
-MACVLAN with SR-IOV, you must set up bridging or MACVLAN before enabling
-SR-IOV. If you are using bridging or MACVLAN in conjunction with SR-IOV, and
-you want to remove the interface from the bridge or MACVLAN, you must follow
-these steps:
-
-1. Destroy SR-IOV VFs if they exist
-2. Remove the interface from the bridge or MACVLAN
-3. Recreate SRIOV VFs as needed
-
-
 Additional Features and Configurations
 ======================================
 
diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index 8b016511561f..b4bca1d964a9 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -814,25 +814,6 @@  static inline bool ice_is_switchdev_running(struct ice_pf *pf)
 	return pf->switchdev.is_running;
 }
 
-/**
- * ice_set_sriov_cap - enable SRIOV in PF flags
- * @pf: PF struct
- */
-static inline void ice_set_sriov_cap(struct ice_pf *pf)
-{
-	if (pf->hw.func_caps.common_cap.sr_iov_1_1)
-		set_bit(ICE_FLAG_SRIOV_CAPABLE, pf->flags);
-}
-
-/**
- * ice_clear_sriov_cap - disable SRIOV in PF flags
- * @pf: PF struct
- */
-static inline void ice_clear_sriov_cap(struct ice_pf *pf)
-{
-	clear_bit(ICE_FLAG_SRIOV_CAPABLE, pf->flags);
-}
-
 #define ICE_FD_STAT_CTR_BLOCK_COUNT	256
 #define ICE_FD_STAT_PF_IDX(base_idx) \
 			((base_idx) * ICE_FD_STAT_CTR_BLOCK_COUNT)
diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c
index ee5b36941ba3..5a7753bda324 100644
--- a/drivers/net/ethernet/intel/ice/ice_lag.c
+++ b/drivers/net/ethernet/intel/ice/ice_lag.c
@@ -6,15 +6,6 @@ 
 #include "ice.h"
 #include "ice_lag.h"
 
-/**
- * ice_lag_nop_handler - no-op Rx handler to disable LAG
- * @pskb: pointer to skb pointer
- */
-rx_handler_result_t ice_lag_nop_handler(struct sk_buff __always_unused **pskb)
-{
-	return RX_HANDLER_PASS;
-}
-
 /**
  * ice_lag_set_primary - set PF LAG state as Primary
  * @lag: LAG info struct
@@ -158,7 +149,6 @@  ice_lag_link(struct ice_lag *lag, struct netdev_notifier_changeupper_info *info)
 		lag->upper_netdev = upper;
 	}
 
-	ice_clear_sriov_cap(pf);
 	ice_clear_rdma_cap(pf);
 
 	lag->bonded = true;
@@ -205,7 +195,6 @@  ice_lag_unlink(struct ice_lag *lag,
 	}
 
 	lag->peer_netdev = NULL;
-	ice_set_sriov_cap(pf);
 	ice_set_rdma_cap(pf);
 	lag->bonded = false;
 	lag->role = ICE_LAG_NONE;
@@ -229,7 +218,6 @@  static void ice_lag_unregister(struct ice_lag *lag, struct net_device *netdev)
 	if (lag->upper_netdev) {
 		dev_put(lag->upper_netdev);
 		lag->upper_netdev = NULL;
-		ice_set_sriov_cap(pf);
 		ice_set_rdma_cap(pf);
 	}
 	/* perform some cleanup in case we come back */
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 */
-	/* 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;
 }