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>, Jiri Pirko <jiri@resnulli.us>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"xiyou.wangcong@gmail.com" <xiyou.wangcong@gmail.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"mleitner@redhat.com" <mleitner@redhat.com>,
	"dcaratti@redhat.com" <dcaratti@redhat.com>,
	Eric Dumazet <edumazet@google.com>
Subject: Re: [PATCH net-next 00/13] Control action percpu counters allocation by netlink flag
Date: Fri, 25 Oct 2019 16:08:36 +0000	[thread overview]
Message-ID: <vbfr230u7su.fsf@mellanox.com> (raw)
In-Reply-To: <7488b589-4e34-d94e-e8e1-aa8ab773891e@mojatatu.com>


On Fri 25 Oct 2019 at 18:43, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 2019-10-25 11:18 a.m., Vlad Buslov wrote:
>>
>
>> The problem with this approach is that it only works when actions are
>> created through act API, and not when they are created together with
>> filter by cls API which doesn't expect or parse TCA_ROOT. That is why I
>> wanted to have something in tcf_action_init_1() which is called by both
>> of them.
>>
>
> Aha. So the call path for tcf_action_init_1() via cls_api also needs
> to have this infra. I think i understand better what you wanted
> to do earlier with changing those enums.
>
> This is fixable - we just need to have the classifier side also take a
> new action root flags bitfield (I can send a sample). To your original

Trying to add this infra to cls API leads back to my original question:
how do I do it in backward compatible manner? I assume that we can't
break users of RTM_NEWTFILTER.

> comment, it is ugly. But maybe "fixing it" is pushing the boundaries
> and we should just go on and let your original approach in.
> My only worry is, given this is uapi, if we go back to your original
> idea we continue to propagate the bad design and we cant take it back
> (all the tooling etc would be cast for the next 5 years even if we
> did fix it in a month). Thoughts?

I don't see anything particularly ugly with extending either
action-specific attributes or TCA_ACT_* attributes, so its hard for me
to reason about problems with current approach :)

>
> cheers,
> jamal

      parent reply	other threads:[~2019-10-25 16:08 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-22 14:17 [PATCH net-next 00/13] Control action percpu counters allocation by netlink flag Vlad Buslov
2019-10-22 14:17 ` [PATCH net-next 01/13] net: sched: extract common action counters update code into function Vlad Buslov
2019-10-22 14:17 ` [PATCH net-next 02/13] net: sched: extract bstats " Vlad Buslov
2019-10-22 14:17 ` [PATCH net-next 03/13] net: sched: extract qstats update code into functions Vlad Buslov
2019-10-22 14:17 ` [PATCH net-next 04/13] net: sched: don't expose action qstats to skb_tc_reinsert() Vlad Buslov
2019-10-22 14:17 ` [PATCH net-next 05/13] net: sched: modify stats helper functions to support regular stats Vlad Buslov
2019-10-22 14:17 ` [PATCH net-next 06/13] net: sched: add action fast initialization flag Vlad Buslov
2019-10-22 14:17 ` [PATCH net-next 07/13] net: sched: act_gact: support fast init flag to skip pcpu allocation Vlad Buslov
2019-10-22 14:17 ` [PATCH net-next 08/13] net: sched: act_csum: " Vlad Buslov
2019-10-22 14:18 ` [PATCH net-next 09/13] net: sched: act_mirred: support fast init flag to skip pcpu alloc Vlad Buslov
2019-10-22 14:18 ` [PATCH net-next 10/13] net: sched: tunnel_key: " Vlad Buslov
2019-10-22 14:18 ` [PATCH net-next 11/13] net: sched: act_vlan: support fast init flag to skip pcpu allocation Vlad Buslov
2019-10-22 14:18 ` [PATCH net-next 12/13] net: sched: act_ct: " Vlad Buslov
2019-10-22 14:18 ` [PATCH net-next 13/13] tc-testing: implement tests for new fast_init action flag Vlad Buslov
2019-10-22 14:28 ` [PATCH iproute2-next] tc: implement support for action flags Vlad Buslov
2019-10-22 14:35 ` [PATCH net-next 00/13] Control action percpu counters allocation by netlink flag Marcelo Ricardo Leitner
2019-10-22 14:52   ` Vlad Buslov
2019-10-22 18:17     ` Roman Mashak
2019-10-23  6:38       ` Vlad Buslov
2019-10-23 13:02         ` Jamal Hadi Salim
2019-10-23 13:08           ` Vlad Buslov
2019-10-22 15:15 ` Marcelo Ricardo Leitner
2019-10-22 15:52   ` Vlad Buslov
2019-10-22 17:09     ` Marcelo Ricardo Leitner
2019-10-23  6:42       ` Vlad Buslov
2019-10-24 15:12       ` Roopa Prabhu
2019-10-24 15:18         ` Vlad Buslov
2019-10-24 17:31           ` Marcelo Ricardo Leitner
2019-10-24 18:03             ` Vlad Buslov
2019-10-23 12:49 ` Jamal Hadi Salim
2019-10-23 13:04   ` Vlad Buslov
2019-10-23 14:21     ` Jamal Hadi Salim
2019-10-23 15:04       ` Vlad Buslov
2019-10-24  7:35       ` Jiri Pirko
2019-10-24 15:23         ` Vlad Buslov
2019-10-24 16:12           ` Jamal Hadi Salim
2019-10-24 16:44             ` Vlad Buslov
2019-10-24 17:17               ` Jamal Hadi Salim
2019-10-24 18:05                 ` Vlad Buslov
2019-10-25 11:46                   ` Jamal Hadi Salim
2019-10-25 11:55                     ` Vlad Buslov
2019-10-25 14:26             ` Vlad Buslov
2019-10-25 14:57               ` Jamal Hadi Salim
2019-10-25 15:08                 ` Jamal Hadi Salim
2019-10-25 15:18                   ` Vlad Buslov
2019-10-25 15:43                     ` Jamal Hadi Salim
2019-10-25 16:06                       ` Jamal Hadi Salim
2019-10-25 16:10                         ` Vlad Buslov
2019-10-25 16:29                           ` Jamal Hadi Salim
2019-10-25 16:53                             ` Vlad Buslov
2019-10-25 18:17                               ` Jamal Hadi Salim
2019-10-25 21:05                                 ` Jamal Hadi Salim
2019-10-25 21:10                                   ` Jamal Hadi Salim
2019-10-25 21:52                                     ` Jamal Hadi Salim
2019-10-26  9:44                                       ` Vlad Buslov
2019-10-26 12:26                                         ` Jamal Hadi Salim
2019-10-26 14:52                                           ` Roman Mashak
2019-10-26 16:06                                             ` Jamal Hadi Salim
2019-10-26 16:42                                               ` Vlad Buslov
2019-10-26 18:38                                                 ` Jamal Hadi Salim
2019-10-27  8:31                                                   ` Vlad Buslov
2019-10-27  9:13                                                     ` Jamal Hadi Salim
2019-10-25 16:27                         ` Vlad Buslov
2019-10-25 16:08                       ` Vlad Buslov [this message]

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=vbfr230u7su.fsf@mellanox.com \
    --to=vladbu@mellanox.com \
    --cc=davem@davemloft.net \
    --cc=dcaratti@redhat.com \
    --cc=edumazet@google.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=mleitner@redhat.com \
    --cc=netdev@vger.kernel.org \
    --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).