Skip to content

Commit

Permalink
ofproto-dpif-trace: Support detailed output for conjunctive match.
Browse files Browse the repository at this point in the history
A conjunctive flow consists of two or more multiple flows with
conjunction actions. When input to the ofproto/trace command
matches a conjunctive flow, it outputs flows of all dimensions.

Signed-off-by: Nobuhiro MIKI <[email protected]>
Signed-off-by: 0-day Robot <[email protected]>
  • Loading branch information
bobuhiro11 authored and ovsrobot committed Sep 15, 2023
1 parent a6bda3c commit 6cf493c
Show file tree
Hide file tree
Showing 10 changed files with 120 additions and 30 deletions.
38 changes: 32 additions & 6 deletions lib/classifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,20 @@ trie_ctx_init(struct trie_ctx *ctx, const struct cls_trie *trie)
ctx->lookup_done = false;
}

static void
append_conj_flows(struct ovs_list *conj_flows,
struct cls_conjunction_set **soft, size_t n_soft)
{
struct flow *f;
int i;

for (i = 0; i < n_soft; i++) {
f = xmalloc(sizeof *f);
miniflow_expand(&soft[i]->match->flow, f);
ovs_list_push_back(conj_flows, &f->list_node);
}
}

struct conjunctive_match {
struct hmap_node hmap_node;
uint32_t id;
Expand Down Expand Up @@ -933,11 +947,15 @@ free_conjunctive_matches(struct hmap *matches,
* recursion within this function itself.
*
* 'flow' is non-const to allow for temporary modifications during the lookup.
* Any changes are restored before returning. */
* Any changes are restored before returning.
*
* 'conj_flows' is an optional parameter. If it is non-null, the matching
* conjunctive flows are appended to the list. */
static const struct cls_rule *
classifier_lookup__(const struct classifier *cls, ovs_version_t version,
struct flow *flow, struct flow_wildcards *wc,
bool allow_conjunctive_matches)
bool allow_conjunctive_matches,
struct ovs_list *conj_flows)
{
struct trie_ctx trie_ctx[CLS_MAX_TRIES];
const struct cls_match *match;
Expand Down Expand Up @@ -1097,10 +1115,14 @@ classifier_lookup__(const struct classifier *cls, ovs_version_t version,
const struct cls_rule *rule;

flow->conj_id = id;
rule = classifier_lookup__(cls, version, flow, wc, false);
rule = classifier_lookup__(cls, version, flow, wc, false,
conj_flows);
flow->conj_id = saved_conj_id;

if (rule) {
if (conj_flows) {
append_conj_flows(conj_flows, soft, n_soft);
}
free_conjunctive_matches(&matches,
cm_stubs, ARRAY_SIZE(cm_stubs));
if (soft != soft_stub) {
Expand Down Expand Up @@ -1161,12 +1183,16 @@ classifier_lookup__(const struct classifier *cls, ovs_version_t version,
* flow_wildcards_init_catchall()).
*
* 'flow' is non-const to allow for temporary modifications during the lookup.
* Any changes are restored before returning. */
* Any changes are restored before returning.
*
* 'conj_flows' is an optional parameter. If it is non-null, the matching
* conjunctive flows are appended to the list. */
const struct cls_rule *
classifier_lookup(const struct classifier *cls, ovs_version_t version,
struct flow *flow, struct flow_wildcards *wc)
struct flow *flow, struct flow_wildcards *wc,
struct ovs_list *conj_flows)
{
return classifier_lookup__(cls, version, flow, wc, true);
return classifier_lookup__(cls, version, flow, wc, true, conj_flows);
}

/* Finds and returns a rule in 'cls' with exactly the same priority and
Expand Down
4 changes: 3 additions & 1 deletion lib/classifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@
* parallel to the rule's removal. */

#include "cmap.h"
#include "openvswitch/list.h"
#include "openvswitch/match.h"
#include "openvswitch/meta-flow.h"
#include "pvector.h"
Expand Down Expand Up @@ -398,7 +399,8 @@ static inline void classifier_publish(struct classifier *);
* and each other. */
const struct cls_rule *classifier_lookup(const struct classifier *,
ovs_version_t, struct flow *,
struct flow_wildcards *);
struct flow_wildcards *,
struct ovs_list *conj_flows);
bool classifier_rule_overlaps(const struct classifier *,
const struct cls_rule *, ovs_version_t);
const struct cls_rule *classifier_find_rule_exactly(const struct classifier *,
Expand Down
5 changes: 3 additions & 2 deletions lib/ovs-router.c
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ ovs_router_lookup(uint32_t mark, const struct in6_addr *ip6_dst,
const struct cls_rule *cr_src;
struct flow flow_src = {.ipv6_dst = *src, .pkt_mark = mark};

cr_src = classifier_lookup(&cls, OVS_VERSION_MAX, &flow_src, NULL);
cr_src = classifier_lookup(&cls, OVS_VERSION_MAX, &flow_src, NULL,
NULL);
if (cr_src) {
struct ovs_router_entry *p_src = ovs_router_entry_cast(cr_src);
if (!p_src->local) {
Expand All @@ -126,7 +127,7 @@ ovs_router_lookup(uint32_t mark, const struct in6_addr *ip6_dst,
}
}

cr = classifier_lookup(&cls, OVS_VERSION_MAX, &flow, NULL);
cr = classifier_lookup(&cls, OVS_VERSION_MAX, &flow, NULL, NULL);
if (cr) {
struct ovs_router_entry *p = ovs_router_entry_cast(cr);

Expand Down
6 changes: 3 additions & 3 deletions lib/tnl-ports.c
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ map_insert(odp_port_t port, struct eth_addr mac, struct in6_addr *addr,
tnl_port_init_flow(&match.flow, mac, addr, nw_proto, tp_port);

do {
cr = classifier_lookup(&cls, OVS_VERSION_MAX, &match.flow, NULL);
cr = classifier_lookup(&cls, OVS_VERSION_MAX, &match.flow, NULL, NULL);
p = tnl_port_cast(cr);
/* Try again if the rule was released before we get the reference. */
} while (p && !ovs_refcount_try_ref_rcu(&p->ref_cnt));
Expand Down Expand Up @@ -247,7 +247,7 @@ map_delete(struct eth_addr mac, struct in6_addr *addr,

tnl_port_init_flow(&flow, mac, addr, nw_proto, tp_port);

cr = classifier_lookup(&cls, OVS_VERSION_MAX, &flow, NULL);
cr = classifier_lookup(&cls, OVS_VERSION_MAX, &flow, NULL, NULL);
tnl_port_unref(cr);
}

Expand Down Expand Up @@ -305,7 +305,7 @@ odp_port_t
tnl_port_map_lookup(struct flow *flow, struct flow_wildcards *wc)
{
const struct cls_rule *cr = classifier_lookup(&cls, OVS_VERSION_MAX, flow,
wc);
wc, NULL);

return (cr) ? tnl_port_cast(cr)->portno : ODPP_NONE;
}
Expand Down
47 changes: 41 additions & 6 deletions ofproto/ofproto-dpif-xlate.c
Original file line number Diff line number Diff line change
Expand Up @@ -866,6 +866,33 @@ xlate_report_action_set(const struct xlate_ctx *ctx, const char *verb)
}
}

static void
xlate_report_conj_matches(const struct xlate_ctx *ctx)
{
struct match match;
struct flow *f;
int count;
int i = 0;

count = ovs_list_size(&ctx->xout->conj_flows);

LIST_FOR_EACH (f, list_node, &ctx->xout->conj_flows) {
match_init(&match, f, ctx->xin->wc);

/* Hide unnecessary fields. */
match.wc.masks.conj_id = 0;
match.wc.masks.recirc_id = 0;
match.wc.masks.in_port.ofp_port = 0;

struct ds s = DS_EMPTY_INITIALIZER;

match_format(&match, NULL, &s, OFP_DEFAULT_PRIORITY);
xlate_report_debug(ctx, OFT_DETAIL, "conj(%d/%d). %s",
++i, count, ds_cstr(&s));

ds_destroy(&s);
}
}

/* If tracing is enabled in 'ctx', appends a node representing 'rule' (in
* OpenFlow table 'table_id') to the trace and makes this node the parent for
Expand Down Expand Up @@ -918,6 +945,8 @@ xlate_report_table(const struct xlate_ctx *ctx, struct rule_dpif *rule,
ctx->xin->trace = &oftrace_report(ctx->xin->trace, OFT_TABLE,
ds_cstr(&s))->subs;
ds_destroy(&s);

xlate_report_conj_matches(ctx);
}

/* If tracing is enabled in 'ctx', adds an OFT_DETAIL trace node to 'ctx'
Expand Down Expand Up @@ -4653,7 +4682,8 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t in_port, uint8_t table_id,
ctx->xin->resubmit_stats,
&ctx->table_id, in_port,
may_packet_in, honor_table_miss,
ctx->xin->xcache);
ctx->xin->xcache,
&ctx->xout->conj_flows);
/* Swap back. */
if (with_ct_orig) {
tuple_swap(&ctx->xin->flow, ctx->wc);
Expand Down Expand Up @@ -7967,10 +7997,9 @@ xlate_optimize_odp_actions(struct xlate_in *xin)
enum xlate_error
xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
{
*xout = (struct xlate_out) {
.slow = 0,
.recircs = RECIRC_REFS_EMPTY_INITIALIZER,
};
xout->slow = 0;
xout->recircs = RECIRC_REFS_EMPTY_INITIALIZER;
ovs_list_init(&xout->conj_flows);

struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
struct xbridge *xbridge = xbridge_lookup(xcfg, xin->ofproto);
Expand Down Expand Up @@ -8181,7 +8210,8 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
ctx.rule = rule_dpif_lookup_from_table(
ctx.xbridge->ofproto, ctx.xin->tables_version, flow, ctx.wc,
ctx.xin->resubmit_stats, &ctx.table_id,
flow->in_port.ofp_port, true, true, ctx.xin->xcache);
flow->in_port.ofp_port, true, true, ctx.xin->xcache,
&ctx.xout->conj_flows);
if (ctx.xin->resubmit_stats) {
rule_dpif_credit_stats(ctx.rule, ctx.xin->resubmit_stats, false);
}
Expand Down Expand Up @@ -8375,6 +8405,11 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
ofpbuf_uninit(&scratch_actions);
ofpbuf_delete(ctx.encap_data);

/* Clean up 'conj_flows' as it is no longer needed. */
LIST_FOR_EACH_POP (flow, list_node, &xout->conj_flows) {
free(flow);
}

/* Make sure we return a "drop flow" in case of an error. */
if (ctx.error) {
xout->slow = 0;
Expand Down
3 changes: 3 additions & 0 deletions ofproto/ofproto-dpif-xlate.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ struct xlate_out {

/* Recirc action IDs on which references are held. */
struct recirc_refs recircs;

/* List of matching conjunctive flows. */
struct ovs_list conj_flows;
};

struct xlate_in {
Expand Down
25 changes: 18 additions & 7 deletions ofproto/ofproto-dpif.c
Original file line number Diff line number Diff line change
Expand Up @@ -4383,15 +4383,20 @@ ofproto_dpif_get_tables_version(struct ofproto_dpif *ofproto)
* a reference.
*
* 'flow' is non-const to allow for temporary modifications during the lookup.
* Any changes are restored before returning. */
* Any changes are restored before returning.
*
* 'conj_flows' is an optional parameter. If it is non-null, the matching
* conjunctive flows are appended to the list. */
static struct rule_dpif *
rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, ovs_version_t version,
uint8_t table_id, struct flow *flow,
struct flow_wildcards *wc)
struct flow_wildcards *wc,
struct ovs_list *conj_flows)
{
struct classifier *cls = &ofproto->up.tables[table_id].cls;
return rule_dpif_cast(rule_from_cls_rule(classifier_lookup(cls, version,
flow, wc)));
flow, wc,
conj_flows)));
}

void
Expand Down Expand Up @@ -4433,15 +4438,19 @@ ofproto_dpif_credit_table_stats(struct ofproto_dpif *ofproto, uint8_t table_id,
* 'in_port'. This is needed for resubmit action support.
*
* 'flow' is non-const to allow for temporary modifications during the lookup.
* Any changes are restored before returning. */
* Any changes are restored before returning.
*
* 'conj_flows' is an optional parameter. If it is non-null, the matching
* conjunctive flows are appended to the list. */
struct rule_dpif *
rule_dpif_lookup_from_table(struct ofproto_dpif *ofproto,
ovs_version_t version, struct flow *flow,
struct flow_wildcards *wc,
const struct dpif_flow_stats *stats,
uint8_t *table_id, ofp_port_t in_port,
bool may_packet_in, bool honor_table_miss,
struct xlate_cache *xcache)
struct xlate_cache *xcache,
struct ovs_list *conj_flows)
{
ovs_be16 old_tp_src = flow->tp_src, old_tp_dst = flow->tp_dst;
ofp_port_t old_in_port = flow->in_port.ofp_port;
Expand Down Expand Up @@ -4497,7 +4506,8 @@ rule_dpif_lookup_from_table(struct ofproto_dpif *ofproto,
next_id++, next_id += (next_id == TBL_INTERNAL))
{
*table_id = next_id;
rule = rule_dpif_lookup_in_table(ofproto, version, next_id, flow, wc);
rule = rule_dpif_lookup_in_table(ofproto, version, next_id, flow, wc,
conj_flows);
if (stats) {
struct oftable *tbl = &ofproto->up.tables[next_id];
unsigned long orig;
Expand Down Expand Up @@ -6680,7 +6690,8 @@ ofproto_dpif_add_internal_flow(struct ofproto_dpif *ofproto,

rule = rule_dpif_lookup_in_table(ofproto,
ofproto_dpif_get_tables_version(ofproto),
TBL_INTERNAL, &match->flow, &match->wc);
TBL_INTERNAL, &match->flow, &match->wc,
NULL);
if (rule) {
*rulep = &rule->up;
} else {
Expand Down
3 changes: 2 additions & 1 deletion ofproto/ofproto-dpif.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ struct rule_dpif *rule_dpif_lookup_from_table(struct ofproto_dpif *,
ofp_port_t in_port,
bool may_packet_in,
bool honor_table_miss,
struct xlate_cache *);
struct xlate_cache *,
struct ovs_list *conj_flows);

void rule_dpif_credit_stats(struct rule_dpif *,
const struct dpif_flow_stats *, bool);
Expand Down
11 changes: 11 additions & 0 deletions tests/classifier.at
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,17 @@ for src in 0 1 2 3 4 5 6 7; do
])
done
done
dnl Check detailed output for conjunctive match.
AT_CHECK([ovs-appctl ofproto/trace br0 "in_port=1,dl_type=0x0800,nw_src=10.0.0.7,nw_dst=10.0.0.5" | grep conj], [0], [dnl
0. conj_id=1, priority 32768
-> conj(1/2). ip,nw_src=10.0.0.7,nw_dst=0.0.0.0,nw_frag=no
-> conj(2/2). ip,nw_src=0.0.0.0,nw_dst=10.0.0.5,nw_frag=no
])
AT_CHECK([ovs-appctl ofproto/trace br0 "in_port=1,dl_type=0x0800,nw_src=10.0.0.1,nw_dst=10.0.0.5" | grep conj], [0], [dnl
0. conj_id=1, priority 32768
-> conj(1/2). ip,nw_src=10.0.0.1,nw_dst=0.0.0.0,nw_frag=no
-> conj(2/2). ip,nw_src=0.0.0.0,nw_dst=10.0.0.5,nw_frag=no
])
OVS_VSWITCHD_STOP
AT_CLEANUP

Expand Down
8 changes: 4 additions & 4 deletions tests/test-classifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ compare_classifiers(struct classifier *cls, size_t n_invisible_rules,
/* This assertion is here to suppress a GCC 4.9 array-bounds warning */
ovs_assert(cls->n_tries <= CLS_MAX_TRIES);

cr0 = classifier_lookup(cls, version, &flow, &wc);
cr0 = classifier_lookup(cls, version, &flow, &wc, NULL);
cr1 = tcls_lookup(tcls, &flow);
assert((cr0 == NULL) == (cr1 == NULL));
if (cr0 != NULL) {
Expand All @@ -454,7 +454,7 @@ compare_classifiers(struct classifier *cls, size_t n_invisible_rules,
/* Make sure the rule should have been visible. */
assert(cls_rule_visible_in_version(cr0, version));
}
cr2 = classifier_lookup(cls, version, &flow, NULL);
cr2 = classifier_lookup(cls, version, &flow, NULL, NULL);
assert(cr2 == cr0);
}
}
Expand Down Expand Up @@ -1370,10 +1370,10 @@ lookup_classifier(void *aux_)
if (aux->use_wc) {
flow_wildcards_init_catchall(&wc);
cr = classifier_lookup(aux->cls, version, &aux->lookup_flows[x],
&wc);
&wc, NULL);
} else {
cr = classifier_lookup(aux->cls, version, &aux->lookup_flows[x],
NULL);
NULL, NULL);
}
if (cr) {
hits++;
Expand Down

0 comments on commit 6cf493c

Please sign in to comment.