On Thu, 6 Nov 2025 21:38:59 +0530 I Viswanath wrote:

On Wed, 5 Nov 2025 at 06:16, Jakub Kicinski [off-list ref] wrote:

quoted

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);
}

Yes, I think this flow looks good.

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?

Avoiding rtnl_lock is not a goal right now. We should still take
rtnl_lock around read_config() in case the driver assumes rtnl_lock
is held around all its config paths.

The objective is just to simplify the driver. Avoid the state
management and work scheduling in every driver that needs to the config
async. IOW we just want to give drivers an ndo_set_rx_mode that can
sleep.