| Message ID | 20210903103441.33178-2-odivlad@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Add multiple routing tables support to Logical Routers | expand |
On Fri, Sep 3, 2021 at 6:35 AM Vladislav Odintsov <odivlad@gmail.com> wrote: > > When IC port_binding exists and transit switch is deleted, > the orphan port_binding if left in the IC_SB_DB. > > This patch fixes such situation and adds test for this case. > > Signed-off-by: Vladislav Odintsov <odivlad@gmail.com> This patch is already merged, right ? https://github.com/ovn-org/ovn/commit/94fc72fce2ce4eed8f3efad5a3303e0b6232f1dc Or I'm wrong here ? Thanks Numan > --- > ic/ovn-ic.c | 35 +++++++++++++++++++++++++++++++-- > tests/ovn-ic.at | 52 +++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 85 insertions(+), 2 deletions(-) > > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > index 9eb792ea5..418f19147 100644 > --- a/ic/ovn-ic.c > +++ b/ic/ovn-ic.c > @@ -66,6 +66,7 @@ struct ic_context { > struct ovsdb_idl_index *nbrec_port_by_name; > 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_az; > struct ovsdb_idl_index *icsbrec_port_binding_by_ts; > struct ovsdb_idl_index *icsbrec_route_by_ts; > struct ovsdb_idl_index *icsbrec_route_by_ts_az; > @@ -175,7 +176,7 @@ allocate_ts_dp_key(struct hmap *dp_tnlids) > } > > static void > -ts_run(struct ic_context *ctx) > +ts_run(struct ic_context *ctx, const struct icsbrec_availability_zone *az) > { > const struct icnbrec_transit_switch *ts; > > @@ -240,8 +241,25 @@ ts_run(struct ic_context *ctx) > * SB, to avoid uncommitted ISB datapath tunnel key to be synced back to > * AZ. */ > if (ctx->ovnisb_txn) { > + struct shash isb_pbs = SHASH_INITIALIZER(&isb_pbs); > + const struct icsbrec_port_binding *isb_pb; > + const struct icsbrec_port_binding *isb_pb_key = > + icsbrec_port_binding_index_init_row( > + ctx->icsbrec_port_binding_by_az); > + > + icsbrec_port_binding_index_set_availability_zone(isb_pb_key, az); > + > + ICSBREC_PORT_BINDING_FOR_EACH (isb_pb, ctx->ovnisb_idl) { > + shash_add(&isb_pbs, isb_pb->transit_switch, isb_pb); > + } > + > /* Create ISB Datapath_Binding */ > ICNBREC_TRANSIT_SWITCH_FOR_EACH (ts, ctx->ovninb_idl) { > + while (shash_find_and_delete(&isb_pbs, ts->name)) { > + /* There may be multiple Port_Bindings */ > + continue; > + } > + > isb_dp = shash_find_and_delete(&isb_dps, ts->name); > if (!isb_dp) { > /* Allocate tunnel key */ > @@ -261,6 +279,13 @@ ts_run(struct ic_context *ctx) > SHASH_FOR_EACH (node, &isb_dps) { > icsbrec_datapath_binding_delete(node->data); > } > + > + SHASH_FOR_EACH (node, &isb_pbs) { > + icsbrec_port_binding_delete(node->data); > + } > + > + icsbrec_port_binding_index_destroy_row(isb_pb_key); > + shash_destroy(&isb_pbs); > } > ovn_destroy_tnlids(&dp_tnlids); > shash_destroy(&isb_dps); > @@ -1493,7 +1518,7 @@ ovn_db_run(struct ic_context *ctx) > return; > } > > - ts_run(ctx); > + ts_run(ctx, az); > gateway_run(ctx, az); > port_binding_run(ctx, az); > route_run(ctx, az); > @@ -1684,6 +1709,11 @@ main(int argc, char *argv[]) > struct ovsdb_idl_index *sbrec_chassis_by_name > = ovsdb_idl_index_create1(ovnsb_idl_loop.idl, > &sbrec_chassis_col_name); > + > + struct ovsdb_idl_index *icsbrec_port_binding_by_az > + = ovsdb_idl_index_create1(ovnisb_idl_loop.idl, > + &icsbrec_port_binding_col_availability_zone); > + > struct ovsdb_idl_index *icsbrec_port_binding_by_ts > = ovsdb_idl_index_create1(ovnisb_idl_loop.idl, > &icsbrec_port_binding_col_transit_switch); > @@ -1736,6 +1766,7 @@ main(int argc, char *argv[]) > .nbrec_port_by_name = nbrec_port_by_name, > .sbrec_port_binding_by_name = sbrec_port_binding_by_name, > .sbrec_chassis_by_name = sbrec_chassis_by_name, > + .icsbrec_port_binding_by_az = icsbrec_port_binding_by_az, > .icsbrec_port_binding_by_ts = icsbrec_port_binding_by_ts, > .icsbrec_route_by_ts = icsbrec_route_by_ts, > .icsbrec_route_by_ts_az = icsbrec_route_by_ts_az, > diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at > index ee78f4794..b6a8edb68 100644 > --- a/tests/ovn-ic.at > +++ b/tests/ovn-ic.at > @@ -64,6 +64,58 @@ OVN_CLEANUP_IC([az1]) > AT_CLEANUP > ]) > > + > +AT_SETUP([ovn-ic -- port bindings]) > + > +ovn_init_ic_db > +net_add n1 > + > +# 1 GW per AZ > +for i in 1 2; do > + az=az$i > + ovn_start $az > + sim_add gw-$az > + as gw-$az > + check ovs-vsctl add-br br-phys > + ovn_az_attach $az n1 br-phys 192.168.1.$i > + check ovs-vsctl set open . external-ids:ovn-is-interconn=true > +done > + > +ovn_as az1 > + > +# create transit switch and connect to LR > +check ovn-ic-nbctl ts-add ts1 > +check ovn-nbctl lr-add lr1 > +check ovn-nbctl lrp-add lr1 lrp1 00:00:00:00:00:01 10.0.0.1/24 > +check ovn-nbctl lrp-set-gateway-chassis lrp1 gw-az1 > + > +check ovn-nbctl lsp-add ts1 lsp1 -- \ > + lsp-set-addresses lsp1 router -- \ > + lsp-set-type lsp1 router -- \ > + lsp-set-options lsp1 router-port=lrp1 > + > +OVS_WAIT_UNTIL([ovn-sbctl list datapath_binding | grep interconn-ts | grep ts1]) > + > +# check port binding appeared > +AT_CHECK([ovn-ic-sbctl show | grep -A2 lsp1], [0], [dnl > + port lsp1 > + transit switch: ts1 > + address: [["00:00:00:00:00:01 10.0.0.1/24"]] > +]) > + > +# remove transit switch and check if port_binding is deleted > +check ovn-ic-nbctl ts-del ts1 > +OVS_WAIT_UNTIL([test -z "$(ovn-ic-sbctl show | grep lsp1)"]) > + > +for i in 1 2; do > + az=az$i > + OVN_CLEANUP_SBOX(gw-$az) > + OVN_CLEANUP_AZ([$az]) > +done > +OVN_CLEANUP_IC > +AT_CLEANUP > + > + > OVN_FOR_EACH_NORTHD([ > AT_SETUP([ovn-ic -- gateway sync]) > > -- > 2.30.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Hi Numan, yes, you’re correct. Regards, Vladislav Odintsov > On 14 Sep 2021, at 21:41, Numan Siddique <numans@ovn.org> wrote: > > On Fri, Sep 3, 2021 at 6:35 AM Vladislav Odintsov <odivlad@gmail.com <mailto:odivlad@gmail.com>> wrote: >> >> When IC port_binding exists and transit switch is deleted, >> the orphan port_binding if left in the IC_SB_DB. >> >> This patch fixes such situation and adds test for this case. >> >> Signed-off-by: Vladislav Odintsov <odivlad@gmail.com <mailto:odivlad@gmail.com>> > > This patch is already merged, right ? > https://github.com/ovn-org/ovn/commit/94fc72fce2ce4eed8f3efad5a3303e0b6232f1dc <https://github.com/ovn-org/ovn/commit/94fc72fce2ce4eed8f3efad5a3303e0b6232f1dc> > Or I'm wrong here ? > > Thanks > Numan > >> --- >> ic/ovn-ic.c | 35 +++++++++++++++++++++++++++++++-- >> tests/ovn-ic.at | 52 +++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 85 insertions(+), 2 deletions(-) >> >> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c >> index 9eb792ea5..418f19147 100644 >> --- a/ic/ovn-ic.c >> +++ b/ic/ovn-ic.c >> @@ -66,6 +66,7 @@ struct ic_context { >> struct ovsdb_idl_index *nbrec_port_by_name; >> 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_az; >> struct ovsdb_idl_index *icsbrec_port_binding_by_ts; >> struct ovsdb_idl_index *icsbrec_route_by_ts; >> struct ovsdb_idl_index *icsbrec_route_by_ts_az; >> @@ -175,7 +176,7 @@ allocate_ts_dp_key(struct hmap *dp_tnlids) >> } >> >> static void >> -ts_run(struct ic_context *ctx) >> +ts_run(struct ic_context *ctx, const struct icsbrec_availability_zone *az) >> { >> const struct icnbrec_transit_switch *ts; >> >> @@ -240,8 +241,25 @@ ts_run(struct ic_context *ctx) >> * SB, to avoid uncommitted ISB datapath tunnel key to be synced back to >> * AZ. */ >> if (ctx->ovnisb_txn) { >> + struct shash isb_pbs = SHASH_INITIALIZER(&isb_pbs); >> + const struct icsbrec_port_binding *isb_pb; >> + const struct icsbrec_port_binding *isb_pb_key = >> + icsbrec_port_binding_index_init_row( >> + ctx->icsbrec_port_binding_by_az); >> + >> + icsbrec_port_binding_index_set_availability_zone(isb_pb_key, az); >> + >> + ICSBREC_PORT_BINDING_FOR_EACH (isb_pb, ctx->ovnisb_idl) { >> + shash_add(&isb_pbs, isb_pb->transit_switch, isb_pb); >> + } >> + >> /* Create ISB Datapath_Binding */ >> ICNBREC_TRANSIT_SWITCH_FOR_EACH (ts, ctx->ovninb_idl) { >> + while (shash_find_and_delete(&isb_pbs, ts->name)) { >> + /* There may be multiple Port_Bindings */ >> + continue; >> + } >> + >> isb_dp = shash_find_and_delete(&isb_dps, ts->name); >> if (!isb_dp) { >> /* Allocate tunnel key */ >> @@ -261,6 +279,13 @@ ts_run(struct ic_context *ctx) >> SHASH_FOR_EACH (node, &isb_dps) { >> icsbrec_datapath_binding_delete(node->data); >> } >> + >> + SHASH_FOR_EACH (node, &isb_pbs) { >> + icsbrec_port_binding_delete(node->data); >> + } >> + >> + icsbrec_port_binding_index_destroy_row(isb_pb_key); >> + shash_destroy(&isb_pbs); >> } >> ovn_destroy_tnlids(&dp_tnlids); >> shash_destroy(&isb_dps); >> @@ -1493,7 +1518,7 @@ ovn_db_run(struct ic_context *ctx) >> return; >> } >> >> - ts_run(ctx); >> + ts_run(ctx, az); >> gateway_run(ctx, az); >> port_binding_run(ctx, az); >> route_run(ctx, az); >> @@ -1684,6 +1709,11 @@ main(int argc, char *argv[]) >> struct ovsdb_idl_index *sbrec_chassis_by_name >> = ovsdb_idl_index_create1(ovnsb_idl_loop.idl, >> &sbrec_chassis_col_name); >> + >> + struct ovsdb_idl_index *icsbrec_port_binding_by_az >> + = ovsdb_idl_index_create1(ovnisb_idl_loop.idl, >> + &icsbrec_port_binding_col_availability_zone); >> + >> struct ovsdb_idl_index *icsbrec_port_binding_by_ts >> = ovsdb_idl_index_create1(ovnisb_idl_loop.idl, >> &icsbrec_port_binding_col_transit_switch); >> @@ -1736,6 +1766,7 @@ main(int argc, char *argv[]) >> .nbrec_port_by_name = nbrec_port_by_name, >> .sbrec_port_binding_by_name = sbrec_port_binding_by_name, >> .sbrec_chassis_by_name = sbrec_chassis_by_name, >> + .icsbrec_port_binding_by_az = icsbrec_port_binding_by_az, >> .icsbrec_port_binding_by_ts = icsbrec_port_binding_by_ts, >> .icsbrec_route_by_ts = icsbrec_route_by_ts, >> .icsbrec_route_by_ts_az = icsbrec_route_by_ts_az, >> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at >> index ee78f4794..b6a8edb68 100644 >> --- a/tests/ovn-ic.at >> +++ b/tests/ovn-ic.at >> @@ -64,6 +64,58 @@ OVN_CLEANUP_IC([az1]) >> AT_CLEANUP >> ]) >> >> + >> +AT_SETUP([ovn-ic -- port bindings]) >> + >> +ovn_init_ic_db >> +net_add n1 >> + >> +# 1 GW per AZ >> +for i in 1 2; do >> + az=az$i >> + ovn_start $az >> + sim_add gw-$az >> + as gw-$az >> + check ovs-vsctl add-br br-phys >> + ovn_az_attach $az n1 br-phys 192.168.1.$i >> + check ovs-vsctl set open . external-ids:ovn-is-interconn=true >> +done >> + >> +ovn_as az1 >> + >> +# create transit switch and connect to LR >> +check ovn-ic-nbctl ts-add ts1 >> +check ovn-nbctl lr-add lr1 >> +check ovn-nbctl lrp-add lr1 lrp1 00:00:00:00:00:01 10.0.0.1/24 >> +check ovn-nbctl lrp-set-gateway-chassis lrp1 gw-az1 >> + >> +check ovn-nbctl lsp-add ts1 lsp1 -- \ >> + lsp-set-addresses lsp1 router -- \ >> + lsp-set-type lsp1 router -- \ >> + lsp-set-options lsp1 router-port=lrp1 >> + >> +OVS_WAIT_UNTIL([ovn-sbctl list datapath_binding | grep interconn-ts | grep ts1]) >> + >> +# check port binding appeared >> +AT_CHECK([ovn-ic-sbctl show | grep -A2 lsp1], [0], [dnl >> + port lsp1 >> + transit switch: ts1 >> + address: [["00:00:00:00:00:01 10.0.0.1/24"]] >> +]) >> + >> +# remove transit switch and check if port_binding is deleted >> +check ovn-ic-nbctl ts-del ts1 >> +OVS_WAIT_UNTIL([test -z "$(ovn-ic-sbctl show | grep lsp1)"]) >> + >> +for i in 1 2; do >> + az=az$i >> + OVN_CLEANUP_SBOX(gw-$az) >> + OVN_CLEANUP_AZ([$az]) >> +done >> +OVN_CLEANUP_IC >> +AT_CLEANUP >> + >> + >> OVN_FOR_EACH_NORTHD([ >> AT_SETUP([ovn-ic -- gateway sync]) >> >> -- >> 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 9eb792ea5..418f19147 100644 --- a/ic/ovn-ic.c +++ b/ic/ovn-ic.c @@ -66,6 +66,7 @@ struct ic_context { struct ovsdb_idl_index *nbrec_port_by_name; 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_az; struct ovsdb_idl_index *icsbrec_port_binding_by_ts; struct ovsdb_idl_index *icsbrec_route_by_ts; struct ovsdb_idl_index *icsbrec_route_by_ts_az; @@ -175,7 +176,7 @@ allocate_ts_dp_key(struct hmap *dp_tnlids) } static void -ts_run(struct ic_context *ctx) +ts_run(struct ic_context *ctx, const struct icsbrec_availability_zone *az) { const struct icnbrec_transit_switch *ts; @@ -240,8 +241,25 @@ ts_run(struct ic_context *ctx) * SB, to avoid uncommitted ISB datapath tunnel key to be synced back to * AZ. */ if (ctx->ovnisb_txn) { + struct shash isb_pbs = SHASH_INITIALIZER(&isb_pbs); + const struct icsbrec_port_binding *isb_pb; + const struct icsbrec_port_binding *isb_pb_key = + icsbrec_port_binding_index_init_row( + ctx->icsbrec_port_binding_by_az); + + icsbrec_port_binding_index_set_availability_zone(isb_pb_key, az); + + ICSBREC_PORT_BINDING_FOR_EACH (isb_pb, ctx->ovnisb_idl) { + shash_add(&isb_pbs, isb_pb->transit_switch, isb_pb); + } + /* Create ISB Datapath_Binding */ ICNBREC_TRANSIT_SWITCH_FOR_EACH (ts, ctx->ovninb_idl) { + while (shash_find_and_delete(&isb_pbs, ts->name)) { + /* There may be multiple Port_Bindings */ + continue; + } + isb_dp = shash_find_and_delete(&isb_dps, ts->name); if (!isb_dp) { /* Allocate tunnel key */ @@ -261,6 +279,13 @@ ts_run(struct ic_context *ctx) SHASH_FOR_EACH (node, &isb_dps) { icsbrec_datapath_binding_delete(node->data); } + + SHASH_FOR_EACH (node, &isb_pbs) { + icsbrec_port_binding_delete(node->data); + } + + icsbrec_port_binding_index_destroy_row(isb_pb_key); + shash_destroy(&isb_pbs); } ovn_destroy_tnlids(&dp_tnlids); shash_destroy(&isb_dps); @@ -1493,7 +1518,7 @@ ovn_db_run(struct ic_context *ctx) return; } - ts_run(ctx); + ts_run(ctx, az); gateway_run(ctx, az); port_binding_run(ctx, az); route_run(ctx, az); @@ -1684,6 +1709,11 @@ main(int argc, char *argv[]) struct ovsdb_idl_index *sbrec_chassis_by_name = ovsdb_idl_index_create1(ovnsb_idl_loop.idl, &sbrec_chassis_col_name); + + struct ovsdb_idl_index *icsbrec_port_binding_by_az + = ovsdb_idl_index_create1(ovnisb_idl_loop.idl, + &icsbrec_port_binding_col_availability_zone); + struct ovsdb_idl_index *icsbrec_port_binding_by_ts = ovsdb_idl_index_create1(ovnisb_idl_loop.idl, &icsbrec_port_binding_col_transit_switch); @@ -1736,6 +1766,7 @@ main(int argc, char *argv[]) .nbrec_port_by_name = nbrec_port_by_name, .sbrec_port_binding_by_name = sbrec_port_binding_by_name, .sbrec_chassis_by_name = sbrec_chassis_by_name, + .icsbrec_port_binding_by_az = icsbrec_port_binding_by_az, .icsbrec_port_binding_by_ts = icsbrec_port_binding_by_ts, .icsbrec_route_by_ts = icsbrec_route_by_ts, .icsbrec_route_by_ts_az = icsbrec_route_by_ts_az, diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at index ee78f4794..b6a8edb68 100644 --- a/tests/ovn-ic.at +++ b/tests/ovn-ic.at @@ -64,6 +64,58 @@ OVN_CLEANUP_IC([az1]) AT_CLEANUP ]) + +AT_SETUP([ovn-ic -- port bindings]) + +ovn_init_ic_db +net_add n1 + +# 1 GW per AZ +for i in 1 2; do + az=az$i + ovn_start $az + sim_add gw-$az + as gw-$az + check ovs-vsctl add-br br-phys + ovn_az_attach $az n1 br-phys 192.168.1.$i + check ovs-vsctl set open . external-ids:ovn-is-interconn=true +done + +ovn_as az1 + +# create transit switch and connect to LR +check ovn-ic-nbctl ts-add ts1 +check ovn-nbctl lr-add lr1 +check ovn-nbctl lrp-add lr1 lrp1 00:00:00:00:00:01 10.0.0.1/24 +check ovn-nbctl lrp-set-gateway-chassis lrp1 gw-az1 + +check ovn-nbctl lsp-add ts1 lsp1 -- \ + lsp-set-addresses lsp1 router -- \ + lsp-set-type lsp1 router -- \ + lsp-set-options lsp1 router-port=lrp1 + +OVS_WAIT_UNTIL([ovn-sbctl list datapath_binding | grep interconn-ts | grep ts1]) + +# check port binding appeared +AT_CHECK([ovn-ic-sbctl show | grep -A2 lsp1], [0], [dnl + port lsp1 + transit switch: ts1 + address: [["00:00:00:00:00:01 10.0.0.1/24"]] +]) + +# remove transit switch and check if port_binding is deleted +check ovn-ic-nbctl ts-del ts1 +OVS_WAIT_UNTIL([test -z "$(ovn-ic-sbctl show | grep lsp1)"]) + +for i in 1 2; do + az=az$i + OVN_CLEANUP_SBOX(gw-$az) + OVN_CLEANUP_AZ([$az]) +done +OVN_CLEANUP_IC +AT_CLEANUP + + OVN_FOR_EACH_NORTHD([ AT_SETUP([ovn-ic -- gateway sync])
When IC port_binding exists and transit switch is deleted, the orphan port_binding if left in the IC_SB_DB. This patch fixes such situation and adds test for this case. Signed-off-by: Vladislav Odintsov <odivlad@gmail.com> --- ic/ovn-ic.c | 35 +++++++++++++++++++++++++++++++-- tests/ovn-ic.at | 52 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 2 deletions(-)