[ovs-dev,v2,0/3] OVN: add Controller Events

There are situations where arrival of certain types of traffic into OVS
does not warrant a "typical" action, such as output to a specific port
or dropping. Rather, the decision about what to do needs to be left to a
CMS.

The series here introduces a new table, Controller_Event, for this
purpose. Traffic into OVS can raise a 'controller' event that results in
a Controller_Event being written to the southbound database. The
intention is for a CMS to see the events and take some sort of action.
When the CMS has seen the event and taken appropriate action, then it
can remove the correponding row in Controller_Event table.

Controller events are only added to the southbound database if the CMS
enable the feature in the NB nb_global table setting options:controller_event
to true.

This series introduces a new event, empty_lb_backends. This event is
raised if a received packet is destined for a load balancer VIP that has
no configured backend destinations. For this event, the event info
includes the load balancer VIP, the load balancer UUID, and the
transport protocol.

The use case for this particular event is for the CMS to supply backend
resources to handle this traffic. For example, in Openshift, this event
can be used to spin up new containers to handle the incoming traffic.

Changes since v1:
- improve documentation
- fix code style
- moved event_to_string and string_to_event routines in
  ovn/lib/logical-fields.c
- set GC timeout to 10s
- removed unnecessary ip parameter in 'empty_lb_backends' logical flow
  definition
- substituted ovs_assert() with a warning log in
  pinctrl_handle_empty_lb_backends_opts()
- fixed commit message of patch 2/3
- remove 'handled' column in Controller_Event table

Changes since RFCv2:
- introduce event sequence number
- improve documentation

Changes since RFCv1:
- added garbage collector for event hash table
- rename send_event in trigger_event
- modify event_type from int to string in trigger_event action
- added chassis column in Controller_Event as weak reference to
  Chassis table
- added monitoring to 'local' rows in Controller_Event table
- fix typos

Lorenzo Bianconi (3):
  OVN: introduce Controller_Event table
  OVN: introduce trigger_event() action
  OVN: use trigger_event action to report 'empty_lb_rule' events

 include/ovn/actions.h           |  18 ++-
 include/ovn/logical-fields.h    |   7 +
 ovn/controller/lflow.c          |  26 +++-
 ovn/controller/ovn-controller.c |  10 ++
 ovn/controller/pinctrl.c        | 265 ++++++++++++++++++++++++++++++++
 ovn/controller/pinctrl.h        |   2 +
 ovn/lib/actions.c               | 176 +++++++++++++++++++++
 ovn/lib/logical-fields.c        |  21 +++
 ovn/lib/ovn-l7.h                |  46 ++++++
 ovn/northd/ovn-northd.c         |  33 ++++
 ovn/ovn-nb.xml                  |  11 ++
 ovn/ovn-sb.ovsschema            |  20 ++-
 ovn/ovn-sb.xml                  |  61 ++++++++
 ovn/utilities/ovn-trace.c       |   3 +
 tests/ovn.at                    |  75 +++++++++
 tests/test-ovn.c                |  11 +-
 16 files changed, 776 insertions(+), 9 deletions(-)

On Thu, Jul 11, 2019 at 05:48:42PM +0200, Lorenzo Bianconi wrote:
> There are situations where arrival of certain types of traffic into OVS
> does not warrant a "typical" action, such as output to a specific port
> or dropping. Rather, the decision about what to do needs to be left to a
> CMS.
> 
> The series here introduces a new table, Controller_Event, for this
> purpose. Traffic into OVS can raise a 'controller' event that results in
> a Controller_Event being written to the southbound database. The
> intention is for a CMS to see the events and take some sort of action.
> When the CMS has seen the event and taken appropriate action, then it
> can remove the correponding row in Controller_Event table.

The series seems OK to me.  However: do we need to do something to
rate-limit sending messages to ovn-controller?  It seems like a flood of
packets could trigger a flood of OpenFlow messages to ovn-controller,
which could result in poor service.

