[ovs-dev] [PATCH ovn v2] ic: learn routes to LR only from corresponding transit switch
Vladislav Odintsov
odivlad at gmail.com
Thu Aug 19 17:11:16 UTC 2021
Thanks!
Regards,
Vladislav Odintsov
> On 19 Aug 2021, at 20:06, Han Zhou <zhouhan at gmail.com> wrote:
>
> On Thu, Aug 19, 2021 at 8:30 AM Vladislav Odintsov <odivlad at gmail.com>
> wrote:
>>
>> This commit fixes an error where in case of LRs were connected
>> between different AZs with ovn-ic, their routes were leaking
>> from one LR attached to its transit switch to another LR
>> attached to another transit switch.
>>
>> Testcase, which reproduces described problem is added as well.
>>
>> Signed-off-by: Vladislav Odintsov <odivlad at gmail.com>
>> ---
>> v1 -> v2:
>> - Address Han's review comments:
>> - Make comments and test more clear to understand.
>> - Use ovsdb_idl_index to optimize transit switch lookup.
>> ---
>> ic/ovn-ic.c | 17 ++++++++++++++-
>> tests/ovn-ic.at | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 71 insertions(+), 1 deletion(-)
>>
>> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
>> index d69583956..75e798cd1 100644
>> --- a/ic/ovn-ic.c
>> +++ b/ic/ovn-ic.c
>> @@ -67,6 +67,7 @@ struct ic_context {
>> struct ovsdb_idl_index *sbrec_chassis_by_name;
>> struct ovsdb_idl_index *sbrec_port_binding_by_name;
>> struct ovsdb_idl_index *icsbrec_port_binding_by_ts;
>> + struct ovsdb_idl_index *icsbrec_route_by_ts;
>> };
>>
>> struct ic_state {
>> @@ -1156,7 +1157,14 @@ sync_learned_route(struct ic_context *ctx,
>> {
>> ovs_assert(ctx->ovnnb_txn);
>> const struct icsbrec_route *isb_route;
>> - ICSBREC_ROUTE_FOR_EACH (isb_route, ctx->ovnisb_idl) {
>> + const struct icsbrec_route *isb_route_key =
>> + icsbrec_route_index_init_row(ctx->icsbrec_route_by_ts);
>> +
>> + icsbrec_route_index_set_transit_switch(isb_route_key,
>> +
> ic_lr->isb_pb->transit_switch);
>> +
>> + ICSBREC_ROUTE_FOR_EACH_EQUAL (isb_route, isb_route_key,
>> + ctx->icsbrec_route_by_ts) {
>> if (isb_route->availability_zone == az) {
>> continue;
>> }
>> @@ -1208,6 +1216,8 @@ sync_learned_route(struct ic_context *ctx,
>> ic_lr->lr, nb_route);
>> }
>> }
>> + icsbrec_route_index_destroy_row(isb_route_key);
>> +
>> /* Delete extra learned routes. */
>> struct ic_route_info *route_learned, *next;
>> HMAP_FOR_EACH_SAFE (route_learned, next, node,
> &ic_lr->routes_learned) {
>> @@ -1678,6 +1688,10 @@ main(int argc, char *argv[])
>> = ovsdb_idl_index_create1(ovnisb_idl_loop.idl,
>>
> &icsbrec_port_binding_col_transit_switch);
>>
>> + struct ovsdb_idl_index *icsbrec_route_by_ts
>> + = ovsdb_idl_index_create1(ovnisb_idl_loop.idl,
>> + &icsbrec_route_col_transit_switch);
>> +
>> /* Main loop. */
>> exiting = false;
>> state.had_lock = false;
>> @@ -1718,6 +1732,7 @@ main(int argc, char *argv[])
>> .sbrec_port_binding_by_name = sbrec_port_binding_by_name,
>> .sbrec_chassis_by_name = sbrec_chassis_by_name,
>> .icsbrec_port_binding_by_ts = icsbrec_port_binding_by_ts,
>> + .icsbrec_route_by_ts = icsbrec_route_by_ts,
>> };
>>
>> if (!state.had_lock &&
> ovsdb_idl_has_lock(ovnsb_idl_loop.idl)) {
>> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
>> index 2a4fba031..ee78f4794 100644
>> --- a/tests/ovn-ic.at
>> +++ b/tests/ovn-ic.at
>> @@ -315,6 +315,61 @@ OVS_WAIT_WHILE([ovn_as az2 ovn-nbctl lr-route-list
> lr2 | grep learned | grep 192
>> # AZ1 shouldn't learn 10.11 any more.
>> OVS_WAIT_WHILE([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep learned |
> grep 10.11])
>>
>> +# cleanup
>> +ovn-ic-nbctl --if-exists ts-del ts1
>> +ovn_as az1 ovn-nbctl lr-del lr1
>> +ovn_as az2 ovn-nbctl lr-del lr2
>> +
>> +# Create new transit switches and LRs. Test topology is next:
>> +# logical router (lr11) - transit switch (ts11) - logical router (lr12)
>> +# logical router (lr21) - transit switch (ts22) - logical router (lr22)
>> +#
>> +# lr12 has static route 10.0.0.0/24 and directly connected network
> 192.168.0.0/24
>> +for i in 1 2; do
>> + ovn_as az$i
>> +
>> + # Ensure route learning at AZ level
>> + ovn-nbctl set nb_global . options:ic-route-learn=true
>> + # Ensure route advertising at AZ level
>> + ovn-nbctl set nb_global . options:ic-route-adv=true
>> + # Drop blacklist
>> + ovn-nbctl remove nb_global . options ic-route-blacklist
>> +
>> + for j in 1 2; do
>> + ts=ts$j$j
>> + ovn-ic-nbctl --may-exist ts-add $ts
>> +
>> + # Create LRP and connect to TS
>> + lr=lr$j$i
>> + echo lr: $lr, ts: $ts
>> + ovn-nbctl lr-add $lr
>> + ovn-nbctl lrp-add $lr lrp-$lr-$ts aa:aa:aa:aa:aa:0$j
> 169.254.100.$i/24
>> + ovn-nbctl lsp-add $ts lsp-$ts-$lr \
>> + -- lsp-set-addresses lsp-$ts-$lr router \
>> + -- lsp-set-type lsp-$ts-$lr router \
>> + -- lsp-set-options lsp-$ts-$lr router-port=lrp-$lr-$ts
>> + done
>> +done
>> +
>> +# Create directly-connected routes
>> +ovn_as az2 ovn-nbctl lrp-add lr12 lrp-lr12-ls2 aa:aa:aa:aa:bb:01 "
> 192.168.0.1/24"
>> +ovn_as az2 ovn-nbctl lr-route-add lr12 10.10.10.0/24 192.168.0.10
>> +
>> +echo az1
>> +ovn_as az1 ovn-nbctl show
>> +echo az2
>> +ovn_as az2 ovn-nbctl show
>> +
>> +# Test routes from lr12 were learned to lr11
>> +AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr11 |
>> + grep learned | awk '{print $1, $2}' | sort], [0], [dnl
>> +10.10.10.0/24 169.254.100.2
>> +192.168.0.0/24 169.254.100.2
>> +])
>> +
>> +# Test routes from lr12 didn't leak as learned to lr21
>> +AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr21], [0], [])
>> +
>> OVN_CLEANUP_IC([az1], [az2])
>>
>> AT_CLEANUP
>> --
>> 2.30.0
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
> Thanks Vladislav. I applied to all branches.
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list