Skip to contentSkip to conversation

net: modify tunnel's name when changing link's name

PATCH NAK Duan Jiong 4 participants 7 messages May 20, 2014 – May 22, 2014

Subsystems (4)
Networking Network drivers IPv4/IPv6 Networking IPsec
lore
Share & tools
lore

Duan Jiong

author
#0 lore raw

PATCH 1/1 net: modify tunnel's name when changing link's name

After executing command "ip tunnel add before local 192.168.0.19 mode gre", the tunnel "before" and the link "before" are created.

Now if user executes command "ip link set dev before name after", the tunnel's name is still "before", but the link's name is "after". At last, user executes command "ip tunnel del before", and error "No such device" will arise.

ip-tunnel uses ioctl to handle command. It firstly gets link through tunnel name, but because link has a different name with tunnel, so the error "No such device" arised.

In order to handle this situation, tunnel's name should be modified when changing link's name.

Signed-off-by Duan Jiong <[email protected]>
---
 include/linux/netdevice.h | 3 +++
 include/net/ip6_tunnel.h  | 1 +
 include/net/ip_tunnels.h  | 1 +
 net/core/dev.c            | 4 ++++
 net/ipv4/ip_gre.c         | 2 ++
 net/ipv4/ip_tunnel.c      | 8 ++++++++
 net/ipv4/ip_vti.c         | 1 +
 net/ipv4/ipip.c           | 1 +
 net/ipv6/ip6_gre.c        | 2 ++
 net/ipv6/ip6_tunnel.c     | 8 ++++++++
 net/ipv6/ip6_vti.c        | 1 +
 net/ipv6/sit.c            | 1 +
 12 files changed, 33 insertions(+)
include/linux/netdevice.h +3 -0
index 2dea98c..6a5fe67 100644
@@ -995,6 +995,8 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
  *	Callback to use for xmit over the accelerated station. This
  *	is used in place of ndo_start_xmit on accelerated net
  *	devices.
