[v2,24/24] doc: port representors in cnxk

Message ID 20231219174003.72901-25-hkalra@marvell.com (mailing list archive)
State Changes Requested, archived
Delegated to: Jerin Jacob
Headers
Series net/cnxk: support for port representors |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/Intel-compilation warning apply issues
ci/iol-abi-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS

Commit Message

Harman Kalra Dec. 19, 2023, 5:40 p.m. UTC
  Updating the CNXK PMD documentation with the added support
for port representors.

Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
 MAINTAINERS                          |  1 +
 doc/guides/nics/cnxk.rst             | 58 ++++++++++++++++++++++++++++
 doc/guides/nics/features/cnxk.ini    |  3 ++
 doc/guides/nics/features/cnxk_vf.ini |  4 ++
 4 files changed, 66 insertions(+)
  

Comments

Thomas Monjalon Dec. 20, 2023, 9:37 a.m. UTC | #1
19/12/2023 18:40, Harman Kalra:
> +The CNXK driver supports port representor model by adding virtual ethernet
> +ports providing a logical representation in DPDK for physical function(PF) or
> +SR-IOV virtual function (VF) devices for control and monitoring.
> +
> +Base device or parent device underneath these representor ports is a eswitch
> +device which is not a cnxk ethernet device but has NIC RX and TX capabilities.
> +Each representor port is represented by a RQ and SQ pair of this eswitch
> +device.
> +
> +Current implementation supports representors for both physical function and
> +virtual function.

A doc comes with its implementation, so no need to say "current implementation".

> +
> +These port representor ethdev instances can be spawned on an as needed basis
> +through configuration parameters passed to the driver of the underlying
> +base device using devargs ``-a <base PCI BDF>,representor=pf*vf*``
> +
> +.. note::
> +
> +   Representor ports to be created for respective representees should be
> +   defined via these representor devargs.
> +   Eg. To create a representor for representee PF1VF0, devargs to be passed
> +   is ``-a <base PCI BDF>,representor=pf0vf0``
> +
> +   For PF representor
> +   ``-a <base PCI BDF>,representor=pf2``
> +
> +   For defining range of vfs, say 5 representor ports under a PF
> +   ``-a <base PCI BDF>,representor=pf0vf[0-4]``
> +
> +   For representing different VFs under different PFs
> +   ``-a <base PCI BDF>,representor=pf0vf[1,2],representor=pf1vf[2-5]``

It looks like something we should describe globally for ethdev,
instead of driver documentation.

> +In case of exception path (i.e. until the flow definition is offloaded to the
> +hardware), packets transmitted by the VFs shall be received by these
> +representor port, while packets transmitted by representor ports shall be
> +received by respective VFs.

Not clear. How is it related to any offload?

> +On receiving the VF traffic via these representor ports, applications holding
> +these representor ports can decide to offload the traffic flow into the HW.
> +Henceforth the matching traffic shall be directly steered to the respective
> +VFs without being received by the application.

Using "these" makes no sense here. Please prefer "the representor ports".

> +Current virtual representor port PMD supports following operations:

Again, no need of "current".

[...]
>     +---+------------+-------------------------------------------------------+
>     | 2 | NPC        | --log-level='pmd\.net.cnxk\.flow,8'                   |
>     +---+------------+-------------------------------------------------------+
> +   | 3 | REP        | --log-level='pmd\.net.cnxk\.rep,8'                   |
> +   +---+------------+-------------------------------------------------------+
> +   | 4 | ESW        | --log-level='pmd\.net.cnxk\.esw,8'                   |
> +   +---+------------+-------------------------------------------------------+

Why it is not aligned?

> --- a/doc/guides/nics/features/cnxk_vf.ini
> +++ b/doc/guides/nics/features/cnxk_vf.ini
> @@ -64,6 +64,8 @@ mpls                 = Y
>  nvgre                = Y
>  pppoes               = Y
>  raw                  = Y
> +represented_port     = Y
> +port_representor     = Y
>  sctp                 = Y

It should be in alphabetical order.
  
Harman Kalra Dec. 21, 2023, 1:28 p.m. UTC | #2
Hi Thomas,

