[iwl-next v1] ice: add recipe priority check in search

[iwl-next v1] ice: add recipe priority check in search

[Michal Swiatkowski]

The new recipe should be added even if exactly the same recipe already
exists with different priority.

Example use case is when the rule is being added from TC tool context.
It should has the highest priority, but if the recipe already exists
the rule will inherit it priority. It can lead to the situation when
the rule added from TC tool has lower priority than expected.

The solution is to check the recipe priority when trying to find
existing one.

Previous recipe is still useful. Example:
RID 8 -> priority 4
RID 10 -> priority 7

The difference is only in priority rest is let's say eth + mac +
direction.

Adding ARP + MAC_A + RX on RID 8, forward to VF0_VSI
After that IP + MAC_B + RX on RID 10 (from TC tool), forward to PF0

Both will work.

In case of adding ARP + MAC_A + RX on RID 8, forward to VF0_VSI
ARP + MAC_A + RX on RID 10, forward to PF0.

Only second one will match, but this is expected.

Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
---
 drivers/net/ethernet/intel/ice/ice_switch.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
index 79d91e95358c..6a4a11fa5f14 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.c
+++ b/drivers/net/ethernet/intel/ice/ice_switch.c
@@ -4784,7 +4784,8 @@ ice_find_recp(struct ice_hw *hw, struct ice_prot_lkup_ext *lkup_exts,
 			 */
 			if (found && recp[i].tun_type == rinfo->tun_type &&
 			    recp[i].need_pass_l2 == rinfo->need_pass_l2 &&
-			    recp[i].allow_pass_l2 == rinfo->allow_pass_l2)
+			    recp[i].allow_pass_l2 == rinfo->allow_pass_l2 &&
+			    recp[i].priority == rinfo->priority)
 				return i; /* Return the recipe ID */
 		}
 	}
-- 
2.42.0

Re: [iwl-next v1] ice: add recipe priority check in search

[Simon Horman]

On Fri, Oct 11, 2024 at 09:03:28AM +0200, Michal Swiatkowski wrote:
> The new recipe should be added even if exactly the same recipe already
> exists with different priority.
> 
> Example use case is when the rule is being added from TC tool context.
> It should has the highest priority, but if the recipe already exists
> the rule will inherit it priority. It can lead to the situation when
> the rule added from TC tool has lower priority than expected.
> 
> The solution is to check the recipe priority when trying to find
> existing one.
> 
> Previous recipe is still useful. Example:
> RID 8 -> priority 4
> RID 10 -> priority 7
> 
> The difference is only in priority rest is let's say eth + mac +
> direction.
> 
> Adding ARP + MAC_A + RX on RID 8, forward to VF0_VSI
> After that IP + MAC_B + RX on RID 10 (from TC tool), forward to PF0
> 
> Both will work.
> 
> In case of adding ARP + MAC_A + RX on RID 8, forward to VF0_VSI
> ARP + MAC_A + RX on RID 10, forward to PF0.
> 
> Only second one will match, but this is expected.
> 
> Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

Reviewed-by: Simon Horman <horms@kernel.org>

RE: [Intel-wired-lan] [iwl-next v1] ice: add recipe priority check in search

[Buvaneswaran, Sujai]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Michal Swiatkowski
> Sent: Friday, October 11, 2024 12:33 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Szycik, Marcin <marcin.szycik@intel.com>;
> Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>
> Subject: [Intel-wired-lan] [iwl-next v1] ice: add recipe priority check in search
> 
> The new recipe should be added even if exactly the same recipe already
> exists with different priority.
> 
> Example use case is when the rule is being added from TC tool context.
> It should has the highest priority, but if the recipe already exists the rule will
> inherit it priority. It can lead to the situation when the rule added from TC
> tool has lower priority than expected.
> 
> The solution is to check the recipe priority when trying to find existing one.
> 
> Previous recipe is still useful. Example:
> RID 8 -> priority 4
> RID 10 -> priority 7
> 
> The difference is only in priority rest is let's say eth + mac + direction.
> 
> Adding ARP + MAC_A + RX on RID 8, forward to VF0_VSI After that IP +
> MAC_B + RX on RID 10 (from TC tool), forward to PF0
> 
> Both will work.
> 
> In case of adding ARP + MAC_A + RX on RID 8, forward to VF0_VSI ARP +
> MAC_A + RX on RID 10, forward to PF0.
> 
> Only second one will match, but this is expected.
> 
> Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_switch.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

Hi,

System hang is observed when creating the VFs in Switchdev mode with the latest next-queue kernel. Need to powercycle the server to recover the system.
This issue is blocking the validation of this patch.

