On 2020/07/17 1:14, William Tu wrote:
On Mon, Jun 29, 2020 at 8:30 AM Toshiaki Makita
<[email protected]> wrote:

The following commit will introduce another offload driver using XDP.
When using afxdp netdev, both of TC and XDP will be supported, so let's
add an other_config to specify which offload driver is preferable.
When not specified and multiple offload drivers can be used, TC will be
used if netdev supports it.

Signed-off-by: Toshiaki Makita <[email protected]>
---
  lib/netdev-offload.c | 37 +++++++++++++++++++++++++++++--------
  1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
index ab97a292e..669513646 100644
--- a/lib/netdev-offload.c
+++ b/lib/netdev-offload.c
@@ -60,6 +60,9 @@ VLOG_DEFINE_THIS_MODULE(netdev_offload);

  static bool netdev_flow_api_enabled = false;

+#define FLOW_API_DRIVER_DEFAULT "linux_tc"
+static const char *netdev_flow_api_driver = NULL;
+
  /* Protects 'netdev_flow_apis'.  */
  static struct ovs_mutex netdev_flow_api_provider_mutex = 
OVS_MUTEX_INITIALIZER;

@@ -171,18 +174,30 @@ netdev_flow_api_equals(const struct netdev *netdev1,
  static int
  netdev_assign_flow_api(struct netdev *netdev)
  {
-    struct netdev_registered_flow_api *rfa;
+    struct netdev_registered_flow_api *rfa, *current_rfa = NULL;

      CMAP_FOR_EACH (rfa, cmap_node, &netdev_flow_apis) {
+        if (netdev_flow_api_driver &&
+            strcmp(netdev_flow_api_driver, rfa->flow_api->type)) {
+            continue;
+        }
          if (!rfa->flow_api->init_flow_api(netdev)) {

IIUC, when using the default linux_tc api driver, the XDP offload driver
will also be init, (netdev_xdp_init_flow_api), right??

It's possible.

Maybe there is a better way to handle this?
Do we need to clean up the resources initialized from the
netdev_xdp_int_flow_api?

In patch 2/4, uninit callback does not exist in struct netdev_flow_api
(we cannot call flow_api->uninit_flow_api() at this point).
So I added uninit in patch 3/4.

Excerpt from patch 3/4:

@@ -185,6 +185,9 @@ netdev_assign_flow_api(struct netdev *netdev)
             if (!current_rfa ||
                 (!netdev_flow_api_driver &&
                  !strcmp(FLOW_API_DRIVER_DEFAULT, rfa->flow_api->type))) {
+                if (current_rfa && current_rfa->flow_api->uninit_flow_api) {
+                    current_rfa->flow_api->uninit_flow_api(netdev);
+                }
                 current_rfa = rfa;

I guess this is not intuitive.
Also we can do it in a more smart way: don't initialize other flow apis at all when the default api can be initialized.
I'll think about it.

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

Reply via email to