[ovs-dev] [PATCH v2] dpif-netlink: Fix issues of the offloaded flows counter

Jianbo Liu jianbol at nvidia.com
Tue Dec 15 04:50:08 UTC 2020


The n_offloaded_flows counter is saved in dpif, and this is the first
one when ofproto is created. When flow operation is done by ovs-appctl
commands, such as, dpctl/add-flow, a new dpif is opened, and the
n_offloaded_flows in it can't be used. So, instead of using counter,
the number of offloaded flows is queried from each netdev, then sum
them up. To achieve this, a new API is added in netdev_flow_api to get
how many flows assigned to a netdev.

In order to get better performance, this number is calculated directly
from tc_to_ufid hmap for netdev-offload-tc, because flow dumping by tc
takes much time if there are many flows offloaded.

Fixes: af0618470507 ("dpif-netlink: Count the number of offloaded rules")
Signed-off-by: Jianbo Liu <jianbol at nvidia.com>
---
 lib/dpif-netlink.c               |  9 ---------
 lib/dpif.c                       | 22 ++++++++++++++++++++++
 lib/dpif.h                       |  3 ++-
 lib/netdev-offload-provider.h    |  4 ++++
 lib/netdev-offload-tc.c          | 20 ++++++++++++++++++++
 lib/netdev-offload.c             | 27 +++++++++++++++++++++++++++
 lib/netdev-offload.h             |  3 +++
 ofproto/ofproto-dpif-upcall.c    | 10 ++++------
 tests/system-offloads-traffic.at |  4 ++++
 9 files changed, 86 insertions(+), 16 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 6858ba612..2f881e4fa 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -208,7 +208,6 @@ struct dpif_netlink {
     /* Change notification. */
     struct nl_sock *port_notifier; /* vport multicast group subscriber. */
     bool refresh_channels;
-    struct atomic_count n_offloaded_flows;
 };
 
 static void report_loss(struct dpif_netlink *, struct dpif_channel *,
@@ -654,7 +653,6 @@ dpif_netlink_run(struct dpif *dpif_)
 static int
 dpif_netlink_get_stats(const struct dpif *dpif_, struct dpif_dp_stats *stats)
 {
-    struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
     struct dpif_netlink_dp dp;
     struct ofpbuf *buf;
     int error;
@@ -680,7 +678,6 @@ dpif_netlink_get_stats(const struct dpif *dpif_, struct dpif_dp_stats *stats)
         }
         ofpbuf_delete(buf);
     }
-    stats->n_offloaded_flows = atomic_count_get(&dpif->n_offloaded_flows);
     return error;
 }
 
@@ -2192,9 +2189,6 @@ try_send_to_netdev(struct dpif_netlink *dpif, struct dpif_op *op)
         }
 
         err = parse_flow_put(dpif, put);
-        if (!err && (put->flags & DPIF_FP_CREATE)) {
-            atomic_count_inc(&dpif->n_offloaded_flows);
-        }
         log_flow_put_message(&dpif->dpif, &this_module, put, 0);
         break;
     }
@@ -2209,9 +2203,6 @@ try_send_to_netdev(struct dpif_netlink *dpif, struct dpif_op *op)
                                 dpif_normalize_type(dpif_type(&dpif->dpif)),
                                 del->ufid,
                                 del->stats);
-        if (!err) {
-            atomic_count_dec(&dpif->n_offloaded_flows);
-        }
         log_flow_del_message(&dpif->dpif, &this_module, del, 0);
         break;
     }
diff --git a/lib/dpif.c b/lib/dpif.c
index ac2860764..7f01aa48b 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -2018,3 +2018,25 @@ dpif_bond_stats_get(struct dpif *dpif, uint32_t bond_id,
            ? dpif->dpif_class->bond_stats_get(dpif, bond_id, n_bytes)
            : EOPNOTSUPP;
 }