Thanks for reviewing.
Please find responses inline.

Thanks
Harman

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, December 20, 2023 3:08 PM
> To: Harman Kalra <hkalra@marvell.com>
> Cc: Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>; Kiran Kumar
> Kokkilagadda <kirankumark@marvell.com>; Sunil Kumar Kori
> <skori@marvell.com>; Satha Koteswara Rao Kottidi
> <skoteshwar@marvell.com>; dev@dpdk.org; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>
> Subject: [EXT] Re: [PATCH v2 24/24] doc: port representors in cnxk
> 
> External Email
> 
> ----------------------------------------------------------------------
> 19/12/2023 18:40, Harman Kalra:
> > +The CNXK driver supports port representor model by adding virtual
> > +ethernet ports providing a logical representation in DPDK for
> > +physical function(PF) or SR-IOV virtual function (VF) devices for control
> and monitoring.
> > +
> > +Base device or parent device underneath these representor ports is a
> > +eswitch device which is not a cnxk ethernet device but has NIC RX and TX
> capabilities.
> > +Each representor port is represented by a RQ and SQ pair of this
> > +eswitch device.
> > +
> > +Current implementation supports representors for both physical
> > +function and virtual function.
> 
> A doc comes with its implementation, so no need to say "current
> implementation".

Ack, I will fix this.


> 
> > +
> > +These port representor ethdev instances can be spawned on an as
> > +needed basis through configuration parameters passed to the driver of
> > +the underlying base device using devargs ``-a <base PCI
> > +BDF>,representor=pf*vf*``
> > +
> > +.. note::
> > +
> > +   Representor ports to be created for respective representees should be
> > +   defined via these representor devargs.
> > +   Eg. To create a representor for representee PF1VF0, devargs to be
> passed
> > +   is ``-a <base PCI BDF>,representor=pf0vf0``
> > +
> > +   For PF representor
> > +   ``-a <base PCI BDF>,representor=pf2``
> > +
> > +   For defining range of vfs, say 5 representor ports under a PF
> > +   ``-a <base PCI BDF>,representor=pf0vf[0-4]``
> > +
> > +   For representing different VFs under different PFs
> > +   ``-a <base PCI
> > + BDF>,representor=pf0vf[1,2],representor=pf1vf[2-5]``
> 
> It looks like something we should describe globally for ethdev, instead of
> driver documentation.

DPDK  generic representor devarg parser (rte_eth_devargs_parse_representor_ports()) can parse first
3 cases i.e. a <base PCI BDF>,representor=pf0vf0 .... ``-a <base PCI BDF>,representor=pf0vf[0-4]``,
while 4 case was a special case which our PMD needs.

Representor devargs are processed as part of new device (eswitch) PMD only, normal CNXK
PMD won't accept representor as a devarg. Hence all devargs we define under eswitch PCI device
and all the required representors are created while probing eswitch device probing.

In the following format we are defining representors for which PFs and VFs should be created:
Eg. 
	-a <base PCI BDF >,representor=pf0vf[1,2],representor=pf1vf[2-5]
Here
	VF representor will be created only for PF0VF1, PF2VF2, PF1VF2.....PF1VF5
Although there may be n no of PF VF combinations but user wants representors for this devices only.

Please let us know your opinion if "-a <base PCI BDF >,representor=pf0vf[1,2],representor=pf1vf[2-5]"
format handling can also be handled in common code. We can push a separate patch for it.

> 
> > +In case of exception path (i.e. until the flow definition is
> > +offloaded to the hardware), packets transmitted by the VFs shall be
> > +received by these representor port, while packets transmitted by
> > +representor ports shall be received by respective VFs.
> 
> Not clear. How is it related to any offload?

Point what I wanted to highlight here is, until the flow rule for a fastpath is identified
and installed (offloaded) to the HW, packet flow will take the slow path (exception path)
 i.e. for every packet sent out via VF should be received by its representor port and
vice versa.
Once the application installs the rule packets can take fast path i.e. directly
from VF to destination (wire or other VF), representors will not come in the 
datapath for fast processing.


