On 12/17/2021 12:06 AM, Ilya Maximets wrote:
External email: Use caution opening links or attachments


New 'options:dummy-rte-flow' configuration knob for netdev-dpdk
ports, i.e. type=dpdk/dpdkvhostuserclient, to turn on dummy rte_flow
API.  This version of API will do nothing but reply with success to
every request, except for tunnel restoration info, which needs to
report that there is no relevant information.

Use 'name<number>' format for port names in order to get more
distinguishable port ids for virtual ports that doesn't have them.
DPDK port ids will be equal to 1000 + <number>.

To use virtual ports we can use such DPDK ports (vdev attribute, "af_packet" for example), so there won't be a need for those fake DPDK port numbers.

If we still keep this option, we can simply use DPDK_ETH_PORT_ID_INVALID. What further information does this dummy port-id provide?


Ex.:

   ovs-vsctl add-port ovsbr vhost0 \
       -- set Interface vhost0 type=dpdkvhostuserclient \
                               options:vhost-server-path=/tmp/vhost0 \
                               options:dummy-rte-flow=true

In this example, vhost0 will report port_id = 1000 to the offloading
module, therefore that number will be used in rte_flow flows.
Ports with valid DPDK port ids will keep using them.

It's OK to have a dummy implementation that accepts offloading
requests, but does nothing useful, because all the flows are
additionally installed to normal flow tables in userspace datapath
and all flow dumps are managed directly by the netdev-offload-dpdk
module without looking at what is actually installed in HW.

Since some port IDs are fake, it's probably better to not mix
real and dummy offloading.

This feature is useful for testing since it allows to debug a lot of
aspects of flow offloading and netdev-offload-dpdk implementation
in a fully virtual environment.  E.g. should allow writing some system
tests for netdev-offload-dpdk.
Is this commit supposed to be followed by another commit to show the benefit of it? testing perhaps?

Option works for physical ports too, so can be used for testing with
physical NICs that doesn't support offload, doesn't support all the
features or if the real offloading is undesirable for some reason.

Since the real DPDK ports are used, traffic can be injected at high
rate to multiple PMD threads, so some sort of stress/performance
testing for the higher layers of offloading infrastructure can be done.

Option is for testing only, so not adding any documentation.
Configuration should be done in the same command in which the port is
added, otherwise flow API will not be initialized.  Disabling in
runtime will not have any effect.  Enabling in runtime after the port
is added may result in inability to remove flows previously installed
by the real rte_flow API.

Signed-off-by: Ilya Maximets <[email protected]>
---
  lib/netdev-dpdk.c | 78 +++++++++++++++++++++++++++++++++++++++--------
  1 file changed, 65 insertions(+), 13 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 6782d3e8f..9342b51ca 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -60,6 +60,7 @@
  #include "ovs-rcu.h"
  #include "ovs-thread.h"
  #include "packets.h"
+#include "random.h"
  #include "smap.h"
  #include "sset.h"
  #include "timeval.h"
