Hi David, Gentle ping on this v2. I believe it addresses your comments on v1 (unconditional L2 padding reset at first_seg, test packets stored in files with Scapy comments).
Ilya, adding you in Cc in case you'd like to take a look as well. Thanks, Mikhail чт, 1 окт. 2026 г. в 16:30, <[email protected]>: > 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. > > Drop the L2 padding from the first segment unconditionally. When > partial segmentation leaves the whole packet to HW segmentation, also > remove the padding bytes, as they must not reach the NIC. The result > of dp_packet_gso_partial_nr_segs() depends on the padding, so evaluate > it only once. 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.") > Suggested-by: David Marchand <[email protected]> > Signed-off-by: Mikhail Dmitrichenko <[email protected]> > --- > > Notes: > v2: > - Drop the L2 padding unconditionally, also when partial segmentation > leaves the whole packet to HW segmentation (David). > - Evaluate dp_packet_gso_partial_nr_segs() only once, as its result > depends on the L2 padding. > - Test: store the packets in files, with Scapy comments (David). > > lib/dp-packet-gso.c | 17 ++++++++-- > tests/dpif-netdev.at | 75 ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 90 insertions(+), 2 deletions(-) > > diff --git a/lib/dp-packet-gso.c b/lib/dp-packet-gso.c > index 8c8817352..78a1ff592 100644 > --- a/lib/dp-packet-gso.c > +++ b/lib/dp-packet-gso.c > @@ -183,7 +183,9 @@ static void > dp_packet_gso__(struct dp_packet *p, struct dp_packet_batch *batch, > bool partial_seg) > { > + bool sw_last_seg = false; > struct dp_packet *seg; > + uint16_t l2_pad_size; > unsigned int n_segs; > uint16_t tso_segsz; > size_t data_len; > @@ -216,7 +218,10 @@ dp_packet_gso__(struct dp_packet *p, struct > dp_packet_batch *batch, > } > > if (partial_seg) { > - if (dp_packet_gso_partial_nr_segs(p) != 1) { > + /* Evaluated only once, as the result depends on the L2 padding > + * which is dropped below. */ > + sw_last_seg = dp_packet_gso_partial_nr_segs(p) != 1; > + if (sw_last_seg) { > goto last_seg; > } > goto first_seg; > @@ -239,13 +244,21 @@ last_seg: > dp_packet_batch_add(batch, seg); > > first_seg: > + /* The L2 padding, if any, follows the TCP payload. Drop it from the > + * first segment: either it is cut off when trimming the segment > below, > + * or it must not be passed to HW segmentation. */ > + 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) { > + if (sw_last_seg) { > dp_packet_set_size(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); > } > + } 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..fee931750 100644 > --- a/tests/dpif-netdev.at > +++ b/tests/dpif-netdev.at > @@ -3333,6 +3333,81 @@ 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. > +AT_DATA([tcp4_frame], m4_join([], > +dnl p = Ether(src='8a:bf:7e:2f:05:84', dst='0a:8f:39:4f:e0:73') > +[0a8f394fe0738abf7e2f05840800], > +dnl p /= IP(src='192.168.123.2', dst='192.168.123.1', tos=1, id=0) > +[45010410000000004006ff92c0a87b02c0a87b01], > +dnl p /= TCP(sport=54392, dport=5201, flags='A', window=0) > +[d47814510000000000000000501000004bce0000], > +dnl p /= Raw(bytes(1000)) > +m4_format([%02000d], [0]) > +)) > + > +AT_DATA([tcp6_frame], m4_join([], > +dnl p = Ether(src='8a:bf:7e:2f:05:84', dst='0a:8f:39:4f:e0:73') > +[0a8f394fe0738abf7e2f058486dd], > +dnl p /= IPv6(src='2001:cafe::88', dst='2001:cafe::92', hlim=0) > > +[6000000003fc06002001cafe0000000000000000000000882001cafe000000000000000000000092], > +dnl p /= TCP(sport=54392, dport=5201, flags='A', window=0) > +[d4781451000000000000000050100000ec090000], > +dnl p /= Raw(bytes(1000)) > +m4_format([%02000d], [0]) > +)) > + > +dnl Append L2 padding shorter (10 bytes) and longer (600 bytes) than the > +dnl TCP header and payload of the first segment. > +pad_short=$(printf '%0*d' 20 0) > +pad_long=$(printf '%0*d' 1200 0) > +AT_CHECK([ovs-appctl netdev-dummy/receive p1 "$(cat > tcp4_frame)${pad_short}"]) > +AT_CHECK([ovs-appctl netdev-dummy/receive p1 "$(cat > tcp4_frame)${pad_long}"]) > +AT_CHECK([ovs-appctl netdev-dummy/receive p1 "$(cat > tcp6_frame)${pad_short}"]) > +AT_CHECK([ovs-appctl netdev-dummy/receive p1 "$(cat > tcp6_frame)${pad_long}"]) > + > +dnl Padding must be dropped: 2x 500 byte payloads per packet. > +zero500=$(printf '%0*d' 1000 0) > +AT_CHECK_UNQUOTED([ovs-pcap p2.pcap], [0], [dnl > +[0a8f394fe0738abf7e2f058408004501021c0000000040060187c0a87b02c0a87b01]dnl > +[d47814510000000000000000501000004dc20000${zero500}] > +[0a8f394fe0738abf7e2f058408004501021c0001000040060186c0a87b02c0a87b01]dnl > +[d4781451000001f400000000501000004bce0000${zero500}] > +[0a8f394fe0738abf7e2f058408004501021c0000000040060187c0a87b02c0a87b01]dnl > +[d47814510000000000000000501000004dc20000${zero500}] > +[0a8f394fe0738abf7e2f058408004501021c0001000040060186c0a87b02c0a87b01]dnl > +[d4781451000001f400000000501000004bce0000${zero500}] > > +[0a8f394fe0738abf7e2f058486dd60000000020806002001cafe0000000000000000000000]dnl > > +[882001cafe000000000000000000000092d4781451000000000000000050100000edfd0000]dnl > +[${zero500}] > > +[0a8f394fe0738abf7e2f058486dd60000000020806002001cafe0000000000000000000000]dnl > > +[882001cafe000000000000000000000092d4781451000001f40000000050100000ec090000]dnl > +[${zero500}] > > +[0a8f394fe0738abf7e2f058486dd60000000020806002001cafe0000000000000000000000]dnl > > +[882001cafe000000000000000000000092d4781451000000000000000050100000edfd0000]dnl > +[${zero500}] > > +[0a8f394fe0738abf7e2f058486dd60000000020806002001cafe0000000000000000000000]dnl > > +[882001cafe000000000000000000000092d4781451000001f40000000050100000ec090000]dnl > +[${zero500}] > +]) > + > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > AT_SETUP([dpif-netdev - tunnel tso fallback]) > AT_KEYWORDS([userspace offload]) > OVS_VSWITCHD_START([set Open_vSwitch . > other_config:userspace-tso-enable=true \ > -- > 2.25.1 > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
