All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] net: vrf: Fix ping failed when vrf mtu is set to 0
@ 2019-04-04 12:01 linmiaohe
  2019-04-04 17:52 ` David Miller
  2019-04-07  1:12 ` David Miller
  0 siblings, 2 replies; 5+ messages in thread
From: linmiaohe @ 2019-04-04 12:01 UTC (permalink / raw)
  To: dsa, shrijeet, davem, netdev, linux-kernel; +Cc: Mingfangsen

From: Miaohe Lin <linmiaohe@huawei.com>

When the mtu of a vrf device is set to 0, it would cause ping
failed.So I think we should limit dev->min_mtu to ETH_MIN_MTU
to solve this problem. And if dev->max_mtu still be 0 can be
confusing, so I limit dev->max_mtu to ETH_MAX_MTU.

Here is the reproduce step:

1.Config vrf interface and set mtu to 0:
# ip link show vrf vrf1
3: enp4s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel
master vrf1 state UP mode DEFAULT group default qlen 1000
    link/ether 52:54:00:9e:dd:c1 brd ff:ff:ff:ff:ff:ff
# ip link set vrf1 mtu 0

2.Ping peer:
# ip addr show enp4s0
3: enp4s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel
master vrf1 state UP group default qlen 1000
    link/ether 52:54:00:9e:dd:c1 brd ff:ff:ff:ff:ff:ff
    inet 10.0.0.1/16 scope global enp4s0
       valid_lft forever preferred_lft forever
# ip vrf exec vrf1 ping 10.0.0.2 -c 1
connect: Network is unreachable

3.Set mtu to default value, ping works:
# ip link set vrf1 mtu 65536
# ip vrf exec vrf1 ping 10.0.0.2 -c 1
PING 10.0.0.2 (10.0.0.2) 56(84) bytes of data.
64 bytes from 10.0.0.2: icmp_seq=1 ttl=64 time=1.88 ms

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 drivers/net/vrf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index 7c1430ed0244..632d84fee366 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -1274,8 +1274,8 @@ static void vrf_setup(struct net_device *dev)
 	/* default to no qdisc; user can add if desired */
 	dev->priv_flags |= IFF_NO_QUEUE;

-	dev->min_mtu = 0;
-	dev->max_mtu = 0;
+	dev->min_mtu = ETH_MIN_MTU;
+	dev->max_mtu = ETH_MAX_MTU;
 }

 static int vrf_validate(struct nlattr *tb[], struct nlattr *data[],
-- 
2.19.1




.



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

* Re: [PATCH net] net: vrf: Fix ping failed when vrf mtu is set to 0
  2019-04-04 12:01 [PATCH net] net: vrf: Fix ping failed when vrf mtu is set to 0 linmiaohe
@ 2019-04-04 17:52 ` David Miller
  2019-04-04 18:05   ` David Ahern
  2019-04-07  1:12 ` David Miller
  1 sibling, 1 reply; 5+ messages in thread
From: David Miller @ 2019-04-04 17:52 UTC (permalink / raw)
  To: linmiaohe; +Cc: dsa, shrijeet, netdev, linux-kernel, mingfangsen

From: linmiaohe <linmiaohe@huawei.com>
Date: Thu, 4 Apr 2019 20:01:13 +0800

> When the mtu of a vrf device is set to 0, it would cause ping
> failed.So I think we should limit dev->min_mtu to ETH_MIN_MTU
> to solve this problem. And if dev->max_mtu still be 0 can be
> confusing, so I limit dev->max_mtu to ETH_MAX_MTU.

David A., please review.

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

* Re: [PATCH net] net: vrf: Fix ping failed when vrf mtu is set to 0
  2019-04-04 17:52 ` David Miller
@ 2019-04-04 18:05   ` David Ahern
  0 siblings, 0 replies; 5+ messages in thread
From: David Ahern @ 2019-04-04 18:05 UTC (permalink / raw)
  To: David Miller, linmiaohe; +Cc: shrijeet, netdev, linux-kernel, mingfangsen

On 4/4/19 11:52 AM, David Miller wrote:
> From: linmiaohe <linmiaohe@huawei.com>
> Date: Thu, 4 Apr 2019 20:01:13 +0800
> 
>> When the mtu of a vrf device is set to 0, it would cause ping
>> failed.So I think we should limit dev->min_mtu to ETH_MIN_MTU
>> to solve this problem. And if dev->max_mtu still be 0 can be
>> confusing, so I limit dev->max_mtu to ETH_MAX_MTU.
> 
> David A., please review.
> 

The failure is because in_device (and inet6_dev) are destroyed when the
MTU is below min size for IPv4 (and IPv6). The MTU is really irrelevant
for VRF except for odd cases like this. Really changing the MTU should
fail with an error message that MTU can not be changed for these devices.

As for the suggested ETH_MIN_MTU / MAX_MTU it works for IPv4 since
IPV4_MIN_MTU = 68 = ETH_MIN_MTU, but it does not solve the problem for
IPv6 which has a min MTU of IPV6_MIN_MTU = 1280.

Fixes tag should be:
Fixes: ad49bc6361ca2 ("net: vrf: remove MTU limits for vrf device")

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

* Re: [PATCH net] net: vrf: Fix ping failed when vrf mtu is set to 0
  2019-04-04 12:01 [PATCH net] net: vrf: Fix ping failed when vrf mtu is set to 0 linmiaohe
  2019-04-04 17:52 ` David Miller
@ 2019-04-07  1:12 ` David Miller
  2019-04-07  2:29   ` linmiaohe
  1 sibling, 1 reply; 5+ messages in thread
From: David Miller @ 2019-04-07  1:12 UTC (permalink / raw)
  To: linmiaohe; +Cc: dsa, shrijeet, netdev, linux-kernel, mingfangsen

From: linmiaohe <linmiaohe@huawei.com>
Date: Thu, 4 Apr 2019 20:01:13 +0800

> From: Miaohe Lin <linmiaohe@huawei.com>
> 
> When the mtu of a vrf device is set to 0, it would cause ping
> failed.So I think we should limit dev->min_mtu to ETH_MIN_MTU
> to solve this problem. And if dev->max_mtu still be 0 can be
> confusing, so I limit dev->max_mtu to ETH_MAX_MTU.
> 
> Here is the reproduce step:

Just in case it isn't clear, David Ahern gave you feedback and
asked you to adjust things so that the problem is solved for ipv6
as well.  He also gave you an appropriate Fixes: tag to add to
your commit message.

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

* Re: [PATCH net] net: vrf: Fix ping failed when vrf mtu is set to 0
  2019-04-07  1:12 ` David Miller
@ 2019-04-07  2:29   ` linmiaohe
  0 siblings, 0 replies; 5+ messages in thread
From: linmiaohe @ 2019-04-07  2:29 UTC (permalink / raw)
  To: David Miller, dsa; +Cc: shrijeet, netdev, linux-kernel, mingfangsen



On 2019/4/7 9:12, David Miller wrote:
> From: linmiaohe <linmiaohe@huawei.com>
> Date: Thu, 4 Apr 2019 20:01:13 +0800
> 
>> From: Miaohe Lin <linmiaohe@huawei.com>
>>
>> When the mtu of a vrf device is set to 0, it would cause ping
>> failed.So I think we should limit dev->min_mtu to ETH_MIN_MTU
>> to solve this problem. And if dev->max_mtu still be 0 can be
>> confusing, so I limit dev->max_mtu to ETH_MAX_MTU.
>>
>> Here is the reproduce step:
> 
> Just in case it isn't clear, David Ahern gave you feedback and
> asked you to adjust things so that the problem is solved for ipv6
> as well.  He also gave you an appropriate Fixes: tag to add to
> your commit message.
> 
> .
> 

I'am sorry, I'am on my holiday, so I found these emalis just now.
I will adjust things in v2 so that this patch will solve for ipv6 as well.
And add an appropriate fixes to my commit message.
Thank you very much.


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

end of thread, other threads:[~2019-04-07  2:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-04 12:01 [PATCH net] net: vrf: Fix ping failed when vrf mtu is set to 0 linmiaohe
2019-04-04 17:52 ` David Miller
2019-04-04 18:05   ` David Ahern
2019-04-07  1:12 ` David Miller
2019-04-07  2:29   ` linmiaohe

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.