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 <[email protected]>
> Acked-by: Roni Bar Yanai <[email protected]>
> ---
>  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);
> 

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to