Bug 2139194

A possibility that came to mind was to add a "tag", for example,
SB.Port_Binding.options:monitor-tag.

We could add a new mode to ovn-controller to only monitor port bindings
that have a given (set of) tag(s).  This could be specified as an
external_id in the local OVS DB Open_vSwitch table.  Like this the
monitor condition is constant, regardless of how many OVS ports there
are.

The downside is that the CMS needs to explicitly request this mode to
be enabled on every chassis.  We need some "synchronization" and use
this mode in ovn-controller if and only if ovn-northd has been upgraded
to set this port binding option.

(In reply to Dumitru Ceara from comment #1)
> A possibility that came to mind was to add a "tag", for example,
> SB.Port_Binding.options:monitor-tag.
> 
> We could add a new mode to ovn-controller to only monitor port bindings
> that have a given (set of) tag(s).  This could be specified as an
> external_id in the local OVS DB Open_vSwitch table.  Like this the
> monitor condition is constant, regardless of how many OVS ports there
> are.
> 
> The downside is that the CMS needs to explicitly request this mode to
> be enabled on every chassis.  We need some "synchronization" and use
> this mode in ovn-controller if and only if ovn-northd has been upgraded
> to set this port binding option.

Wasn't one thought that we could use requested-chassis as that tag? eg, we could have a generic tag, but we could also have "field=value" where field=requested-chassis and value=chassis-name that the CMS would already be setting, and northd would already be propagating through.

(In reply to Dan Williams from comment #2)
> (In reply to Dumitru Ceara from comment #1)
> > A possibility that came to mind was to add a "tag", for example,
> > SB.Port_Binding.options:monitor-tag.
> > 
> > We could add a new mode to ovn-controller to only monitor port bindings
> > that have a given (set of) tag(s).  This could be specified as an
> > external_id in the local OVS DB Open_vSwitch table.  Like this the
> > monitor condition is constant, regardless of how many OVS ports there
> > are.
> > 
> > The downside is that the CMS needs to explicitly request this mode to
> > be enabled on every chassis.  We need some "synchronization" and use
> > this mode in ovn-controller if and only if ovn-northd has been upgraded
> > to set this port binding option.
> 
> Wasn't one thought that we could use requested-chassis as that tag? eg, we
> could have a generic tag, but we could also have "field=value" where
> field=requested-chassis and value=chassis-name that the CMS would already be
> setting, and northd would already be propagating through.

I actually mean: field=chassis (not requested-chassis)

This wouldn't require any additional northd or DB schema support, but isn't as flexible as an arbitrary "monitor-tag" of course.

(In reply to Dan Williams from comment #3)
> (In reply to Dan Williams from comment #2)
> > (In reply to Dumitru Ceara from comment #1)
> > > A possibility that came to mind was to add a "tag", for example,
> > > SB.Port_Binding.options:monitor-tag.
> > > 
> > > We could add a new mode to ovn-controller to only monitor port bindings
> > > that have a given (set of) tag(s).  This could be specified as an
> > > external_id in the local OVS DB Open_vSwitch table.  Like this the
> > > monitor condition is constant, regardless of how many OVS ports there
> > > are.
> > > 
> > > The downside is that the CMS needs to explicitly request this mode to
> > > be enabled on every chassis.  We need some "synchronization" and use
> > > this mode in ovn-controller if and only if ovn-northd has been upgraded
> > > to set this port binding option.
> > 
> > Wasn't one thought that we could use requested-chassis as that tag? eg, we
> > could have a generic tag, but we could also have "field=value" where
> > field=requested-chassis and value=chassis-name that the CMS would already be
> > setting, and northd would already be propagating through.
> 
> I actually mean: field=chassis (not requested-chassis)
> 

I'm not sure that would work.  ovn-controller sets the 'chassis' field
after it claims the logical port.  What should work is to change the
monitor condition to only request port bindings that have
Port_Binding.options.requested-chassis=<local-chassis>.

This still has the downside that we need to enable this mode in
ovn-controller through config.  That's because a CMS is not obliged to
set requested-chassis and ovn-controller should still claim ports that
don't have it set.

> This wouldn't require any additional northd or DB schema support, but isn't
> as flexible as an arbitrary "monitor-tag" of course.

After some offline brainstorming with Ilya it seems that there might be
a way to implement this without the need for config knobs or changes to
northd/schema:

The reason why we filter for port bindings whose name matches local OVS
iface-ids is because (in the general case) we need to first bind a
logical port locally before declaring the logical datapath it is
assigned to as "local".

However, once we know that a datapath is local, we should:
a. update the monitor condition to include all port bindings that point
   to a local datapath.
b. remove the iface-ids that correspond to ports bound to local
   datapaths from the monitor condition because "a" already covers these.

"a" is already happening today:
https://github.com/ovn-org/ovn/blob/40df933ce49946c69099a902b154d7bfee4358a3/controller/ovn-controller.c#L260

"b" should be straightforward to implement.

This means that it's possible that new OVS iface-ids end up being part
of the monitor condition briefly but likely we won't have many of these
at once.  Also, as and as soon as a port binding is created for them
they'll be removed from the condition.

Patch posted: https://patchwork.ozlabs.org/project/ovn/patch/20230606145706.175849-1-dceara@redhat.com/

V2 posted: https://patchwork.ozlabs.org/project/ovn/list/?series=360676&state=*

ovn23.09 fast-datapath-rhel-9 clone created at https://bugzilla.redhat.com/show_bug.cgi?id=2224400

Will be picked up downstream in ovn23.09 via bug 2224400.  Closing this instance as fixed in next release.