[ovs-dev] [PATCH v2 2/2] netdev-dpdk: Add option to configure VF MAC address.

Ilya Maximets i.maximets at ovn.org
Tue Sep 15 22:08:02 UTC 2020


On 7/7/20 11:18 AM, Gaetan Rivet wrote:
> In some cloud topologies, using DPDK VF representors in guest requires
> configuring a VF before it is assigned to the guest.
> 
> A first basic option for such configuration is setting the VF MAC
> address. Add a key 'dpdk-vf-mac' to the 'options' column of the Interface
> table.
> 
> This option can be used as such:
> 
>    $ ovs-vsctl add-port br0 dpdk-rep0 -- set Interface dpdk-rep0 type=dpdk \
>       options:dpdk-vf-mac=00:11:22:33:44:55
> 
> Signed-off-by: Gaetan Rivet <grive at u256.net>
> Suggested-by: Ilya Maximets <i.maximets at ovn.org>
> Acked-by: Eli Britstein <elibr at mellanox.com>
> ---
>  Documentation/topics/dpdk/phy.rst | 12 ++++
>  NEWS                              |  2 +
>  lib/netdev-dpdk.c                 | 93 +++++++++++++++++++++++++------
>  vswitchd/vswitch.xml              |  6 ++
>  4 files changed, 96 insertions(+), 17 deletions(-)
> 
> diff --git a/Documentation/topics/dpdk/phy.rst b/Documentation/topics/dpdk/phy.rst
> index 38e52c8de..44c2b7cd0 100644
> --- a/Documentation/topics/dpdk/phy.rst
> +++ b/Documentation/topics/dpdk/phy.rst
> @@ -379,6 +379,18 @@ an eth device whose mac address is ``00:11:22:33:44:55``::
>      $ ovs-vsctl add-port br0 dpdk-mac -- set Interface dpdk-mac type=dpdk \
>         options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55"
>  
> +Representor specific configuration
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +In some topologies, a VF must be configured before being assigned to a
> +guest (VM) machine.  This configuration is done through VF-specific fields
> +in the ``options`` column of the Interface table.
> +
> +- Configure the VF MAC address::
> +
> +    $ ovs-vsctl add-port br0 dpdk-rep0 -- set Interface dpdk-rep0 type=dpdk \
> +       options:dpdk-vf-mac=00:11:22:33:44:55
> +
>  Jumbo Frames
>  ------------
>  
> diff --git a/NEWS b/NEWS
> index 0116b3ea0..e79122968 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,8 @@ Post-v2.13.0
>       * Deprecated DPDK pdump packet capture support removed.
>       * Deprecated DPDK ring ports (dpdkr) are no longer supported.
>       * Add hardware offload support for VLAN Push/Pop actions (experimental).
> +     * New 'options:dpdk-vf-mac' field for DPDK interface of VF ports,
> +       that allows configuring the MAC address of a VF representor.
>     - Linux datapath:
>       * Support for kernel versions up to 5.5.x.
>     - AF_XDP:
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 6324ed96f..a493a45ab 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1885,6 +1885,42 @@ dpdk_process_queue_size(struct netdev *netdev, const struct smap *args,
>      }
>  }
>  
> +static int
> +netdev_dpdk_set_etheraddr__(struct netdev_dpdk *dev, const struct eth_addr mac)
> +    OVS_REQUIRES(dev->mutex)
> +{
> +    int err = 0;
> +
> +    if (!eth_addr_equals(dev->hwaddr, mac)) {
> +        if (dev->type == DPDK_DEV_ETH) {
> +            struct rte_ether_addr ea;
> +
> +            memcpy(ea.addr_bytes, mac.ea, ETH_ADDR_LEN);
> +            err = -rte_eth_dev_default_mac_addr_set(dev->port_id, &ea);
> +        }
> +        if (!err) {
> +            dev->hwaddr = mac;
> +            netdev_change_seq_changed(&dev->up);
> +        } else {
> +            VLOG_WARN("%s: Failed to set requested mac("ETH_ADDR_FMT"): %s",
> +                      netdev_get_name(&dev->up), ETH_ADDR_ARGS(mac),
> +                      rte_strerror(err));
> +        }
> +    }
> +
> +    return err;
> +}
> +
> +static bool
> +dpdk_port_is_representor(struct netdev_dpdk *dev)
> +    OVS_REQUIRES(dev->mutex)
> +{
> +    struct rte_eth_dev_info dev_info;
> +
> +    rte_eth_dev_info_get(dev->port_id, &dev_info);
> +    return (*dev_info.dev_flags) & RTE_ETH_DEV_REPRESENTOR;
> +}
> +
>  static int
>  netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
>                         char **errp)
> @@ -1898,6 +1934,7 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
>          {RTE_FC_RX_PAUSE, RTE_FC_FULL    }
>      };
>      const char *new_devargs;
> +    const char *vf_mac;
>      int err = 0;
>  
>      ovs_mutex_lock(&dpdk_mutex);
> @@ -1968,6 +2005,43 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
>          goto out;
>      }
>  
> +    vf_mac = smap_get(args, "dpdk-vf-mac");

