netdev - Re: [PATCH v2 net-next 0/7] net_sched: act: lockless operation

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <559A7099.9000506@mojatatu.com>
Date:	Mon, 06 Jul 2015 08:12:09 -0400
From:	Jamal Hadi Salim <jhs@...atatu.com>
To:	Eric Dumazet <edumazet@...gle.com>,
	"David S. Miller" <davem@...emloft.net>
CC:	netdev <netdev@...r.kernel.org>,
	Alexei Starovoitov <ast@...mgrid.com>,
	John Fastabend <john.fastabend@...il.com>,
	Eric Dumazet <edumazet@...il.com>
Subject: Re: [PATCH v2 net-next 0/7] net_sched: act: lockless operation

On 07/06/15 02:41, Eric Dumazet wrote:
> As mentioned by Alexei last week in Budapest, it is a bit weird
> to take a spinlock in order to drop a packet in a tc filter...
>
> Lets add percpu infra for tc actions and use it for gact & mirred.
>
> Before changes, my host with 8 RX queues was handling 5 Mpps with gact,
> and more than 11 Mpps after.
>
> Mirred change is not yet visible if ifb+qdisc is used, as ifb is
> not yet multi queue enabled, but is a step forward.
>

on mirred:
The main thing that is read-write is the stats. lastuse timestamp
as well, but that one doesnt have to be accurate (used mainly
to help decide when to flush out policies). so the lock around
all that code is not necessary.

BTW: I was wondering based on what you said earlier on false sharing
if tcf_common can be re-arranged to avoid that?

For mirred, you may have to add for overlimit stat in the common
structure (we should really increment drop stats when we return
TC_ACT_SHOT).

Having said that: we want to maintain the fact that an action
instance (based on index) can be used across different policies.
i dont see any issue with the way you are proceeding for that
to continue to work. i.e i can say something like:
- create an action to redirect or mirror to port blah index 10
- create filter one on eth1 with 1 of the actions being mirred index 10
- create filter two on eth0 with 1 of the actions being mirred index 10

On IFB:
we use tasklets originally to avoid re-ordering. I think tasklets would
still be useful to keep, with each tied to a queue?

cheers,
jamal
--
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