* [PATCH net-next] udp: Neaten and reduce size of compute_score functions
@ 2014-12-02 1:39 Joe Perches
2014-12-02 2:59 ` Eric Dumazet
0 siblings, 1 reply; 8+ messages in thread
From: Joe Perches @ 2014-12-02 1:39 UTC (permalink / raw)
To: netdev; +Cc: LKML
The compute_score functions are a bit difficult to read.
Neaten them a bit to reduce object sizes and make them a
bit more intelligible.
Return early to avoid indentation and avoid unnecessary
initializations.
(allyesconfig, but w/ -O2 and no profiling)
$ size net/ipv[46]/udp.o.*
text data bss dec hex filename
28680 1184 25 29889 74c1 net/ipv4/udp.o.new
28756 1184 25 29965 750d net/ipv4/udp.o.old
17600 1010 2 18612 48b4 net/ipv6/udp.o.new
17632 1010 2 18644 48d4 net/ipv6/udp.o.old
Signed-off-by: Joe Perches <joe@perches.com>
---
net/ipv4/udp.c | 111 +++++++++++++++++++++++++++++++-------------------------
net/ipv6/udp.c | 113 ++++++++++++++++++++++++++++++++-------------------------
2 files changed, 125 insertions(+), 99 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index b2d6068..227e6ad 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -336,38 +336,45 @@ int udp_v4_get_port(struct sock *sk, unsigned short snum)
return udp_lib_get_port(sk, snum, ipv4_rcv_saddr_equal, hash2_nulladdr);
}
-static inline int compute_score(struct sock *sk, struct net *net, __be32 saddr,
- unsigned short hnum,
- __be16 sport, __be32 daddr, __be16 dport, int dif)
+static inline int compute_score(struct sock *sk, struct net *net,
+ __be32 saddr, unsigned short hnum, __be16 sport,
+ __be32 daddr, __be16 dport, int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
- if (net_eq(sock_net(sk), net) && udp_sk(sk)->udp_port_hash == hnum &&
- !ipv6_only_sock(sk)) {
- struct inet_sock *inet = inet_sk(sk);
+ if (!(net_eq(sock_net(sk), net) &&
+ udp_sk(sk)->udp_port_hash == hnum &&
+ !ipv6_only_sock(sk)))
+ return -1;
- score = (sk->sk_family == PF_INET ? 2 : 1);
- if (inet->inet_rcv_saddr) {
- if (inet->inet_rcv_saddr != daddr)
- return -1;
- score += 4;
- }
- if (inet->inet_daddr) {
- if (inet->inet_daddr != saddr)
- return -1;
- score += 4;
- }
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score += 4;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score += 4;
- }
+ score = (sk->sk_family == PF_INET) ? 2 : 1;
+ inet = inet_sk(sk);
+
+ if (inet->inet_rcv_saddr) {
+ if (inet->inet_rcv_saddr != daddr)
+ return -1;
+ score += 4;
+ }
+
+ if (inet->inet_daddr) {
+ if (inet->inet_daddr != saddr)
+ return -1;
+ score += 4;
+ }
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
+ return -1;
+ score += 4;
+ }
+
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score += 4;
}
+
return score;
}
@@ -378,33 +385,39 @@ static inline int compute_score2(struct sock *sk, struct net *net,
__be32 saddr, __be16 sport,
__be32 daddr, unsigned int hnum, int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
- if (net_eq(sock_net(sk), net) && !ipv6_only_sock(sk)) {
- struct inet_sock *inet = inet_sk(sk);
+ if (!(net_eq(sock_net(sk), net) && !ipv6_only_sock(sk)))
+ return -1;
- if (inet->inet_rcv_saddr != daddr)
+ inet = inet_sk(sk);
+
+ if (inet->inet_rcv_saddr != daddr)
+ return -1;
+ if (inet->inet_num != hnum)
+ return -1;
+
+ score = (sk->sk_family == PF_INET) ? 2 : 1;
+
+ if (inet->inet_daddr) {
+ if (inet->inet_daddr != saddr)
return -1;
- if (inet->inet_num != hnum)
+ score += 4;
+ }
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
return -1;
+ score += 4;
+ }
- score = (sk->sk_family == PF_INET ? 2 : 1);
- if (inet->inet_daddr) {
- if (inet->inet_daddr != saddr)
- return -1;
- score += 4;
- }
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score += 4;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score += 4;
- }
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score += 4;
}
+
return score;
}
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 7cfb5d7..0b9644a 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -148,72 +148,85 @@ static inline int compute_score(struct sock *sk, struct net *net,
const struct in6_addr *daddr, __be16 dport,
int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
- if (net_eq(sock_net(sk), net) && udp_sk(sk)->udp_port_hash == hnum &&
- sk->sk_family == PF_INET6) {
- struct inet_sock *inet = inet_sk(sk);
+ if (!(net_eq(sock_net(sk), net) &&
+ udp_sk(sk)->udp_port_hash == hnum &&
+ sk->sk_family == PF_INET6))
+ return -1;
- score = 0;
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score++;
- }
- if (!ipv6_addr_any(&sk->sk_v6_rcv_saddr)) {
- if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
- return -1;
- score++;
- }
- if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
- if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
- return -1;
- score++;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score++;
- }
+ score = 0;
+ inet = inet_sk(sk);
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
+ return -1;
+ score++;
}
+
+ if (!ipv6_addr_any(&sk->sk_v6_rcv_saddr)) {
+ if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
+ return -1;
+ score++;
+ }
+
+ if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
+ if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
+ return -1;
+ score++;
+ }
+
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score++;
+ }
+
return score;
}
#define SCORE2_MAX (1 + 1 + 1)
static inline int compute_score2(struct sock *sk, struct net *net,
- const struct in6_addr *saddr, __be16 sport,
- const struct in6_addr *daddr, unsigned short hnum,
- int dif)
+ const struct in6_addr *saddr, __be16 sport,
+ const struct in6_addr *daddr,
+ unsigned short hnum, int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
- if (net_eq(sock_net(sk), net) && udp_sk(sk)->udp_port_hash == hnum &&
- sk->sk_family == PF_INET6) {
- struct inet_sock *inet = inet_sk(sk);
+ if (!(net_eq(sock_net(sk), net) &&
+ udp_sk(sk)->udp_port_hash == hnum &&
+ sk->sk_family == PF_INET6))
+ return -1;
- if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
+ if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
+ return -1;
+
+ score = 0;
+ inet = inet_sk(sk);
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
return -1;
- score = 0;
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score++;
- }
- if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
- if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
- return -1;
- score++;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score++;
- }
+ score++;
}
+
+ if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
+ if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
+ return -1;
+ score++;
+ }
+
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score++;
+ }
+
return score;
}
-
/* called with read_rcu_lock() */
static struct sock *udp6_lib_lookup2(struct net *net,
const struct in6_addr *saddr, __be16 sport,
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] udp: Neaten and reduce size of compute_score functions
2014-12-02 1:39 [PATCH net-next] udp: Neaten and reduce size of compute_score functions Joe Perches
@ 2014-12-02 2:59 ` Eric Dumazet
2014-12-02 3:09 ` Joe Perches
0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2014-12-02 2:59 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, LKML
On Mon, 2014-12-01 at 17:39 -0800, Joe Perches wrote:
> The compute_score functions are a bit difficult to read.
>
> Neaten them a bit to reduce object sizes and make them a
> bit more intelligible.
>
> Return early to avoid indentation and avoid unnecessary
> initializations.
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> net/ipv4/udp.c | 111 +++++++++++++++++++++++++++++++-------------------------
> net/ipv6/udp.c | 113 ++++++++++++++++++++++++++++++++-------------------------
> 2 files changed, 125 insertions(+), 99 deletions(-)
>
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index b2d6068..227e6ad 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -336,38 +336,45 @@ int udp_v4_get_port(struct sock *sk, unsigned short snum)
> return udp_lib_get_port(sk, snum, ipv4_rcv_saddr_equal, hash2_nulladdr);
> }
>
> -static inline int compute_score(struct sock *sk, struct net *net, __be32 saddr,
> - unsigned short hnum,
> - __be16 sport, __be32 daddr, __be16 dport, int dif)
> +static inline int compute_score(struct sock *sk, struct net *net,
> + __be32 saddr, unsigned short hnum, __be16 sport,
> + __be32 daddr, __be16 dport, int dif)
> {
> - int score = -1;
> + int score;
> + struct inet_sock *inet;
>
> - if (net_eq(sock_net(sk), net) && udp_sk(sk)->udp_port_hash == hnum &&
> - !ipv6_only_sock(sk)) {
> - struct inet_sock *inet = inet_sk(sk);
> + if (!(net_eq(sock_net(sk), net) &&
> + udp_sk(sk)->udp_port_hash == hnum &&
> + !ipv6_only_sock(sk)))
> + return -1
Or even better :
if (!net_eq(sock_net(sk), net) ||
udp_sk(sk)->udp_port_hash != hnum ||
ipv6_only_sock(sk))
return -1;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] udp: Neaten and reduce size of compute_score functions
2014-12-02 2:59 ` Eric Dumazet
@ 2014-12-02 3:09 ` Joe Perches
2014-12-02 4:29 ` [PATCH V2 " Joe Perches
2014-12-02 4:44 ` [PATCH " Eric Dumazet
0 siblings, 2 replies; 8+ messages in thread
From: Joe Perches @ 2014-12-02 3:09 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, LKML
On Mon, 2014-12-01 at 18:59 -0800, Eric Dumazet wrote:
> On Mon, 2014-12-01 at 17:39 -0800, Joe Perches wrote:
> > The compute_score functions are a bit difficult to read.
> >
> > Neaten them a bit to reduce object sizes and make them a
> > bit more intelligible.
> >
> > Return early to avoid indentation and avoid unnecessary
> > initializations.
[]
> > + if (!(net_eq(sock_net(sk), net) &&
> > + udp_sk(sk)->udp_port_hash == hnum &&
> > + !ipv6_only_sock(sk)))
> > + return -1
>
> Or even better :
>
>
> if (!net_eq(sock_net(sk), net) ||
> udp_sk(sk)->udp_port_hash != hnum ||
> ipv6_only_sock(sk))
> return -1;
Hi Eric.
Yeah, I thought about it but thought it
simpler to not change the logic.
Either way is fine with me.
David?
btw: the same thing can be done for the v6 block too:
+ if (!(net_eq(sock_net(sk), net) && !ipv6_only_sock(sk)))
+ return -1;
- if (inet->inet_rcv_saddr != daddr)
+ inet = inet_sk(sk);
+
+ if (inet->inet_rcv_saddr != daddr)
+ return -1;
+ if (inet->inet_num != hnum)
+ return -1;
to:
if (!net_eq(sock_net(sk, net) ||
ipv6_only_sock(sk))
return -1;
inet = inet_sk(sk);
if (inet->inet_rcv_saddr != daddr ||
inet->inet_num != hnum)
return -1;
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH V2 net-next] udp: Neaten and reduce size of compute_score functions
2014-12-02 3:09 ` Joe Perches
@ 2014-12-02 4:29 ` Joe Perches
2014-12-02 5:08 ` Eric Dumazet
2014-12-09 1:29 ` David Miller
2014-12-02 4:44 ` [PATCH " Eric Dumazet
1 sibling, 2 replies; 8+ messages in thread
From: Joe Perches @ 2014-12-02 4:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, LKML
The compute_score functions are a bit difficult to read.
Neaten them a bit to reduce object sizes and make them a
bit more intelligible.
Return early to avoid indentation and avoid unnecessary
initializations.
(allyesconfig, but w/ -O2 and no profiling)
$ size net/ipv[46]/udp.o.*
text data bss dec hex filename
28680 1184 25 29889 74c1 net/ipv4/udp.o.new
28756 1184 25 29965 750d net/ipv4/udp.o.old
17600 1010 2 18612 48b4 net/ipv6/udp.o.new
17632 1010 2 18644 48d4 net/ipv6/udp.o.old
Signed-off-by: Joe Perches <joe@perches.com>
---
Change the && == blocks to || !=
No change in compiled object files.
Keeps Eric happy too.
net/ipv4/udp.c | 111 +++++++++++++++++++++++++++++++-------------------------
net/ipv6/udp.c | 113 ++++++++++++++++++++++++++++++++-------------------------
2 files changed, 125 insertions(+), 99 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index b2d6068..dd8e006 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -336,38 +336,45 @@ int udp_v4_get_port(struct sock *sk, unsigned short snum)
return udp_lib_get_port(sk, snum, ipv4_rcv_saddr_equal, hash2_nulladdr);
}
-static inline int compute_score(struct sock *sk, struct net *net, __be32 saddr,
- unsigned short hnum,
- __be16 sport, __be32 daddr, __be16 dport, int dif)
+static inline int compute_score(struct sock *sk, struct net *net,
+ __be32 saddr, unsigned short hnum, __be16 sport,
+ __be32 daddr, __be16 dport, int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
- if (net_eq(sock_net(sk), net) && udp_sk(sk)->udp_port_hash == hnum &&
- !ipv6_only_sock(sk)) {
- struct inet_sock *inet = inet_sk(sk);
+ if (!net_eq(sock_net(sk), net) ||
+ udp_sk(sk)->udp_port_hash != hnum ||
+ ipv6_only_sock(sk))
+ return -1;
- score = (sk->sk_family == PF_INET ? 2 : 1);
- if (inet->inet_rcv_saddr) {
- if (inet->inet_rcv_saddr != daddr)
- return -1;
- score += 4;
- }
- if (inet->inet_daddr) {
- if (inet->inet_daddr != saddr)
- return -1;
- score += 4;
- }
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score += 4;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score += 4;
- }
+ score = (sk->sk_family == PF_INET) ? 2 : 1;
+ inet = inet_sk(sk);
+
+ if (inet->inet_rcv_saddr) {
+ if (inet->inet_rcv_saddr != daddr)
+ return -1;
+ score += 4;
+ }
+
+ if (inet->inet_daddr) {
+ if (inet->inet_daddr != saddr)
+ return -1;
+ score += 4;
}
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
+ return -1;
+ score += 4;
+ }
+
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score += 4;
+ }
+
return score;
}
@@ -378,33 +385,39 @@ static inline int compute_score2(struct sock *sk, struct net *net,
__be32 saddr, __be16 sport,
__be32 daddr, unsigned int hnum, int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
+
+ if (!net_eq(sock_net(sk), net) ||
+ ipv6_only_sock(sk))
+ return -1;
- if (net_eq(sock_net(sk), net) && !ipv6_only_sock(sk)) {
- struct inet_sock *inet = inet_sk(sk);
+ inet = inet_sk(sk);
- if (inet->inet_rcv_saddr != daddr)
+ if (inet->inet_rcv_saddr != daddr ||
+ inet->inet_num != hnum)
+ return -1;
+
+ score = (sk->sk_family == PF_INET) ? 2 : 1;
+
+ if (inet->inet_daddr) {
+ if (inet->inet_daddr != saddr)
return -1;
- if (inet->inet_num != hnum)
+ score += 4;
+ }
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
return -1;
+ score += 4;
+ }
- score = (sk->sk_family == PF_INET ? 2 : 1);
- if (inet->inet_daddr) {
- if (inet->inet_daddr != saddr)
- return -1;
- score += 4;
- }
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score += 4;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score += 4;
- }
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score += 4;
}
+
return score;
}
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 7cfb5d7..7f964322 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -148,72 +148,85 @@ static inline int compute_score(struct sock *sk, struct net *net,
const struct in6_addr *daddr, __be16 dport,
int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
- if (net_eq(sock_net(sk), net) && udp_sk(sk)->udp_port_hash == hnum &&
- sk->sk_family == PF_INET6) {
- struct inet_sock *inet = inet_sk(sk);
+ if (!net_eq(sock_net(sk), net) ||
+ udp_sk(sk)->udp_port_hash != hnum ||
+ sk->sk_family != PF_INET6)
+ return -1;
- score = 0;
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score++;
- }
- if (!ipv6_addr_any(&sk->sk_v6_rcv_saddr)) {
- if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
- return -1;
- score++;
- }
- if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
- if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
- return -1;
- score++;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score++;
- }
+ score = 0;
+ inet = inet_sk(sk);
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
+ return -1;
+ score++;
}
+
+ if (!ipv6_addr_any(&sk->sk_v6_rcv_saddr)) {
+ if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
+ return -1;
+ score++;
+ }
+
+ if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
+ if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
+ return -1;
+ score++;
+ }
+
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score++;
+ }
+
return score;
}
#define SCORE2_MAX (1 + 1 + 1)
static inline int compute_score2(struct sock *sk, struct net *net,
- const struct in6_addr *saddr, __be16 sport,
- const struct in6_addr *daddr, unsigned short hnum,
- int dif)
+ const struct in6_addr *saddr, __be16 sport,
+ const struct in6_addr *daddr,
+ unsigned short hnum, int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
- if (net_eq(sock_net(sk), net) && udp_sk(sk)->udp_port_hash == hnum &&
- sk->sk_family == PF_INET6) {
- struct inet_sock *inet = inet_sk(sk);
+ if (!net_eq(sock_net(sk), net) ||
+ udp_sk(sk)->udp_port_hash != hnum ||
+ sk->sk_family != PF_INET6)
+ return -1;
- if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
+ if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
+ return -1;
+
+ score = 0;
+ inet = inet_sk(sk);
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
return -1;
- score = 0;
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score++;
- }
- if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
- if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
- return -1;
- score++;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score++;
- }
+ score++;
}
+
+ if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
+ if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
+ return -1;
+ score++;
+ }
+
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score++;
+ }
+
return score;
}
-
/* called with read_rcu_lock() */
static struct sock *udp6_lib_lookup2(struct net *net,
const struct in6_addr *saddr, __be16 sport,
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] udp: Neaten and reduce size of compute_score functions
2014-12-02 3:09 ` Joe Perches
2014-12-02 4:29 ` [PATCH V2 " Joe Perches
@ 2014-12-02 4:44 ` Eric Dumazet
1 sibling, 0 replies; 8+ messages in thread
From: Eric Dumazet @ 2014-12-02 4:44 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, LKML
On Mon, 2014-12-01 at 19:09 -0800, Joe Perches wrote:
> On Mon, 2014-12-01 at 18:59 -0800, Eric Dumazet wrote:
> > On Mon, 2014-12-01 at 17:39 -0800, Joe Perches wrote:
> > > The compute_score functions are a bit difficult to read.
> > >
> > > Neaten them a bit to reduce object sizes and make them a
> > > bit more intelligible.
> > >
> > > Return early to avoid indentation and avoid unnecessary
> > > initializations.
> []
> > > + if (!(net_eq(sock_net(sk), net) &&
> > > + udp_sk(sk)->udp_port_hash == hnum &&
> > > + !ipv6_only_sock(sk)))
> > > + return -1
> >
> > Or even better :
> >
> >
> > if (!net_eq(sock_net(sk), net) ||
> > udp_sk(sk)->udp_port_hash != hnum ||
> > ipv6_only_sock(sk))
> > return -1;
>
> Hi Eric.
>
> Yeah, I thought about it but thought it
> simpler to not change the logic.
>
> Either way is fine with me.
Your patch does not change the logic at all.
My suggestion is about avoiding double negates which are really hard to
read. This is simple boolean logic rules.
Code was :
if (a && x == y && !ipv6_only_sock(sk))) {
EXPR1;
} else {
return -1;
}
So the 'logical' way of negating the condition is actually to not add
double negations and use :
if (!a || x != y || ipv6_only_sock(sk)))
return -1;
EXPR1;
Instead of the less readable form :
if (!(a && x == y && !ipv6_only_sock(sk))))
return -1;
EXPR1;
You do not have to ask David permission for this, really.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 net-next] udp: Neaten and reduce size of compute_score functions
2014-12-02 4:29 ` [PATCH V2 " Joe Perches
@ 2014-12-02 5:08 ` Eric Dumazet
2014-12-02 5:26 ` Joe Perches
2014-12-09 1:29 ` David Miller
1 sibling, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2014-12-02 5:08 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, LKML
On Mon, 2014-12-01 at 20:29 -0800, Joe Perches wrote:
> The compute_score functions are a bit difficult to read.
>
> Neaten them a bit to reduce object sizes and make them a
> bit more intelligible.
>
> Return early to avoid indentation and avoid unnecessary
> initializations.
>
> (allyesconfig, but w/ -O2 and no profiling)
hmm... Not sure how you get such large numbers...
>
> $ size net/ipv[46]/udp.o.*
> text data bss dec hex filename
> 28680 1184 25 29889 74c1 net/ipv4/udp.o.new
> 28756 1184 25 29965 750d net/ipv4/udp.o.old
> 17600 1010 2 18612 48b4 net/ipv6/udp.o.new
> 17632 1010 2 18644 48d4 net/ipv6/udp.o.old
Here I have :
# size net/ipv4/udp.o.*
text data bss dec hex filename
21989 616 9 22614 5856 net/ipv4/udp.o.old
21957 616 9 22582 5836 net/ipv4/udp.o.new
With CONFIG_CC_OPTIMIZE_FOR_SIZE=y I even have an opposite result (code
gets bigger after your patch)
# size net/ipv4/udp.o.*
text data bss dec hex filename
17242 600 9 17851 45bb net/ipv4/udp.o.old
17256 600 9 17865 45c9 net/ipv4/udp.o.new
Anyway, your patch looks fine to me, no matter what the code size is.
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 net-next] udp: Neaten and reduce size of compute_score functions
2014-12-02 5:08 ` Eric Dumazet
@ 2014-12-02 5:26 ` Joe Perches
0 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2014-12-02 5:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, LKML
On Mon, 2014-12-01 at 21:08 -0800, Eric Dumazet wrote:
> On Mon, 2014-12-01 at 20:29 -0800, Joe Perches wrote:
> > The compute_score functions are a bit difficult to read.
> >
> > Neaten them a bit to reduce object sizes and make them a
> > bit more intelligible.
> >
> > Return early to avoid indentation and avoid unnecessary
> > initializations.
> >
> > (allyesconfig, but w/ -O2 and no profiling)
>
> hmm... Not sure how you get such large numbers...
Nor I particularly, but I do with gcc 4.9.1
> > $ size net/ipv[46]/udp.o.*
> > text data bss dec hex filename
> > 28680 1184 25 29889 74c1 net/ipv4/udp.o.new
> > 28756 1184 25 29965 750d net/ipv4/udp.o.old
[]
> Here I have :
>
> # size net/ipv4/udp.o.*
> text data bss dec hex filename
> 21989 616 9 22614 5856 net/ipv4/udp.o.old
> 21957 616 9 22582 5836 net/ipv4/udp.o.new
Curious. What gcc version?
with that 4.9.1 gcc version and a defconfig (x86-64) I get:
$ size net/ipv[46]/udp.o*
text data bss dec hex filename
21328 672 9 22009 55f9 net/ipv4/udp.o.new
21312 672 9 21993 55e9 net/ipv4/udp.o.old
14463 588 2 15053 3acd net/ipv6/udp.o.new
14527 588 2 15117 3b0d net/ipv6/udp.o.old
and defconfig x86-32:
$ size net/ipv[46]/udp.o*
text data bss dec hex filename
19626 324 5 19955 4df3 net/ipv4/udp.o.new
19706 324 5 20035 4e43 net/ipv4/udp.o.old
14189 300 2 14491 389b net/ipv6/udp.o.new
14125 300 2 14427 385b net/ipv6/udp.o.old
> With CONFIG_CC_OPTIMIZE_FOR_SIZE=y I even have an opposite result (code
> gets bigger after your patch)
>
> # size net/ipv4/udp.o.*
> text data bss dec hex filename
> 17242 600 9 17851 45bb net/ipv4/udp.o.old
> 17256 600 9 17865 45c9 net/ipv4/udp.o.new
>
> Anyway, your patch looks fine to me, no matter what the code size is.
>
> Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 net-next] udp: Neaten and reduce size of compute_score functions
2014-12-02 4:29 ` [PATCH V2 " Joe Perches
2014-12-02 5:08 ` Eric Dumazet
@ 2014-12-09 1:29 ` David Miller
1 sibling, 0 replies; 8+ messages in thread
From: David Miller @ 2014-12-09 1:29 UTC (permalink / raw)
To: joe; +Cc: eric.dumazet, netdev, linux-kernel
From: Joe Perches <joe@perches.com>
Date: Mon, 01 Dec 2014 20:29:06 -0800
> The compute_score functions are a bit difficult to read.
>
> Neaten them a bit to reduce object sizes and make them a
> bit more intelligible.
>
> Return early to avoid indentation and avoid unnecessary
> initializations.
>
> (allyesconfig, but w/ -O2 and no profiling)
>
> $ size net/ipv[46]/udp.o.*
> text data bss dec hex filename
> 28680 1184 25 29889 74c1 net/ipv4/udp.o.new
> 28756 1184 25 29965 750d net/ipv4/udp.o.old
> 17600 1010 2 18612 48b4 net/ipv6/udp.o.new
> 17632 1010 2 18644 48d4 net/ipv6/udp.o.old
>
> Signed-off-by: Joe Perches <joe@perches.com>
Applied, thanks Joe.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-12-09 1:29 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-02 1:39 [PATCH net-next] udp: Neaten and reduce size of compute_score functions Joe Perches
2014-12-02 2:59 ` Eric Dumazet
2014-12-02 3:09 ` Joe Perches
2014-12-02 4:29 ` [PATCH V2 " Joe Perches
2014-12-02 5:08 ` Eric Dumazet
2014-12-02 5:26 ` Joe Perches
2014-12-09 1:29 ` David Miller
2014-12-02 4:44 ` [PATCH " Eric Dumazet
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).