We need to report this config option in netdev_dpdk_get_config() function, so
we need to store it somewhere in netdev-dpdk.   And since we will store it
we could check if it actually changed and avoid parsing and configuration
attempts if not.

> +    if (vf_mac) {

There is still a question what to do if 'dpdk-vf-mac' removed from the
database.  In this case OVS will not do anything and will keep last
configured MAC on this interface.  This is a bit contr-intuitive.
Also, you're prohibiting setting of MAC address to all zeroes which might
be the original MAC of the VF (not sure how exactly if works with mlx cards,
but, IIRC, it works this way for some NICs, i.e. initial MACs for VFs are
all zeroes). Might be not an issue though since libvirt tries to use differet
MAC to clear devices that doesn't support all-zeroes as their MAC.

Anyway, this must be documented that OVS will leave this MAC configured
on a VF in case of port removal or clearing the database entry.  Because
this very likely will produce ussues for users of this feature.

> +        struct eth_addr mac;
> +
> +        err = EINVAL;
> +
> +        if (!dpdk_port_is_representor(dev)) {
> +            VLOG_ERR_BUF(errp, "'%s' is trying to set the VF MAC '%s' "
> +                         "but 'options:dpdk-vf-mac' is only supported for "
> +                         "VF representors.",
> +                         netdev_get_name(netdev), vf_mac);
> +            goto out;
> +        }
> +        if (!eth_addr_from_string(vf_mac, &mac)) {
> +            VLOG_ERR_BUF(errp, "interface '%s': cannot parse MAC '%s'",

Should it be 'VF MAC' insted of just 'MAC'?

> +                         netdev_get_name(netdev), vf_mac);
> +            goto out;
> +        }
> +        if (eth_addr_is_multicast(mac)) {
> +            VLOG_ERR_BUF(errp,
> +                         "interface '%s': cannot set MAC to multicast address",
> +                         netdev_get_name(netdev));
> +            goto out;
> +        }
> +        if (eth_addr_is_zero(mac)) {
> +            VLOG_ERR_BUF(errp,
> +                         "interface '%s': cannot set MAC to all zero address",
> +                         netdev_get_name(netdev));
> +            goto out;
> +        }
> +
> +        err = netdev_dpdk_set_etheraddr__(dev, mac);
> +        if (err) {
> +            goto out;
> +        }
> +    }
> +
>      lsc_interrupt_mode = smap_get_bool(args, "dpdk-lsc-interrupt", false);
>      if (dev->requested_lsc_interrupt_mode != lsc_interrupt_mode) {
>          dev->requested_lsc_interrupt_mode = lsc_interrupt_mode;
> @@ -2916,25 +2990,10 @@ netdev_dpdk_set_etheraddr(struct netdev *netdev, const struct eth_addr mac)
>      int err = 0;
>  
>      ovs_mutex_lock(&dev->mutex);
> -    if (!eth_addr_equals(dev->hwaddr, mac)) {
> -        if (dev->type == DPDK_DEV_ETH) {
> -            struct rte_ether_addr ea;
> -
> -            memcpy(ea.addr_bytes, mac.ea, ETH_ADDR_LEN);
> -            err = rte_eth_dev_default_mac_addr_set(dev->port_id, &ea);
> -        }
> -        if (!err) {
> -            dev->hwaddr = mac;
> -            netdev_change_seq_changed(netdev);
> -        } else {
> -            VLOG_WARN("%s: Failed to set requested mac("ETH_ADDR_FMT"): %s",
> -                      netdev_get_name(netdev), ETH_ADDR_ARGS(mac),
> -                      rte_strerror(-err));
> -        }
> -    }
> +    err = netdev_dpdk_set_etheraddr__(dev, mac);
>      ovs_mutex_unlock(&dev->mutex);
>  
> -    return -err;
> +    return err;
>  }
>  
>  static int
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index b6acb34ca..b8a991c23 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -3272,6 +3272,12 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
>            descriptors will be used by default.
>          </p>
>        </column>
> +
> +      <column name="options" key="dpdk-vf-mac">
> +        <p>Ethernet address to set for this VF interface.  If unset then the
> +        default <ref table="Interface" column="mac"/> address is used.</p>

This part of the documentation is not correct as 'mac' column will not be used
for non-internal interfaces and DPDK interfaces are never internal in current
implementation.

> +        <p>This option may only be used with dpdk VF representors.</p>
> +      </column>
>      </group>
>  
>      <group title="EMC (Exact Match Cache) Configuration">
> 



More information about the dev mailing list