> 
> > +On receiving the VF traffic via these representor ports, applications
> > +holding these representor ports can decide to offload the traffic flow into
> the HW.
> > +Henceforth the matching traffic shall be directly steered to the
> > +respective VFs without being received by the application.
> 
> Using "these" makes no sense here. Please prefer "the representor ports".

Ack, will fix this

> 
> > +Current virtual representor port PMD supports following operations:
> 
> Again, no need of "current".

Ack, will fix this

> 
> [...]
> >     +---+------------+-------------------------------------------------------+
> >     | 2 | NPC        | --log-level='pmd\.net.cnxk\.flow,8'                   |
> >
> > +---+------------+----------------------------------------------------
> > ---+
> > +   | 3 | REP        | --log-level='pmd\.net.cnxk\.rep,8'                   |
> > +   +---+------------+-------------------------------------------------------+
> > +   | 4 | ESW        | --log-level='pmd\.net.cnxk\.esw,8'                   |
> > +
> > + +---+------------+--------------------------------------------------
> > + -----+
> 
> Why it is not aligned?

Sorry, my bad I will fix this

> 
> > --- a/doc/guides/nics/features/cnxk_vf.ini
> > +++ b/doc/guides/nics/features/cnxk_vf.ini
> > @@ -64,6 +64,8 @@ mpls                 = Y
> >  nvgre                = Y
> >  pppoes               = Y
> >  raw                  = Y
> > +represented_port     = Y
> > +port_representor     = Y
> >  sctp                 = Y
> 
> It should be in alphabetical order.

Ack, will fix this

> 
>
  
Thomas Monjalon Dec. 21, 2023, 6:33 p.m. UTC | #3
21/12/2023 14:28, Harman Kalra:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 19/12/2023 18:40, Harman Kalra:
> > > +   Representor ports to be created for respective representees should be
> > > +   defined via these representor devargs.
> > > +   Eg. To create a representor for representee PF1VF0, devargs to be
> > passed
> > > +   is ``-a <base PCI BDF>,representor=pf0vf0``
> > > +
> > > +   For PF representor
> > > +   ``-a <base PCI BDF>,representor=pf2``
> > > +
> > > +   For defining range of vfs, say 5 representor ports under a PF
> > > +   ``-a <base PCI BDF>,representor=pf0vf[0-4]``
> > > +
> > > +   For representing different VFs under different PFs
> > > +   ``-a <base PCI
> > > + BDF>,representor=pf0vf[1,2],representor=pf1vf[2-5]``
> > 
> > It looks like something we should describe globally for ethdev, instead of
> > driver documentation.
> 
> DPDK  generic representor devarg parser (rte_eth_devargs_parse_representor_ports()) can parse first
> 3 cases i.e. a <base PCI BDF>,representor=pf0vf0 .... ``-a <base PCI BDF>,representor=pf0vf[0-4]``,
> while 4 case was a special case which our PMD needs.
> 
> Representor devargs are processed as part of new device (eswitch) PMD only, normal CNXK
> PMD won't accept representor as a devarg. Hence all devargs we define under eswitch PCI device
> and all the required representors are created while probing eswitch device probing.
> 
> In the following format we are defining representors for which PFs and VFs should be created:
> Eg. 
> 	-a <base PCI BDF >,representor=pf0vf[1,2],representor=pf1vf[2-5]
> Here
> 	VF representor will be created only for PF0VF1, PF2VF2, PF1VF2.....PF1VF5
> Although there may be n no of PF VF combinations but user wants representors for this devices only.
> 
> Please let us know your opinion if "-a <base PCI BDF >,representor=pf0vf[1,2],representor=pf1vf[2-5]"
> format handling can also be handled in common code. We can push a separate patch for it.

I think yes it could be moved to common code in ethdev.


> > > +In case of exception path (i.e. until the flow definition is
> > > +offloaded to the hardware), packets transmitted by the VFs shall be
> > > +received by these representor port, while packets transmitted by
> > > +representor ports shall be received by respective VFs.
> > 
> > Not clear. How is it related to any offload?
> 
> Point what I wanted to highlight here is, until the flow rule for a fastpath is identified
> and installed (offloaded) to the HW, packet flow will take the slow path (exception path)
>  i.e. for every packet sent out via VF should be received by its representor port and
> vice versa.

