subject:"Re\: \[PATCH net\-next 0\/1\] netvsc\: another VF datapath fix"

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

2017-08-09 Thread Stephen Hemminger

On Wed, 09 Aug 2017 11:03:05 +0200
Vitaly Kuznetsov  wrote:

> Stephen Hemminger  writes:
> 
> > The following would allow udev a chance at the device.
> >  
> 
> This would of course work but the approach is a bit hackish and can
> still fail on a loaded system. Raising the pause interval would be an
> option, but again, probably not the best one.
> 
> Let me try to send an RFC removing the check it dev_change_name() and if
> it turns out that it can't be removed we can go back to your patch. But
> in case it can we can leave without it.
> 
> Thanks,

I don't want to require change of semantics of core networking code
for one driver. Changing name of up device has been blocked for so
long that allowing it might break existing userspace apps. It might
be ok in the future, but netvsc needs to work without that change.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

2017-08-08 Thread Stephen Hemminger

The following would allow udev a chance at the device.



>From 37fb240a6107834c3dd3f57caede9d73b807f414 Mon Sep 17 00:00:00 2001
From: Stephen Hemminger 
Date: Tue, 8 Aug 2017 08:39:24 -0700
Subject: [PATCH] netvsc: delay setup of VF device

When VF device is discovered, delay bring it automatically up in
order to allow userspace to some simple changes (like renaming).

