diff mbox series

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

Message ID 20190506190001.6567-1-ssuryaextr@gmail.com
State Accepted
Delegated to: David Miller
Headers show
Series [net] vrf: sit mtu should not be updated when vrf netdev is the link | expand

Commit Message

Stephen Suryaputra May 6, 2019, 7 p.m. UTC
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(-)

Comments

David Ahern May 6, 2019, 7:54 p.m. UTC | #1
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?
Stephen Suryaputra May 6, 2019, 8:28 p.m. UTC | #2
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.
David Ahern May 6, 2019, 8:33 p.m. UTC | #3
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>
David Miller May 7, 2019, 7:20 p.m. UTC | #4
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.
diff mbox series

Patch

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);