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

[Ben Pfaff]

That function is pretty heavily parameterized, so you might find that
getting suitable parameters (notably the symbol table) is the main
challenge.  If you have trouble, then we can consider the other
approaches proposed.

On Tue, Jun 07, 2016 at 10:22:04AM -0700, Aaron Rosen wrote:
> Sure!
> 
> On Tue, Jun 7, 2016 at 10:18 AM, Ben Pfaff <blp at ovn.org> wrote:
> 
> > That's an interesting idea, do you want to take a shot at it?
> >
> > On Tue, Jun 07, 2016 at 10:09:42AM -0700, Aaron Rosen wrote:
> > > If we want to provide a validator to the caller (i.e the neutron plugin)
> > > would it mostly be a matter of making:
> > >
> > > actions_parse_string()  -
> > > https://github.com/openvswitch/ovs/blob/master/ovn/lib/actions.c#L554
> > >
> > > callable via python? I can start looking into making that work.
> > >
> > > Aaron
> > >
> > > On Fri, May 20, 2016 at 12:53 PM, Ben Pfaff <blp at ovn.org> wrote:
> > >
> > > > Well, there'd be a Conjunction (or And) class, a Disjunction (or Or)
> > > > class, a Comparison class, and so on, and then a method on each class
> > to
> > > > render it to a string.  This is a pain to deal with in C, which is why
> > > > ovn-northd generally doesn't do it, but it might be less painful in
> > > > Python.
> > > >
> > > > On Fri, May 20, 2016 at 11:55:30AM -0700, Aaron Rosen wrote:
> > > > > Would you mind giving an example of what this would look like?
> > > > >
> > > > > On Friday, May 20, 2016, Ben Pfaff <blp at ovn.org> wrote:
> > > > >
> > > > > > 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
> > > > > > <javascript:;>> wrote:
> > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > On Thu, May 19, 2016 at 11:51 PM, Ben Pfaff <blp at ovn.org
> > > > > > <javascript:;>> 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
> > > > > > > > >
> > > > > >
> > > >
> >