All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <simon.horman@netronome.com>
To: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: David Miller <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	Jiri Pirko <jiri@mellanox.com>,
	netdev@vger.kernel.org, oss-drivers@netronome.com,
	Baowen Zheng <baowen.zheng@corigine.com>
Subject: Re: [PATCH net-next v2] net/sched: act_police: add support for packet-per-second policing
Date: Mon, 1 Feb 2021 13:33:53 +0100	[thread overview]
Message-ID: <20210201123352.GB25935@netronome.com> (raw)
In-Reply-To: <0c47b7d7-dc2b-3422-62ff-92fea8300036@mojatatu.com>

On Fri, Jan 29, 2021 at 09:30:00AM -0500, Jamal Hadi Salim wrote:
> On 2021-01-29 5:28 a.m., Simon Horman wrote:
> > From: Baowen Zheng <baowen.zheng@corigine.com>
> > 
> > Allow a policer action to enforce a rate-limit based on packets-per-second,
> > configurable using a packet-per-second rate and burst parameters. This may
> > be used in conjunction with existing byte-per-second rate limiting in the
> > same policer action.
> > 
> > e.g.
> > tc filter add dev tap1 parent ffff: u32 match \
> >                u32 0 0 police pkts_rate 3000 pkts_burst 1000
> > 
> > Testing was unable to uncover a performance impact of this change on
> > existing features.
> > 
> 
> Ido's comment is important: Why not make packet rate vs byte rate
> mutually exclusive? If someone uses packet rate then you make sure
> they dont interleave with attributes for byte rate and vice-versa.
> 
> I dont see featurewise impact - but certainly the extra check
> in the fastpath will likely have a small performance impact
> at high rates. Probably not a big deal (if Eric D. is not looking).
> Side note: this policer (with your addition) is now supporting 3 policer
> algorithms - i wonder if it makes sense to start spliting the different
> policers (which will solve the performance impact).

Sorry, I somehow missed Ido's email until you and he pointed it out
in this thread.

Regarding splitting up the policer action. I think there is some value to
the current setup in terms of code re-use and allowing combinations of
features. But I do agree it would be a conversation worth having at some
point.

  parent reply	other threads:[~2021-02-01 12:34 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20210129102856.6225-1-simon.horman@netronome.com>
2021-01-29 23:04 ` [PATCH net-next v2] net/sched: act_police: add support for packet-per-second policing Cong Wang
2021-01-30 14:57   ` Ido Schimmel
2021-02-01 13:07     ` Simon Horman
2021-02-01 13:02   ` Simon Horman
2021-02-02  5:26     ` Cong Wang
     [not found] ` <0c47b7d7-dc2b-3422-62ff-92fea8300036@mojatatu.com>
2021-02-01 12:33   ` Simon Horman [this message]
2021-02-02 13:59     ` 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=20210201123352.GB25935@netronome.com \
    --to=simon.horman@netronome.com \
    --cc=baowen.zheng@corigine.com \
    --cc=davem@davemloft.net \
    --cc=jhs@mojatatu.com \
    --cc=jiri@mellanox.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=oss-drivers@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.