[PATCH net-next?] pktgen: Use simpler test for non-zero ipv6 address

[PATCH net-next?] pktgen: Use simpler test for non-zero ipv6 address

[Joe Perches]

Reduces object size and should be slightly faster.

allyesconfig:

$ size net/core/pktgen.o*
   text	   data	    bss	    dec	    hex	filename
  52251	   4293	  11824	  68368	  10b10	net/core/pktgen.o.new
  52310	   4293	  11848	  68451	  10b63	net/core/pktgen.o.old

Signed-off-by: Joe Perches <joe@perches.com>
---
Found by looking for if (foo) ; tests with a perl regex

Yes Eric, it could be 2 compares instead of 4 on 64-bit
systems with HAS_EFFICIENT_UNALIGNED_ACCESS.  Maybe later
or if there are other tests that could become something
like ipv6_is_zeronet.

cheers, Joe

 net/core/pktgen.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 148e73d..3aa8417 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2422,11 +2422,10 @@ static void mod_cur_headers(struct pktgen_dev *pkt_dev)
 		}
 	} else {		/* IPV6 * */
 
-		if (pkt_dev->min_in6_daddr.s6_addr32[0] == 0 &&
-		    pkt_dev->min_in6_daddr.s6_addr32[1] == 0 &&
-		    pkt_dev->min_in6_daddr.s6_addr32[2] == 0 &&
-		    pkt_dev->min_in6_daddr.s6_addr32[3] == 0) ;
-		else {
+		if (pkt_dev->min_in6_daddr.s6_addr32[0] |
+		    pkt_dev->min_in6_daddr.s6_addr32[1] |
+		    pkt_dev->min_in6_daddr.s6_addr32[2] |
+		    pkt_dev->min_in6_daddr.s6_addr32[3]) {
 			int i;
 
 			/* Only random destinations yet */

Re: [PATCH net-next?] pktgen: Use simpler test for non-zero ipv6 address

[Brian Haley]

On 10/10/2012 02:42 PM, Joe Perches wrote:
> Found by looking for if (foo) ; tests with a perl regex
> 
> Yes Eric, it could be 2 compares instead of 4 on 64-bit
> systems with HAS_EFFICIENT_UNALIGNED_ACCESS.  Maybe later
> or if there are other tests that could become something
> like ipv6_is_zeronet.
> 
> cheers, Joe
> 
>  net/core/pktgen.c |    9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index 148e73d..3aa8417 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -2422,11 +2422,10 @@ static void mod_cur_headers(struct pktgen_dev *pkt_dev)
>  		}
>  	} else {		/* IPV6 * */
>  
> -		if (pkt_dev->min_in6_daddr.s6_addr32[0] == 0 &&
> -		    pkt_dev->min_in6_daddr.s6_addr32[1] == 0 &&
> -		    pkt_dev->min_in6_daddr.s6_addr32[2] == 0 &&
> -		    pkt_dev->min_in6_daddr.s6_addr32[3] == 0) ;
> -		else {
> +		if (pkt_dev->min_in6_daddr.s6_addr32[0] |
> +		    pkt_dev->min_in6_daddr.s6_addr32[1] |
> +		    pkt_dev->min_in6_daddr.s6_addr32[2] |
> +		    pkt_dev->min_in6_daddr.s6_addr32[3]) {
>  			int i;

Why not just use ipv6_addr_any() ?  It has an HAS_EFFICIENT_UNALIGNED_ACCESS
check too.

-Brian

Re: [PATCH net-next?] pktgen: Use simpler test for non-zero ipv6 address

[Eric Dumazet]

On Wed, 2012-10-10 at 11:42 -0700, Joe Perches wrote:
> Reduces object size and should be slightly faster.
> 
> allyesconfig:
> 
> $ size net/core/pktgen.o*
>    text	   data	    bss	    dec	    hex	filename
>   52251	   4293	  11824	  68368	  10b10	net/core/pktgen.o.new
>   52310	   4293	  11848	  68451	  10b63	net/core/pktgen.o.old
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> Found by looking for if (foo) ; tests with a perl regex
> 
> Yes Eric, it could be 2 compares instead of 4 on 64-bit
> systems with HAS_EFFICIENT_UNALIGNED_ACCESS.  Maybe later
> or if there are other tests that could become something
> like ipv6_is_zeronet.
> 
> cheers, Joe
> 
>  net/core/pktgen.c |    9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index 148e73d..3aa8417 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -2422,11 +2422,10 @@ static void mod_cur_headers(struct pktgen_dev *pkt_dev)
>  		}
>  	} else {		/* IPV6 * */
>  
> -		if (pkt_dev->min_in6_daddr.s6_addr32[0] == 0 &&
> -		    pkt_dev->min_in6_daddr.s6_addr32[1] == 0 &&
> -		    pkt_dev->min_in6_daddr.s6_addr32[2] == 0 &&
> -		    pkt_dev->min_in6_daddr.s6_addr32[3] == 0) ;
> -		else {
> +		if (pkt_dev->min_in6_daddr.s6_addr32[0] |
> +		    pkt_dev->min_in6_daddr.s6_addr32[1] |
> +		    pkt_dev->min_in6_daddr.s6_addr32[2] |
> +		    pkt_dev->min_in6_daddr.s6_addr32[3]) {
>  			int i;
>  
>  			/* Only random destinations yet */
> 



What about ipv6_addr_any() ?

anyway net-next is not opened yet

[PATCH net-next? V2] pktgen: Use simpler test for non-zero ipv6 address

[Joe Perches]

Reduces object size and should be slightly faster.

allyesconfig: 
$ size net/core/pktgen.o*
   text	   data	    bss	    dec	    hex	filename
  52284	   4321	  11840	  68445	  10b5d	net/core/pktgen.o.new
  52310	   4293	  11848	  68451	  10b63	net/core/pktgen.o.old
 
Signed-off-by: Joe Perches <joe@perches.com>
---
> What about ipv6_addr_any() ?

That's better I guess.
I forgot about it and didn't see it.
I saw the IPV6_ADDR_ANY type tests and
didn't look further.

Anyway, it's odd that it generates slightly larger code
than the original patch's direct tests in 32bit with
gcc 4.7.2.  Perhaps an interesting lack of optimization?

cheers, Joe

 net/core/pktgen.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 148e73d..a811a7d 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2422,11 +2422,7 @@ static void mod_cur_headers(struct pktgen_dev *pkt_dev)
 		}
 	} else {		/* IPV6 * */
 
-		if (pkt_dev->min_in6_daddr.s6_addr32[0] == 0 &&
-		    pkt_dev->min_in6_daddr.s6_addr32[1] == 0 &&
-		    pkt_dev->min_in6_daddr.s6_addr32[2] == 0 &&
-		    pkt_dev->min_in6_daddr.s6_addr32[3] == 0) ;
-		else {
+		if (!ipv6_addr_any(&pkt_dev->min_in6_daddr)) {
 			int i;
 
 			/* Only random destinations yet */

[RFC net-next] treewide: s/ipv4_is_<foo>()/ipv4_addr_<foo>/

[Joe Perches]

ipv4 and ipv6 use different styles for these tests.

ipv4_is_<foo>(__be32)
ipv6_addr_<foo>(struct in6_addr *)

Perhaps it'd be good to convert the ipv4 tests to the ipv6 style.

There are ~100 instances of ipv4_is_<foo> tests treewide.

[RFC net-next] treewide: s/is_<foo>_ether_addr/eth_addr_<foo>

[Joe Perches]

Maybe all the is_<foo>_ether_addr  functions should be renamed
to eth_addr_<foo> for more api/style symmetry.

$ git grep --name-only -E "\bis_\w+_ether_addr" | \
  xargs sed -r -i -e 's/\bis_(\w+)_ether_addr\b/eth_addr_\1/g'
$ git diff --shortstat
 304 files changed, 690 insertions(+), 690 deletions(-)

Maybe add #defines of the old names for a few releases.

Re: [PATCH net-next? V2] pktgen: Use simpler test for non-zero ipv6 address

[Cong Wang]

On Thu, Oct 11, 2012 at 3:23 AM, Joe Perches <joe@perches.com> wrote:
> Reduces object size and should be slightly faster.
>
> allyesconfig:
> $ size net/core/pktgen.o*
>    text    data     bss     dec     hex filename
>   52284    4321   11840   68445   10b5d net/core/pktgen.o.new
>   52310    4293   11848   68451   10b63 net/core/pktgen.o.old
>
> Signed-off-by: Joe Perches <joe@perches.com>

Looks good.

This should go to -net, net-next is not open currently.

Thanks.

RE: [PATCH net-next?] pktgen: Use simpler test for non-zero ipv6 address

[David Laight]

> -		if (pkt_dev->min_in6_daddr.s6_addr32[0] == 0 &&
> -		    pkt_dev->min_in6_daddr.s6_addr32[1] == 0 &&
> -		    pkt_dev->min_in6_daddr.s6_addr32[2] == 0 &&
> -		    pkt_dev->min_in6_daddr.s6_addr32[3] == 0) ;
> -		else {
> +		if (pkt_dev->min_in6_daddr.s6_addr32[0] |
> +		    pkt_dev->min_in6_daddr.s6_addr32[1] |
> +		    pkt_dev->min_in6_daddr.s6_addr32[2] |
> +		    pkt_dev->min_in6_daddr.s6_addr32[3]) {

Given the likely values, it might be worth using:
		if (pkt_dev->min_in6_daddr.s6_addr32[0] == 0 &&
		    pkt_dev->min_in6_daddr.s6_addr32[1] |
		    pkt_dev->min_in6_daddr.s6_addr32[2] |
		    pkt_dev->min_in6_daddr.s6_addr32[3]) {

	David

RE: [RFC net-next] treewide: s/ipv4_is_<foo>()/ipv4_addr_<foo>/

[David Laight]

> ipv4 and ipv6 use different styles for these tests.
> 
> ipv4_is_<foo>(__be32)
> ipv6_addr_<foo>(struct in6_addr *)

I presume there is a 'const' in there ...

> Perhaps it'd be good to convert the ipv4 tests to the ipv6 style.

You don't want to force an IPv4 address (which might be in a register)
be written out to stack.
Taking the address also has implications for the optimiser.

	David

Re: [RFC net-next] treewide: s/ipv4_is_<foo>()/ipv4_addr_<foo>/

[Joe Perches]

On Thu, 2012-10-11 at 09:11 +0100, David Laight wrote:
> > ipv4 and ipv6 use different styles for these tests.
> > 
> > ipv4_is_<foo>(__be32)
> > ipv6_addr_<foo>(struct in6_addr *)
> 
> I presume there is a 'const' in there ...
> 
> > Perhaps it'd be good to convert the ipv4 tests to the ipv6 style.
> 
> You don't want to force an IPv4 address (which might be in a register)
> be written out to stack.
> Taking the address also has implications for the optimiser.

Of course not, I'm just talking about renaming.

Re: [PATCH net-next? V2] pktgen: Use simpler test for non-zero ipv6 address

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Wed, 10 Oct 2012 12:23:25 -0700

> Reduces object size and should be slightly faster.
> 
> allyesconfig: 
> $ size net/core/pktgen.o*
>    text	   data	    bss	    dec	    hex	filename
>   52284	   4321	  11840	  68445	  10b5d	net/core/pktgen.o.new
>   52310	   4293	  11848	  68451	  10b63	net/core/pktgen.o.old
>  
> Signed-off-by: Joe Perches <joe@perches.com>

Please resubmit when net-next opens.