linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2] ipv6: add IFLA_INET6_RA_MTU to expose mtu value in the RA message
@ 2021-07-31  1:52 Rocco Yue
  2021-07-31 17:17 ` David Ahern
  0 siblings, 1 reply; 7+ messages in thread
From: Rocco Yue @ 2021-07-31  1:52 UTC (permalink / raw)
  To: David S . Miller, David Ahern, Jakub Kicinski, Hideaki YOSHIFUJI
  Cc: netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
	rocco.yue, chao.song, zhuoliang.zhang, Rocco Yue

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.

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/net/if_inet6.h b/include/net/if_inet6.h
index 71bb4cc4d05d..5cc260dead33 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -213,6 +213,8 @@ struct inet6_dev {
 
 	unsigned long		tstamp; /* ipv6InterfaceTable update timestamp */
 	struct rcu_head		rcu;
+
+	unsigned int		ra_mtu;
 };
 
 static inline void ipv6_eth_mc_map(const struct in6_addr *addr, char *buf)
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)
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index c467c6419893..9c6417a8e2b7 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1496,6 +1496,11 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 		memcpy(&n, ((u8 *)(ndopts.nd_opts_mtu+1))+2, sizeof(mtu));
 		mtu = ntohl(n);
 
+		if (in6_dev->ra_mtu != mtu) {
+			in6_dev->ra_mtu = mtu;
+			ND_PRINTK(2, info, "update ra_mtu to %d\n", in6_dev->ra_mtu);
+		}
+
 		if (mtu < IPV6_MIN_MTU || mtu > skb->dev->mtu) {
 			ND_PRINTK(2, warn, "RA: invalid mtu: %d\n", mtu);
 		} else if (in6_dev->cnf.mtu6 != mtu) {
diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
index d208b2af697f..303085cdc271 100644
--- a/tools/include/uapi/linux/if_link.h
+++ b/tools/include/uapi/linux/if_link.h
@@ -230,6 +230,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
 };
 
-- 
2.18.0

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v2] ipv6: add IFLA_INET6_RA_MTU to expose mtu value in the RA message
  2021-07-31  1:52 [PATCH net-next v2] ipv6: add IFLA_INET6_RA_MTU to expose mtu value in the RA message Rocco Yue
@ 2021-07-31 17:17 ` David Ahern
  2021-08-02  3:19   ` Rocco Yue
  0 siblings, 1 reply; 7+ messages in thread
From: David Ahern @ 2021-07-31 17:17 UTC (permalink / raw)
  To: Rocco Yue, David S . Miller, David Ahern, Jakub Kicinski,
	Hideaki YOSHIFUJI
  Cc: netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
	rocco.yue, chao.song, zhuoliang.zhang

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".


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v2] ipv6: add IFLA_INET6_RA_MTU to expose mtu value in the RA message
  2021-07-31 17:17 ` David Ahern
@ 2021-08-02  3:19   ` Rocco Yue
  2021-08-02 12:40     ` Rocco Yue
  2021-08-02 13:37     ` David Ahern
  0 siblings, 2 replies; 7+ messages in thread
From: Rocco Yue @ 2021-08-02  3:19 UTC (permalink / raw)
  To: David Ahern
  Cc: David S . Miller, Jakub Kicinski, Hideaki YOSHIFUJI, netdev,
	linux-kernel, linux-arm-kernel, linux-mediatek, rocco.yue,
	chao.song, zhuoliang.zhang, Rocco Yue

On Sat, 2021-07-31 at 11:17 -0600, David Ahern wrote:
On 7/30/21 7:52 PM, Rocco Yue wrote:
>> 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?

Hi David,

In the requirement of mobile operator at&t in 2021:
AT&T <CDR-CDS-116> Prioritize Lower MTU value:
If the MTU values that the device receives from the network in the PCO
IPv4 <CDR-CDS-110> and the RA IPv6 <CDR-CDS-112> procedures are different,
then the device shall use the lower MTU value for both IPv4 and IPv6.

And in the 3GPP 23.060:
The PDP PDUs shall be routed and transferred between the MS and the GGSN
or P-GW as N-PDUs. In order to avoid IP layer fragmentation between the
MS and the GGSN or P-GW, the link MTU size in the MS should be set to the
value provided by the network as a part of the IP configuration. This
applies to both IPv6 and IPv4.

That means user needs to be able to correctly read the mtu value carried
in the RA message so that user can correctly compare PCO ipv4 mtu and
RA ipv6 mtu.

>> @@ -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".

will do.

Thanks
Rocco

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v2] ipv6: add IFLA_INET6_RA_MTU to expose mtu value in the RA message
  2021-08-02  3:19   ` Rocco Yue
