netdev - Re: [PATCH v2] xen/netfront: Fix TX response spurious interrupts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4f54ed1a-e265-43db-b4f2-f3c0d3b3dd00@suse.com>
Date: Fri, 18 Jul 2025 09:19:17 +0200
From: Jürgen Groß <jgross@...e.com>
To: Jakub Kicinski <kuba@...nel.org>,
 Anthoine Bourgeois <anthoine.bourgeois@...es.tech>
Cc: Stefano Stabellini <sstabellini@...nel.org>,
 Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>,
 Wei Liu <wei.liu@...nel.org>, Paul Durrant <paul@....org>,
 xen-devel@...ts.xenproject.org, netdev@...r.kernel.org,
 Elliott Mitchell <ehem+xen@....com>
Subject: Re: [PATCH v2] xen/netfront: Fix TX response spurious interrupts

On 17.07.25 16:29, Jakub Kicinski wrote:
> On Tue, 15 Jul 2025 16:11:29 +0000 Anthoine Bourgeois wrote:
>> Fixes: b27d47950e48 ("xen/netfront: harden netfront against event channel storms")
> 
> Not entirely sure who you expect to apply this patch, but if networking
> then I wouldn't classify this is a fix. The "regression" happened 4
> years ago. And this patch doesn't seem to be tuning the logic added by
> the cited commit. I think this is an optimization, -next material, and
> therefore there should be no Fixes tag here. You can refer to the commit
> without the tag.

I think in the end it is a fix of the initial xen-netfront.c contribution
(commit 0d160211965b).

I'm fine to change the Fixes: tag and apply the patch via the Xen tree.

> 
>> @@ -849,9 +847,6 @@ static netdev_tx_t xennet_start_xmit(struct sk_buff *skb, struct net_device *dev
>>   	tx_stats->packets++;
>>   	u64_stats_update_end(&tx_stats->syncp);
>>   
>> -	/* Note: It is not safe to access skb after xennet_tx_buf_gc()! */
>> -	xennet_tx_buf_gc(queue);
>> -
>>   	if (!netfront_tx_slot_available(queue))
>>   		netif_tx_stop_queue(netdev_get_tx_queue(dev, queue->id));
> 
> I thought normally reaping completions from the Tx path is done
> to prevent the queue from filling up, when the device-generated
> completions are slow or the queue is short. I say "normally" but
> this is relatively a uncommon thing to do in networking.
> Maybe it's my lack of Xen knowledge but it would be good to add to
> the commit message why these calls where here in the first place.

I guess the reason for this addition is unknown (singular, as the XDP related
one was probably just a copy-and-paste), as it has been there since the first
version of the driver.


Juergen

Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3684 bytes)

Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (496 bytes)