[ovs-dev] [PATCH 13/18] ofproto: Add functions to configure multicast snooping

[Ben Pfaff]

On Fri, Apr 11, 2014 at 06:34:18PM -0300, Flavio Leitner wrote:
> Acked-by: Thomas Graf <tgraf at redhat.com>
> Signed-off-by: Flavio Leitner <fbl at redhat.com>

I think that the 'enabled' member of struct
ofproto_mcast_snooping_settings is unnecessary, because if snooping is
not to be enabled then a null pointer seems to be used instead of a
struct with 'enabled' false.  It doesn't look like the code here or in
later patches actually reads or writes 'enabled'.

Does changing the flood_unreg setting require revalidation?
Currently, it doesn't appear to trigger revalidation, but I suspect
that it can change the actions associated with a flow, and if so then
it need to revalidate.

It looks like set_mcast_snooping_port() is dangerous and will
assert-fail if multicast snooping is not configured.  I guess that
might be OK, as long as it's intentional.

Do you think that struct ofproto_port_mcast_snooping_settings will
acquire more members?  If not, then a single 'bool' would be OK
instead of a struct.