Message-ID: <b4b5e1e7-ebef-5d20-67b6-a3324e886942@mojatatu.com>
Date:   Mon, 20 May 2019 12:29:23 -0400
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Edward Cree <ecree@...arflare.com>, Jiri Pirko <jiri@...nulli.us>,
        Pablo Neira Ayuso <pablo@...filter.org>,
        David Miller <davem@...emloft.net>
Cc:     netdev <netdev@...r.kernel.org>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Andy Gospodarek <andy@...yhouse.net>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Michael Chan <michael.chan@...adcom.com>,
        Vishal Kulkarni <vishal@...lsio.com>
Subject: Re: [RFC PATCH v2 net-next 0/3] flow_offload: Re-add per-action
 statistics

On 2019-05-20 12:10 p.m., Edward Cree wrote:
> On 20/05/2019 16:38, Jamal Hadi Salim wrote:
>> That is fine then if i could do:
>>
>> tc actions add action drop index 104
>> then
>> followed by for example the two filters you show below..
> That seems to work.

nice.


>> Is your hardware not using explicit indices into a stats table?
> No; we ask the HW to allocate a counter and it returns us a counter ID (which
>   bears no relation to the action index).  So I have an rhashtable keyed on
>   the cookie (or on the action-type & action_index, when using the other
>   version of my patches) which stores the HW counter ID; and the entry in that
>   hashtable is what I attach to the driver's action struct.
> 
>> Beauty.  Assuming the stats are being synced to the kernel?
>> Test 1:
>> What does "tc -s actions ls action drop index 104" show?
> It produces no output, but
>      `tc -s actions get action drop index 104`
> or
>      `tc -s actions list action gact index 104`

sorry meant that. Hopefully it shows accumulated stats from both
filter hits and correct ref counts and bind counts.

> shows the same stats as `tc -s filter show ...` did for that action.
>> Test 2:
>> Delete one of the filters above then dump actions again as above.
> Ok, that's weird: after I delete one, the other (in `tc -s filter show ...`)
>   no longer shows the shared action.
> 

Sounds weird.

> # tc filter del dev $vfrep parent ffff: pref 49151
> # tc -stats filter show dev $vfrep parent ffff:
> filter protocol arp pref 49152 flower chain 0
> filter protocol arp pref 49152 flower chain 0 handle 0x1
>    eth_type arp
>    skip_sw
>    in_hw in_hw_count 1
>          action order 1: vlan  push id 100 protocol 802.1Q priority 0 pipe
>           index 1 ref 1 bind 1 installed 180 sec used 180 sec
>          Action statistics:
>          Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>          backlog 0b 0p requeues 0
> 
>          action order 2: mirred (Egress Mirror to device $pf) pipe
>          index 101 ref 1 bind 1 installed 180 sec used 169 sec
>          Action statistics:
>          Sent 256 bytes 4 pkt (dropped 0, overlimits 0 requeues 0)
>          Sent software 0 bytes 0 pkt
>          Sent hardware 256 bytes 4 pkt
>          backlog 0b 0p requeues 0
> 
>          action order 3: vlan  pop pipe
>           index 2 ref 1 bind 1 installed 180 sec used 180 sec
>          Action statistics:
>          Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>          backlog 0b 0p requeues 0
> 
> #
> 

Hold on, did you intentionaly add that "protocol arp" there?
Or is that just a small development time quark?

> Yet `tc -s actions get` still shows it...
> 
> # tc -s actions get action drop index 104
> total acts 0
> 
>          action order 1: gact action drop
>           random type none pass val 0
>           index 104 ref 2 bind 1 installed 812 sec used 797 sec
>          Action statistics:
>          Sent 534 bytes 7 pkt (dropped 7, overlimits 0 requeues 0)
>          Sent software 0 bytes 0 pkt
>          Sent hardware 534 bytes 7 pkt
>          backlog 0b 0p requeues 0

Assuming you first created the action then bound it, the action
output looks correct. "ref 2" implies two refcounts, one by
the first action creation and the second by the filter binding.
Also "bind 1" implies only one filter is referencing it.
Before you deleted it should have been "ref 3" and "bind 2".

> # tc filter show dev $vfrep parent ffff:
> filter protocol arp pref 49152 flower chain 0
> filter protocol arp pref 49152 flower chain 0 handle 0x1
>    eth_type arp
>    skip_sw
>    in_hw in_hw_count 1
>          action order 1: vlan  push id 100 protocol 802.1Q priority 0 pipe
>           index 1 ref 1 bind 1
> 
>          action order 3: vlan  pop pipe
>           index 2 ref 1 bind 1

I see the arp listing again.
Maybe just a bug.

> # tc -s actions get action mirred index 101
> total acts 0
> 
>          action order 1: mirred (Egress Mirror to device $pf) pipe
>          index 101 ref 1 bind 1 installed 796 sec used 785 sec
>          Action statistics:
>          Sent 256 bytes 4 pkt (dropped 0, overlimits 0 requeues 0)
>          Sent software 0 bytes 0 pkt
>          Sent hardware 256 bytes 4 pkt
>          backlog 0b 0p requeues 0
> #

Assuming in this case you added by value the actions? Bind and ref
are 1 each.
Maybe dump after each step and it will be easier to see what is
happening.

> Curiouser and curiouser... it seems that after I delete one of the rules,
>   TC starts to get very confused and actions start disappearing from rule
>   dumps.  Yet those actions still exist according to `tc actions list`.
> I don't *think* my changes can have caused this, but I'll try a test on a
>   vanilla kernel just to make sure the same thing happens there.
> 

Possible an offload bug that was already in existence. Can you try the
same steps but without offloading and see if you see the same behavior?

cheers,
jamal

Powered by blists - more mailing lists