* [dpdk-dev] [PATCH] bus/vmbus: Fix crash when handling packets in secondary process
@ 2021-07-20 14:58 jerb
2021-07-20 15:28 ` Stephen Hemminger
0 siblings, 1 reply; 4+ messages in thread
From: jerb @ 2021-07-20 14:58 UTC (permalink / raw)
To: sthemmin, longli; +Cc: dev, jerb
Have secondary processes construct their own copy of primary channel with
own mappings.
Remove vmbus_channel primary ptr from struct mapped_vmbus_resource as its
not used.
Populate virtual memory address "addr" in struct rte_mem_resource for
secondary processes as netvsc will attempt to reference it thus causing
a crash. It was initialized for primary processes but not for secondary.
Signed-off-by: jerb <jonathan.erb@banduracyber.com>
---
drivers/bus/vmbus/private.h | 1 -
drivers/bus/vmbus/vmbus_channel.c | 4 +---
drivers/bus/vmbus/vmbus_common_uio.c | 15 ++++++++++-----
3 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/bus/vmbus/private.h b/drivers/bus/vmbus/private.h
index 528d60a42f..746212bd5f 100644
--- a/drivers/bus/vmbus/private.h
+++ b/drivers/bus/vmbus/private.h
@@ -42,7 +42,6 @@ struct mapped_vmbus_resource {
rte_uuid_t id;
int nb_maps;
- struct vmbus_channel *primary;
struct vmbus_map maps[VMBUS_MAX_RESOURCE];
char path[PATH_MAX];
};
diff --git a/drivers/bus/vmbus/vmbus_channel.c b/drivers/bus/vmbus/vmbus_channel.c
index f67f1c438a..119b9b367e 100644
--- a/drivers/bus/vmbus/vmbus_channel.c
+++ b/drivers/bus/vmbus/vmbus_channel.c
@@ -351,10 +351,8 @@ int rte_vmbus_chan_open(struct rte_vmbus_device *device,
err = vmbus_chan_create(device, device->relid, 0,
device->monitor_id, new_chan);
- if (!err) {
+ if (!err)
device->primary = *new_chan;
- uio_res->primary = *new_chan;
- }
return err;
}
diff --git a/drivers/bus/vmbus/vmbus_common_uio.c b/drivers/bus/vmbus/vmbus_common_uio.c
index 8582e32c1d..7b9a8ef434 100644
--- a/drivers/bus/vmbus/vmbus_common_uio.c
+++ b/drivers/bus/vmbus/vmbus_common_uio.c
@@ -69,8 +69,10 @@ vmbus_uio_map_secondary(struct rte_vmbus_device *dev)
fd, offset,
uio_res->maps[i].size, 0);
- if (mapaddr == uio_res->maps[i].addr)
+ if (mapaddr == uio_res->maps[i].addr) {
+ dev->resource[i].addr = mapaddr;
continue; /* successful map */
+ }
if (mapaddr == MAP_FAILED)
VMBUS_LOG(ERR,
@@ -88,9 +90,9 @@ vmbus_uio_map_secondary(struct rte_vmbus_device *dev)
/* fd is not needed in secondary process, close it */
close(fd);
- dev->primary = uio_res->primary;
- if (!dev->primary) {
- VMBUS_LOG(ERR, "missing primary channel");
+ if (vmbus_chan_create(dev, dev->relid, 0,
+ dev->monitor_id, &dev->primary)) {
+ VMBUS_LOG(ERR, "cannot create primary channel");
return -1;
}
@@ -211,8 +213,11 @@ vmbus_uio_unmap_resource(struct rte_vmbus_device *dev)
return;
/* secondary processes - just free maps */
- if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+ if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+ if (dev->primary != NULL)
+ rte_free(dev->primary);
return vmbus_uio_unmap(uio_res);
+ }
TAILQ_REMOVE(uio_res_list, uio_res, next);
--
2.17.1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [dpdk-dev] [PATCH] bus/vmbus: Fix crash when handling packets in secondary process
2021-07-20 14:58 [dpdk-dev] [PATCH] bus/vmbus: Fix crash when handling packets in secondary process jerb
@ 2021-07-20 15:28 ` Stephen Hemminger
2021-07-21 0:17 ` Long Li
0 siblings, 1 reply; 4+ messages in thread
From: Stephen Hemminger @ 2021-07-20 15:28 UTC (permalink / raw)
To: jerb, Long Li; +Cc: dev
Looks good, minor comment. You don't have to check for NULL before calling rte_free().
Rte_free(NULL) is a NOP like free(NULL).
Sorry for top posting; but if you send to my Microsoft account you are stuck with what
Outlook can do...
-----Original Message-----
From: jerb <jonathan.erb@banduracyber.com>
Sent: Tuesday, July 20, 2021 7:59 AM
To: Stephen Hemminger <sthemmin@microsoft.com>; Long Li <longli@microsoft.com>
Cc: dev@dpdk.org; jerb <jonathan.erb@banduracyber.com>
Subject: [PATCH] bus/vmbus: Fix crash when handling packets in secondary process
Have secondary processes construct their own copy of primary channel with
own mappings.
Remove vmbus_channel primary ptr from struct mapped_vmbus_resource as its
not used.
Populate virtual memory address "addr" in struct rte_mem_resource for
secondary processes as netvsc will attempt to reference it thus causing
a crash. It was initialized for primary processes but not for secondary.
Signed-off-by: jerb <jonathan.erb@banduracyber.com>
---
drivers/bus/vmbus/private.h | 1 -
drivers/bus/vmbus/vmbus_channel.c | 4 +---
drivers/bus/vmbus/vmbus_common_uio.c | 15 ++++++++++-----
3 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/bus/vmbus/private.h b/drivers/bus/vmbus/private.h
index 528d60a42f..746212bd5f 100644
--- a/drivers/bus/vmbus/private.h
+++ b/drivers/bus/vmbus/private.h
@@ -42,7 +42,6 @@ struct mapped_vmbus_resource {
rte_uuid_t id;
int nb_maps;
- struct vmbus_channel *primary;
struct vmbus_map maps[VMBUS_MAX_RESOURCE];
char path[PATH_MAX];
};
diff --git a/drivers/bus/vmbus/vmbus_channel.c b/drivers/bus/vmbus/vmbus_channel.c
index f67f1c438a..119b9b367e 100644
--- a/drivers/bus/vmbus/vmbus_channel.c
+++ b/drivers/bus/vmbus/vmbus_channel.c
@@ -351,10 +351,8 @@ int rte_vmbus_chan_open(struct rte_vmbus_device *device,
err = vmbus_chan_create(device, device->relid, 0,
device->monitor_id, new_chan);
- if (!err) {
+ if (!err)
device->primary = *new_chan;
- uio_res->primary = *new_chan;
- }
return err;
}
diff --git a/drivers/bus/vmbus/vmbus_common_uio.c b/drivers/bus/vmbus/vmbus_common_uio.c
index 8582e32c1d..7b9a8ef434 100644
--- a/drivers/bus/vmbus/vmbus_common_uio.c
+++ b/drivers/bus/vmbus/vmbus_common_uio.c
@@ -69,8 +69,10 @@ vmbus_uio_map_secondary(struct rte_vmbus_device *dev)
fd, offset,
uio_res->maps[i].size, 0);
- if (mapaddr == uio_res->maps[i].addr)
+ if (mapaddr == uio_res->maps[i].addr) {
+ dev->resource[i].addr = mapaddr;
continue; /* successful map */
+ }
if (mapaddr == MAP_FAILED)
VMBUS_LOG(ERR,
@@ -88,9 +90,9 @@ vmbus_uio_map_secondary(struct rte_vmbus_device *dev)
/* fd is not needed in secondary process, close it */
close(fd);
- dev->primary = uio_res->primary;
- if (!dev->primary) {
- VMBUS_LOG(ERR, "missing primary channel");
+ if (vmbus_chan_create(dev, dev->relid, 0,
+ dev->monitor_id, &dev->primary)) {
+ VMBUS_LOG(ERR, "cannot create primary channel");
return -1;
}
@@ -211,8 +213,11 @@ vmbus_uio_unmap_resource(struct rte_vmbus_device *dev)
return;
/* secondary processes - just free maps */
- if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+ if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+ if (dev->primary != NULL)
+ rte_free(dev->primary);
return vmbus_uio_unmap(uio_res);
+ }
TAILQ_REMOVE(uio_res_list, uio_res, next);
--
2.17.1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [dpdk-dev] [PATCH] bus/vmbus: Fix crash when handling packets in secondary process
2021-07-20 15:28 ` Stephen Hemminger
@ 2021-07-21 0:17 ` Long Li
2021-07-22 19:15 ` Thomas Monjalon
0 siblings, 1 reply; 4+ messages in thread
From: Long Li @ 2021-07-21 0:17 UTC (permalink / raw)
To: Stephen Hemminger, jerb; +Cc: dev
> -----Original Message-----
> From: Stephen Hemminger <sthemmin@microsoft.com>
> Sent: Tuesday, July 20, 2021 8:29 AM
> To: jerb <jonathan.erb@banduracyber.com>; Long Li <longli@microsoft.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] bus/vmbus: Fix crash when handling packets in secondary
> process
>
> Looks good, minor comment. You don't have to check for NULL before calling
> rte_free().
> Rte_free(NULL) is a NOP like free(NULL).
>
> Sorry for top posting; but if you send to my Microsoft account you are stuck
> with what Outlook can do...
>
> -----Original Message-----
> From: jerb <jonathan.erb@banduracyber.com>
> Sent: Tuesday, July 20, 2021 7:59 AM
> To: Stephen Hemminger <sthemmin@microsoft.com>; Long Li
> <longli@microsoft.com>
> Cc: dev@dpdk.org; jerb <jonathan.erb@banduracyber.com>
> Subject: [PATCH] bus/vmbus: Fix crash when handling packets in secondary
> process
>
> Have secondary processes construct their own copy of primary channel with
> own mappings.
>
> Remove vmbus_channel primary ptr from struct mapped_vmbus_resource as
> its not used.
>
> Populate virtual memory address "addr" in struct rte_mem_resource for
> secondary processes as netvsc will attempt to reference it thus causing a crash.
> It was initialized for primary processes but not for secondary.
>
> Signed-off-by: jerb <jonathan.erb@banduracyber.com>
Looks good.
This should also go to stable.
> ---
> drivers/bus/vmbus/private.h | 1 -
> drivers/bus/vmbus/vmbus_channel.c | 4 +---
> drivers/bus/vmbus/vmbus_common_uio.c | 15 ++++++++++-----
> 3 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/bus/vmbus/private.h b/drivers/bus/vmbus/private.h index
> 528d60a42f..746212bd5f 100644
> --- a/drivers/bus/vmbus/private.h
> +++ b/drivers/bus/vmbus/private.h
> @@ -42,7 +42,6 @@ struct mapped_vmbus_resource {
>
> rte_uuid_t id;
> int nb_maps;
> - struct vmbus_channel *primary;
> struct vmbus_map maps[VMBUS_MAX_RESOURCE];
> char path[PATH_MAX];
> };
> diff --git a/drivers/bus/vmbus/vmbus_channel.c
> b/drivers/bus/vmbus/vmbus_channel.c
> index f67f1c438a..119b9b367e 100644
> --- a/drivers/bus/vmbus/vmbus_channel.c
> +++ b/drivers/bus/vmbus/vmbus_channel.c
> @@ -351,10 +351,8 @@ int rte_vmbus_chan_open(struct rte_vmbus_device
> *device,
>
> err = vmbus_chan_create(device, device->relid, 0,
> device->monitor_id, new_chan);
> - if (!err) {
> + if (!err)
> device->primary = *new_chan;
> - uio_res->primary = *new_chan;
> - }
>
> return err;
> }
> diff --git a/drivers/bus/vmbus/vmbus_common_uio.c
> b/drivers/bus/vmbus/vmbus_common_uio.c
> index 8582e32c1d..7b9a8ef434 100644
> --- a/drivers/bus/vmbus/vmbus_common_uio.c
> +++ b/drivers/bus/vmbus/vmbus_common_uio.c
> @@ -69,8 +69,10 @@ vmbus_uio_map_secondary(struct rte_vmbus_device
> *dev)
> fd, offset,
> uio_res->maps[i].size, 0);
>
> - if (mapaddr == uio_res->maps[i].addr)
> + if (mapaddr == uio_res->maps[i].addr) {
> + dev->resource[i].addr = mapaddr;
> continue; /* successful map */
> + }
>
> if (mapaddr == MAP_FAILED)
> VMBUS_LOG(ERR,
> @@ -88,9 +90,9 @@ vmbus_uio_map_secondary(struct rte_vmbus_device
> *dev)
> /* fd is not needed in secondary process, close it */
> close(fd);
>
> - dev->primary = uio_res->primary;
> - if (!dev->primary) {
> - VMBUS_LOG(ERR, "missing primary channel");
> + if (vmbus_chan_create(dev, dev->relid, 0,
> + dev->monitor_id, &dev->primary)) {
> + VMBUS_LOG(ERR, "cannot create primary channel");
> return -1;
> }
>
> @@ -211,8 +213,11 @@ vmbus_uio_unmap_resource(struct
> rte_vmbus_device *dev)
> return;
>
> /* secondary processes - just free maps */
> - if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> + if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> + if (dev->primary != NULL)
> + rte_free(dev->primary);
> return vmbus_uio_unmap(uio_res);
> + }
>
> TAILQ_REMOVE(uio_res_list, uio_res, next);
>
> --
> 2.17.1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [dpdk-dev] [PATCH] bus/vmbus: Fix crash when handling packets in secondary process
2021-07-21 0:17 ` Long Li
@ 2021-07-22 19:15 ` Thomas Monjalon
0 siblings, 0 replies; 4+ messages in thread
From: Thomas Monjalon @ 2021-07-22 19:15 UTC (permalink / raw)
To: jerb; +Cc: Stephen Hemminger, dev, Long Li
21/07/2021 02:17, Long Li:
> From: Stephen Hemminger <sthemmin@microsoft.com>
> >
> > Looks good, minor comment. You don't have to check for NULL before calling
> > rte_free().
> > Rte_free(NULL) is a NOP like free(NULL).
> >
> > Sorry for top posting; but if you send to my Microsoft account you are stuck
> > with what Outlook can do...
> >
> From: jerb <jonathan.erb@banduracyber.com>
> >
> > Have secondary processes construct their own copy of primary channel with
> > own mappings.
> >
> > Remove vmbus_channel primary ptr from struct mapped_vmbus_resource as
> > its not used.
> >
> > Populate virtual memory address "addr" in struct rte_mem_resource for
> > secondary processes as netvsc will attempt to reference it thus causing a crash.
> > It was initialized for primary processes but not for secondary.
> >
> > Signed-off-by: jerb <jonathan.erb@banduracyber.com>
Please give your complete name.
> Looks good.
>
> This should also go to stable.
Please send a v2 with Fixes and Cc: stable@dpdk.org lines
as documented in the contributor's guide.
If you don't know how to do, the maintainers can help.
Thank you
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-07-22 19:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-20 14:58 [dpdk-dev] [PATCH] bus/vmbus: Fix crash when handling packets in secondary process jerb
2021-07-20 15:28 ` Stephen Hemminger
2021-07-21 0:17 ` Long Li
2021-07-22 19:15 ` Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).