RE: [PATCH net] hv_netvsc: Fix VF namespace also in netvsc_open

[Haiyang Zhang]

> -----Original Message-----
> From: Stephen Hemminger <stephen@xxxxxxxxxxxxxxxxxx>
> Sent: Thursday, October 3, 2024 11:49 AM
> To: Paolo Abeni <pabeni@xxxxxxxxxx>
> Cc: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>; linux-hyperv@xxxxxxxxxxxxxxx;
> netdev@xxxxxxxxxxxxxxx; KY Srinivasan <kys@xxxxxxxxxxxxx>;
> wei.liu@xxxxxxxxxx; Dexuan Cui <decui@xxxxxxxxxxxxx>;
> edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; davem@xxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH net] hv_netvsc: Fix VF namespace also in netvsc_open
> 
> On Thu, 3 Oct 2024 11:34:49 +0200
> Paolo Abeni <pabeni@xxxxxxxxxx> wrote:
> 
> > On 9/27/24 22:54, Haiyang Zhang wrote:
> > > The existing code moves VF to the same namespace as the synthetic
> device
> > > during netvsc_register_vf(). But, if the synthetic device is moved to
> a
> > > new namespace after the VF registration, the VF won't be moved
> together.
> > >
> > > To make the behavior more consistent, add a namespace check to
> netvsc_open(),
> > > and move the VF if it is not in the same namespace.
> > >
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Fixes: c0a41b887ce6 ("hv_netvsc: move VF to same namespace as netvsc
> device")
> > > Signed-off-by: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> >
> > This looks strange to me. Skimming over the code it looks like that
> with
> > VF you really don't mean a Virtual Function...
> 
> In Hyper-V/Azure, there is a feature called "Accelerated Networking"
> where
> a Virtual Function (VF) is associated with the synthetic network
> interface.
> The VF may be added/removed by hypervisor while network is running and
> driver
> needs to follow and track that.
> 
> >
> > Looking at the blamed commit, it looks like that having both the
> > synthetic and the "VF" device in different namespaces is an intended
> > use-case. This change would make such scenario more difficult and could
> > possibly break existing use-cases.
> 
> That commit was trying to solve the case where a network interface
> was isolated at boot. The VF device shows up after the
> synthetic device has been registered.
> 
> 
> > Why do you think it will be more consistent? If the user moved the
> > synthetic device in another netns, possibly/likely the user intended to
> > keep both devices separated.
> 
> Splitting the two across namespaces is not useful. The VF is a secondary
> device and doing anything directly on the VF will not give good results.
> Linux does not have a way to hide or lock out network devices, if it did
> the VF would be so marked.
> 
> This patch is trying to handle the case where userspace moves
> the synthetic network device and the VF is left in wrong namespace.
> 
> Moving the device when brought up is not the best solution. Probably
> better to
> do it when the network device is moved to another namespace.
> Which is visible in driver as NETDEV_REGISTER event.
> The driver already handles this (for VF events) in netvsc_netdev_event()
> it would just have to look at events on the netvsc device as well.
Thank you for the suggestion. I will look into this idea: let the netvsc_netdev_event() 
to monitor the NETDEV_REGISTER from netvsc devices.
This will come from __dev_change_net_namespace -> call_netdevice_notifiers(NETDEV_REGISTER, dev).

- Haiyang