On Fri, 27 Feb 2026 at 23:36, Ilya Maximets <[email protected]> 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 <[email protected]>
> > Acked-by: Eli Britstein <[email protected]>
> > Acked-by: Eelco Chaudron <[email protected]>
> > ---
> > 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
