On Tue, May 20, 2014 at 03:36:06PM -0700, Ben Pfaff wrote: > On Fri, Apr 11, 2014 at 06:34:18PM -0300, Flavio Leitner wrote: > > Acked-by: Thomas Graf <tg...@redhat.com> > > Signed-off-by: Flavio Leitner <f...@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'.
Sorry, it's a leftover from a previous version. I will remove it. > 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. Thinking again, you're right. There is a condition where the actions associated with a flow must change. I will fix in v2. > 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. It could be better. I will see how to improve it. > 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. Very likely, but I guess we can use a single 'bool' for now. fbl _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
