public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Tim Gardner <tim.gardner@canonical.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	"David S. Miller" <davem@davemloft.net>,
	Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>,
	James Morris <jmorris@namei.org>,
	Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
	Patrick McHardy <kaber@trash.net>,
	Gao feng <gaofeng@cn.fujitsu.com>, Joe Perches <joe@perches.com>,
	Veaceslav Falico <vfalico@redhat.com>
Subject: Re: [PATCH net-next v2] net: neighbour: Remove CONFIG_ARPD
Date: Thu, 29 Aug 2013 16:39:04 -0700	[thread overview]
Message-ID: <87wqn4ysmf.fsf@xmission.com> (raw)
In-Reply-To: <1377779927-28500-1-git-send-email-tim.gardner@canonical.com> (Tim Gardner's message of "Thu, 29 Aug 2013 06:38:47 -0600")

Tim Gardner <tim.gardner@canonical.com> writes:

> This config option is superfluous in that it only guards a call
> to neigh_app_ns(). Enabling CONFIG_ARPD by default has no
> change in behavior. There will now be call to __neigh_notify()
> for each ARP resolution, which has no impact unless there is a
> user space daemon waiting to receive the notification, i.e.,
> the case for which CONFIG_ARPD was designed anyways.

This looks good to me, and much less magic to maintain.

Eric


> Suggested-by: Eric W. Biederman <ebiederm@xmission.com>
Reviewed-by: "Eric W. Biederman" <ebiederm@xmission.com>

> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> Cc: James Morris <jmorris@namei.org>
> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: "Eric W. Biederman" <ebiederm@xmission.com>
> Cc: Gao feng <gaofeng@cn.fujitsu.com>
> Cc: Joe Perches <joe@perches.com>
> Cc: Veaceslav Falico <vfalico@redhat.com>
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
>
> Eric's suggestion to simply remove the config option makes sense
> to me. If acceptable then I'll submit a patch series that also removes
> CONFIG_ARPD from the various arch defconfigs.
>
>  net/core/neighbour.c |    2 --
>  net/ipv4/Kconfig     |   16 ----------------
>  net/ipv4/arp.c       |    2 --
>  net/ipv6/ndisc.c     |    2 --
>  4 files changed, 22 deletions(-)
>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 60533db..6072610 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -2759,13 +2759,11 @@ errout:
>  		rtnl_set_sk_err(net, RTNLGRP_NEIGH, err);
>  }
>  
> -#ifdef CONFIG_ARPD
>  void neigh_app_ns(struct neighbour *n)
>  {
>  	__neigh_notify(n, RTM_GETNEIGH, NLM_F_REQUEST);
>  }
>  EXPORT_SYMBOL(neigh_app_ns);
> -#endif /* CONFIG_ARPD */
>  
>  #ifdef CONFIG_SYSCTL
>  static int zero;
> diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
> index 37cf1a6..05c57f0 100644
> --- a/net/ipv4/Kconfig
> +++ b/net/ipv4/Kconfig
> @@ -259,22 +259,6 @@ config IP_PIMSM_V2
>  	  gated-5). This routing protocol is not used widely, so say N unless
>  	  you want to play with it.
>  
> -config ARPD
> -	bool "IP: ARP daemon support"
> -	---help---
> -	  The kernel maintains an internal cache which maps IP addresses to
> -	  hardware addresses on the local network, so that Ethernet
> -	  frames are sent to the proper address on the physical networking
> -	  layer. Normally, kernel uses the ARP protocol to resolve these
> -	  mappings.
> -
> -	  Saying Y here adds support to have an user space daemon to do this
> -	  resolution instead. This is useful for implementing an alternate
> -	  address resolution protocol (e.g. NHRP on mGRE tunnels) and also for
> -	  testing purposes.
> -
> -	  If unsure, say N.
> -
>  config SYN_COOKIES
>  	bool "IP: TCP syncookie support"
>  	---help---
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index 4429b01..7808093 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -368,9 +368,7 @@ static void arp_solicit(struct neighbour *neigh, struct sk_buff *skb)
>  	} else {
>  		probes -= neigh->parms->app_probes;
>  		if (probes < 0) {
> -#ifdef CONFIG_ARPD
>  			neigh_app_ns(neigh);
> -#endif
>  			return;
>  		}
>  	}
> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
> index 04d31c2..d5693ad 100644
> --- a/net/ipv6/ndisc.c
> +++ b/net/ipv6/ndisc.c
> @@ -663,9 +663,7 @@ static void ndisc_solicit(struct neighbour *neigh, struct sk_buff *skb)
>  		}
>  		ndisc_send_ns(dev, neigh, target, target, saddr);
>  	} else if ((probes -= neigh->parms->app_probes) < 0) {
> -#ifdef CONFIG_ARPD
>  		neigh_app_ns(neigh);
> -#endif
>  	} else {
>  		addrconf_addr_solict_mult(target, &mcaddr);
>  		ndisc_send_ns(dev, NULL, target, &mcaddr, saddr);

  reply	other threads:[~2013-08-29 23:39 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-28 18:24 [PATCH net-next 1/1] net: neighbour: Simplify ifdefs around neigh_app_ns() Tim Gardner
2013-08-28 18:51 ` Joe Perches
2013-08-28 19:09   ` Tim Gardner
2013-08-29  1:26     ` Joe Perches
2013-08-29  1:32       ` Eric W. Biederman
2013-08-28 23:36 ` Eric W. Biederman
2013-08-29 12:38   ` [PATCH net-next v2] net: neighbour: Remove CONFIG_ARPD Tim Gardner
2013-08-29 23:39     ` Eric W. Biederman [this message]
2013-09-04  1:42     ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87wqn4ysmf.fsf@xmission.com \
    --to=ebiederm@xmission.com \
    --cc=davem@davemloft.net \
    --cc=gaofeng@cn.fujitsu.com \
    --cc=jmorris@namei.org \
    --cc=joe@perches.com \
    --cc=kaber@trash.net \
    --cc=kuznet@ms2.inr.ac.ru \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=tim.gardner@canonical.com \
    --cc=vfalico@redhat.com \
    --cc=yoshfuji@linux-ipv6.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox