On Mon, Jun 26, 2023 at 7:25 AM Numan Siddique <[email protected]> wrote:
>
> On Sun, Jun 18, 2023 at 11:48 AM Han Zhou <[email protected]> wrote:
> >
> > For incremental processing, it is important to maintain relationship
> > between the inputs and the logical flows generated. This patch creates
> > the links between ovn_port and logical flows. The same data structure
> > may be expanded to maintain links between logical flows and other types
> > of inputs.
> >
> > This patch also refactors the temp_lflow_list operations to
> > collected_lflows with helper functions to start and end collecting. It
> > still uses global variables just to avoid updating all the lflow_add_...
> > related code all over the northd.c file.
> >
> > Signed-off-by: Han Zhou <[email protected]>
>
> Hi Han,
>
> Please see a few comments below. I did review all the 3 patches in the
series.
> They LGTM overall. I'd like to do some more testing before providing my
Acks.
>
Thanks for your review!
>
> > ---
> > northd/northd.c | 271 +++++++++++++++++++++++++++++++-----------------
> > 1 file changed, 178 insertions(+), 93 deletions(-)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 98f528f93cfc..aa0f853ce2db 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -1457,6 +1457,19 @@ struct ovn_port_routable_addresses {
> > size_t n_addrs;
> > };
> >
> > +/* A node that maintains link between an object (such as an ovn_port)
and
> > + * a lflow. */
> > +struct lflow_ref_node {
> > + /* This list follows different lflows referenced by the same
object. List
> > + * head is, for example, ovn_port->lflows. */
> > + struct ovs_list lflow_list_node;
> > + /* This list follows different objects that reference the same
lflow. List
> > + * head is ovn_lflow->referenced_by. */
> > + struct ovs_list ref_list_node;
> > + /* The lflow. */
> > + struct ovn_lflow *lflow;
> > +};
> > +
> > /* A logical switch port or logical router port.
> > *
> > * In steady state, an ovn_port points to a northbound
Logical_Switch_Port
> > @@ -1548,6 +1561,28 @@ struct ovn_port {
> >
> > /* Temporarily used for traversing a list (or hmap) of ports. */
> > bool visited;
> > +
> > + /* List of struct lflow_ref_node that points to the lflows
generated by
> > + * this ovn_port.
> > + *
> > + * This data is initialized and destroyed by the en_northd node,
but
> > + * populated and used only by the en_lflow node. Ideally this data
should
> > + * be maintained as part of en_lflow's data (struct lflow_data): a
hash
> > + * index from ovn_port key to lflows. However, it would be less
efficient
> > + * and more complex:
> > + *
> > + * 1. It would require an extra search (using the index) to find
the
> > + * lflows.
> > + *
> > + * 2. Building the index needs to be thread-safe, using either a
global
> > + * lock which is obviously less efficient, or hash-based lock
array which
> > + * is more complex.
> > + *
> > + * Adding the list here is more straightforward. The drawback is
that we
> > + * need to keep in mind that this data belongs to en_lflow node,
so never
> > + * access it from any other nodes.
> > + */
> > + struct ovs_list lflows;
> > };
>
>
> >
> > static bool lsp_can_be_inc_processed(const struct
nbrec_logical_switch_port *);
> > @@ -1635,6 +1670,8 @@ ovn_port_create(struct hmap *ports, const char
*key,
> > ovn_port_set_nb(op, nbsp, nbrp);
> > op->l3dgw_port = op->cr_port = NULL;
> > hmap_insert(ports, &op->key_node, hash_string(op->key, 0));
> > +
> > + ovs_list_init(&op->lflows);
> > return op;
> > }
> >
> > @@ -1665,6 +1702,13 @@ ovn_port_destroy_orphan(struct ovn_port *port)
> > destroy_lport_addresses(&port->proxy_arp_addrs);
> > free(port->json_key);
> > free(port->key);
> > +
> > + struct lflow_ref_node *l;
> > + LIST_FOR_EACH_SAFE (l, lflow_list_node, &port->lflows) {
> > + ovs_list_remove(&l->lflow_list_node);
> > + ovs_list_remove(&l->ref_list_node);
> > + free(l);
> > + }
> > free(port);
> > }
> >
> > @@ -4893,6 +4937,7 @@ static struct ovn_port *
> > ls_port_create(struct ovsdb_idl_txn *ovnsb_txn, struct hmap *ls_ports,
> > const char *key, const struct nbrec_logical_switch_port
*nbsp,
> > struct ovn_datapath *od, const struct
sbrec_port_binding *sb,
> > + struct ovs_list *lflows,
> > const struct sbrec_mirror_table *sbrec_mirror_table,
> > const struct sbrec_chassis_table *sbrec_chassis_table,
> > struct ovsdb_idl_index *sbrec_chassis_by_name,
> > @@ -4903,6 +4948,9 @@ ls_port_create(struct ovsdb_idl_txn *ovnsb_txn,
struct hmap *ls_ports,
> > parse_lsp_addrs(op);
> > op->od = od;
> > hmap_insert(&od->ports, &op->dp_node,
hmap_node_hash(&op->key_node));
> > + if (lflows) {
> > + ovs_list_splice(&op->lflows, lflows->next, lflows);
> > + }
> >
> > /* Assign explicitly requested tunnel ids first. */
> > if (!ovn_port_assign_requested_tnl_id(sbrec_chassis_table, op)) {
> > @@ -5082,7 +5130,7 @@ northd_handle_ls_changes(struct ovsdb_idl_txn
*ovnsb_idl_txn,
> > goto fail;
> > }
> > op = ls_port_create(ovnsb_idl_txn, &nd->ls_ports,
> > - new_nbsp->name, new_nbsp, od, NULL,
> > + new_nbsp->name, new_nbsp, od,
NULL, NULL,
> > ni->sbrec_mirror_table,
> > ni->sbrec_chassis_table,
> > ni->sbrec_chassis_by_name,
> > @@ -5114,13 +5162,16 @@ northd_handle_ls_changes(struct ovsdb_idl_txn
*ovnsb_idl_txn,
> > op->visited = true;
> > continue;
> > }
> > + struct ovs_list lflows = OVS_LIST_INITIALIZER(&lflows);
> > + ovs_list_splice(&lflows, op->lflows.next, &op->lflows);
> > ovn_port_destroy(&nd->ls_ports, op);
> > op = ls_port_create(ovnsb_idl_txn, &nd->ls_ports,
> > - new_nbsp->name, new_nbsp, od, sb,
> > + new_nbsp->name, new_nbsp, od, sb,
&lflows,
> > ni->sbrec_mirror_table,
> > ni->sbrec_chassis_table,
> > ni->sbrec_chassis_by_name,
> > ni->sbrec_chassis_by_hostname);
> > + ovs_assert(ovs_list_is_empty(&lflows));
> > if (!op) {
> > goto fail;
> > }
> > @@ -5577,7 +5628,8 @@ ovn_igmp_group_destroy(struct hmap *igmp_groups,
> >
> > struct ovn_lflow {
> > struct hmap_node hmap_node;
> > - struct ovs_list list_node;
> > + struct ovs_list list_node; /* For temporary list of lflows.
Don't remove
> > + at destroy. */
> >
> > struct ovn_datapath *od; /* 'logical_datapath' in SB schema.
*/
> > unsigned long *dpg_bitmap; /* Bitmap of all datapaths by their
'index'.*/
> > @@ -5591,6 +5643,8 @@ struct ovn_lflow {
> > size_t n_ods; /* Number of datapaths referenced by
'od' and
> > * 'dpg_bitmap'. */
> > struct ovn_dp_group *dpg; /* Link to unique Sb datapath group.
*/
> > +
> > + struct ovs_list referenced_by; /* List of struct lflow_ref_node.
*/
> > const char *where;
> >
> > struct uuid sb_uuid; /* SB DB row uuid, specified by
northd. */
> > @@ -5640,6 +5694,7 @@ ovn_lflow_init(struct ovn_lflow *lflow, struct
ovn_datapath *od,
> > char *stage_hint, const char *where)
> > {
> > ovs_list_init(&lflow->list_node);
> > + ovs_list_init(&lflow->referenced_by);
> > lflow->dpg_bitmap = bitmap_allocate(dp_bitmap_len);
> > lflow->od = od;
> > lflow->stage = stage;
> > @@ -5767,20 +5822,30 @@ ovn_dp_group_add_with_reference(struct
ovn_lflow *lflow_ref,
> > }
> > }
> >
> > +/* This global variable collects the lflows generated by
do_ovn_lflow_add().
> > + * start_collecting_lflows() will enable the lflow collection and the
calls to
> > + * do_ovn_lflow_add (or the macros ovn_lflow_add_...) will add
generated lflows
> > + * to the list. end_collecting_lflows() will disable it. */
> > +static thread_local struct ovs_list collected_lflows;
> > +static thread_local bool collecting_lflows = false;
> > +
> > +static void
> > +start_collecting_lflows(void)
> > +{
> > + ovs_assert(!collecting_lflows);
> > + ovs_list_init(&collected_lflows);
> > + collecting_lflows = true;
> > +}
> > +
> > +static void
> > +end_collecting_lflows(void)
> > +{
> > + ovs_assert(collecting_lflows);
> > + collecting_lflows = false;
> > +}
> > +
>
> I think we can avoid these functions and the thread local variable
> "collected_lflows".
>
> I'd suggest the below
>
> ----------------------------
>
> static void
> do_ovn_lflow_add(struct hmap *lflow_map, const struct ovn_datapath *od,
> const unsigned long *dp_bitmap, size_t dp_bitmap_len,
> uint32_t hash, enum ovn_stage stage, uint16_t priority,
> const char *match, const char *actions, const char
*io_port,
> struct ovs_list *lflow_ref_list,
> const struct ovsdb_idl_row *stage_hint,
> const char *where, const char *ctrl_meter)
> OVS_REQUIRES(fake_hash_mutex)
> {
> ...
> ...
> /* At the end. */
> if (lflow_ref_list) {
> struct lflow_ref_node *lfrn = xmalloc(sizeof *lfrn);
> lfrn->lflow = lflow;
> ovs_list_insert(lflow_ref_list, &lfrn->lflow_list_node);
> ovs_list_insert(&lflow->referenced_by, &lfrn->ref_list_node);
> }
> }
>
>
> #define ovn_lflow_add_with_lport_and_hint(LFLOW_MAP, OD, STAGE, PRIORITY,
\
> MATCH, ACTIONS, IN_OUT_PORT, \
> LFLOW_REF_LIST, STAGE_HINT) \
> ovn_lflow_add_at(LFLOW_MAP, OD, NULL, 0, STAGE, PRIORITY, MATCH,
ACTIONS, \
> IN_OUT_PORT, LFLOW_REF_LIST, NULL, STAGE_HINT, \
> OVS_SOURCE_LOCATOR)
>
> And pass &op->lflows in ovn_lflow_add_with_lport_and_hint()
>
> -----------------------------
>
> What do you think ? Definitely this would result in a bit more work
> as it would require a lot of (tedious) changes.
> I think this is a better approach.
>
Firstly, I think it is not good to use "lflow_ref_list" directly in the
parameter, because there can be more than one object contributing to the
lflow generation. When we implement I-P for more inputs, a single lflow may
be referenced by multiple objects. We can't pass multiple lflow_ref_list to
the function either, because the number of such lists is unknown. For
example, a lflow may be generated as a result of a LSP, a DP and a LB
backend. If we want to implement I-P for LSP, DP and LB backend, we need to
track the reference for all of them. So the current idea is just to collect
a list of lflows generated by a higher level function, such as the
build_lswitch_and_lrouter_iterate_by_lsp. When implementing I-P for more
than one object of the same lflow, this needs to be more fine-grained.
Secondly, I agree with you adding a new parameter to the do_ovn_lflow_add
is cleaner. For the collected list I mentioned, it can be a parameter
instead of a thread local variable. However, as you mentioned, the change
is going to be all over the northd.c code, not only for the
ovn_lflow_add_xxx, but also the functions calling the ovn_lflow_add_xxx
macros, and upper layer functions calling those functions, and so on.
Unfortunately C doesn't support optional args. At this moment I am not sure
if the interface is stable enough for the incremental-processing, so I am
not sure if it is worth such a big change. If we need to modify them again
later, all such changes are going to be wasted. On the other hand, although
the thread local variable is not the best way, I think it is still clear
and manageable, if we call the start_collecting_lflows and
end_collecting_lflows in pairs. So, is it ok to leave it for this patch and
in the future when this mechanism proves to work well for more I-P, we can
have a separate patch to refactor (which will include all the tedious
function call changes). What do you think?
Thanks,
Han
> Also I'm planning to work on top of your patches to add I-P for load
> balancers in lflow engine (or perhaps I-P for datapath changes)
>
> My idea is to have a lflow ref list stored in "struct ovn_datapath"
> similar to the way you have done in this patch in "struct ovn_port"
>
> And while adding the flows using one of the macro variants
> 'ovn_lflow_add*' pass &od->lflows.
>
> Please let me know your comments.
>
> Only concern I have with this patch is the "op->lflows" modified by
> the lflow engine node.
> But I agree with your added comments and also thinking to use the same
> approach for datapath I-P handling,
> And I don't have a better approach at the moment. So I'm fine with it.
>
> Thanks
> Numan
>
>
> > /* Adds a row with the specified contents to the Logical_Flow table.
> > - * Version to use when hash bucket locking is NOT required.
> > - *
> > - * Note: This function can add generated lflows to the global variable
> > - * temp_lflow_list as its output, controlled by the global variable
> > - * add_lflow_to_temp_list. The caller of the ovn_lflow_add_... marcros
can get
> > - * a list of lflows generated by setting add_lflow_to_temp_list to
true. The
> > - * caller is responsible for initializing the temp_lflow_list, and also
> > - * reset the add_lflow_to_temp_list to false when it is no longer
needed.
> > - * XXX: this mechanism is temporary and will be replaced when we add
hash index
> > - * to lflow_data and refactor related functions.
> > - */
> > -static bool add_lflow_to_temp_list = false;
> > -static struct ovs_list temp_lflow_list;
> > + * Version to use when hash bucket locking is NOT required. */
> > static void
> > do_ovn_lflow_add(struct hmap *lflow_map, const struct ovn_datapath *od,
> > const unsigned long *dp_bitmap, size_t dp_bitmap_len,
> > @@ -5797,7 +5862,7 @@ do_ovn_lflow_add(struct hmap *lflow_map, const
struct ovn_datapath *od,
> > size_t bitmap_len = od ? ods_size(od->datapaths) : dp_bitmap_len;
> > ovs_assert(bitmap_len);
> >
> > - if (add_lflow_to_temp_list) {
> > + if (collecting_lflows) {
> > ovs_assert(od);
> > ovs_assert(!dp_bitmap);
> > } else {
> > @@ -5829,8 +5894,8 @@ do_ovn_lflow_add(struct hmap *lflow_map, const
struct ovn_datapath *od,
> > thread_lflow_counter++;
> > }
> >
> > - if (add_lflow_to_temp_list) {
> > - ovs_list_insert(&temp_lflow_list, &lflow->list_node);
> > + if (collecting_lflows) {
> > + ovs_list_insert(&collected_lflows, &lflow->list_node);
> > }
> > }
> >
> > @@ -5950,10 +6015,28 @@ ovn_lflow_destroy(struct hmap *lflows, struct
ovn_lflow *lflow)
> > free(lflow->io_port);
> > free(lflow->stage_hint);
> > free(lflow->ctrl_meter);
> > + struct lflow_ref_node *l;
> > + LIST_FOR_EACH_SAFE (l, ref_list_node, &lflow->referenced_by) {
> > + ovs_list_remove(&l->lflow_list_node);
> > + ovs_list_remove(&l->ref_list_node);
> > + free(l);
> > + }
> > free(lflow);
> > }
> > }
> >
> > +static void
> > +link_ovn_port_to_lflows(struct ovn_port *op, struct ovs_list *lflows)
> > +{
> > + struct ovn_lflow *f;
> > + LIST_FOR_EACH (f, list_node, lflows) {
> > + struct lflow_ref_node *lfrn = xmalloc(sizeof *lfrn);
> > + lfrn->lflow = f;
> > + ovs_list_insert(&op->lflows, &lfrn->lflow_list_node);
> > + ovs_list_insert(&f->referenced_by, &lfrn->ref_list_node);
> > + }
> > +}
> > +
> > static bool
> > build_dhcpv4_action(struct ovn_port *op, ovs_be32 offer_ip,
> > struct ds *options_action, struct ds
*response_action,
> > @@ -15483,6 +15566,7 @@ build_lswitch_and_lrouter_iterate_by_lsp(struct
ovn_port *op,
> > struct hmap *lflows)
> > {
> > ovs_assert(op->nbsp);
> > + start_collecting_lflows();
> >
> > /* Build Logical Switch Flows. */
> > build_lswitch_port_sec_op(op, lflows, actions, match);
> > @@ -15497,6 +15581,9 @@ build_lswitch_and_lrouter_iterate_by_lsp(struct
ovn_port *op,
> > /* Build Logical Router Flows. */
> > build_ip_routing_flows_for_router_type_lsp(op, lr_ports, lflows);
> > build_arp_resolve_flows_for_lsp(op, lflows, lr_ports, match,
actions);
> > +
> > + link_ovn_port_to_lflows(op, &collected_lflows);
> > + end_collecting_lflows();
> > }
> >
> > /* Helper function to combine all lflow generation which is iterated
by logical
> > @@ -16223,14 +16310,10 @@ bool lflow_handle_northd_ls_changes(struct
ovsdb_idl_txn *ovnsb_txn,
> > {
> > struct ls_change *ls_change;
> > LIST_FOR_EACH (ls_change, list_node, &ls_changes->updated) {
> > - ovs_list_init(&temp_lflow_list);
> > - add_lflow_to_temp_list = true;
> > if (!ovs_list_is_empty(&ls_change->updated_ports) ||
> > !ovs_list_is_empty(&ls_change->deleted_ports)) {
> > /* XXX: implement lflow index so that we can handle
updated and
> > * deleted LSPs incrementally. */
> > - ovs_list_init(&temp_lflow_list);
> > - add_lflow_to_temp_list = false;
> > return false;
> > }
> >
> > @@ -16277,83 +16360,85 @@ bool lflow_handle_northd_ls_changes(struct
ovsdb_idl_txn *ovnsb_txn,
> >
sbrec_multicast_group_update_ports_addvalue(sbmc_unknown,
> > op->sb);
> > }
> > - }
> > - /* Sync the newly added flows to SB. */
> > - struct ovn_lflow *lflow;
> > - LIST_FOR_EACH (lflow, list_node, &temp_lflow_list) {
> > - size_t n_datapaths;
> > - struct ovn_datapath **datapaths_array;
> > - if (ovn_stage_to_datapath_type(lflow->stage) == DP_SWITCH)
{
> > - n_datapaths = ods_size(lflow_input->ls_datapaths);
> > - datapaths_array = lflow_input->ls_datapaths->array;
> > - } else {
> > - n_datapaths = ods_size(lflow_input->lr_datapaths);
> > - datapaths_array = lflow_input->lr_datapaths->array;
> > - }
> > - uint32_t n_ods = bitmap_count1(lflow->dpg_bitmap,
n_datapaths);
> > - ovs_assert(n_ods == 1);
> > - /* There is only one datapath, so it should be moved out
of the
> > - * group to a single 'od'. */
> > - size_t index = bitmap_scan(lflow->dpg_bitmap, true, 0,
> > - n_datapaths);
> >
> > - bitmap_set0(lflow->dpg_bitmap, index);
> > - lflow->od = datapaths_array[index];
> > -
> > - /* Logical flow should be re-hashed to allow lookups. */
> > - uint32_t hash = hmap_node_hash(&lflow->hmap_node);
> > - /* Remove from lflows. */
> > - hmap_remove(lflows, &lflow->hmap_node);
> > - hash =
ovn_logical_flow_hash_datapath(&lflow->od->sb->header_.uuid,
> > - hash);
> > - /* Add back. */
> > - hmap_insert(lflows, &lflow->hmap_node, hash);
> > -
> > - /* Sync to SB. */
> > - const struct sbrec_logical_flow *sbflow;
> > - lflow->sb_uuid = uuid_random();
> > - sbflow = sbrec_logical_flow_insert_persist_uuid(ovnsb_txn,
> > -
&lflow->sb_uuid);
> > - const char *pipeline =
ovn_stage_get_pipeline_name(lflow->stage);
> > - uint8_t table = ovn_stage_get_table(lflow->stage);
> > - sbrec_logical_flow_set_logical_datapath(sbflow,
lflow->od->sb);
> > - sbrec_logical_flow_set_logical_dp_group(sbflow, NULL);
> > - sbrec_logical_flow_set_pipeline(sbflow, pipeline);
> > - sbrec_logical_flow_set_table_id(sbflow, table);
> > - sbrec_logical_flow_set_priority(sbflow, lflow->priority);
> > - sbrec_logical_flow_set_match(sbflow, lflow->match);
> > - sbrec_logical_flow_set_actions(sbflow, lflow->actions);
> > - if (lflow->io_port) {
> > - struct smap tags = SMAP_INITIALIZER(&tags);
> > - smap_add(&tags, "in_out_port", lflow->io_port);
> > - sbrec_logical_flow_set_tags(sbflow, &tags);
> > - smap_destroy(&tags);
> > - }
> > - sbrec_logical_flow_set_controller_meter(sbflow,
lflow->ctrl_meter);
> > - /* Trim the source locator lflow->where, which looks
something like
> > - * "ovn/northd/northd.c:1234", down to just the part
following the
> > - * last slash, e.g. "northd.c:1234". */
> > - const char *slash = strrchr(lflow->where, '/');
> > + /* Sync the newly added flows to SB. */
> > + struct lflow_ref_node *lfrn;
> > + LIST_FOR_EACH (lfrn, lflow_list_node, &op->lflows) {
> > + struct ovn_lflow *lflow = lfrn->lflow;
> > + size_t n_datapaths;
> > + struct ovn_datapath **datapaths_array;
> > + if (ovn_stage_to_datapath_type(lflow->stage) ==
DP_SWITCH) {
> > + n_datapaths = ods_size(lflow_input->ls_datapaths);
> > + datapaths_array = lflow_input->ls_datapaths->array;
> > + } else {
> > + n_datapaths = ods_size(lflow_input->lr_datapaths);
> > + datapaths_array = lflow_input->lr_datapaths->array;
> > + }
> > + uint32_t n_ods = bitmap_count1(lflow->dpg_bitmap,
n_datapaths);
> > + ovs_assert(n_ods == 1);
> > + /* There is only one datapath, so it should be moved
out of the
> > + * group to a single 'od'. */
> > + size_t index = bitmap_scan(lflow->dpg_bitmap, true, 0,
> > + n_datapaths);
> > +
> > + bitmap_set0(lflow->dpg_bitmap, index);
> > + lflow->od = datapaths_array[index];
> > +
> > + /* Logical flow should be re-hashed to allow lookups.
*/
> > + uint32_t hash = hmap_node_hash(&lflow->hmap_node);
> > + /* Remove from lflows. */
> > + hmap_remove(lflows, &lflow->hmap_node);
> > + hash = ovn_logical_flow_hash_datapath(
> > +
&lflow->od->sb->header_.uuid, hash);
> > + /* Add back. */
> > + hmap_insert(lflows, &lflow->hmap_node, hash);
> > +
> > + /* Sync to SB. */
> > + const struct sbrec_logical_flow *sbflow;
> > + lflow->sb_uuid = uuid_random();
> > + sbflow = sbrec_logical_flow_insert_persist_uuid(
> > + ovnsb_txn,
&lflow->sb_uuid);
> > + const char *pipeline = ovn_stage_get_pipeline_name(
> > +
lflow->stage);
> > + uint8_t table = ovn_stage_get_table(lflow->stage);
> > + sbrec_logical_flow_set_logical_datapath(sbflow,
lflow->od->sb);
> > + sbrec_logical_flow_set_logical_dp_group(sbflow, NULL);
> > + sbrec_logical_flow_set_pipeline(sbflow, pipeline);
> > + sbrec_logical_flow_set_table_id(sbflow, table);
> > + sbrec_logical_flow_set_priority(sbflow,
lflow->priority);
> > + sbrec_logical_flow_set_match(sbflow, lflow->match);
> > + sbrec_logical_flow_set_actions(sbflow, lflow->actions);
> > + if (lflow->io_port) {
> > + struct smap tags = SMAP_INITIALIZER(&tags);
> > + smap_add(&tags, "in_out_port", lflow->io_port);
> > + sbrec_logical_flow_set_tags(sbflow, &tags);
> > + smap_destroy(&tags);
> > + }
> > + sbrec_logical_flow_set_controller_meter(sbflow,
> > +
lflow->ctrl_meter);
> > + /* Trim the source locator lflow->where, which looks
something
> > + * like "ovn/northd/northd.c:1234", down to just the
part
> > + * following the last slash, e.g. "northd.c:1234". */
> > + const char *slash = strrchr(lflow->where, '/');
> > #if _WIN32
> > - const char *backslash = strrchr(lflow->where, '\\');
> > - if (!slash || backslash > slash) {
> > - slash = backslash;
> > - }
> > + const char *backslash = strrchr(lflow->where, '\\');
> > + if (!slash || backslash > slash) {
> > + slash = backslash;
> > + }
> > #endif
> > - const char *where = slash ? slash + 1 : lflow->where;
> > + const char *where = slash ? slash + 1 : lflow->where;
> >
> > - struct smap ids = SMAP_INITIALIZER(&ids);
> > - smap_add(&ids, "stage-name",
ovn_stage_to_str(lflow->stage));
> > - smap_add(&ids, "source", where);
> > - if (lflow->stage_hint) {
> > - smap_add(&ids, "stage-hint", lflow->stage_hint);
> > + struct smap ids = SMAP_INITIALIZER(&ids);
> > + smap_add(&ids, "stage-name",
ovn_stage_to_str(lflow->stage));
> > + smap_add(&ids, "source", where);
> > + if (lflow->stage_hint) {
> > + smap_add(&ids, "stage-hint", lflow->stage_hint);
> > + }
> > + sbrec_logical_flow_set_external_ids(sbflow, &ids);
> > + smap_destroy(&ids);
> > }
> > - sbrec_logical_flow_set_external_ids(sbflow, &ids);
> > - smap_destroy(&ids);
> > }
> > }
> > - ovs_list_init(&temp_lflow_list);
> > - add_lflow_to_temp_list = false;
> > return true;
> >
> > }
> > --
> > 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
