Bram Yvahk wrote:
> Only send a 'need to frag' ICMP message when the "Don't Fragment" bit
> is set. If it's not set then the packet can/will be fragmented.
>
> This fixes sending an 'need to frag' message on a client that did not
> set the DF bit, i.e.:
>
> $ ping -s 1300 -M dont -c5 192.168.235.2
> PING 192.168.235.3 (192.168.235.3) 1300(1328) bytes of data.
> From 192.168.236.254 icmp_seq=1 Frag needed and DF set (mtu = 1214)
>
> Signed-off-by: Bram Yvahk <bram-yv...@mail.wizbit.be>
> ---
> net/ipv4/ip_vti.c | 43 +++++++++++++++++++++++++++--------------
> net/ipv6/ip6_vti.c | 56
> +++++++++++++++++++++++++++++++++++++++---------------
> 2 files changed, 70 insertions(+), 29 deletions(-)
>
>
Parts in the patch that I don't really like:
1) adds almost identical code in ipv4/ip_vti.c and ipv6/ip6_vti.c
(is there a place to share this code?)
2) 'vti_tunnel_check_size'/'vti6_tunnel_check_size' functions:
Name and implementation of this function is based on the xfrm code
but to me "check" in the function name implies that it's (only)
checking something but in this case it's doing a bit more: it also
updates
the path-mtu and transmitting an ICMP message.
3) return value of 'vti6_tunnel_check_size':
In XFRM ('xfrm4_tunnel_check_size'/'xfrm6_tunnel_check_size') return
value of '0' and '-EMSGSIZE' is used.
I opted not to follow this because of how the code is called in
'vti6_xmit'. It has an 'err' variable but it is initialized to '-1'.
What I considered:
- let 'vti6_tunnel_check_size' return 0 and -EMSGSIZE: this matches
with XFRM but it makes the 'err' variable in 'vti_xmit' a bit
confusing.
Before calling 'vti6_tunnel_check_size' err=-1 means no error, after
calling 'vti6_tunnel_check_size' err=0 means no error..
- let 'vti6_tunnel_check_size' return 0 and -EMSGSIZE and use a second
err variable in vti6_xmit (i.e.
if ((err2 = vti6_tunnel_check_size(..)) < 0) { err=err2; goto ... }
- let 'vti6_tunnel_check_size' return -1 and -EMSGSIZE: this matches
with XFRM and works but to me it seems confusing for the function to
return -1 when there is no error...
- let 'vti6_tunnel_check_size' return a bool and set error in 'vti6_xmit'
- change vti6_xmit to initialize err to 0 (but I've no idea of the impact
of that)
The least ugly solution to me was the bool so I went with that.
4) no error is set in vti_xmit (ip_vti): when the packet is too large it
doesn't set an error and always returns 'NETDEV_TX_OK'. [This was already
the case so that behaviour is kept but it doesn't look right...]
