Re: [PATCH net] net: mvpp2: cls: Add missing NETIF_F_NTUPLE flag

From: Jakub Kicinski
Date: Thu May 09 2019 - 13:45:31 EST


On Thu, 9 May 2019 07:14:08 +0200, Maxime Chevallier wrote:
> Hello Jakub, David,
>
> On Tue, 7 May 2019 10:28:03 -0700
> Jakub Kicinski <jakub.kicinski@xxxxxxxxxxxxx> wrote:
>
> >> - if (mvpp22_rss_is_supported())
> >> + if (mvpp22_rss_is_supported()) {
> >> dev->hw_features |= NETIF_F_RXHASH;
> >> + dev->features |= NETIF_F_NTUPLE;
> >
> >Hm, why not in hw_features?
>
> Because as of today, there's nothing implemented to disable
> classification offload in the driver, so the feature can't be toggled.
>
> Is this an issue ? Sorry if I'm doing this wrong, but I didn't see any
> indication that this feature has to be host-writeable.

No I don't think it's an issue, I was expecting you'd flush all the
filters when feature is disabled (remove them entirely), I didn't
expect that to be too hard.

> I can make so that it's toggle-able, but it's not as straightforward as
> we would think, since the classifier is also used for RSS (so, we can't
> just disable the classifier as a whole, we would have to invalidate
> each registered flow).

Ack, I don't think disabling the hardware components is required.
Just remove the existing filters, and don't allow new ones.
But no strong feelings here, feel free to repost with:

Acked-by: Jakub Kicinski <jakub.kicinski@xxxxxxxxxxxxx>

if flushing the filters is too much hassle.