diff mbox

[1/1] ipv4: arp: update neighbour address when a gratuitous arp is received and arp_accept is set

Message ID 1387826492-25413-1-git-send-email-noureddine@aristanetworks.com
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Salam Noureddine Dec. 23, 2013, 7:21 p.m. UTC 
Gratuitous arp packets are useful in switchover scenarios to update
client arp tables as quickly as possible. Currently, the mac address
of a neighbour is only updated after a locktime period has elapsed
since the last update. In most use cases such delays are unacceptable
for network admins. Moreover, the "updated" field of the neighbour
stucture doesn't record the last time the address of a neighbour
changed but records any change that happens to theneighbour. This is
clearly a bug since locktime uses that field as meaning "addr_updated".
With this observation, I was able to perpetuate a stale address by
sending a stream of gratuitous arp packets spaced less than locktime
apart. With this change the address is updated when a gratuitous arp
is received and the arp_accept sysctl is set.

Signed-off-by: Salam Noureddine <noureddine@aristanetworks.com>
---
 net/ipv4/arp.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

Comments

Hannes Frederic Sowa Dec. 23, 2013, 10:30 p.m. UTC | #1 
On Mon, Dec 23, 2013 at 11:21:32AM -0800, Salam Noureddine wrote:
> @@ -910,7 +913,9 @@ static int arp_process(struct sk_buff *skb)
>  		   agents are active. Taking the first reply prevents
>  		   arp trashing and chooses the fastest router.
>  		 */
> -		override = time_after(jiffies, n->updated + n->parms->locktime);
> +		override = time_after(jiffies,
> +				      n->updated + n->parms->locktime) ||
> +			   is_garp;

Your patch is fine, but you need to rebase it again on net-next.
n->parms->locktime has been since updated to NEIGH_VAR(n->parms, LOCKTIME)
and thus this patch does not apply to net-next any more.

Greetings,

  Hannes

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 7808093..c791d09 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -728,6 +728,7 @@  static int arp_process(struct sk_buff *skb)
 	int addr_type;
 	struct neighbour *n;
 	struct net *net = dev_net(dev);
+	bool is_garp = false;
 
 	/* arp_rcv below verifies the ARP header and verifies the device
 	 * is ARP'able.
@@ -894,10 +895,12 @@  static int arp_process(struct sk_buff *skb)
 		   It is possible, that this option should be enabled for some
 		   devices (strip is candidate)
 		 */
+		is_garp = arp->ar_op == htons(ARPOP_REQUEST) && tip == sip &&
+			  inet_addr_type(net, sip) == RTN_UNICAST;
+
 		if (n == NULL &&
-		    (arp->ar_op == htons(ARPOP_REPLY) ||
-		     (arp->ar_op == htons(ARPOP_REQUEST) && tip == sip)) &&
-		    inet_addr_type(net, sip) == RTN_UNICAST)
+		    ((arp->ar_op == htons(ARPOP_REPLY)  &&
+		      inet_addr_type(net, sip) == RTN_UNICAST) || is_garp))
 			n = __neigh_lookup(&arp_tbl, &sip, dev, 1);
 	}
 
@@ -910,7 +913,9 @@  static int arp_process(struct sk_buff *skb)
 		   agents are active. Taking the first reply prevents
 		   arp trashing and chooses the fastest router.
 		 */
-		override = time_after(jiffies, n->updated + n->parms->locktime);
+		override = time_after(jiffies,
+				      n->updated + n->parms->locktime) ||
+			   is_garp;
 
 		/* Broadcast replies and request packets
 		   do not assert neighbour reachability.