| Message ID | 8d0cd0ddda46c5be1cf24d0b7aefbffc4d26fb3c.1568220566.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce localnet egress QoS support | expand |
On Wed, Sep 11, 2019 at 7:21 PM Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote: > > Introduce add_egress_interface_mappings routine in order to collect as > egress interfaces all ovs bridge interfaces marked with ovn-egress-iface > in the external_ids column of ovs interface table. > ovn-egress-iface is used to indicate to which localnet ports QoS egress > shaping has to be applied. > Refactor add_bridge_mappings routine > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> Hi Lorenzo, A few comments below. Thanks, Dumitru > --- > controller/binding.c | 60 +++++++++++++++++++++++++++++ > controller/binding.h | 4 ++ > controller/ovn-controller.c | 3 +- > controller/patch.c | 76 +++++++++++++++++++++---------------- > controller/patch.h | 4 ++ > 5 files changed, 113 insertions(+), 34 deletions(-) > > diff --git a/controller/binding.c b/controller/binding.c > index 47d4fea43..39fd79cd0 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -18,6 +18,7 @@ > #include "ha-chassis.h" > #include "lflow.h" > #include "lport.h" > +#include "patch.h" > > #include "lib/bitmap.h" > #include "openvswitch/poll-loop.h" > @@ -520,6 +521,9 @@ consider_local_datapath(struct ovsdb_idl_txn *ovnsb_idl_txn, > } else if (!strcmp(binding_rec->type, "localnet")) { > /* Add all localnet ports to local_lports so that we allocate ct zones > * for them. */ > + if (qos_map && ovs_idl_txn) { > + get_qos_params(binding_rec, qos_map); > + } Can we move this before the comment above? Or update the comment? > sset_add(local_lports, binding_rec->logical_port); > } else if (!strcmp(binding_rec->type, "external")) { > if (ha_chassis_group_contains(binding_rec->ha_chassis_group, > @@ -632,6 +636,57 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec, > ld->localnet_port = binding_rec; > } > > +static void > +add_egress_interface_mappings(const struct ovsrec_bridge_table *bridge_tb, > + const struct ovsrec_open_vswitch_table *ovs_tb, > + const struct sbrec_port_binding_table *pb_tb, > + struct sset *egress_ifaces) > +{ > + struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings); > + > + add_ovs_bridge_mappings(ovs_tb, bridge_tb, &bridge_mappings); > + > + const struct sbrec_port_binding *binding; > + SBREC_PORT_BINDING_TABLE_FOR_EACH (binding, pb_tb) { > + const char *network = smap_get(&binding->options, "network_name"); > + if (!network) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_ERR_RL(&rl, "%s port '%s' has no network name.", > + binding->type, binding->logical_port); > + continue; > + } > + struct ovsrec_bridge *br_ln = shash_find_data(&bridge_mappings, > + network); > + if (!br_ln) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_ERR_RL(&rl, "bridge not found for %s port '%s' " > + "with network name '%s'", > + binding->type, binding->logical_port, network); > + continue; > + } > + > + /* Add egress-ifaces from the connected bridge */ > + for (size_t i = 0; i < br_ln->n_ports; i++) { > + const struct ovsrec_port *port_rec = br_ln->ports[i]; > + > + for (size_t j = 0; j < port_rec->n_interfaces; j++) { > + const struct ovsrec_interface *iface_rec; > + > + iface_rec = port_rec->interfaces[j]; > + bool is_egress_iface = smap_get_bool(&iface_rec->external_ids, > + "ovn-egress-iface", > + false); > + if (!is_egress_iface) { > + continue; > + } > + sset_add(egress_ifaces, iface_rec->name); > + } > + } > + } > + > + shash_destroy(&bridge_mappings); > +} > + > void > binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > struct ovsdb_idl_txn *ovs_idl_txn, > @@ -644,6 +699,8 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > const struct ovsrec_bridge *br_int, > const struct sbrec_chassis *chassis_rec, > const struct sset *active_tunnels, > + const struct ovsrec_bridge_table *bridge_table, > + const struct ovsrec_open_vswitch_table *ovs_table, > struct hmap *local_datapaths, struct sset *local_lports, > struct sset *local_lport_ids) > { > @@ -656,6 +713,9 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > struct sset egress_ifaces = SSET_INITIALIZER(&egress_ifaces); > struct hmap qos_map; > > + add_egress_interface_mappings(bridge_table, ovs_table, port_binding_table, > + &egress_ifaces); Couldn't we add the localnet egress interfaces in consider_localnet_port() instead? We would avoid walking all the port bindings twice. > + > hmap_init(&qos_map); > if (br_int) { > get_local_iface_ids(br_int, &lport_to_iface, local_lports, > diff --git a/controller/binding.h b/controller/binding.h > index bae162ede..924891c1b 100644 > --- a/controller/binding.h > +++ b/controller/binding.h > @@ -26,6 +26,8 @@ struct ovsdb_idl_txn; > struct ovsrec_bridge; > struct ovsrec_port_table; > struct ovsrec_qos_table; > +struct ovsrec_bridge_table; > +struct ovsrec_open_vswitch_table; > struct sbrec_chassis; > struct sbrec_port_binding_table; > struct sset; > @@ -42,6 +44,8 @@ void binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > const struct ovsrec_bridge *br_int, > const struct sbrec_chassis *, > const struct sset *active_tunnels, > + const struct ovsrec_bridge_table *bridge_table, > + const struct ovsrec_open_vswitch_table *ovs_table, > struct hmap *local_datapaths, > struct sset *local_lports, struct sset *local_lport_ids); > bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index b5449b824..03867a2c9 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -1045,7 +1045,8 @@ en_runtime_data_run(struct engine_node *node) > sbrec_port_binding_by_name, > port_table, qos_table, pb_table, > br_int, chassis, > - active_tunnels, local_datapaths, > + active_tunnels, bridge_table, > + ovs_table, local_datapaths, > local_lports, local_lport_ids); > > update_ct_zones(local_lports, local_datapaths, ct_zones, > diff --git a/controller/patch.c b/controller/patch.c > index a6770c6d5..f2053de7b 100644 > --- a/controller/patch.c > +++ b/controller/patch.c > @@ -129,6 +129,48 @@ remove_port(const struct ovsrec_bridge_table *bridge_table, > } > } > > +void > +add_ovs_bridge_mappings(const struct ovsrec_open_vswitch_table *ovs_table, > + const struct ovsrec_bridge_table *bridge_table, > + struct shash *bridge_mappings) > +{ > + const struct ovsrec_open_vswitch *cfg; > + > + cfg = ovsrec_open_vswitch_table_first(ovs_table); > + if (cfg) { > + const char *mappings_cfg; > + char *cur, *next, *start; > + > + mappings_cfg = smap_get(&cfg->external_ids, "ovn-bridge-mappings"); > + if (!mappings_cfg || !mappings_cfg[0]) { > + return; > + } > + > + next = start = xstrdup(mappings_cfg); > + while ((cur = strsep(&next, ",")) && *cur) { > + const struct ovsrec_bridge *ovs_bridge; > + char *network, *bridge = cur; > + > + network = strsep(&bridge, ":"); > + if (!bridge || !*network || !*bridge) { > + VLOG_ERR("Invalid ovn-bridge-mappings configuration: '%s'", > + mappings_cfg); > + break; > + } > + > + ovs_bridge = get_bridge(bridge_table, bridge); > + if (!ovs_bridge) { > + VLOG_WARN("Bridge '%s' not found for network '%s'", > + bridge, network); > + continue; > + } > + > + shash_add(bridge_mappings, network, ovs_bridge); > + } > + free(start); > + } > +} > + > /* Obtains external-ids:ovn-bridge-mappings from OVSDB and adds patch ports for > * the local bridge mappings. Removes any patch ports for bridge mappings that > * already existed from 'existing_ports'. */ > @@ -142,41 +184,9 @@ add_bridge_mappings(struct ovsdb_idl_txn *ovs_idl_txn, > const struct sbrec_chassis *chassis) > { > /* Get ovn-bridge-mappings. */ > - const char *mappings_cfg = ""; > - const struct ovsrec_open_vswitch *cfg; > - cfg = ovsrec_open_vswitch_table_first(ovs_table); > - if (cfg) { > - mappings_cfg = smap_get(&cfg->external_ids, "ovn-bridge-mappings"); > - if (!mappings_cfg || !mappings_cfg[0]) { > - return; > - } > - } > - > - /* Parse bridge mappings. */ > struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings); > - char *cur, *next, *start; > - next = start = xstrdup(mappings_cfg); > - while ((cur = strsep(&next, ",")) && *cur) { > - char *network, *bridge = cur; > - const struct ovsrec_bridge *ovs_bridge; > - > - network = strsep(&bridge, ":"); > - if (!bridge || !*network || !*bridge) { > - VLOG_ERR("Invalid ovn-bridge-mappings configuration: '%s'", > - mappings_cfg); > - break; > - } > > - ovs_bridge = get_bridge(bridge_table, bridge); > - if (!ovs_bridge) { > - VLOG_WARN("Bridge '%s' not found for network '%s'", > - bridge, network); > - continue; > - } > - > - shash_add(&bridge_mappings, network, ovs_bridge); > - } > - free(start); > + add_ovs_bridge_mappings(ovs_table, bridge_table, &bridge_mappings); > > const struct sbrec_port_binding *binding; > SBREC_PORT_BINDING_TABLE_FOR_EACH (binding, port_binding_table) { > diff --git a/controller/patch.h b/controller/patch.h > index 9018e4967..49b0b2e90 100644 > --- a/controller/patch.h > +++ b/controller/patch.h > @@ -30,7 +30,11 @@ struct ovsrec_open_vswitch_table; > struct ovsrec_port_table; > struct sbrec_port_binding_table; > struct sbrec_chassis; > +struct shash; > > +void add_ovs_bridge_mappings(const struct ovsrec_open_vswitch_table *ovs_table, > + const struct ovsrec_bridge_table *bridge_table, > + struct shash *bridge_mappings); > void patch_run(struct ovsdb_idl_txn *ovs_idl_txn, > const struct ovsrec_bridge_table *, > const struct ovsrec_open_vswitch_table *, > -- > 2.21.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/controller/binding.c b/controller/binding.c index 47d4fea43..39fd79cd0 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -18,6 +18,7 @@ #include "ha-chassis.h" #include "lflow.h" #include "lport.h" +#include "patch.h" #include "lib/bitmap.h" #include "openvswitch/poll-loop.h" @@ -520,6 +521,9 @@ consider_local_datapath(struct ovsdb_idl_txn *ovnsb_idl_txn, } else if (!strcmp(binding_rec->type, "localnet")) { /* Add all localnet ports to local_lports so that we allocate ct zones * for them. */ + if (qos_map && ovs_idl_txn) { + get_qos_params(binding_rec, qos_map); + } sset_add(local_lports, binding_rec->logical_port); } else if (!strcmp(binding_rec->type, "external")) { if (ha_chassis_group_contains(binding_rec->ha_chassis_group, @@ -632,6 +636,57 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec, ld->localnet_port = binding_rec; } +static void +add_egress_interface_mappings(const struct ovsrec_bridge_table *bridge_tb, + const struct ovsrec_open_vswitch_table *ovs_tb, + const struct sbrec_port_binding_table *pb_tb, + struct sset *egress_ifaces) +{ + struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings); + + add_ovs_bridge_mappings(ovs_tb, bridge_tb, &bridge_mappings); + + const struct sbrec_port_binding *binding; + SBREC_PORT_BINDING_TABLE_FOR_EACH (binding, pb_tb) { + const char *network = smap_get(&binding->options, "network_name"); + if (!network) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); + VLOG_ERR_RL(&rl, "%s port '%s' has no network name.", + binding->type, binding->logical_port); + continue; + } + struct ovsrec_bridge *br_ln = shash_find_data(&bridge_mappings, + network); + if (!br_ln) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); + VLOG_ERR_RL(&rl, "bridge not found for %s port '%s' " + "with network name '%s'", + binding->type, binding->logical_port, network); + continue; + } + + /* Add egress-ifaces from the connected bridge */ + for (size_t i = 0; i < br_ln->n_ports; i++) { + const struct ovsrec_port *port_rec = br_ln->ports[i]; + + for (size_t j = 0; j < port_rec->n_interfaces; j++) { + const struct ovsrec_interface *iface_rec; + + iface_rec = port_rec->interfaces[j]; + bool is_egress_iface = smap_get_bool(&iface_rec->external_ids, + "ovn-egress-iface", + false); + if (!is_egress_iface) { + continue; + } + sset_add(egress_ifaces, iface_rec->name); + } + } + } + + shash_destroy(&bridge_mappings); +} + void binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn, struct ovsdb_idl_txn *ovs_idl_txn, @@ -644,6 +699,8 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn, const struct ovsrec_bridge *br_int, const struct sbrec_chassis *chassis_rec, const struct sset *active_tunnels, + const struct ovsrec_bridge_table *bridge_table, + const struct ovsrec_open_vswitch_table *ovs_table, struct hmap *local_datapaths, struct sset *local_lports, struct sset *local_lport_ids) { @@ -656,6 +713,9 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn, struct sset egress_ifaces = SSET_INITIALIZER(&egress_ifaces); struct hmap qos_map; + add_egress_interface_mappings(bridge_table, ovs_table, port_binding_table, + &egress_ifaces); + hmap_init(&qos_map); if (br_int) { get_local_iface_ids(br_int, &lport_to_iface, local_lports, diff --git a/controller/binding.h b/controller/binding.h index bae162ede..924891c1b 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -26,6 +26,8 @@ struct ovsdb_idl_txn; struct ovsrec_bridge; struct ovsrec_port_table; struct ovsrec_qos_table; +struct ovsrec_bridge_table; +struct ovsrec_open_vswitch_table; struct sbrec_chassis; struct sbrec_port_binding_table; struct sset; @@ -42,6 +44,8 @@ void binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn, const struct ovsrec_bridge *br_int, const struct sbrec_chassis *, const struct sset *active_tunnels, + const struct ovsrec_bridge_table *bridge_table, + const struct ovsrec_open_vswitch_table *ovs_table, struct hmap *local_datapaths, struct sset *local_lports, struct sset *local_lport_ids); bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index b5449b824..03867a2c9 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1045,7 +1045,8 @@ en_runtime_data_run(struct engine_node *node) sbrec_port_binding_by_name, port_table, qos_table, pb_table, br_int, chassis, - active_tunnels, local_datapaths, + active_tunnels, bridge_table, + ovs_table, local_datapaths, local_lports, local_lport_ids); update_ct_zones(local_lports, local_datapaths, ct_zones, diff --git a/controller/patch.c b/controller/patch.c index a6770c6d5..f2053de7b 100644 --- a/controller/patch.c +++ b/controller/patch.c @@ -129,6 +129,48 @@ remove_port(const struct ovsrec_bridge_table *bridge_table, } } +void +add_ovs_bridge_mappings(const struct ovsrec_open_vswitch_table *ovs_table, + const struct ovsrec_bridge_table *bridge_table, + struct shash *bridge_mappings) +{ + const struct ovsrec_open_vswitch *cfg; + + cfg = ovsrec_open_vswitch_table_first(ovs_table); + if (cfg) { + const char *mappings_cfg; + char *cur, *next, *start; + + mappings_cfg = smap_get(&cfg->external_ids, "ovn-bridge-mappings"); + if (!mappings_cfg || !mappings_cfg[0]) { + return; + } + + next = start = xstrdup(mappings_cfg); + while ((cur = strsep(&next, ",")) && *cur) { + const struct ovsrec_bridge *ovs_bridge; + char *network, *bridge = cur; + + network = strsep(&bridge, ":"); + if (!bridge || !*network || !*bridge) { + VLOG_ERR("Invalid ovn-bridge-mappings configuration: '%s'", + mappings_cfg); + break; + } + + ovs_bridge = get_bridge(bridge_table, bridge); + if (!ovs_bridge) { + VLOG_WARN("Bridge '%s' not found for network '%s'", + bridge, network); + continue; + } + + shash_add(bridge_mappings, network, ovs_bridge); + } + free(start); + } +} + /* Obtains external-ids:ovn-bridge-mappings from OVSDB and adds patch ports for * the local bridge mappings. Removes any patch ports for bridge mappings that * already existed from 'existing_ports'. */ @@ -142,41 +184,9 @@ add_bridge_mappings(struct ovsdb_idl_txn *ovs_idl_txn, const struct sbrec_chassis *chassis) { /* Get ovn-bridge-mappings. */ - const char *mappings_cfg = ""; - const struct ovsrec_open_vswitch *cfg; - cfg = ovsrec_open_vswitch_table_first(ovs_table); - if (cfg) { - mappings_cfg = smap_get(&cfg->external_ids, "ovn-bridge-mappings"); - if (!mappings_cfg || !mappings_cfg[0]) { - return; - } - } - - /* Parse bridge mappings. */ struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings); - char *cur, *next, *start; - next = start = xstrdup(mappings_cfg); - while ((cur = strsep(&next, ",")) && *cur) { - char *network, *bridge = cur; - const struct ovsrec_bridge *ovs_bridge; - - network = strsep(&bridge, ":"); - if (!bridge || !*network || !*bridge) { - VLOG_ERR("Invalid ovn-bridge-mappings configuration: '%s'", - mappings_cfg); - break; - } - ovs_bridge = get_bridge(bridge_table, bridge); - if (!ovs_bridge) { - VLOG_WARN("Bridge '%s' not found for network '%s'", - bridge, network); - continue; - } - - shash_add(&bridge_mappings, network, ovs_bridge); - } - free(start); + add_ovs_bridge_mappings(ovs_table, bridge_table, &bridge_mappings); const struct sbrec_port_binding *binding; SBREC_PORT_BINDING_TABLE_FOR_EACH (binding, port_binding_table) { diff --git a/controller/patch.h b/controller/patch.h index 9018e4967..49b0b2e90 100644 --- a/controller/patch.h +++ b/controller/patch.h @@ -30,7 +30,11 @@ struct ovsrec_open_vswitch_table; struct ovsrec_port_table; struct sbrec_port_binding_table; struct sbrec_chassis; +struct shash; +void add_ovs_bridge_mappings(const struct ovsrec_open_vswitch_table *ovs_table, + const struct ovsrec_bridge_table *bridge_table, + struct shash *bridge_mappings); void patch_run(struct ovsdb_idl_txn *ovs_idl_txn, const struct ovsrec_bridge_table *, const struct ovsrec_open_vswitch_table *,
Introduce add_egress_interface_mappings routine in order to collect as egress interfaces all ovs bridge interfaces marked with ovn-egress-iface in the external_ids column of ovs interface table. ovn-egress-iface is used to indicate to which localnet ports QoS egress shaping has to be applied. Refactor add_bridge_mappings routine Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- controller/binding.c | 60 +++++++++++++++++++++++++++++ controller/binding.h | 4 ++ controller/ovn-controller.c | 3 +- controller/patch.c | 76 +++++++++++++++++++++---------------- controller/patch.h | 4 ++ 5 files changed, 113 insertions(+), 34 deletions(-)