| Message ID | c91e9ec1416e6bbe87f4089ad56addf9a7284902.1730713432.git.felix.huettner@stackit.cloud |
|---|---|
| State | Superseded |
| Headers | show |
| Series | OVN Fabric integration | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
On Nov 04, Felix Huettner via dev wrote: > learned routes must be bound to a lrp on which the routes where learned. > In case the lrp is deleted for whatever reason no ovn-controller would > clean these routes up, therefor we do this in northd. > > Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud> > --- > northd/en-northd.c | 2 ++ > northd/inc-proc-northd.c | 1 + > northd/northd.c | 31 ++++++++++++++++++++++++++++--- > northd/northd.h | 1 + > 4 files changed, 32 insertions(+), 3 deletions(-) > > diff --git a/northd/en-northd.c b/northd/en-northd.c > index 6e90336f6..8152ccbcf 100644 > --- a/northd/en-northd.c > +++ b/northd/en-northd.c > @@ -101,6 +101,8 @@ northd_get_input_data(struct engine_node *node, > EN_OVSDB_GET(engine_get_input("SB_chassis_template_var", node)); > input_data->sbrec_mirror_table = > EN_OVSDB_GET(engine_get_input("SB_mirror", node)); > + input_data->sbrec_route_table = > + EN_OVSDB_GET(engine_get_input("SB_route", node)); > > struct ed_type_lb_data *lb_data = > engine_get_input_data("lb_data", node); > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > index 741295709..59cb50853 100644 > --- a/northd/inc-proc-northd.c > +++ b/northd/inc-proc-northd.c > @@ -204,6 +204,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > engine_add_input(&en_northd, &en_sb_service_monitor, NULL); > engine_add_input(&en_northd, &en_sb_static_mac_binding, NULL); > engine_add_input(&en_northd, &en_sb_chassis_template_var, NULL); > + engine_add_input(&en_northd, &en_sb_route, NULL); > engine_add_input(&en_northd, &en_sb_fdb, northd_sb_fdb_change_handler); > engine_add_input(&en_northd, &en_global_config, > northd_global_config_handler); > diff --git a/northd/northd.c b/northd/northd.c > index b4412e70c..d93137a2d 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -3417,6 +3417,28 @@ cleanup_mac_bindings( > } > } > > +/* Remove received route entries that refer to logical_ports which are > + * deleted. */ > +static void > +cleanup_routes( > + const struct sbrec_route_table *sbrec_route_table, > + struct hmap *lr_datapaths, struct hmap *lr_ports) > +{ > + const struct sbrec_route *r; > + SBREC_ROUTE_TABLE_FOR_EACH_SAFE (r, sbrec_route_table) { > + const struct ovn_datapath *od = > + ovn_datapath_from_sbrec(NULL, lr_datapaths, r->datapath); > + if (strcmp(r->type, "receive")) { > + continue; > + } > + > + if (!od || ovn_datapath_is_stale(od) || > + !ovn_port_find(lr_ports, r->logical_port)) { > + sbrec_route_delete(r); > + } > + } > +} > + > static void > cleanup_sb_ha_chassis_groups( > const struct sbrec_ha_chassis_group_table *sbrec_ha_chassis_group_table, > @@ -4220,6 +4242,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn, > const struct sbrec_mirror_table *sbrec_mirror_table, > const struct sbrec_mac_binding_table *sbrec_mac_binding_table, > const struct sbrec_ha_chassis_group_table *sbrec_ha_chassis_group_table, > + const struct sbrec_route_table *sbrec_route_table, > struct ovsdb_idl_index *sbrec_chassis_by_name, > struct ovsdb_idl_index *sbrec_chassis_by_hostname, > struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name, > @@ -4245,7 +4268,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn, > &tag_alloc_table, &sb_only, &nb_only, &both); > > /* Purge stale Mac_Bindings if ports are deleted. */ > - bool remove_mac_bindings = !ovs_list_is_empty(&sb_only); > + bool any_sb_port_deleted = !ovs_list_is_empty(&sb_only); > > /* Assign explicitly requested tunnel ids first. */ > struct ovn_port *op; > @@ -4287,7 +4310,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn, > * Mac_Bindings are purged. > */ > if (op->od->sb != op->sb->datapath) { > - remove_mac_bindings = true; > + any_sb_port_deleted = true; > } > if (op->nbsp) { > tag_alloc_create_new_tag(&tag_alloc_table, op->nbsp); > @@ -4333,8 +4356,9 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn, > hmap_insert(lr_ports, &op->key_node, op->key_node.hash); > } > > - if (remove_mac_bindings) { > + if (any_sb_port_deleted) { > cleanup_mac_bindings(sbrec_mac_binding_table, lr_datapaths, lr_ports); > + cleanup_routes(sbrec_route_table, lr_datapaths, lr_ports); Can we manage it in en_routes_sync_run()? Regards, Lorenzo > } > > tag_alloc_destroy(&tag_alloc_table); > @@ -19021,6 +19045,7 @@ ovnnb_db_run(struct northd_input *input_data, > input_data->sbrec_mirror_table, > input_data->sbrec_mac_binding_table, > input_data->sbrec_ha_chassis_group_table, > + input_data->sbrec_route_table, > input_data->sbrec_chassis_by_name, > input_data->sbrec_chassis_by_hostname, > input_data->sbrec_ha_chassis_grp_by_name, > diff --git a/northd/northd.h b/northd/northd.h > index 126d58626..7b318087a 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -51,6 +51,7 @@ struct northd_input { > const struct sbrec_chassis_template_var_table > *sbrec_chassis_template_var_table; > const struct sbrec_mirror_table *sbrec_mirror_table; > + const struct sbrec_route_table *sbrec_route_table; > > /* Northd lb data node inputs*/ > const struct hmap *lbs; > -- > 2.47.0 > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Thu, Nov 14, 2024 at 03:14:08PM +0100, Lorenzo Bianconi wrote: > On Nov 04, Felix Huettner via dev wrote: > > learned routes must be bound to a lrp on which the routes where learned. > > In case the lrp is deleted for whatever reason no ovn-controller would > > clean these routes up, therefor we do this in northd. > > > > Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud> > > --- > > northd/en-northd.c | 2 ++ > > northd/inc-proc-northd.c | 1 + > > northd/northd.c | 31 ++++++++++++++++++++++++++++--- > > northd/northd.h | 1 + > > 4 files changed, 32 insertions(+), 3 deletions(-) > > > > diff --git a/northd/en-northd.c b/northd/en-northd.c > > index 6e90336f6..8152ccbcf 100644 > > --- a/northd/en-northd.c > > +++ b/northd/en-northd.c > > @@ -101,6 +101,8 @@ northd_get_input_data(struct engine_node *node, > > EN_OVSDB_GET(engine_get_input("SB_chassis_template_var", node)); > > input_data->sbrec_mirror_table = > > EN_OVSDB_GET(engine_get_input("SB_mirror", node)); > > + input_data->sbrec_route_table = > > + EN_OVSDB_GET(engine_get_input("SB_route", node)); > > > > struct ed_type_lb_data *lb_data = > > engine_get_input_data("lb_data", node); > > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > > index 741295709..59cb50853 100644 > > --- a/northd/inc-proc-northd.c > > +++ b/northd/inc-proc-northd.c > > @@ -204,6 +204,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > > engine_add_input(&en_northd, &en_sb_service_monitor, NULL); > > engine_add_input(&en_northd, &en_sb_static_mac_binding, NULL); > > engine_add_input(&en_northd, &en_sb_chassis_template_var, NULL); > > + engine_add_input(&en_northd, &en_sb_route, NULL); > > engine_add_input(&en_northd, &en_sb_fdb, northd_sb_fdb_change_handler); > > engine_add_input(&en_northd, &en_global_config, > > northd_global_config_handler); > > diff --git a/northd/northd.c b/northd/northd.c > > index b4412e70c..d93137a2d 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -3417,6 +3417,28 @@ cleanup_mac_bindings( > > } > > } > > > > +/* Remove received route entries that refer to logical_ports which are > > + * deleted. */ > > +static void > > +cleanup_routes( > > + const struct sbrec_route_table *sbrec_route_table, > > + struct hmap *lr_datapaths, struct hmap *lr_ports) > > +{ > > + const struct sbrec_route *r; > > + SBREC_ROUTE_TABLE_FOR_EACH_SAFE (r, sbrec_route_table) { > > + const struct ovn_datapath *od = > > + ovn_datapath_from_sbrec(NULL, lr_datapaths, r->datapath); > > + if (strcmp(r->type, "receive")) { > > + continue; > > + } > > + > > + if (!od || ovn_datapath_is_stale(od) || > > + !ovn_port_find(lr_ports, r->logical_port)) { > > + sbrec_route_delete(r); > > + } > > + } > > +} > > + > > static void > > cleanup_sb_ha_chassis_groups( > > const struct sbrec_ha_chassis_group_table *sbrec_ha_chassis_group_table, > > @@ -4220,6 +4242,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn, > > const struct sbrec_mirror_table *sbrec_mirror_table, > > const struct sbrec_mac_binding_table *sbrec_mac_binding_table, > > const struct sbrec_ha_chassis_group_table *sbrec_ha_chassis_group_table, > > + const struct sbrec_route_table *sbrec_route_table, > > struct ovsdb_idl_index *sbrec_chassis_by_name, > > struct ovsdb_idl_index *sbrec_chassis_by_hostname, > > struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name, > > @@ -4245,7 +4268,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn, > > &tag_alloc_table, &sb_only, &nb_only, &both); > > > > /* Purge stale Mac_Bindings if ports are deleted. */ > > - bool remove_mac_bindings = !ovs_list_is_empty(&sb_only); > > + bool any_sb_port_deleted = !ovs_list_is_empty(&sb_only); > > > > /* Assign explicitly requested tunnel ids first. */ > > struct ovn_port *op; > > @@ -4287,7 +4310,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn, > > * Mac_Bindings are purged. > > */ > > if (op->od->sb != op->sb->datapath) { > > - remove_mac_bindings = true; > > + any_sb_port_deleted = true; > > } > > if (op->nbsp) { > > tag_alloc_create_new_tag(&tag_alloc_table, op->nbsp); > > @@ -4333,8 +4356,9 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn, > > hmap_insert(lr_ports, &op->key_node, op->key_node.hash); > > } > > > > - if (remove_mac_bindings) { > > + if (any_sb_port_deleted) { > > cleanup_mac_bindings(sbrec_mac_binding_table, lr_datapaths, lr_ports); > > + cleanup_routes(sbrec_route_table, lr_datapaths, lr_ports); > > Can we manage it in en_routes_sync_run()? Hi Lorenzo, yes will do so for v3. This makes the change also a lot smaller. Thanks for the review Felix > > Regards, > Lorenzo > > > } > > > > tag_alloc_destroy(&tag_alloc_table); > > @@ -19021,6 +19045,7 @@ ovnnb_db_run(struct northd_input *input_data, > > input_data->sbrec_mirror_table, > > input_data->sbrec_mac_binding_table, > > input_data->sbrec_ha_chassis_group_table, > > + input_data->sbrec_route_table, > > input_data->sbrec_chassis_by_name, > > input_data->sbrec_chassis_by_hostname, > > input_data->sbrec_ha_chassis_grp_by_name, > > diff --git a/northd/northd.h b/northd/northd.h > > index 126d58626..7b318087a 100644 > > --- a/northd/northd.h > > +++ b/northd/northd.h > > @@ -51,6 +51,7 @@ struct northd_input { > > const struct sbrec_chassis_template_var_table > > *sbrec_chassis_template_var_table; > > const struct sbrec_mirror_table *sbrec_mirror_table; > > + const struct sbrec_route_table *sbrec_route_table; > > > > /* Northd lb data node inputs*/ > > const struct hmap *lbs; > > -- > > 2.47.0 > > > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/northd/en-northd.c b/northd/en-northd.c index 6e90336f6..8152ccbcf 100644 --- a/northd/en-northd.c +++ b/northd/en-northd.c @@ -101,6 +101,8 @@ northd_get_input_data(struct engine_node *node, EN_OVSDB_GET(engine_get_input("SB_chassis_template_var", node)); input_data->sbrec_mirror_table = EN_OVSDB_GET(engine_get_input("SB_mirror", node)); + input_data->sbrec_route_table = + EN_OVSDB_GET(engine_get_input("SB_route", node)); struct ed_type_lb_data *lb_data = engine_get_input_data("lb_data", node); diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index 741295709..59cb50853 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -204,6 +204,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(&en_northd, &en_sb_service_monitor, NULL); engine_add_input(&en_northd, &en_sb_static_mac_binding, NULL); engine_add_input(&en_northd, &en_sb_chassis_template_var, NULL); + engine_add_input(&en_northd, &en_sb_route, NULL); engine_add_input(&en_northd, &en_sb_fdb, northd_sb_fdb_change_handler); engine_add_input(&en_northd, &en_global_config, northd_global_config_handler); diff --git a/northd/northd.c b/northd/northd.c index b4412e70c..d93137a2d 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -3417,6 +3417,28 @@ cleanup_mac_bindings( } } +/* Remove received route entries that refer to logical_ports which are + * deleted. */ +static void +cleanup_routes( + const struct sbrec_route_table *sbrec_route_table, + struct hmap *lr_datapaths, struct hmap *lr_ports) +{ + const struct sbrec_route *r; + SBREC_ROUTE_TABLE_FOR_EACH_SAFE (r, sbrec_route_table) { + const struct ovn_datapath *od = + ovn_datapath_from_sbrec(NULL, lr_datapaths, r->datapath); + if (strcmp(r->type, "receive")) { + continue; + } + + if (!od || ovn_datapath_is_stale(od) || + !ovn_port_find(lr_ports, r->logical_port)) { + sbrec_route_delete(r); + } + } +} + static void cleanup_sb_ha_chassis_groups( const struct sbrec_ha_chassis_group_table *sbrec_ha_chassis_group_table, @@ -4220,6 +4242,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn, const struct sbrec_mirror_table *sbrec_mirror_table, const struct sbrec_mac_binding_table *sbrec_mac_binding_table, const struct sbrec_ha_chassis_group_table *sbrec_ha_chassis_group_table, + const struct sbrec_route_table *sbrec_route_table, struct ovsdb_idl_index *sbrec_chassis_by_name, struct ovsdb_idl_index *sbrec_chassis_by_hostname, struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name, @@ -4245,7 +4268,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn, &tag_alloc_table, &sb_only, &nb_only, &both); /* Purge stale Mac_Bindings if ports are deleted. */ - bool remove_mac_bindings = !ovs_list_is_empty(&sb_only); + bool any_sb_port_deleted = !ovs_list_is_empty(&sb_only); /* Assign explicitly requested tunnel ids first. */ struct ovn_port *op; @@ -4287,7 +4310,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn, * Mac_Bindings are purged. */ if (op->od->sb != op->sb->datapath) { - remove_mac_bindings = true; + any_sb_port_deleted = true; } if (op->nbsp) { tag_alloc_create_new_tag(&tag_alloc_table, op->nbsp); @@ -4333,8 +4356,9 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn, hmap_insert(lr_ports, &op->key_node, op->key_node.hash); } - if (remove_mac_bindings) { + if (any_sb_port_deleted) { cleanup_mac_bindings(sbrec_mac_binding_table, lr_datapaths, lr_ports); + cleanup_routes(sbrec_route_table, lr_datapaths, lr_ports); } tag_alloc_destroy(&tag_alloc_table); @@ -19021,6 +19045,7 @@ ovnnb_db_run(struct northd_input *input_data, input_data->sbrec_mirror_table, input_data->sbrec_mac_binding_table, input_data->sbrec_ha_chassis_group_table, + input_data->sbrec_route_table, input_data->sbrec_chassis_by_name, input_data->sbrec_chassis_by_hostname, input_data->sbrec_ha_chassis_grp_by_name, diff --git a/northd/northd.h b/northd/northd.h index 126d58626..7b318087a 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -51,6 +51,7 @@ struct northd_input { const struct sbrec_chassis_template_var_table *sbrec_chassis_template_var_table; const struct sbrec_mirror_table *sbrec_mirror_table; + const struct sbrec_route_table *sbrec_route_table; /* Northd lb data node inputs*/ const struct hmap *lbs;
learned routes must be bound to a lrp on which the routes where learned. In case the lrp is deleted for whatever reason no ovn-controller would clean these routes up, therefor we do this in northd. Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud> --- northd/en-northd.c | 2 ++ northd/inc-proc-northd.c | 1 + northd/northd.c | 31 ++++++++++++++++++++++++++++--- northd/northd.h | 1 + 4 files changed, 32 insertions(+), 3 deletions(-)