From: Davide Caratti <dcara...@redhat.com>
Date: Wed, 13 Dec 2017 10:48:38 +0100
> Then, in the data path, use READ_ONCE() to
> read those values, to avoid lock contention among multiple readers.
...
> @@ -544,14 +543,12 @@ static int tcf_csum(struct sk_buff *skb, const struct
> tc_action *a,
>
> tcf_lastuse_update(&p->tcf_tm);
> bstats_cpu_update(this_cpu_ptr(p->common.cpu_bstats), skb);
> - spin_lock(&p->tcf_lock);
> - action = p->tcf_action;
> - update_flags = p->update_flags;
> - spin_unlock(&p->tcf_lock);
>
> + action = READ_ONCE(p->tcf_action);
> if (unlikely(action == TC_ACT_SHOT))
> goto drop;
>
> + update_flags = READ_ONCE(p->update_flags);
> switch (tc_skb_protocol(skb)) {
> case cpu_to_be16(ETH_P_IP):
> if (!tcf_csum_ipv4(skb, update_flags))
That's not why the lock is here.
We must read both action and flags atomically so that they are consistent
with eachother.
We must never use action from one configuration change and flags from
yet another.
Find a way to load both of these values with a single cpu load, then you
can legally remove the lock.
