* RE: [PATCH] PCI: hv: Use kfree(hbus) in hv_pci_probe()'s error handling path
From: Michael Kelley @ 2020-01-18 16:51 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, lorenzo.pieralisi@arm.com, bhelgaas@google.com,
linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, Sasha Levin
In-Reply-To: <1578350351-129783-1-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Monday, January 6, 2020 2:39 PM
>
> Now that we use kzalloc() to allocate the hbus buffer, we should use
> kfree() in the error path as well.
>
> Also remove the type casting, since it's unnecessary in C.
>
> Fixes: 877b911a5ba0 ("PCI: hv: Avoid a kmemleak false positive caused by the hbus buffer")
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>
> Sorry for missing the error handling path.
>
> drivers/pci/controller/pci-hyperv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 9977abff92fc..15011a349520 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -2922,7 +2922,7 @@ static int hv_pci_probe(struct hv_device *hdev,
> * positive by using kmemleak_alloc() and kmemleak_free() to ask
> * kmemleak to track and scan the hbus buffer.
> */
> - hbus = (struct hv_pcibus_device *)kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> + hbus = kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> if (!hbus)
> return -ENOMEM;
> hbus->state = hv_pcibus_init;
> @@ -3058,7 +3058,7 @@ static int hv_pci_probe(struct hv_device *hdev,
> free_dom:
> hv_put_dom_num(hbus->sysdata.domain);
> free_bus:
> - free_page((unsigned long)hbus);
> + kfree(hbus);
> return ret;
> }
>
> --
> 2.19.1
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH] Drivers: hv: vmbus: Ignore CHANNELMSG_TL_CONNECT_RESULT(23)
From: Michael Kelley @ 2020-01-18 17:29 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, Sasha Levin
Cc: Sunil Muthuswamy, Andrea Parri
In-Reply-To: <1578619515-115811-1-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Thursday, January 9, 2020 5:25 PM
>
> When a Linux hv_sock app tries to connect to a Service GUID on which no
> host app is listening, a recent host (RS3+) sends a
> CHANNELMSG_TL_CONNECT_RESULT (23) message to Linux and this triggers such
> a warning:
>
> unknown msgtype=23
> WARNING: CPU: 2 PID: 0 at drivers/hv/vmbus_drv.c:1031 vmbus_on_msg_dpc
>
> Actually Linux can safely ignore the message because the Linux app's
> connect() will time out in 2 seconds: see VSOCK_DEFAULT_CONNECT_TIMEOUT
> and vsock_stream_connect(). We don't bother to make use of the message
> because: 1) it's only supported on recent hosts; 2) a non-trivial effort
> is required to use the message in Linux, but the benefit is small.
>
> So, let's not see the warning by silently ignoring the message.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> drivers/hv/vmbus_drv.c | 5 +++++
> include/linux/hyperv.h | 2 ++
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 4ef5a66df680..c838b6f5f726 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1033,6 +1033,11 @@ void vmbus_on_msg_dpc(unsigned long data)
> }
>
> entry = &channel_message_table[hdr->msgtype];
> +
> + /* Linux ignores some messages, e.g. CHANNELMSG_TL_CONNECT_RESULT. */
> + if (!entry->message_handler)
> + goto msg_handled;
> +
FWIW, with this new check, all of the validity checks in vmbus_onmessage() are
redundant and could be removed. There's already a check here that ensures
msgtype won't be too big, and this new check ensures that message_handler is
not NULL.
> if (entry->handler_type == VMHT_BLOCKING) {
> ctx = kmalloc(sizeof(*ctx), GFP_ATOMIC);
> if (ctx == NULL)
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 26f3aeeae1ca..41c58011431e 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -425,6 +425,8 @@ enum vmbus_channel_message_type {
> CHANNELMSG_19 = 19,
> CHANNELMSG_20 = 20,
> CHANNELMSG_TL_CONNECT_REQUEST = 21,
> + CHANNELMSG_22 = 22,
> + CHANNELMSG_TL_CONNECT_RESULT = 23,
> CHANNELMSG_COUNT
> };
For completeness, I'd like to see the channel_message_table also updated
with these new entries so that everything stays in sync and is explicitly
defined.
Michael
>
> --
> 2.19.1
^ permalink raw reply
* RE: [PATCH V2] x86/Hyper-V: Balloon up according to request page number
From: Michael Kelley @ 2020-01-18 19:29 UTC (permalink / raw)
To: lantianyu1986@gmail.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, sashal@kernel.org
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets, stable@vger.kernel.org
In-Reply-To: <20200116141600.23391-1-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Thursday, January 16, 2020 6:16 AM
>
> Current code has assumption that balloon request memory size aligns
> with 2MB. But actually Hyper-V doesn't guarantee such alignment. When
> balloon driver receives non-aligned balloon request, it produces warning
> and balloon up more memory than requested in order to keep 2MB alignment.
> Remove the warning and balloon up memory according to actual requested
> memory size.
>
> Fixes: f6712238471a ("hv: hv_balloon: avoid memory leak on alloc_error of 2MB memory
> block")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> Change since v2:
> - Change logic of switching alloc_unit from 2MB to 4KB
> in the balloon_up() to avoid redundant iteration when
> handle non-aligned page request.
> - Remove 2MB alignment operation and comment in balloon_up()
> ---
> drivers/hv/hv_balloon.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index 7f3e7ab22d5d..536807efbc35 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -1684,7 +1684,7 @@ static unsigned int alloc_balloon_pages(struct
> hv_dynmem_device *dm,
> if (num_pages < alloc_unit)
> return 0;
The above test is no longer necessary. The num_pages < alloc_unit
case is handled implicitly by your new 'for' loop condition.
>
> - for (i = 0; (i * alloc_unit) < num_pages; i++) {
> + for (i = 0; i < num_pages / alloc_unit; i++) {
> if (bl_resp->hdr.size + sizeof(union dm_mem_page_range) >
> HV_HYP_PAGE_SIZE)
> return i * alloc_unit;
> @@ -1722,7 +1722,7 @@ static unsigned int alloc_balloon_pages(struct
> hv_dynmem_device *dm,
>
> }
>
> - return num_pages;
> + return i * alloc_unit;
> }
>
> static void balloon_up(union dm_msg_info *msg_info)
> @@ -1737,9 +1737,6 @@ static void balloon_up(union dm_msg_info *msg_info)
> long avail_pages;
> unsigned long floor;
>
> - /* The host balloons pages in 2M granularity. */
> - WARN_ON_ONCE(num_pages % PAGES_IN_2M != 0);
> -
> /*
> * We will attempt 2M allocations. However, if we fail to
> * allocate 2M chunks, we will go back to PAGE_SIZE allocations.
> @@ -1749,14 +1746,13 @@ static void balloon_up(union dm_msg_info *msg_info)
> avail_pages = si_mem_available();
> floor = compute_balloon_floor();
>
> - /* Refuse to balloon below the floor, keep the 2M granularity. */
> + /* Refuse to balloon below the floor. */
> if (avail_pages < num_pages || avail_pages - num_pages < floor) {
> pr_warn("Balloon request will be partially fulfilled. %s\n",
> avail_pages < num_pages ? "Not enough memory." :
> "Balloon floor reached.");
>
> num_pages = avail_pages > floor ? (avail_pages - floor) : 0;
> - num_pages -= num_pages % PAGES_IN_2M;
> }
>
> while (!done) {
> @@ -1770,7 +1766,7 @@ static void balloon_up(union dm_msg_info *msg_info)
> num_ballooned = alloc_balloon_pages(&dm_device, num_pages,
> bl_resp, alloc_unit);
>
> - if (alloc_unit != 1 && num_ballooned == 0) {
> + if (alloc_unit != 1 && num_ballooned != num_pages) {
Maybe I'm missing something, but I don't think Vitaly's optimization works. If
alloc_unit specifies 2 Mbytes, and num_pages specifies 3 Mbytes, then num_ballooned
will come back as 2 Mbytes, which is correct. But if we revert alloc_unit to 1 page and
"continue" in that case, we will lose the 2 Mbytes of memory (it's not freed), and the
next time through the loop will try to allocate only 1 Mbyte (because num_pages
will be decremented by num_ballooned). I think the original code does the right thing.
Michael
> alloc_unit = 1;
> continue;
> }
> --
> 2.14.5
^ permalink raw reply
* [PATCH v2] Drivers: hv: vmbus: Ignore CHANNELMSG_TL_CONNECT_RESULT(23)
From: Dexuan Cui @ 2020-01-19 23:29 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, linux-hyperv, linux-kernel,
mikelley, Alexander.Levin
Cc: sunilmut, Andrea.Parri, weh, Dexuan Cui
When a Linux hv_sock app tries to connect to a Service GUID on which no
host app is listening, a recent host (RS3+) sends a
CHANNELMSG_TL_CONNECT_RESULT (23) message to Linux and this triggers such
a warning:
unknown msgtype=23
WARNING: CPU: 2 PID: 0 at drivers/hv/vmbus_drv.c:1031 vmbus_on_msg_dpc
Actually Linux can safely ignore the message because the Linux app's
connect() will time out in 2 seconds: see VSOCK_DEFAULT_CONNECT_TIMEOUT
and vsock_stream_connect(). We don't bother to make use of the message
because: 1) it's only supported on recent hosts; 2) a non-trivial effort
is required to use the message in Linux, but the benefit is small.
So, let's not see the warning by silently ignoring the message.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
In v2 (followed Michael Kelley's suggestions):
Removed the redundant code in vmbus_onmessage()
Added the new enries into channel_message_table[].
drivers/hv/channel_mgmt.c | 21 +++++++--------------
drivers/hv/vmbus_drv.c | 4 ++++
include/linux/hyperv.h | 2 ++
3 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 8eb167540b4f..0370364169c4 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -1351,6 +1351,8 @@ channel_message_table[CHANNELMSG_COUNT] = {
{ CHANNELMSG_19, 0, NULL },
{ CHANNELMSG_20, 0, NULL },
{ CHANNELMSG_TL_CONNECT_REQUEST, 0, NULL },
+ { CHANNELMSG_22, 0, NULL },
+ { CHANNELMSG_TL_CONNECT_RESULT, 0, NULL },
};
/*
@@ -1362,25 +1364,16 @@ void vmbus_onmessage(void *context)
{
struct hv_message *msg = context;
struct vmbus_channel_message_header *hdr;
- int size;
hdr = (struct vmbus_channel_message_header *)msg->u.payload;
- size = msg->header.payload_size;
trace_vmbus_on_message(hdr);
- if (hdr->msgtype >= CHANNELMSG_COUNT) {
- pr_err("Received invalid channel message type %d size %d\n",
- hdr->msgtype, size);
- print_hex_dump_bytes("", DUMP_PREFIX_NONE,
- (unsigned char *)msg->u.payload, size);
- return;
- }
-
- if (channel_message_table[hdr->msgtype].message_handler)
- channel_message_table[hdr->msgtype].message_handler(hdr);
- else
- pr_err("Unhandled channel message type %d\n", hdr->msgtype);
+ /*
+ * vmbus_on_msg_dpc() makes sure the hdr->msgtype here can not go
+ * out of bound and the message_handler pointer can not be NULL.
+ */
+ channel_message_table[hdr->msgtype].message_handler(hdr);
}
/*
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 4ef5a66df680..029378c27421 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1033,6 +1033,10 @@ void vmbus_on_msg_dpc(unsigned long data)
}
entry = &channel_message_table[hdr->msgtype];
+
+ if (!entry->message_handler)
+ goto msg_handled;
+
if (entry->handler_type == VMHT_BLOCKING) {
ctx = kmalloc(sizeof(*ctx), GFP_ATOMIC);
if (ctx == NULL)
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 8fa0938f9aee..692c89ccf5df 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -425,6 +425,8 @@ enum vmbus_channel_message_type {
CHANNELMSG_19 = 19,
CHANNELMSG_20 = 20,
CHANNELMSG_TL_CONNECT_REQUEST = 21,
+ CHANNELMSG_22 = 22,
+ CHANNELMSG_TL_CONNECT_RESULT = 23,
CHANNELMSG_COUNT
};
--
2.19.1
^ permalink raw reply related
* RE: [PATCH] Drivers: hv: vmbus: Ignore CHANNELMSG_TL_CONNECT_RESULT(23)
From: Dexuan Cui @ 2020-01-19 23:35 UTC (permalink / raw)
To: Michael Kelley, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, Sasha Levin
Cc: Sunil Muthuswamy, Andrea Parri
In-Reply-To: <MW2PR2101MB10524758141F09806D8677FCD7300@MW2PR2101MB1052.namprd21.prod.outlook.com>
> From: Michael Kelley
> Sent: Saturday, January 18, 2020 9:29 AM
> > +
> > + /* Linux ignores some messages, e.g.
> CHANNELMSG_TL_CONNECT_RESULT. */
> > + if (!entry->message_handler)
> > + goto msg_handled;
> > +
>
> FWIW, with this new check, all of the validity checks in vmbus_onmessage()
> are redundant and could be removed. There's already a check here that
> ensures msgtype won't be too big, and this new check ensures that
> message_handler is not NULL.
Agreed. I just sent a v2 that removes the redundant code.
> > if (entry->handler_type == VMHT_BLOCKING) {
> > ctx = kmalloc(sizeof(*ctx), GFP_ATOMIC);
> > if (ctx == NULL)
> > diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> > index 26f3aeeae1ca..41c58011431e 100644
> > --- a/include/linux/hyperv.h
> > +++ b/include/linux/hyperv.h
> > @@ -425,6 +425,8 @@ enum vmbus_channel_message_type {
> > CHANNELMSG_19 = 19,
> > CHANNELMSG_20 = 20,
> > CHANNELMSG_TL_CONNECT_REQUEST = 21,
> > + CHANNELMSG_22 = 22,
> > + CHANNELMSG_TL_CONNECT_RESULT = 23,
> > CHANNELMSG_COUNT
> > };
>
> For completeness, I'd like to see the channel_message_table also updated
> with these new entries so that everything stays in sync and is explicitly
> defined.
>
> Michael
I fixed this in v2 as well.
Thanks!
Dexuan
^ permalink raw reply
* RE: [PATCH v2] Drivers: hv: vmbus: Ignore CHANNELMSG_TL_CONNECT_RESULT(23)
From: Michael Kelley @ 2020-01-20 0:51 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, Sasha Levin
Cc: Sunil Muthuswamy, Andrea Parri, Wei Hu
In-Reply-To: <1579476562-125673-1-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Sunday, January 19, 2020 3:29 PM
>
> When a Linux hv_sock app tries to connect to a Service GUID on which no
> host app is listening, a recent host (RS3+) sends a
> CHANNELMSG_TL_CONNECT_RESULT (23) message to Linux and this triggers such
> a warning:
>
> unknown msgtype=23
> WARNING: CPU: 2 PID: 0 at drivers/hv/vmbus_drv.c:1031 vmbus_on_msg_dpc
>
> Actually Linux can safely ignore the message because the Linux app's
> connect() will time out in 2 seconds: see VSOCK_DEFAULT_CONNECT_TIMEOUT
> and vsock_stream_connect(). We don't bother to make use of the message
> because: 1) it's only supported on recent hosts; 2) a non-trivial effort
> is required to use the message in Linux, but the benefit is small.
>
> So, let's not see the warning by silently ignoring the message.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>
> In v2 (followed Michael Kelley's suggestions):
> Removed the redundant code in vmbus_onmessage()
> Added the new enries into channel_message_table[].
>
> drivers/hv/channel_mgmt.c | 21 +++++++--------------
> drivers/hv/vmbus_drv.c | 4 ++++
> include/linux/hyperv.h | 2 ++
> 3 files changed, 13 insertions(+), 14 deletions(-)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH V2] x86/Hyper-V: Balloon up according to request page number
From: Tianyu Lan @ 2020-01-20 7:56 UTC (permalink / raw)
To: Michael Kelley, lantianyu1986@gmail.com, KY Srinivasan,
Haiyang Zhang, Stephen Hemminger, sashal@kernel.org
Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
vkuznets, stable@vger.kernel.org
In-Reply-To: <MW2PR2101MB10520A27DC77E3B2F15EC75FD7300@MW2PR2101MB1052.namprd21.prod.outlook.com>
Hi Michael:
Thanks for your review.
> From: Michael Kelley <mikelley@microsoft.com>
> Sent: Sunday, January 19, 2020 3:29 AM
> To: lantianyu1986@gmail.com; KY Srinivasan <kys@microsoft.com>; Haiyang
> Zhang <haiyangz@microsoft.com>; Stephen Hemminger
> <sthemmin@microsoft.com>; sashal@kernel.org
> Cc: Tianyu Lan <Tianyu.Lan@microsoft.com>; linux-hyperv@vger.kernel.org;
> linux-kernel@vger.kernel.org; vkuznets <vkuznets@redhat.com>;
> stable@vger.kernel.org
> Subject: RE: [PATCH V2] x86/Hyper-V: Balloon up according to request page
> number
>
> From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Thursday, January 16,
> 2020 6:16 AM
> >
> > Current code has assumption that balloon request memory size aligns
> > with 2MB. But actually Hyper-V doesn't guarantee such alignment. When
> > balloon driver receives non-aligned balloon request, it produces
> > warning and balloon up more memory than requested in order to keep 2MB
> alignment.
> > Remove the warning and balloon up memory according to actual requested
> > memory size.
> >
> > Fixes: f6712238471a ("hv: hv_balloon: avoid memory leak on alloc_error
> > of 2MB memory
> > block")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> > ---
> > Change since v2:
> > - Change logic of switching alloc_unit from 2MB to 4KB
> > in the balloon_up() to avoid redundant iteration when
> > handle non-aligned page request.
> > - Remove 2MB alignment operation and comment in balloon_up()
> > ---
> > drivers/hv/hv_balloon.c | 12 ++++--------
> > 1 file changed, 4 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c index
> > 7f3e7ab22d5d..536807efbc35 100644
> > --- a/drivers/hv/hv_balloon.c
> > +++ b/drivers/hv/hv_balloon.c
> > @@ -1684,7 +1684,7 @@ static unsigned int alloc_balloon_pages(struct
> > hv_dynmem_device *dm,
> > if (num_pages < alloc_unit)
> > return 0;
>
> The above test is no longer necessary. The num_pages < alloc_unit case is
> handled implicitly by your new 'for' loop condition.
>
Yes, will update in the next version.
> >
> > - for (i = 0; (i * alloc_unit) < num_pages; i++) {
> > + for (i = 0; i < num_pages / alloc_unit; i++) {
> > if (bl_resp->hdr.size + sizeof(union dm_mem_page_range) >
> > HV_HYP_PAGE_SIZE)
> > return i * alloc_unit;
> > @@ -1722,7 +1722,7 @@ static unsigned int alloc_balloon_pages(struct
> > hv_dynmem_device *dm,
> >
> > }
> >
> > - return num_pages;
> > + return i * alloc_unit;
> > }
> >
> > static void balloon_up(union dm_msg_info *msg_info) @@ -1737,9
> > +1737,6 @@ static void balloon_up(union dm_msg_info *msg_info)
> > long avail_pages;
> > unsigned long floor;
> >
> > - /* The host balloons pages in 2M granularity. */
> > - WARN_ON_ONCE(num_pages % PAGES_IN_2M != 0);
> > -
> > /*
> > * We will attempt 2M allocations. However, if we fail to
> > * allocate 2M chunks, we will go back to PAGE_SIZE allocations.
> > @@ -1749,14 +1746,13 @@ static void balloon_up(union dm_msg_info
> *msg_info)
> > avail_pages = si_mem_available();
> > floor = compute_balloon_floor();
> >
> > - /* Refuse to balloon below the floor, keep the 2M granularity. */
> > + /* Refuse to balloon below the floor. */
> > if (avail_pages < num_pages || avail_pages - num_pages < floor) {
> > pr_warn("Balloon request will be partially fulfilled. %s\n",
> > avail_pages < num_pages ? "Not enough memory." :
> > "Balloon floor reached.");
> >
> > num_pages = avail_pages > floor ? (avail_pages - floor) : 0;
> > - num_pages -= num_pages % PAGES_IN_2M;
> > }
> >
> > while (!done) {
> > @@ -1770,7 +1766,7 @@ static void balloon_up(union dm_msg_info
> *msg_info)
> > num_ballooned = alloc_balloon_pages(&dm_device,
> num_pages,
> > bl_resp, alloc_unit);
> >
> > - if (alloc_unit != 1 && num_ballooned == 0) {
> > + if (alloc_unit != 1 && num_ballooned != num_pages) {
>
> Maybe I'm missing something, but I don't think Vitaly's optimization works. If
> alloc_unit specifies 2 Mbytes, and num_pages specifies 3 Mbytes, then
> num_ballooned will come back as 2 Mbytes, which is correct. But if we revert
> alloc_unit to 1 page and "continue" in that case, we will lose the 2 Mbytes of
> memory (it's not freed), and the next time through the loop will try to allocate
> only 1 Mbyte (because num_pages will be decremented by num_ballooned). I
> think the original code does the right thing.
>
> Michael
Sorry. I should remove the "continue" here and then it will work. Will fix this in the
next version.
>
> > alloc_unit = 1;
> > continue;
> > }
> > --
> > 2.14.5
^ permalink raw reply
* [PATCH V3] x86/Hyper-V: Balloon up according to request page number
From: lantianyu1986 @ 2020-01-20 8:41 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, michael.h.kelley
Cc: Tianyu Lan, linux-hyperv, linux-kernel, vkuznets, stable
From: Tianyu Lan <Tianyu.Lan@microsoft.com>
Current code has assumption that balloon request memory size aligns
with 2MB. But actually Hyper-V doesn't guarantee such alignment. When
balloon driver receives non-aligned balloon request, it produces warning
and balloon up more memory than requested in order to keep 2MB alignment.
Remove the warning and balloon up memory according to actual requested
memory size.
Fixes: f6712238471a ("hv: hv_balloon: avoid memory leak on alloc_error of 2MB memory block")
Cc: stable@vger.kernel.org
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
Change since v2:
- Remove check between request page number and alloc_unit
in the alloc_balloon_pages() because it's redundant with
new change.
- Remove the "continue" just follwoing alloc_unit switch
from 2MB to 4K in order to avoid skipping allocated
memory.
Change since v1:
- Change logic of switching alloc_unit from 2MB to 4KB
in the balloon_up() to avoid redundant iteration when
handle non-aligned page request.
- Remove 2MB alignment operation and comment in balloon_up()
---
drivers/hv/hv_balloon.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 7f3e7ab22d5d..73092a7a3345 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -1681,10 +1681,7 @@ static unsigned int alloc_balloon_pages(struct hv_dynmem_device *dm,
unsigned int i, j;
struct page *pg;
- if (num_pages < alloc_unit)
- return 0;
-
- for (i = 0; (i * alloc_unit) < num_pages; i++) {
+ for (i = 0; i < num_pages / alloc_unit; i++) {
if (bl_resp->hdr.size + sizeof(union dm_mem_page_range) >
HV_HYP_PAGE_SIZE)
return i * alloc_unit;
@@ -1722,7 +1719,7 @@ static unsigned int alloc_balloon_pages(struct hv_dynmem_device *dm,
}
- return num_pages;
+ return i * alloc_unit;
}
static void balloon_up(union dm_msg_info *msg_info)
@@ -1737,9 +1734,6 @@ static void balloon_up(union dm_msg_info *msg_info)
long avail_pages;
unsigned long floor;
- /* The host balloons pages in 2M granularity. */
- WARN_ON_ONCE(num_pages % PAGES_IN_2M != 0);
-
/*
* We will attempt 2M allocations. However, if we fail to
* allocate 2M chunks, we will go back to PAGE_SIZE allocations.
@@ -1749,14 +1743,13 @@ static void balloon_up(union dm_msg_info *msg_info)
avail_pages = si_mem_available();
floor = compute_balloon_floor();
- /* Refuse to balloon below the floor, keep the 2M granularity. */
+ /* Refuse to balloon below the floor. */
if (avail_pages < num_pages || avail_pages - num_pages < floor) {
pr_warn("Balloon request will be partially fulfilled. %s\n",
avail_pages < num_pages ? "Not enough memory." :
"Balloon floor reached.");
num_pages = avail_pages > floor ? (avail_pages - floor) : 0;
- num_pages -= num_pages % PAGES_IN_2M;
}
while (!done) {
@@ -1770,10 +1763,8 @@ static void balloon_up(union dm_msg_info *msg_info)
num_ballooned = alloc_balloon_pages(&dm_device, num_pages,
bl_resp, alloc_unit);
- if (alloc_unit != 1 && num_ballooned == 0) {
+ if (alloc_unit != 1 && num_ballooned != num_pages)
alloc_unit = 1;
- continue;
- }
if (num_ballooned == 0 || num_ballooned == num_pages) {
pr_debug("Ballooned %u out of %u requested pages.\n",
--
2.14.5
^ permalink raw reply related
* Re: [PATCH net-next 1/3] vsock: add network namespace support
From: David Miller @ 2020-01-20 9:06 UTC (permalink / raw)
To: sgarzare
Cc: netdev, linux-kernel, jhansen, jasowang, kvm, stefanha,
virtualization, linux-hyperv, mst, decui
In-Reply-To: <20200116172428.311437-2-sgarzare@redhat.com>
From: Stefano Garzarella <sgarzare@redhat.com>
Date: Thu, 16 Jan 2020 18:24:26 +0100
> This patch adds 'netns' module param to enable this new feature
> (disabled by default), because it changes vsock's behavior with
> network namespaces and could break existing applications.
Sorry, no.
I wonder if you can even design a legitimate, reasonable, use case
where these netns changes could break things.
I am totally against adding a module parameter for this, it's
incredibly confusing for users and will create a test scenerio
that is strongly less likely to be covered.
^ permalink raw reply
* Re: [PATCH net-next 1/3] vsock: add network namespace support
From: Stefano Garzarella @ 2020-01-20 10:17 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-kernel, jhansen, jasowang, kvm, stefanha,
virtualization, linux-hyperv, mst, decui
In-Reply-To: <20200120.100610.546818167633238909.davem@davemloft.net>
On Mon, Jan 20, 2020 at 10:06:10AM +0100, David Miller wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
> Date: Thu, 16 Jan 2020 18:24:26 +0100
>
> > This patch adds 'netns' module param to enable this new feature
> > (disabled by default), because it changes vsock's behavior with
> > network namespaces and could break existing applications.
>
> Sorry, no.
>
> I wonder if you can even design a legitimate, reasonable, use case
> where these netns changes could break things.
I forgot to mention the use case.
I tried the RFC with Kata containers and we found that Kata shim-v1
doesn't work (Kata shim-v2 works as is) because there are the following
processes involved:
- kata-runtime (runs in the init_netns) opens /dev/vhost-vsock and
passes it to qemu
- kata-shim (runs in a container) wants to talk with the guest but the
vsock device is assigned to the init_netns and kata-shim runs in a
different netns, so the communication is not allowed
But, as you said, this could be a wrong design, indeed they already
found a fix, but I was not sure if others could have the same issue.
In this case, do you think it is acceptable to make this change in
the vsock's behavior with netns and ask the user to change the design?
>
> I am totally against adding a module parameter for this, it's
> incredibly confusing for users and will create a test scenerio
> that is strongly less likely to be covered.
>
Got it, I'll remove the module parameter!
Thanks,
Stefano
^ permalink raw reply
* RE: [PATCH V2] x86/Hyper-V: Balloon up according to request page number
From: Vitaly Kuznetsov @ 2020-01-20 10:21 UTC (permalink / raw)
To: Michael Kelley, lantianyu1986@gmail.com, KY Srinivasan,
Haiyang Zhang, Stephen Hemminger, sashal@kernel.org
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
In-Reply-To: <MW2PR2101MB10520A27DC77E3B2F15EC75FD7300@MW2PR2101MB1052.namprd21.prod.outlook.com>
Michael Kelley <mikelley@microsoft.com> writes:
> From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Thursday, January 16, 2020 6:16 AM
>>
>> Current code has assumption that balloon request memory size aligns
>> with 2MB. But actually Hyper-V doesn't guarantee such alignment. When
>> balloon driver receives non-aligned balloon request, it produces warning
>> and balloon up more memory than requested in order to keep 2MB alignment.
>> Remove the warning and balloon up memory according to actual requested
>> memory size.
>>
>> Fixes: f6712238471a ("hv: hv_balloon: avoid memory leak on alloc_error of 2MB memory
>> block")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
>> ---
>> Change since v2:
>> - Change logic of switching alloc_unit from 2MB to 4KB
>> in the balloon_up() to avoid redundant iteration when
>> handle non-aligned page request.
>> - Remove 2MB alignment operation and comment in balloon_up()
>> ---
>> drivers/hv/hv_balloon.c | 12 ++++--------
>> 1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
>> index 7f3e7ab22d5d..536807efbc35 100644
>> --- a/drivers/hv/hv_balloon.c
>> +++ b/drivers/hv/hv_balloon.c
>> @@ -1684,7 +1684,7 @@ static unsigned int alloc_balloon_pages(struct
>> hv_dynmem_device *dm,
>> if (num_pages < alloc_unit)
>> return 0;
>
> The above test is no longer necessary. The num_pages < alloc_unit
> case is handled implicitly by your new 'for' loop condition.
>
>>
>> - for (i = 0; (i * alloc_unit) < num_pages; i++) {
>> + for (i = 0; i < num_pages / alloc_unit; i++) {
>> if (bl_resp->hdr.size + sizeof(union dm_mem_page_range) >
>> HV_HYP_PAGE_SIZE)
>> return i * alloc_unit;
>> @@ -1722,7 +1722,7 @@ static unsigned int alloc_balloon_pages(struct
>> hv_dynmem_device *dm,
>>
>> }
>>
>> - return num_pages;
>> + return i * alloc_unit;
>> }
>>
>> static void balloon_up(union dm_msg_info *msg_info)
>> @@ -1737,9 +1737,6 @@ static void balloon_up(union dm_msg_info *msg_info)
>> long avail_pages;
>> unsigned long floor;
>>
>> - /* The host balloons pages in 2M granularity. */
>> - WARN_ON_ONCE(num_pages % PAGES_IN_2M != 0);
>> -
>> /*
>> * We will attempt 2M allocations. However, if we fail to
>> * allocate 2M chunks, we will go back to PAGE_SIZE allocations.
>> @@ -1749,14 +1746,13 @@ static void balloon_up(union dm_msg_info *msg_info)
>> avail_pages = si_mem_available();
>> floor = compute_balloon_floor();
>>
>> - /* Refuse to balloon below the floor, keep the 2M granularity. */
>> + /* Refuse to balloon below the floor. */
>> if (avail_pages < num_pages || avail_pages - num_pages < floor) {
>> pr_warn("Balloon request will be partially fulfilled. %s\n",
>> avail_pages < num_pages ? "Not enough memory." :
>> "Balloon floor reached.");
>>
>> num_pages = avail_pages > floor ? (avail_pages - floor) : 0;
>> - num_pages -= num_pages % PAGES_IN_2M;
>> }
>>
>> while (!done) {
>> @@ -1770,7 +1766,7 @@ static void balloon_up(union dm_msg_info *msg_info)
>> num_ballooned = alloc_balloon_pages(&dm_device, num_pages,
>> bl_resp, alloc_unit);
>>
>> - if (alloc_unit != 1 && num_ballooned == 0) {
>> + if (alloc_unit != 1 && num_ballooned != num_pages) {
>
> Maybe I'm missing something, but I don't think Vitaly's optimization works. If
> alloc_unit specifies 2 Mbytes, and num_pages specifies 3 Mbytes, then num_ballooned
> will come back as 2 Mbytes, which is correct. But if we revert alloc_unit to 1 page and
> "continue" in that case, we will lose the 2 Mbytes of memory (it's not freed), and the
> next time through the loop will try to allocate only 1 Mbyte (because num_pages
> will be decremented by num_ballooned). I think the original code does the right thing.
>
True, nice catch! What I meant to say is that we can avoid sending two
replies to the host and get away with just one. Unfortunately,
"num_ballooned != num_pages" modification is not sufficient as
alloc_balloon_pages() will overwrite bl_resp->range_array[] (as it
always starts from 0). I think we can still achieve the goal by
allocating bl_resp outside of the loop and filling it from
range_array[range_count] instead of range_array[i] but it seems to big
of a change for now particular gain. Let's just drop the idea.
> Michael
>
>> alloc_unit = 1;
>> continue;
>> }
>> --
>> 2.14.5
>
--
Vitaly
^ permalink raw reply
* Re: [PATCH net-next 1/3] vsock: add network namespace support
From: Michael S. Tsirkin @ 2020-01-20 12:03 UTC (permalink / raw)
To: Stefano Garzarella
Cc: David Miller, netdev, linux-kernel, jhansen, jasowang, kvm,
stefanha, virtualization, linux-hyperv, decui
In-Reply-To: <20200120101735.uyh4o64gb4njakw5@steredhat>
On Mon, Jan 20, 2020 at 11:17:35AM +0100, Stefano Garzarella wrote:
> On Mon, Jan 20, 2020 at 10:06:10AM +0100, David Miller wrote:
> > From: Stefano Garzarella <sgarzare@redhat.com>
> > Date: Thu, 16 Jan 2020 18:24:26 +0100
> >
> > > This patch adds 'netns' module param to enable this new feature
> > > (disabled by default), because it changes vsock's behavior with
> > > network namespaces and could break existing applications.
> >
> > Sorry, no.
> >
> > I wonder if you can even design a legitimate, reasonable, use case
> > where these netns changes could break things.
>
> I forgot to mention the use case.
> I tried the RFC with Kata containers and we found that Kata shim-v1
> doesn't work (Kata shim-v2 works as is) because there are the following
> processes involved:
> - kata-runtime (runs in the init_netns) opens /dev/vhost-vsock and
> passes it to qemu
> - kata-shim (runs in a container) wants to talk with the guest but the
> vsock device is assigned to the init_netns and kata-shim runs in a
> different netns, so the communication is not allowed
> But, as you said, this could be a wrong design, indeed they already
> found a fix, but I was not sure if others could have the same issue.
>
> In this case, do you think it is acceptable to make this change in
> the vsock's behavior with netns and ask the user to change the design?
David's question is what would be a usecase that's broken
(as opposed to fixed) by enabling this by default.
If it does exist, you need a way for userspace to opt-in,
module parameter isn't that.
>
> >
> > I am totally against adding a module parameter for this, it's
> > incredibly confusing for users and will create a test scenerio
> > that is strongly less likely to be covered.
> >
>
> Got it, I'll remove the module parameter!
>
> Thanks,
> Stefano
^ permalink raw reply
* Re: [PATCH net-next 1/3] vsock: add network namespace support
From: Stefano Garzarella @ 2020-01-20 13:58 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David Miller, netdev, linux-kernel, Jorgen Hansen, Jason Wang,
kvm, Stefan Hajnoczi, virtualization, linux-hyperv, Dexuan Cui
In-Reply-To: <20200120060601-mutt-send-email-mst@kernel.org>
On Mon, Jan 20, 2020 at 1:03 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> On Mon, Jan 20, 2020 at 11:17:35AM +0100, Stefano Garzarella wrote:
> > On Mon, Jan 20, 2020 at 10:06:10AM +0100, David Miller wrote:
> > > From: Stefano Garzarella <sgarzare@redhat.com>
> > > Date: Thu, 16 Jan 2020 18:24:26 +0100
> > >
> > > > This patch adds 'netns' module param to enable this new feature
> > > > (disabled by default), because it changes vsock's behavior with
> > > > network namespaces and could break existing applications.
> > >
> > > Sorry, no.
> > >
> > > I wonder if you can even design a legitimate, reasonable, use case
> > > where these netns changes could break things.
> >
> > I forgot to mention the use case.
> > I tried the RFC with Kata containers and we found that Kata shim-v1
> > doesn't work (Kata shim-v2 works as is) because there are the following
> > processes involved:
> > - kata-runtime (runs in the init_netns) opens /dev/vhost-vsock and
> > passes it to qemu
> > - kata-shim (runs in a container) wants to talk with the guest but the
> > vsock device is assigned to the init_netns and kata-shim runs in a
> > different netns, so the communication is not allowed
> > But, as you said, this could be a wrong design, indeed they already
> > found a fix, but I was not sure if others could have the same issue.
> >
> > In this case, do you think it is acceptable to make this change in
> > the vsock's behavior with netns and ask the user to change the design?
>
> David's question is what would be a usecase that's broken
> (as opposed to fixed) by enabling this by default.
Yes, I got that. Thanks for clarifying.
I just reported a broken example that can be fixed with a different
design (due to the fact that before this series, vsock devices were
accessible to all netns).
>
> If it does exist, you need a way for userspace to opt-in,
> module parameter isn't that.
Okay, but I honestly can't find a case that can't be solved.
So I don't know whether to add an option (ioctl, sysfs ?) or wait for
a real case to come up.
I'll try to see better if there's any particular case where we need
to disable netns in vsock.
Thanks,
Stefano
^ permalink raw reply
* Re: [EXTERNAL] Re: [RFC PATCH V2 2/10] mm: expose is_mem_section_removable() symbol
From: Michal Hocko @ 2020-01-20 14:14 UTC (permalink / raw)
To: Tianyu Lan
Cc: David Hildenbrand, lantianyu1986@gmail.com, KY Srinivasan,
Haiyang Zhang, Stephen Hemminger, sashal@kernel.org,
akpm@linux-foundation.org, Michael Kelley,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, vkuznets, eric.devolder@oracle.com,
vbabka@suse.cz, osalvador@suse.de, Pasha Tatashin,
rppt@linux.ibm.com
In-Reply-To: <PS1P15301MB034764C1FFA3D2711DAED14C92360@PS1P15301MB0347.APCP153.PROD.OUTLOOK.COM>
On Fri 17-01-20 16:35:03, Tianyu Lan wrote:
[...]
> > Could you describe your usecase in more details please?
>
> Hyper-V sends hot-remove request message which just contains requested
> page number but not provide detail range. So Hyper-V driver needs to search
> suitable memory block in system memory to return back to host if there is no
> memory hot-add before. So I used the is_mem_section_removable() do such check.
As David described, you would be much better of by using
alloc_contig_range to find a memory that would be suitable for
hotremoving without any races.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH net-next 1/3] vsock: add network namespace support
From: Michael S. Tsirkin @ 2020-01-20 16:04 UTC (permalink / raw)
To: Stefano Garzarella
Cc: David Miller, netdev, linux-kernel, Jorgen Hansen, Jason Wang,
kvm, Stefan Hajnoczi, virtualization, linux-hyperv, Dexuan Cui
In-Reply-To: <CAGxU2F6VH8Eb5UH_9KjN6MONbZEo1D7EHAiocVVus6jW55BJDg@mail.gmail.com>
On Mon, Jan 20, 2020 at 02:58:01PM +0100, Stefano Garzarella wrote:
> On Mon, Jan 20, 2020 at 1:03 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Mon, Jan 20, 2020 at 11:17:35AM +0100, Stefano Garzarella wrote:
> > > On Mon, Jan 20, 2020 at 10:06:10AM +0100, David Miller wrote:
> > > > From: Stefano Garzarella <sgarzare@redhat.com>
> > > > Date: Thu, 16 Jan 2020 18:24:26 +0100
> > > >
> > > > > This patch adds 'netns' module param to enable this new feature
> > > > > (disabled by default), because it changes vsock's behavior with
> > > > > network namespaces and could break existing applications.
> > > >
> > > > Sorry, no.
> > > >
> > > > I wonder if you can even design a legitimate, reasonable, use case
> > > > where these netns changes could break things.
> > >
> > > I forgot to mention the use case.
> > > I tried the RFC with Kata containers and we found that Kata shim-v1
> > > doesn't work (Kata shim-v2 works as is) because there are the following
> > > processes involved:
> > > - kata-runtime (runs in the init_netns) opens /dev/vhost-vsock and
> > > passes it to qemu
> > > - kata-shim (runs in a container) wants to talk with the guest but the
> > > vsock device is assigned to the init_netns and kata-shim runs in a
> > > different netns, so the communication is not allowed
> > > But, as you said, this could be a wrong design, indeed they already
> > > found a fix, but I was not sure if others could have the same issue.
> > >
> > > In this case, do you think it is acceptable to make this change in
> > > the vsock's behavior with netns and ask the user to change the design?
> >
> > David's question is what would be a usecase that's broken
> > (as opposed to fixed) by enabling this by default.
>
> Yes, I got that. Thanks for clarifying.
> I just reported a broken example that can be fixed with a different
> design (due to the fact that before this series, vsock devices were
> accessible to all netns).
>
> >
> > If it does exist, you need a way for userspace to opt-in,
> > module parameter isn't that.
>
> Okay, but I honestly can't find a case that can't be solved.
> So I don't know whether to add an option (ioctl, sysfs ?) or wait for
> a real case to come up.
>
> I'll try to see better if there's any particular case where we need
> to disable netns in vsock.
>
> Thanks,
> Stefano
Me neither. so what did you have in mind when you wrote:
"could break existing applications"?
^ permalink raw reply
* Re: [PATCH net-next 1/3] vsock: add network namespace support
From: Stefano Garzarella @ 2020-01-20 16:53 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David Miller, netdev, linux-kernel, Jorgen Hansen, Jason Wang,
kvm, Stefan Hajnoczi, virtualization, linux-hyperv, Dexuan Cui
In-Reply-To: <20200120110319-mutt-send-email-mst@kernel.org>
On Mon, Jan 20, 2020 at 5:04 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> On Mon, Jan 20, 2020 at 02:58:01PM +0100, Stefano Garzarella wrote:
> > On Mon, Jan 20, 2020 at 1:03 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > On Mon, Jan 20, 2020 at 11:17:35AM +0100, Stefano Garzarella wrote:
> > > > On Mon, Jan 20, 2020 at 10:06:10AM +0100, David Miller wrote:
> > > > > From: Stefano Garzarella <sgarzare@redhat.com>
> > > > > Date: Thu, 16 Jan 2020 18:24:26 +0100
> > > > >
> > > > > > This patch adds 'netns' module param to enable this new feature
> > > > > > (disabled by default), because it changes vsock's behavior with
> > > > > > network namespaces and could break existing applications.
> > > > >
> > > > > Sorry, no.
> > > > >
> > > > > I wonder if you can even design a legitimate, reasonable, use case
> > > > > where these netns changes could break things.
> > > >
> > > > I forgot to mention the use case.
> > > > I tried the RFC with Kata containers and we found that Kata shim-v1
> > > > doesn't work (Kata shim-v2 works as is) because there are the following
> > > > processes involved:
> > > > - kata-runtime (runs in the init_netns) opens /dev/vhost-vsock and
> > > > passes it to qemu
> > > > - kata-shim (runs in a container) wants to talk with the guest but the
> > > > vsock device is assigned to the init_netns and kata-shim runs in a
> > > > different netns, so the communication is not allowed
> > > > But, as you said, this could be a wrong design, indeed they already
> > > > found a fix, but I was not sure if others could have the same issue.
> > > >
> > > > In this case, do you think it is acceptable to make this change in
> > > > the vsock's behavior with netns and ask the user to change the design?
> > >
> > > David's question is what would be a usecase that's broken
> > > (as opposed to fixed) by enabling this by default.
> >
> > Yes, I got that. Thanks for clarifying.
> > I just reported a broken example that can be fixed with a different
> > design (due to the fact that before this series, vsock devices were
> > accessible to all netns).
> >
> > >
> > > If it does exist, you need a way for userspace to opt-in,
> > > module parameter isn't that.
> >
> > Okay, but I honestly can't find a case that can't be solved.
> > So I don't know whether to add an option (ioctl, sysfs ?) or wait for
> > a real case to come up.
> >
> > I'll try to see better if there's any particular case where we need
> > to disable netns in vsock.
> >
> > Thanks,
> > Stefano
>
> Me neither. so what did you have in mind when you wrote:
> "could break existing applications"?
I had in mind:
1. the Kata case. It is fixable (the fix is not merged on kata), but
older versions will not work with newer Linux.
2. a single process running on init_netns that wants to communicate with
VMs handled by VMMs running in different netns, but this case can be
solved opening the /dev/vhost-vsock in the same netns of the process
that wants to communicate with the VMs (init_netns in this case), and
passig it to the VMM.
These cases can work with vsock+netns, but they require changes because
I'm modifying the vsock behavior with netns.
^ permalink raw reply
* RE: [RFC PATCH V2 1/10] mm/resource: Move child to new resource when release mem region.
From: Michael Kelley @ 2020-01-20 18:34 UTC (permalink / raw)
To: lantianyu1986@gmail.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, sashal@kernel.org, akpm@linux-foundation.org,
dan.j.williams@intel.com, jgg@ziepe.ca,
dave.hansen@linux.intel.com, richardw.yang@linux.intel.com,
namit@vmware.com, Tianyu Lan, david@redhat.com,
christophe.leroy@c-s.fr
Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, vkuznets, eric.devolder@oracle.com
In-Reply-To: <20200107130950.2983-2-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, January 7, 2020 5:10 AM
>
> When release mem region, old mem region may be splited to
> two regions. Current allocate new struct resource for high
> end mem region but not move child resources whose ranges are
> in the high end range to new resource. When adjust old mem
> region's range, adjust_resource() detects child region's range
> is out of new range and return error. Move child resources to
> high end resource before adjusting old mem range.
>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> kernel/resource.c | 38 ++++++++++++++++++++++++++++++++++----
> 1 file changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 76036a41143b..1c7362825134 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -181,6 +181,38 @@ static struct resource *alloc_resource(gfp_t flags)
> return res;
> }
>
> +static void move_child_to_newresource(struct resource *old,
> + struct resource *new)
> +{
> + struct resource *tmp, **p, **np;
> +
> + if (!old->child)
> + return;
I don't think the above test is needed. This case is handled by the first
three lines of the "for" loop.
> +
> + p = &old->child;
> + np = &new->child;
> +
> + for (;;) {
> + tmp = *p;
> + if (!tmp)
> + break;
> +
> + if (tmp->start >= new->start && tmp->end <= new->end) {
> + tmp->parent = new;
> + *np = tmp;
> + np = &tmp->sibling;
> + *p = tmp->sibling;
> +
> + if (!tmp->sibling)
> + *np = NULL;
I don't think the above two lines are right. They seem tautological. If the ! were
removed it would be clearing the sibling link for the child as it exists under its new
parent, which should be done. But the child that is moved to the new parent always
becomes the last entry in the new parent's child list. So could you just unconditionally
do tmp->sibling = NULL? That link will get fixed up if another child is moved.
Michael
> + continue;
> + }
> +
> + p = &tmp->sibling;
> + }
> +}
> +
> +
> /* Return the conflict entry if you can't request it */
> static struct resource * __request_resource(struct resource *root, struct resource *new)
> {
> @@ -1246,9 +1278,6 @@ EXPORT_SYMBOL(__release_region);
> * Note:
> * - Additional release conditions, such as overlapping region, can be
> * supported after they are confirmed as valid cases.
> - * - When a busy memory resource gets split into two entries, the code
> - * assumes that all children remain in the lower address entry for
> - * simplicity. Enhance this logic when necessary.
> */
> int release_mem_region_adjustable(struct resource *parent,
> resource_size_t start, resource_size_t size)
> @@ -1331,11 +1360,12 @@ int release_mem_region_adjustable(struct resource *parent,
> new_res->sibling = res->sibling;
> new_res->child = NULL;
>
> + move_child_to_newresource(res, new_res);
> + res->sibling = new_res;
> ret = __adjust_resource(res, res->start,
> start - res->start);
> if (ret)
> break;
> - res->sibling = new_res;
> new_res = NULL;
> }
>
> --
> 2.14.5
^ permalink raw reply
* RE: [RFC PATCH V2 3/10] x86/Hyper-V/Balloon: Replace hot-add and balloon up works with a common work
From: Michael Kelley @ 2020-01-20 19:12 UTC (permalink / raw)
To: lantianyu1986@gmail.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, sashal@kernel.org, david@redhat.com
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets, eric.devolder@oracle.com
In-Reply-To: <20200107130950.2983-4-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, January 7, 2020 5:10 AM
>
> The mem hot-remove operation and balloon down will be added
> or moved into work context. Add a common work to handle
> opeations of mem hot-add/remove and balloon up/down.
Let me suggest some improved wording for this commit message:
The Hyper-V balloon driver currently has separate work contexts for memory
hot-add operations and for balloon up (i.e., remove memory from the guest)
operations. Hot-remove is being added and must be done in a work context.
And finally, balloon down is currently not done in a work context, but needs
to move to a work context because of a Hyper-V bug that confuses balloon
operations and hot-remove. Rather than add code for two additional work
contexts, consolidate the current hot-add and balloon up operations onto
a single work context. Subsequent patches will add hot-remove and
balloon down to that same work context.
My assumption is that the consolidation into a single work context is
just code simplification. If there's another reason, please describe it.
The code is this patch looks OK to me.
Michael
>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> drivers/hv/hv_balloon.c | 86 ++++++++++++++++++++++++++++++-------------------
> 1 file changed, 52 insertions(+), 34 deletions(-)
>
^ permalink raw reply
* RE: [RFC PATCH V2 1/10] mm/resource: Move child to new resource when release mem region.
From: Michael Kelley @ 2020-01-20 19:20 UTC (permalink / raw)
To: lantianyu1986@gmail.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, sashal@kernel.org, akpm@linux-foundation.org,
dan.j.williams@intel.com, jgg@ziepe.ca,
dave.hansen@linux.intel.com, richardw.yang@linux.intel.com,
namit@vmware.com, Tianyu Lan, david@redhat.com,
christophe.leroy@c-s.fr
Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, vkuznets, eric.devolder@oracle.com
In-Reply-To: <20200107130950.2983-2-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, January 7, 2020 5:10 AM
>
> When release mem region, old mem region may be splited to
> two regions. Current allocate new struct resource for high
> end mem region but not move child resources whose ranges are
> in the high end range to new resource. When adjust old mem
> region's range, adjust_resource() detects child region's range
> is out of new range and return error. Move child resources to
> high end resource before adjusting old mem range.
Let me also suggests some wording improvements to the commit message:
When releasing a mem region, the old mem region may need to be
split into two regions. In this case, the current code allocates the new
region and adjust the original region to specify a smaller range. But child
regions that fall into the range of the new region are not moved to that
new region. Consequently, when running __adjust_resource() on the
original region, it detects that the child region's range is out of the new
range, and returns an error.
Fix this by moving appropriate child resources to the new region before
adjusting the original mem region range.
>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> kernel/resource.c | 38 ++++++++++++++++++++++++++++++++++----
> 1 file changed, 34 insertions(+), 4 deletions(-)
>
^ permalink raw reply
* RE: [PATCH V3] x86/Hyper-V: Balloon up according to request page number
From: Michael Kelley @ 2020-01-20 21:17 UTC (permalink / raw)
To: lantianyu1986@gmail.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, sashal@kernel.org
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets, stable@vger.kernel.org
In-Reply-To: <20200120084149.4791-1-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Monday, January 20, 2020 12:42 AM
>
> Current code has assumption that balloon request memory size aligns
> with 2MB. But actually Hyper-V doesn't guarantee such alignment. When
> balloon driver receives non-aligned balloon request, it produces warning
> and balloon up more memory than requested in order to keep 2MB alignment.
> Remove the warning and balloon up memory according to actual requested
> memory size.
>
> Fixes: f6712238471a ("hv: hv_balloon: avoid memory leak on alloc_error of 2MB memory
> block")
> Cc: stable@vger.kernel.org
> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> Change since v2:
> - Remove check between request page number and alloc_unit
> in the alloc_balloon_pages() because it's redundant with
> new change.
> - Remove the "continue" just follwoing alloc_unit switch
> from 2MB to 4K in order to avoid skipping allocated
> memory.
>
> Change since v1:
> - Change logic of switching alloc_unit from 2MB to 4KB
> in the balloon_up() to avoid redundant iteration when
> handle non-aligned page request.
> - Remove 2MB alignment operation and comment in balloon_up()
> ---
> drivers/hv/hv_balloon.c | 17 ++++-------------
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index 7f3e7ab22d5d..73092a7a3345 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -1681,10 +1681,7 @@ static unsigned int alloc_balloon_pages(struct
> hv_dynmem_device *dm,
> unsigned int i, j;
> struct page *pg;
>
> - if (num_pages < alloc_unit)
> - return 0;
> -
> - for (i = 0; (i * alloc_unit) < num_pages; i++) {
> + for (i = 0; i < num_pages / alloc_unit; i++) {
> if (bl_resp->hdr.size + sizeof(union dm_mem_page_range) >
> HV_HYP_PAGE_SIZE)
> return i * alloc_unit;
> @@ -1722,7 +1719,7 @@ static unsigned int alloc_balloon_pages(struct
> hv_dynmem_device *dm,
>
> }
>
> - return num_pages;
> + return i * alloc_unit;
> }
>
> static void balloon_up(union dm_msg_info *msg_info)
> @@ -1737,9 +1734,6 @@ static void balloon_up(union dm_msg_info *msg_info)
> long avail_pages;
> unsigned long floor;
>
> - /* The host balloons pages in 2M granularity. */
> - WARN_ON_ONCE(num_pages % PAGES_IN_2M != 0);
> -
> /*
> * We will attempt 2M allocations. However, if we fail to
> * allocate 2M chunks, we will go back to PAGE_SIZE allocations.
> @@ -1749,14 +1743,13 @@ static void balloon_up(union dm_msg_info *msg_info)
> avail_pages = si_mem_available();
> floor = compute_balloon_floor();
>
> - /* Refuse to balloon below the floor, keep the 2M granularity. */
> + /* Refuse to balloon below the floor. */
> if (avail_pages < num_pages || avail_pages - num_pages < floor) {
> pr_warn("Balloon request will be partially fulfilled. %s\n",
> avail_pages < num_pages ? "Not enough memory." :
> "Balloon floor reached.");
>
> num_pages = avail_pages > floor ? (avail_pages - floor) : 0;
> - num_pages -= num_pages % PAGES_IN_2M;
> }
>
> while (!done) {
> @@ -1770,10 +1763,8 @@ static void balloon_up(union dm_msg_info *msg_info)
> num_ballooned = alloc_balloon_pages(&dm_device, num_pages,
> bl_resp, alloc_unit);
>
> - if (alloc_unit != 1 && num_ballooned == 0) {
> + if (alloc_unit != 1 && num_ballooned != num_pages)
> alloc_unit = 1;
> - continue;
> - }
I don't think removing the "continue" works either. Suppose the requested
size is 1 Mbyte. With an alloc_unit of 2M, alloc_balloon_pages() will return
zero. The code above will set alloc_unit to 1, which is correct. But without the
"continue", the code below will mark "done" as true, and we won't loop back
around to try again with alloc_unit set to 1.
I think the original code needs to stay as is.
Michael
>
> if (num_ballooned == 0 || num_ballooned == num_pages) {
> pr_debug("Ballooned %u out of %u requested pages.\n",
> --
> 2.14.5
^ permalink raw reply
* Re: [PATCH net-next 1/3] vsock: add network namespace support
From: Michael S. Tsirkin @ 2020-01-20 22:02 UTC (permalink / raw)
To: Stefano Garzarella
Cc: David Miller, netdev, linux-kernel, Jorgen Hansen, Jason Wang,
kvm, Stefan Hajnoczi, virtualization, linux-hyperv, Dexuan Cui
In-Reply-To: <CAGxU2F5=DQJ56sH4BUqp_7rvaXSF9bFHp4QkpLApJQK0bmd4MA@mail.gmail.com>
On Mon, Jan 20, 2020 at 05:53:39PM +0100, Stefano Garzarella wrote:
> On Mon, Jan 20, 2020 at 5:04 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Mon, Jan 20, 2020 at 02:58:01PM +0100, Stefano Garzarella wrote:
> > > On Mon, Jan 20, 2020 at 1:03 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > On Mon, Jan 20, 2020 at 11:17:35AM +0100, Stefano Garzarella wrote:
> > > > > On Mon, Jan 20, 2020 at 10:06:10AM +0100, David Miller wrote:
> > > > > > From: Stefano Garzarella <sgarzare@redhat.com>
> > > > > > Date: Thu, 16 Jan 2020 18:24:26 +0100
> > > > > >
> > > > > > > This patch adds 'netns' module param to enable this new feature
> > > > > > > (disabled by default), because it changes vsock's behavior with
> > > > > > > network namespaces and could break existing applications.
> > > > > >
> > > > > > Sorry, no.
> > > > > >
> > > > > > I wonder if you can even design a legitimate, reasonable, use case
> > > > > > where these netns changes could break things.
> > > > >
> > > > > I forgot to mention the use case.
> > > > > I tried the RFC with Kata containers and we found that Kata shim-v1
> > > > > doesn't work (Kata shim-v2 works as is) because there are the following
> > > > > processes involved:
> > > > > - kata-runtime (runs in the init_netns) opens /dev/vhost-vsock and
> > > > > passes it to qemu
> > > > > - kata-shim (runs in a container) wants to talk with the guest but the
> > > > > vsock device is assigned to the init_netns and kata-shim runs in a
> > > > > different netns, so the communication is not allowed
> > > > > But, as you said, this could be a wrong design, indeed they already
> > > > > found a fix, but I was not sure if others could have the same issue.
> > > > >
> > > > > In this case, do you think it is acceptable to make this change in
> > > > > the vsock's behavior with netns and ask the user to change the design?
> > > >
> > > > David's question is what would be a usecase that's broken
> > > > (as opposed to fixed) by enabling this by default.
> > >
> > > Yes, I got that. Thanks for clarifying.
> > > I just reported a broken example that can be fixed with a different
> > > design (due to the fact that before this series, vsock devices were
> > > accessible to all netns).
> > >
> > > >
> > > > If it does exist, you need a way for userspace to opt-in,
> > > > module parameter isn't that.
> > >
> > > Okay, but I honestly can't find a case that can't be solved.
> > > So I don't know whether to add an option (ioctl, sysfs ?) or wait for
> > > a real case to come up.
> > >
> > > I'll try to see better if there's any particular case where we need
> > > to disable netns in vsock.
> > >
> > > Thanks,
> > > Stefano
> >
> > Me neither. so what did you have in mind when you wrote:
> > "could break existing applications"?
>
> I had in mind:
> 1. the Kata case. It is fixable (the fix is not merged on kata), but
> older versions will not work with newer Linux.
meaning they will keep not working, right?
> 2. a single process running on init_netns that wants to communicate with
> VMs handled by VMMs running in different netns, but this case can be
> solved opening the /dev/vhost-vsock in the same netns of the process
> that wants to communicate with the VMs (init_netns in this case), and
> passig it to the VMM.
again right now they just don't work, right?
> These cases can work with vsock+netns, but they require changes because
> I'm modifying the vsock behavior with netns.
^ permalink raw reply
* [PATCH V2,net-next, 0/2] hv_netvsc: Add XDP support
From: Haiyang Zhang @ 2020-01-20 22:22 UTC (permalink / raw)
To: sashal, linux-hyperv, netdev
Cc: haiyangz, kys, sthemmin, olaf, vkuznets, davem, linux-kernel
Add XDP support and update related document.
Haiyang Zhang (2):
hv_netvsc: Add XDP support
hv_netvsc: Update document for XDP support
.../networking/device_drivers/microsoft/netvsc.txt | 21 +++
drivers/net/hyperv/Makefile | 2 +-
drivers/net/hyperv/hyperv_net.h | 21 ++-
drivers/net/hyperv/netvsc.c | 31 ++-
drivers/net/hyperv/netvsc_bpf.c | 209 +++++++++++++++++++++
drivers/net/hyperv/netvsc_drv.c | 175 ++++++++++++++---
drivers/net/hyperv/rndis_filter.c | 2 +-
7 files changed, 422 insertions(+), 39 deletions(-)
create mode 100644 drivers/net/hyperv/netvsc_bpf.c
--
1.8.3.1
^ permalink raw reply
* [PATCH V2,net-next, 1/2] hv_netvsc: Add XDP support
From: Haiyang Zhang @ 2020-01-20 22:22 UTC (permalink / raw)
To: sashal, linux-hyperv, netdev
Cc: haiyangz, kys, sthemmin, olaf, vkuznets, davem, linux-kernel
In-Reply-To: <1579558957-62496-1-git-send-email-haiyangz@microsoft.com>
This patch adds support of XDP in native mode for hv_netvsc driver, and
transparently sets the XDP program on the associated VF NIC as well.
Setting / unsetting XDP program on synthetic NIC (netvsc) propagates to
VF NIC automatically. Setting / unsetting XDP program on VF NIC directly
is not recommended, also not propagated to synthetic NIC, and may be
overwritten by setting of synthetic NIC.
XDP program cannot run with LRO (RSC) enabled, so you need to disable LRO
before running XDP:
ethtool -K eth0 lro off
XDP actions not yet supported:
XDP_REDIRECT
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
Changes:
v2: Added XDP_TX support. Addressed review comments.
---
drivers/net/hyperv/Makefile | 2 +-
drivers/net/hyperv/hyperv_net.h | 21 +++-
drivers/net/hyperv/netvsc.c | 31 +++++-
drivers/net/hyperv/netvsc_bpf.c | 209 ++++++++++++++++++++++++++++++++++++++
drivers/net/hyperv/netvsc_drv.c | 175 +++++++++++++++++++++++++------
drivers/net/hyperv/rndis_filter.c | 2 +-
6 files changed, 401 insertions(+), 39 deletions(-)
create mode 100644 drivers/net/hyperv/netvsc_bpf.c
diff --git a/drivers/net/hyperv/Makefile b/drivers/net/hyperv/Makefile
index 3a2aa07..0db7cca 100644
--- a/drivers/net/hyperv/Makefile
+++ b/drivers/net/hyperv/Makefile
@@ -1,4 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-$(CONFIG_HYPERV_NET) += hv_netvsc.o
-hv_netvsc-y := netvsc_drv.o netvsc.o rndis_filter.o netvsc_trace.o
+hv_netvsc-y := netvsc_drv.o netvsc.o rndis_filter.o netvsc_trace.o netvsc_bpf.o
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index dc44819..abda736 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -142,6 +142,8 @@ struct netvsc_device_info {
u32 send_section_size;
u32 recv_section_size;
+ struct bpf_prog *bprog;
+
u8 rss_key[NETVSC_HASH_KEYLEN];
};
@@ -189,7 +191,8 @@ int netvsc_send(struct net_device *net,
struct hv_netvsc_packet *packet,
struct rndis_message *rndis_msg,
struct hv_page_buffer *page_buffer,
- struct sk_buff *skb);
+ struct sk_buff *skb,
+ bool xdp_tx);
void netvsc_linkstatus_callback(struct net_device *net,
struct rndis_message *resp);
int netvsc_recv_callback(struct net_device *net,
@@ -198,6 +201,16 @@ int netvsc_recv_callback(struct net_device *net,
void netvsc_channel_cb(void *context);
int netvsc_poll(struct napi_struct *napi, int budget);
+u32 netvsc_run_xdp(struct net_device *ndev, struct netvsc_channel *nvchan,
+ struct xdp_buff *xdp);
+unsigned int netvsc_xdp_fraglen(unsigned int len);
+struct bpf_prog *netvsc_xdp_get(struct netvsc_device *nvdev);
+int netvsc_xdp_set(struct net_device *dev, struct bpf_prog *prog,
+ struct netlink_ext_ack *extack,
+ struct netvsc_device *nvdev);
+int netvsc_vf_setxdp(struct net_device *vf_netdev, struct bpf_prog *prog);
+int netvsc_bpf(struct net_device *dev, struct netdev_bpf *bpf);
+
int rndis_set_subchannel(struct net_device *ndev,
struct netvsc_device *nvdev,
struct netvsc_device_info *dev_info);
@@ -832,6 +845,8 @@ struct nvsp_message {
#define RNDIS_MAX_PKT_DEFAULT 8
#define RNDIS_PKT_ALIGN_DEFAULT 8
+#define NETVSC_XDP_HDRM 256
+
struct multi_send_data {
struct sk_buff *skb; /* skb containing the pkt */
struct hv_netvsc_packet *pkt; /* netvsc pkt pending */
@@ -867,6 +882,7 @@ struct netvsc_stats {
u64 bytes;
u64 broadcast;
u64 multicast;
+ u64 xdp_drop;
struct u64_stats_sync syncp;
};
@@ -972,6 +988,9 @@ struct netvsc_channel {
atomic_t queue_sends;
struct nvsc_rsc rsc;
+ struct bpf_prog __rcu *bpf_prog;
+ struct xdp_rxq_info xdp_rxq;
+
struct netvsc_stats tx_stats;
struct netvsc_stats rx_stats;
};
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index eab83e7..ae3f308 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -122,8 +122,10 @@ static void free_netvsc_device(struct rcu_head *head)
vfree(nvdev->send_buf);
kfree(nvdev->send_section_map);
- for (i = 0; i < VRSS_CHANNEL_MAX; i++)
+ for (i = 0; i < VRSS_CHANNEL_MAX; i++) {
+ xdp_rxq_info_unreg(&nvdev->chan_table[i].xdp_rxq);
vfree(nvdev->chan_table[i].mrc.slots);
+ }
kfree(nvdev);
}
@@ -900,7 +902,8 @@ int netvsc_send(struct net_device *ndev,
struct hv_netvsc_packet *packet,
struct rndis_message *rndis_msg,
struct hv_page_buffer *pb,
- struct sk_buff *skb)
+ struct sk_buff *skb,
+ bool xdp_tx)
{
struct net_device_context *ndev_ctx = netdev_priv(ndev);
struct netvsc_device *net_device
@@ -923,10 +926,11 @@ int netvsc_send(struct net_device *ndev,
packet->send_buf_index = NETVSC_INVALID_INDEX;
packet->cp_partial = false;
- /* Send control message directly without accessing msd (Multi-Send
- * Data) field which may be changed during data packet processing.
+ /* Send a control message or XDP packet directly without accessing
+ * msd (Multi-Send Data) field which may be changed during data packet
+ * processing.
*/
- if (!skb)
+ if (!skb || xdp_tx)
return netvsc_send_pkt(device, packet, net_device, pb, skb);
/* batch packets in send buffer if possible */
@@ -1392,6 +1396,21 @@ struct netvsc_device *netvsc_device_add(struct hv_device *device,
nvchan->net_device = net_device;
u64_stats_init(&nvchan->tx_stats.syncp);
u64_stats_init(&nvchan->rx_stats.syncp);
+
+ ret = xdp_rxq_info_reg(&nvchan->xdp_rxq, ndev, i);
+
+ if (ret) {
+ netdev_err(ndev, "xdp_rxq_info_reg fail: %d\n", ret);
+ goto cleanup2;
+ }
+
+ ret = xdp_rxq_info_reg_mem_model(&nvchan->xdp_rxq,
+ MEM_TYPE_PAGE_SHARED, NULL);
+
+ if (ret) {
+ netdev_err(ndev, "xdp reg_mem_model fail: %d\n", ret);
+ goto cleanup2;
+ }
}
/* Enable NAPI handler before init callbacks */
@@ -1437,6 +1456,8 @@ struct netvsc_device *netvsc_device_add(struct hv_device *device,
cleanup:
netif_napi_del(&net_device->chan_table[0].napi);
+
+cleanup2:
free_netvsc_device(&net_device->rcu);
return ERR_PTR(ret);
diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c
new file mode 100644
index 0000000..68ed0ea
--- /dev/null
+++ b/drivers/net/hyperv/netvsc_bpf.c
@@ -0,0 +1,209 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2019, Microsoft Corporation.
+ *
+ * Author:
+ * Haiyang Zhang <haiyangz@microsoft.com>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+#include <linux/ethtool.h>
+#include <linux/bpf.h>
+#include <linux/bpf_trace.h>
+#include <linux/kernel.h>
+#include <net/xdp.h>
+
+#include <linux/mutex.h>
+#include <linux/rtnetlink.h>
+
+#include "hyperv_net.h"
+
+u32 netvsc_run_xdp(struct net_device *ndev, struct netvsc_channel *nvchan,
+ struct xdp_buff *xdp)
+{
+ struct page *page = NULL;
+ void *data = nvchan->rsc.data[0];
+ u32 len = nvchan->rsc.len[0];
+ struct bpf_prog *prog;
+ u32 act = XDP_PASS;
+
+ xdp->data_hard_start = NULL;
+
+ rcu_read_lock();
+ prog = rcu_dereference(nvchan->bpf_prog);
+
+ if (!prog)
+ goto out;
+
+ /* allocate page buffer for data */
+ page = alloc_page(GFP_ATOMIC);
+ if (!page) {
+ act = XDP_DROP;
+ goto out;
+ }
+
+ xdp->data_hard_start = page_address(page);
+ xdp->data = xdp->data_hard_start + NETVSC_XDP_HDRM;
+ xdp_set_data_meta_invalid(xdp);
+ xdp->data_end = xdp->data + len;
+ xdp->rxq = &nvchan->xdp_rxq;
+ xdp->handle = 0;
+
+ memcpy(xdp->data, data, len);
+
+ act = bpf_prog_run_xdp(prog, xdp);
+
+ switch (act) {
+ case XDP_PASS:
+ case XDP_TX:
+ case XDP_DROP:
+ break;
+
+ case XDP_ABORTED:
+ trace_xdp_exception(ndev, prog, act);
+ break;
+
+ default:
+ bpf_warn_invalid_xdp_action(act);
+ }
+
+out:
+ rcu_read_unlock();
+
+ if (page && act != XDP_PASS && act != XDP_TX) {
+ __free_page(page);
+ xdp->data_hard_start = NULL;
+ }
+
+ return act;
+}
+
+unsigned int netvsc_xdp_fraglen(unsigned int len)
+{
+ return SKB_DATA_ALIGN(len) +
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+}
+
+struct bpf_prog *netvsc_xdp_get(struct netvsc_device *nvdev)
+{
+ return rtnl_dereference(nvdev->chan_table[0].bpf_prog);
+}
+
+int netvsc_xdp_set(struct net_device *dev, struct bpf_prog *prog,
+ struct netlink_ext_ack *extack,
+ struct netvsc_device *nvdev)
+{
+ struct bpf_prog *old_prog;
+ int buf_max, i;
+
+ old_prog = netvsc_xdp_get(nvdev);
+
+ if (!old_prog && !prog)
+ return 0;
+
+ buf_max = NETVSC_XDP_HDRM + netvsc_xdp_fraglen(dev->mtu + ETH_HLEN);
+ if (prog && buf_max > PAGE_SIZE) {
+ netdev_err(dev, "XDP: mtu:%u too large, buf_max:%u\n",
+ dev->mtu, buf_max);
+ NL_SET_ERR_MSG_MOD(extack, "XDP: mtu too large");
+
+ return -EOPNOTSUPP;
+ }
+
+ if (prog && (dev->features & NETIF_F_LRO)) {
+ netdev_err(dev, "XDP: not support LRO\n");
+ NL_SET_ERR_MSG_MOD(extack, "XDP: not support LRO");
+
+ return -EOPNOTSUPP;
+ }
+
+ if (prog)
+ bpf_prog_add(prog, nvdev->num_chn);
+
+ for (i = 0; i < nvdev->num_chn; i++)
+ rcu_assign_pointer(nvdev->chan_table[i].bpf_prog, prog);
+
+ if (old_prog)
+ for (i = 0; i < nvdev->num_chn; i++)
+ bpf_prog_put(old_prog);
+
+ return 0;
+}
+
+int netvsc_vf_setxdp(struct net_device *vf_netdev, struct bpf_prog *prog)
+{
+ struct netdev_bpf xdp;
+ bpf_op_t ndo_bpf;
+
+ ASSERT_RTNL();
+
+ if (!vf_netdev)
+ return 0;
+
+ ndo_bpf = vf_netdev->netdev_ops->ndo_bpf;
+ if (!ndo_bpf)
+ return 0;
+
+ memset(&xdp, 0, sizeof(xdp));
+
+ xdp.command = XDP_SETUP_PROG;
+ xdp.prog = prog;
+
+ return ndo_bpf(vf_netdev, &xdp);
+}
+
+static u32 netvsc_xdp_query(struct netvsc_device *nvdev)
+{
+ struct bpf_prog *prog = netvsc_xdp_get(nvdev);
+
+ if (prog)
+ return prog->aux->id;
+
+ return 0;
+}
+
+int netvsc_bpf(struct net_device *dev, struct netdev_bpf *bpf)
+{
+ struct net_device_context *ndevctx = netdev_priv(dev);
+ struct netvsc_device *nvdev = rtnl_dereference(ndevctx->nvdev);
+ struct net_device *vf_netdev = rtnl_dereference(ndevctx->vf_netdev);
+ struct netlink_ext_ack *extack = bpf->extack;
+ int ret;
+
+ if (!nvdev || nvdev->destroy) {
+ if (bpf->command == XDP_QUERY_PROG) {
+ bpf->prog_id = 0;
+ return 0; /* Query must always succeed */
+ } else {
+ return -ENODEV;
+ }
+ }
+
+ switch (bpf->command) {
+ case XDP_SETUP_PROG:
+ ret = netvsc_xdp_set(dev, bpf->prog, extack, nvdev);
+
+ if (ret)
+ return ret;
+
+ ret = netvsc_vf_setxdp(vf_netdev, bpf->prog);
+
+ if (ret) {
+ netdev_err(dev, "vf_setxdp failed:%d\n", ret);
+ NL_SET_ERR_MSG_MOD(extack, "vf_setxdp failed");
+
+ netvsc_xdp_set(dev, NULL, extack, nvdev);
+ }
+
+ return ret;
+
+ case XDP_QUERY_PROG:
+ bpf->prog_id = netvsc_xdp_query(nvdev);
+ return 0;
+
+ default:
+ return -EINVAL;
+ }
+}
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index f3f9eb8..3c9a969 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -25,6 +25,7 @@
#include <linux/slab.h>
#include <linux/rtnetlink.h>
#include <linux/netpoll.h>
+#include <linux/bpf.h>
#include <net/arp.h>
#include <net/route.h>
@@ -519,7 +520,7 @@ static int netvsc_vf_xmit(struct net_device *net, struct net_device *vf_netdev,
return rc;
}
-static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
+static int netvsc_xmit(struct sk_buff *skb, struct net_device *net, bool xdp_tx)
{
struct net_device_context *net_device_ctx = netdev_priv(net);
struct hv_netvsc_packet *packet = NULL;
@@ -686,7 +687,7 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
/* timestamp packet in software */
skb_tx_timestamp(skb);
- ret = netvsc_send(net, packet, rndis_msg, pb, skb);
+ ret = netvsc_send(net, packet, rndis_msg, pb, skb, xdp_tx);
if (likely(ret == 0))
return NETDEV_TX_OK;
@@ -709,6 +710,11 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
goto drop;
}
+static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *ndev)
+{
+ return netvsc_xmit(skb, ndev, false);
+}
+
/*
* netvsc_linkstatus_callback - Link up/down notification
*/
@@ -751,6 +757,14 @@ void netvsc_linkstatus_callback(struct net_device *net,
schedule_delayed_work(&ndev_ctx->dwork, 0);
}
+static int netvsc_xdp_xmit(struct sk_buff *skb, struct net_device *ndev)
+{
+ skb->queue_mapping = skb_get_rx_queue(skb);
+ __skb_push(skb, ETH_HLEN);
+
+ return netvsc_xmit(skb, ndev, true);
+}
+
static void netvsc_comp_ipcsum(struct sk_buff *skb)
{
struct iphdr *iph = (struct iphdr *)skb->data;
@@ -760,7 +774,8 @@ static void netvsc_comp_ipcsum(struct sk_buff *skb)
}
static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
- struct netvsc_channel *nvchan)
+ struct netvsc_channel *nvchan,
+ struct xdp_buff *xdp)
{
struct napi_struct *napi = &nvchan->napi;
const struct ndis_pkt_8021q_info *vlan = nvchan->rsc.vlan;
@@ -768,18 +783,37 @@ static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
nvchan->rsc.csum_info;
const u32 *hash_info = nvchan->rsc.hash_info;
struct sk_buff *skb;
+ void *xbuf = xdp->data_hard_start;
int i;
- skb = napi_alloc_skb(napi, nvchan->rsc.pktlen);
- if (!skb)
- return skb;
+ if (xbuf) {
+ unsigned int hdroom = xdp->data - xdp->data_hard_start;
+ unsigned int xlen = xdp->data_end - xdp->data;
+ unsigned int frag_size = netvsc_xdp_fraglen(hdroom + xlen);
- /*
- * Copy to skb. This copy is needed here since the memory pointed by
- * hv_netvsc_packet cannot be deallocated
- */
- for (i = 0; i < nvchan->rsc.cnt; i++)
- skb_put_data(skb, nvchan->rsc.data[i], nvchan->rsc.len[i]);
+ skb = build_skb(xbuf, frag_size);
+
+ if (!skb) {
+ __free_page(virt_to_page(xbuf));
+ return NULL;
+ }
+
+ skb_reserve(skb, hdroom);
+ skb_put(skb, xlen);
+ skb->dev = napi->dev;
+ } else {
+ skb = napi_alloc_skb(napi, nvchan->rsc.pktlen);
+
+ if (!skb)
+ return NULL;
+
+ /* Copy to skb. This copy is needed here since the memory
+ * pointed by hv_netvsc_packet cannot be deallocated.
+ */
+ for (i = 0; i < nvchan->rsc.cnt; i++)
+ skb_put_data(skb, nvchan->rsc.data[i],
+ nvchan->rsc.len[i]);
+ }
skb->protocol = eth_type_trans(skb, net);
@@ -829,13 +863,25 @@ int netvsc_recv_callback(struct net_device *net,
struct vmbus_channel *channel = nvchan->channel;
u16 q_idx = channel->offermsg.offer.sub_channel_index;
struct sk_buff *skb;
- struct netvsc_stats *rx_stats;
+ struct netvsc_stats *rx_stats = &nvchan->rx_stats;
+ struct xdp_buff xdp;
+ u32 act;
if (net->reg_state != NETREG_REGISTERED)
return NVSP_STAT_FAIL;
+ act = netvsc_run_xdp(net, nvchan, &xdp);
+
+ if (act != XDP_PASS && act != XDP_TX) {
+ u64_stats_update_begin(&rx_stats->syncp);
+ rx_stats->xdp_drop++;
+ u64_stats_update_end(&rx_stats->syncp);
+
+ return NVSP_STAT_SUCCESS; /* consumed by XDP */
+ }
+
/* Allocate a skb - TODO direct I/O to pages? */
- skb = netvsc_alloc_recv_skb(net, nvchan);
+ skb = netvsc_alloc_recv_skb(net, nvchan, &xdp);
if (unlikely(!skb)) {
++net_device_ctx->eth_stats.rx_no_memory;
@@ -844,12 +890,16 @@ int netvsc_recv_callback(struct net_device *net,
skb_record_rx_queue(skb, q_idx);
+ if (act == XDP_TX) {
+ netvsc_xdp_xmit(skb, net);
+ return NVSP_STAT_SUCCESS;
+ }
+
/*
* Even if injecting the packet, record the statistics
* on the synthetic device because modifying the VF device
* statistics will not work correctly.
*/
- rx_stats = &nvchan->rx_stats;
u64_stats_update_begin(&rx_stats->syncp);
rx_stats->packets++;
rx_stats->bytes += nvchan->rsc.pktlen;
@@ -886,10 +936,11 @@ static void netvsc_get_channels(struct net_device *net,
/* Alloc struct netvsc_device_info, and initialize it from either existing
* struct netvsc_device, or from default values.
*/
-static struct netvsc_device_info *netvsc_devinfo_get
- (struct netvsc_device *nvdev)
+static
+struct netvsc_device_info *netvsc_devinfo_get(struct netvsc_device *nvdev)
{
struct netvsc_device_info *dev_info;
+ struct bpf_prog *prog;
dev_info = kzalloc(sizeof(*dev_info), GFP_ATOMIC);
@@ -897,6 +948,8 @@ static void netvsc_get_channels(struct net_device *net,
return NULL;
if (nvdev) {
+ ASSERT_RTNL();
+
dev_info->num_chn = nvdev->num_chn;
dev_info->send_sections = nvdev->send_section_cnt;
dev_info->send_section_size = nvdev->send_section_size;
@@ -905,6 +958,12 @@ static void netvsc_get_channels(struct net_device *net,
memcpy(dev_info->rss_key, nvdev->extension->rss_key,
NETVSC_HASH_KEYLEN);
+
+ prog = netvsc_xdp_get(nvdev);
+ if (prog) {
+ bpf_prog_inc(prog);
+ dev_info->bprog = prog;
+ }
} else {
dev_info->num_chn = VRSS_CHANNEL_DEFAULT;
dev_info->send_sections = NETVSC_DEFAULT_TX;
@@ -916,6 +975,17 @@ static void netvsc_get_channels(struct net_device *net,
return dev_info;
}
+/* Free struct netvsc_device_info */
+static void netvsc_devinfo_put(struct netvsc_device_info *dev_info)
+{
+ if (dev_info->bprog) {
+ ASSERT_RTNL();
+ bpf_prog_put(dev_info->bprog);
+ }
+
+ kfree(dev_info);
+}
+
static int netvsc_detach(struct net_device *ndev,
struct netvsc_device *nvdev)
{
@@ -927,6 +997,8 @@ static int netvsc_detach(struct net_device *ndev,
if (cancel_work_sync(&nvdev->subchan_work))
nvdev->num_chn = 1;
+ netvsc_xdp_set(ndev, NULL, NULL, nvdev);
+
/* If device was up (receiving) then shutdown */
if (netif_running(ndev)) {
netvsc_tx_disable(nvdev, ndev);
@@ -960,7 +1032,8 @@ static int netvsc_attach(struct net_device *ndev,
struct hv_device *hdev = ndev_ctx->device_ctx;
struct netvsc_device *nvdev;
struct rndis_device *rdev;
- int ret;
+ struct bpf_prog *prog;
+ int ret = 0;
nvdev = rndis_filter_device_add(hdev, dev_info);
if (IS_ERR(nvdev))
@@ -976,6 +1049,13 @@ static int netvsc_attach(struct net_device *ndev,
}
}
+ prog = dev_info->bprog;
+ if (prog) {
+ ret = netvsc_xdp_set(ndev, prog, NULL, nvdev);
+ if (ret)
+ goto err1;
+ }
+
/* In any case device is now ready */
netif_device_attach(ndev);
@@ -985,7 +1065,7 @@ static int netvsc_attach(struct net_device *ndev,
if (netif_running(ndev)) {
ret = rndis_filter_open(nvdev);
if (ret)
- goto err;
+ goto err2;
rdev = nvdev->extension;
if (!rdev->link_state)
@@ -994,9 +1074,10 @@ static int netvsc_attach(struct net_device *ndev,
return 0;
-err:
+err2:
netif_device_detach(ndev);
+err1:
rndis_filter_device_remove(hdev, nvdev);
return ret;
@@ -1046,7 +1127,7 @@ static int netvsc_set_channels(struct net_device *net,
}
out:
- kfree(device_info);
+ netvsc_devinfo_put(device_info);
return ret;
}
@@ -1153,7 +1234,7 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
dev_set_mtu(vf_netdev, orig_mtu);
out:
- kfree(device_info);
+ netvsc_devinfo_put(device_info);
return ret;
}
@@ -1378,8 +1459,8 @@ static int netvsc_set_mac_addr(struct net_device *ndev, void *p)
/* statistics per queue (rx/tx packets/bytes) */
#define NETVSC_PCPU_STATS_LEN (num_present_cpus() * ARRAY_SIZE(pcpu_stats))
-/* 4 statistics per queue (rx/tx packets/bytes) */
-#define NETVSC_QUEUE_STATS_LEN(dev) ((dev)->num_chn * 4)
+/* 5 statistics per queue (rx/tx packets/bytes, rx xdp_drop) */
+#define NETVSC_QUEUE_STATS_LEN(dev) ((dev)->num_chn * 5)
static int netvsc_get_sset_count(struct net_device *dev, int string_set)
{
@@ -1411,6 +1492,7 @@ static void netvsc_get_ethtool_stats(struct net_device *dev,
struct netvsc_ethtool_pcpu_stats *pcpu_sum;
unsigned int start;
u64 packets, bytes;
+ u64 xdp_drop;
int i, j, cpu;
if (!nvdev)
@@ -1439,9 +1521,11 @@ static void netvsc_get_ethtool_stats(struct net_device *dev,
start = u64_stats_fetch_begin_irq(&qstats->syncp);
packets = qstats->packets;
bytes = qstats->bytes;
+ xdp_drop = qstats->xdp_drop;
} while (u64_stats_fetch_retry_irq(&qstats->syncp, start));
data[i++] = packets;
data[i++] = bytes;
+ data[i++] = xdp_drop;
}
pcpu_sum = kvmalloc_array(num_possible_cpus(),
@@ -1489,6 +1573,8 @@ static void netvsc_get_strings(struct net_device *dev, u32 stringset, u8 *data)
p += ETH_GSTRING_LEN;
sprintf(p, "rx_queue_%u_bytes", i);
p += ETH_GSTRING_LEN;
+ sprintf(p, "rx_queue_%u_xdp_drop", i);
+ p += ETH_GSTRING_LEN;
}
for_each_present_cpu(cpu) {
@@ -1785,10 +1871,27 @@ static int netvsc_set_ringparam(struct net_device *ndev,
}
out:
- kfree(device_info);
+ netvsc_devinfo_put(device_info);
return ret;
}
+static netdev_features_t netvsc_fix_features(struct net_device *ndev,
+ netdev_features_t features)
+{
+ struct net_device_context *ndevctx = netdev_priv(ndev);
+ struct netvsc_device *nvdev = rtnl_dereference(ndevctx->nvdev);
+
+ if (!nvdev || nvdev->destroy)
+ return features;
+
+ if ((features & NETIF_F_LRO) && netvsc_xdp_get(nvdev)) {
+ features ^= NETIF_F_LRO;
+ netdev_info(ndev, "Skip LRO - unsupported with XDP\n");
+ }
+
+ return features;
+}
+
static int netvsc_set_features(struct net_device *ndev,
netdev_features_t features)
{
@@ -1875,12 +1978,14 @@ static void netvsc_set_msglevel(struct net_device *ndev, u32 val)
.ndo_start_xmit = netvsc_start_xmit,
.ndo_change_rx_flags = netvsc_change_rx_flags,
.ndo_set_rx_mode = netvsc_set_rx_mode,
+ .ndo_fix_features = netvsc_fix_features,
.ndo_set_features = netvsc_set_features,
.ndo_change_mtu = netvsc_change_mtu,
.ndo_validate_addr = eth_validate_addr,
.ndo_set_mac_address = netvsc_set_mac_addr,
.ndo_select_queue = netvsc_select_queue,
.ndo_get_stats64 = netvsc_get_stats64,
+ .ndo_bpf = netvsc_bpf,
};
/*
@@ -2167,6 +2272,7 @@ static int netvsc_register_vf(struct net_device *vf_netdev)
{
struct net_device_context *net_device_ctx;
struct netvsc_device *netvsc_dev;
+ struct bpf_prog *prog;
struct net_device *ndev;
int ret;
@@ -2211,6 +2317,9 @@ static int netvsc_register_vf(struct net_device *vf_netdev)
vf_netdev->wanted_features = ndev->features;
netdev_update_features(vf_netdev);
+ prog = netvsc_xdp_get(netvsc_dev);
+ netvsc_vf_setxdp(vf_netdev, prog);
+
return NOTIFY_OK;
}
@@ -2252,6 +2361,8 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev)
netdev_info(ndev, "VF unregistering: %s\n", vf_netdev->name);
+ netvsc_vf_setxdp(vf_netdev, NULL);
+
netdev_rx_handler_unregister(vf_netdev);
netdev_upper_dev_unlink(vf_netdev, ndev);
RCU_INIT_POINTER(net_device_ctx->vf_netdev, NULL);
@@ -2363,14 +2474,14 @@ static int netvsc_probe(struct hv_device *dev,
list_add(&net_device_ctx->list, &netvsc_dev_list);
rtnl_unlock();
- kfree(device_info);
+ netvsc_devinfo_put(device_info);
return 0;
register_failed:
rtnl_unlock();
rndis_filter_device_remove(dev, nvdev);
rndis_failed:
- kfree(device_info);
+ netvsc_devinfo_put(device_info);
devinfo_failed:
free_percpu(net_device_ctx->vf_stats);
no_stats:
@@ -2398,8 +2509,10 @@ static int netvsc_remove(struct hv_device *dev)
rtnl_lock();
nvdev = rtnl_dereference(ndev_ctx->nvdev);
- if (nvdev)
+ if (nvdev) {
cancel_work_sync(&nvdev->subchan_work);
+ netvsc_xdp_set(net, NULL, NULL, nvdev);
+ }
/*
* Call to the vsc driver to let it know that the device is being
@@ -2472,11 +2585,11 @@ static int netvsc_resume(struct hv_device *dev)
ret = netvsc_attach(net, device_info);
- rtnl_unlock();
-
- kfree(device_info);
+ netvsc_devinfo_put(device_info);
net_device_ctx->saved_netvsc_dev_info = NULL;
+ rtnl_unlock();
+
return ret;
}
static const struct hv_vmbus_device_id id_table[] = {
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index e66d77d..b81ceba 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -235,7 +235,7 @@ static int rndis_filter_send_request(struct rndis_device *dev,
trace_rndis_send(dev->ndev, 0, &req->request_msg);
rcu_read_lock_bh();
- ret = netvsc_send(dev->ndev, packet, NULL, pb, NULL);
+ ret = netvsc_send(dev->ndev, packet, NULL, pb, NULL, false);
rcu_read_unlock_bh();
return ret;
--
1.8.3.1
^ permalink raw reply related
* [PATCH V2,net-next, 2/2] hv_netvsc: Update document for XDP support
From: Haiyang Zhang @ 2020-01-20 22:22 UTC (permalink / raw)
To: sashal, linux-hyperv, netdev
Cc: haiyangz, kys, sthemmin, olaf, vkuznets, davem, linux-kernel
In-Reply-To: <1579558957-62496-1-git-send-email-haiyangz@microsoft.com>
Added the new section in the document regarding XDP support
by hv_netvsc driver.
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
.../networking/device_drivers/microsoft/netvsc.txt | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/Documentation/networking/device_drivers/microsoft/netvsc.txt b/Documentation/networking/device_drivers/microsoft/netvsc.txt
index 3bfa635..cd63556 100644
--- a/Documentation/networking/device_drivers/microsoft/netvsc.txt
+++ b/Documentation/networking/device_drivers/microsoft/netvsc.txt
@@ -82,3 +82,24 @@ Features
contain one or more packets. The send buffer is an optimization, the driver
will use slower method to handle very large packets or if the send buffer
area is exhausted.
+
+ XDP support
+ -----------
+ XDP (eXpress Data Path) is a feature that runs eBPF bytecode at the early
+ stage when packets arrive at a NIC card. The goal is to increase performance
+ for packet processing, reducing the overhead of SKB allocation and other
+ upper network layers.
+
+ hv_netvsc supports XDP in native mode, and transparently sets the XDP
+ program on the associated VF NIC as well.
+
+ Setting / unsetting XDP program on synthetic NIC (netvsc) propagates to
+ VF NIC automatically. Setting / unsetting XDP program on VF NIC directly
+ is not recommended, also not propagated to synthetic NIC, and may be
+ overwritten by setting of synthetic NIC.
+
+ XDP program cannot run with LRO (RSC) enabled, so you need to disable LRO
+ before running XDP:
+ ethtool -K eth0 lro off
+
+ XDP_REDIRECT action is not yet supported.
--
1.8.3.1
^ permalink raw reply related
* [PATCH 1/2] pci: hyperv: x86: Move hypercall related definitions into tlfs header
From: Boqun Feng @ 2020-01-21 1:57 UTC (permalink / raw)
To: linux-hyperv, linux-kernel
Cc: Michael Kelley, Boqun Feng, K. Y. Srinivasan, Haiyang Zhang,
Stephen Hemminger, Sasha Levin, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS
For future support of virtual PCI on non-x86 platform.
Signed-off-by: Boqun Feng (Microsoft) <boqun.feng@gmail.com>
---
arch/x86/include/asm/hyperv-tlfs.h | 4 ++++
drivers/pci/controller/pci-hyperv.c | 6 ------
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 5f10f7f2098d..b9ebc20b2385 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -376,6 +376,7 @@ struct hv_tsc_emulation_status {
#define HVCALL_SEND_IPI_EX 0x0015
#define HVCALL_POST_MESSAGE 0x005c
#define HVCALL_SIGNAL_EVENT 0x005d
+#define HVCALL_RETARGET_INTERRUPT 0x007e
#define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
#define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
@@ -405,6 +406,9 @@ enum HV_GENERIC_SET_FORMAT {
HV_GENERIC_SET_ALL,
};
+/* Declare standard hypercall field values. */
+#define HV_PARTITION_ID_SELF ((u64)-1)
+
#define HV_HYPERCALL_RESULT_MASK GENMASK_ULL(15, 0)
#define HV_HYPERCALL_FAST_BIT BIT(16)
#define HV_HYPERCALL_VARHEAD_OFFSET 17
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 9977abff92fc..aacfcc90d929 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -406,12 +406,6 @@ struct pci_eject_response {
static int pci_ring_size = (4 * PAGE_SIZE);
-/*
- * Definitions or interrupt steering hypercall.
- */
-#define HV_PARTITION_ID_SELF ((u64)-1)
-#define HVCALL_RETARGET_INTERRUPT 0x7e
-
struct hv_interrupt_entry {
u32 source; /* 1 for MSI(-X) */
u32 reserved1;
--
2.24.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox