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?
> 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
