[ovs-dev] ovn: is it possible to add validation on acl match

[Ben Pfaff]

Another way to make it harder to send bad matches would be to construct
them in a structured way rather than as strings.

On Fri, May 20, 2016 at 09:50:43AM -0700, Ben Pfaff wrote:
> Would it be useful to provide a parser in Python for matches and
> actions?  Then most issues could be found before anything is sent to the
> database.
> 
> (At this point I'm brainstorming.)
> 
> On Fri, May 20, 2016 at 09:29:28AM -0700, Aaron Rosen wrote:
> > Makes sense, getting the logging in OpenStack and in northd should
> > definitely help improve visibility for us to detect this sooner. Even
> > though we won't be able to completely prevent it from the openstack side I
> > think this is still a good safe guard.
> > 
> > On Fri, May 20, 2016 at 7:21 AM, Russell Bryant <russell at ovn.org> wrote:
> > 
> > >
> > >
> > > On Thu, May 19, 2016 at 11:51 PM, Ben Pfaff <blp at ovn.org> wrote:
> > >
> > >> On Thu, May 19, 2016 at 08:42:15PM -0700, Aaron Rosen wrote:
> > >> > I'm wondering if it would be possible to add any additional validation
> > >> on
> > >> > the match column in the ACL table (and potentially other places in the
> > >> > future)?
> > >> >
> > >> > For example, we had a silly bug in the ovn plugin where if someone
> > >> created
> > >> > a security group rule and specified the protocol number as 6 instead of
> > >> > tcp,  we forgot to convert the protocol number 6 to tcp and ended up
> > >> > pushing a rule that looked like this:
> > >> >
> > >> >   to-lport  1002 (outport == "c48a1ff1-a184-491a-9ffd-3db06ebd18ee" &&
> > >> ip4
> > >> > && 6 && *6.dst *== 22) allow-related
> > >>
> > >> We could validate it in ovn-northd so that it doesn't get pushed down to
> > >> the southbound database, either just logging it at northd or adding some
> > >> kind of status or error column to the ACL table so that we could push
> > >> the problem back up.  Is that the kind of thing you're looking for?
> > >
> > >
> > > Validation in ovn-northd and reporting an error state in the ACL table
> > > sounds good to me.
> > >
> > > We can watch events in our plugin for when ACL rows get updated and check
> > > to see if the error column was set.  We can at least log an error on the
> > > OpenStack side in that case.  It would be asynchronous from the OpenStack
> > > API call, so we wouldn't be able to return an error in the API, though.
> > >
> > > --
> > > Russell Bryant
> > >