netdev - Re: [PATCH nf-next 0/4] netfilter: bridge: remove broute hook

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: <c3d1e77b-b3e2-cc27-d569-ddcea9460273@cumulusnetworks.com>
Date:   Thu, 11 Apr 2019 22:46:16 +0300
From:   Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To:     Florian Westphal <fw@...len.de>, netfilter-devel@...r.kernel.org
Cc:     roopa@...ulusnetworks.com, netdev@...r.kernel.org
Subject: Re: [PATCH nf-next 0/4] netfilter: bridge: remove broute hook

On 11/04/2019 17:36, Florian Westphal wrote:
> This series removes the 'broute' hook by promoting ebtables' broute table
> to a normal ebtables table (invoked via normal PREROUTING netfilter hook).
> 
> The downside is that nf_hook_slow() needs to be duplicated in br_input.c
> (see patch 3).
> 
> However, I think its worth the price as this allows to remove the
> br_should_route_hook.
> 
> There are quite some changes in bridge specific code, if you prefer
> I can re-submit this for net-next instead of nf-next.
> 
> Main motivation is to provide 'ebtables -t broute' functionality via
> nftables later on, this can then be done without touching the bridge
> or netfilter core infrastructure again.
> 
> Florian Westphal (4):
>       selftests: netfilter: add ebtables broute test case
>       bridge: reduce size of input cb to 16 bytes
>       bridge: netfilter: unroll NF_HOOK helper in bridge input path
>       bridge: broute: make broute a real ebtables table
> 
>  include/linux/if_bridge.h                           |    3 
>  include/net/netfilter/nf_queue.h                    |    3 
>  net/bridge/br_arp_nd_proxy.c                        |   18 +-
>  net/bridge/br_input.c                               |   72 +++++++--
>  net/bridge/br_private.h                             |   15 +-
>  net/bridge/netfilter/ebtable_broute.c               |   63 ++++++--
>  net/bridge/netfilter/ebtables.c                     |    7 
>  net/netfilter/core.c                                |    1 
>  net/netfilter/nf_internals.h                        |    3 
>  net/netfilter/nf_queue.c                            |    1 
>  tools/testing/selftests/netfilter/Makefile          |    2 
>  tools/testing/selftests/netfilter/bridge_brouter.sh |  146 ++++++++++++++++++++
>  12 files changed, 268 insertions(+), 66 deletions(-)
> 

The set looks good to me, the only little thing is the new memset() in br_handle_frame(),
before we would lazily zero the fields when needed but that would save us from future
bugs where one could forget to initialize the field.
Now we can remove most of the explicit cb field zeroing and rely on the memset.

Nice work! For the set:

Acked-by: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>