[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