* Re: [PATCH net] l2tp: fix missing refcount drop in pppol2tp_tunnel_ioctl()
From: David Miller @ 2018-08-10 18:04 UTC (permalink / raw)
To: g.nault; +Cc: netdev, jchapman
In-Reply-To: <20180810175837.GB1430@alphalink.fr>
From: Guillaume Nault <g.nault@alphalink.fr>
Date: Fri, 10 Aug 2018 19:58:38 +0200
> As far as I can see, f664e37dcc52 ("l2tp: fix missing refcount drop in
> pppol2tp_tunnel_ioctl()") is still in your -stable queue, but the two
> patches it depends on haven't made their way to -stable. I'd suggest to
> either drop this patch from your -stable queue, or to also queue up
> ecd012e45ab5 ("l2tp: filter out non-PPP sessions in pppol2tp_tunnel_ioctl()")
> and
> f664e37dcc52 ("l2tp: fix missing refcount drop in pppol2tp_tunnel_ioctl()").
Thanks Guillaume, I'll sort this out the next time I work on stable
submissions.
^ permalink raw reply
* Re: [PATCH net] l2tp: fix missing refcount drop in pppol2tp_tunnel_ioctl()
From: Guillaume Nault @ 2018-08-10 17:58 UTC (permalink / raw)
To: David Miller; +Cc: netdev, jchapman
In-Reply-To: <20180805112157.64rx4btyuwvxlzwb@kdev>
On Sun, Aug 05, 2018 at 01:24:13PM +0200, Guillaume Nault wrote:
> On Fri, Aug 03, 2018 at 12:42:22PM -0700, David Miller wrote:
> > From: Guillaume Nault <g.nault@alphalink.fr>
> > Date: Fri, 3 Aug 2018 17:00:11 +0200
> >
> > > If 'session' is not NULL and is not a PPP pseudo-wire, then we fail to
> > > drop the reference taken by l2tp_session_get().
> > >
> > > Fixes: ecd012e45ab5 ("l2tp: filter out non-PPP sessions in pppol2tp_tunnel_ioctl()")
> > > Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
> > > ---
> > > Sorry for the stupid mistake. I guess I got blinded by the apparent
> > > simplicity of the bug when I wrote the original patch.
> >
> > Applied, thanks.
> >
> > I'm pretty sure I backported the commit this fixes, so I'm queueing
> > this up for -stable as well.
> >
> Well, I think it wasn't. I didn't receive any notification from the
> stable team about it and I don't see it in Greg's stable queue nor
> in any -stable tree.
>
> Also, we'd have to queue 90904ff5f958 ("l2tp: fix pseudo-wire type for
> sessions created by pppol2tp_connect()") first, which is necessary for
> properly identifying PPP sessions.
>
> To recapitulate, three patches are needed to fix the original bug:
>
> * 90904ff5f958 ("l2tp: fix pseudo-wire type for sessions created by
> pppol2tp_connect()"): allows later patches to check if a session is
> PPP.
>
> * ecd012e45ab5 ("l2tp: filter out non-PPP sessions in
> pppol2tp_tunnel_ioctl()"): refuses calling pppol2tp_session_ioctl()
> on non-PPP sessions. This fixes an invalid pointer dereference when
> the session is Ethernet. Unfortunately it fails to drop the
> reference it takes on the session.
>
> * f664e37dcc52 ("l2tp: fix missing refcount drop in
> pppol2tp_tunnel_ioctl()"): fixes the memory leak introduced by the
> previous patch.
>
Hi Dave,
As far as I can see, f664e37dcc52 ("l2tp: fix missing refcount drop in
pppol2tp_tunnel_ioctl()") is still in your -stable queue, but the two
patches it depends on haven't made their way to -stable. I'd suggest to
either drop this patch from your -stable queue, or to also queue up
ecd012e45ab5 ("l2tp: filter out non-PPP sessions in pppol2tp_tunnel_ioctl()")
and
f664e37dcc52 ("l2tp: fix missing refcount drop in pppol2tp_tunnel_ioctl()").
^ permalink raw reply
* [PATCH net-next v2 15/15] net: sched: act_police: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Use tcf spinlock to protect police action private data from concurrent
modification during dump. (init already uses tcf spinlock when changing
police action state)
Pass tcf spinlock as estimator lock argument to gen_replace_estimator()
during action init.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_police.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 1f3192ea8df7..88c16d80c1cf 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -274,14 +274,15 @@ static int tcf_act_police_dump(struct sk_buff *skb, struct tc_action *a,
struct tcf_police *police = to_police(a);
struct tc_police opt = {
.index = police->tcf_index,
- .action = police->tcf_action,
- .mtu = police->tcfp_mtu,
- .burst = PSCHED_NS2TICKS(police->tcfp_burst),
.refcnt = refcount_read(&police->tcf_refcnt) - ref,
.bindcnt = atomic_read(&police->tcf_bindcnt) - bind,
};
struct tcf_t t;
+ spin_lock_bh(&police->tcf_lock);
+ opt.action = police->tcf_action;
+ opt.mtu = police->tcfp_mtu;
+ opt.burst = PSCHED_NS2TICKS(police->tcfp_burst);
if (police->rate_present)
psched_ratecfg_getrate(&opt.rate, &police->rate);
if (police->peak_present)
@@ -301,10 +302,12 @@ static int tcf_act_police_dump(struct sk_buff *skb, struct tc_action *a,
t.expires = jiffies_to_clock_t(police->tcf_tm.expires);
if (nla_put_64bit(skb, TCA_POLICE_TM, sizeof(t), &t, TCA_POLICE_PAD))
goto nla_put_failure;
+ spin_unlock_bh(&police->tcf_lock);
return skb->len;
nla_put_failure:
+ spin_unlock_bh(&police->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 14/15] net: core: protect rate estimator statistics pointer with lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Extend gen_new_estimator() to also take stats_lock when re-assigning rate
estimator statistics pointer. (to be used by unlocked actions)
Rename 'stats_lock' to 'lock' and change argument description to explain
that it is now also used for control path.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
include/net/gen_stats.h | 4 ++--
net/core/gen_estimator.c | 21 +++++++++++++--------
2 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/include/net/gen_stats.h b/include/net/gen_stats.h
index 0304ba2ae353..883bb9085f15 100644
--- a/include/net/gen_stats.h
+++ b/include/net/gen_stats.h
@@ -59,13 +59,13 @@ int gnet_stats_finish_copy(struct gnet_dump *d);
int gen_new_estimator(struct gnet_stats_basic_packed *bstats,
struct gnet_stats_basic_cpu __percpu *cpu_bstats,
struct net_rate_estimator __rcu **rate_est,
- spinlock_t *stats_lock,
+ spinlock_t *lock,
seqcount_t *running, struct nlattr *opt);
void gen_kill_estimator(struct net_rate_estimator __rcu **ptr);
int gen_replace_estimator(struct gnet_stats_basic_packed *bstats,
struct gnet_stats_basic_cpu __percpu *cpu_bstats,
struct net_rate_estimator __rcu **ptr,
- spinlock_t *stats_lock,
+ spinlock_t *lock,
seqcount_t *running, struct nlattr *opt);
bool gen_estimator_active(struct net_rate_estimator __rcu **ptr);
bool gen_estimator_read(struct net_rate_estimator __rcu **ptr,
diff --git a/net/core/gen_estimator.c b/net/core/gen_estimator.c
index 98fd12721221..e4e442d70c2d 100644
--- a/net/core/gen_estimator.c
+++ b/net/core/gen_estimator.c
@@ -112,7 +112,7 @@ static void est_timer(struct timer_list *t)
* @bstats: basic statistics
* @cpu_bstats: bstats per cpu
* @rate_est: rate estimator statistics
- * @stats_lock: statistics lock
+ * @lock: lock for statistics and control path
* @running: qdisc running seqcount
* @opt: rate estimator configuration TLV
*
@@ -128,7 +128,7 @@ static void est_timer(struct timer_list *t)
int gen_new_estimator(struct gnet_stats_basic_packed *bstats,
struct gnet_stats_basic_cpu __percpu *cpu_bstats,
struct net_rate_estimator __rcu **rate_est,
- spinlock_t *stats_lock,
+ spinlock_t *lock,
seqcount_t *running,
struct nlattr *opt)
{
@@ -154,19 +154,22 @@ int gen_new_estimator(struct gnet_stats_basic_packed *bstats,
seqcount_init(&est->seq);
intvl_log = parm->interval + 2;
est->bstats = bstats;
- est->stats_lock = stats_lock;
+ est->stats_lock = lock;
est->running = running;
est->ewma_log = parm->ewma_log;
est->intvl_log = intvl_log;
est->cpu_bstats = cpu_bstats;
- if (stats_lock)
+ if (lock)
local_bh_disable();
est_fetch_counters(est, &b);
- if (stats_lock)
+ if (lock)
local_bh_enable();
est->last_bytes = b.bytes;
est->last_packets = b.packets;
+
+ if (lock)
+ spin_lock_bh(lock);
old = rcu_dereference_protected(*rate_est, 1);
if (old) {
del_timer_sync(&old->timer);
@@ -179,6 +182,8 @@ int gen_new_estimator(struct gnet_stats_basic_packed *bstats,
mod_timer(&est->timer, est->next_jiffies);
rcu_assign_pointer(*rate_est, est);
+ if (lock)
+ spin_unlock_bh(lock);
if (old)
kfree_rcu(old, rcu);
return 0;
@@ -209,7 +214,7 @@ EXPORT_SYMBOL(gen_kill_estimator);
* @bstats: basic statistics
* @cpu_bstats: bstats per cpu
* @rate_est: rate estimator statistics
- * @stats_lock: statistics lock
+ * @lock: lock for statistics and control path
* @running: qdisc running seqcount (might be NULL)
* @opt: rate estimator configuration TLV
*
@@ -221,11 +226,11 @@ EXPORT_SYMBOL(gen_kill_estimator);
int gen_replace_estimator(struct gnet_stats_basic_packed *bstats,
struct gnet_stats_basic_cpu __percpu *cpu_bstats,
struct net_rate_estimator __rcu **rate_est,
- spinlock_t *stats_lock,
+ spinlock_t *lock,
seqcount_t *running, struct nlattr *opt)
{
return gen_new_estimator(bstats, cpu_bstats, rate_est,
- stats_lock, running, opt);
+ lock, running, opt);
}
EXPORT_SYMBOL(gen_replace_estimator);
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 06/15] net: sched: act_pedit: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Rearrange pedit init code to only access pedit action data while holding
tcf spinlock. Change keys allocation type to atomic to allow it to execute
while holding tcf spinlock. Take tcf spinlock in dump function when
accessing pedit action data.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_pedit.c | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 43ba999b2d23..3f62da72ab6a 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -187,44 +187,38 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
tcf_idr_cleanup(tn, parm->index);
goto out_free;
}
- p = to_pedit(*a);
- keys = kmalloc(ksize, GFP_KERNEL);
- if (!keys) {
- tcf_idr_release(*a, bind);
- ret = -ENOMEM;
- goto out_free;
- }
ret = ACT_P_CREATED;
} else if (err > 0) {
if (bind)
goto out_free;
if (!ovr) {
- tcf_idr_release(*a, bind);
ret = -EEXIST;
- goto out_free;
- }
- p = to_pedit(*a);
- if (p->tcfp_nkeys && p->tcfp_nkeys != parm->nkeys) {
- keys = kmalloc(ksize, GFP_KERNEL);
- if (!keys) {
- ret = -ENOMEM;
- goto out_free;
- }
+ goto out_release;
}
} else {
return err;
}
+ p = to_pedit(*a);
spin_lock_bh(&p->tcf_lock);
- p->tcfp_flags = parm->flags;
- p->tcf_action = parm->action;
- if (keys) {
+
+ if (ret == ACT_P_CREATED ||
+ (p->tcfp_nkeys && p->tcfp_nkeys != parm->nkeys)) {
+ keys = kmalloc(ksize, GFP_ATOMIC);
+ if (!keys) {
+ spin_unlock_bh(&p->tcf_lock);
+ ret = -ENOMEM;
+ goto out_release;
+ }
kfree(p->tcfp_keys);
p->tcfp_keys = keys;
p->tcfp_nkeys = parm->nkeys;
}
memcpy(p->tcfp_keys, parm->keys, ksize);
+ p->tcfp_flags = parm->flags;
+ p->tcf_action = parm->action;
+
kfree(p->tcfp_keys_ex);
p->tcfp_keys_ex = keys_ex;
@@ -232,6 +226,9 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
+
+out_release:
+ tcf_idr_release(*a, bind);
out_free:
kfree(keys_ex);
return ret;
@@ -410,6 +407,7 @@ static int tcf_pedit_dump(struct sk_buff *skb, struct tc_action *a,
if (unlikely(!opt))
return -ENOBUFS;
+ spin_lock_bh(&p->tcf_lock);
memcpy(opt->keys, p->tcfp_keys,
p->tcfp_nkeys * sizeof(struct tc_pedit_key));
opt->index = p->tcf_index;
@@ -432,11 +430,13 @@ static int tcf_pedit_dump(struct sk_buff *skb, struct tc_action *a,
tcf_tm_dump(&t, &p->tcf_tm);
if (nla_put_64bit(skb, TCA_PEDIT_TM, sizeof(t), &t, TCA_PEDIT_PAD))
goto nla_put_failure;
+ spin_unlock_bh(&p->tcf_lock);
kfree(opt);
return skb->len;
nla_put_failure:
+ spin_unlock_bh(&p->tcf_lock);
nlmsg_trim(skb, b);
kfree(opt);
return -1;
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 13/15] net: sched: act_mirred: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Re-introduce mirred list spinlock, that was removed some time ago, in order
to protect it from concurrent modifications, instead of relying on rtnl
lock.
Use tcf spinlock to protect mirred action private data from concurrent
modification in init and dump. Rearrange access to mirred data in order to
be performed only while holding the lock.
Rearrange net dev access to always hold reference while working with it,
instead of relying on rntl lock.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_mirred.c | 78 +++++++++++++++++++++++++++++++++-----------------
1 file changed, 51 insertions(+), 27 deletions(-)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 7a045cc7fe3b..327be257033d 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -30,6 +30,7 @@
#include <net/tc_act/tc_mirred.h>
static LIST_HEAD(mirred_list);
+static DEFINE_SPINLOCK(mirred_list_lock);
static bool tcf_mirred_is_act_redirect(int action)
{
@@ -62,13 +63,23 @@ static bool tcf_mirred_can_reinsert(int action)
return false;
}
+static struct net_device *tcf_mirred_dev_dereference(struct tcf_mirred *m)
+{
+ return rcu_dereference_protected(m->tcfm_dev,
+ lockdep_is_held(&m->tcf_lock));
+}
+
static void tcf_mirred_release(struct tc_action *a)
{
struct tcf_mirred *m = to_mirred(a);
struct net_device *dev;
+ spin_lock(&mirred_list_lock);
list_del(&m->tcfm_list);
- dev = rtnl_dereference(m->tcfm_dev);
+ spin_unlock(&mirred_list_lock);
+
+ /* last reference to action, no need to lock */
+ dev = rcu_dereference_protected(m->tcfm_dev, 1);
if (dev)
dev_put(dev);
}
@@ -128,22 +139,9 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
NL_SET_ERR_MSG_MOD(extack, "Unknown mirred option");
return -EINVAL;
}
- if (parm->ifindex) {
- dev = __dev_get_by_index(net, parm->ifindex);
- if (dev == NULL) {
- if (exists)
- tcf_idr_release(*a, bind);
- else
- tcf_idr_cleanup(tn, parm->index);
- return -ENODEV;
- }
- mac_header_xmit = dev_is_mac_header_xmit(dev);
- } else {
- dev = NULL;
- }
if (!exists) {
- if (!dev) {
+ if (!parm->ifindex) {
tcf_idr_cleanup(tn, parm->index);
NL_SET_ERR_MSG_MOD(extack, "Specified device does not exist");
return -EINVAL;
@@ -161,19 +159,31 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
}
m = to_mirred(*a);
- ASSERT_RTNL();
+ spin_lock(&m->tcf_lock);
m->tcf_action = parm->action;
m->tcfm_eaction = parm->eaction;
- if (dev != NULL) {
- if (ret != ACT_P_CREATED)
- dev_put(rcu_dereference_protected(m->tcfm_dev, 1));
- dev_hold(dev);
- rcu_assign_pointer(m->tcfm_dev, dev);
+
+ if (parm->ifindex) {
+ dev = dev_get_by_index(net, parm->ifindex);
+ if (!dev) {
+ spin_unlock(&m->tcf_lock);
+ tcf_idr_release(*a, bind);
+ return -ENODEV;
+ }
+ mac_header_xmit = dev_is_mac_header_xmit(dev);
+ rcu_swap_protected(m->tcfm_dev, dev,
+ lockdep_is_held(&m->tcf_lock));
+ if (dev)
+ dev_put(dev);
m->tcfm_mac_header_xmit = mac_header_xmit;
}
+ spin_unlock(&m->tcf_lock);
if (ret == ACT_P_CREATED) {
+ spin_lock(&mirred_list_lock);
list_add(&m->tcfm_list, &mirred_list);
+ spin_unlock(&mirred_list_lock);
+
tcf_idr_insert(tn, *a);
}
@@ -287,26 +297,33 @@ static int tcf_mirred_dump(struct sk_buff *skb, struct tc_action *a, int bind,
{
unsigned char *b = skb_tail_pointer(skb);
struct tcf_mirred *m = to_mirred(a);
- struct net_device *dev = rtnl_dereference(m->tcfm_dev);
struct tc_mirred opt = {
.index = m->tcf_index,
- .action = m->tcf_action,
.refcnt = refcount_read(&m->tcf_refcnt) - ref,
.bindcnt = atomic_read(&m->tcf_bindcnt) - bind,
- .eaction = m->tcfm_eaction,
- .ifindex = dev ? dev->ifindex : 0,
};
+ struct net_device *dev;
struct tcf_t t;
+ spin_lock(&m->tcf_lock);
+ opt.action = m->tcf_action;
+ opt.eaction = m->tcfm_eaction;
+ dev = tcf_mirred_dev_dereference(m);
+ if (dev)
+ opt.ifindex = dev->ifindex;
+
if (nla_put(skb, TCA_MIRRED_PARMS, sizeof(opt), &opt))
goto nla_put_failure;
tcf_tm_dump(&t, &m->tcf_tm);
if (nla_put_64bit(skb, TCA_MIRRED_TM, sizeof(t), &t, TCA_MIRRED_PAD))
goto nla_put_failure;
+ spin_unlock(&m->tcf_lock);
+
return skb->len;
nla_put_failure:
+ spin_unlock(&m->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
@@ -337,15 +354,19 @@ static int mirred_device_event(struct notifier_block *unused,
ASSERT_RTNL();
if (event == NETDEV_UNREGISTER) {
+ spin_lock(&mirred_list_lock);
list_for_each_entry(m, &mirred_list, tcfm_list) {
- if (rcu_access_pointer(m->tcfm_dev) == dev) {
+ spin_lock(&m->tcf_lock);
+ if (tcf_mirred_dev_dereference(m) == dev) {
dev_put(dev);
/* Note : no rcu grace period necessary, as
* net_device are already rcu protected.
*/
RCU_INIT_POINTER(m->tcfm_dev, NULL);
}
+ spin_unlock(&m->tcf_lock);
}
+ spin_unlock(&mirred_list_lock);
}
return NOTIFY_DONE;
@@ -358,10 +379,13 @@ static struct notifier_block mirred_device_notifier = {
static struct net_device *tcf_mirred_get_dev(const struct tc_action *a)
{
struct tcf_mirred *m = to_mirred(a);
- struct net_device *dev = rtnl_dereference(m->tcfm_dev);
+ struct net_device *dev;
+ rcu_read_lock();
+ dev = rcu_dereference(m->tcfm_dev);
if (dev)
dev_hold(dev);
+ rcu_read_unlock();
return dev;
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 12/15] net: sched: extend action ops with put_dev callback
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
As a preparation for removing dependency on rtnl lock from rules update
path, all users of shared objects must take reference while working with
them.
Extend action ops with put_dev() API to be used on net device returned by
get_dev().
Modify mirred action (only action that implements get_dev callback):
- Take reference to net device in get_dev.
- Implement put_dev API that releases reference to net device.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
include/net/act_api.h | 1 +
net/sched/act_mirred.c | 12 +++++++++++-
net/sched/cls_api.c | 1 +
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index 8c9bc02d05e1..1ad5b19e83a9 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -101,6 +101,7 @@ struct tc_action_ops {
void (*stats_update)(struct tc_action *, u64, u32, u64);
size_t (*get_fill_size)(const struct tc_action *act);
struct net_device *(*get_dev)(const struct tc_action *a);
+ void (*put_dev)(struct net_device *dev);
int (*delete)(struct net *net, u32 index);
};
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index b26d060da08e..7a045cc7fe3b 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -358,8 +358,17 @@ static struct notifier_block mirred_device_notifier = {
static struct net_device *tcf_mirred_get_dev(const struct tc_action *a)
{
struct tcf_mirred *m = to_mirred(a);
+ struct net_device *dev = rtnl_dereference(m->tcfm_dev);
+
+ if (dev)
+ dev_hold(dev);
- return rtnl_dereference(m->tcfm_dev);
+ return dev;
+}
+
+static void tcf_mirred_put_dev(struct net_device *dev)
+{
+ dev_put(dev);
}
static int tcf_mirred_delete(struct net *net, u32 index)
@@ -382,6 +391,7 @@ static struct tc_action_ops act_mirred_ops = {
.lookup = tcf_mirred_search,
.size = sizeof(struct tcf_mirred),
.get_dev = tcf_mirred_get_dev,
+ .put_dev = tcf_mirred_put_dev,
.delete = tcf_mirred_delete,
};
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index e8b0bbd0883f..bdabf8144e21 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -2167,6 +2167,7 @@ static int tc_exts_setup_cb_egdev_call(struct tcf_exts *exts,
if (!dev)
continue;
ret = tc_setup_cb_egdev_call(dev, type, type_data, err_stop);
+ a->ops->put_dev(dev);
if (ret < 0)
return ret;
ok_count += ret;
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 05/15] net: sched: act_ipt: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Use tcf spinlock to protect ipt action private data from concurrent
modification during dump. Ipt init already takes tcf spinlock when
modifying ipt state.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_ipt.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index 0dc787a57798..e149f0e66cb6 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -288,6 +288,7 @@ static int tcf_ipt_dump(struct sk_buff *skb, struct tc_action *a, int bind,
* for foolproof you need to not assume this
*/
+ spin_lock_bh(&ipt->tcf_lock);
t = kmemdup(ipt->tcfi_t, ipt->tcfi_t->u.user.target_size, GFP_ATOMIC);
if (unlikely(!t))
goto nla_put_failure;
@@ -307,10 +308,12 @@ static int tcf_ipt_dump(struct sk_buff *skb, struct tc_action *a, int bind,
if (nla_put_64bit(skb, TCA_IPT_TM, sizeof(tm), &tm, TCA_IPT_PAD))
goto nla_put_failure;
+ spin_unlock_bh(&ipt->tcf_lock);
kfree(t);
return skb->len;
nla_put_failure:
+ spin_unlock_bh(&ipt->tcf_lock);
nlmsg_trim(skb, b);
kfree(t);
return -1;
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 11/15] net: sched: act_vlan: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Use tcf spinlock to protect vlan action private data from concurrent
modification during dump and init. Use rcu swap operation to reassign
params pointer under protection of tcf lock. (old params value is not used
by init, so there is no need of standalone rcu dereference step)
Remove rtnl assertion that is no longer necessary.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_vlan.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 15a0ee214c9c..5bde17fe3608 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -109,7 +109,7 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
{
struct tc_action_net *tn = net_generic(net, vlan_net_id);
struct nlattr *tb[TCA_VLAN_MAX + 1];
- struct tcf_vlan_params *p, *p_old;
+ struct tcf_vlan_params *p;
struct tc_vlan *parm;
struct tcf_vlan *v;
int action;
@@ -202,26 +202,24 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
v = to_vlan(*a);
- ASSERT_RTNL();
p = kzalloc(sizeof(*p), GFP_KERNEL);
if (!p) {
tcf_idr_release(*a, bind);
return -ENOMEM;
}
- v->tcf_action = parm->action;
-
- p_old = rtnl_dereference(v->vlan_p);
-
p->tcfv_action = action;
p->tcfv_push_vid = push_vid;
p->tcfv_push_prio = push_prio;
p->tcfv_push_proto = push_proto;
- rcu_assign_pointer(v->vlan_p, p);
+ spin_lock(&v->tcf_lock);
+ v->tcf_action = parm->action;
+ rcu_swap_protected(v->vlan_p, p, lockdep_is_held(&v->tcf_lock));
+ spin_unlock(&v->tcf_lock);
- if (p_old)
- kfree_rcu(p_old, rcu);
+ if (p)
+ kfree_rcu(p, rcu);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
@@ -243,16 +241,18 @@ static int tcf_vlan_dump(struct sk_buff *skb, struct tc_action *a,
{
unsigned char *b = skb_tail_pointer(skb);
struct tcf_vlan *v = to_vlan(a);
- struct tcf_vlan_params *p = rtnl_dereference(v->vlan_p);
+ struct tcf_vlan_params *p;
struct tc_vlan opt = {
.index = v->tcf_index,
.refcnt = refcount_read(&v->tcf_refcnt) - ref,
.bindcnt = atomic_read(&v->tcf_bindcnt) - bind,
- .action = v->tcf_action,
- .v_action = p->tcfv_action,
};
struct tcf_t t;
+ spin_lock(&v->tcf_lock);
+ opt.action = v->tcf_action;
+ p = rcu_dereference_protected(v->vlan_p, lockdep_is_held(&v->tcf_lock));
+ opt.v_action = p->tcfv_action;
if (nla_put(skb, TCA_VLAN_PARMS, sizeof(opt), &opt))
goto nla_put_failure;
@@ -268,9 +268,12 @@ static int tcf_vlan_dump(struct sk_buff *skb, struct tc_action *a,
tcf_tm_dump(&t, &v->tcf_tm);
if (nla_put_64bit(skb, TCA_VLAN_TM, sizeof(t), &t, TCA_VLAN_PAD))
goto nla_put_failure;
+ spin_unlock(&v->tcf_lock);
+
return skb->len;
nla_put_failure:
+ spin_unlock(&v->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 09/15] net: sched: act_skbmod: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Move read of skbmod_p rcu pointer to be protected by tcf spinlock. Use tcf
spinlock to protect private skbmod data from concurrent modification during
dump.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_skbmod.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index c437c6d51a71..e9c86ade3b40 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -156,7 +156,6 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
d = to_skbmod(*a);
- ASSERT_RTNL();
p = kzalloc(sizeof(struct tcf_skbmod_params), GFP_KERNEL);
if (unlikely(!p)) {
tcf_idr_release(*a, bind);
@@ -166,10 +165,10 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
p->flags = lflags;
d->tcf_action = parm->action;
- p_old = rtnl_dereference(d->skbmod_p);
-
if (ovr)
spin_lock_bh(&d->tcf_lock);
+ /* Protected by tcf_lock if overwriting existing action. */
+ p_old = rcu_dereference_protected(d->skbmod_p, 1);
if (lflags & SKBMOD_F_DMAC)
ether_addr_copy(p->eth_dst, daddr);
@@ -205,15 +204,18 @@ static int tcf_skbmod_dump(struct sk_buff *skb, struct tc_action *a,
{
struct tcf_skbmod *d = to_skbmod(a);
unsigned char *b = skb_tail_pointer(skb);
- struct tcf_skbmod_params *p = rtnl_dereference(d->skbmod_p);
+ struct tcf_skbmod_params *p;
struct tc_skbmod opt = {
.index = d->tcf_index,
.refcnt = refcount_read(&d->tcf_refcnt) - ref,
.bindcnt = atomic_read(&d->tcf_bindcnt) - bind,
- .action = d->tcf_action,
};
struct tcf_t t;
+ spin_lock_bh(&d->tcf_lock);
+ opt.action = d->tcf_action;
+ p = rcu_dereference_protected(d->skbmod_p,
+ lockdep_is_held(&d->tcf_lock));
opt.flags = p->flags;
if (nla_put(skb, TCA_SKBMOD_PARMS, sizeof(opt), &opt))
goto nla_put_failure;
@@ -231,8 +233,10 @@ static int tcf_skbmod_dump(struct sk_buff *skb, struct tc_action *a,
if (nla_put_64bit(skb, TCA_SKBMOD_TM, sizeof(t), &t, TCA_SKBMOD_PAD))
goto nla_put_failure;
+ spin_unlock_bh(&d->tcf_lock);
return skb->len;
nla_put_failure:
+ spin_unlock_bh(&d->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 10/15] net: sched: act_tunnel_key: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Use tcf lock to protect tunnel key action struct private data from
concurrent modification in init and dump. Use rcu swap operation to
reassign params pointer under protection of tcf lock. (old params value is
not used by init, so there is no need of standalone rcu dereference step)
Remove rtnl lock assertion that is no longer required.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_tunnel_key.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index d42d9e112789..ba2ae9f75ef5 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -204,7 +204,6 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
{
struct tc_action_net *tn = net_generic(net, tunnel_key_net_id);
struct nlattr *tb[TCA_TUNNEL_KEY_MAX + 1];
- struct tcf_tunnel_key_params *params_old;
struct tcf_tunnel_key_params *params_new;
struct metadata_dst *metadata = NULL;
struct tc_tunnel_key *parm;
@@ -346,24 +345,22 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
t = to_tunnel_key(*a);
- ASSERT_RTNL();
params_new = kzalloc(sizeof(*params_new), GFP_KERNEL);
if (unlikely(!params_new)) {
tcf_idr_release(*a, bind);
NL_SET_ERR_MSG(extack, "Cannot allocate tunnel key parameters");
return -ENOMEM;
}
-
- params_old = rtnl_dereference(t->params);
-
- t->tcf_action = parm->action;
params_new->tcft_action = parm->t_action;
params_new->tcft_enc_metadata = metadata;
- rcu_assign_pointer(t->params, params_new);
-
- if (params_old)
- kfree_rcu(params_old, rcu);
+ spin_lock(&t->tcf_lock);
+ t->tcf_action = parm->action;
+ rcu_swap_protected(t->params, params_new,
+ lockdep_is_held(&t->tcf_lock));
+ spin_unlock(&t->tcf_lock);
+ if (params_new)
+ kfree_rcu(params_new, rcu);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
@@ -485,12 +482,13 @@ static int tunnel_key_dump(struct sk_buff *skb, struct tc_action *a,
.index = t->tcf_index,
.refcnt = refcount_read(&t->tcf_refcnt) - ref,
.bindcnt = atomic_read(&t->tcf_bindcnt) - bind,
- .action = t->tcf_action,
};
struct tcf_t tm;
- params = rtnl_dereference(t->params);
-
+ spin_lock(&t->tcf_lock);
+ params = rcu_dereference_protected(t->params,
+ lockdep_is_held(&t->tcf_lock));
+ opt.action = t->tcf_action;
opt.t_action = params->tcft_action;
if (nla_put(skb, TCA_TUNNEL_KEY_PARMS, sizeof(opt), &opt))
@@ -522,10 +520,12 @@ static int tunnel_key_dump(struct sk_buff *skb, struct tc_action *a,
if (nla_put_64bit(skb, TCA_TUNNEL_KEY_TM, sizeof(tm),
&tm, TCA_TUNNEL_KEY_PAD))
goto nla_put_failure;
+ spin_unlock(&t->tcf_lock);
return skb->len;
nla_put_failure:
+ spin_unlock(&t->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 08/15] net: sched: act_simple: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Use tcf spinlock to protect private simple action data from concurrent
modification during dump. (simple init already uses tcf spinlock when
changing action state)
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_simple.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index aa51152e0066..18e4452574cd 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -156,10 +156,11 @@ static int tcf_simp_dump(struct sk_buff *skb, struct tc_action *a,
.index = d->tcf_index,
.refcnt = refcount_read(&d->tcf_refcnt) - ref,
.bindcnt = atomic_read(&d->tcf_bindcnt) - bind,
- .action = d->tcf_action,
};
struct tcf_t t;
+ spin_lock_bh(&d->tcf_lock);
+ opt.action = d->tcf_action;
if (nla_put(skb, TCA_DEF_PARMS, sizeof(opt), &opt) ||
nla_put_string(skb, TCA_DEF_DATA, d->tcfd_defdata))
goto nla_put_failure;
@@ -167,9 +168,12 @@ static int tcf_simp_dump(struct sk_buff *skb, struct tc_action *a,
tcf_tm_dump(&t, &d->tcf_tm);
if (nla_put_64bit(skb, TCA_DEF_TM, sizeof(t), &t, TCA_DEF_PAD))
goto nla_put_failure;
+ spin_unlock_bh(&d->tcf_lock);
+
return skb->len;
nla_put_failure:
+ spin_unlock_bh(&d->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 04/15] net: sched: act_ife: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Use tcf spinlock and rcu to protect params pointer from concurrent
modification during dump and init. Use rcu swap operation to reassign
params pointer under protection of tcf lock. (old params value is not used
by init, so there is no need of standalone rcu dereference step)
Ife action has meta-actions that are compiled as standalone modules. Rtnl
mutex must be released while loading a kernel module. In order to support
execution without rtnl mutex, propagate 'rtnl_held' argument to meta action
loading functions. When requesting meta action module, conditionally
release rtnl lock depending on 'rtnl_held' argument.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_ife.c | 40 +++++++++++++++++++++++++---------------
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index df4060e32d43..5d200495e467 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -268,7 +268,8 @@ static const char *ife_meta_id2name(u32 metaid)
* under ife->tcf_lock for existing action
*/
static int load_metaops_and_vet(struct tcf_ife_info *ife, u32 metaid,
- void *val, int len, bool exists)
+ void *val, int len, bool exists,
+ bool rtnl_held)
{
struct tcf_meta_ops *ops = find_ife_oplist(metaid);
int ret = 0;
@@ -278,9 +279,11 @@ static int load_metaops_and_vet(struct tcf_ife_info *ife, u32 metaid,
#ifdef CONFIG_MODULES
if (exists)
spin_unlock_bh(&ife->tcf_lock);
- rtnl_unlock();
+ if (rtnl_held)
+ rtnl_unlock();
request_module("ife-meta-%s", ife_meta_id2name(metaid));
- rtnl_lock();
+ if (rtnl_held)
+ rtnl_lock();
if (exists)
spin_lock_bh(&ife->tcf_lock);
ops = find_ife_oplist(metaid);
@@ -421,7 +424,7 @@ static void tcf_ife_cleanup(struct tc_action *a)
/* under ife->tcf_lock for existing action */
static int populate_metalist(struct tcf_ife_info *ife, struct nlattr **tb,
- bool exists)
+ bool exists, bool rtnl_held)
{
int len = 0;
int rc = 0;
@@ -433,7 +436,8 @@ static int populate_metalist(struct tcf_ife_info *ife, struct nlattr **tb,
val = nla_data(tb[i]);
len = nla_len(tb[i]);
- rc = load_metaops_and_vet(ife, i, val, len, exists);
+ rc = load_metaops_and_vet(ife, i, val, len, exists,
+ rtnl_held);
if (rc != 0)
return rc;
@@ -454,7 +458,7 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
struct tc_action_net *tn = net_generic(net, ife_net_id);
struct nlattr *tb[TCA_IFE_MAX + 1];
struct nlattr *tb2[IFE_META_MAX + 1];
- struct tcf_ife_params *p, *p_old;
+ struct tcf_ife_params *p;
struct tcf_ife_info *ife;
u16 ife_type = ETH_P_IFE;
struct tc_ife *parm;
@@ -558,7 +562,7 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
return err;
}
- err = populate_metalist(ife, tb2, exists);
+ err = populate_metalist(ife, tb2, exists, rtnl_held);
if (err)
goto metadata_parse_err;
@@ -581,13 +585,13 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
}
ife->tcf_action = parm->action;
+ /* protected by tcf_lock when modifying existing action */
+ rcu_swap_protected(ife->params, p, 1);
+
if (exists)
spin_unlock_bh(&ife->tcf_lock);
-
- p_old = rtnl_dereference(ife->params);
- rcu_assign_pointer(ife->params, p);
- if (p_old)
- kfree_rcu(p_old, rcu);
+ if (p)
+ kfree_rcu(p, rcu);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
@@ -600,16 +604,20 @@ static int tcf_ife_dump(struct sk_buff *skb, struct tc_action *a, int bind,
{
unsigned char *b = skb_tail_pointer(skb);
struct tcf_ife_info *ife = to_ife(a);
- struct tcf_ife_params *p = rtnl_dereference(ife->params);
+ struct tcf_ife_params *p;
struct tc_ife opt = {
.index = ife->tcf_index,
.refcnt = refcount_read(&ife->tcf_refcnt) - ref,
.bindcnt = atomic_read(&ife->tcf_bindcnt) - bind,
- .action = ife->tcf_action,
- .flags = p->flags,
};
struct tcf_t t;
+ spin_lock_bh(&ife->tcf_lock);
+ opt.action = ife->tcf_action;
+ p = rcu_dereference_protected(ife->params,
+ lockdep_is_held(&ife->tcf_lock));
+ opt.flags = p->flags;
+
if (nla_put(skb, TCA_IFE_PARMS, sizeof(opt), &opt))
goto nla_put_failure;
@@ -635,9 +643,11 @@ static int tcf_ife_dump(struct sk_buff *skb, struct tc_action *a, int bind,
pr_info("Failed to dump metalist\n");
}
+ spin_unlock_bh(&ife->tcf_lock);
return skb->len;
nla_put_failure:
+ spin_unlock_bh(&ife->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 03/15] net: sched: act_gact: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Use tcf spinlock to protect gact action private state from concurrent
modification during dump and init. Remove rtnl assertion that is no longer
necessary.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_gact.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index 661b72b9147d..bfccd34a3968 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -113,7 +113,7 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
gact = to_gact(*a);
- ASSERT_RTNL();
+ spin_lock(&gact->tcf_lock);
gact->tcf_action = parm->action;
#ifdef CONFIG_GACT_PROB
if (p_parm) {
@@ -126,6 +126,8 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
gact->tcfg_ptype = p_parm->ptype;
}
#endif
+ spin_unlock(&gact->tcf_lock);
+
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
@@ -178,10 +180,11 @@ static int tcf_gact_dump(struct sk_buff *skb, struct tc_action *a,
.index = gact->tcf_index,
.refcnt = refcount_read(&gact->tcf_refcnt) - ref,
.bindcnt = atomic_read(&gact->tcf_bindcnt) - bind,
- .action = gact->tcf_action,
};
struct tcf_t t;
+ spin_lock(&gact->tcf_lock);
+ opt.action = gact->tcf_action;
if (nla_put(skb, TCA_GACT_PARMS, sizeof(opt), &opt))
goto nla_put_failure;
#ifdef CONFIG_GACT_PROB
@@ -199,9 +202,12 @@ static int tcf_gact_dump(struct sk_buff *skb, struct tc_action *a,
tcf_tm_dump(&t, &gact->tcf_tm);
if (nla_put_64bit(skb, TCA_GACT_TM, sizeof(t), &t, TCA_GACT_PAD))
goto nla_put_failure;
+ spin_unlock(&gact->tcf_lock);
+
return skb->len;
nla_put_failure:
+ spin_unlock(&gact->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 07/15] net: sched: act_sample: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Use tcf spinlock to protect private sample action data from concurrent
modification during dump and init.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_sample.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index 2608ccc83e5e..81071afe1b43 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -80,11 +80,13 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
}
s = to_sample(*a);
+ spin_lock(&s->tcf_lock);
s->tcf_action = parm->action;
s->rate = nla_get_u32(tb[TCA_SAMPLE_RATE]);
s->psample_group_num = nla_get_u32(tb[TCA_SAMPLE_PSAMPLE_GROUP]);
psample_group = psample_group_get(net, s->psample_group_num);
if (!psample_group) {
+ spin_unlock(&s->tcf_lock);
tcf_idr_release(*a, bind);
return -ENOMEM;
}
@@ -94,6 +96,7 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
s->truncate = true;
s->trunc_size = nla_get_u32(tb[TCA_SAMPLE_TRUNC_SIZE]);
}
+ spin_unlock(&s->tcf_lock);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
@@ -105,7 +108,8 @@ static void tcf_sample_cleanup(struct tc_action *a)
struct tcf_sample *s = to_sample(a);
struct psample_group *psample_group;
- psample_group = rtnl_dereference(s->psample_group);
+ /* last reference to action, no need to lock */
+ psample_group = rcu_dereference_protected(s->psample_group, 1);
RCU_INIT_POINTER(s->psample_group, NULL);
if (psample_group)
psample_group_put(psample_group);
@@ -174,12 +178,13 @@ static int tcf_sample_dump(struct sk_buff *skb, struct tc_action *a,
struct tcf_sample *s = to_sample(a);
struct tc_sample opt = {
.index = s->tcf_index,
- .action = s->tcf_action,
.refcnt = refcount_read(&s->tcf_refcnt) - ref,
.bindcnt = atomic_read(&s->tcf_bindcnt) - bind,
};
struct tcf_t t;
+ spin_lock(&s->tcf_lock);
+ opt.action = s->tcf_action;
if (nla_put(skb, TCA_SAMPLE_PARMS, sizeof(opt), &opt))
goto nla_put_failure;
@@ -196,9 +201,12 @@ static int tcf_sample_dump(struct sk_buff *skb, struct tc_action *a,
if (nla_put_u32(skb, TCA_SAMPLE_PSAMPLE_GROUP, s->psample_group_num))
goto nla_put_failure;
+ spin_unlock(&s->tcf_lock);
+
return skb->len;
nla_put_failure:
+ spin_unlock(&s->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 02/15] net: sched: act_csum: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Use tcf lock to protect csum action struct private data from concurrent
modification in init and dump. Use rcu swap operation to reassign params
pointer under protection of tcf lock. (old params value is not used by
init, so there is no need of standalone rcu dereference step)
Remove rtnl assertion that is no longer necessary.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_csum.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 648a3a35b720..f01c59ba6d12 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -50,7 +50,7 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
struct netlink_ext_ack *extack)
{
struct tc_action_net *tn = net_generic(net, csum_net_id);
- struct tcf_csum_params *params_old, *params_new;
+ struct tcf_csum_params *params_new;
struct nlattr *tb[TCA_CSUM_MAX + 1];
struct tc_csum *parm;
struct tcf_csum *p;
@@ -88,20 +88,22 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
}
p = to_tcf_csum(*a);
- ASSERT_RTNL();
params_new = kzalloc(sizeof(*params_new), GFP_KERNEL);
if (unlikely(!params_new)) {
tcf_idr_release(*a, bind);
return -ENOMEM;
}
- params_old = rtnl_dereference(p->params);
+ params_new->update_flags = parm->update_flags;
+ spin_lock(&p->tcf_lock);
p->tcf_action = parm->action;
- params_new->update_flags = parm->update_flags;
- rcu_assign_pointer(p->params, params_new);
- if (params_old)
- kfree_rcu(params_old, rcu);
+ rcu_swap_protected(p->params, params_new,
+ lockdep_is_held(&p->tcf_lock));
+ spin_unlock(&p->tcf_lock);
+
+ if (params_new)
+ kfree_rcu(params_new, rcu);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
@@ -599,11 +601,13 @@ static int tcf_csum_dump(struct sk_buff *skb, struct tc_action *a, int bind,
.index = p->tcf_index,
.refcnt = refcount_read(&p->tcf_refcnt) - ref,
.bindcnt = atomic_read(&p->tcf_bindcnt) - bind,
- .action = p->tcf_action,
};
struct tcf_t t;
- params = rtnl_dereference(p->params);
+ spin_lock(&p->tcf_lock);
+ params = rcu_dereference_protected(p->params,
+ lockdep_is_held(&p->tcf_lock));
+ opt.action = p->tcf_action;
opt.update_flags = params->update_flags;
if (nla_put(skb, TCA_CSUM_PARMS, sizeof(opt), &opt))
@@ -612,10 +616,12 @@ static int tcf_csum_dump(struct sk_buff *skb, struct tc_action *a, int bind,
tcf_tm_dump(&t, &p->tcf_tm);
if (nla_put_64bit(skb, TCA_CSUM_TM, sizeof(t), &t, TCA_CSUM_PAD))
goto nla_put_failure;
+ spin_unlock(&p->tcf_lock);
return skb->len;
nla_put_failure:
+ spin_unlock(&p->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 01/15] net: sched: act_bpf: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>
Use tcf spinlock to protect bpf action private data from concurrent
modification during dump and init. Remove rtnl lock assertion that is no
longer necessary.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_bpf.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index 6203eb075c9a..9e8a33f9fee3 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -143,11 +143,12 @@ static int tcf_bpf_dump(struct sk_buff *skb, struct tc_action *act,
.index = prog->tcf_index,
.refcnt = refcount_read(&prog->tcf_refcnt) - ref,
.bindcnt = atomic_read(&prog->tcf_bindcnt) - bind,
- .action = prog->tcf_action,
};
struct tcf_t tm;
int ret;
+ spin_lock(&prog->tcf_lock);
+ opt.action = prog->tcf_action;
if (nla_put(skb, TCA_ACT_BPF_PARMS, sizeof(opt), &opt))
goto nla_put_failure;
@@ -163,9 +164,11 @@ static int tcf_bpf_dump(struct sk_buff *skb, struct tc_action *act,
TCA_ACT_BPF_PAD))
goto nla_put_failure;
+ spin_unlock(&prog->tcf_lock);
return skb->len;
nla_put_failure:
+ spin_unlock(&prog->tcf_lock);
nlmsg_trim(skb, tp);
return -1;
}
@@ -264,7 +267,7 @@ static void tcf_bpf_prog_fill_cfg(const struct tcf_bpf *prog,
{
cfg->is_ebpf = tcf_bpf_is_ebpf(prog);
/* updates to prog->filter are prevented, since it's called either
- * with rtnl lock or during final cleanup in rcu callback
+ * with tcf lock or during final cleanup in rcu callback
*/
cfg->filter = rcu_dereference_protected(prog->filter, 1);
@@ -336,8 +339,8 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
goto out;
prog = to_bpf(*act);
- ASSERT_RTNL();
+ spin_lock(&prog->tcf_lock);
if (res != ACT_P_CREATED)
tcf_bpf_prog_fill_cfg(prog, &old);
@@ -349,6 +352,7 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
prog->tcf_action = parm->action;
rcu_assign_pointer(prog->filter, cfg.filter);
+ spin_unlock(&prog->tcf_lock);
if (res == ACT_P_CREATED) {
tcf_idr_insert(tn, *act);
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v2 00/15] Remove rtnl lock dependency from all action implementations
From: Vlad Buslov @ 2018-08-10 17:51 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
edumazet, keescook, marcelo.leitner, Vlad Buslov
Currently, all netlink protocol handlers for updating rules, actions and
qdiscs are protected with single global rtnl lock which removes any
possibility for parallelism. This patch set is a second step to remove
rtnl lock dependency from TC rules update path.
Recently, new rtnl registration flag RTNL_FLAG_DOIT_UNLOCKED was added.
Handlers registered with this flag are called without RTNL taken. End
goal is to have rule update handlers(RTM_NEWTFILTER, RTM_DELTFILTER,
etc.) to be registered with UNLOCKED flag to allow parallel execution.
However, there is no intention to completely remove or split rtnl lock
itself. This patch set addresses specific problems in implementation of
tc actions that prevent their control path from being executed
concurrently. Additional changes are required to refactor classifiers
API and individual classifiers for parallel execution. This patch set
lays groundwork to eventually register rule update handlers as
rtnl-unlocked.
Action API is already prepared for parallel execution with previous
patch set, which means that action ops that use action API for their
implementation do not require additional modifications. (delete, search,
etc.) Action API implements concurrency-safe reference counting and
guarantees that cleanup/delete is called only once, after last reference
to action is released.
The goal of this change is to update specific actions APIs that access
action private state directly, in order to be independent from external
locking. General approach is to re-use existing tcf_lock spinlock (used
by some action implementation to synchronize control path with data
path) to protect action private state from concurrent modification. If
action has rcu-protected pointer, tcf spinlock is used to protect its
update code, instead of relying on rtnl lock.
Some actions need to determine rtnl mutex status in order to release it.
For example, ife action can load additional kernel modules(meta ops) and
must make sure that no locks are held during module load. In such cases
'rtnl_held' argument is used to conditionally release rtnl mutex.
Changes from V1 to V2:
- Patch 12:
- new patch
- Patch 14:
- refactor gen_new_estimator() to reuse stats_lock when re-assigning
rate estimator statistics pointer
- Remove mirred and tunnel_key helper function changes. (to be submitted
and standalone patch)
Vlad Buslov (15):
net: sched: act_bpf: remove dependency on rtnl lock
net: sched: act_csum: remove dependency on rtnl lock
net: sched: act_gact: remove dependency on rtnl lock
net: sched: act_ife: remove dependency on rtnl lock
net: sched: act_ipt: remove dependency on rtnl lock
net: sched: act_pedit: remove dependency on rtnl lock
net: sched: act_sample: remove dependency on rtnl lock
net: sched: act_simple: remove dependency on rtnl lock
net: sched: act_skbmod: remove dependency on rtnl lock
net: sched: act_tunnel_key: remove dependency on rtnl lock
net: sched: act_vlan: remove dependency on rtnl lock
net: sched: extend action ops with put_dev callback
net: sched: act_mirred: remove dependency on rtnl lock
net: core: protect rate estimator statistics pointer with lock
net: sched: act_police: remove dependency on rtnl lock
include/net/act_api.h | 1 +
include/net/gen_stats.h | 4 +--
net/core/gen_estimator.c | 21 ++++++-----
net/sched/act_bpf.c | 10 ++++--
net/sched/act_csum.c | 24 ++++++++-----
net/sched/act_gact.c | 10 ++++--
net/sched/act_ife.c | 40 +++++++++++++--------
net/sched/act_ipt.c | 3 ++
net/sched/act_mirred.c | 88 ++++++++++++++++++++++++++++++++--------------
net/sched/act_pedit.c | 40 ++++++++++-----------
net/sched/act_police.c | 9 +++--
net/sched/act_sample.c | 12 +++++--
net/sched/act_simple.c | 6 +++-
net/sched/act_skbmod.c | 14 +++++---
net/sched/act_tunnel_key.c | 26 +++++++-------
net/sched/act_vlan.c | 27 +++++++-------
net/sched/cls_api.c | 1 +
17 files changed, 214 insertions(+), 122 deletions(-)
--
2.7.5
^ permalink raw reply
* Re: [PATCH 0/8] Netfilter updates for net-next
From: David Miller @ 2018-08-10 17:43 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <20180810111622.3981-1-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Fri, 10 Aug 2018 13:16:14 +0200
> The following batch contains netfilter updates for your net-next tree:
>
> 1) Expose NFT_OSF_MAXGENRELEN maximum OS name length from the new OS
> passive fingerprint matching extension, from Fernando Fernandez.
>
> 2) Add extension to support for fine grain conntrack timeout policies
> from nf_tables. As preparation works, this patchset moves
> nf_ct_untimeout() to nf_conntrack_timeout and it also decouples the
> timeout policy from the ctnl_timeout object, most work done by
> Harsha Sharma.
>
> 3) Enable connection tracking when conntrack helper is in place.
>
> 4) Missing enumeration in uapi header when splitting original xt_osf
> to nfnetlink_osf, also from Fernando.
>
> 5) Fix a sparse warning due to incorrect typing in the nf_osf_find(),
> from Wei Yongjun.
>
> You can pull these changes from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git
Pulled, thank you.
^ permalink raw reply
* Re: [PATCH][net-next] packet: switch kvzalloc to allocate memory
From: David Miller @ 2018-08-10 17:43 UTC (permalink / raw)
To: lirongqing; +Cc: netdev
In-Reply-To: <1533895200-31366-1-git-send-email-lirongqing@baidu.com>
From: Li RongQing <lirongqing@baidu.com>
Date: Fri, 10 Aug 2018 18:00:00 +0800
> @@ -4275,7 +4259,7 @@ static int packet_set_ring(struct sock *sk, union tpacket_req_u *req_u,
>
> err = -ENOMEM;
> order = get_order(req->tp_block_size);
> - pg_vec = alloc_pg_vec(req, order);
> + pg_vec = alloc_pg_vec(req);
> if (unlikely(!pg_vec))
> goto out;
> switch (po->tp_version) {
Variable 'order' is now unused.
^ permalink raw reply
* Re: [PATCH v2 0/2] net/sctp: Avoid allocating high order memory with kmalloc()
From: Marcelo Ricardo Leitner @ 2018-08-10 17:41 UTC (permalink / raw)
To: Konstantin Khorenko
Cc: oleg.babin, netdev, linux-sctp, David S . Miller, Vlad Yasevich,
Neil Horman, Xin Long, Andrey Ryabinin
In-Reply-To: <deb7cbdc-55b9-3ff5-8f13-cfa61e08eebe@virtuozzo.com>
On Fri, Aug 10, 2018 at 08:03:51PM +0300, Konstantin Khorenko wrote:
> On 08/09/2018 11:43 AM, Konstantin Khorenko wrote:
> > On 08/04/2018 02:36 AM, Marcelo Ricardo Leitner wrote:
> > > On Fri, Aug 03, 2018 at 07:21:00PM +0300, Konstantin Khorenko wrote:
> > > ...
> > > > Performance results:
> > > > ====================
> > > > * Kernel: v4.18-rc6 - stock and with 2 patches from Oleg (earlier in this thread)
> > > > * Node: CPU (8 cores): Intel(R) Xeon(R) CPU E31230 @ 3.20GHz
> > > > RAM: 32 Gb
> > > >
> > > > * netperf: taken from https://github.com/HewlettPackard/netperf.git,
> > > > compiled from sources with sctp support
> > > > * netperf server and client are run on the same node
> > > > * ip link set lo mtu 1500
> > > >
> > > > The script used to run tests:
> > > > # cat run_tests.sh
> > > > #!/bin/bash
> > > >
> > > > for test in SCTP_STREAM SCTP_STREAM_MANY SCTP_RR SCTP_RR_MANY; do
> > > > echo "TEST: $test";
> > > > for i in `seq 1 3`; do
> > > > echo "Iteration: $i";
> > > > set -x
> > > > netperf -t $test -H localhost -p 22222 -S 200000,200000 -s 200000,200000 \
> > > > -l 60 -- -m 1452;
> > > > set +x
> > > > done
> > > > done
> > > > ================================================
> > > >
> > > > Results (a bit reformatted to be more readable):
> > > ...
> > >
> > > Nice, good numbers.
> > >
> > > I'm missing some test that actually uses more than 1 stream. All tests
> > > in netperf uses only 1 stream. They can use 1 or Many associations on
> > > a socket, but not multiple streams. That means the numbers here show
> > > that we shouldn't see any regression on the more traditional uses, per
> > > Michael's reply on the other email, but it is not testing how it will
> > > behave if we go crazy and use the 64k streams (worst case).
> > >
> > > You'll need some other tool to test it. One idea is sctp_test, from
> > > lksctp-tools. Something like:
> > >
> > > Server side:
> > > ./sctp_test -H 172.0.0.1 -P 22222 -l -d 0
> > > Client side:
> > > time ./sctp_test -H 172.0.0.1 -P 22221 \
> > > -h 172.0.0.1 -p 22222 -s \
> > > -c 1 -M 65535 -T -t 1 -x 100000 -d 0
> > >
> > > And then measure the difference on how long each test took. Can you
> > > get these too?
> > >
> > > Interesting that in my laptop just to start this test for the first
> > > time can took some *seconds*. Seems kernel had a hard time
> > > defragmenting the memory here. :)
>
> Hi Marcelo,
>
> got 3 of 4 results, please take a look, but i failed to measure running
> the test on stock kernel when memory is fragmented, test fails with
> *** connect: Cannot allocate memory ***
Hah, okay.
>
>
> Performance results:
> ====================
> * Kernel: v4.18-rc8 - stock and with 2 patches v3
> * Node: CPU (8 cores): Intel(R) Xeon(R) CPU E31230 @ 3.20GHz
> RAM: 32 Gb
>
> * sctp_test: https://github.com/sctp/lksctp-tools
> * both server and client are run on the same node
> * ip link set lo mtu 1500
> * sysctl -w vm.max_map_count=65530000 (need it to make memory fragmented)
>
> The script used to run tests:
> =============================
> # cat run_sctp_test.sh
> #!/bin/bash
>
> set -x
>
> uname -r
> ip link set lo mtu 1500
> swapoff -a
>
> free
> cat /proc/buddyinfo
>
> ./src/apps/sctp_test -H 127.0.0.1 -P 22222 -l -d 0 &
> sleep 3
>
> time ./src/apps/sctp_test -H 127.0.0.1 -P 22221 -h 127.0.0.1 -p 22222 \
> -s -c 1 -M 65535 -T -t 1 -x 100000 -d 0 1>/dev/null
>
> killall -9 lt-sctp_test
> ===============================
>
> Results (a bit reformatted to be more readable):
>
> 1) ms stock kernel v4.18-rc8, no memory fragmentation
> Info about memory - more or less same to iterations:
> # free
> total used free shared buff/cache available
> Mem: 32906008 213156 32178184 764 514668 32260968
> Swap: 0 0 0
>
> cat /proc/buddyinfo
> Node 0, zone DMA 0 1 1 0 2 1 1 0 1 1 3
> Node 0, zone DMA32 1 3 5 4 2 2 3 6 6 4 867
> Node 0, zone Normal 551 422 160 204 193 34 15 7 22 19 6956
>
> test 1 test 2 test 3
> real 0m14.715s 0m14.593s 0m15.954s
> user 0m0.954s 0m0.955s 0m0.854s
> sys 0m13.388s 0m12.537s 0m13.749s
>
> 2) kernel with fixes, no memory fragmentation
> 'free' and 'buddyinfo' similar to 1)
>
> test 1 test 2 test 3
> real 0m14.959s 0m14.693s 0m14.762s
> user 0m0.948s 0m0.921s 0m0.929s
> sys 0m13.538s 0m13.225s 0m13.217s
>
> 3) kernel with fixes, memory fragmented
> (mmap() all available RAM, touch all pages, munmap() half of pages (each second page), do it again for RAM/2)
> 'free':
> total used free shared buff/cache available
> Mem: 32906008 30555200 302740 764 2048068 266452
> Mem: 32906008 30379948 541436 764 1984624 442376
> Mem: 32906008 30717312 262380 764 1926316 109908
>
> /proc/buddyinfo:
> Node 0, zone Normal 40773 37 34 29 0 0 0 0 0 0 0
> Node 0, zone Normal 100332 68 8 4 2 1 1 0 0 0 0
> Node 0, zone Normal 31113 7 2 1 0 0 0 0 0 0 0
>
> test 1 test 2 test 3
> real 0m14.159s 0m15.252s 0m15.826s
> user 0m0.839s 0m1.004s 0m1.048s
> sys 0m11.827s 0m14.240s 0m14.778s
Nice. Looks like there won't be (noticeable) performance regressions
on where it was functional, and it will help make it functional in
case memory is fragmented. With some overhead, but it at least works.
Thanks for running all theses.
>
>
> --
> Best regards,
>
> Konstantin Khorenko,
> Virtuozzo Linux Kernel Team
^ permalink raw reply
* Re: [Query]: DSA Understanding
From: Florian Fainelli @ 2018-08-10 17:36 UTC (permalink / raw)
To: Lad, Prabhakar, Andrew Lunn; +Cc: netdev
In-Reply-To: <CA+V-a8svEqsDqVr7Z+SWJ1eg2Pz5yJZnhiFtgjUd=OUzOcQOtQ@mail.gmail.com>
On 08/10/2018 04:26 AM, Lad, Prabhakar wrote:
> Hi Andrew,
>
> On Thu, Aug 9, 2018 at 6:23 PM Andrew Lunn <andrew@lunn.ch> wrote:
>>
>>> Its coming from the switch lan4 I have attached the png, where
>>> C4:F3:12:08:FE:7F is
>>> the mac of lan4, which is broadcast to ff:ff:ff:ff:ff:ff, which is
>>> causing rx counter on
>>> PC to go up.
>>
>> So, big packets are making it from the switch to the PC. But the small
>> ARP packets are not.
>>
>> This is what Florian was suggesting.
>>
>> ARP packets are smaller than 64 bytes, which is the minimum packet
>> size for Ethernet. Any packets smaller than 64 bytes are called runt
>> packets. They have to be padded upto 64 bytes in order to make them
>> valid. Otherwise the destination, or any switch along the path, might
>> throw them away.
>>
>> What could be happening is that the CSPW driver or hardware is padding
>> the packet to 64 bytes. But that packet has a DSA header in it. The
>> switch removes the header, recalculate the checksum and sends the
>> packet. It is now either 4 or 8 bytes smaller, depending on what DSA
>> header was used. It then becomes a runt packet.
>>
> Thank you for the clarification, this really helped me out.
>
>> Florian had to fix this problem recently.
>>
>> http://patchwork.ozlabs.org/patch/836534/
>>
> But seems like this patch was never accepted, instead
> brcm_tag_xmit_ll() does it if I am understanding it correctly.
> similarly to this ksz_xmit() is taking care of padding.
net/dsa/tag_brcm.c ended up doing the padding because that was a more
generic and central location:
https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/net/dsa/tag_brcm.c#n73
>
>> You probably need something similar for the cpsw.
>>
> looking at the tag_ksz.c in xmit function this is taken care of
I agree, this should be padding packets correctly, can you still
instrument cpsw to make sure that what comes to its ndo_start_xmit() is
ETH_ZLEN + tag_len or more?
>
> /* For Ingress (Host -> KSZ), 2 bytes are added before FCS.
> * ---------------------------------------------------------------------------
> * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes)
> * ---------------------------------------------------------------------------
> * tag0 : Prioritization (not used now)
> * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x10=port5)
> *
> * For Egress (KSZ -> Host), 1 byte is added before FCS.
> * ---------------------------------------------------------------------------
> * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
> * ---------------------------------------------------------------------------
> * tag0 : zero-based value represents port
> * (eg, 0x00=port1, 0x02=port3, 0x06=port7)
> */
>
> #define KSZ_INGRESS_TAG_LEN 2
> #define KSZ_EGRESS_TAG_LEN 1
>
> static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
> {
> struct dsa_slave_priv *p = netdev_priv(dev);
> struct sk_buff *nskb;
> int padlen;
> u8 *tag;
>
> padlen = (skb->len >= ETH_ZLEN) ? 0 : ETH_ZLEN - skb->len;
>
> if (skb_tailroom(skb) >= padlen + KSZ_INGRESS_TAG_LEN) {
> /* Let dsa_slave_xmit() free skb */
> if (__skb_put_padto(skb, skb->len + padlen, false))
> return NULL;
>
> nskb = skb;
> } else {
> nskb = alloc_skb(NET_IP_ALIGN + skb->len +
> padlen + KSZ_INGRESS_TAG_LEN, GFP_ATOMIC);
> if (!nskb)
> return NULL;
> skb_reserve(nskb, NET_IP_ALIGN);
>
> skb_reset_mac_header(nskb);
> skb_set_network_header(nskb,
> skb_network_header(skb) - skb->head);
> skb_set_transport_header(nskb,
> skb_transport_header(skb) - skb->head);
> skb_copy_and_csum_dev(skb, skb_put(nskb, skb->len));
>
> /* Let skb_put_padto() free nskb, and let dsa_slave_xmit() free
> * skb
> */
> if (skb_put_padto(nskb, nskb->len + padlen))
> return NULL;
>
> consume_skb(skb);
> }
>
> tag = skb_put(nskb, KSZ_INGRESS_TAG_LEN);
> tag[0] = 0;
> tag[1] = 1 << p->dp->index; /* destination port */
>
> return nskb;
> }
>
> Cheers,
> --Prabhakar Lad
>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next] cxgb4: add support to display DCB info
From: David Miller @ 2018-08-10 17:27 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, nirranjan, indranil, dt, leedom
In-Reply-To: <1533892621-3478-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Fri, 10 Aug 2018 14:47:01 +0530
> display Data Center bridging information in debug
> fs.
>
> Signed-off-by: Casey Leedom <leedom@chelsio.com>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
Applied.
^ permalink raw reply
* [PATCH net-next] openvswitch: Derive IP protocol number for IPv6 later frags
From: Yi-Hung Wei @ 2018-08-10 17:19 UTC (permalink / raw)
To: netdev, u9012063; +Cc: Yi-Hung Wei
Currently, OVS only parses the IP protocol number for the first
IPv6 fragment, but sets the IP protocol number for the later fragments
to be NEXTHDF_FRAGMENT. This patch tries to derive the IP protocol
number for the IPV6 later frags so that we can match that.
Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
---
net/openvswitch/flow.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 56b8e7167790..3d654c4f71be 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -297,7 +297,13 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key)
nh_len = payload_ofs - nh_ofs;
skb_set_transport_header(skb, nh_ofs + nh_len);
- key->ip.proto = nexthdr;
+ if (key->ip.frag == OVS_FRAG_TYPE_LATER) {
+ unsigned int offset = 0;
+
+ key->ip.proto = ipv6_find_hdr(skb, &offset, -1, NULL, NULL);
+ } else {
+ key->ip.proto = nexthdr;
+ }
return nh_len;
}
--
2.7.4
^ permalink raw reply related
* [PATCH v3 2/2] net/sctp: Replace in/out stream arrays with flex_array
From: Konstantin Khorenko @ 2018-08-10 17:11 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: oleg.babin, netdev, linux-sctp, David S . Miller, Vlad Yasevich,
Neil Horman, Xin Long, Andrey Ryabinin, Konstantin Khorenko
In-Reply-To: <20180810171143.21592-1-khorenko@virtuozzo.com>
This path replaces physically contiguous memory arrays
allocated using kmalloc_array() with flexible arrays.
This enables to avoid memory allocation failures on the
systems under a memory stress.
Signed-off-by: Oleg Babin <obabin@virtuozzo.com>
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
include/net/sctp/structs.h | 9 ++---
net/sctp/stream.c | 88 ++++++++++++++++++++++++++++++++++------------
2 files changed, 71 insertions(+), 26 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index ce4bf844f573..f922db8029e6 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -57,6 +57,7 @@
#include <linux/atomic.h> /* This gets us atomic counters. */
#include <linux/skbuff.h> /* We need sk_buff_head. */
#include <linux/workqueue.h> /* We need tq_struct. */
+#include <linux/flex_array.h> /* We need flex_array. */
#include <linux/sctp.h> /* We need sctp* header structs. */
#include <net/sctp/auth.h> /* We need auth specific structs */
#include <net/ip.h> /* For inet_skb_parm */
@@ -1431,8 +1432,8 @@ struct sctp_stream_in {
};
struct sctp_stream {
- struct sctp_stream_out *out;
- struct sctp_stream_in *in;
+ struct flex_array *out;
+ struct flex_array *in;
__u16 outcnt;
__u16 incnt;
/* Current stream being sent, if any */
@@ -1458,14 +1459,14 @@ static inline struct sctp_stream_out *sctp_stream_out(
const struct sctp_stream *stream,
__u16 sid)
{
- return ((struct sctp_stream_out *)(stream->out)) + sid;
+ return flex_array_get(stream->out, sid);
}
static inline struct sctp_stream_in *sctp_stream_in(
const struct sctp_stream *stream,
__u16 sid)
{
- return ((struct sctp_stream_in *)(stream->in)) + sid;
+ return flex_array_get(stream->in, sid);
}
#define SCTP_SO(s, i) sctp_stream_out((s), (i))
diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index 7ca6fe4e7882..ffb940d3b57c 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -37,6 +37,53 @@
#include <net/sctp/sm.h>
#include <net/sctp/stream_sched.h>
+static struct flex_array *fa_alloc(size_t elem_size, size_t elem_count,
+ gfp_t gfp)
+{
+ struct flex_array *result;
+ int err;
+
+ result = flex_array_alloc(elem_size, elem_count, gfp);
+ if (result) {
+ err = flex_array_prealloc(result, 0, elem_count, gfp);
+ if (err) {
+ flex_array_free(result);
+ result = NULL;
+ }
+ }
+
+ return result;
+}
+
+static void fa_free(struct flex_array *fa)
+{
+ if (fa)
+ flex_array_free(fa);
+}
+
+static void fa_copy(struct flex_array *fa, struct flex_array *from,
+ size_t index, size_t count)
+{
+ void *elem;
+
+ while (count--) {
+ elem = flex_array_get(from, index);
+ flex_array_put(fa, index, elem, 0);
+ index++;
+ }
+}
+
+static void fa_zero(struct flex_array *fa, size_t index, size_t count)
+{
+ void *elem;
+
+ while (count--) {
+ elem = flex_array_get(fa, index);
+ memset(elem, 0, fa->element_size);
+ index++;
+ }
+}
+
/* Migrates chunks from stream queues to new stream queues if needed,
* but not across associations. Also, removes those chunks to streams
* higher than the new max.
@@ -78,34 +125,33 @@ static void sctp_stream_outq_migrate(struct sctp_stream *stream,
* sctp_stream_update will swap ->out pointers.
*/
for (i = 0; i < outcnt; i++) {
- kfree(new->out[i].ext);
- new->out[i].ext = stream->out[i].ext;
- stream->out[i].ext = NULL;
+ kfree(SCTP_SO(new, i)->ext);
+ SCTP_SO(new, i)->ext = SCTP_SO(stream, i)->ext;
+ SCTP_SO(stream, i)->ext = NULL;
}
}
for (i = outcnt; i < stream->outcnt; i++)
- kfree(stream->out[i].ext);
+ kfree(SCTP_SO(stream, i)->ext);
}
static int sctp_stream_alloc_out(struct sctp_stream *stream, __u16 outcnt,
gfp_t gfp)
{
- struct sctp_stream_out *out;
+ struct flex_array *out;
+ size_t elem_size = sizeof(struct sctp_stream_out);
- out = kmalloc_array(outcnt, sizeof(*out), gfp);
+ out = fa_alloc(elem_size, outcnt, gfp);
if (!out)
return -ENOMEM;
if (stream->out) {
- memcpy(out, stream->out, min(outcnt, stream->outcnt) *
- sizeof(*out));
- kfree(stream->out);
+ fa_copy(out, stream->out, 0, min(outcnt, stream->outcnt));
+ fa_free(stream->out);
}
if (outcnt > stream->outcnt)
- memset(out + stream->outcnt, 0,
- (outcnt - stream->outcnt) * sizeof(*out));
+ fa_zero(out, stream->outcnt, (outcnt - stream->outcnt));
stream->out = out;
@@ -115,22 +161,20 @@ static int sctp_stream_alloc_out(struct sctp_stream *stream, __u16 outcnt,
static int sctp_stream_alloc_in(struct sctp_stream *stream, __u16 incnt,
gfp_t gfp)
{
- struct sctp_stream_in *in;
-
- in = kmalloc_array(incnt, sizeof(*stream->in), gfp);
+ struct flex_array *in;
+ size_t elem_size = sizeof(struct sctp_stream_in);
+ in = fa_alloc(elem_size, incnt, gfp);
if (!in)
return -ENOMEM;
if (stream->in) {
- memcpy(in, stream->in, min(incnt, stream->incnt) *
- sizeof(*in));
- kfree(stream->in);
+ fa_copy(in, stream->in, 0, min(incnt, stream->incnt));
+ fa_free(stream->in);
}
if (incnt > stream->incnt)
- memset(in + stream->incnt, 0,
- (incnt - stream->incnt) * sizeof(*in));
+ fa_zero(in, stream->incnt, (incnt - stream->incnt));
stream->in = in;
@@ -174,7 +218,7 @@ int sctp_stream_init(struct sctp_stream *stream, __u16 outcnt, __u16 incnt,
ret = sctp_stream_alloc_in(stream, incnt, gfp);
if (ret) {
sched->free(stream);
- kfree(stream->out);
+ fa_free(stream->out);
stream->out = NULL;
stream->outcnt = 0;
goto out;
@@ -206,8 +250,8 @@ void sctp_stream_free(struct sctp_stream *stream)
sched->free(stream);
for (i = 0; i < stream->outcnt; i++)
kfree(SCTP_SO(stream, i)->ext);
- kfree(stream->out);
- kfree(stream->in);
+ fa_free(stream->out);
+ fa_free(stream->in);
}
void sctp_stream_clear(struct sctp_stream *stream)
--
2.15.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox