linux-kernel - Re: [RFC/RFT PATCH net-next v3 1/2] net: Add ndo_write_rx

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: <CAPrAcgMXw5e6mi1tU=c7UaQdcKZhhC8j-y9woQk0bk8SeV4+8A@mail.gmail.com>
Date: Thu, 6 Nov 2025 21:38:59 +0530
From: I Viswanath <viswanathiyyappan@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com, 
	horms@...nel.org, sdf@...ichev.me, kuniyu@...gle.com, ahmed.zaki@...el.com, 
	aleksander.lobakin@...el.com, jacob.e.keller@...el.com, 
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org, 
	skhan@...uxfoundation.org, linux-kernel-mentees@...ts.linux.dev, 
	david.hunter.linux@...il.com, khalid@...nel.org
Subject: Re: [RFC/RFT PATCH net-next v3 1/2] net: Add ndo_write_rx_config and
 helper structs and functions:

On Wed, 5 Nov 2025 at 06:16, Jakub Kicinski <kuba@...nel.org> wrote:
> I wouldn't use atomic flags. IIRC ndo_set_rx_mode is called under
> netif_addr_lock_bh(), so we can reuse that lock, have update_config()
> assume ownership of the pending config and update it directly.
> And read_config() (which IIUC runs from a wq) can take that lock
> briefly, and swap which config is pending.

How does this look?

It's possible for the actual work of set_rx_mode to be in a work item
so we need to validate that dev->addr_list_lock is held in update_config()

// These variables will be part of dev->netif_rx_config_ctx in the final code
bool pending_cfg_ready = false;
struct netif_rx_config *ready, *pending;

void update_config()
{
    WARN_ONCE(!spin_is_locked(&dev->addr_list_lock),
    "netif_update_rx_config() called without netif_addr_lock_bh()\n");

    int rc = netif_prepare_rx_config(&pending);
    if (rc)
        return;

    pending_cfg_ready = true;
}

void read_config()
{
    // We could introduce a new lock for this but
    // reusing the addr lock works well enough
    netif_addr_lock_bh();

    // There's no point continuing if the pending config
    // is not ready
    if(!pending_cfg_ready) {
       netif_addr_unlock_bh();
       return;
    }

    swap(ready, pending);
    pending_cfg_ready = false;

    netif_addr_unlock_bh();

    do_io(ready);
}

On the topic of virtio_net:

set_rx_mode in virtio_net schedules and does the actual work in a work
item, so would
the correct justification here be moving I/O out of the rtnl lock?

If this looks good, I will start working on v4