I suspect the answer is "yes", but maybe it is "yes, but it's OK for
2.12 and we can add rate-limiting in the next release".

What do you think?

Thanks,

Ben.

> On Thu, Jul 11, 2019 at 05:48:42PM +0200, Lorenzo Bianconi wrote:
> > There are situations where arrival of certain types of traffic into OVS
> > does not warrant a "typical" action, such as output to a specific port
> > or dropping. Rather, the decision about what to do needs to be left to a
> > CMS.
> > 
> > The series here introduces a new table, Controller_Event, for this
> > purpose. Traffic into OVS can raise a 'controller' event that results in
> > a Controller_Event being written to the southbound database. The
> > intention is for a CMS to see the events and take some sort of action.
> > When the CMS has seen the event and taken appropriate action, then it
> > can remove the correponding row in Controller_Event table.
> 
> The series seems OK to me.  However: do we need to do something to
> rate-limit sending messages to ovn-controller?  It seems like a flood of
> packets could trigger a flood of OpenFlow messages to ovn-controller,
> which could result in poor service.
> 
> I suspect the answer is "yes", but maybe it is "yes, but it's OK for
> 2.12 and we can add rate-limiting in the next release".
> 
> What do you think?

Hi Ben,

in pinctrl_handle_empty_lb_backends_opts() I added a limit to event_table
size. If we overcame this limit, ovn-controller discards the received event
and it does not notify main thread. Do you think we need some more sophisticated?
If so, and if you agree, I think we can work on it after 2.12 release.

Regards,
Lorenzo

> 
> Thanks,
> 
> Ben.

On Fri, Jul 12, 2019 at 11:46 AM Lorenzo Bianconi <
lorenzo.bianconi@redhat.com> wrote:
>
> > On Thu, Jul 11, 2019 at 05:48:42PM +0200, Lorenzo Bianconi wrote:
> > > There are situations where arrival of certain types of traffic into
OVS
> > > does not warrant a "typical" action, such as output to a specific port
> > > or dropping. Rather, the decision about what to do needs to be left
to a
> > > CMS.
> > >
> > > The series here introduces a new table, Controller_Event, for this
> > > purpose. Traffic into OVS can raise a 'controller' event that results
in
> > > a Controller_Event being written to the southbound database. The
> > > intention is for a CMS to see the events and take some sort of action.
> > > When the CMS has seen the event and taken appropriate action, then it
> > > can remove the correponding row in Controller_Event table.
> >
> > The series seems OK to me.  However: do we need to do something to
> > rate-limit sending messages to ovn-controller?  It seems like a flood of
> > packets could trigger a flood of OpenFlow messages to ovn-controller,
> > which could result in poor service.
> >
> > I suspect the answer is "yes", but maybe it is "yes, but it's OK for
> > 2.12 and we can add rate-limiting in the next release".
> >
> > What do you think?
>
> Hi Ben,
>
> in pinctrl_handle_empty_lb_backends_opts() I added a limit to event_table
> size. If we overcame this limit, ovn-controller discards the received
event
> and it does not notify main thread. Do you think we need some more
sophisticated?
> If so, and if you agree, I think we can work on it after 2.12 release.
>
> Regards,
> Lorenzo
>
> >
> > Thanks,
> >
> > Ben.
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Hi Lorenzo,

I am worried about the flooding, too. Thanks for adding the limit for
event_table. It is good that it won't notify the main thread and update SB
DB continuously. However, pinctrl thread would still get overloaded if
controller messages keep flooding to ovn-controller. Would it be better to
use meter action in the flow to ratelimit the messages to ovn-controller,
something like what ACL log ratelimit does?

I think it is ok for the ratelimit improvement to be in next release, since
the feature is not enabled by default (i.e. controller_event opt in NB
global is not set).

Thanks,
Han

> On Fri, Jul 12, 2019 at 11:46 AM Lorenzo Bianconi <
> lorenzo.bianconi@redhat.com> wrote:
> >
> > > On Thu, Jul 11, 2019 at 05:48:42PM +0200, Lorenzo Bianconi wrote:
> > > > There are situations where arrival of certain types of traffic into
> OVS
> > > > does not warrant a "typical" action, such as output to a specific port
> > > > or dropping. Rather, the decision about what to do needs to be left
> to a
> > > > CMS.
> > > >
> > > > The series here introduces a new table, Controller_Event, for this
> > > > purpose. Traffic into OVS can raise a 'controller' event that results
> in
> > > > a Controller_Event being written to the southbound database. The
> > > > intention is for a CMS to see the events and take some sort of action.
> > > > When the CMS has seen the event and taken appropriate action, then it
> > > > can remove the correponding row in Controller_Event table.
> > >
> > > The series seems OK to me.  However: do we need to do something to
> > > rate-limit sending messages to ovn-controller?  It seems like a flood of
> > > packets could trigger a flood of OpenFlow messages to ovn-controller,
> > > which could result in poor service.
> > >
> > > I suspect the answer is "yes", but maybe it is "yes, but it's OK for
> > > 2.12 and we can add rate-limiting in the next release".
> > >
> > > What do you think?
> >
> > Hi Ben,
> >
> > in pinctrl_handle_empty_lb_backends_opts() I added a limit to event_table
> > size. If we overcame this limit, ovn-controller discards the received
> event
> > and it does not notify main thread. Do you think we need some more
> sophisticated?
> > If so, and if you agree, I think we can work on it after 2.12 release.
> >
> > Regards,
> > Lorenzo
> >
> > >
> > > Thanks,
> > >
> > > Ben.
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> Hi Lorenzo,
> 
> I am worried about the flooding, too. Thanks for adding the limit for
> event_table. It is good that it won't notify the main thread and update SB
> DB continuously. However, pinctrl thread would still get overloaded if
> controller messages keep flooding to ovn-controller. Would it be better to
> use meter action in the flow to ratelimit the messages to ovn-controller,
> something like what ACL log ratelimit does?
> 
> I think it is ok for the ratelimit improvement to be in next release, since
> the feature is not enabled by default (i.e. controller_event opt in NB
> global is not set).

Thx for the review Han, I will add the ratelimit improvement to my ToDo list :)

Regards,
Lorenzo

> 
> Thanks,
> Han

On Fri, Jul 12, 2019 at 12:36:39PM -0700, Han Zhou wrote:
> On Fri, Jul 12, 2019 at 11:46 AM Lorenzo Bianconi <
> lorenzo.bianconi@redhat.com> wrote:
> >
> > > On Thu, Jul 11, 2019 at 05:48:42PM +0200, Lorenzo Bianconi wrote:
> > > > There are situations where arrival of certain types of traffic into
> OVS
> > > > does not warrant a "typical" action, such as output to a specific port
> > > > or dropping. Rather, the decision about what to do needs to be left
> to a
> > > > CMS.
> > > >
> > > > The series here introduces a new table, Controller_Event, for this
> > > > purpose. Traffic into OVS can raise a 'controller' event that results
> in
> > > > a Controller_Event being written to the southbound database. The
> > > > intention is for a CMS to see the events and take some sort of action.
> > > > When the CMS has seen the event and taken appropriate action, then it
> > > > can remove the correponding row in Controller_Event table.
> > >
> > > The series seems OK to me.  However: do we need to do something to
> > > rate-limit sending messages to ovn-controller?  It seems like a flood of
> > > packets could trigger a flood of OpenFlow messages to ovn-controller,
> > > which could result in poor service.
> > >
> > > I suspect the answer is "yes", but maybe it is "yes, but it's OK for
> > > 2.12 and we can add rate-limiting in the next release".
> > >
> > > What do you think?
> >
> > Hi Ben,
> >
> > in pinctrl_handle_empty_lb_backends_opts() I added a limit to event_table
> > size. If we overcame this limit, ovn-controller discards the received
> event
> > and it does not notify main thread. Do you think we need some more
> sophisticated?
> > If so, and if you agree, I think we can work on it after 2.12 release.
> >
> > Regards,
> > Lorenzo
> >
> > >
> > > Thanks,
> > >
> > > Ben.
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> Hi Lorenzo,
> 
> I am worried about the flooding, too. Thanks for adding the limit for
> event_table. It is good that it won't notify the main thread and update SB
> DB continuously. However, pinctrl thread would still get overloaded if
> controller messages keep flooding to ovn-controller. Would it be better to
> use meter action in the flow to ratelimit the messages to ovn-controller,
> something like what ACL log ratelimit does?

