All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Edward Cree <ecree@solarflare.com>, Jiri Pirko <jiri@resnulli.us>,
	netdev@vger.kernel.org, davem@davemloft.net, saeedm@mellanox.com,
	leon@kernel.org, michael.chan@broadcom.com, vishal@chelsio.com,
	jeffrey.t.kirsher@intel.com, idosch@mellanox.com,
	aelior@marvell.com, peppe.cavallaro@st.com,
	alexandre.torgue@st.com, jhs@mojatatu.com,
	xiyou.wangcong@gmail.com, mlxsw@mellanox.com,
	netfilter-devel@vger.kernel.org
Subject: Re: [patch net-next v2 01/12] flow_offload: Introduce offload of HW stats type
Date: Tue, 3 Mar 2020 11:34:57 -0800	[thread overview]
Message-ID: <20200303113457.0a622b7f@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <20200303172525.2p6jwa6u7qx5onji@salvia>

On Tue, 3 Mar 2020 18:25:25 +0100 Pablo Neira Ayuso wrote:
> On Mon, Mar 02, 2020 at 02:49:28PM -0800, Jakub Kicinski wrote:
> > On Mon, 2 Mar 2020 22:46:59 +0100 Pablo Neira Ayuso wrote:  
> [...]
> > > The real question is: if you think this tc counter+action scheme can
> > > be used by netfilter, then please explain how.  
> > 
> > In Jiri's latest patch set the counter type is per action, so just
> > "merge right" the counter info into the next action and the models 
> > are converted.  
> 
> The input "merge right" approach might work.
> 
> > If user is silly and has multiple counter actions in a row - the
> > pipe/no-op action comes into play (that isn't part of this set, 
> > as Jiri said).  
> 
> Probably gact pipe action with counters can be mapped to the counter
> action that netfilter needs. Is this a valid use-case you consider for
> the tc hardware offload?

Once actions can be shared I think it'd be a pretty useful thing for tc
hardware offloads in case HW has limited counters.

> > Can you give us examples of what wouldn't work? Can you for instance
> > share the counter across rules?  
> 
> Yes, there might be counters that are shared accross rules, see
> nfacct. Two different rules might refer to the same counter, IIRC
> there is a way to do this in tc too.

Yup, not implemented for offload, tho.

> > Also neither proposal addresses the problem of reporting _different_
> > counter values at different stages in the pipeline, i.e. moving from
> > stats per flow to per action. But nobody seems to be willing to work 
> > on that.  
> 
> You mean, in case that different counter types are specified, eg. one
> action using delayed and another action using immediate?

I meant the work Ed just pointed to, and what you ask about below.

> > AFAICT with Jiri's change we only need one check in the drivers to
> > convert from old scheme to new, with explicit action we need two
> > (additional one being ignoring the counter action). Not a big deal,
> > but 1 is less than 2 🤷‍♂️  
> 
> What changes are expected to retrieve counter stats?
> 
> Will per-flow stats remain in place after this place?

In theory it doesn't have to, because action stats are more flexible.

