On Wed, Feb 23, 2022 at 2:44 PM Numan Siddique <[email protected]> wrote:
>
> /
>
> On Wed, Feb 9, 2022 at 12:56 PM Han Zhou <[email protected]> wrote:
> >
> > To avoid reprocessing the lflow when a referenced address set has new
> > addresses added, this patch generates a fake address set that only
> > contains the added addresses for flow generation, and then eliminates
> > the flows that are not related to the newly added addresses.
> >
> > Scale test shows obvious performance gains because the time complexity
> > changed from O(n) to O(1). The bigger the size of address set, the more
> > CPU savings. With the AS size of 10k, the test shows ~40x speed up.
> >
> > Test setup:
> > CPU: Intel(R) Core(TM) i9-7920X CPU @ 2.90GHz.
> > 5 ACL all referencing an address set of 10,000 IPs.
> >
> > Measure the time spent by ovn-controller for handling one IP
> > addition from the address set.
> >
> > Before: ~400ms
> > After: 11-12ms
> >
> > Signed-off-by: Han Zhou <[email protected]>
>
> Hi Han,
>
> I found a couple of small typos which you can consider fixing while
> applying the patches.
>
> Please see the patch 0 which has my Ack.
>
> Thanks
> Numan
>
> > ---
> > controller/lflow-conj-ids.c | 20 ++
> > controller/lflow-conj-ids.h | 1 +
> > controller/lflow.c | 360 ++++++++++++++++++++++++++++++++++--
> > controller/ovn-controller.c | 8 +
> > include/ovn/expr.h | 1 +
> > lib/expr.c | 2 +-
> > tests/ovn-controller.at | 41 ++--
> > 7 files changed, 397 insertions(+), 36 deletions(-)
> >
> > diff --git a/controller/lflow-conj-ids.c b/controller/lflow-conj-ids.c
> > index bfe63862a..372391e5a 100644
> > --- a/controller/lflow-conj-ids.c
> > +++ b/controller/lflow-conj-ids.c
> > @@ -157,6 +157,26 @@ lflow_conj_ids_alloc_specified(struct conj_ids
*conj_ids,
> > return true;
> > }
> >
> > +/* Find and return the start id that is allocated to the logical flow.
Return
> > + * 0 if not found. */
> > +uint32_t
> > +lflow_conj_ids_find(struct conj_ids *conj_ids, const struct uuid
*lflow_uuid)
> > +{
> > + struct lflow_conj_node *lflow_conj;
> > + bool found = false;
> > + HMAP_FOR_EACH_WITH_HASH (lflow_conj, hmap_node,
uuid_hash(lflow_uuid),
> > + &conj_ids->lflow_conj_ids) {
> > + if (uuid_equals(&lflow_conj->lflow_uuid, lflow_uuid)) {
> > + found = true;
> > + break;
> > + }
> > + }
> > + if (!found) {
> > + return 0;
> > + }
> > + return lflow_conj->start_conj_id;
> > +}
> > +
> > /* Frees the conjunction IDs used by lflow_uuid. */
> > void
> > lflow_conj_ids_free(struct conj_ids *conj_ids, const struct uuid
*lflow_uuid)
> > diff --git a/controller/lflow-conj-ids.h b/controller/lflow-conj-ids.h
> > index 6da0a612c..a1f7a95a5 100644
> > --- a/controller/lflow-conj-ids.h
> > +++ b/controller/lflow-conj-ids.h
> > @@ -35,6 +35,7 @@ bool lflow_conj_ids_alloc_specified(struct conj_ids *,
> > const struct uuid *lflow_uuid,
> > uint32_t start_conj_id, uint32_t
n_conjs);
> > void lflow_conj_ids_free(struct conj_ids *, const struct uuid
*lflow_uuid);
> > +uint32_t lflow_conj_ids_find(struct conj_ids *, const struct uuid
*lflow_uuid);
> > void lflow_conj_ids_init(struct conj_ids *);
> > void lflow_conj_ids_destroy(struct conj_ids *);
> > void lflow_conj_ids_clear(struct conj_ids *);
> > diff --git a/controller/lflow.c b/controller/lflow.c
> > index 9b1184c0e..658b3109b 100644
> > --- a/controller/lflow.c
> > +++ b/controller/lflow.c
> > @@ -74,6 +74,19 @@ struct condition_aux {
> > struct lflow_resource_ref *lfrr;
> > };
> >
> > +static struct expr *
> > +convert_match_to_expr(const struct sbrec_logical_flow *,
> > + const struct sbrec_datapath_binding *,
> > + struct expr **prereqs, const struct shash
*addr_sets,
> > + const struct shash *port_groups,
> > + struct lflow_resource_ref *, bool
*pg_addr_set_ref);
> > +static void
> > +add_matches_to_flow_table(const struct sbrec_logical_flow *,
> > + const struct sbrec_datapath_binding *,
> > + struct hmap *matches, uint8_t ptable,
> > + uint8_t output_ptable, struct ofpbuf
*ovnacts,
> > + bool ingress, struct lflow_ctx_in *,
> > + struct lflow_ctx_out *);
> > static void
> > consider_logical_flow(const struct sbrec_logical_flow *lflow,
> > struct hmap *dhcp_opts, struct hmap *dhcpv6_opts,
> > @@ -513,6 +526,262 @@ as_info_from_expr_const(const char *as_name,
const union expr_constant *c,
> > return true;
> > }
> >
> > +/* Parses the lflow regarding the changed address set 'as_name', and
gererates/
> s/gererates/generates
>
Ack.
>
> > + * ovs flows for the newly added addresses in 'as_diff_added' only. It
is
> > + * similar to consider_logical_flow__, with the below differences:
> > + *
> > + * - It has one more arg 'as_ref_count' to deduce how many flows are
expected
> > + * to be added.
> > + * - It uses a small fake address set that contains only the added
addresses
> > + * to replace the original address set temporarily and restores it
after
> > + * parsing.
> > + * - It doesn't check or touch lflow-cache, because lflow-cache is
disabled
> > + * when address-sets/port-groups are used.
> > + * - It doesn't check non-local lports because it should have been
checked
> > + * when the lflow is initially parsed, and if it is non-local and
skipped
> > + * then it wouldn't have the address set parsed and referenced.
> > + *
> > + * Because of these differences, it is just cleaner to keep it as a
separate
> > + * function. */
> > +static bool
> > +consider_lflow_for_added_as_ips__(
> > + const struct sbrec_logical_flow *lflow,
> > + const struct sbrec_datapath_binding *dp,
> > + const char *as_name,
> > + size_t as_ref_count,
> > + const struct expr_constant_set *as_diff_added,
> > + struct hmap *dhcp_opts,
> > + struct hmap *dhcpv6_opts,
> > + struct hmap *nd_ra_opts,
> > + struct controller_event_options
*controller_event_opts,
> > + struct lflow_ctx_in *l_ctx_in,
> > + struct lflow_ctx_out *l_ctx_out)
> > +{
> > + bool handled = true;
> > + if (!get_local_datapath(l_ctx_in->local_datapaths,
dp->tunnel_key)) {
> > + VLOG_DBG("Skip lflow "UUID_FMT" for non-local datapath
%"PRId64,
> > + UUID_ARGS(&lflow->header_.uuid), dp->tunnel_key);
> > + return true;
> > + }
> > +
> > + /* Determine translation of logical table IDs to physical table
IDs. */
> > + bool ingress = !strcmp(lflow->pipeline, "ingress");
> > +
> > + /* Determine translation of logical table IDs to physical table
IDs. */
> > + uint8_t first_ptable = (ingress
> > + ? OFTABLE_LOG_INGRESS_PIPELINE
> > + : OFTABLE_LOG_EGRESS_PIPELINE);
> > + uint8_t ptable = first_ptable + lflow->table_id;
> > + uint8_t output_ptable = (ingress
> > + ? OFTABLE_REMOTE_OUTPUT
> > + : OFTABLE_SAVE_INPORT);
> > +
> > + uint64_t ovnacts_stub[1024 / 8];
> > + struct ofpbuf ovnacts = OFPBUF_STUB_INITIALIZER(ovnacts_stub);
> > + struct ovnact_parse_params pp = {
> > + .symtab = &symtab,
> > + .dhcp_opts = dhcp_opts,
> > + .dhcpv6_opts = dhcpv6_opts,
> > + .nd_ra_opts = nd_ra_opts,
> > + .controller_event_opts = controller_event_opts,
> > +
> > + .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS,
> > + .n_tables = LOG_PIPELINE_LEN,
> > + .cur_ltable = lflow->table_id,
> > + };
> > + struct expr *prereqs = NULL;
> > + char *error;
> > +
> > + error = ovnacts_parse_string(lflow->actions, &pp, &ovnacts,
&prereqs);
> > + if (error) {
> > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> > + VLOG_WARN_RL(&rl, "error parsing actions \"%s\": %s",
> > + lflow->actions, error);
> > + free(error);
> > + ovnacts_free(ovnacts.data, ovnacts.size);
> > + ofpbuf_uninit(&ovnacts);
> > + return true;
> > + }
> > +
> > + struct lookup_port_aux aux = {
> > + .sbrec_multicast_group_by_name_datapath
> > + = l_ctx_in->sbrec_multicast_group_by_name_datapath,
> > + .sbrec_port_binding_by_name =
l_ctx_in->sbrec_port_binding_by_name,
> > + .dp = dp,
> > + .lflow = lflow,
> > + .lfrr = l_ctx_out->lfrr,
> > + };
> > + struct condition_aux cond_aux = {
> > + .sbrec_port_binding_by_name =
l_ctx_in->sbrec_port_binding_by_name,
> > + .dp = dp,
> > + .chassis = l_ctx_in->chassis,
> > + .active_tunnels = l_ctx_in->active_tunnels,
> > + .lflow = lflow,
> > + .lfrr = l_ctx_out->lfrr,
> > + };
> > +
> > + struct hmap matches = HMAP_INITIALIZER(&matches);
> > + const struct expr_constant_set *fake_as = as_diff_added;
> > + struct expr_constant_set *new_fake_as = NULL;
> > + struct in6_addr dummy_ip;
> > + bool has_dummy_ip = false;
> > + ovs_assert(as_diff_added->n_values);
> > +
> > + /* When there is only 1 element, we append a dummy address and
create a
> > + * fake address set with 2 elements, so that the lflow parsing
would
> > + * generate exactly the same format of flows as it would when
parsing with
> > + * the original address set. */
> > + if (as_diff_added->n_values == 1) {
> > + new_fake_as = xzalloc(sizeof *new_fake_as);
> > + new_fake_as->values = xzalloc(sizeof *new_fake_as->values * 2);
> > + new_fake_as->n_values = 2;
> > + new_fake_as->values[0] = new_fake_as->values[1] =
> > + as_diff_added->values[0];
> > + /* Make a dummy ip that is different from the real one. */
> > + new_fake_as->values[1].value.u8_val++;
> > + dummy_ip = new_fake_as->values[1].value.ipv6;
> > + has_dummy_ip = true;
> > + fake_as = new_fake_as;
> > + }
> > +
> > + /* Temporarily replace the address set in addr_sets with the
fake_as, so
> > + * that the cost of lflow parsing is related to the delta but not
the
> > + * original size of the address set. It is possible that there are
other
> > + * address sets used by this logical flow and their size can be
big. In
> > + * such case the parsing cost is still high. In practice, big
address
> > + * sets are likely to be updated more frequently that small
address sets,
> > + * so this approach should still be effetive overall.
>
> s/effetive/effective
Ack.
All fixed. Thanks Numan.
>
> > + *
> > + * XXX: if necessary, we can optimize this by checking all the
address set
> > + * references in this lflow, and replace all the "big" address
sets with a
> > + * small faked one. */
> > + struct expr_constant_set *real_as =
> > + shash_replace((struct shash *)l_ctx_in->addr_sets, as_name,
fake_as);
> > + /* We are here because of the address set update, so it must be
found. */
> > + ovs_assert(real_as);
> > +
> > + struct expr *expr = convert_match_to_expr(lflow, dp, &prereqs,
> > + l_ctx_in->addr_sets,
> > + l_ctx_in->port_groups,
> > + l_ctx_out->lfrr, NULL);
> > + shash_replace((struct shash *)l_ctx_in->addr_sets, as_name,
real_as);
> > + if (new_fake_as) {
> > + expr_constant_set_destroy(new_fake_as);
> > + free(new_fake_as);
> > + }
> > + if (!expr) {
> > + goto done;
> > + }
> > +
> > + expr = expr_evaluate_condition(expr, is_chassis_resident_cb,
> > + &cond_aux);
> > + expr = expr_normalize(expr);
> > +
> > + uint32_t start_conj_id = 0;
> > + uint32_t n_conjs = 0;
> > + n_conjs = expr_to_matches(expr, lookup_port_cb, &aux, &matches);
> > + if (hmap_is_empty(&matches)) {
> > + VLOG_DBG("lflow "UUID_FMT" matches are empty, skip",
> > + UUID_ARGS(&lflow->header_.uuid));
> > + goto done;
> > + }
> > +
> > + /* Discard the matches unrelated to the added addresses in the AS
> > + * 'as_name'. */
> > + struct expr_match *m, *m_next;
> > + HMAP_FOR_EACH_SAFE (m, m_next, hmap_node, &matches) {
> > + if (!m->as_name || strcmp(m->as_name, as_name) ||
> > + (has_dummy_ip && !memcmp(&m->as_ip, &dummy_ip, sizeof
dummy_ip))) {
> > + hmap_remove(&matches, &m->hmap_node);
> > + expr_match_destroy(m);
> > + continue;
> > + }
> > + }
> > +
> > + /* The number of matches generated by the new addresses should
match the
> > + * number of items in the as_diff_added and the reference count of
the AS
> > + * in this lflow. Otherwise, it means we hit some complex/corner
cases that
> > + * the generated matches can't be mapped from the items in the
> > + * as_diff_added. So we need to fall back to reprocessing the
lflow.
> > + */
> > + if (hmap_count(&matches) != as_ref_count *
as_diff_added->n_values) {
> > + VLOG_DBG("lflow "UUID_FMT", addrset %s: Generated flows count "
> > + "(%"PRIuSIZE") " "doesn't match added addresses count
"
> > + "(%"PRIuSIZE") and ref_count (%"PRIuSIZE"). "
> > + "Need reprocessing.",
> > + UUID_ARGS(&lflow->header_.uuid), as_name,
> > + hmap_count(&matches), as_diff_added->n_values,
as_ref_count);
> > + handled = false;
> > + goto done;
> > + }
> > + if (n_conjs) {
> > + start_conj_id = lflow_conj_ids_find(l_ctx_out->conj_ids,
> > + &lflow->header_.uuid);
> > + if (!start_conj_id) {
> > + VLOG_DBG("lflow "UUID_FMT" didn't have conjunctions. "
> > + "Need reprocessing",
UUID_ARGS(&lflow->header_.uuid));
> > + handled = false;
> > + goto done;
> > + }
> > + expr_matches_prepare(&matches, start_conj_id - 1);
> > + }
> > + add_matches_to_flow_table(lflow, dp, &matches, ptable,
output_ptable,
> > + &ovnacts, ingress, l_ctx_in, l_ctx_out);
> > +done:
> > + expr_destroy(prereqs);
> > + ovnacts_free(ovnacts.data, ovnacts.size);
> > + ofpbuf_uninit(&ovnacts);
> > + expr_destroy(expr);
> > + expr_matches_destroy(&matches);
> > + return handled;
> > +}
> > +
> > +static bool
> > +consider_lflow_for_added_as_ips(
> > + const struct sbrec_logical_flow *lflow,
> > + const char *as_name,
> > + size_t as_ref_count,
> > + const struct expr_constant_set *as_diff_added,
> > + struct hmap *dhcp_opts,
> > + struct hmap *dhcpv6_opts,
> > + struct hmap *nd_ra_opts,
> > + struct controller_event_options
*controller_event_opts,
> > + struct lflow_ctx_in *l_ctx_in,
> > + struct lflow_ctx_out *l_ctx_out)
> > +{
> > + const struct sbrec_logical_dp_group *dp_group =
lflow->logical_dp_group;
> > + const struct sbrec_datapath_binding *dp = lflow->logical_datapath;
> > +
> > + if (!dp_group && !dp) {
> > + VLOG_DBG("lflow "UUID_FMT" has no datapath binding, skip",
> > + UUID_ARGS(&lflow->header_.uuid));
> > + return true;
> > + }
> > + ovs_assert(!dp_group || !dp);
> > +
> > + if (dp) {
> > + return consider_lflow_for_added_as_ips__(lflow, dp, as_name,
> > + as_ref_count,
as_diff_added,
> > + dhcp_opts,
dhcpv6_opts,
> > + nd_ra_opts,
> > + controller_event_opts,
> > + l_ctx_in, l_ctx_out);
> > + }
> > + for (size_t i = 0; dp_group && i < dp_group->n_datapaths; i++) {
> > + if (!consider_lflow_for_added_as_ips__(lflow,
dp_group->datapaths[i],
> > + as_name, as_ref_count,
> > + as_diff_added,
dhcp_opts,
> > + dhcpv6_opts, nd_ra_opts,
> > + controller_event_opts,
l_ctx_in,
> > + l_ctx_out)) {
> > + return false;
> > + }
> > + }
> > + return true;
> > +}
> > +
> > +/* Check if an address set update can be handled without reprocessing
the
> > + * lflow. */
> > static bool
> > as_update_can_be_handled(const char *as_name, struct addr_set_diff
*as_diff,
> > struct lflow_ctx_in *l_ctx_in)
> > @@ -582,19 +851,18 @@ as_update_can_be_handled(const char *as_name,
struct addr_set_diff *as_diff,
> > * It could have been combined as:
> > *
> > * ip.src == $as1 && tcp.dst == {p1, p2, p3, p4}
> > + *
> > + * Note: addresses additions still can be processed
incrementally in
> > + * this case, although deletions cannot.
> > */
> > bool
> > lflow_handle_addr_set_update(const char *as_name,
> > struct addr_set_diff *as_diff,
> > - struct lflow_ctx_in *l_ctx_in OVS_UNUSED,
> > + struct lflow_ctx_in *l_ctx_in,
> > struct lflow_ctx_out *l_ctx_out,
> > bool *changed)
> > {
> > - if (as_diff->added) {
> > - return false;
> > - }
> > - ovs_assert(as_diff->deleted);
> > -
> > + ovs_assert(as_diff->added || as_diff->deleted);
> > if (!as_update_can_be_handled(as_name, as_diff, l_ctx_in)) {
> > return false;
> > }
> > @@ -609,6 +877,31 @@ lflow_handle_addr_set_update(const char *as_name,
> >
> > *changed = false;
> >
> > + struct hmap dhcp_opts = HMAP_INITIALIZER(&dhcp_opts);
> > + struct hmap dhcpv6_opts = HMAP_INITIALIZER(&dhcpv6_opts);
> > + struct hmap nd_ra_opts = HMAP_INITIALIZER(&nd_ra_opts);
> > + struct controller_event_options controller_event_opts;
> > +
> > + if (as_diff->added) {
> > + const struct sbrec_dhcp_options *dhcp_opt_row;
> > + SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row,
> > +
l_ctx_in->dhcp_options_table) {
> > + dhcp_opt_add(&dhcp_opts, dhcp_opt_row->name,
dhcp_opt_row->code,
> > + dhcp_opt_row->type);
> > + }
> > +
> > + const struct sbrec_dhcpv6_options *dhcpv6_opt_row;
> > + SBREC_DHCPV6_OPTIONS_TABLE_FOR_EACH(dhcpv6_opt_row,
> > +
l_ctx_in->dhcpv6_options_table) {
> > + dhcp_opt_add(&dhcpv6_opts, dhcpv6_opt_row->name,
> > + dhcpv6_opt_row->code, dhcpv6_opt_row->type);
> > + }
> > +
> > + nd_ra_opts_init(&nd_ra_opts);
> > + controller_event_opts_init(&controller_event_opts);
> > + }
> > +
> > + bool ret = true;
> > struct lflow_ref_list_node *lrln;
> > HMAP_FOR_EACH (lrln, hmap_node, &rlfn->lflow_uuids) {
> > if (lflows_processed_find(l_ctx_out->lflows_processed,
> > @@ -617,21 +910,56 @@ lflow_handle_addr_set_update(const char *as_name,
> > UUID_ARGS(&lrln->lflow_uuid));
> > continue;
> > }
> > - for (size_t i = 0; i < as_diff->deleted->n_values; i++) {
> > - union expr_constant *c = &as_diff->deleted->values[i];
> > + const struct sbrec_logical_flow *lflow =
> > +
sbrec_logical_flow_table_get_for_uuid(l_ctx_in->logical_flow_table,
> > + &lrln->lflow_uuid);
> > + if (!lflow) {
> > + /* lflow deletion should be handled in the corresponding
input
> > + * handler, so we can skip here. */
> > + VLOG_DBG("lflow "UUID_FMT" not found while handling
updates of "
> > + "address set %s, skip.",
> > + UUID_ARGS(&lrln->lflow_uuid), as_name);
> > + continue;
> > + }
> > + *changed = true;
> > +
> > + if (as_diff->deleted) {
> > struct addrset_info as_info;
> > - if (!as_info_from_expr_const(as_name, c, &as_info)) {
> > - continue;
> > + for (size_t i = 0; i < as_diff->deleted->n_values; i++) {
> > + union expr_constant *c = &as_diff->deleted->values[i];
> > + if (!as_info_from_expr_const(as_name, c, &as_info)) {
> > + continue;
> > + }
> > + if
(!ofctrl_remove_flows_for_as_ip(l_ctx_out->flow_table,
> > + &lrln->lflow_uuid,
&as_info,
> > + lrln->ref_count)) {
> > + ret = false;
> > + goto done;
> > + }
> > }
> > - if (!ofctrl_remove_flows_for_as_ip(l_ctx_out->flow_table,
> > - &lrln->lflow_uuid,
&as_info,
> > - lrln->ref_count)) {
> > - return false;
> > + }
> > +
> > + if (as_diff->added) {
> > + if (!consider_lflow_for_added_as_ips(lflow, as_name,
> > + lrln->ref_count,
> > + as_diff->added,
&dhcp_opts,
> > + &dhcpv6_opts,
&nd_ra_opts,
> > +
&controller_event_opts,
> > + l_ctx_in, l_ctx_out))
{
> > + ret = false;
> > + goto done;
> > }
> > - *changed = true;
> > }
> > }
> > - return true;
> > +
> > +done:
> > + if (as_diff->added) {
> > + dhcp_opts_destroy(&dhcp_opts);
> > + dhcp_opts_destroy(&dhcpv6_opts);
> > + nd_ra_opts_destroy(&nd_ra_opts);
> > + controller_event_opts_destroy(&controller_event_opts);
> > + }
> > + return ret;
> > }
> >
> > bool
> > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > index 5119a3277..b8cd11378 100644
> > --- a/controller/ovn-controller.c
> > +++ b/controller/ovn-controller.c
> > @@ -3263,6 +3263,14 @@ main(int argc, char *argv[])
> > engine_add_input(&en_pflow_output, &en_ovs_open_vswitch, NULL);
> > engine_add_input(&en_pflow_output, &en_ovs_bridge, NULL);
> >
> > + /* Keep en_addr_sets before en_runtime_data because
> > + * lflow_output_runtime_data_handler may *partially* reprocess a
lflow when
> > + * the lflow is attached to a DP group and a new DP in that DP
group is
> > + * added locally, i.e. reprocessing the lflow for the new DP only
but not
> > + * for the other DPs in the group. If we handle en_addr_sets after
this,
> > + * incrementally processing an updated address set for the added
IPs may
> > + * end up adding redundant flows/conjunctions for the lflow agaist
the new
> > + * DP because it has been processed on the DP already. */
> > engine_add_input(&en_lflow_output, &en_addr_sets,
> > lflow_output_addr_sets_handler);
> > engine_add_input(&en_lflow_output, &en_port_groups,
> > diff --git a/include/ovn/expr.h b/include/ovn/expr.h
> > index cb3baf001..3b141b034 100644
> > --- a/include/ovn/expr.h
> > +++ b/include/ovn/expr.h
> > @@ -484,6 +484,7 @@ uint32_t expr_to_matches(const struct expr *,
> > unsigned int *portp),
> > const void *aux,
> > struct hmap *matches);
> > +void expr_match_destroy(struct expr_match *);
> > void expr_matches_destroy(struct hmap *matches);
> > size_t expr_matches_prepare(struct hmap *matches, uint32_t
conj_id_ofs);
> > void expr_matches_print(const struct hmap *matches, FILE *);
> > diff --git a/lib/expr.c b/lib/expr.c
> > index 2c178d87f..47ef6108e 100644
> > --- a/lib/expr.c
> > +++ b/lib/expr.c
> > @@ -3054,7 +3054,7 @@ expr_match_new(const struct match *m, uint8_t
clause, uint8_t n_clauses,
> > return match;
> > }
> >
> > -static void
> > +void
> > expr_match_destroy(struct expr_match *match)
> > {
> > free(match->as_name);
> > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> > index b5f19b442..260986f15 100644
> > --- a/tests/ovn-controller.at
> > +++ b/tests/ovn-controller.at
> > @@ -904,7 +904,7 @@
priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3
actions=dr
> > done
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[10
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[2
> > ])
> >
> > # Remove the IPs from as1, 1 IP each time.
> > @@ -955,7 +955,8 @@
priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.1.3
actions=dr
> > done
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[10
> > +# When change from 0 to 2, still reprocessing.
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[1
> > ])
> >
> > # Add and remove IPs at the same time.
> > @@ -973,7 +974,7 @@ AT_CHECK([ovs-ofctl dump-flows br-int table=44 |
grep -c 10\.0\.0\.22], [0], [1
> > AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.10],
[1], [ignore])
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[1
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[0
> > ])
> >
> > # Add 1 and remove 2
> > @@ -988,7 +989,7 @@ AT_CHECK([ovs-ofctl dump-flows br-int table=44 |
grep -c 10\.0\.0\.10], [0], [1
> > ])
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[1
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[0
> > ])
> >
> > # Add 1 and remove 1
> > @@ -1002,7 +1003,7 @@ AT_CHECK([ovs-ofctl dump-flows br-int table=44 |
grep -c 10\.0\.0\.21], [0], [1
> > AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.10],
[1], [ignore])
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[1
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[0
> > ])
> >
> > # Add 2 and remove 2
> > @@ -1019,7 +1020,7 @@ AT_CHECK([ovs-ofctl dump-flows br-int table=44 |
grep 10\.0\.0\.8], [1], [ignore
> > AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.9],
[1], [ignore])
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[1
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[0
> > ])
> >
> > OVN_CLEANUP([hv1])
> > @@ -1093,7 +1094,7 @@
priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=333
actions=conjun
> > done
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[10
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[2
> > ])
> >
> > # Remove the IPs from as1, 1 IP each time.
> > @@ -1150,7 +1151,7 @@
priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=333
actions=conjun
> > done
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[10
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[1
> > ])
> >
> > # Add and remove IPs at the same time.
> > @@ -1168,7 +1169,7 @@ AT_CHECK([ovs-ofctl dump-flows br-int table=44 |
grep -c 10\.0\.0\.22], [0], [1
> > AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.10],
[1], [ignore])
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[1
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[0
> > ])
> >
> > # Add 1 and remove 2
> > @@ -1183,7 +1184,7 @@ AT_CHECK([ovs-ofctl dump-flows br-int table=44 |
grep -c 10\.0\.0\.10], [0], [1
> > ])
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[1
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[0
> > ])
> >
> > # Add 1 and remove 1
> > @@ -1197,7 +1198,7 @@ AT_CHECK([ovs-ofctl dump-flows br-int table=44 |
grep -c 10\.0\.0\.21], [0], [1
> > AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.10],
[1], [ignore])
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[1
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[0
> > ])
> >
> > # Add 2 and remove 2
> > @@ -1214,7 +1215,7 @@ AT_CHECK([ovs-ofctl dump-flows br-int table=44 |
grep 10\.0\.0\.8], [1], [ignore
> > AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.9],
[1], [ignore])
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[1
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[0
> > ])
> >
> > OVN_CLEANUP([hv1])
> > @@ -1288,7 +1289,7 @@
priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3
actions=co
> > done
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[10
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[2
> > ])
> >
> > # Remove the IPs from as1 and as2, 1 IP each time.
> > @@ -1341,7 +1342,7 @@
priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3,nw_dst=10.
> > done
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[9
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[1
> > ])
> >
> > # Add 1 more IP back to as2
> > @@ -1639,7 +1640,7 @@
priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3
actions=co
> > done
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[10
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[2
> > ])
> >
> > # Remove the IPs from as1, 1 IP each time.
> > @@ -1697,7 +1698,7 @@
priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.1.3
actions=co
> > done
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[10
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[1
> > ])
> >
> > # Test the case when there are two references to the same AS but one
of the
> > @@ -1842,7 +1843,7 @@
priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=202
actions=conjun
> > ])
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[2
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[0
> > ])
> >
> > # Remove those 2 IPs from each AS, should return to the initial state
> > @@ -1870,6 +1871,8 @@
priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=202
actions=conjun
> > ])
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > +# Because of the combined conjunction, AS cannot be tracked for the
flow for
> > +# 10.0.0.33, so removing would trigger reprocessing.
> > AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[2
> > ])
> >
> > @@ -1926,7 +1929,7 @@
priority=1100,reg15=0x$port_key,metadata=0x$dp_key,dl_src=aa:aa:aa:aa:aa:03
acti
> > done
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[5
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[2
> > ])
> >
> > # Remove the MACs from as1.
> > @@ -2007,7 +2010,7 @@
priority=1100,ipv6,reg15=0x$port_key,metadata=0x$dp_key,ipv6_src=ff::3
actions=d
> > done
> >
> > reprocess_count_new=$(read_counter consider_logical_flow)
> > -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[5
> > +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0],
[2
> > ])
> >
> > # Remove the IPs from as1, 1 IP each time.
> > --
> > 2.30.2
> >
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
