netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vlad Buslov <vladbu@mellanox.com>
To: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Vlad Buslov <vladbu@mellanox.com>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	Edward Cree <ecree@solarflare.com>, Jiri Pirko <jiri@resnulli.us>,
	Pablo Neira Ayuso <pablo@netfilter.org>,
	David Miller <davem@davemloft.net>,
	netdev <netdev@vger.kernel.org>,
	Andy Gospodarek <andy@greyhouse.net>,
	Jakub Kicinski <jakub.kicinski@netronome.com>,
	Michael Chan <michael.chan@broadcom.com>,
	Vishal Kulkarni <vishal@chelsio.com>,
	Lucas Bates <lucasb@mojatatu.com>
Subject: Re: [RFC PATCH v2 net-next 0/3] flow_offload: Re-add per-action statistics
Date: Wed, 22 May 2019 17:49:53 +0000	[thread overview]
Message-ID: <vbfa7fee5vm.fsf@mellanox.com> (raw)
In-Reply-To: <b147865f-5224-b66b-2824-8c1c8986900f@mojatatu.com>


On Wed 22 May 2019 at 20:24, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 2019-05-22 11:08 a.m., Vlad Buslov wrote:
>>
>> On Tue 21 May 2019 at 16:23, Vlad Buslov <vladbu@mellanox.com> wrote:
>
>>
>> It seems that culprit in this case is tc_action->order field. It is used
>> as nla attrtype when dumping actions. Initially it is set according to
>> ordering of actions of filter that creates them. However, it is
>> overwritten in tca_action_gd() each time action is dumped through action
>> API (according to action position in tb array) and when new filter is
>> attached to shared action (according to action order on the filter).
>> With this we have another way to reproduce the bug:
>>
>> sudo tc qdisc add dev lo ingress
>>
>> #shared action is the first action (order 1)
>> sudo tc filter add dev lo parent ffff: protocol ip prio 8 u32 \
>> match ip dst 127.0.0.8/32 flowid 1:10 \
>> action drop index 104 \
>> action vlan push id 100 protocol 802.1q
>>
>> #shared action is the the second action (order 2)
>> sudo tc filter add dev lo parent ffff: protocol ip prio 8 u32 \
>> match ip dst 127.0.0.10/32 flowid 1:10 \
>> action vlan push id 101 protocol 802.1q \
>> action drop index 104
>>
>> # Now action is only visible on one filter
>> sudo tc -s filter ls dev lo parent ffff: protocol ip
>>
>
> Ok, thanks for chasing this. A test case i had in mind is to
> maybe have 3 actions. Add the drop in the middle for one
> and at the begging for another and see if they are visible
> with the patch.
> If you dont have time I can test maybe AM tommorow.

Works with my patch:

~$ sudo tc qdisc del dev lo ingress
~$ sudo tc qdisc add dev lo ingress
~$ sudo tc filter add dev lo parent ffff: protocol ip prio 8 u32 \
> match ip dst 127.0.0.8/32 flowid 1:10 \
> action drop index 104 \
> mirred egress redirect dev ens1f0 \
> action vlan push id 100 protocol 802.1q
~$ sudo tc filter add dev lo parent ffff: protocol ip prio 8 u32 \
> match ip dst 127.0.0.10/32 flowid 1:10 \
> action vlan push id 101 protocol 802.1q \
> action drop index 104 \
> mirred egress redirect dev ens1f0
~$ sudo tc -s filter ls dev lo parent ffff: protocol ip
filter pref 8 u32 chain 0
filter pref 8 u32 chain 0 fh 800: ht divisor 1
filter pref 8 u32 chain 0 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:10 not_in_hw  (rule hit 0 success 0)
  match 7f000008/ffffffff at 16 (success 0 )
        action order 1: gact action drop
         random type none pass val 0
         index 104 ref 2 bind 2 installed 8 sec used 8 sec
        Action statistics:
        Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0

        action order 2: mirred (Egress Redirect to device ens1f0) stolen
        index 1 ref 1 bind 1 installed 8 sec used 8 sec
        Action statistics:
        Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0

        action order 3: vlan  push id 100 protocol 802.1Q priority 0 pipe
         index 1 ref 1 bind 1 installed 8 sec used 8 sec
        Action statistics:
        Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0

