diff mbox series

[RFC,net-next,6/7] selftests: net: Use the provided dpctl rather than the vswitchd for tests.

Message ID 20240613181333.984810-7-aconole@redhat.com (mailing list archive)
State New
Headers show
Series selftests: net: Switch pmtu.sh to use the internal ovs script. | expand

Commit Message

Aaron Conole June 13, 2024, 6:13 p.m. UTC
The current pmtu test infrastucture requires an installed copy of the
ovs-vswitchd userspace.  This means that any automated or constrained
environments may not have the requisite tools to run the tests.  However,
the pmtu tests don't require any special classifier processing.  Indeed
they are only using the vswitchd in the most basic mode - as a NORMAL
switch.

However, the ovs-dpctl kernel utility can now program all the needed basic
flows to allow traffic to traverse the tunnels and provide support for at
least testing some basic pmtu scenarios.  More complicated flow pipelines
can be added to the internal ovs test infrastructure, but that is work for
the future.  For now, enable the most common cases - wide mega flows with
no other prerequisites.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 tools/testing/selftests/net/pmtu.sh | 87 ++++++++++++++++++++++-------
 1 file changed, 67 insertions(+), 20 deletions(-)

Comments

Stefano Brivio June 13, 2024, 8:37 p.m. UTC | #1
On Thu, 13 Jun 2024 14:13:32 -0400
Aaron Conole <aconole@redhat.com> wrote:

> The current pmtu test infrastucture requires an installed copy of the
> ovs-vswitchd userspace.  This means that any automated or constrained
> environments may not have the requisite tools to run the tests.  However,
> the pmtu tests don't require any special classifier processing.  Indeed
> they are only using the vswitchd in the most basic mode - as a NORMAL
> switch.
> 
> However, the ovs-dpctl kernel utility can now program all the needed basic
> flows to allow traffic to traverse the tunnels and provide support for at
> least testing some basic pmtu scenarios.

I didn't know about that tool, that looks like a nice improvement. A
few comments below (mostly nits):

> More complicated flow pipelines
> can be added to the internal ovs test infrastructure, but that is work for
> the future.  For now, enable the most common cases - wide mega flows with
> no other prerequisites.
> 
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  tools/testing/selftests/net/pmtu.sh | 87 ++++++++++++++++++++++-------
>  1 file changed, 67 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
> index cfc84958025a..7f4f35d88dcc 100755
> --- a/tools/testing/selftests/net/pmtu.sh
> +++ b/tools/testing/selftests/net/pmtu.sh
> @@ -846,22 +846,73 @@ setup_ovs_vxlan_or_geneve() {
>  	type="${1}"
>  	a_addr="${2}"
>  	b_addr="${3}"
> +	dport="6081"
>  
>  	if [ "${type}" = "vxlan" ]; then
> +		dport="4789"
>  		opts="${opts} ttl 64 dstport 4789"
>  		opts_b="local ${b_addr}"
>  	fi
>  
> -	run_cmd ovs-vsctl add-port ovs_br0 ${type}_a -- \
> -		set interface ${type}_a type=${type} \
> -		options:remote_ip=${b_addr} options:key=1 options:csum=true || return 1
> -
> +	run_cmd python3 ./openvswitch/ovs-dpctl.py add-if ovs_br0 ${type}_a -t ${type}

In some restricted environments, it might actually be more convenient
to carry around ovs-vsctl than Python with (Python) libraries.

Nowadays I typically (albeit seldom) run kselftests in throw-away VM
images made by mbuto (https://mbuto.sh/, see demo on the right), and
while it copies python3 and dynamic libraries from the host, adding
Python libraries such as pyroute2 gets quite complicated.

So I'm wondering, if it's not too messy: could we have two functions
starting from approximately here (say, setup_ovs_dpctl() and
setup_ovs_vsctl()), try with ovs-dpctl first, and, if that fails,
fall back to ovs-vsctl?

>  	run_cmd ${ns_b} ip link add ${type}_b type ${type} id 1 ${opts_b} remote ${a_addr} ${opts} || return 1
>  
>  	run_cmd ${ns_b} ip addr add ${tunnel4_b_addr}/${tunnel4_mask} dev ${type}_b
>  	run_cmd ${ns_b} ip addr add ${tunnel6_b_addr}/${tunnel6_mask} dev ${type}_b
>  
> +	run_cmd ip link set ${type}_a up
>  	run_cmd ${ns_b} ip link set ${type}_b up
> +
> +	ports=$(python3 ./openvswitch/ovs-dpctl.py show)
> +	br0_port=$(echo "$ports" | grep -E "\sovs_br0" | sed -e 's@port @@' | cut -d: -f1 | xargs)
> +	type_a_port=$(echo "$ports" | grep ${type}_a | sed -e 's@port @@' | cut -d: -f1 | xargs)
> +	veth_a_port=$(echo "$ports" | grep veth_A | sed -e 's@port @@' | cut -d: -f1 | xargs)
> +
> +	v4_a_tun="${prefix4}.${a_r1}.1"
> +	v4_b_tun="${prefix4}.${b_r1}.1"
> +
> +	v6_a_tun="${prefix6}:${a_r1}::1"
> +	v6_b_tun="${prefix6}:${b_r1}::1"
> +
> +	if [ "${v4_a_tun}" == "${a_addr}" ]; then

I see now that 05d92cb0e919 ("selftests/net: change shebang to bash to
support "source"") turned this into a Bash script (for no real reason,
lib.sh could have simply been sourced with '.' instead).

Other than that, this happily runs with dash and possibly others, and:

  $ checkbashisms -f pmtu.sh 
  possible bashism in pmtu.sh line 201 (should be '.', not 'source'):
  source lib.sh
  possible bashism in pmtu.sh line 202 (should be '.', not 'source'):
  source net_helper.sh

Would it be possible to change this to POSIX shell:

	if [ "${v4_a_tun}" = "${a_addr}" ]; then

even just for consistency with the rest of the file?

> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
> +		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x0800),ipv4()" \
> +		    "set(tunnel(tun_id=1,dst=${v4_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
> +		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x86dd),ipv6()" \
> +		    "set(tunnel(tun_id=1,dst=${v4_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
> +		    "recirc_id(0),tunnel(tun_id=1,src=${v4_b_tun},dst=${v4_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x0800),ipv4()" \
> +		    "${veth_a_port}"
> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
> +		    "recirc_id(0),tunnel(tun_id=1,src=${v4_b_tun},dst=${v4_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x86dd),ipv6()" \
> +		    "${veth_a_port}"
> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
> +		    "recirc_id(0),tunnel(tun_id=1,src=${v4_b_tun},dst=${v4_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x0806),arp()" \
> +		    "${veth_a_port}"
> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
> +		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x0806),arp(sip=${veth4_c_addr},tip=${tunnel4_b_addr})" \
> +		    "set(tunnel(tun_id=1,dst=${v4_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
> +	else
> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
> +		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x0800),ipv4()" \
> +		    "set(tunnel(tun_id=1,ipv6_dst=${v6_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
> +		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x86dd),ipv6()" \
> +		    "set(tunnel(tun_id=1,ipv6_dst=${v6_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
> +		    "recirc_id(0),tunnel(tun_id=1,ipv6_src=${v6_b_tun},ipv6_dst=${v6_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x0800),ipv4()" \
> +		    "${veth_a_port}"
> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
> +		    "recirc_id(0),tunnel(tun_id=1,ipv6_src=${v6_b_tun},ipv6_dst=${v6_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x86dd),ipv6()" \
> +		    "${veth_a_port}"
> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
> +		    "recirc_id(0),tunnel(tun_id=1,ipv6_src=${v6_b_tun},ipv6_dst=${v6_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x0806),arp()" \
> +		    "${veth_a_port}"
> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
> +		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x0806),arp(sip=${veth4_c_addr},tip=${tunnel4_b_addr})" \
> +		    "set(tunnel(tun_id=1,ipv6_dst=${v6_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
> +	fi
>  }
>  
>  setup_ovs_geneve4() {
> @@ -881,7 +932,7 @@ setup_ovs_vxlan6() {
>  }
>  
>  setup_ovs_bridge() {
> -	run_cmd ovs-vsctl add-br ovs_br0 || return $ksft_skip
> +	run_cmd python3 ./openvswitch/ovs-dpctl.py add-dp ovs_br0 || return $ksft_skip
>  	run_cmd ip link set ovs_br0 up
>  
>  	run_cmd ${ns_c} ip link add veth_C-A type veth peer name veth_A-C
> @@ -891,7 +942,7 @@ setup_ovs_bridge() {
>  	run_cmd ${ns_c} ip link set veth_C-A up
>  	run_cmd ${ns_c} ip addr add ${veth4_c_addr}/${veth4_mask} dev veth_C-A
>  	run_cmd ${ns_c} ip addr add ${veth6_c_addr}/${veth6_mask} dev veth_C-A
> -	run_cmd ovs-vsctl add-port ovs_br0 veth_A-C
> +	run_cmd python3 ./openvswitch/ovs-dpctl.py add-if ovs_br0 veth_A-C
>  
>  	# Move veth_A-R1 to init
>  	run_cmd ${ns_a} ip link set veth_A-R1 netns 1
> @@ -942,8 +993,10 @@ cleanup() {
>  
>  	ip link del veth_A-C			2>/dev/null
>  	ip link del veth_A-R1			2>/dev/null
> -	ovs-vsctl --if-exists del-port vxlan_a	2>/dev/null
> -	ovs-vsctl --if-exists del-br ovs_br0	2>/dev/null
> +	# squelch the output of the del-if commands since it can be wordy
> +	python3 ./openvswitch/ovs-dpctl.py del-if ovs_br0 -d true vxlan_a	>/dev/null	2>&1
> +	python3 ./openvswitch/ovs-dpctl.py del-if ovs_br0 -d true geneve_a	>/dev/null	2>&1
> +	python3 ./openvswitch/ovs-dpctl.py del-dp ovs_br0 >/dev/null	2>&1

The idea behind those tabs before 2>/dev/null was to keep them aligned
and make those redirections a bit easier on the eyes.

If you add more, you could keep those aligned as well -- or just decide
that lines are too long and drop the tabs altogether.

>  	rm -f "$tmpoutfile"
>  }
>  
> @@ -1407,16 +1460,10 @@ test_pmtu_ipvX_over_ovs_vxlanY_or_geneveY_exception() {
>  		exp_mtu=$((${ll_mtu} - 40          - 8          - 8                   - 14))
>  	fi
>  
> -	if [ "${type}" = "vxlan" ]; then
> -		tun_a="vxlan_sys_4789"
> -	elif [ "${type}" = "geneve" ]; then
> -		tun_a="genev_sys_6081"
> -	fi
> -
> -	trace ""        "${tun_a}"  "${ns_b}"  ${type}_b \
> -	      ""        veth_A-R1   "${ns_r1}" veth_R1-A \
> -	      "${ns_b}" veth_B-R1   "${ns_r1}" veth_R1-B \
> -	      ""        ovs_br0     ""         veth-A-C  \
> +	trace ""        "${type}_a"  "${ns_b}"  ${type}_b \
> +	      ""        veth_A-R1    "${ns_r1}" veth_R1-A \
> +	      "${ns_b}" veth_B-R1    "${ns_r1}" veth_R1-B \
> +	      ""        ovs_br0      ""         veth-A_C  \
>  	      "${ns_c}" veth_C-A
>  
>  	if [ ${family} -eq 4 ]; then
> @@ -1436,8 +1483,8 @@ test_pmtu_ipvX_over_ovs_vxlanY_or_geneveY_exception() {
>  	mtu "${ns_b}"  veth_B-R1 ${ll_mtu}
>  	mtu "${ns_r1}" veth_R1-B ${ll_mtu}
>  
> -	mtu ""        ${tun_a}  $((${ll_mtu} + 1000))
> -	mtu "${ns_b}" ${type}_b $((${ll_mtu} + 1000))
> +	mtu ""        ${type}_a  $((${ll_mtu} + 1000))
> +	mtu "${ns_b}" ${type}_b  $((${ll_mtu} + 1000))
>  
>  	run_cmd ${ns_c} ${ping} -q -M want -i 0.1 -c 20 -s $((${ll_mtu} + 500)) ${dst} || return 1
>
Aaron Conole June 14, 2024, 3:53 p.m. UTC | #2
Hi Stefano,

Thanks for the review!

Stefano Brivio <sbrivio@redhat.com> writes:

> On Thu, 13 Jun 2024 14:13:32 -0400
> Aaron Conole <aconole@redhat.com> wrote:
>
>> The current pmtu test infrastucture requires an installed copy of the
>> ovs-vswitchd userspace.  This means that any automated or constrained
>> environments may not have the requisite tools to run the tests.  However,
>> the pmtu tests don't require any special classifier processing.  Indeed
>> they are only using the vswitchd in the most basic mode - as a NORMAL
>> switch.
>> 
>> However, the ovs-dpctl kernel utility can now program all the needed basic
>> flows to allow traffic to traverse the tunnels and provide support for at
>> least testing some basic pmtu scenarios.
>
> I didn't know about that tool, that looks like a nice improvement. A
> few comments below (mostly nits):

It didn't at the time, so no worries :)

>> More complicated flow pipelines
>> can be added to the internal ovs test infrastructure, but that is work for
>> the future.  For now, enable the most common cases - wide mega flows with
>> no other prerequisites.
>> 
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>>  tools/testing/selftests/net/pmtu.sh | 87 ++++++++++++++++++++++-------
>>  1 file changed, 67 insertions(+), 20 deletions(-)
>> 
>> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
>> index cfc84958025a..7f4f35d88dcc 100755
>> --- a/tools/testing/selftests/net/pmtu.sh
>> +++ b/tools/testing/selftests/net/pmtu.sh
>> @@ -846,22 +846,73 @@ setup_ovs_vxlan_or_geneve() {
>>  	type="${1}"
>>  	a_addr="${2}"
>>  	b_addr="${3}"
>> +	dport="6081"
>>  
>>  	if [ "${type}" = "vxlan" ]; then
>> +		dport="4789"
>>  		opts="${opts} ttl 64 dstport 4789"
>>  		opts_b="local ${b_addr}"
>>  	fi
>>  
>> -	run_cmd ovs-vsctl add-port ovs_br0 ${type}_a -- \
>> -		set interface ${type}_a type=${type} \
>> -		options:remote_ip=${b_addr} options:key=1 options:csum=true || return 1
>> -
>> +	run_cmd python3 ./openvswitch/ovs-dpctl.py add-if ovs_br0 ${type}_a -t ${type}
>
> In some restricted environments, it might actually be more convenient
> to carry around ovs-vsctl than Python with (Python) libraries.
>
> Nowadays I typically (albeit seldom) run kselftests in throw-away VM
> images made by mbuto (https://mbuto.sh/, see demo on the right), and
> while it copies python3 and dynamic libraries from the host, adding
> Python libraries such as pyroute2 gets quite complicated.
>
> So I'm wondering, if it's not too messy: could we have two functions
> starting from approximately here (say, setup_ovs_dpctl() and
> setup_ovs_vsctl()), try with ovs-dpctl first, and, if that fails,
> fall back to ovs-vsctl?

It didn't seem to be too bad - so I went ahead and made that change.  It
tested well, so I'll resubmit it with that.

>>  	run_cmd ${ns_b} ip link add ${type}_b type ${type} id 1
>> ${opts_b} remote ${a_addr} ${opts} || return 1
>>  
>>  	run_cmd ${ns_b} ip addr add ${tunnel4_b_addr}/${tunnel4_mask} dev ${type}_b
>>  	run_cmd ${ns_b} ip addr add ${tunnel6_b_addr}/${tunnel6_mask} dev ${type}_b
>>  
>> +	run_cmd ip link set ${type}_a up
>>  	run_cmd ${ns_b} ip link set ${type}_b up
>> +
>> +	ports=$(python3 ./openvswitch/ovs-dpctl.py show)
>> +	br0_port=$(echo "$ports" | grep -E "\sovs_br0" | sed -e 's@port @@' | cut -d: -f1 | xargs)
>> +	type_a_port=$(echo "$ports" | grep ${type}_a | sed -e 's@port @@' | cut -d: -f1 | xargs)
>> +	veth_a_port=$(echo "$ports" | grep veth_A | sed -e 's@port @@' | cut -d: -f1 | xargs)
>> +
>> +	v4_a_tun="${prefix4}.${a_r1}.1"
>> +	v4_b_tun="${prefix4}.${b_r1}.1"
>> +
>> +	v6_a_tun="${prefix6}:${a_r1}::1"
>> +	v6_b_tun="${prefix6}:${b_r1}::1"
>> +
>> +	if [ "${v4_a_tun}" == "${a_addr}" ]; then
>
> I see now that 05d92cb0e919 ("selftests/net: change shebang to bash to
> support "source"") turned this into a Bash script (for no real reason,
> lib.sh could have simply been sourced with '.' instead).
>
> Other than that, this happily runs with dash and possibly others, and:
>
>   $ checkbashisms -f pmtu.sh 
>   possible bashism in pmtu.sh line 201 (should be '.', not 'source'):
>   source lib.sh
>   possible bashism in pmtu.sh line 202 (should be '.', not 'source'):
>   source net_helper.sh
>
> Would it be possible to change this to POSIX shell:
>
> 	if [ "${v4_a_tun}" = "${a_addr}" ]; then
>
> even just for consistency with the rest of the file?

done.

>> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
>> +		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x0800),ipv4()" \
>> +		    "set(tunnel(tun_id=1,dst=${v4_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
>> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
>> +		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x86dd),ipv6()" \
>> +		    "set(tunnel(tun_id=1,dst=${v4_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
>> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
>> +		    "recirc_id(0),tunnel(tun_id=1,src=${v4_b_tun},dst=${v4_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x0800),ipv4()" \
>> +		    "${veth_a_port}"
>> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
>> +		    "recirc_id(0),tunnel(tun_id=1,src=${v4_b_tun},dst=${v4_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x86dd),ipv6()" \
>> +		    "${veth_a_port}"
>> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
>> +		    "recirc_id(0),tunnel(tun_id=1,src=${v4_b_tun},dst=${v4_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x0806),arp()" \
>> +		    "${veth_a_port}"
>> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
>> +		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x0806),arp(sip=${veth4_c_addr},tip=${tunnel4_b_addr})" \
>> +		    "set(tunnel(tun_id=1,dst=${v4_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
>> +	else
>> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
>> +		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x0800),ipv4()" \
>> +		    "set(tunnel(tun_id=1,ipv6_dst=${v6_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
>> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
>> +		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x86dd),ipv6()" \
>> +		    "set(tunnel(tun_id=1,ipv6_dst=${v6_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
>> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
>> +		    "recirc_id(0),tunnel(tun_id=1,ipv6_src=${v6_b_tun},ipv6_dst=${v6_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x0800),ipv4()" \
>> +		    "${veth_a_port}"
>> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
>> +		    "recirc_id(0),tunnel(tun_id=1,ipv6_src=${v6_b_tun},ipv6_dst=${v6_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x86dd),ipv6()" \
>> +		    "${veth_a_port}"
>> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
>> +		    "recirc_id(0),tunnel(tun_id=1,ipv6_src=${v6_b_tun},ipv6_dst=${v6_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x0806),arp()" \
>> +		    "${veth_a_port}"
>> +		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
>> +		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x0806),arp(sip=${veth4_c_addr},tip=${tunnel4_b_addr})" \
>> +		    "set(tunnel(tun_id=1,ipv6_dst=${v6_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
>> +	fi
>>  }
>>  
>>  setup_ovs_geneve4() {
>> @@ -881,7 +932,7 @@ setup_ovs_vxlan6() {
>>  }
>>  
>>  setup_ovs_bridge() {
>> -	run_cmd ovs-vsctl add-br ovs_br0 || return $ksft_skip
>> +	run_cmd python3 ./openvswitch/ovs-dpctl.py add-dp ovs_br0 || return $ksft_skip
>>  	run_cmd ip link set ovs_br0 up
>>  
>>  	run_cmd ${ns_c} ip link add veth_C-A type veth peer name veth_A-C
>> @@ -891,7 +942,7 @@ setup_ovs_bridge() {
>>  	run_cmd ${ns_c} ip link set veth_C-A up
>>  	run_cmd ${ns_c} ip addr add ${veth4_c_addr}/${veth4_mask} dev veth_C-A
>>  	run_cmd ${ns_c} ip addr add ${veth6_c_addr}/${veth6_mask} dev veth_C-A
>> -	run_cmd ovs-vsctl add-port ovs_br0 veth_A-C
>> +	run_cmd python3 ./openvswitch/ovs-dpctl.py add-if ovs_br0 veth_A-C
>>  
>>  	# Move veth_A-R1 to init
>>  	run_cmd ${ns_a} ip link set veth_A-R1 netns 1
>> @@ -942,8 +993,10 @@ cleanup() {
>>  
>>  	ip link del veth_A-C			2>/dev/null
>>  	ip link del veth_A-R1			2>/dev/null
>> -	ovs-vsctl --if-exists del-port vxlan_a	2>/dev/null
>> -	ovs-vsctl --if-exists del-br ovs_br0	2>/dev/null
>> +	# squelch the output of the del-if commands since it can be wordy
>> +	python3 ./openvswitch/ovs-dpctl.py del-if ovs_br0 -d true vxlan_a	>/dev/null	2>&1
>> +	python3 ./openvswitch/ovs-dpctl.py del-if ovs_br0 -d true geneve_a	>/dev/null	2>&1
>> +	python3 ./openvswitch/ovs-dpctl.py del-dp ovs_br0 >/dev/null	2>&1
>
> The idea behind those tabs before 2>/dev/null was to keep them aligned
> and make those redirections a bit easier on the eyes.
>
> If you add more, you could keep those aligned as well -- or just decide
> that lines are too long and drop the tabs altogether.

Yeah, looks like I missed the /dev/null tabs but those were supposed to
be lined up.  Anyway, I rewrote this part so that it looks better (I
think).

>>  	rm -f "$tmpoutfile"
>>  }
>>  
>> @@ -1407,16 +1460,10 @@ test_pmtu_ipvX_over_ovs_vxlanY_or_geneveY_exception() {
>>  		exp_mtu=$((${ll_mtu} - 40          - 8          - 8                   - 14))
>>  	fi
>>  
>> -	if [ "${type}" = "vxlan" ]; then
>> -		tun_a="vxlan_sys_4789"
>> -	elif [ "${type}" = "geneve" ]; then
>> -		tun_a="genev_sys_6081"
>> -	fi
>> -
>> -	trace ""        "${tun_a}"  "${ns_b}"  ${type}_b \
>> -	      ""        veth_A-R1   "${ns_r1}" veth_R1-A \
>> -	      "${ns_b}" veth_B-R1   "${ns_r1}" veth_R1-B \
>> -	      ""        ovs_br0     ""         veth-A-C  \
>> +	trace ""        "${type}_a"  "${ns_b}"  ${type}_b \
>> +	      ""        veth_A-R1    "${ns_r1}" veth_R1-A \
>> +	      "${ns_b}" veth_B-R1    "${ns_r1}" veth_R1-B \
>> +	      ""        ovs_br0      ""         veth-A_C  \
>>  	      "${ns_c}" veth_C-A
>>  
>>  	if [ ${family} -eq 4 ]; then
>> @@ -1436,8 +1483,8 @@ test_pmtu_ipvX_over_ovs_vxlanY_or_geneveY_exception() {
>>  	mtu "${ns_b}"  veth_B-R1 ${ll_mtu}
>>  	mtu "${ns_r1}" veth_R1-B ${ll_mtu}
>>  
>> -	mtu ""        ${tun_a}  $((${ll_mtu} + 1000))
>> -	mtu "${ns_b}" ${type}_b $((${ll_mtu} + 1000))
>> +	mtu ""        ${type}_a  $((${ll_mtu} + 1000))
>> +	mtu "${ns_b}" ${type}_b  $((${ll_mtu} + 1000))
>>  
>>  	run_cmd ${ns_c} ${ping} -q -M want -i 0.1 -c 20 -s $((${ll_mtu} + 500)) ${dst} || return 1
>>
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index cfc84958025a..7f4f35d88dcc 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -846,22 +846,73 @@  setup_ovs_vxlan_or_geneve() {
 	type="${1}"
 	a_addr="${2}"
 	b_addr="${3}"
+	dport="6081"
 
 	if [ "${type}" = "vxlan" ]; then
+		dport="4789"
 		opts="${opts} ttl 64 dstport 4789"
 		opts_b="local ${b_addr}"
 	fi
 
-	run_cmd ovs-vsctl add-port ovs_br0 ${type}_a -- \
-		set interface ${type}_a type=${type} \
-		options:remote_ip=${b_addr} options:key=1 options:csum=true || return 1
-
+	run_cmd python3 ./openvswitch/ovs-dpctl.py add-if ovs_br0 ${type}_a -t ${type}
 	run_cmd ${ns_b} ip link add ${type}_b type ${type} id 1 ${opts_b} remote ${a_addr} ${opts} || return 1
 
 	run_cmd ${ns_b} ip addr add ${tunnel4_b_addr}/${tunnel4_mask} dev ${type}_b
 	run_cmd ${ns_b} ip addr add ${tunnel6_b_addr}/${tunnel6_mask} dev ${type}_b
 
+	run_cmd ip link set ${type}_a up
 	run_cmd ${ns_b} ip link set ${type}_b up
+
+	ports=$(python3 ./openvswitch/ovs-dpctl.py show)
+	br0_port=$(echo "$ports" | grep -E "\sovs_br0" | sed -e 's@port @@' | cut -d: -f1 | xargs)
+	type_a_port=$(echo "$ports" | grep ${type}_a | sed -e 's@port @@' | cut -d: -f1 | xargs)
+	veth_a_port=$(echo "$ports" | grep veth_A | sed -e 's@port @@' | cut -d: -f1 | xargs)
+
+	v4_a_tun="${prefix4}.${a_r1}.1"
+	v4_b_tun="${prefix4}.${b_r1}.1"
+
+	v6_a_tun="${prefix6}:${a_r1}::1"
+	v6_b_tun="${prefix6}:${b_r1}::1"
+
+	if [ "${v4_a_tun}" == "${a_addr}" ]; then
+		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
+		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x0800),ipv4()" \
+		    "set(tunnel(tun_id=1,dst=${v4_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
+		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
+		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x86dd),ipv6()" \
+		    "set(tunnel(tun_id=1,dst=${v4_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
+		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
+		    "recirc_id(0),tunnel(tun_id=1,src=${v4_b_tun},dst=${v4_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x0800),ipv4()" \
+		    "${veth_a_port}"
+		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
+		    "recirc_id(0),tunnel(tun_id=1,src=${v4_b_tun},dst=${v4_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x86dd),ipv6()" \
+		    "${veth_a_port}"
+		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
+		    "recirc_id(0),tunnel(tun_id=1,src=${v4_b_tun},dst=${v4_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x0806),arp()" \
+		    "${veth_a_port}"
+		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
+		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x0806),arp(sip=${veth4_c_addr},tip=${tunnel4_b_addr})" \
+		    "set(tunnel(tun_id=1,dst=${v4_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
+	else
+		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
+		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x0800),ipv4()" \
+		    "set(tunnel(tun_id=1,ipv6_dst=${v6_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
+		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
+		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x86dd),ipv6()" \
+		    "set(tunnel(tun_id=1,ipv6_dst=${v6_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
+		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
+		    "recirc_id(0),tunnel(tun_id=1,ipv6_src=${v6_b_tun},ipv6_dst=${v6_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x0800),ipv4()" \
+		    "${veth_a_port}"
+		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
+		    "recirc_id(0),tunnel(tun_id=1,ipv6_src=${v6_b_tun},ipv6_dst=${v6_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x86dd),ipv6()" \
+		    "${veth_a_port}"
+		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
+		    "recirc_id(0),tunnel(tun_id=1,ipv6_src=${v6_b_tun},ipv6_dst=${v6_a_tun}),in_port(${type_a_port}),eth(),eth_type(0x0806),arp()" \
+		    "${veth_a_port}"
+		run_cmd python3 ./openvswitch/ovs-dpctl.py add-flow ovs_br0 \
+		    "recirc_id(0),in_port(${veth_a_port}),eth(),eth_type(0x0806),arp(sip=${veth4_c_addr},tip=${tunnel4_b_addr})" \
+		    "set(tunnel(tun_id=1,ipv6_dst=${v6_b_tun},ttl=64,tp_dst=${dport},flags(df|csum))),${type_a_port}"
+	fi
 }
 
 setup_ovs_geneve4() {
@@ -881,7 +932,7 @@  setup_ovs_vxlan6() {
 }
 
 setup_ovs_bridge() {
-	run_cmd ovs-vsctl add-br ovs_br0 || return $ksft_skip
+	run_cmd python3 ./openvswitch/ovs-dpctl.py add-dp ovs_br0 || return $ksft_skip
 	run_cmd ip link set ovs_br0 up
 
 	run_cmd ${ns_c} ip link add veth_C-A type veth peer name veth_A-C
@@ -891,7 +942,7 @@  setup_ovs_bridge() {
 	run_cmd ${ns_c} ip link set veth_C-A up
 	run_cmd ${ns_c} ip addr add ${veth4_c_addr}/${veth4_mask} dev veth_C-A
 	run_cmd ${ns_c} ip addr add ${veth6_c_addr}/${veth6_mask} dev veth_C-A
-	run_cmd ovs-vsctl add-port ovs_br0 veth_A-C
+	run_cmd python3 ./openvswitch/ovs-dpctl.py add-if ovs_br0 veth_A-C
 
 	# Move veth_A-R1 to init
 	run_cmd ${ns_a} ip link set veth_A-R1 netns 1
@@ -942,8 +993,10 @@  cleanup() {
 
 	ip link del veth_A-C			2>/dev/null
 	ip link del veth_A-R1			2>/dev/null
-	ovs-vsctl --if-exists del-port vxlan_a	2>/dev/null
-	ovs-vsctl --if-exists del-br ovs_br0	2>/dev/null
+	# squelch the output of the del-if commands since it can be wordy
+	python3 ./openvswitch/ovs-dpctl.py del-if ovs_br0 -d true vxlan_a	>/dev/null	2>&1
+	python3 ./openvswitch/ovs-dpctl.py del-if ovs_br0 -d true geneve_a	>/dev/null	2>&1
+	python3 ./openvswitch/ovs-dpctl.py del-dp ovs_br0 >/dev/null	2>&1
 	rm -f "$tmpoutfile"
 }
 
@@ -1407,16 +1460,10 @@  test_pmtu_ipvX_over_ovs_vxlanY_or_geneveY_exception() {
 		exp_mtu=$((${ll_mtu} - 40          - 8          - 8                   - 14))
 	fi
 
-	if [ "${type}" = "vxlan" ]; then
-		tun_a="vxlan_sys_4789"
-	elif [ "${type}" = "geneve" ]; then
-		tun_a="genev_sys_6081"
-	fi
-
-	trace ""        "${tun_a}"  "${ns_b}"  ${type}_b \
-	      ""        veth_A-R1   "${ns_r1}" veth_R1-A \
-	      "${ns_b}" veth_B-R1   "${ns_r1}" veth_R1-B \
-	      ""        ovs_br0     ""         veth-A-C  \
+	trace ""        "${type}_a"  "${ns_b}"  ${type}_b \
+	      ""        veth_A-R1    "${ns_r1}" veth_R1-A \
+	      "${ns_b}" veth_B-R1    "${ns_r1}" veth_R1-B \
+	      ""        ovs_br0      ""         veth-A_C  \
 	      "${ns_c}" veth_C-A
 
 	if [ ${family} -eq 4 ]; then
@@ -1436,8 +1483,8 @@  test_pmtu_ipvX_over_ovs_vxlanY_or_geneveY_exception() {
 	mtu "${ns_b}"  veth_B-R1 ${ll_mtu}
 	mtu "${ns_r1}" veth_R1-B ${ll_mtu}
 
-	mtu ""        ${tun_a}  $((${ll_mtu} + 1000))
-	mtu "${ns_b}" ${type}_b $((${ll_mtu} + 1000))
+	mtu ""        ${type}_a  $((${ll_mtu} + 1000))
+	mtu "${ns_b}" ${type}_b  $((${ll_mtu} + 1000))
 
 	run_cmd ${ns_c} ${ping} -q -M want -i 0.1 -c 20 -s $((${ll_mtu} + 500)) ${dst} || return 1