| 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
| ||
|
Date: Tue, 19 Dec 2017 14:44:37 -0800
From: "Samudrala, Sridhar" <sridhar.samudrala@...el.com>
To: Stephen Hemminger <stephen@...workplumber.org>,
netdev@...r.kernel.org
Cc: Stephen Hemminger <sthemmin@...rosoft.com>
Subject: Re: [RFC] hv_netvsc: automatically name slave VF network device
On 12/19/2017 11:35 AM, Stephen Hemminger wrote:
> Rename the VF device to ethX_vf based on the ethX as the
> synthetic device. This eliminates the need for delay on setup,
> and the PCI (udev based) naming is not reproducible on Hyper-V
> anyway. The name of the VF does not matter since all control
> operations take place the primary device. It does make the
> user experience better to associate the names.
>
> Based on feedback from all.systems.go talk.
> The downside is that it requires exporting a symbol from netdev
> core which makes it harder to backport.
>
> Signed-off-by: Stephen Hemminger <sthemmin@...rosoft.com>
> ---
> drivers/net/hyperv/hyperv_net.h | 1 -
> drivers/net/hyperv/netvsc_drv.c | 53 +++++++++++------------------------------
> net/core/dev.c | 1 +
> 3 files changed, 15 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
> index 0db3bd1ea06f..5013189123e6 100644
> --- a/drivers/net/hyperv/hyperv_net.h
> +++ b/drivers/net/hyperv/hyperv_net.h
> @@ -762,7 +762,6 @@ 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 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 c5584c2d440e..8a0afe9a5064 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -49,7 +49,6 @@
> #define RING_SIZE_MIN 64
>
> #define LINKCHANGE_INT (2 * HZ)
> -#define VF_TAKEOVER_INT (HZ / 10)
>
> static unsigned int ring_size __ro_after_init = 128;
> module_param(ring_size, uint, S_IRUGO);
> @@ -1760,7 +1759,7 @@ static rx_handler_result_t netvsc_vf_handle_frame(struct sk_buff **pskb)
> static int netvsc_vf_join(struct net_device *vf_netdev,
> struct net_device *ndev)
> {
> - struct net_device_context *ndev_ctx = netdev_priv(ndev);
> + char vf_name[IFNAMSIZ];
> int ret;
>
> ret = netdev_rx_handler_register(vf_netdev,
> @@ -1783,23 +1782,14 @@ 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_delayed_work(&ndev_ctx->vf_takeover, VF_TAKEOVER_INT);
> -
> call_netdevice_notifiers(NETDEV_JOIN, vf_netdev);
>
> - netdev_info(vf_netdev, "joined to %s\n", ndev->name);
> - return 0;
> -
> -upper_link_failed:
> - netdev_rx_handler_unregister(vf_netdev);
> -rx_handler_failed:
> - return ret;
> -}
> -
> -static void __netvsc_vf_setup(struct net_device *ndev,
> - struct net_device *vf_netdev)
> -{
> - int ret;
> + /* set the name of VF device based on upper device name */
> + snprintf(vf_name, IFNAMSIZ, "%s_vf", ndev->name);
> + ret = dev_change_name(vf_netdev, vf_name);
> + if (ret != 0)
> + netdev_warn(vf_netdev,
> + "can not rename device: (%d)\n", ret);
It is possible that upper device name can change after this call. I
noticed this
when i tried this approach with virtio_net.
Also, what should happen if the upper device is unloaded? Should we rename
the VF name?
>
> /* Align MTU of VF with master */
> ret = dev_set_mtu(vf_netdev, ndev->mtu);
> @@ -1807,34 +1797,21 @@ static void __netvsc_vf_setup(struct net_device *ndev,
> netdev_warn(vf_netdev,
> "unable to change mtu to %u\n", ndev->mtu);
>
> + /* If upper device is already up, bring up slave */
> if (netif_running(ndev)) {
> ret = dev_open(vf_netdev);
> if (ret)
> netdev_warn(vf_netdev,
> "unable to open: %d\n", ret);
> }
> -}
> -
> -/* Setup VF as slave of the synthetic device.
> - * Runs in workqueue to avoid recursion in netlink callbacks.
> - */
> -static void netvsc_vf_setup(struct work_struct *w)
> -{
> - struct net_device_context *ndev_ctx
> - = 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_delayed_work(&ndev_ctx->vf_takeover, 0);
> - return;
> - }
>
> - vf_netdev = rtnl_dereference(ndev_ctx->vf_netdev);
> - if (vf_netdev)
> - __netvsc_vf_setup(ndev, vf_netdev);
> + netdev_info(vf_netdev, "joined to %s\n", ndev->name);
> + return 0;
>
> - rtnl_unlock();
> + upper_link_failed:
> + netdev_rx_handler_unregister(vf_netdev);
> + rx_handler_failed:
> + return ret;
> }
>
> static int netvsc_register_vf(struct net_device *vf_netdev)
> @@ -1904,7 +1881,6 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev)
> return NOTIFY_DONE;
>
> net_device_ctx = netdev_priv(ndev);
> - cancel_delayed_work_sync(&net_device_ctx->vf_takeover);
>
> netdev_info(ndev, "VF unregistering: %s\n", vf_netdev->name);
>
> @@ -1947,7 +1923,6 @@ static int netvsc_probe(struct hv_device *dev,
>
> spin_lock_init(&net_device_ctx->lock);
> INIT_LIST_HEAD(&net_device_ctx->reconfig_events);
> - INIT_DELAYED_WORK(&net_device_ctx->vf_takeover, netvsc_vf_setup);
>
> net_device_ctx->vf_stats
> = netdev_alloc_pcpu_stats(struct netvsc_vf_pcpu_stats);
> diff --git a/net/core/dev.c b/net/core/dev.c
> index c7db39926769..f51358a90efa 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1238,6 +1238,7 @@ int dev_change_name(struct net_device *dev, const char *newname)
>
> return err;
> }
> +EXPORT_SYMBOL(dev_change_name);
>
> /**
> * dev_set_alias - change ifalias of a device
Powered by blists - more mailing lists