Yeap, I’ll add documentation on this option, thanks. I don’t see any scenario for user to modify this field. The only one that I could imagine is an override for already existed static route with a user-defined static made as "connected" one. I don’t think it has a real world use case, but maybe I’m wrong here.
Regards, Vladislav Odintsov > On 17 Nov 2021, at 05:13, Numan Siddique <[email protected]> wrote: > > On Sat, Nov 13, 2021 at 4:44 AM Vladislav Odintsov <[email protected] > <mailto:[email protected]>> wrote: >> >> This commits adds ability to save route's origin while IC learning. >> Directly connected routes are saved in IC SB DB with "connected" >> origin column value. >> Static routes have "static" value in origin column. >> >> This logic would be used in next patch to compute priority >> for lr_in_ip_routing stage lflows. >> >> Signed-off-by: Vladislav Odintsov <[email protected] >> <mailto:[email protected]>> > > Hi Vladislav, > > The patch LGTM. > > I think you missed documenting the option "origin" for the NB > Logical_Router_Static_Route > option. Was this intentional? From what I understand, ovn-ic will > set this option. > So for a setup where ovn-ic is not used, can a user/CMS configure this > option ? > > I think it would be good to document this in ovn-nb.xml. > > With the documentation added: > Acked-by: Numan Siddique <[email protected] <mailto:[email protected]>> > > Numan > >> --- >> ic/ovn-ic.c | 34 +++++++++++++++++++-------- >> lib/ovn-util.h | 3 +++ >> ovn-ic-sb.ovsschema | 7 ++++-- >> ovn-ic-sb.xml | 10 ++++++++ >> tests/ovn-ic.at | 57 +++++++++++++++++++++++++++++++++++++++++++++ >> 5 files changed, 99 insertions(+), 12 deletions(-) >> >> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c >> index 303e93a4f..70abae108 100644 >> --- a/ic/ovn-ic.c >> +++ b/ic/ovn-ic.c >> @@ -854,6 +854,7 @@ struct ic_route_info { >> struct in6_addr prefix; >> unsigned int plen; >> struct in6_addr nexthop; >> + const char *origin; >> >> /* Either nb_route or nb_lrp is set and the other one must be NULL. >> * - For a route that is learned from IC-SB, or a static route that is >> @@ -867,22 +868,25 @@ struct ic_route_info { >> >> static uint32_t >> ic_route_hash(const struct in6_addr *prefix, unsigned int plen, >> - const struct in6_addr *nexthop) >> + const struct in6_addr *nexthop, const char *origin) >> { >> uint32_t basis = hash_bytes(prefix, sizeof *prefix, (uint32_t)plen); >> + basis = hash_string(origin, basis); >> return hash_bytes(nexthop, sizeof *nexthop, basis); >> } >> >> static struct ic_route_info * >> ic_route_find(struct hmap *routes, const struct in6_addr *prefix, >> - unsigned int plen, const struct in6_addr *nexthop) >> + unsigned int plen, const struct in6_addr *nexthop, >> + const char *origin) >> { >> struct ic_route_info *r; >> - uint32_t hash = ic_route_hash(prefix, plen, nexthop); >> + uint32_t hash = ic_route_hash(prefix, plen, nexthop, origin); >> HMAP_FOR_EACH_WITH_HASH (r, node, hash, routes) { >> if (ipv6_addr_equals(&r->prefix, prefix) && >> r->plen == plen && >> - ipv6_addr_equals(&r->nexthop, nexthop)) { >> + ipv6_addr_equals(&r->nexthop, nexthop) && >> + !strcmp(r->origin, origin)) { >> return r; >> } >> } >> @@ -926,13 +930,15 @@ add_to_routes_learned(struct hmap *routes_learned, >> &prefix, &plen, &nexthop)) { >> return false; >> } >> + const char *origin = smap_get_def(&nb_route->options, "origin", ""); >> struct ic_route_info *ic_route = xzalloc(sizeof *ic_route); >> ic_route->prefix = prefix; >> ic_route->plen = plen; >> ic_route->nexthop = nexthop; >> ic_route->nb_route = nb_route; >> + ic_route->origin = origin; >> hmap_insert(routes_learned, &ic_route->node, >> - ic_route_hash(&prefix, plen, &nexthop)); >> + ic_route_hash(&prefix, plen, &nexthop, origin)); >> return true; >> } >> >> @@ -1093,8 +1099,9 @@ add_to_routes_ad(struct hmap *routes_ad, >> ic_route->plen = plen; >> ic_route->nexthop = nexthop; >> ic_route->nb_route = nb_route; >> + ic_route->origin = ROUTE_ORIGIN_STATIC; >> hmap_insert(routes_ad, &ic_route->node, >> - ic_route_hash(&prefix, plen, &nexthop)); >> + ic_route_hash(&prefix, plen, &nexthop, >> ROUTE_ORIGIN_STATIC)); >> } >> >> static void >> @@ -1143,8 +1150,10 @@ add_network_to_routes_ad(struct hmap *routes_ad, >> const char *network, >> ic_route->plen = plen; >> ic_route->nexthop = nexthop; >> ic_route->nb_lrp = nb_lrp; >> + ic_route->origin = ROUTE_ORIGIN_CONNECTED; >> hmap_insert(routes_ad, &ic_route->node, >> - ic_route_hash(&prefix, plen, &nexthop)); >> + ic_route_hash(&prefix, plen, &nexthop, >> + ROUTE_ORIGIN_CONNECTED)); >> } >> >> static bool >> @@ -1206,7 +1215,8 @@ sync_learned_route(struct ic_context *ctx, >> continue; >> } >> struct ic_route_info *route_learned >> - = ic_route_find(&ic_lr->routes_learned, &prefix, plen, >> &nexthop); >> + = ic_route_find(&ic_lr->routes_learned, &prefix, plen, &nexthop, >> + isb_route->origin); >> if (route_learned) { >> /* Sync external-ids */ >> struct uuid ext_id; >> @@ -1233,6 +1243,8 @@ sync_learned_route(struct ic_context *ctx, >> UUID_ARGS(&isb_route->header_.uuid)); >> nbrec_logical_router_static_route_update_external_ids_setkey( >> nb_route, "ic-learned-route", uuid_s); >> + nbrec_logical_router_static_route_update_options_setkey( >> + nb_route, "origin", isb_route->origin); >> free(uuid_s); >> nbrec_logical_router_update_static_routes_addvalue( >> ic_lr->lr, nb_route); >> @@ -1297,8 +1309,9 @@ advertise_route(struct ic_context *ctx, >> icsbrec_route_delete(isb_route); >> continue; >> } >> - struct ic_route_info *route_adv = >> - ic_route_find(routes_ad, &prefix, plen, &nexthop); >> + struct ic_route_info *route_adv = ic_route_find(routes_ad, &prefix, >> + plen, &nexthop, >> + isb_route->origin); >> if (!route_adv) { >> /* Delete the extra route from IC-SB. */ >> VLOG_DBG("Delete route %s -> %s from IC-SB, which is not found" >> @@ -1338,6 +1351,7 @@ advertise_route(struct ic_context *ctx, >> } >> icsbrec_route_set_ip_prefix(isb_route, prefix_s); >> icsbrec_route_set_nexthop(isb_route, nexthop_s); >> + icsbrec_route_set_origin(isb_route, route_adv->origin); >> free(prefix_s); >> free(nexthop_s); >> >> diff --git a/lib/ovn-util.h b/lib/ovn-util.h >> index 2fa92e069..a923c3b65 100644 >> --- a/lib/ovn-util.h >> +++ b/lib/ovn-util.h >> @@ -25,6 +25,9 @@ >> #define ovn_print_version(MIN_OFP, MAX_OFP) \ >> ovs_print_version(MIN_OFP, MAX_OFP) >> >> +#define ROUTE_ORIGIN_CONNECTED "connected" >> +#define ROUTE_ORIGIN_STATIC "static" >> + >> struct nbrec_logical_router_port; >> struct sbrec_logical_flow; >> struct svec; >> diff --git a/ovn-ic-sb.ovsschema b/ovn-ic-sb.ovsschema >> index 5364b21b4..42ce85d7d 100644 >> --- a/ovn-ic-sb.ovsschema >> +++ b/ovn-ic-sb.ovsschema >> @@ -1,7 +1,7 @@ >> { >> "name": "OVN_IC_Southbound", >> - "version": "1.0.0", >> - "cksum": "108951192 6585", >> + "version": "1.1.0", >> + "cksum": "423535838 6733", >> "tables": { >> "IC_SB_Global": { >> "columns": { >> @@ -94,6 +94,9 @@ >> "refTable": "Availability_Zone"}}}, >> "ip_prefix": {"type": "string"}, >> "nexthop": {"type": "string"}, >> + "origin": {"type": {"key": { >> + "type": "string", >> + "enum": ["set", ["connected", "static"]]}}}, >> "external_ids": { >> "type": {"key": "string", "value": "string", >> "min": 0, "max": "unlimited"}}}, >> diff --git a/ovn-ic-sb.xml b/ovn-ic-sb.xml >> index 3582cff47..d8338e4d3 100644 >> --- a/ovn-ic-sb.xml >> +++ b/ovn-ic-sb.xml >> @@ -313,6 +313,16 @@ >> <column name="nexthop"> >> Nexthop IP address for this route. >> </column> >> + >> + <column name="origin"> >> + Can be one of <code>connected</code> or <code>static</code>. >> Routes to >> + directly-connected subnets - LRP's CIDRs are inserted to OVN IC SB >> DB >> + with <code>connected</code> value in <ref column="origin"/>. Static >> + routes are inserted to OVN IC SB DB with <code>static</code> value. >> + Next when route is learned to another AZ NB DB by ovn-ic, route >> origin >> + is synced to <ref table="Logical_Router_Static_Route" >> column="options" >> + key="origin"/>. >> + </column> >> </group> >> >> <group title="Common Columns"> >> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at >> index 9086974a3..7e8498b2f 100644 >> --- a/tests/ovn-ic.at >> +++ b/tests/ovn-ic.at >> @@ -423,6 +423,63 @@ AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr11 | >> # Test routes from lr12 didn't leak as learned to lr21 >> AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr21], [0], []) >> >> +# cleanup >> +ovn-ic-nbctl --if-exists ts-del ts1 >> +ovn_as az1 ovn-nbctl lr-del lr11 >> +ovn_as az1 ovn-nbctl lr-del lr21 >> +ovn_as az2 ovn-nbctl lr-del lr12 >> +ovn_as az2 ovn-nbctl lr-del lr22 >> + >> +# check routes origin advertisement and learning >> + >> +# setup topology with connected, static and source routes >> +ovn-ic-nbctl ts-add ts1 >> +for i in 1 2; do >> + ovn_as az$i >> + >> + # Enable route learning at AZ level >> + ovn-nbctl set nb_global . options:ic-route-learn=true >> + # Enable route advertising at AZ level >> + ovn-nbctl set nb_global . options:ic-route-adv=true >> + >> + # Create LRP and connect to TS >> + ovn-nbctl lr-add lr$i >> + ovn-nbctl lrp-add lr$i lrp-lr$i-ts1 aa:aa:aa:aa:aa:0$i 169.254.100.$i/24 >> + ovn-nbctl lsp-add ts1 lsp-ts1-lr$i \ >> + -- lsp-set-addresses lsp-ts1-lr$i router \ >> + -- lsp-set-type lsp-ts1-lr$i router \ >> + -- lsp-set-options lsp-ts1-lr$i router-port=lrp-lr$i-ts1 >> + >> + ovn-nbctl lrp-add lr$i lrp-lr$i-p$i 00:00:00:00:00:0$i 192.168.$i.1/24 >> + >> + # Create static routes >> + ovn-nbctl lr-route-add lr$i 10.11.$i.0/24 169.254.0.1 >> + >> + # Create a src-ip route, which shouldn't be synced >> + ovn-nbctl --policy=src-ip lr-route-add lr$i 10.22.$i.0/24 169.254.0.2 >> +done >> + >> +for i in 1 2; do >> + OVS_WAIT_UNTIL([ovn_as az$i ovn-nbctl lr-route-list lr$i | grep >> learned]) >> +done >> + >> +# check that advertised routes in ic-sb have correct origin >> +ovn-ic-sbctl list route >> +wait_row_count ic-sb:Route 1 ip_prefix=10.11.1.0/24 origin=static >> +wait_row_count ic-sb:Route 1 ip_prefix=192.168.1.1/24 origin=connected >> +wait_row_count ic-sb:Route 1 ip_prefix=10.11.2.0/24 origin=static >> +wait_row_count ic-sb:Route 1 ip_prefix=192.168.2.1/24 origin=connected >> + >> +# check that learned routes in ic-sb have correct origin >> + >> +ovn_as az1 >> +wait_row_count nb:Logical_Router_Static_Route 1 ip_prefix=10.11.2.0/24 >> options:origin=static >> +wait_row_count nb:Logical_Router_Static_Route 1 ip_prefix=192.168.2.1/24 >> options:origin=connected >> + >> +ovn_as az2 >> +wait_row_count nb:Logical_Router_Static_Route 1 ip_prefix=10.11.1.0/24 >> options:origin=static >> +wait_row_count nb:Logical_Router_Static_Route 1 ip_prefix=192.168.1.1/24 >> options:origin=connected >> + >> OVN_CLEANUP_IC([az1], [az2]) >> >> AT_CLEANUP >> -- >> 2.30.0 >> >> _______________________________________________ >> dev mailing list >> [email protected] <mailto:[email protected]> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> >> > _______________________________________________ > dev mailing list > [email protected] <mailto:[email protected]> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
