[RFC] ipv4: support for request type gratuitous ARP

[RFC] ipv4: support for request type gratuitous ARP

[Octavian Purdila]

Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
---

I've noticed that even though we currently support response type gratuitous ARP
[response type, source mac, dest mac, source IP, source IP] *with a clean ARP table*
we do not support the request type [request type, source mac, ff:ff:ff:ff:ff:ff, source IP, source IP].

This patch makes request type work as well, but RFC2002 says that gratuitous ARP
(both request and response) must update the ARP table *if* the IP already
exists in the table:

          In either case, for a gratuitous ARP, the ARP packet MUST be
          transmitted as a local broadcast packet on the local link.  As
          specified in [16], any node receiving any ARP packet (Request or
          Reply) MUST update its local ARP cache with the Sender Protocol
          and Hardware Addresses in the ARP packet, if the receiving node
          has an entry for that IP address already in its ARP cache.  This
          requirement in the ARP protocol applies even for ARP Request
          packets, and for ARP Reply packets that do not match any ARP
          Request transmitted by the receiving node [16].

so, I am not sure if this is right. But current behavior for response type
gratuitous ARP does not seem to be covered by the RFC either.

 net/ipv4/arp.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index c95cd93..81ef2d5 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -811,8 +811,13 @@ static int arp_process(struct sk_buff *skb)
 		goto out;
 	}
 
-	if (arp->ar_op == htons(ARPOP_REQUEST) &&
-	    ip_route_input(skb, tip, sip, 0, dev) == 0) {
+	if (arp->ar_op == htons(ARPOP_REQUEST)) {
+		/* gratuitous ARP */
+		if (tip == sip) {
+			n = neigh_event_ns(&arp_tbl, sha, &sip, dev);
+			goto update;
+		} else if (ip_route_input(skb, tip, sip, 0, dev) != 0)
+			goto update_lookup;
 
 		rt = skb_rtable(skb);
 		addr_type = rt->rt_type;
@@ -853,6 +858,7 @@ static int arp_process(struct sk_buff *skb)
 		}
 	}
 
+update_lookup:
 	/* Update our ARP tables */
 
 	n = __neigh_lookup(&arp_tbl, &sip, dev, 0);
@@ -868,6 +874,7 @@ static int arp_process(struct sk_buff *skb)
 			n = __neigh_lookup(&arp_tbl, &sip, dev, 1);
 	}
 
