[PATCH] Change MAC without bringing interface down

[PATCH] Change MAC without bringing interface down

[Tommi Virtanen]

	Hi. There are cases when you want to change your MAC address,
	such as in Linux-HA when the other box fails, and your hot
	standby activates itself.

	Some NICs, such as e100, e1000 and uml ;) allow you to do that
	without bringing the interface down. Which is nice, because it
	allows the link to stay up, and not make switches scratch their
	head for up to a minute.

	The problem is, if you do that, the old MAC address is stored
	in the hh_cache entries, and used in future traffic also.


	Here's a patch that

	1. makes eth_header_cache_update also update the source MAC in
	   hh_cache.

	2. adds a NETDEV_CHANGEADDR notifier in net/core/neighbour.c
	   that walks through all the neighbour entries and updates
	   the hh_cache's for all the neighbour entries related to
	   given device.

	3. adds neighbour_init() to allow registration of said notifier.


	Please comment.



===== include/net/neighbour.h 1.1 vs edited =====
--- 1.1/include/net/neighbour.h	Tue Feb  5 19:39:48 2002
+++ edited/include/net/neighbour.h	Mon Aug 18 11:17:25 2003
@@ -275,6 +275,8 @@
 	return neigh_create(tbl, pkey, dev);
 }
 
+extern void		neighbour_init(void);
+
 #endif
 #endif
 
===== net/core/dev.c 1.36 vs edited =====
--- 1.36/net/core/dev.c	Fri May 23 20:59:51 2003
+++ edited/net/core/dev.c	Mon Aug 18 11:54:30 2003
@@ -93,6 +93,7 @@
 #include <linux/if_bridge.h>
 #include <linux/divert.h>
 #include <net/dst.h>
+#include <net/neighbour.h>
 #include <net/pkt_sched.h>
 #include <net/profile.h>
 #include <net/checksum.h>
@@ -2849,6 +2850,7 @@
 	open_softirq(NET_RX_SOFTIRQ, net_rx_action, NULL);
 
 	dst_init();
+	neighbour_init();
 	dev_mcast_init();
 
 #ifdef CONFIG_NET_SCHED
===== net/core/neighbour.c 1.9 vs edited =====
--- 1.9/net/core/neighbour.c	Thu Jun 12 09:24:41 2003
+++ edited/net/core/neighbour.c	Mon Aug 18 11:53:50 2003
@@ -1558,3 +1558,46 @@
 }
 
 #endif	/* CONFIG_SYSCTL */
+
+static int neighbour_dev_event(struct notifier_block *this, unsigned long event, void *ptr)
+{
+	struct net_device *dev = ptr;
+
+	switch (event) {
+		struct neigh_table *tbl;
+	case NETDEV_CHANGEADDR:
+		read_lock(&neigh_tbl_lock);
+		for (tbl = neigh_tables; tbl; tbl=tbl->next) {
+			int i;
+
+			for (i=0; i<=NEIGH_HASHMASK; i++) {
+				struct neighbour *n, **np;
+
+				np = &tbl->hash_buckets[i];
+				write_lock(&tbl->lock);
+
+				while ((n = *np) != NULL) {
+					if (n->dev == dev) {
+						neigh_update_hhs(n);
+					}
+					np = &n->next;
+				}
+				write_unlock(&tbl->lock);
+			}
+		}
+		read_unlock(&neigh_tbl_lock);
+		break;
+	}
+	return NOTIFY_DONE;
+}
+
+struct notifier_block neighbour_dev_notifier = {
+	neighbour_dev_event,
+	NULL,
+	0
+};
+
+void __init neighbour_init(void)
+{
+	register_netdevice_notifier(&neighbour_dev_notifier);
+}
===== net/ethernet/eth.c 1.3 vs edited =====
--- 1.3/net/ethernet/eth.c	Fri Jun 20 00:22:53 2003
+++ edited/net/ethernet/eth.c	Mon Aug 18 11:55:36 2003
@@ -238,6 +238,11 @@
 
 void eth_header_cache_update(struct hh_cache *hh, struct net_device *dev, unsigned char * haddr)
 {
-	memcpy(((u8*)hh->hh_data) + HH_DATA_OFF(sizeof(struct ethhdr)),
-	       haddr, dev->addr_len);
+	struct ethhdr *eth;
+
+	eth = (struct ethhdr*)
+		(((u8*)hh->hh_data) + (HH_DATA_OFF(sizeof(*eth))));
+
+	memcpy(eth->h_source, dev->dev_addr, dev->addr_len);
+	memcpy(eth->h_dest, haddr, dev->addr_len);
 }


