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] [thread-next>] [day] [month] [year] [list]
Message-ID:
 <DS1PEPF00012A5F768C4AED4E00327DE243CA7D2@DS1PEPF00012A5F.namprd21.prod.outlook.com>
Date: Tue, 30 Jan 2024 13:43:51 +0000
From: Haiyang Zhang <haiyangz@...rosoft.com>
To: Shradha Gupta <shradhagupta@...ux.microsoft.com>, KY Srinivasan
	<kys@...rosoft.com>, Wei Liu <wei.liu@...nel.org>, Dexuan Cui
	<decui@...rosoft.com>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet
	<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Wojciech Drewek <wojciech.drewek@...el.com>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: Shradha Gupta <shradhagupta@...rosoft.com>, "stable@...r.kernel.org"
	<stable@...r.kernel.org>
Subject: RE: [PATCH] hv_netvsc:Register VF in netvsc_probe if
 NET_DEVICE_REGISTER missed



> -----Original Message-----
> From: Shradha Gupta <shradhagupta@...ux.microsoft.com>
> Sent: Tuesday, January 30, 2024 2:19 AM
> To: KY Srinivasan <kys@...rosoft.com>; Haiyang Zhang
> <haiyangz@...rosoft.com>; Wei Liu <wei.liu@...nel.org>; Dexuan Cui
> <decui@...rosoft.com>; David S. Miller <davem@...emloft.net>; Eric
> Dumazet <edumazet@...gle.com>; Jakub Kicinski <kuba@...nel.org>; Paolo
> Abeni <pabeni@...hat.com>; Wojciech Drewek <wojciech.drewek@...el.com>;
> linux-hyperv@...r.kernel.org; netdev@...r.kernel.org; linux-
> kernel@...r.kernel.org
> Cc: Shradha Gupta <shradhagupta@...ux.microsoft.com>; Shradha Gupta
> <shradhagupta@...rosoft.com>; stable@...r.kernel.org
> Subject: [PATCH] hv_netvsc:Register VF in netvsc_probe if
> NET_DEVICE_REGISTER missed
> 
> If hv_netvsc driver is removed and reloaded, the NET_DEVICE_REGISTER
> handler cannot perform VF register successfully as the register call
> is received before netvsc_probe is finished. This is because we
> register register_netdevice_notifier() very early(even before
> vmbus_driver_register()).
> To fix this, we try to register each such matching VF( if it is visible
> as a netdevice) at the end of netvsc_probe.
> 
> Cc: stable@...r.kernel.org
> Fixes: 85520856466e ("hv_netvsc: Fix race of register_netdevice_notifier
> and VF register")
> Suggested-by: Dexuan Cui <decui@...rosoft.com>
> Signed-off-by: Shradha Gupta <shradhagupta@...ux.microsoft.com>
> Tested-on: Ubuntu22
> Testcases: LISA testsuites
> 	   verify_reload_hyperv_modules, perf_tcp_ntttcp_sriov
> ---
>  drivers/net/hyperv/netvsc_drv.c | 49 ++++++++++++++++++++++++++++-----
>  1 file changed, 42 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/hyperv/netvsc_drv.c
> b/drivers/net/hyperv/netvsc_drv.c
> index 706ea5263e87..25c4dc9cc4bd 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -42,6 +42,10 @@
>  #define LINKCHANGE_INT (2 * HZ)
>  #define VF_TAKEOVER_INT (HZ / 10)
> 
> +/* Macros to define the context of vf registration */
> +#define VF_REG_IN_PROBE		1
> +#define VF_REG_IN_RECV_CBACK	2
> +
>  static unsigned int ring_size __ro_after_init = 128;
>  module_param(ring_size, uint, 0444);
>  MODULE_PARM_DESC(ring_size, "Ring buffer size (# of pages)");
> @@ -2183,7 +2187,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 *ndev, int context)
>  {
>  	struct net_device_context *ndev_ctx = netdev_priv(ndev);
>  	int ret;
> @@ -2205,8 +2209,11 @@ static int netvsc_vf_join(struct net_device
> *vf_netdev,
>  			   ndev->name, ret);
>  		goto upper_link_failed;
>  	}
> -
> -	schedule_delayed_work(&ndev_ctx->vf_takeover, VF_TAKEOVER_INT);
> +	/* If this registration is called from probe context vf_takeover
> +	 * is taken care of later in probe itself.
> +	 */
> +	if (context == VF_REG_IN_RECV_CBACK)
> +		schedule_delayed_work(&ndev_ctx->vf_takeover,
> VF_TAKEOVER_INT);
> 
>  	call_netdevice_notifiers(NETDEV_JOIN, vf_netdev);
> 
> @@ -2344,7 +2351,7 @@ static int netvsc_prepare_bonding(struct net_device
> *vf_netdev)
>  	return NOTIFY_DONE;
>  }
> 
> -static int netvsc_register_vf(struct net_device *vf_netdev)
> +static int netvsc_register_vf(struct net_device *vf_netdev, int context)
>  {
>  	struct net_device_context *net_device_ctx;
>  	struct netvsc_device *netvsc_dev;
> @@ -2384,7 +2391,7 @@ static int netvsc_register_vf(struct net_device
> *vf_netdev)
> 
>  	netdev_info(ndev, "VF registering: %s\n", vf_netdev->name);
> 
> -	if (netvsc_vf_join(vf_netdev, ndev) != 0)
> +	if (netvsc_vf_join(vf_netdev, ndev, context) != 0)
>  		return NOTIFY_DONE;
> 
>  	dev_hold(vf_netdev);
> @@ -2485,7 +2492,7 @@ static int netvsc_unregister_vf(struct net_device
> *vf_netdev)
>  static int netvsc_probe(struct hv_device *dev,
>  			const struct hv_vmbus_device_id *dev_id)
>  {
> -	struct net_device *net = NULL;
> +	struct net_device *net = NULL, *vf_netdev;
>  	struct net_device_context *net_device_ctx;
>  	struct netvsc_device_info *device_info = NULL;
>  	struct netvsc_device *nvdev;
> @@ -2597,6 +2604,34 @@ static int netvsc_probe(struct hv_device *dev,
>  	}
> 
>  	list_add(&net_device_ctx->list, &netvsc_dev_list);
> +
> +	/* When the hv_netvsc driver is removed and readded, the
> +	 * NET_DEVICE_REGISTER for the vf device is replayed before probe
> +	 * is complete. This is because register_netdevice_notifier() gets
> +	 * registered before vmbus_driver_register() so that callback func
> +	 * is set before probe and we don't miss events like
> NETDEV_POST_INIT
> +	 * So, in this section we try to register each matching
> +	 * vf device that is present as a netdevice, knowing that it's
> register
> +	 * call is not processed in the netvsc_netdev_notifier(as probing
> is
> +	 * progress and get_netvsc_byslot fails).
> +	 */
> +	for_each_netdev(dev_net(net), vf_netdev) {
> +		if (vf_netdev->netdev_ops == &device_ops)
> +			continue;
> +
> +		if (vf_netdev->type != ARPHRD_ETHER)
> +			continue;
> +
> +		if (is_vlan_dev(vf_netdev))
> +			continue;
> +
> +		if (netif_is_bond_master(vf_netdev))
> +			continue;
> +
> +		netvsc_prepare_bonding(vf_netdev);
> +		netvsc_register_vf(vf_netdev, VF_REG_IN_PROBE);
> +		__netvsc_vf_setup(net, vf_netdev);
> +	}
>  	rtnl_unlock();
> 
>  	netvsc_devinfo_put(device_info);
> @@ -2773,7 +2808,7 @@ static int netvsc_netdev_event(struct
> notifier_block *this,
>  	case NETDEV_POST_INIT:
>  		return netvsc_prepare_bonding(event_dev);
>  	case NETDEV_REGISTER:
> -		return netvsc_register_vf(event_dev);
> +		return netvsc_register_vf(event_dev, VF_REG_IN_RECV_CBACK);
>  	case NETDEV_UNREGISTER:
>  		return netvsc_unregister_vf(event_dev);
>  	case NETDEV_UP:
> --
> 2.34.1

Please use [PATCH net] on the subject to specify the branch.

Everything else looks fine.

Reviewed-by: Haiyang Zhang <haiyangz@...rosoft.com>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