* [PATCH] ipv6: Honor route mtu if it is within limit of dev mtu
@ 2021-02-22 16:32 Kaustubh Pandey
2021-02-24 18:47 ` Jakub Kicinski
2021-02-24 20:28 ` David Ahern
0 siblings, 2 replies; 3+ messages in thread
From: Kaustubh Pandey @ 2021-02-22 16:32 UTC (permalink / raw)
Cc: Kaustubh Pandey, David S. Miller, Hideaki YOSHIFUJI, David Ahern,
Jakub Kicinski, netdev, linux-kernel, sharathv, chinagar
When netdevice MTU is increased via sysfs, NETDEV_CHANGEMTU is raised.
addrconf_notify -> rt6_mtu_change -> rt6_mtu_change_route ->
fib6_nh_mtu_change
As part of handling NETDEV_CHANGEMTU notification we land up on a
condition where if route mtu is less than dev mtu and route mtu equals
ipv6_devconf mtu, route mtu gets updated.
Due to this v6 traffic end up using wrong MTU then configured earlier.
This commit fixes this by removing comparison with ipv6_devconf
and updating route mtu only when it is greater than incoming dev mtu.
This can be easily reproduced with below script:
pre-condition:
device up(mtu = 1500) and route mtu for both v4 and v6 is 1500
test-script:
ip route change 192.168.0.0/24 dev eth0 src 192.168.0.1 mtu 1400
ip -6 route change 2001::/64 dev eth0 metric 256 mtu 1400
echo 1400 > /sys/class/net/eth0/mtu
ip route change 192.168.0.0/24 dev eth0 src 192.168.0.1 mtu 1500
echo 1500 > /sys/class/net/eth0/mtu
Signed-off-by: Kaustubh Pandey <kapandey@codeaurora.org>
---
net/ipv6/route.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 1536f49..653b6c7 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4813,8 +4813,7 @@ static int fib6_nh_mtu_change(struct fib6_nh *nh, void *_arg)
struct inet6_dev *idev = __in6_dev_get(arg->dev);
u32 mtu = f6i->fib6_pmtu;
- if (mtu >= arg->mtu ||
- (mtu < arg->mtu && mtu == idev->cnf.mtu6))
+ if (mtu >= arg->mtu)
fib6_metric_set(f6i, RTAX_MTU, arg->mtu);
spin_lock_bh(&rt6_exception_lock);
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ipv6: Honor route mtu if it is within limit of dev mtu
2021-02-22 16:32 [PATCH] ipv6: Honor route mtu if it is within limit of dev mtu Kaustubh Pandey
@ 2021-02-24 18:47 ` Jakub Kicinski
2021-02-24 20:28 ` David Ahern
1 sibling, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2021-02-24 18:47 UTC (permalink / raw)
To: Hideaki YOSHIFUJI, David Ahern
Cc: Kaustubh Pandey, David S. Miller, netdev, linux-kernel, sharathv,
chinagar
On Mon, 22 Feb 2021 22:02:35 +0530 Kaustubh Pandey wrote:
> When netdevice MTU is increased via sysfs, NETDEV_CHANGEMTU is raised.
>
> addrconf_notify -> rt6_mtu_change -> rt6_mtu_change_route ->
> fib6_nh_mtu_change
>
> As part of handling NETDEV_CHANGEMTU notification we land up on a
> condition where if route mtu is less than dev mtu and route mtu equals
> ipv6_devconf mtu, route mtu gets updated.
>
> Due to this v6 traffic end up using wrong MTU then configured earlier.
> This commit fixes this by removing comparison with ipv6_devconf
> and updating route mtu only when it is greater than incoming dev mtu.
>
> This can be easily reproduced with below script:
> pre-condition:
> device up(mtu = 1500) and route mtu for both v4 and v6 is 1500
>
> test-script:
> ip route change 192.168.0.0/24 dev eth0 src 192.168.0.1 mtu 1400
> ip -6 route change 2001::/64 dev eth0 metric 256 mtu 1400
> echo 1400 > /sys/class/net/eth0/mtu
> ip route change 192.168.0.0/24 dev eth0 src 192.168.0.1 mtu 1500
> echo 1500 > /sys/class/net/eth0/mtu
>
> Signed-off-by: Kaustubh Pandey <kapandey@codeaurora.org>
> ---
> net/ipv6/route.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 1536f49..653b6c7 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -4813,8 +4813,7 @@ static int fib6_nh_mtu_change(struct fib6_nh *nh, void *_arg)
> struct inet6_dev *idev = __in6_dev_get(arg->dev);
> u32 mtu = f6i->fib6_pmtu;
>
> - if (mtu >= arg->mtu ||
> - (mtu < arg->mtu && mtu == idev->cnf.mtu6))
> + if (mtu >= arg->mtu)
> fib6_metric_set(f6i, RTAX_MTU, arg->mtu);
>
> spin_lock_bh(&rt6_exception_lock);
David, Hideaki - any thoughts on this one? Can we change this long
standing behavior?
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ipv6: Honor route mtu if it is within limit of dev mtu
2021-02-22 16:32 [PATCH] ipv6: Honor route mtu if it is within limit of dev mtu Kaustubh Pandey
2021-02-24 18:47 ` Jakub Kicinski
@ 2021-02-24 20:28 ` David Ahern
1 sibling, 0 replies; 3+ messages in thread
From: David Ahern @ 2021-02-24 20:28 UTC (permalink / raw)
To: Kaustubh Pandey, Stefano Brivio
Cc: David S. Miller, Hideaki YOSHIFUJI, David Ahern, Jakub Kicinski,
netdev, linux-kernel, sharathv, chinagar
On 2/22/21 9:32 AM, Kaustubh Pandey wrote:
> When netdevice MTU is increased via sysfs, NETDEV_CHANGEMTU is raised.
>
> addrconf_notify -> rt6_mtu_change -> rt6_mtu_change_route ->
> fib6_nh_mtu_change
>
> As part of handling NETDEV_CHANGEMTU notification we land up on a
> condition where if route mtu is less than dev mtu and route mtu equals
> ipv6_devconf mtu, route mtu gets updated.
>
> Due to this v6 traffic end up using wrong MTU then configured earlier.
> This commit fixes this by removing comparison with ipv6_devconf
> and updating route mtu only when it is greater than incoming dev mtu.
>
> This can be easily reproduced with below script:
> pre-condition:
> device up(mtu = 1500) and route mtu for both v4 and v6 is 1500
>
> test-script:
> ip route change 192.168.0.0/24 dev eth0 src 192.168.0.1 mtu 1400
> ip -6 route change 2001::/64 dev eth0 metric 256 mtu 1400
> echo 1400 > /sys/class/net/eth0/mtu
> ip route change 192.168.0.0/24 dev eth0 src 192.168.0.1 mtu 1500
> echo 1500 > /sys/class/net/eth0/mtu
>
> Signed-off-by: Kaustubh Pandey <kapandey@codeaurora.org>
> ---
> net/ipv6/route.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 1536f49..653b6c7 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -4813,8 +4813,7 @@ static int fib6_nh_mtu_change(struct fib6_nh *nh, void *_arg)
> struct inet6_dev *idev = __in6_dev_get(arg->dev);
> u32 mtu = f6i->fib6_pmtu;
>
> - if (mtu >= arg->mtu ||
> - (mtu < arg->mtu && mtu == idev->cnf.mtu6))
> + if (mtu >= arg->mtu)
> fib6_metric_set(f6i, RTAX_MTU, arg->mtu);
>
> spin_lock_bh(&rt6_exception_lock);
>
The existing logic mirrors what is done for exceptions, see
rt6_mtu_change_route_allowed and commit e9fa1495d738.
It seems right to me to drop the mtu == idev->cnf.mtu6 comparison in
which case the exceptions should do the same.
Added author of e9fa1495d738 in case I am overlooking something.
Test case should be added to tools/testing/selftests/net/pmtu.sh, and
did you run that script with the proposed change?
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-02-24 20:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-22 16:32 [PATCH] ipv6: Honor route mtu if it is within limit of dev mtu Kaustubh Pandey
2021-02-24 18:47 ` Jakub Kicinski
2021-02-24 20:28 ` David Ahern
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.