[PATCH net] vrf: sit mtu should not be updated when vrf netdev is the link

[PATCH net] vrf: sit mtu should not be updated when vrf netdev is the link

[Stephen Suryaputra]

VRF netdev mtu isn't typically set and have an mtu of 65536. When the
link of a tunnel is set, the tunnel mtu is changed from 1480 to the link
mtu minus tunnel header. In the case of VRF netdev is the link, then the
tunnel mtu becomes 65516. So, fix it by not setting the tunnel mtu in
this case.

Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>
---
 net/ipv6/sit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index b2109b74857d..971d60bf9640 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -1084,7 +1084,7 @@ static void ipip6_tunnel_bind_dev(struct net_device *dev)
 	if (!tdev && tunnel->parms.link)
 		tdev = __dev_get_by_index(tunnel->net, tunnel->parms.link);
 
-	if (tdev) {
+	if (tdev && !netif_is_l3_master(tdev)) {
 		int t_hlen = tunnel->hlen + sizeof(struct iphdr);
 
 		dev->hard_header_len = tdev->hard_header_len + sizeof(struct iphdr);
-- 
2.17.1

Re: [PATCH net] vrf: sit mtu should not be updated when vrf netdev is the link

[David Ahern]

On 5/6/19 1:00 PM, Stephen Suryaputra wrote:
> VRF netdev mtu isn't typically set and have an mtu of 65536. When the
> link of a tunnel is set, the tunnel mtu is changed from 1480 to the link
> mtu minus tunnel header. In the case of VRF netdev is the link, then the
> tunnel mtu becomes 65516. So, fix it by not setting the tunnel mtu in
> this case.
> 
> Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>
> ---
>  net/ipv6/sit.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
> index b2109b74857d..971d60bf9640 100644
> --- a/net/ipv6/sit.c
> +++ b/net/ipv6/sit.c
> @@ -1084,7 +1084,7 @@ static void ipip6_tunnel_bind_dev(struct net_device *dev)
>  	if (!tdev && tunnel->parms.link)
>  		tdev = __dev_get_by_index(tunnel->net, tunnel->parms.link);
>  
> -	if (tdev) {
> +	if (tdev && !netif_is_l3_master(tdev)) {
>  		int t_hlen = tunnel->hlen + sizeof(struct iphdr);
>  
>  		dev->hard_header_len = tdev->hard_header_len + sizeof(struct iphdr);
> 

can you explain how tdev is a VRF device? What's the config setup for
this case?

Re: [PATCH net] vrf: sit mtu should not be updated when vrf netdev is the link

[Stephen Suryaputra]

On Mon, May 06, 2019 at 01:54:16PM -0600, David Ahern wrote:
> On 5/6/19 1:00 PM, Stephen Suryaputra wrote:
> > VRF netdev mtu isn't typically set and have an mtu of 65536. When the
> > link of a tunnel is set, the tunnel mtu is changed from 1480 to the link
> > mtu minus tunnel header. In the case of VRF netdev is the link, then the
> > tunnel mtu becomes 65516. So, fix it by not setting the tunnel mtu in
> > this case.
> > 
> > Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>
> > ---
> >  net/ipv6/sit.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
> > index b2109b74857d..971d60bf9640 100644
> > --- a/net/ipv6/sit.c
> > +++ b/net/ipv6/sit.c
> > @@ -1084,7 +1084,7 @@ static void ipip6_tunnel_bind_dev(struct net_device *dev)
> >  	if (!tdev && tunnel->parms.link)
> >  		tdev = __dev_get_by_index(tunnel->net, tunnel->parms.link);
> >  
> > -	if (tdev) {
> > +	if (tdev && !netif_is_l3_master(tdev)) {
> >  		int t_hlen = tunnel->hlen + sizeof(struct iphdr);
> >  
> >  		dev->hard_header_len = tdev->hard_header_len + sizeof(struct iphdr);
> > 
> 
> can you explain how tdev is a VRF device? What's the config setup for
> this case?

Hi David,

tdev is set to VRF device per your suggestion to my colleague back in
2017:
	https://www.spinics.net/lists/netdev/msg462706.html.
Specifically this on this follow up:
	https://www.spinics.net/lists/netdev/msg463287.html

His basic config before your suggestion is available in:
	https://www.spinics.net/lists/netdev/msg462770.html

He and I had a refresher discussion this am trying to figure out if tdev
should be a slave device. This is true if the local addr is specified.
In this case the addr has to bound to a slave device. Then the underlay
VRF can be derived from it. But if only remote is specified, then there
isn't a straightforward way to associate the remote with a VRF unless
tdev is set to a VRF device.

Re: [PATCH net] vrf: sit mtu should not be updated when vrf netdev is the link

[David Ahern]

On 5/6/19 2:28 PM, Stephen Suryaputra wrote:
> On Mon, May 06, 2019 at 01:54:16PM -0600, David Ahern wrote:
>> On 5/6/19 1:00 PM, Stephen Suryaputra wrote:
>>> VRF netdev mtu isn't typically set and have an mtu of 65536. When the
>>> link of a tunnel is set, the tunnel mtu is changed from 1480 to the link
>>> mtu minus tunnel header. In the case of VRF netdev is the link, then the
>>> tunnel mtu becomes 65516. So, fix it by not setting the tunnel mtu in
>>> this case.
>>>
>>> Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>
>>> ---
>>>  net/ipv6/sit.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
>>> index b2109b74857d..971d60bf9640 100644
>>> --- a/net/ipv6/sit.c
>>> +++ b/net/ipv6/sit.c
>>> @@ -1084,7 +1084,7 @@ static void ipip6_tunnel_bind_dev(struct net_device *dev)
>>>  	if (!tdev && tunnel->parms.link)
>>>  		tdev = __dev_get_by_index(tunnel->net, tunnel->parms.link);
>>>  
>>> -	if (tdev) {
>>> +	if (tdev && !netif_is_l3_master(tdev)) {
>>>  		int t_hlen = tunnel->hlen + sizeof(struct iphdr);
>>>  
>>>  		dev->hard_header_len = tdev->hard_header_len + sizeof(struct iphdr);
>>>
>>
>> can you explain how tdev is a VRF device? What's the config setup for
>> this case?
> 
> Hi David,
> 
> tdev is set to VRF device per your suggestion to my colleague back in
> 2017:
> 	https://www.spinics.net/lists/netdev/msg462706.html.
> Specifically this on this follow up:
> 	https://www.spinics.net/lists/netdev/msg463287.html
> 
> His basic config before your suggestion is available in:
> 	https://www.spinics.net/lists/netdev/msg462770.html
> 
> He and I had a refresher discussion this am trying to figure out if tdev
> should be a slave device. This is true if the local addr is specified.
> In this case the addr has to bound to a slave device. Then the underlay
> VRF can be derived from it. But if only remote is specified, then there
> isn't a straightforward way to associate the remote with a VRF unless
> tdev is set to a VRF device.
> 

Right. Thanks for the reminder.

Reviewed-by: David Ahern <dsahern@gmail.com>

Re: [PATCH net] vrf: sit mtu should not be updated when vrf netdev is the link

[David Miller]

From: Stephen Suryaputra <ssuryaextr@gmail.com>
Date: Mon,  6 May 2019 15:00:01 -0400

> VRF netdev mtu isn't typically set and have an mtu of 65536. When the
> link of a tunnel is set, the tunnel mtu is changed from 1480 to the link
> mtu minus tunnel header. In the case of VRF netdev is the link, then the
> tunnel mtu becomes 65516. So, fix it by not setting the tunnel mtu in
> this case.
> 
> Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>

Applied and queued up for -stable.