On Thu, Jan 28, 2021 at 8:02 PM Vadim Fedorenko [off-list ref] 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 <redacted>
Signed-off-by: Vadim Fedorenko <redacted>
Acked-by: Willem de Bruijn <willemb@google.com>
It is easy to verify that if hard_header_len is zero the calculation
does not change. And as discussed, ip_gre is the only ip_tunnel
user that sometimes has it non-zero (for legacy reasons that
we cannot revert now). In that case it is equivalent to tun->hlen +
sizeof(struct iphdr). LGTM. Thanks!
Btw, ip6_gre might need the same after commit 832ba596494b
("net: ip6_gre: set dev->hard_header_len when using header_ops")
On Thu, Jan 28, 2021 at 9:21 PM Willem de Bruijn
[off-list ref] wrote:
On Thu, Jan 28, 2021 at 8:02 PM Vadim Fedorenko [off-list ref] wrote:
quoted
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 <redacted>
Signed-off-by: Vadim Fedorenko <redacted>
Acked-by: Willem de Bruijn <willemb@google.com>
It is easy to verify that if hard_header_len is zero the calculation
does not change. And as discussed, ip_gre is the only ip_tunnel
user that sometimes has it non-zero (for legacy reasons that
we cannot revert now). In that case it is equivalent to tun->hlen +
sizeof(struct iphdr). LGTM. Thanks!
Actually, following that reasoning, we can just remove
dev->hard_header_len from these calculations, no need for branching.
Btw, ip6_gre might need the same after commit 832ba596494b
("net: ip6_gre: set dev->hard_header_len when using header_ops")
On 29.01.2021 02:38, Willem de Bruijn wrote:
On Thu, Jan 28, 2021 at 9:21 PM Willem de Bruijn
[off-list ref] wrote:
quoted
On Thu, Jan 28, 2021 at 8:02 PM Vadim Fedorenko [off-list ref] wrote:
quoted
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 <redacted>
Signed-off-by: Vadim Fedorenko <redacted>
Acked-by: Willem de Bruijn <willemb@google.com>
It is easy to verify that if hard_header_len is zero the calculation
does not change. And as discussed, ip_gre is the only ip_tunnel
user that sometimes has it non-zero (for legacy reasons that
we cannot revert now). In that case it is equivalent to tun->hlen +
sizeof(struct iphdr). LGTM. Thanks!
Actually, following that reasoning, we can just remove
dev->hard_header_len from these calculations, no need for branching.
Shouldn't we rely on what users provide? I mean for the future changes.
But yeah, maybe you are right. All users should set tun->hlen correctly and in
this case ip_tunnel should simply add it's own overhead (sizeof(struct iphdr))
to calculate the MTU-related values. Going to eliminate branching?
quoted
Btw, ip6_gre might need the same after commit 832ba596494b
("net: ip6_gre: set dev->hard_header_len when using header_ops")
Sure, will make it too after clarifying situation with ip_tunnel.