Re: [PATCH net-next v2] ipv6: add IFLA_INET6_RA_MTU to expose mtu value in the RA message

From: David Ahern <dsahern@gmail.com>
To: Rocco Yue <rocco.yue@mediatek.com>,
	"David S . Miller" <davem@davemloft.net>,
	David Ahern <dsahern@kernel.org>,
	Jakub Kicinski <kuba@kernel.org>,
	Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, rocco.yue@gmail.com,
	chao.song@mediatek.com, zhuoliang.zhang@mediatek.com
Subject: Re: [PATCH net-next v2] ipv6: add IFLA_INET6_RA_MTU to expose mtu value in the RA message
Date: Sat, 31 Jul 2021 11:17:53 -0600	[thread overview]
Message-ID: <5be90cf4-f603-c2f2-fd7e-3886854457ba@gmail.com> (raw)
In-Reply-To: <20210731015230.11589-1-rocco.yue@mediatek.com>

On 7/30/21 7:52 PM, Rocco Yue wrote:
> The kernel provides a "/proc/sys/net/ipv6/conf/<iface>/mtu"
> file, which can temporarily record the mtu value of the last
> received RA message when the RA mtu value is lower than the
> interface mtu, but this proc has following limitations:
> 
> (1) when the interface mtu (/sys/class/net/<iface>/mtu) is
> updeated, mtu6 (/proc/sys/net/ipv6/conf/<iface>/mtu) will
> be updated to the value of interface mtu;
> (2) mtu6 (/proc/sys/net/ipv6/conf/<iface>/mtu) only affect
> ipv6 connection, and not affect ipv4.
> 
> Therefore, when the mtu option is carried in the RA message,
> there will be a problem that the user sometimes cannot obtain
> RA mtu value correctly by reading mtu6.
> 
> After this patch set, if a RA message carries the mtu option,
> you can send a netlink msg which nlmsg_type is RTM_GETLINK,
> and then by parsing the attribute of IFLA_INET6_RA_MTU to
> get the mtu value carried in the RA message received on the
> inet6 device.
> 
> In this way, if the MTU values that the device receives from
> the network in the PCO IPv4 and the RA IPv6 procedures are
> different, the user space process can read ra_mtu to get
> the mtu value carried in the RA message without worrying
> about the issue of ipv4 being stuck due to the late arrival
> of RA message. After comparing the value of ra_mtu and ipv4
> mtu, then the device can use the lower MTU value for both
> IPv4 and IPv6.

you are storing the value and sending to userspace but never using it
when sending a message. What's the pointing of processing the MTU in the
RA if you are not going to use it to control message size?

> 
> Signed-off-by: Rocco Yue <rocco.yue@mediatek.com>
> ---
>  include/net/if_inet6.h             | 2 ++
>  include/uapi/linux/if_link.h       | 1 +
>  net/ipv6/addrconf.c                | 5 +++++
>  net/ipv6/ndisc.c                   | 5 +++++
>  tools/include/uapi/linux/if_link.h | 1 +
>  5 files changed, 14 insertions(+)
> 
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 4882e81514b6..fcd1ae29f154 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -417,6 +417,7 @@ enum {
>  	IFLA_INET6_ICMP6STATS,	/* statistics (icmpv6)		*/
>  	IFLA_INET6_TOKEN,	/* device token			*/
>  	IFLA_INET6_ADDR_GEN_MODE, /* implicit address generator mode */
> +	IFLA_INET6_RA_MTU,	/* mtu carried in the RA message  */
>  	__IFLA_INET6_MAX
>  };
>  
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 3bf685fe64b9..98eeaba9f86c 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -5537,6 +5537,7 @@ static inline size_t inet6_ifla6_size(void)
>  	     + nla_total_size(ICMP6_MIB_MAX * 8) /* IFLA_INET6_ICMP6STATS */
>  	     + nla_total_size(sizeof(struct in6_addr)) /* IFLA_INET6_TOKEN */
>  	     + nla_total_size(1) /* IFLA_INET6_ADDR_GEN_MODE */
> +	     + nla_total_size(4) /* IFLA_INET6_RA_MTU */
>  	     + 0;
>  }
>  
> @@ -5645,6 +5646,9 @@ static int inet6_fill_ifla6_attrs(struct sk_buff *skb, struct inet6_dev *idev,
>  	if (nla_put_u8(skb, IFLA_INET6_ADDR_GEN_MODE, idev->cnf.addr_gen_mode))
>  		goto nla_put_failure;
>  
> +	if (nla_put_u32(skb, IFLA_INET6_RA_MTU, idev->ra_mtu))
> +		goto nla_put_failure;
> +
>  	return 0;
>  
>  nla_put_failure:
> @@ -5761,6 +5765,7 @@ static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token,
>  static const struct nla_policy inet6_af_policy[IFLA_INET6_MAX + 1] = {
>  	[IFLA_INET6_ADDR_GEN_MODE]	= { .type = NLA_U8 },
>  	[IFLA_INET6_TOKEN]		= { .len = sizeof(struct in6_addr) },
> +	[IFLA_INET6_RA_MTU]		= { .type = NLA_U32 },
>  };
>  
>  static int check_addr_gen_mode(int mode)

Its value is derived from an RA not set by userspace, so set the type to
NLA_REJECT so that inet6_validate_link_af will reject messages that have
IFLA_INET6_RA_MTU set. You can set "reject_message" in the policy to
return a message that "IFLA_INET6_RA_MTU can not be set".