* [GIT PULL nf-next v2] IPVS fixes for v3.10
@ 2013-04-23 3:03 Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 1/9] ipvs: properly dereference dest_dst in ip_vs_forget_dev Simon Horman
` (9 more replies)
0 siblings, 10 replies; 11+ messages in thread
From: Simon Horman @ 2013-04-23 3:03 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Dan Carpenter, Simon Horman
Hi Pablo,
this is a revision of a previous pull request.
The previous pull request contained one fix-all patch.
As per your request it has been split into a number of separate patches.
This request also includes several new patches that fix
yet more problems flagged by static analysis.
Separately, I have also posted the following SCTP patch
"[PATCH] sctp: Correct type and usage of sctp_end_cksum()"
With this series and the above patch applied I
believe that IPVS is now sparse-clean.
Much thanks to Julian Anastasov for his patience, and
Dan Carpenter and Hans Schillstrom for their help.
The following changes since commit 3e3251b3f289528732edab386ddf73ac428359b7:
net: sctp: minor: remove dead code from sctp_packet (2013-04-22 16:25:21 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-next.git tags/ipvs3-for-v3.10
for you to fetch changes up to 38561437d056b11f679f9735d68ad597ba67dc84:
ipvs: Use network byte order for sync message size (2013-04-23 11:43:06 +0900)
----------------------------------------------------------------
IPVS fixes for v3.10
This series fixes a number of problems which were flagged by
static analysis.
The first two patches in the series resolve problems that
have been introduced since v3.9.
* ipvs: fix sparse warnings for ip_vs_conn listing
* ipvs: properly dereference dest_dst in ip_vs_forget_dev
The remaining patches resolve problems that have been around for longer.
It is not obvious to me that any of the problems resolved by
these patches manifest as bugs.
----------------------------------------------------------------
Dan Carpenter (1):
ipvs: off by one in set_sctp_state()
Julian Anastasov (5):
ipvs: properly dereference dest_dst in ip_vs_forget_dev
ipvs: fix sparse warnings for ip_vs_conn listing
ipvs: fix the remaining sparse warnings in ip_vs_ctl.c
ipvs: fix sparse warnings in lblc and lblcr
ipvs: fix sparse warnings for some parameters
Simon Horman (3):
ipvs: Avoid shadowing net variable in ip_vs_leave()
ipvs: Use min3() in ip_vs_dbg_callid()
ipvs: Use network byte order for sync message size
include/net/ip_vs.h | 8 ++---
include/uapi/linux/ip_vs.h | 4 +--
net/netfilter/ipvs/ip_vs_conn.c | 14 ++++-----
net/netfilter/ipvs/ip_vs_core.c | 7 +++--
net/netfilter/ipvs/ip_vs_ctl.c | 55 ++++++++++++++++++++++-------------
net/netfilter/ipvs/ip_vs_lblc.c | 2 +-
net/netfilter/ipvs/ip_vs_lblcr.c | 2 +-
net/netfilter/ipvs/ip_vs_pe_sip.c | 3 +-
net/netfilter/ipvs/ip_vs_proto_sctp.c | 2 +-
net/netfilter/ipvs/ip_vs_sync.c | 21 +++++--------
10 files changed, 63 insertions(+), 55 deletions(-)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH nf-next 1/9] ipvs: properly dereference dest_dst in ip_vs_forget_dev
2013-04-23 3:03 [GIT PULL nf-next v2] IPVS fixes for v3.10 Simon Horman
@ 2013-04-23 3:03 ` Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 2/9] ipvs: fix sparse warnings for ip_vs_conn listing Simon Horman
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Simon Horman @ 2013-04-23 3:03 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Dan Carpenter, Simon Horman
From: Julian Anastasov <ja@ssi.bg>
Use rcu_dereference_protected to resolve
sparse warning, found by kbuild test robot:
net/netfilter/ipvs/ip_vs_ctl.c:1464:35: warning: dereference of
noderef expression
Problem from commit 026ace060dfe29
("ipvs: optimize dst usage for real server")
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_ctl.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 9e4074c..5a65444 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1460,8 +1460,11 @@ void ip_vs_service_net_cleanup(struct net *net)
static inline void
ip_vs_forget_dev(struct ip_vs_dest *dest, struct net_device *dev)
{
+ struct ip_vs_dest_dst *dest_dst;
+
spin_lock_bh(&dest->dst_lock);
- if (dest->dest_dst && dest->dest_dst->dst_cache->dev == dev) {
+ dest_dst = rcu_dereference_protected(dest->dest_dst, 1);
+ if (dest_dst && dest_dst->dst_cache->dev == dev) {
IP_VS_DBG_BUF(3, "Reset dev:%s dest %s:%u ,dest->refcnt=%d\n",
dev->name,
IP_VS_DBG_ADDR(dest->af, &dest->addr),
--
1.8.2.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH nf-next 2/9] ipvs: fix sparse warnings for ip_vs_conn listing
2013-04-23 3:03 [GIT PULL nf-next v2] IPVS fixes for v3.10 Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 1/9] ipvs: properly dereference dest_dst in ip_vs_forget_dev Simon Horman
@ 2013-04-23 3:03 ` Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 3/9] ipvs: fix the remaining sparse warnings in ip_vs_ctl.c Simon Horman
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Simon Horman @ 2013-04-23 3:03 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Dan Carpenter, Simon Horman
From: Julian Anastasov <ja@ssi.bg>
kbuild test robot reports for sparse warnings
in commit 088339a57d6042 ("ipvs: convert connection locking"):
net/netfilter/ipvs/ip_vs_conn.c:962:13: warning: context imbalance
in 'ip_vs_conn_array' - wrong count at exit
include/linux/rcupdate.h:326:30: warning: context imbalance in
'ip_vs_conn_seq_next' - unexpected unlock
include/linux/rcupdate.h:326:30: warning: context imbalance in
'ip_vs_conn_seq_stop' - unexpected unlock
Fix it by running ip_vs_conn_array under RCU lock
to avoid conditional locking and by adding proper RCU
annotations.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_conn.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index de64758..a083bda 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -966,7 +966,6 @@ static void *ip_vs_conn_array(struct seq_file *seq, loff_t pos)
struct ip_vs_iter_state *iter = seq->private;
for (idx = 0; idx < ip_vs_conn_tab_size; idx++) {
- rcu_read_lock();
hlist_for_each_entry_rcu(cp, &ip_vs_conn_tab[idx], c_list) {
/* __ip_vs_conn_get() is not needed by
* ip_vs_conn_seq_show and ip_vs_conn_sync_seq_show
@@ -977,16 +976,19 @@ static void *ip_vs_conn_array(struct seq_file *seq, loff_t pos)
}
}
rcu_read_unlock();
+ rcu_read_lock();
}
return NULL;
}
static void *ip_vs_conn_seq_start(struct seq_file *seq, loff_t *pos)
+ __acquires(RCU)
{
struct ip_vs_iter_state *iter = seq->private;
iter->l = NULL;
+ rcu_read_lock();
return *pos ? ip_vs_conn_array(seq, *pos - 1) :SEQ_START_TOKEN;
}
@@ -1006,28 +1008,24 @@ static void *ip_vs_conn_seq_next(struct seq_file *seq, void *v, loff_t *pos)
e = rcu_dereference(hlist_next_rcu(&cp->c_list));
if (e)
return hlist_entry(e, struct ip_vs_conn, c_list);
- rcu_read_unlock();
idx = l - ip_vs_conn_tab;
while (++idx < ip_vs_conn_tab_size) {
- rcu_read_lock();
hlist_for_each_entry_rcu(cp, &ip_vs_conn_tab[idx], c_list) {
iter->l = &ip_vs_conn_tab[idx];
return cp;
}
rcu_read_unlock();
+ rcu_read_lock();
}
iter->l = NULL;
return NULL;
}
static void ip_vs_conn_seq_stop(struct seq_file *seq, void *v)
+ __releases(RCU)
{
- struct ip_vs_iter_state *iter = seq->private;
- struct hlist_head *l = iter->l;
-
- if (l)
- rcu_read_unlock();
+ rcu_read_unlock();
}
static int ip_vs_conn_seq_show(struct seq_file *seq, void *v)
--
1.8.2.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH nf-next 3/9] ipvs: fix the remaining sparse warnings in ip_vs_ctl.c
2013-04-23 3:03 [GIT PULL nf-next v2] IPVS fixes for v3.10 Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 1/9] ipvs: properly dereference dest_dst in ip_vs_forget_dev Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 2/9] ipvs: fix sparse warnings for ip_vs_conn listing Simon Horman
@ 2013-04-23 3:03 ` Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 4/9] ipvs: fix sparse warnings in lblc and lblcr Simon Horman
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Simon Horman @ 2013-04-23 3:03 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Dan Carpenter, Simon Horman
From: Julian Anastasov <ja@ssi.bg>
- RCU annotations for ip_vs_info_seq_start and _stop
- __percpu for cpustats
- properly dereference svc->pe in ip_vs_genl_fill_service
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_ctl.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 5a65444..64075a7 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1937,8 +1937,8 @@ static struct ip_vs_service *ip_vs_info_array(struct seq_file *seq, loff_t pos)
}
static void *ip_vs_info_seq_start(struct seq_file *seq, loff_t *pos)
+ __acquires(RCU)
{
-
rcu_read_lock();
return *pos ? ip_vs_info_array(seq, *pos - 1) : SEQ_START_TOKEN;
}
@@ -1993,6 +1993,7 @@ static void *ip_vs_info_seq_next(struct seq_file *seq, void *v, loff_t *pos)
}
static void ip_vs_info_seq_stop(struct seq_file *seq, void *v)
+ __releases(RCU)
{
rcu_read_unlock();
}
@@ -2137,7 +2138,7 @@ static int ip_vs_stats_percpu_show(struct seq_file *seq, void *v)
{
struct net *net = seq_file_single_net(seq);
struct ip_vs_stats *tot_stats = &net_ipvs(net)->tot_stats;
- struct ip_vs_cpu_stats *cpustats = tot_stats->cpustats;
+ struct ip_vs_cpu_stats __percpu *cpustats = tot_stats->cpustats;
struct ip_vs_stats_user rates;
int i;
@@ -2874,6 +2875,7 @@ static int ip_vs_genl_fill_service(struct sk_buff *skb,
struct ip_vs_service *svc)
{
struct ip_vs_scheduler *sched;
+ struct ip_vs_pe *pe;
struct nlattr *nl_service;
struct ip_vs_flags flags = { .flags = svc->flags,
.mask = ~0 };
@@ -2895,9 +2897,9 @@ static int ip_vs_genl_fill_service(struct sk_buff *skb,
}
sched = rcu_dereference_protected(svc->scheduler, 1);
+ pe = rcu_dereference_protected(svc->pe, 1);
if (nla_put_string(skb, IPVS_SVC_ATTR_SCHED_NAME, sched->name) ||
- (svc->pe &&
- nla_put_string(skb, IPVS_SVC_ATTR_PE_NAME, svc->pe->name)) ||
+ (pe && nla_put_string(skb, IPVS_SVC_ATTR_PE_NAME, pe->name)) ||
nla_put(skb, IPVS_SVC_ATTR_FLAGS, sizeof(flags), &flags) ||
nla_put_u32(skb, IPVS_SVC_ATTR_TIMEOUT, svc->timeout / HZ) ||
nla_put_u32(skb, IPVS_SVC_ATTR_NETMASK, svc->netmask))
--
1.8.2.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH nf-next 4/9] ipvs: fix sparse warnings in lblc and lblcr
2013-04-23 3:03 [GIT PULL nf-next v2] IPVS fixes for v3.10 Simon Horman
` (2 preceding siblings ...)
2013-04-23 3:03 ` [PATCH nf-next 3/9] ipvs: fix the remaining sparse warnings in ip_vs_ctl.c Simon Horman
@ 2013-04-23 3:03 ` Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 5/9] ipvs: fix sparse warnings for some parameters Simon Horman
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Simon Horman @ 2013-04-23 3:03 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Dan Carpenter, Simon Horman
From: Julian Anastasov <ja@ssi.bg>
kbuild test robot reports for sparse warnings in
commits c2a4ffb70eef39 ("ipvs: convert lblc scheduler to rcu")
and c5549571f975ab ("ipvs: convert lblcr scheduler to rcu").
Fix it by removing extra __rcu annotation.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_lblc.c | 2 +-
net/netfilter/ipvs/ip_vs_lblcr.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_lblc.c b/net/netfilter/ipvs/ip_vs_lblc.c
index b2cc252..5ea26bd 100644
--- a/net/netfilter/ipvs/ip_vs_lblc.c
+++ b/net/netfilter/ipvs/ip_vs_lblc.c
@@ -104,7 +104,7 @@ struct ip_vs_lblc_entry {
*/
struct ip_vs_lblc_table {
struct rcu_head rcu_head;
- struct hlist_head __rcu bucket[IP_VS_LBLC_TAB_SIZE]; /* hash bucket */
+ struct hlist_head bucket[IP_VS_LBLC_TAB_SIZE]; /* hash bucket */
struct timer_list periodic_timer; /* collect stale entries */
atomic_t entries; /* number of entries */
int max_size; /* maximum size of entries */
diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
index feb9656..50123c2 100644
--- a/net/netfilter/ipvs/ip_vs_lblcr.c
+++ b/net/netfilter/ipvs/ip_vs_lblcr.c
@@ -284,7 +284,7 @@ struct ip_vs_lblcr_entry {
*/
struct ip_vs_lblcr_table {
struct rcu_head rcu_head;
- struct hlist_head __rcu bucket[IP_VS_LBLCR_TAB_SIZE]; /* hash bucket */
+ struct hlist_head bucket[IP_VS_LBLCR_TAB_SIZE]; /* hash bucket */
atomic_t entries; /* number of entries */
int max_size; /* maximum size of entries */
struct timer_list periodic_timer; /* collect stale entries */
--
1.8.2.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH nf-next 5/9] ipvs: fix sparse warnings for some parameters
2013-04-23 3:03 [GIT PULL nf-next v2] IPVS fixes for v3.10 Simon Horman
` (3 preceding siblings ...)
2013-04-23 3:03 ` [PATCH nf-next 4/9] ipvs: fix sparse warnings in lblc and lblcr Simon Horman
@ 2013-04-23 3:03 ` Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 6/9] ipvs: Avoid shadowing net variable in ip_vs_leave() Simon Horman
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Simon Horman @ 2013-04-23 3:03 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Dan Carpenter, Simon Horman
From: Julian Anastasov <ja@ssi.bg>
Some service fields are in network order:
- netmask: used once in network order and also as prefix len for IPv6
- port
Other parameters are in host order:
- struct ip_vs_flags: flags and mask moved between user and kernel only
- sync state: moved between user and kernel only
- syncid: sent over network as single octet
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
include/net/ip_vs.h | 8 ++++----
include/uapi/linux/ip_vs.h | 4 ++--
net/netfilter/ipvs/ip_vs_core.c | 3 ++-
net/netfilter/ipvs/ip_vs_ctl.c | 40 ++++++++++++++++++++++++----------------
4 files changed, 32 insertions(+), 23 deletions(-)
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index f9f5b05..4c062cc 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -678,7 +678,7 @@ struct ip_vs_service_user_kern {
u16 af;
u16 protocol;
union nf_inet_addr addr; /* virtual ip address */
- u16 port;
+ __be16 port;
u32 fwmark; /* firwall mark of service */
/* virtual service options */
@@ -686,14 +686,14 @@ struct ip_vs_service_user_kern {
char *pe_name;
unsigned int flags; /* virtual service flags */
unsigned int timeout; /* persistent timeout in sec */
- u32 netmask; /* persistent netmask */
+ __be32 netmask; /* persistent netmask or plen */
};
struct ip_vs_dest_user_kern {
/* destination server address */
union nf_inet_addr addr;
- u16 port;
+ __be16 port;
/* real server options */
unsigned int conn_flags; /* connection flags */
@@ -721,7 +721,7 @@ struct ip_vs_service {
__u32 fwmark; /* firewall mark of the service */
unsigned int flags; /* service status flags */
unsigned int timeout; /* persistent timeout in ticks */
- __be32 netmask; /* grouping granularity */
+ __be32 netmask; /* grouping granularity, mask/plen */
struct net *net;
struct list_head destinations; /* real server d-linked list */
diff --git a/include/uapi/linux/ip_vs.h b/include/uapi/linux/ip_vs.h
index 8a2d438..a245377 100644
--- a/include/uapi/linux/ip_vs.h
+++ b/include/uapi/linux/ip_vs.h
@@ -280,8 +280,8 @@ struct ip_vs_daemon_user {
#define IPVS_GENL_VERSION 0x1
struct ip_vs_flags {
- __be32 flags;
- __be32 mask;
+ __u32 flags;
+ __u32 mask;
};
/* Generic Netlink command attributes */
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index f26fe33..a0d7bd3 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -235,7 +235,8 @@ ip_vs_sched_persist(struct ip_vs_service *svc,
/* Mask saddr with the netmask to adjust template granularity */
#ifdef CONFIG_IP_VS_IPV6
if (svc->af == AF_INET6)
- ipv6_addr_prefix(&snet.in6, &iph->saddr.in6, svc->netmask);
+ ipv6_addr_prefix(&snet.in6, &iph->saddr.in6,
+ (__force __u32) svc->netmask);
else
#endif
snet.ip = iph->saddr.ip & svc->netmask;
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 64075a7..5b142fb 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1164,9 +1164,13 @@ ip_vs_add_service(struct net *net, struct ip_vs_service_user_kern *u,
}
#ifdef CONFIG_IP_VS_IPV6
- if (u->af == AF_INET6 && (u->netmask < 1 || u->netmask > 128)) {
- ret = -EINVAL;
- goto out_err;
+ if (u->af == AF_INET6) {
+ __u32 plen = (__force __u32) u->netmask;
+
+ if (plen < 1 || plen > 128) {
+ ret = -EINVAL;
+ goto out_err;
+ }
}
#endif
@@ -1277,9 +1281,13 @@ ip_vs_edit_service(struct ip_vs_service *svc, struct ip_vs_service_user_kern *u)
}
#ifdef CONFIG_IP_VS_IPV6
- if (u->af == AF_INET6 && (u->netmask < 1 || u->netmask > 128)) {
- ret = -EINVAL;
- goto out;
+ if (u->af == AF_INET6) {
+ __u32 plen = (__force __u32) u->netmask;
+
+ if (plen < 1 || plen > 128) {
+ ret = -EINVAL;
+ goto out;
+ }
}
#endif
@@ -2892,7 +2900,7 @@ static int ip_vs_genl_fill_service(struct sk_buff *skb,
} else {
if (nla_put_u16(skb, IPVS_SVC_ATTR_PROTOCOL, svc->protocol) ||
nla_put(skb, IPVS_SVC_ATTR_ADDR, sizeof(svc->addr), &svc->addr) ||
- nla_put_u16(skb, IPVS_SVC_ATTR_PORT, svc->port))
+ nla_put_be16(skb, IPVS_SVC_ATTR_PORT, svc->port))
goto nla_put_failure;
}
@@ -2902,7 +2910,7 @@ static int ip_vs_genl_fill_service(struct sk_buff *skb,
(pe && nla_put_string(skb, IPVS_SVC_ATTR_PE_NAME, pe->name)) ||
nla_put(skb, IPVS_SVC_ATTR_FLAGS, sizeof(flags), &flags) ||
nla_put_u32(skb, IPVS_SVC_ATTR_TIMEOUT, svc->timeout / HZ) ||
- nla_put_u32(skb, IPVS_SVC_ATTR_NETMASK, svc->netmask))
+ nla_put_be32(skb, IPVS_SVC_ATTR_NETMASK, svc->netmask))
goto nla_put_failure;
if (ip_vs_genl_fill_stats(skb, IPVS_SVC_ATTR_STATS, &svc->stats))
goto nla_put_failure;
@@ -3015,7 +3023,7 @@ static int ip_vs_genl_parse_service(struct net *net,
} else {
usvc->protocol = nla_get_u16(nla_protocol);
nla_memcpy(&usvc->addr, nla_addr, sizeof(usvc->addr));
- usvc->port = nla_get_u16(nla_port);
+ usvc->port = nla_get_be16(nla_port);
usvc->fwmark = 0;
}
@@ -3055,7 +3063,7 @@ static int ip_vs_genl_parse_service(struct net *net,
usvc->sched_name = nla_data(nla_sched);
usvc->pe_name = nla_pe ? nla_data(nla_pe) : NULL;
usvc->timeout = nla_get_u32(nla_timeout);
- usvc->netmask = nla_get_u32(nla_netmask);
+ usvc->netmask = nla_get_be32(nla_netmask);
}
return 0;
@@ -3081,7 +3089,7 @@ static int ip_vs_genl_fill_dest(struct sk_buff *skb, struct ip_vs_dest *dest)
return -EMSGSIZE;
if (nla_put(skb, IPVS_DEST_ATTR_ADDR, sizeof(dest->addr), &dest->addr) ||
- nla_put_u16(skb, IPVS_DEST_ATTR_PORT, dest->port) ||
+ nla_put_be16(skb, IPVS_DEST_ATTR_PORT, dest->port) ||
nla_put_u32(skb, IPVS_DEST_ATTR_FWD_METHOD,
(atomic_read(&dest->conn_flags) &
IP_VS_CONN_F_FWD_MASK)) ||
@@ -3190,7 +3198,7 @@ static int ip_vs_genl_parse_dest(struct ip_vs_dest_user_kern *udest,
memset(udest, 0, sizeof(*udest));
nla_memcpy(&udest->addr, nla_addr, sizeof(udest->addr));
- udest->port = nla_get_u16(nla_port);
+ udest->port = nla_get_be16(nla_port);
/* If a full entry was requested, check for the additional fields */
if (full_entry) {
@@ -3215,8 +3223,8 @@ static int ip_vs_genl_parse_dest(struct ip_vs_dest_user_kern *udest,
return 0;
}
-static int ip_vs_genl_fill_daemon(struct sk_buff *skb, __be32 state,
- const char *mcast_ifn, __be32 syncid)
+static int ip_vs_genl_fill_daemon(struct sk_buff *skb, __u32 state,
+ const char *mcast_ifn, __u32 syncid)
{
struct nlattr *nl_daemon;
@@ -3237,8 +3245,8 @@ nla_put_failure:
return -EMSGSIZE;
}
-static int ip_vs_genl_dump_daemon(struct sk_buff *skb, __be32 state,
- const char *mcast_ifn, __be32 syncid,
+static int ip_vs_genl_dump_daemon(struct sk_buff *skb, __u32 state,
+ const char *mcast_ifn, __u32 syncid,
struct netlink_callback *cb)
{
void *hdr;
--
1.8.2.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH nf-next 6/9] ipvs: Avoid shadowing net variable in ip_vs_leave()
2013-04-23 3:03 [GIT PULL nf-next v2] IPVS fixes for v3.10 Simon Horman
` (4 preceding siblings ...)
2013-04-23 3:03 ` [PATCH nf-next 5/9] ipvs: fix sparse warnings for some parameters Simon Horman
@ 2013-04-23 3:03 ` Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 7/9] ipvs: Use min3() in ip_vs_dbg_callid() Simon Horman
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Simon Horman @ 2013-04-23 3:03 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Dan Carpenter, Simon Horman
Flagged by sparse.
Compile and sparse tested only.
Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index a0d7bd3..085b588 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -584,9 +584,9 @@ int ip_vs_leave(struct ip_vs_service *svc, struct sk_buff *skb,
#ifdef CONFIG_IP_VS_IPV6
if (svc->af == AF_INET6) {
if (!skb->dev) {
- struct net *net = dev_net(skb_dst(skb)->dev);
+ struct net *net_ = dev_net(skb_dst(skb)->dev);
- skb->dev = net->loopback_dev;
+ skb->dev = net_->loopback_dev;
}
icmpv6_send(skb, ICMPV6_DEST_UNREACH, ICMPV6_PORT_UNREACH, 0);
} else
--
1.8.2.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH nf-next 7/9] ipvs: Use min3() in ip_vs_dbg_callid()
2013-04-23 3:03 [GIT PULL nf-next v2] IPVS fixes for v3.10 Simon Horman
` (5 preceding siblings ...)
2013-04-23 3:03 ` [PATCH nf-next 6/9] ipvs: Avoid shadowing net variable in ip_vs_leave() Simon Horman
@ 2013-04-23 3:03 ` Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 8/9] ipvs: off by one in set_sctp_state() Simon Horman
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Simon Horman @ 2013-04-23 3:03 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Dan Carpenter, Simon Horman
There are two motivations for this:
1. It improves readability to my eyes
2. Using nested min() calls results in a shadowed _min1 variable,
which is a bit untidy. Sparse complained about this.
I have also replaced (size_t)64 with a variable of type size_t and value 64.
This also improves readability to my eyes.
Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_pe_sip.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/ipvs/ip_vs_pe_sip.c b/net/netfilter/ipvs/ip_vs_pe_sip.c
index 00cc024..9a8f421 100644
--- a/net/netfilter/ipvs/ip_vs_pe_sip.c
+++ b/net/netfilter/ipvs/ip_vs_pe_sip.c
@@ -13,7 +13,8 @@ static const char *ip_vs_dbg_callid(char *buf, size_t buf_len,
const char *callid, size_t callid_len,
int *idx)
{
- size_t len = min(min(callid_len, (size_t)64), buf_len - *idx - 1);
+ size_t max_len = 64;
+ size_t len = min3(max_len, callid_len, buf_len - *idx - 1);
memcpy(buf + *idx, callid, len);
buf[*idx+len] = '\0';
*idx += len + 1;
--
1.8.2.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH nf-next 8/9] ipvs: off by one in set_sctp_state()
2013-04-23 3:03 [GIT PULL nf-next v2] IPVS fixes for v3.10 Simon Horman
` (6 preceding siblings ...)
2013-04-23 3:03 ` [PATCH nf-next 7/9] ipvs: Use min3() in ip_vs_dbg_callid() Simon Horman
@ 2013-04-23 3:03 ` Simon Horman
2013-04-23 3:04 ` [PATCH nf-next 9/9] ipvs: Use network byte order for sync message size Simon Horman
2013-04-24 23:42 ` [GIT PULL nf-next v2] IPVS fixes for v3.10 Pablo Neira Ayuso
9 siblings, 0 replies; 11+ messages in thread
From: Simon Horman @ 2013-04-23 3:03 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Dan Carpenter, Simon Horman
From: Dan Carpenter <dan.carpenter@oracle.com>
The sctp_events[] come from sch->type in set_sctp_state(). They are
between 0-255 so that means we need 256 elements in the array.
I believe that because of how the code is aligned there is normally a
hole after sctp_events[] so this patch doesn't actually change anything.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_proto_sctp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
index 6e14a7b..8646488 100644
--- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
@@ -208,7 +208,7 @@ enum ipvs_sctp_event_t {
IP_VS_SCTP_EVE_LAST
};
-static enum ipvs_sctp_event_t sctp_events[255] = {
+static enum ipvs_sctp_event_t sctp_events[256] = {
IP_VS_SCTP_EVE_DATA_CLI,
IP_VS_SCTP_EVE_INIT_CLI,
IP_VS_SCTP_EVE_INIT_ACK_CLI,
--
1.8.2.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH nf-next 9/9] ipvs: Use network byte order for sync message size
2013-04-23 3:03 [GIT PULL nf-next v2] IPVS fixes for v3.10 Simon Horman
` (7 preceding siblings ...)
2013-04-23 3:03 ` [PATCH nf-next 8/9] ipvs: off by one in set_sctp_state() Simon Horman
@ 2013-04-23 3:04 ` Simon Horman
2013-04-24 23:42 ` [GIT PULL nf-next v2] IPVS fixes for v3.10 Pablo Neira Ayuso
9 siblings, 0 replies; 11+ messages in thread
From: Simon Horman @ 2013-04-23 3:04 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Dan Carpenter, Simon Horman
struct ip_vs_sync_mesg and ip_vs_sync_mesg_v0 are both sent across the wire
and used internally to store IPVS synchronisation messages.
Up until now the scheme used has been to convert the size field
to network byte order before sending a message on the wire and
convert it to host byte order when sending a message.
This patch changes that scheme to always treat the field
as being network byte order. This seems appropriate as
the structure is sent across the wire. And by consistently
treating the field has network byte order it is now possible
to take advantage of sparse to flag any future miss-use.
Acked-by: Julian Anastasov <ja@ssi.bg>
Acked-by: Hans Schillstrom <hans@schillstrom.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_sync.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 8e57077..f6046d9 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -246,7 +246,7 @@ struct ip_vs_sync_thread_data {
struct ip_vs_sync_mesg_v0 {
__u8 nr_conns;
__u8 syncid;
- __u16 size;
+ __be16 size;
/* ip_vs_sync_conn entries start here */
};
@@ -255,7 +255,7 @@ struct ip_vs_sync_mesg_v0 {
struct ip_vs_sync_mesg {
__u8 reserved; /* must be zero */
__u8 syncid;
- __u16 size;
+ __be16 size;
__u8 nr_conns;
__s8 version; /* SYNC_PROTO_VER */
__u16 spare;
@@ -335,7 +335,7 @@ ip_vs_sync_buff_create(struct netns_ipvs *ipvs)
sb->mesg->reserved = 0; /* old nr_conns i.e. must be zero now */
sb->mesg->version = SYNC_PROTO_VER;
sb->mesg->syncid = ipvs->master_syncid;
- sb->mesg->size = sizeof(struct ip_vs_sync_mesg);
+ sb->mesg->size = htons(sizeof(struct ip_vs_sync_mesg));
sb->mesg->nr_conns = 0;
sb->mesg->spare = 0;
sb->head = (unsigned char *)sb->mesg + sizeof(struct ip_vs_sync_mesg);
@@ -418,7 +418,7 @@ ip_vs_sync_buff_create_v0(struct netns_ipvs *ipvs)
mesg = (struct ip_vs_sync_mesg_v0 *)sb->mesg;
mesg->nr_conns = 0;
mesg->syncid = ipvs->master_syncid;
- mesg->size = sizeof(struct ip_vs_sync_mesg_v0);
+ mesg->size = htons(sizeof(struct ip_vs_sync_mesg_v0));
sb->head = (unsigned char *)mesg + sizeof(struct ip_vs_sync_mesg_v0);
sb->end = (unsigned char *)mesg + ipvs->send_mesg_maxlen;
sb->firstuse = jiffies;
@@ -582,7 +582,7 @@ static void ip_vs_sync_conn_v0(struct net *net, struct ip_vs_conn *cp,
}
m->nr_conns++;
- m->size += len;
+ m->size = htons(ntohs(m->size) + len);
buff->head += len;
/* check if there is a space for next one */
@@ -693,7 +693,7 @@ sloop:
p = buff->head;
buff->head += pad + len;
- m->size += pad + len;
+ m->size = htons(ntohs(m->size) + pad + len);
/* Add ev. padding from prev. sync_conn */
while (pad--)
*(p++) = 0;
@@ -1175,10 +1175,8 @@ static void ip_vs_process_message(struct net *net, __u8 *buffer,
IP_VS_DBG(2, "BACKUP, message header too short\n");
return;
}
- /* Convert size back to host byte order */
- m2->size = ntohs(m2->size);
- if (buflen != m2->size) {
+ if (buflen != ntohs(m2->size)) {
IP_VS_DBG(2, "BACKUP, bogus message size\n");
return;
}
@@ -1544,10 +1542,7 @@ ip_vs_send_sync_msg(struct socket *sock, struct ip_vs_sync_mesg *msg)
int msize;
int ret;
- msize = msg->size;
-
- /* Put size in network byte order */
- msg->size = htons(msg->size);
+ msize = ntohs(msg->size);
ret = ip_vs_send_async(sock, (char *)msg, msize);
if (ret >= 0 || ret == -EAGAIN)
--
1.8.2.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [GIT PULL nf-next v2] IPVS fixes for v3.10
2013-04-23 3:03 [GIT PULL nf-next v2] IPVS fixes for v3.10 Simon Horman
` (8 preceding siblings ...)
2013-04-23 3:04 ` [PATCH nf-next 9/9] ipvs: Use network byte order for sync message size Simon Horman
@ 2013-04-24 23:42 ` Pablo Neira Ayuso
9 siblings, 0 replies; 11+ messages in thread
From: Pablo Neira Ayuso @ 2013-04-24 23:42 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Dan Carpenter
On Tue, Apr 23, 2013 at 12:03:51PM +0900, Simon Horman wrote:
> Hi Pablo,
>
> this is a revision of a previous pull request.
> The previous pull request contained one fix-all patch.
> As per your request it has been split into a number of separate patches.
> This request also includes several new patches that fix
> yet more problems flagged by static analysis.
>
> Separately, I have also posted the following SCTP patch
> "[PATCH] sctp: Correct type and usage of sctp_end_cksum()"
>
> With this series and the above patch applied I
> believe that IPVS is now sparse-clean.
>
> Much thanks to Julian Anastasov for his patience, and
> Dan Carpenter and Hans Schillstrom for their help.
Pulled, thanks Simon!
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-04-24 23:42 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-23 3:03 [GIT PULL nf-next v2] IPVS fixes for v3.10 Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 1/9] ipvs: properly dereference dest_dst in ip_vs_forget_dev Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 2/9] ipvs: fix sparse warnings for ip_vs_conn listing Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 3/9] ipvs: fix the remaining sparse warnings in ip_vs_ctl.c Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 4/9] ipvs: fix sparse warnings in lblc and lblcr Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 5/9] ipvs: fix sparse warnings for some parameters Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 6/9] ipvs: Avoid shadowing net variable in ip_vs_leave() Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 7/9] ipvs: Use min3() in ip_vs_dbg_callid() Simon Horman
2013-04-23 3:03 ` [PATCH nf-next 8/9] ipvs: off by one in set_sctp_state() Simon Horman
2013-04-23 3:04 ` [PATCH nf-next 9/9] ipvs: Use network byte order for sync message size Simon Horman
2013-04-24 23:42 ` [GIT PULL nf-next v2] IPVS fixes for v3.10 Pablo Neira Ayuso
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).