+
+int
+dpif_get_dp_offloaded_flows(struct dpif *dpif, uint64_t *n_flows)
+{
+    const char *dpif_type_str = dpif_normalize_type(dpif_type(dpif));
+    struct dpif_port_dump port_dump;
+    struct dpif_port dpif_port;
+    int ret, n_devs = 0;
+    uint64_t nflows;
+
+    *n_flows = 0;
+    DPIF_PORT_FOR_EACH (&dpif_port, &port_dump, dpif) {
+        ret = netdev_ports_get_n_flows(dpif_type_str, &dpif_port, &nflows);
+        if (!ret) {
+            *n_flows += nflows;
+        } else if (ret == EOPNOTSUPP) {
+            continue;
+        }
+        n_devs++;
+    }
+    return n_devs ? 0 : EOPNOTSUPP;
+}
diff --git a/lib/dpif.h b/lib/dpif.h
index 7ef148c6d..9d0d662fc 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -429,7 +429,6 @@ struct dpif_dp_stats {
     uint64_t n_missed;          /* Number of flow table misses. */
     uint64_t n_lost;            /* Number of misses not sent to userspace. */
     uint64_t n_flows;           /* Number of flows present. */
-    uint64_t n_offloaded_flows; /* Number of offloaded flows present. */
     uint64_t n_mask_hit;        /* Number of mega flow masks visited for
                                    flow table matches. */
     uint32_t n_masks;           /* Number of mega flow masks. */
@@ -438,6 +437,8 @@ int dpif_get_dp_stats(const struct dpif *, struct dpif_dp_stats *);
 
 int dpif_set_features(struct dpif *, uint32_t new_features);
 
+int dpif_get_dp_offloaded_flows(struct dpif *dpif, uint64_t *n_flows);
+
 
 /* Port operations. */
 
diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h
index 0bed7bf61..cf859d1b4 100644
--- a/lib/netdev-offload-provider.h
+++ b/lib/netdev-offload-provider.h
@@ -83,6 +83,10 @@ struct netdev_flow_api {
     int (*flow_del)(struct netdev *, const ovs_u128 *ufid,
                     struct dpif_flow_stats *);
 
+    /* Get the number of flows offloaded to netdev.
+     * Return 0 if successful, otherwise returns a positive errno value. */
+    int (*flow_get_n_flows)(struct netdev *, uint64_t *n_flows);
+
     /* Initializies the netdev flow api.
      * Return 0 if successful, otherwise returns a positive errno value. */
     int (*init_flow_api)(struct netdev *);
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 2a772a971..f17980a47 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -1904,6 +1904,25 @@ netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED,
     return error;
 }
 
+static int
+netdev_tc_get_n_flows(struct netdev *netdev, uint64_t *n_flows)
+{
+    struct ufid_tc_data *data, *next;
+    uint64_t total = 0;
+
+    ovs_mutex_lock(&ufid_lock);
+    HMAP_FOR_EACH_SAFE (data, next, tc_to_ufid_node, &tc_to_ufid) {
+        if (data->netdev != netdev) {
+            continue;
+        }
+        total++;
+   }
+   ovs_mutex_unlock(&ufid_lock);
+
+   *n_flows = total;
+   return 0;
+}
+
 static void
 probe_multi_mask_per_prio(int ifindex)
 {
@@ -2044,5 +2063,6 @@ const struct netdev_flow_api netdev_offload_tc = {
    .flow_put = netdev_tc_flow_put,
    .flow_get = netdev_tc_flow_get,
    .flow_del = netdev_tc_flow_del,
+   .flow_get_n_flows = netdev_tc_get_n_flows,
    .init_flow_api = netdev_tc_init_flow_api,
 };
diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
index 2da3bc701..696877058 100644
--- a/lib/netdev-offload.c
+++ b/lib/netdev-offload.c
@@ -280,6 +280,17 @@ netdev_flow_del(struct netdev *netdev, const ovs_u128 *ufid,
            : EOPNOTSUPP;
 }
 
+int
+netdev_flow_get_n_flows(struct netdev *netdev, uint64_t *n_flows)
+{
+    const struct netdev_flow_api *flow_api =
+        ovsrcu_get(const struct netdev_flow_api *, &netdev->flow_api);
+
+    return (flow_api && flow_api->flow_get_n_flows)
+           ? flow_api->flow_get_n_flows(netdev, n_flows)
+           : EOPNOTSUPP;
+}
+
 int
 netdev_init_flow_api(struct netdev *netdev)
 {
@@ -602,6 +613,22 @@ netdev_ports_remove(odp_port_t port_no, const char *dpif_type)
     return ret;
 }
 
+int
+netdev_ports_get_n_flows(const char *dpif_type, struct dpif_port *dpif_port,
+                         uint64_t *n_flows)
+{
+    struct port_to_netdev_data *data;
+    int ret = EOPNOTSUPP;
+
+    ovs_rwlock_rdlock(&netdev_hmap_rwlock);
+    data = netdev_ports_lookup(dpif_port->port_no, dpif_type);
+    if (data) {
+        ret = netdev_flow_get_n_flows(data->netdev, n_flows);
+    }
+    ovs_rwlock_unlock(&netdev_hmap_rwlock);
+    return ret;
+}
+
 odp_port_t
 netdev_ifindex_to_odp_port(int ifindex)
 {
diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h
index 4c0ed2ae8..ffd321bfc 100644
--- a/lib/netdev-offload.h
+++ b/lib/netdev-offload.h
@@ -103,6 +103,7 @@ bool netdev_any_oor(void);
 bool netdev_is_flow_api_enabled(void);
 void netdev_set_flow_api_enabled(const struct smap *ovs_other_config);
 bool netdev_is_offload_rebalance_policy_enabled(void);
+int netdev_flow_get_n_flows(struct netdev *netdev, uint64_t *n_flows);
 
 struct dpif_port;
 int netdev_ports_insert(struct netdev *, const char *dpif_type,
@@ -124,6 +125,8 @@ int netdev_ports_flow_get(const char *dpif_type, struct match *match,
                           struct dpif_flow_stats *stats,
                           struct dpif_flow_attrs *attrs,
                           struct ofpbuf *buf);
+int netdev_ports_get_n_flows(const char *dpif_type,
+                             struct dpif_port *dpif_port, uint64_t *n_flows);
 
 #ifdef  __cplusplus
 }
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 19b92dfe0..75496ac6d 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -175,7 +175,6 @@ struct udpif {
 
     /* n_flows_mutex prevents multiple threads updating these concurrently. */
     atomic_uint n_flows;               /* Number of flows in the datapath. */
-    atomic_uint n_offloaded_flows;     /* Number of the offloaded flows. */
     atomic_llong n_flows_timestamp;    /* Last time n_flows was updated. */
     struct ovs_mutex n_flows_mutex;
 
@@ -731,8 +730,6 @@ udpif_get_n_flows(struct udpif *udpif)
         dpif_get_dp_stats(udpif->dpif, &stats);
         flow_count = stats.n_flows;
         atomic_store_relaxed(&udpif->n_flows, flow_count);
-        atomic_store_relaxed(&udpif->n_offloaded_flows,
-                             stats.n_offloaded_flows);
         ovs_mutex_unlock(&udpif->n_flows_mutex);
     } else {
         atomic_read_relaxed(&udpif->n_flows, &flow_count);
@@ -2904,10 +2901,10 @@ upcall_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
                     const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
 {
     struct ds ds = DS_EMPTY_INITIALIZER;
+    uint64_t n_offloaded_flows;
     struct udpif *udpif;
 
     LIST_FOR_EACH (udpif, list_node, &all_udpifs) {
-        unsigned int n_offloaded_flows;
         unsigned int flow_limit;
         bool ufid_enabled;
         size_t i;
@@ -2919,8 +2916,9 @@ upcall_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
         ds_put_format(&ds, "  flows         : (current %lu)"
             " (avg %u) (max %u) (limit %u)\n", udpif_get_n_flows(udpif),
             udpif->avg_n_flows, udpif->max_n_flows, flow_limit);
-        atomic_read_relaxed(&udpif->n_offloaded_flows, &n_offloaded_flows);
-        ds_put_format(&ds, "  offloaded flows : %u\n", n_offloaded_flows);
+        if (!dpif_get_dp_offloaded_flows(udpif->dpif, &n_offloaded_flows)) {
+            ds_put_format(&ds, "  offloaded flows : %ld\n", n_offloaded_flows);
+        }
         ds_put_format(&ds, "  dump duration : %lldms\n", udpif->dump_duration);
         ds_put_format(&ds, "  ufid enabled : ");
         if (ufid_enabled) {
diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at
index 379a8a5e9..c03ffd598 100644
--- a/tests/system-offloads-traffic.at
+++ b/tests/system-offloads-traffic.at
@@ -32,6 +32,8 @@ in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9, bytes:882, used:
 
 AT_CHECK([ovs-appctl dpctl/dump-flows type=offloaded], [0], [])
 
+AT_CHECK([ovs-appctl upcall/show | grep "offloaded flows : 0"], [1], [ignore])
+
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
@@ -64,5 +66,7 @@ in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9, bytes:756, used:
 in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9, bytes:756, used:0.001s, actions:output
 ])
 
+AT_CHECK([ovs-appctl upcall/show | grep -E "offloaded flows : [[1-9]]"], [0], [ignore])
+
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
-- 
2.26.2



More information about the dev mailing list