Hi Lorenzo,
The top line of the commit message says that qos_physical_network is
configured in the other_config column, but it appears to actually be in
the options column.
I think this change should include a test in ovn-northd.at. It should
ensure that qos_network_name is set on port_bindings when we expect, and
that it is not set on port_bindings when we expect.
I have one more finding below.
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 | 4 ++++
northd/northd.c | 8 ++++++++
ovn-sb.xml | 5 +++++
3 files changed, 17 insertions(+)
diff --git a/controller/binding.c b/controller/binding.c
index ad19a4092..6dc5a1645 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. */
@@ -197,6 +200,7 @@ get_qos_queue(const struct sbrec_port_binding *pb, struct
hmap *queue_map)
if (!q) {
q = xzalloc(sizeof *q);
hmap_insert(queue_map, &q->node, hash);
+ q->network = xstrdup(network);
q->port = xstrdup(pb->logical_port);
q->queue_id = queue_id;
}
diff --git a/northd/northd.c b/northd/northd.c
index 25df0957e..53217347d 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -3505,7 +3505,15 @@ 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");
+ smap_add(&options, "qos_physical_network",
+ physical_network);
What happens if the localnet port does not have options:network_name set?
+ }
smap_add_format(&options,
"qdisc_queue_id", "%d", queue_id);
}
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 52705f2be..6a34b75db 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -3666,6 +3666,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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
