On 9/10/25 4:46 PM, David Marchand wrote:
> TSO packets were incorrectly treated as too big by the check_pkt_len
> action with the userspace datapath.
> Adjust the check by looking at the requested segment size.
>
> Fixes: 29cf9c1b3b9c ("userspace: Add TCP Segmentation Offload support")
> Reported-at: https://issues.redhat.com/browse/FDP-1631
> Signed-off-by: David Marchand <david.march...@redhat.com>
> ---
> Changes since v1:
> - added check on payload presence in packet,
> - updated the added test name for consistency,
> - used helpers for creating bridge and ports in unit tests,
> - added coverage for segmentation size larger than checked packet length,
>
> ---
> lib/odp-execute.c | 19 +++++++++++++++
> tests/dpif-netdev.at | 57 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 76 insertions(+)
>
> diff --git a/lib/odp-execute.c b/lib/odp-execute.c
> index 649f8e99a5..7f4e337f8a 100644
> --- a/lib/odp-execute.c
> +++ b/lib/odp-execute.c
> @@ -797,6 +797,25 @@ odp_execute_check_pkt_len(void *dp, struct dp_packet
> *packet, bool steal,
> uint32_t size = dp_packet_get_send_len(packet)
> - dp_packet_l2_pad_size(packet);
>
> + if (dp_packet_get_tso_segsz(packet)) {
> + const void *payload;
> + uint32_t segsize;
> +
> + if (dp_packet_tunnel(packet)) {
> + payload = dp_packet_get_inner_tcp_payload(packet);
> + } else {
> + payload = dp_packet_get_tcp_payload(packet);
> + }
> + if (payload) {
> + /* Evaluate a segment maximum length for this TSO packet. */
> + segsize = (char *) payload - (char *) dp_packet_data(packet)
> + + dp_packet_get_tso_segsz(packet);
> + if (segsize < size) {
> + size = segsize;
> + }
> + }
> + }
> +
> a = attrs[OVS_CHECK_PKT_LEN_ATTR_PKT_LEN];
> if (size > nl_attr_get_u16(a)) {
> a = attrs[OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER];
> diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
> index 6804a9c5c9..d1300ec67d 100644
> --- a/tests/dpif-netdev.at
> +++ b/tests/dpif-netdev.at
> @@ -2245,6 +2245,63 @@ AT_CHECK_UNQUOTED([ovs-pcap p1.pcap | sort], [0], [dnl
> OVS_VSWITCHD_STOP
> AT_CLEANUP
>
> +AT_SETUP([userspace offload - tso + check_pkt_len])
> +OVS_VSWITCHD_START(
> + [set Open_vSwitch . other_config:userspace-tso-enable=true])
> +add_of_br 1
We don't really need another bridge, br0 is there by default.
> +add_of_ports --pcap br1 `seq 1 3`
> +
> +AT_CHECK([ovs-appctl vlog/set netdev_dummy:file:dbg])
> +
> +AT_DATA([flows.txt], [dnl
> +table=0,in_port=1,ip
> actions=check_pkt_larger(514)->NXM_NX_REG0[[0]],resubmit(,1)
> +table=1,ip,reg0=0x0 actions=output:2
> +table=1,ip,reg0=0x1 actions=output:3
> +])
> +AT_CHECK([ovs-ofctl del-flows br1])
> +AT_CHECK([ovs-ofctl add-flows br1 flows.txt])
> +
> +flow_s="in_port(1),eth(src=8a:bf:7e:2f:05:84,dst=0a:8f:39:4f:e0:73),eth_type(0x0800),
> \
> +
> ipv4(src=192.168.123.2,dst=192.168.123.1,proto=6,tos=1,ttl=64,frag=no), \
> + tcp(src=54392,dst=5201),tcp_flags(ack)"
> +
> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 "${flow_s}"])
> +
> +AT_CHECK([ovs-appctl dpctl/dump-flows | strip_used | strip_stats], [0], [dnl
> +flow-dump from the main thread:
> +recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
> packets:0, bytes:0, used:never, actions:check_pkt_len(size=514,gt(3),le(2))
> +])
> +
> +AT_CHECK([ovs-pcap p2-tx.pcap | wc -l], [0], [1
> +])
> +AT_CHECK([ovs-pcap p3-tx.pcap | wc -l], [0], [0
> +])
> +
> +AT_CHECK([ovs-vsctl set Interface p1 options:ol_tso_segsz=460])
> +
> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 "${flow_s}"])
> +AT_CHECK([ovs-pcap p2-tx.pcap | wc -l], [0], [2
> +])
> +AT_CHECK([ovs-pcap p3-tx.pcap | wc -l], [0], [0
> +])
> +
> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 "${flow_s}" --len 974])
> +AT_CHECK([ovs-pcap p2-tx.pcap | wc -l], [0], [4
> +])
> +AT_CHECK([ovs-pcap p3-tx.pcap | wc -l], [0], [0
> +])
> +
> +AT_CHECK([ovs-vsctl set Interface p1 options:ol_tso_segsz=500])
I think, 462 is a more appropriate value here to test the boundaries.
It's just above the threshold and the second segment will still fit,
but should be forwarded along with the first one.
I made these two changes to save some iterations and applied the fix.
Backported down to 3.3. Thanks!
Note: backporting this kind of changes is a bit of a pain due to each
branch having different implementation of offloads.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
