Re: [B.A.T.M.A.N.] [PATCH 1/1] batman-adv: Always flood IGMP/MLD reports

[Marek Lindner <mareklindner@neomailbox.ch>]

[-- Attachment #1: Type: text/plain, Size: 2591 bytes --]

On Sunday, June 28, 2015 05:59:14 Linus Lüssing wrote:
> > I am not quite clear on what the patch does. It helps to support a feature
> > that is yet to come (upcoming bridge integration) or improves the
> > situation
> > today ?
> 
> The former. It's not fixing or improving anything for the current
> implementation.

If this patch isn't doing anything (for now) maybe it should be merged when it 
becomes useful ?


> > > +#if IS_ENABLED(CONFIG_IPV6)
> > > 
> > >  	case ETH_P_IPV6:
> > >  		return batadv_mcast_forw_mode_check_ipv6(bat_priv, skb,
> > >  		
> > >  							 is_unsnoopable);
> > > 
> > > +#endif
> > > 
> > >  	default:
> > >  		return -EINVAL;
> > >  	
> > >  	}
> > 
> > This hunk seems not really related to the patch itself ?
> 
> I think it's necessary for the new ipv6_mc_check_mld() which isn't
> there if building a kernel without any IPv6 support.

You may want to send a separate patch then ? This change seems unrelated to 
the rest.


> > By removing mcast v1 you effectively are breaking compatibility with all
> > nodes running v1 and require everyone to upgrade to v2.
> 
> No, no v1 nodes are required to upgrade. v1 and v2 nodes are still
> able to communicate. v1 nodes (like any pre v1 node) might downgrade the
> mesh to a mcast-optimizations-disabled state for now though, yes.

I do understand that 'normal' packet exchange is not affected. However, the 
TVLVs were introduced with the intend of maintaining best possible 
compatibility with future versions. With the first tvlv version bump we already 
require upgrading everyone or compatibility is already broken ? Is there 
really nothing we can do ? v2 could at least be compatible to v1 ?


> We cannot safely use the multicast optimizations with bridges if
> there are nodes which do not handle reports properly. The bump
> isn't needed for any packet changes but the internal, local behaviour
> of a node.
> 
> I haven't heard of anyone using the multicast optimization feature
> in practice yet (that is, a setup without bridges), so I think it
> is safe to do a version bump?

A version bump for a feature which does not do anything useful yet (see my 
initial question) ? How likely is it that we will need another version bump by 
the time this feature does become useful ?


> Hm, good question :). Need to recheck, I vaguely remember having
> had issues with the IP header parsing due to unset skb network
> headers.

If it is related to this patch please add a comment. The change is non-
obvious.

Cheers,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]