Re: [dpdk-dev] [PATCH v2 4/5] doc: add documentation for szedata2 PMD

["Mcnamara, John" <john.mcnamara@intel.com>]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Matej Vido
> Sent: Friday, September 18, 2015 9:33 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2 4/5] doc: add documentation for szedata2 PMD
> 
> Signed-off-by: Matej Vido <matejvido@gmail.com>
> Reviewed-by: Jan Viktorin <viktorin@rehivetech.com>

Hi,

Thanks for the new PMD and the documentation.

The DPDJ documentation guidelines provide some guidance on how to format and build the DPDK documentation:

    http://dpdk.org/doc/guides/contributing/documentation.html

In terms of documentation for a PMD the Mellanox MLX4 docs are a good example:

    http://dpdk.org/doc/guides/nics/mlx4.html

Some other comments below:




> +SZEDATA2 PMD
> +============

No need for an acronym in the title. Follow the MLX4 example and use something like " SZEDATA2 poll mode driver library".



> +SZEDATA2 PMD is virtual PMD which uses sze2 layer to communicate with
> +COMBO cards (COMBO-80G, COMBO-100G) using interface provided by libsze2
> library.

The introduction can be brief but should include enough information to explain the PMD for someone encountering it for the first time.

Some links would help with this. See the MLX4 doc as an example:


> +
> +.. note::
> +
> +   This driver has external dependencies. Therefore it is disabled in
> default
> +   configuration files. It can be enabled by setting
> CONFIG_RTE_LIBRTE_PMD_SZEDATA2=y

Format configuration items as verbatim text with  double backquotes:`` CONFIG_RTE_LIBRTE_PMD_SZEDATA2=y ``


> +
> +Using PMD
> +---------

Better: Using the Szedata PMD



> +
> +SZEDATA2 PMD can be created by passing --vdev= option to EAL in the

Again format --vdev as ``--vdev``. Also apply to other similar cases.



> +following format:
> +
> +.. code-block:: console
> +
> +    --vdev
> 'DEVICE_NAME,dev_path=PATH_TO_SZEDATA2_DEVICE,rx_ifaces=RX_MASK,tx_ifaces=
> TX_MASK'

This code line exceeds 80 chars and will exceed the page width in the PDF docs. If possible see if it can be shortened. Maybe use DEVICE instead of DEVICE_NAME and PATH instead of PATH_TO. The next section has an explanation anyway.


> +
> +DEVICE_NAME and options dev_path, rx_ifaces, tx_ifaces are mandatory
> +and must be separated by commas.

Use the ```` quotes again.


> +
> +*   DEVICE_NAME: contains prefix eth_szedata2 followed by numbers or
> letters,
> +    must be unique for each virtual device
> +
> +*   dev_path: Defines path to szedata2 device.
> +    Value is valid path to szedata2 device.
> +
> +        dev_path=/dev/szedataII0

Code blocks should be prefixed by :: to render them correctly. See the guidelines and also view the output from "make doc-guides-html".

Same for the other items below.


> +
> +Example of usage
> +^^^^^^^^^^^^^^^^

The convention is to use ~~~~~ for third level headings (see the guidelines). However, this could probably be a second level heading.


> +
> +Read packets from 0. and 1. receive channel and write them to 0. and 1.
> +transmit channel
> +
> +.. code-block:: console
> +
> +    $RTE_TARGET/app/testpmd -c 0xf -n 2 --vdev
> + 'eth_szedata20,dev_path=/dev/szedataII0,rx_ifaces=0x3,tx_ifaces=0x3'
> + -- --port-topology=chained --rxq=2 --txq=2 --nb-cores=2


Again this code line is >80 chars and won't look good in the PDF. I'd suggest the following:

testpmd -c 0xf -n 2 \
 --vdev 'eth_szedata20,dev_path=/dev/szedataII0,rx_ifaces=3,tx_ifaces=3' \
 -- --port-topology=chained --rxq=2 --txq=2 --nb-cores=2



> diff --git a/doc/guides/prog_guide/source_org.rst
> b/doc/guides/prog_guide/source_org.rst
> index ae11b3b..2393002 100644
> --- a/doc/guides/prog_guide/source_org.rst
> +++ b/doc/guides/prog_guide/source_org.rst
> @@ -106,6 +106,7 @@ The drivers directory has a *net* subdirectory which
> contains::
>      +-- null               # NULL poll mode driver for testing
>      +-- pcap               # PCAP poll mode driver
>      +-- ring               # Ring poll mode driver
> +    +-- szedata2           # szedata2 poll mode driver
>      +-- virtio             # Virtio poll mode driver
>      +-- vmxnet3            # VMXNET3 poll mode driver
>      +-- xenvirt            # Xen virtio poll mode driver

It isn't necessary to update this section. It is meant to be representative rather than comprehensive (at least to me). However if you leave it in then captilize the name like the others.


Thanks,

John