diff mbox series

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

Message ID 8eef09f268d48863d00d7e573e61ec935b440dc8.1604866011.git.grive@u256.net
State Changes Requested
Headers show
Series netdev-dpdk: support changing VF MAC | expand

Commit Message

Gaetan Rivet Nov. 8, 2020, 8:09 p.m. UTC
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

Issue: 1981388
Change-Id: Iaec052592fe0a66a4a2d8ed34ccafe105423d16a
Signed-off-by: Gaetan Rivet <grive@u256.net>
Suggested-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Eli Britstein <elibr@nvidia.com>
---
 Documentation/topics/dpdk/phy.rst | 53 ++++++++++++++++++++++++
 NEWS                              |  2 +
 lib/netdev-dpdk.c                 | 67 +++++++++++++++++++++++++++++++
 vswitchd/vswitch.xml              | 18 +++++++++
 4 files changed, 140 insertions(+)

Comments

Kevin Traynor Nov. 9, 2020, 4:49 p.m. UTC | #1
On 08/11/2020 20:09, 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
> 
> Issue: 1981388
> Change-Id: Iaec052592fe0a66a4a2d8ed34ccafe105423d16a
> Signed-off-by: Gaetan Rivet <grive@u256.net>
> Suggested-by: Ilya Maximets <i.maximets@ovn.org>
> Acked-by: Eli Britstein <elibr@nvidia.com>
> ---

Thanks for adding to the documentation Gaetan.

Acked-by: Kevin Traynor <ktraynor@redhat.com>
Kevin Traynor Nov. 10, 2020, 9:42 a.m. UTC | #2
On 09/11/2020 16:49, Kevin Traynor wrote:
> On 08/11/2020 20:09, 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
>>
>> Issue: 1981388
>> Change-Id: Iaec052592fe0a66a4a2d8ed34ccafe105423d16a
>> Signed-off-by: Gaetan Rivet <grive@u256.net>
>> Suggested-by: Ilya Maximets <i.maximets@ovn.org>
>> Acked-by: Eli Britstein <elibr@nvidia.com>
>> ---
> 
> Thanks for adding to the documentation Gaetan.
> 
> Acked-by: Kevin Traynor <ktraynor@redhat.com>
> 

Wait. Travis build is failing with clang - I presume the ovs robot will
report this also.
https://travis-ci.org/github/kevuzaj/ovs/builds/742494330

> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Gaetan Rivet Nov. 10, 2020, 10:18 a.m. UTC | #3
On 10/11/20 09:42 +0000, Kevin Traynor wrote:
> On 09/11/2020 16:49, Kevin Traynor wrote:
> > On 08/11/2020 20:09, 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
> >>
> >> Issue: 1981388
> >> Change-Id: Iaec052592fe0a66a4a2d8ed34ccafe105423d16a
> >> Signed-off-by: Gaetan Rivet <grive@u256.net>
> >> Suggested-by: Ilya Maximets <i.maximets@ovn.org>
> >> Acked-by: Eli Britstein <elibr@nvidia.com>
> >> ---
> > 
> > Thanks for adding to the documentation Gaetan.
> > 
> > Acked-by: Kevin Traynor <ktraynor@redhat.com>
> > 
> 
> Wait. Travis build is failing with clang - I presume the ovs robot will
> report this also.
> https://travis-ci.org/github/kevuzaj/ovs/builds/742494330
> 

Ah yes sorry, good catch. I did not retry with clang in the latest
versions. Will fix and resubmit.
diff mbox series

Patch

diff --git a/Documentation/topics/dpdk/phy.rst b/Documentation/topics/dpdk/phy.rst
index 55a98e2b0..3ae259817 100644
--- a/Documentation/topics/dpdk/phy.rst
+++ b/Documentation/topics/dpdk/phy.rst
@@ -379,6 +379,59 @@  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
+
+The requested MAC address is assigned to the port and is listed as part of its options::
+
+    $ ovs-appctl dpctl/show
+    [...]
+      port 3: dpdk-rep0 (dpdk: configured_rx_queues=1, ..., dpdk-vf-mac=00:11:22:33:44:55, ...)
+    [...]
+
+    $ ovs-vsctl show
+    [...]
+            Port dpdk-rep0
+                Interface dpdk-rep0
+                    type: dpdk
+                    options: {dpdk-devargs="<representor devargs>", dpdk-vf-mac="00:11:22:33:44:55"}
+    [...]
+
+    $ ovs-vsctl get Interface dpdk-rep0 status
+    {dpdk-vf-mac="00:11:22:33:44:55", ...}
+
+    $ ovs-vsctl list Interface dpdk-rep0 | grep 'mac_in_use\|options'
+    [...]
+    mac_in_use          : "00:11:22:33:44:55"
+    [...]
+    options             : {dpdk-devargs="<representor devargs>", dpdk-vf-mac="00:11:22:33:44:55"}
+    [...]
+
+The value listed as ``dpdk-vf-mac`` is only a request from the user and is possibly not yet applied.
+
+When the requested configuration is successfully applied to the port, this MAC address
+is then also shown in the column ``mac_in_use`` of the ``Interface`` table.  On failure
+however, ``mac_in_use`` will keep its previous value, which will thus differ from ``dpdk-vf-mac``.
+
 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">