Message ID | 1515675774-1301-1-git-send-email-alexey.kodanev@oracle.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
Series | [net] ip6_gre: init dev->mtu and dev->hard_header_len correctly | expand |
From: Alexey Kodanev <alexey.kodanev@oracle.com> Date: Thu, 11 Jan 2018 16:02:54 +0300 > For ip6gretap, reset dev->mtu to zero in ip6gre_tap_setup() > after ether_setup(), in order for it to work with the new check > in ip6gre_tunnel_init_common(). This part is error prone. Please instead add a new boolean argument to ip6gre_tunnel_init_common: "bool set_mtu". Set it to true when it is invoked from ip6gre_tap_init() and false when it is invoked from ip6gre_tunnel_init(). Thank you.
On 01/16/2018 07:32 PM, David Miller wrote: > From: Alexey Kodanev <alexey.kodanev@oracle.com> > Date: Thu, 11 Jan 2018 16:02:54 +0300 > >> For ip6gretap, reset dev->mtu to zero in ip6gre_tap_setup() >> after ether_setup(), in order for it to work with the new check >> in ip6gre_tunnel_init_common(). > > This part is error prone. Please instead add a new boolean argument > to ip6gre_tunnel_init_common: "bool set_mtu". Set it to true when > it is invoked from ip6gre_tap_init() and false when it is invoked > from ip6gre_tunnel_init(). Hi David, This way it won't fix the first regression mentioned in the patch for ip6gretap, i.e. if a user sets a MTU manually on the device creation. May be it would be less error prone if a MTU is adjusted again in ip6gre_newlink with "tb[IFLA_MTU]" parameter and ip6gre_tnl_link_config() is moved after register_netdevice()? I haven't tested the following patch yet but it looks like it is fixing all the mentioned cases as well: diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c index 7726959..d12550e 100644 --- a/net/ipv6/ip6_gre.c +++ b/net/ipv6/ip6_gre.c @@ -337,11 +337,12 @@ static void ip6gre_tunnel_unlink(struct ip6gre_net *ign, struct ip6_tnl *t) nt->dev = dev; nt->net = dev_net(dev); - ip6gre_tnl_link_config(nt, 1); if (register_netdevice(dev) < 0) goto failed_free; + ip6gre_tnl_link_config(nt, 1); + /* Can use a lockless transmit, unless we generate output sequences */ if (!(nt->parms.o_flags & TUNNEL_SEQ)) dev->features |= NETIF_F_LLTX; @@ -1303,7 +1304,6 @@ static void ip6gre_netlink_parms(struct nlattr *data[], static int ip6gre_tap_init(struct net_device *dev) { - struct ip6_tnl *tunnel; int ret; ret = ip6gre_tunnel_init_common(dev); @@ -1312,10 +1312,6 @@ static int ip6gre_tap_init(struct net_device *dev) dev->priv_flags |= IFF_LIVE_ADDR_CHANGE; - tunnel = netdev_priv(dev); - - ip6gre_tnl_link_config(tunnel, 1); - return 0; } @@ -1408,12 +1404,16 @@ static int ip6gre_newlink(struct net *src_net, struct net_device *dev, nt->dev = dev; nt->net = dev_net(dev); - ip6gre_tnl_link_config(nt, !tb[IFLA_MTU]); err = register_netdevice(dev); if (err) goto out; + ip6gre_tnl_link_config(nt, !tb[IFLA_MTU]); + + if (tb[IFLA_MTU]) + dev->mtu = nla_get_u32(tb[IFLA_MTU]); + dev_hold(dev); ip6gre_tunnel_link(ign, nt); Thanks, Alexey
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c index 7726959..edf65d0 100644 --- a/net/ipv6/ip6_gre.c +++ b/net/ipv6/ip6_gre.c @@ -337,7 +337,6 @@ static void ip6gre_tunnel_unlink(struct ip6gre_net *ign, struct ip6_tnl *t) nt->dev = dev; nt->net = dev_net(dev); - ip6gre_tnl_link_config(nt, 1); if (register_netdevice(dev) < 0) goto failed_free; @@ -1047,6 +1046,7 @@ static void ip6gre_tnl_init_features(struct net_device *dev) static int ip6gre_tunnel_init_common(struct net_device *dev) { struct ip6_tnl *tunnel; + int set_mtu = !dev->mtu; int ret; int t_hlen; @@ -1072,13 +1072,16 @@ static int ip6gre_tunnel_init_common(struct net_device *dev) t_hlen = tunnel->hlen + sizeof(struct ipv6hdr); dev->hard_header_len = LL_MAX_HEADER + t_hlen; - dev->mtu = ETH_DATA_LEN - t_hlen; - if (dev->type == ARPHRD_ETHER) - dev->mtu -= ETH_HLEN; - if (!(tunnel->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT)) - dev->mtu -= 8; + if (set_mtu) { + dev->mtu = ETH_DATA_LEN - t_hlen; + if (dev->type == ARPHRD_ETHER) + dev->mtu -= ETH_HLEN; + if (!(tunnel->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT)) + dev->mtu -= 8; + } ip6gre_tnl_init_features(dev); + ip6gre_tnl_link_config(tunnel, set_mtu); return 0; } @@ -1303,7 +1306,6 @@ static void ip6gre_netlink_parms(struct nlattr *data[], static int ip6gre_tap_init(struct net_device *dev) { - struct ip6_tnl *tunnel; int ret; ret = ip6gre_tunnel_init_common(dev); @@ -1312,10 +1314,6 @@ static int ip6gre_tap_init(struct net_device *dev) dev->priv_flags |= IFF_LIVE_ADDR_CHANGE; - tunnel = netdev_priv(dev); - - ip6gre_tnl_link_config(tunnel, 1); - return 0; } @@ -1335,6 +1333,7 @@ static void ip6gre_tap_setup(struct net_device *dev) ether_setup(dev); + dev->mtu = 0; dev->max_mtu = 0; dev->netdev_ops = &ip6gre_tap_netdev_ops; dev->needs_free_netdev = true; @@ -1408,7 +1407,6 @@ static int ip6gre_newlink(struct net *src_net, struct net_device *dev, nt->dev = dev; nt->net = dev_net(dev); - ip6gre_tnl_link_config(nt, !tb[IFLA_MTU]); err = register_netdevice(dev); if (err)
Commit b05229f44228 ("gre6: Cleanup GREv6 transmit path, call common GRE functions") moved dev->mtu initialization from ip6gre_tunnel_setup() to ip6gre_tunnel_init(), as a result, the previously set values, before ndo_init(), are reset in the following cases: * rtnl_create_link() can update dev->mtu from IFLA_MTU parameter * ip6gre_tnl_link_config() is invoked before ndo_init() in netlink and ioctl setup, so ndo_init() can reset MTU adjustments with the lower device MTU as well, dev->mtu and dev->hard_header_len. Not applicable for ip6gretap because it has one more call to ip6gre_tnl_link_config(tunnel, 1) in ip6gre_tap_init(). Since, initially net_device allocated with kvzalloc, make sure that dev->mtu is zero, i.e. not changed, before setting default MTU inside ndo_init(), and invoke ip6gre_tnl_link_config after setting default values. For ip6gretap, reset dev->mtu to zero in ip6gre_tap_setup() after ether_setup(), in order for it to work with the new check in ip6gre_tunnel_init_common(). Fixes: b05229f44228 ("gre6: Cleanup GREv6 transmit path, call common GRE functions") Fixes: db2ec95d1ba4 ("ip6_gre: Fix MTU setting") Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com> --- net/ipv6/ip6_gre.c | 22 ++++++++++------------ 1 files changed, 10 insertions(+), 12 deletions(-)