Phil Oester <kernel@xxxxxxxxxxxx> wrote:
> On Thu, Jun 20, 2013 at 11:58:46AM +0200, Pablo Neira Ayuso wrote:
> > On Thu, Jun 20, 2013 at 11:22:59AM +0200, Florian Westphal wrote:
> > > Phil Oester <kernel@xxxxxxxxxxxx> wrote:
> > > > kfree_skb already checks for skb existence - remove redundant check from
> > > > nf_conntrack_put_reasm.
> > >
> > > This is called for every free'd skb, in almost all cases
> > > the condition will be false.
> > >
> > > So I think its ok to keep this check.
> >
> > I see, we'll hit branch mispredictions most of the time. I have kept
> > this back. Thanks Florian.
>
> If we're concerned about branch mispredictions, we should change the check to
> match the one at the top of kfree_skb:
> if (unlikely(!skb))
> return;
Its the opposite. For kfree_skb skb is (virtually) never NULL.
> However nf_conntrack_put_reasm is only used in 3 places in the entire tree.
> Unclear why we don't just call kfree_skb directly in these 3 places.
I guess what you could do is s/nf_conntrack_put_reasm/kfree_skb/g and
also change skb_release_head_state() to use
#ifdef NET_SKBUFF_NF_DEFRAG_NEEDED
if (skb->nfct) /* usually NULL */
kfree_skb(skb->nfct);
#endif
[ or show that there is no performance degradation for workloads where
we end up calling kfree_skb(NULL) in hot path ]
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
