mbox series

[net-next,0/4] net: mvpp2: cls: Add classification

Message ID 20190430131429.19361-1-maxime.chevallier@bootlin.com
Headers show
Series net: mvpp2: cls: Add classification | expand

Message

Maxime Chevallier April 30, 2019, 1:14 p.m. UTC
Hi everyone,

This series is a rework of the previously standalone patch adding
classification support for mvpp2 :

https://lore.kernel.org/netdev/20190423075031.26074-1-maxime.chevallier@bootlin.com/

This patch has been reworked according to Saeed's review, to make sure
that the location of the rule is always respected and serves as a way to
prioritize rules between each other. This the 3rd iteration of this
submission, but since it's now a series, I reset the revision numbering.

This series implements that in a limited configuration for now, since we
limit the total number of rules per port to 4.

The main factors for this limitation are that :
 - We share the classification tables between all ports (4 max, although
   one is only used for internal loopback), hence we have to perform a
   logical separation between rules, which is done today by dedicated
   ranges for each port in each table

 - The "Flow table", which dictates which lookups operations are
   performed for an ingress packet, in subdivided into 22 "sub flows",
   each corresponding to a traffic type based on the L3 proto, L4
   proto, the presence or not of a VLAN tag and the L3 fragmentation.

   This makes so that when adding a rule, it has to be added into each
   of these subflows, introducing duplications of entries and limiting
   our max number of entries.

These limitations can be overcomed in several ways, but for readability
sake, I'd rather submit basic classification offload support for now,
and improve it gradually.

This series also adds a small cosmetic cleanup patch (1), and also adds
support for the "Drop" action compared to the first submission of this
feature. It is simple enough to be added with this basic support.

Compared to the first submissions, the NETIF_F_NTUPLE flag was also
removed, following Saeed's comment.

Thanks,

Maxime

Maxime Chevallier (4):
  net: mvpp2: cls: Remove extra whitespace in mvpp2_cls_flow_write
  net: mvpp2: cls: Use a bitfield to represent the flow_type
  net: mvpp2: cls: Add Classification offload support
  net: mvpp2: cls: Allow dropping packets with classification offload

 drivers/net/ethernet/marvell/mvpp2/mvpp2.h    |  42 ++
 .../net/ethernet/marvell/mvpp2/mvpp2_cls.c    | 497 +++++++++++++++---
 .../net/ethernet/marvell/mvpp2/mvpp2_cls.h    |  70 ++-
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   |  20 +-
 4 files changed, 545 insertions(+), 84 deletions(-)

Comments

David Miller May 1, 2019, 9:13 p.m. UTC | #1
From: Maxime Chevallier <maxime.chevallier@bootlin.com>
Date: Tue, 30 Apr 2019 15:14:25 +0200

> This series is a rework of the previously standalone patch adding
> classification support for mvpp2 :
 ...

Series applied, thanks.
Jakub Kicinski May 4, 2019, 6:53 a.m. UTC | #2
On Tue, 30 Apr 2019 15:14:25 +0200, Maxime Chevallier wrote:
> Compared to the first submissions, the NETIF_F_NTUPLE flag was also
> removed, following Saeed's comment.

You should probably add it back, even though the stack only uses
NETIF_F_NTUPLE for aRFS the ethtool APIs historically depend on the
drivers doing a lot of the validation.

The flag was added by:

15682bc488d4 ("ethtool: Introduce n-tuple filter programming support")

your initial use of the flag was correct.
Maxime Chevallier May 6, 2019, 8 a.m. UTC | #3
Hello Jakub,

On Sat, 4 May 2019 02:53:53 -0400
Jakub Kicinski <jakub.kicinski@netronome.com> wrote:

>On Tue, 30 Apr 2019 15:14:25 +0200, Maxime Chevallier wrote:
>> Compared to the first submissions, the NETIF_F_NTUPLE flag was also
>> removed, following Saeed's comment.  
>
>You should probably add it back, even though the stack only uses
>NETIF_F_NTUPLE for aRFS the ethtool APIs historically depend on the
>drivers doing a lot of the validation.

OK my bad, reading your previous comments again, I should indeed have
left it.

I'll re-add the flag, do you think this should go through -net or wait
until net-next reopens ?

Thanks,

Maxime
Jakub Kicinski May 6, 2019, 5:54 p.m. UTC | #4
On Mon, 6 May 2019 10:00:26 +0200, Maxime Chevallier wrote:
> Hello Jakub,
> 
> On Sat, 4 May 2019 02:53:53 -0400
> Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> 
> >On Tue, 30 Apr 2019 15:14:25 +0200, Maxime Chevallier wrote:  
> >> Compared to the first submissions, the NETIF_F_NTUPLE flag was also
> >> removed, following Saeed's comment.    
> >
> >You should probably add it back, even though the stack only uses
> >NETIF_F_NTUPLE for aRFS the ethtool APIs historically depend on the
> >drivers doing a lot of the validation.  
> 
> OK my bad, reading your previous comments again, I should indeed have
> left it.
> 
> I'll re-add the flag, do you think this should go through -net or wait
> until net-next reopens ?

I think the patch should be relatively simple and clean?  So I'd try for
net, with a Fixes tag, it's a slight ABI correction and we are still
in the merge window period.  So I'd go for net :)