-- 
:(){ :|:&};:

Re: [PATCH] Change MAC without bringing interface down

[David S. Miller]

On Mon, 18 Aug 2003 12:13:12 +0300
Tommi Virtanen <tv@debian.org> wrote:

> 	Here's a patch that
> 
> 	1. makes eth_header_cache_update also update the source MAC in
> 	   hh_cache.
> 
> 	2. adds a NETDEV_CHANGEADDR notifier in net/core/neighbour.c
> 	   that walks through all the neighbour entries and updates
> 	   the hh_cache's for all the neighbour entries related to
> 	   given device.
> 
> 	3. adds neighbour_init() to allow registration of said notifier.

I would suggest to fix this by instead flushing the hh
entries (just like neigh_destroy() does).

This way, we don't have to fix up every single header cache
implementation, that's painful :(

Re: [PATCH] Change MAC without bringing interface down

[jamal]

[-- Attachment #1: Type: text/plain, Size: 1406 bytes --]

Editing the header may be the better approach if you have a few hundred 
hhs already inserted (this was the issue i was faced with when VRRP was
failing over when i was playing with it). I tried to flush the cache but
it took longer to recover it. 
The patch posted may not be the better way to do this. It should be per
neighbour table owner - at the moment only ARP and ndisc really care
about this. 
Heres an old patch i have circa 2.4.2 that i have been sitting on
planning to convert it to latest kernels; the ndisc portion is missing;
If this is found agreeable Tommi you can clean it up and bring it to par
with latest kernel

cheers,
jamal


On Mon, 2003-08-18 at 07:19, David S. Miller wrote:
> On Mon, 18 Aug 2003 12:13:12 +0300
> Tommi Virtanen <tv@debian.org> wrote:
> 
> > 	Here's a patch that
> > 
> > 	1. makes eth_header_cache_update also update the source MAC in
> > 	   hh_cache.
> > 
> > 	2. adds a NETDEV_CHANGEADDR notifier in net/core/neighbour.c
> > 	   that walks through all the neighbour entries and updates
> > 	   the hh_cache's for all the neighbour entries related to
> > 	   given device.
> > 
> > 	3. adds neighbour_init() to allow registration of said notifier.
> 
> I would suggest to fix this by instead flushing the hh
> entries (just like neigh_destroy() does).
> 
> This way, we don't have to fix up every single header cache
> implementation, that's painful :(
> 
> 

[-- Attachment #2: neighp --]
[-- Type: text/plain, Size: 4496 bytes --]

--- 242/include/net/neighbour.h	2003-07-20 19:42:33.000000000 -0400
+++ 242/include/net/patchedneighbour.h	2003-07-20 19:42:38.000000000 -0400
@@ -180,6 +180,7 @@
 extern void			neigh_destroy(struct neighbour *neigh);
 extern int			__neigh_event_send(struct neighbour *neigh, struct sk_buff *skb);
 extern int			neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new, int override, int arp);
+extern int			neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev);
 extern int			neigh_ifdown(struct neigh_table *tbl, struct net_device *dev);
 extern int			neigh_resolve_output(struct sk_buff *skb);
 extern int			neigh_connected_output(struct sk_buff *skb);
--- 242/net/core/neighbour.c	2003-07-20 19:38:19.000000000 -0400
+++ 242/net/core/patchedneighbour.c	2003-08-18 07:15:35.000000000 -0400
@@ -50,6 +50,7 @@
 static void neigh_app_notify(struct neighbour *n);
 #endif
 static int pneigh_ifdown(struct neigh_table *tbl, struct net_device *dev);
+int neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev);
 
 static int neigh_glbl_allocs;
 static struct neigh_table *neigh_tables;
@@ -410,6 +411,52 @@
 	return -ENOENT;
 }
 
