> On 5/28/25 12:18 PM, Lorenzo Bianconi wrote:
> > Introduce en_route_exchange_status I-P node to store Last route_exchange
> > netlink operation and immediately trigger en_route_exchange if netlink
> > configuration fails.
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com>
> > ---
>
> Hi Lorenzo,
Hi Dumitru,
>
> > controller/ovn-controller.c | 52 ++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 51 insertions(+), 1 deletion(-)
> >
> > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > index 89207bb4b..7578c2b37 100644
> > --- a/controller/ovn-controller.c
> > +++ b/controller/ovn-controller.c
> > @@ -5288,6 +5288,18 @@ struct ed_type_route_exchange {
> > struct ovsdb_idl *sb_idl;
> > };
> >
> > +/* Last route_exchange netlink operation. */
> > +static bool route_exchange_nl_status;
> > +
> > +static bool
> > +route_exchange_status_run(void)
> > +{
> > + if (route_exchange_nl_status) {
> > + poll_immediate_wake();
>
> I would move this poll_immediate_wake() to
> en_route_exchange_status_run(), when we set the state to EN_UPDATED.
ack, fine. I will fix it in v3.
>
> > + }
> > + return route_exchange_nl_status;
> > +}
> > +
>
> I think the code would be better encapsulated if we move this chunk
> (static variable and _run() function) to route-exchange.c. We could
> export the route_exchange_status_run() function in route-exchange.h.
>
> route_exchange_run() could then set the route_exchange_nl_status
> variable internally.
ack, fine. I will fix it in v3.
Regards,
Lorenzo
>
> > static enum engine_node_state
> > en_route_exchange_run(struct engine_node *node, void *data)
> > {
> > @@ -5323,7 +5335,7 @@ en_route_exchange_run(struct engine_node *node, void
> > *data)
> >
> > hmap_init(&r_ctx_out.route_table_watches);
> >
> > - route_exchange_run(&r_ctx_in, &r_ctx_out);
> > + route_exchange_nl_status = !!route_exchange_run(&r_ctx_in, &r_ctx_out);
> > route_table_notify_update_watches(&r_ctx_out.route_table_watches);
> >
> > route_table_watch_request_cleanup(&r_ctx_out.route_table_watches);
> > @@ -5383,6 +5395,38 @@ en_route_table_notify_cleanup(void *data OVS_UNUSED)
> > {
> > }
> >
> > +struct ed_type_route_exchange_status {
> > + bool netlink_trigger_run;
> > +};
> > +
> > +static void *
> > +en_route_exchange_status_init(struct engine_node *node OVS_UNUSED,
> > + struct engine_arg *arg OVS_UNUSED)
> > +{
> > + return xzalloc(sizeof(struct ed_type_route_exchange_status));
> > +}
> > +
> > +static enum engine_node_state
> > +en_route_exchange_status_run(struct engine_node *node OVS_UNUSED, void
> > *data)
> > +{
> > + struct ed_type_route_exchange_status *res = data;
> > + enum engine_node_state state;
> > +
> > + if (res->netlink_trigger_run) {
> > + state = EN_UPDATED;
> > + } else {
> > + state = EN_UNCHANGED;
> > + }
> > + res->netlink_trigger_run = false;
> > +
> > + return state;
> > +}
> > +
> > +static void
> > +en_route_exchange_status_cleanup(void *data OVS_UNUSED)
> > +{
> > +}
> > +
> > /* Returns false if the northd internal version stored in SB_Global
> > * and ovn-controller internal version don't match.
> > */
> > @@ -5690,6 +5734,7 @@ main(int argc, char *argv[])
> > ENGINE_NODE(route);
> > ENGINE_NODE(route_table_notify);
> > ENGINE_NODE(route_exchange);
> > + ENGINE_NODE(route_exchange_status);
> >
> > #define SB_NODE(NAME) ENGINE_NODE_SB(NAME);
> > SB_NODES
> > @@ -5727,6 +5772,7 @@ main(int argc, char *argv[])
> > engine_add_input(&en_route_exchange, &en_sb_port_binding,
> > engine_noop_handler);
> > engine_add_input(&en_route_exchange, &en_route_table_notify, NULL);
> > + engine_add_input(&en_route_exchange, &en_route_exchange_status, NULL);
> >
> > engine_add_input(&en_addr_sets, &en_sb_address_set,
> > addr_sets_sb_address_set_handler);
> > @@ -6258,6 +6304,10 @@ main(int argc, char *argv[])
> > engine_get_internal_data(&en_route_table_notify);
> > rtn->changed = route_table_notify_run();
> >
> > + struct ed_type_route_exchange_status *res =
> > +
> > engine_get_internal_data(&en_route_exchange_status);
> > + res->netlink_trigger_run = route_exchange_status_run();
> > +
> > stopwatch_start(CONTROLLER_LOOP_STOPWATCH_NAME,
> > time_msec());
> >
>
> Regards,
> Dumitru
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
