[ovs-dev] [PATCH 3/3] ovn: Add address_set() support for ACLs.

[Mickey Spiegel]

One comment below.

-----"dev" <dev-bounces at openvswitch.org> wrote: -----

>To: Ben Pfaff <blp at ovn.org>
>From: Russell Bryant 
>Sent by: "dev" 
>Date: 04/12/2016 09:37AM
>Cc: ovs dev <dev at openvswitch.org>
>Subject: Re: [ovs-dev] [PATCH 3/3] ovn: Add address_set() support for
>ACLs.
>
>On Mon, Apr 11, 2016 at 12:08 PM, Ben Pfaff <blp at 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 <russell at 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.

Thinking of the use of remote_group_id in OpenStack security groups, I believe it would be common for the same address set to be used in all logical switches that are connected to the same logical router. Since there is no notion of project in OVN, it is not so obvious what to scope address sets to.
Would it be good enough to have controllers check to see if the address set is used by any of the local logical switches?

Mickey

>> 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 at openvswitch.org
>http://openvswitch.org/mailman/listinfo/dev
>