All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-wired-lan] [PATCH iwl-next v4] ice: Fix VF-VF filter rules in switchdev mode
@ 2023-09-26  7:45 Aniruddha Paul
  2023-09-26 10:13 ` Przemek Kitszel
  0 siblings, 1 reply; 2+ messages in thread
From: Aniruddha Paul @ 2023-09-26  7:45 UTC (permalink / raw)
  To: intel-wired-lan; +Cc: Aniruddha Paul, marcin.szycik, Przemek Kitszel

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>
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;
 
-- 
2.40.1

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [Intel-wired-lan] [PATCH iwl-next v4] ice: Fix VF-VF filter rules in switchdev mode
  2023-09-26  7:45 [Intel-wired-lan] [PATCH iwl-next v4] ice: Fix VF-VF filter rules in switchdev mode Aniruddha Paul
@ 2023-09-26 10:13 ` Przemek Kitszel
  0 siblings, 0 replies; 2+ messages in thread
From: Przemek Kitszel @ 2023-09-26 10:13 UTC (permalink / raw)
  To: Aniruddha Paul, intel-wired-lan; +Cc: marcin.szycik

On 9/26/23 09:45, 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")
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Aniruddha Paul <aniruddha.paul@intel.com>

You have forgot to apply Wojciech Drewek's Reviewed-by (was on e1000' v2 
IIRC),
and for public submissions we should CC netdev.

public versions should start at v1 :)
(it would be best to resend it tommorow, after 24h from first submission)

please

> ---
>   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;
>   

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-09-26 15:08 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-26  7:45 [Intel-wired-lan] [PATCH iwl-next v4] ice: Fix VF-VF filter rules in switchdev mode Aniruddha Paul
2023-09-26 10:13 ` Przemek Kitszel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.