[ovs-dev] [PATCH 11/14] ovn-controller: Add data structure for indexing lports, multicast groups.

Justin Pettit jpettit at ovn.org
Tue Dec 22 05:01:00 UTC 2015


> On Dec 8, 2015, at 5:08 PM, Ben Pfaff <blp at ovn.org> wrote:
> 
> +struct lport {
> +    struct hmap_node name_node; /* Index by name. */
> +    struct hmap_node key_node;  /* Index by (dp->tunnel_key, tunnel_key). */

I could go either way, but do you think it might be clearler to change these arguments to "dp_key, port_key"?

> +    const struct sbrec_port_binding *sb;

What do you think about changing this member name to "pb"?  I think it may be clearer.  It also matches the use in lflow.c.

> +struct mcgroup {
> +    struct hmap_node dp_name_node; /* Index by (logical datapath, name). */
> +    const struct sbrec_multicast_group *sb;

Similar to above, I wonder if it would be clearer to call this "mg" or something.

Not a biggie, but the init/destroy functions and lookup functions are defined in opposite order between lport and mcgroup.

> +/* Multicast group index
> + * =====================
> + *
> + * This is separate from the logical port index because of namespace issues:
> + * logical port names are globally unique, but multicast group names are only
> + * unique within the scope of a logical datapath. */
> +
> +/* A multicast group.
> + *
> + * Multicast groups could be indexed by number also, but so far the clients do
> + * not need this index. */

This seems redundant.  Is that intentional?

Acked-by: Justin Pettit <jpettit at ovn.org>

--Justin





More information about the dev mailing list