On Fri, Sep 27, 2013 at 6:03 PM, Hannes Frederic Sowa <han...@stressinduktion.org> wrote: > On Fri, Sep 27, 2013 at 05:36:45PM +0100, Oussama Ghorbel wrote: >> Please see my comments below >> >> Regards, >> Oussama >> >> On Fri, Sep 27, 2013 at 11:58 AM, Hannes Frederic Sowa >> <han...@stressinduktion.org> wrote: >> > On Fri, Sep 27, 2013 at 11:45:48AM +0100, Oussama Ghorbel wrote: >> >> The ip6_tunnel.c module would be then dependent on ip_tunnel.c and may >> >> be it would not be good thing? >> > >> > It could just be a static inline in some shared header. So there would >> > be no compile-time dependency. >> > >> >> The higher limit of mtu in ip_tunnel_change_mtu() is a calculated value. >> This high limit is calculated using the netdev priv struct ip_tunnel >> (field hlen). >> This netdev priv struct is different in ipv6, it's a ip6_tnl struct. >> Therefore implementing a beautiful function like >> ip_tunnel_valid_mtu(struct net_device *dev, int mtu) won't be >> feasible, unless we implement it in macro or something like like >> ip_tunnel_valid_mtu(struct net_device *dev, int hlen, int mtu) which >> seems not very nice ... >> What do yo think? > > Ok, let's go with one function per protocol type. Seems easier. > > It seems to get more hairy, because it depends on the tunnel driver if the > prepended ip header is accounted in hard_header_len. :/ > > I don't know if it works out cleanly. Otherwise I would be ok if the checks > just get repeated in ip6_tunnel and leave the rest as-is. > Yes, It will be the clean way to do it. >> >> >> As I have check in v3.10 there is no call from ip6_tunnel to ip_tunnel... >> >> >> >> For information, there is no check for the maximum MTU for ipv4 in the >> >> patch as this is not done for ipv6. >> > >> > I understand, but it would be better to limit the MTU here. There is a >> > (non-jumo) IPV6_MAXPLEN constant. >> > >> > Looking through the source it seems grev6 does actually check this, >> > so it would not hurt adding them here, too. >> >> what if jumbograms is used, in that case, we can't use IPV6_MAXPLEN. >> the limit would be the the full unsigned int. >> However checking the higher limit for ipv4 would be useful. > > Linux currently cannot create "jumbograms" (only the receiving side > is supported). > I understand, but what are the benefit from this limit or the harm from not specifying it? Please check this comment from eth.c
/** * eth_change_mtu - set new MTU size * @dev: network device * @new_mtu: new Maximum Transfer Unit * * Allow changing MTU size. Needs to be overridden for devices * supporting jumbo frames. */ int eth_change_mtu(struct net_device *dev, int new_mtu) So wouldn't be a good idea to let our function open for jumbo frames...? >> Please also note in case the tunnel mode is any (ipv4 or ipv6 means >> ip6_tnl.parms.proto = 0), then we will be required to take the most >> restrict limit from both ipv4 and ipv6 which means the lower limit >> will be 1280 and the higher limit will be about 65535 >> Do you agree on this? > > Agreed, this would be needed for e.g. the sit driver, which now can > handle both protocols. > > Thanks, > > Hannes > Thanks, Oussama -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
