netdev - Re: [PATCH net-next 0/9] net: Refactor ip_route_input

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56023DDF.8000707@gmail.com>
Date:	Tue, 22 Sep 2015 22:51:27 -0700
From:	Alexander Duyck <alexander.duyck@...il.com>
To:	David Ahern <dsa@...ulusnetworks.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 0/9] net: Refactor ip_route_input_slow

On 09/22/2015 03:55 PM, David Ahern wrote:
> ip_route_input_slow is a maze of gotos (9 of them!) making it error
> prone and difficult to read. This patchset refactors it, removing all
> but 2 of the labels. The brd_input label for broadcast path requires
> too many inputs to make a reasonble helper out of it so I left it as is.
>
> None of these patches change functionality, only move code around
> to make ip_route_input_slow more readable.
>
> Size comparison using gcc version 4.7.2 (Debian 4.7.2-5)
>
> With CONFIG_IP_ROUTE_VERBOSE:
>      Before patches:
>      $ size kbuild2/net/ipv4/route.o
>         text	   data	    bss	    dec	    hex	filename
>        20615	   2321	     32	  22968	   59b8	kbuild2/net/ipv4/route.o
>
>      After patches:
>      $ size kbuild/net/ipv4/route.o
>         text	   data	    bss	    dec	    hex	filename
>        20774	   2321	     32	  23127	   5a57	kbuild/net/ipv4/route.o
>
> An increase of 159 bytes with CONFIG_IP_ROUTE_VERBOSE.
>
> Without CONFIG_IP_ROUTE_VERBOSE:
>      Before patches:
>      $ size kbuild2/net/ipv4/route.o
>         text	   data	    bss	    dec	    hex	filename
>        19778	   2321	     32	  22131	   5673	kbuild2/net/ipv4/route.o
>
>      After patches:
>      $ size kbuild/net/ipv4/route.o
>         text	   data	    bss	    dec	    hex	filename
>        19858	   2321	     32	  22211	   56c3	kbuild/net/ipv4/route.o
>
> An increase of 80 bytes without CONFIG_IP_ROUTE_VERBOSE.
>
> David Ahern (9):
>    net: Remove martian_source_keep_err goto label
>    net: Remove e_inval label from ip_route_input_slow
>    net: Remove e_nobufs label from ip_route_input_slow
>    net: Move rth handling from ip_route_input_slow to helper
>    net: Move martian_destination to helper
>    net: Remove martian_source goto
>    net: Remove martian_destination label
>    net: Remove local_input label
>    net: Remove no_route label
>
>   net/ipv4/route.c | 239 ++++++++++++++++++++++++++++++++-----------------------
>   1 file changed, 139 insertions(+), 100 deletions(-)
>

One option you might consider when untangling this is to just return the 
values instead of leaving any labels.  I just did a quick test on my 
system with gcc version 5.1.1 and going through and just replacing all 
of the labels with returns actually resulted in smaller code since the 
compiler was smart enough to just combine the returns anyway.

You may also want to increase the scope of this patch set to include 
__mkroute_input as it ends up being compiled into this function as 
well.  From what I have seen there is a bit of redundancy with some of 
the code from local_input.

- Alex
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html