Re: [patch net-next 5/8] Introduce sample tc action

On 16-11-10 11:58 AM, Yotam Gigi wrote:

quoted

-----Original Message-----
From: John Fastabend [mailto:john.fastabend@gmail.com]
Sent: Thursday, November 10, 2016 9:38 PM
To: Jiri Pirko <jiri@resnulli.us>; netdev@vger.kernel.org
Cc: davem@davemloft.net; Yotam Gigi <redacted>; Ido Schimmel
[off-list ref]; Elad Raz [off-list ref]; Nogah Frankel
[off-list ref]; Or Gerlitz [off-list ref];
jhs@mojatatu.com; geert+renesas@glider.be; stephen@networkplumber.org;
xiyou.wangcong@gmail.com; linux@roeck-us.net; roopa@cumulusnetworks.com
Subject: Re: [patch net-next 5/8] Introduce sample tc action

On 16-11-10 11:35 AM, John Fastabend wrote:

quoted

On 16-11-10 03:23 AM, Jiri Pirko wrote:

quoted

From: Yotam Gigi <redacted>

This action allow the user to sample traffic matched by tc classifier.
The sampling consists of choosing packets randomly, truncating them,
adding some informative metadata regarding the interface and the original
packet size and mark them with specific mark, to allow further tc rules to
match and process. The marked sample packets are then injected into the
device ingress qdisc using netif_receive_skb.

The packets metadata is packed using the ife encapsulation protocol, and
the outer packet's ethernet dest, source and eth_type, along with the
rate, mark and the optional truncation size can be configured from
userspace.

Example:
To sample ingress traffic from interface eth1, and redirect the sampled
the sampled packets to interface dummy0, one may use the commands:

tc qdisc add dev eth1 handle ffff: ingress

tc filter add dev eth1 parent ffff: \
	   matchall action sample rate 12 mark 17

tc filter add parent ffff: dev eth1 protocol all \
	   u32 match mark 17 0xff \
	   action mirred egress redirect dev dummy0

Where the first command adds an ingress qdisc and the second starts
sampling every 12'th packet on dev eth1 and marks the sampled packets with
17. The third command catches the sampled packets, which are marked with
17, and redirects them to dev dummy0.

The sampling algorithm was not randomized based on the above commit
log? It really needs to be for all the reasons Roopa mentioned earlier.
Did I miss some email on why it didn't get implemented?

Also there was an indication the already is actually implemented
correctly so don't we need the hw/sw to behave the same. The whole
argument about sw/hw parity, etc.

sorry bit of a typo there corrected 2nd paragraph here...

Also there was an indication the hardware is already implemented \
correctly so don't we need the hw/sw to behave the same. The argument
about sw/hw parity, etc.

Our hardware currently does not support sampling with random behavior, so 
we did implement it in software too. 

But, the API is extensible and it is possible to add a random keyword to 
the tc action to allow random sampling. In that case, the keyword will be
implemented in sw only and our driver will fail offloading it.

For many use cases this will be limiting but OK maybe this is good
enough for something and we can add a flag/attribute to support random
sampling. Works for me.