filter pref 8 u32 chain 0 fh 800::801 order 2049 key ht 800 bkt 0 flowid 1:10 not_in_hw  (rule hit 0 success 0)
  match 7f00000a/ffffffff at 16 (success 0 )
        action order 1: vlan  push id 101 protocol 802.1Q priority 0 pipe
         index 2 ref 1 bind 1 installed 4 sec used 4 sec
        Action statistics:
        Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0

        action order 2: gact action drop
         random type none pass val 0
         index 104 ref 2 bind 2 installed 8 sec used 8 sec
        Action statistics:
        Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0

        action order 3: mirred (Egress Redirect to device ens1f0) stolen
        index 2 ref 1 bind 1 installed 4 sec used 4 sec
        Action statistics:
        Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0


  reply	other threads:[~2019-05-22 17:50 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-15 19:39 [RFC PATCH v2 net-next 0/3] flow_offload: Re-add per-action statistics Edward Cree
2019-05-15 19:42 ` [RFC PATCH v2 net-next 1/3] flow_offload: copy tcfa_index into flow_action_entry Edward Cree
2019-05-18 20:30   ` Jamal Hadi Salim
2019-05-15 19:42 ` [RFC PATCH v2 net-next 2/3] flow_offload: restore ability to collect separate stats per action Edward Cree
2019-05-18 20:35   ` Jamal Hadi Salim
2019-05-20 15:46     ` Edward Cree
2019-05-20 15:55       ` Jamal Hadi Salim
2019-05-15 19:42 ` [RFC PATCH v2 net-next 3/3] flow_offload: include linux/kernel.h from flow_offload.h Edward Cree
2019-05-17 15:27 ` [RFC PATCH v2 net-next 0/3] flow_offload: Re-add per-action statistics Edward Cree
2019-05-17 17:14   ` Edward Cree
2019-05-18 20:39     ` Jamal Hadi Salim
2019-05-20 15:26       ` Edward Cree
2019-05-20 15:38         ` Jamal Hadi Salim
2019-05-20 16:10           ` Edward Cree
2019-05-20 16:29             ` Jamal Hadi Salim
2019-05-20 16:32               ` Jamal Hadi Salim
2019-05-20 17:58               ` Edward Cree
2019-05-20 18:36               ` Edward Cree
2019-05-20 21:12                 ` Jamal Hadi Salim
2019-05-21 12:46                   ` Jamal Hadi Salim
2019-05-21 13:23                     ` Vlad Buslov
2019-05-22 15:08                       ` Vlad Buslov
2019-05-22 15:13                         ` [RFC PATCH net-next] net: sched: don't use tc_action->order during action dump Vlad Buslov
2019-05-22 17:24                         ` [RFC PATCH v2 net-next 0/3] flow_offload: Re-add per-action statistics Jamal Hadi Salim
2019-05-22 17:49                           ` Vlad Buslov [this message]
2019-05-22 18:23                             ` Jamal Hadi Salim
2019-05-22 18:26                               ` Jamal Hadi Salim
2019-05-19  0:22   ` Pablo Neira Ayuso
2019-05-20 15:37     ` Edward Cree
2019-05-20 15:40       ` Jamal Hadi Salim
2019-05-20 15:44       ` Pablo Neira Ayuso
2019-05-18 20:30 ` Jamal Hadi Salim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=vbfa7fee5vm.fsf@mellanox.com \
    --to=vladbu@mellanox.com \
    --cc=andy@greyhouse.net \
    --cc=davem@davemloft.net \
    --cc=ecree@solarflare.com \
    --cc=jakub.kicinski@netronome.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=lucasb@mojatatu.com \
    --cc=michael.chan@broadcom.com \
    --cc=netdev@vger.kernel.org \
    --cc=pablo@netfilter.org \
    --cc=vishal@chelsio.com \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).