+ * void (*ndo_tunnel_rename)(struct net_device *dev)
+ * 	Callback to use for renaming tunnel when rename net_device.
  */
 struct net_device_ops {
 	int			(*ndo_init)(struct net_device *dev);

@@ -1141,6 +1143,7 @@ struct net_device_ops {
 	netdev_tx_t		(*ndo_dfwd_start_xmit) (struct sk_buff *skb,
 							struct net_device *dev,
 							void *priv);
+	void			(*ndo_tunnel_rename) (struct net_device *dev);
 };
 
 /**
include/net/ip6_tunnel.h +1 -0
index a5593da..08cb9c7 100644
@@ -69,6 +69,7 @@ int ip6_tnl_xmit_ctl(struct ip6_tnl *t);
 __u16 ip6_tnl_parse_tlv_enc_lim(struct sk_buff *skb, __u8 *raw);
 __u32 ip6_tnl_get_cap(struct ip6_tnl *t, const struct in6_addr *laddr,
 			     const struct in6_addr *raddr);
+void ip6_tunnel_rename(struct net_device *dev);
 
 static inline void ip6tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
 {
include/net/ip_tunnels.h +1 -0
index a4daf9e..b951ebb 100644
@@ -113,6 +113,7 @@ void ip_tunnel_delete_net(struct ip_tunnel_net *itn, struct rtnl_link_ops *ops);
 void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 		    const struct iphdr *tnl_params, const u8 protocol);
 int ip_tunnel_ioctl(struct net_device *dev, struct ip_tunnel_parm *p, int cmd);
+void ip_tunnel_rename(struct net_device *dev);
 int ip_tunnel_change_mtu(struct net_device *dev, int new_mtu);
 
 struct rtnl_link_stats64 *ip_tunnel_get_stats64(struct net_device *dev,
net/core/dev.c +4 -0
index 867adb2..7afc216 100644
@@ -1086,6 +1086,7 @@ int dev_change_name(struct net_device *dev, const char *newname)
 	int err = 0;
 	int ret;
 	struct net *net;
+	struct net_device_ops *ops = dev->netdev_ops;
 
 	ASSERT_RTNL();
 	BUG_ON(!dev_net(dev));

@@ -1148,6 +1149,9 @@ rollback:
 		}
 	}
 
+	if (ops->ndo_tunnel_rename)
+		ops->ndo_tunnel_rename(dev);
+
 	return err;
 }
net/ipv4/ip_gre.c +2 -0
index c5a557a..960a57d 100644
@@ -453,6 +453,7 @@ static const struct net_device_ops ipgre_netdev_ops = {
 	.ndo_do_ioctl		= ipgre_tunnel_ioctl,
 	.ndo_change_mtu		= ip_tunnel_change_mtu,
 	.ndo_get_stats64	= ip_tunnel_get_stats64,
+	.ndo_tunnel_rename	= ip_tunnel_rename,
 };
 
 #define GRE_FEATURES (NETIF_F_SG |		\

@@ -643,6 +644,7 @@ static const struct net_device_ops gre_tap_netdev_ops = {
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_change_mtu		= ip_tunnel_change_mtu,
 	.ndo_get_stats64	= ip_tunnel_get_stats64,
+	.ndo_tunnel_rename	= ip_tunnel_rename,
 };
 
 static void ipgre_tap_setup(struct net_device *dev)
net/ipv4/ip_tunnel.c +8 -0
index 059176d..7d13c6a 100644
@@ -818,6 +818,14 @@ done:
 }
 EXPORT_SYMBOL_GPL(ip_tunnel_ioctl);
 
+void ip_tunnel_rename(struct net_device *dev)
+{
+	struct ip_tunnel *tunnel = netdev_priv(dev);
+
+	strcpy(tunnel->parms.name, dev->name);
+}
+EXPORT_SYMBOL_GPL(ip_tunnel_rename);
+
 int ip_tunnel_change_mtu(struct net_device *dev, int new_mtu)
 {
 	struct ip_tunnel *tunnel = netdev_priv(dev);
net/ipv4/ip_vti.c +1 -0
index afcee51..397c02b 100644
@@ -332,6 +332,7 @@ static const struct net_device_ops vti_netdev_ops = {
 	.ndo_do_ioctl	= vti_tunnel_ioctl,
 	.ndo_change_mtu	= ip_tunnel_change_mtu,
 	.ndo_get_stats64 = ip_tunnel_get_stats64,
+	.ndo_tunnel_rename = ip_tunnel_rename,
 };
 
 static void vti_tunnel_setup(struct net_device *dev)
net/ipv4/ipip.c +1 -0
index 812b183..b759acb 100644
@@ -270,6 +270,7 @@ static const struct net_device_ops ipip_netdev_ops = {
 	.ndo_do_ioctl	= ipip_tunnel_ioctl,
 	.ndo_change_mtu = ip_tunnel_change_mtu,
 	.ndo_get_stats64 = ip_tunnel_get_stats64,
+	.ndo_tunnel_rename = ip_tunnel_rename,
 };
 
 #define IPIP_FEATURES (NETIF_F_SG |		\
net/ipv6/ip6_gre.c +2 -0
index 3873181..e9fb229 100644
@@ -1208,6 +1208,7 @@ static const struct net_device_ops ip6gre_netdev_ops = {
 	.ndo_do_ioctl		= ip6gre_tunnel_ioctl,
 	.ndo_change_mtu		= ip6gre_tunnel_change_mtu,
 	.ndo_get_stats64	= ip_tunnel_get_stats64,
+	.ndo_tunnel_rename	= ip6_tunnel_rename,
 };
 
 static void ip6gre_dev_free(struct net_device *dev)

@@ -1481,6 +1482,7 @@ static const struct net_device_ops ip6gre_tap_netdev_ops = {
 	.ndo_validate_addr = eth_validate_addr,
 	.ndo_change_mtu = ip6gre_tunnel_change_mtu,
 	.ndo_get_stats64 = ip_tunnel_get_stats64,
+	.ndo_tunnel_rename = ip6_tunnel_rename,
 };
 
 static void ip6gre_tap_setup(struct net_device *dev)
net/ipv6/ip6_tunnel.c +8 -0
index b05b609..bd6ba9b 100644
@@ -1453,6 +1453,13 @@ ip6_tnl_change_mtu(struct net_device *dev, int new_mtu)
 	return 0;
 }
 
+void ip6_tunnel_rename(struct net_device *dev)
+{
+	struct ip6_tnl *tnl = netdev_priv(dev);
+
+	strcpy(tnl->parms.name, dev->name);
+}
+EXPORT_SYMBOL_GPL(ip6_tunnel_rename);
 
 static const struct net_device_ops ip6_tnl_netdev_ops = {
 	.ndo_uninit	= ip6_tnl_dev_uninit,

@@ -1460,6 +1467,7 @@ static const struct net_device_ops ip6_tnl_netdev_ops = {
 	.ndo_do_ioctl	= ip6_tnl_ioctl,
 	.ndo_change_mtu = ip6_tnl_change_mtu,
 	.ndo_get_stats	= ip6_get_stats,
+	.ndo_tunnel_rename = ip6_tunnel_rename,
 };
net/ipv6/ip6_vti.c +1 -0
index 2953c0c..fa48539 100644
@@ -781,6 +781,7 @@ static const struct net_device_ops vti6_netdev_ops = {
 	.ndo_do_ioctl	= vti6_ioctl,
 	.ndo_change_mtu = vti6_change_mtu,
 	.ndo_get_stats64 = ip_tunnel_get_stats64,
+	.ndo_tunnel_rename = ip6_tunnel_rename,
 };
 
 /**
net/ipv6/sit.c +1 -0
index e5a453c..31c9561 100644
@@ -1318,6 +1318,7 @@ static const struct net_device_ops ipip6_netdev_ops = {
 	.ndo_do_ioctl	= ipip6_tunnel_ioctl,
 	.ndo_change_mtu	= ipip6_tunnel_change_mtu,
 	.ndo_get_stats64 = ip_tunnel_get_stats64,
+	.ndo_tunnel_rename = ip6_tunnel_rename,
 };
 
 static void ipip6_dev_free(struct net_device *dev)
Signature · 1 line
1.8.3.1
NAK
#1 lore raw
Quoted 9 lines · Duan Jiong

After executing command "ip tunnel add before local 192.168.0.19 mode gre", the tunnel "before" and the link "before" are created.

Now if user executes command "ip link set dev before name after", the tunnel's name is still "before", but the link's name is "after". At last, user executes command "ip tunnel del before", and error "No such device" will arise.

Quoted 7 lines

ip-tunnel uses ioctl to handle command. It firstly gets link through tunnel name, but because link has a different name with tunnel, so the error "No such device" arised.

In order to handle this situation, tunnel's name should be modified when changing link's name.

NACK.

Renaming is just one of the events we miss for tunnels, you obviously don't want add one netops for each of these events. Instead, we should have a netdev notifier for every such tunnel devices.

I am working on a patch.

#2 lore raw
Quoted 2 lines · Cong Wang

I am working on a patch.

For now, I think we just fix the renaming issue. Please try this patch below (I already tested it actually).

IPv6 part is on the way.

commit 6825b782796215a4c701c44cdb0253e936cf2957

Author: Cong Wang <[email protected]>
Date:   Tue May 20 15:24:43 2014 -0700

ipv4, tunnel: update tunnel name when netdev name is changed

Signed-off-by Cong Wang <[email protected]>
include/net/ip_tunnels.h +1 -0
index a4daf9e..0f6805f 100644
@@ -130,6 +130,7 @@ int ip_tunnel_newlink(struct net_device *dev, struct nlattr *tb[],
 		      struct ip_tunnel_parm *p);
 void ip_tunnel_setup(struct net_device *dev, int net_id);
 void ip_tunnel_dst_reset_all(struct ip_tunnel *t);
+struct ip_tunnel *ip_tunnel_get_by_dev(struct ip_tunnel_net *itn, struct net_device *dev);
 
 /* Extract dsfield from inner protocol */
 static inline u8 ip_tunnel_get_dsfield(const struct iphdr *iph,
net/ipv4/ip_gre.c +38 -1
index c5a557a..7255edf 100644
@@ -778,6 +778,37 @@ static struct pernet_operations ipgre_tap_net_ops = {
 	.size = sizeof(struct ip_tunnel_net),
 };
 
+static int ip_gre_event(struct notifier_block *unused, unsigned long event,
+			void *ptr)
+{
+	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
+	struct net *net = dev_net(dev);
+	struct ip_tunnel *t;
+	struct ip_tunnel_net *itn;
+
+	if (event != NETDEV_CHANGENAME)
+		return NOTIFY_DONE;
+
+	itn = net_generic(net, ipgre_net_id);
+	t = ip_tunnel_get_by_dev(itn, dev);
+	if (!t) {
+		itn = net_generic(net, gre_tap_net_id);
+		t = ip_tunnel_get_by_dev(itn, dev);
+	}
+
+	if (!t)
+		return NOTIFY_DONE;
+
+	if (t->parms.name[0])
+		strlcpy(t->parms.name, dev->name, IFNAMSIZ);
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block ip_gre_notifier = {
+	.notifier_call = ip_gre_event
+};
+
 static int __init ipgre_init(void)
 {
 	int err;

@@ -806,8 +837,13 @@ static int __init ipgre_init(void)
 	if (err < 0)
 		goto tap_ops_failed;
 
-	return 0;
+	err = register_netdevice_notifier(&ip_gre_notifier);
+	if (err < 0)
+		goto netdev_notify_failed;
 
+	return 0;
+netdev_notify_failed:
+	rtnl_link_unregister(&ipgre_tap_ops);
 tap_ops_failed:
 	rtnl_link_unregister(&ipgre_link_ops);
 rtnl_link_failed:

@@ -821,6 +857,7 @@ pnet_tap_faied:
 
 static void __exit ipgre_fini(void)
 {
+	unregister_netdevice_notifier(&ip_gre_notifier);
 	rtnl_link_unregister(&ipgre_tap_ops);
 	rtnl_link_unregister(&ipgre_link_ops);
 	gre_cisco_unregister(&ipgre_protocol);
net/ipv4/ip_tunnel.c +18 -0
index 289c6ee..a5070ea 100644
@@ -1053,4 +1053,22 @@ void ip_tunnel_setup(struct net_device *dev, int net_id)
 }
 EXPORT_SYMBOL_GPL(ip_tunnel_setup);
 
+struct ip_tunnel *ip_tunnel_get_by_dev(struct ip_tunnel_net *itn, struct net_device *dev)
+{
+	int h;
+
+	for (h = 0; h < IP_TNL_HASH_SIZE; h++) {
+		struct hlist_head *thead = &itn->tunnels[h];
+		struct ip_tunnel *t;
+
+		hlist_for_each_entry_rcu(t, thead, hash_node) {
+			if (t->dev == dev)
+				return t;
+		}
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(ip_tunnel_get_by_dev);
+
 MODULE_LICENSE("GPL");
net/ipv4/ip_vti.c +32 -0
index afcee51..5e0a495 100644
@@ -523,6 +523,32 @@ static struct rtnl_link_ops vti_link_ops __read_mostly = {
 	.fill_info	= vti_fill_info,
 };
 
+static int vti_event(struct notifier_block *unused, unsigned long event,
+		     void *ptr)
+{
+	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
+	struct ip_tunnel *t;
+	struct net *net = dev_net(dev);
+	struct ip_tunnel_net *itn;
+
+	if (event != NETDEV_CHANGENAME)
+		return NOTIFY_DONE;
+
+	itn = net_generic(net, vti_net_id);
+	t = ip_tunnel_get_by_dev(itn, dev);
+	if (!t)
+		return NOTIFY_DONE;
+
+	if (t->parms.name[0])
+		strlcpy(t->parms.name, dev->name, IFNAMSIZ);
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block vti_notifier = {
+	.notifier_call = vti_event
+};
+
 static int __init vti_init(void)
 {
 	int err;

@@ -562,9 +588,14 @@ static int __init vti_init(void)
 	err = rtnl_link_register(&vti_link_ops);
 	if (err < 0)
 		goto rtnl_link_failed;
+	err = register_netdevice_notifier(&vti_notifier);
+	if (err < 0)
+		goto netdev_notify_failed;
 
 	return err;
 
+netdev_notify_failed:
+	rtnl_link_unregister(&vti_link_ops);
 rtnl_link_failed:
 	xfrm4_protocol_deregister(&vti_ipcomp4_protocol, IPPROTO_COMP);
 	xfrm4_protocol_deregister(&vti_ah4_protocol, IPPROTO_AH);

@@ -575,6 +606,7 @@ rtnl_link_failed:
 
 static void __exit vti_fini(void)
 {
+	unregister_netdevice_notifier(&vti_notifier);
 	rtnl_link_unregister(&vti_link_ops);
 	if (xfrm4_protocol_deregister(&vti_ipcomp4_protocol, IPPROTO_COMP))
 		pr_info("vti close: can't deregister tunnel\n");
net/ipv4/ipip.c +32 -0
index 812b183..ccd2b75fa 100644
@@ -446,6 +446,32 @@ static struct pernet_operations ipip_net_ops = {
 	.size = sizeof(struct ip_tunnel_net),
 };
 
+static int ipip_event(struct notifier_block *unused, unsigned long event,
+		      void *ptr)
+{
+	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
+	struct ip_tunnel *t;
+	struct net *net = dev_net(dev);
+	struct ip_tunnel_net *itn;
+
+	if (event != NETDEV_CHANGENAME)
+		return NOTIFY_DONE;
+
+	itn = net_generic(net, ipip_net_id);
+	t = ip_tunnel_get_by_dev(itn, dev);
+	if (!t)
+		return NOTIFY_DONE;
+
+	if (t->parms.name[0])
+		strlcpy(t->parms.name, dev->name, IFNAMSIZ);
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block ipip_notifier = {
+	.notifier_call = ipip_event
+};
+
 static int __init ipip_init(void)
 {
 	int err;

@@ -463,10 +489,15 @@ static int __init ipip_init(void)
 	err = rtnl_link_register(&ipip_link_ops);
 	if (err < 0)
 		goto rtnl_link_failed;
+	err = register_netdevice_notifier(&ipip_notifier);
+	if (err < 0)
+		goto netdev_notify_failed;
 
 out:
 	return err;
 
+netdev_notify_failed:
+	rtnl_link_unregister(&ipip_link_ops);
 rtnl_link_failed:
 	xfrm4_tunnel_deregister(&ipip_handler, AF_INET);
 xfrm_tunnel_failed:

@@ -476,6 +507,7 @@ xfrm_tunnel_failed:
 
 static void __exit ipip_fini(void)
 {
+	unregister_netdevice_notifier(&ipip_notifier);
 	rtnl_link_unregister(&ipip_link_ops);
 	if (xfrm4_tunnel_deregister(&ipip_handler, AF_INET))
 		pr_info("%s: can't deregister tunnel\n", __func__);
#3 lore raw
Quoted 8 lines · On Tue, 20 May 2014 16:20:31 -0700 Cong Wang <[email protected]> wrote:
Quoted 2 lines · Cong Wang

I am working on a patch.

For now, I think we just fix the renaming issue. Please try this patch below (I already tested it actually).

IPv6 part is on the way.

Would it be possible for tunnel params name to point to dev->name instead of having to keep a copy up to date?

#4 lore raw
In reply to Cong Wang
Quoted 8 lines · Cong Wang
Quoted 2 lines · Cong Wang

I am working on a patch.

For now, I think we just fix the renaming issue. Please try this patch below (I already tested it actually).

IPv6 part is on the way.

This looks like bloat.

Why do we need to copy dev->name into tunnel->param.name ?

struct ip_tunnel_parm is legacy, but we do not need to store anything in tunnel->param.name

This can be filled when/if sent to userland.

#5 lore raw
Quoted 10 lines · Eric Dumazet
Quoted 8 lines · Cong Wang
Quoted 2 lines · Cong Wang

I am working on a patch.

For now, I think we just fix the renaming issue. Please try this patch below (I already tested it actually).

IPv6 part is on the way.

Quoted 10 lines

This looks like bloat.

Why do we need to copy dev->name into tunnel->param.name ?

struct ip_tunnel_parm is legacy, but we do not need to store anything in tunnel->param.name

This can be filled when/if sent to userland.

Yeah, that should work as well, it's a even quicker fix.

I think we will need a notifier for tunnels anyway, for example, when the lower device (iflink) gets unregistered.

#6 lore raw
Quoted 18 lines · Cong Wang
Quoted 17 lines · Eric Dumazet
Quoted 8 lines · Cong Wang
Quoted 2 lines · Cong Wang

I am working on a patch.

For now, I think we just fix the renaming issue. Please try this patch below (I already tested it actually).

IPv6 part is on the way.

This looks like bloat.

Why do we need to copy dev->name into tunnel->param.name ?

struct ip_tunnel_parm is legacy, but we do not need to store anything in tunnel->param.name

Quoted 8 lines
Quoted 3 lines

This can be filled when/if sent to userland.

Yeah, that should work as well, it's a even quicker fix.

I think we will need a notifier for tunnels anyway, for example, when the lower device (iflink) gets unregistered.

Or a even quicker fix:

net/ipv4/ip_tunnel.c +0 -0
index 289c6ee..e1dce3b 100644
@@ -735,6 +735,8 @@ int ip_tunnel_ioctl(struct net_device *dev, struct

ip_tunnel_parm *p, int cmd)

                        if (t == NULL)
                                t = netdev_priv(dev);
                }
+               /* Device name might have been changed */
+               strlcpy(t->parms.name, t->dev->name, IFNAMSIZ);
                memcpy(p, &t->parms, sizeof(*p));
                break;