Re: ipvs netns exit causes crash in conntrack.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



	Hello,

On Fri, 10 Jun 2011, Hans Schillstrom wrote:

> > 	In IPVS we can add checks to exit ip_vs_conn_drop_conntrack()
> > early if net cleanup is in progress because conntrack can be
> > loaded after IPVS and its cleanup called before IPVS.
> > We do not need to delete conntrack while in IPVS cleanup.
> 
> This is a quite simple patch ... 
> The enable flag is set to 0 already when an exit begins 

	Correct, this should work because we clear the
flag during device cleanup.

> So it's just to check it
> maybe it should be atomic, but for my quick test "volatile" will do.

	Yes, may be this check will need smp_rmb, not
atomic operations, sort of:

	if (cp->flags & IP_VS_CONN_F_NFCT) {
		smp_rmb():
		if (ipvs->enable)
			ip_vs_conn_drop_conntrack(cp);
	}

	And corresponding smp_wmb:

static void __net_exit __ip_vs_dev_cleanup(struct net *net)
{
	EnterFunction(2);
	net_ipvs(net)->enable = 0;	/* Disable packet reception */
--->>>	smp_wmb();
	__ip_vs_sync_cleanup(net);
	LeaveFunction(2);
}


	All other places do not need smp_rmb, it is not fatal
there.

	Also, we can put some comments that we should not access
conntracks during subsys cleanup because nf_conntrack_find_get
can not be used after conntrack cleanup for the net.

	One option to avoid such checks is the netfilter
framework to provide priority based list of netns init/exit
handlers, so that all modules are ordered by known
priority based on their dependency. For example,
nf_register_netns_subsys_hooks function can be created
in a similar way like nf_register_hooks. By this way,
IPVS should be one of the first to be called for cleanup
and such checks for enable flag will not be needed.

> diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> index bf28ac2..ae5e9e2 100644
> --- a/net/netfilter/ipvs/ip_vs_conn.c
> +++ b/net/netfilter/ipvs/ip_vs_conn.c
> @@ -775,8 +775,8 @@ static void ip_vs_conn_expire(unsigned long data)
>                 /* does anybody control me? */
>                 if (cp->control)
>                         ip_vs_control_del(cp);
> -
> -               if (cp->flags & IP_VS_CONN_F_NFCT)
> +               /* Do not try do drop ct when netns is dying */
> +               if (cp->flags & IP_VS_CONN_F_NFCT && ipvs->enable)
>                         ip_vs_conn_drop_conntrack(cp);
> 
>                 ip_vs_pe_put(cp->pe);

Regards

--
Julian Anastasov <ja@xxxxxx>
--
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


[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux