On 28.01.2021 21:48, Willem de Bruijn wrote:
On Wed, Jan 27, 2021 at 11:14 PM Vadim Fedorenko <[email protected]> wrote:dev->hard_header_len for tunnel interface is set only when header_ops are set too and already contains full overhead of any tunnel encapsulation. That's why there is not need to use this overhead twice in mtu calc. Fixes: fdafed459998 ("ip_gre: set dev->hard_header_len and dev->needed_headroom properly") Reported-by: Slava Bacherikov <[email protected]> Signed-off-by: Vadim Fedorenko <[email protected]> --- net/ipv4/ip_tunnel.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c index 64594aa..ad78825 100644 --- a/net/ipv4/ip_tunnel.c +++ b/net/ipv4/ip_tunnel.c @@ -317,7 +317,7 @@ static int ip_tunnel_bind_dev(struct net_device *dev) } dev->needed_headroom = t_hlen + hlen; - mtu -= (dev->hard_header_len + t_hlen); + mtu -= dev->hard_header_len ? : t_hlen;Safety of this change also depends on whether any other ip tunnels might have non-zero hard_header_len.
Yes, sure.
I haven't fully checked yet, but at first scan I only see one other instance of header_ops, and that ip_tunnel_header_ops does not have a create implementation.
Yes. The calls to ip_tunnel_setup are in ip_gre.c, ip_tunnel.c, ip_vti.c and ipip.c. All of them except of ip_gre use ip_tunnel_header_ops which doesn't have create implementation and have hard_header_len set to 0.
if (mtu < IPV4_MIN_MTU) mtu = IPV4_MIN_MTU; @@ -347,7 +347,7 @@ static struct ip_tunnel *ip_tunnel_create(struct net *net, nt = netdev_priv(dev); t_hlen = nt->hlen + sizeof(struct iphdr); dev->min_mtu = ETH_MIN_MTU; - dev->max_mtu = IP_MAX_MTU - dev->hard_header_len - t_hlen; + dev->max_mtu = IP_MAX_MTU - dev->hard_header_len ? : t_hlen;here and elsewhere: subtraction takes precedence over ternary conditional, so (IP_MAX_MTU - ..) always true.
Oh, sure, my bad. Will fix it in v2, thanks!
