On Mon, Dec 21, 2015 at 09:01:00PM -0800, Justin Pettit wrote:
>
> > On Dec 8, 2015, at 5:08 PM, Ben Pfaff <b...@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"?
You mean, just change the comment, like this? Done.
struct hmap_node key_node; /* Index by (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.
OK, done.
> > +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.
OK, done.
> Not a biggie, but the init/destroy functions and lookup functions are
> defined in opposite order between lport and mcgroup.
OK, I made it consistent.
> > +/* 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?
OK, I combined the comments.
> Acked-by: Justin Pettit <jpet...@ovn.org>
Thanks for the review.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
