Message ID | 20241011070328.45874-1-michal.swiatkowski@linux.intel.com |
---|---|
State | Under Review |
Delegated to: | Anthony Nguyen |
Headers | show |
Series | [iwl-next,v1] ice: add recipe priority check in search | expand |
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>
> -----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
> -----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
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
> -----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>
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 */ } }