Message ID | 20190507123635.17782-1-maxime.chevallier@bootlin.com |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | [net] net: mvpp2: cls: Add missing NETIF_F_NTUPLE flag | expand |
On Tue, 7 May 2019 14:36:35 +0200, Maxime Chevallier wrote: > Now that the mvpp2 driver supports classification offloading, we must > add the NETIF_F_NTUPLE to the features list. > > Fixes: 90b509b39ac9 ("net: mvpp2: cls: Add Classification offload support") > Reported-by: Jakub Kicinski <jakub.kicinski@netronome.com> > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> > --- > Hello David, > > This patch applies on top of a commit 90b509b39ac9, which is in net-next > but hasn't made it to -net yet. > > Thanks, > > Maxime > > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > index 25fbed2b8d94..1f164c893936 100644 > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > @@ -5040,8 +5040,10 @@ static int mvpp2_port_probe(struct platform_device *pdev, > dev->hw_features |= features | NETIF_F_RXCSUM | NETIF_F_GRO | > NETIF_F_HW_VLAN_CTAG_FILTER; > > - 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? > + } > > if (port->pool_long->id == MVPP2_BM_JUMBO && port->id != 0) { > dev->features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM);
From: Maxime Chevallier <maxime.chevallier@bootlin.com> Date: Tue, 7 May 2019 14:36:35 +0200 > Now that the mvpp2 driver supports classification offloading, we must > add the NETIF_F_NTUPLE to the features list. > > Fixes: 90b509b39ac9 ("net: mvpp2: cls: Add Classification offload support") > Reported-by: Jakub Kicinski <jakub.kicinski@netronome.com> > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> > --- > Hello David, > > This patch applies on top of a commit 90b509b39ac9, which is in net-next > but hasn't made it to -net yet. > > Thanks, > > Maxime > > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > index 25fbed2b8d94..1f164c893936 100644 > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > @@ -5040,8 +5040,10 @@ static int mvpp2_port_probe(struct platform_device *pdev, > dev->hw_features |= features | NETIF_F_RXCSUM | NETIF_F_GRO | > NETIF_F_HW_VLAN_CTAG_FILTER; > > - if (mvpp22_rss_is_supported()) > + if (mvpp22_rss_is_supported()) { > dev->hw_features |= NETIF_F_RXHASH; > + dev->features |= NETIF_F_NTUPLE; > + } As Jakub said, this definitely looks like a typo and this should be hw_features.
Hello Jakub, David, On Tue, 7 May 2019 10:28:03 -0700 Jakub Kicinski <jakub.kicinski@netronome.com> 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. 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). Thanks, Maxime
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@netronome.com> 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@netronome.com> if flushing the filters is too much hassle.
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c index 25fbed2b8d94..1f164c893936 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c @@ -5040,8 +5040,10 @@ static int mvpp2_port_probe(struct platform_device *pdev, dev->hw_features |= features | NETIF_F_RXCSUM | NETIF_F_GRO | NETIF_F_HW_VLAN_CTAG_FILTER; - if (mvpp22_rss_is_supported()) + if (mvpp22_rss_is_supported()) { dev->hw_features |= NETIF_F_RXHASH; + dev->features |= NETIF_F_NTUPLE; + } if (port->pool_long->id == MVPP2_BM_JUMBO && port->id != 0) { dev->features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM);
Now that the mvpp2 driver supports classification offloading, we must add the NETIF_F_NTUPLE to the features list. Fixes: 90b509b39ac9 ("net: mvpp2: cls: Add Classification offload support") Reported-by: Jakub Kicinski <jakub.kicinski@netronome.com> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- Hello David, This patch applies on top of a commit 90b509b39ac9, which is in net-next but hasn't made it to -net yet. Thanks, Maxime drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)