[ovs-dev] [PATCH ovs v1 2/2] netdev-dpdk: Allow setting MAC on DPDK interfaces

Ilya Maximets i.maximets at ovn.org
Tue Mar 24 13:16:06 UTC 2020


On 3/5/20 3:15 PM, Noa Ezra wrote:
> Adding a command for setting MAC of DPDK interfaces using:
> ovs-appctl netdev-dpdk/set-mac <interface> <mac>
> 
> Signed-off-by: Noa Ezra <noae at mellanox.com>
> Acked-by: Roni Bar Yanai <roniba at mellanox.com>
> ---
>  lib/netdev-dpdk.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)

I'm still not 100% sure that it is the way we need to go since
this configuration will not survive OVS restarts, i.e. it's difficult
to use/maintain in something like OpenStack environment.
(Also, DPDK currently doesn't have any protection from MAC changing
on VFs from the inside of VM.)
But anyway.  Comments inline.

Best regards, Ilya Maximets.

> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index e375b3d..2b8adac 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -3917,6 +3917,38 @@ out:
>      netdev_close(netdev);
>  }
>  
> +static void
> +netdev_dpdk_set_mac(struct unixctl_conn *conn, int argc OVS_UNUSED,
> +                    const char *argv[], void *aux OVS_UNUSED)
> +{
> +    struct netdev *netdev = NULL;
> +    char *response = NULL;
> +    struct eth_addr mac;
> +    int error;
> +
> +    netdev = netdev_from_name(argv[1]);
> +    if (!netdev || !is_dpdk_class(netdev->netdev_class)) {
> +        unixctl_command_reply_error(conn, "Not a DPDK Interface");

netdev needs to be closed.

> +        return;
> +    }
> +
> +    if (!argv[2] || !eth_addr_from_string(argv[2], &mac)) {

need to check that argc > 2 for consistency.

> +        response = xasprintf("No MAC address to set.");

Why this is not treated as error?  i.e. _reply_error().

> +        goto out;
> +    }
> +
> +    error = netdev_dpdk_set_etheraddr(netdev, mac);
> +    if (error) {
> +        response = xasprintf("interface %s: setting MAC failed (%s)",
> +                             argv[1], ovs_strerror(error));

Why this is not treated as error?

> +    }
> +    response = xasprintf("set-mac done.");
> +
> +out:
> +    unixctl_command_reply(conn, response);

response leaked.

> +    netdev_close(netdev);
> +}
> +
>  /*
>   * Set virtqueue flags so that we do not receive interrupts.
>   */
> @@ -4256,6 +4288,10 @@ netdev_dpdk_class_init(void)
>                                   "[netdev]", 0, 1,
>                                   netdev_dpdk_get_mempool_info, NULL);
>  
> +        unixctl_command_register("netdev-dpdk/set-mac",
> +                                 "[netdev] [mac]", 2, 2,
> +                                 netdev_dpdk_set_mac, NULL);

This command needs to be documented in man pages and NEWS.

> +
>          ret = rte_eth_dev_callback_register(RTE_ETH_ALL,
>                                              RTE_ETH_EVENT_INTR_RESET,
>                                              dpdk_eth_event_callback, NULL);
> 



More information about the dev mailing list