Hi Lorenzo, On 8/22/24 23:59, Lorenzo Bianconi wrote: > This reverts commit aeae21335a8bccbb9fe7746fcf4ed2d2a0e1c7a4. > Bump ovn-sb.ovsschema version to 20.37.0. >
This needs a minor rebase. Same comment as for the first two patches in the series: please add some information about why this is reverted. I know you added it into the cover letter but that doesn't get committed. Please add a reference to our internal issue too: https://issues.redhat.com/browse/FDP-750 Otherwise, this looks good to me. Regards, Dumitru > Signed-off-by: Lorenzo Bianconi <[email protected]> > --- > lib/ovn-util.h | 2 -- > northd/en-northd.c | 35 -------------------- > northd/en-northd.h | 4 --- > northd/inc-proc-northd.c | 7 +--- > northd/northd.c | 70 ---------------------------------------- > northd/northd.h | 10 ------ > ovn-sb.ovsschema | 18 ++--------- > ovn-sb.xml | 31 ------------------ > tests/ovn-northd.at | 4 --- > 9 files changed, 3 insertions(+), 178 deletions(-) > > diff --git a/lib/ovn-util.h b/lib/ovn-util.h > index 622fec531..7b98b9b9a 100644 > --- a/lib/ovn-util.h > +++ b/lib/ovn-util.h > @@ -38,8 +38,6 @@ > #define STT_TUNNEL_OVERHEAD 18 > #define VXLAN_TUNNEL_OVERHEAD 30 > > -#define ECMP_NEXTHOP_IDS_LEN 65535 > - > struct eth_addr; > struct nbrec_logical_router_port; > struct ovsrec_flow_sample_collector_set_table; > diff --git a/northd/en-northd.c b/northd/en-northd.c > index 63f93bbf4..34f0a7df7 100644 > --- a/northd/en-northd.c > +++ b/northd/en-northd.c > @@ -404,25 +404,6 @@ en_bfd_sync_run(struct engine_node *node, void *data) > engine_set_node_state(node, EN_UPDATED); > } > > -void > -en_ecmp_nexthop_run(struct engine_node *node, void *data) > -{ > - const struct engine_context *eng_ctx = engine_get_context(); > - struct static_routes_data *static_routes_data = > - engine_get_input_data("static_routes", node); > - struct ecmp_nexthop_data *enh_data = data; > - const struct sbrec_ecmp_nexthop_table *sbrec_ecmp_nexthop_table = > - EN_OVSDB_GET(engine_get_input("SB_ecmp_nexthop", node)); > - > - ecmp_nexthop_destroy(data); > - ecmp_nexthop_init(data); > - build_ecmp_nexthop_table(eng_ctx->ovnsb_idl_txn, > - &static_routes_data->parsed_routes, > - &enh_data->nexthops, > - sbrec_ecmp_nexthop_table); > - engine_set_node_state(node, EN_UPDATED); > -} > - > void > *en_northd_init(struct engine_node *node OVS_UNUSED, > struct engine_arg *arg OVS_UNUSED) > @@ -473,16 +454,6 @@ void > return data; > } > > -void > -*en_ecmp_nexthop_init(struct engine_node *node OVS_UNUSED, > - struct engine_arg *arg OVS_UNUSED) > -{ > - struct ecmp_nexthop_data *data = xzalloc(sizeof *data); > - > - ecmp_nexthop_init(data); > - return data; > -} > - > void > en_northd_cleanup(void *data) > { > @@ -555,9 +526,3 @@ en_bfd_sync_cleanup(void *data) > { > bfd_destroy(data); > } > - > -void > -en_ecmp_nexthop_cleanup(void *data) > -{ > - ecmp_nexthop_destroy(data); > -} > diff --git a/northd/en-northd.h b/northd/en-northd.h > index 2666cc67e..631a7c17a 100644 > --- a/northd/en-northd.h > +++ b/northd/en-northd.h > @@ -42,9 +42,5 @@ bool bfd_sync_northd_change_handler(struct engine_node > *node, > void *data OVS_UNUSED); > void en_bfd_sync_run(struct engine_node *node, void *data); > void en_bfd_sync_cleanup(void *data OVS_UNUSED); > -void en_ecmp_nexthop_run(struct engine_node *node, void *data); > -void *en_ecmp_nexthop_init(struct engine_node *node OVS_UNUSED, > - struct engine_arg *arg OVS_UNUSED); > -void en_ecmp_nexthop_cleanup(void *data); > > #endif /* EN_NORTHD_H */ > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > index 06c7ee2b8..1f79916a5 100644 > --- a/northd/inc-proc-northd.c > +++ b/northd/inc-proc-northd.c > @@ -103,8 +103,7 @@ static unixctl_cb_func chassis_features_list; > SB_NODE(fdb, "fdb") \ > SB_NODE(static_mac_binding, "static_mac_binding") \ > SB_NODE(chassis_template_var, "chassis_template_var") \ > - SB_NODE(logical_dp_group, "logical_dp_group") \ > - SB_NODE(ecmp_nexthop, "ecmp_nexthop") > + SB_NODE(logical_dp_group, "logical_dp_group") > > enum sb_engine_node { > #define SB_NODE(NAME, NAME_STR) SB_##NAME, > @@ -163,7 +162,6 @@ static ENGINE_NODE(route_policies, "route_policies"); > static ENGINE_NODE(static_routes, "static_routes"); > static ENGINE_NODE(bfd, "bfd"); > static ENGINE_NODE(bfd_sync, "bfd_sync"); > -static ENGINE_NODE(ecmp_nexthop, "ecmp_nexthop"); > > void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > struct ovsdb_idl_loop *sb) > @@ -266,9 +264,6 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > engine_add_input(&en_bfd_sync, &en_route_policies, NULL); > engine_add_input(&en_bfd_sync, &en_northd, > bfd_sync_northd_change_handler); > > - engine_add_input(&en_ecmp_nexthop, &en_sb_ecmp_nexthop, NULL); > - engine_add_input(&en_ecmp_nexthop, &en_static_routes, NULL); > - > engine_add_input(&en_sync_meters, &en_nb_acl, NULL); > engine_add_input(&en_sync_meters, &en_nb_meter, NULL); > engine_add_input(&en_sync_meters, &en_sb_meter, NULL); > diff --git a/northd/northd.c b/northd/northd.c > index 51ee5db1d..fb18fec90 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -10670,64 +10670,6 @@ build_bfd_map(const struct nbrec_bfd_table > *nbrec_bfd_table, > } > } > > -void > -build_ecmp_nexthop_table( > - struct ovsdb_idl_txn *ovnsb_txn, > - struct hmap *routes, > - struct simap *nexthops, > - const struct sbrec_ecmp_nexthop_table *sbrec_ecmp_nexthop_table) > -{ > - if (!ovnsb_txn) { > - return; > - } > - > - unsigned long *nexthop_ids = bitmap_allocate(ECMP_NEXTHOP_IDS_LEN); > - const struct sbrec_ecmp_nexthop *sb_ecmp_nexthop; > - SBREC_ECMP_NEXTHOP_TABLE_FOR_EACH (sb_ecmp_nexthop, > - sbrec_ecmp_nexthop_table) { > - simap_put(nexthops, sb_ecmp_nexthop->nexthop, > - sb_ecmp_nexthop->id); > - bitmap_set1(nexthop_ids, sb_ecmp_nexthop->id); > - } > - > - struct sset nb_nexthops_sset = SSET_INITIALIZER(&nb_nexthops_sset); > - > - struct parsed_route *pr; > - HMAP_FOR_EACH (pr, key_node, routes) { > - if (!pr->ecmp_symmetric_reply) { > - continue; > - } > - > - const struct nbrec_logical_router_static_route *r = pr->route; > - if (!simap_contains(nexthops, r->nexthop)) { > - int id = bitmap_scan(nexthop_ids, 0, 1, ECMP_NEXTHOP_IDS_LEN); > - if (id == ECMP_NEXTHOP_IDS_LEN) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, > 1); > - VLOG_WARN_RL(&rl, "nexthop id address space is exhausted"); > - continue; > - } > - bitmap_set1(nexthop_ids, id); > - simap_put(nexthops, r->nexthop, id); > - > - sb_ecmp_nexthop = sbrec_ecmp_nexthop_insert(ovnsb_txn); > - sbrec_ecmp_nexthop_set_nexthop(sb_ecmp_nexthop, r->nexthop); > - sbrec_ecmp_nexthop_set_id(sb_ecmp_nexthop, id); > - } > - sset_add(&nb_nexthops_sset, r->nexthop); > - } > - > - SBREC_ECMP_NEXTHOP_TABLE_FOR_EACH_SAFE (sb_ecmp_nexthop, > - sbrec_ecmp_nexthop_table) { > - if (!sset_contains(&nb_nexthops_sset, sb_ecmp_nexthop->nexthop)) { > - simap_find_and_delete(nexthops, sb_ecmp_nexthop->nexthop); > - sbrec_ecmp_nexthop_delete(sb_ecmp_nexthop); > - } > - } > - > - sset_destroy(&nb_nexthops_sset); > - bitmap_free(nexthop_ids); > -} > - > /* Returns a string of the IP address of the router port 'op' that > * overlaps with 'ip_s". If one is not found, returns NULL. > * > @@ -18781,12 +18723,6 @@ bfd_init(struct bfd_data *data) > hmap_init(&data->bfd_connections); > } > > -void > -ecmp_nexthop_init(struct ecmp_nexthop_data *data) > -{ > - simap_init(&data->nexthops); > -} > - > void > northd_destroy(struct northd_data *data) > { > @@ -18868,12 +18804,6 @@ static_routes_destroy(struct static_routes_data > *data) > __bfd_destroy(&data->bfd_active_connections); > } > > -void > -ecmp_nexthop_destroy(struct ecmp_nexthop_data *data) > -{ > - simap_destroy(&data->nexthops); > -} > - > void > ovnnb_db_run(struct northd_input *input_data, > struct northd_data *data, > diff --git a/northd/northd.h b/northd/northd.h > index afd5f7113..9e326b746 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -195,10 +195,6 @@ struct bfd_data { > struct hmap bfd_connections; > }; > > -struct ecmp_nexthop_data { > - struct simap nexthops; > -}; > - > struct lr_nat_table; > > struct lflow_input { > @@ -742,12 +738,6 @@ void static_routes_destroy(struct static_routes_data *); > void bfd_init(struct bfd_data *); > void bfd_destroy(struct bfd_data *); > > -void build_ecmp_nexthop_table(struct ovsdb_idl_txn *, > - struct hmap *, struct simap *, > - const struct sbrec_ecmp_nexthop_table *); > -void ecmp_nexthop_init(struct ecmp_nexthop_data *); > -void ecmp_nexthop_destroy(struct ecmp_nexthop_data *); > - > struct lflow_table; > struct lr_stateful_tracked_data; > struct ls_stateful_tracked_data; > diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema > index 9d8e0ac46..73abf2c8d 100644 > --- a/ovn-sb.ovsschema > +++ b/ovn-sb.ovsschema > @@ -1,7 +1,7 @@ > { > "name": "OVN_Southbound", > - "version": "20.36.0", > - "cksum": "1845967275 32154", > + "version": "20.37.0", > + "cksum": "1950136776 31493", > "tables": { > "SB_Global": { > "columns": { > @@ -610,20 +610,6 @@ > "refTable": "Datapath_Binding"}}}}, > "indexes": [["logical_port", "ip"]], > "isRoot": true}, > - "ECMP_Nexthop": { > - "columns": { > - "nexthop": {"type": "string"}, > - "id": {"type": {"key": {"type": "integer", > - "minInteger": 0, > - "maxInteger": 65535}}}, > - "external_ids": { > - "type": {"key": "string", "value": "string", > - "min": 0, "max": "unlimited"}}, > - "options": { > - "type": {"key": "string", "value": "string", > - "min": 0, "max": "unlimited"}}}, > - "indexes": [["nexthop"]], > - "isRoot": true}, > "Chassis_Template_Var": { > "columns": { > "chassis": {"type": "string"}, > diff --git a/ovn-sb.xml b/ovn-sb.xml > index c11296d7c..746cc6308 100644 > --- a/ovn-sb.xml > +++ b/ovn-sb.xml > @@ -5178,35 +5178,4 @@ tcp.flags = RST; > The set of variable values for a given chassis. > </column> > </table> > - > - <table name="ECMP_Nexthop"> > - <p> > - Each record in this table represents an active ECMP route committed by > - <code>ovn-northd</code> to <code>ovs</code> connection-tracking table. > - <code>ECMP_Nexthop</code> table is used by <code>ovn-controller</code> > - to track active ct entries and to flush stale ones. > - </p> > - <column name="nexthop"> > - <p> > - Nexthop IP address for this ECMP route. Nexthop IP address should > - be the IP address of a connected router port or the IP address of > - an external device used as nexthop for the given destination. > - </p> > - </column> > - > - <column name="id"> > - <p> > - Nexthop unique identifier. Nexthop ID is used to track active > - ecmp-symmetric-reply connections and flush stale ones. > - </p> > - </column> > - > - <column name="options"> > - Reserved for future use. > - </column> > - > - <column name="external_ids"> > - See <em>External IDs</em> at the beginning of this document. > - </column> > - </table> > </database> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 93ccbce6b..fddf222b3 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -6800,7 +6800,6 @@ check ovn-nbctl lsp-set-addresses public-lr0 router > check ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public > > check ovn-nbctl --wait=sb --ecmp-symmetric-reply lr-route-add lr0 1.0.0.1 > 192.168.0.10 > -check_row_count ECMP_Nexthop 1 > > ovn-sbctl dump-flows lr0 > lr0flows > > @@ -6818,7 +6817,6 @@ AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | > ovn_strip_lflows], [0], [dn > ]) > > check ovn-nbctl --wait=sb --ecmp-symmetric-reply lr-route-add lr0 1.0.0.1 > 192.168.0.20 > -check_row_count ECMP_Nexthop 2 > > ovn-sbctl dump-flows lr0 > lr0flows > AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | ovn_strip_lflows], [0], [dnl > @@ -6848,7 +6846,6 @@ AT_CHECK([grep -e "lr_in_arp_resolve.*ecmp" lr0flows | > ovn_strip_lflows], [0], [ > > # add ecmp route with wrong nexthop > check ovn-nbctl --wait=sb --ecmp-symmetric-reply lr-route-add lr0 1.0.0.1 > 192.168.1.20 > -check_row_count ECMP_Nexthop 2 > > ovn-sbctl dump-flows lr0 > lr0flows > AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | ovn_strip_lflows], [0], [dnl > @@ -6868,7 +6865,6 @@ AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | > sed 's/192\.168\.0\..0/192. > > check ovn-nbctl lr-route-del lr0 > wait_row_count nb:Logical_Router_Static_Route 0 > -check_row_count ECMP_Nexthop 0 > > check ovn-nbctl --wait=sb lr-route-add lr0 1.0.0.0/24 192.168.0.10 > ovn-sbctl dump-flows lr0 > lr0flows _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
