Skip to content

Commit

Permalink
xfrm: dst_entries_init() per-net dst_ops
Browse files Browse the repository at this point in the history 
Remove the dst_entries_init/destroy calls for xfrm4 and xfrm6 dst_ops
templates; their dst_entries counters will never be used.  Move the
xfrm dst_ops initialization from the common xfrm/xfrm_policy.c to
xfrm4/xfrm4_policy.c and xfrm6/xfrm6_policy.c, and call dst_entries_init
and dst_entries_destroy for each net namespace.

The ipv4 and ipv6 xfrms each create dst_ops template, and perform
dst_entries_init on the templates.  The template values are copied to each
net namespace's xfrm.xfrm*_dst_ops.  The problem there is the dst_ops
pcpuc_entries field is a percpu counter and cannot be used correctly by
simply copying it to another object.

The result of this is a very subtle bug; changes to the dst entries
counter from one net namespace may sometimes get applied to a different
net namespace dst entries counter.  This is because of how the percpu
counter works; it has a main count field as well as a pointer to the
percpu variables.  Each net namespace maintains its own main count
variable, but all point to one set of percpu variables.  When any net
namespace happens to change one of the percpu variables to outside its
small batch range, its count is moved to the net namespace's main count
variable.  So with multiple net namespaces operating concurrently, the
dst_ops entries counter can stray from the actual value that it should
be; if counts are consistently moved from one net namespace to another
(which my testing showed is likely), then one net namespace winds up
with a negative dst_ops count while another winds up with a continually
increasing count, eventually reaching its gc_thresh limit, which causes
all new traffic on the net namespace to fail with -ENOBUFS.

Signed-off-by: Dan Streetman <[email protected]>
Signed-off-by: Dan Streetman <[email protected]>
Signed-off-by: Steffen Klassert <[email protected]>
  • Loading branch information
@klassert
Dan Streetman authored and klassert committed Nov 3, 2015
1 parent 5f6c99e commit a8a572a
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 62 deletions.
46 changes: 37 additions & 9 deletions net/ipv4/xfrm4_policy.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ static void xfrm4_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
xfrm_dst_ifdown(dst, dev);
}

