On Mon, May 15, 2023 at 4:53 PM Lorenzo Bianconi
<[email protected]> wrote:
>
> > Hi Lorenzo,
>
> Hi Mark,
>
> >
> > This patch and patch 3 both have the same commit message (the top line is
> > different, but the rest is the same). This doesn't do a good job of
> > explaining the individual changes in patches 2 and 3.
>
> ack, I will fix it.
Hi Lorenzo,
I have a few comments (which applies to the whole series and not just
this patch).
1. I tested this patch series and found a crash. Below is the trace.
[root@ovn-central ~]# /data/ovn-northd
2023-05-15T23:38:47Z|00001|ovn_northd|INFO|OVN internal version is :
[23.03.90-20.27.2-70.6]
2023-05-15T23:38:47Z|00002|reconnect|INFO|unix:/usr/local/var/run/ovn/ovnnb_db.sock:
connecting...
2023-05-15T23:38:47Z|00003|reconnect|INFO|unix:/usr/local/var/run/ovn/ovnnb_db.sock:
connected
2023-05-15T23:38:47Z|00004|ovn_northd|INFO|OVN NB IDL reconnected,
force recompute.
2023-05-15T23:38:47Z|00005|reconnect|INFO|unix:/usr/local/var/run/ovn/ovnsb_db.sock:
connecting...
2023-05-15T23:38:47Z|00006|reconnect|INFO|unix:/usr/local/var/run/ovn/ovnsb_db.sock:
connected
2023-05-15T23:38:47Z|00007|ovn_northd|INFO|OVN SB IDL reconnected,
force recompute.
2023-05-15T23:38:47Z|00008|ovn_northd|INFO|ovn-northd lock acquired.
This ovn-northd instance is now active.
AddressSanitizer:DEADLYSIGNAL
=================================================================
==288==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000
(pc 0x7f019742303d bp 0x7ffd92aae9a0 sp 0x7ffd92aae148 T0)
==288==The signal is caused by a READ memory access.
==288==Hint: address points to the zero page.
#0 0x7f019742303d in __strlen_avx2 (/lib64/libc.so.6+0x15603d)
#1 0x7f0197c1336c in __interceptor_strlen.part.0
(/lib64/libasan.so.8+0x4c36c)
#2 0x624a6d in xstrdup lib/util.c:205
#3 0x61a862 in smap_add lib/smap.c:55
#4 0x4224ed in ovn_port_update_sbrec ../northd/northd.c:3514
#5 0x42bf8f in build_ports ../northd/northd.c:4697
#6 0x47878d in ovnnb_db_run ../northd/northd.c:16647
#7 0x47a609 in northd_run ../northd/northd.c:16962
#8 0x4805af in en_northd_run ../northd/en-northd.c:119
#9 0x4b529f in engine_recompute ../lib/inc-proc-eng.c:415
#10 0x4b5a16 in engine_run_node ../lib/inc-proc-eng.c:477
#11 0x4b5c76 in engine_run ../lib/inc-proc-eng.c:528
#12 0x4866c4 in inc_proc_northd_run ../northd/inc-proc-northd.c:283
#13 0x47e189 in main ../northd/ovn-northd.c:920
#14 0x7f01972f450f in __libc_start_call_main (/lib64/libc.so.6+0x2750f)
#15 0x7f01972f45c8 in __libc_start_main@GLIBC_2.2.5
(/lib64/libc.so.6+0x275c8)
#16 0x4089d4 in _start (/data/ovn-northd+0x4089d4)
I was able to reproduce this by setting the qos params on a logical
port except "network_name".
I think there will be a similar crash in ovn-controller (because of
patch 3 of this series) if ovn-controllers are updated before
ovn-northd.
Please make sure to check for NULL ptr before calling xstrdup
2. As I had mentioned in the earlier email , ovn-northd needs to
properly allocate the queue id now that we support setting qos params
fo the normal VIF logical ports (with localnet port connected to the
logical switch)
Looks like ovn-northd allocates a queue id starting from 1 for
each chassis. Maybe as a simple fix, it can start from queue id 2.
But I'm not quite familiar with that part of the code.
3. Lets say a normal VIF logical port is configured with qos params.
When the ovn-controller releases this logical port (due to ovs
interface getting deleted or external_ids:iface-id of the ovs
interface getting cleared)
It doesn't delete the qos queue for this logical port. Triggering
a recompute fixes this issue. I think you need to call
remove_stale_qos_entry() in the function
binding_handle_ovs_interface_changes() or in consider_iface_release()
Please add some unit tests in ovn.at to cover this scenario.
4. The test case "ovn-controller incremental processing" passes until
patch 3 of the series but starts failing in patch 4,5 and 6. It
passes again in patch 7 and onwards.
This is consistent in my local testing. Please make sure that it
passes for all the individual patches.
"ovn-controller incremental processing FAILED
(ovn-performance.at:562)"
Thanks
Numan
>
> Regards,
> Lorenzo
>
> >
> > On 5/15/23 06:03, Lorenzo Bianconi wrote:
> > > This is a preliminary patch to rework OVN QoS implementation in order to
> > > configure it through OVS QoS table instead of running tc command
> > > directly bypassing OVS.
> > >
> > > Signed-off-by: Lorenzo Bianconi <[email protected]>
> > > ---
> > > controller/binding.c | 201 ++++++++++++++++++------------------
> > > controller/binding.h | 3 +
> > > controller/ovn-controller.c | 8 +-
> > > 3 files changed, 109 insertions(+), 103 deletions(-)
> > >
> > > diff --git a/controller/binding.c b/controller/binding.c
> > > index a0fbacc97..ad19a4092 100644
> > > --- a/controller/binding.c
> > > +++ b/controller/binding.c
> > > @@ -109,14 +109,6 @@ binding_wait(void)
> > > }
> > > }
> > > -struct qos_queue {
> > > - struct hmap_node node;
> > > - uint32_t queue_id;
> > > - uint32_t min_rate;
> > > - uint32_t max_rate;
> > > - uint32_t burst;
> > > -};
> > > -
> > > void
> > > binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> > > {
> > > @@ -147,8 +139,48 @@ static void update_lport_tracking(const struct
> > > sbrec_port_binding *pb,
> > > struct hmap *tracked_dp_bindings,
> > > bool claimed);
> > > +struct qos_queue {
> > > + struct hmap_node node;
> > > +
> > > + char *port;
> > > +
> > > + uint32_t queue_id;
> > > + uint32_t min_rate;
> > > + uint32_t max_rate;
> > > + uint32_t burst;
> > > +};
> > > +
> > > +static struct qos_queue *
> > > +find_qos_queue(struct hmap *queue_map, uint32_t hash, const char *port)
> > > +{
> > > + struct qos_queue *q;
> > > + HMAP_FOR_EACH_WITH_HASH (q, node, hash, queue_map) {
> > > + if (!strcmp(q->port, port)) {
> > > + return q;
> > > + }
> > > + }
> > > + return NULL;
> > > +}
> > > +
> > > static void
> > > -get_qos_params(const struct sbrec_port_binding *pb, struct hmap
> > > *queue_map)
> > > +qos_queue_erase_entry(struct qos_queue *q)
> > > +{
> > > + free(q->port);
> > > + free(q);
> > > +}
> > > +
> > > +void
> > > +destroy_qos_map(struct hmap *qos_map)
> > > +{
> > > + struct qos_queue *q;
> > > + HMAP_FOR_EACH_POP (q, node, qos_map) {
> > > + qos_queue_erase_entry(q);
> > > + }
> > > + hmap_destroy(qos_map);
> > > +}
> > > +
> > > +static void
> > > +get_qos_queue(const struct sbrec_port_binding *pb, struct hmap
> > > *queue_map)
> > > {
> > > uint32_t min_rate = smap_get_int(&pb->options, "qos_min_rate", 0);
> > > uint32_t max_rate = smap_get_int(&pb->options, "qos_max_rate", 0);
> > > @@ -160,12 +192,17 @@ get_qos_params(const struct sbrec_port_binding *pb,
> > > struct hmap *queue_map)
> > > return;
> > > }
> > > - struct qos_queue *node = xzalloc(sizeof *node);
> > > - hmap_insert(queue_map, &node->node, hash_int(queue_id, 0));
> > > - node->min_rate = min_rate;
> > > - node->max_rate = max_rate;
> > > - node->burst = burst;
> > > - node->queue_id = queue_id;
> > > + uint32_t hash = hash_string(pb->logical_port, 0);
> > > + struct qos_queue *q = find_qos_queue(queue_map, hash,
> > > pb->logical_port);
> > > + if (!q) {
> > > + q = xzalloc(sizeof *q);
> > > + hmap_insert(queue_map, &q->node, hash);
> > > + q->port = xstrdup(pb->logical_port);
> > > + q->queue_id = queue_id;
> > > + }
> > > + q->min_rate = min_rate;
> > > + q->max_rate = max_rate;
> > > + q->burst = burst;
> > > }
> > > static const struct ovsrec_qos *
> > > @@ -354,17 +391,6 @@ setup_qos(const char *egress_iface, struct hmap
> > > *queue_map)
> > > netdev_close(netdev_phy);
> > > }
> > > -static void
> > > -destroy_qos_map(struct hmap *qos_map)
> > > -{
> > > - struct qos_queue *qos_queue;
> > > - HMAP_FOR_EACH_POP (qos_queue, node, qos_map) {
> > > - free(qos_queue);
> > > - }
> > > -
> > > - hmap_destroy(qos_map);
> > > -}
> > > -
> > > /*
> > > * Get the encap from the chassis for this port. The interface
> > > * may have an external_ids:encap-ip=<encap-ip> set; if so we
> > > @@ -651,7 +677,7 @@ static struct binding_lport
> > > *local_binding_get_primary_or_localport_lport(
> > > static bool local_binding_handle_stale_binding_lports(
> > > struct local_binding *lbinding, struct binding_ctx_in *b_ctx_in,
> > > - struct binding_ctx_out *b_ctx_out, struct hmap *qos_map);
> > > + struct binding_ctx_out *b_ctx_out);
> > > static struct binding_lport *binding_lport_create(
> > > const struct sbrec_port_binding *,
> > > @@ -1469,8 +1495,7 @@ consider_vif_lport_(const struct sbrec_port_binding
> > > *pb,
> > > bool can_bind,
> > > struct binding_ctx_in *b_ctx_in,
> > > struct binding_ctx_out *b_ctx_out,
> > > - struct binding_lport *b_lport,
> > > - struct hmap *qos_map)
> > > + struct binding_lport *b_lport)
> > > {
> > > bool lbinding_set = b_lport && is_lbinding_set(b_lport->lbinding);
> > > @@ -1502,8 +1527,8 @@ consider_vif_lport_(const struct sbrec_port_binding
> > > *pb,
> > > tracked_datapath_lport_add(pb, TRACKED_RESOURCE_UPDATED,
> > >
> > > b_ctx_out->tracked_dp_bindings);
> > > }
> > > - if (b_lport->lbinding->iface && qos_map &&
> > > b_ctx_in->ovs_idl_txn) {
> > > - get_qos_params(pb, qos_map);
> > > + if (b_lport->lbinding->iface && b_ctx_in->ovs_idl_txn) {
> > > + get_qos_queue(pb, b_ctx_out->qos_map);
> > > }
> > > } else {
> > > /* We could, but can't claim the lport. */
> > > @@ -1537,8 +1562,7 @@ static bool
> > > consider_vif_lport(const struct sbrec_port_binding *pb,
> > > struct binding_ctx_in *b_ctx_in,
> > > struct binding_ctx_out *b_ctx_out,
> > > - struct local_binding *lbinding,
> > > - struct hmap *qos_map)
> > > + struct local_binding *lbinding)
> > > {
> > > bool can_bind =
> > > lport_can_bind_on_this_chassis(b_ctx_in->chassis_rec, pb);
> > > @@ -1570,15 +1594,13 @@ consider_vif_lport(const struct
> > > sbrec_port_binding *pb,
> > > }
> > > }
> > > - return consider_vif_lport_(pb, can_bind, b_ctx_in, b_ctx_out,
> > > - b_lport, qos_map);
> > > + return consider_vif_lport_(pb, can_bind, b_ctx_in, b_ctx_out,
> > > b_lport);
> > > }
> > > static bool
> > > consider_container_lport(const struct sbrec_port_binding *pb,
> > > struct binding_ctx_in *b_ctx_in,
> > > - struct binding_ctx_out *b_ctx_out,
> > > - struct hmap *qos_map)
> > > + struct binding_ctx_out *b_ctx_out)
> > > {
> > > struct shash *local_bindings = &b_ctx_out->lbinding_data->bindings;
> > > struct local_binding *parent_lbinding;
> > > @@ -1627,7 +1649,7 @@ consider_container_lport(const struct
> > > sbrec_port_binding *pb,
> > > /* Its possible that the parent lport is not considered yet.
> > > * So call consider_vif_lport() to process it first. */
> > > consider_vif_lport(parent_pb, b_ctx_in, b_ctx_out,
> > > - parent_lbinding, qos_map);
> > > + parent_lbinding);
> > > parent_b_lport = binding_lport_find(binding_lports,
> > > pb->parent_port);
> > > } else {
> > > @@ -1659,14 +1681,13 @@ consider_container_lport(const struct
> > > sbrec_port_binding *pb,
> > > bool can_bind =
> > > lport_can_bind_on_this_chassis(b_ctx_in->chassis_rec, pb);
> > > return consider_vif_lport_(pb, can_bind, b_ctx_in, b_ctx_out,
> > > - container_b_lport, qos_map);
> > > + container_b_lport);
> > > }
> > > static bool
> > > consider_virtual_lport(const struct sbrec_port_binding *pb,
> > > struct binding_ctx_in *b_ctx_in,
> > > - struct binding_ctx_out *b_ctx_out,
> > > - struct hmap *qos_map)
> > > + struct binding_ctx_out *b_ctx_out)
> > > {
> > > struct shash *local_bindings = &b_ctx_out->lbinding_data->bindings;
> > > struct local_binding *parent_lbinding =
> > > @@ -1694,7 +1715,7 @@ consider_virtual_lport(const struct
> > > sbrec_port_binding *pb,
> > > /* Its possible that the parent lport is not considered
> > > yet.
> > > * So call consider_vif_lport() to process it first. */
> > > consider_vif_lport(parent_pb, b_ctx_in, b_ctx_out,
> > > - parent_lbinding, qos_map);
> > > + parent_lbinding);
> > > }
> > > }
> > > @@ -1708,7 +1729,7 @@ consider_virtual_lport(const struct
> > > sbrec_port_binding *pb,
> > > }
> > > if (!consider_vif_lport_(pb, true, b_ctx_in, b_ctx_out,
> > > - virtual_b_lport, qos_map)) {
> > > + virtual_b_lport)) {
> > > return false;
> > > }
> > > @@ -1826,15 +1847,14 @@ consider_l3gw_lport(const struct
> > > sbrec_port_binding *pb,
> > > static void
> > > consider_localnet_lport(const struct sbrec_port_binding *pb,
> > > struct binding_ctx_in *b_ctx_in,
> > > - struct binding_ctx_out *b_ctx_out,
> > > - struct hmap *qos_map)
> > > + struct binding_ctx_out *b_ctx_out)
> > > {
> > > /* Add all localnet ports to local_ifaces so that we allocate ct
> > > zones
> > > * for them. */
> > > update_local_lports(pb->logical_port, b_ctx_out);
> > > - if (qos_map && b_ctx_in->ovs_idl_txn) {
> > > - get_qos_params(pb, qos_map);
> > > + if (b_ctx_in->ovs_idl_txn) {
> > > + get_qos_queue(pb, b_ctx_out->qos_map);
> > > }
> > > update_related_lport(pb, b_ctx_out);
> > > @@ -1950,16 +1970,11 @@ binding_run(struct binding_ctx_in *b_ctx_in,
> > > struct binding_ctx_out *b_ctx_out)
> > > }
> > > struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings);
> > > - struct hmap qos_map;
> > > - hmap_init(&qos_map);
> > > if (b_ctx_in->br_int) {
> > > build_local_bindings(b_ctx_in, b_ctx_out);
> > > }
> > > - struct hmap *qos_map_ptr =
> > > - !sset_is_empty(b_ctx_out->egress_ifaces) ? &qos_map : NULL;
> > > -
> > > struct ovs_list localnet_lports =
> > > OVS_LIST_INITIALIZER(&localnet_lports);
> > > struct ovs_list external_lports =
> > > OVS_LIST_INITIALIZER(&external_lports);
> > > struct ovs_list multichassis_ports = OVS_LIST_INITIALIZER(
> > > @@ -1996,7 +2011,7 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct
> > > binding_ctx_out *b_ctx_out)
> > > break;
> > > case LP_VIF:
> > > - consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL,
> > > qos_map_ptr);
> > > + consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL);
> > > if (pb->additional_chassis) {
> > > struct lport *multichassis_lport = xmalloc(
> > > sizeof *multichassis_lport);
> > > @@ -2007,11 +2022,11 @@ binding_run(struct binding_ctx_in *b_ctx_in,
> > > struct binding_ctx_out *b_ctx_out)
> > > break;
> > > case LP_CONTAINER:
> > > - consider_container_lport(pb, b_ctx_in, b_ctx_out,
> > > qos_map_ptr);
> > > + consider_container_lport(pb, b_ctx_in, b_ctx_out);
> > > break;
> > > case LP_VIRTUAL:
> > > - consider_virtual_lport(pb, b_ctx_in, b_ctx_out, qos_map_ptr);
> > > + consider_virtual_lport(pb, b_ctx_in, b_ctx_out);
> > > break;
> > > case LP_L2GATEWAY:
> > > @@ -2034,7 +2049,7 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct
> > > binding_ctx_out *b_ctx_out)
> > > break;
> > > case LP_LOCALNET: {
> > > - consider_localnet_lport(pb, b_ctx_in, b_ctx_out, &qos_map);
> > > + consider_localnet_lport(pb, b_ctx_in, b_ctx_out);
> > > struct lport *lnet_lport = xmalloc(sizeof *lnet_lport);
> > > lnet_lport->pb = pb;
> > > ovs_list_push_back(&localnet_lports,
> > > &lnet_lport->list_node);
> > > @@ -2096,12 +2111,10 @@ binding_run(struct binding_ctx_in *b_ctx_in,
> > > struct binding_ctx_out *b_ctx_out)
> > > b_ctx_in->qos_table, b_ctx_out->egress_ifaces))
> > > {
> > > const char *entry;
> > > SSET_FOR_EACH (entry, b_ctx_out->egress_ifaces) {
> > > - setup_qos(entry, &qos_map);
> > > + setup_qos(entry, b_ctx_out->qos_map);
> > > }
> > > }
> > > - destroy_qos_map(&qos_map);
> > > -
> > > cleanup_claimed_port_timestamps();
> > > }
> > > @@ -2190,8 +2203,7 @@ static bool
> > > consider_iface_claim(const struct ovsrec_interface *iface_rec,
> > > const char *iface_id,
> > > struct binding_ctx_in *b_ctx_in,
> > > - struct binding_ctx_out *b_ctx_out,
> > > - struct hmap *qos_map)
> > > + struct binding_ctx_out *b_ctx_out)
> > > {
> > > update_local_lports(iface_id, b_ctx_out);
> > > smap_replace(b_ctx_out->local_iface_ids, iface_rec->name, iface_id);
> > > @@ -2239,7 +2251,7 @@ consider_iface_claim(const struct ovsrec_interface
> > > *iface_rec,
> > > }
> > > if (lport_type == LP_VIF &&
> > > - !consider_vif_lport(pb, b_ctx_in, b_ctx_out, lbinding, qos_map))
> > > {
> > > + !consider_vif_lport(pb, b_ctx_in, b_ctx_out, lbinding)) {
> > > return false;
> > > }
> > > @@ -2250,8 +2262,7 @@ consider_iface_claim(const struct ovsrec_interface
> > > *iface_rec,
> > > * claim the container lbindings. */
> > > LIST_FOR_EACH (b_lport, list_node, &lbinding->binding_lports) {
> > > if (b_lport->type == LP_CONTAINER) {
> > > - if (!consider_container_lport(b_lport->pb, b_ctx_in,
> > > b_ctx_out,
> > > - qos_map)) {
> > > + if (!consider_container_lport(b_lport->pb, b_ctx_in,
> > > b_ctx_out)) {
> > > return false;
> > > }
> > > }
> > > @@ -2538,10 +2549,6 @@ binding_handle_ovs_interface_changes(struct
> > > binding_ctx_in *b_ctx_in,
> > > return false;
> > > }
> > > - struct hmap qos_map = HMAP_INITIALIZER(&qos_map);
> > > - struct hmap *qos_map_ptr =
> > > - sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map;
> > > -
> > > /*
> > > * We consider an OVS interface for claiming if the following
> > > * 2 conditions are met:
> > > @@ -2570,24 +2577,22 @@ binding_handle_ovs_interface_changes(struct
> > > binding_ctx_in *b_ctx_in,
> > > if (iface_id && ofport > 0 &&
> > > is_iface_in_int_bridge(iface_rec, b_ctx_in->br_int)) {
> > > handled = consider_iface_claim(iface_rec, iface_id,
> > > b_ctx_in,
> > > - b_ctx_out, qos_map_ptr);
> > > + b_ctx_out);
> > > if (!handled) {
> > > break;
> > > }
> > > }
> > > }
> > > - if (handled && qos_map_ptr && set_noop_qos(b_ctx_in->ovs_idl_txn,
> > > - b_ctx_in->port_table,
> > > - b_ctx_in->qos_table,
> > > -
> > > b_ctx_out->egress_ifaces)) {
> > > + if (handled && set_noop_qos(b_ctx_in->ovs_idl_txn,
> > > b_ctx_in->port_table,
> > > + b_ctx_in->qos_table,
> > > + b_ctx_out->egress_ifaces)) {
> > > const char *entry;
> > > SSET_FOR_EACH (entry, b_ctx_out->egress_ifaces) {
> > > - setup_qos(entry, &qos_map);
> > > + setup_qos(entry, b_ctx_out->qos_map);
> > > }
> > > }
> > > - destroy_qos_map(&qos_map);
> > > return handled;
> > > }
> > > @@ -2686,18 +2691,17 @@ static bool
> > > handle_updated_vif_lport(const struct sbrec_port_binding *pb,
> > > enum en_lport_type lport_type,
> > > struct binding_ctx_in *b_ctx_in,
> > > - struct binding_ctx_out *b_ctx_out,
> > > - struct hmap *qos_map)
> > > + struct binding_ctx_out *b_ctx_out)
> > > {
> > > bool claimed = (pb->chassis == b_ctx_in->chassis_rec);
> > > bool handled = true;
> > > if (lport_type == LP_VIRTUAL) {
> > > - handled = consider_virtual_lport(pb, b_ctx_in, b_ctx_out,
> > > qos_map);
> > > + handled = consider_virtual_lport(pb, b_ctx_in, b_ctx_out);
> > > } else if (lport_type == LP_CONTAINER) {
> > > - handled = consider_container_lport(pb, b_ctx_in, b_ctx_out,
> > > qos_map);
> > > + handled = consider_container_lport(pb, b_ctx_in, b_ctx_out);
> > > } else {
> > > - handled = consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL,
> > > qos_map);
> > > + handled = consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL);
> > > }
> > > if (!handled) {
> > > @@ -2727,7 +2731,7 @@ handle_updated_vif_lport(const struct
> > > sbrec_port_binding *pb,
> > > LIST_FOR_EACH (b_lport, list_node, &lbinding->binding_lports) {
> > > if (b_lport->type == LP_CONTAINER) {
> > > handled = consider_container_lport(b_lport->pb, b_ctx_in,
> > > - b_ctx_out, qos_map);
> > > + b_ctx_out);
> > > if (!handled) {
> > > return false;
> > > }
> > > @@ -2792,8 +2796,7 @@ consider_patch_port_for_local_datapaths(const
> > > struct sbrec_port_binding *pb,
> > > static bool
> > > handle_updated_port(struct binding_ctx_in *b_ctx_in,
> > > struct binding_ctx_out *b_ctx_out,
> > > - const struct sbrec_port_binding *pb,
> > > - struct hmap *qos_map_ptr)
> > > + const struct sbrec_port_binding *pb)
> > > {
> > > update_active_pb_ras_pd(pb, b_ctx_out->local_active_ports_ipv6_pd,
> > > "ipv6_prefix_delegation");
> > > @@ -2815,7 +2818,7 @@ handle_updated_port(struct binding_ctx_in *b_ctx_in,
> > > if (b_lport->lbinding) {
> > > if (!local_binding_handle_stale_binding_lports(
> > > - b_lport->lbinding, b_ctx_in, b_ctx_out,
> > > qos_map_ptr)) {
> > > + b_lport->lbinding, b_ctx_in, b_ctx_out)) {
> > > return false;
> > > }
> > > }
> > > @@ -2829,7 +2832,7 @@ handle_updated_port(struct binding_ctx_in *b_ctx_in,
> > > case LP_VIRTUAL:
> > > update_ld_multichassis_ports(pb, b_ctx_out->local_datapaths);
> > > handled = handle_updated_vif_lport(pb, lport_type, b_ctx_in,
> > > - b_ctx_out, qos_map_ptr);
> > > + b_ctx_out);
> > > break;
> > > case LP_LOCALPORT:
> > > @@ -2889,7 +2892,7 @@ handle_updated_port(struct binding_ctx_in *b_ctx_in,
> > > break;
> > > case LP_LOCALNET: {
> > > - consider_localnet_lport(pb, b_ctx_in, b_ctx_out, qos_map_ptr);
> > > + consider_localnet_lport(pb, b_ctx_in, b_ctx_out);
> > > struct shash bridge_mappings =
> > > SHASH_INITIALIZER(&bridge_mappings);
> > > @@ -3029,10 +3032,6 @@ delete_done:
> > > return false;
> > > }
> > > - struct hmap qos_map = HMAP_INITIALIZER(&qos_map);
> > > - struct hmap *qos_map_ptr =
> > > - sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map;
> > > -
> > > SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb,
> > >
> > > b_ctx_in->port_binding_table) {
> > > /* Loop to handle create and update changes only. */
> > > @@ -3044,7 +3043,7 @@ delete_done:
> > > update_ld_peers(pb, b_ctx_out->local_datapaths);
> > > }
> > > - handled = handle_updated_port(b_ctx_in, b_ctx_out, pb,
> > > qos_map_ptr);
> > > + handled = handle_updated_port(b_ctx_in, b_ctx_out, pb);
> > > if (!handled) {
> > > break;
> > > }
> > > @@ -3061,7 +3060,7 @@ delete_done:
> > > sset_find_and_delete(b_ctx_out->postponed_ports, port_name);
> > > continue;
> > > }
> > > - handled = handle_updated_port(b_ctx_in, b_ctx_out, pb,
> > > qos_map_ptr);
> > > + handled = handle_updated_port(b_ctx_in, b_ctx_out, pb);
> > > if (!handled) {
> > > break;
> > > }
> > > @@ -3107,17 +3106,15 @@ delete_done:
> > > shash_destroy(&bridge_mappings);
> > > }
> > > - if (handled && qos_map_ptr && set_noop_qos(b_ctx_in->ovs_idl_txn,
> > > - b_ctx_in->port_table,
> > > - b_ctx_in->qos_table,
> > > -
> > > b_ctx_out->egress_ifaces)) {
> > > + if (handled && set_noop_qos(b_ctx_in->ovs_idl_txn,
> > > b_ctx_in->port_table,
> > > + b_ctx_in->qos_table,
> > > + b_ctx_out->egress_ifaces)) {
> > > const char *entry;
> > > SSET_FOR_EACH (entry, b_ctx_out->egress_ifaces) {
> > > - setup_qos(entry, &qos_map);
> > > + setup_qos(entry, b_ctx_out->qos_map);
> > > }
> > > }
> > > - destroy_qos_map(&qos_map);
> > > return handled;
> > > }
> > > @@ -3265,8 +3262,7 @@ local_binding_add_lport(struct shash
> > > *binding_lports,
> > > static bool
> > > local_binding_handle_stale_binding_lports(struct local_binding
> > > *lbinding,
> > > struct binding_ctx_in
> > > *b_ctx_in,
> > > - struct binding_ctx_out
> > > *b_ctx_out,
> > > - struct hmap *qos_map)
> > > + struct binding_ctx_out
> > > *b_ctx_out)
> > > {
> > > /* Check if this lbinding has a primary binding_lport or
> > > * localport binding_lport or not. */
> > > @@ -3288,14 +3284,15 @@ local_binding_handle_stale_binding_lports(struct
> > > local_binding *lbinding,
> > > pb = b_lport->pb;
> > > binding_lport_delete(&b_ctx_out->lbinding_data->lports,
> > > b_lport);
> > > - handled = consider_virtual_lport(pb, b_ctx_in, b_ctx_out,
> > > qos_map);
> > > + handled = consider_virtual_lport(pb, b_ctx_in, b_ctx_out);
> > > } else if (b_lport->type == LP_CONTAINER &&
> > > pb_lport_type == LP_CONTAINER) {
> > > /* For container lport, binding_lport is preserved so that
> > > when
> > > * the parent port is created, it can be considered.
> > > * consider_container_lport() creates the binding_lport for
> > > the parent
> > > * port (with iface set to NULL). */
> > > - handled = consider_container_lport(b_lport->pb, b_ctx_in,
> > > b_ctx_out, qos_map);
> > > + handled = consider_container_lport(b_lport->pb, b_ctx_in,
> > > + b_ctx_out);
> > > } else {
> > > /* This can happen when the lport type changes from one type
> > > * to another. Eg. from normal lport to external. Release
> > > the
> > > diff --git a/controller/binding.h b/controller/binding.h
> > > index 5b73c6a4b..87ee7b540 100644
> > > --- a/controller/binding.h
> > > +++ b/controller/binding.h
> > > @@ -92,6 +92,7 @@ struct binding_ctx_out {
> > > bool non_vif_ports_changed;
> > > struct sset *egress_ifaces;
> > > + struct hmap *qos_map;
> > > /* smap of OVS interface name as key and
> > > * OVS interface external_ids:iface-id as value. */
> > > struct smap *local_iface_ids;
> > > @@ -260,4 +261,6 @@ void binding_wait(void);
> > > /* Clean up module state. */
> > > void binding_destroy(void);
> > > +void destroy_qos_map(struct hmap *);
> > > +
> > > #endif /* controller/binding.h */
> > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > > index de90025f0..ee0e495ba 100644
> > > --- a/controller/ovn-controller.c
> > > +++ b/controller/ovn-controller.c
> > > @@ -1341,6 +1341,7 @@ struct ed_type_runtime_data {
> > > /* runtime data engine private data. */
> > > struct sset egress_ifaces;
> > > + struct hmap qos_map;
> > > struct smap local_iface_ids;
> > > /* Tracked data. See below for more details and comments. */
> > > @@ -1400,7 +1401,7 @@ struct ed_type_runtime_data {
> > > *
> > > ---------------------------------------------------------------------
> > > * | local_iface_ids | This is used internally within the runtime data
> > > |
> > > * | egress_ifaces | engine (used only in binding.c) and hence there
> > > |
> > > - * | | there is no need to track.
> > > |
> > > + * | qos_map | there is no need to track.
> > > |
> > > *
> > > ---------------------------------------------------------------------
> > > * | | Active tunnels is built in the
> > > |
> > > * | | bfd_calculate_active_tunnels() for the tunnel
> > > |
> > > @@ -1437,6 +1438,7 @@ en_runtime_data_init(struct engine_node *node
> > > OVS_UNUSED,
> > > related_lports_init(&data->related_lports);
> > > sset_init(&data->active_tunnels);
> > > sset_init(&data->egress_ifaces);
> > > + hmap_init(&data->qos_map);
> > > smap_init(&data->local_iface_ids);
> > > local_binding_data_init(&data->lbinding_data);
> > > shash_init(&data->local_active_ports_ipv6_pd);
> > > @@ -1457,6 +1459,7 @@ en_runtime_data_cleanup(void *data)
> > > related_lports_destroy(&rt_data->related_lports);
> > > sset_destroy(&rt_data->active_tunnels);
> > > sset_destroy(&rt_data->egress_ifaces);
> > > + destroy_qos_map(&rt_data->qos_map);
> > > smap_destroy(&rt_data->local_iface_ids);
> > > local_datapaths_destroy(&rt_data->local_datapaths);
> > > shash_destroy(&rt_data->local_active_ports_ipv6_pd);
> > > @@ -1545,6 +1548,7 @@ init_binding_ctx(struct engine_node *node,
> > > b_ctx_out->related_lports_changed = false;
> > > b_ctx_out->non_vif_ports_changed = false;
> > > b_ctx_out->egress_ifaces = &rt_data->egress_ifaces;
> > > + b_ctx_out->qos_map = &rt_data->qos_map;
> > > b_ctx_out->lbinding_data = &rt_data->lbinding_data;
> > > b_ctx_out->local_iface_ids = &rt_data->local_iface_ids;
> > > b_ctx_out->postponed_ports = rt_data->postponed_ports;
> > > @@ -1575,12 +1579,14 @@ en_runtime_data_run(struct engine_node *node,
> > > void *data)
> > > related_lports_destroy(&rt_data->related_lports);
> > > sset_destroy(active_tunnels);
> > > sset_destroy(&rt_data->egress_ifaces);
> > > + destroy_qos_map(&rt_data->qos_map);
> > > smap_destroy(&rt_data->local_iface_ids);
> > > hmap_init(local_datapaths);
> > > sset_init(local_lports);
> > > related_lports_init(&rt_data->related_lports);
> > > sset_init(active_tunnels);
> > > sset_init(&rt_data->egress_ifaces);
> > > + hmap_init(&rt_data->qos_map);
> > > smap_init(&rt_data->local_iface_ids);
> > > local_binding_data_init(&rt_data->lbinding_data);
> > > }
> >
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