That's the kind of overloading I was concerned about too.

> I think it is ok for the ratelimit improvement to be in next release, since
> the feature is not enabled by default (i.e. controller_event opt in NB
> global is not set).

Good point.  Let's consider this a to-do item then.

On Fri, Jul 12, 2019 at 09:50:04PM +0200, Lorenzo Bianconi wrote:
> > On Fri, Jul 12, 2019 at 11:46 AM Lorenzo Bianconi <
> > lorenzo.bianconi@redhat.com> wrote:
> > >
> > > > On Thu, Jul 11, 2019 at 05:48:42PM +0200, Lorenzo Bianconi wrote:
> > > > > There are situations where arrival of certain types of traffic into
> > OVS
> > > > > does not warrant a "typical" action, such as output to a specific port
> > > > > or dropping. Rather, the decision about what to do needs to be left
> > to a
> > > > > CMS.
> > > > >
> > > > > The series here introduces a new table, Controller_Event, for this
> > > > > purpose. Traffic into OVS can raise a 'controller' event that results
> > in
> > > > > a Controller_Event being written to the southbound database. The
> > > > > intention is for a CMS to see the events and take some sort of action.
> > > > > When the CMS has seen the event and taken appropriate action, then it
> > > > > can remove the correponding row in Controller_Event table.
> > > >
> > > > The series seems OK to me.  However: do we need to do something to
> > > > rate-limit sending messages to ovn-controller?  It seems like a flood of
> > > > packets could trigger a flood of OpenFlow messages to ovn-controller,
> > > > which could result in poor service.
> > > >
> > > > I suspect the answer is "yes", but maybe it is "yes, but it's OK for
> > > > 2.12 and we can add rate-limiting in the next release".
> > > >
> > > > What do you think?
> > >
> > > Hi Ben,
> > >
> > > in pinctrl_handle_empty_lb_backends_opts() I added a limit to event_table
> > > size. If we overcame this limit, ovn-controller discards the received
> > event
> > > and it does not notify main thread. Do you think we need some more
> > sophisticated?
> > > If so, and if you agree, I think we can work on it after 2.12 release.
> > >
> > > Regards,
> > > Lorenzo
> > >
> > > >
> > > > Thanks,
> > > >
> > > > Ben.
> > > _______________________________________________
> > > dev mailing list
> > > dev@openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > 
> > Hi Lorenzo,
> > 
> > I am worried about the flooding, too. Thanks for adding the limit for
> > event_table. It is good that it won't notify the main thread and update SB
> > DB continuously. However, pinctrl thread would still get overloaded if
> > controller messages keep flooding to ovn-controller. Would it be better to
> > use meter action in the flow to ratelimit the messages to ovn-controller,
> > something like what ACL log ratelimit does?
> > 
> > I think it is ok for the ratelimit improvement to be in next release, since
> > the feature is not enabled by default (i.e. controller_event opt in NB
> > global is not set).
> 
> Thx for the review Han, I will add the ratelimit improvement to my ToDo list :)

I applied this series to master.