netdev: Dynamic per-port Flow API.

Current issues with Flow API:

* OVS calls offloading functions regardless of successful
  flow API initialization. (ex. on init_flow_api failure)
* Static initilaization of Flow API for a netdev_class forbids
  having different offloading types for different instances
  of netdev with the same netdev_class. (ex. different vports in
  'system' and 'netdev' datapaths at the same time)

Solution:

* Move Flow API from the netdev_class to netdev instance.
* Make Flow API dynamic, i.e. probe the APIs and choose the
  suitable one.

Side effects:

* Flow API providers localized as possible in their modules.
* Now we have an ability to make runtime checks. For example,
  we could check if particular device supports features we
  need, like if dpdk device supports RSS+MARK action.

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Acked-by: Roi Dayan <roid@mellanox.com>

lib/automake.mk

@@ -137,6 +137,7 @@ lib_libopenvswitch_la_SOURCES = \
 	lib/namemap.c \
 	lib/netdev-dpdk.h \
 	lib/netdev-dummy.c \
+	lib/netdev-offload-provider.h \
 	lib/netdev-provider.h \
 	lib/netdev-rte-offloads.h \
 	lib/netdev-vport.c \
@@ -393,7 +394,6 @@ lib_libopenvswitch_la_SOURCES += \
 	lib/netdev-linux.c \
 	lib/netdev-linux.h \
 	lib/netdev-tc-offloads.c \
-	lib/netdev-tc-offloads.h \
 	lib/netlink-conntrack.c \
 	lib/netlink-conntrack.h \
 	lib/netlink-notifier.c \

lib/dpdk.c

@@ -34,6 +34,7 @@
 #include "dirs.h"
 #include "fatal-signal.h"
 #include "netdev-dpdk.h"
+#include "netdev-rte-offloads.h"
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/vlog.h"
 #include "ovs-numa.h"
@@ -454,6 +455,7 @@ dpdk_init__(const struct smap *ovs_other_config)
 
     /* Finally, register the dpdk classes */
     netdev_dpdk_register();
+    netdev_dpdk_flow_api_register();
     return true;
 }
 

lib/netdev-dpdk.c

@@ -4212,6 +4212,27 @@ unlock:
     return err;
 }
 
+bool
+netdev_dpdk_flow_api_supported(struct netdev *netdev)
+{
+    struct netdev_dpdk *dev;
+    bool ret = false;
+
+    if (!is_dpdk_class(netdev->netdev_class)) {
+        goto out;
+    }
+
+    dev = netdev_dpdk_cast(netdev);
+    ovs_mutex_lock(&dev->mutex);
+    if (dev->type == DPDK_DEV_ETH) {
+        /* TODO: Check if we able to offload some minimal flow. */
+        ret = true;
+    }
+    ovs_mutex_unlock(&dev->mutex);
+out:
+    return ret;
+}
+
 int
 netdev_dpdk_rte_flow_destroy(struct netdev *netdev,
                              struct rte_flow *rte_flow,
@@ -4276,8 +4297,7 @@ netdev_dpdk_rte_flow_create(struct netdev *netdev,
     .get_features = netdev_dpdk_get_features,           \
     .get_status = netdev_dpdk_get_status,               \
     .reconfigure = netdev_dpdk_reconfigure,             \
-    .rxq_recv = netdev_dpdk_rxq_recv,                   \
+    .rxq_recv = netdev_dpdk_rxq_recv
-    DPDK_FLOW_OFFLOAD_API
 
 static const struct netdev_class dpdk_class = {
     .type = "dpdk",

lib/netdev-dpdk.h

@@ -34,6 +34,9 @@ struct rte_flow_action;
 
 void netdev_dpdk_register(void);
 void free_dpdk_buf(struct dp_packet *);
+
+bool netdev_dpdk_flow_api_supported(struct netdev *);
+
 int
 netdev_dpdk_rte_flow_destroy(struct netdev *netdev,
                              struct rte_flow *rte_flow,

lib/netdev-dummy.c

@@ -24,6 +24,7 @@
 #include "dp-packet.h"
 #include "dpif-netdev.h"
 #include "flow.h"
+#include "netdev-offload-provider.h"
 #include "netdev-provider.h"
 #include "netdev-vport.h"
 #include "odp-util.h"
@@ -1523,10 +1524,6 @@ exit:
     return error ? -1 : 0;
 }
 
-#define DUMMY_FLOW_OFFLOAD_API                          \
-    .flow_put = netdev_dummy_flow_put,                  \
-    .flow_del = netdev_dummy_flow_del
-
 #define NETDEV_DUMMY_CLASS_COMMON                       \
     .run = netdev_dummy_run,                            \
     .wait = netdev_dummy_wait,                          \
@@ -1559,8 +1556,7 @@ exit:
     .rxq_dealloc = netdev_dummy_rxq_dealloc,            \
     .rxq_recv = netdev_dummy_rxq_recv,                  \
     .rxq_wait = netdev_dummy_rxq_wait,                  \
-    .rxq_drain = netdev_dummy_rxq_drain,                \
+    .rxq_drain = netdev_dummy_rxq_drain
-    DUMMY_FLOW_OFFLOAD_API
 
 static const struct netdev_class dummy_class = {
     NETDEV_DUMMY_CLASS_COMMON,
@@ -1578,6 +1574,20 @@ static const struct netdev_class dummy_pmd_class = {
     .is_pmd = true,
     .reconfigure = netdev_dummy_reconfigure
 };
+
+static int
+netdev_dummy_offloads_init_flow_api(struct netdev *netdev)
+{
+    return is_dummy_class(netdev->netdev_class) ? 0 : EOPNOTSUPP;
+}
+
+static const struct netdev_flow_api netdev_dummy_offloads = {
+    .type = "dummy",
+    .flow_put = netdev_dummy_flow_put,
+    .flow_del = netdev_dummy_flow_del,
+    .init_flow_api = netdev_dummy_offloads_init_flow_api,
+};
+
 
 /* Helper functions. */
 
@@ -2024,5 +2034,7 @@ netdev_dummy_register(enum dummy_level level)
     netdev_register_provider(&dummy_internal_class);
     netdev_register_provider(&dummy_pmd_class);
 
+    netdev_register_flow_api_provider(&netdev_dummy_offloads);
+
     netdev_vport_tunnel_register();
 }

lib/netdev-linux.c

@@ -55,7 +55,6 @@
 #include "hash.h"
 #include "openvswitch/hmap.h"
 #include "netdev-provider.h"
-#include "netdev-tc-offloads.h"
 #include "netdev-vport.h"
 #include "netlink-notifier.h"
 #include "netlink-socket.h"
@@ -3321,7 +3320,6 @@ exit:
 
 const struct netdev_class netdev_linux_class = {
     NETDEV_LINUX_CLASS_COMMON,
-    LINUX_FLOW_OFFLOAD_API,
     .type = "system",
     .construct = netdev_linux_construct,
     .get_stats = netdev_linux_get_stats,
@@ -3341,7 +3339,6 @@ const struct netdev_class netdev_tap_class = {
 
 const struct netdev_class netdev_internal_class = {
     NETDEV_LINUX_CLASS_COMMON,
-    LINUX_FLOW_OFFLOAD_API,
     .type = "internal",
     .construct = netdev_linux_construct,
     .get_stats = netdev_internal_get_stats,

lib/netdev-linux.h

@@ -29,14 +29,4 @@ int netdev_linux_ethtool_set_flag(struct netdev *netdev, uint32_t flag,
                                   const char *flag_name, bool enable);
 int linux_get_ifindex(const char *netdev_name);
 
-#define LINUX_FLOW_OFFLOAD_API                          \
-   .flow_flush = netdev_tc_flow_flush,                  \
-   .flow_dump_create = netdev_tc_flow_dump_create,      \
-   .flow_dump_destroy = netdev_tc_flow_dump_destroy,    \
-   .flow_dump_next = netdev_tc_flow_dump_next,          \
-   .flow_put = netdev_tc_flow_put,                      \
-   .flow_get = netdev_tc_flow_get,                      \
-   .flow_del = netdev_tc_flow_del,                      \
-   .init_flow_api = netdev_tc_init_flow_api
-
 #endif /* netdev-linux.h */

lib/netdev-offload-provider.h

@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2016 Nicira, Inc.
+ * Copyright (c) 2019 Samsung Electronics Co.,Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef NETDEV_FLOW_API_PROVIDER_H
+#define NETDEV_FLOW_API_PROVIDER_H 1
+
+#include "flow.h"
+#include "openvswitch/types.h"
+#include "packets.h"
+
+#ifdef  __cplusplus
+extern "C" {
+#endif
+
+struct netdev_flow_api {
+    char *type;
+    /* Flush all offloaded flows from a netdev.
+     * Return 0 if successful, otherwise returns a positive errno value. */
+    int (*flow_flush)(struct netdev *);
+
+    /* Flow dumping interface.
+     *
+     * This is the back-end for the flow dumping interface described in
+     * dpif.h.  Please read the comments there first, because this code
+     * closely follows it.
+     *
+     * On success returns 0 and allocates data, on failure returns
+     * positive errno. */
+    int (*flow_dump_create)(struct netdev *, struct netdev_flow_dump **dump);
+    int (*flow_dump_destroy)(struct netdev_flow_dump *);
+
+    /* Returns true if there are more flows to dump.
+     * 'rbuffer' is used as a temporary buffer and needs to be pre allocated
+     * by the caller.  While there are more flows the same 'rbuffer'
+     * should be provided. 'wbuffer' is used to store dumped actions and needs
+     * to be pre allocated by the caller. */
+    bool (*flow_dump_next)(struct netdev_flow_dump *, struct match *,
+                           struct nlattr **actions,
+                           struct dpif_flow_stats *stats,
+                           struct dpif_flow_attrs *attrs, ovs_u128 *ufid,
+                           struct ofpbuf *rbuffer, struct ofpbuf *wbuffer);
+
+    /* Offload the given flow on netdev.
+     * To modify a flow, use the same ufid.
+     * 'actions' are in netlink format, as with struct dpif_flow_put.
+     * 'info' is extra info needed to offload the flow.
+     * 'stats' is populated according to the rules set out in the description
+     * above 'struct dpif_flow_put'.
+     * Return 0 if successful, otherwise returns a positive errno value. */
+    int (*flow_put)(struct netdev *, struct match *, struct nlattr *actions,
+                    size_t actions_len, const ovs_u128 *ufid,
+                    struct offload_info *info, struct dpif_flow_stats *);
+
+    /* Queries a flow specified by ufid on netdev.
+     * Fills output buffer as 'wbuffer' in flow_dump_next, which
+     * needs to be be pre allocated.
+     * Return 0 if successful, otherwise returns a positive errno value. */
+    int (*flow_get)(struct netdev *, struct match *, struct nlattr **actions,
+                    const ovs_u128 *ufid, struct dpif_flow_stats *,
+                    struct dpif_flow_attrs *, struct ofpbuf *wbuffer);
+
+    /* Delete a flow specified by ufid from netdev.
+     * 'stats' is populated according to the rules set out in the description
+     * above 'struct dpif_flow_del'.
+     * Return 0 if successful, otherwise returns a positive errno value. */
+    int (*flow_del)(struct netdev *, const ovs_u128 *ufid,
+                    struct dpif_flow_stats *);
+
+    /* Initializies the netdev flow api.
+     * Return 0 if successful, otherwise returns a positive errno value. */
+    int (*init_flow_api)(struct netdev *);
+};
+
+int netdev_register_flow_api_provider(const struct netdev_flow_api *);
+int netdev_unregister_flow_api_provider(const char *type);
+
+#ifdef __linux__
+extern const struct netdev_flow_api netdev_tc_offloads;
+#endif
+
+#ifdef  __cplusplus
+}
+#endif
+
+#endif /* NETDEV_FLOW_API_PROVIDER_H */

lib/netdev-provider.h

@@ -23,6 +23,7 @@
 #include "netdev.h"
 #include "openvswitch/list.h"
 #include "ovs-numa.h"
+#include "ovs-rcu.h"
 #include "packets.h"
 #include "seq.h"
 #include "openvswitch/shash.h"
@@ -93,6 +94,9 @@ struct netdev {
     int n_rxq;
     struct shash_node *node;            /* Pointer to element in global map. */
     struct ovs_list saved_flags_list; /* Contains "struct netdev_saved_flags". */
+
+    /* Functions to control flow offloading. */
+    OVSRCU_TYPE(const struct netdev_flow_api *) flow_api;
     struct netdev_hw_info hw_info;	/* offload-capable netdev info */
 };
 
@@ -822,69 +826,6 @@ struct netdev_class {
     /* Discards all packets waiting to be received from 'rx'. */
     int (*rxq_drain)(struct netdev_rxq *rx);
 
-    /* ## -------------------------------- ## */
-    /* ## netdev flow offloading functions ## */
-    /* ## -------------------------------- ## */
-
-    /* If a particular netdev class does not support offloading flows,
-     * all these function pointers must be NULL. */
-
-    /* Flush all offloaded flows from a netdev.
-     * Return 0 if successful, otherwise returns a positive errno value. */
-    int (*flow_flush)(struct netdev *);
-
-    /* Flow dumping interface.
-     *
-     * This is the back-end for the flow dumping interface described in
-     * dpif.h.  Please read the comments there first, because this code
-     * closely follows it.
-     *
-     * On success returns 0 and allocates data, on failure returns
-     * positive errno. */
-    int (*flow_dump_create)(struct netdev *, struct netdev_flow_dump **dump);
-    int (*flow_dump_destroy)(struct netdev_flow_dump *);
-
-    /* Returns true if there are more flows to dump.
-     * 'rbuffer' is used as a temporary buffer and needs to be pre allocated
-     * by the caller.  While there are more flows the same 'rbuffer'
-     * should be provided. 'wbuffer' is used to store dumped actions and needs
-     * to be pre allocated by the caller. */
-    bool (*flow_dump_next)(struct netdev_flow_dump *, struct match *,
-                           struct nlattr **actions,
-                           struct dpif_flow_stats *stats,
-                           struct dpif_flow_attrs *attrs, ovs_u128 *ufid,
-                           struct ofpbuf *rbuffer, struct ofpbuf *wbuffer);
-
-    /* Offload the given flow on netdev.
-     * To modify a flow, use the same ufid.
-     * 'actions' are in netlink format, as with struct dpif_flow_put.
-     * 'info' is extra info needed to offload the flow.
-     * 'stats' is populated according to the rules set out in the description
-     * above 'struct dpif_flow_put'.
-     * Return 0 if successful, otherwise returns a positive errno value. */
-    int (*flow_put)(struct netdev *, struct match *, struct nlattr *actions,
-                    size_t actions_len, const ovs_u128 *ufid,
-                    struct offload_info *info, struct dpif_flow_stats *);
-
-    /* Queries a flow specified by ufid on netdev.
-     * Fills output buffer as 'wbuffer' in flow_dump_next, which
-     * needs to be be pre allocated.
-     * Return 0 if successful, otherwise returns a positive errno value. */
-    int (*flow_get)(struct netdev *, struct match *, struct nlattr **actions,
-                    const ovs_u128 *ufid, struct dpif_flow_stats *,
-                    struct dpif_flow_attrs *, struct ofpbuf *wbuffer);
-
-    /* Delete a flow specified by ufid from netdev.
-     * 'stats' is populated according to the rules set out in the description
-     * above 'struct dpif_flow_del'.
-     * Return 0 if successful, otherwise returns a positive errno value. */
-    int (*flow_del)(struct netdev *, const ovs_u128 *ufid,
-                    struct dpif_flow_stats *);
-
-    /* Initializies the netdev flow api.
-     * Return 0 if successful, otherwise returns a positive errno value. */
-    int (*init_flow_api)(struct netdev *);
-
     /* Get a block_id from the netdev.
      * Returns the block_id or 0 if none exists for netdev. */
     uint32_t (*get_block_id)(struct netdev *);

lib/netdev-rte-offloads.c

@@ -21,6 +21,7 @@
 
 #include "cmap.h"
 #include "dpif-netdev.h"
+#include "netdev-offload-provider.h"
 #include "netdev-provider.h"
 #include "openvswitch/match.h"
 #include "openvswitch/vlog.h"
@@ -29,6 +30,23 @@
 
 VLOG_DEFINE_THIS_MODULE(netdev_rte_offloads);
 
+/* Thread-safety
+ * =============
+ *
+ * Below API is NOT thread safe in following terms:
+ *
+ *  - The caller must be sure that none of these functions will be called
+ *    simultaneously.  Even for different 'netdev's.
+ *
+ *  - The caller must be sure that 'netdev' will not be destructed/deallocated.
+ *
+ *  - The caller must be sure that 'netdev' configuration will not be changed.
+ *    For example, simultaneous call of 'netdev_reconfigure()' for the same
+ *    'netdev' is forbidden.
+ *
+ * For current implementation all above restrictions could be fulfilled by
+ * taking the datapath 'port_mutex' in lib/dpif-netdev.c.  */
+
 /*
  * A mapping from ufid to dpdk rte_flow.
  */
@@ -689,7 +707,7 @@ netdev_rte_offloads_destroy_flow(struct netdev *netdev,
     return ret;
 }
 
-int
+static int
 netdev_rte_offloads_flow_put(struct netdev *netdev, struct match *match,
                              struct nlattr *actions, size_t actions_len,
                              const ovs_u128 *ufid, struct offload_info *info,
@@ -719,7 +737,7 @@ netdev_rte_offloads_flow_put(struct netdev *netdev, struct match *match,
                                         actions_len, ufid, info);
 }
 
-int
+static int
 netdev_rte_offloads_flow_del(struct netdev *netdev, const ovs_u128 *ufid,
                              struct dpif_flow_stats *stats OVS_UNUSED)
 {
@@ -731,3 +749,21 @@ netdev_rte_offloads_flow_del(struct netdev *netdev, const ovs_u128 *ufid,
 
     return netdev_rte_offloads_destroy_flow(netdev, ufid, rte_flow);
 }
+
+static int
+netdev_rte_offloads_init_flow_api(struct netdev *netdev)
+{
+    return netdev_dpdk_flow_api_supported(netdev) ? 0 : EOPNOTSUPP;
+}
+
+static const struct netdev_flow_api netdev_dpdk_offloads = {
+    .type = "dpdk_flow_api",
+    .flow_put = netdev_rte_offloads_flow_put,
+    .flow_del = netdev_rte_offloads_flow_del,
+    .init_flow_api = netdev_rte_offloads_init_flow_api,
+};
+
+void netdev_dpdk_flow_api_register(void)
+{
+    netdev_register_flow_api_provider(&netdev_dpdk_offloads);
+}

lib/netdev-rte-offloads.h

@@ -14,44 +14,9 @@
  * limitations under the License.
  */
 
-#ifndef NETDEV_VPORT_OFFLOADS_H
+#ifndef NETDEV_DPDK_OFFLOADS_H
-#define NETDEV_VPORT_OFFLOADS_H 1
+#define NETDEV_DPDK_OFFLOADS_H 1
 
-#include "openvswitch/types.h"
+void netdev_dpdk_flow_api_register(void);
-
-struct netdev;
-struct match;
-struct nlattr;
-struct offload_info;
-struct dpif_flow_stats;
-
-/* Thread-safety
- * =============
- *
- * Below API is NOT thread safe in following terms:
- *
- *  - The caller must be sure that none of these functions will be called
- *    simultaneously.  Even for different 'netdev's.
- *
- *  - The caller must be sure that 'netdev' will not be destructed/deallocated.
- *
- *  - The caller must be sure that 'netdev' configuration will not be changed.
- *    For example, simultaneous call of 'netdev_reconfigure()' for the same
- *    'netdev' is forbidden.
- *
- * For current implementation all above restrictions could be fulfilled by
- * taking the datapath 'port_mutex' in lib/dpif-netdev.c.  */
-
-int netdev_rte_offloads_flow_put(struct netdev *netdev, struct match *match,
-                                 struct nlattr *actions, size_t actions_len,
-                                 const ovs_u128 *ufid,
-                                 struct offload_info *info,
-                                 struct dpif_flow_stats *stats);
-int netdev_rte_offloads_flow_del(struct netdev *netdev, const ovs_u128 *ufid,
-                                 struct dpif_flow_stats *stats);
-
-#define DPDK_FLOW_OFFLOAD_API                   \
-    .flow_put = netdev_rte_offloads_flow_put,   \
-    .flow_del = netdev_rte_offloads_flow_del
 
 #endif /* netdev-rte-offloads.h */

lib/netdev-tc-offloads.c

@@ -16,7 +16,6 @@
  */
 
 #include <config.h>
-#include "netdev-tc-offloads.h"
 
 #include <errno.h>
 #include <linux/if_ether.h>
@@ -31,6 +30,8 @@
 #include "openvswitch/util.h"
 #include "openvswitch/vlog.h"
 #include "netdev-linux.h"
+#include "netdev-offload-provider.h"
+#include "netdev-provider.h"
 #include "netlink.h"
 #include "netlink-socket.h"
 #include "odp-netlink.h"
@@ -350,7 +351,7 @@ get_block_id_from_netdev(struct netdev *netdev)
     return 0;
 }
 
-int
+static int
 netdev_tc_flow_flush(struct netdev *netdev)
 {
     enum tc_qdisc_hook hook = get_tc_qdisc_hook(netdev);
@@ -368,7 +369,7 @@ netdev_tc_flow_flush(struct netdev *netdev)
     return tc_flush(ifindex, block_id, hook);
 }
 
-int
+static int
 netdev_tc_flow_dump_create(struct netdev *netdev,
                            struct netdev_flow_dump **dump_out)
 {
@@ -395,7 +396,7 @@ netdev_tc_flow_dump_create(struct netdev *netdev,
     return 0;
 }
 
-int
+static int
 netdev_tc_flow_dump_destroy(struct netdev_flow_dump *dump)
 {
     nl_dump_done(dump->nl_dump);
@@ -729,7 +730,7 @@ parse_tc_flower_to_match(struct tc_flower *flower,
     return 0;
 }
 
-bool
+static bool
 netdev_tc_flow_dump_next(struct netdev_flow_dump *dump,
                          struct match *match,
                          struct nlattr **actions,
@@ -1082,7 +1083,7 @@ flower_match_to_tun_opt(struct tc_flower *flower, const struct flow_tnl *tnl,
     flower->mask.tunnel.metadata.present.len = tnl->metadata.present.len;
 }
 
-int
+static int
 netdev_tc_flow_put(struct netdev *netdev, struct match *match,
                    struct nlattr *actions, size_t actions_len,
                    const ovs_u128 *ufid, struct offload_info *info,
@@ -1379,7 +1380,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
     return err;
 }
 
-int
+static int
 netdev_tc_flow_get(struct netdev *netdev OVS_UNUSED,
                    struct match *match,
                    struct nlattr **actions,
@@ -1434,7 +1435,7 @@ netdev_tc_flow_get(struct netdev *netdev OVS_UNUSED,
     return 0;
 }
 
-int
+static int
 netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED,
                    const ovs_u128 *ufid,
                    struct dpif_flow_stats *stats)
@@ -1554,7 +1555,7 @@ probe_tc_block_support(int ifindex)
     }
 }
 
-int
+static int
 netdev_tc_init_flow_api(struct netdev *netdev)
 {
     static struct ovsthread_once multi_mask_once = OVSTHREAD_ONCE_INITIALIZER;
@@ -1566,8 +1567,8 @@ netdev_tc_init_flow_api(struct netdev *netdev)
 
     ifindex = netdev_get_ifindex(netdev);
     if (ifindex < 0) {
-        VLOG_ERR_RL(&error_rl, "init: failed to get ifindex for %s: %s",
+        VLOG_INFO("init: failed to get ifindex for %s: %s",
-                    netdev_get_name(netdev), ovs_strerror(-ifindex));
+                  netdev_get_name(netdev), ovs_strerror(-ifindex));
         return -ifindex;
     }
 
@@ -1588,8 +1589,8 @@ netdev_tc_init_flow_api(struct netdev *netdev)
     error = tc_add_del_qdisc(ifindex, true, block_id, hook);
 
     if (error && error != EEXIST) {
-        VLOG_ERR("failed adding ingress qdisc required for offloading: %s",
+        VLOG_INFO("failed adding ingress qdisc required for offloading: %s",
-                 ovs_strerror(error));
+                  ovs_strerror(error));
         return error;
     }
 
@@ -1597,3 +1598,15 @@ netdev_tc_init_flow_api(struct netdev *netdev)
 
     return 0;
 }
+
+const struct netdev_flow_api netdev_tc_offloads = {
+   .type = "linux_tc",
+   .flow_flush = netdev_tc_flow_flush,
+   .flow_dump_create = netdev_tc_flow_dump_create,
+   .flow_dump_destroy = netdev_tc_flow_dump_destroy,
+   .flow_dump_next = netdev_tc_flow_dump_next,
+   .flow_put = netdev_tc_flow_put,
+   .flow_get = netdev_tc_flow_get,
+   .flow_del = netdev_tc_flow_del,
+   .init_flow_api = netdev_tc_init_flow_api,
+};

lib/netdev-tc-offloads.h

@@ -1,44 +0,0 @@
-/*
- * Copyright (c) 2016 Mellanox Technologies, Ltd.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at:
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#ifndef NETDEV_TC_OFFLOADS_H
-#define NETDEV_TC_OFFLOADS_H 1
-
-#include "netdev-provider.h"
-
-int netdev_tc_flow_flush(struct netdev *);
-int netdev_tc_flow_dump_create(struct netdev *, struct netdev_flow_dump **);
-int netdev_tc_flow_dump_destroy(struct netdev_flow_dump *);
-bool netdev_tc_flow_dump_next(struct netdev_flow_dump *, struct match *,
-                              struct nlattr **actions,
-                              struct dpif_flow_stats *,
-                              struct dpif_flow_attrs *,
-                              ovs_u128 *ufid,
-                              struct ofpbuf *rbuffer,
-                              struct ofpbuf *wbuffer);
-int netdev_tc_flow_put(struct netdev *, struct match *,
-                       struct nlattr *actions, size_t actions_len,
-                       const ovs_u128 *, struct offload_info *,
-                       struct dpif_flow_stats *);
-int netdev_tc_flow_get(struct netdev *, struct match *,
-                       struct nlattr **actions, const ovs_u128 *,
-                       struct dpif_flow_stats *,
-                       struct dpif_flow_attrs *, struct ofpbuf *);
-int netdev_tc_flow_del(struct netdev *, const ovs_u128 *,
-                        struct dpif_flow_stats *);
-int netdev_tc_init_flow_api(struct netdev *);
-
-#endif /* netdev-tc-offloads.h */

lib/netdev-vport.c

@@ -47,7 +47,6 @@
 #include "unaligned.h"
 #include "unixctl.h"
 #include "openvswitch/vlog.h"
-#include "netdev-tc-offloads.h"
 #ifdef __linux__
 #include "netdev-linux.h"
 #endif
@@ -1116,10 +1115,8 @@ netdev_vport_get_ifindex(const struct netdev *netdev_)
 }
 
 #define NETDEV_VPORT_GET_IFINDEX netdev_vport_get_ifindex
-#define NETDEV_FLOW_OFFLOAD_API , LINUX_FLOW_OFFLOAD_API
 #else /* !__linux__ */
 #define NETDEV_VPORT_GET_IFINDEX NULL
-#define NETDEV_FLOW_OFFLOAD_API
 #endif /* __linux__ */
 
 #define VPORT_FUNCTIONS_COMMON                      \
@@ -1133,8 +1130,7 @@ netdev_vport_get_ifindex(const struct netdev *netdev_)
     .get_etheraddr = netdev_vport_get_etheraddr,    \
     .get_stats = netdev_vport_get_stats,            \
     .get_pt_mode = netdev_vport_get_pt_mode,        \
-    .update_flags = netdev_vport_update_flags       \
+    .update_flags = netdev_vport_update_flags
-    NETDEV_FLOW_OFFLOAD_API
 
 #define TUNNEL_FUNCTIONS_COMMON                     \
     VPORT_FUNCTIONS_COMMON,                         \

lib/netdev.c

@@ -39,6 +39,7 @@
 #include "fatal-signal.h"
 #include "hash.h"
 #include "openvswitch/list.h"
+#include "netdev-offload-provider.h"
 #include "netdev-provider.h"
 #include "netdev-vport.h"
 #include "odp-netlink.h"
@@ -98,6 +99,22 @@ struct netdev_registered_class {
 
 static bool netdev_flow_api_enabled = false;
 
+/* Protects 'netdev_flow_apis'.  */
+static struct ovs_mutex netdev_flow_api_provider_mutex = OVS_MUTEX_INITIALIZER;
+
+/* Contains 'struct netdev_registered_flow_api's. */
+static struct cmap netdev_flow_apis = CMAP_INITIALIZER;
+
+struct netdev_registered_flow_api {
+    struct cmap_node cmap_node; /* In 'netdev_flow_apis', by flow_api->type. */
+    const struct netdev_flow_api *flow_api;
+
+    /* Number of references: one for the flow_api itself and one for every
+     * instance of the netdev that uses it. */
+    struct ovs_refcount refcnt;
+};
+
+
 /* This is set pretty low because we probably won't learn anything from the
  * additional log messages. */
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
@@ -146,6 +163,8 @@ netdev_initialize(void)
         netdev_register_provider(&netdev_internal_class);
         netdev_register_provider(&netdev_tap_class);
         netdev_vport_tunnel_register();
+
+        netdev_register_flow_api_provider(&netdev_tc_offloads);
 #endif
 #if defined(__FreeBSD__) || defined(__NetBSD__)
         netdev_register_provider(&netdev_tap_class);
@@ -279,6 +298,87 @@ netdev_unregister_provider(const char *type)
     return error;
 }
 
+static struct netdev_registered_flow_api *
+netdev_lookup_flow_api(const char *type)
+{
+    struct netdev_registered_flow_api *rfa;
+    CMAP_FOR_EACH_WITH_HASH (rfa, cmap_node, hash_string(type, 0),
+                             &netdev_flow_apis) {
+        if (!strcmp(type, rfa->flow_api->type)) {
+            return rfa;
+        }
+    }
+    return NULL;
+}
+
+/* Registers a new netdev flow api provider. */
+int
+netdev_register_flow_api_provider(const struct netdev_flow_api *new_flow_api)
+    OVS_EXCLUDED(netdev_flow_api_provider_mutex)
+{
+    int error = 0;
+
+    if (!new_flow_api->init_flow_api) {
+        VLOG_WARN("attempted to register invalid flow api provider: %s",
+                   new_flow_api->type);
+        error = EINVAL;
+    }
+
+    ovs_mutex_lock(&netdev_flow_api_provider_mutex);
+    if (netdev_lookup_flow_api(new_flow_api->type)) {
+        VLOG_WARN("attempted to register duplicate flow api provider: %s",
+                   new_flow_api->type);
+        error = EEXIST;
+    } else {
+        struct netdev_registered_flow_api *rfa;
+
+        rfa = xmalloc(sizeof *rfa);
+        cmap_insert(&netdev_flow_apis, &rfa->cmap_node,
+                    hash_string(new_flow_api->type, 0));
+        rfa->flow_api = new_flow_api;
+        ovs_refcount_init(&rfa->refcnt);
+        VLOG_DBG("netdev: flow API '%s' registered.", new_flow_api->type);
+    }
+    ovs_mutex_unlock(&netdev_flow_api_provider_mutex);
+
+    return error;
+}
+
+/* Unregisters a netdev flow api provider.  'type' must have been previously
+ * registered and not currently be in use by any netdevs.  After unregistration
+ * netdev flow api of that type cannot be used for netdevs.  (However, the
+ * provider may still be accessible from other threads until the next RCU grace
+ * period, so the caller must not free or re-register the same netdev_flow_api
+ * until that has passed.) */
+int
+netdev_unregister_flow_api_provider(const char *type)
+    OVS_EXCLUDED(netdev_flow_api_provider_mutex)
+{
+    struct netdev_registered_flow_api *rfa;
+    int error;
+
+    ovs_mutex_lock(&netdev_flow_api_provider_mutex);
+    rfa = netdev_lookup_flow_api(type);
+    if (!rfa) {
+        VLOG_WARN("attempted to unregister a flow api provider that is not "
+                  "registered: %s", type);
+        error = EAFNOSUPPORT;
+    } else if (ovs_refcount_unref(&rfa->refcnt) != 1) {
+        ovs_refcount_ref(&rfa->refcnt);
+        VLOG_WARN("attempted to unregister in use flow api provider: %s",
+                  type);
+        error = EBUSY;
+    } else  {
+        cmap_remove(&netdev_flow_apis, &rfa->cmap_node,
+                    hash_string(rfa->flow_api->type, 0));
+        ovsrcu_postpone(free, rfa);
+        error = 0;
+    }
+    ovs_mutex_unlock(&netdev_flow_api_provider_mutex);
+
+    return error;
+}
+
 /* Clears 'types' and enumerates the types of all currently registered netdev
  * providers into it.  The caller must first initialize the sset. */
 void
@@ -414,6 +514,7 @@ netdev_open(const char *name, const char *type, struct netdev **netdevp)
                 netdev->reconfigure_seq = seq_create();
                 netdev->last_reconfigure_seq =
                     seq_read(netdev->reconfigure_seq);
+                ovsrcu_set(&netdev->flow_api, NULL);
                 netdev->hw_info.oor = false;
                 netdev->node = shash_add(&netdev_shash, name, netdev);
 
@@ -562,6 +663,8 @@ netdev_unref(struct netdev *dev)
     ovs_assert(dev->ref_cnt);
     if (!--dev->ref_cnt) {
         const struct netdev_class *class = dev->netdev_class;
+        const struct netdev_flow_api *flow_api =
+            ovsrcu_get(const struct netdev_flow_api *, &dev->flow_api);
         struct netdev_registered_class *rc;
 
         dev->netdev_class->destruct(dev);
@@ -576,6 +679,12 @@ netdev_unref(struct netdev *dev)
 
         rc = netdev_lookup_class(class->type);
         ovs_refcount_unref(&rc->refcnt);
+
+        if (flow_api) {
+            struct netdev_registered_flow_api *rfa =
+                                    netdev_lookup_flow_api(flow_api->type);
+            ovs_refcount_unref(&rfa->refcnt);
+        }
     } else {
         ovs_mutex_unlock(&netdev_mutex);
     }
@@ -2148,34 +2257,58 @@ netdev_reconfigure(struct netdev *netdev)
             : EOPNOTSUPP);
 }
 
+static int
+netdev_assign_flow_api(struct netdev *netdev)
+{
+    struct netdev_registered_flow_api *rfa;
+
+    CMAP_FOR_EACH (rfa, cmap_node, &netdev_flow_apis) {
+        if (!rfa->flow_api->init_flow_api(netdev)) {
+            ovs_refcount_ref(&rfa->refcnt);
+            ovsrcu_set(&netdev->flow_api, rfa->flow_api);
+            VLOG_INFO("%s: Assigned flow API '%s'.",
+                      netdev_get_name(netdev), rfa->flow_api->type);
+            return 0;
+        }
+        VLOG_DBG("%s: flow API '%s' is not suitable.",
+                 netdev_get_name(netdev), rfa->flow_api->type);
+    }
+    VLOG_INFO("%s: No suitable flow API found.", netdev_get_name(netdev));
+
+    return -1;
+}
+
 int
 netdev_flow_flush(struct netdev *netdev)
 {
-    const struct netdev_class *class = netdev->netdev_class;
+    const struct netdev_flow_api *flow_api =
+        ovsrcu_get(const struct netdev_flow_api *, &netdev->flow_api);
 
-    return (class->flow_flush
+    return (flow_api && flow_api->flow_flush)
-            ? class->flow_flush(netdev)
+           ? flow_api->flow_flush(netdev)
-            : EOPNOTSUPP);
+           : EOPNOTSUPP;
 }
 
 int
 netdev_flow_dump_create(struct netdev *netdev, struct netdev_flow_dump **dump)
 {
-    const struct netdev_class *class = netdev->netdev_class;
+    const struct netdev_flow_api *flow_api =
+        ovsrcu_get(const struct netdev_flow_api *, &netdev->flow_api);
 
-    return (class->flow_dump_create
+    return (flow_api && flow_api->flow_dump_create)
-            ? class->flow_dump_create(netdev, dump)
+           ? flow_api->flow_dump_create(netdev, dump)
-            : EOPNOTSUPP);
+           : EOPNOTSUPP;
 }
 
 int
 netdev_flow_dump_destroy(struct netdev_flow_dump *dump)
 {
-    const struct netdev_class *class = dump->netdev->netdev_class;
+    const struct netdev_flow_api *flow_api =
+        ovsrcu_get(const struct netdev_flow_api *, &dump->netdev->flow_api);
 
-    return (class->flow_dump_destroy
+    return (flow_api && flow_api->flow_dump_destroy)
-            ? class->flow_dump_destroy(dump)
+           ? flow_api->flow_dump_destroy(dump)
-            : EOPNOTSUPP);
+           : EOPNOTSUPP;
 }
 
 bool
@@ -2184,12 +2317,13 @@ netdev_flow_dump_next(struct netdev_flow_dump *dump, struct match *match,
                       struct dpif_flow_attrs *attrs, ovs_u128 *ufid,
                       struct ofpbuf *rbuffer, struct ofpbuf *wbuffer)
 {
-    const struct netdev_class *class = dump->netdev->netdev_class;
+    const struct netdev_flow_api *flow_api =
+        ovsrcu_get(const struct netdev_flow_api *, &dump->netdev->flow_api);
 
-    return (class->flow_dump_next
+    return (flow_api && flow_api->flow_dump_next)
-            ? class->flow_dump_next(dump, match, actions, stats, attrs,
+           ? flow_api->flow_dump_next(dump, match, actions, stats, attrs,
-                                    ufid, rbuffer, wbuffer)
+                                      ufid, rbuffer, wbuffer)
-            : false);
+           : false;
 }
 
 int
@@ -2198,12 +2332,13 @@ netdev_flow_put(struct netdev *netdev, struct match *match,
                 const ovs_u128 *ufid, struct offload_info *info,
                 struct dpif_flow_stats *stats)
 {
-    const struct netdev_class *class = netdev->netdev_class;
+    const struct netdev_flow_api *flow_api =
+        ovsrcu_get(const struct netdev_flow_api *, &netdev->flow_api);
 
-    return (class->flow_put
+    return (flow_api && flow_api->flow_put)
-            ? class->flow_put(netdev, match, actions, act_len, ufid,
+           ? flow_api->flow_put(netdev, match, actions, act_len, ufid,
-                              info, stats)
+                                info, stats)
-            : EOPNOTSUPP);
+           : EOPNOTSUPP;
 }
 
 int
@@ -2212,36 +2347,43 @@ netdev_flow_get(struct netdev *netdev, struct match *match,
                 struct dpif_flow_stats *stats,
                 struct dpif_flow_attrs *attrs, struct ofpbuf *buf)
 {
-    const struct netdev_class *class = netdev->netdev_class;
+    const struct netdev_flow_api *flow_api =
+        ovsrcu_get(const struct netdev_flow_api *, &netdev->flow_api);
 
-    return (class->flow_get
+    return (flow_api && flow_api->flow_get)
-            ? class->flow_get(netdev, match, actions, ufid, stats, attrs, buf)
+           ? flow_api->flow_get(netdev, match, actions, ufid,
-            : EOPNOTSUPP);
+                                stats, attrs, buf)
+           : EOPNOTSUPP;
 }
 
 int
 netdev_flow_del(struct netdev *netdev, const ovs_u128 *ufid,
                 struct dpif_flow_stats *stats)
 {
-    const struct netdev_class *class = netdev->netdev_class;
+    const struct netdev_flow_api *flow_api =
+        ovsrcu_get(const struct netdev_flow_api *, &netdev->flow_api);
 
-    return (class->flow_del
+    return (flow_api && flow_api->flow_del)
-            ? class->flow_del(netdev, ufid, stats)
+           ? flow_api->flow_del(netdev, ufid, stats)
-            : EOPNOTSUPP);
+           : EOPNOTSUPP;
 }
 
 int
 netdev_init_flow_api(struct netdev *netdev)
 {
-    const struct netdev_class *class = netdev->netdev_class;
-
     if (!netdev_is_flow_api_enabled()) {
         return EOPNOTSUPP;
     }
 
-    return (class->init_flow_api
+    if (ovsrcu_get(const struct netdev_flow_api *, &netdev->flow_api)) {
-            ? class->init_flow_api(netdev)
+        return 0;
-            : EOPNOTSUPP);
+    }
+
+    if (netdev_assign_flow_api(netdev)) {
+        return EOPNOTSUPP;
+    }
+
+    return 0;
 }
 
 uint32_t
@@ -2573,7 +2715,6 @@ netdev_is_offload_rebalance_policy_enabled(void)
     return netdev_offload_rebalance_policy;
 }
 
-#ifdef __linux__
 static void
 netdev_ports_flow_init(void)
 {
@@ -2597,8 +2738,10 @@ netdev_set_flow_api_enabled(const struct smap *ovs_other_config)
 
             VLOG_INFO("netdev: Flow API Enabled");
 
+#ifdef __linux__
             tc_set_policy(smap_get_def(ovs_other_config, "tc-policy",
                                        TC_POLICY_DEFAULT));
+#endif
 
             if (smap_get_bool(ovs_other_config, "offload-rebalance", false)) {
                 netdev_offload_rebalance_policy = true;
@@ -2610,9 +2753,3 @@ netdev_set_flow_api_enabled(const struct smap *ovs_other_config)
         }
     }
 }
-#else
-void
-netdev_set_flow_api_enabled(const struct smap *ovs_other_config OVS_UNUSED)
-{
-}
-#endif

tests/dpif-netdev.at

@@ -361,9 +361,9 @@ AT_CLEANUP
 
 m4_define([DPIF_NETDEV_FLOW_HW_OFFLOAD],
   [AT_SETUP([dpif-netdev - partial hw offload - $1])
-   AT_SKIP_IF([test "$IS_WIN32" = "yes" || test "$IS_BSD" = "yes"])
    OVS_VSWITCHD_START(
-     [add-port br0 p1 -- set interface p1 type=$1 ofport_request=1 options:pstream=punix:$OVS_RUNDIR/p1.sock -- \
+     [add-port br0 p1 -- \
+      set interface p1 type=$1 ofport_request=1 options:pstream=punix:$OVS_RUNDIR/p1.sock options:ifindex=1 -- \
       set bridge br0 datapath-type=dummy \
                      other-config:datapath-id=1234 fail-mode=secure], [], [],
       [m4_if([$1], [dummy-pmd], [--dummy-numa="0,0,0,0,1,1,1,1"], [])])

tests/ofproto-macros.at

@@ -173,7 +173,6 @@ m4_define([_OVS_VSWITCHD_START],
 /ofproto|INFO|datapath ID changed to fedcba9876543210/d
 /dpdk|INFO|DPDK Disabled - Use other_config:dpdk-init to enable/d
 /netlink_socket|INFO|netlink: could not enable listening to all nsid/d
-/netdev: Flow API/d
 /probe tc:/d
 /tc: Using policy/d']])
 ])