+update:
 	if (n) {
 		int state = NUD_REACHABLE;
 		int override;
-- 
1.5.6.5

Re: [RFC] ipv4: support for request type gratuitous ARP

[Laurent Chavey]

Reviewed-by: Laurent Chavey <chavey@google.com>

On Mon, Jan 4, 2010 at 2:04 PM, Octavian Purdila <opurdila@ixiacom.com> wrote:
>
> Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
> ---
>
> I've noticed that even though we currently support response type gratuitous ARP
> [response type, source mac, dest mac, source IP, source IP] *with a clean ARP table*
> we do not support the request type [request type, source mac, ff:ff:ff:ff:ff:ff, source IP, source IP].
>
> This patch makes request type work as well, but RFC2002 says that gratuitous ARP
> (both request and response) must update the ARP table *if* the IP already
> exists in the table:
>
>          In either case, for a gratuitous ARP, the ARP packet MUST be
>          transmitted as a local broadcast packet on the local link.  As
>          specified in [16], any node receiving any ARP packet (Request or
>          Reply) MUST update its local ARP cache with the Sender Protocol
>          and Hardware Addresses in the ARP packet, if the receiving node
>          has an entry for that IP address already in its ARP cache.  This
>          requirement in the ARP protocol applies even for ARP Request
>          packets, and for ARP Reply packets that do not match any ARP
>          Request transmitted by the receiving node [16].
>
> so, I am not sure if this is right. But current behavior for response type
> gratuitous ARP does not seem to be covered by the RFC either.
>
>  net/ipv4/arp.c |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index c95cd93..81ef2d5 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -811,8 +811,13 @@ static int arp_process(struct sk_buff *skb)
>                goto out;
>        }
>
> -       if (arp->ar_op == htons(ARPOP_REQUEST) &&
> -           ip_route_input(skb, tip, sip, 0, dev) == 0) {
> +       if (arp->ar_op == htons(ARPOP_REQUEST)) {
> +               /* gratuitous ARP */
> +               if (tip == sip) {
> +                       n = neigh_event_ns(&arp_tbl, sha, &sip, dev);
> +                       goto update;
> +               } else if (ip_route_input(skb, tip, sip, 0, dev) != 0)
> +                       goto update_lookup;
>
>                rt = skb_rtable(skb);
>                addr_type = rt->rt_type;
> @@ -853,6 +858,7 @@ static int arp_process(struct sk_buff *skb)
>                }
>        }
>
> +update_lookup:
>        /* Update our ARP tables */
>
>        n = __neigh_lookup(&arp_tbl, &sip, dev, 0);
> @@ -868,6 +874,7 @@ static int arp_process(struct sk_buff *skb)
>                        n = __neigh_lookup(&arp_tbl, &sip, dev, 1);
>        }
>
> +update:
>        if (n) {
>                int state = NUD_REACHABLE;
>                int override;
> --
> 1.5.6.5
> --
> 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
>

Re: [RFC] ipv4: support for request type gratuitous ARP

[David Miller]

From: Octavian Purdila <opurdila@ixiacom.com>
Date: Tue, 5 Jan 2010 00:04:44 +0200

> 
> Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
> ---
> 
> I've noticed that even though we currently support response type gratuitous ARP
> [response type, source mac, dest mac, source IP, source IP] *with a clean ARP table*
> we do not support the request type [request type, source mac, ff:ff:ff:ff:ff:ff, source IP, source IP].

Please don't submit your patches in this manner.

All of these relevant, interesting, details belong in the commit
message.  But any text you place aftr the "---" line will be omitted
from the commit message when your patch is applied by automated GIT
tools.

I've done some research and I'm happy to apply your patch to
net-next-2.6 once it is submitted properly.

In fact we need to do some more research in this area because
generally we should more mimick the processing order of ARP prescribed
in the RFC.  In particular we should test the operation code lastly,
which would avoid these kinds of inconsistencies.

I'm worried though about security issues as well, as we make more the
acceptance more and more liberal, it becomes that much easier for
machines on the local network to poison ARP entries and use that to
either accept all traffic destined for a particular node or simply
deny that node access to the network.

In particular the kernel currently explicitly does not accept
unsolicited ARP, and this is controlled by the ARP_ACCEPT per-device
option.

	if (IPV4_DEVCONF_ALL(dev_net(dev), ARP_ACCEPT)) {
		/* Unsolicited ARP is not accepted by default.
		   It is possible, that this option should be enabled for some
		   devices (strip is candidate)
		 */
...

Re: [RFC] ipv4: support for request type gratuitous ARP

[Octavian Purdila]

On Sunday 10 January 2010 23:21:05 you wrote:
> From: Octavian Purdila <opurdila@ixiacom.com>
> Date: Tue, 5 Jan 2010 00:04:44 +0200
> 
> > Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
> > ---
> >
> > I've noticed that even though we currently support response type
> > gratuitous ARP [response type, source mac, dest mac, source IP, source
> > IP] *with a clean ARP table* we do not support the request type [request
> > type, source mac, ff:ff:ff:ff:ff:ff, source IP, source IP].
> 
> Please don't submit your patches in this manner.
> 
> All of these relevant, interesting, details belong in the commit
> message.  But any text you place aftr the "---" line will be omitted
> from the commit message when your patch is applied by automated GIT
> tools.
> 

Got it.

> I've done some research and I'm happy to apply your patch to
> net-next-2.6 once it is submitted properly.
> 

Thanks, I'll resubmitted it properly in a bit.

> In fact we need to do some more research in this area because
> generally we should more mimick the processing order of ARP prescribed
> in the RFC.  In particular we should test the operation code lastly,
> which would avoid these kinds of inconsistencies.
> 
> I'm worried though about security issues as well, as we make more the
> acceptance more and more liberal, it becomes that much easier for
> machines on the local network to poison ARP entries and use that to
> either accept all traffic destined for a particular node or simply
> deny that node access to the network.
> 
> In particular the kernel currently explicitly does not accept
> unsolicited ARP, and this is controlled by the ARP_ACCEPT per-device
> option.
> 
> 	if (IPV4_DEVCONF_ALL(dev_net(dev), ARP_ACCEPT)) {
> 		/* Unsolicited ARP is not accepted by default.
> 		   It is possible, that this option should be enabled for some
> 		   devices (strip is candidate)
> 		 */
> ...
> 

Good point, I'll try to respin the patch to do this check for the request type 
gratuitous ARP as well.

Thanks!

[PATCH v2] ipv4: support for request type gratuitous ARP

[Octavian Purdila]

Even though we currently support response type gratuitous ARP
[response type, source mac, dest mac, source IP, source IP] we do not
support the request type [request type, source mac, ff:ff:ff:ff:ff:ff,
source IP, source IP].

RFC2002 says:

     In either case, for a gratuitous ARP, the ARP packet MUST be
     transmitted as a local broadcast packet on the local link.  As
     specified in [16], any node receiving any ARP packet (Request or
     Reply) MUST update its local ARP cache with the Sender Protocol
     and Hardware Addresses in the ARP packet, if the receiving node
     has an entry for that IP address already in its ARP cache.  This
     requirement in the ARP protocol applies even for ARP Request
     packets, and for ARP Reply packets that do not match any ARP
     Request transmitted by the receiving node [16].

This patch adds support for request type gratuitous ARP, but due to
security reasons the ARP table is updated only if the per device
ARP_ACCEPT option is enabled.

Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
---
 net/ipv4/arp.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index c95cd93..588fed8 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -811,8 +811,13 @@ static int arp_process(struct sk_buff *skb)
 		goto out;
 	}
 
-	if (arp->ar_op == htons(ARPOP_REQUEST) &&
-	    ip_route_input(skb, tip, sip, 0, dev) == 0) {
+	if (arp->ar_op == htons(ARPOP_REQUEST)) {
+		/* gratuitous ARP */
+		if (tip == sip && IPV4_DEVCONF_ALL(dev_net(dev), ARP_ACCEPT)) {
+			n = neigh_event_ns(&arp_tbl, sha, &sip, dev);
+			goto update;
+		} else if (ip_route_input(skb, tip, sip, 0, dev) != 0)
+			goto update_lookup;
 
 		rt = skb_rtable(skb);
 		addr_type = rt->rt_type;
@@ -853,6 +858,7 @@ static int arp_process(struct sk_buff *skb)
 		}
 	}
 
+update_lookup:
 	/* Update our ARP tables */
 
 	n = __neigh_lookup(&arp_tbl, &sip, dev, 0);
@@ -868,6 +874,7 @@ static int arp_process(struct sk_buff *skb)
 			n = __neigh_lookup(&arp_tbl, &sip, dev, 1);
 	}
 
+update:
 	if (n) {
 		int state = NUD_REACHABLE;
 		int override;
-- 
1.5.6.5

Re: [PATCH v2] ipv4: support for request type gratuitous ARP

[Julian Anastasov]

	Hello,

On Sun, 17 Jan 2010, Octavian Purdila wrote:

> Even though we currently support response type gratuitous ARP
> [response type, source mac, dest mac, source IP, source IP] we do not
> support the request type [request type, source mac, ff:ff:ff:ff:ff:ff,
> source IP, source IP].
> 
> RFC2002 says:
> 
>      In either case, for a gratuitous ARP, the ARP packet MUST be
>      transmitted as a local broadcast packet on the local link.  As
>      specified in [16], any node receiving any ARP packet (Request or
>      Reply) MUST update its local ARP cache with the Sender Protocol
>      and Hardware Addresses in the ARP packet, if the receiving node
>      has an entry for that IP address already in its ARP cache.  This
>      requirement in the ARP protocol applies even for ARP Request
>      packets, and for ARP Reply packets that do not match any ARP
>      Request transmitted by the receiving node [16].
> 
> This patch adds support for request type gratuitous ARP, but due to
> security reasons the ARP table is updated only if the per device
> ARP_ACCEPT option is enabled.

	May be I'm missing something but the description and
the changed code do not match. You claim this patch now supports
dest mac ff:ff:ff:ff:ff:ff while without the patch the
'skb->pkt_type != PACKET_HOST' check usually updates the
existing entry with state NUD_STALE (neigh_update). What
exactly is not handled correctly?

	I think, the existing entry should be updated
properly even without the patch, when sip=tip. May be
there is unwanted ARP reply? It is not clear from this email. Can
you give example why we need to override ip_route_input? What is
the case that there is existing entry for sip that we want
to update but for some reason we do not like ip_route_input
result and entry is not updated? I don't think, ip_route_input
will do something different when sip=tip=unicast address.
This is not much different from the case when tip is IP
from the same subnet as sip. There are only 2 'goto out' but
both places update the entry depending on some policy.

	Also, you call neigh_event_ns which creates new entry while
RFC talks about updating existing entry: "if the receiving node
has an entry for that IP address already in its ARP cache".
We don't want to create new entry. After neigh_event_ns you
jump again to update (2nd unneeded update).

	As for arp_accept, it only allows creating new ARP
entries on received reply, but when disabled it does not
prevent updates for existing entries.

	For me, I see only one problem in kernel, may be
you should name this change "ARP PROXY: Ignore
gratuitous ARP requests" and you just need to add one line:

        if (arp->ar_op == htons(ARPOP_REQUEST) &&
+	    sip != tip &&
            ip_route_input(skb, tip, sip, 0, dev) == 0) {

	Because ARP PROXY works with tip, it does not need
to work just for sip. Still, existing sip will be updated
but no reply will be sent which is more possible when
the request is a broadcast. As result, for gratuitous ARP
requests and replies what we will do is just an update without
using routing or creating unneeded proxy entries.

> Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
> ---
>  net/ipv4/arp.c |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index c95cd93..588fed8 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -811,8 +811,13 @@ static int arp_process(struct sk_buff *skb)
>  		goto out;
>  	}
>  
> -	if (arp->ar_op == htons(ARPOP_REQUEST) &&
> -	    ip_route_input(skb, tip, sip, 0, dev) == 0) {
> +	if (arp->ar_op == htons(ARPOP_REQUEST)) {
> +		/* gratuitous ARP */
> +		if (tip == sip && IPV4_DEVCONF_ALL(dev_net(dev), ARP_ACCEPT)) {
> +			n = neigh_event_ns(&arp_tbl, sha, &sip, dev);
> +			goto update;
> +		} else if (ip_route_input(skb, tip, sip, 0, dev) != 0)
> +			goto update_lookup;
>  
>  		rt = skb_rtable(skb);
>  		addr_type = rt->rt_type;
> @@ -853,6 +858,7 @@ static int arp_process(struct sk_buff *skb)
>  		}
>  	}
>  
> +update_lookup:
>  	/* Update our ARP tables */
>  
>  	n = __neigh_lookup(&arp_tbl, &sip, dev, 0);
> @@ -868,6 +874,7 @@ static int arp_process(struct sk_buff *skb)
>  			n = __neigh_lookup(&arp_tbl, &sip, dev, 1);
>  	}
>  
> +update:
>  	if (n) {
>  		int state = NUD_REACHABLE;
>  		int override;
> 

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH v2] ipv4: support for request type gratuitous ARP

[Octavian Purdila]

[-- Attachment #1: Type: Text/Plain, Size: 3084 bytes --]

On Sunday 17 January 2010 15:45:45 you wrote:

> 	May be I'm missing something but the description and
> the changed code do not match. You claim this patch now supports
> dest mac ff:ff:ff:ff:ff:ff while without the patch the
> 'skb->pkt_type != PACKET_HOST' check usually updates the
> existing entry with state NUD_STALE (neigh_update). What
> exactly is not handled correctly?
> 
> 	I think, the existing entry should be updated
> properly even without the patch, when sip=tip. May be
> there is unwanted ARP reply? It is not clear from this email. Can
> you give example why we need to override ip_route_input? What is
> the case that there is existing entry for sip that we want
> to update but for some reason we do not like ip_route_input
> result and entry is not updated? I don't think, ip_route_input
> will do something different when sip=tip=unicast address.
> This is not much different from the case when tip is IP
> from the same subnet as sip. There are only 2 'goto out' but
> both places update the entry depending on some policy.
> 
> 	Also, you call neigh_event_ns which creates new entry while
> RFC talks about updating existing entry: "if the receiving node
> has an entry for that IP address already in its ARP cache".
> We don't want to create new entry. After neigh_event_ns you
> jump again to update (2nd unneeded update).
> 
> 	As for arp_accept, it only allows creating new ARP
> entries on received reply, but when disabled it does not
> prevent updates for existing entries.
> 

Hi Julian,

Thanks for looking at this patch ! You are indeed correct, we don't need to do anything special when we want to update an existing ARP entry.

But I also want to be able to create a new ARP entry not only update an existing one (and we can do that with Linux today, but only with response type grat arp).

How about this new version?

[PATCH] ipv4: allow warming up the ARP cache with request type gratuitous ARP

If the per device ARP_ACCEPT option is enabled, currently we only allow
creating new ARP cache entries for response type gratuitous ARP.

Allowing gratuitous ARP to create new ARP entries (not only to update
existing ones) is useful when we want to avoid unnecessary delays for
the first packet of a stream.

This patch allows request type gratuitous ARP to create new ARP cache
entries as well. This is useful when we want to warm-up the ARP cache
entries for a large number of hosts on the same LAN.

Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
---
 net/ipv4/arp.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 0787092..1940b4d 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -907,7 +907,8 @@ static int arp_process(struct sk_buff *skb)
 		   devices (strip is candidate)
 		 */
 		if (n == NULL &&
-		    arp->ar_op == htons(ARPOP_REPLY) &&
+		    (arp->ar_op == htons(ARPOP_REPLY) ||
+		     (arp->ar_op == htons(ARPOP_REQUEST) && tip == sip)) &&
 		    inet_addr_type(net, sip) == RTN_UNICAST)
 			n = __neigh_lookup(&arp_tbl, &sip, dev, 1);
 	}
-- 
1.5.6.5


[-- Attachment #2: gratarp.c --]
[-- Type: text/x-csrc, Size: 3447 bytes --]

/*
 * Send gratuitous ARP, based on send_arp from the Linux fake package.
 */

/* send_arp.c

   This program sends out one ARP packet with source/target IP and Ethernet
   hardware addresses suuplied by the user.  It compiles and works on Linux
   and will probably work on any Unix that has SOCK_PACKET.

   The idea behind this program is a proof of a concept, nothing more.  It
   comes as is, no warranty.  However, you're allowed to use it under one
   condition: you must use your brain simultaneously.  If this condition is
   not met, you shall forget about this program and go RTFM immediately.

   yuri volobuev'97
   volobuev@t1.chem.umn.edu

*/

#include <stdio.h>
#include <ctype.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <netdb.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <linux/if_ether.h>
#include <sys/ioctl.h>
#include <net/if.h>
#include <netinet/ether.h>
#include <sys/socket.h>
#include <netpacket/packet.h>
#include <net/ethernet.h> /* the L2 protocols */


#define ETH_HW_ADDR_LEN 6
#define IP_ADDR_LEN 4
#define ARP_FRAME_TYPE 0x0806
#define ETHER_HW_TYPE 1
#define IP_PROTO_TYPE 0x0800
#define OP_ARP_REQUEST 1
#define OP_ARP_REPLY 2

struct arp_packet {
        u_char targ_hw_addr[ETH_HW_ADDR_LEN];
        u_char src_hw_addr[ETH_HW_ADDR_LEN];
        u_short frame_type;
        u_short hw_type;
        u_short prot_type;
        u_char hw_addr_size;
        u_char prot_addr_size;
        u_short op;
        u_char sndr_hw_addr[ETH_HW_ADDR_LEN];
        u_char sndr_ip_addr[IP_ADDR_LEN];
        u_char rcpt_hw_addr[ETH_HW_ADDR_LEN];
        u_char rcpt_ip_addr[IP_ADDR_LEN];
        u_char padding[18];
};

int main(int argc,char** argv)
{
	struct arp_packet pkt = { { 0, }, };
	struct sockaddr_ll sa = {
		.sll_family = AF_PACKET,
	};
	struct ifreq ifr;
	int sock;

	if (argc != 2 && argc != 3) {
		fprintf(stderr, "usage: gratarp ifname [target_mac_address]\n");
		return 1;
	}

	sock = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_RARP));
	if (sock<0){
		perror("socket");
		exit(1);
        }

	pkt.frame_type = htons(ARP_FRAME_TYPE);
	pkt.hw_type = htons(ETHER_HW_TYPE);
	pkt.prot_type = htons(IP_PROTO_TYPE);
	pkt.hw_addr_size = ETH_HW_ADDR_LEN;
	pkt.prot_addr_size = IP_ADDR_LEN;

	memset (&ifr, 0, sizeof(ifr));
	memcpy (&ifr.ifr_name, argv[1], strlen(argv[1]));

	if (ioctl(sock, SIOCGIFINDEX, &ifr) < 0)
		perror("SIOCGIFINDEX");
	sa.sll_ifindex = ifr.ifr_ifindex;

	if (ioctl(sock, SIOCGIFHWADDR, &ifr) < 0) {
		perror("SIOCGIFHWADDR");
		return 2;
	}
	memcpy(pkt.src_hw_addr, ifr.ifr_hwaddr.sa_data, 6);
	memcpy(pkt.sndr_hw_addr, ifr.ifr_hwaddr.sa_data, 6);

	/* pickup the target mac address */
	if (argc > 2) {
		struct ether_addr *eth_addr = ether_aton(argv[2]);
		if (!eth_addr) {
			perror("ether_aton\n");
			return 4;
		}
		memcpy(pkt.targ_hw_addr, eth_addr, 6);
		memcpy(pkt.rcpt_hw_addr, eth_addr, 6);
		memcpy(sa.sll_addr, eth_addr, 6);
		pkt.op = htons(OP_ARP_REPLY);
	} else {
		pkt.op = htons(OP_ARP_REQUEST);
		memset(pkt.targ_hw_addr, 0xff, 6);
	}
	sa.sll_halen = 6;

	if (ioctl(sock, SIOCGIFADDR, &ifr) < 0) {
		perror("SIOCGIFADDR");
		return 5;
	}
	memcpy(pkt.sndr_ip_addr, &((struct sockaddr_in*)&ifr.ifr_addr)->sin_addr, 4);
	memcpy(pkt.rcpt_ip_addr, &((struct sockaddr_in*)&ifr.ifr_addr)->sin_addr, 4);

	if (sendto(sock, &pkt, sizeof(pkt), 0, (struct sockaddr*)&sa, sizeof(sa)) < 0){
		perror("sendto");
		return 4;
        }

	return 0;
}

