Stephen Hemminger
2018-Apr-10 21:26 UTC
[RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
On Tue, 10 Apr 2018 11:59:50 -0700 Sridhar Samudrala <sridhar.samudrala at intel.com> wrote:> Use the registration/notification framework supported by the generic > bypass infrastructure. > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com> > ---Thanks for doing this. Your current version has couple show stopper issues. First, the slave device is instantly taking over the slave. This doesn't allow udev/systemd to do its device rename of the slave device. Netvsc uses a delayed work to workaround this. Secondly, the select queue needs to call queue selection in VF. The bonding/teaming logic doesn't work well for UDP flows. Commit b3bf5666a510 ("hv_netvsc: defer queue selection to VF") fixed this performance problem. Lastly, more indirection is bad in current climate. I am not completely adverse to this but it needs to be fast, simple and completely transparent.
Samudrala, Sridhar
2018-Apr-10 22:56 UTC
[RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
On 4/10/2018 2:26 PM, Stephen Hemminger wrote:> On Tue, 10 Apr 2018 11:59:50 -0700 > Sridhar Samudrala <sridhar.samudrala at intel.com> wrote: > >> Use the registration/notification framework supported by the generic >> bypass infrastructure. >> >> Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com> >> --- > Thanks for doing this. Your current version has couple show stopper > issues. > > First, the slave device is instantly taking over the slave. > This doesn't allow udev/systemd to do its device rename of the slave > device. Netvsc uses a delayed work to workaround this.OK. I guess you are referring to the dev_set_mtu() and dev_open() calls that are made in bypass_slave_register() and you want to defer them to be done after a delay.? I could avoid these calls in case of netvsc based on bypass_ops.> > Secondly, the select queue needs to call queue selection in VF. > The bonding/teaming logic doesn't work well for UDP flows. > Commit b3bf5666a510 ("hv_netvsc: defer queue selection to VF") > fixed this performance problem.netvsc should not be using bypass_select_queue() as? that ndo op gets used only with 3-netdev model. Anyway, will look into updating bypass_select_queue() based on your fix.> > Lastly, more indirection is bad in current climate. > > I am not completely adverse to this but it needs to be fast, simple > and completely transparent.Not sure we can avoid this indirection if we want to commonize the code,? but use different models for virtio-net and netvsc. On the other hand, these patches avoid calls to get_netvsc_bymac() and get_netvsc_by_ref() that go through all the devices for all the netdev events. netvsc lookups should be much faster. Thanks Sridhar
Michael S. Tsirkin
2018-Apr-10 23:28 UTC
[RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
On Tue, Apr 10, 2018 at 02:26:08PM -0700, Stephen Hemminger wrote:> On Tue, 10 Apr 2018 11:59:50 -0700 > Sridhar Samudrala <sridhar.samudrala at intel.com> wrote: > > > Use the registration/notification framework supported by the generic > > bypass infrastructure. > > > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com> > > --- > > Thanks for doing this. Your current version has couple show stopper > issues. > > First, the slave device is instantly taking over the slave. > This doesn't allow udev/systemd to do its device rename of the slave > device. Netvsc uses a delayed work to workaround this.Interesting. Does this mean udev must act within a specific time window then?> Secondly, the select queue needs to call queue selection in VF. > The bonding/teaming logic doesn't work well for UDP flows. > Commit b3bf5666a510 ("hv_netvsc: defer queue selection to VF") > fixed this performance problem. > > Lastly, more indirection is bad in current climate. > > I am not completely adverse to this but it needs to be fast, simple > and completely transparent.
Siwei Liu
2018-Apr-10 23:44 UTC
[RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
On Tue, Apr 10, 2018 at 4:28 PM, Michael S. Tsirkin <mst at redhat.com> wrote:> On Tue, Apr 10, 2018 at 02:26:08PM -0700, Stephen Hemminger wrote: >> On Tue, 10 Apr 2018 11:59:50 -0700 >> Sridhar Samudrala <sridhar.samudrala at intel.com> wrote: >> >> > Use the registration/notification framework supported by the generic >> > bypass infrastructure. >> > >> > Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com> >> > --- >> >> Thanks for doing this. Your current version has couple show stopper >> issues. >> >> First, the slave device is instantly taking over the slave. >> This doesn't allow udev/systemd to do its device rename of the slave >> device. Netvsc uses a delayed work to workaround this. > > Interesting. Does this mean udev must act within a specific time window > then?Sighs, lots of hacks. Why propgating this from driver to a common module. We really need a clean solution. -Siwei> >> Secondly, the select queue needs to call queue selection in VF. >> The bonding/teaming logic doesn't work well for UDP flows. >> Commit b3bf5666a510 ("hv_netvsc: defer queue selection to VF") >> fixed this performance problem. >> >> Lastly, more indirection is bad in current climate. >> >> I am not completely adverse to this but it needs to be fast, simple >> and completely transparent.
Michael S. Tsirkin
2018-Apr-11 01:21 UTC
[RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
On Tue, Apr 10, 2018 at 02:26:08PM -0700, Stephen Hemminger wrote:> On Tue, 10 Apr 2018 11:59:50 -0700 > Sridhar Samudrala <sridhar.samudrala at intel.com> wrote: > > > Use the registration/notification framework supported by the generic > > bypass infrastructure. > > > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com> > > --- > > Thanks for doing this. Your current version has couple show stopper > issues. > > First, the slave device is instantly taking over the slave. > This doesn't allow udev/systemd to do its device rename of the slave > device. Netvsc uses a delayed work to workaround this. > > Secondly, the select queue needs to call queue selection in VF. > The bonding/teaming logic doesn't work well for UDP flows. > Commit b3bf5666a510 ("hv_netvsc: defer queue selection to VF") > fixed this performance problem. > > Lastly, more indirection is bad in current climate.Well right now netvsc does an indirect call to the PT device, does it not? If you really want max performance when PT is in use you need to do the reverse and have PT forward to netvsc.> I am not completely adverse to this but it needs to be fast, simple > and completely transparent.
Jiri Pirko
2018-Apr-11 07:50 UTC
[RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
Wed, Apr 11, 2018 at 01:28:51AM CEST, mst at redhat.com wrote:>On Tue, Apr 10, 2018 at 02:26:08PM -0700, Stephen Hemminger wrote: >> On Tue, 10 Apr 2018 11:59:50 -0700 >> Sridhar Samudrala <sridhar.samudrala at intel.com> wrote: >> >> > Use the registration/notification framework supported by the generic >> > bypass infrastructure. >> > >> > Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com> >> > --- >> >> Thanks for doing this. Your current version has couple show stopper >> issues. >> >> First, the slave device is instantly taking over the slave. >> This doesn't allow udev/systemd to do its device rename of the slave >> device. Netvsc uses a delayed work to workaround this. > >Interesting. Does this mean udev must act within a specific time window >then?Yeah. That is scarry. Also, wrong.> >> Secondly, the select queue needs to call queue selection in VF. >> The bonding/teaming logic doesn't work well for UDP flows. >> Commit b3bf5666a510 ("hv_netvsc: defer queue selection to VF") >> fixed this performance problem. >> >> Lastly, more indirection is bad in current climate. >> >> I am not completely adverse to this but it needs to be fast, simple >> and completely transparent.
Jiri Pirko
2018-Apr-11 07:53 UTC
[RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
Tue, Apr 10, 2018 at 11:26:08PM CEST, stephen at networkplumber.org wrote:>On Tue, 10 Apr 2018 11:59:50 -0700 >Sridhar Samudrala <sridhar.samudrala at intel.com> wrote: > >> Use the registration/notification framework supported by the generic >> bypass infrastructure. >> >> Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com> >> --- > >Thanks for doing this. Your current version has couple show stopper >issues. > >First, the slave device is instantly taking over the slave. >This doesn't allow udev/systemd to do its device rename of the slave >device. Netvsc uses a delayed work to workaround this.Wait. Why the fact a device is enslaved has to affect the udev in any way? If it does, smells like a bug in udev.> >Secondly, the select queue needs to call queue selection in VF. >The bonding/teaming logic doesn't work well for UDP flows. >Commit b3bf5666a510 ("hv_netvsc: defer queue selection to VF") >fixed this performance problem. > >Lastly, more indirection is bad in current climate. > >I am not completely adverse to this but it needs to be fast, simple >and completely transparent.
Siwei Liu
2019-Feb-22 01:14 UTC
net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
Sorry for replying to this ancient thread. There was some remaining issue that I don't think the initial net_failover patch got addressed cleanly, see: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1815268 The renaming of 'eth0' to 'ens4' fails because the udev userspace was not specifically writtten for such kernel automatic enslavement. Specifically, if it is a bond or team, the slave would typically get renamed *before* virtual device gets created, that's what udev can control (without getting netdev opened early by the other part of kernel) and other userspace components for e.g. initramfs, init-scripts can coordinate well in between. The in-kernel auto-enslavement of net_failover breaks this userspace convention, which don't provides a solution if user care about consistent naming on the slave netdevs specifically. Previously this issue had been specifically called out when IFF_HIDDEN and the 1-netdev was proposed, but no one gives out a solution to this problem ever since. Please share your mind how to proceed and solve this userspace issue if netdev does not welcome a 1-netdev model. On Wed, Apr 11, 2018 at 12:53 AM Jiri Pirko <jiri at resnulli.us> wrote:> > Tue, Apr 10, 2018 at 11:26:08PM CEST, stephen at networkplumber.org wrote: > >On Tue, 10 Apr 2018 11:59:50 -0700 > >Sridhar Samudrala <sridhar.samudrala at intel.com> wrote: > > > >> Use the registration/notification framework supported by the generic > >> bypass infrastructure. > >> > >> Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com> > >> --- > > > >Thanks for doing this. Your current version has couple show stopper > >issues. > > > >First, the slave device is instantly taking over the slave. > >This doesn't allow udev/systemd to do its device rename of the slave > >device. Netvsc uses a delayed work to workaround this. > > Wait. Why the fact a device is enslaved has to affect the udev in any > way? If it does, smells like a bug in udev.See above for clarifications. Thanks,> > > > > >Secondly, the select queue needs to call queue selection in VF. > >The bonding/teaming logic doesn't work well for UDP flows. > >Commit b3bf5666a510 ("hv_netvsc: defer queue selection to VF") > >fixed this performance problem. > > > >Lastly, more indirection is bad in current climate. > > > >I am not completely adverse to this but it needs to be fast, simple > >and completely transparent.
Seemingly Similar Threads
- [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
- [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
- [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
- [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
- [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework