[v5,1/5] devargs: unify scratch buffer storage

Message ID 1618283653-16510-2-git-send-email-xuemingl@nvidia.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series eal: enable global device syntax by default |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Xueming Li April 13, 2021, 3:14 a.m. UTC
  In current design, legacy parser rte_devargs_parse() saved scratch
buffer to devargs.args while new parser rte_devargs_layers_parse() saved
to devargs.data. Code using devargs had to know the difference and
cleaned up memory accordingly - error prone.

This patch unifies scratch buffer to data field, introduces
rte_devargs_reset() function to wrap the memory clean up logic.

Signed-off-by: Xueming Li <xuemingl@nvidia.com>
Acked-by: Ray Kinsella <mdr@ashroe.eu>
Reviewed-by: Gaetan Rivet <grive@u256.net>
---
 app/test-pmd/config.c                        |  3 +-
 app/test-pmd/testpmd.c                       |  5 +--
 drivers/bus/vdev/vdev.c                      |  9 +++---
 drivers/net/failsafe/failsafe_args.c         |  3 +-
 drivers/net/failsafe/failsafe_eal.c          |  2 +-
 examples/multi_process/hotplug_mp/commands.c |  6 ++--
 lib/librte_eal/common/eal_common_dev.c       |  9 +++---
 lib/librte_eal/common/eal_common_devargs.c   | 34 +++++++++++---------
 lib/librte_eal/common/hotplug_mp.c           |  6 ++--
 lib/librte_eal/include/rte_devargs.h         | 18 ++++++++---
 lib/librte_eal/version.map                   |  1 +
 lib/librte_ethdev/rte_ethdev.c               |  8 ++---
 12 files changed, 58 insertions(+), 46 deletions(-)
  

Comments

