diff mbox series

[iwl-next,v1] ice: Fix VF-VF filter rules in switchdev mode

Message ID 20230927104253.1729049-1-aniruddha.paul@intel.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [iwl-next,v1] ice: Fix VF-VF filter rules in switchdev mode | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1340 this patch: 1340
netdev/cc_maintainers fail 3 blamed authors not CCed: horms@kernel.org marcin.szycik@linux.intel.com anthony.l.nguyen@intel.com; 8 maintainers not CCed: pabeni@redhat.com jesse.brandeburg@intel.com davem@davemloft.net horms@kernel.org edumazet@google.com marcin.szycik@linux.intel.com anthony.l.nguyen@intel.com kuba@kernel.org
netdev/build_clang success Errors and warnings before: 1363 this patch: 1363
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1363 this patch: 1363
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Aniruddha Paul Sept. 27, 2023, 10:42 a.m. UTC 
Any packet leaving VSI i.e VF's VSI is considered as
egress traffic by HW, thus failing to match the added
rule.

Mark the direction for redirect rules as below:
1. VF-VF - Egress
2. Uplink-VF - Ingress
3. VF-Uplink - Egress
4. Link_Partner-Uplink - Ingress
5. Link_Partner-VF - Ingress

Fixes: 0960a27bd479 ("ice: Add direction metadata")
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Signed-off-by: Aniruddha Paul <aniruddha.paul@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_tc_lib.c | 90 ++++++++++++++-------
 1 file changed, 62 insertions(+), 28 deletions(-)

Comments

Tony Nguyen Sept. 28, 2023, 9:42 p.m. UTC | #1 
On 9/27/2023 3:42 AM, Aniruddha Paul wrote:
> Any packet leaving VSI i.e VF's VSI is considered as
> egress traffic by HW, thus failing to match the added
> rule.
> 
> Mark the direction for redirect rules as below:
> 1. VF-VF - Egress
> 2. Uplink-VF - Ingress
> 3. VF-Uplink - Egress
> 4. Link_Partner-Uplink - Ingress
> 5. Link_Partner-VF - Ingress
> 
> Fixes: 0960a27bd479 ("ice: Add direction metadata")

Why iwl-next and not iwl-net? Also, this patch does not compile.

> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
> Signed-off-by: Aniruddha Paul <aniruddha.paul@intel.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_tc_lib.c | 90 ++++++++++++++-------
>   1 file changed, 62 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_tc_lib.c b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> index 37b54db91df2..0e75fc6b3c06 100644
> --- a/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> @@ -630,32 +630,61 @@ bool ice_is_tunnel_supported(struct net_device *dev)
>   	return ice_tc_tun_get_type(dev) != TNL_LAST;
>   }
>   
> -static int
> -ice_eswitch_tc_parse_action(struct ice_tc_flower_fltr *fltr,
> -			    struct flow_action_entry *act)
> +static bool ice_tc_is_dev_uplink(struct net_device *dev)
> +{
> +	return netif_is_ice(dev) || ice_is_tunnel_supported(dev);
> +}
> +
> +static int ice_tc_setup_redirect_action(struct net_device *filter_dev,
> +					struct ice_tc_flower_fltr *fltr,
> +					struct net_device *target_dev)
>   {
>   	struct ice_repr *repr;
>   
> +	fltr->action.fltr_act = ICE_FWD_TO_VSI;
> +
> +	if (ice_is_port_repr_netdev(filter_dev) &&
> +	    ice_is_port_repr_netdev(target_dev)) {
> +		repr = ice_netdev_to_repr(target_dev);
> +
> +		fltr->dest_vsi = repr->src_vsi;
> +		fltr->direction = ICE_ESWITCH_FLTR_EGRESS;
> +	} else if (ice_is_port_repr_netdev(filter_dev) &&
> +		   ice_tc_is_dev_uplink(target_dev)) {
> +		repr = ice_netdev_to_repr(filter_dev);
> +
> +		fltr->dest_vsi = repr->src_vsi->back->switchdev.uplink_vsi;
> +		fltr->direction = ICE_ESWITCH_FLTR_EGRESS;
> +	} else if (ice_tc_is_dev_uplink(filter_dev) &&
> +		   ice_is_port_repr_netdev(target_dev)) {
> +		repr = ice_netdev_to_repr(target_dev);
> +
> +		fltr->dest_vsi = repr->src_vsi;
> +		fltr->direction = ICE_ESWITCH_FLTR_INGRESS;
> +	} else {
> +		NL_SET_ERR_MSG_MOD(fltr->extack,
> +				   "Unsupported netdevice in switchdev mode");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ice_eswitch_tc_parse_action(struct net_device *filter_dev,
> +				       struct ice_tc_flower_fltr *fltr,
> +				       struct flow_action_entry *act)
> +{
> +	int err;
> +
>   	switch (act->id) {
>   	case FLOW_ACTION_DROP:
>   		fltr->action.fltr_act = ICE_DROP_PACKET;
>   		break;
>   
>   	case FLOW_ACTION_REDIRECT:
> -		fltr->action.fltr_act = ICE_FWD_TO_VSI;
> -
> -		if (ice_is_port_repr_netdev(act->dev)) {
> -			repr = ice_netdev_to_repr(act->dev);
> -
> -			fltr->dest_vsi = repr->src_vsi;
> -			fltr->direction = ICE_ESWITCH_FLTR_INGRESS;
> -		} else if (netif_is_ice(act->dev) ||
> -			   ice_is_tunnel_supported(act->dev)) {
> -			fltr->direction = ICE_ESWITCH_FLTR_EGRESS;
> -		} else {
> -			NL_SET_ERR_MSG_MOD(fltr->extack, "Unsupported netdevice in switchdev mode");
> -			return -EINVAL;
> -		}
> +		err = ice_tc_setup_redirect_action(filter_dev, fltr, act->dev);
> +		if (err)
> +			return err;
>   
>   		break;
>   
> @@ -696,10 +725,6 @@ ice_eswitch_add_tc_fltr(struct ice_vsi *vsi, struct ice_tc_flower_fltr *fltr)
>   		goto exit;
>   	}
>   
> -	/* egress traffic is always redirect to uplink */
> -	if (fltr->direction == ICE_ESWITCH_FLTR_EGRESS)
> -		fltr->dest_vsi = vsi->back->switchdev.uplink_vsi;
> -
>   	rule_info.sw_act.fltr_act = fltr->action.fltr_act;
>   	if (fltr->action.fltr_act != ICE_DROP_PACKET)
>   		rule_info.sw_act.vsi_handle = fltr->dest_vsi->idx;
> @@ -713,13 +738,21 @@ ice_eswitch_add_tc_fltr(struct ice_vsi *vsi, struct ice_tc_flower_fltr *fltr)
>   	rule_info.flags_info.act_valid = true;
>   
>   	if (fltr->direction == ICE_ESWITCH_FLTR_INGRESS) {
> +		/* Uplink to VF */
>   		rule_info.sw_act.flag |= ICE_FLTR_RX;
>   		rule_info.sw_act.src = hw->pf_id;
>   		rule_info.flags_info.act = ICE_SINGLE_ACT_LB_ENABLE;
> -	} else {
> +	} else if (fltr->direction == ICE_ESWITCH_FLTR_EGRESS &&
> +		   fltr->dest_vsi == vsi->back->switchdev.uplink_vsi) {
> +		/* VF to Uplink */
>   		rule_info.sw_act.flag |= ICE_FLTR_TX;
>   		rule_info.sw_act.src = vsi->idx;
>   		rule_info.flags_info.act = ICE_SINGLE_ACT_LAN_ENABLE;
> +	} else {
> +		/* VF to VF */
> +		rule_info.sw_act.flag |= ICE_FLTR_TX;
> +		rule_info.sw_act.src = vsi->idx;
> +		rule_info.flags_info.act = ICE_SINGLE_ACT_LB_ENABLE;
>   	}
>   
>   	/* specify the cookie as filter_rule_id */
> @@ -1745,16 +1778,17 @@ ice_tc_parse_action(struct ice_vsi *vsi, struct ice_tc_flower_fltr *fltr,
>   
>   /**
>    * ice_parse_tc_flower_actions - Parse the actions for a TC filter
> + * @filter_dev: Pointer to device on which filter is being added
>    * @vsi: Pointer to VSI
>    * @cls_flower: Pointer to TC flower offload structure
>    * @fltr: Pointer to TC flower filter structure
>    *
>    * Parse the actions for a TC filter
>    */
> -static int
> -ice_parse_tc_flower_actions(struct ice_vsi *vsi,
> -			    struct flow_cls_offload *cls_flower,
> -			    struct ice_tc_flower_fltr *fltr)
> +static int ice_parse_tc_flower_actions(struct net_device *filter_dev,
> +				       struct ice_vsi *vsi,
> +				       struct flow_cls_offload *cls_flower,
> +				       struct ice_tc_flower_fltr *fltr)
>   {
>   	struct flow_rule *rule = flow_cls_offload_flow_rule(cls_flower);
>   	struct flow_action *flow_action = &rule->action;
> @@ -1769,7 +1803,7 @@ ice_parse_tc_flower_actions(struct ice_vsi *vsi,
>   
>   	flow_action_for_each(i, act, flow_action) {
>   		if (ice_is_eswitch_mode_switchdev(vsi->back))
> -			err = ice_eswitch_tc_parse_action(fltr, act);
> +			err = ice_eswitch_tc_parse_action(filter_dev, fltr, act);
>   		else
>   			err = ice_tc_parse_action(vsi, fltr, act);
>   		if (err)
> @@ -1856,7 +1890,7 @@ ice_add_tc_fltr(struct net_device *netdev, struct ice_vsi *vsi,
>   	if (err < 0)
>   		goto err;
>   
> -	err = ice_parse_tc_flower_actions(vsi, f, fltr);
> +	err = ice_parse_tc_flower_actions(netdev, vsi, f, fltr);
>   	if (err < 0)
>   		goto err;
>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/ice_tc_lib.c b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
index 37b54db91df2..0e75fc6b3c06 100644
--- a/drivers/net/ethernet/intel/ice/ice_tc_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
@@ -630,32 +630,61 @@  bool ice_is_tunnel_supported(struct net_device *dev)
 	return ice_tc_tun_get_type(dev) != TNL_LAST;
 }
 
-static int
-ice_eswitch_tc_parse_action(struct ice_tc_flower_fltr *fltr,
-			    struct flow_action_entry *act)
+static bool ice_tc_is_dev_uplink(struct net_device *dev)
+{
+	return netif_is_ice(dev) || ice_is_tunnel_supported(dev);
+}
+
+static int ice_tc_setup_redirect_action(struct net_device *filter_dev,
+					struct ice_tc_flower_fltr *fltr,
+					struct net_device *target_dev)
 {
 	struct ice_repr *repr;
 
+	fltr->action.fltr_act = ICE_FWD_TO_VSI;
+
+	if (ice_is_port_repr_netdev(filter_dev) &&
+	    ice_is_port_repr_netdev(target_dev)) {
+		repr = ice_netdev_to_repr(target_dev);
+
+		fltr->dest_vsi = repr->src_vsi;
+		fltr->direction = ICE_ESWITCH_FLTR_EGRESS;
+	} else if (ice_is_port_repr_netdev(filter_dev) &&
+		   ice_tc_is_dev_uplink(target_dev)) {
+		repr = ice_netdev_to_repr(filter_dev);
+
+		fltr->dest_vsi = repr->src_vsi->back->switchdev.uplink_vsi;
+		fltr->direction = ICE_ESWITCH_FLTR_EGRESS;
+	} else if (ice_tc_is_dev_uplink(filter_dev) &&
+		   ice_is_port_repr_netdev(target_dev)) {
+		repr = ice_netdev_to_repr(target_dev);
+
+		fltr->dest_vsi = repr->src_vsi;
+		fltr->direction = ICE_ESWITCH_FLTR_INGRESS;
+	} else {
+		NL_SET_ERR_MSG_MOD(fltr->extack,
+				   "Unsupported netdevice in switchdev mode");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int ice_eswitch_tc_parse_action(struct net_device *filter_dev,
+				       struct ice_tc_flower_fltr *fltr,
+				       struct flow_action_entry *act)
+{
+	int err;
+
 	switch (act->id) {
 	case FLOW_ACTION_DROP:
 		fltr->action.fltr_act = ICE_DROP_PACKET;
 		break;
 
 	case FLOW_ACTION_REDIRECT:
-		fltr->action.fltr_act = ICE_FWD_TO_VSI;
-
-		if (ice_is_port_repr_netdev(act->dev)) {
-			repr = ice_netdev_to_repr(act->dev);
-
-			fltr->dest_vsi = repr->src_vsi;
-			fltr->direction = ICE_ESWITCH_FLTR_INGRESS;
-		} else if (netif_is_ice(act->dev) ||
-			   ice_is_tunnel_supported(act->dev)) {
-			fltr->direction = ICE_ESWITCH_FLTR_EGRESS;
-		} else {
-			NL_SET_ERR_MSG_MOD(fltr->extack, "Unsupported netdevice in switchdev mode");
-			return -EINVAL;
-		}
+		err = ice_tc_setup_redirect_action(filter_dev, fltr, act->dev);
+		if (err)
+			return err;
 
 		break;
 
@@ -696,10 +725,6 @@  ice_eswitch_add_tc_fltr(struct ice_vsi *vsi, struct ice_tc_flower_fltr *fltr)
 		goto exit;
 	}
 
-	/* egress traffic is always redirect to uplink */
-	if (fltr->direction == ICE_ESWITCH_FLTR_EGRESS)
-		fltr->dest_vsi = vsi->back->switchdev.uplink_vsi;
-
 	rule_info.sw_act.fltr_act = fltr->action.fltr_act;
 	if (fltr->action.fltr_act != ICE_DROP_PACKET)
 		rule_info.sw_act.vsi_handle = fltr->dest_vsi->idx;
@@ -713,13 +738,21 @@  ice_eswitch_add_tc_fltr(struct ice_vsi *vsi, struct ice_tc_flower_fltr *fltr)
 	rule_info.flags_info.act_valid = true;
 
 	if (fltr->direction == ICE_ESWITCH_FLTR_INGRESS) {
+		/* Uplink to VF */
 		rule_info.sw_act.flag |= ICE_FLTR_RX;
 		rule_info.sw_act.src = hw->pf_id;
 		rule_info.flags_info.act = ICE_SINGLE_ACT_LB_ENABLE;
-	} else {
+	} else if (fltr->direction == ICE_ESWITCH_FLTR_EGRESS &&
+		   fltr->dest_vsi == vsi->back->switchdev.uplink_vsi) {
+		/* VF to Uplink */
 		rule_info.sw_act.flag |= ICE_FLTR_TX;
 		rule_info.sw_act.src = vsi->idx;
 		rule_info.flags_info.act = ICE_SINGLE_ACT_LAN_ENABLE;
+	} else {
+		/* VF to VF */
+		rule_info.sw_act.flag |= ICE_FLTR_TX;
+		rule_info.sw_act.src = vsi->idx;
+		rule_info.flags_info.act = ICE_SINGLE_ACT_LB_ENABLE;
 	}
 
 	/* specify the cookie as filter_rule_id */
@@ -1745,16 +1778,17 @@  ice_tc_parse_action(struct ice_vsi *vsi, struct ice_tc_flower_fltr *fltr,
 
 /**
  * ice_parse_tc_flower_actions - Parse the actions for a TC filter
+ * @filter_dev: Pointer to device on which filter is being added
  * @vsi: Pointer to VSI
  * @cls_flower: Pointer to TC flower offload structure
  * @fltr: Pointer to TC flower filter structure
  *
  * Parse the actions for a TC filter
  */
-static int
-ice_parse_tc_flower_actions(struct ice_vsi *vsi,
-			    struct flow_cls_offload *cls_flower,
-			    struct ice_tc_flower_fltr *fltr)
+static int ice_parse_tc_flower_actions(struct net_device *filter_dev,
+				       struct ice_vsi *vsi,
+				       struct flow_cls_offload *cls_flower,
+				       struct ice_tc_flower_fltr *fltr)
 {
 	struct flow_rule *rule = flow_cls_offload_flow_rule(cls_flower);
 	struct flow_action *flow_action = &rule->action;
@@ -1769,7 +1803,7 @@  ice_parse_tc_flower_actions(struct ice_vsi *vsi,
 
 	flow_action_for_each(i, act, flow_action) {
 		if (ice_is_eswitch_mode_switchdev(vsi->back))
-			err = ice_eswitch_tc_parse_action(fltr, act);
+			err = ice_eswitch_tc_parse_action(filter_dev, fltr, act);
 		else
 			err = ice_tc_parse_action(vsi, fltr, act);
 		if (err)
@@ -1856,7 +1890,7 @@  ice_add_tc_fltr(struct net_device *netdev, struct ice_vsi *vsi,
 	if (err < 0)
 		goto err;
 
-	err = ice_parse_tc_flower_actions(vsi, f, fltr);
+	err = ice_parse_tc_flower_actions(netdev, vsi, f, fltr);
 	if (err < 0)
 		goto err;