RE: [PATCH net,v3] hyperv: Fix the carrier status setting
-Original Message- From: Jason Wang [mailto:jasow...@redhat.com] Sent: Wednesday, February 12, 2014 10:52 PM To: Haiyang Zhang; da...@davemloft.net; net...@vger.kernel.org Cc: KY Srinivasan; o...@aepfle.de; linux-ker...@vger.kernel.org; driverdev- de...@linuxdriverproject.org Subject: Re: [PATCH net,v3] hyperv: Fix the carrier status setting On 02/13/2014 08:54 AM, Haiyang Zhang wrote: Without this patch, the cat /sys/class/net/ethN/operstate shows unknown, and ethtool ethN shows Link detected: yes, when VM boots up with or without vNIC connected. This patch fixed the problem. Signed-off-by: Haiyang Zhang haiya...@microsoft.com Reviewed-by: K. Y. Srinivasan k...@microsoft.com --- drivers/net/hyperv/netvsc_drv.c | 53 --- 1 files changed, 38 insertions(+), 15 deletions(-) diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 7756118..7141a19 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -88,8 +88,12 @@ static int netvsc_open(struct net_device *net) { struct net_device_context *net_device_ctx = netdev_priv(net); struct hv_device *device_obj = net_device_ctx-device_ctx; + struct netvsc_device *nvdev; + struct rndis_device *rdev; int ret = 0; + netif_carrier_off(net); + /* Open up the device */ ret = rndis_filter_open(device_obj); if (ret != 0) { @@ -99,6 +103,11 @@ static int netvsc_open(struct net_device *net) netif_start_queue(net); + nvdev = hv_get_drvdata(device_obj); + rdev = nvdev-extension; + if (!rdev-link_state) + netif_carrier_on(net); + Maybe you can just schedule the work here and then you can drop the rtnl_lock in netvsc_link_change() ? The rtnl_lock will still be necessary in the netvsc_link_change(), because we want to prevent it getting wrong rdev pointer when netvsc_change_mtu is removing/adding rndis device. + + if (notify) + netdev_notify_peers(net); } Looks like this forces arp_notify here. Is it expected? Yes, this is expected. It's required after live migration. Thanks, - Haiyang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net,v3] hyperv: Fix the carrier status setting
On 02/13/2014 11:04 PM, Haiyang Zhang wrote: -Original Message- From: Jason Wang [mailto:jasow...@redhat.com] Sent: Wednesday, February 12, 2014 10:52 PM To: Haiyang Zhang; da...@davemloft.net; net...@vger.kernel.org Cc: KY Srinivasan; o...@aepfle.de; linux-ker...@vger.kernel.org; driverdev- de...@linuxdriverproject.org Subject: Re: [PATCH net,v3] hyperv: Fix the carrier status setting On 02/13/2014 08:54 AM, Haiyang Zhang wrote: Without this patch, the cat /sys/class/net/ethN/operstate shows unknown, and ethtool ethN shows Link detected: yes, when VM boots up with or without vNIC connected. This patch fixed the problem. Signed-off-by: Haiyang Zhang haiya...@microsoft.com Reviewed-by: K. Y. Srinivasan k...@microsoft.com --- drivers/net/hyperv/netvsc_drv.c | 53 --- 1 files changed, 38 insertions(+), 15 deletions(-) diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 7756118..7141a19 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -88,8 +88,12 @@ static int netvsc_open(struct net_device *net) { struct net_device_context *net_device_ctx = netdev_priv(net); struct hv_device *device_obj = net_device_ctx-device_ctx; + struct netvsc_device *nvdev; + struct rndis_device *rdev; int ret = 0; + netif_carrier_off(net); + /* Open up the device */ ret = rndis_filter_open(device_obj); if (ret != 0) { @@ -99,6 +103,11 @@ static int netvsc_open(struct net_device *net) netif_start_queue(net); + nvdev = hv_get_drvdata(device_obj); + rdev = nvdev-extension; + if (!rdev-link_state) + netif_carrier_on(net); + Maybe you can just schedule the work here and then you can drop the rtnl_lock in netvsc_link_change() ? The rtnl_lock will still be necessary in the netvsc_link_change(), because we want to prevent it getting wrong rdev pointer when netvsc_change_mtu is removing/adding rndis device. Ok. + + if (notify) + netdev_notify_peers(net); } Looks like this forces arp_notify here. Is it expected? Yes, this is expected. It's required after live migration. Thanks, - Haiyang Yes, this does not change the current behaviour. (arp_notify is meaningless for netvsc). Acked-by: Jason Wang jasow...@redhat.com The patch is also needed for stable. Thanks -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net,v3] hyperv: Fix the carrier status setting
From: Haiyang Zhang haiya...@microsoft.com Date: Wed, 12 Feb 2014 16:54:27 -0800 Without this patch, the cat /sys/class/net/ethN/operstate shows unknown, and ethtool ethN shows Link detected: yes, when VM boots up with or without vNIC connected. This patch fixed the problem. Signed-off-by: Haiyang Zhang haiya...@microsoft.com Reviewed-by: K. Y. Srinivasan k...@microsoft.com Applied and queued up for -stable, thanks. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH net,v3] hyperv: Fix the carrier status setting
Without this patch, the cat /sys/class/net/ethN/operstate shows unknown, and ethtool ethN shows Link detected: yes, when VM boots up with or without vNIC connected. This patch fixed the problem. Signed-off-by: Haiyang Zhang haiya...@microsoft.com Reviewed-by: K. Y. Srinivasan k...@microsoft.com --- drivers/net/hyperv/netvsc_drv.c | 53 --- 1 files changed, 38 insertions(+), 15 deletions(-) diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 7756118..7141a19 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -88,8 +88,12 @@ static int netvsc_open(struct net_device *net) { struct net_device_context *net_device_ctx = netdev_priv(net); struct hv_device *device_obj = net_device_ctx-device_ctx; + struct netvsc_device *nvdev; + struct rndis_device *rdev; int ret = 0; + netif_carrier_off(net); + /* Open up the device */ ret = rndis_filter_open(device_obj); if (ret != 0) { @@ -99,6 +103,11 @@ static int netvsc_open(struct net_device *net) netif_start_queue(net); + nvdev = hv_get_drvdata(device_obj); + rdev = nvdev-extension; + if (!rdev-link_state) + netif_carrier_on(net); + return ret; } @@ -229,23 +238,24 @@ void netvsc_linkstatus_callback(struct hv_device *device_obj, struct net_device *net; struct net_device_context *ndev_ctx; struct netvsc_device *net_device; + struct rndis_device *rdev; net_device = hv_get_drvdata(device_obj); + rdev = net_device-extension; + + rdev-link_state = status != 1; + net = net_device-ndev; - if (!net) { - netdev_err(net, got link status but net device - not initialized yet\n); + if (!net || net-reg_state != NETREG_REGISTERED) return; - } + ndev_ctx = netdev_priv(net); if (status == 1) { - netif_carrier_on(net); - ndev_ctx = netdev_priv(net); schedule_delayed_work(ndev_ctx-dwork, 0); schedule_delayed_work(ndev_ctx-dwork, msecs_to_jiffies(20)); } else { - netif_carrier_off(net); + schedule_delayed_work(ndev_ctx-dwork, 0); } } @@ -388,17 +398,35 @@ static const struct net_device_ops device_ops = { * current context when receiving RNDIS_STATUS_MEDIA_CONNECT event. So, add * another netif_notify_peers() into a delayed work, otherwise GARP packet * will not be sent after quick migration, and cause network disconnection. + * Also, we update the carrier status here. */ -static void netvsc_send_garp(struct work_struct *w) +static void netvsc_link_change(struct work_struct *w) { struct net_device_context *ndev_ctx; struct net_device *net; struct netvsc_device *net_device; + struct rndis_device *rdev; + bool notify; + + rtnl_lock(); ndev_ctx = container_of(w, struct net_device_context, dwork.work); net_device = hv_get_drvdata(ndev_ctx-device_ctx); + rdev = net_device-extension; net = net_device-ndev; - netdev_notify_peers(net); + + if (rdev-link_state) { + netif_carrier_off(net); + notify = false; + } else { + netif_carrier_on(net); + notify = true; + } + + rtnl_unlock(); + + if (notify) + netdev_notify_peers(net); } @@ -414,13 +442,10 @@ static int netvsc_probe(struct hv_device *dev, if (!net) return -ENOMEM; - /* Set initial state */ - netif_carrier_off(net); - net_device_ctx = netdev_priv(net); net_device_ctx-device_ctx = dev; hv_set_drvdata(dev, net); - INIT_DELAYED_WORK(net_device_ctx-dwork, netvsc_send_garp); + INIT_DELAYED_WORK(net_device_ctx-dwork, netvsc_link_change); INIT_WORK(net_device_ctx-work, do_set_multicast); net-netdev_ops = device_ops; @@ -443,8 +468,6 @@ static int netvsc_probe(struct hv_device *dev, } memcpy(net-dev_addr, device_info.mac_adr, ETH_ALEN); - netif_carrier_on(net); - ret = register_netdev(net); if (ret != 0) { pr_err(Unable to register netdev.\n); -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net,v3] hyperv: Fix the carrier status setting
On 02/13/2014 08:54 AM, Haiyang Zhang wrote: Without this patch, the cat /sys/class/net/ethN/operstate shows unknown, and ethtool ethN shows Link detected: yes, when VM boots up with or without vNIC connected. This patch fixed the problem. Signed-off-by: Haiyang Zhang haiya...@microsoft.com Reviewed-by: K. Y. Srinivasan k...@microsoft.com --- drivers/net/hyperv/netvsc_drv.c | 53 --- 1 files changed, 38 insertions(+), 15 deletions(-) diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 7756118..7141a19 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -88,8 +88,12 @@ static int netvsc_open(struct net_device *net) { struct net_device_context *net_device_ctx = netdev_priv(net); struct hv_device *device_obj = net_device_ctx-device_ctx; + struct netvsc_device *nvdev; + struct rndis_device *rdev; int ret = 0; + netif_carrier_off(net); + /* Open up the device */ ret = rndis_filter_open(device_obj); if (ret != 0) { @@ -99,6 +103,11 @@ static int netvsc_open(struct net_device *net) netif_start_queue(net); + nvdev = hv_get_drvdata(device_obj); + rdev = nvdev-extension; + if (!rdev-link_state) + netif_carrier_on(net); + Maybe you can just schedule the work here and then you can drop the rtnl_lock in netvsc_link_change() ? return ret; } @@ -229,23 +238,24 @@ void netvsc_linkstatus_callback(struct hv_device *device_obj, struct net_device *net; struct net_device_context *ndev_ctx; struct netvsc_device *net_device; + struct rndis_device *rdev; net_device = hv_get_drvdata(device_obj); + rdev = net_device-extension; + + rdev-link_state = status != 1; + net = net_device-ndev; - if (!net) { - netdev_err(net, got link status but net device - not initialized yet\n); + if (!net || net-reg_state != NETREG_REGISTERED) return; - } + ndev_ctx = netdev_priv(net); if (status == 1) { - netif_carrier_on(net); - ndev_ctx = netdev_priv(net); schedule_delayed_work(ndev_ctx-dwork, 0); schedule_delayed_work(ndev_ctx-dwork, msecs_to_jiffies(20)); } else { - netif_carrier_off(net); + schedule_delayed_work(ndev_ctx-dwork, 0); } } @@ -388,17 +398,35 @@ static const struct net_device_ops device_ops = { * current context when receiving RNDIS_STATUS_MEDIA_CONNECT event. So, add * another netif_notify_peers() into a delayed work, otherwise GARP packet * will not be sent after quick migration, and cause network disconnection. + * Also, we update the carrier status here. */ -static void netvsc_send_garp(struct work_struct *w) +static void netvsc_link_change(struct work_struct *w) { struct net_device_context *ndev_ctx; struct net_device *net; struct netvsc_device *net_device; + struct rndis_device *rdev; + bool notify; + + rtnl_lock(); ndev_ctx = container_of(w, struct net_device_context, dwork.work); net_device = hv_get_drvdata(ndev_ctx-device_ctx); + rdev = net_device-extension; net = net_device-ndev; - netdev_notify_peers(net); + + if (rdev-link_state) { + netif_carrier_off(net); + notify = false; + } else { + netif_carrier_on(net); + notify = true; + } + + rtnl_unlock(); + + if (notify) + netdev_notify_peers(net); } Looks like this forces arp_notify here. Is it expected? Other looks good. @@ -414,13 +442,10 @@ static int netvsc_probe(struct hv_device *dev, if (!net) return -ENOMEM; - /* Set initial state */ - netif_carrier_off(net); - net_device_ctx = netdev_priv(net); net_device_ctx-device_ctx = dev; hv_set_drvdata(dev, net); - INIT_DELAYED_WORK(net_device_ctx-dwork, netvsc_send_garp); + INIT_DELAYED_WORK(net_device_ctx-dwork, netvsc_link_change); INIT_WORK(net_device_ctx-work, do_set_multicast); net-netdev_ops = device_ops; @@ -443,8 +468,6 @@ static int netvsc_probe(struct hv_device *dev, } memcpy(net-dev_addr, device_info.mac_adr, ETH_ALEN); - netif_carrier_on(net); - ret = register_netdev(net); if (ret != 0) { pr_err(Unable to register netdev.\n); ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel