[ovs-dev] [PATCH v2 1/2] netdev-offload: Implement terse dump support
Roi Dayan
roid at mellanox.com
Thu Jun 4 10:47:00 UTC 2020
From: Vlad Buslov <vladbu at mellanox.com>
In order to improve revalidator performance by minimizing unnecessary
copying of data, extend netdev-offloads to support terse dump mode. Extend
netdev_flow_api->flow_dump_create() with 'terse' bool argument. Implement
support for terse dump in functions that convert netlink to flower and
flower to match. Set flow stats "used" value based on difference in number
of flow packets because lastuse timestamp is not included in TC terse dump.
Kernel API support is implemented in following patch.
Signed-off-by: Vlad Buslov <vladbu at mellanox.com>
Reviewed-by: Roi Dayan <roid at mellanox.com>
---
lib/dpif-netlink.c | 70 ++++++++++++++++++++++---------------------
lib/netdev-offload-provider.h | 3 +-
lib/netdev-offload-tc.c | 67 +++++++++++++++++++++++++++++++----------
lib/netdev-offload.c | 10 ++++---
lib/netdev-offload.h | 6 ++--
ofproto/ofproto-dpif-upcall.c | 24 +++++++++++++--
6 files changed, 122 insertions(+), 58 deletions(-)
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index dc642100fc58..01485c7dc4fa 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1445,7 +1445,8 @@ start_netdev_dump(const struct dpif *dpif_,
dump->netdev_current_dump = 0;
dump->netdev_dumps
= netdev_ports_flow_dump_create(dpif_->dpif_class,
- &dump->netdev_dumps_num);
+ &dump->netdev_dumps_num,
+ dump->up.terse);
ovs_mutex_unlock(&dump->netdev_lock);
}
@@ -1640,41 +1641,42 @@ dpif_netlink_netdev_match_to_dpif_flow(struct match *match,
struct dpif_flow_attrs *attrs,
ovs_u128 *ufid,
struct dpif_flow *flow,
- bool terse OVS_UNUSED)
-{
-
- struct odp_flow_key_parms odp_parms = {
- .flow = &match->flow,
- .mask = &match->wc.masks,
- .support = {
- .max_vlan_headers = 2,
- .recirc = true,
- .ct_state = true,
- .ct_zone = true,
- .ct_mark = true,
- .ct_label = true,
- },
- };
- size_t offset;
-
+ bool terse)
+{
memset(flow, 0, sizeof *flow);
- /* Key */
- offset = key_buf->size;
- flow->key = ofpbuf_tail(key_buf);
- odp_flow_key_from_flow(&odp_parms, key_buf);
- flow->key_len = key_buf->size - offset;
-
- /* Mask */
- offset = mask_buf->size;
- flow->mask = ofpbuf_tail(mask_buf);
- odp_parms.key_buf = key_buf;
- odp_flow_key_from_mask(&odp_parms, mask_buf);
- flow->mask_len = mask_buf->size - offset;
-
- /* Actions */
- flow->actions = nl_attr_get(actions);
- flow->actions_len = nl_attr_get_size(actions);
+ if (!terse) {
+ struct odp_flow_key_parms odp_parms = {
+ .flow = &match->flow,
+ .mask = &match->wc.masks,
+ .support = {
+ .max_vlan_headers = 2,
+ .recirc = true,
+ .ct_state = true,
+ .ct_zone = true,
+ .ct_mark = true,
+ .ct_label = true,
+ },
+ };
+ size_t offset;
+
+ /* Key */
+ offset = key_buf->size;
+ flow->key = ofpbuf_tail(key_buf);
+ odp_flow_key_from_flow(&odp_parms, key_buf);
+ flow->key_len = key_buf->size - offset;
+
+ /* Mask */
+ offset = mask_buf->size;
+ flow->mask = ofpbuf_tail(mask_buf);
+ odp_parms.key_buf = key_buf;
+ odp_flow_key_from_mask(&odp_parms, mask_buf);
+ flow->mask_len = mask_buf->size - offset;
+
+ /* Actions */
+ flow->actions = nl_attr_get(actions);
+ flow->actions_len = nl_attr_get_size(actions);
+ }
/* Stats */
memcpy(&flow->stats, stats, sizeof *stats);
diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h
index 5a809c0cdf1f..0bed7bf61edb 100644
--- a/lib/netdev-offload-provider.h
+++ b/lib/netdev-offload-provider.h
@@ -42,7 +42,8 @@ struct netdev_flow_api {
*
* 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_create)(struct netdev *, struct netdev_flow_dump **dump,
+ bool terse);
int (*flow_dump_destroy)(struct netdev_flow_dump *);
/* Returns true if there are more flows to dump.
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index e188e63e560e..3ba70db4690b 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -366,7 +366,8 @@ netdev_tc_flow_flush(struct netdev *netdev)
static int
netdev_tc_flow_dump_create(struct netdev *netdev,
- struct netdev_flow_dump **dump_out)
+ struct netdev_flow_dump **dump_out,
+ bool terse)
{
enum tc_qdisc_hook hook = get_tc_qdisc_hook(netdev);
struct netdev_flow_dump *dump;
@@ -386,6 +387,7 @@ netdev_tc_flow_dump_create(struct netdev *netdev,
dump = xzalloc(sizeof *dump);
dump->nl_dump = xzalloc(sizeof *dump->nl_dump);
dump->netdev = netdev_ref(netdev);
+ dump->terse = terse;
id = tc_make_tcf_id(ifindex, block_id, prio, hook);
tc_dump_flower_start(&id, dump->nl_dump);
@@ -502,13 +504,53 @@ flower_tun_opt_to_match(struct match *match, struct tc_flower *flower)
match->wc.masks.tunnel.flags |= FLOW_TNL_F_UDPIF;
}
+static void
+parse_tc_flower_to_stats(struct tc_flower *flower,
+ struct dpif_flow_stats *stats)
+{
+ if (!stats) {
+ return;
+ }
+
+ memset(stats, 0, sizeof *stats);
+ stats->n_packets = get_32aligned_u64(&flower->stats.n_packets);
+ stats->n_bytes = get_32aligned_u64(&flower->stats.n_bytes);
+ stats->used = flower->lastused;
+}
+
+static void
+parse_tc_flower_to_attrs(struct tc_flower *flower,
+ struct dpif_flow_attrs *attrs)
+{
+ attrs->offloaded = (flower->offloaded_state == TC_OFFLOADED_STATE_IN_HW ||
+ flower->offloaded_state ==
+ TC_OFFLOADED_STATE_UNDEFINED);
+ attrs->dp_layer = "tc";
+ attrs->dp_extra_info = NULL;
+}
+
+static int
+parse_tc_flower_terse_to_match(struct tc_flower *flower,
+ struct match *match,
+ struct dpif_flow_stats *stats,
+ struct dpif_flow_attrs *attrs)
+{
+ match_init_catchall(match);
+
+ parse_tc_flower_to_stats(flower, stats);
+ parse_tc_flower_to_attrs(flower, attrs);
+
+ return 0;
+}
+
static int
parse_tc_flower_to_match(struct tc_flower *flower,
struct match *match,
struct nlattr **actions,
struct dpif_flow_stats *stats,
struct dpif_flow_attrs *attrs,
- struct ofpbuf *buf)
+ struct ofpbuf *buf,
+ bool terse)
{
size_t act_off;
struct tc_flower_key *key = &flower->key;
@@ -517,6 +559,10 @@ parse_tc_flower_to_match(struct tc_flower *flower,
struct tc_action *action;
int i;
+ if (terse) {
+ return parse_tc_flower_terse_to_match(flower, match, stats, attrs);
+ }
+
ofpbuf_clear(buf);
match_init_catchall(match);
@@ -863,17 +909,8 @@ parse_tc_flower_to_match(struct tc_flower *flower,
*actions = ofpbuf_at_assert(buf, act_off, sizeof(struct nlattr));
- if (stats) {
- memset(stats, 0, sizeof *stats);
- stats->n_packets = get_32aligned_u64(&flower->stats.n_packets);
- stats->n_bytes = get_32aligned_u64(&flower->stats.n_bytes);
- stats->used = flower->lastused;
- }
-
- attrs->offloaded = (flower->offloaded_state == TC_OFFLOADED_STATE_IN_HW)
- || (flower->offloaded_state == TC_OFFLOADED_STATE_UNDEFINED);
- attrs->dp_layer = "tc";
- attrs->dp_extra_info = NULL;
+ parse_tc_flower_to_stats(flower, stats);
+ parse_tc_flower_to_attrs(flower, attrs);
return 0;
}
@@ -905,7 +942,7 @@ netdev_tc_flow_dump_next(struct netdev_flow_dump *dump,
}
if (parse_tc_flower_to_match(&flower, match, actions, stats, attrs,
- wbuffer)) {
+ wbuffer, dump->terse)) {
continue;
}
@@ -1784,7 +1821,7 @@ netdev_tc_flow_get(struct netdev *netdev,
}
in_port = netdev_ifindex_to_odp_port(id.ifindex);
- parse_tc_flower_to_match(&flower, match, actions, stats, attrs, buf);
+ parse_tc_flower_to_match(&flower, match, actions, stats, attrs, buf, false);
match->wc.masks.in_port.odp_port = u32_to_odp(UINT32_MAX);
match->flow.in_port.odp_port = in_port;
diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
index 32eab5910760..ab97a292ebac 100644
--- a/lib/netdev-offload.c
+++ b/lib/netdev-offload.c
@@ -201,13 +201,14 @@ netdev_flow_flush(struct netdev *netdev)
}
int
-netdev_flow_dump_create(struct netdev *netdev, struct netdev_flow_dump **dump)
+netdev_flow_dump_create(struct netdev *netdev, struct netdev_flow_dump **dump,
+ bool terse)
{
const struct netdev_flow_api *flow_api =
ovsrcu_get(const struct netdev_flow_api *, &netdev->flow_api);
return (flow_api && flow_api->flow_dump_create)
- ? flow_api->flow_dump_create(netdev, dump)
+ ? flow_api->flow_dump_create(netdev, dump, terse)
: EOPNOTSUPP;
}
@@ -436,7 +437,8 @@ netdev_ports_flow_flush(const struct dpif_class *dpif_class)
}
struct netdev_flow_dump **
-netdev_ports_flow_dump_create(const struct dpif_class *dpif_class, int *ports)
+netdev_ports_flow_dump_create(const struct dpif_class *dpif_class, int *ports,
+ bool terse)
{
struct port_to_netdev_data *data;
struct netdev_flow_dump **dumps;
@@ -454,7 +456,7 @@ netdev_ports_flow_dump_create(const struct dpif_class *dpif_class, int *ports)
HMAP_FOR_EACH (data, portno_node, &port_to_netdev) {
if (data->dpif_class == dpif_class) {
- if (netdev_flow_dump_create(data->netdev, &dumps[i])) {
+ if (netdev_flow_dump_create(data->netdev, &dumps[i], terse)) {
continue;
}
diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h
index b4b882a56aac..87f5852c8d60 100644
--- a/lib/netdev-offload.h
+++ b/lib/netdev-offload.h
@@ -80,7 +80,8 @@ struct offload_info {
};
int netdev_flow_flush(struct netdev *);
-int netdev_flow_dump_create(struct netdev *, struct netdev_flow_dump **dump);
+int netdev_flow_dump_create(struct netdev *, struct netdev_flow_dump **dump,
+ bool terse);
int netdev_flow_dump_destroy(struct netdev_flow_dump *);
bool netdev_flow_dump_next(struct netdev_flow_dump *, struct match *,
struct nlattr **actions, struct dpif_flow_stats *,
@@ -114,7 +115,8 @@ odp_port_t netdev_ifindex_to_odp_port(int ifindex);
struct netdev_flow_dump **netdev_ports_flow_dump_create(
const struct dpif_class *,
- int *ports);
+ int *ports,
+ bool terse);
void netdev_ports_flow_flush(const struct dpif_class *);
int netdev_ports_flow_del(const struct dpif_class *, const ovs_u128 *ufid,
struct dpif_flow_stats *stats);
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 5e08ef10dad6..920f29a6f36a 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2576,6 +2576,25 @@ udpif_update_flow_pps(struct udpif *udpif, struct udpif_key *ukey,
ukey->flow_time = udpif->dpif->current_ms;
}
+static long long int
+udpif_update_used(struct udpif *udpif, struct udpif_key *ukey,
+ struct dpif_flow_stats *stats)
+ OVS_REQUIRES(ukey->mutex)
+{
+ if (!udpif->dump->terse) {
+ return ukey->created;
+ }
+
+ if (stats->n_packets > ukey->stats.n_packets) {
+ stats->used = udpif->dpif->current_ms;
+ } else if (ukey->stats.used) {
+ stats->used = ukey->stats.used;
+ } else {
+ stats->used = ukey->created;
+ }
+ return stats->used;
+}
+
static void
revalidate(struct revalidator *revalidator)
{
@@ -2631,6 +2650,7 @@ revalidate(struct revalidator *revalidator)
for (f = flows; f < &flows[n_dumped]; f++) {
long long int used = f->stats.used;
struct recirc_refs recircs = RECIRC_REFS_EMPTY_INITIALIZER;
+ struct dpif_flow_stats stats = f->stats;
enum reval_result result;
struct udpif_key *ukey;
bool already_dumped;
@@ -2675,12 +2695,12 @@ revalidate(struct revalidator *revalidator)
}
if (!used) {
- used = ukey->created;
+ used = udpif_update_used(udpif, ukey, &stats);
}
if (kill_them_all || (used && used < now - max_idle)) {
result = UKEY_DELETE;
} else {
- result = revalidate_ukey(udpif, ukey, &f->stats, &odp_actions,
+ result = revalidate_ukey(udpif, ukey, &stats, &odp_actions,
reval_seq, &recircs,
f->attrs.offloaded);
}
--
2.8.4
More information about the dev
mailing list