| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 10 Sep 2014 17:56:44 -0700
From: Eric Dumazet <eric.dumazet@...il.com>
To: John Fastabend <john.fastabend@...il.com>
Cc: xiyou.wangcong@...il.com, davem@...emloft.net, jhs@...atatu.com,
netdev@...r.kernel.org, paulmck@...ux.vnet.ibm.com,
brouer@...hat.com
Subject: Re: [net-next PATCH v4 02/16] net: rcu-ify tcf_proto
On Wed, 2014-09-10 at 08:47 -0700, John Fastabend wrote:
> rcu'ify tcf_proto this allows calling tc_classify() without holding
> any locks. Updaters are protected by RTNL.
>
> This patch prepares the core net_sched infrastracture for running
> the classifier/action chains without holding the qdisc lock however
> it does nothing to ensure cls_xxx and act_xxx types also work without
> locking. Additional patches are required to address the fall out.
>
> Signed-off-by: John Fastabend <john.r.fastabend@...el.com>
> ---
...
> diff --git a/net/sched/sch_choke.c b/net/sched/sch_choke.c
> index ed30e43..4b52b70 100644
> --- a/net/sched/sch_choke.c
> +++ b/net/sched/sch_choke.c
> @@ -57,7 +57,7 @@ struct choke_sched_data {
>
> /* Variables */
> struct red_vars vars;
> - struct tcf_proto *filter_list;
> + struct tcf_proto __rcu *filter_list;
> struct {
> u32 prob_drop; /* Early probability drops */
> u32 prob_mark; /* Early probability marks */
> @@ -193,9 +193,11 @@ static bool choke_classify(struct sk_buff *skb,
> {
> struct choke_sched_data *q = qdisc_priv(sch);
> struct tcf_result res;
> + struct tcf_proto *fl;
> int result;
>
> - result = tc_classify(skb, q->filter_list, &res);
> + fl = rcu_dereference_bh(q->filter_list);
Hmm... please change the caller to pass fl.
Idea is to read q->filter_list once.
> + result = tc_classify(skb, fl, &res);
> if (result >= 0) {
> #ifdef CONFIG_NET_CLS_ACT
> switch (result) {
> @@ -244,12 +246,14 @@ static bool choke_match_random(const struct choke_sched_data *q,
> unsigned int *pidx)
> {
> struct sk_buff *oskb;
> + struct tcf_proto *fl;
>
> if (q->head == q->tail)
> return false;
>
> oskb = choke_peek_random(q, pidx);
> - if (q->filter_list)
> + fl = rcu_dereference_bh(q->filter_list);
You could use rcu_access_pointer() and not have this fl variable.
> + if (fl)
> return choke_get_classid(nskb) == choke_get_classid(oskb);
>
> return choke_match_flow(oskb, nskb);
> @@ -259,9 +263,11 @@ static int choke_enqueue(struct sk_buff *skb, struct Qdisc *sch)
> {
> struct choke_sched_data *q = qdisc_priv(sch);
> const struct red_parms *p = &q->parms;
> + struct tcf_proto *fl;
> int ret = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
>
> - if (q->filter_list) {
> + fl = rcu_dereference_bh(q->filter_list);
> + if (fl) {
> /* If using external classifiers, get result and record it. */
> if (!choke_classify(skb, sch, &ret))
Here I think you should pass fl as an additional parameter to
choke_classify()
OR, just use rcu_access_pointer() here as you do not deref
q->filter_list here.
> goto other_drop; /* Packet was eaten by filter */
> @@ -554,7 +560,8 @@ static unsigned long choke_bind(struct Qdisc *sch, unsigned long parent,
> return 0;
> }
>
> -static struct tcf_proto **choke_find_tcf(struct Qdisc *sch, unsigned long cl)
> +static struct tcf_proto __rcu **choke_find_tcf(struct Qdisc *sch,
> + unsigned long cl)
> {
> struct choke_sched_data *q = qdisc_priv(sch);
>
remaining part seems fine.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists