[dpdk-dev] bus/vdev: scan and probe vdev in secondary processes
Checks
Commit Message
Base on primary/secondary communication channel [1], we add vdev
action to scan virtual devices in secondary processes.
[1] http://dpdk.org/dev/patchwork/patch/31838/
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
drivers/bus/vdev/vdev.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 100 insertions(+), 4 deletions(-)
Comments
Hey Jianfeng,
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jianfeng Tan
> Sent: Thursday, November 30, 2017 1:59 PM
> To: dev@dpdk.org
> Cc: Jianfeng Tan
> Subject: [dpdk-dev] [PATCH] bus/vdev: scan and probe vdev in
> secondary processes
>
> Base on primary/secondary communication channel [1], we add vdev
> action to scan virtual devices in secondary processes.
>
> [1] http://dpdk.org/dev/patchwork/patch/31838/
>
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
> drivers/bus/vdev/vdev.c | 104
> ++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 100 insertions(+), 4 deletions(-)
>
We recently stumbled across a problem where running dpdk-pdump against
a primary which had an rte_eth_bond virtual device would lead to a crash.
We realized this was happening because the vdev bus wasn't being built
properly on the secondary, leading to it corrupting the ethdev structures
for the eth_bond device on the primary. This corruption occurred because pdump
used what it thought was the first free dpdk port id for its pcap_pmd. That
port id collided with the eth_bond pmd, causing the problem.
My feeling is that this patch will fix that problem, since probing the virtual
bus from the primary, rather than just from the eal cmdline arguments, will
allow the secondary to choose an used port id. Am I correct in my understanding?
Thanks,
Kyle
Hi Kyle,
On 12/1/2017 6:27 AM, Kyle Larose wrote:
> Hey Jianfeng,
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jianfeng Tan
>> Sent: Thursday, November 30, 2017 1:59 PM
>> To: dev@dpdk.org
>> Cc: Jianfeng Tan
>> Subject: [dpdk-dev] [PATCH] bus/vdev: scan and probe vdev in
>> secondary processes
>>
>> Base on primary/secondary communication channel [1], we add vdev
>> action to scan virtual devices in secondary processes.
>>
>> [1] http://dpdk.org/dev/patchwork/patch/31838/
>>
>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>> ---
>> drivers/bus/vdev/vdev.c | 104
>> ++++++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 100 insertions(+), 4 deletions(-)
>>
>
>
> We recently stumbled across a problem where running dpdk-pdump against
> a primary which had an rte_eth_bond virtual device would lead to a crash.
>
> We realized this was happening because the vdev bus wasn't being built
> properly on the secondary, leading to it corrupting the ethdev structures
> for the eth_bond device on the primary. This corruption occurred because pdump
> used what it thought was the first free dpdk port id for its pcap_pmd. That
> port id collided with the eth_bond pmd, causing the problem.
>
> My feeling is that this patch will fix that problem, since probing the virtual
> bus from the primary, rather than just from the eal cmdline arguments, will
> allow the secondary to choose an used port id. Am I correct in my understanding?
Yes, that's an issue we want to address in this patch set: those portid
(either physical port or virtual port) will be synchronized to
secondary, if the secondary wants to add port (private to the
secondary), it will use unused portid.
But we still not address the problem that, both the primary and the
secondary are allocating new devices which needs a centralized portid
management mechanism.
Thanks,
Jianfeng
>
> Thanks,
>
> Kyle
>
>
@@ -106,7 +106,7 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)
name = rte_vdev_device_name(dev);
- VDEV_LOG(DEBUG, "Search driver %s to probe device %s\n", name,
+ VDEV_LOG(DEBUG, "Search driver %s to probe device %s", name,
rte_vdev_device_name(dev));
if (vdev_parse(name, &driver))
@@ -193,7 +193,7 @@ rte_vdev_init(const char *name, const char *args)
ret = vdev_probe_all_drivers(dev);
if (ret) {
if (ret > 0)
- VDEV_LOG(ERR, "no driver found for %s\n", name);
+ VDEV_LOG(ERR, "no driver found for %s", name);
goto fail;
}
@@ -216,7 +216,7 @@ vdev_remove_driver(struct rte_vdev_device *dev)
const struct rte_vdev_driver *driver;
if (!dev->device.driver) {
- VDEV_LOG(DEBUG, "no driver attach to device %s\n", name);
+ VDEV_LOG(DEBUG, "no driver attach to device %s", name);
return 1;
}
@@ -255,12 +255,105 @@ rte_vdev_uninit(const char *name)
return 0;
}
+struct vdev_action_params {
+#define VDEV_SCAN_REQUEST 1
+#define VDEV_SCAN_RESPONSE 2
+ int type;
+ char name[32];
+};
+
+static int vdev_plug(struct rte_device *dev);
+
+static int
+vdev_action(const void *params, int len,
+ int fds[] __rte_unused,
+ int fds_num __rte_unused)
+{
+ struct rte_vdev_device *dev;
+ struct rte_devargs *devargs;
+ struct vdev_action_params ou_params;
+ const struct vdev_action_params *in_params = params;
+
+ switch (in_params->type) {
+ case VDEV_SCAN_REQUEST:
+ ou_params.type = VDEV_SCAN_RESPONSE;
+ TAILQ_FOREACH(dev, &vdev_device_list, next) {
+ VDEV_LOG(INFO, "push vdev, %s", dev->device.name);
+ strncpy(ou_params.name, dev->device.name, 32);
+ rte_eal_mp_sendmsg("vdev", &ou_params,
+ len, NULL, 0);
+ }
+ break;
+ case VDEV_SCAN_RESPONSE:
+ VDEV_LOG(INFO, "get vdev, %s", in_params->name);
+
+ if (strlen(in_params->name) == 0) {
+ VDEV_LOG(ERR, "invalid name was passed");
+ break;
+ }
+
+ dev = find_vdev(in_params->name);
+ if (dev) {
+ VDEV_LOG(ERR, "vdev already exists: %s", in_params->name);
+ break;
+ }
+
+ devargs = alloc_devargs(in_params->name, NULL);
+ if (!devargs) {
+ VDEV_LOG(ERR, "failed to allocate memory");
+ break;
+ }
+
+ dev = calloc(1, sizeof(*dev));
+ if (!dev) {
+ VDEV_LOG(ERR, "failed to allocate memory");
+ free(devargs);
+ break;
+ }
+
+ dev->device.devargs = devargs;
+ dev->device.numa_node = 0; /* to be corrected in probe() */
+ dev->device.name = devargs->name;
+
+ TAILQ_INSERT_TAIL(&devargs_list, devargs, next);
+ TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
+
+ if (vdev_plug(&dev->device) < 0) {
+ VDEV_LOG(ERR, "failed to plug device %s", in_params->name);
+ free(devargs);
+ free(dev);
+ } else
+ VDEV_LOG(INFO, "plug in device: %s", in_params->name);
+
+ break;
+ default:
+ VDEV_LOG(ERR, "vdev cannot recognize this message");
+ }
+
+ return 0;
+}
+
static int
vdev_scan(void)
{
struct rte_vdev_device *dev;
struct rte_devargs *devargs;
+ if (rte_eal_mp_action_register("vdev", vdev_action) < 0) {
+ VDEV_LOG(ERR, "vdev fails to add action");
+ return -1;
+ }
+
+ if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+ struct vdev_action_params params;
+
+ params.type = VDEV_SCAN_REQUEST;
+ rte_eal_mp_sendmsg("vdev", ¶ms,
+ sizeof(params), NULL, 0);
+
+ return 0;
+ }
+
/* for virtual devices we scan the devargs_list populated via cmdline */
TAILQ_FOREACH(devargs, &devargs_list, next) {
@@ -290,6 +383,9 @@ vdev_probe(void)
{
struct rte_vdev_device *dev;
+ if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+ return 0;
+
/* call the init function for each virtual device */
TAILQ_FOREACH(dev, &vdev_device_list, next) {
@@ -297,7 +393,7 @@ vdev_probe(void)
continue;
if (vdev_probe_all_drivers(dev)) {
- VDEV_LOG(ERR, "failed to initialize %s device\n",
+ VDEV_LOG(ERR, "failed to initialize %s device",
rte_vdev_device_name(dev));
return -1;
}