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 |
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?
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.
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>
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 --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);
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(-)