Eelco Chaudron via dev <[email protected]> writes:
> Relocate the tc flow dump functionality from netdev-offload
> to dpif-offload, centralizing flow dump handling in the
> dpif-offload layer.
>
> Signed-off-by: Eelco Chaudron <[email protected]>
> ---
>
> v2 changes:
> - Use ovs_mutex_destroy() instead of ovs_mutex_init() in
> dpif_offload_tc_flow_dump_destroy().
> ---
> lib/dpif-offload-provider.h | 3 +
> lib/dpif-offload-tc.c | 248 ++++++++++++++++++++++++++++++++++--
> lib/dpif-offload.c | 6 +
> lib/netdev-offload-tc.c | 34 +++--
> lib/netdev-offload-tc.h | 9 ++
> 5 files changed, 267 insertions(+), 33 deletions(-)
>
> diff --git a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h
> index 2c761c13e..8840441c0 100644
> --- a/lib/dpif-offload-provider.h
> +++ b/lib/dpif-offload-provider.h
> @@ -281,6 +281,7 @@ bool dpif_offload_port_mgr_add(struct
> dpif_offload_port_mgr *,
> struct dpif_offload_port_mgr_port *dpif_offload_port_mgr_remove(
> struct dpif_offload_port_mgr *, odp_port_t, bool keep_netdev_ref);
> void dpif_offload_port_mgr_uninit(struct dpif_offload_port_mgr *);
> +size_t dpif_offload_port_mgr_port_count(struct dpif_offload_port_mgr *);
> struct dpif_offload_port_mgr_port *dpif_offload_port_mgr_find_by_ifindex(
> struct dpif_offload_port_mgr *, int ifindex);
> struct dpif_offload_port_mgr_port *dpif_offload_port_mgr_find_by_netdev(
> @@ -292,6 +293,8 @@ void dpif_offload_port_mgr_traverse_ports(
> bool (*cb)(struct dpif_offload_port_mgr_port *, void *),
> void *aux);
>
> +#define DPIF_OFFLOAD_PORT_MGR_PORT_FOR_EACH(PORT, PORT_MGR) \
> + CMAP_FOR_EACH (PORT, odp_port_node, &(PORT_MGR)->odp_port_to_port)
Since you're spinning another version, I think this might be better
added in
[07/40] dpif-offload: Add port registration and management APIs.
At least because it can be used as the FOR construct in
`dpif_offload_port_mgr_traverse_ports`
> /* Global functions, called by the dpif layer or offload providers. */
> void dp_offload_initialize(void);
> diff --git a/lib/dpif-offload-tc.c b/lib/dpif-offload-tc.c
> index 781fc53c6..94f3573cc 100644
> --- a/lib/dpif-offload-tc.c
> +++ b/lib/dpif-offload-tc.c
> @@ -19,13 +19,16 @@
>
> #include "dpif-offload.h"
> #include "dpif-offload-provider.h"
> +#include "netdev-offload.h"
> #include "netdev-offload-tc.h"
> #include "netdev-provider.h"
> #include "netdev-vport.h"
> +#include "odp-util.h"
> #include "tc.h"
> #include "util.h"
>
> #include "openvswitch/json.h"
> +#include "openvswitch/match.h"
> #include "openvswitch/vlog.h"
>
> VLOG_DEFINE_THIS_MODULE(dpif_offload_tc);
> @@ -39,6 +42,30 @@ struct dpif_offload_tc {
> struct ovsthread_once once_enable; /* Track first-time enablement. */
> };
>
> +/* tc's flow dump specific data structures. */
> +struct dpif_offload_tc_flow_dump {
> + struct dpif_offload_flow_dump dump;
> + struct ovs_mutex netdev_dump_mutex;
> + size_t netdev_dump_index;
> + size_t netdev_dump_count;
> + struct netdev_flow_dump *netdev_dumps[];
> +};
> +
> +#define FLOW_DUMP_MAX_BATCH 50
> +
> +struct dpif_offload_tc_flow_dump_thread {
> + struct dpif_offload_flow_dump_thread thread;
> + struct dpif_offload_tc_flow_dump *dump;
> + bool netdev_dump_done;
> + size_t netdev_dump_index;
> +
> + /* (Flows/Key/Mask/Actions) Buffers for netdev dumping */
> + struct ofpbuf nl_flows;
> + struct odputil_keybuf keybuf[FLOW_DUMP_MAX_BATCH];
> + struct odputil_keybuf maskbuf[FLOW_DUMP_MAX_BATCH];
> + struct odputil_keybuf actbuf[FLOW_DUMP_MAX_BATCH];
> +};
> +
> static struct dpif_offload_tc *
> dpif_offload_tc_cast(const struct dpif_offload *offload)
> {
> @@ -284,46 +311,239 @@ dpif_offload_tc_flow_flush(const struct dpif_offload
> *offload)
> return err;
> }
>
> +static struct dpif_offload_tc_flow_dump *
> +dpif_offload_tc_flow_dump_cast(struct dpif_offload_flow_dump *dump)
> +{
> + return CONTAINER_OF(dump, struct dpif_offload_tc_flow_dump, dump);
> +}
> +
> +static struct dpif_offload_tc_flow_dump_thread *
> +dpif_offload_tc_flow_dump_thread_cast(
> + struct dpif_offload_flow_dump_thread *thread)
> +{
> + return CONTAINER_OF(thread, struct dpif_offload_tc_flow_dump_thread,
> + thread);
> +}
> +
> static struct dpif_offload_flow_dump *
> -dpif_offload_tc_flow_dump_create(const struct dpif_offload *offload,
> +dpif_offload_tc_flow_dump_create(const struct dpif_offload *offload_,
> bool terse)
> {
> - struct dpif_offload_flow_dump *dump;
> + struct dpif_offload_tc *offload = dpif_offload_tc_cast(offload_);
> + struct dpif_offload_port_mgr_port *port;
> + struct dpif_offload_tc_flow_dump *dump;
> + size_t added_port_count = 0;
> + size_t port_count;
> +
> + port_count = dpif_offload_port_mgr_port_count(offload->port_mgr);
> +
> + dump = xmalloc(sizeof *dump +
> + (port_count * sizeof(struct netdev_flow_dump)));
>
> - dump = xmalloc(sizeof *dump);
> - dpif_offload_flow_dump_init(dump, offload, terse);
> - return dump;
> + dpif_offload_flow_dump_init(&dump->dump, offload_, terse);
> +
> + DPIF_OFFLOAD_PORT_MGR_PORT_FOR_EACH (port, offload->port_mgr) {
> + if (added_port_count >= port_count) {
> + break;
> + }
> + if (netdev_offload_tc_flow_dump_create(
> + port->netdev, &dump->netdev_dumps[added_port_count], terse)) {
> + continue;
> + }
> + dump->netdev_dumps[added_port_count]->port = port->port_no;
> + added_port_count++;
> + }
> + dump->netdev_dump_count = added_port_count;
> + dump->netdev_dump_index = 0;
> + ovs_mutex_init(&dump->netdev_dump_mutex);
> + return &dump->dump;
> }
>
> static int
> -dpif_offload_tc_flow_dump_next(struct dpif_offload_flow_dump_thread *thread,
> - struct dpif_flow *flows, int max_flows)
> +dpif_offload_tc_netdev_match_to_dpif_flow(struct match *match,
> + struct ofpbuf *key_buf,
> + struct ofpbuf *mask_buf,
> + struct nlattr *actions,
> + struct dpif_flow_stats *stats,
> + struct dpif_flow_attrs *attrs,
> + ovs_u128 *ufid,
> + struct dpif_flow *flow,
> + bool terse)
> {
> - ovs_assert(thread && flows && max_flows);
> + memset(flow, 0, sizeof *flow);
> +
> + 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);
> +
> + /* UFID */
> + flow->ufid_present = true;
> + flow->ufid = *ufid;
> +
> + flow->pmd_id = PMD_ID_NULL;
> +
> + memcpy(&flow->attrs, attrs, sizeof *attrs);
> +
> return 0;
> }
>
> +static void
> +dpif_offload_tc_advance_provider_dump(
> + struct dpif_offload_tc_flow_dump_thread *thread)
> +{
> + struct dpif_offload_tc_flow_dump *dump = thread->dump;
> +
> + ovs_mutex_lock(&dump->netdev_dump_mutex);
> +
> + /* If we haven't finished (dumped all providers). */
> + if (dump->netdev_dump_index < dump->netdev_dump_count) {
> + /* If we are the first to find that current dump is finished
> + * advance it. */
> + if (thread->netdev_dump_index == dump->netdev_dump_index) {
> + thread->netdev_dump_index = ++dump->netdev_dump_index;
> + /* Did we just finish the last dump? If so we are done. */
> + if (dump->netdev_dump_index == dump->netdev_dump_count) {
> + thread->netdev_dump_done = true;
> + }
> + } else {
> + /* Otherwise, we are behind, catch up. */
> + thread->netdev_dump_index = dump->netdev_dump_index;
> + }
> + } else {
> + /* Some other thread finished. */
> + thread->netdev_dump_done = true;
> + }
> +
> + ovs_mutex_unlock(&dump->netdev_dump_mutex);
> +}
> +
> static int
> -dpif_offload_tc_flow_dump_destroy(struct dpif_offload_flow_dump *dump)
> +dpif_offload_tc_flow_dump_next(struct dpif_offload_flow_dump_thread *thread_,
> + struct dpif_flow *flows, int max_flows)
> {
> + struct dpif_offload_tc_flow_dump_thread *thread;
> + int n_flows = 0;
> +
> + thread = dpif_offload_tc_flow_dump_thread_cast(thread_);
> + max_flows = MIN(max_flows, FLOW_DUMP_MAX_BATCH);
> +
> + while (!thread->netdev_dump_done && n_flows < max_flows) {
> + struct odputil_keybuf *maskbuf = &thread->maskbuf[n_flows];
> + struct odputil_keybuf *keybuf = &thread->keybuf[n_flows];
> + struct odputil_keybuf *actbuf = &thread->actbuf[n_flows];
> + struct dpif_flow *f = &flows[n_flows];
> + struct netdev_flow_dump *netdev_dump;
> + int cur = thread->netdev_dump_index;
> + struct ofpbuf key, mask, act;
> + struct dpif_flow_stats stats;
> + struct dpif_flow_attrs attrs;
> + struct nlattr *actions;
> + struct match match;
> + ovs_u128 ufid;
> + bool has_next;
> +
> + netdev_dump = thread->dump->netdev_dumps[cur];
> + ofpbuf_use_stack(&key, keybuf, sizeof *keybuf);
> + ofpbuf_use_stack(&act, actbuf, sizeof *actbuf);
> + ofpbuf_use_stack(&mask, maskbuf, sizeof *maskbuf);
> + has_next = netdev_offload_tc_flow_dump_next(netdev_dump, &match,
> + &actions, &stats, &attrs,
> + &ufid,
> + &thread->nl_flows,
> + &act);
> +
> + if (has_next) {
> + dpif_offload_tc_netdev_match_to_dpif_flow(&match,
> + &key, &mask,
> + actions,
> + &stats,
> + &attrs,
> + &ufid,
> + f,
> +
> thread->dump->dump.terse
> + );
> + n_flows++;
> + } else {
> + dpif_offload_tc_advance_provider_dump(thread);
> + }
> + }
> + return n_flows;
> +}
> +
> +static int
> +dpif_offload_tc_flow_dump_destroy(struct dpif_offload_flow_dump *dump_)
> +{
> + struct dpif_offload_tc_flow_dump *dump;
> + int error = 0;
> +
> + dump = dpif_offload_tc_flow_dump_cast(dump_);
> + for (int i = 0; i < dump->netdev_dump_count; i++) {
> + struct netdev_flow_dump *dump_netdev = dump->netdev_dumps[i];
> + int rc = netdev_offload_tc_flow_dump_destroy(dump_netdev);
> +
> + if (rc && !error) {
> + error = rc;
> + }
> + }
> + ovs_mutex_destroy(&dump->netdev_dump_mutex);
> free(dump);
> - return 0;
> + return error;
> }
>
> static struct dpif_offload_flow_dump_thread *
> dpif_offload_tc_flow_dump_thread_create(struct dpif_offload_flow_dump *dump)
> {
> - struct dpif_offload_flow_dump_thread *thread;
> + struct dpif_offload_tc_flow_dump_thread *thread;
>
> thread = xmalloc(sizeof *thread);
> - dpif_offload_flow_dump_thread_init(thread, dump);
> - return thread;
> + dpif_offload_flow_dump_thread_init(&thread->thread, dump);
> + thread->dump = dpif_offload_tc_flow_dump_cast(dump);
> + thread->netdev_dump_index = 0;
> + thread->netdev_dump_done = !thread->dump->netdev_dump_count;
> + ofpbuf_init(&thread->nl_flows, NL_DUMP_BUFSIZE);
> + return &thread->thread;
> }
>
> static void
> dpif_offload_tc_flow_dump_thread_destroy(
> - struct dpif_offload_flow_dump_thread *thread)
> + struct dpif_offload_flow_dump_thread *thread_)
> {
> + struct dpif_offload_tc_flow_dump_thread *thread;
> +
> + thread = dpif_offload_tc_flow_dump_thread_cast(thread_);
> + ofpbuf_uninit(&thread->nl_flows);
> free(thread);
> }
>
> diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c
> index df7bf5082..51a250a73 100644
> --- a/lib/dpif-offload.c
> +++ b/lib/dpif-offload.c
> @@ -1242,3 +1242,9 @@ dpif_offload_port_mgr_traverse_ports(
> }
> }
> }
> +
> +size_t
> +dpif_offload_port_mgr_port_count(struct dpif_offload_port_mgr *mgr)
> +{
> + return cmap_count(&mgr->odp_port_to_port);
> +}
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index af9a10356..289dbbf72 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -586,10 +586,10 @@ netdev_offload_tc_flow_flush(struct netdev *netdev)
> return 0;
> }
>
> -static int
> -netdev_tc_flow_dump_create(struct netdev *netdev,
> - struct netdev_flow_dump **dump_out,
> - bool terse)
> +int
> +netdev_offload_tc_flow_dump_create(struct netdev *netdev,
> + struct netdev_flow_dump **dump_out,
> + bool terse)
> {
> enum tc_qdisc_hook hook = get_tc_qdisc_hook(netdev);
> struct netdev_flow_dump *dump;
> @@ -618,9 +618,8 @@ netdev_tc_flow_dump_create(struct netdev *netdev,
>
> return 0;
> }
> -
> -static int
> -netdev_tc_flow_dump_destroy(struct netdev_flow_dump *dump)
> +int
> +netdev_offload_tc_flow_dump_destroy(struct netdev_flow_dump *dump)
> {
> nl_dump_done(dump->nl_dump);
> netdev_close(dump->netdev);
> @@ -1351,15 +1350,15 @@ parse_tc_flower_to_match(const struct netdev *netdev,
> return 0;
> }
>
> -static bool
> -netdev_tc_flow_dump_next(struct netdev_flow_dump *dump,
> - struct match *match,
> - struct nlattr **actions,
> - struct dpif_flow_stats *stats,
> - struct dpif_flow_attrs *attrs,
> - ovs_u128 *ufid,
> - struct ofpbuf *rbuffer,
> - struct ofpbuf *wbuffer)
> +bool
> +netdev_offload_tc_flow_dump_next(struct netdev_flow_dump *dump,
> + struct match *match,
> + struct nlattr **actions,
> + struct dpif_flow_stats *stats,
> + struct dpif_flow_attrs *attrs,
> + ovs_u128 *ufid,
> + struct ofpbuf *rbuffer,
> + struct ofpbuf *wbuffer)
> {
> struct netdev *netdev = dump->netdev;
> struct ofpbuf nl_flow;
> @@ -3442,9 +3441,6 @@ dpif_offload_tc_meter_del(const struct dpif_offload
> *offload OVS_UNUSED,
>
> const struct netdev_flow_api netdev_offload_tc = {
> .type = "linux_tc",
> - .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,
> diff --git a/lib/netdev-offload-tc.h b/lib/netdev-offload-tc.h
> index 5002ab00b..ba06737ef 100644
> --- a/lib/netdev-offload-tc.h
> +++ b/lib/netdev-offload-tc.h
> @@ -24,6 +24,15 @@ struct netdev;
> /* Netdev-specific offload functions. These should only be used by the
> * associated dpif offload provider. */
> int netdev_offload_tc_flow_flush(struct netdev *);
> +int netdev_offload_tc_flow_dump_create(struct netdev *,
> + struct netdev_flow_dump **, bool
> terse);
> +int netdev_offload_tc_flow_dump_destroy(struct netdev_flow_dump *);
> +bool netdev_offload_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);
> void dpif_offload_tc_meter_init(void);
> int dpif_offload_tc_meter_set(const struct dpif_offload *, ofproto_meter_id,
> struct ofputil_meter_config *);
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
