Date:   Tue, 30 Apr 2019 01:08:43 +0300
From:   Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To:     Pablo Neira Ayuso <pablo@...filter.org>,
        netfilter-devel@...r.kernel.org
Cc:     davem@...emloft.net, netdev@...r.kernel.org,
        roopa@...ulusnetworks.com
Subject: Re: [PATCH 7/9 net-next,v2] netfilter: bridge: add connection
 tracking system

On 29/04/2019 22:50, Pablo Neira Ayuso wrote:
> This patch adds basic connection tracking support for the bridge,
> including initial IPv4 support.
> 
> This patch register two hooks to deal with the bridge forwarding path,
> one at bridge prerouting hook to call nf_conntrack_in() and another at
> the bridge postrouting hook to confirm the entry.
> 
> The conntrack bridge prerouting hook defragments packets and it passes
> them to nf_conntrack_in() to look up for an existing entry, otherwise a
> new entry is created in the conntrack table.
> 
> The conntrack bridge postrouting hook confirms new entries, ie. if this
> is the first packet seen of this flow that went through the entire
> bridge layer, then it adds the entries to the hashtable and (if needed)
> it refragments the skbuff into the original fragments, leaving the
> geometry as is if possible. Exceptions are linearized skbuffs, eg.
> skbuffs that are passed up to nfqueue and conntrack helpers, as well
> as cloned skbuff for the local delivery (eg. tcpdump) or in case of
> bridge port flooding.
> 
> The packet defragmentation is done through the ip_defrag() call.  This
> forces us to save the bridge control buffer, reset the IP control buffer
> area and then restore it after call. This function also bumps the IP
> fragmentation statistics. The maximum fragment length is stored in the
> control buffer and it is used to refragment the skbuff.
> 
> The new fraglist iterator and fragment transformer APIs is used to
> implement the bridge refragmentation code. The br_ip_fragment() function
> drops the packet in case the maximum fragment size seen is larger than
> the output port MTU.
> 
> This patchset follows the principle that conntrack should not drop
> packets, so users can do it through policy via invalid state.
> 
> Like br_netfilter, there is no refragmentation for packets that are
> passed up for local delivery, ie. prerouting -> input path. There are
> calls to nf_reset() already in several spots since time ago already, eg.
> af_packet, that show that skbuff fraglist from the netif_rx path is
> supported already.
> 
> The helpers are called from the postrouting hook, before confirmation,
> from there we may see packet floods to bridge ports. Then, although
> unlikely, this may result in exercising the helpers many times for each
> clone. It would be good to explore how to pass all the packets in a list
> to the conntrack hook to do this handle only once for this case.
> 
> This patch is based on original work from Florian Westphal.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@...filter.org>
> ---
> v2: no changes.
> 
>  include/net/netfilter/nf_conntrack_bridge.h |   7 +
>  include/net/netfilter/nf_conntrack_core.h   |   3 +
>  net/bridge/br_device.c                      |   1 +
>  net/bridge/br_private.h                     |   1 +
>  net/bridge/netfilter/Kconfig                |  14 ++
>  net/bridge/netfilter/Makefile               |   3 +
>  net/bridge/netfilter/nf_conntrack_bridge.c  | 378 ++++++++++++++++++++++++++++
>  net/netfilter/nf_conntrack_proto.c          |   7 +-
>  8 files changed, 410 insertions(+), 4 deletions(-)
>  create mode 100644 net/bridge/netfilter/nf_conntrack_bridge.c
> 
[snip]
> diff --git a/net/bridge/netfilter/Kconfig b/net/bridge/netfilter/Kconfig
> index 9a0159aebe1a..eb61197d8af8 100644
> --- a/net/bridge/netfilter/Kconfig
> +++ b/net/bridge/netfilter/Kconfig
> @@ -18,6 +18,20 @@ config NF_LOG_BRIDGE
>  	tristate "Bridge packet logging"
>  	select NF_LOG_COMMON
>  
> +config NF_CONNTRACK_BRIDGE
> +	tristate "IPv4/IPV6 bridge connection tracking support"
> +	depends on NF_CONNTRACK
> +	default n
> +	help
> +	  Connection tracking keeps a record of what packets have passed
> +	  through your machine, in order to figure out how they are related
> +	  into connections. This is used to enhance packet filtering via
> +	  stateful policies. Enable this if you want native tracking from
> +	  the bridge. This is provides a replacement for the `br_netfilter'
                      ^^^^^^^^^^^^^^^^
Same comment like the previous time, "This is provides". Should be "This provides" ?
http://patchwork.ozlabs.org/patch/1085949/

> +	  infrastructure.

Cheers,
 Nik

Powered by blists - more mailing lists