> On Wed, May 17, 2023 at 5:02 AM Lorenzo Bianconi
> <[email protected]> wrote:
> >
> > Introduce qos_physical_network in port_binding config column in order to
> > indicate the name of the egress network name where traffic shaping will
> > be applied.
> > 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 | 8 ++++++++
> > northd/northd.c | 10 ++++++++++
> > ovn-sb.xml | 5 +++++
> > tests/ovn-northd.at | 22 ++++++++++++++++++++++
> > 4 files changed, 45 insertions(+)
> >
> > diff --git a/controller/binding.c b/controller/binding.c
> > index ad19a4092..91437dbb8 100644
> > --- a/controller/binding.c
> > +++ b/controller/binding.c
> > @@ -142,6 +142,7 @@ static void update_lport_tracking(const struct
> > sbrec_port_binding *pb,
> > struct qos_queue {
> > struct hmap_node node;
> >
> > + char *network;
> > char *port;
> >
> > uint32_t queue_id;
> > @@ -165,6 +166,7 @@ find_qos_queue(struct hmap *queue_map, uint32_t hash,
> > const char *port)
> > static void
> > qos_queue_erase_entry(struct qos_queue *q)
> > {
> > + free(q->network);
> > free(q->port);
> > free(q);
> > }
> > @@ -186,6 +188,7 @@ get_qos_queue(const struct sbrec_port_binding *pb,
> > struct hmap *queue_map)
> > uint32_t max_rate = smap_get_int(&pb->options, "qos_max_rate", 0);
> > uint32_t burst = smap_get_int(&pb->options, "qos_burst", 0);
> > uint32_t queue_id = smap_get_int(&pb->options, "qdisc_queue_id", 0);
> > + const char *network = smap_get(&pb->options, "qos_physical_network");
> >
> > if ((!min_rate && !max_rate && !burst) || !queue_id) {
> > /* Qos is not configured for this port. */
> > @@ -200,6 +203,11 @@ get_qos_queue(const struct sbrec_port_binding *pb,
> > struct hmap *queue_map)
> > q->port = xstrdup(pb->logical_port);
> > q->queue_id = queue_id;
> > }
> > +
> > + free(q->network);
>
> There is a potential double free error for the q->network.
>
> I tested it locally and could reproduce the crash. After freeing
> please set q->network to NULL.
ack, thx I will fix it.
Regards,
Lorenzo
>
> Numan
>
> > + if (network) {
> > + q->network = xstrdup(network);
> > + }
> > q->min_rate = min_rate;
> > q->max_rate = max_rate;
> > q->burst = burst;
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 7190cd18f..470f76809 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -3505,7 +3505,17 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn
> > *ovnsb_txn,
> > }
> >
> > smap_clone(&options, &op->nbsp->options);
> > +
> > if (queue_id) {
> > + if (op->od->n_localnet_ports) {
> > + struct ovn_port *port = op->od->localnet_ports[0];
> > + const char *physical_network = smap_get(
> > + &port->nbsp->options, "network_name");
> > + if (physical_network) {
> > + smap_add(&options, "qos_physical_network",
> > + physical_network);
> > + }
> > + }
> > smap_add_format(&options,
> > "qdisc_queue_id", "%d", queue_id);
> > }
> > diff --git a/ovn-sb.xml b/ovn-sb.xml
> > index ead9efcab..0988cb1f8 100644
> > --- a/ovn-sb.xml
> > +++ b/ovn-sb.xml
> > @@ -3691,6 +3691,11 @@ tcp.flags = RST;
> > interface, in bits.
> > </column>
> >
> > + <column name="options" key="qos_physical_network">
> > + If set, indicates the name of the egress network name where traffic
> > + shaping will be applied.
> > + </column>
> > +
> > <column name="options" key="qdisc_queue_id"
> > type='{"type": "integer", "minInteger": 1, "maxInteger":
> > 61440}'>
> > Indicates the queue number on the physical device. This is same as
> > the
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 1f6169b77..a9af0f76a 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -9064,3 +9064,25 @@ mac_binding_timestamp: true
> >
> > AT_CLEANUP
> > ])
> > +
> > +OVN_FOR_EACH_NORTHD_NO_HV([
> > +AT_SETUP([check OVN QoS])
> > +AT_KEYWORDS([OVN-QoS])
> > +ovn_start
> > +
> > +check ovn-nbctl ls-add ls
> > +check ovn-nbctl lsp-add ls public
> > +check ovn-nbctl lsp-set-type public localnet
> > +check ovn-nbctl lsp-set-addresses public unknown
> > +
> > +check_column "" sb:Port_Binding options logical_port=public
> > +
> > +check ovn-nbctl --wait=sb set Logical_Switch_Port public
> > options:qos_min_rate=200000
> > +AT_CHECK([fetch_column sb:Port_Binding options logical_port=public |grep
> > -q 'qos_min_rate=200000'])
> > +AT_CHECK([fetch_column sb:Port_Binding options logical_port=public |grep
> > -q 'qos_physical_network'],[1])
> > +
> > +check ovn-nbctl --wait=sb set Logical_Switch_Port public
> > options:qos_min_rate=200000 options:network_name=phys
> > +AT_CHECK([fetch_column sb:Port_Binding options logical_port=public |grep
> > -q 'qos_physical_network=phys'])
> > +
> > +AT_CLEANUP
> > +])
> > --
> > 2.40.1
> >
> > _______________________________________________
> > 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