@ 2021-08-02 12:40     ` Rocco Yue
  2021-08-02 13:35       ` David Ahern
  2021-08-02 13:37     ` David Ahern
  1 sibling, 1 reply; 7+ messages in thread
From: Rocco Yue @ 2021-08-02 12:40 UTC (permalink / raw)
  To: David Ahern
  Cc: David S . Miller, Jakub Kicinski, Hideaki YOSHIFUJI, netdev,
	linux-kernel, linux-arm-kernel, linux-mediatek, rocco.yue,
	chao.song, zhuoliang.zhang, Rocco Yue

On Sat, 2021-07-31 at 11:17 -0600, David Ahern wrote:
On 7/30/21 7:52 PM, Rocco Yue wrote:

> 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".

Hi David,

Regarding setting "reject_message" in the policy, after reviewing
the code, I fell that it is unnecessary, because the cost of
implementing it seems to be a bit high, which requires modifying
the function interface. The reasons is as follows:

The parameter "struct netlink_ext_ack *extack" is not exposed in the
function inet6_validate_link_af(), and the last argument when calling
nla_parse_nested_deprecated() is NULL, which makes the user space not
notified even if reject_message is set.

static int inet6_validate_link_af(...)
{
...
	err = nla_parse_nested_deprecated(tb, IFLA_INET6_MAX, nla,
					  inet6_af_policy, NULL);
...
}


Only when extack is not NULL, reject_message is valid.

static int validate_nla(...)
{
...
	switch (pt->type) {
	case NLA_REJECT:
		if (extack && pt->reject_message) {
			NL_SET_BAD_ATTR(extack, nla);
			extack->_msg = pt->reject_message;
			return -EINVAL;
		}
		err = -EINVAL;
		goto out_err;
...
}


Thanks
Rocco

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v2] ipv6: add IFLA_INET6_RA_MTU to expose mtu value in the RA message
  2021-08-02 12:40     ` Rocco Yue
@ 2021-08-02 13:35       ` David Ahern
  0 siblings, 0 replies; 7+ messages in thread
From: David Ahern @ 2021-08-02 13:35 UTC (permalink / raw)
  To: Rocco Yue, David Ahern
  Cc: David S . Miller, Jakub Kicinski, Hideaki YOSHIFUJI, netdev,
	linux-kernel, linux-arm-kernel, linux-mediatek, rocco.yue,
	chao.song, zhuoliang.zhang

On 8/2/21 6:40 AM, Rocco Yue wrote:
> On Sat, 2021-07-31 at 11:17 -0600, David Ahern wrote:
> On 7/30/21 7:52 PM, Rocco Yue wrote:
> 
>> 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".
> 
> Hi David,
> 
> Regarding setting "reject_message" in the policy, after reviewing
> the code, I fell that it is unnecessary, because the cost of
> implementing it seems to be a bit high, which requires modifying
> the function interface. The reasons is as follows:

The policy can be setup now to do the right thing once the extack
argument is available.

do_setlink() has an extack argument. It calls validate_linkmsg which
calls validate_link_af meaning support can be added in a single patch.
If you decide to do it, then it should be a separate patch preceding
this one.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v2] ipv6: add IFLA_INET6_RA_MTU to expose mtu value in the RA message
  2021-08-02  3:19   ` Rocco Yue
  2021-08-02 12:40     ` Rocco Yue
@ 2021-08-02 13:37     ` David Ahern
  2021-08-03 11:57       ` Rocco Yue
  1 sibling, 1 reply; 7+ messages in thread
From: David Ahern @ 2021-08-02 13:37 UTC (permalink / raw)
  To: Rocco Yue, David Ahern
  Cc: David S . Miller, Jakub Kicinski, Hideaki YOSHIFUJI, netdev,
	linux-kernel, linux-arm-kernel, linux-mediatek, rocco.yue,
	chao.song, zhuoliang.zhang

On 8/1/21 9:19 PM, Rocco Yue wrote:
> On Sat, 2021-07-31 at 11:17 -0600, David Ahern wrote:
> On 7/30/21 7:52 PM, Rocco Yue wrote:
>>> 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?
> 
> Hi David,
> 
> In the requirement of mobile operator at&t in 2021:
> AT&T <CDR-CDS-116> Prioritize Lower MTU value:
> If the MTU values that the device receives from the network in the PCO
> IPv4 <CDR-CDS-110> and the RA IPv6 <CDR-CDS-112> procedures are different,
> then the device shall use the lower MTU value for both IPv4 and IPv6.
> 
> And in the 3GPP 23.060:
> The PDP PDUs shall be routed and transferred between the MS and the GGSN
> or P-GW as N-PDUs. In order to avoid IP layer fragmentation between the
> MS and the GGSN or P-GW, the link MTU size in the MS should be set to the
> value provided by the network as a part of the IP configuration. This
> applies to both IPv6 and IPv4.
> 
> That means user needs to be able to correctly read the mtu value carried
> in the RA message so that user can correctly compare PCO ipv4 mtu and
> RA ipv6 mtu.
> 

Then userspace should get a link notification when ra_mtu is set so it
does not have to poll.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v2] ipv6: add IFLA_INET6_RA_MTU to expose mtu value in the RA message
  2021-08-02 13:37     ` David Ahern
@ 2021-08-03 11:57       ` Rocco Yue
  0 siblings, 0 replies; 7+ messages in thread
From: Rocco Yue @ 2021-08-03 11:57 UTC (permalink / raw)
  To: David Ahern
  Cc: David S . Miller, Jakub Kicinski, Hideaki YOSHIFUJI, netdev,
	linux-kernel, linux-arm-kernel, linux-mediatek, rocco.yue,
	chao.song, zhuoliang.zhang, Rocco Yue

On Mon, 2021-08-02 at 07:35 -0600, David Ahern wrote:
> On 8/2/21 6:40 AM, Rocco Yue wrote:
>> 
>> Regarding setting "reject_message" in the policy, after reviewing
>> the code, I fell that it is unnecessary, because the cost of
>> implementing it seems to be a bit high, which requires modifying
>> the function interface. The reasons is as follows:
> 
> The policy can be setup now to do the right thing once the extack
> argument is available.
> 
> do_setlink() has an extack argument. It calls validate_linkmsg which
> calls validate_link_af meaning support can be added in a single patch.
> If you decide to do it, then it should be a separate patch preceding
> this one.
> 

Hi David,

Thanks for your advice,
I will send a separate patch to add extack arg firstly.

> Then userspace should get a link notification when ra_mtu is set so it
> does not have to poll.

It make sense, I will do it.

Thanks
Rocco

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-08-03 12:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-31  1:52 [PATCH net-next v2] ipv6: add IFLA_INET6_RA_MTU to expose mtu value in the RA message Rocco Yue
2021-07-31 17:17 ` David Ahern
2021-08-02  3:19   ` Rocco Yue
2021-08-02 12:40     ` Rocco Yue
2021-08-02 13:35       ` David Ahern
2021-08-02 13:37     ` David Ahern
2021-08-03 11:57       ` Rocco Yue

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).