The previous email was blocked by ML because of the embedded picture, so I replaced it with a link and resend.
On Thu, Sep 3, 2020 at 2:17 PM Han Zhou <[email protected]> wrote: > > > > On Thu, Sep 3, 2020 at 6:00 AM Mark Michelson <[email protected]> wrote: >> >> On 9/2/20 2:55 PM, Han Zhou wrote: >> > >> > >> > On Wed, Sep 2, 2020 at 8:53 AM Mark Michelson <[email protected] >> > <mailto:[email protected]>> wrote: >> > > >> > > I think this can be simplified some. >> > > >> > > Specifically, I don't think every ovn_flow needs to have a corresponding >> > > sb_flow_ref created. The only time that sb_flow_ref logic is needed is >> > > when OF flows get appended to an existing flow. In every other case, >> > either >> > > >> > > a) A southbound logical flow UUID corresponds to exactly one OF flow, or >> > > b) A southbound object UUID corresponds to multiple OF flows, but they >> > > are unrelated to each other. >> > > >> > > So why not only create sb_flow_refs in the ofctrl_add_or_append_flow() >> > > case? This way, you save a lot of unnecessary memory and processing >> > > overhead during "normal" cases. >> > >> > Thanks for the review. Sorry that I didn't get your point completely. >> > You are right that currently there is only one scenario, the >> > ofctrl_add_or_append_flow(), that will lead to one openflow being >> > referenced by multiple SB lflows. However, to handle such case the data >> > structure will need to be able to represent it. The new data structure >> > with the sb_flow_ref that cross references between openflow rules and SB >> > objects is for this purpose. Due to the data structure change, the >> > add/remove operations implementation are updated accordingly. It is >> > general enough to handle both the *normal* situation and the *special* >> > situation. Are you saying that we should have different data structures >> > and different logic between *normal* and *special*? But that would only >> > make the code more complex rather than simple, unnecessarily. Or did you >> > mean something else? >> >> My main concern was with the amount of overhead required for simple >> cases. However, as you pointed out, if you special-case the conjunction >> situation by using a different structure only in that case, you risk >> having unmaintainable code. >> >> I have a secondary concern with the odd way that an sb_flow_ref could be >> part of multiple lists, and how there is a circular reference between >> sb_flow_ref and its contained ovn_flow. It is hard to visualize and >> seems easy for someone to mess up later. I did think of an alternative >> method for doing this. Here's how it would work: >> >> The desired flow table has a hashmap of sb_to_flow structures. The >> sb_to_flow struct would look exactly as you have it now, only instead of >> the "flows" field referring to a list of sb_flow_ref structures, they >> would instead refer to a list of ovn_flow structures. >> > > Thanks for clarifying. Unfortunately this wouldn't work, because a single ovn_flow can be referenced by multiple SB flows, so each ovn_flow may need to be in multiple such lists, which is impossible without a separate list node structure (i.e. we can't link a ovn_flow struct in multiple lists with just an embedded ovs_list struct in ovn_flow struct). This is why the "flows" field in sb_to_flow struct needs to refer to a separate struct "sb_flow_ref" that references ovn_flow struct, indirectly. > > The sb_flow_ref is in fact easy to visualize if you think of it as a link between a pair of ovn_flow and SB uuid. Please see the below picture (embedded in email) which is how I visualized it when designing this. It is the most effective way I could think of so far to represent the M:N relationship. If the picture helps, I can draw an ASCII version and document it in the code. > https://docs.google.com/presentation/d/12sfhFvvPS3jDSe_9B6Sd5zSnICqje4YFvzQGLLEp9eE/edit?usp=sharing > >> >> Then ovn_flow would be updated to have >> struct ovs_list sb_uuids; >> in it. >> >> This would be a list of UUIDs: >> >> struct uuid_list { >> struct uuid uuid; >> struct ovs_list list_node; >> }; >> > > Initially I was tying to do the same - creating separate nodes for uuid list, but now that we already have sb_flow_ref nodes that reference ovn_flows, instead of allocating extra nodes for uuid list, this information is combined in sb_flow_ref node. This is more efficient because only one node is required to link between a pair of ovn_flow and sb_uuid, i.e. the lists of sb_uuids in each ovn_flow and the lists of ovn_flow references in each sb_to_flow are merged, as dipicted in the above picture. This model represents the links in a more straightforward way and also saves 50% of the dynamic memory allocations. Does this address your concerns? > > Thanks, > Han > >> >> This list of UUIDs on the ovn_flow allows for you to maintain a list of >> all SB UUIDs that refer to this particular flow. >> >> So now the algorithm becomes: >> >> Add new ovn_flow: >> Create sb_to_flow for this SB UUID and add to hashmap. >> Create ovn_flow >> Add SB UUID to ovn_flow's sb_uuids list. >> Add ovn_flow to sb_to_flow list. >> >> Append to an ovn_flow: >> Create sb_to_flow for this SB UUID and add to hashmap. >> Find existing ovn_flow. >> Add SB UUID to existing ovn_flow's sb_uuids list. >> >> When lflow is modified or removed: >> Look up sb_to_flow structure for this lflow's UUID >> Iterate over the ovn_flows in the sb_to_flow. >> In the ovn_flow, remove the lflow's UUID from the list. >> Iterate over ovn_flow's sb_uuids, and recurse. >> Remove sb_to_flow from hashmap. >> >> >> This approach gets rid of sb_flow_ref and the odd list structures. For >> the nominal case where an ovn_flow has only one UUID in its sb_uuids >> list, it should be just as efficient as what you've implemented. The >> case of removing flows for conjunctive matches may be slightly slower >> though. What do you think? >> >> > >> > > >> > > On 8/21/20 3:16 PM, Han Zhou wrote: >> > > > When translating lflows to OVS flows, different lflows can refer to >> > same OVS >> > > > flow as a result of calling ofctrl_add_or_append_flow(), >> > particularly for >> > > > conjunction combinding. However, the implementation doesn't work with >> > > > incremental processing, because when any of the lflows are removed, >> > we rely on >> > > > the lflow's uuid to remove the OVS flow in the desired flow table. >> > Currently >> > > > only one single lflow uuid is maintained in the desired flow, so >> > removing one >> > > > of the lflows that references to the same desired flow resulted in >> > wrong >> > > > behavior: either removing flows that are used by other lflows, or >> > the existing >> > > > flows are not updated (part of the conjunction actions should be >> > removed from >> > > > the flow). >> > > > >> > > > To solve the problem, this patch maintains the cross reference >> > (M:N) between >> > > > lflows (and other sb objects) and desired flows, and handles the >> > flow removal >> > > > and updates with a flood-removal and re-add approach. >> > > > >> > > > Fixes: e659bab31a9 ("Combine conjunctions with identical matches >> > into one flow.") >> > > > Cc: Mark Michelson <[email protected] <mailto: [email protected]>> >> > > > Signed-off-by: Han Zhou <[email protected] <mailto:[email protected]>> >> > > > --- >> > > > controller/lflow.c | 103 +++++++++++------ >> > > > controller/ofctrl.c | 319 >> > +++++++++++++++++++++++++++++++++++++++++----------- >> > > > controller/ofctrl.h | 31 ++++- >> > > > tests/ovn.at <http://ovn.at> | 90 +++++++++++++++ >> > > > 4 files changed, 438 insertions(+), 105 deletions(-) >> > > > >> > > > diff --git a/controller/lflow.c b/controller/lflow.c >> > > > index 0c35b7d..6fbd36f 100644 >> > > > --- a/controller/lflow.c >> > > > +++ b/controller/lflow.c >> > > > @@ -342,41 +342,56 @@ lflow_handle_changed_flows(struct >> > lflow_ctx_in *l_ctx_in, >> > > > struct hmap nd_ra_opts = HMAP_INITIALIZER(&nd_ra_opts); >> > > > nd_ra_opts_init(&nd_ra_opts); >> > > > >> > > > - /* Handle removed flows first, and then other flows, so that when >> > > > - * the flows being added and removed have same match conditions >> > > > - * can be processed in the proper order */ >> > > > + struct controller_event_options controller_event_opts; >> > > > + controller_event_opts_init(&controller_event_opts); >> > > > + >> > > > + /* Handle flow removing first (for deleted or updated lflows), >> > and then >> > > > + * handle reprocessing or adding flows, so that when the flows >> > being >> > > > + * removed and added with same match conditions can be >> > processed in the >> > > > + * proper order */ >> > > > + >> > > > + struct hmap flood_remove_nodes = >> > HMAP_INITIALIZER(&flood_remove_nodes); >> > > > + struct ofctrl_flood_remove_node *ofrn, *next; >> > > > SBREC_LOGICAL_FLOW_TABLE_FOR_EACH_TRACKED (lflow, >> > > > >> > l_ctx_in->logical_flow_table) { >> > > > - /* Remove any flows that should be removed. */ >> > > > - if (sbrec_logical_flow_is_deleted(lflow)) { >> > > > - VLOG_DBG("handle deleted lflow "UUID_FMT, >> > > > + if (!sbrec_logical_flow_is_new(lflow)) { >> > > > + VLOG_DBG("delete lflow "UUID_FMT, >> > > > UUID_ARGS(&lflow->header_.uuid)); >> > > > - ofctrl_remove_flows(l_ctx_out->flow_table, >> > &lflow->header_.uuid); >> > > > - /* Delete entries from lflow resource reference. */ >> > > > - lflow_resource_destroy_lflow(l_ctx_out->lfrr, >> > > > - &lflow->header_.uuid); >> > > > + ofrn = xmalloc(sizeof *ofrn); >> > > > + ofrn->sb_uuid = lflow->header_.uuid; >> > > > + hmap_insert(&flood_remove_nodes, &ofrn->hmap_node, >> > > > + uuid_hash(&ofrn->sb_uuid)); >> > > > } >> > > > } >> > > > + ofctrl_flood_remove_flows(l_ctx_out->flow_table, >> > &flood_remove_nodes); >> > > > + HMAP_FOR_EACH (ofrn, hmap_node, &flood_remove_nodes) { >> > > > + /* Delete entries from lflow resource reference. */ >> > > > + lflow_resource_destroy_lflow(l_ctx_out->lfrr, &ofrn->sb_uuid); >> > > > + /* Reprocessing the lflow if the sb record is not deleted. */ >> > > > + lflow = sbrec_logical_flow_table_get_for_uuid( >> > > > + l_ctx_in->logical_flow_table, &ofrn->sb_uuid); >> > > > + if (lflow) { >> > > > + VLOG_DBG("re-add lflow "UUID_FMT, >> > > > + UUID_ARGS(&lflow->header_.uuid)); >> > > > + if (!consider_logical_flow(lflow, &dhcp_opts, >> > &dhcpv6_opts, >> > > > + &nd_ra_opts, >> > &controller_event_opts, >> > > > + l_ctx_in, l_ctx_out)) { >> > > > + ret = false; >> > > > + break; >> > > > + } >> > > > + } >> > > > + } >> > > > + HMAP_FOR_EACH_SAFE (ofrn, next, hmap_node, &flood_remove_nodes) { >> > > > + hmap_remove(&flood_remove_nodes, &ofrn->hmap_node); >> > > > + free(ofrn); >> > > > + } >> > > > + hmap_destroy(&flood_remove_nodes); >> > > > >> > > > - struct controller_event_options controller_event_opts; >> > > > - controller_event_opts_init(&controller_event_opts); >> > > > - >> > > > + /* Now handle new lflows only. */ >> > > > SBREC_LOGICAL_FLOW_TABLE_FOR_EACH_TRACKED (lflow, >> > > > >> > l_ctx_in->logical_flow_table) { >> > > > - if (!sbrec_logical_flow_is_deleted(lflow)) { >> > > > - /* Now, add/modify existing flows. If the logical >> > > > - * flow is a modification, just remove the flows >> > > > - * for this row, and then add new flows. */ >> > > > - if (!sbrec_logical_flow_is_new(lflow)) { >> > > > - VLOG_DBG("handle updated lflow "UUID_FMT, >> > > > - UUID_ARGS(&lflow->header_.uuid)); >> > > > - ofctrl_remove_flows(l_ctx_out->flow_table, >> > > > - &lflow->header_.uuid); >> > > > - /* Delete entries from lflow resource reference. */ >> > > > - lflow_resource_destroy_lflow(l_ctx_out->lfrr, >> > > > - &lflow->header_.uuid); >> > > > - } >> > > > - VLOG_DBG("handle new lflow "UUID_FMT, >> > > > + if (sbrec_logical_flow_is_new(lflow)) { >> > > > + VLOG_DBG("add lflow "UUID_FMT, >> > > > UUID_ARGS(&lflow->header_.uuid)); >> > > > if (!consider_logical_flow(lflow, &dhcp_opts, >> > &dhcpv6_opts, >> > > > &nd_ra_opts, >> > &controller_event_opts, >> > > > @@ -420,7 +435,6 @@ lflow_handle_changed_ref(enum ref_type >> > ref_type, const char *ref_name, >> > > > * when reparsing the lflows. */ >> > > > LIST_FOR_EACH (lrln, ref_list, &rlfn->ref_lflow_head) { >> > > > ovs_list_remove(&lrln->lflow_list); >> > > > - lflow_resource_destroy_lflow(l_ctx_out->lfrr, >> > &lrln->lflow_uuid); >> > > > } >> > > > >> > > > struct hmap dhcp_opts = HMAP_INITIALIZER(&dhcp_opts); >> > > > @@ -446,22 +460,32 @@ lflow_handle_changed_ref(enum ref_type >> > ref_type, const char *ref_name, >> > > > controller_event_opts_init(&controller_event_opts); >> > > > >> > > > /* Re-parse the related lflows. */ >> > > > + /* Firstly, flood remove the flows from desired flow table. */ >> > > > + struct hmap flood_remove_nodes = >> > HMAP_INITIALIZER(&flood_remove_nodes); >> > > > + struct ofctrl_flood_remove_node *ofrn, *ofrn_next; >> > > > LIST_FOR_EACH (lrln, ref_list, &rlfn->ref_lflow_head) { >> > > > + VLOG_DBG("Reprocess lflow "UUID_FMT" for resource type: %d," >> > > > + " name: %s.", >> > > > + UUID_ARGS(&lrln->lflow_uuid), >> > > > + ref_type, ref_name); >> > > > + ofctrl_flood_remove_add_node(&flood_remove_nodes, >> > &lrln->lflow_uuid); >> > > > + } >> > > > + ofctrl_flood_remove_flows(l_ctx_out->flow_table, >> > &flood_remove_nodes); >> > > > + >> > > > + /* Secondly, for each lflow that is actually removed, >> > reprocessing it. */ >> > > > + HMAP_FOR_EACH (ofrn, hmap_node, &flood_remove_nodes) { >> > > > + lflow_resource_destroy_lflow(l_ctx_out->lfrr, &ofrn->sb_uuid); >> > > > + >> > > > const struct sbrec_logical_flow *lflow = >> > > > >> > sbrec_logical_flow_table_get_for_uuid(l_ctx_in->logical_flow_table, >> > > > - &lrln->lflow_uuid); >> > > > + &ofrn->sb_uuid); >> > > > if (!lflow) { >> > > > - VLOG_DBG("Reprocess lflow "UUID_FMT" for resource >> > type: %d," >> > > > - " name: %s - not found.", >> > > > - UUID_ARGS(&lrln->lflow_uuid), >> > > > + VLOG_DBG("lflow "UUID_FMT" not found while >> > reprocessing for" >> > > > + " resource type: %d, name: %s.", >> > > > + UUID_ARGS(&ofrn->sb_uuid), >> > > > ref_type, ref_name); >> > > > continue; >> > > > } >> > > > - VLOG_DBG("Reprocess lflow "UUID_FMT" for resource type: %d," >> > > > - " name: %s.", >> > > > - UUID_ARGS(&lrln->lflow_uuid), >> > > > - ref_type, ref_name); >> > > > - ofctrl_remove_flows(l_ctx_out->flow_table, &lrln->lflow_uuid); >> > > > >> > > > if (!consider_logical_flow(lflow, &dhcp_opts, &dhcpv6_opts, >> > > > &nd_ra_opts, >> > &controller_event_opts, >> > > > @@ -471,6 +495,11 @@ lflow_handle_changed_ref(enum ref_type >> > ref_type, const char *ref_name, >> > > > } >> > > > *changed = true; >> > > > } >> > > > + HMAP_FOR_EACH_SAFE (ofrn, ofrn_next, hmap_node, >> > &flood_remove_nodes) { >> > > > + hmap_remove(&flood_remove_nodes, &ofrn->hmap_node); >> > > > + free(ofrn); >> > > > + } >> > > > + hmap_destroy(&flood_remove_nodes); >> > > > >> > > > LIST_FOR_EACH_SAFE (lrln, next, ref_list, >> > &rlfn->ref_lflow_head) { >> > > > ovs_list_remove(&lrln->ref_list); >> > > > diff --git a/controller/ofctrl.c b/controller/ofctrl.c >> > > > index 919db6d..c500f52 100644 >> > > > --- a/controller/ofctrl.c >> > > > +++ b/controller/ofctrl.c >> > > > @@ -54,8 +54,12 @@ VLOG_DEFINE_THIS_MODULE(ofctrl); >> > > > /* An OpenFlow flow. */ >> > > > struct ovn_flow { >> > > > struct hmap_node match_hmap_node; /* For match based hashing. */ >> > > > - struct hindex_node uuid_hindex_node; /* For uuid based hashing. */ >> > > > struct ovs_list list_node; /* For handling lists of flows. */ >> > > > + struct ovs_list references; /* A list of struct sb_flow_ref >> > nodes, which >> > > > + references this flow. (There >> > are cases >> > > > + that multiple SB entities share >> > the same >> > > > + desired OpenFlow flow, e.g. when >> > > > + conjunction is used.) */ >> > > > >> > > > /* Key. */ >> > > > uint8_t table_id; >> > > > @@ -63,21 +67,34 @@ struct ovn_flow { >> > > > struct minimatch match; >> > > > >> > > > /* Data. */ >> > > > - struct uuid sb_uuid; >> > > > struct ofpact *ofpacts; >> > > > size_t ofpacts_len; >> > > > uint64_t cookie; >> > > > }; >> > > > >> > > > +struct sb_to_flow { >> > > > + struct hmap_node hmap_node; /* Node in >> > > > + >> > ovn_desired_flow_table.uuid_flow_table. */ >> > > > + struct uuid sb_uuid; >> > > > + struct ovs_list flows; /* A list of struct sb_flow_ref nodes >> > that are >> > > > + referenced by the sb_uuid. */ >> > > > +}; >> > > > + >> > > > +struct sb_flow_ref { >> > > > + struct ovs_list sb_list; /* List node in ovn_flow.references. */ >> > > > + struct ovs_list flow_list; /* List node in >> > sb_to_flow.ovn_flows. */ >> > > > + struct ovn_flow *flow; >> > > > + struct uuid sb_uuid; >> > > > +}; >> > > > + >> > > > static struct ovn_flow *ovn_flow_alloc(uint8_t table_id, uint16_t >> > priority, >> > > > uint64_t cookie, >> > > > const struct match *match, >> > > > - const struct ofpbuf *actions, >> > > > - const struct uuid *sb_uuid); >> > > > + const struct ofpbuf *actions); >> > > > static uint32_t ovn_flow_match_hash(const struct ovn_flow *); >> > > > static struct ovn_flow *ovn_flow_lookup(struct hmap *flow_table, >> > > > const struct ovn_flow >> > *target, >> > > > - bool cmp_sb_uuid); >> > > > + const struct uuid *sb_uuid); >> > > > static char *ovn_flow_to_string(const struct ovn_flow *); >> > > > static void ovn_flow_log(const struct ovn_flow *, const char >> > *action); >> > > > static void ovn_flow_destroy(struct ovn_flow *); >> > > > @@ -644,13 +661,46 @@ ofctrl_recv(const struct ofp_header *oh, enum >> > ofptype type) >> > > > } >> > > > } >> > > > >> > > > +static struct sb_to_flow * >> > > > +sb_to_flow_find(struct hmap *uuid_flow_table, const struct uuid >> > *sb_uuid) >> > > > +{ >> > > > + struct sb_to_flow *stf; >> > > > + HMAP_FOR_EACH_WITH_HASH (stf, hmap_node, uuid_hash(sb_uuid), >> > > > + uuid_flow_table) { >> > > > + if (uuid_equals(sb_uuid, &stf->sb_uuid)) { >> > > > + return stf; >> > > > + } >> > > > + } >> > > > + return NULL; >> > > > +} >> > > > + >> > > > +static void >> > > > +link_flow_to_sb(struct ovn_desired_flow_table *flow_table, >> > > > + struct ovn_flow *f, const struct uuid *sb_uuid) >> > > > +{ >> > > > + struct sb_flow_ref *sfr = xmalloc(sizeof *sfr); >> > > > + sfr->flow = f; >> > > > + sfr->sb_uuid = *sb_uuid; >> > > > + ovs_list_insert(&f->references, &sfr->sb_list); >> > > > + struct sb_to_flow *stf = >> > sb_to_flow_find(&flow_table->uuid_flow_table, >> > > > + sb_uuid); >> > > > + if (!stf) { >> > > > + stf = xmalloc(sizeof *stf); >> > > > + stf->sb_uuid = *sb_uuid; >> > > > + ovs_list_init(&stf->flows); >> > > > + hmap_insert(&flow_table->uuid_flow_table, &stf->hmap_node, >> > > > + uuid_hash(sb_uuid)); >> > > > + } >> > > > + ovs_list_insert(&stf->flows, &sfr->flow_list); >> > > > +} >> > > > + >> > > > /* Flow table interfaces to the rest of ovn-controller. */ >> > > > >> > > > /* Adds a flow to 'desired_flows' with the specified 'match' and >> > 'actions' to >> > > > * the OpenFlow table numbered 'table_id' with the given >> > 'priority' and >> > > > * OpenFlow 'cookie'. The caller retains ownership of 'match' >> > and 'actions'. >> > > > * >> > > > - * The flow is also added to a hash index based on sb_uuid. >> > > > + * The flow is also linked to the sb_uuid that generates it. >> > > > * >> > > > * This just assembles the desired flow table in memory. Nothing >> > is actually >> > > > * sent to the switch until a later call to ofctrl_put(). >> > > > @@ -665,11 +715,9 @@ ofctrl_check_and_add_flow(struct >> > ovn_desired_flow_table *flow_table, >> > > > bool log_duplicate_flow) >> > > > { >> > > > struct ovn_flow *f = ovn_flow_alloc(table_id, priority, >> > cookie, match, >> > > > - actions, sb_uuid); >> > > > + actions); >> > > > >> > > > - ovn_flow_log(f, "ofctrl_add_flow"); >> > > > - >> > > > - if (ovn_flow_lookup(&flow_table->match_flow_table, f, true)) { >> > > > + if (ovn_flow_lookup(&flow_table->match_flow_table, f, sb_uuid)) { >> > > > if (log_duplicate_flow) { >> > > > static struct vlog_rate_limit rl = >> > VLOG_RATE_LIMIT_INIT(5, 5); >> > > > if (!VLOG_DROP_DBG(&rl)) { >> > > > @@ -684,31 +732,8 @@ ofctrl_check_and_add_flow(struct >> > ovn_desired_flow_table *flow_table, >> > > > >> > > > hmap_insert(&flow_table->match_flow_table, &f->match_hmap_node, >> > > > f->match_hmap_node.hash); >> > > > - hindex_insert(&flow_table->uuid_flow_table, &f->uuid_hindex_node, >> > > > - f->uuid_hindex_node.hash); >> > > > -} >> > > > - >> > > > -/* Removes a bundles of flows from the flow table. */ >> > > > -void >> > > > -ofctrl_remove_flows(struct ovn_desired_flow_table *flow_table, >> > > > - const struct uuid *sb_uuid) >> > > > -{ >> > > > - struct ovn_flow *f, *next; >> > > > - HINDEX_FOR_EACH_WITH_HASH_SAFE (f, next, uuid_hindex_node, >> > > > - uuid_hash(sb_uuid), >> > > > - &flow_table->uuid_flow_table) { >> > > > - if (uuid_equals(&f->sb_uuid, sb_uuid)) { >> > > > - ovn_flow_log(f, "ofctrl_remove_flow"); >> > > > - hmap_remove(&flow_table->match_flow_table, >> > > > - &f->match_hmap_node); >> > > > - hindex_remove(&flow_table->uuid_flow_table, >> > &f->uuid_hindex_node); >> > > > - ovn_flow_destroy(f); >> > > > - } >> > > > - } >> > > > - >> > > > - /* remove any related group and meter info */ >> > > > - ovn_extend_table_remove_desired(groups, sb_uuid); >> > > > - ovn_extend_table_remove_desired(meters, sb_uuid); >> > > > + link_flow_to_sb(flow_table, f, sb_uuid); >> > > > + ovn_flow_log(f, "ofctrl_add_flow"); >> > > > } >> > > > >> > > > void >> > > > @@ -721,6 +746,9 @@ ofctrl_add_flow(struct ovn_desired_flow_table >> > *desired_flows, >> > > > match, actions, sb_uuid, true); >> > > > } >> > > > >> > > > +/* Either add a new flow, or append actions on an existing flow. >> > If the >> > > > + * flow existed, a new link will also be created between the new >> > sb_uuid >> > > > + * and the existing flow. */ >> > > > void >> > > > ofctrl_add_or_append_flow(struct ovn_desired_flow_table >> > *desired_flows, >> > > > uint8_t table_id, uint16_t priority, >> > uint64_t cookie, >> > > > @@ -729,12 +757,10 @@ ofctrl_add_or_append_flow(struct >> > ovn_desired_flow_table *desired_flows, >> > > > const struct uuid *sb_uuid) >> > > > { >> > > > struct ovn_flow *f = ovn_flow_alloc(table_id, priority, >> > cookie, match, >> > > > - actions, sb_uuid); >> > > > - >> > > > - ovn_flow_log(f, "ofctrl_add_or_append_flow"); >> > > > + actions); >> > > > >> > > > struct ovn_flow *existing; >> > > > - existing = ovn_flow_lookup(&desired_flows->match_flow_table, >> > f, false); >> > > > + existing = ovn_flow_lookup(&desired_flows->match_flow_table, >> > f, NULL); >> > > > if (existing) { >> > > > /* There's already a flow with this particular match. >> > Append the >> > > > * action to that flow rather than adding a new flow >> > > > @@ -751,11 +777,169 @@ ofctrl_add_or_append_flow(struct >> > ovn_desired_flow_table *desired_flows, >> > > > >> > > > ofpbuf_uninit(&compound); >> > > > ovn_flow_destroy(f); >> > > > + f = existing; >> > > > } else { >> > > > hmap_insert(&desired_flows->match_flow_table, >> > &f->match_hmap_node, >> > > > f->match_hmap_node.hash); >> > > > - hindex_insert(&desired_flows->uuid_flow_table, >> > &f->uuid_hindex_node, >> > > > - f->uuid_hindex_node.hash); >> > > > + } >> > > > + link_flow_to_sb(desired_flows, f, sb_uuid); >> > > > + >> > > > + if (existing) { >> > > > + ovn_flow_log(f, "ofctrl_add_or_append_flow (append)"); >> > > > + } else { >> > > > + ovn_flow_log(f, "ofctrl_add_or_append_flow (add)"); >> > > > + } >> > > > +} >> > > > + >> > > > +/* Remove ovn_flows for the given "sb_to_flow" node in the >> > uuid_flow_table. >> > > > + * Optionally log the message for each flow that is acturally >> > removed, if >> > > > + * log_msg is not NULL. */ >> > > > +static void >> > > > +remove_flows_from_sb_to_flow(struct ovn_desired_flow_table >> > *flow_table, >> > > > + struct sb_to_flow *stf, >> > > > + const char *log_msg) >> > > > +{ >> > > > + struct sb_flow_ref *sfr, *next; >> > > > + LIST_FOR_EACH_SAFE (sfr, next, flow_list, &stf->flows) { >> > > > + ovs_list_remove(&sfr->sb_list); >> > > > + ovs_list_remove(&sfr->flow_list); >> > > > + struct ovn_flow *f = sfr->flow; >> > > > + free(sfr); >> > > > + >> > > > + if (ovs_list_is_empty(&f->references)) { >> > > > + if (log_msg) { >> > > > + ovn_flow_log(f, log_msg); >> > > > + } >> > > > + hmap_remove(&flow_table->match_flow_table, >> > > > + &f->match_hmap_node); >> > > > + ovn_flow_destroy(f); >> > > > + } >> > > > + } >> > > > + hmap_remove(&flow_table->uuid_flow_table, &stf->hmap_node); >> > > > + free(stf); >> > > > +} >> > > > + >> > > > +void >> > > > +ofctrl_remove_flows(struct ovn_desired_flow_table *flow_table, >> > > > + const struct uuid *sb_uuid) >> > > > +{ >> > > > + struct sb_to_flow *stf = >> > sb_to_flow_find(&flow_table->uuid_flow_table, >> > > > + sb_uuid); >> > > > + if (stf) { >> > > > + remove_flows_from_sb_to_flow(flow_table, stf, >> > "ofctrl_remove_flow"); >> > > > + } >> > > > + >> > > > + /* remove any related group and meter info */ >> > > > + ovn_extend_table_remove_desired(groups, sb_uuid); >> > > > + ovn_extend_table_remove_desired(meters, sb_uuid); >> > > > +} >> > > > + >> > > > +static struct ofctrl_flood_remove_node * >> > > > +flood_remove_find_node(struct hmap *flood_remove_nodes, struct >> > uuid *sb_uuid) >> > > > +{ >> > > > + struct ofctrl_flood_remove_node *ofrn; >> > > > + HMAP_FOR_EACH_WITH_HASH (ofrn, hmap_node, uuid_hash(sb_uuid), >> > > > + flood_remove_nodes) { >> > > > + if (uuid_equals(&ofrn->sb_uuid, sb_uuid)) { >> > > > + return ofrn; >> > > > + } >> > > > + } >> > > > + return NULL; >> > > > +} >> > > > + >> > > > +void >> > > > +ofctrl_flood_remove_add_node(struct hmap *flood_remove_nodes, >> > > > + const struct uuid *sb_uuid) >> > > > +{ >> > > > + struct ofctrl_flood_remove_node *ofrn = xmalloc(sizeof *ofrn); >> > > > + ofrn->sb_uuid = *sb_uuid; >> > > > + hmap_insert(flood_remove_nodes, &ofrn->hmap_node, >> > uuid_hash(sb_uuid)); >> > > > +} >> > > > + >> > > > +static void >> > > > +flood_remove_flows_for_sb_uuid(struct ovn_desired_flow_table >> > *flow_table, >> > > > + const struct uuid *sb_uuid, >> > > > + struct hmap *flood_remove_nodes) >> > > > +{ >> > > > + struct sb_to_flow *stf = >> > sb_to_flow_find(&flow_table->uuid_flow_table, >> > > > + sb_uuid); >> > > > + if (!stf) { >> > > > + return; >> > > > + } >> > > > + >> > > > + /* ovn_flows that have other references and waiting to be >> > removed. */ >> > > > + struct ovs_list to_be_removed = >> > OVS_LIST_INITIALIZER(&to_be_removed); >> > > > + >> > > > + /* Traverse all flows for the given sb_uuid. */ >> > > > + struct sb_flow_ref *sfr, *next; >> > > > + LIST_FOR_EACH_SAFE (sfr, next, flow_list, &stf->flows) { >> > > > + struct ovn_flow *f = sfr->flow; >> > > > + ovn_flow_log(f, "flood remove"); >> > > > + >> > > > + ovs_list_remove(&sfr->sb_list); >> > > > + ovs_list_remove(&sfr->flow_list); >> > > > + free(sfr); >> > > > + >> > > > + ovs_assert(ovs_list_is_empty(&f->list_node)); >> > > > + if (ovs_list_is_empty(&f->references)) { >> > > > + /* This is to optimize the case when most flows have only >> > > > + * one referencing sb_uuid, so to_be_removed list should >> > > > + * be empty in most cases. */ >> > > > + hmap_remove(&flow_table->match_flow_table, >> > > > + &f->match_hmap_node); >> > > > + ovn_flow_destroy(f); >> > > > + } else { >> > > > + ovs_list_insert(&to_be_removed, &f->list_node); >> > > > + } >> > > > + } >> > > > + hmap_remove(&flow_table->uuid_flow_table, &stf->hmap_node); >> > > > + free(stf); >> > > > + >> > > > + /* Traverse other referencing sb_uuids for the flows in the >> > to_be_removed >> > > > + * list. */ >> > > > + >> > > > + /* Detach the items in f->references from the sfr.flow_list lists, >> > > > + * so that recursive calls will not mess up the sfr.sb_list >> > list. */ >> > > > + struct ovn_flow *f, *f_next; >> > > > + LIST_FOR_EACH (f, list_node, &to_be_removed) { >> > > > + ovs_assert(!ovs_list_is_empty(&f->references)); >> > > > + LIST_FOR_EACH (sfr, sb_list, &f->references) { >> > > > + ovs_list_remove(&sfr->flow_list); >> > > > + } >> > > > + } >> > > > + LIST_FOR_EACH_SAFE (f, f_next, list_node, &to_be_removed) { >> > > > + LIST_FOR_EACH_SAFE (sfr, next, sb_list, &f->references) { >> > > > + if (!flood_remove_find_node(flood_remove_nodes, >> > &sfr->sb_uuid)) { >> > > > + ofctrl_flood_remove_add_node(flood_remove_nodes, >> > > > + &sfr->sb_uuid); >> > > > + flood_remove_flows_for_sb_uuid(flow_table, >> > &sfr->sb_uuid, >> > > > + flood_remove_nodes); >> > > > + } >> > > > + ovs_list_remove(&sfr->sb_list); >> > > > + free(sfr); >> > > > + } >> > > > + ovs_list_remove(&f->list_node); >> > > > + hmap_remove(&flow_table->match_flow_table, >> > > > + &f->match_hmap_node); >> > > > + ovn_flow_destroy(f); >> > > > + } >> > > > + >> > > > +} >> > > > + >> > > > +void >> > > > +ofctrl_flood_remove_flows(struct ovn_desired_flow_table *flow_table, >> > > > + struct hmap *flood_remove_nodes) >> > > > +{ >> > > > + struct ofctrl_flood_remove_node *ofrn; >> > > > + HMAP_FOR_EACH (ofrn, hmap_node, flood_remove_nodes) { >> > > > + flood_remove_flows_for_sb_uuid(flow_table, &ofrn->sb_uuid, >> > > > + flood_remove_nodes); >> > > > + } >> > > > + >> > > > + /* remove any related group and meter info */ >> > > > + HMAP_FOR_EACH (ofrn, hmap_node, flood_remove_nodes) { >> > > > + ovn_extend_table_remove_desired(groups, &ofrn->sb_uuid); >> > > > + ovn_extend_table_remove_desired(meters, &ofrn->sb_uuid); >> > > > } >> > > > } >> > > > >> > > > @@ -763,18 +947,17 @@ ofctrl_add_or_append_flow(struct >> > ovn_desired_flow_table *desired_flows, >> > > > >> > > > static struct ovn_flow * >> > > > ovn_flow_alloc(uint8_t table_id, uint16_t priority, uint64_t cookie, >> > > > - const struct match *match, const struct ofpbuf >> > *actions, >> > > > - const struct uuid *sb_uuid) >> > > > + const struct match *match, const struct ofpbuf >> > *actions) >> > > > { >> > > > struct ovn_flow *f = xmalloc(sizeof *f); >> > > > + ovs_list_init(&f->references); >> > > > + ovs_list_init(&f->list_node); >> > > > f->table_id = table_id; >> > > > f->priority = priority; >> > > > minimatch_init(&f->match, match); >> > > > f->ofpacts = xmemdup(actions->data, actions->size); >> > > > f->ofpacts_len = actions->size; >> > > > - f->sb_uuid = *sb_uuid; >> > > > f->match_hmap_node.hash = ovn_flow_match_hash(f); >> > > > - f->uuid_hindex_node.hash = uuid_hash(&f->sb_uuid); >> > > > f->cookie = cookie; >> > > > >> > > > return f; >> > > > @@ -793,23 +976,27 @@ static struct ovn_flow * >> > > > ovn_flow_dup(struct ovn_flow *src) >> > > > { >> > > > struct ovn_flow *dst = xmalloc(sizeof *dst); >> > > > + ovs_list_init(&dst->references); >> > > > dst->table_id = src->table_id; >> > > > dst->priority = src->priority; >> > > > minimatch_clone(&dst->match, &src->match); >> > > > dst->ofpacts = xmemdup(src->ofpacts, src->ofpacts_len); >> > > > dst->ofpacts_len = src->ofpacts_len; >> > > > - dst->sb_uuid = src->sb_uuid; >> > > > dst->match_hmap_node.hash = src->match_hmap_node.hash; >> > > > - dst->uuid_hindex_node.hash = uuid_hash(&src->sb_uuid); >> > > > dst->cookie = src->cookie; >> > > > return dst; >> > > > } >> > > > >> > > > /* Finds and returns an ovn_flow in 'flow_table' whose key is >> > identical to >> > > > - * 'target''s key, or NULL if there is none. */ >> > > > + * 'target''s key, or NULL if there is none. >> > > > + * >> > > > + * If sb_uuid is not NULL, the function will also check if the >> > found flow is >> > > > + * referenced by the sb_uuid. >> > > > + * >> > > > + * NOTE: sb_uuid can only be used for ovn_desired_flow_table >> > lookup. */ >> > > > static struct ovn_flow * >> > > > ovn_flow_lookup(struct hmap *flow_table, const struct ovn_flow >> > *target, >> > > > - bool cmp_sb_uuid) >> > > > + const struct uuid *sb_uuid) >> > > > { >> > > > struct ovn_flow *f; >> > > > >> > > > @@ -818,9 +1005,16 @@ ovn_flow_lookup(struct hmap *flow_table, >> > const struct ovn_flow *target, >> > > > if (f->table_id == target->table_id >> > > > && f->priority == target->priority >> > > > && minimatch_equal(&f->match, &target->match)) { >> > > > - if (!cmp_sb_uuid || uuid_equals(&target->sb_uuid, >> > &f->sb_uuid)) { >> > > > + if (!sb_uuid) { >> > > > return f; >> > > > } >> > > > + ovs_assert(flow_table != &installed_flows); >> > > > + struct sb_flow_ref *sfr; >> > > > + LIST_FOR_EACH (sfr, sb_list, &f->references) { >> > > > + if (uuid_equals(sb_uuid, &sfr->sb_uuid)) { >> > > > + return f; >> > > > + } >> > > > + } >> > > > } >> > > > } >> > > > return NULL; >> > > > @@ -830,7 +1024,7 @@ static char * >> > > > ovn_flow_to_string(const struct ovn_flow *f) >> > > > { >> > > > struct ds s = DS_EMPTY_INITIALIZER; >> > > > - ds_put_format(&s, "sb_uuid="UUID_FMT", ", UUID_ARGS(&f->sb_uuid)); >> > > > + >> > > > ds_put_format(&s, "cookie=%"PRIx64", ", f->cookie); >> > > > ds_put_format(&s, "table_id=%"PRIu8", ", f->table_id); >> > > > ds_put_format(&s, "priority=%"PRIu16", ", f->priority); >> > > > @@ -855,6 +1049,7 @@ static void >> > > > ovn_flow_destroy(struct ovn_flow *f) >> > > > { >> > > > if (f) { >> > > > + ovs_assert(ovs_list_is_empty(&f->references)); >> > > > minimatch_destroy(&f->match); >> > > > free(f->ofpacts); >> > > > free(f); >> > > > @@ -866,18 +1061,16 @@ void >> > > > ovn_desired_flow_table_init(struct ovn_desired_flow_table >> > *flow_table) >> > > > { >> > > > hmap_init(&flow_table->match_flow_table); >> > > > - hindex_init(&flow_table->uuid_flow_table); >> > > > + hmap_init(&flow_table->uuid_flow_table); >> > > > } >> > > > >> > > > void >> > > > ovn_desired_flow_table_clear(struct ovn_desired_flow_table >> > *flow_table) >> > > > { >> > > > - struct ovn_flow *f, *next; >> > > > - HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, >> > > > - &flow_table->match_flow_table) { >> > > > - hmap_remove(&flow_table->match_flow_table, >> > &f->match_hmap_node); >> > > > - hindex_remove(&flow_table->uuid_flow_table, >> > &f->uuid_hindex_node); >> > > > - ovn_flow_destroy(f); >> > > > + struct sb_to_flow *stf, *next; >> > > > + HMAP_FOR_EACH_SAFE (stf, next, hmap_node, >> > > > + &flow_table->uuid_flow_table) { >> > > > + remove_flows_from_sb_to_flow(flow_table, stf, NULL); >> > > > } >> > > > } >> > > > >> > > > @@ -886,7 +1079,7 @@ ovn_desired_flow_table_destroy(struct >> > ovn_desired_flow_table *flow_table) >> > > > { >> > > > ovn_desired_flow_table_clear(flow_table); >> > > > hmap_destroy(&flow_table->match_flow_table); >> > > > - hindex_destroy(&flow_table->uuid_flow_table); >> > > > + hmap_destroy(&flow_table->uuid_flow_table); >> > > > } >> > > > >> > > > static void >> > > > @@ -1221,7 +1414,7 @@ ofctrl_put(struct ovn_desired_flow_table >> > *flow_table, >> > > > struct ovn_flow *i, *next; >> > > > HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, &installed_flows) { >> > > > struct ovn_flow *d = >> > ovn_flow_lookup(&flow_table->match_flow_table, >> > > > - i, false); >> > > > + i, NULL); >> > > > if (!d) { >> > > > /* Installed flow is no longer desirable. Delete it >> > from the >> > > > * switch and from installed_flows. */ >> > > > @@ -1237,10 +1430,6 @@ ofctrl_put(struct ovn_desired_flow_table >> > *flow_table, >> > > > hmap_remove(&installed_flows, &i->match_hmap_node); >> > > > ovn_flow_destroy(i); >> > > > } else { >> > > > - if (!uuid_equals(&i->sb_uuid, &d->sb_uuid)) { >> > > > - /* Update installed flow's UUID. */ >> > > > - i->sb_uuid = d->sb_uuid; >> > > > - } >> > > > if (!ofpacts_equal(i->ofpacts, i->ofpacts_len, >> > > > d->ofpacts, d->ofpacts_len) || >> > > > i->cookie != d->cookie) { >> > > > @@ -1277,7 +1466,7 @@ ofctrl_put(struct ovn_desired_flow_table >> > *flow_table, >> > > > * in the installed flow table. */ >> > > > struct ovn_flow *d; >> > > > HMAP_FOR_EACH (d, match_hmap_node, >> > &flow_table->match_flow_table) { >> > > > - i = ovn_flow_lookup(&installed_flows, d, false); >> > > > + i = ovn_flow_lookup(&installed_flows, d, NULL); >> > > > if (!i) { >> > > > /* Send flow_mod to add flow. */ >> > > > struct ofputil_flow_mod fm = { >> > > > diff --git a/controller/ofctrl.h b/controller/ofctrl.h >> > > > index 37f06db..8789ce4 100644 >> > > > --- a/controller/ofctrl.h >> > > > +++ b/controller/ofctrl.h >> > > > @@ -35,8 +35,9 @@ struct ovn_desired_flow_table { >> > > > /* Hash map flow table using flow match conditions as hash key.*/ >> > > > struct hmap match_flow_table; >> > > > >> > > > - /* SB uuid index for the nodes in match_flow_table.*/ >> > > > - struct hindex uuid_flow_table; >> > > > + /* SB uuid index for the cross reference nodes that link to >> > the nodes in >> > > > + * match_flow_table.*/ >> > > > + struct hmap uuid_flow_table; >> > > > }; >> > > > >> > > > /* Interface for OVN main loop. */ >> > > > @@ -74,7 +75,30 @@ void ofctrl_add_or_append_flow(struct >> > ovn_desired_flow_table *desired_flows, >> > > > const struct ofpbuf *actions, >> > > > const struct uuid *sb_uuid); >> > > > >> > > > -void ofctrl_remove_flows(struct ovn_desired_flow_table *, const >> > struct uuid *); >> > > > +/* Removes a bundles of flows from the flow table for a specific >> > sb_uuid. The >> > > > + * flows are removed only if they are not referenced by any other >> > sb_uuid(s). >> > > > + * For flood-removing all related flows referenced by other >> > sb_uuid(s), use >> > > > + * ofctrl_flood_remove_flows(). */ >> > > > +void ofctrl_remove_flows(struct ovn_desired_flow_table *, >> > > > + const struct uuid *sb_uuid); >> > > > + >> > > > +/* The function ofctrl_flood_remove_flows flood-removes flows from >> > the desired >> > > > + * flow table for the sb_uuids provided in the flood_remove_nodes >> > argument. >> > > > + * For each given sb_uuid in flood_remove_nodes, it removes all >> > the flows >> > > > + * generated by the sb_uuid, and if any of the flows are >> > referenced by another >> > > > + * sb_uuid, it continues removing all the flows used by that >> > sb_uuid as well, >> > > > + * and so on, recursively. >> > > > + * >> > > > + * It adds all the sb_uuids that are actually removed in the >> > > > + * flood_remove_nodes. */ >> > > > +struct ofctrl_flood_remove_node { >> > > > + struct hmap_node hmap_node; >> > > > + struct uuid sb_uuid; >> > > > +}; >> > > > +void ofctrl_flood_remove_flows(struct ovn_desired_flow_table *, >> > > > + struct hmap *flood_remove_nodes); >> > > > +void ofctrl_flood_remove_add_node(struct hmap *flood_remove_nodes, >> > > > + const struct uuid *sb_uuid); >> > > > >> > > > void ovn_desired_flow_table_init(struct ovn_desired_flow_table *); >> > > > void ovn_desired_flow_table_clear(struct ovn_desired_flow_table *); >> > > > @@ -86,6 +110,7 @@ void ofctrl_check_and_add_flow(struct >> > ovn_desired_flow_table *, >> > > > const struct ofpbuf *ofpacts, >> > > > const struct uuid *, bool >> > log_duplicate_flow); >> > > > >> > > > + >> > > > bool ofctrl_is_connected(void); >> > > > void ofctrl_set_probe_interval(int probe_interval); >> > > > >> > > > diff --git a/tests/ovn.at <http://ovn.at> b/tests/ovn.at >> > <http://ovn.at> >> > > > index d14f381..14def6e 100644 >> > > > --- a/tests/ovn.at <http://ovn.at> >> > > > +++ b/tests/ovn.at <http://ovn.at> >> > > > @@ -13583,6 +13583,8 @@ AT_CHECK([cat 2.packets], [0], [expout]) >> > > > >> > > > OVS_WAIT_UNTIL([test 9 = `as hv1 ovs-ofctl dump-flows br-int | \ >> > > > grep conjunction | wc -l`]) >> > > > +OVS_WAIT_UNTIL([test 3 = `as hv1 ovs-ofctl dump-flows br-int | \ >> > > > +grep conjunction.*conjunction | wc -l`]) >> > > > OVS_WAIT_UNTIL([test 2 = `as hv1 ovs-ofctl dump-flows br-int | \ >> > > > grep conj_id | wc -l`]) >> > > > >> > > > @@ -13600,9 +13602,97 @@ sip=`ip_to_hex 10 0 0 4` >> > > > dip=`ip_to_hex 10 0 0 7` >> > > > >> > > > test_ip 1 f00000000001 f00000000002 $sip $dip >> > > > + >> > > > $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in <http://ovs-pcap.in>" >> > hv1/vif2-tx.pcap > 2.packets >> > > > AT_CHECK([cat 2.packets], [0], []) >> > > > >> > > > +# Remove the first ACL, and verify that the conjunction flows are >> > updated >> > > > +# properly. >> > > > +# There should be total of 6 flows present with conjunction action >> > and 1 flow >> > > > +# with conj match. Eg. >> > > > +# table=44, priority=2001,conj_id=3,metadata=0x1 actions=drop >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.7 >> > actions=conjunction(4,2/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 >> > actions=conjunction(4,2/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 >> > actions=conjunction(4,2/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 >> > actions=conjunction(4,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 >> > actions=conjunction(4,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 >> > actions=conjunction(4,1/2) >> > > > + >> > > > +ovn-nbctl acl-del ls1 to-lport 1001 \ >> > > > +'ip4 && ip4.src == $set1 && ip4.dst == $set1' >> > > > + >> > > > +OVS_WAIT_UNTIL([test 6 = `as hv1 ovs-ofctl dump-flows br-int | \ >> > > > +grep conjunction | wc -l`]) >> > > > +OVS_WAIT_UNTIL([test 0 = `as hv1 ovs-ofctl dump-flows br-int | \ >> > > > +grep conjunction.*conjunction | wc -l`]) >> > > > +OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-ofctl dump-flows br-int | \ >> > > > +grep conj_id | wc -l`]) >> > > > + >> > > > +# Add the ACL back >> > > > +ovn-nbctl acl-add ls1 to-lport 1001 \ >> > > > +'ip4 && ip4.src == $set1 && ip4.dst == $set1' allow >> > > > +# Add one more ACL with more overlapping >> > > > +ovn-nbctl acl-add ls1 to-lport 1001 \ >> > > > +'ip4 && ip4.src == $set1 && ip4.dst == {10.0.0.9, 10.0.0.10}' drop >> > > > + >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 >> > actions=conjunction(4,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.7 >> > actions=conjunction(4,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.4 >> > actions=conjunction(5,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.5 >> > actions=conjunction(5,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.6 >> > actions=conjunction(5,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 >> > actions=conjunction(4,1/2),conjunction(6,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.10 >> > actions=conjunction(6,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 >> > actions=conjunction(4,2/2),conjunction(5,2/2),conjunction(6,2/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 >> > actions=conjunction(4,2/2),conjunction(5,2/2),conjunction(6,2/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 >> > actions=conjunction(4,2/2),conjunction(5,2/2),conjunction(6,2/2) >> > > > + >> > > > +OVS_WAIT_UNTIL([test 10 = `as hv1 ovs-ofctl dump-flows br-int | \ >> > > > +grep conjunction | wc -l`]) >> > > > +OVS_WAIT_UNTIL([test 4 = `as hv1 ovs-ofctl dump-flows br-int | \ >> > > > +grep conjunction.*conjunction | wc -l`]) >> > > > +OVS_WAIT_UNTIL([test 3 = `as hv1 ovs-ofctl dump-flows br-int | \ >> > > > +grep conjunction.*conjunction.*conjunction | wc -l`]) >> > > > + >> > > > +# Remove 10.0.0.7 from address set2. All flows should be updated >> > properly. >> > > > +ovn-nbctl set Address_Set set2 \ >> > > > +addresses=\"10.0.0.8\",\"10.0.0.9\" >> > > > + >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.4 >> > actions=conjunction(9,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.10 >> > actions=conjunction(7,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 >> > actions=conjunction(8,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.5 >> > actions=conjunction(9,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 >> > actions=conjunction(7,1/2),conjunction(8,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.6 >> > actions=conjunction(9,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 >> > actions=conjunction(7,2/2),conjunction(8,2/2),conjunction(9,2/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 >> > actions=conjunction(7,2/2),conjunction(8,2/2),conjunction(9,2/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 >> > actions=conjunction(7,2/2),conjunction(8,2/2),conjunction(9,2/2) >> > > > + >> > > > +OVS_WAIT_UNTIL([test 9 = `as hv1 ovs-ofctl dump-flows br-int | \ >> > > > +grep conjunction | wc -l`]) >> > > > +OVS_WAIT_UNTIL([test 4 = `as hv1 ovs-ofctl dump-flows br-int | \ >> > > > +grep conjunction.*conjunction | wc -l`]) >> > > > +OVS_WAIT_UNTIL([test 3 = `as hv1 ovs-ofctl dump-flows br-int | \ >> > > > +grep conjunction.*conjunction.*conjunction | wc -l`]) >> > > > + >> > > > +# Remove an ACL again >> > > > +ovn-nbctl acl-del ls1 to-lport 1001 \ >> > > > +'ip4 && ip4.src == $set1 && ip4.dst == $set1' >> > > > + >> > > > +ovn-nbctl --wait=hv sync >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.10 >> > actions=conjunction(10,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 >> > actions=conjunction(11,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 >> > actions=conjunction(10,1/2),conjunction(11,1/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 >> > actions=conjunction(10,2/2),conjunction(11,2/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 >> > actions=conjunction(10,2/2),conjunction(11,2/2) >> > > > +# priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 >> > actions=conjunction(10,2/2),conjunction(11,2/2) >> > > > + >> > > > +OVS_WAIT_UNTIL([test 6 = `as hv1 ovs-ofctl dump-flows br-int | \ >> > > > +grep conjunction | wc -l`]) >> > > > +OVS_WAIT_UNTIL([test 4 = `as hv1 ovs-ofctl dump-flows br-int | \ >> > > > +grep conjunction.*conjunction | wc -l`]) >> > > > +OVS_WAIT_UNTIL([test 0 = `as hv1 ovs-ofctl dump-flows br-int | \ >> > > > +grep conjunction.*conjunction.*conjunction | wc -l`]) >> > > > + >> > > > OVN_CLEANUP([hv1]) >> > > > AT_CLEANUP >> > > > >> > > > >> > > >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
