Re: [dpdk-dev] [PATCH v6 2/2] doc: add user-space ethtool sample app guide

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

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Remy Horton
> Sent: Friday, November 6, 2015 4:11 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v6 2/2] doc: add user-space ethtool sample app
> guide

Hi,

Thanks for this. A few minor comments below:


> +EthTool Sample Application
> +==========================
> +
> +The Ethtool sample application shows an implementation of an
> +EthTool-like API and provides a console environment that allows its use
> +to query and change Ethernet card parameters. The sample is based upon
> +a simple L2 frame reflector.

In some place the app is called EthTool and in some Ethtool. Use one or the other.

Although "EthTool-like" should probably be "ethtool-like" since it is like "ethtool"
and that is usually written in lowercase. 


> +
> +Compiling the Application
> +-------------------------
> +
> +To compile the application:
> +
> +#.  Go to the sample application directory:
> +
> +    .. code-block:: console
> +
> +        export RTE_SDK=/path/to/rte_sdk cd ${RTE_SD}/examples/ethtool

Split the two commands onto two lines.


> +Running the Application
> +-----------------------
> +
> +The application requires an available core for each port, plus one.
> +The only available options are the standard ones for the EAL:
> +
> +.. code-block:: console
> +
> +    ./ethtool-app/ethtool-app/${RTE_TARGET}/ethtool [EAL options]
> +
> +Refer to the *DPDK Getting Started Guide* for general information on
> +running applications and the Environment Abstraction Layer (EAL)
> +options.
> +
> +Using the application
> +---------------------
> +
> +The application is console-driven using the cmdline DPDK interface:
> +
> +.. code-block:: console
> +
> +        EthApp>
> +
> +From this interface the available commands and descriptions of what
> +they do as as follows:
> +
> +drvinfo
> +        Print driver info
> +eeprom
> +        Dump EEPROM to file

This definition list doesn't render very well. Maybe better as:

* ``drvinfo``: Print driver info.
* ``eeprom``: Dump EEPROM to file.
* ``link``: Print port link states.
* ``macaddr``: Gets/sets MAC address.
* ``mtu``: Set NIC MTU.
* ``open``: Open port.
* ``pause``: Get/set port pause state.
* ``portstats``: Print port statistics.
* ``regs``: Dump port register(s) to file.
* ``ringparam``: Get/set ring parameters.
* ``rxmode``: Toggle port Rx mode.
* ``stop``: Stop port.
* ``validate``: Check that given MAC address is valid unicast address.
* ``vlan``: Add/remove VLAN id.
* ``quit``: Exit program.



> +Explaination
> +------------

Typo here:

    Explanation
    -----------


> +EthTool interface
> +-----------------
> +
> +The EthTool interface is built as a separate library, and implements
> +the following functions:
> +
> +- rte_ethtool_get_drvinfo
> +- rte_ethtool_get_regs_len
> +- rte_ethtool_get_regs


Maybe better as fixed width text:

- ``rte_ethtool_get_drvinfo()``
- ``rte_ethtool_get_regs_len()``
- ``rte_ethtool_get_regs()``
- ``rte_ethtool_get_link()``
- ``rte_ethtool_get_eeprom_len()``
- ``rte_ethtool_get_eeprom()``
- ``rte_ethtool_set_eeprom()``
- ``rte_ethtool_get_pauseparam()``
- ``rte_ethtool_set_pauseparam()``
- ``rte_ethtool_net_open()``
- ``rte_ethtool_net_stop()``
- ``rte_ethtool_net_get_mac_addr()``
- ``rte_ethtool_net_set_mac_addr()``
- ``rte_ethtool_net_validate_addr()``
- ``rte_ethtool_net_change_mtu()``
- ``rte_ethtool_net_get_stats64()``
- ``rte_ethtool_net_vlan_rx_add_vid()``
- ``rte_ethtool_net_vlan_rx_kill_vid()``
- ``rte_ethtool_net_set_rx_mode()``
- ``rte_ethtool_get_ringparam()``
- ``rte_ethtool_set_ringparam()``


John.
--