[ovs-dev] [PATCH ovn v2 1/6] ic: remove port_binding on ts deletion

Vladislav Odintsov odivlad at gmail.com
Tue Sep 14 18:55:06 UTC 2021


Hi Numan,

yes, you’re correct.

Regards,
Vladislav Odintsov

> On 14 Sep 2021, at 21:41, Numan Siddique <numans at ovn.org> wrote:
> 
> On Fri, Sep 3, 2021 at 6:35 AM Vladislav Odintsov <odivlad at gmail.com <mailto:odivlad at 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 at gmail.com <mailto:odivlad at 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 at openvswitch.org <mailto:dev at openvswitch.org>
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
>> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org <mailto:dev at openvswitch.org>
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>


More information about the dev mailing list