Re: [dpdk-dev] [PATCH v3 1/4] ena: Amazon ENA documentation

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

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jan Medala
> Sent: Monday, February 22, 2016 7:27 PM
> To: dev@dpdk.org
> Cc: matua@amazon.com; Evgeny Schemeilin <evgenys@amazon.com>
> Subject: [dpdk-dev] [PATCH v3 1/4] ena: Amazon ENA documentation

Hi,

Thanks for this.

For reference the DPDK Documentation Guidelines are here:

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

Some general comments below:

> ---
>  doc/guides/nics/ena.rst | 238

You will need to add the new doc to the index file in the same dir
in order for it to build and be included in the overall docs:

    doc/guides/nics/index.rst

You can build and test the docs as follows:

    make -j doc-guides-html
    firefox build/doc/html/guides/nics/ena.html
    
    make -j doc-guides-pdf
    mupdf build/doc/pdf/guides/nics.pdf


> +ENA Poll Mode Driver
> +=====================

The underline should be the same length as the title and should be
followed by one blank line to separate it from the text. Most titles
in the doc need to be fixed like this.


> +ENA PMD is the DPDK poll-mode driver for the Amazon Elastic Network
> +Adapter (ENA) family.

Maybe better as:

  The ENA PMD is a DPDK poll-mode driver for the Amazon Elastic
  Network Adapter (ENA) family.

Also, you should insert a short description of ENA or link to external
documentation here for people not familiar with ENA.

> +
> +Overview
> +--------
> +The ENA driver exposes a lightweight management interface with a
> +minimal set of memory mapped registers and extendable command set

s/extendable/an extendable/

> +through an Admin Queue.
> +
> +The driver supports a wide range of ENA adapter, is link-speed

s/adapter/adapters/


> +independent (i.e., the same driver is used for 10GbE, 25GbE, 40GbE,
> +etc.), and it negotiates and supports extendable feature set.

s/extendable/an extendable/


> +
> +Refer to ena_admin_defs.h for the list of supported Get/Set Feature
> +properties.

Use ```` quotes for filenames, variable, paths, etc, here and elsewhere: ``ena_admin_defs.h``


> +    This is the requested size of receive/transmit queues, while the
> actual size will be the minimum between the requested size and the maximal
> receive/transmit supported by the device.

This line, and the previous one, are very long in relation to the rest of the
lines in the document. Best to make it consistent.



> +How to build the suite?
> +-----------------------
> +The build instructions for the DPDK suite should as follows:
> +
> +By default the ENA PMD library will be built into the DPDK library.
> +
> +For configuring and using UIO and VFIO frameworks, please refer the
> +documentation that comes with DPDK suite.

I'd suggest replacing this section with something like:

    Building DPDK
    -------------
    
    See the :ref:`DPDK Getting Started Guide for Linux <linux_gsg>` for
    instructions on how to build DPDK.
    
    By default the ENA PMD library will be built into the DPDK library.
    
    For configuring and using UIO and VFIO frameworks, also refer to the
    "Getting Started Guide".


> +
> +Supported ENA adapters
> +----------------------------
> +Current ENA PMD supports the following ENA adapters including:
> +
> +- 1d0f:ec20 - ENA VF
> +- 1d0f:1ec2 - ENA LLQ VF

Quote the addresses here:

    - ``1d0f:ec20`` - ENA VF
    - ``1d0f:1ec2`` - ENA LLQ VF



> +
> +Supported Operating Systems
> +---------------------------
> +Any Linux distribution fulfilling the conditions described in
> +Dependencies section of DPDK documentation.

s/DPDK/the DPDK/ and maybe add a link.


> +
> +Supported features
> +------------------
> +- Jumbo frames up to 9K
> +- Port Hardware Statistics
> +- IPv4/TCP/UDP checksum offload
> +- TSO offload
> +- Multiple receive and transmit queues
> +- RSS
> +- Low Latency Queue for Tx
> +
> +Known bugs and Unsupported features in this release
> +---------------------------------------------------

It would better to just call this section "Unsupported features".


> +Contact Information
> +-------------------
> +Any questions or bugs should be reported to DPDK community and to the
> +ENA PMD
> +maintainers:
> +
> +- Jan Medala <jan@semihalf.com>
> +- Jakub Palider <jpa@semihalf.com>
> +- Netanel Belgazal <netanel@amazon.com>
> +- Evgeny Schemeilin <evgenys@amazon.com>

I would suggest omitting this section and just adding all of these names
to the MAINTAINERS file (if not already done in another patch).


Thanks,

John.