[OpenWrt-Devel] [PATCHv2 netifd 2/3] bridge: Allow setting multicast_to_unicast option

[Linus Lüssing]

On Thu, Aug 06, 2015 at 11:09:02PM +0200, Felix Fietkau wrote:
> On 2015-08-06 22:10, Linus Lüssing wrote:
> > On Wed, Aug 05, 2015 at 01:38:28PM +0200, Felix Fietkau wrote:
> >> On 2015-08-05 01:00, Linus Lüssing wrote:
> >> > With this patch the multicast_to_unicast feature can be disabled for all
> >> > wireless interfaces via an according option on the uci bridge interface.
> >> > 
> >> > This patch also exports the setting information to wireless handler
> >> > scripts. The hostapd script will need that information to determine
> >> > whether to enable or disable ap-isolation, for instance.
> >> > 
> >> > Signed-off-by: Linus Lüssing <linus.luessing at c0d3.blue>
> >> I think it would be better to store these flags in the bridge config
> >> instead of the generic device config, and either add a blob_buf argument
> >> in struct device_hotplug_ops -> prepare, or add a new callback get_info,
> >> which adds the bridge info for use in the wireless json.
> > 
> > These would then be options per bridge, but not per bridge port,
> > wouldn't they?
> Right.
> 
> > For the multicast_to_unicast feature, okay (currently this patch
> > only allows setting it "globally" for a bridge but not on a bridge
> > port basis - though I was thinking about maybe making it bridge
> > port specific once someone needs that). For the multicast_router
> > option I'd need that on a bridge port basis.
> OK, then leave it as device options for now until we have a better way
> of specifiying bridge port settings. Same for multicast_to_unicast.

Ok, thanks :). (we want to fix and use bridge snooping +
multicast-to-unicast here as soon as possible so I'm quite fond of
simple solutions for now :) )

> 
> >> I would like to avoid polluting the generic device options with bridge
> >> specific stuff.
> >> 
> >> This patch should probably be merged with the previous one afterwards,
> >> since the first one without any other changes will cause regressions.
> > 
> > Regressions, where/why? If this patch were ommitted from this
> > patchset then no harm should be done (unless I'm missing
> > something?).
> The regression is: patch 1 enables hairpin mode for wireless interfaces,
> with the assumption that the wireless script enables isolate mode for
> multicast-to-unicast enable configurations.
> The wireless script can only do that if patch 2 is applies, because that
> one passes the required options.

Not passing the multicast_to_unicast option from netifd to
hostapd.sh is not a regression. Actually even with patch 2 there
are cases where no multicast_to_unicast json option is passed -
that's when no multicast_to_unicast option was set in uci.

With netifd patch 1, without netifd patch 2 and with openwrt patch
1 things will work fine without a regression because
multicast_to_unicast is then empty and hostapd.sh will assume the
default which is that multicast_to_unicast was enabled.


However, netifd patch 1 without openwrt patch 1 is a regression,
yes (which is independant of netifd patch 2). Which I couldn't do
with a single patch as it's in two packages. But I like your
suggestion for netifd patch 1 which will also get rid of an
"intermediate regression" :). So with that change I think I can
leave netifd patch 2 and netifd 3 as is, without any regressions.

Cheers, Linus
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel