> Hi Lorenzo, I have one finding below. > > On 5/15/23 06:03, Lorenzo Bianconi wrote: > > Introduce ovsport_lookup_by_name and ovsport_lookup_by_qos routines in > > order to speed-up ovs port lookup based on port name or qos. > > > > Acked-By: Ihar Hrachyshka <[email protected]> > > Tested-by: Rodolfo Alonso <[email protected]> > > Signed-off-by: Lorenzo Bianconi <[email protected]> > > --- > > controller/binding.c | 33 ++++++++++++--------------------- > > controller/binding.h | 2 +- > > controller/ovn-controller.c | 13 +++++++++---- > > controller/ovsport.c | 32 ++++++++++++++++++++++++++++++++ > > controller/ovsport.h | 5 +++++ > > 5 files changed, 59 insertions(+), 26 deletions(-) > > > > diff --git a/controller/binding.c b/controller/binding.c > > index 7ec3d5d5d..db25ee391 100644 > > --- a/controller/binding.c > > +++ b/controller/binding.c > > @@ -36,6 +36,7 @@ > > #include "lport.h" > > #include "ovn-controller.h" > > #include "patch.h" > > +#include "ovsport.h" > > VLOG_DEFINE_THIS_MODULE(binding); > > @@ -293,7 +294,7 @@ add_ovs_qos_table_entry(struct ovsdb_idl_txn > > *ovs_idl_txn, > > static void > > remove_stale_qos_entry(struct ovsdb_idl_txn *ovs_idl_txn, > > const struct sbrec_port_binding *pb, > > - const struct ovsrec_port_table *port_table, > > + struct ovsdb_idl_index *ovsrec_port_by_qos, > > const struct ovsrec_qos_table *qos_table, > > struct hmap *queue_map) > > { > > @@ -326,13 +327,8 @@ remove_stale_qos_entry(struct ovsdb_idl_txn > > *ovs_idl_txn, > > ovsrec_queue_delete(queue); > > if (qos->n_queues == 1) { > > - const struct ovsrec_port *port = NULL, *iter; > > - OVSREC_PORT_TABLE_FOR_EACH (iter, port_table) { > > - if (iter->qos == qos) { > > - port = iter; > > - break; > > - } > > - } > > + const struct ovsrec_port *port = > > + ovsport_lookup_by_qos(ovsrec_port_by_qos, qos); > > if (port) { > > ovsrec_port_set_qos(port, NULL); > > } > > @@ -361,9 +357,8 @@ configure_qos(const struct sbrec_port_binding *pb, > > if ((!min_rate && !max_rate && !burst) || !queue_id) { > > /* Qos is not configured for this port. */ > > remove_stale_qos_entry(b_ctx_in->ovs_idl_txn, pb, > > - b_ctx_in->port_table, > > - b_ctx_in->qos_table, > > - b_ctx_out->qos_map); > > + b_ctx_in->ovsrec_port_by_qos, > > + b_ctx_in->qos_table, b_ctx_out->qos_map); > > return; > > } > > @@ -403,7 +398,7 @@ configure_qos(const struct sbrec_port_binding *pb, > > static void > > ovs_qos_entries_gc(struct ovsdb_idl_txn *ovs_idl_txn, > > - const struct ovsrec_port_table *port_table, > > + struct ovsdb_idl_index *ovsrec_port_by_qos, > > const struct ovsrec_qos_table *qos_table, > > struct hmap *queue_map) > > { > > @@ -435,13 +430,8 @@ ovs_qos_entries_gc(struct ovsdb_idl_txn *ovs_idl_txn, > > } > > if (n_queue_deleted && n_queue_deleted == n_queues) { > > - const struct ovsrec_port *port = NULL, *iter; > > - OVSREC_PORT_TABLE_FOR_EACH (iter, port_table) { > > - if (iter->qos == qos) { > > - port = iter; > > - break; > > - } > > - } > > + const struct ovsrec_port *port = > > + ovsport_lookup_by_qos(ovsrec_port_by_qos, qos); > > if (port) { > > ovsrec_port_set_qos(port, NULL); > > } > > @@ -2165,7 +2155,7 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct > > binding_ctx_out *b_ctx_out) > > shash_destroy(&bridge_mappings); > > /* Remove stale QoS entries. */ > > - ovs_qos_entries_gc(b_ctx_in->ovs_idl_txn, b_ctx_in->port_table, > > + ovs_qos_entries_gc(b_ctx_in->ovs_idl_txn, b_ctx_in->ovsrec_port_by_qos, > > b_ctx_in->qos_table, b_ctx_out->qos_map); > > cleanup_claimed_port_timestamps(); > > @@ -3019,7 +3009,8 @@ binding_handle_port_binding_changes(struct > > binding_ctx_in *b_ctx_in, > > shash_add(&deleted_other_pbs, pb->logical_port, pb); > > } > > - remove_stale_qos_entry(b_ctx_in->ovs_idl_txn, pb, > > b_ctx_in->port_table, > > + remove_stale_qos_entry(b_ctx_in->ovs_idl_txn, pb, > > + b_ctx_in->ovsrec_port_by_qos, > > b_ctx_in->qos_table, b_ctx_out->qos_map); > > } > > diff --git a/controller/binding.h b/controller/binding.h > > index 87ee7b540..6b97bcff0 100644 > > --- a/controller/binding.h > > +++ b/controller/binding.h > > @@ -46,7 +46,7 @@ struct binding_ctx_in { > > struct ovsdb_idl_index *sbrec_datapath_binding_by_key; > > struct ovsdb_idl_index *sbrec_port_binding_by_datapath; > > struct ovsdb_idl_index *sbrec_port_binding_by_name; > > - const struct ovsrec_port_table *port_table; > > + struct ovsdb_idl_index *ovsrec_port_by_qos; > > const struct ovsrec_qos_table *qos_table; > > const struct sbrec_port_binding_table *port_binding_table; > > const struct ovsrec_bridge *br_int; > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > > index 3ddf4795e..1dd1099a4 100644 > > --- a/controller/ovn-controller.c > > +++ b/controller/ovn-controller.c > > @@ -1498,9 +1498,6 @@ init_binding_ctx(struct engine_node *node, > > = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); > > ovs_assert(chassis); > > - const struct ovsrec_port_table *port_table = > > - EN_OVSDB_GET(engine_get_input("OVS_port", node)); > > - > > struct ed_type_ovs_interface_shadow *iface_shadow = > > engine_get_input_data("ovs_interface_shadow", node); > > @@ -1525,6 +1522,10 @@ init_binding_ctx(struct engine_node *node, > > engine_get_input("SB_port_binding", node), > > "datapath"); > > + struct ovsdb_idl_index *ovsrec_port_by_qos = > > + engine_ovsdb_node_get_index( > > + engine_get_input("OVS_port", node), "qos"); > > + > > struct controller_engine_ctx *ctrl_ctx = > > engine_get_context()->client_ctx; > > b_ctx_in->ovnsb_idl_txn = engine_get_context()->ovnsb_idl_txn; > > @@ -1532,7 +1533,7 @@ init_binding_ctx(struct engine_node *node, > > b_ctx_in->sbrec_datapath_binding_by_key = > > sbrec_datapath_binding_by_key; > > b_ctx_in->sbrec_port_binding_by_datapath = > > sbrec_port_binding_by_datapath; > > b_ctx_in->sbrec_port_binding_by_name = sbrec_port_binding_by_name; > > - b_ctx_in->port_table = port_table; > > + b_ctx_in->ovsrec_port_by_qos = ovsrec_port_by_qos; > > b_ctx_in->iface_table = iface_shadow->iface_table; > > b_ctx_in->iface_table_external_ids_old = > > &iface_shadow->iface_table_external_ids_old; > > @@ -4478,6 +4479,9 @@ main(int argc, char *argv[]) > > struct ovsdb_idl_index *ovsrec_port_by_name > > = ovsdb_idl_index_create1(ovs_idl_loop.idl, > > &ovsrec_port_col_name); > > + struct ovsdb_idl_index *ovsrec_port_by_qos > > + = ovsdb_idl_index_create1(ovs_idl_loop.idl, > > + &ovsrec_port_col_qos); > > struct ovsdb_idl_index *ovsrec_flow_sample_collector_set_by_id > > = ovsdb_idl_index_create2(ovs_idl_loop.idl, > > > > &ovsrec_flow_sample_collector_set_col_bridge, > > @@ -4832,6 +4836,7 @@ main(int argc, char *argv[]) > > > > sbrec_chassis_template_var_index_by_chassis); > > engine_ovsdb_node_add_index(&en_ovs_flow_sample_collector_set, "id", > > ovsrec_flow_sample_collector_set_by_id); > > + engine_ovsdb_node_add_index(&en_ovs_port, "qos", ovsrec_port_by_qos); > > struct ed_type_lflow_output *lflow_output_data = > > engine_get_internal_data(&en_lflow_output); > > diff --git a/controller/ovsport.c b/controller/ovsport.c > > index ec38c3fca..ba260dee6 100644 > > --- a/controller/ovsport.c > > +++ b/controller/ovsport.c > > @@ -216,6 +216,38 @@ ovsrec_port * ovsport_lookup_by_interface( > > interfaces, 1); > > } > > +const struct ovsrec_port * > > +ovsport_lookup_by_name(struct ovsdb_idl_index *ovsrec_port_by_name, > > + const char *name) > > +{ > > + const struct ovsrec_port *port = > > + ovsrec_port_index_init_row(ovsrec_port_by_name); > > + ovsrec_port_index_set_name(port, name); > > + > > + const struct ovsrec_port *retval = > > + ovsrec_port_index_find(ovsrec_port_by_name, port); > > + > > + ovsrec_port_index_destroy_row(port); > > + > > + return retval; > > +} > > + > > +const struct ovsrec_port * > > +ovsport_lookup_by_qos(struct ovsdb_idl_index *ovsrec_port_by_qos, > > + const struct ovsrec_qos *qos) > > +{ > > + const struct ovsrec_port *port = > > + ovsrec_port_index_init_row(ovsrec_port_by_qos); > > + ovsrec_port_index_set_qos(port, qos); > > + > > + const struct ovsrec_port *retval = > > + ovsrec_port_index_find(ovsrec_port_by_qos, port); > > + > > + ovsrec_port_index_destroy_row(port); > > + > > + return retval; > > +} > > + > > /* Update an interface map column with the key/value pairs present in the > > * provided smap, only applying changes when necessary. */ > > static void > > diff --git a/controller/ovsport.h b/controller/ovsport.h > > index e355ff7ff..dfd4f13f0 100644 > > --- a/controller/ovsport.h > > +++ b/controller/ovsport.h > > @@ -33,6 +33,7 @@ struct ovsrec_bridge; > > struct ovsrec_port; > > struct ovsrec_interface; > > struct ovsdb_idl_index; > > +struct ovsrec_qos; > > void ovsport_create(struct ovsdb_idl_txn *ovs_idl_txn, > > const struct ovsrec_bridge *bridge, > > @@ -56,5 +57,9 @@ const struct ovsrec_port * ovsport_lookup_by_interfaces( > > const size_t n_interfaces); > > const struct ovsrec_port * ovsport_lookup_by_interface( > > struct ovsdb_idl_index *, struct ovsrec_interface *); > > +const struct ovsrec_port * ovsport_lookup_by_name( > > + struct ovsdb_idl_index *, const char *); > > ovsport_lookup_by_name() is declared and defined but never used.
ack, it was used in a previous version but now we can get rid ot it. Regards, Lorenzo > > > +const struct ovsrec_port * ovsport_lookup_by_qos( > > + struct ovsdb_idl_index *, const struct ovsrec_qos *); > > #endif /* lib/ovsport.h */ > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
