[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <e5cd09d8-9c67-6af2-6d1a-92cb037be994@intel.com>
Date: Thu, 28 Sep 2023 14:42:06 -0700
From: Tony Nguyen <anthony.l.nguyen@...el.com>
To: Aniruddha Paul <aniruddha.paul@...el.com>,
<intel-wired-lan@...ts.osuosl.org>
CC: <marcin.szycik@...el.com>, <netdev@...r.kernel.org>, Przemek Kitszel
<przemyslaw.kitszel@...el.com>, Wojciech Drewek <wojciech.drewek@...el.com>
Subject: Re: [PATCH iwl-next,v1] ice: Fix VF-VF filter rules in switchdev mode
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@...el.com>
> Reviewed-by: Wojciech Drewek <wojciech.drewek@...el.com>
> Signed-off-by: Aniruddha Paul <aniruddha.paul@...el.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;
>
Powered by blists - more mailing lists