[xdp-hints] Re: [PATCH bpf-next v12 5/5] igc: Add launch time support to XDP ZC

["Song, Yoong Siang" <yoong.siang.song@intel.com>]

On Friday, March 7, 2025 9:25 PM, Bouska, Zdenek <zdenek.bouska@siemens.com> wrote:

[...]

>> @@ -2996,7 +3035,13 @@ static void igc_xdp_xmit_zc(struct igc_ring *ring)
>>  	ntu = ring->next_to_use;
>>  	budget = igc_desc_unused(ring);
>>
>> -	while (xsk_tx_peek_desc(pool, &xdp_desc) && budget--) {
>> +	/* Packets with launch time require one data descriptor and one context
>> +	 * descriptor. When the launch time falls into the next Qbv cycle, we
>> +	 * may need to insert an empty packet, which requires two more
>> +	 * descriptors. Therefore, to be safe, we always ensure we have at least
>> +	 * 4 descriptors available.
>> +	 */
>> +	while (xsk_tx_peek_desc(pool, &xdp_desc) && budget >= 4) {
>
>I think that here is a bug: some frames could be missed if budget < 4.
>I was able to reproduce it by sending 100000x 60 B frames with minimal IPG
>(672 ns between starts of frames) on 1Gbit/s. Always 1026 frames were not sent
>and were missing a AF_XDP competition. Interesting was that then even when I
>sent more
>frames for hours it still was 1026 frames not sent and missing competition.
>
>Bug seems to be fixed when I change this line to:
>
>	while (budget >= 4 && xsk_tx_peek_desc(pool, &xdp_desc)) {
>
>Do you think this is a good fix?
>
>I think this bug is also in original code base, but I was only able to reproduce
>it with launch time.
>

Thank you for pointing out this issue and for providing a detailed
explanation of your findings. I personally agree with your proposed fix
that make sure there is enough budget in the driver, before go peek the xsk
descriptor. Do you plan to submit bug fix patch to iwl-net?

Best regards,
Siang