[PATCH net-next v2 0/6] tc: adjust network header after 2nd vlan push

[PATCH net-next v2 0/6] tc: adjust network header after 2nd vlan push

[Boris Sukholitko]

<tldr>
skb network header of the single-tagged vlan packet continues to point the
vlan payload (e.g. IP) after second vlan tag is pushed by tc act_vlan. This
causes problem at the dissector which expects double-tagged packet network
header to point to the inner vlan.

The fix is to adjust network header in tcf_act_vlan.c but requires
refactoring of skb_vlan_push function.
</tldr>

Consider the following shell script snippet configuring TC rules on the
veth interface:

ip link add veth0 type veth peer veth1
ip link set veth0 up
ip link set veth1 up

tc qdisc add dev veth0 clsact

tc filter add dev veth0 ingress pref 10 chain 0 flower \
	num_of_vlans 2 cvlan_ethtype 0x800 action goto chain 5
tc filter add dev veth0 ingress pref 20 chain 0 flower \
	num_of_vlans 1 action vlan push id 100 \
	protocol 0x8100 action goto chain 5
tc filter add dev veth0 ingress pref 30 chain 5 flower \
	num_of_vlans 2 cvlan_ethtype 0x800 action simple sdata "success"

Sending double-tagged vlan packet with the IP payload inside:

cat <<ENDS | text2pcap - - | tcpreplay -i veth1 -
0000  00 00 00 00 00 11 00 00 00 00 00 22 81 00 00 64   ..........."...d
0010  81 00 00 14 08 00 45 04 00 26 04 d2 00 00 7f 11   ......E..&......
0020  18 ef 0a 00 00 01 14 00 00 02 00 00 00 00 00 12   ................
0030  e1 c7 00 00 00 00 00 00 00 00 00 00               ............
ENDS

will match rule 10, goto rule 30 in chain 5 and correctly emit "success" to
the dmesg.

OTOH, sending single-tagged vlan packet:

cat <<ENDS | text2pcap - - | tcpreplay -i veth1 -
0000  00 00 00 00 00 11 00 00 00 00 00 22 81 00 00 14   ..........."....
0010  08 00 45 04 00 2a 04 d2 00 00 7f 11 18 eb 0a 00   ..E..*..........
0020  00 01 14 00 00 02 00 00 00 00 00 16 e1 bf 00 00   ................
0030  00 00 00 00 00 00 00 00 00 00 00 00               ............
ENDS

will match rule 20, will push the second vlan tag but will *not* match
rule 30. IOW, the match at rule 30 fails if the second vlan was freshly
pushed by the kernel.

Lets look at  __skb_flow_dissect working on the double-tagged vlan packet.
Here is the relevant code from around net/core/flow_dissector.c:1277
copy-pasted here for convenience:

	if (dissector_vlan == FLOW_DISSECTOR_KEY_MAX &&
	    skb && skb_vlan_tag_present(skb)) {
		proto = skb->protocol;
	} else {
		vlan = __skb_header_pointer(skb, nhoff, sizeof(_vlan),
					    data, hlen, &_vlan);
		if (!vlan) {
			fdret = FLOW_DISSECT_RET_OUT_BAD;
			break;
		}

		proto = vlan->h_vlan_encapsulated_proto;
		nhoff += sizeof(*vlan);
	}

The "else" clause above gets the protocol of the encapsulated packet from
the skb data at the network header location. printk debugging has showed
that in the good double-tagged packet case proto is
htons(0x800 == ETH_P_IP) as expected. However in the single-tagged packet
case proto is garbage leading to the failure to match tc filter 30.

proto is being set from the skb header pointed by nhoff parameter which is
defined at the beginning of __skb_flow_dissect
(net/core/flow_dissector.c:1055 in the current version):

		nhoff = skb_network_offset(skb);

Therefore the culprit seems to be that the skb network offset is different
between double-tagged packet received from the interface and single-tagged
packet having its vlan tag pushed by TC.

Lets look at the interesting points of the lifetime of the single/double
tagged packets as they traverse our packet flow.

Both of them will start at __netif_receive_skb_core where the first vlan
tag will be stripped:

	if (eth_type_vlan(skb->protocol)) {
		skb = skb_vlan_untag(skb);
		if (unlikely(!skb))
			goto out;
	}

At this stage in double-tagged case skb->data points to the second vlan tag
while in single-tagged case skb->data points to the network (eg. IP)
header.

Looking at TC vlan push action (net/sched/act_vlan.c) we have the following
code at tcf_vlan_act (interesting points are in square brackets):

	if (skb_at_tc_ingress(skb))
[1]		skb_push_rcsum(skb, skb->mac_len);

	....

	case TCA_VLAN_ACT_PUSH:
		err = skb_vlan_push(skb, p->tcfv_push_proto, p->tcfv_push_vid |
				    (p->tcfv_push_prio << VLAN_PRIO_SHIFT),
				    0);
		if (err)
			goto drop;
		break;

	....

out:
	if (skb_at_tc_ingress(skb))
[3]		skb_pull_rcsum(skb, skb->mac_len);

And skb_vlan_push (net/core/skbuff.c:6204) function does:

		err = __vlan_insert_tag(skb, skb->vlan_proto,
					skb_vlan_tag_get(skb));
		if (err)
			return err;

		skb->protocol = skb->vlan_proto;
[2]		skb->mac_len += VLAN_HLEN;

in the case of pushing the second tag. Lets look at what happens with
skb->data of the single-tagged packet at each of the above points:

1. As a result of the skb_push_rcsum, skb->data is moved back to the start
   of the packet.

2. First VLAN tag is moved from the skb into packet buffer, skb->mac_len is
   incremented, skb->data still points to the start of the packet.

3. As a result of the skb_pull_rcsum, skb->data is moved forward by the
   modified skb->mac_len, thus pointing to the network header again.

Then __skb_flow_dissect will get confused by having double-tagged vlan
packet with the skb->data at the network header.

Our solution for the bug is to preserve "skb->data at second vlan header"
semantics in the act_vlan.c. We do this by manipulating skb->network_header
rather than skb->mac_len in act_vlan.c. skb_reset_mac_len is done at the
end.

More about the patch series:

* patches 1-3 refactor skb_vlan_push to make skb_vlan_flush helper
* patch 4 open codes skb_vlan_push in act_vlan.c
* patch 5 contains the actual fix
* patch 6 adds test to tc_action.sh selftests

Thanks,
Boris.

v2:
    - add test to tc_actions.sh

Boris Sukholitko (6):
  skb: add skb_vlan_flush helper
  skb: move mac_len adjustment out of skb_vlan_flush
  skb: export skb_vlan_flush
  act_vlan: open code skb_vlan_push
  act_vlan: adjust network header
  selftests: forwarding: tc_actions: test vlan flush

 include/linux/skbuff.h                        |  1 +
 net/core/skbuff.c                             | 38 ++++++++++++-------
 net/sched/act_vlan.c                          | 12 ++++--
 .../selftests/net/forwarding/tc_actions.sh    | 22 ++++++++++-
 4 files changed, 55 insertions(+), 18 deletions(-)

-- 
2.42.0

[PATCH net-next v2 1/6] skb: add skb_vlan_flush helper

[Boris Sukholitko]

Refactor flushing of the inner vlan in the skb_vlan_push by moving the code
into static skb_vlan_flush helper.

Signed-off-by: Boris Sukholitko <boris.sukholitko@broadcom.com>
---
 net/core/skbuff.c | 40 ++++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 83f8cd8aa2d1..23f0db1db048 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -6220,30 +6220,38 @@ int skb_vlan_pop(struct sk_buff *skb)
 }
 EXPORT_SYMBOL(skb_vlan_pop);
 
+static int skb_vlan_flush(struct sk_buff *skb)
+{
+	int offset = skb->data - skb_mac_header(skb);
+	int err;
+
+	if (WARN_ONCE(offset,
+		      "skb_vlan_push got skb with skb->data not at mac header (offset %d)\n",
+		      offset)) {
+		return -EINVAL;
+	}
+
+	err = __vlan_insert_tag(skb, skb->vlan_proto,
+				skb_vlan_tag_get(skb));
+	if (err)
+		return err;
+
+	skb->protocol = skb->vlan_proto;
+	skb->mac_len += VLAN_HLEN;
+
+	skb_postpush_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
+	return 0;
+}
+
 /* Push a vlan tag either into hwaccel or into payload (if hwaccel tag present).
  * Expects skb->data at mac header.
  */
 int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
 {
 	if (skb_vlan_tag_present(skb)) {
-		int offset = skb->data - skb_mac_header(skb);
-		int err;
-
-		if (WARN_ONCE(offset,
-			      "skb_vlan_push got skb with skb->data not at mac header (offset %d)\n",
-			      offset)) {
-			return -EINVAL;
-		}
-
-		err = __vlan_insert_tag(skb, skb->vlan_proto,
-					skb_vlan_tag_get(skb));
+		int err = skb_vlan_flush(skb);
 		if (err)
 			return err;
-
-		skb->protocol = skb->vlan_proto;
-		skb->mac_len += VLAN_HLEN;
-
-		skb_postpush_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
 	}
 	__vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
 	return 0;
-- 
2.42.0

[PATCH net-next v2 2/6] skb: move mac_len adjustment out of skb_vlan_flush

[Boris Sukholitko]

Let its callers worry about skb headers adjustment.

Signed-off-by: Boris Sukholitko <boris.sukholitko@broadcom.com>
---
 net/core/skbuff.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 23f0db1db048..1bd817c8ddc8 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -6237,7 +6237,6 @@ static int skb_vlan_flush(struct sk_buff *skb)
 		return err;
 
 	skb->protocol = skb->vlan_proto;
-	skb->mac_len += VLAN_HLEN;
 
 	skb_postpush_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
 	return 0;
@@ -6252,6 +6251,8 @@ int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
 		int err = skb_vlan_flush(skb);
 		if (err)
 			return err;
+
+		skb->mac_len += VLAN_HLEN;
 	}
 	__vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
 	return 0;
-- 
2.42.0

[PATCH net-next v2 3/6] skb: export skb_vlan_flush

[Boris Sukholitko]

Make skb_vlan_flush callable by other customers of skbuff.

Signed-off-by: Boris Sukholitko <boris.sukholitko@broadcom.com>
---
 include/linux/skbuff.h | 1 +
 net/core/skbuff.c      | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index cf8f6ce06742..5a9f06691c80 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4054,6 +4054,7 @@ int skb_ensure_writable(struct sk_buff *skb, unsigned int write_len);
 int skb_ensure_writable_head_tail(struct sk_buff *skb, struct net_device *dev);
 int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci);
 int skb_vlan_pop(struct sk_buff *skb);
+int skb_vlan_flush(struct sk_buff *skb);
 int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci);
 int skb_eth_pop(struct sk_buff *skb);
 int skb_eth_push(struct sk_buff *skb, const unsigned char *dst,
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 1bd817c8ddc8..e28b2c8b717d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -6220,7 +6220,7 @@ int skb_vlan_pop(struct sk_buff *skb)
 }
 EXPORT_SYMBOL(skb_vlan_pop);
 
-static int skb_vlan_flush(struct sk_buff *skb)
+int skb_vlan_flush(struct sk_buff *skb)
 {
 	int offset = skb->data - skb_mac_header(skb);
 	int err;
@@ -6241,6 +6241,7 @@ static int skb_vlan_flush(struct sk_buff *skb)
 	skb_postpush_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
 	return 0;
 }
+EXPORT_SYMBOL(skb_vlan_flush);
 
 /* Push a vlan tag either into hwaccel or into payload (if hwaccel tag present).
  * Expects skb->data at mac header.
-- 
2.42.0

[PATCH net-next v2 4/6] act_vlan: open code skb_vlan_push

[Boris Sukholitko]

Prepare to do act_vlan specific network header adjustment by
copy-pasting several lines of skb_vlan_push code.

Signed-off-by: Boris Sukholitko <boris.sukholitko@broadcom.com>
---
 net/sched/act_vlan.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 22f4b1e8ade9..84b79096df2a 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -49,10 +49,15 @@ TC_INDIRECT_SCOPE int tcf_vlan_act(struct sk_buff *skb,
 			goto drop;
 		break;
 	case TCA_VLAN_ACT_PUSH:
-		err = skb_vlan_push(skb, p->tcfv_push_proto, p->tcfv_push_vid |
+		if (skb_vlan_tag_present(skb)) {
+			err = skb_vlan_flush(skb);
+			if (err)
+				goto drop;
+
+			skb->mac_len += VLAN_HLEN;
+		}
+		__vlan_hwaccel_put_tag(skb, p->tcfv_push_proto, p->tcfv_push_vid |
 				    (p->tcfv_push_prio << VLAN_PRIO_SHIFT));
-		if (err)
-			goto drop;
 		break;
 	case TCA_VLAN_ACT_MODIFY:
 		/* No-op if no vlan tag (either hw-accel or in-payload) */
-- 
2.42.0

[PATCH net-next v2 5/6] act_vlan: adjust network header

[Boris Sukholitko]

Consider the following shell script snippet configuring TC rules on the
veth interface:

ip link add veth0 type veth peer veth1
ip link set veth0 up
ip link set veth1 up

tc qdisc add dev veth0 clsact

tc filter add dev veth0 ingress pref 10 chain 0 flower \
	num_of_vlans 2 cvlan_ethtype 0x800 action goto chain 5
tc filter add dev veth0 ingress pref 20 chain 0 flower \
	num_of_vlans 1 action vlan push id 100 \
	protocol 0x8100 action goto chain 5
tc filter add dev veth0 ingress pref 30 chain 5 flower \
	num_of_vlans 2 cvlan_ethtype 0x800 action simple sdata "success"

Sending double-tagged vlan packet with the IP payload inside:

cat <<ENDS | text2pcap - - | tcpreplay -i veth1 -
0000  00 00 00 00 00 11 00 00 00 00 00 22 81 00 00 64   ..........."...d
0010  81 00 00 14 08 00 45 04 00 26 04 d2 00 00 7f 11   ......E..&......
0020  18 ef 0a 00 00 01 14 00 00 02 00 00 00 00 00 12   ................
0030  e1 c7 00 00 00 00 00 00 00 00 00 00               ............
ENDS

will match rule 10, goto rule 30 in chain 5 and correctly emit "success" to
the dmesg.

OTOH, sending single-tagged vlan packet:

cat <<ENDS | text2pcap - - | tcpreplay -i veth1 -
0000  00 00 00 00 00 11 00 00 00 00 00 22 81 00 00 14   ..........."....
0010  08 00 45 04 00 2a 04 d2 00 00 7f 11 18 eb 0a 00   ..E..*..........
0020  00 01 14 00 00 02 00 00 00 00 00 16 e1 bf 00 00   ................
0030  00 00 00 00 00 00 00 00 00 00 00 00               ............
ENDS

will match rule 20, will push the second vlan tag but will *not* match
rule 30. IOW, the match at rule 30 fails if the second vlan was freshly
pushed by the kernel.

Lets look at  __skb_flow_dissect working on the double-tagged vlan packet.
Here is the relevant code from around net/core/flow_dissector.c:1277
copy-pasted here for convenience:

	if (dissector_vlan == FLOW_DISSECTOR_KEY_MAX &&
	    skb && skb_vlan_tag_present(skb)) {
		proto = skb->protocol;
	} else {
		vlan = __skb_header_pointer(skb, nhoff, sizeof(_vlan),
					    data, hlen, &_vlan);
		if (!vlan) {
			fdret = FLOW_DISSECT_RET_OUT_BAD;
			break;
		}

		proto = vlan->h_vlan_encapsulated_proto;
		nhoff += sizeof(*vlan);
	}

The "else" clause above gets the protocol of the encapsulated packet from
the skb data at the network header location. printk debugging has showed
that in the good double-tagged packet case proto is
htons(0x800 == ETH_P_IP) as expected. However in the single-tagged packet
case proto is garbage leading to the failure to match tc filter 30.

proto is being set from the skb header pointed by nhoff parameter which is
defined at the beginning of __skb_flow_dissect
(net/core/flow_dissector.c:1055 in the current version):

		nhoff = skb_network_offset(skb);

Therefore the culprit seems to be that the skb network offset is different
between double-tagged packet received from the interface and single-tagged
packet having its vlan tag pushed by TC.

Lets look at the interesting points of the lifetime of the single/double
tagged packets as they traverse our packet flow.

Both of them will start at __netif_receive_skb_core where the first vlan
tag will be stripped:

	if (eth_type_vlan(skb->protocol)) {
		skb = skb_vlan_untag(skb);
		if (unlikely(!skb))
			goto out;
	}

At this stage in double-tagged case skb->data points to the second vlan tag
while in single-tagged case skb->data points to the network (eg. IP)
header.

Looking at TC vlan push action (net/sched/act_vlan.c) we have the following
code at tcf_vlan_act (interesting points are in square brackets):

	if (skb_at_tc_ingress(skb))
[1]		skb_push_rcsum(skb, skb->mac_len);

	....

	case TCA_VLAN_ACT_PUSH:
		if (skb_vlan_tag_present(skb)) {
			int err = skb_vlan_flush(skb);
			if (err)
				goto drop;

[2]			skb->mac_len += VLAN_HLEN;
		}
		break;

	....

out:
	if (skb_at_tc_ingress(skb))
[3]		skb_pull_rcsum(skb, skb->mac_len);

Lets look at what happens with skb->data of the single-tagged packet at
each of the above points:

1. As a result of the skb_push_rcsum, skb->data is moved back to the start
   of the packet.

2. First VLAN tag is moved from the skb into packet buffer, skb->mac_len is
   incremented, skb->data still points to the start of the packet.

3. As a result of the skb_pull_rcsum, skb->data is moved forward by the
   modified skb->mac_len, thus pointing to the network header again.

Then __skb_flow_dissect will get confused by having double-tagged vlan
packet with the skb->data at the network header.

The same bug happens also on the egress path. The root cause there is that
skb->network_header is in disagreement with skb->mac_len.

This patch fixes both of the above problems by adjusting
skb->network_header rather than skb->mac_vlan. skb->mac_vlan is being reset
accordingly after skb_pull_rcsum is done.

Signed-off-by: Boris Sukholitko <boris.sukholitko@broadcom.com>
---
 net/sched/act_vlan.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 84b79096df2a..c113f8026f1f 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -54,7 +54,7 @@ TC_INDIRECT_SCOPE int tcf_vlan_act(struct sk_buff *skb,
 			if (err)
 				goto drop;
 
-			skb->mac_len += VLAN_HLEN;
+			skb->network_header -= VLAN_HLEN;
 		}
 		__vlan_hwaccel_put_tag(skb, p->tcfv_push_proto, p->tcfv_push_vid |
 				    (p->tcfv_push_prio << VLAN_PRIO_SHIFT));
@@ -101,6 +101,7 @@ TC_INDIRECT_SCOPE int tcf_vlan_act(struct sk_buff *skb,
 	if (skb_at_tc_ingress(skb))
 		skb_pull_rcsum(skb, skb->mac_len);
 
+	skb_reset_mac_len(skb);
 	return action;
 
 drop:
-- 
2.42.0

[PATCH net-next v2 6/6] selftests: forwarding: tc_actions: test vlan flush

[Boris Sukholitko]

Add new test checking the correctness of inner vlan flushing to the skb
data when outer vlan tag is added through act_vlan.

Signed-off-by: Boris Sukholitko <boris.sukholitko@broadcom.com>
---
 .../selftests/net/forwarding/tc_actions.sh    | 22 ++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/forwarding/tc_actions.sh b/tools/testing/selftests/net/forwarding/tc_actions.sh
index 589629636502..65ff80d66b17 100755
--- a/tools/testing/selftests/net/forwarding/tc_actions.sh
+++ b/tools/testing/selftests/net/forwarding/tc_actions.sh
@@ -4,7 +4,7 @@
 ALL_TESTS="gact_drop_and_ok_test mirred_egress_redirect_test \
 	mirred_egress_mirror_test matchall_mirred_egress_mirror_test \
 	gact_trap_test mirred_egress_to_ingress_test \
-	mirred_egress_to_ingress_tcp_test"
+	mirred_egress_to_ingress_tcp_test vlan_flush_test"
 NUM_NETIFS=4
 source tc_common.sh
 source lib.sh
@@ -244,6 +244,26 @@ mirred_egress_to_ingress_tcp_test()
 	log_test "mirred_egress_to_ingress_tcp ($tcflags)"
 }
 
