Home » Mailing lists » Devel » [PATCH 0/8 2.6.25] a set of small code cleanups
[PATCH 0/8 2.6.25] a set of small code cleanups [message #26017] |
Mon, 14 January 2008 14:59  |
den
Messages: 494 Registered: December 2005
|
Senior Member |
|
|
This set contains a set of small improvements found in IPv4 code during
preparations to support ARP in the network namespace. These fixes are
mostly independent except 2 last ones.
Signed-off-by: Denis V. Lunev <den@openvz.org>
|
|
|
[PATCH 1/8 net-2.6.25] [ARP] Move inet_addr_type call after simple error checks in arp_contructor. [message #26018 is a reply to message #26017] |
Mon, 14 January 2008 14:59   |
den
Messages: 494 Registered: December 2005
|
Senior Member |
|
|
The neighbour entry will be destroyed in the case of error, so it is pointless
to perform constly routing table lookup in this case.
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
net/ipv4/arp.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index b715ec0..49c24ff 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -235,8 +235,6 @@ static int arp_constructor(struct neighbour *neigh)
struct in_device *in_dev;
struct neigh_parms *parms;
- neigh->type = inet_addr_type(&init_net, addr);
-
rcu_read_lock();
in_dev = __in_dev_get_rcu(dev);
if (in_dev == NULL) {
@@ -244,6 +242,8 @@ static int arp_constructor(struct neighbour *neigh)
return -EINVAL;
}
+ neigh->type = inet_addr_type(&init_net, addr);
+
parms = in_dev->arp_parms;
__neigh_parms_put(neigh->parms);
neigh->parms = neigh_parms_clone(parms);
--
1.5.3.rc5
|
|
|
[PATCH 2/8 net-2.6.25] [NETNS] Make arp code network namespace consistent. [message #26019 is a reply to message #26017] |
Mon, 14 January 2008 14:59   |
den
Messages: 494 Registered: December 2005
|
Senior Member |
|
|
Some calls in the arp.c have network namespace as an argument. Getting
init_net inside these functions is simply inconsistent. Fix this.
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
net/ipv4/arp.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 49c24ff..0db7d49 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -969,13 +969,13 @@ static int arp_req_set_public(struct net *net, struct arpreq *r,
if (mask && mask != htonl(0xFFFFFFFF))
return -EINVAL;
if (!dev && (r->arp_flags & ATF_COM)) {
- dev = dev_getbyhwaddr(&init_net, r->arp_ha.sa_family,
+ dev = dev_getbyhwaddr(net, r->arp_ha.sa_family,
r->arp_ha.sa_data);
if (!dev)
return -ENODEV;
}
if (mask) {
- if (pneigh_lookup(&arp_tbl, &init_net, &ip, dev, 1) == NULL)
+ if (pneigh_lookup(&arp_tbl, net, &ip, dev, 1) == NULL)
return -ENOBUFS;
return 0;
}
@@ -1084,7 +1084,7 @@ static int arp_req_delete_public(struct net *net, struct arpreq *r,
__be32 mask = ((struct sockaddr_in *)&r->arp_netmask)->sin_addr.s_addr;
if (mask == htonl(0xFFFFFFFF))
- return pneigh_delete(&arp_tbl, &init_net, &ip, dev);
+ return pneigh_delete(&arp_tbl, net, &ip, dev);
if (mask)
return -EINVAL;
@@ -1162,7 +1162,7 @@ int arp_ioctl(struct net *net, unsigned int cmd, void __user *arg)
rtnl_lock();
if (r.arp_dev[0]) {
err = -ENODEV;
- if ((dev = __dev_get_by_name(&init_net, r.arp_dev)) == NULL)
+ if ((dev = __dev_get_by_name(net, r.arp_dev)) == NULL)
goto out;
/* Mmmm... It is wrong... ARPHRD_NETROM==0 */
--
1.5.3.rc5
|
|
|
|
[PATCH 8/8 net-2.6.25] [NETNS] Process inet_confirm_addr in the correct namespace. [message #26021 is a reply to message #26017] |
Mon, 14 January 2008 15:00   |
den
Messages: 494 Registered: December 2005
|
Senior Member |
|
|
inet_confirm_addr can be called with NULL in_dev from arp_ignore iff
scope is RT_SCOPE_LINK.
Lets always pass the device and check for RT_SCOPE_LINK scope inside
inet_confirm_addr. This let us take network namespace from in_device a
need for an additional argument.
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
net/ipv4/arp.c | 1 -
net/ipv4/devinet.c | 6 ++++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index f38e1a9..b715ec0 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -402,7 +400,6 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
case 3: /* Do not reply for scope host addresses */
sip = 0;
scope = RT_SCOPE_LINK;
- in_dev = NULL;
break;
case 4: /* Reserved */
case 5:
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index dc1665a..4569c69 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -978,13 +978,15 @@ __be32 inet_confirm_addr(struct in_device *in_dev,
{
__be32 addr = 0;
struct net_device *dev;
+ struct net *net;
- if (in_dev != NULL)
+ if (scope != RT_SCOPE_LINK)
return confirm_addr_indev(in_dev, dst, local, scope);
+ net = in_dev->dev->nd_net;
read_lock(&dev_base_lock);
rcu_read_lock();
- for_each_netdev(&init_net, dev) {
+ for_each_netdev(net, dev) {
if ((in_dev = __in_dev_get_rcu(dev))) {
addr = confirm_addr_indev(in_dev, dst, local, scope);
if (addr)
--
1.5.3.rc5
|
|
|
[PATCH 3/8 net-2.6.25] [IPV4] fib_rules_unregister is essentially void. [message #26022 is a reply to message #26017] |
Mon, 14 January 2008 14:59   |
den
Messages: 494 Registered: December 2005
|
Senior Member |
|
|
fib_rules_unregister is called only after successful register and the return
code is never checked.
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
include/net/fib_rules.h | 2 +-
net/core/fib_rules.c | 21 ++++-----------------
2 files changed, 5 insertions(+), 18 deletions(-)
diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
index 88f870f..34349f9 100644
--- a/include/net/fib_rules.h
+++ b/include/net/fib_rules.h
@@ -104,7 +104,7 @@ static inline u32 frh_get_table(struct fib_rule_hdr *frh, struct nlattr **nla)
}
extern int fib_rules_register(struct net *, struct fib_rules_ops *);
-extern int fib_rules_unregister(struct net *, struct fib_rules_ops *);
+extern void fib_rules_unregister(struct net *, struct fib_rules_ops *);
extern void fib_rules_cleanup_ops(struct fib_rules_ops *);
extern int fib_rules_lookup(struct fib_rules_ops *,
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 0eecf4c..42ccaf5 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -115,29 +115,16 @@ void fib_rules_cleanup_ops(struct fib_rules_ops *ops)
}
EXPORT_SYMBOL_GPL(fib_rules_cleanup_ops);
-int fib_rules_unregister(struct net *net, struct fib_rules_ops *ops)
+void fib_rules_unregister(struct net *net, struct fib_rules_ops *ops)
{
- int err = 0;
- struct fib_rules_ops *o;
spin_lock(&net->rules_mod_lock);
- list_for_each_entry(o, &net->rules_ops, list) {
- if (o == ops) {
- list_del_rcu(&o->list);
- fib_rules_cleanup_ops(ops);
- goto out;
- }
- }
-
- err = -ENOENT;
-out:
+ list_del_rcu(&ops->list);
+ fib_rules_cleanup_ops(ops);
spin_unlock(&net->rules_mod_lock);
synchronize_rcu();
- if (!err)
- release_net(net);
-
- return err;
+ release_net(net);
}
EXPORT_SYMBOL_GPL(fib_rules_unregister);
--
1.5.3.rc5
|
|
|
[PATCH 6/8 net-2.6.25] [ARP] neigh_parms_put(destroy) are essentially local to core/neighbour.c. [message #26023 is a reply to message #26017] |
Mon, 14 January 2008 14:59   |
den
Messages: 494 Registered: December 2005
|
Senior Member |
|
|
Make them static.
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
include/net/neighbour.h | 7 -------
net/core/neighbour.c | 11 ++++++++++-
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index a0d42a5..ebbfb50 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -213,7 +213,6 @@ extern struct neighbour *neigh_event_ns(struct neigh_table *tbl,
extern struct neigh_parms *neigh_parms_alloc(struct net_device *dev, struct neigh_table *tbl);
extern void neigh_parms_release(struct neigh_table *tbl, struct neigh_parms *parms);
-extern void neigh_parms_destroy(struct neigh_parms *parms);
extern unsigned long neigh_rand_reach_time(unsigned long base);
extern void pneigh_enqueue(struct neigh_table *tbl, struct neigh_parms *p,
@@ -254,12 +253,6 @@ static inline void __neigh_parms_put(struct neigh_parms *parms)
atomic_dec(&parms->refcnt);
}
-static inline void neigh_parms_put(struct neigh_parms *parms)
-{
- if (atomic_dec_and_test(&parms->refcnt))
- neigh_parms_destroy(parms);
-}
-
static inline struct neigh_parms *neigh_parms_clone(struct neigh_parms *parms)
{
atomic_inc(&parms->refcnt);
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 9b0b773..41394db 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -55,6 +55,8 @@
#define PNEIGH_HASHMASK 0xF
+static inline void neigh_parms_put(struct neigh_parms *parms);
+
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);
@@ -1348,7 +1350,7 @@ void neigh_parms_release(struct neigh_table *tbl, struct neigh_parms *parms)
NEIGH_PRINTK1("neigh_parms_release: not found\n");
}
-void neigh_parms_destroy(struct neigh_parms *parms)
+static void neigh_parms_destroy(struct neigh_parms *parms)
{
release_net(parms->net);
if (parms->dev)
@@ -1356,6 +1358,13 @@ void neigh_parms_destroy(struct neigh_parms *parms)
kfree(parms);
}
+static inline void neigh_parms_put(struct neigh_parms *parms)
+{
+ if (atomic_dec_and_test(&parms->refcnt))
+ neigh_parms_destroy(parms);
+}
+
+
static struct lock_class_key neigh_table_proxy_queue_class;
void neigh_table_init_no_netlink(struct neigh_table *tbl)
--
1.5.3.rc5
|
|
|
[PATCH 4/8 net-2.6.25] [ARP] Remove overkill checks from neigh_param_alloc. [message #26024 is a reply to message #26017] |
Mon, 14 January 2008 14:59   |
den
Messages: 494 Registered: December 2005
|
Senior Member |
|
|
Valid network device is always passed into neigh_param_alloc, so remove
extra checking for dev == NULL. Additionally, cleanup bogus netns assignment.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
---
net/core/neighbour.c | 18 +++++++-----------
1 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index af49137..32f1a23 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1301,10 +1301,7 @@ struct neigh_parms *neigh_parms_alloc(struct net_device *dev,
struct neigh_parms *p, *ref;
struct net *net;
- net = &init_net;
- if (dev)
- net = dev->nd_net;
-
+ net = dev->nd_net;
ref = lookup_neigh_params(tbl, net, 0);
if (!ref)
return NULL;
@@ -1316,15 +1313,14 @@ struct neigh_parms *neigh_parms_alloc(struct net_device *dev,
INIT_RCU_HEAD(&p->rcu_head);
p->reachable_time =
neigh_rand_reach_time(p->base_reachable_time);
- if (dev) {
- if (dev->neigh_setup && dev->neigh_setup(dev, p)) {
- kfree(p);
- return NULL;
- }
- dev_hold(dev);
- p->dev = dev;
+ if (dev->neigh_setup && dev->neigh_setup(dev, p)) {
+ kfree(p);
+ return NULL;
}
+
+ dev_hold(dev);
+ p->dev = dev;
p->net = hold_net(net);
p->sysctl_table = NULL;
write_lock_bh(&tbl->lock);
--
1.5.3.rc5
|
|
|
[PATCH 7/8 net-2.6.25] [IPV4] Remove extra argument from arp_ignore. [message #26025 is a reply to message #26017] |
Mon, 14 January 2008 15:00   |
den
Messages: 494 Registered: December 2005
|
Senior Member |
|
|
arp_ignore has two arguments: dev & in_dev. dev is used for inet_confirm_addr
calling only.
inet_confirm_addr, in turn, either gets in_dev from the device passed or
iterates over all network devices if the device passed is NULL. It seems
logical to directly pass in_dev into inet_confirm_addr.
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
include/linux/inetdevice.h | 2 +-
net/ipv4/arp.c | 11 +++++------
net/ipv4/devinet.c | 17 ++++++-----------
3 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index b3c5081..45f3731 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -135,7 +135,7 @@ extern int devinet_ioctl(unsigned int cmd, void __user *);
extern void devinet_init(void);
extern struct in_device *inetdev_by_index(int);
extern __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope);
-extern __be32 inet_confirm_addr(const struct net_device *dev, __be32 dst, __be32 local, int scope);
+extern __be32 inet_confirm_addr(struct in_device *in_dev, __be32 dst, __be32 local, int scope);
extern struct in_ifaddr *inet_ifa_byprefix(struct in_device *in_dev, __be32 prefix, __be32 mask);
static __inline__ int inet_ifa_match(__be32 addr, struct in_ifaddr *ifa)
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index e944d98..f38e1a9 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -382,8 +382,7 @@ static void arp_solicit(struct neighbour *neigh, struct sk_buff *skb)
read_unlock_bh(&neigh->lock);
}
-static int arp_ignore(struct in_device *in_dev, struct net_device *dev,
- __be32 sip, __be32 tip)
+static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
{
int scope;
@@ -403,7 +402,7 @@ static int arp_ignore(struct in_device *in_dev, struct net_device *dev,
case 3: /* Do not reply for scope host addresses */
sip = 0;
scope = RT_SCOPE_LINK;
- dev = NULL;
+ in_dev = NULL;
break;
case 4: /* Reserved */
case 5:
@@ -415,7 +414,7 @@ static int arp_ignore(struct in_device *in_dev, struct net_device *dev,
default:
return 0;
}
- return !inet_confirm_addr(dev, sip, tip, scope);
+ return !inet_confirm_addr(in_dev, sip, tip, scope);
}
static int arp_filter(__be32 sip, __be32 tip, struct net_device *dev)
@@ -807,7 +806,7 @@ static int arp_process(struct sk_buff *skb)
if (sip == 0) {
if (arp->ar_op == htons(ARPOP_REQUEST) &&
inet_addr_type(&init_net, tip) == RTN_LOCAL &&
- !arp_ignore(in_dev,dev,sip,tip))
+ !arp_ignore(in_dev, sip, tip))
arp_send(ARPOP_REPLY, ETH_P_ARP, sip, dev, tip, sha,
dev->dev_addr, sha);
goto out;
@@ -825,7 +824,7 @@ static int arp_process(struct sk_buff *skb)
int dont_send = 0;
if (!dont_send)
- dont_send |= arp_ignore(in_dev,dev,sip,tip);
+ dont_send |= arp_ignore(in_dev,sip,tip);
if (!dont_send && IN_DEV_ARPFILTER(in_dev))
dont_send |= arp_filter(sip,tip,dev);
if (!dont_send)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 03db15b..dc1665a 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -968,24 +968,19 @@ static __be32 confirm_addr_indev(struct in_device *in_dev, __be32 dst,
/*
* Confirm that local IP address exists using wildcards:
- * - dev: only on this interface, 0=any interface
+ * - in_dev: only on this interface, 0=any interface
* - dst: only in the same subnet as dst, 0=any dst
* - local: address, 0=autoselect the local address
* - scope: maximum allowed scope value for the local address
*/
-__be32 inet_confirm_addr(const struct net_device *dev, __be32 dst, __be32 local, int scope)
+__be32 inet_confirm_addr(struct in_device *in_dev,
+ __be32 dst, __be32 local, int scope)
{
__be32 addr = 0;
- struct in_device *in_dev;
-
- if (dev) {
- rcu_read_lock();
- if ((in_dev = __in_dev_get_rcu(dev)))
- addr = confirm_addr_indev(in_dev, dst, local, scope);
- rcu_read_unlock();
+ struct net_device *dev;
- return addr;
- }
+ if (in_dev != NULL)
+ return confirm_addr_indev(in_dev, dst, local, scope);
read_lock(&dev_base_lock);
rcu_read_lock();
--
1.5.3.rc5
|
|
|
Re: [PATCH 6/8 net-2.6.25] [ARP] neigh_parms_put(destroy) are essentially local to core/neighbour.c. [message #26062 is a reply to message #26023] |
Tue, 15 January 2008 07:04   |
davem
Messages: 463 Registered: February 2006
|
Senior Member |
|
|
From: "Denis V. Lunev" <den@openvz.org>
Date: Mon, 14 Jan 2008 17:59:59 +0300
> Make them static.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
It's completely awkward to put the inline after all
the call sites. A non-global-optimizing compiler
won't be able to even inline it.
I've reworked this patch as follows when applying
it.
diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index a0d42a5..ebbfb50 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -213,7 +213,6 @@ extern struct neighbour *neigh_event_ns(struct neigh_table *tbl,
extern struct neigh_parms *neigh_parms_alloc(struct net_device *dev, struct neigh_table *tbl);
extern void neigh_parms_release(struct neigh_table *tbl, struct neigh_parms *parms);
-extern void neigh_parms_destroy(struct neigh_parms *parms);
extern unsigned long neigh_rand_reach_time(unsigned long base);
extern void pneigh_enqueue(struct neigh_table *tbl, struct neigh_parms *p,
@@ -254,12 +253,6 @@ static inline void __neigh_parms_put(struct neigh_parms *parms)
atomic_dec(&parms->refcnt);
}
-static inline void neigh_parms_put(struct neigh_parms *parms)
-{
- if (atomic_dec_and_test(&parms->refcnt))
- neigh_parms_destroy(parms);
-}
-
static inline struct neigh_parms *neigh_parms_clone(struct neigh_parms *parms)
{
atomic_inc(&parms->refcnt);
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 9b0b773..7cc772a 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -577,6 +577,13 @@ static int pneigh_ifdown(struct neigh_table *tbl, struct net_device *dev)
return -ENOENT;
}
+static void neigh_parms_destroy(struct neigh_parms *parms);
+
+static inline void neigh_parms_put(struct neigh_parms *parms)
+{
+ if (atomic_dec_and_test(&parms->refcnt))
+ neigh_parms_destroy(parms);
+}
/*
* neighbour must already be out of the table;
@@ -1348,7 +1355,7 @@ void neigh_parms_release(struct neigh_table *tbl, struct neigh_parms *parms)
NEIGH_PRINTK1("neigh_parms_release: not found\n");
}
-void neigh_parms_destroy(struct neigh_parms *parms)
+static void neigh_parms_destroy(struct neigh_parms *parms)
{
release_net(parms->net);
if (parms->dev)
|
|
|
Re: [PATCH 0/8 2.6.25] a set of small code cleanups [message #26063 is a reply to message #26017] |
Tue, 15 January 2008 07:06  |
davem
Messages: 463 Registered: February 2006
|
Senior Member |
|
|
From: "Denis V. Lunev" <den@openvz.org>
Date: Mon, 14 Jan 2008 17:59:23 +0300
> This set contains a set of small improvements found in IPv4 code during
> preparations to support ARP in the network namespace. These fixes are
> mostly independent except 2 last ones.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
All applied, thanks Denis.
|
|
|
Goto Forum:
Current Time: Thu Aug 14 14:11:51 GMT 2025
Total time taken to generate the page: 0.24414 seconds
|