[dpdk-dev] [PATCH v4 00/41] Pipeline alignment with the P4 language

Dumitrescu, Cristian cristian.dumitrescu at intel.com
Wed Sep 23 18:07:51 CEST 2020


Hi David,

Thanks for looking at this patch series!

> -----Original Message-----
> From: David Marchand <david.marchand at redhat.com>
> Sent: Wednesday, September 23, 2020 12:46 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu at intel.com>
> Cc: dev <dev at dpdk.org>; Thomas Monjalon <thomas at monjalon.net>
> Subject: Re: [dpdk-dev] [PATCH v4 00/41] Pipeline alignment with the P4
> language
> 
> Hello Cristian,
> 
> On Thu, Sep 10, 2020 at 5:27 PM Cristian Dumitrescu
> <cristian.dumitrescu at intel.com> wrote:
> > Cristian Dumitrescu (41):
> >   pipeline: add new SWX pipeline type
> >   pipeline: add SWX pipeline input port
> >   pipeline: add SWX pipeline output port
> >   pipeline: add SWX headers and meta-data
> >   pipeline: add SWX extern objects and funcs
> >   pipeline: add SWX pipeline action
> >   pipeline: add SWX pipeline tables
> >   pipeline: add SWX pipeline instructions
> >   pipeline: add SWX rx and extract instructions
> >   pipeline: add SWX tx and emit instructions
> >   pipeline: add header validate and invalidate SWX instructions
> >   pipeline: add SWX mov instruction
> >   pipeline: add SWX dma instruction
> >   pipeline: introduce SWX add instruction
> >   pipeline: introduce SWX sub instruction
> >   pipeline: introduce SWX ckadd instruction
> >   pipeline: introduce SWX cksub instruction
> >   pipeline: introduce SWX and instruction
> >   pipeline: introduce SWX or instruction
> >   pipeline: introduce SWX xor instruction
> >   pipeline: introduce SWX shl instruction
> >   pipeline: introduce SWX shr instruction
> >   pipeline: introduce SWX table instruction
> >   pipeline: introduce SWX extern instruction
> >   pipeline: introduce SWX jmp and return instructions
> >   pipeline: add SWX instruction description
> >   pipeline: add SWX instruction verifier
> >   pipeline: add SWX instruction optimizer
> >   pipeline: add SWX pipeline query API
> >   pipeline: add SWX pipeline flush
> >   pipeline: add SWX table update high level API
> >   pipeline: add SWX pipeline specification file
> >   port: add ethernet device SWX port
> >   port: add source and sink SWX ports
> >   table: add exact match SWX table
> >   examples/pipeline: add new example application
> >   examples/pipeline: add message passing mechanism
> >   examples/pipeline: add configuration commands
> >   examples/pipeline: add l2fwd example
> >   examples/pipeline: add l2fwd with MAC swap example
> >   examples/pipeline: add VXLAN encapsulation example
> 
> - This new feature is the future of the pipeline library: it will need
> unit tests and/or tests in the CI to catch regressions.
> 

Agreed, this is work in progress and will materialize in new test cases upstreamed into DPDK Test Framework (DTS)  during the 20.11 and 21.02 time frame. They are based on the new sample app introduced by this patch set, which already has some tests (see examples/pipeline/examples folder), but having them automated into DTS is WIP.

> - Many MACRO_WITH_FLOW_CONTROL warnings reported by checkpatches.
> 

Yes, I fixed all the other code style issues, this is the only one remaining. It is basically due to a recurring CHECK() macro. And it will also ripples over the code, so IMO it is time consuming & error prone to remove with no real benefit.

We also already have many places in DPDK that use the same pattern. I suggest we ignore this warning, are you OK with it?

> - On the patch titles, check-git-log.sh reports:
> Wrong headline case:
>             "pipeline: add SWX dma instruction": dma --> DMA
> Wrong headline case:
>             "pipeline: add SWX rx and extract instructions": rx --> Rx
> Wrong headline case:
>             "pipeline: add SWX tx and emit instructions": tx --> Tx
> Wrong headline case:
>             "pipeline: introduce SWX xor instruction": xor --> XOR
> 

I can do this change, but IMO it is not the right choice here, as in this particular case we have instructions that are called "rx", "tx", "dma", "and", "or", "xor", etc. So it is the name of an instruction rather than a text abbreviation. Hence, I think these messages are not really applicable here. What do you think?

> - We have yet another new example, please declare it in MAINTAINERS.
> 

Yes, will fix in V5.

> - I checked per patch compilation which is fine, thanks.
> 

Great, thanks!

> 
> --
> David Marchand

Regards,
Cristian


More information about the dev mailing list