On Mon, Apr 11, 2016 at 12:08 PM, Ben Pfaff <b...@ovn.org> wrote:

> On Tue, Apr 05, 2016 at 05:24:19PM -0400, Russell Bryant wrote:
> > This feature was originally proposed here:
> >
> >   http://openvswitch.org/pipermail/dev/2016-March/067440.html
> >
> > A common use case for OVN ACLs involves needing to match a set of IP
> > addresses.
> >
> >    outport == "lp1" && ip4.src == {10.0.0.5, 10.0.0.25, 10.0.0.50}
> >
> > This example match only has 3 addresses, but it could easily have
> > hundreds of addresses.  In some cases, the same large set of addresses
> > needs to be used in several ACLs.
> >
> > This patch adds a new Address_Set table to OVN_Northbound so that a set
> > of addresses can be specified once and then referred to by name in ACLs.
> > To recreate the above example, you would first create an address set:
> >
> >   $ ovn-nbctl create Address_Set name=set1
> addresses=10.0.0.5,10.0.0.25,10.0.0.50
> >
> > Then you can refer to this address set by name in an ACL match:
> >
> >   outport == "lp1" && ip4.src == address_set(set1)
> >
> > Signed-off-by: Russell Bryant <russ...@ovn.org>
>
> It might be worth making address sets scoped somehow.  Otherwise it
> might eventually become a bottleneck if there are many address sets that
> a given hypervisor does not need to know about.
>

Good point.

The primary use case I have for this is to use in the match column of OVN
ACLs.  ACLs are scoped to logical switches, so we could do the same for
address sets.  Having them scoped equally makes sense to me.

The only downside I can think of is that it will cause OpenStack to have to
duplicate the same address set across multiple logical switches in some
cases.  I think that's probably OK though.  It's still a huge improvement
over what it does today.


> I am not sure that I am in favor of the restriction that all of the
> addresses in a given set have the same form, described here:
>
> +      Each row in this table represents a named set of addresses.
> +      An address set may contain MAC, IPv4, or IPv6 addresses, but
> +      a single address set must contain addresses all of the same type.
>
> This restriction might be surprising to someone already familiar with
> the OVN matching language, which otherwise doesn't have such a
> restriction.
>

I made up the restriction, mainly because I wasn't sure how mixing types
would be useful.  I can certainly drop it.  I don't think there's actually
anything in the code trying to enforce it, anyway.


> I don't see anything that indicates that an address set can include CIDR
> or bitwise matches, e.g. 192.168.0.0/24 or
> 01:00:00:00:00:00/01:00:00:00:00:00.  I don't know why that would be
> restricted.
>

I just didn't think of it.  Explicitly including and testing that makes
sense.


> I haven't done a detailed review yet but it sounds like you plan to post
> a v2.
>

Thanks for the feedback!  I'll incorporate your comments on both patches in
the next revision (v3).

-- 
Russell Bryant
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to