Re: [PATCHv2] deliver events for conntracks created via ctnetlink

[Patrick McHardy <kaber@trash.net>]

Pablo Neira Ayuso wrote:
> As for now, the creation and update of conntracks via ctnetlink do not
> propagate an event to userspace. This can result in inconsistent
> situations if several userspace processes modify the connection tracking
> table by means of ctnetlink at the same time. Specifically, using the
> conntrack command line tool and conntrackd at the same time can trigger
> unconsistencies.
> 
> This patch fixes this inconsistent situation. Note that the deletion
> does not suffer from this problem.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> 

Unfortunately all the change functions are deadlock prone,
they are called while holding the conntrack lock and
event delivery might trigger destruction of the conntrack
entry already in the cache, which takes the lock again.

Perhaps we can do all this much easier. Conntrack updates
over netlink are a lot more rare than events triggered
by packet processing. What do you think about just sending
the full entry on successful changes over ctnetlink?

A few minor nits:

> +			atomic_inc(&ct->ct_general.use);

Should be using nf_conntrack_get().

Also the patch adds newlines excessively, to a file already
containing about 20% empty lines. Things like

>  		ct->status |= IPS_SEQ_ADJUST;
> +
> +		nf_conntrack_event_cache_ct(IPCT_NATSEQADJ, ct);

or

>  	rcu_read_unlock();
>  
> +	nf_ct_deliver_cached_events(ct);
> +
> +	nf_ct_put(ct);
> +
>  	return 0;

don't aid readability.