* [net-next PATCH v3] ipv4: allow warming up the ARP cache with request type gratuitous ARP
@ 2010-01-18 22:58 Octavian Purdila
2010-01-18 23:39 ` Mark Smith
2010-01-19 10:12 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Octavian Purdila @ 2010-01-18 22:58 UTC (permalink / raw)
To: David Miller; +Cc: Julian Anastasov, Laurent Chavey, netdev
If the per device ARP_ACCEPT option is enable, 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 populate the ARP cache
entries for a large number of hosts on the same LAN.
Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
---
Documentation/networking/ip-sysctl.txt | 15 ++++++++++++---
net/ipv4/arp.c | 3 ++-
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index c532884..2dc7a1d 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -852,9 +852,18 @@ arp_notify - BOOLEAN
or hardware address changes.
arp_accept - BOOLEAN
- Define behavior when gratuitous arp replies are received:
- 0 - drop gratuitous arp frames
- 1 - accept gratuitous arp frames
+ Define behavior for gratuitous ARP frames who's IP is not
+ already present in the ARP table:
+ 0 - don't create new entries in the ARP table
+ 1 - create new entries in the ARP table
+
+ Both replies and requests type gratuitous arp will trigger the
+ ARP table to be updated, if this setting is on.
+
+ If the ARP table already contains the IP address of the
+ gratuitous arp frame, the arp table will be updated regardless
+ if this setting is on or off.
+
app_solicit - INTEGER
The maximum number of probes to send to the user space ARP daemon
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [net-next PATCH v3] ipv4: allow warming up the ARP cache with request type gratuitous ARP
2010-01-18 22:58 [net-next PATCH v3] ipv4: allow warming up the ARP cache with request type gratuitous ARP Octavian Purdila
@ 2010-01-18 23:39 ` Mark Smith
2010-01-19 10:11 ` David Miller
2010-01-19 10:12 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Mark Smith @ 2010-01-18 23:39 UTC (permalink / raw)
To: Octavian Purdila; +Cc: David Miller, Julian Anastasov, Laurent Chavey, netdev
Hi Octavian,
On Tue, 19 Jan 2010 00:58:44 +0200
Octavian Purdila <opurdila@ixiacom.com> wrote:
> If the per device ARP_ACCEPT option is enable, 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 populate the ARP cache
> entries for a large number of hosts on the same LAN.
>
I don't think this complies with the ARP RFC (RFC826). Walking through
the validation steps, Gratuitous ARP seems to only pass if there is an
existing entry in the table, otherwise they'd fail, as the target
address in the GA packet doesn't match that of the receiving host.
I can guess your motivation for this, however I wonder if going outside
the spec for relatively rare testing cases (in comparison to all the ARP
usage on all IP LANs) is wise. While ARP is vulnerable to DoS attacks, I
think allowing GAs to generate new ARP entries would increase its
vulnerability. There may also be some other unexpected consequences
that haven't been identified.
Possibly for your test environments, maybe static ARP entries might
be a feasible alternative that's still compliant with RFC826?
Regards,
Mark.
> Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
> ---
> Documentation/networking/ip-sysctl.txt | 15 ++++++++++++---
> net/ipv4/arp.c | 3 ++-
> 2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
> index c532884..2dc7a1d 100644
> --- a/Documentation/networking/ip-sysctl.txt
> +++ b/Documentation/networking/ip-sysctl.txt
> @@ -852,9 +852,18 @@ arp_notify - BOOLEAN
> or hardware address changes.
>
> arp_accept - BOOLEAN
> - Define behavior when gratuitous arp replies are received:
> - 0 - drop gratuitous arp frames
> - 1 - accept gratuitous arp frames
> + Define behavior for gratuitous ARP frames who's IP is not
> + already present in the ARP table:
> + 0 - don't create new entries in the ARP table
> + 1 - create new entries in the ARP table
> +
> + Both replies and requests type gratuitous arp will trigger the
> + ARP table to be updated, if this setting is on.
> +
> + If the ARP table already contains the IP address of the
> + gratuitous arp frame, the arp table will be updated regardless
> + if this setting is on or off.
> +
>
> app_solicit - INTEGER
> The maximum number of probes to send to the user space ARP daemon
> 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
>
> --
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [net-next PATCH v3] ipv4: allow warming up the ARP cache with request type gratuitous ARP
2010-01-18 23:39 ` Mark Smith
@ 2010-01-19 10:11 ` David Miller
2010-01-19 10:48 ` Mark Smith
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2010-01-19 10:11 UTC (permalink / raw)
To: lk-netdev; +Cc: opurdila, ja, chavey, netdev
From: Mark Smith <lk-netdev@lk-netdev.nosense.org>
Date: Tue, 19 Jan 2010 10:09:13 +1030
> I don't think this complies with the ARP RFC (RFC826). Walking through
> the validation steps, Gratuitous ARP seems to only pass if there is an
> existing entry in the table, otherwise they'd fail, as the target
> address in the GA packet doesn't match that of the receiving host.
It's fine if it's guarded by the per-device ARP_ACCEPT sysctl which
defaults to off, and that's what he's doing.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [net-next PATCH v3] ipv4: allow warming up the ARP cache with request type gratuitous ARP
2010-01-19 10:11 ` David Miller
@ 2010-01-19 10:48 ` Mark Smith
2010-01-19 11:13 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Mark Smith @ 2010-01-19 10:48 UTC (permalink / raw)
To: David Miller; +Cc: opurdila, ja, chavey, netdev
Hi Dave,
On Tue, 19 Jan 2010 02:11:38 -0800 (PST)
David Miller <davem@davemloft.net> wrote:
> From: Mark Smith <lk-netdev@lk-netdev.nosense.org>
> Date: Tue, 19 Jan 2010 10:09:13 +1030
>
> > I don't think this complies with the ARP RFC (RFC826). Walking through
> > the validation steps, Gratuitous ARP seems to only pass if there is an
> > existing entry in the table, otherwise they'd fail, as the target
> > address in the GA packet doesn't match that of the receiving host.
>
> It's fine if it's guarded by the per-device ARP_ACCEPT sysctl which
> defaults to off, and that's what he's doing.
Fair enough.
I wasn't familiar with the ARP_ACCEPT sysctl. It seems that this patch
is not only adding the feature of creating new ARP table entries in
response to GARPs, but is also fixing the ARP implementation because it
looks like the default wasn't to accept GARPs, despite the validation
steps in RFC826 allowing them.
ARP_ACCEPT / the /proc "arp_accept" file is really confusingly named -
unless you knew better I doubt anybody would guess it's only referring
to GARPs. Is there any chance of renaming it or symlinking a new name
of something like "garp_accept" to the old one? Is this a significant
enough behaviour change that it creates that opportunity?
Regards,
Mark.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [net-next PATCH v3] ipv4: allow warming up the ARP cache with request type gratuitous ARP
2010-01-19 10:48 ` Mark Smith
@ 2010-01-19 11:13 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2010-01-19 11:13 UTC (permalink / raw)
To: lk-netdev; +Cc: opurdila, ja, chavey, netdev
From: Mark Smith <lk-netdev@lk-netdev.nosense.org>
Date: Tue, 19 Jan 2010 21:18:18 +1030
> ARP_ACCEPT / the /proc "arp_accept" file is really confusingly named -
> unless you knew better I doubt anybody would guess it's only referring
> to GARPs. Is there any chance of renaming it or symlinking a new name
> of something like "garp_accept" to the old one? Is this a significant
> enough behaviour change that it creates that opportunity?
It's been there forever, it's well documented, and creating
symlinks is messy.
So no.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [net-next PATCH v3] ipv4: allow warming up the ARP cache with request type gratuitous ARP
2010-01-18 22:58 [net-next PATCH v3] ipv4: allow warming up the ARP cache with request type gratuitous ARP Octavian Purdila
2010-01-18 23:39 ` Mark Smith
@ 2010-01-19 10:12 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2010-01-19 10:12 UTC (permalink / raw)
To: opurdila; +Cc: ja, chavey, netdev
From: Octavian Purdila <opurdila@ixiacom.com>
Date: Tue, 19 Jan 2010 00:58:44 +0200
> If the per device ARP_ACCEPT option is enable, 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 populate the ARP cache
> entries for a large number of hosts on the same LAN.
>
> Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-01-19 11:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-18 22:58 [net-next PATCH v3] ipv4: allow warming up the ARP cache with request type gratuitous ARP Octavian Purdila
2010-01-18 23:39 ` Mark Smith
2010-01-19 10:11 ` David Miller
2010-01-19 10:48 ` Mark Smith
2010-01-19 11:13 ` David Miller
2010-01-19 10:12 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).