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 lvs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
