diff mbox series

[net,v2] net: vrf: Fix ping failed when vrf mtu is set to 0

Message ID c0ee938d-479d-5c92-93ce-e06c24057b62@huawei.com
State Superseded
Delegated to: David Miller
Headers show
Series [net,v2] net: vrf: Fix ping failed when vrf mtu is set to 0 | expand

Commit Message

Miaohe Lin April 7, 2019, 6:46 a.m. UTC
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 vrf mtu in a reasonable range
to solve this problem. I set dev->min_mtu to 1280, so it will
works for both ipv4 and ipv6. And if dev->max_mtu still be 0
can be confusing, so I set dev->max_mtu to 0xFFFFU.

Here is the reproduce step:

1.Config vrf interface and set mtu to 0:
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

2.Ping peer:
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
connect: Network is unreachable

3.Set mtu to default value, ping works:
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

Fixes: ad49bc6361ca2 ("net: vrf: remove MTU limits for vrf device")
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
V1->V2:
- fix patch commit message
- adjust to work for ipv6

 drivers/net/vrf.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

David Ahern April 7, 2019, 2:30 p.m. UTC | #1
On 4/6/19 11:46 PM, linmiaohe wrote:
> diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
> index 7c1430ed0244..4d35d37f225a 100644
> --- a/drivers/net/vrf.c
> +++ b/drivers/net/vrf.c
> @@ -43,6 +43,12 @@
> 
>  #define FIB_RULE_PREF  1000       /* default preference for FIB rules */
> 
> +/* The MTU is really irrelevant for VRF except for odd cases, so limit it
> + * to a reasonable range which both works for IPV4 and IPV6.
> + */
> +#define VRF_MIN_MTU	1280      /* same as IPV6_MIN_MTU */
> +#define VRF_MAX_MTU	0xFFFFU   /* same as IP_MAX_MTU */

Since VRF does not care about MTU, there is no reason for VRF specific
macros.

> +
>  static unsigned int vrf_net_id;
> 
>  struct net_vrf {
> @@ -1274,8 +1280,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 = VRF_MIN_MTU;
> +	dev->max_mtu = VRF_MAX_MTU;

Set these to IPV6_MIN_MTU and ETH_MAX_MTU and add a comment that VRF
devices do not care about MTU, but if the MTU is set too low then the
ipv4 and ipv6 protocols are disabled which breaks networking.
Miaohe Lin April 8, 2019, 1:58 a.m. UTC | #2
On 2019/4/7 22:30, David Ahern wrote:
> On 4/6/19 11:46 PM, linmiaohe wrote:
>> diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
>> index 7c1430ed0244..4d35d37f225a 100644
>> --- a/drivers/net/vrf.c
>> +++ b/drivers/net/vrf.c
>> @@ -43,6 +43,12 @@
>>
>>  #define FIB_RULE_PREF  1000       /* default preference for FIB rules */
>>
>> +/* The MTU is really irrelevant for VRF except for odd cases, so limit it
>> + * to a reasonable range which both works for IPV4 and IPV6.
>> + */
>> +#define VRF_MIN_MTU	1280      /* same as IPV6_MIN_MTU */
>> +#define VRF_MAX_MTU	0xFFFFU   /* same as IP_MAX_MTU */
> 
> Since VRF does not care about MTU, there is no reason for VRF specific
> macros.
> 
>> +
>>  static unsigned int vrf_net_id;
>>
>>  struct net_vrf {
>> @@ -1274,8 +1280,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 = VRF_MIN_MTU;
>> +	dev->max_mtu = VRF_MAX_MTU;
> 
> Set these to IPV6_MIN_MTU and ETH_MAX_MTU and add a comment that VRF
> devices do not care about MTU, but if the MTU is set too low then the
> ipv4 and ipv6 protocols are disabled which breaks networking.
> 
> 

Thanks a lot for your advice. I will send v3 latter.
diff mbox series

Patch

diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index 7c1430ed0244..4d35d37f225a 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -43,6 +43,12 @@ 

 #define FIB_RULE_PREF  1000       /* default preference for FIB rules */

+/* The MTU is really irrelevant for VRF except for odd cases, so limit it
+ * to a reasonable range which both works for IPV4 and IPV6.
+ */
+#define VRF_MIN_MTU	1280      /* same as IPV6_MIN_MTU */
+#define VRF_MAX_MTU	0xFFFFU   /* same as IP_MAX_MTU */
+
 static unsigned int vrf_net_id;

 struct net_vrf {
@@ -1274,8 +1280,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 = VRF_MIN_MTU;
+	dev->max_mtu = VRF_MAX_MTU;
 }

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