[ovs-dev] [PATCH ovn v3 07/10] ovn: Add tunnel_key concept to Bindings table, assign in ovn-northd.

[Russell Bryant]

On 04/28/2015 09:11 PM, Ben Pfaff wrote:
> On Tue, Apr 28, 2015 at 05:36:50PM -0700, Justin Pettit wrote:
>>
>>> On Apr 28, 2015, at 5:21 PM, Ben Pfaff <blp at nicira.com> wrote:
>>>
>>> On Tue, Apr 28, 2015 at 03:53:03PM -0400, Russell Bryant wrote:
>>>>
>>>> The code here looks correct and I also tested it.  I was just wondering
>>>> if you could comment on the choice of a 16 bit integer here instead of a
>>>> UUID.  My guess is that it has to do with where this ID will be used in
>>>> a tunnel protocol, but it might be nice to capture that somewhere.
>>>
>>> Thanks for the question, I used it to improve the documentation to:
>>>
>>>    <column name="tunnel_key">
>>>      A number that represents the logical port in the IDs carried within
>>
>> "IDs" sounds a bit strange to me.  What about "metadata"?  Unfortunately, there's no consistent way to refer to those bits, so my suggestion may not be an improvement.
>>
>>>      tunnel protocol packets.  (This avoids wasting space for a whole UUID in
>>>      tunneled packets.  It allows OVN to support encapsulations that cannot
>>>      fit an entire UUID in their tunnel keys.)
>>
>> I think the second sentence in the parenthetical section would be clearer if it started with "Also".
> 
> OK, how about this:
> 
>         A number that represents the logical port in the key (e.g. VXLAN VNI or
>         STT key) field carried within tunnel protocol packets.  (This avoids
>         wasting space for a whole UUID in tunneled packets.  It also allows OVN
>         to support encapsulations that cannot fit an entire UUID in their
>         tunnel keys.)

Yes, that helps.  Thanks.

I wonder how realistic it would be to have 65k ports and hit this as a
limit.  If your deployment has many containers in each VM, it seems like
we could hit that much more quickly than traditionally seen before.

If a protocol can fit a whole UUID in tunneled packets, I suppose we
could optionally switch to using the logical port UUID later?

Another thought I had on the implementation was that the choice of
tunnel_key will get more expensive the more ports you have if it has to
search through thousands of candidate tunnel keys to find one that's
unused.  It sounds like something we might be able to improve later if
it actually shows up when profiling ovn-northd.

With the last proposed doc update:

Acked-by: Russell Bryant <rbryant at redhat.com>

-- 
Russell Bryant