On Jun 20, Ales Musil wrote:
> On Tue, Jun 17, 2025 at 6:16 PM Lorenzo Bianconi via dev <
> ovs-dev@openvswitch.org> wrote:
>
> > According to the OvS documentation, setting interface type to an empty
> > string is equivalent of configuring the type column to system.
> > Fix OvS port selection in get_qos_egress_port_interface() taking into
> > account the CMS can configure the interface setting type to system.
> >
>
> Thank you Lorenzo,
>
> I have a few small commens down below.
>
>
> Missing Fixes: 1c99a50b270d ("northd: apply QoS rules on the localnet port
> related to LSP ports")
>
>
> > Reported-at: https://issues.redhat.com/browse/FDP-1472
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com>
> > ---
> >
>
> controller/binding.c | 18 +++++++++++++++---
> > 1 file changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/controller/binding.c b/controller/binding.c
> > index b2ecd58c1..91cd46e7e 100644
> > --- a/controller/binding.c
> > +++ b/controller/binding.c
> > @@ -191,6 +191,20 @@ destroy_qos_map(struct hmap *qos_map)
> > hmap_destroy(qos_map);
> > }
> >
> > +static bool is_qos_iface(const struct ovsrec_interface *iface)
> >
>
> nit:
> static bool
> is_qos_iface(..)
>
>
> > +{
> > + if (smap_get_bool(&iface->external_ids,
> > + "ovn-egress-iface", false)) {
> >
>
> nit: It can fit into one line.
>
>
> > + return true;
> > + }
> > +
> > + if (!strcmp(iface->type, "") || !strcmp(iface->type, "system")) {
> >
>
> nit: No need to use strcmp for empty string.
>
>
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> > +
> > static const struct ovsrec_interface *
> > get_qos_egress_port_interface(struct shash *bridge_mappings,
> > const struct ovsrec_port **pport,
> > @@ -211,9 +225,7 @@ get_qos_egress_port_interface(struct shash
> > *bridge_mappings,
> > continue;
> > }
> >
> > - if (smap_get_bool(&iface->external_ids,
> > - "ovn-egress-iface", false) ||
> > - !strcmp(iface->type, "")) {
> > + if (is_qos_iface(iface)) {
> > *pport = port;
> > return iface;
> > }
> > --
> > 2.49.0
> >
> > _______________________________________________
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> >
> I have addressed all of the above and merged it into main and backported
> all the way down to 24.03.
ack, thx Ales.
Regards,
Lorenzo
>
> Regards,
> Ales
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