Signed-off-by: Stephen Hemminger 
---
 drivers/net/hyperv/hyperv_net.h |  2 +-
 drivers/net/hyperv/netvsc_drv.c | 11 ++-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index d1ea99a12cf2..f620c90307ed 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -723,7 +723,7 @@ struct net_device_context {
/* State to manage the associated VF interface. */
struct net_device __rcu *vf_netdev;
struct netvsc_vf_pcpu_stats __percpu *vf_stats;
-   struct work_struct vf_takeover;
+   struct delayed_work vf_takeover;
 
/* 1: allocated, serial number is valid. 0: not allocated */
u32 vf_alloc;
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 7ebf0e10e62b..1dff160368a3 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -47,6 +47,7 @@
 
 #define RING_SIZE_MIN 64
 #define LINKCHANGE_INT (2 * HZ)
+#define VF_TAKEOVER_INT (HZ / 10)
 
 static int ring_size = 128;
 module_param(ring_size, int, S_IRUGO);
@@ -1570,7 +1571,7 @@ static int netvsc_vf_join(struct net_device *vf_netdev,
/* set slave flag before open to prevent IPv6 addrconf */
vf_netdev->flags |= IFF_SLAVE;
 
-   schedule_work(_ctx->vf_takeover);
+   schedule_delayed_work(_ctx->vf_takeover, VF_TAKEOVER_INT);
 
netdev_info(vf_netdev, "joined to %s\n", ndev->name);
return 0;
@@ -1608,12 +1609,12 @@ static void __netvsc_vf_setup(struct net_device *ndev,
 static void netvsc_vf_setup(struct work_struct *w)
 {
struct net_device_context *ndev_ctx
-   = container_of(w, struct net_device_context, vf_takeover);
+   = container_of(w, struct net_device_context, vf_takeover.work);
struct net_device *ndev = hv_get_drvdata(ndev_ctx->device_ctx);
struct net_device *vf_netdev;
 
if (!rtnl_trylock()) {
-   schedule_work(w);
+   schedule_delayed_work(_ctx->vf_takeover, 0);
return;
}
 
@@ -1717,7 +1718,7 @@ static int netvsc_unregister_vf(struct net_device 
*vf_netdev)
return NOTIFY_DONE;
 
net_device_ctx = netdev_priv(ndev);
-   cancel_work_sync(_device_ctx->vf_takeover);
+   cancel_delayed_work_sync(_device_ctx->vf_takeover);
 
netdev_info(ndev, "VF unregistering: %s\n", vf_netdev->name);
 
@@ -1759,7 +1760,7 @@ static int netvsc_probe(struct hv_device *dev,
 
spin_lock_init(_device_ctx->lock);
INIT_LIST_HEAD(_device_ctx->reconfig_events);
-   INIT_WORK(_device_ctx->vf_takeover, netvsc_vf_setup);
+   INIT_DELAYED_WORK(_device_ctx->vf_takeover, netvsc_vf_setup);
 
net_device_ctx->vf_stats
= netdev_alloc_pcpu_stats(struct netvsc_vf_pcpu_stats);
-- 
2.11.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

2017-08-08 Thread Vitaly Kuznetsov

Stephen Hemminger  writes:

> On Tue, 08 Aug 2017 17:24:03 +0200
> Vitaly Kuznetsov  wrote:
>
>> Stephen Hemminger  writes:
>> 
>> > On Tue, 08 Aug 2017 16:03:56 +0200
>> > Vitaly Kuznetsov  wrote:
>> >  
>> >> Stephen Hemminger  writes:
>> >>   
>> >> > Previous fix was incomplete.
>> >> >
>> >> 
>> >> Not really related to this patch series (which btw fixes my issue,
>> >> thanks!), but I found one addition issue. Systemd fails to rename VF
>> >> interface:
>> >> 
>> >>  kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1
>> >>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF 
>> >> registering: eth2
>> >>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path 
>> >> switched to VF: eth2
>> >>  kernel: mlx4_en: eth2: Link Up
>> >>  NetworkManager[750]:   [1502200557.0821] device (eth2): link 
>> >> connected
>> >>  NetworkManager[750]:   [1502200557.1004] manager: (eth2): new 
>> >> Ethernet device (/org/freedesktop/NetworkManager/Devices/6)
>> >>  systemd-udevd[6942]: Error changing net interface name 'eth2' to 
>> >> 'enP2p0s2': Device or resource busy
>> >>  systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 
>> >> 'enP2p0s2': Device or resource busy
>> >> 
>> >> With some debug added I figured out what's wrong: __netvsc_vf_setup()
>> >> does dev_open() which sets IFF_UP flag on the interface. When systemd
>> >> tries to rename the interface we get into dev_change_name() and this
>> >> fails with -EBUSY when (dev->flags & IFF_UP).
>> >> 
>> >> The issue is of less importance as we're not supposed to configure VF
>> >> interface now. However, we may still want to get a stable name for it.
>> >> 
>> >> Any idea how this can be fixed?  
>> >
>> > The problem is Network Manager should ignore the VF device. I don't run NM 
>> > on these
>> > interfaces because it causes more issues than it helps (dueling userspace).
>> >
>> > The driver needs to have slave track the master interface. Relying on 
>> > userspace
>> > to bring interface up leads to all the issues the bonding script had.
>> >
>> > One option would be to delay the work of bringing up the slave device to 
>> > allow
>> > small window for renaming to run.  
>> 
>> Actually, I tried removing 'if (dev->flags & IFF_UP)' check from
>> dev_change_name() and everything seems to work fine. The history of this
>> code predates git so I have no idea why it's forbiden to change names of
>> IFF_UP interfaces... I can send an RFC removing the check to figure out
>> the truth :-)
>
> That only works because NM by default brings everything up.
> Many users don't use NM on servers.

I'm not sure NetworkManager is related here: renaming is done by
systemd/udev according to the "predictable interface names" scheme:
https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/

Basically, systemd/udev tries to rename all new interfaces in the system
according to this scheme. And systemd/udev is ubiquitous nowdays.

I tried disabling NetworkManager in my VM and the behavior is not any
different:

 kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1
 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF registering: 
eth2
 kernel: mlx4_en: eth2: Link Up
 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path 
switched to VF: eth2
 systemd-udevd[1785]: Error changing net interface name 'eth2' to 'enP2p0s2': 
Device or resource busy
 systemd-udevd[1785]: could not rename interface '5' from 'eth2' to 'enP2p0s2': 
Device or resource busy

The 'if (dev->flags & IFF_UP)' check in dev_change_name() function
I mentioned prevents renaming interfaces which are already up but I
don't know an obvious reason for it to exist.

-- 
  Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

2017-08-08 Thread Stephen Hemminger

On Tue, 08 Aug 2017 17:24:03 +0200
Vitaly Kuznetsov  wrote:

> Stephen Hemminger  writes:
> 
> > On Tue, 08 Aug 2017 16:03:56 +0200
> > Vitaly Kuznetsov  wrote:
> >  
> >> Stephen Hemminger  writes:
> >>   
> >> > Previous fix was incomplete.
> >> >
> >> 
> >> Not really related to this patch series (which btw fixes my issue,
> >> thanks!), but I found one addition issue. Systemd fails to rename VF
> >> interface:
> >> 
> >>  kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1
> >>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF 
> >> registering: eth2
> >>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path 
> >> switched to VF: eth2
> >>  kernel: mlx4_en: eth2: Link Up
> >>  NetworkManager[750]:   [1502200557.0821] device (eth2): link 
> >> connected
> >>  NetworkManager[750]:   [1502200557.1004] manager: (eth2): new 
> >> Ethernet device (/org/freedesktop/NetworkManager/Devices/6)
> >>  systemd-udevd[6942]: Error changing net interface name 'eth2' to 
> >> 'enP2p0s2': Device or resource busy
> >>  systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 
> >> 'enP2p0s2': Device or resource busy
> >> 
> >> With some debug added I figured out what's wrong: __netvsc_vf_setup()
> >> does dev_open() which sets IFF_UP flag on the interface. When systemd
> >> tries to rename the interface we get into dev_change_name() and this
> >> fails with -EBUSY when (dev->flags & IFF_UP).
> >> 
> >> The issue is of less importance as we're not supposed to configure VF
> >> interface now. However, we may still want to get a stable name for it.
> >> 
> >> Any idea how this can be fixed?  
> >
> > The problem is Network Manager should ignore the VF device. I don't run NM 
> > on these
> > interfaces because it causes more issues than it helps (dueling userspace).
> >
> > The driver needs to have slave track the master interface. Relying on 
> > userspace
> > to bring interface up leads to all the issues the bonding script had.
> >
> > One option would be to delay the work of bringing up the slave device to 
> > allow
> > small window for renaming to run.  
> 
> Actually, I tried removing 'if (dev->flags & IFF_UP)' check from
> dev_change_name() and everything seems to work fine. The history of this
> code predates git so I have no idea why it's forbiden to change names of
> IFF_UP interfaces... I can send an RFC removing the check to figure out
> the truth :-)

That only works because NM by default brings everything up.
Many users don't use NM on servers.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

2017-08-08 Thread Vitaly Kuznetsov

Stephen Hemminger  writes:

> On Tue, 08 Aug 2017 16:03:56 +0200
> Vitaly Kuznetsov  wrote:
>
>> Stephen Hemminger  writes:
>> 
>> > Previous fix was incomplete.
>> >  
>> 
>> Not really related to this patch series (which btw fixes my issue,
>> thanks!), but I found one addition issue. Systemd fails to rename VF
>> interface:
>> 
>>  kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1
>>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF 
>> registering: eth2
>>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path 
>> switched to VF: eth2
>>  kernel: mlx4_en: eth2: Link Up
>>  NetworkManager[750]:   [1502200557.0821] device (eth2): link connected
>>  NetworkManager[750]:   [1502200557.1004] manager: (eth2): new 
>> Ethernet device (/org/freedesktop/NetworkManager/Devices/6)
>>  systemd-udevd[6942]: Error changing net interface name 'eth2' to 
>> 'enP2p0s2': Device or resource busy
>>  systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 
>> 'enP2p0s2': Device or resource busy
>> 
>> With some debug added I figured out what's wrong: __netvsc_vf_setup()
>> does dev_open() which sets IFF_UP flag on the interface. When systemd
>> tries to rename the interface we get into dev_change_name() and this
>> fails with -EBUSY when (dev->flags & IFF_UP).
>> 
>> The issue is of less importance as we're not supposed to configure VF
>> interface now. However, we may still want to get a stable name for it.
>> 
>> Any idea how this can be fixed?
>
> The problem is Network Manager should ignore the VF device. I don't run NM on 
> these
> interfaces because it causes more issues than it helps (dueling userspace).
>
> The driver needs to have slave track the master interface. Relying on 
> userspace
> to bring interface up leads to all the issues the bonding script had.
>
> One option would be to delay the work of bringing up the slave device to allow
> small window for renaming to run.

Actually, I tried removing 'if (dev->flags & IFF_UP)' check from
dev_change_name() and everything seems to work fine. The history of this
code predates git so I have no idea why it's forbiden to change names of
IFF_UP interfaces... I can send an RFC removing the check to figure out
the truth :-)

-- 
  Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

2017-08-08 Thread Stephen Hemminger

On Tue, 08 Aug 2017 16:03:56 +0200
Vitaly Kuznetsov  wrote:

> Stephen Hemminger  writes:
> 
> > Previous fix was incomplete.
> >  
> 
> Not really related to this patch series (which btw fixes my issue,
> thanks!), but I found one addition issue. Systemd fails to rename VF
> interface:
> 
>  kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1
>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF registering: 
> eth2
>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path 
> switched to VF: eth2
>  kernel: mlx4_en: eth2: Link Up
>  NetworkManager[750]:   [1502200557.0821] device (eth2): link connected
>  NetworkManager[750]:   [1502200557.1004] manager: (eth2): new Ethernet 
> device (/org/freedesktop/NetworkManager/Devices/6)
>  systemd-udevd[6942]: Error changing net interface name 'eth2' to 'enP2p0s2': 
> Device or resource busy
>  systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 
> 'enP2p0s2': Device or resource busy
> 
> With some debug added I figured out what's wrong: __netvsc_vf_setup()
> does dev_open() which sets IFF_UP flag on the interface. When systemd
> tries to rename the interface we get into dev_change_name() and this
> fails with -EBUSY when (dev->flags & IFF_UP).
> 
> The issue is of less importance as we're not supposed to configure VF
> interface now. However, we may still want to get a stable name for it.
> 
> Any idea how this can be fixed?

The problem is Network Manager should ignore the VF device. I don't run NM on 
these
interfaces because it causes more issues than it helps (dueling userspace).

The driver needs to have slave track the master interface. Relying on userspace
to bring interface up leads to all the issues the bonding script had.

One option would be to delay the work of bringing up the slave device to allow
small window for renaming to run.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

2017-08-08 Thread Vitaly Kuznetsov

Stephen Hemminger  writes:

> Previous fix was incomplete.
>

Not really related to this patch series (which btw fixes my issue,
thanks!), but I found one addition issue. Systemd fails to rename VF
interface:

 kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1
 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF registering: 
eth2
 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path 
switched to VF: eth2
 kernel: mlx4_en: eth2: Link Up
 NetworkManager[750]:   [1502200557.0821] device (eth2): link connected
 NetworkManager[750]:   [1502200557.1004] manager: (eth2): new Ethernet 
device (/org/freedesktop/NetworkManager/Devices/6)
 systemd-udevd[6942]: Error changing net interface name 'eth2' to 'enP2p0s2': 
Device or resource busy
 systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 'enP2p0s2': 
Device or resource busy

With some debug added I figured out what's wrong: __netvsc_vf_setup()
does dev_open() which sets IFF_UP flag on the interface. When systemd
tries to rename the interface we get into dev_change_name() and this
fails with -EBUSY when (dev->flags & IFF_UP).

The issue is of less importance as we're not supposed to configure VF
interface now. However, we may still want to get a stable name for it.

Any idea how this can be fixed?

-- 
  Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

7 matches

Site Navigation

Mail list logo

Footer information