On 30/10/2020 15:45, 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 <gr...@u256.net>
> Suggested-by: Ilya Maximets <i.maxim...@ovn.org>
> Acked-by: Eli Britstein <el...@nvidia.com>
> ---
> Documentation/topics/dpdk/phy.rst | 25 ++++++++++++
> NEWS | 2 +
> lib/netdev-dpdk.c | 67 +++++++++++++++++++++++++++++++
> vswitchd/vswitch.xml | 18 +++++++++
> 4 files changed, 112 insertions(+)
>
> diff --git a/Documentation/topics/dpdk/phy.rst
> b/Documentation/topics/dpdk/phy.rst
> index 55a98e2b0..1415e1b8c 100644
> --- a/Documentation/topics/dpdk/phy.rst
> +++ b/Documentation/topics/dpdk/phy.rst
> @@ -379,6 +379,31 @@ 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.
> +
> +.. important::
> +
> + Some DPDK port use `bifurcated drivers <bifurcated-drivers>`__,
> + which means that a kernel netdevice remains when Open vSwitch is stopped.
> +
> + In such case, any configuration applied to a VF would remain set on the
> + kernel netdevice, and be inherited from it when Open vSwitch is restarted,
> + even if the options described in this section are unset from Open vSwitch.
> +
> +.. _bifurcated-drivers:
> http://doc.dpdk.org/guides/linux_gsg/linux_drivers.html#bifurcated-driver
> +
> +- Configure the VF MAC address::
> +
> + $ ovs-vsctl set Interface dpdk-rep0 options:dpdk-vf-mac=00:11:22:33:44:55
> +
> +On successful configuration, the requested MAC is shown in the ``mac_in_use``
> +column of the ``Interface`` table.
> +
True, but it's simpler to say it's the configured mac. If you want to
qualify with the config success, you should add for config failure also.
If everyone agrees with using dpdk-vf-mac as output from multiple
commands where it can be different, then I think it deserves a comment
here. Even a slightly nicer version of
ovs-appctl dpctl/show -> configured dpdk-vf-mac=00:11:22:33:44:55
ovs-vsctl show -> requested dpdk-vf-mac=33:11:22:33:44:55
ovs-vsctl get Interface myport status -> configured
dpdk-vf-mac=00:11:22:33:44:55
Aside from that LGTM.
> Jumbo Frames
> ------------
>
> diff --git a/NEWS b/NEWS
> index 8bb5bdc3f..b8cb3e227 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -5,6 +5,8 @@ Post-v2.14.0
> status of the storage that's backing a database.
> - DPDK:
> * Removed support for vhost-user dequeue zero-copy.
> + * New 'options:dpdk-vf-mac' field for DPDK interface of VF ports,
> + that allows configuring the MAC address of a VF representor.
> - The environment variable OVS_UNBOUND_CONF, if set, is now used
> as the DNS resolver's (unbound) configuration file.
> - Linux datapath:
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 084f97807..d678def4b 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -522,6 +522,9 @@ struct netdev_dpdk {
> * otherwise interrupt mode is used. */
> bool requested_lsc_interrupt_mode;
> bool lsc_interrupt_mode;
> +
> + /* VF configuration. */
> + struct eth_addr requested_hwaddr;
> );
>
> PADDED_MEMBERS(CACHE_LINE_SIZE,
> @@ -1692,6 +1695,16 @@ out:
> return ret;
> }
>
> +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_get_config(const struct netdev *netdev, struct smap *args)
> {
> @@ -1726,6 +1739,11 @@ netdev_dpdk_get_config(const struct netdev *netdev,
> struct smap *args)
> }
> smap_add(args, "lsc_interrupt_mode",
> dev->lsc_interrupt_mode ? "true" : "false");
> +
> + if (dpdk_port_is_representor(dev)) {
> + smap_add_format(args, "dpdk-vf-mac", ETH_ADDR_FMT,
> + ETH_ADDR_ARGS(dev->requested_hwaddr));
> + }
> }
> ovs_mutex_unlock(&dev->mutex);
>
> @@ -1905,6 +1923,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);
> @@ -1975,6 +1994,28 @@ netdev_dpdk_set_config(struct netdev *netdev, const
> struct smap *args,
> goto out;
> }
>
> + vf_mac = smap_get(args, "dpdk-vf-mac");
> + if (vf_mac) {
> + struct eth_addr mac;
> +
> + if (!dpdk_port_is_representor(dev)) {
> + VLOG_WARN_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);
> + } else if (!eth_addr_from_string(vf_mac, &mac)) {
> + VLOG_WARN_BUF(errp, "interface '%s': cannot parse VF MAC '%s'.",
> + netdev_get_name(netdev), vf_mac);
> + } else if (eth_addr_is_multicast(mac)) {
> + VLOG_WARN_BUF(errp,
> + "interface '%s': cannot set VF MAC to multicast "
> + "address '%s'.", netdev_get_name(netdev), vf_mac);
> + } else if (!eth_addr_equals(dev->requested_hwaddr, mac)) {
> + dev->requested_hwaddr = mac;
> + netdev_request_reconfigure(netdev);
> + }
> + }
> +
> 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;
> @@ -3703,6 +3744,11 @@ netdev_dpdk_get_status(const struct netdev *netdev,
> struct smap *args)
> smap_add(args, "link_speed",
> netdev_dpdk_link_speed_to_str__(link_speed));
>
> + if (dpdk_port_is_representor(dev)) {
> + smap_add_format(args, "dpdk-vf-mac", ETH_ADDR_FMT,
> + ETH_ADDR_ARGS(dev->hwaddr));
> + }
> +
> return 0;
> }
>
> @@ -4939,6 +4985,7 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
> && dev->lsc_interrupt_mode == dev->requested_lsc_interrupt_mode
> && dev->rxq_size == dev->requested_rxq_size
> && dev->txq_size == dev->requested_txq_size
> + && eth_addr_equals(dev->hwaddr, dev->requested_hwaddr)
> && dev->socket_id == dev->requested_socket_id
> && dev->started && !dev->reset_needed) {
> /* Reconfiguration is unnecessary */
> @@ -4970,6 +5017,14 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
> dev->txq_size = dev->requested_txq_size;
>
> rte_free(dev->tx_q);
> +
> + if (!eth_addr_equals(dev->hwaddr, dev->requested_hwaddr)) {
> + err = netdev_dpdk_set_etheraddr__(dev, dev->requested_hwaddr);
> + if (err) {
> + goto out;
> + }
> + }
> +
> err = dpdk_eth_dev_init(dev);
> if (dev->hw_ol_features & NETDEV_TX_TSO_OFFLOAD) {
> netdev->ol_flags |= NETDEV_TX_OFFLOAD_TCP_TSO;
> @@ -4981,6 +5036,18 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
> }
> }
>
> + /* If both requested and actual hwaddr were previously
> + * unset (initialized to 0), then first device init above
> + * will have set actual hwaddr to something new.
> + * This would trigger spurious MAC reconfiguration unless
> + * the requested MAC is kept in sync.
> + *
> + * This is harmless in case requested_hwaddr was
> + * configured by the user, as netdev_dpdk_set_etheraddr__()
> + * will have succeeded to get to this point.
> + */
> + dev->requested_hwaddr = dev->hwaddr;
> +
> dev->tx_q = netdev_dpdk_alloc_txq(netdev->n_txq);
> if (!dev->tx_q) {
> err = ENOMEM;
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index d0890b843..89a876796 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -3275,6 +3275,24 @@ 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 MAC address is used:
> + </p>
> + <ul>
> + <li>
> + For most drivers, the default MAC address assigned by their
> + hardware.
> + </li>
> + <li>
> + For bifurcated drivers, the MAC currently used by the kernel
> + netdevice.
> + </li>
> + </ul>
> + <p>This option may only be used with dpdk VF representors.</p>
> + </column>
> </group>
>
> <group title="EMC (Exact Match Cache) Configuration">
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
