On 11/29/24 12:36, Lorenzo Bianconi wrote:
> Set HTB Qdisc scheduler root max-rate according to the reported
> link-speed if it is available (use default value if the link_speed is
> not reported by OvS). This patch allows more precise shaping when the
> NIC speed is lower than 34Gbps (current default value) and even take into
> account cases where NIC capacity is greater than 34 Gbps.
> However, rely on default value for veth and tap interfaces since the
> reported link speed is not accurate in this case.
> 
> Signed-off-by: Lorenzo Bianconi <[email protected]>
> ---
> Changes since v1:
> - use default max-rate value for veth and tap interfaces
> ---
>  controller/binding.c        | 20 ++++++++++++++++----
>  controller/ovn-controller.c |  1 +
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/controller/binding.c b/controller/binding.c
> index 117d0ff5d..7371ac433 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -244,6 +244,7 @@ add_or_del_qos_port(const char *ovn_port, bool add)
>  static bool
>  add_ovs_qos_table_entry(struct ovsdb_idl_txn *ovs_idl_txn,
>                          const struct ovsrec_port *port,
> +                        const struct ovsrec_interface *iface,
>                          unsigned long long min_rate,
>                          unsigned long long max_rate,
>                          unsigned long long burst,
> @@ -262,7 +263,18 @@ add_ovs_qos_table_entry(struct ovsdb_idl_txn 
> *ovs_idl_txn,
>          qos = ovsrec_qos_insert(ovs_idl_txn);
>          ovsrec_qos_set_type(qos, OVN_QOS_TYPE);
>          ovsrec_port_set_qos(port, qos);
> -        smap_add_format(&other_config, "max-rate", "%lld", OVN_QOS_MAX_RATE);

I'd add an empty line here.  May probably be added on commit.



> +        const char *drv_name = smap_get_def(&iface->status, "driver_name", 
> "");
> +        /* Link speed for virtual interfaces (e.g. veth or tap is inaccurate
> +         * so use default value for them while rely on link speed for real
> +         * NICs. */

The ')' and a comma before 'so' are missing.

I assume, comments above can be fixed on commit.

With that:
Acked-by: Ilya Maximets <[email protected]>

> +        if (!strcmp(drv_name, "veth") || !strcmp(drv_name, "tap") ||
> +            !iface->n_link_speed) {
> +            smap_add_format(&other_config, "max-rate", "%lld",
> +                            OVN_QOS_MAX_RATE);
> +        } else {
> +            smap_add_format(&other_config, "max-rate", "%lld",
> +                            (long long int) iface->link_speed[0]);
> +        }
>          ovsrec_qos_set_other_config(qos, &other_config);
>          smap_clear(&other_config);
>  
> @@ -391,9 +403,9 @@ configure_qos(const struct sbrec_port_binding *pb,
>          }
>          if (iface) {
>              /* Add new QoS entries. */
> -            if (add_ovs_qos_table_entry(ovs_idl_txn, port, min_rate,
> -                                    max_rate, burst, queue_id,
> -                                    pb->logical_port)) {
> +            if (add_ovs_qos_table_entry(ovs_idl_txn, port, iface,
> +                                        min_rate, max_rate, burst,
> +                                        queue_id, pb->logical_port)) {
>                  if (!q) {
>                      q = xzalloc(sizeof *q);
>                      hmap_insert(qos_map, &q->node, hash);
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 187f600b1..f988b233e 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -830,6 +830,7 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>      ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_options);
>      ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_ofport);
>      ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_external_ids);
> +    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_link_speed);
>      ovsdb_idl_track_add_column(ovs_idl, &ovsrec_port_col_name);
>      ovsdb_idl_track_add_column(ovs_idl, &ovsrec_port_col_interfaces);
>      ovsdb_idl_track_add_column(ovs_idl, &ovsrec_port_col_external_ids);

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to