That's the case for any flow rule, right?
I don't think it is specific to VF and representors.

> Once the application installs the rule packets can take fast path i.e. directly
> from VF to destination (wire or other VF), representors will not come in the 
> datapath for fast processing.

You probably need to rephrase to explain what happens in VF scenario
without being something which looks like an exception.
  
Harman Kalra Jan. 11, 2024, 6:48 a.m. UTC | #4
Hi Thomas

Thanks for the review
Please see inline

Thanks
Harman

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Friday, December 22, 2023 12:04 AM
> To: Harman Kalra <hkalra@marvell.com>
> Cc: Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>; Kiran Kumar
> Kokkilagadda <kirankumark@marvell.com>; Sunil Kumar Kori
> <skori@marvell.com>; Satha Koteswara Rao Kottidi
> <skoteshwar@marvell.com>; dev@dpdk.org; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>
> Subject: Re: [EXT] Re: [PATCH v2 24/24] doc: port representors in cnxk
> 

<snip>


> >
> > In the following format we are defining representors for which PFs and VFs
> should be created:
> > Eg.
> > 	-a <base PCI BDF >,representor=pf0vf[1,2],representor=pf1vf[2-5]
> > Here
> > 	VF representor will be created only for PF0VF1, PF2VF2,
> > PF1VF2.....PF1VF5 Although there may be n no of PF VF combinations but
> user wants representors for this devices only.
> >
> > Please let us know your opinion if "-a <base PCI BDF
> >,representor=pf0vf[1,2],representor=pf1vf[2-5]"
> > format handling can also be handled in common code. We can push a
> separate patch for it.
> 
> I think yes it could be moved to common code in ethdev.

I have pushed a series for the change:
https://patches.dpdk.org/project/dpdk/list/?series=30781

> 
> 
> > > > +In case of exception path (i.e. until the flow definition is
> > > > +offloaded to the hardware), packets transmitted by the VFs shall
> > > > +be received by these representor port, while packets transmitted
> > > > +by representor ports shall be received by respective VFs.
> > >
> > > Not clear. How is it related to any offload?
> >
> > Point what I wanted to highlight here is, until the flow rule for a
> > fastpath is identified and installed (offloaded) to the HW, packet
> > flow will take the slow path (exception path)  i.e. for every packet
> > sent out via VF should be received by its representor port and vice versa.
> 
> That's the case for any flow rule, right?
> I don't think it is specific to VF and representors.

Yes, will remove generic point

> 
> > Once the application installs the rule packets can take fast path i.e.
> > directly from VF to destination (wire or other VF), representors will
> > not come in the datapath for fast processing.
> 
> You probably need to rephrase to explain what happens in VF scenario
> without being something which looks like an exception.

Sure, will reword in next series.

>
  

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 0d1c8126e3..2716178e18 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -827,6 +827,7 @@  M: Nithin Dabilpuram <ndabilpuram@marvell.com>
 M: Kiran Kumar K <kirankumark@marvell.com>
 M: Sunil Kumar Kori <skori@marvell.com>
 M: Satha Rao <skoteshwar@marvell.com>
+M: Harman Kalra <hkalra@marvell.com>
 T: git://dpdk.org/next/dpdk-next-net-mrvl
 F: drivers/common/cnxk/
 F: drivers/net/cnxk/
diff --git a/doc/guides/nics/cnxk.rst b/doc/guides/nics/cnxk.rst
index 9ec52e380f..5fd1f6513a 100644
--- a/doc/guides/nics/cnxk.rst
+++ b/doc/guides/nics/cnxk.rst
@@ -37,6 +37,9 @@  Features of the CNXK Ethdev PMD are:
 - Inline IPsec processing support
 - Ingress meter support
 - Queue based priority flow control support
+- Port representors
+- Represented port pattern matching and action
+- Port representor pattern matching and action
 
 Prerequisites
 -------------
@@ -613,6 +616,57 @@  Runtime Config Options for inline device
    With the above configuration, driver would poll for aging flows every 50
    seconds.
 
