Re: [PATCH] bridge: Superfluous skb->nfct check in br_nf_dev_queue_xmit

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Sun, May 04, 2014 at 11:04:29PM +0400, Vasily Averin wrote:
> On 05/04/2014 04:54 PM, Pablo Neira Ayuso wrote:
> > On Wed, Apr 30, 2014 at 12:54:50PM +0400, Vasily Averin wrote:
> >> Currently bridge can silently drop ipv4 fragments.
> >> If node have loaded nf_defrag_ipv4 module but have no nf_conntrack_ipv4,
> >> br_nf_pre_routing defragments incoming ipv4 fragments
> >> but nfct check in br_nf_dev_queue_xmit does not allow re-fragment combined packet back,
> >> and therefore it is dropped in br_dev_queue_push_xmit without incrementing of any failcounters
> > 
> > If no further objections, I'll push this original patch appending this comment
> > to the description:
> > 
> > [ It seems the only way to hit the ip_fragment code in the bridge xmit
> >   path is to have a fragment list whose reassembled fragments go over
> >   the mtu. This only happens if nf_defrag is enabled. Thanks to
> >   Florian Westphal for providing feedback to clarify this. ]
> 
> I have not objections, however I still do not understand why #if
> IS_ENABLED(CONFIG_NF_CONNTRACK_IPV4) is required in
> br_dev_queue_push_xmit()?
> 
> If ipv4 defragmentation is required not only for conntracks but for
> TPROXY target and xt_socket match I think we need to use
> NF_DEFRAG_IPV4 instead.

Before your patch, this was checking for skb->nfct which is defined by

#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)

in include/linux/skbuff.h.

But after removing that skb->nfct check, we can safely change it to
CONFIG_NF_DEFRAG_IPV4.

You can send me a new patch version including this change.