netdev - Re: [PATCH] hv_netvsc: update VF after name has changed.

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] [day] [month] [year] [list]

Message-ID: <20180125111915.0479ebc4@xeon-e3>
Date:   Thu, 25 Jan 2018 11:19:15 -0800
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     David Miller <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, sthemmin@...rosoft.com
Subject: Re: [PATCH] hv_netvsc: update VF after name has changed.

On Tue, 26 Dec 2017 17:11:19 -0500 (EST)
David Miller <davem@...emloft.net> wrote:

> From: Stephen Hemminger <stephen@...workplumber.org>
> Date: Tue, 26 Dec 2017 13:51:43 -0800
> 
> > On Tue, 26 Dec 2017 12:25:12 -0500 (EST)
> > David Miller <davem@...emloft.net> wrote:
> >   
> >> From: Stephen Hemminger <stephen@...workplumber.org>
> >> Date: Wed, 20 Dec 2017 14:33:23 -0800
> >>   
> >> > Since commit 6123c66854c1 ("netvsc: delay setup of VF device")
> >> > the automatic bring up of the VF is delayed to allow userspace (udev)
> >> > a chance to rename the device. This delay is problematic because
> >> > it delays boot and may not be long enough for some cases.
> >> > 
> >> > Instead, use the rename can be used to trigger the next step
> >> > in setup to happen immediately.
> >> > 
> >> > The VF initialization sequence now looks like:
> >> >    * hotplug causes VF network device probe to create network device
> >> >    * netvsc notifier joins VF with netvsc and schedules VF to be
> >> >      setup after timer expires
> >> >    * udev in userspace renames device
> >> >    * if netvsc notifier detects rename, it can cancel timer
> >> >      and do immediate setup
> >> > 
> >> > The delay can also be increased to allow for slower rules.
> >> > Still need the delayed work to handle the case where rename is
> >> > not done.
> >> > 
> >> > Signed-off-by: Stephen Hemminger <sthemmin@...rosoft.com>    
> >> 
> >> I'm still seriously perplexed by this whole situation.  
> > 
> > See state diagram below
> >    
> >> Why can't you bring up a VF interface simply because of a text string
> >> used to reffered to it?  
> > 
> > VF network device can not have its name changed if up.
> > The network device kernel API does not allow name change of network
> > device that is up.  There was a patch from Vitaly to allow this, but it
> > was deemed to be too risky.
> > 
> >    https://patchwork.ozlabs.org/patch/799646/  
> 
> Thank you for the state diagram.
> 
> I still, unfortunately, don't see the problem.
> 
> If userland changes the name of the VF device, then they will change
> it before bringing the VF device up.
> 
> Thanks for your patience with me.

Coming back to this after meltdown/spectre hiatus...
The patch optimizes the boot time by getting rid of the delay in bringing up
VF network device. The current upstream code has the netvsc driver bring up
VF device under the covers after 100ms. As you observed, this sucks;
100ms is a compromise value. The timeout is too short to ensure that udev
rename rule has been done by then. With some underpowered machines (or really
slow storage) it can take much longer than that. Conversely, many machines
put out lots of packets during boot process and until the VF is brought
up the packets have to go over the slow path.

Perhaps the timeout should be made longer, but impossible to predict
what the right value is.  That was why the patch used an additional state
transistion to short circuit it.