* [PATCH net-next 1/1] net: neighbour: Simplify ifdefs around neigh_app_ns()
@ 2013-08-28 18:24 Tim Gardner
2013-08-28 18:51 ` Joe Perches
2013-08-28 23:36 ` Eric W. Biederman
0 siblings, 2 replies; 9+ messages in thread
From: Tim Gardner @ 2013-08-28 18:24 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Tim Gardner, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, Eric W. Biederman, Gao feng,
Joe Perches
Drop a couple of ifdef/endif pairs by moving the ifdef
surrounding neigh_app_ns() to the interior of neigh_app_ns().
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>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
This is an admittedly trivial change. I stumbled on it while trying to figure
out why Ubuntu doesn't have CONFIG_ARPD enabled.
net/core/neighbour.c | 4 ++--
net/ipv4/arp.c | 2 --
net/ipv6/ndisc.c | 2 --
3 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 60533db..049dd9e 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2759,13 +2759,13 @@ errout:
rtnl_set_sk_err(net, RTNLGRP_NEIGH, err);
}
-#ifdef CONFIG_ARPD
void neigh_app_ns(struct neighbour *n)
{
+#ifdef CONFIG_ARPD
__neigh_notify(n, RTM_GETNEIGH, NLM_F_REQUEST);
+#endif /* CONFIG_ARPD */
}
EXPORT_SYMBOL(neigh_app_ns);
-#endif /* CONFIG_ARPD */
#ifdef CONFIG_SYSCTL
static int zero;
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);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/1] net: neighbour: Simplify ifdefs around neigh_app_ns()
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-28 23:36 ` Eric W. Biederman
1 sibling, 1 reply; 9+ messages in thread
From: Joe Perches @ 2013-08-28 18:51 UTC (permalink / raw)
To: Tim Gardner
Cc: netdev, linux-kernel, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy,
Eric W. Biederman, Gao feng
On Wed, 2013-08-28 at 12:24 -0600, Tim Gardner wrote:
> Drop a couple of ifdef/endif pairs by moving the ifdef
> surrounding neigh_app_ns() to the interior of neigh_app_ns().
[]
> This is an admittedly trivial change. I stumbled on it while trying to figure
> out why Ubuntu doesn't have CONFIG_ARPD enabled.
I'd be more inclined to make neigh_app_ns static inline
in the .h file and remove the EXPORT_SYMBOL
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 60533db..049dd9e 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -2759,13 +2759,13 @@ errout:
> rtnl_set_sk_err(net, RTNLGRP_NEIGH, err);
> }
>
> -#ifdef CONFIG_ARPD
> void neigh_app_ns(struct neighbour *n)
> {
> +#ifdef CONFIG_ARPD
> __neigh_notify(n, RTM_GETNEIGH, NLM_F_REQUEST);
> +#endif /* CONFIG_ARPD */
> }
> EXPORT_SYMBOL(neigh_app_ns);
> -#endif /* CONFIG_ARPD */
>
> #ifdef CONFIG_SYSCTL
> static int zero;
> 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);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/1] net: neighbour: Simplify ifdefs around neigh_app_ns()
2013-08-28 18:51 ` Joe Perches
@ 2013-08-28 19:09 ` Tim Gardner
2013-08-29 1:26 ` Joe Perches
0 siblings, 1 reply; 9+ messages in thread
From: Tim Gardner @ 2013-08-28 19:09 UTC (permalink / raw)
To: Joe Perches
Cc: netdev, linux-kernel, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy,
Eric W. Biederman, Gao feng
On 08/28/2013 12:51 PM, Joe Perches wrote:
> On Wed, 2013-08-28 at 12:24 -0600, Tim Gardner wrote:
>> Drop a couple of ifdef/endif pairs by moving the ifdef
>> surrounding neigh_app_ns() to the interior of neigh_app_ns().
> []
>> This is an admittedly trivial change. I stumbled on it while trying to figure
>> out why Ubuntu doesn't have CONFIG_ARPD enabled.
>
> I'd be more inclined to make neigh_app_ns static inline
> in the .h file and remove the EXPORT_SYMBOL
>
I thought about that as well, but then you'd have to extern
__neigh_notify(), which is currently a static function and large enough
to not really be suitable for inlining. Seems like unnecessary churn to me.
rtg
--
Tim Gardner tim.gardner@canonical.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/1] net: neighbour: Simplify ifdefs around neigh_app_ns()
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 23:36 ` Eric W. Biederman
2013-08-29 12:38 ` [PATCH net-next v2] net: neighbour: Remove CONFIG_ARPD Tim Gardner
1 sibling, 1 reply; 9+ messages in thread
From: Eric W. Biederman @ 2013-08-28 23:36 UTC (permalink / raw)
To: Tim Gardner
Cc: netdev, linux-kernel, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, Gao feng,
Joe Perches
Tim Gardner <tim.gardner@canonical.com> writes:
> Drop a couple of ifdef/endif pairs by moving the ifdef
> surrounding neigh_app_ns() to the interior of neigh_app_ns().
Can we please just remove the CONFIG_ARPD option entirely.
There really is no savings to keeping this option configurable and the
option only removes a netlink notification.
> 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>
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
>
> This is an admittedly trivial change. I stumbled on it while trying to figure
> out why Ubuntu doesn't have CONFIG_ARPD enabled.
>
> net/core/neighbour.c | 4 ++--
> net/ipv4/arp.c | 2 --
> net/ipv6/ndisc.c | 2 --
> 3 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 60533db..049dd9e 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -2759,13 +2759,13 @@ errout:
> rtnl_set_sk_err(net, RTNLGRP_NEIGH, err);
> }
>
> -#ifdef CONFIG_ARPD
> void neigh_app_ns(struct neighbour *n)
> {
> +#ifdef CONFIG_ARPD
> __neigh_notify(n, RTM_GETNEIGH, NLM_F_REQUEST);
> +#endif /* CONFIG_ARPD */
> }
> EXPORT_SYMBOL(neigh_app_ns);
> -#endif /* CONFIG_ARPD */
>
> #ifdef CONFIG_SYSCTL
> static int zero;
> 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);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/1] net: neighbour: Simplify ifdefs around neigh_app_ns()
2013-08-28 19:09 ` Tim Gardner
@ 2013-08-29 1:26 ` Joe Perches
2013-08-29 1:32 ` Eric W. Biederman
0 siblings, 1 reply; 9+ messages in thread
From: Joe Perches @ 2013-08-29 1:26 UTC (permalink / raw)
To: Tim Gardner
Cc: netdev, linux-kernel, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy,
Eric W. Biederman, Gao feng
On Wed, 2013-08-28 at 13:09 -0600, Tim Gardner wrote:
> On 08/28/2013 12:51 PM, Joe Perches wrote:
> > On Wed, 2013-08-28 at 12:24 -0600, Tim Gardner wrote:
> >> Drop a couple of ifdef/endif pairs by moving the ifdef
> >> surrounding neigh_app_ns() to the interior of neigh_app_ns().
> > []
> >> This is an admittedly trivial change. I stumbled on it while trying to figure
> >> out why Ubuntu doesn't have CONFIG_ARPD enabled.
> > I'd be more inclined to make neigh_app_ns static inline
> > in the .h file and remove the EXPORT_SYMBOL
> I thought about that as well, but then you'd have to extern
> __neigh_notify(), which is currently a static function and large enough
> to not really be suitable for inlining. Seems like unnecessary churn to me.
Hi Tim.
As is, this makes the call to neight_app_ns
impossible to optimize away.
Perhaps this:
(this does add a possibly unused neigh_notify as a global symbol)
Rename __neigh_notify to neigh_notify and make public
Add static inline neigh_app_ns
Remove #ifdefs around use of neigh_app_ns
---
Compile tested only
include/net/neighbour.h | 10 +++++++++-
net/core/neighbour.c | 15 +++------------
net/ipv4/arp.c | 2 --
net/ipv6/ndisc.c | 2 --
4 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 536501a..a08b0a7 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -249,7 +249,15 @@ static inline struct net *pneigh_net(const struct pneigh_entry *pneigh)
return read_pnet(&pneigh->net);
}
-void neigh_app_ns(struct neighbour *n);
+void neigh_notify(struct neighbour *n, int type, int flags);
+
+static inline void neigh_app_ns(struct neighbour *n)
+{
+#ifdef CONFIG_ARPD
+ neigh_notify(n, RTM_GETNEIGH, NLM_F_REQUEST);
+#endif
+}
+
void neigh_for_each(struct neigh_table *tbl,
void (*cb)(struct neighbour *, void *), void *cookie);
void __neigh_for_each_release(struct neigh_table *tbl,
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 60533db..6ec5f86 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -50,7 +50,6 @@ do { \
#define PNEIGH_HASHMASK 0xF
static void neigh_timer_handler(unsigned long arg);
-static void __neigh_notify(struct neighbour *n, int type, int flags);
static void neigh_update_notify(struct neighbour *neigh);
static int pneigh_ifdown(struct neigh_table *tbl, struct net_device *dev);
@@ -103,7 +102,7 @@ static void neigh_cleanup_and_release(struct neighbour *neigh)
if (neigh->parms->neigh_cleanup)
neigh->parms->neigh_cleanup(neigh);
- __neigh_notify(neigh, RTM_DELNEIGH, 0);
+ neigh_notify(neigh, RTM_DELNEIGH, 0);
neigh_release(neigh);
}
@@ -2215,7 +2214,7 @@ nla_put_failure:
static void neigh_update_notify(struct neighbour *neigh)
{
call_netevent_notifiers(NETEVENT_NEIGH_UPDATE, neigh);
- __neigh_notify(neigh, RTM_NEWNEIGH, 0);
+ neigh_notify(neigh, RTM_NEWNEIGH, 0);
}
static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
@@ -2735,7 +2734,7 @@ static inline size_t neigh_nlmsg_size(void)
+ nla_total_size(4); /* NDA_PROBES */
}
-static void __neigh_notify(struct neighbour *n, int type, int flags)
+void neigh_notify(struct neighbour *n, int type, int flags)
{
struct net *net = dev_net(n->dev);
struct sk_buff *skb;
@@ -2759,14 +2758,6 @@ 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;
static int int_max = INT_MAX;
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);
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/1] net: neighbour: Simplify ifdefs around neigh_app_ns()
2013-08-29 1:26 ` Joe Perches
@ 2013-08-29 1:32 ` Eric W. Biederman
0 siblings, 0 replies; 9+ messages in thread
From: Eric W. Biederman @ 2013-08-29 1:32 UTC (permalink / raw)
To: Joe Perches
Cc: Tim Gardner, netdev, linux-kernel, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, Gao feng
Joe Perches <joe@perches.com> writes:
> On Wed, 2013-08-28 at 13:09 -0600, Tim Gardner wrote:
>> On 08/28/2013 12:51 PM, Joe Perches wrote:
>> > On Wed, 2013-08-28 at 12:24 -0600, Tim Gardner wrote:
>> >> Drop a couple of ifdef/endif pairs by moving the ifdef
>> >> surrounding neigh_app_ns() to the interior of neigh_app_ns().
>> > []
>> >> This is an admittedly trivial change. I stumbled on it while trying to figure
>> >> out why Ubuntu doesn't have CONFIG_ARPD enabled.
>
>> > I'd be more inclined to make neigh_app_ns static inline
>> > in the .h file and remove the EXPORT_SYMBOL
>
>> I thought about that as well, but then you'd have to extern
>> __neigh_notify(), which is currently a static function and large enough
>> to not really be suitable for inlining. Seems like unnecessary churn to me.
>
> Hi Tim.
>
> As is, this makes the call to neight_app_ns
> impossible to optimize away.
>
> Perhaps this:
>
> (this does add a possibly unused neigh_notify as a global symbol)
>
> Rename __neigh_notify to neigh_notify and make public
> Add static inline neigh_app_ns
> Remove #ifdefs around use of neigh_app_ns
Again. Why not just remove CONFIG_ARPD entirely? A config option to
compile out a single line of code seems like a real waste.
Eric
> ---
>
> Compile tested only
>
> include/net/neighbour.h | 10 +++++++++-
> net/core/neighbour.c | 15 +++------------
> net/ipv4/arp.c | 2 --
> net/ipv6/ndisc.c | 2 --
> 4 files changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/include/net/neighbour.h b/include/net/neighbour.h
> index 536501a..a08b0a7 100644
> --- a/include/net/neighbour.h
> +++ b/include/net/neighbour.h
> @@ -249,7 +249,15 @@ static inline struct net *pneigh_net(const struct pneigh_entry *pneigh)
> return read_pnet(&pneigh->net);
> }
>
> -void neigh_app_ns(struct neighbour *n);
> +void neigh_notify(struct neighbour *n, int type, int flags);
> +
> +static inline void neigh_app_ns(struct neighbour *n)
> +{
> +#ifdef CONFIG_ARPD
> + neigh_notify(n, RTM_GETNEIGH, NLM_F_REQUEST);
> +#endif
> +}
> +
> void neigh_for_each(struct neigh_table *tbl,
> void (*cb)(struct neighbour *, void *), void *cookie);
> void __neigh_for_each_release(struct neigh_table *tbl,
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 60533db..6ec5f86 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -50,7 +50,6 @@ do { \
> #define PNEIGH_HASHMASK 0xF
>
> static void neigh_timer_handler(unsigned long arg);
> -static void __neigh_notify(struct neighbour *n, int type, int flags);
> static void neigh_update_notify(struct neighbour *neigh);
> static int pneigh_ifdown(struct neigh_table *tbl, struct net_device *dev);
>
> @@ -103,7 +102,7 @@ static void neigh_cleanup_and_release(struct neighbour *neigh)
> if (neigh->parms->neigh_cleanup)
> neigh->parms->neigh_cleanup(neigh);
>
> - __neigh_notify(neigh, RTM_DELNEIGH, 0);
> + neigh_notify(neigh, RTM_DELNEIGH, 0);
> neigh_release(neigh);
> }
>
> @@ -2215,7 +2214,7 @@ nla_put_failure:
> static void neigh_update_notify(struct neighbour *neigh)
> {
> call_netevent_notifiers(NETEVENT_NEIGH_UPDATE, neigh);
> - __neigh_notify(neigh, RTM_NEWNEIGH, 0);
> + neigh_notify(neigh, RTM_NEWNEIGH, 0);
> }
>
> static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
> @@ -2735,7 +2734,7 @@ static inline size_t neigh_nlmsg_size(void)
> + nla_total_size(4); /* NDA_PROBES */
> }
>
> -static void __neigh_notify(struct neighbour *n, int type, int flags)
> +void neigh_notify(struct neighbour *n, int type, int flags)
> {
> struct net *net = dev_net(n->dev);
> struct sk_buff *skb;
> @@ -2759,14 +2758,6 @@ 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;
> static int int_max = INT_MAX;
> 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);
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next v2] net: neighbour: Remove CONFIG_ARPD
2013-08-28 23:36 ` Eric W. Biederman
@ 2013-08-29 12:38 ` Tim Gardner
2013-08-29 23:39 ` Eric W. Biederman
2013-09-04 1:42 ` David Miller
0 siblings, 2 replies; 9+ messages in thread
From: Tim Gardner @ 2013-08-29 12:38 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Tim Gardner, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, Eric W. Biederman, Gao feng,
Joe Perches, Veaceslav Falico
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.
Suggested-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);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net-next v2] net: neighbour: Remove CONFIG_ARPD
2013-08-29 12:38 ` [PATCH net-next v2] net: neighbour: Remove CONFIG_ARPD Tim Gardner
@ 2013-08-29 23:39 ` Eric W. Biederman
2013-09-04 1:42 ` David Miller
1 sibling, 0 replies; 9+ messages in thread
From: Eric W. Biederman @ 2013-08-29 23:39 UTC (permalink / raw)
To: Tim Gardner
Cc: netdev, linux-kernel, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, Gao feng,
Joe Perches, Veaceslav Falico
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);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next v2] net: neighbour: Remove CONFIG_ARPD
2013-08-29 12:38 ` [PATCH net-next v2] net: neighbour: Remove CONFIG_ARPD Tim Gardner
2013-08-29 23:39 ` Eric W. Biederman
@ 2013-09-04 1:42 ` David Miller
1 sibling, 0 replies; 9+ messages in thread
From: David Miller @ 2013-09-04 1:42 UTC (permalink / raw)
To: tim.gardner
Cc: netdev, linux-kernel, kuznet, jmorris, yoshfuji, kaber, ebiederm,
gaofeng, joe, vfalico
From: Tim Gardner <tim.gardner@canonical.com>
Date: Thu, 29 Aug 2013 06:38:47 -0600
> 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.
>
> Suggested-by: Eric W. Biederman <ebiederm@xmission.com>
...
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-09-04 1:42 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2013-09-04 1:42 ` 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).