netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Pirko <jiri@resnulli.us>
To: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: davem@davemloft.net, xiyou.wangcong@gmail.com,
	eric.dumazet@gmail.com, netdev@vger.kernel.org,
	Simon Horman <simon.horman@netronome.com>,
	Benjamin LaHaise <bcrl@kvack.org>
Subject: Re: [PATCH net-next v8 2/3] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
Date: Wed, 26 Apr 2017 15:56:27 +0200	[thread overview]
Message-ID: <20170426135627.GI1867@nanopsycho.orion> (raw)
In-Reply-To: <10fe2c22-8e76-543e-dd24-ddce5813ab69@mojatatu.com>

Wed, Apr 26, 2017 at 03:14:38PM CEST, jhs@mojatatu.com wrote:
>On 17-04-26 08:08 AM, Jiri Pirko wrote:
>> Wed, Apr 26, 2017 at 01:48:29PM CEST, jhs@mojatatu.com wrote:
>> > On 17-04-26 02:19 AM, Jiri Pirko wrote:
>> > > Tue, Apr 25, 2017 at 10:29:40PM CEST, jhs@mojatatu.com wrote:
>> > > > On 17-04-25 12:04 PM, Jiri Pirko wrote:
>
>> > I have experience with dealing with a massive amount of various dumps
>> > and (batch) sets and it always boils down to one thing:
>> > _how much data is exchanged between user and kernel_
>> > 3 flags encoded as bits in a u32 attribute cost 64 bits.
>> > Encoded separately cost 3x that.
>> > 
>> > Believe me, it _does make a difference_ in performance.
>> > 
>> > My least favorite subsystem is bridge. The bridge code has
>> > tons of flags in those entries that are sent to/from kernel as u8
>> > attributes. It is painful.
>> > 
>> > For something more recent, lets look at this commit from Ben on Flower:
>> > +       TCA_FLOWER_KEY_MPLS_TTL,        /* u8 - 8 bits */
>> > +       TCA_FLOWER_KEY_MPLS_BOS,        /* u8 - 1 bit */
>> > +       TCA_FLOWER_KEY_MPLS_TC,         /* u8 - 3 bits */
>> > +       TCA_FLOWER_KEY_MPLS_LABEL,      /* be32 - 20 bits */
>> > 
>> > Yes, that looks pretty, but:
>> > That would have fit in one attribute with a u32. Mask attributes would
>> > be eliminated with a second 32 bit - all in the same singular
>> > attribute.
>> > 
>> > Imagine if i have 1M flower entries. If you add up the mask the cost
>> > of these things is about 3*2*64 bits more per entry compared to putting
>> > the mpls info/mask in one attribute.
>> > At 1M entries that is a few MBs of data being exchanged.
>> 
>> I can do the math :) Yet still, I would like to see the numbers :)
>> Because I believe that is the only way to end this lenghty converstation
>> once and forever...
>> 
>
>Jiri, what are you arguing about if you have done the math? ;->

I can do 3*2*64. What I cannot do is to figure out the real performance
impact.


>You want me to show you that getting or setting less data is good for
>performance?
>Look at the third patch: Why do i think it is necessary to send only
>actions that have changed? Precisely to reduce the amount of data
>being transported. The second patch - to reduce the amount of crossing
>user space to kernel space (which is going to happen more with increased
>data I have to transport between the user and the kernel).
>
>Again: You are looking at this from a manageability point of view which
>is useful but not the only input into a design. If i can squeeze more
>data without killing usability - I am all for it. It just doesnt
>compute that it is ok to use a flag per attribute because it looks
>beautiful.

Hmm. Now that I'm thinking about it, why don't we have NLA_FLAGS with
couple of helpers around it? It will be obvious what the attr is, all
kernel code would use the same helpers. Would be nice.

  reply	other threads:[~2017-04-26 13:56 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-25 11:54 [PATCH net-next v8 0/3] net sched actions: improve dump performance Jamal Hadi Salim
2017-04-25 11:54 ` [PATCH net-next v8 1/3] net sched actions: Use proper root attribute table for actions Jamal Hadi Salim
2017-04-25 18:42   ` Simon Horman
2017-04-25 11:54 ` [PATCH net-next v8 2/3] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch Jamal Hadi Salim
2017-04-25 12:13   ` Jiri Pirko
2017-04-25 13:01     ` Jamal Hadi Salim
2017-04-25 16:04       ` Jiri Pirko
2017-04-25 20:29         ` Jamal Hadi Salim
2017-04-26  6:19           ` Jiri Pirko
2017-04-26 11:07             ` Simon Horman
2017-04-26 13:00               ` Jamal Hadi Salim
2017-04-26 11:48             ` Jamal Hadi Salim
2017-04-26 12:08               ` Jiri Pirko
2017-04-26 13:14                 ` Jamal Hadi Salim
2017-04-26 13:56                   ` Jiri Pirko [this message]
2017-04-26 20:07                     ` Jamal Hadi Salim
2017-04-27  6:30                       ` Jiri Pirko
2017-04-28  1:22                         ` Jamal Hadi Salim
2017-04-28  7:02                           ` Jiri Pirko
2017-04-28 12:30                             ` Jamal Hadi Salim
2017-04-28 13:21                               ` Jiri Pirko
2017-04-28 13:42                                 ` Jamal Hadi Salim
2017-04-28 14:02                                   ` Jiri Pirko
2017-04-30 10:34                                 ` Jamal Hadi Salim
2017-04-26 11:02         ` Simon Horman
2017-04-26 12:46           ` Jamal Hadi Salim
2017-04-26 13:05             ` Jiri Pirko
2017-04-26 14:46               ` David Miller
2017-04-26 14:58                 ` Jiri Pirko
2017-04-28 12:21                 ` Simon Horman
2017-04-28 12:55                   ` Jamal Hadi Salim
2017-04-28 13:21                     ` Jiri Pirko
2017-04-25 11:54 ` [PATCH net-next v8 3/3] net sched actions: add time filter for action dumping 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=20170426135627.GI1867@nanopsycho.orion \
    --to=jiri@resnulli.us \
    --cc=bcrl@kvack.org \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=jhs@mojatatu.com \
    --cc=netdev@vger.kernel.org \
    --cc=simon.horman@netronome.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).