-----Original Message----- From: Greg Kroah-Hartman gregkh@linuxfoundation.org Sent: Wednesday, October 17, 2018 9:17 AM To: Haiyang Zhang haiyangz@microsoft.com Cc: linux-kernel@vger.kernel.org; stable@vger.kernel.org; Stephen Hemminger sthemmin@microsoft.com; David S. Miller davem@davemloft.net; Sasha Levin Alexander.Levin@microsoft.com Subject: Re: [PATCH 4.18 101/135] hv_netvsc: pair VF based on serial number
On Wed, Oct 17, 2018 at 01:02:17PM +0000, Haiyang Zhang wrote:
-----Original Message----- From: Greg Kroah-Hartman gregkh@linuxfoundation.org Sent: Wednesday, October 17, 2018 3:46 AM To: Haiyang Zhang haiyangz@microsoft.com Cc: linux-kernel@vger.kernel.org; stable@vger.kernel.org; Stephen Hemminger sthemmin@microsoft.com; David S. Miller davem@davemloft.net; Sasha Levin Alexander.Levin@microsoft.com Subject: Re: [PATCH 4.18 101/135] hv_netvsc: pair VF based on serial number
On Tue, Oct 16, 2018 at 07:13:12PM +0000, Haiyang Zhang wrote:
-----Original Message----- From: linux-kernel-owner@vger.kernel.org <linux-kernel- owner@vger.kernel.org> On Behalf Of Greg Kroah-Hartman Sent: Tuesday, October 16, 2018 1:06 PM To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org; stable@vger.kernel.org; Stephen Hemminger sthemmin@microsoft.com; David S. Miller davem@davemloft.net; Sasha Levin Alexander.Levin@microsoft.com Subject: [PATCH 4.18 101/135] hv_netvsc: pair VF based on serial number
4.18-stable review patch. If anyone has any objections, please let me
know.
From: Stephen Hemminger stephen@networkplumber.org
[ Upstream commit 00d7ddba1143623b31bc2c15d18216e2da031b14 ]
Matching network device based on MAC address is problematic since a non VF network device can be creted with a duplicate MAC address causing confusion and problems. The VMBus API does provide a serial number that is a better matching method.
Signed-off-by: Stephen Hemminger sthemmin@microsoft.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Sasha Levin alexander.levin@microsoft.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
drivers/net/hyperv/netvsc.c | 3 ++ drivers/net/hyperv/netvsc_drv.c | 58 ++++++++++++++++++++++-------
2 files changed, 36 insertions(+), 25 deletions(-)
--- a/drivers/net/hyperv/netvsc.c +++ b/drivers/net/hyperv/netvsc.c @@ -1203,6 +1203,9 @@ static void netvsc_send_vf(struct net_de
net_device_ctx->vf_alloc = nvmsg-
msg.v4_msg.vf_assoc.allocated;
net_device_ctx->vf_serial = nvmsg-
msg.v4_msg.vf_assoc.serial;
- netdev_info(ndev, "VF slot %u %s\n",
net_device_ctx->vf_serial,
net_device_ctx->vf_alloc ? "added" : "removed");
}
static void netvsc_receive_inband(struct net_device *ndev, --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -1794,20 +1794,6 @@ out_unlock: rtnl_unlock(); }
-static struct net_device *get_netvsc_bymac(const u8 *mac) -{
- struct net_device_context *ndev_ctx;
- list_for_each_entry(ndev_ctx, &netvsc_dev_list, list) {
struct net_device *dev = hv_get_drvdata(ndev_ctx-
device_ctx);
if (ether_addr_equal(mac, dev->perm_addr))
return dev;
- }
- return NULL;
-}
static struct net_device *get_netvsc_byref(struct net_device
*vf_netdev) {
struct net_device_context *net_device_ctx; @@ -1936,26 +1922,48
@@
static void netvsc_vf_setup(struct work_ rtnl_unlock(); }
+/* Find netvsc by VMBus serial number.
- The PCI hyperv controller records the serial number as the slot.
- */
+static struct net_device *get_netvsc_byslot(const struct +net_device +*vf_netdev) {
- struct device *parent = vf_netdev->dev.parent;
- struct net_device_context *ndev_ctx;
- struct pci_dev *pdev;
- if (!parent || !dev_is_pci(parent))
return NULL; /* not a PCI device */
- pdev = to_pci_dev(parent);
- if (!pdev->slot) {
netdev_notice(vf_netdev, "no PCI slot information\n");
return NULL;
- }
- list_for_each_entry(ndev_ctx, &netvsc_dev_list, list) {
if (!ndev_ctx->vf_alloc)
continue;
if (ndev_ctx->vf_serial == pdev->slot->number)
return hv_get_drvdata(ndev_ctx->device_ctx);
Greg,
I had a fix to this patch. If not already, Could you include my following fix
together?
hv_netvsc: fix vf serial matching with pci slot info https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg it.k ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fdavem%2Fnet-
next.git%2F
commit%2F%3Fid%3D005479556197f80139771960dda0dfdcd2d2aad5&da
ta=02%
7C01%7Chaiyangz%40microsoft.com%7C6ce194a47449435fe82a08d63404a94e
%7C7
2f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636753591959679786&
;sdata
=ReWYMZW%2FYtoFSbFSxny9VGw6V8v9J4sGanfJjbVd34A%3D&reserved=
0
I can not take a patch that is not "upstream" yet, sorry.
Also, this really feels like a "new feature" to me. Can someone explain what bug this is fixing?
Patch #A: hv_netvsc: pair VF based on serial number https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.k ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fnext%2Flinux-
next.git%2
Fcommit%2F%3Fid%3D00d7ddba1143623b31bc2c15d18216e2da031b14& data=02
%7C01%7Chaiyangz%40microsoft.com%7C2e07aa3f8a9d4f32526008d63432d
f14%7C
72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636753790439794786&am p;sdat
a=0ZyQ7jJdgsEZbh8TqA0eypvPUJVQaxSJcSNYuhLSl%2Fc%3D&reserved=0
Patch #B: hv_netvsc: fix vf serial matching with pci slot info https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.k ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fnext%2Flinux-
next.git%2
Fcommit%2F%3Fid%3D005479556197f80139771960dda0dfdcd2d2aad5&d ata=02
%7C01%7Chaiyangz%40microsoft.com%7C2e07aa3f8a9d4f32526008d63432d
f14%7C
72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636753790439794786&am p;sdat
a=JPfFlatrkt1K6zMLHmZDRG51cCLwVk9pXqHVtLm26hA%3D&reserved=0
The patch #A is a feature that use VF serial number for matching. But #A has
a bug causing mismatch.
Patch #B, upstream now (see the link to Linux-next), is a fix to #A.
You didn't answer my original question here, what bug is this fixing?
Patch #A: hv_netvsc: pair VF based on serial number https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?...
Patch #B: hv_netvsc: fix vf serial matching with pci slot info https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?...
The VF NIC needs to be paired with synthetic NIC on HyperV -- to do that we use MAC address matching before the patch #A. But a non VF NIC can also have the same MAC, which shouldn't be paired with synthetic NIC. So a better method is implemented by #A to use VF serial number for matching.
But, #A has a bug, which causes matching to fail. Patch #B fixed it by extracting the VF serial number correctly from slot info.
Thanks, - Haiyang