Re: [PATCH v2] ipv4: support for request type gratuitous ARP

[Julian Anastasov]

	Hello,

On Sun, 17 Jan 2010, Octavian Purdila wrote:

> On Sunday 17 January 2010 15:45:45 you wrote:
> 
> > 	May be I'm missing something but the description and
> > the changed code do not match. You claim this patch now supports
> > dest mac ff:ff:ff:ff:ff:ff while without the patch the
> > 'skb->pkt_type != PACKET_HOST' check usually updates the
> > existing entry with state NUD_STALE (neigh_update). What

> Hi Julian,
> 
> Thanks for looking at this patch ! You are indeed correct, we don't need to do anything special when we want to update an existing ARP entry.
> 
> But I also want to be able to create a new ARP entry not only update an existing one (and we can do that with Linux today, but only with response type grat arp).
> 
> How about this new version?

	Looks correct to me. You will save some CPU cycles
if the 'arp->ar_op == htons(ARPOP_REQUEST)' check is not added.
May be you also need to change arp_accept in 
Documentation/networking/ip-sysctl.txt to show that we do not 
differentiate Gratuitous ARP requests from replies.

> [PATCH] ipv4: allow warming up the ARP cache with request type gratuitous ARP
> 
> If the per device ARP_ACCEPT option is enabled, currently we only allow
> creating new ARP cache entries for response type gratuitous ARP.
> 
> Allowing gratuitous ARP to create new ARP entries (not only to update
> existing ones) is useful when we want to avoid unnecessary delays for
> the first packet of a stream.
> 
> This patch allows request type gratuitous ARP to create new ARP cache
> entries as well. This is useful when we want to warm-up the ARP cache
> entries for a large number of hosts on the same LAN.
> 
> Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
> ---
>  net/ipv4/arp.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index 0787092..1940b4d 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -907,7 +907,8 @@ static int arp_process(struct sk_buff *skb)
>  		   devices (strip is candidate)
>  		 */
>  		if (n == NULL &&
> -		    arp->ar_op == htons(ARPOP_REPLY) &&
> +		    (arp->ar_op == htons(ARPOP_REPLY) ||
> +		     (arp->ar_op == htons(ARPOP_REQUEST) && tip == sip)) &&
>  		    inet_addr_type(net, sip) == RTN_UNICAST)
>  			n = __neigh_lookup(&arp_tbl, &sip, dev, 1);
>  	}

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH v2] ipv4: support for request type gratuitous ARP

[David Miller]

From: Julian Anastasov <ja@ssi.bg>
Date: Sun, 17 Jan 2010 22:43:54 +0200 (EET)

> 	Looks correct to me. You will save some CPU cycles
> if the 'arp->ar_op == htons(ARPOP_REQUEST)' check is not added.
> May be you also need to change arp_accept in 
> Documentation/networking/ip-sysctl.txt to show that we do not 
> differentiate Gratuitous ARP requests from replies.

Octavian, please make the documentation fix for 'arp_accept'
and resubmit your patch.

It otherwise looks fine to me now.

Thanks.