[Intel-wired-lan] [PATCH iwl-next v2] ice: Add support for packet mirroring using hardware in switchdev mode

[Intel-wired-lan] [PATCH iwl-next v2] ice: Add support for packet mirroring using hardware in switchdev mode

[Andrii Staikov]

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

Signed-off-by: Andrii Staikov <andrii.staikov@intel.com>
---
v1 -> v2: no need for changes in ice_add_tc_flower_adv_fltr()
---
 drivers/net/ethernet/intel/ice/ice_switch.c | 25 +++++++++++++++------
 drivers/net/ethernet/intel/ice/ice_tc_lib.c | 13 +++++++++++
 drivers/net/ethernet/intel/ice/ice_type.h   |  1 +
 3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
index 2f77b684ff76..d915b72e8dbb 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.c
+++ b/drivers/net/ethernet/intel/ice/ice_switch.c
@@ -6100,6 +6100,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;
@@ -6112,9 +6113,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);
@@ -6150,12 +6153,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) {
@@ -6182,6 +6188,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 |= (rinfo->sw_act.fwd_id.hw_vsi_id << ICE_SINGLE_ACT_MIRROR_VSI_ID_S) &
+		       ICE_SINGLE_ACT_MIRROR_VSI_ID_M;
+		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 37b54db91df2..db34df1890f7 100644
--- a/drivers/net/ethernet/intel/ice/ice_tc_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
@@ -659,6 +659,19 @@ ice_eswitch_tc_parse_action(struct ice_tc_flower_fltr *fltr,
 
 		break;
 
+	case FLOW_ACTION_MIRRED:
+		fltr->action.fltr_act = ICE_MIRROR_PACKET;
+
+		if (ice_is_port_repr_netdev(act->dev)) {
+			repr = ice_netdev_to_repr(act->dev);
+
+			fltr->dest_vsi = repr->src_vsi;
+		} else {
+			NL_SET_ERR_MSG_MOD(fltr->extack, "Provided netdevice doesn't support mirroring");
+			return -EINVAL;
+		}
+		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 02db9e5810e6..f5c35dc8766f 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -1047,6 +1047,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

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

Re: [Intel-wired-lan] [PATCH iwl-next v2] ice: Add support for packet mirroring using hardware in switchdev mode

[Paolo Abeni]

Hi all,

On Tue, 2023-09-12 at 11:29 +0200, Andrii Staikov wrote:
> 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
> 
> Signed-off-by: Andrii Staikov <andrii.staikov@intel.com>

The amount of patches that IMHO should land only into intel-specific
MLs and instead reaches also netdev, recently increased.

Please try harder to apply proper constraints to your traffic, netdev
is already busy enough!

Thanks,

Paolo

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

Re: [Intel-wired-lan] [PATCH iwl-next v2] ice: Add support for packet mirroring using hardware in switchdev mode

[Alexander Lobakin]

From: Paolo Abeni <pabeni@redhat.com>
Date: Tue, 12 Sep 2023 11:56:15 +0200

> Hi all,
> 
> On Tue, 2023-09-12 at 11:29 +0200, Andrii Staikov wrote:
>> 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
>>
>> Signed-off-by: Andrii Staikov <andrii.staikov@intel.com>
> 
> The amount of patches that IMHO should land only into intel-specific
> MLs and instead reaches also netdev, recently increased.

Let's clarify what you mean by "intel-specific MLs".
Do you mean our internal MLs and review or the open one, IWL?

IWL is mostly rudimentary. It's open, but almost nobody outside of Intel
sits there, which means 2/3 of patches doesn't get enough attention and
reviews. It's used by our validation as well, but that's it.
Our internal ML for Ethernet patches works as usually. I realize roughly
half of all patches pass it without a Reviewed-by tag and it's something
we're actively working on. If all goes well, no patches without a proper
review will go outside Intel's internal Ethernet MLs.
Now, the second part,

> 
> Please try harder to apply proper constraints to your traffic, netdev
> is already busy enough!

Do you want us to stop CCing netdev when we send patches to the outer
review to IWL?
This would mean they will once again start missing enough attention from
the outside. I hope you don't want our patches to be reviewed *only* by
Intel folks, right? I don't feel this a good idea.
That's why we started CCing netdev this year. And we do that when we
send patches to IWL, i.e. outside. It's not like "ok, let's Cc netdev
instead of going through our internal review process".

Our clients, partners (e.g. Czech RedHat), our developers, want our
patches to have proper complex review. Dropping netdev would mean that a
patch of some non-corpo guy will be reviewed more carefully and at the
end will have better quality than an Intel patch, which "shouldn't
overburden netdev".
Saying "we'll see them when Tony sends a PR" also doesn't work well for
me. A patch gets taken into a PR once it passes internal review, then
validation, this always do take a while. Imagine waiting for a month for
your patch to be sent in a PR to get a negative review, so that you have
to repeat this process again and wait for another month to get some more
change requests and again :D

In a couple months, no our patches will hit netdev without a proper
Reviewed-by obtained during the internal review, let's not take corner
cases and effectively hide our code from the world?
I don't think you'd like to put a huge banner on netdev's lore saying
"please also take a look at intel-wired-lan" :z I also don't want ppl to
behave like Greg KH some time ago when he said "where's your damn
internal RB, stop abusing LKML" in reply to my early RFC PoC sent only
for an open discussion xD

> 
> Thanks,
> 
> Paolo

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

Re: [Intel-wired-lan] [PATCH iwl-next v2] ice: Add support for packet mirroring using hardware in switchdev mode

[Paolo Abeni]

On Tue, 2023-09-12 at 16:41 +0200, Alexander Lobakin wrote:
> From: Paolo Abeni <pabeni@redhat.com>
> Date: Tue, 12 Sep 2023 11:56:15 +0200
> 
> > Hi all,
> > 
> > On Tue, 2023-09-12 at 11:29 +0200, Andrii Staikov wrote:
> > > 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
> > > 
> > > Signed-off-by: Andrii Staikov <andrii.staikov@intel.com>
> > 
> > The amount of patches that IMHO should land only into intel-specific
> > MLs and instead reaches also netdev, recently increased.
> 
> Let's clarify what you mean by "intel-specific MLs".
> Do you mean our internal MLs and review or the open one, IWL?
> 
> IWL is mostly rudimentary. It's open, but almost nobody outside of Intel
> sits there, which means 2/3 of patches doesn't get enough attention and
> reviews. It's used by our validation as well, but that's it.
> Our internal ML for Ethernet patches works as usually. I realize roughly
> half of all patches pass it without a Reviewed-by tag and it's something
> we're actively working on. If all goes well, no patches without a proper
> review will go outside Intel's internal Ethernet MLs.
> Now, the second part,
> 
> > 
> > Please try harder to apply proper constraints to your traffic, netdev
> > is already busy enough!
> 
> Do you want us to stop CCing netdev when we send patches to the outer
> review to IWL?
> This would mean they will once again start missing enough attention from
> the outside. I hope you don't want our patches to be reviewed *only* by
> Intel folks, right? I don't feel this a good idea.
> That's why we started CCing netdev this year. And we do that when we
> send patches to IWL, i.e. outside. It's not like "ok, let's Cc netdev
> instead of going through our internal review process".
> 
> Our clients, partners (e.g. Czech RedHat), our developers, want our
> patches to have proper complex review. Dropping netdev would mean that a
> patch of some non-corpo guy will be reviewed more carefully and at the
> end will have better quality than an Intel patch, which "shouldn't
> overburden netdev".
> Saying "we'll see them when Tony sends a PR" also doesn't work well for
> me. A patch gets taken into a PR once it passes internal review, then
> validation, this always do take a while. Imagine waiting for a month for
> your patch to be sent in a PR to get a negative review, so that you have
> to repeat this process again and wait for another month to get some more
> change requests and again :D
> 
> In a couple months, no our patches will hit netdev without a proper
> Reviewed-by obtained during the internal review, let's not take corner
> cases and effectively hide our code from the world?
> I don't think you'd like to put a huge banner on netdev's lore saying
> "please also take a look at intel-wired-lan" :z I also don't want ppl to
> behave like Greg KH some time ago when he said "where's your damn
> internal RB, stop abusing LKML" in reply to my early RFC PoC sent only
> for an open discussion xD

I was under the impression that some patches landed on IWL cc-ing
netdev possibly unintentionally, e.g.:

https://lore.kernel.org/netdev/20230904021455.3944605-1-junfeng.guo@intel.com/

My intention was to raise attention on such events.

Cheers,

Paolo

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

Re: [Intel-wired-lan] [PATCH iwl-next v2] ice: Add support for packet mirroring using hardware in switchdev mode

[Buvaneswaran, Sujai]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Andrii Staikov
> Sent: Tuesday, September 12, 2023 3:00 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Staikov, Andrii <andrii.staikov@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next v2] ice: Add support for packet
> mirroring using hardware in switchdev mode
> 
> 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
> 
> Signed-off-by: Andrii Staikov <andrii.staikov@intel.com>
> ---
> v1 -> v2: no need for changes in ice_add_tc_flower_adv_fltr()
> ---
>  drivers/net/ethernet/intel/ice/ice_switch.c | 25 +++++++++++++++------
> drivers/net/ethernet/intel/ice/ice_tc_lib.c | 13 +++++++++++
>  drivers/net/ethernet/intel/ice/ice_type.h   |  1 +
>  3 files changed, 32 insertions(+), 7 deletions(-)
> 

Tested this patch with HW offload enabled on both PF and VF. Packet mirroring is working on the PF but it fails on the packets originating from VF.

Configured below mirror rules for mirroring the packets from both PF1 and VF1 to VF2.

tc filter add dev $PF1 ingress protocol ip prio 1 flower src_mac b4:96:91:9f:64:08 skip_sw action mirred egress mirror dev $VF2_PR
tc filter add dev $VF1_PR ingress protocol ip prio 1 flower src_mac 52:54:00:00:16:01 skip_sw action mirred egress mirror dev $VF2_PR

With the above mirror rules, the packets from PF got mirrored to VF2. But packets originating from VF1 are not seen in VF2.
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan