[PATCH net-next v2 0/4] net/sched: act_tunnel_key: add support for TUNNEL_DONT_FRAGMENT

[PATCH net-next v2 0/4] net/sched: act_tunnel_key: add support for TUNNEL_DONT_FRAGMENT

[Davide Caratti]

- patch 1 extends TC tunnel_key action to add support for TUNNEL_DONT_FRAGMENT 
- patch 2 extends tdc to skip tests when iproute2 support is missing
- patch 3 adds a tdc test case to verify functionality of the control plane
- patch 4 adds a net/forwarding test case to verify functionality of the data plane

v1->v2:
 - split patchset to have test cases in separate patches (thanks to
   Pedro Tammela)
 - change TCA_TUNNEL_KEY_NO_FRAG attribute, so the it is a flag istead
   of u8 (thanks to Jakub Kicinski)
 - add missing TEST_PROGS assignment (thanks to Jakub Kicinski)

Davide Caratti (4):
  net/sched: act_tunnel_key: add support for "don't fragment"
  selftests: tc-testing: extend the "skip" property
  selftests: tc-testing: add tunnel_key "nofrag" test case
  selftests: forwarding: add tunnel_key "nofrag" test case

 include/uapi/linux/tc_act/tc_tunnel_key.h     |   1 +
 net/sched/act_tunnel_key.c                    |   5 +
 .../testing/selftests/net/forwarding/Makefile |   1 +
 .../selftests/net/forwarding/tc_tunnel_key.sh | 161 ++++++++++++++++++
 .../creating-testcases/AddingTestCases.txt    |   4 +-
 .../tc-tests/actions/tunnel_key.json          |  25 +++
 tools/testing/selftests/tc-testing/tdc.py     |  21 ++-
 7 files changed, 211 insertions(+), 7 deletions(-)
 create mode 100755 tools/testing/selftests/net/forwarding/tc_tunnel_key.sh

-- 
2.39.2

[PATCH net-next v2 1/4] net/sched: act_tunnel_key: add support for "don't fragment"

[Davide Caratti]

extend "act_tunnel_key" to allow specifying TUNNEL_DONT_FRAGMENT.

Suggested-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 include/uapi/linux/tc_act/tc_tunnel_key.h | 1 +
 net/sched/act_tunnel_key.c                | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/include/uapi/linux/tc_act/tc_tunnel_key.h b/include/uapi/linux/tc_act/tc_tunnel_key.h
index 49ad4033951b..37c6f612f161 100644
--- a/include/uapi/linux/tc_act/tc_tunnel_key.h
+++ b/include/uapi/linux/tc_act/tc_tunnel_key.h
@@ -34,6 +34,7 @@ enum {
 					 */
 	TCA_TUNNEL_KEY_ENC_TOS,		/* u8 */
 	TCA_TUNNEL_KEY_ENC_TTL,		/* u8 */
+	TCA_TUNNEL_KEY_NO_FRAG,		/* flag */
 	__TCA_TUNNEL_KEY_MAX,
 };
 
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 2d12d2626415..5f852631343f 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -420,6 +420,9 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
 		    nla_get_u8(tb[TCA_TUNNEL_KEY_NO_CSUM]))
 			flags &= ~TUNNEL_CSUM;
 
+		if (nla_get_flag(tb[TCA_TUNNEL_KEY_NO_FRAG]))
+			flags |= TUNNEL_DONT_FRAGMENT;
+
 		if (tb[TCA_TUNNEL_KEY_ENC_DST_PORT])
 			dst_port = nla_get_be16(tb[TCA_TUNNEL_KEY_ENC_DST_PORT]);
 
@@ -747,6 +750,8 @@ static int tunnel_key_dump(struct sk_buff *skb, struct tc_action *a,
 				   key->tp_dst)) ||
 		    nla_put_u8(skb, TCA_TUNNEL_KEY_NO_CSUM,
 			       !(key->tun_flags & TUNNEL_CSUM)) ||
+		    ((key->tun_flags & TUNNEL_DONT_FRAGMENT) &&
+				nla_put_flag(skb, TCA_TUNNEL_KEY_NO_FRAG)) ||
 		    tunnel_key_opts_dump(skb, info))
 			goto nla_put_failure;
 
-- 
2.39.2

[PATCH net-next v2 2/4] selftests: tc-testing: extend the "skip" property

[Davide Caratti]

currently, users can skip individual test cases by means of writing

  "skip": "yes"

in the scenario file. Extend this functionality by allowing the execution
of a command, written in the "skip" property for a specific test case. If
such property is present, tdc executes that command and skips the test if
the return value is non-zero.

Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 .../creating-testcases/AddingTestCases.txt    |  4 +++-
 tools/testing/selftests/tc-testing/tdc.py     | 21 +++++++++++++------
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/tc-testing/creating-testcases/AddingTestCases.txt b/tools/testing/selftests/tc-testing/creating-testcases/AddingTestCases.txt
index a28571aff0e1..130c49ef8576 100644
--- a/tools/testing/selftests/tc-testing/creating-testcases/AddingTestCases.txt
+++ b/tools/testing/selftests/tc-testing/creating-testcases/AddingTestCases.txt
@@ -37,7 +37,9 @@ skip:         A completely optional key, if the corresponding value is "yes"
               then tdc will not execute the test case in question. However,
               this test case will still appear in the results output but
               marked as skipped. This key can be placed anywhere inside the
-              test case at the top level.
+              test case at the top level. It's possible to specify a command
+              in the value of "skip": in this case, the test is skipped when
+              the return value is not zero.
 category:     A list of single-word descriptions covering what the command
               under test is testing. Example: filter, actions, u32, gact, etc.
 setup:        The list of commands required to ensure the command under test
diff --git a/tools/testing/selftests/tc-testing/tdc.py b/tools/testing/selftests/tc-testing/tdc.py
index 7bd94f8e490a..cc355ead1ff0 100755
--- a/tools/testing/selftests/tc-testing/tdc.py
+++ b/tools/testing/selftests/tc-testing/tdc.py
@@ -361,13 +361,22 @@ def run_one_test(pm, args, index, tidx):
     print("Test " + tidx["id"] + ": " + tidx["name"])
 
     if 'skip' in tidx:
+        if (args.verbose > 0):
+            print('probe command for test skip')
         if tidx['skip'] == 'yes':
-            res = TestResult(tidx['id'], tidx['name'])
-            res.set_result(ResultState.skip)
-            res.set_errormsg('Test case designated as skipped.')
-            pm.call_pre_case(tidx, test_skip=True)
-            pm.call_post_execute()
-            return res
+            # 'yes' would block forever: preserve existing skipped test
+            #  replacing 'yes' with 'false'
+            (p, procout) = exec_cmd(args, pm, 'execute', '/bin/false')
+        else:
+            (p, procout) = exec_cmd(args, pm, 'execute', tidx['skip'])
+        if p:
+            if (p.returncode != 0):
+                res = TestResult(tidx['id'], tidx['name'])
+                res.set_result(ResultState.skip)
+                res.set_errormsg('probe command failed: test skipped.')
+                pm.call_pre_case(tidx, test_skip=True)
+                pm.call_post_execute()
+                return res
 
     # populate NAMES with TESTID for this test
     NAMES['TESTID'] = tidx['id']
-- 
2.39.2

[PATCH net-next v2 3/4] selftests: tc-testing: add tunnel_key "nofrag" test case

[Davide Caratti]

 # ./tdc.py -e 6bda -l
 6bda: (actions, tunnel_key) Add tunnel_key action with nofrag option

Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 .../tc-tests/actions/tunnel_key.json          | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json b/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
index b40ee602918a..1ae51eadc477 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
@@ -983,5 +983,30 @@
         "teardown": [
             "$TC actions flush action tunnel_key"
         ]
+    },
+    {
+        "id": "6bda",
+        "name": "Add tunnel_key action with nofrag option",
+        "category": [
+            "actions",
+            "tunnel_key"
+        ],
+        "skip": "$TC actions add action tunnel_key help 2>&1 | grep -q nofrag",
+        "setup": [
+            [
+                "$TC action flush action tunnel_key",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action tunnel_key set src_ip 10.10.10.1 dst_ip 10.10.10.2 id 1111 nofrag index 222",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action tunnel_key index 222",
+        "matchPattern": "action order [0-9]+: tunnel_key.*src_ip 10.10.10.1.*dst_ip 10.10.10.2.*key_id 1111.*csum.*nofrag pipe.*index 222",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action tunnel_key"
+        ]
     }
 ]
-- 
2.39.2

[PATCH net-next v2 4/4] selftests: forwarding: add tunnel_key "nofrag" test case

[Davide Caratti]

Add a selftest that configures metadata tunnel encapsulation using the TC
"tunnel_key" action: it includes a test case for setting "nofrag" flag.

Example output:

 # selftests: net/forwarding: tc_tunnel_key.sh
 # TEST: tunnel_key nofrag (skip_hw)                                   [ OK ]
 # INFO: Could not test offloaded functionality
 ok 1 selftests: net/forwarding: tc_tunnel_key.sh

Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 .../testing/selftests/net/forwarding/Makefile |   1 +
 .../selftests/net/forwarding/tc_tunnel_key.sh | 161 ++++++++++++++++++
 2 files changed, 162 insertions(+)
 create mode 100755 tools/testing/selftests/net/forwarding/tc_tunnel_key.sh

diff --git a/tools/testing/selftests/net/forwarding/Makefile b/tools/testing/selftests/net/forwarding/Makefile
index 91201ab3c4fc..236f6b796a52 100644
--- a/tools/testing/selftests/net/forwarding/Makefile
+++ b/tools/testing/selftests/net/forwarding/Makefile
@@ -85,6 +85,7 @@ TEST_PROGS = bridge_igmp.sh \
 	tc_mpls_l2vpn.sh \
 	tc_police.sh \
 	tc_shblocks.sh \
+	tc_tunnel_key.sh \
 	tc_vlan_modify.sh \
 	vxlan_asymmetric_ipv6.sh \
 	vxlan_asymmetric.sh \
diff --git a/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh b/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh
new file mode 100755
index 000000000000..5ac184d51809
--- /dev/null
+++ b/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh
@@ -0,0 +1,161 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Kselftest framework requirement - SKIP code is 4.
+ksft_skip=4
+
+ALL_TESTS="tunnel_key_nofrag_test"
+
+NUM_NETIFS=4
+source tc_common.sh
+source lib.sh
+
+tcflags="skip_hw"
+
+h1_create()
+{
+	simple_if_init $h1 192.0.2.1/24
+	forwarding_enable
+	mtu_set $h1 1500
+	tunnel_create h1-et vxlan 192.0.2.1 192.0.2.2 dev $h1 dstport 0 external
+	tc qdisc add dev h1-et clsact
+	mtu_set h1-et 1230
+	mtu_restore $h1
+	mtu_set $h1 1000
+}
+
+h1_destroy()
+{
+	tc qdisc del dev h1-et clsact
+	tunnel_destroy h1-et
+	forwarding_restore
+	mtu_restore $h1
+	simple_if_fini $h1 192.0.2.1/24
+}
+
+h2_create()
+{
+	simple_if_init $h2 192.0.2.2/24
+}
+
+h2_destroy()
+{
+	simple_if_fini $h2 192.0.2.2/24
+}
+
+switch_create()
+{
+	simple_if_init $swp1 192.0.2.2/24
+	tc qdisc add dev $swp1 clsact
+	simple_if_init $swp2 192.0.2.1/24
+}
+
+switch_destroy()
+{
+	simple_if_fini $swp2 192.0.2.1/24
+	tc qdisc del dev $swp1 clsact
+	simple_if_fini $swp1 192.0.2.2/24
+}
+
+setup_prepare()
+{
+	h1=${NETIFS[p1]}
+	swp1=${NETIFS[p2]}
+
+	swp2=${NETIFS[p3]}
+	h2=${NETIFS[p4]}
+
+	h1mac=$(mac_get $h1)
+	h2mac=$(mac_get $h2)
+
+	swp1origmac=$(mac_get $swp1)
+	swp2origmac=$(mac_get $swp2)
+	ip link set $swp1 address $h2mac
+	ip link set $swp2 address $h1mac
+
+	vrf_prepare
+
+	h1_create
+	h2_create
+	switch_create
+
+	if ! tc action add action tunnel_key help 2>&1 | grep -q nofrag; then
+		log_test "SKIP: iproute doesn't support nofrag"
+		exit $ksft_skip
+	fi
+}
+
+cleanup()
+{
+	pre_cleanup
+
+	switch_destroy
+	h2_destroy
+	h1_destroy
+
+	vrf_cleanup
+
+	ip link set $swp2 address $swp2origmac
+	ip link set $swp1 address $swp1origmac
+}
+
+tunnel_key_nofrag_test()
+{
+	RET=0
+	local i
+
+	tc filter add dev $swp1 ingress protocol ip pref 100 handle 100 \
+		flower ip_flags nofrag action drop
+	tc filter add dev $swp1 ingress protocol ip pref 101 handle 101 \
+		flower ip_flags firstfrag action drop
+	tc filter add dev $swp1 ingress protocol ip pref 102 handle 102 \
+		flower ip_flags nofirstfrag action drop
+
+	# test 'nofrag' set
+	tc filter add dev h1-et egress protocol all pref 1 handle 1 matchall $tcflags \
+		action tunnel_key set src_ip 192.0.2.1 dst_ip 192.0.2.2 id 42 nofrag index 10
+	$MZ h1-et -c 1 -p 930 -a 00:aa:bb:cc:dd:ee -b 00:ee:dd:cc:bb:aa -t ip -q
+	tc_check_packets "dev $swp1 ingress" 100 1
+	check_err $? "packet smaller than MTU was not tunneled"
+
+	$MZ h1-et -c 1 -p 931 -a 00:aa:bb:cc:dd:ee -b 00:ee:dd:cc:bb:aa -t ip -q
+	tc_check_packets "dev $swp1 ingress" 100 1
+	check_err $? "packet bigger than MTU matched nofrag (nofrag was set)"
+	tc_check_packets "dev $swp1 ingress" 101 0
+	check_err $? "packet bigger than MTU matched firstfrag (nofrag was set)"
+	tc_check_packets "dev $swp1 ingress" 102 0
+	check_err $? "packet bigger than MTU matched nofirstfrag (nofrag was set)"
+
+	# test 'nofrag' cleared
+	tc actions change action tunnel_key set src_ip 192.0.2.1 dst_ip 192.0.2.2 id 42 index 10
+	$MZ h1-et -c 1 -p 931 -a 00:aa:bb:cc:dd:ee -b 00:ee:dd:cc:bb:aa -t ip -q
+	tc_check_packets "dev $swp1  ingress" 100 1
+	check_err $? "packet bigger than MTU matched nofrag (nofrag was unset)"
+	tc_check_packets "dev $swp1  ingress" 101 1
+	check_err $? "packet bigger than MTU didn't match firstfrag (nofrag was unset) "
+	tc_check_packets "dev $swp1 ingress" 102 1
+	check_err $? "packet bigger than MTU didn't match nofirstfrag (nofrag was unset) "
+
+	for i in 100 101 102; do
+		tc filter del dev $swp1 ingress protocol ip pref $i handle $i flower
+	done
+	tc filter del dev h1-et egress pref 1 handle 1 matchall
+
+	log_test "tunnel_key nofrag ($tcflags)"
+}
+
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tests_run
+
+tc_offload_check
+if [[ $? -ne 0 ]]; then
+	log_info "Could not test offloaded functionality"
+else
+	tcflags="skip_sw"
+	tests_run
+fi
+
+exit $EXIT_STATUS
-- 
2.39.2

Re: [PATCH net-next v2 2/4] selftests: tc-testing: extend the "skip" property

[Pedro Tammela]

