| Message ID | 20211005202442.85322-2-odivlad@gmail.com |
|---|---|
| State | Accepted |
| 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-_ovn-kubernetes | success | github build: passed |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
On Tue, Oct 5, 2021 at 1:25 PM Vladislav Odintsov <odivlad@gmail.com> wrote: > > This commit adds a small optimization by utilizing ovsdb_index > to iterate over port_bindings. > Prior to this change each iteration checked availability_zone > and continued processing only if port_binding belons to local AZ. > > Now we run against port_bindings from local AZ only and don't check > availability_zone. > > Signed-off-by: Vladislav Odintsov <odivlad@gmail.com> > Acked-by: Numan Siddique <numans@ovn.org> > --- > ic/ovn-ic.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > index 99356253d..303e93a4f 100644 > --- a/ic/ovn-ic.c > +++ b/ic/ovn-ic.c > @@ -68,6 +68,7 @@ struct ic_context { > 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_port_binding_by_ts_az; > struct ovsdb_idl_index *icsbrec_route_by_ts; > struct ovsdb_idl_index *icsbrec_route_by_ts_az; > }; > @@ -1386,17 +1387,14 @@ route_run(struct ic_context *ctx, > 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_ts); > + ctx->icsbrec_port_binding_by_ts_az); > icsbrec_port_binding_index_set_transit_switch(isb_pb_key, ts->name); > + icsbrec_port_binding_index_set_availability_zone(isb_pb_key, az); This index row needs to be destroyed by calling icsbrec_port_binding_index_destroy_row() after the below FOR loop. Otherwise this patch looks good to me. Thanks, Han > > /* Each port on TS maps to a logical router, which is stored in the > * external_ids:router-id of the IC SB port_binding record. */ > ICSBREC_PORT_BINDING_FOR_EACH_EQUAL (isb_pb, isb_pb_key, > - ctx->icsbrec_port_binding_by_ts) { > - if (isb_pb->availability_zone != az) { > - continue; > - } > - > + ctx->icsbrec_port_binding_by_ts_az) { > const char *ts_lrp_name = > get_lrp_name_by_ts_port_name(ctx, isb_pb->logical_port); > if (!ts_lrp_name) { > @@ -1713,6 +1711,11 @@ main(int argc, char *argv[]) > = ovsdb_idl_index_create1(ovnisb_idl_loop.idl, > &icsbrec_port_binding_col_transit_switch); > > + struct ovsdb_idl_index *icsbrec_port_binding_by_ts_az > + = ovsdb_idl_index_create2(ovnisb_idl_loop.idl, > + &icsbrec_port_binding_col_transit_switch, > + &icsbrec_port_binding_col_availability_zone); > + > struct ovsdb_idl_index *icsbrec_route_by_ts > = ovsdb_idl_index_create1(ovnisb_idl_loop.idl, > &icsbrec_route_col_transit_switch); > @@ -1763,6 +1766,7 @@ main(int argc, char *argv[]) > .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_port_binding_by_ts_az = icsbrec_port_binding_by_ts_az, > .icsbrec_route_by_ts = icsbrec_route_by_ts, > .icsbrec_route_by_ts_az = icsbrec_route_by_ts_az, > }; > -- > 2.30.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Regards, Vladislav Odintsov > On 14 Oct 2021, at 06:23, Han Zhou <hzhou@ovn.org> wrote: > > > > On Tue, Oct 5, 2021 at 1:25 PM Vladislav Odintsov <odivlad@gmail.com <mailto:odivlad@gmail.com>> wrote: > > > > This commit adds a small optimization by utilizing ovsdb_index > > to iterate over port_bindings. > > Prior to this change each iteration checked availability_zone > > and continued processing only if port_binding belons to local AZ. > > > > Now we run against port_bindings from local AZ only and don't check > > availability_zone. > > > > Signed-off-by: Vladislav Odintsov <odivlad@gmail.com <mailto:odivlad@gmail.com>> > > Acked-by: Numan Siddique <numans@ovn.org <mailto:numans@ovn.org>> > > --- > > ic/ovn-ic.c | 16 ++++++++++------ > > 1 file changed, 10 insertions(+), 6 deletions(-) > > > > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > > index 99356253d..303e93a4f 100644 > > --- a/ic/ovn-ic.c > > +++ b/ic/ovn-ic.c > > @@ -68,6 +68,7 @@ struct ic_context { > > 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_port_binding_by_ts_az; > > struct ovsdb_idl_index *icsbrec_route_by_ts; > > struct ovsdb_idl_index *icsbrec_route_by_ts_az; > > }; > > @@ -1386,17 +1387,14 @@ route_run(struct ic_context *ctx, > > 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_ts); > > + ctx->icsbrec_port_binding_by_ts_az); > > icsbrec_port_binding_index_set_transit_switch(isb_pb_key, ts->name); > > + icsbrec_port_binding_index_set_availability_zone(isb_pb_key, az); > > This index row needs to be destroyed by calling icsbrec_port_binding_index_destroy_row() after the below FOR loop. This index is not new construction here. I’ve just replaced ovsdb_index_1 with ovsdb_index_2 which supports availability zone field. icsbrec_port_binding_index_destroy_row() is called on line 1488 in this commit. > > > Otherwise this patch looks good to me. > > Thanks, > Han > > > > > /* Each port on TS maps to a logical router, which is stored in the > > * external_ids:router-id of the IC SB port_binding record. */ > > ICSBREC_PORT_BINDING_FOR_EACH_EQUAL (isb_pb, isb_pb_key, > > - ctx->icsbrec_port_binding_by_ts) { > > - if (isb_pb->availability_zone != az) { > > - continue; > > - } > > - > > + ctx->icsbrec_port_binding_by_ts_az) { > > const char *ts_lrp_name = > > get_lrp_name_by_ts_port_name(ctx, isb_pb->logical_port); > > if (!ts_lrp_name) { > > @@ -1713,6 +1711,11 @@ main(int argc, char *argv[]) > > = ovsdb_idl_index_create1(ovnisb_idl_loop.idl, > > &icsbrec_port_binding_col_transit_switch); > > > > + struct ovsdb_idl_index *icsbrec_port_binding_by_ts_az > > + = ovsdb_idl_index_create2(ovnisb_idl_loop.idl, > > + &icsbrec_port_binding_col_transit_switch, > > + &icsbrec_port_binding_col_availability_zone); > > + > > struct ovsdb_idl_index *icsbrec_route_by_ts > > = ovsdb_idl_index_create1(ovnisb_idl_loop.idl, > > &icsbrec_route_col_transit_switch); > > @@ -1763,6 +1766,7 @@ main(int argc, char *argv[]) > > .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_port_binding_by_ts_az = icsbrec_port_binding_by_ts_az, > > .icsbrec_route_by_ts = icsbrec_route_by_ts, > > .icsbrec_route_by_ts_az = icsbrec_route_by_ts_az, > > }; > > -- > > 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>
On Thu, Oct 14, 2021 at 1:55 AM Vladislav Odintsov <odivlad@gmail.com> wrote: > > > > Regards, > Vladislav Odintsov > > On 14 Oct 2021, at 06:23, Han Zhou <hzhou@ovn.org> wrote: > > > > On Tue, Oct 5, 2021 at 1:25 PM Vladislav Odintsov <odivlad@gmail.com> wrote: > > > > This commit adds a small optimization by utilizing ovsdb_index > > to iterate over port_bindings. > > Prior to this change each iteration checked availability_zone > > and continued processing only if port_binding belons to local AZ. > > > > Now we run against port_bindings from local AZ only and don't check > > availability_zone. > > > > Signed-off-by: Vladislav Odintsov <odivlad@gmail.com> > > Acked-by: Numan Siddique <numans@ovn.org> > > --- > > ic/ovn-ic.c | 16 ++++++++++------ > > 1 file changed, 10 insertions(+), 6 deletions(-) > > > > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > > index 99356253d..303e93a4f 100644 > > --- a/ic/ovn-ic.c > > +++ b/ic/ovn-ic.c > > @@ -68,6 +68,7 @@ struct ic_context { > > 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_port_binding_by_ts_az; > > struct ovsdb_idl_index *icsbrec_route_by_ts; > > struct ovsdb_idl_index *icsbrec_route_by_ts_az; > > }; > > @@ -1386,17 +1387,14 @@ route_run(struct ic_context *ctx, > > 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_ts); > > + ctx->icsbrec_port_binding_by_ts_az); > > icsbrec_port_binding_index_set_transit_switch(isb_pb_key, ts->name); > > + icsbrec_port_binding_index_set_availability_zone(isb_pb_key, az); > > This index row needs to be destroyed by calling icsbrec_port_binding_index_destroy_row() after the below FOR loop. > > > This index is not new construction here. I’ve just replaced ovsdb_index_1 with ovsdb_index_2 which supports availability zone field. icsbrec_port_binding_index_destroy_row() is called on line 1488 in this commit. > Sorry, my bad. I applied this patch to the main branch. > > Otherwise this patch looks good to me. > > Thanks, > Han > > > > > /* Each port on TS maps to a logical router, which is stored in the > > * external_ids:router-id of the IC SB port_binding record. */ > > ICSBREC_PORT_BINDING_FOR_EACH_EQUAL (isb_pb, isb_pb_key, > > - ctx->icsbrec_port_binding_by_ts) { > > - if (isb_pb->availability_zone != az) { > > - continue; > > - } > > - > > + ctx->icsbrec_port_binding_by_ts_az) { > > const char *ts_lrp_name = > > get_lrp_name_by_ts_port_name(ctx, isb_pb->logical_port); > > if (!ts_lrp_name) { > > @@ -1713,6 +1711,11 @@ main(int argc, char *argv[]) > > = ovsdb_idl_index_create1(ovnisb_idl_loop.idl, > > &icsbrec_port_binding_col_transit_switch); > > > > + struct ovsdb_idl_index *icsbrec_port_binding_by_ts_az > > + = ovsdb_idl_index_create2(ovnisb_idl_loop.idl, > > + &icsbrec_port_binding_col_transit_switch, > > + &icsbrec_port_binding_col_availability_zone); > > + > > struct ovsdb_idl_index *icsbrec_route_by_ts > > = ovsdb_idl_index_create1(ovnisb_idl_loop.idl, > > &icsbrec_route_col_transit_switch); > > @@ -1763,6 +1766,7 @@ main(int argc, char *argv[]) > > .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_port_binding_by_ts_az = icsbrec_port_binding_by_ts_az, > > .icsbrec_route_by_ts = icsbrec_route_by_ts, > > .icsbrec_route_by_ts_az = icsbrec_route_by_ts_az, > > }; > > -- > > 2.30.0 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c index 99356253d..303e93a4f 100644 --- a/ic/ovn-ic.c +++ b/ic/ovn-ic.c @@ -68,6 +68,7 @@ struct ic_context { 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_port_binding_by_ts_az; struct ovsdb_idl_index *icsbrec_route_by_ts; struct ovsdb_idl_index *icsbrec_route_by_ts_az; }; @@ -1386,17 +1387,14 @@ route_run(struct ic_context *ctx, 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_ts); + ctx->icsbrec_port_binding_by_ts_az); icsbrec_port_binding_index_set_transit_switch(isb_pb_key, ts->name); + icsbrec_port_binding_index_set_availability_zone(isb_pb_key, az); /* Each port on TS maps to a logical router, which is stored in the * external_ids:router-id of the IC SB port_binding record. */ ICSBREC_PORT_BINDING_FOR_EACH_EQUAL (isb_pb, isb_pb_key, - ctx->icsbrec_port_binding_by_ts) { - if (isb_pb->availability_zone != az) { - continue; - } - + ctx->icsbrec_port_binding_by_ts_az) { const char *ts_lrp_name = get_lrp_name_by_ts_port_name(ctx, isb_pb->logical_port); if (!ts_lrp_name) { @@ -1713,6 +1711,11 @@ main(int argc, char *argv[]) = ovsdb_idl_index_create1(ovnisb_idl_loop.idl, &icsbrec_port_binding_col_transit_switch); + struct ovsdb_idl_index *icsbrec_port_binding_by_ts_az + = ovsdb_idl_index_create2(ovnisb_idl_loop.idl, + &icsbrec_port_binding_col_transit_switch, + &icsbrec_port_binding_col_availability_zone); + struct ovsdb_idl_index *icsbrec_route_by_ts = ovsdb_idl_index_create1(ovnisb_idl_loop.idl, &icsbrec_route_col_transit_switch); @@ -1763,6 +1766,7 @@ main(int argc, char *argv[]) .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_port_binding_by_ts_az = icsbrec_port_binding_by_ts_az, .icsbrec_route_by_ts = icsbrec_route_by_ts, .icsbrec_route_by_ts_az = icsbrec_route_by_ts_az, };