[PATCH net 0/2] net: dst_metadata: fix DF flag extraction on tunnel rx

[PATCH net 0/2] net: dst_metadata: fix DF flag extraction on tunnel rx

[Ilya Maximets]

Two patches here, first fixes the issue where tunnel core doesn't
actually extract DF bit from the outer IP header, even though both
OVS and TC flower allow matching on it.  More details in the commit
message.

The second is a selftest for openvswitch that reproduces the issue,
but also just adds some basic coverage for the tunnel metadata
extraction and related openvswitch uAPI.

Ilya Maximets (2):
  net: dst_metadata: fix IP_DF bit not extracted from tunnel headers
  selftests: openvswitch: add a simple test for tunnel metadata

 include/net/dst_metadata.h                    | 11 ++-
 .../selftests/net/openvswitch/openvswitch.sh  | 88 +++++++++++++++++--
 2 files changed, 90 insertions(+), 9 deletions(-)

-- 
2.50.1

[PATCH net 1/2] net: dst_metadata: fix IP_DF bit not extracted from tunnel headers

[Ilya Maximets]

Both OVS and TC flower allow extracting and matching on the DF bit of
the outer IP header via OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT in the
OVS_KEY_ATTR_TUNNEL and TCA_FLOWER_KEY_FLAGS_TUNNEL_DONT_FRAGMENT in
the TCA_FLOWER_KEY_ENC_FLAGS respectively.  Flow dissector extracts
this information as FLOW_DIS_F_TUNNEL_DONT_FRAGMENT from the tunnel
info key.

However, the IP_TUNNEL_DONT_FRAGMENT_BIT in the tunnel key is never
actually set, because the tunneling code doesn't actually extract it
from the IP header.  OAM and CRIT_OPT are extracted by the tunnel
implementation code, same code also sets the KEY flag, if present.
UDP tunnel core takes care of setting the CSUM flag if the checksum
is present in the UDP header, but the DONT_FRAGMENT is not handled at
any layer.

Fix that by checking the bit and setting the corresponding flag while
populating the tunnel info in the IP layer where it belongs.

Not using __assign_bit as we don't really need to clear the bit in a
just initialized field.  It also doesn't seem like using __assign_bit
will make the code look better.

Clearly, users didn't rely on this functionality for anything very
important until now.  The reason why this doesn't break OVS logic is
that it only matches on what kernel previously parsed out and if kernel
consistently reports this bit as zero, OVS will only match on it to be
zero, which sort of works.  But it is still a bug that the uAPI reports
and allows matching on the field that is not actually checked in the
packet.  And this is causing misleading -df reporting in OVS datapath
flows, while the tunnel traffic actually has the bit set in most cases.

This may also cause issues if a hardware properly implements support
for tunnel flag matching as it will disagree with the implementation
in a software path of TC flower.

Fixes: 7d5437c709de ("openvswitch: Add tunneling interface.")
Fixes: 1d17568e74de ("net/sched: cls_flower: add support for matching tunnel control flags")
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 include/net/dst_metadata.h | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/net/dst_metadata.h b/include/net/dst_metadata.h
index 4160731dcb6e..b7c6736a7fe7 100644
--- a/include/net/dst_metadata.h
+++ b/include/net/dst_metadata.h
@@ -3,6 +3,7 @@
 #define __NET_DST_METADATA_H 1
 
 #include <linux/skbuff.h>
+#include <net/ip.h>
 #include <net/ip_tunnels.h>
 #include <net/macsec.h>
 #include <net/dst.h>
@@ -220,9 +221,15 @@ static inline struct metadata_dst *ip_tun_rx_dst(struct sk_buff *skb,
 						 int md_size)
 {
 	const struct iphdr *iph = ip_hdr(skb);
+	struct metadata_dst *tun_dst;
+
+	tun_dst = __ip_tun_set_dst(iph->saddr, iph->daddr, iph->tos, iph->ttl,
+				   0, flags, tunnel_id, md_size);
 
-	return __ip_tun_set_dst(iph->saddr, iph->daddr, iph->tos, iph->ttl,
-				0, flags, tunnel_id, md_size);
+	if (iph->frag_off & htons(IP_DF))
+		__set_bit(IP_TUNNEL_DONT_FRAGMENT_BIT,
+			  tun_dst->u.tun_info.key.tun_flags);
+	return tun_dst;
 }
 
 static inline struct metadata_dst *__ipv6_tun_set_dst(const struct in6_addr *saddr,
-- 
2.50.1

[PATCH net 2/2] selftests: openvswitch: add a simple test for tunnel metadata

[Ilya Maximets]

This test ensures that upon receiving decapsulated packets from a
tunnel interface in openvswitch, the tunnel metadata fields are
properly populated.  This partially covers interoperability of the
kernel tunnel ports and openvswitch tunnels (LWT) and parsing and
formatting of the tunnel metadata fields of the openvswitch netlink
uAPI.  Doing so, this test also ensures that fields and flags are
properly extracted during decapsulation by the tunnel core code,
serving as a regression test for the previously fixed issue with the
DF bit not being extracted from the outer IP header.

The ovs-dpctl.py script already supports all that is necessary for
the tunnel ports for this test, so we only need to adjust the
ovs_add_if() function to pass the '-t' port type argument in order
to be able to create tunnel ports in the openvswitch datapath.

Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 .../selftests/net/openvswitch/openvswitch.sh  | 88 +++++++++++++++++--
 1 file changed, 81 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index 3c8d3455d8e7..b327d3061ed5 100755
--- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
+++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
@@ -25,6 +25,7 @@ tests="
 	nat_related_v4				ip4-nat-related: ICMP related matches work with SNAT
 	netlink_checks				ovsnl: validate netlink attrs and settings
 	upcall_interfaces			ovs: test the upcall interfaces
+	tunnel_metadata				ovs: test extraction of tunnel metadata
 	drop_reason				drop: test drop reasons are emitted
 	psample					psample: Sampling packets with psample"
 
@@ -113,13 +114,13 @@ ovs_add_dp () {
 }
 
 ovs_add_if () {
-	info "Adding IF to DP: br:$2 if:$3"
-	if [ "$4" != "-u" ]; then
-		ovs_sbx "$1" python3 $ovs_base/ovs-dpctl.py add-if "$2" "$3" \
-		    || return 1
+	info "Adding IF to DP: br:$3 if:$4 ($2)"
+	if [ "$5" != "-u" ]; then
+		ovs_sbx "$1" python3 $ovs_base/ovs-dpctl.py add-if \
+		    -t "$2" "$3" "$4" || return 1
 	else
 		python3 $ovs_base/ovs-dpctl.py add-if \
-		    -u "$2" "$3" >$ovs_dir/$3.out 2>$ovs_dir/$3.err &
+		    -u -t "$2" "$3" "$4" >$ovs_dir/$4.out 2>$ovs_dir/$4.err &
 		pid=$!
 		on_exit "ovs_sbx $1 kill -TERM $pid 2>/dev/null"
 	fi
@@ -166,9 +167,9 @@ ovs_add_netns_and_veths () {
 	fi
 
 	if [ "$7" != "-u" ]; then
-		ovs_add_if "$1" "$2" "$4" || return 1
+		ovs_add_if "$1" "netdev" "$2" "$4" || return 1
 	else
-		ovs_add_if "$1" "$2" "$4" -u || return 1
+		ovs_add_if "$1" "netdev" "$2" "$4" -u || return 1
 	fi
 
 	if [ $TRACING -eq 1 ]; then
@@ -756,6 +757,79 @@ test_upcall_interfaces() {
 	return 0
 }
 
+ovs_add_kernel_tunnel() {
+	local sbxname=$1; shift
+	local ns=$1; shift
+	local tnl_type=$1; shift
+	local name=$1; shift
+	local addr=$1; shift
+
+	info "setting up kernel ${tnl_type} tunnel ${name}"
+	ovs_sbx "${sbxname}" ip -netns ${ns} link add dev ${name} type ${tnl_type} $* || return 1
+	on_exit "ovs_sbx ${sbxname} ip -netns ${ns} link del ${name} >/dev/null 2>&1"
+	ovs_sbx "${sbxname}" ip -netns ${ns} addr add dev ${name} ${addr} || return 1
+	ovs_sbx "${sbxname}" ip -netns ${ns} link set dev ${name} mtu 1450 up || return 1
+}
+
+test_tunnel_metadata() {
+	which arping >/dev/null 2>&1 || return $ksft_skip
+
+	sbxname="test_tunnel_metadata"
+	sbx_add "${sbxname}" || return 1
+
+	info "setting up new DP"
+	ovs_add_dp "${sbxname}" tdp0 -V 2:1 || return 1
+
+	ovs_add_netns_and_veths "${sbxname}" tdp0 tns left0 l0 \
+		172.31.110.1/24 || return 1
+
+	info "removing veth interface from openvswitch and setting IP"
+	ovs_del_if "${sbxname}" tdp0 left0 || return 1
+	ovs_sbx "${sbxname}" ip addr add 172.31.110.2/24 dev left0 || return 1
+	ovs_sbx "${sbxname}" ip link set left0 up || return 1
+
+	info "setting up tunnel port in openvswitch"
+	ovs_add_if "${sbxname}" "vxlan" tdp0 ovs-vxlan0 -u || return 1
+	on_exit "ovs_sbx ${sbxname} ip link del ovs-vxlan0"
+	ovs_wait ip link show ovs-vxlan0 &>/dev/null || return 1
+	ovs_sbx "${sbxname}" ip link set ovs-vxlan0 up || return 1
+
+	configs=$(echo '
+	    1 172.31.221.1/24 1155332 32   set   udpcsum flags\(df\|csum\)
+	    2 172.31.222.1/24 1234567 45   set noudpcsum flags\(df\)
+	    3 172.31.223.1/24 1020304 23 unset   udpcsum flags\(csum\)
+	    4 172.31.224.1/24 1357986 15 unset noudpcsum' | sed '/^$/d')
+
+	while read -r i addr id ttl df csum flags; do
+		ovs_add_kernel_tunnel "${sbxname}" tns vxlan vxlan${i} ${addr} \
+			remote 172.31.110.2 id ${id} dstport 4789 \
+			ttl ${ttl} df ${df} ${csum} || return 1
+	done <<< "${configs}"
+
+	ovs_wait grep -q 'listening on upcall packet handler' \
+		${ovs_dir}/ovs-vxlan0.out || return 1
+
+	info "sending arping"
+	for i in 1 2 3 4; do
+		ovs_sbx "${sbxname}" ip netns exec tns \
+			arping -I vxlan${i} 172.31.22${i}.2 -c 1 \
+			>${ovs_dir}/arping.stdout 2>${ovs_dir}/arping.stderr
+	done
+
+	info "checking that received decapsulated packets carry correct metadata"
+	while read -r i addr id ttl df csum flags; do
+		arp_hdr="arp\\(sip=172.31.22${i}.1,tip=172.31.22${i}.2,op=1,sha="
+		addrs="src=172.31.110.1,dst=172.31.110.2"
+		ports="tp_src=[0-9]*,tp_dst=4789"
+		tnl_md="tunnel\\(tun_id=${id},${addrs},ttl=${ttl},${ports},${flags}\\)"
+
+		ovs_sbx "${sbxname}" grep -qE "MISS upcall.*${tnl_md}.*${arp_hdr}" \
+			${ovs_dir}/ovs-vxlan0.out || return 1
+	done <<< "${configs}"
+
+	return 0
+}
+
 run_test() {
 	(
 	tname="$1"
-- 
2.50.1

Re: [PATCH net 1/2] net: dst_metadata: fix IP_DF bit not extracted from tunnel headers

[Ido Schimmel]

On Fri, Sep 05, 2025 at 03:30:55PM +0200, Ilya Maximets wrote:
> @@ -220,9 +221,15 @@ static inline struct metadata_dst *ip_tun_rx_dst(struct sk_buff *skb,
>  						 int md_size)
>  {
>  	const struct iphdr *iph = ip_hdr(skb);
> +	struct metadata_dst *tun_dst;
> +
> +	tun_dst = __ip_tun_set_dst(iph->saddr, iph->daddr, iph->tos, iph->ttl,
> +				   0, flags, tunnel_id, md_size);
>  
> -	return __ip_tun_set_dst(iph->saddr, iph->daddr, iph->tos, iph->ttl,
> -				0, flags, tunnel_id, md_size);
> +	if (iph->frag_off & htons(IP_DF))
> +		__set_bit(IP_TUNNEL_DONT_FRAGMENT_BIT,
> +			  tun_dst->u.tun_info.key.tun_flags);

Shouldn't you check that tun_dst isn't NULL?

> +	return tun_dst;
>  }

Re: [PATCH net 1/2] net: dst_metadata: fix IP_DF bit not extracted from tunnel headers

[Ilya Maximets]

On 9/9/25 10:41 AM, Ido Schimmel wrote:
> On Fri, Sep 05, 2025 at 03:30:55PM +0200, Ilya Maximets wrote:
>> @@ -220,9 +221,15 @@ static inline struct metadata_dst *ip_tun_rx_dst(struct sk_buff *skb,
>>  						 int md_size)
>>  {
>>  	const struct iphdr *iph = ip_hdr(skb);
>> +	struct metadata_dst *tun_dst;
>> +
>> +	tun_dst = __ip_tun_set_dst(iph->saddr, iph->daddr, iph->tos, iph->ttl,
>> +				   0, flags, tunnel_id, md_size);
>>  
>> -	return __ip_tun_set_dst(iph->saddr, iph->daddr, iph->tos, iph->ttl,
>> -				0, flags, tunnel_id, md_size);
>> +	if (iph->frag_off & htons(IP_DF))
>> +		__set_bit(IP_TUNNEL_DONT_FRAGMENT_BIT,
>> +			  tun_dst->u.tun_info.key.tun_flags);
> 
> Shouldn't you check that tun_dst isn't NULL?

That's true.  It should be:  if (tun_dst && ...)
I had too many versions of this change and lost the check in the end.

Will wait a bit and then send a v2.

Thanks!

Best regards, Ilya Maximets.

Re: [PATCH net 2/2] selftests: openvswitch: add a simple test for tunnel metadata

[Aaron Conole]

Ilya Maximets <i.maximets@ovn.org> writes:

> This test ensures that upon receiving decapsulated packets from a
> tunnel interface in openvswitch, the tunnel metadata fields are
> properly populated.  This partially covers interoperability of the
> kernel tunnel ports and openvswitch tunnels (LWT) and parsing and
> formatting of the tunnel metadata fields of the openvswitch netlink
> uAPI.  Doing so, this test also ensures that fields and flags are
> properly extracted during decapsulation by the tunnel core code,
> serving as a regression test for the previously fixed issue with the
> DF bit not being extracted from the outer IP header.
>
> The ovs-dpctl.py script already supports all that is necessary for
> the tunnel ports for this test, so we only need to adjust the
> ovs_add_if() function to pass the '-t' port type argument in order
> to be able to create tunnel ports in the openvswitch datapath.
>
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> ---

Thanks for the test case - it looks good to me.  One thing I think would
be a useful future enhancement for us is to add json output so that we
can use something like 'jq' to parse the upcall output.  At the moment,
the grep will work, but doesn't let us change anything.  But that is
work for a future patch.

Reviewed-by: Aaron Conole <aconole@redhat.com>

>  .../selftests/net/openvswitch/openvswitch.sh  | 88 +++++++++++++++++--
>  1 file changed, 81 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> index 3c8d3455d8e7..b327d3061ed5 100755
> --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
> +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> @@ -25,6 +25,7 @@ tests="
>  	nat_related_v4				ip4-nat-related: ICMP related matches work with SNAT
>  	netlink_checks				ovsnl: validate netlink attrs and settings
>  	upcall_interfaces			ovs: test the upcall interfaces
> +	tunnel_metadata				ovs: test extraction of tunnel metadata
>  	drop_reason				drop: test drop reasons are emitted
>  	psample					psample: Sampling packets with psample"
>  
> @@ -113,13 +114,13 @@ ovs_add_dp () {
>  }
>  
>  ovs_add_if () {
> -	info "Adding IF to DP: br:$2 if:$3"
> -	if [ "$4" != "-u" ]; then
> -		ovs_sbx "$1" python3 $ovs_base/ovs-dpctl.py add-if "$2" "$3" \
> -		    || return 1
> +	info "Adding IF to DP: br:$3 if:$4 ($2)"
> +	if [ "$5" != "-u" ]; then
> +		ovs_sbx "$1" python3 $ovs_base/ovs-dpctl.py add-if \
> +		    -t "$2" "$3" "$4" || return 1
>  	else
>  		python3 $ovs_base/ovs-dpctl.py add-if \
> -		    -u "$2" "$3" >$ovs_dir/$3.out 2>$ovs_dir/$3.err &
> +		    -u -t "$2" "$3" "$4" >$ovs_dir/$4.out 2>$ovs_dir/$4.err &
>  		pid=$!
>  		on_exit "ovs_sbx $1 kill -TERM $pid 2>/dev/null"
>  	fi
> @@ -166,9 +167,9 @@ ovs_add_netns_and_veths () {
>  	fi
>  
>  	if [ "$7" != "-u" ]; then
> -		ovs_add_if "$1" "$2" "$4" || return 1
> +		ovs_add_if "$1" "netdev" "$2" "$4" || return 1
>  	else
> -		ovs_add_if "$1" "$2" "$4" -u || return 1
> +		ovs_add_if "$1" "netdev" "$2" "$4" -u || return 1
>  	fi
>  
>  	if [ $TRACING -eq 1 ]; then
> @@ -756,6 +757,79 @@ test_upcall_interfaces() {
>  	return 0
>  }
>  
> +ovs_add_kernel_tunnel() {
> +	local sbxname=$1; shift
> +	local ns=$1; shift
> +	local tnl_type=$1; shift
> +	local name=$1; shift
> +	local addr=$1; shift
> +
> +	info "setting up kernel ${tnl_type} tunnel ${name}"
> +	ovs_sbx "${sbxname}" ip -netns ${ns} link add dev ${name} type ${tnl_type} $* || return 1
> +	on_exit "ovs_sbx ${sbxname} ip -netns ${ns} link del ${name} >/dev/null 2>&1"
> +	ovs_sbx "${sbxname}" ip -netns ${ns} addr add dev ${name} ${addr} || return 1
> +	ovs_sbx "${sbxname}" ip -netns ${ns} link set dev ${name} mtu 1450 up || return 1
> +}
> +
> +test_tunnel_metadata() {
> +	which arping >/dev/null 2>&1 || return $ksft_skip
> +
> +	sbxname="test_tunnel_metadata"
> +	sbx_add "${sbxname}" || return 1
> +
> +	info "setting up new DP"
> +	ovs_add_dp "${sbxname}" tdp0 -V 2:1 || return 1
> +
> +	ovs_add_netns_and_veths "${sbxname}" tdp0 tns left0 l0 \
> +		172.31.110.1/24 || return 1
> +
> +	info "removing veth interface from openvswitch and setting IP"
> +	ovs_del_if "${sbxname}" tdp0 left0 || return 1
> +	ovs_sbx "${sbxname}" ip addr add 172.31.110.2/24 dev left0 || return 1
> +	ovs_sbx "${sbxname}" ip link set left0 up || return 1
> +
> +	info "setting up tunnel port in openvswitch"
> +	ovs_add_if "${sbxname}" "vxlan" tdp0 ovs-vxlan0 -u || return 1
> +	on_exit "ovs_sbx ${sbxname} ip link del ovs-vxlan0"
> +	ovs_wait ip link show ovs-vxlan0 &>/dev/null || return 1
> +	ovs_sbx "${sbxname}" ip link set ovs-vxlan0 up || return 1
> +
> +	configs=$(echo '
> +	    1 172.31.221.1/24 1155332 32   set   udpcsum flags\(df\|csum\)
> +	    2 172.31.222.1/24 1234567 45   set noudpcsum flags\(df\)
> +	    3 172.31.223.1/24 1020304 23 unset   udpcsum flags\(csum\)
> +	    4 172.31.224.1/24 1357986 15 unset noudpcsum' | sed '/^$/d')
> +
> +	while read -r i addr id ttl df csum flags; do
> +		ovs_add_kernel_tunnel "${sbxname}" tns vxlan vxlan${i} ${addr} \
> +			remote 172.31.110.2 id ${id} dstport 4789 \
> +			ttl ${ttl} df ${df} ${csum} || return 1
> +	done <<< "${configs}"
> +
> +	ovs_wait grep -q 'listening on upcall packet handler' \
> +		${ovs_dir}/ovs-vxlan0.out || return 1
> +
> +	info "sending arping"
> +	for i in 1 2 3 4; do
> +		ovs_sbx "${sbxname}" ip netns exec tns \
> +			arping -I vxlan${i} 172.31.22${i}.2 -c 1 \
> +			>${ovs_dir}/arping.stdout 2>${ovs_dir}/arping.stderr
> +	done
> +
> +	info "checking that received decapsulated packets carry correct metadata"
> +	while read -r i addr id ttl df csum flags; do
> +		arp_hdr="arp\\(sip=172.31.22${i}.1,tip=172.31.22${i}.2,op=1,sha="
> +		addrs="src=172.31.110.1,dst=172.31.110.2"
> +		ports="tp_src=[0-9]*,tp_dst=4789"
> +		tnl_md="tunnel\\(tun_id=${id},${addrs},ttl=${ttl},${ports},${flags}\\)"
> +
> +		ovs_sbx "${sbxname}" grep -qE "MISS upcall.*${tnl_md}.*${arp_hdr}" \
> +			${ovs_dir}/ovs-vxlan0.out || return 1
> +	done <<< "${configs}"
> +
> +	return 0
> +}
> +
>  run_test() {
>  	(
>  	tname="$1"