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.
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