static struct dst_ops xfrm4_dst_ops = {
static struct dst_ops xfrm4_dst_ops_template = {
.family = AF_INET,
.gc = xfrm4_garbage_collect,
.update_pmtu = xfrm4_update_pmtu,
Expand All @@ -250,7 +250,7 @@ static struct dst_ops xfrm4_dst_ops = {

static struct xfrm_policy_afinfo xfrm4_policy_afinfo = {
.family = AF_INET,
.dst_ops = &xfrm4_dst_ops,
.dst_ops = &xfrm4_dst_ops_template,
.dst_lookup = xfrm4_dst_lookup,
.get_saddr = xfrm4_get_saddr,
.decode_session = _decode_session4,
Expand All @@ -272,7 +272,7 @@ static struct ctl_table xfrm4_policy_table[] = {
{ }
};

static int __net_init xfrm4_net_init(struct net *net)
static int __net_init xfrm4_net_sysctl_init(struct net *net)
{
struct ctl_table *table;
struct ctl_table_header *hdr;
Expand Down Expand Up @@ -300,7 +300,7 @@ static int __net_init xfrm4_net_init(struct net *net)
return -ENOMEM;
}

static void __net_exit xfrm4_net_exit(struct net *net)
static void __net_exit xfrm4_net_sysctl_exit(struct net *net)
{
struct ctl_table *table;

Expand All @@ -312,12 +312,44 @@ static void __net_exit xfrm4_net_exit(struct net *net)
if (!net_eq(net, &init_net))
kfree(table);
}
#else /* CONFIG_SYSCTL */
static int inline xfrm4_net_sysctl_init(struct net *net)
{
return 0;
}

static void inline xfrm4_net_sysctl_exit(struct net *net)
{
}
#endif

static int __net_init xfrm4_net_init(struct net *net)
{
int ret;

memcpy(&net->xfrm.xfrm4_dst_ops, &xfrm4_dst_ops_template,
sizeof(xfrm4_dst_ops_template));
ret = dst_entries_init(&net->xfrm.xfrm4_dst_ops);
if (ret)
return ret;

ret = xfrm4_net_sysctl_init(net);
if (ret)
dst_entries_destroy(&net->xfrm.xfrm4_dst_ops);

return ret;
}

static void __net_exit xfrm4_net_exit(struct net *net)
{
xfrm4_net_sysctl_exit(net);
dst_entries_destroy(&net->xfrm.xfrm4_dst_ops);
}

static struct pernet_operations __net_initdata xfrm4_net_ops = {
.init = xfrm4_net_init,
.exit = xfrm4_net_exit,
};
#endif

static void __init xfrm4_policy_init(void)
{
Expand All @@ -326,13 +358,9 @@ static void __init xfrm4_policy_init(void)

void __init xfrm4_init(void)
{
dst_entries_init(&xfrm4_dst_ops);

xfrm4_state_init();
xfrm4_policy_init();
xfrm4_protocol_init();
#ifdef CONFIG_SYSCTL
register_pernet_subsys(&xfrm4_net_ops);
#endif
}

53 changes: 38 additions & 15 deletions net/ipv6/xfrm6_policy.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ static void xfrm6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
xfrm_dst_ifdown(dst, dev);
}

static struct dst_ops xfrm6_dst_ops = {
static struct dst_ops xfrm6_dst_ops_template = {
.family = AF_INET6,
.gc = xfrm6_garbage_collect,
.update_pmtu = xfrm6_update_pmtu,
Expand All @@ -295,7 +295,7 @@ static struct dst_ops xfrm6_dst_ops = {

static struct xfrm_policy_afinfo xfrm6_policy_afinfo = {
.family = AF_INET6,
.dst_ops = &xfrm6_dst_ops,
.dst_ops = &xfrm6_dst_ops_template,
.dst_lookup = xfrm6_dst_lookup,
.get_saddr = xfrm6_get_saddr,
.decode_session = _decode_session6,
Expand Down Expand Up @@ -327,7 +327,7 @@ static struct ctl_table xfrm6_policy_table[] = {
{ }
};

static int __net_init xfrm6_net_init(struct net *net)
static int __net_init xfrm6_net_sysctl_init(struct net *net)
{
struct ctl_table *table;
struct ctl_table_header *hdr;
Expand Down Expand Up @@ -355,7 +355,7 @@ static int __net_init xfrm6_net_init(struct net *net)
return -ENOMEM;
}

static void __net_exit xfrm6_net_exit(struct net *net)
static void __net_exit xfrm6_net_sysctl_exit(struct net *net)
{
struct ctl_table *table;

Expand All @@ -367,24 +367,52 @@ static void __net_exit xfrm6_net_exit(struct net *net)
if (!net_eq(net, &init_net))
kfree(table);
}
#else /* CONFIG_SYSCTL */
static int inline xfrm6_net_sysctl_init(struct net *net)
{
return 0;
}

static void inline xfrm6_net_sysctl_exit(struct net *net)
{
}
#endif

static int __net_init xfrm6_net_init(struct net *net)
{
int ret;

memcpy(&net->xfrm.xfrm6_dst_ops, &xfrm6_dst_ops_template,
sizeof(xfrm6_dst_ops_template));
ret = dst_entries_init(&net->xfrm.xfrm6_dst_ops);
if (ret)
return ret;

ret = xfrm6_net_sysctl_init(net);
if (ret)
dst_entries_destroy(&net->xfrm.xfrm6_dst_ops);

return ret;
}

static void __net_exit xfrm6_net_exit(struct net *net)
{
xfrm6_net_sysctl_exit(net);
dst_entries_destroy(&net->xfrm.xfrm6_dst_ops);
}

static struct pernet_operations xfrm6_net_ops = {
.init = xfrm6_net_init,
.exit = xfrm6_net_exit,
};
#endif

int __init xfrm6_init(void)
{
int ret;

dst_entries_init(&xfrm6_dst_ops);

ret = xfrm6_policy_init();
if (ret) {
dst_entries_destroy(&xfrm6_dst_ops);
if (ret)
goto out;
}
ret = xfrm6_state_init();
if (ret)
goto out_policy;
Expand All @@ -393,9 +421,7 @@ int __init xfrm6_init(void)
if (ret)
goto out_state;

#ifdef CONFIG_SYSCTL
register_pernet_subsys(&xfrm6_net_ops);
#endif
out:
return ret;
out_state:
Expand All @@ -407,11 +433,8 @@ int __init xfrm6_init(void)

void xfrm6_fini(void)
{
#ifdef CONFIG_SYSCTL
unregister_pernet_subsys(&xfrm6_net_ops);
#endif
xfrm6_protocol_fini();
xfrm6_policy_fini();
xfrm6_state_fini();
dst_entries_destroy(&xfrm6_dst_ops);
}
38 changes: 0 additions & 38 deletions net/xfrm/xfrm_policy.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2807,7 +2807,6 @@ static struct neighbour *xfrm_neigh_lookup(const struct dst_entry *dst,

int xfrm_policy_register_afinfo(struct xfrm_policy_afinfo *afinfo)
{
struct net *net;
int err = 0;
if (unlikely(afinfo == NULL))
return -EINVAL;
Expand Down Expand Up @@ -2838,26 +2837,6 @@ int xfrm_policy_register_afinfo(struct xfrm_policy_afinfo *afinfo)
}
spin_unlock(&xfrm_policy_afinfo_lock);

rtnl_lock();
for_each_net(net) {
struct dst_ops *xfrm_dst_ops;

switch (afinfo->family) {
case AF_INET:
xfrm_dst_ops = &net->xfrm.xfrm4_dst_ops;
break;
#if IS_ENABLED(CONFIG_IPV6)
case AF_INET6:
xfrm_dst_ops = &net->xfrm.xfrm6_dst_ops;
break;
#endif
default:
BUG();
}
*xfrm_dst_ops = *afinfo->dst_ops;
}
rtnl_unlock();

return err;
}
EXPORT_SYMBOL(xfrm_policy_register_afinfo);
Expand Down Expand Up @@ -2893,22 +2872,6 @@ int xfrm_policy_unregister_afinfo(struct xfrm_policy_afinfo *afinfo)
}
EXPORT_SYMBOL(xfrm_policy_unregister_afinfo);

static void __net_init xfrm_dst_ops_init(struct net *net)
{
struct xfrm_policy_afinfo *afinfo;

rcu_read_lock();
afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET]);
if (afinfo)
net->xfrm.xfrm4_dst_ops = *afinfo->dst_ops;
#if IS_ENABLED(CONFIG_IPV6)
afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET6]);
if (afinfo)
net->xfrm.xfrm6_dst_ops = *afinfo->dst_ops;
#endif
rcu_read_unlock();
}

static int xfrm_dev_event(struct notifier_block *this, unsigned long event, void *ptr)
{
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
Expand Down Expand Up @@ -3057,7 +3020,6 @@ static int __net_init xfrm_net_init(struct net *net)
rv = xfrm_policy_init(net);
if (rv < 0)
goto out_policy;
xfrm_dst_ops_init(net);
rv = xfrm_sysctl_init(net);
if (rv < 0)
goto out_sysctl;
Expand Down

0 comments on commit a8a572a

Please sign in to comment.