Hello,
On Wed, 30 Sept 2026 at 15:18, Mikhail Dmitrichenko via dev
<[email protected]> wrote:
>
> From: Mikhail Dmitrichenko <[email protected]>
>
> Software segmentation reuses the original packet as the first segment
> and trims it with dp_packet_set_size(). Any L2 padding follows the TCP
> payload, so it is cut off by the trim, but 'l2_pad_size' is left as is.
>
> dp_packet_l3_size(), dp_packet_l4_size() and their inner variants
> subtract 'l2_pad_size' from the packet size, so for the first segment
> they under-report the actual length by the size of the former padding.
> As a result, dp_packet_gso_update_segment() writes a wrong IPv4 total
> length or IPv6 payload length, and the TCP checksum computed later in
> software does not cover the whole segment.
>
> If the padding is longer than the TCP header and 'tso_segsz' together,
> these sizes wrap around. The IPv6 payload length becomes bogus and
> packet_tcp_complete_csum() reads past the end of the packet buffer:
> up to 64 KB for IPv6 and until it faults for IPv4, which crashes
> ovs-vswitchd:
>
> SIGSEGV detected, backtrace:
> csum_continue at lib/csum.c:46
> packet_tcp_complete_csum at lib/packets.c:2120
> dp_packet_ol_send_prepare at lib/dp-packet.c:601
> netdev_send at lib/netdev.c:845
> dp_netdev_pmd_flush_output_on_port at lib/dpif-netdev.c:4402
>
> Such a packet can be received on a port with userspace TSO enabled when
> the IP length is shorter than the frame, e.g. from a VM, and be sent to
> a port without TSO support.
>
> Reset 'l2_pad_size' when trimming the first segment. The other segments
> are freshly allocated and never carry padding.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: ef762327f6d3 ("dp-packet-gso: Refactor software segmentation code.")
> Signed-off-by: Mikhail Dmitrichenko <[email protected]>
> ---
> lib/dp-packet-gso.c | 14 ++++++++--
> tests/dpif-netdev.at | 61 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 73 insertions(+), 2 deletions(-)
>
> diff --git a/lib/dp-packet-gso.c b/lib/dp-packet-gso.c
> index 8c8817352..b89b54078 100644
> --- a/lib/dp-packet-gso.c
> +++ b/lib/dp-packet-gso.c
> @@ -179,6 +179,15 @@ dp_packet_gso_update_segment(struct dp_packet *seg,
> unsigned int seg_no,
> }
> }
>
> +/* Trims the first segment 'p' to 'size' bytes. The L2 padding, if any,
> + * follows the TCP payload and is cut off along with it. */
> +static void
> +dp_packet_gso_trim_first_seg(struct dp_packet *p, size_t size)
> +{
> + dp_packet_set_size(p, size);
> + dp_packet_set_l2_pad_size(p, 0);
> +}
> +
> static void
> dp_packet_gso__(struct dp_packet *p, struct dp_packet_batch *batch,
> bool partial_seg)
> @@ -241,7 +250,8 @@ last_seg:
> first_seg:
> if (partial_seg) {
> if (dp_packet_gso_partial_nr_segs(p) != 1) {
> - dp_packet_set_size(p, hdr_len + (n_segs - 1) * tso_segsz);
> + dp_packet_gso_trim_first_seg(p,
> + hdr_len + (n_segs - 1) * tso_segsz);
> if (n_segs == 2) {
> /* No need to ask HW segmentation, we already did the job. */
> dp_packet_set_tso_segsz(p, 0);
> @@ -249,7 +259,7 @@ first_seg:
> }
> } else {
> /* Trim the first segment and reset TSO. */
> - dp_packet_set_size(p, hdr_len + tso_segsz);
> + dp_packet_gso_trim_first_seg(p, hdr_len + tso_segsz);
> dp_packet_set_tso_segsz(p, 0);
> }
> dp_packet_gso_update_segment(p, 0, n_segs, tso_segsz, udp_tnl, gre_tnl);
The current fix looks correct, though I would prefer a different form.
Having such a helper leaves a special case for the partial
segmentation + l4 size is a multiple of tso segment size.
In this case, L2 padding is not supposed to survive HW TSO anyway.
So it seems more robust to unconditionnally drop the l2 padding from the start.
Something like (probaby needs comments..):
diff --git a/lib/dp-packet-gso.c b/lib/dp-packet-gso.c
index 8c88173520..72597845a3 100644
--- a/lib/dp-packet-gso.c
+++ b/lib/dp-packet-gso.c
@@ -184,6 +184,7 @@ dp_packet_gso__(struct dp_packet *p, struct
dp_packet_batch *batch,
bool partial_seg)
{
struct dp_packet *seg;
+ uint16_t l2_pad_size;
unsigned int n_segs;
uint16_t tso_segsz;
size_t data_len;
@@ -239,6 +240,9 @@ last_seg:
dp_packet_batch_add(batch, seg);
first_seg:
+ l2_pad_size = dp_packet_l2_pad_size(p);
+ dp_packet_set_l2_pad_size(p, 0);
+
if (partial_seg) {
if (dp_packet_gso_partial_nr_segs(p) != 1) {
dp_packet_set_size(p, hdr_len + (n_segs - 1) * tso_segsz);
@@ -246,6 +250,8 @@ first_seg:
/* No need to ask HW segmentation, we already did the job. */
dp_packet_set_tso_segsz(p, 0);
}
+ } else {
+ dp_packet_set_size(p, dp_packet_size(p) - l2_pad_size);
}
} else {
/* Trim the first segment and reset TSO. */
> diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
> index 14f238e62..cc4b1d57a 100644
> --- a/tests/dpif-netdev.at
> +++ b/tests/dpif-netdev.at
> @@ -3333,6 +3333,67 @@ AT_CHECK_UNQUOTED([ovs-pcap p2.pcap], [0], [dnl
> OVS_VSWITCHD_STOP
> AT_CLEANUP
>
> +AT_SETUP([dpif-netdev - tso + l2 padding])
> +AT_KEYWORDS([userspace offload])
> +OVS_VSWITCHD_START(
> + [set Open_vSwitch . other_config:userspace-tso-enable=true -- \
> + add-br br1 -- set bridge br1 datapath-type=dummy -- \
> + add-port br1 p1 -- \
> + set Interface p1 type=dummy -- \
> + add-port br1 p2 -- \
> + set Interface p2 type=dummy])
> +
> +AT_CHECK([ovs-ofctl add-flow br1 in_port=p1,actions=output:p2])
> +
> +AT_CHECK([ovs-vsctl set Interface p2 options:tx_pcap=p2.pcap -- \
> + set Interface p1 options:ol_tso_segsz=500])
> +
> +dnl IPv4/TCP and IPv6/TCP packets with 1000 bytes of payload, to be
> +dnl followed by L2 padding.
> +zero500=$(printf '%0*d' 1000 0)
> +pkt4="0a8f394fe0738abf7e2f0584080045010410000000004006000"
> +pkt4="${pkt4}0c0a87b02c0a87b01d47814510000000000000000501000000000"
> +pkt4="${pkt4}0000${zero500}${zero500}"
> +pkt6="0a8f394fe0738abf7e2f058486dd6000000003fc06002001cafe00000000000000"
> +pkt6="${pkt6}00000000882001cafe000000000000000000000092d47814510000000000000000"
> +pkt6="${pkt6}5010000000000000${zero500}${zero500}"
I prefer to store generated packets in some file, so it is easier to
reproduce/debug the unit tests.
And a little more comments on the packet content, so that someone who
needs to change it can quickly regenerate.
For example in the current patch, the first line stops at an odd offset (51?).
There are other similar crafted packets in dpif-netdev.at, look for
AT_DATA([good_frame] for example (note that you may still use the
$zero500 in there).
--
David Marchand
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