+vlan_flush_test()
+{
+	ip link add x$h1 type veth peer x$h2
+	ip link set x$h1 up
+	ip link set x$h2 up
+
+	tc qdisc add dev x$h1 clsact
+	tc filter add dev x$h1 ingress pref 20 chain 0 handle 20 flower num_of_vlans 1 \
+		action vlan push id 100 protocol 0x8100 action goto chain 5
+	tc filter add dev x$h1 ingress pref 30 chain 5 handle 30 flower num_of_vlans 2 \
+		cvlan_ethtype 0x800 action pass
+
+	$MZ x$h2 -t udp -Q 10 -q
+	tc_check_packets "dev x$h1 ingress" 30 1
+	check_err $? "No double-vlan packets received"
+
+	ip link del x$h1
+	log_test "vlan_flush_test ($tcflags)"
+}
+
 setup_prepare()
 {
 	h1=${NETIFS[p1]}
-- 
2.42.0

Re: [PATCH net-next v2 0/6] tc: adjust network header after 2nd vlan push

[Jakub Kicinski]

On Wed, 14 Aug 2024 16:06:12 +0300 Boris Sukholitko wrote:
> v2:
>     - add test to tc_actions.sh

Discussion on v1 has not concluded, you'll have to repost once it does.
-- 
pw-bot: cr

Re: [PATCH net-next v2 6/6] selftests: forwarding: tc_actions: test vlan flush

[Ido Schimmel]

On Wed, Aug 14, 2024 at 04:06:18PM +0300, Boris Sukholitko wrote:
> Add new test checking the correctness of inner vlan flushing to the skb
> data when outer vlan tag is added through act_vlan.
> 
> Signed-off-by: Boris Sukholitko <boris.sukholitko@broadcom.com>
> ---
>  .../selftests/net/forwarding/tc_actions.sh    | 22 ++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/net/forwarding/tc_actions.sh b/tools/testing/selftests/net/forwarding/tc_actions.sh
> index 589629636502..65ff80d66b17 100755
> --- a/tools/testing/selftests/net/forwarding/tc_actions.sh
> +++ b/tools/testing/selftests/net/forwarding/tc_actions.sh
> @@ -4,7 +4,7 @@
>  ALL_TESTS="gact_drop_and_ok_test mirred_egress_redirect_test \
>  	mirred_egress_mirror_test matchall_mirred_egress_mirror_test \
>  	gact_trap_test mirred_egress_to_ingress_test \
> -	mirred_egress_to_ingress_tcp_test"
> +	mirred_egress_to_ingress_tcp_test vlan_flush_test"
>  NUM_NETIFS=4
>  source tc_common.sh
>  source lib.sh
> @@ -244,6 +244,26 @@ mirred_egress_to_ingress_tcp_test()
>  	log_test "mirred_egress_to_ingress_tcp ($tcflags)"
>  }
>  
> +vlan_flush_test()
> +{
> +	ip link add x$h1 type veth peer x$h2
> +	ip link set x$h1 up
> +	ip link set x$h2 up

The test already creates the needed topology, there is no need to create
more interfaces. You can use $h1 and $swp1 which are either a veth pair
(default if you didn't configure anything) or two connected physical
ports.

> +
> +	tc qdisc add dev x$h1 clsact
> +	tc filter add dev x$h1 ingress pref 20 chain 0 handle 20 flower num_of_vlans 1 \

Please use $tcflags like other test cases.

> +		action vlan push id 100 protocol 0x8100 action goto chain 5
> +	tc filter add dev x$h1 ingress pref 30 chain 5 handle 30 flower num_of_vlans 2 \

The cover letter says that the bug also exists on egress so I suggest
checking that as well to avoid future regressions.

> +		cvlan_ethtype 0x800 action pass
> +
> +	$MZ x$h2 -t udp -Q 10 -q

For consistency, please invoke $MZ with similar parameters to other test
cases.

> +	tc_check_packets "dev x$h1 ingress" 30 1
> +	check_err $? "No double-vlan packets received"
> +
> +	ip link del x$h1
> +	log_test "vlan_flush_test ($tcflags)"
> +}
> +
>  setup_prepare()
>  {
>  	h1=${NETIFS[p1]}
> -- 
> 2.42.0
> 
>