Re: [PATCH net-next 00/13] Control action percpu counters allocation by netlink flag

[Marcelo Ricardo Leitner <mleitner@redhat.com>]

On Tue, Oct 22, 2019 at 03:52:31PM +0000, Vlad Buslov wrote:
> 
> On Tue 22 Oct 2019 at 18:15, Marcelo Ricardo Leitner <mleitner@redhat.com> wrote:
> > On Tue, Oct 22, 2019 at 05:17:51PM +0300, Vlad Buslov wrote:
> >> - Extend actions that are used for hardware offloads with optional
> >>   netlink 32bit flags field. Add TCA_ACT_FLAGS_FAST_INIT action flag and
> >>   update affected actions to not allocate percpu counters when the flag
> >>   is set.
> >
> > I just went over all the patches and they mostly make sense to me. So
> > far the only point I'm uncertain of is the naming of the flag,
> > "fast_init".  That is not clear on what it does and can be overloaded
> > with other stuff later and we probably don't want that.
> 
> I intentionally named it like that because I do want to overload it with
> other stuff in future, instead of adding new flag value for every single
> small optimization we might come up with :)

Hah :-)

> 
> Also, I didn't want to hardcode implementation details into UAPI that we
> will have to maintain for long time after percpu allocator in kernel is
> potentially replaced with something new and better (like idr is being
> replaced with xarray now, for example)

I see. OTOH, this also means that the UAPI here would be unstable
(different meanings over time for the same call), and hopefully new
behaviors would always be backwards compatible.

> 
> Anyway, lets see what other people think. I'm open to changing it.
> 
> >
> > Say, for example, we want percpu counters but to disable allocating
> > the stats for hw, to make the counter in 28169abadb08 ("net/sched: Add
> > hardware specific counters to TC actions") optional.
> >
> > So what about:
> > TCA_ACT_FLAGS_NO_PERCPU_STATS
> > TCA_ACT_FLAGS_NO_HW_STATS (this one to be done on a subsequent patchset, yes)
> > ?
> >
> >   Marcelo
>