In practice I doubt anyone will take on the conversion, so we'll have
to live with two ways for a while..

  reply	other threads:[~2020-03-03 19:35 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-28 17:24 [patch net-next v2 00/12] net: allow user specify TC action HW stats type Jiri Pirko
2020-02-28 17:24 ` [patch net-next v2 01/12] flow_offload: Introduce offload of " Jiri Pirko
2020-02-29 19:29   ` Pablo Neira Ayuso
2020-03-01  8:44     ` Jiri Pirko
2020-03-02 13:20       ` Pablo Neira Ayuso
2020-03-02 13:58         ` Jiri Pirko
2020-03-02 16:29         ` Edward Cree
2020-03-02 19:24           ` Pablo Neira Ayuso
2020-03-02 20:18             ` Jakub Kicinski
2020-03-02 21:46               ` Pablo Neira Ayuso
2020-03-02 22:49                 ` Jakub Kicinski
2020-03-03 17:25                   ` Pablo Neira Ayuso
2020-03-03 19:34                     ` Jakub Kicinski [this message]
2020-03-03 18:55                   ` Edward Cree
2020-03-03 19:26                     ` Jakub Kicinski
2020-03-03 20:27                     ` Pablo Neira Ayuso
2020-03-03 21:06                       ` Edward Cree
2020-03-03 21:25                         ` Pablo Neira Ayuso
2020-03-03 19:13   ` Edward Cree
2020-03-04 14:18     ` Jiri Pirko
2020-02-28 17:24 ` [patch net-next v2 02/12] ocelot_flower: use flow_offload_has_one_action() helper Jiri Pirko
2020-02-29  0:50   ` Vladimir Oltean
2020-02-28 17:24 ` [patch net-next v2 03/12] flow_offload: check for basic action hw stats type Jiri Pirko
2020-02-28 19:40   ` Jakub Kicinski
2020-02-29  7:40     ` Jiri Pirko
2020-02-29 19:18       ` Jakub Kicinski
2020-03-01  9:00         ` Jiri Pirko
2020-03-02 19:33           ` Jakub Kicinski
2020-03-03 11:46             ` Jiri Pirko
2020-02-28 17:24 ` [patch net-next v2 04/12] mlx5: en_tc: Do not allow mixing HW stats types for actions Jiri Pirko
2020-02-28 17:24 ` [patch net-next v2 05/12] mlxsw: spectrum_flower: " Jiri Pirko
2020-02-28 17:24 ` [patch net-next v2 06/12] mlx5: restrict supported HW stats type to "any" Jiri Pirko
2020-02-28 17:25 ` [patch net-next v2 07/12] mlxsw: " Jiri Pirko
2020-02-28 17:25 ` [patch net-next v2 08/12] flow_offload: introduce "immediate" HW stats type and allow it in mlxsw Jiri Pirko
2020-02-29 19:32   ` Pablo Neira Ayuso
2020-03-01  8:47     ` Jiri Pirko
2020-03-02 13:23       ` Pablo Neira Ayuso
2020-03-02 13:59         ` Jiri Pirko
2020-02-28 17:25 ` [patch net-next v2 09/12] flow_offload: introduce "delayed" HW stats type and allow it in mlx5 Jiri Pirko
2020-02-28 17:25 ` [patch net-next v2 10/12] mlxsw: spectrum_acl: Ask device for rule stats only if counter was created Jiri Pirko
2020-02-28 17:25 ` [patch net-next v2 11/12] flow_offload: introduce "disabled" HW stats type and allow it in mlxsw Jiri Pirko
2020-02-28 17:25 ` [patch net-next v2 12/12] sched: act: allow user to specify type of HW stats for a filter Jiri Pirko
2020-02-28 19:59   ` Jakub Kicinski
2020-02-29  7:52     ` Jiri Pirko
2020-02-29 20:14       ` Jakub Kicinski
2020-03-01  8:57         ` Jiri Pirko
2020-03-02 19:39           ` Jakub Kicinski
2020-03-03 13:20             ` Jiri Pirko
2020-03-03 19:48               ` Jakub Kicinski
2020-03-04  8:15                 ` Jiri Pirko
2020-03-03 19:44   ` Jakub Kicinski

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=20200303113457.0a622b7f@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
    --to=kuba@kernel.org \
    --cc=aelior@marvell.com \
    --cc=alexandre.torgue@st.com \
    --cc=davem@davemloft.net \
    --cc=ecree@solarflare.com \
    --cc=idosch@mellanox.com \
    --cc=jeffrey.t.kirsher@intel.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=leon@kernel.org \
    --cc=michael.chan@broadcom.com \
    --cc=mlxsw@mellanox.com \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    --cc=peppe.cavallaro@st.com \
    --cc=saeedm@mellanox.com \
    --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 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.