Thanks,
Sujai B

RE: [Intel-wired-lan] [iwl-next v1] ice: add recipe priority check in search

[Buvaneswaran, Sujai]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Michal Swiatkowski
> Sent: Friday, October 11, 2024 12:33 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Szycik, Marcin <marcin.szycik@intel.com>;
> Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>
> Subject: [Intel-wired-lan] [iwl-next v1] ice: add recipe priority check in search
> 
> The new recipe should be added even if exactly the same recipe already
> exists with different priority.
> 
> Example use case is when the rule is being added from TC tool context.
> It should has the highest priority, but if the recipe already exists the rule will
> inherit it priority. It can lead to the situation when the rule added from TC
> tool has lower priority than expected.
> 
> The solution is to check the recipe priority when trying to find existing one.
> 
> Previous recipe is still useful. Example:
> RID 8 -> priority 4
> RID 10 -> priority 7
> 
> The difference is only in priority rest is let's say eth + mac + direction.
> 
> Adding ARP + MAC_A + RX on RID 8, forward to VF0_VSI After that IP +
> MAC_B + RX on RID 10 (from TC tool), forward to PF0
> 
> Both will work.
> 
> In case of adding ARP + MAC_A + RX on RID 8, forward to VF0_VSI ARP +
> MAC_A + RX on RID 10, forward to PF0.
> 
> Only second one will match, but this is expected.
> 
> Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_switch.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

Hi,

I tried configuring two rules with same match parameters but with different priorities. One rule redirecting the PF traffic to VF_PR1 and other one to VF_PR2.

In this case, I notice that both the VFs are able to receive the same packet from the PF. Can you please confirm if this is expected?

Below are the rules (1 and 3) used.

[root@cbl-mariner ~]# tc filter show dev ens5f0np0 root
filter ingress protocol ip pref 1 flower chain 0 
filter ingress protocol ip pref 1 flower chain 0 handle 0x1 
  dst_mac 52:54:00:00:16:01
  src_mac b4:96:91:9f:65:58
  eth_type ipv4
  skip_sw
  in_hw in_hw_count 1
        action order 1: mirred (Egress Redirect to device eth0) stolen
        index 5 ref 1 bind 1

filter ingress protocol ip pref 1 flower chain 0 handle 0x2 
  dst_mac 52:54:00:00:16:02
  src_mac b4:96:91:9f:65:58
  eth_type ipv4
  skip_sw
  in_hw in_hw_count 1
        action order 1: mirred (Egress Redirect to device eth1) stolen
        index 6 ref 1 bind 1

filter ingress protocol ip pref 7 flower chain 0 
filter ingress protocol ip pref 7 flower chain 0 handle 0x1 
  dst_mac 52:54:00:00:16:01
  src_mac b4:96:91:9f:65:58
  eth_type ipv4
  skip_sw
  in_hw in_hw_count 1
        action order 1: mirred (Egress Redirect to device eth1) stolen
        index 7 ref 1 bind 1

Packet captures:
[root@cbl-mariner ~]# ip netns exec ns1 tcpdump -i ens5f0v0
dropped privs to tcpdump
tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
listening on ens5f0v0, link-type EN10MB (Ethernet), capture size 262144 bytes
15:21:21.428973 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 8001.18:5a:58:a3:1c:e0.8060, length 42
15:21:21.428986 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 8001.18:5a:58:a3:1c:e0.8060, length 43
15:21:21.429001 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83e8.18:5a:58:a3:1c:e0.8060, length 42
15:21:21.429012 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83e9.18:5a:58:a3:1c:e0.8060, length 42
15:21:21.429016 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83ea.18:5a:58:a3:1c:e0.8060, length 42
15:21:21.429029 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83eb.18:5a:58:a3:1c:e0.8060, length 42
15:21:21.429039 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 80c8.18:5a:58:a3:1c:e0.8060, length 42
15:21:21.944173 IP 1.1.1.100 > cbl-mariner: ICMP echo request, id 7, seq 4268, length 64
15:21:21.944182 IP cbl-mariner > 1.1.1.100: ICMP echo reply, id 7, seq 4268, length 64
^C
9 packets captured
9 packets received by filter
0 packets dropped by kernel

