[ovs-dev] [PATCH 13/18] ofproto: Add functions to configure multicast snooping
blp at nicira.com
Tue May 20 22:36:06 UTC 2014
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.
More information about the dev