+Port Representors
+-----------------
+
+The CNXK driver supports port representor model by adding virtual ethernet
+ports providing a logical representation in DPDK for physical function(PF) or
+SR-IOV virtual function (VF) devices for control and monitoring.
+
+Base device or parent device underneath these representor ports is a eswitch
+device which is not a cnxk ethernet device but has NIC RX and TX capabilities.
+Each representor port is represented by a RQ and SQ pair of this eswitch
+device.
+
+Current implementation supports representors for both physical function and
+virtual function.
+
+These port representor ethdev instances can be spawned on an as needed basis
+through configuration parameters passed to the driver of the underlying
+base device using devargs ``-a <base PCI BDF>,representor=pf*vf*``
+
+.. note::
+
+   Representor ports to be created for respective representees should be
+   defined via these representor devargs.
+   Eg. To create a representor for representee PF1VF0, devargs to be passed
+   is ``-a <base PCI BDF>,representor=pf0vf0``
+
+   For PF representor
+   ``-a <base PCI BDF>,representor=pf2``
+
+   For defining range of vfs, say 5 representor ports under a PF
+   ``-a <base PCI BDF>,representor=pf0vf[0-4]``
+
+   For representing different VFs under different PFs
+   ``-a <base PCI BDF>,representor=pf0vf[1,2],representor=pf1vf[2-5]``
+
+In case of exception path (i.e. until the flow definition is offloaded to the
+hardware), packets transmitted by the VFs shall be received by these
+representor port, while packets transmitted by representor ports shall be
+received by respective VFs.
+
+On receiving the VF traffic via these representor ports, applications holding
+these representor ports can decide to offload the traffic flow into the HW.
+Henceforth the matching traffic shall be directly steered to the respective
+VFs without being received by the application.
+
+Current virtual representor port PMD supports following operations:
+
+- Get and clear VF statistics
+- Set mac address
+- Flow operations - create, validate, destroy, query, flush, dump
+
 Debugging Options
 -----------------
 
@@ -627,3 +681,7 @@  Debugging Options
    +---+------------+-------------------------------------------------------+
    | 2 | NPC        | --log-level='pmd\.net.cnxk\.flow,8'                   |
    +---+------------+-------------------------------------------------------+
+   | 3 | REP        | --log-level='pmd\.net.cnxk\.rep,8'                   |
+   +---+------------+-------------------------------------------------------+
+   | 4 | ESW        | --log-level='pmd\.net.cnxk\.esw,8'                   |
+   +---+------------+-------------------------------------------------------+
diff --git a/doc/guides/nics/features/cnxk.ini b/doc/guides/nics/features/cnxk.ini
index 94e7a6ab8d..88d5aaaa4e 100644
--- a/doc/guides/nics/features/cnxk.ini
+++ b/doc/guides/nics/features/cnxk.ini
@@ -73,6 +73,8 @@  mpls                 = Y
 nvgre                = Y
 pppoes               = Y
 raw                  = Y
+represented_port     = Y
+port_representor     = Y
 sctp                 = Y
 tcp                  = Y
 tx_queue             = Y
@@ -96,6 +98,7 @@  pf                   = Y
 port_id              = Y
 queue                = Y
 represented_port     = Y
+port_representor     = Y
 rss                  = Y
 sample               = Y
 security             = Y
diff --git a/doc/guides/nics/features/cnxk_vf.ini b/doc/guides/nics/features/cnxk_vf.ini
index 53aa2a3d0c..7d7a1cad1b 100644
--- a/doc/guides/nics/features/cnxk_vf.ini
+++ b/doc/guides/nics/features/cnxk_vf.ini
@@ -64,6 +64,8 @@  mpls                 = Y
 nvgre                = Y
 pppoes               = Y
 raw                  = Y
+represented_port     = Y
+port_representor     = Y
 sctp                 = Y
 tcp                  = Y
 tx_queue             = Y
@@ -85,6 +87,8 @@  of_set_vlan_pcp      = Y
 of_set_vlan_vid      = Y
 pf                   = Y
 queue                = Y
+represented port     = Y
+port_representor     = Y
 rss                  = Y
 security             = Y
 skip_cman            = Y