[root@cbl-mariner ~]# ip netns exec ns2 tcpdump -i ens5f0v1
dropped privs to tcpdump
tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
listening on ens5f0v1, link-type EN10MB (Ethernet), capture size 262144 bytes
15:21:21.429028 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83eb.18:5a:58:a3:1c:e0.8060, length 42
15:21:21.429040 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 80c8.18:5a:58:a3:1c:e0.8060, length 42
15:21:21.944170 IP 1.1.1.100 > 1.1.1.1: ICMP echo request, id 7, seq 4268, length 64
15:21:22.968162 IP 1.1.1.100 > 1.1.1.1: ICMP echo request, id 7, seq 4269, length 64
15:21:23.432386 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 8001.18:5a:58:a3:1c:e0.8060, length 42
15:21:23.432403 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 8001.18:5a:58:a3:1c:e0.8060, length 43
15:21:23.432430 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83e8.18:5a:58:a3:1c:e0.8060, length 42
15:21:23.432472 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83e9.18:5a:58:a3:1c:e0.8060, length 42
15:21:23.432508 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83ea.18:5a:58:a3:1c:e0.8060, length 42
15:21:23.432549 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83eb.18:5a:58:a3:1c:e0.8060, length 42
15:21:23.432588 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 80c8.18:5a:58:a3:1c:e0.8060, length 42
15:21:23.992156 IP 1.1.1.100 > 1.1.1.1: ICMP echo request, id 7, seq 4270, length 64

Regards,
Sujai B

Re: [Intel-wired-lan] [iwl-next v1] ice: add recipe priority check in search

[Michal Swiatkowski]

On Thu, Nov 07, 2024 at 12:06:26PM +0000, Buvaneswaran, Sujai wrote:
> > -----Original Message-----
> > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> > Michal Swiatkowski
> > Sent: Friday, October 11, 2024 12:33 PM
> > To: intel-wired-lan@lists.osuosl.org
> > Cc: netdev@vger.kernel.org; Szycik, Marcin <marcin.szycik@intel.com>;
> > Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>
> > Subject: [Intel-wired-lan] [iwl-next v1] ice: add recipe priority check in search
> > 
> > The new recipe should be added even if exactly the same recipe already
> > exists with different priority.
> > 
> > Example use case is when the rule is being added from TC tool context.
> > It should has the highest priority, but if the recipe already exists the rule will
> > inherit it priority. It can lead to the situation when the rule added from TC
> > tool has lower priority than expected.
> > 
> > The solution is to check the recipe priority when trying to find existing one.
> > 
> > Previous recipe is still useful. Example:
> > RID 8 -> priority 4
> > RID 10 -> priority 7
> > 
> > The difference is only in priority rest is let's say eth + mac + direction.
> > 
> > Adding ARP + MAC_A + RX on RID 8, forward to VF0_VSI After that IP +
> > MAC_B + RX on RID 10 (from TC tool), forward to PF0
> > 
> > Both will work.
> > 
> > In case of adding ARP + MAC_A + RX on RID 8, forward to VF0_VSI ARP +
> > MAC_A + RX on RID 10, forward to PF0.
> > 
> > Only second one will match, but this is expected.
> > 
> > Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > ---
> >  drivers/net/ethernet/intel/ice/ice_switch.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> 
> Hi,
> 
> I tried configuring two rules with same match parameters but with different priorities. One rule redirecting the PF traffic to VF_PR1 and other one to VF_PR2.
> 
> In this case, I notice that both the VFs are able to receive the same packet from the PF. Can you please confirm if this is expected?
> 
> Below are the rules (1 and 3) used.
> 
> [root@cbl-mariner ~]# tc filter show dev ens5f0np0 root
> filter ingress protocol ip pref 1 flower chain 0 
> filter ingress protocol ip pref 1 flower chain 0 handle 0x1 
>   dst_mac 52:54:00:00:16:01
>   src_mac b4:96:91:9f:65:58
>   eth_type ipv4
>   skip_sw
>   in_hw in_hw_count 1
>         action order 1: mirred (Egress Redirect to device eth0) stolen
>         index 5 ref 1 bind 1
> 
> filter ingress protocol ip pref 1 flower chain 0 handle 0x2 
>   dst_mac 52:54:00:00:16:02
>   src_mac b4:96:91:9f:65:58
>   eth_type ipv4
>   skip_sw
>   in_hw in_hw_count 1
>         action order 1: mirred (Egress Redirect to device eth1) stolen
>         index 6 ref 1 bind 1
> 
> filter ingress protocol ip pref 7 flower chain 0 
> filter ingress protocol ip pref 7 flower chain 0 handle 0x1 
>   dst_mac 52:54:00:00:16:01
>   src_mac b4:96:91:9f:65:58
>   eth_type ipv4
>   skip_sw
>   in_hw in_hw_count 1
>         action order 1: mirred (Egress Redirect to device eth1) stolen
>         index 7 ref 1 bind 1
> 
> Packet captures:
> [root@cbl-mariner ~]# ip netns exec ns1 tcpdump -i ens5f0v0
> dropped privs to tcpdump
> tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
> listening on ens5f0v0, link-type EN10MB (Ethernet), capture size 262144 bytes
> 15:21:21.428973 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 8001.18:5a:58:a3:1c:e0.8060, length 42
> 15:21:21.428986 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 8001.18:5a:58:a3:1c:e0.8060, length 43
> 15:21:21.429001 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83e8.18:5a:58:a3:1c:e0.8060, length 42
> 15:21:21.429012 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83e9.18:5a:58:a3:1c:e0.8060, length 42
> 15:21:21.429016 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83ea.18:5a:58:a3:1c:e0.8060, length 42
> 15:21:21.429029 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83eb.18:5a:58:a3:1c:e0.8060, length 42
> 15:21:21.429039 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 80c8.18:5a:58:a3:1c:e0.8060, length 42
> 15:21:21.944173 IP 1.1.1.100 > cbl-mariner: ICMP echo request, id 7, seq 4268, length 64
> 15:21:21.944182 IP cbl-mariner > 1.1.1.100: ICMP echo reply, id 7, seq 4268, length 64
> ^C
> 9 packets captured
> 9 packets received by filter
> 0 packets dropped by kernel
> 
> [root@cbl-mariner ~]# ip netns exec ns2 tcpdump -i ens5f0v1
> dropped privs to tcpdump
> tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
> listening on ens5f0v1, link-type EN10MB (Ethernet), capture size 262144 bytes
> 15:21:21.429028 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83eb.18:5a:58:a3:1c:e0.8060, length 42
> 15:21:21.429040 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 80c8.18:5a:58:a3:1c:e0.8060, length 42
> 15:21:21.944170 IP 1.1.1.100 > 1.1.1.1: ICMP echo request, id 7, seq 4268, length 64
> 15:21:22.968162 IP 1.1.1.100 > 1.1.1.1: ICMP echo request, id 7, seq 4269, length 64
> 15:21:23.432386 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 8001.18:5a:58:a3:1c:e0.8060, length 42
> 15:21:23.432403 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 8001.18:5a:58:a3:1c:e0.8060, length 43
> 15:21:23.432430 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83e8.18:5a:58:a3:1c:e0.8060, length 42
> 15:21:23.432472 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83e9.18:5a:58:a3:1c:e0.8060, length 42
> 15:21:23.432508 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83ea.18:5a:58:a3:1c:e0.8060, length 42
> 15:21:23.432549 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 83eb.18:5a:58:a3:1c:e0.8060, length 42
> 15:21:23.432588 STP 802.1w, Rapid STP, Flags [Proposal, Learn, Forward, Agreement], bridge-id 80c8.18:5a:58:a3:1c:e0.8060, length 42
> 15:21:23.992156 IP 1.1.1.100 > 1.1.1.1: ICMP echo request, id 7, seq 4270, length 64
> 