+void update_neigh_hh(struct neighbour *neigh)
+{
+
+	struct hh_cache *hh;
+
+	int (*update)(struct neighbour *neigh, struct hh_cache *hh) =
+                neigh->dev->hard_header_cache;
+
+	if (update) {
+		for (hh=neigh->hh; hh; hh=hh->hh_next) {
+			write_lock_bh(&hh->hh_lock);
+			update(neigh, hh);
+			write_unlock_bh(&hh->hh_lock);
+		}
+	}
+	else {
+		printk("device neigh %p does not have a update method!\n",neigh);
+	}
+}
+
+int neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev)
+{
+	int i;
+
+	write_lock_bh(&tbl->lock);
+
+	for (i=0; i<=NEIGH_HASHMASK; i++) {
+		struct neighbour *n, **np;
+
+		np = &tbl->hash_buckets[i];
+		while ((n = *np) != NULL) {
+			if (dev && n->dev != dev) {
+				np = &n->next;
+				continue;
+			}
+			*np = n->next;
+			write_lock_bh(&n->lock);
+			update_neigh_hh(n);
+			write_unlock_bh(&n->lock);
+		}
+	}
+
+        write_unlock_bh(&tbl->lock);
+	return 0;
+}
+
 static int pneigh_ifdown(struct neigh_table *tbl, struct net_device *dev)
 {
 	struct pneigh_entry *n, **np;
@@ -432,6 +479,7 @@
 }
 
 
+
 /*
  *	neighbour must already be out of the table;
  *
@@ -488,8 +536,9 @@
 
 	neigh->output = neigh->ops->output;
 
-	for (hh = neigh->hh; hh; hh = hh->hh_next)
+	for (hh = neigh->hh; hh; hh = hh->hh_next) {
 		hh->hh_output = neigh->ops->output;
+	}
 }
 
 /* Neighbour state is OK;
@@ -507,8 +556,10 @@
 
 	neigh->output = neigh->ops->connected_output;
 
-	for (hh = neigh->hh; hh; hh = hh->hh_next)
+	for (hh = neigh->hh; hh; hh = hh->hh_next) {
 		hh->hh_output = neigh->ops->hh_output;
+	}
+
 }
 
 /*
@@ -924,6 +975,7 @@
 		atomic_inc(&hh->hh_refcnt);
 		dst->hh = hh;
 	}
+
 }
 
 /* This function can be used in contexts, where only old dev_queue_xmit
@@ -961,6 +1013,7 @@
 		int err;
 		struct net_device *dev = neigh->dev;
 		if (dev->hard_header_cache && dst->hh == NULL) {
+
 			write_lock_bh(&neigh->lock);
 			if (dst->hh == NULL)
 				neigh_hh_init(neigh, dst, dst->ops->protocol);
--- 242/net/ipv4/arp.c	2003-07-20 19:37:52.000000000 -0400
+++ 242/net/ipv4/patchedarp.c	2003-08-18 07:19:08.000000000 -0400
@@ -1,6 +1,6 @@
 /* linux/net/inet/arp.c
  *
- * Version:	$Id: arp.c,v 1.90 2000/10/04 09:20:56 anton Exp $
+ * Version:	$Id: arp.c,v 1.1 2003/07/04 14:04:46 root Exp root $
  *
  * Copyright (C) 1994 by Florian  La Roche
  *
@@ -123,6 +123,7 @@
 static void arp_error_report(struct neighbour *neigh, struct sk_buff *skb);
 static void parp_redo(struct sk_buff *skb);
 
+static int arp_n_event(struct notifier_block *this, unsigned long event, void *ptr);
 static struct neigh_ops arp_generic_ops =
 {
 	AF_INET,
@@ -1140,6 +1141,30 @@
 }
 #endif
 
+
+struct notifier_block arp_netdev_notifier = {
+	arp_n_event,
+	NULL,
+	0
+};
+
+static int arp_n_event(struct notifier_block *this, unsigned long event, void *ptr)
+{
+	struct net_device *dev = (struct net_device *)(ptr) ;
+
+	switch (event) {
+	default:
+		break;
+	case NETDEV_CHANGEADDR:
+		neigh_changeaddr(&arp_tbl, dev);
+		break;
+	}
+
+	return NOTIFY_DONE;
+
+}
+
+
 /* Note, that it is not on notifier chain.
    It is necessary, that this routine was called after route cache will be
    flushed.
@@ -1174,6 +1199,7 @@
 #ifdef CONFIG_SYSCTL
 	neigh_sysctl_register(NULL, &arp_tbl.parms, NET_IPV4, NET_IPV4_NEIGH, "ipv4");
 #endif
+	register_netdevice_notifier(&arp_netdev_notifier);
 }
 
 

Re: [PATCH] Change MAC without bringing interface down

[David S. Miller]

On 18 Aug 2003 08:13:44 -0400
jamal <hadi@cyberus.ca> wrote:

> Editing the header may be the better approach if you have a few hundred 
> hhs already inserted (this was the issue i was faced with when VRRP was
> failing over when i was playing with it). I tried to flush the cache but
> it took longer to recover it. 

There is another reason I want the cache flushed.

Those hh cache entries are what ARP resolved to when we
had the previous MAC address, not the one we have now.

The ARPs may come back differently with the new MAC
address.

I _REALLY REALLY_ don't want to have to change the
->update() implementations.  Although one thing I
would consider is a new ->update_src() operation
and if this were NULL we flush the entries instead.

Re: [PATCH] Change MAC without bringing interface down

[jamal]

This is fine by me. I 'll let Tommi take the lead on it unless he is too
busy.

cheers,
jamal

On Mon, 2003-08-18 at 08:12, David S. Miller wrote:
> On 18 Aug 2003 08:13:44 -0400
> jamal <hadi@cyberus.ca> wrote:
> 
> > Editing the header may be the better approach if you have a few hundred 
> > hhs already inserted (this was the issue i was faced with when VRRP was
> > failing over when i was playing with it). I tried to flush the cache but
> > it took longer to recover it. 
> 
> There is another reason I want the cache flushed.
> 
> Those hh cache entries are what ARP resolved to when we
> had the previous MAC address, not the one we have now.
> 
> The ARPs may come back differently with the new MAC
> address.
> 
> I _REALLY REALLY_ don't want to have to change the
> ->update() implementations.  Although one thing I
> would consider is a new ->update_src() operation
> and if this were NULL we flush the entries instead.
> 
> 

Re: [PATCH] Change MAC without bringing interface down

[Tommi Virtanen]

On Mon, Aug 18, 2003 at 05:12:27AM -0700, David S. Miller wrote:
> There is another reason I want the cache flushed.

	Alright, here's an implementation that does that.
	Please comment.

	A single flood ping from uml->host survives MAC
	changes perfectly without dropping a single packet.
	(Can't test realworld performance right now.)

	Note I had to flush the route cache also, that was
	tricking me previously. Clearing neigh_table isn't
	enough, as the hh is reached from dst_entries also.

===== include/net/neighbour.h 1.1 vs edited =====
--- 1.1/include/net/neighbour.h	Tue Feb  5 19:39:48 2002
+++ edited/include/net/neighbour.h	Mon Aug 18 18:52:30 2003
@@ -180,6 +180,7 @@
 extern void			neigh_destroy(struct neighbour *neigh);
 extern int			__neigh_event_send(struct neighbour *neigh, struct sk_buff *skb);
 extern int			neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new, int override, int arp);
+extern void			neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev);
 extern int			neigh_ifdown(struct neigh_table *tbl, struct net_device *dev);
 extern int			neigh_resolve_output(struct sk_buff *skb);
 extern int			neigh_connected_output(struct sk_buff *skb);
===== net/core/neighbour.c 1.9 vs edited =====
--- 1.9/net/core/neighbour.c	Thu Jun 12 09:24:41 2003
+++ edited/net/core/neighbour.c	Mon Aug 18 18:52:41 2003
@@ -50,6 +50,7 @@
 static void neigh_app_notify(struct neighbour *n);
 #endif
 static int pneigh_ifdown(struct neigh_table *tbl, struct net_device *dev);
+void neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev);
 
 static int neigh_glbl_allocs;
 static struct neigh_table *neigh_tables;
@@ -167,6 +168,33 @@
 		dev_put(skb->dev);
 		kfree_skb(skb);
 	}
+}
+
+void neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev)
+{
+	int i;
+
+	write_lock_bh(&tbl->lock);
+
+	for (i=0; i<=NEIGH_HASHMASK; i++) {
+		struct neighbour *n, **np;
+
+		np = &tbl->hash_buckets[i];
+		while ((n = *np) != NULL) {
+			if (dev && n->dev != dev) {
+				np = &n->next;
+				continue;
+			}
+			*np = n->next;
+			write_lock_bh(&n->lock);
+			n->dead = 1;
+			neigh_del_timer(n);
+			write_unlock_bh(&n->lock);
+			neigh_release(n);
+		}
+	}
+
+        write_unlock_bh(&tbl->lock);
 }
 
 int neigh_ifdown(struct neigh_table *tbl, struct net_device *dev)
===== net/ipv4/arp.c 1.11 vs edited =====
--- 1.11/net/ipv4/arp.c	Fri Jun 27 09:03:01 2003
+++ edited/net/ipv4/arp.c	Mon Aug 18 18:51:10 2003
@@ -1212,6 +1212,28 @@
 }
 #endif
 
+static int arp_netdev_event(struct notifier_block *this, unsigned long event, void *ptr)
+{
+	struct net_device *dev = ptr;
+
+	switch (event) {
+	case NETDEV_CHANGEADDR:
+		neigh_changeaddr(&arp_tbl, dev);
+		rt_cache_flush(0);
+		break;
+	default:
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
+struct notifier_block arp_netdev_notifier = {
+	arp_netdev_event,
+	NULL,
+	0
+};
+
 /* Note, that it is not on notifier chain.
    It is necessary, that this routine was called after route cache will be
    flushed.
@@ -1243,6 +1265,7 @@
 #ifdef CONFIG_SYSCTL
 	neigh_sysctl_register(NULL, &arp_tbl.parms, NET_IPV4, NET_IPV4_NEIGH, "ipv4");
 #endif
+	register_netdevice_notifier(&arp_netdev_notifier);
 }
 
 
===== net/ipv6/ndisc.c 1.22 vs edited =====
--- 1.22/net/ipv6/ndisc.c	Tue Jun 24 02:21:28 2003
+++ edited/net/ipv6/ndisc.c	Mon Aug 18 18:51:40 2003
@@ -1336,6 +1336,28 @@
 	return 0;
 }
 
+static int ndisc_netdev_event(struct notifier_block *this, unsigned long event, void *ptr)
+{
+	struct net_device *dev = ptr;
+
+	switch (event) {
+	case NETDEV_CHANGEADDR:
+		neigh_changeaddr(&nd_tbl, dev);
+		rt_cache_flush(0);
+		break;
+	default:
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
+struct notifier_block ndisc_netdev_notifier = {
+	ndisc_netdev_event,
+	NULL,
+	0
+};
+
 int __init ndisc_init(struct net_proto_family *ops)
 {
 	struct sock *sk;
@@ -1377,6 +1399,7 @@
 	neigh_sysctl_register(NULL, &nd_tbl.parms, NET_IPV6, NET_IPV6_NEIGH, "ipv6");
 #endif
 
+	register_netdevice_notifier(&ndisc_netdev_notifier);
 	return 0;
 }
 

-- 
:(){ :|:&};:

Re: [PATCH] Change MAC without bringing interface down

[David S. Miller]

On Mon, 18 Aug 2003 19:04:22 +0300
Tommi Virtanen <tv@debian.org> wrote:

> ===== net/ipv6/ndisc.c 1.22 vs edited =====
> --- 1.22/net/ipv6/ndisc.c	Tue Jun 24 02:21:28 2003
> +++ edited/net/ipv6/ndisc.c	Mon Aug 18 18:51:40 2003
 ...
> +	case NETDEV_CHANGEADDR:
> +		neigh_changeaddr(&nd_tbl, dev);
> +		rt_cache_flush(0);

Don't flush the ipv4 routing cache from ipv6 please :-)

Re: [PATCH] Change MAC without bringing interface down

[Tommi Virtanen]

On Mon, Aug 18, 2003 at 09:06:54AM -0700, David S. Miller wrote:
> > ===== net/ipv6/ndisc.c 1.22 vs edited =====
> > --- 1.22/net/ipv6/ndisc.c	Tue Jun 24 02:21:28 2003
> > +++ edited/net/ipv6/ndisc.c	Mon Aug 18 18:51:40 2003
>  ...
> > +	case NETDEV_CHANGEADDR:
> > +		neigh_changeaddr(&nd_tbl, dev);
> > +		rt_cache_flush(0);
> 
> Don't flush the ipv4 routing cache from ipv6 please :-)

	Good point.

	I don't have an IPv6 test setup, so that side is..
	umm.. improvised ;)

	Here's the updated patch; I feel this is suitable for
	inclusion, performance can always be improved; remember, the
	current situation is either 10s..1min of no traffic passing
	because you had to bring down the interface, or cache lifetime
	of existing flows using the old MAC. This is a whole lot
	better.

===== include/net/neighbour.h 1.1 vs edited =====
--- 1.1/include/net/neighbour.h	Tue Feb  5 19:39:48 2002
+++ edited/include/net/neighbour.h	Mon Aug 18 18:52:30 2003
@@ -180,6 +180,7 @@
 extern void			neigh_destroy(struct neighbour *neigh);
 extern int			__neigh_event_send(struct neighbour *neigh, struct sk_buff *skb);
 extern int			neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new, int override, int arp);
+extern void			neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev);
 extern int			neigh_ifdown(struct neigh_table *tbl, struct net_device *dev);
 extern int			neigh_resolve_output(struct sk_buff *skb);
 extern int			neigh_connected_output(struct sk_buff *skb);
===== net/core/neighbour.c 1.9 vs edited =====
--- 1.9/net/core/neighbour.c	Thu Jun 12 09:24:41 2003
+++ edited/net/core/neighbour.c	Mon Aug 18 18:52:41 2003
@@ -50,6 +50,7 @@
 static void neigh_app_notify(struct neighbour *n);
 #endif
 static int pneigh_ifdown(struct neigh_table *tbl, struct net_device *dev);
+void neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev);
 
 static int neigh_glbl_allocs;
 static struct neigh_table *neigh_tables;
@@ -167,6 +168,33 @@
 		dev_put(skb->dev);
 		kfree_skb(skb);
 	}
+}
+
+void neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev)
+{
+	int i;
+
+	write_lock_bh(&tbl->lock);
+
+	for (i=0; i<=NEIGH_HASHMASK; i++) {
+		struct neighbour *n, **np;
+
+		np = &tbl->hash_buckets[i];
+		while ((n = *np) != NULL) {
+			if (dev && n->dev != dev) {
+				np = &n->next;
+				continue;
+			}
+			*np = n->next;
+			write_lock_bh(&n->lock);
+			n->dead = 1;
+			neigh_del_timer(n);
+			write_unlock_bh(&n->lock);
+			neigh_release(n);
+		}
+	}
+
+        write_unlock_bh(&tbl->lock);
 }
 
 int neigh_ifdown(struct neigh_table *tbl, struct net_device *dev)
===== net/ipv4/arp.c 1.11 vs edited =====
--- 1.11/net/ipv4/arp.c	Fri Jun 27 09:03:01 2003
+++ edited/net/ipv4/arp.c	Mon Aug 18 18:51:10 2003
@@ -1212,6 +1212,28 @@
 }
 #endif
 
+static int arp_netdev_event(struct notifier_block *this, unsigned long event, void *ptr)
+{
+	struct net_device *dev = ptr;
+
+	switch (event) {
+	case NETDEV_CHANGEADDR:
+		neigh_changeaddr(&arp_tbl, dev);
+		rt_cache_flush(0);
+		break;
+	default:
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
+struct notifier_block arp_netdev_notifier = {
+	arp_netdev_event,
+	NULL,
+	0
+};
+
 /* Note, that it is not on notifier chain.
    It is necessary, that this routine was called after route cache will be
    flushed.
@@ -1243,6 +1265,7 @@
 #ifdef CONFIG_SYSCTL
 	neigh_sysctl_register(NULL, &arp_tbl.parms, NET_IPV4, NET_IPV4_NEIGH, "ipv4");
 #endif
+	register_netdevice_notifier(&arp_netdev_notifier);
 }
 
 
===== net/ipv6/ndisc.c 1.22 vs edited =====
--- 1.22/net/ipv6/ndisc.c	Tue Jun 24 02:21:28 2003
+++ edited/net/ipv6/ndisc.c	Mon Aug 18 20:06:11 2003
@@ -1336,6 +1336,28 @@
 	return 0;
 }
 
+static int ndisc_netdev_event(struct notifier_block *this, unsigned long event, void *ptr)
+{
+	struct net_device *dev = ptr;
+
+	switch (event) {
+	case NETDEV_CHANGEADDR:
+		neigh_changeaddr(&nd_tbl, dev);
+		fib6_run_gc(0);
+		break;
+	default:
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
+struct notifier_block ndisc_netdev_notifier = {
+	ndisc_netdev_event,
+	NULL,
+	0
+};
+
 int __init ndisc_init(struct net_proto_family *ops)
 {
 	struct sock *sk;
@@ -1377,6 +1399,7 @@
 	neigh_sysctl_register(NULL, &nd_tbl.parms, NET_IPV6, NET_IPV6_NEIGH, "ipv6");
 #endif
 
+	register_netdevice_notifier(&ndisc_netdev_notifier);
 	return 0;
 }
 


-- 
:(){ :|:&};:

Re: [PATCH] Change MAC without bringing interface down

[Arnaldo Carvalho de Melo]

Em Mon, Aug 18, 2003 at 08:11:44PM +0300, Tommi Virtanen escreveu:
> +void neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev)
> +{
> +	int i;
> +
> +	write_lock_bh(&tbl->lock);
> +
> +	for (i=0; i<=NEIGH_HASHMASK; i++) {


spaces before and after operators, please

> +struct notifier_block arp_netdev_notifier = {
> +	arp_netdev_event,
> +	NULL,
> +	0
> +};

c99 style please, that is:

struct notifier_block arp_netdev_notifier = {
	.notifier_call = arp_netdev_event,
}

> +struct notifier_block ndisc_netdev_notifier = {
> +	ndisc_netdev_event,
> +	NULL,
> +	0
> +};

ditto


Thanks,

- Arnaldo

Re: [PATCH] Change MAC without bringing interface down

[Tommi Virtanen]

On Mon, Aug 18, 2003 at 03:51:53PM -0300, Arnaldo Carvalho de Melo wrote:
> > +	for (i=0; i<=NEIGH_HASHMASK; i++) {
> spaces before and after operators, please

> > +struct notifier_block arp_netdev_notifier = {
> > +	arp_netdev_event,
> > +	NULL,
> > +	0
> > +};
> c99 style please, that is:

> > +struct notifier_block ndisc_netdev_notifier = {
> > +	ndisc_netdev_event,
> > +	NULL,
> > +	0
> > +};
> ditto

	Here's an updated patch with those nits picked.  Those were
	all cut-and-pasted from elsewhere, so if you want perfection,
	there's lots of those to fix.

===== include/net/neighbour.h 1.1 vs edited =====
--- 1.1/include/net/neighbour.h	Tue Feb  5 19:39:48 2002
+++ edited/include/net/neighbour.h	Mon Aug 18 18:52:30 2003
@@ -180,6 +180,7 @@
 extern void			neigh_destroy(struct neighbour *neigh);
 extern int			__neigh_event_send(struct neighbour *neigh, struct sk_buff *skb);
 extern int			neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new, int override, int arp);
+extern void			neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev);
 extern int			neigh_ifdown(struct neigh_table *tbl, struct net_device *dev);
 extern int			neigh_resolve_output(struct sk_buff *skb);
 extern int			neigh_connected_output(struct sk_buff *skb);
===== net/core/neighbour.c 1.9 vs edited =====
--- 1.9/net/core/neighbour.c	Thu Jun 12 09:24:41 2003
+++ edited/net/core/neighbour.c	Tue Aug 19 08:45:14 2003
@@ -50,6 +50,7 @@
 static void neigh_app_notify(struct neighbour *n);
 #endif
 static int pneigh_ifdown(struct neigh_table *tbl, struct net_device *dev);
+void neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev);
 
 static int neigh_glbl_allocs;
 static struct neigh_table *neigh_tables;
@@ -167,6 +168,33 @@
 		dev_put(skb->dev);
 		kfree_skb(skb);
 	}
+}
+
+void neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev)
+{
+	int i;
+
+	write_lock_bh(&tbl->lock);
+
+	for (i=0; i <= NEIGH_HASHMASK; i++) {
+		struct neighbour *n, **np;
+
+		np = &tbl->hash_buckets[i];
+		while ((n = *np) != NULL) {
+			if (dev && n->dev != dev) {
+				np = &n->next;
+				continue;
+			}
+			*np = n->next;
+			write_lock_bh(&n->lock);
+			n->dead = 1;
+			neigh_del_timer(n);
+			write_unlock_bh(&n->lock);
+			neigh_release(n);
+		}
+	}
+
+        write_unlock_bh(&tbl->lock);
 }
 
 int neigh_ifdown(struct neigh_table *tbl, struct net_device *dev)
===== net/ipv4/arp.c 1.11 vs edited =====
--- 1.11/net/ipv4/arp.c	Fri Jun 27 09:03:01 2003
+++ edited/net/ipv4/arp.c	Tue Aug 19 08:45:36 2003
@@ -1212,6 +1212,26 @@
 }
 #endif
 
+static int arp_netdev_event(struct notifier_block *this, unsigned long event, void *ptr)
+{
+	struct net_device *dev = ptr;
+
+	switch (event) {
+	case NETDEV_CHANGEADDR:
+		neigh_changeaddr(&arp_tbl, dev);
+		rt_cache_flush(0);
+		break;
+	default:
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
+struct notifier_block arp_netdev_notifier = {
+	.notifier_call = arp_netdev_event,
+};
+
 /* Note, that it is not on notifier chain.
    It is necessary, that this routine was called after route cache will be
    flushed.
@@ -1243,6 +1263,7 @@
 #ifdef CONFIG_SYSCTL
 	neigh_sysctl_register(NULL, &arp_tbl.parms, NET_IPV4, NET_IPV4_NEIGH, "ipv4");
 #endif
+	register_netdevice_notifier(&arp_netdev_notifier);
 }
 
 
===== net/ipv6/ndisc.c 1.22 vs edited =====
--- 1.22/net/ipv6/ndisc.c	Tue Jun 24 02:21:28 2003
+++ edited/net/ipv6/ndisc.c	Tue Aug 19 08:45:53 2003
@@ -1336,6 +1336,26 @@
 	return 0;
 }
 
+static int ndisc_netdev_event(struct notifier_block *this, unsigned long event, void *ptr)
+{
+	struct net_device *dev = ptr;
+
+	switch (event) {
+	case NETDEV_CHANGEADDR:
+		neigh_changeaddr(&nd_tbl, dev);
+		fib6_run_gc(0);
+		break;
+	default:
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
+struct notifier_block ndisc_netdev_notifier = {
+	.notifier_call = ndisc_netdev_event,
+};
+
 int __init ndisc_init(struct net_proto_family *ops)
 {
 	struct sock *sk;
@@ -1377,6 +1397,7 @@
 	neigh_sysctl_register(NULL, &nd_tbl.parms, NET_IPV6, NET_IPV6_NEIGH, "ipv6");
 #endif
 
+	register_netdevice_notifier(&ndisc_netdev_notifier);
 	return 0;
 }
 

-- 
:(){ :|:&};:

Re: [PATCH] Change MAC without bringing interface down

[Arnaldo Carvalho de Melo]

Em Tue, Aug 19, 2003 at 08:50:34AM +0300, Tommi Virtanen escreveu:
> 	Here's an updated patch with those nits picked.  Those were
> 	all cut-and-pasted from elsewhere, so if you want perfection,
> 	there's lots of those to fix.

Thank you. And yes, I wish there was a consistent style, and have been
fixing these kinds of nits for years :-)

- Arnaldo

Re: [PATCH] Change MAC without bringing interface down

[David S. Miller]

On Tue, 19 Aug 2003 08:50:34 +0300
Tommi Virtanen <tv@debian.org> wrote:

> 	Here's an updated patch with those nits picked.  Those were
> 	all cut-and-pasted from elsewhere, so if you want perfection,
> 	there's lots of those to fix.

This patch is fine, I will apply it.

Thank you.

Re: [PATCH] Change MAC without bringing interface down

[Tommi Virtanen]

On Mon, Aug 18, 2003 at 04:19:11AM -0700, David S. Miller wrote:
> On Mon, 18 Aug 2003 12:13:12 +0300
> > 	1. makes eth_header_cache_update also update the source MAC in
> > 	   hh_cache.
> > 
> > 	2. adds a NETDEV_CHANGEADDR notifier in net/core/neighbour.c
> > 	   that walks through all the neighbour entries and updates
> > 	   the hh_cache's for all the neighbour entries related to
> > 	   given device.
> I would suggest to fix this by instead flushing the hh
> entries (just like neigh_destroy() does).

	This may be a dumb question, but how do I do that?

	If I set them to neigh_blackhole, like neigh_destroy() does,
	e.g. already running gets -ENETDOWN. That's now true and it's
	not nice. I'm having trouble seeing how to "disable" the
	neighbour entry, so it won't be used again.

	If I use neigh_suspect() in an attempt to mark the entry as
	"expired", I see traffic using the "old" MAC as source after a
	while. Apparently, it gets confirmed and then the old MAC
	is used again.

> This way, we don't have to fix up every single header cache
> implementation, that's painful :(

	I agree with that, though I am only interested in ethernet ;)

-- 
:(){ :|:&};:

Re: [PATCH] Change MAC without bringing interface down

[David S. Miller]

On Mon, 18 Aug 2003 17:24:55 +0300
Tommi Virtanen <tv@debian.org> wrote:

> > I would suggest to fix this by instead flushing the hh
> > entries (just like neigh_destroy() does).
> 
> 	This may be a dumb question, but how do I do that?
> 
> 	If I set them to neigh_blackhole, like neigh_destroy() does,
> 	e.g. already running gets -ENETDOWN. That's now true and it's
> 	not nice. I'm having trouble seeing how to "disable" the
> 	neighbour entry, so it won't be used again.

You have to also remove them from the list, like neigh_destroy()
does, are you doing that as well?

Re: [PATCH] Change MAC without bringing interface down

[jamal]

If you are going that approach then please if you have time do some
measurement on editing vs flushing with X number of cache entries.
go with X being a max of 200.

cheers,
jamal

On Mon, 2003-08-18 at 10:24, Tommi Virtanen wrote:

> 
> > This way, we don't have to fix up every single header cache
> > implementation, that's painful :(
> 
> 	I agree with that, though I am only interested in ethernet ;)