Re: [PATCH v1 net-next 3/3] geneve: Support per-netns netdev unregistration.

[Ilya Maximets <i.maximets@ovn.org>]

On 8/4/26 5:24 PM, Kuniyuki Iwashima wrote:
> On Tue, Aug 4, 2026 at 6:47 AM Paolo Abeni <pabeni@redhat.com> wrote:
>>
>> On 7/31/26 6:45 PM, Kuniyuki Iwashima wrote:
>>> geneve_exit_rtnl_net() iterates geneve devices whose sockets
>>> are in the dying netns and queues them for destruction.
>>>
>>> So the devices may reside in different netns.
>>>
>>> Let's use unregister_netdevice_queue_net() to support per-netns
>>> device unregistration.
>>>
>>> list_del() is changed to list_del_init() to avoid queueing the
>>> same device twice.
>>>
>>> Even after geneve_exit_rtnl_net() queues a cross-netns geneve
>>> device, geneve_dellink() can be called concurrently for it.
>>> In such a case, __rtnl_net_unlock() will perform the unregistration.
>>>
>>> Note that geneve uses register_pernet_subsys() instead of _device(),
>>> so default_device_exit_batch() guarantees that the async per-netns
>>> works are flushed before ->exit().
>>>
>>> Tested:
>>>
>>> 1. Create geneve device across two netns.
>>>
>>>   # ip netns add ns1
>>>   # ip netns add ns2
>>>   # ip -n ns1 link add geneve0 link-netns ns2 type geneve external
>>>
>>> 2. Run bpftrace to check that geneve_uninit() is called between
>>>    ->exit_rtnl() and ->exit().
>>>
>>>   # bpftrace -e '#include <linux/netdevice.h>
>>>   kprobe:geneve_uninit {
>>>       $dev = (struct net_device *)arg0;
>>>       printf("PID: %d | DEV: %s%s\n", pid, $dev->name, kstack());
>>>   }
>>>   kprobe:geneve_exit_rtnl_net,
>>>   kprobe:geneve_exit_net {
>>>       printf("PID: %d%s\n", pid, kstack());
>>>   }'
>>>
>>> 3. Remove the netns where the geneve socket resides
>>>
>>>   # ip netns del ns2
>>>
>>> Now, we can see geneve0 is unregistered by per-netns work
>>> instead of cleanup_net() and it finishes before ->exit() to
>>> avoid WARN_ON_ONCE(!list_empty(&gn->sock_list)) there.
>>>
>>>   PID: 571
>>>           geneve_exit_rtnl_net+5
>>>           ops_undo_list+702
>>>           cleanup_net+1122
>>>           process_scheduled_works+2538
>>>   ...
>>>   PID: 1047 | DEV: geneve0
>>>           geneve_uninit+5
>>>           unregister_netdevice_many_notify+7129
>>>           unregister_netdevice_many_net+1050
>>>           rtnl_net_work_func+136
>>>           process_scheduled_works+2538
>>>   ...
>>>   PID: 571
>>>           geneve_exit_net+5
>>>           ops_undo_list+1064
>>>           cleanup_net+1122
>>>           process_scheduled_works+2538
>>>   ...
>>>
>>> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
>>> ---
>>>  drivers/net/geneve.c | 12 +++++++-----
>>>  1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
>>> index f456a85dca77..a6a8978e3b81 100644
>>> --- a/drivers/net/geneve.c
>>> +++ b/drivers/net/geneve.c
>>> @@ -2502,12 +2502,13 @@ static int geneve_changelink(struct net_device *dev, struct nlattr *tb[],
>>>       return err;
>>>  }
>>>
>>> -static void __geneve_dellink(struct net_device *dev, struct list_head *head)
>>> +static void __geneve_dellink(struct net *net, struct net_device *dev,
>>> +                          struct list_head *head)
>>>  {
>>>       struct geneve_dev *geneve = netdev_priv(dev);
>>>
>>> -     list_del(&geneve->next);
>>> -     unregister_netdevice_queue(dev, head);
>>> +     list_del_init(&geneve->next);
>>> +     unregister_netdevice_queue_net(net, dev, head);
>>>  }
>>>
>>>  static void geneve_dellink(struct net_device *dev, struct list_head *head)
>>> @@ -2518,7 +2519,8 @@ static void geneve_dellink(struct net_device *dev, struct list_head *head)
>>>       gn = net_generic(geneve->net, geneve_net_id);
>>>
>>>       mutex_lock(&gn->lock);
>>> -     __geneve_dellink(dev, head);
>>> +     if (!list_empty(&geneve->next))
>>> +             __geneve_dellink(dev_net(dev), dev, head);
>>
>> Sashiko noted that the lockdep chain between dev->lock, utn->loc and
>> gn->lock is not trivial, possibly a documentation follow-up would be useful
>>
>>>       mutex_unlock(&gn->lock);
>>>  }
>>>
>>> @@ -2754,7 +2756,7 @@ static void __net_exit geneve_exit_rtnl_net(struct net *net,
>>>       mutex_lock(&gn->lock);
>>>
>>>       list_for_each_entry_safe(geneve, next, &gn->geneve_list, next)
>>> -             __geneve_dellink(geneve->dev, dev_to_kill);
>>> +             __geneve_dellink(net, geneve->dev, dev_to_kill);
>>
>> Here sashiko foresees some problem with CONFIG_DEBUG_NET_SMALL_RTNL
>> before full conversion to per netns lock even of ovs. Just more
>> follow-up, I guess.
> 
> Is it Sashiko-nipa output ?
> I can't find the comments from patchwork.
> https://patchwork.kernel.org/project/netdevbpf/patch/20260731164612.2148830-2-kuniyu@google.com/
> https://sashiko.dev/#/patchset/20260731164612.2148830-1-kuniyu@google.com
> 
> btw, I was hoping this series would land upstream so that
> I don't need to care about OVS :)
> https://lore.kernel.org/netdev/20260513183559.2141010-1-i.maximets@ovn.org/
FWIW, I have the non-RFC version of this set prepared.  I can post it,
if that helps.

Best regards, Ilya Maximets.