> On Apr 24, 2015, at 3:34 PM, Ben Pfaff <b...@nicira.com> wrote:
>
> When packets travel among nodes in OVN over tunnels, a tunnel key value is
> needed to convey the logical port to which the packet is destined. This
> commit adds a tunnel_key column to the Bindings table and adds code to
> ovn-northd to assign a unique tunnel_key value to each logical port.
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
This patch (and the implementation that follows) goes the route of having each
logical port have a unique tunnel key. I had envisioned a different approach.
I'll briefly write it down here, but then it's probably worth discussing it
more fully in a different forum (or at least a different thread). I was
thinking that we'd use a 24-bit VNI that would identify a logical switch. In
Geneve, it would be in the VNI field, and in STT, it would be in the lower 24
bits of the 64 bit key. The local OVS would receive packets from a particular
VNI, and then look at the DMAC to figure out which logical ports should receive
it.
For port security, we would add a logical port that would uniquely identify a
port within a particular VNI (ie, the value is only unique for a particular
VNI). For this, the logical port would only be used to identify the ingress
port, since the receiving OVS would still be responsible for local replication
and enforcing policies. We would store the logical port in a TLV extension in
Geneve and in the next (let's say) 16 bits of the STT header.
Okay, now that I've written that down, I'll review the code as you've
implemented it. We can have a follow-up discussion on the relative merits of
each approach.
> + /* Choose unique tunnel_key for the logical port. */
> + static uint16_t next_tunnel_key = 1;
> + while (tunnel_key_in_use(&tk_hmap, next_tunnel_key)) {
> + next_tunnel_key++;
> + }
> + sbrec_bindings_set_tunnel_key(binding, next_tunnel_key++);
Were you trying to avoid a tunnel key of zero? If so, I think we'll hit that
on wrap-around. Also, if we have 64K logical ports, I think this will get
stuck in a permanent loop with no way to get out, since it's the only thing
that ever deletes a tunnel_key. Finally, should you add the new tunnel to
tk_hmap so that if we're anywhere near full that we don't assign the same
tunnel key twice?
> + struct binding_hash_node *hash_node;
> + HMAP_FOR_EACH (hash_node, lp_node, &lp_hmap) {
> + hmap_remove(&lp_hmap, &hash_node->lp_node);
Should you be using HMAP_FOR_EACH_SAFE()?
> + "tunnel_key": {
> + "type": {"key": {"type": "integer",
> + "minInteger": 1,
> + "maxInteger": 65535}}},
If we decide to go with a unique id for each logical port, I do worry that 64K
is too small for the total number of logical ports supported by OVN.
--Justin
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
