Skip to contentSkip to conversation

Change MAC without bringing interface down

PATCH In review Tommi Virtanen 4 participants 15 messages Aug 18, 2003 – Aug 19, 2003

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

Tommi Virtanen

author
#0 lore raw

PATCH 1/1 Change MAC without bringing interface down

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 =====

include/net/neighbour.h +2 -0
@@ -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 =====

net/core/dev.c +2 -0
@@ -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 =====

net/core/neighbour.c +43 -0

46 lines not on this page · raw message

===== net/ethernet/eth.c 1.3 vs edited =====

net/ethernet/eth.c +7 -2
@@ -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);
 }
Signature · 1 line
:(){ :|:&};:
#1 lore raw
Quoted 11 lines · On Mon, 18 Aug 2003 12:13:12 +0300 Tommi Virtanen <[email protected]> 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 :(

#2 lore raw

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

Quoted 13 lines · David S. Miller
Quoted 10 lines · On Mon, 18 Aug 2003 12:13:12 +0300 Tommi Virtanen <[email protected]> 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.

Quoted 9 lines
Quoted 1 line

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 :(

include/net/patchedneighbour.h renamed from include/net/neighbour.h +1 -0
@@ -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);
net/core/patchedneighbour.c renamed from net/core/neighbour.c +55 -2

101 lines not on this page · raw message

net/ipv4/patchedarp.c renamed from net/ipv4/arp.c +27 -1
@@ -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);
 }
#3 lore raw
Quoted 4 lines · On 18 Aug 2003 08:13:44 -0400 jamal <[email protected]> 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.

#4 lore raw

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

cheers,
jamal

Quoted 22 lines · David S. Miller
Quoted 4 lines · On 18 Aug 2003 08:13:44 -0400 jamal <[email protected]> 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.

author
#5 lore raw
In reply to David S. Miller
Quoted 1 line · David S. Miller

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 =====

include/net/neighbour.h +1 -0
@@ -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 =====

net/core/neighbour.c +28 -0
@@ -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 =====

net/ipv4/arp.c +23 -0
@@ -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 =====

net/ipv6/ndisc.c +23 -0
@@ -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;
 }
Signature · 1 line
:(){ :|:&};:
#6 lore raw
Quoted 3 lines · On Mon, 18 Aug 2003 19:04:22 +0300 Tommi Virtanen <[email protected]> wrote:

===== net/ipv6/ndisc.c 1.22 vs edited =====

net/ipv6/ndisc.c +0 -0

...

net/ipv6/ndisc.c line 1346

Quoted 3 lines
net/ipv6/ndisc.c +3 -0
+	case NETDEV_CHANGEADDR:
+		neigh_changeaddr(&nd_tbl, dev);
+		rt_cache_flush(0);

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

author
#7 lore raw
Quoted 9 lines · David S. Miller
Quoted 3 lines

===== net/ipv6/ndisc.c 1.22 vs edited =====

net/ipv6/ndisc.c +0 -0

...

net/ipv6/ndisc.c

Quoted 3 lines
net/ipv6/ndisc.c +3 -0
+	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 =====

include/net/neighbour.h +1 -0
@@ -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 =====

net/core/neighbour.c +28 -0
@@ -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 =====

net/ipv4/arp.c +23 -0
@@ -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 =====

net/ipv6/ndisc.c +23 -0
@@ -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;
 }
Signature · 1 line
:(){ :|:&};:
#8 lore raw

Em Mon, Aug 18, 2003 at 08:11:44PM +0300, Tommi Virtanen escreveu:

net/core/neighbour.c line 179

Quoted 7 lines
net/core/neighbour.c +7 -0
+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

net/ipv4/arp.c line 1235

Quoted 5 lines
net/ipv4/arp.c +5 -0
+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,
}

net/ipv6/ndisc.c line 1359

Quoted 5 lines
net/ipv6/ndisc.c +5 -0
+struct notifier_block ndisc_netdev_notifier = {
+	ndisc_netdev_event,
+	NULL,
+	0
+};

ditto

Thanks,

- Arnaldo

author
#9 lore raw
Quoted 10 lines · Arnaldo Carvalho de Melo

Review

Quoted 1 line
(quoted diff) +1 -0
+	for (i=0; i<=NEIGH_HASHMASK; i++) {

spaces before and after operators, please

Review

Quoted 5 lines
(quoted diff) +5 -0
+struct notifier_block arp_netdev_notifier = {
+	arp_netdev_event,
+	NULL,
+	0
+};

c99 style please, that is:

Quoted 6 lines

Review

Quoted 5 lines
(quoted diff) +5 -0
+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 =====

include/net/neighbour.h +1 -0
@@ -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 =====

net/core/neighbour.c +28 -0
@@ -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 =====

net/ipv4/arp.c +21 -0
@@ -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 =====

net/ipv6/ndisc.c +21 -0
@@ -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;
 }
Signature · 1 line
:(){ :|:&};:
#10 lore raw

Em Tue, Aug 19, 2003 at 08:50:34AM +0300, Tommi Virtanen escreveu:

Quoted 3 lines

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

#11 lore raw
In reply to Tommi Virtanen
Quoted 3 lines · On Tue, 19 Aug 2003 08:50:34 +0300 Tommi Virtanen <[email protected]> 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.

author
#12 lore raw
In reply to David S. Miller
Quoted 10 lines · David S. Miller

On Mon, 18 Aug 2003 12:13:12 +0300

Quoted 7 lines

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.

Quoted 2 lines

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

Signature · 1 line
:(){ :|:&};:
#13 lore raw
Quoted 9 lines · On Mon, 18 Aug 2003 17:24:55 +0300 Tommi Virtanen <[email protected]> wrote:
Quoted 2 lines

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?

#14 lore raw
In reply to Tommi Virtanen

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

Quoted 5 lines · Tommi Virtanen
Quoted 3 lines

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