Subject: Re: [PATCHv4 net 6/8] vti: do not confirm neighbor when do pmtu update

From: Guillaume Nault <[email-masked]>
Date: Sat, 21 Dec 2019 19:30:24 +0100

On Fri, Dec 20, 2019 at 11:25:23AM +0800, Hangbin Liu wrote:
> Although ip vti is not affected as __ip_rt_update_pmtu() does not call
> dst_confirm_neigh(), we still not do neigh confirm to keep consistency with
> IPv6 code.
> 
As with the GRE case, the problem is not the IP version used on the
underlay. The problem happens when the tunnel transports IPv6 packets
(which vti can do).

However, it's true that vti is immune to this problem, but that's for
another reason: it's an IFF_NOARP interface (which means vti6 is immune
too).

Patch is good (we really have no reason to confirm neighbour here), but
let's not have a misleading commit message.

> v4: No change.
> v3: Do not remove dst_confirm_neigh, but add a new bool parameter in
>     dst_ops.update_pmtu to control whether we should do neighbor confirm.
>     Also split the big patch to small ones for each area.
> v2: Remove dst_confirm_neigh in __ip6_rt_update_pmtu.
> 
> Reviewed-by: Guillaume Nault <[email-masked]>
> Signed-off-by: Hangbin Liu <[email-masked]>
> ---
>  net/ipv4/ip_vti.c  | 2 +-
>  net/ipv6/ip6_vti.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
> index cfb025606793..fb9f6d60c27c 100644
> --- a/net/ipv4/ip_vti.c
> +++ b/net/ipv4/ip_vti.c
> @@ -214,7 +214,7 @@ static netdev_tx_t vti_xmit(struct sk_buff *skb, struct net_device *dev,
>  
>  	mtu = dst_mtu(dst);
>  	if (skb->len > mtu) {
> -		skb_dst_update_pmtu(skb, mtu);
> +		skb_dst_update_pmtu_no_confirm(skb, mtu);
>  		if (skb->protocol == htons(ETH_P_IP)) {
>  			icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
>  				  htonl(mtu));
> diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
> index 024db17386d2..6f08b760c2a7 100644
> --- a/net/ipv6/ip6_vti.c
> +++ b/net/ipv6/ip6_vti.c
> @@ -479,7 +479,7 @@ vti6_xmit(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
>  
>  	mtu = dst_mtu(dst);
>  	if (skb->len > mtu) {
> -		skb_dst_update_pmtu(skb, mtu);
> +		skb_dst_update_pmtu_no_confirm(skb, mtu);
>  
>  		if (skb->protocol == htons(ETH_P_IPV6)) {
>  			if (mtu < IPV6_MIN_MTU)
> -- 
> 2.19.2
>