Message ID | 20211113094353.17690-2-odivlad@gmail.com |
---|---|
State | Superseded, archived |
Headers | show |
Series | Add multiple routing tables support to Logical Routers | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On Sat, Nov 13, 2021 at 4:44 AM Vladislav Odintsov <odivlad@gmail.com> 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 <odivlad@gmail.com> 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 <numans@ovn.org> 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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
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 <numans@ovn.org> wrote: > > On Sat, Nov 13, 2021 at 4:44 AM Vladislav Odintsov <odivlad@gmail.com <mailto:odivlad@gmail.com>> 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 <odivlad@gmail.com <mailto:odivlad@gmail.com>> > > 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 <numans@ovn.org <mailto:numans@ovn.org>> > > 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 >> dev@openvswitch.org <mailto:dev@openvswitch.org> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> >> > _______________________________________________ > dev mailing list > dev@openvswitch.org <mailto:dev@openvswitch.org> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
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
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 <odivlad@gmail.com> --- 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(-)