Message ID | 0da12059-229f-e167-ad41-14b56993a029@huawei.com |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | [net] net: vrf: Fix ping failed when vrf mtu is set to 0 | expand |
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.
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")
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.
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.
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[],