netdev - Re: [PATCH net-next v4 01/10] tsnep: Use spin_lock

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: <f4c818ba6cb5ac6f34bd73a67b18c5cd9da5f42a.camel@gmail.com>
Date:   Tue, 10 Jan 2023 07:49:50 -0800
From:   Alexander H Duyck <alexander.duyck@...il.com>
To:     Gerhard Engleder <gerhard@...leder-embedded.com>,
        netdev@...r.kernel.org
Cc:     davem@...emloft.net, kuba@...nel.org, edumazet@...gle.com,
        pabeni@...hat.com
Subject: Re: [PATCH net-next v4 01/10] tsnep: Use spin_lock_bh for TX

On Mon, 2023-01-09 at 20:15 +0100, Gerhard Engleder wrote:
> TX processing is done only within BH context. Therefore, _irqsafe
> variant is not necessary.
> 
> Signed-off-by: Gerhard Engleder <gerhard@...leder-embedded.com>

Rather than reducing the context of this why not drop it completely? It
doesn't look like you are running with the NETIF_F_LLTX flag set so
from what I can tell it looks like you are taking the Tx lock in the
xmit path. So Tx queues are protected with the Tx queue lock at the
netdev level via the HARD_TX_LOCK macro.

Since it is already being used in the Tx path to protect multiple
access you could probably just look at getting rid of it entirely.

The only caveat you would need to watch out for is a race between the
cleaning and transmitting which can be addressed via a few barriers
like what was done in the intel drivers via something like the
__ixgbe_maybe_stop_tx function and the logic to wake the queue in the
clean function.

Alternatively if you really feel you need this in the non-xmit path
functions you could just drop the lock and instead use __netif_tx_lock
for those spots that are accessing the queue outside the normal
transmit path.