On 8 Jan 2026, at 10:38, David Marchand wrote:
> On Thu, 8 Jan 2026 at 09:37, Eelco Chaudron <[email protected]> wrote:
>> On 15 Dec 2025, at 14:57, David Marchand via dev wrote:
>>
>>> If we can predict that the segmented traffic will have the same headers,
>>> it is possible to rely on HW segmentation.
>>>
>>> TCP over IPv4 geneve (with checksum on tunnel) on a mlx5 nic:
>>> Before: 7.80 Gbits/sec, 100% cpu (SW segmentation)
>>> After: 10.8 Gbits/sec, 27% cpu (HW segmentation)
>>>
>>> For this optimisation, no touching of original tso_segsz should be
>>> allowed, so revert the commit
>>> 844a7cfa6edd ("netdev-dpdk: Use guest TSO segmentation size hint.").
>>>
>>> Yet, there might still be some non optimal cases where the L4 payload
>>> size would not be a multiple of tso_segsz.
>>> For this condition, "partially" segment: split the "last" segment and keep
>>> n-1 segments data in the original packet which will be segmented by HW.
>>>
>>
>> Hi David,
>>
>> Coverity reports an issue with this series. The abstract is below, and I
>> think a simple cast could fix the sign problemns, for example:
>
> I'll have a try on the sign extension warning and post a patch.
ACK
>>
>> dp_packet_set_size(p, hdr_len +
>> (size_t)(n_segs - 1) * tso_segsz);
>>
>> The memory access ones, I did not investigate. Could you check and send a
>> patch with fixes if needed?
>
> [snip]
>
>> ** CID 556378: Memory - illegal accesses (OVERRUN)
>>
>>
>> _____________________________________________________________________________________________
>> *** CID 556378: Memory - illegal accesses (OVERRUN)
>> /lib/packets.c: 2154 in packet_udp_tunnel_csum()
>> 2148
>> 2149 inner_csum = packet_csum_pseudoheader6(inner_ip6);
>> 2150 }
>> 2151
>> 2152 inner_csum = csum_continue(inner_csum, inner_l4,
>> 2153 (char *) inner_l4_csum_p - (char *) inner_l4);
>>>>> CID 556378: Memory - illegal accesses (OVERRUN)
>>>>> Overrunning array of 2 bytes at byte offset 2 by dereferencing
>>>>> pointer "inner_l4_csum_p + 1".
>> 2154 inner_l4_csum = csum_finish(csum_continue(inner_csum,
>> inner_l4_csum_p + 1,
>> 2155 (char *) inner_l4_data - (char *)(inner_l4_csum_p + 1)));
>> 2156 /* Important: for inner UDP, a null inner_l4_csum here should
>> in theory be
>> 2157 * replaced with 0xffff. However, since the only use of
>> inner_l4_csum is
>> 2158 * for the final outer checksum with a csum_add16() below, we
>> can skip this
>> 2159 * entirely because adding 0xffff will have the same effect as
>> adding 0x0
>
> Hum, this comes from the fact that inner_l4_csum_p points at an
> explicit uint16_t field in the udp header.
> I will try and see if switching to some pointer arithmetics
> (referencing udp header + offsetof csum) pleases coverity...
Guess doing math on uint8_t might work. Good luck...
>> ** CID 556377: Memory - illegal accesses (USE_AFTER_FREE)
>>
>>
>> _____________________________________________________________________________________________
>> *** CID 556377: Memory - illegal accesses (USE_AFTER_FREE)
>> /lib/dp-packet-gso.c: 207 in dp_packet_gso__()
>> 201 dp_packet_batch_add(curr_batch, p);
>> 202
>> 203 if (n_segs == 1) {
>> 204 goto out;
>> 205 }
>> 206
>>>>> CID 556377: Memory - illegal accesses (USE_AFTER_FREE)
>>>>> Calling "dp_packet_tunnel" dereferences freed pointer "p".
>> 207 if (dp_packet_tunnel(p)) {
>> 208 hdr_len = (char *) dp_packet_get_inner_tcp_payload(p)
>> 209 - (char *) dp_packet_eth(p);
>> 210 data_len = dp_packet_get_inner_tcp_payload_length(p);
>> 211 } else {
>> 212 hdr_len = (char *) dp_packet_get_tcp_payload(p)
>
> This one is a false positive.
>
> 3. Condition dp_packet_batch_is_full(curr_batch), taking true branch.
> 198 if (dp_packet_batch_is_full(curr_batch)) {
> 199 curr_batch++;
> 200 }
>
> 4. freed_arg: dp_packet_batch_add frees p.[hide details]
> 201 dp_packet_batch_add(curr_batch, p);
>
>
> Coverity fails to understand that we switched to a new empty batch.
>
> Can you waive CID 556377?
Thanks for the detailed; waived it!
> Thanks for the report.
>
>
> --
> David Marchand
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
