net: fix the mirred packet drop due to the incorrect dst

This issue was found when using OVS HWOL on OVN-k8s. These packets dropped on rx path were seen with output dst, which should've been dropped from the skbs when redirecting them.

The 1st patch is to the fix and the 2nd is a selftest to reproduce and verify it.

Davide Caratti (1):
selftests: add a test case for mirred egress to ingress

Xin Long (1):
net: sched: act_mirred: drop dst for the direction from egress to
ingress

 net/sched/act_mirred.c                        | 11 +++--
 tools/testing/selftests/net/forwarding/config |  1 +
 .../selftests/net/forwarding/tc_actions.sh    | 47 ++++++++++++++++++-
 3 files changed, 55 insertions(+), 4 deletions(-)

2.27.0

Without dropping dst, the packets sent from local mirred/redirected to ingress will may still use the old dst. ip_rcv() will drop it as the old dst is for output and its .input is dst_discard.

This patch is to fix by also dropping dst for those packets that are mirred or redirected from egress to ingress in act_mirred.

Note that we don't drop it for the direction change from ingress to egress, as on which there might be a user case attaching a metadata dst by act_tunnel_key that would be used later.

---
 net/sched/act_mirred.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

index d64b0eeccbe4..efc963ab995a 100644
@@ -19,6 +19,7 @@
 #include <linux/if_arp.h>
 #include <net/net_namespace.h>
 #include <net/netlink.h>
+#include <net/dst.h>
 #include <net/pkt_sched.h>
 #include <net/pkt_cls.h>
 #include <linux/tc_act/tc_mirred.h>

@@ -228,6 +229,7 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
 	bool want_ingress;
 	bool is_redirect;
 	bool expects_nh;
+	bool at_ingress;
 	int m_eaction;
 	int mac_len;
 	bool at_nh;

@@ -263,7 +265,8 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
 	 * ingress - that covers the TC S/W datapath.
 	 */
 	is_redirect = tcf_mirred_is_act_redirect(m_eaction);
-	use_reinsert = skb_at_tc_ingress(skb) && is_redirect &&
+	at_ingress = skb_at_tc_ingress(skb);
+	use_reinsert = at_ingress && is_redirect &&
 		       tcf_mirred_can_reinsert(retval);
 	if (!use_reinsert) {
 		skb2 = skb_clone(skb, GFP_ATOMIC);

@@ -271,10 +274,12 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
 			goto out;
 	}
 
+	want_ingress = tcf_mirred_act_wants_ingress(m_eaction);
+
 	/* All mirred/redirected skbs should clear previous ct info */
 	nf_reset_ct(skb2);
-
-	want_ingress = tcf_mirred_act_wants_ingress(m_eaction);
+	if (want_ingress && !at_ingress) /* drop dst for egress -> ingress */
+		skb_dst_drop(skb2);
 
 	expects_nh = want_ingress || !m_mac_header_xmit;
 	at_nh = skb->data == skb_network_header(skb);

2.27.0

From: Davide Caratti <[email protected]>

add a selftest that verifies the correct behavior of TC act_mirred egress to ingress: in particular, it checks if the dst_entry is removed from skb before redirect egress -> ingress. The correct behavior is: an ICMP 'echo request' generated by ping will be received and generate a reply the same way as the one generated by mausezahn.

---
 tools/testing/selftests/net/forwarding/config |  1 +
 .../selftests/net/forwarding/tc_actions.sh    | 47 ++++++++++++++++++-
 2 files changed, 47 insertions(+), 1 deletion(-)

index a4bd1b087303..697994a9278b 100644
@@ -6,6 +6,7 @@ CONFIG_IPV6_MULTIPLE_TABLES=y
 CONFIG_NET_VRF=m
 CONFIG_BPF_SYSCALL=y
 CONFIG_CGROUP_BPF=y
+CONFIG_NET_ACT_CT=m
 CONFIG_NET_ACT_MIRRED=m
 CONFIG_NET_ACT_MPLS=m
 CONFIG_NET_ACT_VLAN=m

2.27.0

+Cc Davide.

Thanks.

Applied, thanks!