Hi,

Yes, it is expected. We don't support different priority from tc yet, so
no matter what priority from tc you will choose it will be programmed
with the same priority in hw.

Thanks,
Michal

> Regards,
> Sujai B

RE: [Intel-wired-lan] [iwl-next v1] ice: add recipe priority check in search

[Buvaneswaran, Sujai]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Michal Swiatkowski
> Sent: Friday, October 11, 2024 12:33 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Szycik, Marcin <marcin.szycik@intel.com>;
> Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>
> Subject: [Intel-wired-lan] [iwl-next v1] ice: add recipe priority check in search
> 
> The new recipe should be added even if exactly the same recipe already
> exists with different priority.
> 
> Example use case is when the rule is being added from TC tool context.
> It should has the highest priority, but if the recipe already exists the rule will
> inherit it priority. It can lead to the situation when the rule added from TC
> tool has lower priority than expected.
> 
> The solution is to check the recipe priority when trying to find existing one.
> 
> Previous recipe is still useful. Example:
> RID 8 -> priority 4
> RID 10 -> priority 7
> 
> The difference is only in priority rest is let's say eth + mac + direction.
> 
> Adding ARP + MAC_A + RX on RID 8, forward to VF0_VSI After that IP +
> MAC_B + RX on RID 10 (from TC tool), forward to PF0
> 
> Both will work.
> 
> In case of adding ARP + MAC_A + RX on RID 8, forward to VF0_VSI ARP +
> MAC_A + RX on RID 10, forward to PF0.
> 
> Only second one will match, but this is expected.
> 
> Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_switch.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
Tested-by: Sujai Buvaneswaran <sujai.buvaneswaran@intel.com>