On 23/03/2023 10:34, Davide Caratti wrote:
> currently, users can skip individual test cases by means of writing
> 
>    "skip": "yes"
> 
> in the scenario file. Extend this functionality by allowing the execution
> of a command, written in the "skip" property for a specific test case. If
> such property is present, tdc executes that command and skips the test if
> the return value is non-zero.
> 
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>


I saw the use case in patch 3 but I didn't understand how it can happen.
Shouldn't iproute2 at least match the kernel version? I know it's not a 
hard requirement for 99% of use cases, but when running tdc I would 
argue it's the minimum expected.

Re: [PATCH net-next v2 1/4] net/sched: act_tunnel_key: add support for "don't fragment"

[Jakub Kicinski]

On Thu, 23 Mar 2023 14:34:40 +0100 Davide Caratti wrote:
> +		    ((key->tun_flags & TUNNEL_DONT_FRAGMENT) &&
> +				nla_put_flag(skb, TCA_TUNNEL_KEY_NO_FRAG)) ||

indentation looks off :(

Re: [PATCH net-next v2 2/4] selftests: tc-testing: extend the "skip" property

[Davide Caratti]

hello Pedro, thanks for looking at this!

On Thu, Mar 23, 2023 at 11:01:53AM -0300, Pedro Tammela wrote:
> On 23/03/2023 10:34, Davide Caratti wrote:
> > currently, users can skip individual test cases by means of writing
> > 
> >    "skip": "yes"
> > 
> > in the scenario file. Extend this functionality by allowing the execution
> > of a command, written in the "skip" property for a specific test case. If
> > such property is present, tdc executes that command and skips the test if
> > the return value is non-zero.
> > 
> > Signed-off-by: Davide Caratti <dcaratti@redhat.com>
> 
> 
> I saw the use case in patch 3 but I didn't understand how it can happen.
> Shouldn't iproute2 at least match the kernel version? I know it's not a hard
> requirement for 99% of use cases, but when running tdc I would argue it's
> the minimum expected.

sure, but there are distributions where patches are backported: on these
ones, the kernel/iproute version is not so meaningful.
Instead of posting kselftest after the iproute2 support code is merged, I
think it's preferrable to just skip those kselftests that can't run because
they lack userspace bits; and by the way I see we are already taking this
approach elsewhere [1] [2].

-- 
davide

[1] https://elixir.bootlin.com/linux/v6.3-rc4/source/tools/testing/selftests/net/srv6_hl2encap_red_l2vpn_test.sh#L789
[2] https://elixir.bootlin.com/linux/v6.3-rc4/source/tools/testing/selftests/net/rtnetlink.sh#L391

Re: [PATCH net-next v2 2/4] selftests: tc-testing: extend the "skip" property

[Pedro Tammela]

On 27/03/2023 07:02, Davide Caratti wrote:
> hello Pedro, thanks for looking at this!
> 
> On Thu, Mar 23, 2023 at 11:01:53AM -0300, Pedro Tammela wrote:
>> On 23/03/2023 10:34, Davide Caratti wrote:
>>> currently, users can skip individual test cases by means of writing
>>>
>>>     "skip": "yes"
>>>
>>> in the scenario file. Extend this functionality by allowing the execution
>>> of a command, written in the "skip" property for a specific test case. If
>>> such property is present, tdc executes that command and skips the test if
>>> the return value is non-zero.
>>>
>>> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
>>
>>
>> I saw the use case in patch 3 but I didn't understand how it can happen.
>> Shouldn't iproute2 at least match the kernel version? I know it's not a hard
>> requirement for 99% of use cases, but when running tdc I would argue it's
>> the minimum expected.
> 
> sure, but there are distributions where patches are backported: on these
> ones, the kernel/iproute version is not so meaningful.

Oh, of course!

> Instead of posting kselftest after the iproute2 support code is merged, I
> think it's preferrable to just skip those kselftests that can't run because
> they lack userspace bits; and by the way I see we are already taking this
> approach elsewhere [1] [2].
> 

I see, so it makes distribution lives easier.
Wouldn't it be more clear then to have a separate property called 
"depends_on" or something similar?
If someone adds a new feature that depends on iproute2, then it would be 
more natural to just add a "depends_on" property.

Pedro