@@ -519,6 +520,10 @@ struct netdev_dpdk {

          /* VF configuration. */
          struct eth_addr requested_hwaddr;
+
+        /* Dummy flow API. */
+        bool dummy_rte_flow;
+        dpdk_port_t dummy_port_id;
      );

      PADDED_MEMBERS(CACHE_LINE_SIZE,
@@ -1242,6 +1247,9 @@ common_construct(struct netdev *netdev, dpdk_port_t 
port_no,
      dev->started = false;
      dev->reset_needed = false;

+    dev->dummy_rte_flow = false;
+    dev->dummy_port_id = port_no;
+
      ovsrcu_init(&dev->qos_conf, NULL);

      ovsrcu_init(&dev->ingress_policer, NULL);
@@ -1704,6 +1712,11 @@ netdev_dpdk_get_config(const struct netdev *netdev, 
struct smap *args)
      smap_add_format(args, "configured_tx_queues", "%d", netdev->n_txq);
      smap_add_format(args, "mtu", "%d", dev->mtu);

+    if (dev->dummy_rte_flow) {
+        /* It's a debug-only option, not reporting when it is disabled. */
+        smap_add(args, "dummy-rte-flow", "true");
+    }
+
      if (dev->type == DPDK_DEV_ETH) {
          smap_add_format(args, "requested_rxq_descriptors", "%d",
                          dev->requested_rxq_size);
@@ -2008,6 +2021,10 @@ netdev_dpdk_set_config(struct netdev *netdev, const 
struct smap *args,
          netdev_request_reconfigure(netdev);
      }

+    if (!dev->dummy_rte_flow) {
+        dev->dummy_rte_flow = smap_get_bool(args, "dummy-rte-flow", false);
+    }
+
      rx_fc_en = smap_get_bool(args, "rx-flow-ctrl", false);
      tx_fc_en = smap_get_bool(args, "tx-flow-ctrl", false);
      autoneg = smap_get_bool(args, "flow-ctrl-autoneg", false);
@@ -2082,6 +2099,11 @@ netdev_dpdk_vhost_client_set_config(struct netdev 
*netdev,
          VLOG_INFO("Max Tx retries for vhost device '%s' set to %d",
                    netdev_get_name(netdev), max_tx_retries);
      }
+
+    if (!dev->dummy_rte_flow) {
+        dev->dummy_rte_flow = smap_get_bool(args, "dummy-rte-flow", false);
+    }
+
      ovs_mutex_unlock(&dev->mutex);

      return 0;
@@ -5207,8 +5229,29 @@ netdev_dpdk_get_port_id(struct netdev *netdev)
      }

      dev = netdev_dpdk_cast(netdev);
+
      ovs_mutex_lock(&dev->mutex);
      ret = dev->port_id;
+
+    if (dev->dummy_rte_flow && ret == DPDK_ETH_PORT_ID_INVALID) {
+        if (dev->dummy_port_id == DPDK_ETH_PORT_ID_INVALID) {
+            const char *p;
+            int port_no;
+
+            for (p = netdev_get_name(netdev); *p != '\0'; p++) {
+                if (isdigit((unsigned char) *p)) {
+                    port_no = strtol(p, NULL, 10);
+                    if (port_no >= 0) {
+                        dev->dummy_port_id = MAX(1000, RTE_MAX_ETHPORTS + 1)
+                                             + port_no;
+                    }
+                    break;
+                }
+            }
+        }
+        ret = dev->dummy_port_id;
+    }
+
      ovs_mutex_unlock(&dev->mutex);
  out:
      return ret;
@@ -5233,7 +5276,7 @@ netdev_dpdk_flow_api_supported(struct netdev *netdev)

      dev = netdev_dpdk_cast(netdev);
      ovs_mutex_lock(&dev->mutex);
-    if (dev->type == DPDK_DEV_ETH) {
+    if (dev->type == DPDK_DEV_ETH || dev->dummy_rte_flow) {
add a WARN message in case of dummy_rte_flow.
          /* TODO: Check if we able to offload some minimal flow. */
          ret = true;
      }
@@ -5251,7 +5294,8 @@ netdev_dpdk_rte_flow_destroy(struct netdev *netdev,
      int ret;

      ovs_mutex_lock(&dev->mutex);
-    ret = rte_flow_destroy(dev->port_id, rte_flow, error);
+    ret = dev->dummy_rte_flow
+          ? 0 : rte_flow_destroy(dev->port_id, rte_flow, error);
      ovs_mutex_unlock(&dev->mutex);
      return ret;
  }
@@ -5267,7 +5311,9 @@ netdev_dpdk_rte_flow_create(struct netdev *netdev,
      struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);

      ovs_mutex_lock(&dev->mutex);
-    flow = rte_flow_create(dev->port_id, attr, items, actions, error);
+    flow = dev->dummy_rte_flow
+           ? (struct rte_flow *) (uintptr_t) random_uint32()
+           : rte_flow_create(dev->port_id, attr, items, actions, error);
a better predictable IDs can just be a static incremented variable, instead of random.
      ovs_mutex_unlock(&dev->mutex);
      return flow;
  }
@@ -5297,7 +5343,8 @@ netdev_dpdk_rte_flow_query_count(struct netdev *netdev,

      dev = netdev_dpdk_cast(netdev);
      ovs_mutex_lock(&dev->mutex);
-    ret = rte_flow_query(dev->port_id, rte_flow, actions, query, error);
+    ret = dev->dummy_rte_flow
+          ? 0 : rte_flow_query(dev->port_id, rte_flow, actions, query, error);
      ovs_mutex_unlock(&dev->mutex);
      return ret;
  }
@@ -5320,8 +5367,9 @@ netdev_dpdk_rte_flow_tunnel_decap_set(struct netdev 
*netdev,

      dev = netdev_dpdk_cast(netdev);
      ovs_mutex_lock(&dev->mutex);
-    ret = rte_flow_tunnel_decap_set(dev->port_id, tunnel, actions,
-                                    num_of_actions, error);
+    ret = dev->dummy_rte_flow
+          ? 0 : rte_flow_tunnel_decap_set(dev->port_id, tunnel, actions,
+                                          num_of_actions, error);
      ovs_mutex_unlock(&dev->mutex);
      return ret;
  }
@@ -5342,8 +5390,9 @@ netdev_dpdk_rte_flow_tunnel_match(struct netdev *netdev,

      dev = netdev_dpdk_cast(netdev);
      ovs_mutex_lock(&dev->mutex);
-    ret = rte_flow_tunnel_match(dev->port_id, tunnel, items, num_of_items,
-                                error);
+    ret = dev->dummy_rte_flow
+          ? 0 : rte_flow_tunnel_match(dev->port_id, tunnel,
+                                      items, num_of_items, error);
      ovs_mutex_unlock(&dev->mutex);
      return ret;
  }
@@ -5364,7 +5413,8 @@ netdev_dpdk_rte_flow_get_restore_info(struct netdev 
*netdev,

      dev = netdev_dpdk_cast(netdev);
      ovs_mutex_lock(&dev->mutex);
-    ret = rte_flow_get_restore_info(dev->port_id, m, info, error);
+    ret = dev->dummy_rte_flow
+          ? -EINVAL : rte_flow_get_restore_info(dev->port_id, m, info, error);

-ENOTSUP fits better. also this commit can be effective then:

https://mail.openvswitch.org/pipermail/ovs-dev/2021-December/390087.html

      ovs_mutex_unlock(&dev->mutex);
      return ret;
  }
@@ -5385,8 +5435,9 @@ netdev_dpdk_rte_flow_tunnel_action_decap_release(

      dev = netdev_dpdk_cast(netdev);
      ovs_mutex_lock(&dev->mutex);
-    ret = rte_flow_tunnel_action_decap_release(dev->port_id, actions,
-                                               num_of_actions, error);
+    ret = dev->dummy_rte_flow
+          ? 0 : rte_flow_tunnel_action_decap_release(dev->port_id, actions,
+                                                     num_of_actions, error);
      ovs_mutex_unlock(&dev->mutex);
      return ret;
  }
@@ -5406,8 +5457,9 @@ netdev_dpdk_rte_flow_tunnel_item_release(struct netdev 
*netdev,

      dev = netdev_dpdk_cast(netdev);
      ovs_mutex_lock(&dev->mutex);
-    ret = rte_flow_tunnel_item_release(dev->port_id, items, num_of_items,
-                                       error);
+    ret = dev->dummy_rte_flow
+          ? 0 : rte_flow_tunnel_item_release(dev->port_id, items, num_of_items,
+                                             error);
      ovs_mutex_unlock(&dev->mutex);
      return ret;
  }
--
2.31.1

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

Reply via email to