| Message ID | 20201011120554.3374.75656.stgit@dceara.remote.csb |
|---|---|
| State | Accepted |
| Headers | show |
| Series | ofctrl: Add a predictable resolution for conflicting flow actions. | expand |
On Sun, Oct 11, 2020 at 5:06 AM Dumitru Ceara <dceara@redhat.com> wrote: > > Always consider the first "desired flow" in the list of flows that refer an > "installed flow" as the active flow. This simplifies the logic of the flow > update code and is used in a further commit to implement a partial ordering > of desired flows within installed flows. > > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > --- > controller/ofctrl.c | 92 +++++++++++++++++++++------------------------------ > 1 file changed, 37 insertions(+), 55 deletions(-) > > diff --git a/controller/ofctrl.c b/controller/ofctrl.c > index 20cf3ac..74f98e3 100644 > --- a/controller/ofctrl.c > +++ b/controller/ofctrl.c > @@ -188,6 +188,8 @@ struct sb_flow_ref { > * relationship is 1 to N. A link is added when a flow addition is processed. > * A link is removed when a flow deletion is processed, the desired flow > * table is cleared, or the installed flow table is cleared. > + * The first desired_flow in the list is the active one, the one that is > + * actually installed. > */ > struct installed_flow { > struct ovn_flow flow; > @@ -199,11 +201,6 @@ struct installed_flow { > * installed flow, e.g. when there are conflict/duplicated ACLs that > * generates same match conditions). */ > struct ovs_list desired_refs; > - > - /* The corresponding flow in desired table. It must be one of the flows in > - * desired_refs list. If there are more than one flows in references list, > - * this is the one that is actually installed. */ > - struct desired_flow *desired_flow; > }; > > typedef bool > @@ -231,6 +228,8 @@ static struct installed_flow *installed_flow_lookup( > const struct ovn_flow *target); > static void installed_flow_destroy(struct installed_flow *); > static struct installed_flow *installed_flow_dup(struct desired_flow *); > +static struct desired_flow *installed_flow_get_active( > + struct installed_flow *f); > > static uint32_t ovn_flow_match_hash(const struct ovn_flow *); > static char *ovn_flow_to_string(const struct ovn_flow *); > @@ -796,24 +795,6 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype type) > log_openflow_rl(&rl, VLL_DBG, oh, "OpenFlow packet ignored"); > } > } > - > -/* Returns true if a desired flow is active (the one currently installed > - * among the list of desired flows that are linked to the same installed > - * flow). */ > -static inline bool > -desired_flow_is_active(struct desired_flow *d) > -{ > - return (d->installed_flow && d->installed_flow->desired_flow == d); > -} > - > -/* Set a desired flow as the active one among the list of desired flows > - * that are linked to the same installed flow. */ > -static inline void > -desired_flow_set_active(struct desired_flow *d) > -{ > - ovs_assert(d->installed_flow); > - d->installed_flow->desired_flow = d; > -} > > static bool > flow_action_has_conj(const struct ovn_flow *f) > @@ -831,27 +812,22 @@ flow_action_has_conj(const struct ovn_flow *f) > /* Adds the desired flow to the list of desired flows that have same match > * conditions as the installed flow. > * > - * If the newly added desired flow is the first one in the list, it is also set > - * as the active one. > - * > * It is caller's responsibility to make sure the link between the pair didn't > - * exist before. */ > -static void > + * exist before. > + * > + * Returns true if the newly added desired flow is selected to be the active > + * one. > + */ > +static bool > link_installed_to_desired(struct installed_flow *i, struct desired_flow *d) > { > - ovs_assert(i->desired_flow != d); > - if (ovs_list_is_empty(&i->desired_refs)) { > - ovs_assert(!i->desired_flow); > - i->desired_flow = d; > - } > - ovs_list_insert(&i->desired_refs, &d->installed_ref_list_node); > d->installed_flow = i; > + ovs_list_push_back(&i->desired_refs, &d->installed_ref_list_node); > + return installed_flow_get_active(i) == d; > } > > /* Replaces 'old_desired' with 'new_desired' in the list of desired flows > * that have same match conditions as the installed flow. > - * > - * If 'old_desired' was the active flow, 'new_desired' becomes the active one. > */ > static void > replace_installed_to_desired(struct installed_flow *i, > @@ -863,24 +839,22 @@ replace_installed_to_desired(struct installed_flow *i, > &old_desired->installed_ref_list_node); > old_desired->installed_flow = NULL; > new_desired->installed_flow = i; > - if (i->desired_flow == old_desired) { > - i->desired_flow = new_desired; > - } > } > > -static void > +/* Removes the desired flow from the list of desired flows that have the same > + * match conditions as the installed flow. > + * > + * Returns true if the desired flow was the previously active flow. > + */ > +static bool > unlink_installed_to_desired(struct installed_flow *i, struct desired_flow *d) > { > - ovs_assert(i && i->desired_flow && !ovs_list_is_empty(&i->desired_refs)); > + struct desired_flow *old_active = installed_flow_get_active(i); > + > ovs_assert(d && d->installed_flow == i); > ovs_list_remove(&d->installed_ref_list_node); > d->installed_flow = NULL; > - if (i->desired_flow == d) { > - i->desired_flow = ovs_list_is_empty(&i->desired_refs) ? NULL : > - CONTAINER_OF(ovs_list_front(&i->desired_refs), > - struct desired_flow, > - installed_ref_list_node); > - } > + return old_active == d; > } > > static void > @@ -1280,7 +1254,6 @@ installed_flow_dup(struct desired_flow *src) > { > struct installed_flow *dst = xmalloc(sizeof *dst); > ovs_list_init(&dst->desired_refs); > - dst->desired_flow = NULL; > dst->flow.table_id = src->flow.table_id; > dst->flow.priority = src->flow.priority; > minimatch_clone(&dst->flow.match, &src->flow.match); > @@ -1292,6 +1265,17 @@ installed_flow_dup(struct desired_flow *src) > } > > static struct desired_flow * > +installed_flow_get_active(struct installed_flow *f) > +{ > + if (!ovs_list_is_empty(&f->desired_refs)) { > + return CONTAINER_OF(ovs_list_front(&f->desired_refs), > + struct desired_flow, > + installed_ref_list_node); > + } > + return NULL; > +} > + > +static struct desired_flow * > desired_flow_lookup__(struct ovn_desired_flow_table *flow_table, > const struct ovn_flow *target, > desired_flow_match_cb match_cb, > @@ -1439,8 +1423,7 @@ static void > installed_flow_destroy(struct installed_flow *f) > { > if (f) { > - ovs_assert(ovs_list_is_empty(&f->desired_refs)); > - ovs_assert(!f->desired_flow); > + ovs_assert(!installed_flow_get_active(f)); > ovn_flow_uninit(&f->flow); > free(f); > } > @@ -1898,10 +1881,10 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, > /* The desired flow was deleted */ > if (f->installed_flow) { > struct installed_flow *i = f->installed_flow; > - bool was_active = desired_flow_is_active(f); > - unlink_installed_to_desired(i, f); > + bool was_active = unlink_installed_to_desired(i, f); > + struct desired_flow *d = installed_flow_get_active(i); > > - if (!i->desired_flow) { > + if (!d) { > installed_flow_del(&i->flow, msgs); > ovn_flow_log(&i->flow, "removing installed (tracked)"); > > @@ -1912,7 +1895,6 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, > * installed flow, so update the OVS flow for the new > * active flow (at least the cookie will be different, > * even if the actions are the same). */ > - struct desired_flow *d = i->desired_flow; > ovn_flow_log(&i->flow, "updating installed (tracked)"); > installed_flow_mod(&i->flow, &d->flow, msgs); > } > @@ -1931,7 +1913,7 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, > hmap_insert(&installed_flows, &new_node->match_hmap_node, > new_node->flow.hash); > link_installed_to_desired(new_node, f); > - } else if (desired_flow_is_active(f)) { > + } else if (installed_flow_get_active(i) == f) { > /* The installed flow is installed for f, but f has change > * tracked, so it must have been modified. */ > ovn_flow_log(&i->flow, "updating installed (tracked)"); > Thanks Dumitru. I applied patch 1 - 3 of the series to master, with a tiny change to patch 3 just to follow the coding style: ............................... 8>< ................................................................><8 .................................... index 74f98e36b..ba0c61c80 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -228,8 +228,7 @@ static struct installed_flow *installed_flow_lookup( const struct ovn_flow *target); static void installed_flow_destroy(struct installed_flow *); static struct installed_flow *installed_flow_dup(struct desired_flow *); -static struct desired_flow *installed_flow_get_active( - struct installed_flow *f); +static struct desired_flow *installed_flow_get_active(struct installed_flow *); static uint32_t ovn_flow_match_hash(const struct ovn_flow *); static char *ovn_flow_to_string(const struct ovn_flow *);
On 10/13/20 9:03 AM, Han Zhou wrote: > > > On Sun, Oct 11, 2020 at 5:06 AM Dumitru Ceara <dceara@redhat.com > <mailto:dceara@redhat.com>> wrote: >> >> Always consider the first "desired flow" in the list of flows that > refer an >> "installed flow" as the active flow. This simplifies the logic of the > flow >> update code and is used in a further commit to implement a partial > ordering >> of desired flows within installed flows. >> >> Signed-off-by: Dumitru Ceara <dceara@redhat.com > <mailto:dceara@redhat.com>> >> --- >> controller/ofctrl.c | 92 > +++++++++++++++++++++------------------------------ >> 1 file changed, 37 insertions(+), 55 deletions(-) >> >> diff --git a/controller/ofctrl.c b/controller/ofctrl.c >> index 20cf3ac..74f98e3 100644 >> --- a/controller/ofctrl.c >> +++ b/controller/ofctrl.c >> @@ -188,6 +188,8 @@ struct sb_flow_ref { >> * relationship is 1 to N. A link is added when a flow addition is > processed. >> * A link is removed when a flow deletion is processed, the desired flow >> * table is cleared, or the installed flow table is cleared. >> + * The first desired_flow in the list is the active one, the one that is >> + * actually installed. >> */ >> struct installed_flow { >> struct ovn_flow flow; >> @@ -199,11 +201,6 @@ struct installed_flow { >> * installed flow, e.g. when there are conflict/duplicated ACLs that >> * generates same match conditions). */ >> struct ovs_list desired_refs; >> - >> - /* The corresponding flow in desired table. It must be one of the > flows in >> - * desired_refs list. If there are more than one flows in > references list, >> - * this is the one that is actually installed. */ >> - struct desired_flow *desired_flow; >> }; >> >> typedef bool >> @@ -231,6 +228,8 @@ static struct installed_flow *installed_flow_lookup( >> const struct ovn_flow *target); >> static void installed_flow_destroy(struct installed_flow *); >> static struct installed_flow *installed_flow_dup(struct desired_flow *); >> +static struct desired_flow *installed_flow_get_active( >> + struct installed_flow *f); >> >> static uint32_t ovn_flow_match_hash(const struct ovn_flow *); >> static char *ovn_flow_to_string(const struct ovn_flow *); >> @@ -796,24 +795,6 @@ ofctrl_recv(const struct ofp_header *oh, enum > ofptype type) >> log_openflow_rl(&rl, VLL_DBG, oh, "OpenFlow packet ignored"); >> } >> } >> - >> -/* Returns true if a desired flow is active (the one currently installed >> - * among the list of desired flows that are linked to the same installed >> - * flow). */ >> -static inline bool >> -desired_flow_is_active(struct desired_flow *d) >> -{ >> - return (d->installed_flow && d->installed_flow->desired_flow == d); >> -} >> - >> -/* Set a desired flow as the active one among the list of desired flows >> - * that are linked to the same installed flow. */ >> -static inline void >> -desired_flow_set_active(struct desired_flow *d) >> -{ >> - ovs_assert(d->installed_flow); >> - d->installed_flow->desired_flow = d; >> -} >> >> static bool >> flow_action_has_conj(const struct ovn_flow *f) >> @@ -831,27 +812,22 @@ flow_action_has_conj(const struct ovn_flow *f) >> /* Adds the desired flow to the list of desired flows that have same > match >> * conditions as the installed flow. >> * >> - * If the newly added desired flow is the first one in the list, it > is also set >> - * as the active one. >> - * >> * It is caller's responsibility to make sure the link between the > pair didn't >> - * exist before. */ >> -static void >> + * exist before. >> + * >> + * Returns true if the newly added desired flow is selected to be the > active >> + * one. >> + */ >> +static bool >> link_installed_to_desired(struct installed_flow *i, struct > desired_flow *d) >> { >> - ovs_assert(i->desired_flow != d); >> - if (ovs_list_is_empty(&i->desired_refs)) { >> - ovs_assert(!i->desired_flow); >> - i->desired_flow = d; >> - } >> - ovs_list_insert(&i->desired_refs, &d->installed_ref_list_node); >> d->installed_flow = i; >> + ovs_list_push_back(&i->desired_refs, &d->installed_ref_list_node); >> + return installed_flow_get_active(i) == d; >> } >> >> /* Replaces 'old_desired' with 'new_desired' in the list of desired flows >> * that have same match conditions as the installed flow. >> - * >> - * If 'old_desired' was the active flow, 'new_desired' becomes the > active one. >> */ >> static void >> replace_installed_to_desired(struct installed_flow *i, >> @@ -863,24 +839,22 @@ replace_installed_to_desired(struct > installed_flow *i, >> &old_desired->installed_ref_list_node); >> old_desired->installed_flow = NULL; >> new_desired->installed_flow = i; >> - if (i->desired_flow == old_desired) { >> - i->desired_flow = new_desired; >> - } >> } >> >> -static void >> +/* Removes the desired flow from the list of desired flows that have > the same >> + * match conditions as the installed flow. >> + * >> + * Returns true if the desired flow was the previously active flow. >> + */ >> +static bool >> unlink_installed_to_desired(struct installed_flow *i, struct > desired_flow *d) >> { >> - ovs_assert(i && i->desired_flow && > !ovs_list_is_empty(&i->desired_refs)); >> + struct desired_flow *old_active = installed_flow_get_active(i); >> + >> ovs_assert(d && d->installed_flow == i); >> ovs_list_remove(&d->installed_ref_list_node); >> d->installed_flow = NULL; >> - if (i->desired_flow == d) { >> - i->desired_flow = ovs_list_is_empty(&i->desired_refs) ? NULL : >> - CONTAINER_OF(ovs_list_front(&i->desired_refs), >> - struct desired_flow, >> - installed_ref_list_node); >> - } >> + return old_active == d; >> } >> >> static void >> @@ -1280,7 +1254,6 @@ installed_flow_dup(struct desired_flow *src) >> { >> struct installed_flow *dst = xmalloc(sizeof *dst); >> ovs_list_init(&dst->desired_refs); >> - dst->desired_flow = NULL; >> dst->flow.table_id = src->flow.table_id; >> dst->flow.priority = src->flow.priority; >> minimatch_clone(&dst->flow.match, &src->flow.match); >> @@ -1292,6 +1265,17 @@ installed_flow_dup(struct desired_flow *src) >> } >> >> static struct desired_flow * >> +installed_flow_get_active(struct installed_flow *f) >> +{ >> + if (!ovs_list_is_empty(&f->desired_refs)) { >> + return CONTAINER_OF(ovs_list_front(&f->desired_refs), >> + struct desired_flow, >> + installed_ref_list_node); >> + } >> + return NULL; >> +} >> + >> +static struct desired_flow * >> desired_flow_lookup__(struct ovn_desired_flow_table *flow_table, >> const struct ovn_flow *target, >> desired_flow_match_cb match_cb, >> @@ -1439,8 +1423,7 @@ static void >> installed_flow_destroy(struct installed_flow *f) >> { >> if (f) { >> - ovs_assert(ovs_list_is_empty(&f->desired_refs)); >> - ovs_assert(!f->desired_flow); >> + ovs_assert(!installed_flow_get_active(f)); >> ovn_flow_uninit(&f->flow); >> free(f); >> } >> @@ -1898,10 +1881,10 @@ update_installed_flows_by_track(struct > ovn_desired_flow_table *flow_table, >> /* The desired flow was deleted */ >> if (f->installed_flow) { >> struct installed_flow *i = f->installed_flow; >> - bool was_active = desired_flow_is_active(f); >> - unlink_installed_to_desired(i, f); >> + bool was_active = unlink_installed_to_desired(i, f); >> + struct desired_flow *d = installed_flow_get_active(i); >> >> - if (!i->desired_flow) { >> + if (!d) { >> installed_flow_del(&i->flow, msgs); >> ovn_flow_log(&i->flow, "removing installed > (tracked)"); >> >> @@ -1912,7 +1895,6 @@ update_installed_flows_by_track(struct > ovn_desired_flow_table *flow_table, >> * installed flow, so update the OVS flow for the new >> * active flow (at least the cookie will be > different, >> * even if the actions are the same). */ >> - struct desired_flow *d = i->desired_flow; >> ovn_flow_log(&i->flow, "updating installed > (tracked)"); >> installed_flow_mod(&i->flow, &d->flow, msgs); >> } >> @@ -1931,7 +1913,7 @@ update_installed_flows_by_track(struct > ovn_desired_flow_table *flow_table, >> hmap_insert(&installed_flows, &new_node->match_hmap_node, >> new_node->flow.hash); >> link_installed_to_desired(new_node, f); >> - } else if (desired_flow_is_active(f)) { >> + } else if (installed_flow_get_active(i) == f) { >> /* The installed flow is installed for f, but f has > change >> * tracked, so it must have been modified. */ >> ovn_flow_log(&i->flow, "updating installed (tracked)"); >> > > Thanks Dumitru. I applied patch 1 - 3 of the series to master, with a > tiny change to patch 3 just to follow the coding style: > > ............................... 8>< > ................................................................><8 > .................................... > index 74f98e36b..ba0c61c80 100644 > --- a/controller/ofctrl.c > +++ b/controller/ofctrl.c > @@ -228,8 +228,7 @@ static struct installed_flow *installed_flow_lookup( > const struct ovn_flow *target); > static void installed_flow_destroy(struct installed_flow *); > static struct installed_flow *installed_flow_dup(struct desired_flow *); > -static struct desired_flow *installed_flow_get_active( > - struct installed_flow *f); > +static struct desired_flow *installed_flow_get_active(struct > installed_flow *); > > static uint32_t ovn_flow_match_hash(const struct ovn_flow *); > static char *ovn_flow_to_string(const struct ovn_flow *); Ack, looks good to me. Thanks, Dumitru
diff --git a/controller/ofctrl.c b/controller/ofctrl.c index 20cf3ac..74f98e3 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -188,6 +188,8 @@ struct sb_flow_ref { * relationship is 1 to N. A link is added when a flow addition is processed. * A link is removed when a flow deletion is processed, the desired flow * table is cleared, or the installed flow table is cleared. + * The first desired_flow in the list is the active one, the one that is + * actually installed. */ struct installed_flow { struct ovn_flow flow; @@ -199,11 +201,6 @@ struct installed_flow { * installed flow, e.g. when there are conflict/duplicated ACLs that * generates same match conditions). */ struct ovs_list desired_refs; - - /* The corresponding flow in desired table. It must be one of the flows in - * desired_refs list. If there are more than one flows in references list, - * this is the one that is actually installed. */ - struct desired_flow *desired_flow; }; typedef bool @@ -231,6 +228,8 @@ static struct installed_flow *installed_flow_lookup( const struct ovn_flow *target); static void installed_flow_destroy(struct installed_flow *); static struct installed_flow *installed_flow_dup(struct desired_flow *); +static struct desired_flow *installed_flow_get_active( + struct installed_flow *f); static uint32_t ovn_flow_match_hash(const struct ovn_flow *); static char *ovn_flow_to_string(const struct ovn_flow *); @@ -796,24 +795,6 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype type) log_openflow_rl(&rl, VLL_DBG, oh, "OpenFlow packet ignored"); } } - -/* Returns true if a desired flow is active (the one currently installed - * among the list of desired flows that are linked to the same installed - * flow). */ -static inline bool -desired_flow_is_active(struct desired_flow *d) -{ - return (d->installed_flow && d->installed_flow->desired_flow == d); -} - -/* Set a desired flow as the active one among the list of desired flows - * that are linked to the same installed flow. */ -static inline void -desired_flow_set_active(struct desired_flow *d) -{ - ovs_assert(d->installed_flow); - d->installed_flow->desired_flow = d; -} static bool flow_action_has_conj(const struct ovn_flow *f) @@ -831,27 +812,22 @@ flow_action_has_conj(const struct ovn_flow *f) /* Adds the desired flow to the list of desired flows that have same match * conditions as the installed flow. * - * If the newly added desired flow is the first one in the list, it is also set - * as the active one. - * * It is caller's responsibility to make sure the link between the pair didn't - * exist before. */ -static void + * exist before. + * + * Returns true if the newly added desired flow is selected to be the active + * one. + */ +static bool link_installed_to_desired(struct installed_flow *i, struct desired_flow *d) { - ovs_assert(i->desired_flow != d); - if (ovs_list_is_empty(&i->desired_refs)) { - ovs_assert(!i->desired_flow); - i->desired_flow = d; - } - ovs_list_insert(&i->desired_refs, &d->installed_ref_list_node); d->installed_flow = i; + ovs_list_push_back(&i->desired_refs, &d->installed_ref_list_node); + return installed_flow_get_active(i) == d; } /* Replaces 'old_desired' with 'new_desired' in the list of desired flows * that have same match conditions as the installed flow. - * - * If 'old_desired' was the active flow, 'new_desired' becomes the active one. */ static void replace_installed_to_desired(struct installed_flow *i, @@ -863,24 +839,22 @@ replace_installed_to_desired(struct installed_flow *i, &old_desired->installed_ref_list_node); old_desired->installed_flow = NULL; new_desired->installed_flow = i; - if (i->desired_flow == old_desired) { - i->desired_flow = new_desired; - } } -static void +/* Removes the desired flow from the list of desired flows that have the same + * match conditions as the installed flow. + * + * Returns true if the desired flow was the previously active flow. + */ +static bool unlink_installed_to_desired(struct installed_flow *i, struct desired_flow *d) { - ovs_assert(i && i->desired_flow && !ovs_list_is_empty(&i->desired_refs)); + struct desired_flow *old_active = installed_flow_get_active(i); + ovs_assert(d && d->installed_flow == i); ovs_list_remove(&d->installed_ref_list_node); d->installed_flow = NULL; - if (i->desired_flow == d) { - i->desired_flow = ovs_list_is_empty(&i->desired_refs) ? NULL : - CONTAINER_OF(ovs_list_front(&i->desired_refs), - struct desired_flow, - installed_ref_list_node); - } + return old_active == d; } static void @@ -1280,7 +1254,6 @@ installed_flow_dup(struct desired_flow *src) { struct installed_flow *dst = xmalloc(sizeof *dst); ovs_list_init(&dst->desired_refs); - dst->desired_flow = NULL; dst->flow.table_id = src->flow.table_id; dst->flow.priority = src->flow.priority; minimatch_clone(&dst->flow.match, &src->flow.match); @@ -1292,6 +1265,17 @@ installed_flow_dup(struct desired_flow *src) } static struct desired_flow * +installed_flow_get_active(struct installed_flow *f) +{ + if (!ovs_list_is_empty(&f->desired_refs)) { + return CONTAINER_OF(ovs_list_front(&f->desired_refs), + struct desired_flow, + installed_ref_list_node); + } + return NULL; +} + +static struct desired_flow * desired_flow_lookup__(struct ovn_desired_flow_table *flow_table, const struct ovn_flow *target, desired_flow_match_cb match_cb, @@ -1439,8 +1423,7 @@ static void installed_flow_destroy(struct installed_flow *f) { if (f) { - ovs_assert(ovs_list_is_empty(&f->desired_refs)); - ovs_assert(!f->desired_flow); + ovs_assert(!installed_flow_get_active(f)); ovn_flow_uninit(&f->flow); free(f); } @@ -1898,10 +1881,10 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, /* The desired flow was deleted */ if (f->installed_flow) { struct installed_flow *i = f->installed_flow; - bool was_active = desired_flow_is_active(f); - unlink_installed_to_desired(i, f); + bool was_active = unlink_installed_to_desired(i, f); + struct desired_flow *d = installed_flow_get_active(i); - if (!i->desired_flow) { + if (!d) { installed_flow_del(&i->flow, msgs); ovn_flow_log(&i->flow, "removing installed (tracked)"); @@ -1912,7 +1895,6 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, * installed flow, so update the OVS flow for the new * active flow (at least the cookie will be different, * even if the actions are the same). */ - struct desired_flow *d = i->desired_flow; ovn_flow_log(&i->flow, "updating installed (tracked)"); installed_flow_mod(&i->flow, &d->flow, msgs); } @@ -1931,7 +1913,7 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, hmap_insert(&installed_flows, &new_node->match_hmap_node, new_node->flow.hash); link_installed_to_desired(new_node, f); - } else if (desired_flow_is_active(f)) { + } else if (installed_flow_get_active(i) == f) { /* The installed flow is installed for f, but f has change * tracked, so it must have been modified. */ ovn_flow_log(&i->flow, "updating installed (tracked)");
Always consider the first "desired flow" in the list of flows that refer an "installed flow" as the active flow. This simplifies the logic of the flow update code and is used in a further commit to implement a partial ordering of desired flows within installed flows. Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- controller/ofctrl.c | 92 +++++++++++++++++++++------------------------------ 1 file changed, 37 insertions(+), 55 deletions(-)