Re: [ovs-dev] [PATCH 1/2] netdev-dpdk: Enhance logging for port hotplug.
On Fri, 27 Feb 2026 at 23:36, Ilya Maximets wrote:
>
> On 2/26/26 6:24 PM, David Marchand via dev wrote:
> > Currently, if one incorrect mac is set, a first log with little context
> > is displayed, followed by a more complete one.
> >
> > Besides, if no port can be identified with the passed mac, then no
> > explanation is displayed.
> >
> > Report some details in a single log.
> >
> > Before:
> > netdev_dpdk|ERR|invalid mac: 00:00:00:00:00:
> > netdev_dpdk|WARN|Error attaching device 'class=eth,mac=00:00:00:00:00:'
> > to DPDK
> > ...
> > netdev_dpdk|WARN|Error attaching device 'class=eth,mac=00:00:00:00:00:00'
> > to DPDK
> >
> > After:
> > netdev_dpdk|WARN|Error attaching device 'class=eth,mac=00:00:00:00:00:'
> > to DPDK: invalid mac
> > ...
> > netdev_dpdk|WARN|Error attaching device 'class=eth,mac=00:00:00:00:00:00'
> > to DPDK: unknown mac
> >
> > Signed-off-by: David Marchand
> > Acked-by: Eli Britstein
> > Acked-by: Eelco Chaudron
> > ---
> > Changes since RFC v1:
> > - removed redundant "to DPDK" in netdev-dpdk log messages,
> >
> > ---
> > lib/netdev-dpdk.c | 29 -
> > 1 file changed, 20 insertions(+), 9 deletions(-)
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > index 923191da84..c51fe7c258 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -2028,13 +2028,15 @@ netdev_dpdk_lookup_by_port_id(dpdk_port_t port_id)
> > }
> >
> > static dpdk_port_t
> > -netdev_dpdk_get_port_by_mac(const char *mac_str)
> > +netdev_dpdk_get_port_by_mac(const char *mac_str, char **extra_err)
> > {
> > dpdk_port_t port_id;
> > struct eth_addr mac, port_mac;
> >
> > +*extra_err = NULL;
> > +
> > if (!eth_addr_from_string(mac_str, &mac)) {
> > -VLOG_ERR("invalid mac: %s", mac_str);
> > +*extra_err = xstrdup("invalid mac");
>
> The value is always a constant string. Can we use constant string
> pointers and avoid unnecessary memory allocations?
Hum.. probably some string contained a dynamic part (in a non
submitted attempt of mine), but that's not the case here.
Yes, I'll drop this allocation.
>
> > return DPDK_ETH_PORT_ID_INVALID;
> > }
> >
> > @@ -2048,6 +2050,7 @@ netdev_dpdk_get_port_by_mac(const char *mac_str)
> > }
> > }
> >
> > +*extra_err = xstrdup("unknown mac");
> > return DPDK_ETH_PORT_ID_INVALID;
> > }
> >
> > @@ -2084,32 +2087,40 @@ netdev_dpdk_process_devargs(struct netdev_dpdk *dev,
> > OVS_REQUIRES(dpdk_mutex)
> > {
> > dpdk_port_t new_port_id;
> > +char *extra_err = NULL;
> >
> > if (strncmp(devargs, "class=eth,mac=", 14) == 0) {
> > -new_port_id = netdev_dpdk_get_port_by_mac(&devargs[14]);
> > +new_port_id = netdev_dpdk_get_port_by_mac(&devargs[14],
> > &extra_err);
> > } else {
> > new_port_id = netdev_dpdk_get_port_by_devargs(devargs);
> > if (!rte_eth_dev_is_valid_port(new_port_id)) {
> > -/* Device not found in DPDK, attempt to attach it */
> > -if (rte_dev_probe(devargs)) {
> > +int ret;
> > +
> > +/* Port not found in DPDK, attempt to attach the device. */
> > +ret = rte_dev_probe(devargs);
> > +if (ret < 0) {
> > new_port_id = DPDK_ETH_PORT_ID_INVALID;
> > +extra_err = xstrdup(ovs_strerror(-ret));
> > } else {
> > new_port_id = netdev_dpdk_get_port_by_devargs(devargs);
> > if (rte_eth_dev_is_valid_port(new_port_id)) {
> > -/* Attach successful */
> > +/* Port lookup successful. */
> > dev->attached = true;
> > -VLOG_INFO("Device '%s' attached to DPDK", devargs);
> > +VLOG_INFO("Device '%s' attached", devargs);
> > } else {
> > -/* Attach unsuccessful */
> > +/* Port lookup unsuccessful. */
>
> Not sure why these comments have changed. New ones do not contribute
> anything to the understanding of the code, i.e. very obvious. And the
> old ones still seem to be accurate.
Ok.
--
David Marchand
___
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/2] netdev-dpdk: Enhance logging for port hotplug.
On 2/26/26 5:24 PM, David Marchand wrote: > Currently, if one incorrect mac is set, a first log with little context > is displayed, followed by a more complete one. > > Besides, if no port can be identified with the passed mac, then no > explanation is displayed. > > Report some details in a single log. > > Before: > netdev_dpdk|ERR|invalid mac: 00:00:00:00:00: > netdev_dpdk|WARN|Error attaching device 'class=eth,mac=00:00:00:00:00:' > to DPDK > ... > netdev_dpdk|WARN|Error attaching device 'class=eth,mac=00:00:00:00:00:00' > to DPDK > > After: > netdev_dpdk|WARN|Error attaching device 'class=eth,mac=00:00:00:00:00:' > to DPDK: invalid mac > ... > netdev_dpdk|WARN|Error attaching device 'class=eth,mac=00:00:00:00:00:00' > to DPDK: unknown mac > > Signed-off-by: David Marchand > Acked-by: Eli Britstein > Acked-by: Eelco Chaudron LGTM. I will ack next version as Ilya had some changes requested. ___ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/2] netdev-dpdk: Enhance logging for port hotplug.
On 2/26/26 6:24 PM, David Marchand via dev wrote:
> Currently, if one incorrect mac is set, a first log with little context
> is displayed, followed by a more complete one.
>
> Besides, if no port can be identified with the passed mac, then no
> explanation is displayed.
>
> Report some details in a single log.
>
> Before:
> netdev_dpdk|ERR|invalid mac: 00:00:00:00:00:
> netdev_dpdk|WARN|Error attaching device 'class=eth,mac=00:00:00:00:00:'
> to DPDK
> ...
> netdev_dpdk|WARN|Error attaching device 'class=eth,mac=00:00:00:00:00:00'
> to DPDK
>
> After:
> netdev_dpdk|WARN|Error attaching device 'class=eth,mac=00:00:00:00:00:'
> to DPDK: invalid mac
> ...
> netdev_dpdk|WARN|Error attaching device 'class=eth,mac=00:00:00:00:00:00'
> to DPDK: unknown mac
>
> Signed-off-by: David Marchand
> Acked-by: Eli Britstein
> Acked-by: Eelco Chaudron
> ---
> Changes since RFC v1:
> - removed redundant "to DPDK" in netdev-dpdk log messages,
>
> ---
> lib/netdev-dpdk.c | 29 -
> 1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 923191da84..c51fe7c258 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -2028,13 +2028,15 @@ netdev_dpdk_lookup_by_port_id(dpdk_port_t port_id)
> }
>
> static dpdk_port_t
> -netdev_dpdk_get_port_by_mac(const char *mac_str)
> +netdev_dpdk_get_port_by_mac(const char *mac_str, char **extra_err)
> {
> dpdk_port_t port_id;
> struct eth_addr mac, port_mac;
>
> +*extra_err = NULL;
> +
> if (!eth_addr_from_string(mac_str, &mac)) {
> -VLOG_ERR("invalid mac: %s", mac_str);
> +*extra_err = xstrdup("invalid mac");
The value is always a constant string. Can we use constant string
pointers and avoid unnecessary memory allocations?
> return DPDK_ETH_PORT_ID_INVALID;
> }
>
> @@ -2048,6 +2050,7 @@ netdev_dpdk_get_port_by_mac(const char *mac_str)
> }
> }
>
> +*extra_err = xstrdup("unknown mac");
> return DPDK_ETH_PORT_ID_INVALID;
> }
>
> @@ -2084,32 +2087,40 @@ netdev_dpdk_process_devargs(struct netdev_dpdk *dev,
> OVS_REQUIRES(dpdk_mutex)
> {
> dpdk_port_t new_port_id;
> +char *extra_err = NULL;
>
> if (strncmp(devargs, "class=eth,mac=", 14) == 0) {
> -new_port_id = netdev_dpdk_get_port_by_mac(&devargs[14]);
> +new_port_id = netdev_dpdk_get_port_by_mac(&devargs[14], &extra_err);
> } else {
> new_port_id = netdev_dpdk_get_port_by_devargs(devargs);
> if (!rte_eth_dev_is_valid_port(new_port_id)) {
> -/* Device not found in DPDK, attempt to attach it */
> -if (rte_dev_probe(devargs)) {
> +int ret;
> +
> +/* Port not found in DPDK, attempt to attach the device. */
> +ret = rte_dev_probe(devargs);
> +if (ret < 0) {
> new_port_id = DPDK_ETH_PORT_ID_INVALID;
> +extra_err = xstrdup(ovs_strerror(-ret));
> } else {
> new_port_id = netdev_dpdk_get_port_by_devargs(devargs);
> if (rte_eth_dev_is_valid_port(new_port_id)) {
> -/* Attach successful */
> +/* Port lookup successful. */
> dev->attached = true;
> -VLOG_INFO("Device '%s' attached to DPDK", devargs);
> +VLOG_INFO("Device '%s' attached", devargs);
> } else {
> -/* Attach unsuccessful */
> +/* Port lookup unsuccessful. */
Not sure why these comments have changed. New ones do not contribute
anything to the understanding of the code, i.e. very obvious. And the
old ones still seem to be accurate.
> new_port_id = DPDK_ETH_PORT_ID_INVALID;
> +extra_err = xstrdup("port unknown");
> }
> }
> }
> }
>
> if (new_port_id == DPDK_ETH_PORT_ID_INVALID) {
> -VLOG_WARN_BUF(errp, "Error attaching device '%s' to DPDK", devargs);
> +VLOG_WARN_BUF(errp, "Error attaching device '%s': %s", devargs,
> + extra_err ? extra_err : "unknown error");
> }
> +free(extra_err);
>
> return new_port_id;
> }
___
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
