lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <58fd07c6-ce04-f802-08d2-1a65245aca00@amd.com>
Date: Fri, 15 Dec 2023 09:37:31 -0800
From: Brett Creeley <bcreeley@....com>
To: Andrii Staikov <andrii.staikov@...el.com>,
 intel-wired-lan@...ts.osuosl.org
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 Marcin Szycik <marcin.szycik@...el.com>,
 Wojciech Drewek <wojciech.drewek@...el.com>
Subject: Re: [PATCH iwl-next v6] ice: Add support for packet mirroring using
 hardware in switchdev mode



On 12/12/2023 4:51 AM, Andrii Staikov wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> Switchdev mode allows to add mirroring rules to mirror incoming and
> outgoing packets to the interface's port representor. Previously, this was
> available only using software functionality. Add possibility to offload
> this functionality to the NIC hardware.
> 
> Introduce ICE_MIRROR_PACKET filter action to the ice_sw_fwd_act_type enum
> to identify the desired action and pass it to the hardware as well as the
> VSI to mirror.
> 
> Example of tc mirror command using hardware:
>    tc filter add dev ens1f0np0 ingress protocol ip prio 1 flower src_mac
>    b4:96:91:a5:c7:a7 skip_sw action mirred egress mirror dev eth1
> 
> ens1f0np0 - PF
> b4:96:91:a5:c7:a7 - source MAC address
> eth1 - PR of a VF to mirror to
> 
> Co-developed-by: Marcin Szycik <marcin.szycik@...el.com>
> Signed-off-by: Marcin Szycik <marcin.szycik@...el.com>
> Reviewed-by: Wojciech Drewek <wojciech.drewek@...el.com>
> Signed-off-by: Andrii Staikov <andrii.staikov@...el.com>
> ---
> v1 -> v2: no need for changes in ice_add_tc_flower_adv_fltr()
> v2 -> v3: add another if branch for netif_is_ice(act->dev) ||
> ice_is_tunnel_supported(act->dev) for FLOW_ACTION_MIRRED action and
> add direction rules for filters
> v3 -> v4: move setting mirroring into dedicated function
> ice_tc_setup_mirror_action()
> v4 -> v5: Fix packets not mirroring from VF to VF by changing
> ICE_ESWITCH_FLTR_INGRESS to ICE_ESWITCH_FLTR_EGRESS where needed
> v5 -> v6: Additionally fix some tags
> ---
>   drivers/net/ethernet/intel/ice/ice_switch.c | 25 +++++++++----
>   drivers/net/ethernet/intel/ice/ice_tc_lib.c | 41 +++++++++++++++++++++
>   drivers/net/ethernet/intel/ice/ice_type.h   |  1 +
>   3 files changed, 60 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
> index ee19f3aa3d19..4af1ce2657ad 100644
> --- a/drivers/net/ethernet/intel/ice/ice_switch.c
> +++ b/drivers/net/ethernet/intel/ice/ice_switch.c
> @@ -6065,6 +6065,7 @@ ice_add_adv_rule(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
>                rinfo->sw_act.fltr_act == ICE_FWD_TO_Q ||
>                rinfo->sw_act.fltr_act == ICE_FWD_TO_QGRP ||
>                rinfo->sw_act.fltr_act == ICE_DROP_PACKET ||
> +             rinfo->sw_act.fltr_act == ICE_MIRROR_PACKET ||
>                rinfo->sw_act.fltr_act == ICE_NOP)) {
>                  status = -EIO;
>                  goto free_pkt_profile;
> @@ -6077,9 +6078,11 @@ ice_add_adv_rule(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
>          }
> 
>          if (rinfo->sw_act.fltr_act == ICE_FWD_TO_VSI ||
> -           rinfo->sw_act.fltr_act == ICE_NOP)
> +           rinfo->sw_act.fltr_act == ICE_MIRROR_PACKET ||
> +           rinfo->sw_act.fltr_act == ICE_NOP) {
>                  rinfo->sw_act.fwd_id.hw_vsi_id =
>                          ice_get_hw_vsi_num(hw, vsi_handle);
> +       }
> 
>          if (rinfo->src_vsi)
>                  rinfo->sw_act.src = ice_get_hw_vsi_num(hw, rinfo->src_vsi);
> @@ -6115,12 +6118,15 @@ ice_add_adv_rule(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
>                  status = -ENOMEM;
>                  goto free_pkt_profile;
>          }
> -       if (!rinfo->flags_info.act_valid) {
> -               act |= ICE_SINGLE_ACT_LAN_ENABLE;
> -               act |= ICE_SINGLE_ACT_LB_ENABLE;
> -       } else {
> -               act |= rinfo->flags_info.act & (ICE_SINGLE_ACT_LAN_ENABLE |
> -                                               ICE_SINGLE_ACT_LB_ENABLE);
> +
> +       if (rinfo->sw_act.fltr_act != ICE_MIRROR_PACKET) {
> +               if (!rinfo->flags_info.act_valid) {
> +                       act |= ICE_SINGLE_ACT_LAN_ENABLE;
> +                       act |= ICE_SINGLE_ACT_LB_ENABLE;
> +               } else {
> +                       act |= rinfo->flags_info.act & (ICE_SINGLE_ACT_LAN_ENABLE |
> +                                                       ICE_SINGLE_ACT_LB_ENABLE);
> +               }
>          }
> 
>          switch (rinfo->sw_act.fltr_act) {
> @@ -6147,6 +6153,11 @@ ice_add_adv_rule(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
>                  act |= ICE_SINGLE_ACT_VSI_FORWARDING | ICE_SINGLE_ACT_DROP |
>                         ICE_SINGLE_ACT_VALID_BIT;
>                  break;
> +       case ICE_MIRROR_PACKET:
> +               act |= ICE_SINGLE_ACT_OTHER_ACTS;
> +               act |= FIELD_PREP(ICE_SINGLE_ACT_VSI_ID_M,
> +                                 rinfo->sw_act.fwd_id.hw_vsi_id);
> +               break;
>          case ICE_NOP:
>                  act |= FIELD_PREP(ICE_SINGLE_ACT_VSI_ID_M,
>                                    rinfo->sw_act.fwd_id.hw_vsi_id);
> diff --git a/drivers/net/ethernet/intel/ice/ice_tc_lib.c b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> index 08d3bbf4b44c..b890410a2bc0 100644
> --- a/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> @@ -689,6 +689,41 @@ ice_tc_setup_drop_action(struct net_device *filter_dev,
>          return 0;
>   }
> 
> +static int ice_tc_setup_mirror_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_MIRROR_PACKET;
> +
> +       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->eswitch.uplink_vsi;

It seems like multiple places use this and pf->eswitch.uplink_vsi. Would 
it be worth adding a helper function for this, i.e.:

ice_get_eswitch_uplink_vsi(pf)

> +               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)
> @@ -710,6 +745,12 @@ static int ice_eswitch_tc_parse_action(struct net_device *filter_dev,
> 
>                  break;
> 
> +       case FLOW_ACTION_MIRRED:
> +               err = ice_tc_setup_mirror_action(filter_dev, fltr, act->dev);
> +               if (err)
> +                       return err;
> +               break;
> +
>          default:
>                  NL_SET_ERR_MSG_MOD(fltr->extack, "Unsupported action in switchdev mode");
>                  return -EINVAL;
> diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
> index 5a80158e49ed..20c014e9b6c0 100644
> --- a/drivers/net/ethernet/intel/ice/ice_type.h
> +++ b/drivers/net/ethernet/intel/ice/ice_type.h
> @@ -1055,6 +1055,7 @@ enum ice_sw_fwd_act_type {
>          ICE_FWD_TO_Q,
>          ICE_FWD_TO_QGRP,
>          ICE_DROP_PACKET,
> +       ICE_MIRROR_PACKET,
>          ICE_NOP,
>          ICE_INVAL_ACT
>   };
> --
> 2.25.1
> 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