Hi Thomas,
Thanks for reviewing.
Please find responses inline.
Thanks
Harman
> -----Original Message-----
> From: Thomas Monjalon <tho...@monjalon.net>
> Sent: Wednesday, December 20, 2023 3:08 PM
> To: Harman Kalra <hka...@marvell.com>
> Cc: Nithin Kumar Dabilpuram <ndabilpu...@marvell.com>; Kiran Kumar
> Kokkilagadda <kirankum...@marvell.com>; Sunil Kumar Kori
> <sk...@marvell.com>; Satha Koteswara Rao Kottidi
> <skotesh...@marvell.com>; dev@dpdk.org; Jerin Jacob Kollanukkaran
> <jer...@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
>
>
