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