David Marchand April 16, 2021, 7 a.m. UTC | #1
On Tue, Apr 13, 2021 at 5:15 AM Xueming Li <xuemingl@nvidia.com> wrote:
> diff --git a/lib/librte_eal/include/rte_devargs.h b/lib/librte_eal/include/rte_devargs.h
> index 296f19324f..134b44a887 100644
> --- a/lib/librte_eal/include/rte_devargs.h
> +++ b/lib/librte_eal/include/rte_devargs.h
> @@ -60,16 +60,16 @@ struct rte_devargs {
>         /** Name of the device. */
>         char name[RTE_DEV_NAME_MAX_LEN];
>         RTE_STD_C11
> -       union {
> -       /** Arguments string as given by user or "" for no argument. */
> -               char *args;
> +       union { /**< driver-related part of device string. */
> +               const char *args; /**< legacy name. */
>                 const char *drv_str;
>         };
>         struct rte_bus *bus; /**< bus handle. */
>         struct rte_class *cls; /**< class handle. */
>         const char *bus_str; /**< bus-related part of device string. */
>         const char *cls_str; /**< class-related part of device string. */
> -       const char *data; /**< Device string storage. */
> +       char *data;
> +       /**< Raw string including bus, class and driver arguments. */
>  };
>
>  /**

- Flagging this patch for info and its impact on UNH jobs.

This change is fine, but older libabigail versions could not deal with
such changes (anonymous union, changes of const fields).
This results in an ABI check failure in the UNH x86 job on Ubuntu
18.04 (and for some people not using recent libabigail).
I can see the ARM job passes fine, so I suppose it is using a more
recent libabigail (running Ubuntu 20.04 maybe?).

We either need to disable this x86 job or update its libabigail
package (maybe aligning with what we have for public CI which is
libabigail 1.8 manually compiled).


- For the longer term, what do you think of using/extending the .ci/
scripts for use by UNH jobs?
  
Aaron Conole April 16, 2021, 12:32 p.m. UTC | #2
David Marchand <david.marchand@redhat.com> writes:

> On Tue, Apr 13, 2021 at 5:15 AM Xueming Li <xuemingl@nvidia.com> wrote:
>> diff --git a/lib/librte_eal/include/rte_devargs.h b/lib/librte_eal/include/rte_devargs.h
>> index 296f19324f..134b44a887 100644
>> --- a/lib/librte_eal/include/rte_devargs.h
>> +++ b/lib/librte_eal/include/rte_devargs.h
>> @@ -60,16 +60,16 @@ struct rte_devargs {
>>         /** Name of the device. */
>>         char name[RTE_DEV_NAME_MAX_LEN];
>>         RTE_STD_C11
>> -       union {
>> -       /** Arguments string as given by user or "" for no argument. */
>> -               char *args;
>> +       union { /**< driver-related part of device string. */
>> +               const char *args; /**< legacy name. */
>>                 const char *drv_str;
>>         };
>>         struct rte_bus *bus; /**< bus handle. */
>>         struct rte_class *cls; /**< class handle. */
>>         const char *bus_str; /**< bus-related part of device string. */
>>         const char *cls_str; /**< class-related part of device string. */
>> -       const char *data; /**< Device string storage. */
>> +       char *data;
>> +       /**< Raw string including bus, class and driver arguments. */
>>  };
>>
>>  /**
>
> - Flagging this patch for info and its impact on UNH jobs.
>
> This change is fine, but older libabigail versions could not deal with
> such changes (anonymous union, changes of const fields).
> This results in an ABI check failure in the UNH x86 job on Ubuntu
> 18.04 (and for some people not using recent libabigail).
> I can see the ARM job passes fine, so I suppose it is using a more
> recent libabigail (running Ubuntu 20.04 maybe?).
>
> We either need to disable this x86 job or update its libabigail
> package (maybe aligning with what we have for public CI which is
> libabigail 1.8 manually compiled).
>
>
> - For the longer term, what do you think of using/extending the .ci/
> scripts for use by UNH jobs?

I think it would be great if we had some of the scripts shared as a
common resource.  That would also help us to look at fixes / changes
when needed.
  
Lincoln Lavoie April 16, 2021, 12:43 p.m. UTC | #3
All of the UNH ABI testing is moving info containers, so it can be run on
top of each OS, alongside the other compile and unit testing. This is
actually ready now, but hasn't been pushed live this week, because of the
backlog in the system because of the DTS failure.  The additional compile
jobs are already online now, it's just ABI that hasn't been pushed live.

This means the current ABI (what is reporting right now) is running on
18.04 for x86 and 20.04 for aarch64.  The aarch64 one will continue
forward, because we're not going to moving to emulated environments for
testing on that architecture.

This has two implications, first, the scripts for running ABI (and the
other tests) become part of the container definition, and at the last
meeting we talked about moving those definitions into the dpdk-ci repo,
which I think makes sense.  Second, there isn't an operating system to
"maintain" since it's what's inside the container images, which are
periodically rebuilt, but pretty much treated as ephemeral.  Assuming the
container bases / distros have the updated libabigail version packaged with
them.

Cheers,
Lincoln

On Fri, Apr 16, 2021 at 8:32 AM Aaron Conole <aconole@redhat.com> wrote:

> David Marchand <david.marchand@redhat.com> writes:
>
> > On Tue, Apr 13, 2021 at 5:15 AM Xueming Li <xuemingl@nvidia.com> wrote:
> >> diff --git a/lib/librte_eal/include/rte_devargs.h
> b/lib/librte_eal/include/rte_devargs.h
> >> index 296f19324f..134b44a887 100644
> >> --- a/lib/librte_eal/include/rte_devargs.h
> >> +++ b/lib/librte_eal/include/rte_devargs.h
> >> @@ -60,16 +60,16 @@ struct rte_devargs {
> >>         /** Name of the device. */
> >>         char name[RTE_DEV_NAME_MAX_LEN];
> >>         RTE_STD_C11
> >> -       union {
> >> -       /** Arguments string as given by user or "" for no argument. */
> >> -               char *args;
> >> +       union { /**< driver-related part of device string. */
> >> +               const char *args; /**< legacy name. */
> >>                 const char *drv_str;
> >>         };
> >>         struct rte_bus *bus; /**< bus handle. */
> >>         struct rte_class *cls; /**< class handle. */
> >>         const char *bus_str; /**< bus-related part of device string. */
> >>         const char *cls_str; /**< class-related part of device string.
> */
> >> -       const char *data; /**< Device string storage. */
> >> +       char *data;
> >> +       /**< Raw string including bus, class and driver arguments. */
> >>  };
> >>
> >>  /**
> >
> > - Flagging this patch for info and its impact on UNH jobs.
> >
> > This change is fine, but older libabigail versions could not deal with
> > such changes (anonymous union, changes of const fields).
> > This results in an ABI check failure in the UNH x86 job on Ubuntu
> > 18.04 (and for some people not using recent libabigail).
> > I can see the ARM job passes fine, so I suppose it is using a more
> > recent libabigail (running Ubuntu 20.04 maybe?).
> >
> > We either need to disable this x86 job or update its libabigail
> > package (maybe aligning with what we have for public CI which is
> > libabigail 1.8 manually compiled).
> >
> >
> > - For the longer term, what do you think of using/extending the .ci/
> > scripts for use by UNH jobs?
>
> I think it would be great if we had some of the scripts shared as a
> common resource.  That would also help us to look at fixes / changes
> when needed.
>
>
  
Thomas Monjalon April 16, 2021, 12:58 p.m. UTC | #4
16/04/2021 14:43, Lincoln Lavoie:
> All of the UNH ABI testing is moving info containers, so it can be run on
> top of each OS, alongside the other compile and unit testing. This is
> actually ready now, but hasn't been pushed live this week, because of the
> backlog in the system because of the DTS failure.  The additional compile
> jobs are already online now, it's just ABI that hasn't been pushed live.
> 
> This means the current ABI (what is reporting right now) is running on
> 18.04 for x86 and 20.04 for aarch64.  The aarch64 one will continue
> forward, because we're not going to moving to emulated environments for
> testing on that architecture.
> 
> This has two implications, first, the scripts for running ABI (and the
> other tests) become part of the container definition, and at the last
> meeting we talked about moving those definitions into the dpdk-ci repo,
> which I think makes sense.  Second, there isn't an operating system to
> "maintain" since it's what's inside the container images, which are
> periodically rebuilt, but pretty much treated as ephemeral.  Assuming the
> container bases / distros have the updated libabigail version packaged with
> them.

No, the version packaged in the OS is not recent enough.
Please check what is done in Travis and GitHub CI
in the shell function install_libabigail():
https://git.dpdk.org/dpdk/tree/.ci/linux-build.sh#n22


> On Fri, Apr 16, 2021 at 8:32 AM Aaron Conole <aconole@redhat.com> wrote:
> > David Marchand <david.marchand@redhat.com> writes:
> >
> > > On Tue, Apr 13, 2021 at 5:15 AM Xueming Li <xuemingl@nvidia.com> wrote:
> > >> diff --git a/lib/librte_eal/include/rte_devargs.h
> > b/lib/librte_eal/include/rte_devargs.h
> > >> index 296f19324f..134b44a887 100644
> > >> --- a/lib/librte_eal/include/rte_devargs.h
> > >> +++ b/lib/librte_eal/include/rte_devargs.h
> > >> @@ -60,16 +60,16 @@ struct rte_devargs {
> > >>         /** Name of the device. */
> > >>         char name[RTE_DEV_NAME_MAX_LEN];
> > >>         RTE_STD_C11
> > >> -       union {
> > >> -       /** Arguments string as given by user or "" for no argument. */
> > >> -               char *args;
> > >> +       union { /**< driver-related part of device string. */
> > >> +               const char *args; /**< legacy name. */
> > >>                 const char *drv_str;
> > >>         };
> > >>         struct rte_bus *bus; /**< bus handle. */
> > >>         struct rte_class *cls; /**< class handle. */
> > >>         const char *bus_str; /**< bus-related part of device string. */
> > >>         const char *cls_str; /**< class-related part of device string.
> > */
> > >> -       const char *data; /**< Device string storage. */
> > >> +       char *data;
> > >> +       /**< Raw string including bus, class and driver arguments. */
> > >>  };
> > >>
> > >>  /**
> > >
> > > - Flagging this patch for info and its impact on UNH jobs.
> > >
> > > This change is fine, but older libabigail versions could not deal with
> > > such changes (anonymous union, changes of const fields).
> > > This results in an ABI check failure in the UNH x86 job on Ubuntu
> > > 18.04 (and for some people not using recent libabigail).
> > > I can see the ARM job passes fine, so I suppose it is using a more
> > > recent libabigail (running Ubuntu 20.04 maybe?).
> > >
> > > We either need to disable this x86 job or update its libabigail
> > > package (maybe aligning with what we have for public CI which is
> > > libabigail 1.8 manually compiled).
> > >
> > >
> > > - For the longer term, what do you think of using/extending the .ci/
> > > scripts for use by UNH jobs?
> >
> > I think it would be great if we had some of the scripts shared as a
> > common resource.  That would also help us to look at fixes / changes
> > when needed.
> >
> >
> 
>
  
Lincoln Lavoie April 16, 2021, 1:14 p.m. UTC | #5
We can look into that, but that now will need to be tested to work across
all the different OS distros in the containers.

For now, we can install the update on the ubuntu 18.04 worker that is
running the production and remake the reference cache.

Cheers,
Lincoln

On Fri, Apr 16, 2021 at 8:58 AM Thomas Monjalon <thomas@monjalon.net> wrote:

> 16/04/2021 14:43, Lincoln Lavoie:
> > All of the UNH ABI testing is moving info containers, so it can be run on
> > top of each OS, alongside the other compile and unit testing. This is
> > actually ready now, but hasn't been pushed live this week, because of the
> > backlog in the system because of the DTS failure.  The additional compile
> > jobs are already online now, it's just ABI that hasn't been pushed live.
> >
> > This means the current ABI (what is reporting right now) is running on
> > 18.04 for x86 and 20.04 for aarch64.  The aarch64 one will continue
> > forward, because we're not going to moving to emulated environments for
> > testing on that architecture.
> >
> > This has two implications, first, the scripts for running ABI (and the
> > other tests) become part of the container definition, and at the last
> > meeting we talked about moving those definitions into the dpdk-ci repo,
> > which I think makes sense.  Second, there isn't an operating system to
> > "maintain" since it's what's inside the container images, which are
> > periodically rebuilt, but pretty much treated as ephemeral.  Assuming the
> > container bases / distros have the updated libabigail version packaged
> with
> > them.
>
> No, the version packaged in the OS is not recent enough.
> Please check what is done in Travis and GitHub CI
> in the shell function install_libabigail():
> https://git.dpdk.org/dpdk/tree/.ci/linux-build.sh#n22
>
>
> > On Fri, Apr 16, 2021 at 8:32 AM Aaron Conole <aconole@redhat.com> wrote:
> > > David Marchand <david.marchand@redhat.com> writes:
> > >
> > > > On Tue, Apr 13, 2021 at 5:15 AM Xueming Li <xuemingl@nvidia.com>
> wrote:
> > > >> diff --git a/lib/librte_eal/include/rte_devargs.h
> > > b/lib/librte_eal/include/rte_devargs.h
> > > >> index 296f19324f..134b44a887 100644
> > > >> --- a/lib/librte_eal/include/rte_devargs.h
> > > >> +++ b/lib/librte_eal/include/rte_devargs.h
> > > >> @@ -60,16 +60,16 @@ struct rte_devargs {
> > > >>         /** Name of the device. */
> > > >>         char name[RTE_DEV_NAME_MAX_LEN];
> > > >>         RTE_STD_C11
> > > >> -       union {
> > > >> -       /** Arguments string as given by user or "" for no
> argument. */
> > > >> -               char *args;
> > > >> +       union { /**< driver-related part of device string. */
> > > >> +               const char *args; /**< legacy name. */
> > > >>                 const char *drv_str;
> > > >>         };
> > > >>         struct rte_bus *bus; /**< bus handle. */
> > > >>         struct rte_class *cls; /**< class handle. */
> > > >>         const char *bus_str; /**< bus-related part of device
> string. */
> > > >>         const char *cls_str; /**< class-related part of device
> string.
> > > */
> > > >> -       const char *data; /**< Device string storage. */
> > > >> +       char *data;
> > > >> +       /**< Raw string including bus, class and driver arguments.
> */
> > > >>  };
> > > >>
> > > >>  /**
> > > >
> > > > - Flagging this patch for info and its impact on UNH jobs.
> > > >
> > > > This change is fine, but older libabigail versions could not deal
> with
> > > > such changes (anonymous union, changes of const fields).
> > > > This results in an ABI check failure in the UNH x86 job on Ubuntu
> > > > 18.04 (and for some people not using recent libabigail).
> > > > I can see the ARM job passes fine, so I suppose it is using a more
> > > > recent libabigail (running Ubuntu 20.04 maybe?).
> > > >
> > > > We either need to disable this x86 job or update its libabigail
> > > > package (maybe aligning with what we have for public CI which is
> > > > libabigail 1.8 manually compiled).
> > > >
> > > >
> > > > - For the longer term, what do you think of using/extending the .ci/
> > > > scripts for use by UNH jobs?
> > >
> > > I think it would be great if we had some of the scripts shared as a
> > > common resource.  That would also help us to look at fixes / changes
> > > when needed.
> > >
> > >
> >
> >
>
>
>
>
>
>
  

Patch

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index a5e82b7a97..a8bd664097 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -510,8 +510,6 @@  device_infos_display(const char *identifier)
 
 	if (rte_devargs_parsef(&da, "%s", identifier)) {
 		printf("cannot parse identifier\n");
-		if (da.args)
-			free(da.args);
 		return;
 	}
 
@@ -559,6 +557,7 @@  device_infos_display(const char *identifier)
 			}
 		}
 	};
+	rte_devargs_reset(&da);
 }
 
 void
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 96d2e0fcec..d4be23f8f8 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3015,8 +3015,6 @@  detach_devargs(char *identifier)
 	memset(&da, 0, sizeof(da));
 	if (rte_devargs_parsef(&da, "%s", identifier)) {
 		printf("cannot parse identifier\n");
-		if (da.args)
-			free(da.args);
 		return;
 	}
 
@@ -3025,6 +3023,7 @@  detach_devargs(char *identifier)
 			if (ports[port_id].port_status != RTE_PORT_STOPPED) {
 				printf("Port %u not stopped\n", port_id);
 				rte_eth_iterator_cleanup(&iterator);
+				rte_devargs_reset(&da);
 				return;
 			}
 			port_flow_flush(port_id);
@@ -3034,6 +3033,7 @@  detach_devargs(char *identifier)
 	if (rte_eal_hotplug_remove(da.bus->name, da.name) != 0) {
 		TESTPMD_LOG(ERR, "Failed to detach device %s(%s)\n",
 			    da.name, da.bus->name);
+		rte_devargs_reset(&da);
 		return;
 	}
 
@@ -3042,6 +3042,7 @@  detach_devargs(char *identifier)
 	printf("Device %s is detached\n", identifier);
 	printf("Now total ports is %d\n", nb_ports);
 	printf("Done\n");
+	rte_devargs_reset(&da);
 }
 
 void
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index 9a673347ae..d075409942 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -245,13 +245,14 @@  alloc_devargs(const char *name, const char *args)
 
 	devargs->bus = &rte_vdev_bus;
 	if (args)
-		devargs->args = strdup(args);
+		devargs->data = strdup(args);
 	else
-		devargs->args = strdup("");
+		devargs->data = strdup("");
+	devargs->args = devargs->data;
 
 	ret = strlcpy(devargs->name, name, sizeof(devargs->name));
 	if (ret < 0 || ret >= (int)sizeof(devargs->name)) {
-		free(devargs->args);
+		rte_devargs_reset(devargs);
 		free(devargs);
 		return NULL;
 	}
@@ -305,7 +306,7 @@  insert_vdev(const char *name, const char *args,
 
 	return 0;
 fail:
-	free(devargs->args);
+	rte_devargs_reset(devargs);
 	free(devargs);
 	free(dev);
 	return ret;
diff --git a/drivers/net/failsafe/failsafe_args.c b/drivers/net/failsafe/failsafe_args.c
index 707490b94c..b203e02d9a 100644
--- a/drivers/net/failsafe/failsafe_args.c
+++ b/drivers/net/failsafe/failsafe_args.c
@@ -451,8 +451,7 @@  failsafe_args_free(struct rte_eth_dev *dev)
 		sdev->cmdline = NULL;
 		free(sdev->fd_str);
 		sdev->fd_str = NULL;
-		free(sdev->devargs.args);
-		sdev->devargs.args = NULL;
+		rte_devargs_reset(&sdev->devargs);
 	}
 }
 
diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index b9fc508673..cb4a2abc02 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -79,7 +79,7 @@  fs_bus_init(struct rte_eth_dev *dev)
 					rte_eth_devices[pid].device->devargs;
 
 			/* Take control of probed device. */
-			free(da->args);
+			rte_devargs_reset(da);
 			memset(da, 0, sizeof(*da));
 			if (probed_da != NULL)
 				snprintf(devstr, sizeof(devstr), "%s,%s",
diff --git a/examples/multi_process/hotplug_mp/commands.c b/examples/multi_process/hotplug_mp/commands.c
index a8a39d07f7..48fd329583 100644
--- a/examples/multi_process/hotplug_mp/commands.c
+++ b/examples/multi_process/hotplug_mp/commands.c
@@ -121,8 +121,6 @@  static void cmd_dev_attach_parsed(void *parsed_result,
 
 	if (rte_devargs_parsef(&da, "%s", res->devargs)) {
 		cmdline_printf(cl, "cannot parse devargs\n");
-		if (da.args)
-			free(da.args);
 		return;
 	}
 
@@ -131,6 +129,7 @@  static void cmd_dev_attach_parsed(void *parsed_result,
 	else
 		cmdline_printf(cl, "failed to attached device %s\n",
 				da.name);
+	rte_devargs_reset(&da);
 }
 
 cmdline_parse_token_string_t cmd_dev_attach_attach =
@@ -168,8 +167,6 @@  static void cmd_dev_detach_parsed(void *parsed_result,
 
 	if (rte_devargs_parsef(&da, "%s", res->devargs)) {
 		cmdline_printf(cl, "cannot parse devargs\n");
-		if (da.args)
-			free(da.args);
 		return;
 	}
 
@@ -180,6 +177,7 @@  static void cmd_dev_detach_parsed(void *parsed_result,
 	else
 		cmdline_printf(cl, "failed to dettach device %s\n",
 			da.name);
+	rte_devargs_reset(&da);
 }
 
 cmdline_parse_token_string_t cmd_dev_detach_detach =
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 8a3bd3100a..148a23830a 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -185,10 +185,8 @@  local_dev_probe(const char *devargs, struct rte_device **new_dev)
 	return ret;
 
 err_devarg:
-	if (rte_devargs_remove(da) != 0) {
-		free(da->args);
-		free(da);
-	}
+	if (rte_devargs_remove(da) != 0)
+		rte_devargs_reset(da);
 	return ret;
 }
 
@@ -586,7 +584,8 @@  rte_dev_iterator_init(struct rte_dev_iterator *it,
 	it->bus_str = NULL;
 	it->cls_str = NULL;
 
-	devargs.data = dev_str;
+	/* Setting data field implies no malloc in parsing. */
+	devargs.data = (void *)(intptr_t)dev_str;
 	if (rte_devargs_layers_parse(&devargs, dev_str))
 		goto get_out;
 
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index fcf3d9a3cc..48f85ee9c0 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -150,7 +150,7 @@  rte_devargs_layers_parse(struct rte_devargs *devargs,
 	 * their parsing afterward.
 	 */
 	if (devargs->data != devstr) {
-		char *s = (void *)(intptr_t)(devargs->data);
+		char *s = devargs->data;
 
 		while ((s = strchr(s, '/'))) {
 			*s = '\0';
@@ -219,13 +219,14 @@  rte_devargs_parse(struct rte_devargs *da, const char *dev)
 	da->bus = bus;
 	/* Parse eventual device arguments */
 	if (devname[i] == ',')
-		da->args = strdup(&devname[i + 1]);
+		da->data = strdup(&devname[i + 1]);
 	else
-		da->args = strdup("");
-	if (da->args == NULL) {
+		da->data = strdup("");
+	if (da->data == NULL) {
 		RTE_LOG(ERR, EAL, "not enough memory to parse arguments\n");
 		return -ENOMEM;
 	}
+	da->drv_str = da->data;
 	return 0;
 }
 
@@ -260,6 +261,16 @@  rte_devargs_parsef(struct rte_devargs *da, const char *format, ...)
 	return ret;
 }
 
+void
+rte_devargs_reset(struct rte_devargs *da)
+{
+	if (da == NULL)
+		return;
+	if (da->data)
+		free(da->data);
+	da->data = NULL;
+}
+
 int
 rte_devargs_insert(struct rte_devargs **da)
 {
@@ -276,15 +287,8 @@  rte_devargs_insert(struct rte_devargs **da)
 		if (strcmp(listed_da->bus->name, (*da)->bus->name) == 0 &&
 				strcmp(listed_da->name, (*da)->name) == 0) {
 			/* device already in devargs list, must be updated */
-			listed_da->type = (*da)->type;
-			listed_da->policy = (*da)->policy;
-			free(listed_da->args);
-			listed_da->args = (*da)->args;
-			listed_da->bus = (*da)->bus;
-			listed_da->cls = (*da)->cls;
-			listed_da->bus_str = (*da)->bus_str;
-			listed_da->cls_str = (*da)->cls_str;
-			listed_da->data = (*da)->data;
+			rte_devargs_reset(listed_da);
+			*listed_da = **da;
 			/* replace provided devargs with found one */
 			free(*da);
 			*da = listed_da;
@@ -326,7 +330,7 @@  rte_devargs_add(enum rte_devtype devtype, const char *devargs_str)
 
 fail:
 	if (devargs) {
-		free(devargs->args);
+		rte_devargs_reset(devargs);
 		free(devargs);
 	}
 
@@ -346,7 +350,7 @@  rte_devargs_remove(struct rte_devargs *devargs)
 		if (strcmp(d->bus->name, devargs->bus->name) == 0 &&
 		    strcmp(d->name, devargs->name) == 0) {
 			TAILQ_REMOVE(&devargs_list, d, next);
-			free(d->args);
+			rte_devargs_reset(d);
 			free(d);
 			return 0;
 		}
diff --git a/lib/librte_eal/common/hotplug_mp.c b/lib/librte_eal/common/hotplug_mp.c
index ee791903b3..ae6010e8f8 100644
--- a/lib/librte_eal/common/hotplug_mp.c
+++ b/lib/librte_eal/common/hotplug_mp.c
@@ -95,6 +95,7 @@  __handle_secondary_request(void *param)
 
 	tmp_req = *req;
 
+	memset(&da, 0, sizeof(da));
 	if (req->t == EAL_DEV_REQ_TYPE_ATTACH) {
 		ret = local_dev_probe(req->devargs, &dev);
 		if (ret != 0) {
@@ -118,8 +119,6 @@  __handle_secondary_request(void *param)
 		ret = rte_devargs_parse(&da, req->devargs);
 		if (ret != 0)
 			goto finish;
-		free(da.args); /* we don't need those */
-		da.args = NULL;
 
 		ret = eal_dev_hotplug_request_to_secondary(&tmp_req);
 		if (ret != 0) {
@@ -176,6 +175,7 @@  __handle_secondary_request(void *param)
 	if (ret)
 		RTE_LOG(ERR, EAL, "failed to send response to secondary\n");
 
+	rte_devargs_reset(&da);
 	free(bundle->peer);
 	free(bundle);
 }
@@ -283,7 +283,7 @@  static void __handle_primary_request(void *param)
 
 		ret = local_dev_remove(dev);
 quit:
-		free(da->args);
+		rte_devargs_reset(da);
 		free(da);
 		break;
 	default:
diff --git a/lib/librte_eal/include/rte_devargs.h b/lib/librte_eal/include/rte_devargs.h
index 296f19324f..134b44a887 100644
--- a/lib/librte_eal/include/rte_devargs.h
+++ b/lib/librte_eal/include/rte_devargs.h
@@ -60,16 +60,16 @@  struct rte_devargs {
 	/** Name of the device. */
 	char name[RTE_DEV_NAME_MAX_LEN];
 	RTE_STD_C11
-	union {
-	/** Arguments string as given by user or "" for no argument. */
-		char *args;
+	union { /**< driver-related part of device string. */
+		const char *args; /**< legacy name. */
 		const char *drv_str;
 	};
 	struct rte_bus *bus; /**< bus handle. */
 	struct rte_class *cls; /**< class handle. */
 	const char *bus_str; /**< bus-related part of device string. */
 	const char *cls_str; /**< class-related part of device string. */
-	const char *data; /**< Device string storage. */
+	char *data;
+	/**< Raw string including bus, class and driver arguments. */
 };
 
 /**
@@ -145,6 +145,16 @@  rte_devargs_parsef(struct rte_devargs *da,
 		   const char *format, ...)
 __rte_format_printf(2, 0);
 
+/**
+ * Free resources in devargs.
+ *
+ * @param da
+ *   The devargs structure holding the device information.
+ */
+__rte_experimental
+void
+rte_devargs_reset(struct rte_devargs *da);
+
 /**
  * Insert an rte_devargs in the global list.
  *
diff --git a/lib/librte_eal/version.map b/lib/librte_eal/version.map
index e7217ae288..fe5c3dac98 100644
--- a/lib/librte_eal/version.map
+++ b/lib/librte_eal/version.map
@@ -410,6 +410,7 @@  EXPERIMENTAL {
 	rte_power_pause; # WINDOWS_NO_EXPORT
 
 	# added in 21.05
+	rte_devargs_reset;
 	rte_intr_callback_unregister_sync;
 	rte_log_list_types;
 	rte_thread_key_create;
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 6b5cfd696d..0419500fc3 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -193,13 +193,14 @@  int
 rte_eth_iterator_init(struct rte_dev_iterator *iter, const char *devargs_str)
 {
 	int ret;
-	struct rte_devargs devargs = {.args = NULL};
+	struct rte_devargs devargs;
 	const char *bus_param_key;
 	char *bus_str = NULL;
 	char *cls_str = NULL;
 	int str_size;
 
 	memset(iter, 0, sizeof(*iter));
+	memset(&devargs, 0, sizeof(devargs));
 
 	/*
 	 * The devargs string may use various syntaxes:
@@ -244,8 +245,6 @@  rte_eth_iterator_init(struct rte_dev_iterator *iter, const char *devargs_str)
 		goto error;
 	}
 	iter->cls_str = cls_str;
-	free(devargs.args); /* allocated by rte_devargs_parse() */
-	devargs.args = NULL;
 
 	iter->bus = devargs.bus;
 	if (iter->bus->dev_iterate == NULL) {
@@ -278,13 +277,14 @@  rte_eth_iterator_init(struct rte_dev_iterator *iter, const char *devargs_str)
 
 end:
 	iter->cls = rte_class_find_by_name("eth");
+	rte_devargs_reset(&devargs);
 	return 0;
 
 error:
 	if (ret == -ENOTSUP)
 		RTE_ETHDEV_LOG(ERR, "Bus %s does not support iterating.\n",
 				iter->bus->name);
-	free(devargs.args);
+	rte_devargs_reset(&devargs);
 	free(bus_str);
 	free(cls_str);
 	return ret;