Re: [dpdk-dev] [PATCH v2] doc: Merge l3fwd and l3fwd-acl documentation files

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

> -----Original Message-----
> From: Ravi Kerur [mailto:rkerur@gmail.com]
> Sent: Monday, May 8, 2017 10:00 PM
> To: dev@dpdk.org
> Cc: Mcnamara, John <john.mcnamara@intel.com>; thomas@monjalon.net; Ravi
> Kerur <rkerur@gmail.com>
> Subject: [PATCH v2] doc: Merge l3fwd and l3fwd-acl documentation files
> 
> Merge l3fwd and l3fwd-acl documentation to reflect the code.
> Add examples of LPM and Exact Match rules in .svg files.
> 
> Signed-off-by: Ravi Kerur <rkerur@gmail.com>
> ---
> v2:
> 	Remove binary PNG from .svg files.
> 
> v1:
> 	Merge relevant contents of l3fwd and l3fwd-acl documentation.
> 	Modify l3fwd document with ACL specific information.
> 	Remove l3fwd-acl documentation file.
> ---
>  doc/guides/sample_app_ug/img/ipv4_hash_rule.svg    | 108 ++++++
>  doc/guides/sample_app_ug/img/ipv4_lpm_rule.svg     |  96 +++++
>  doc/guides/sample_app_ug/index.rst                 |   1 -
>  doc/guides/sample_app_ug/l3_forward.rst            | 326
> ++++++++++++++++-
>  .../sample_app_ug/l3_forward_access_ctrl.rst       | 385 ----------------
> -----
>  5 files changed, 521 insertions(+), 395 deletions(-)  create mode 100644
> doc/guides/sample_app_ug/img/ipv4_hash_rule.svg
>  create mode 100644 doc/guides/sample_app_ug/img/ipv4_lpm_rule.svg
> 


Thanks once more for the doc. Some comments below.


> ...
>      Redistribution and use in source and binary forms, with or without @@
> -31,12 +31,18 @@
>  L3 Forwarding Sample Application
>  ================================
> 
> -The L3 Forwarding application is a simple example of packet processing
> using the DPDK.
> -The application performs L3 forwarding.
> +The L3 Forwarding with Hash, LPM or Access Control application is a
> simple example of packet processing using the DPDK.
> +The application performs L3 forwarding when Hash or LPM is used.
> +The application performs a security check on received packets when Access
> Control is used.
> +Packets that are in the Access Control List (ACL), which is loaded during
> initialization, are dropped.
> +Others are forwarded to the correct port.


For the introduction I would suggest the following rewording:

L3 Forwarding Sample Application
================================

The L3 Forwarding application is a simple example of packet processing using
the DPDK with Hash, LPM (Longest Prefix match) or Access Control.

The application performs L3 forwarding when Hash or LPM is used.
Alternatively, the application performs a security check on received packets
when Access Control is used.  Packets that are in the Access Control List
(ACL), which is loaded during initialization, are dropped.  Others packets are
forwarded to the correct port.




>  The main difference from the L2 Forwarding sample application is that the
> forwarding decision @@ -49,15 +55,263 @@ The hash object is used in
> correlation with a flow table to map each input packe  The hash lookup key
> is represented by a DiffServ 5-tuple composed of the following fields read
> from the input packet:
>  Source IP Address, Destination IP Address, Protocol, Source Port and
> Destination Port.

This is fine but would be a bit clearer as a list:


The hash lookup key is represented by a 5-tuple composed of the following fields read from the input packet:

* Source IP Address.
* Destination IP Address.
* Protocol.
* Source Port.
* Destination Port.



> +
> +.. _figure_ipv4_lpm_rule:
> +
> +.. figure:: img/ipv4_lpm_rule.*
> +
> +   A typical IPv4 LPM rule
> +
> +*   A typical IPv4 hash rule line should have a format as shown below:
> +
> +
> +.. _figure_ipv4_hash_rule:
> +
> +.. figure:: img/ipv4_hash_rule.*
> +
> +   A typical IPv4 hash rule
> +


Unfortunately these figures don't render very well. I would suggest replacing
them with text, like below:


A typical IPv4 LPM rule line in an input file would look like the following::


    # Dest Address/mask   Eth port
    # =================   ========

    L100.10.10.10/24      0


A typical IPv4 Hash rule line in an input file would look like the following::

    # Src Address  Dst Address  Src Port  Dst port  Protocol  Eth port
    # ===========  ===========  ========  ========  ========  ========

    E100.10.10.0  100.10.0.0    110       111       0x6       0


There a few other thing that could be improved in the doc but these issues were
already there in text that you are merging so apart from these suggestion
the rest of the doc is okay for now. We should try to come back and improve
the consistency at a later date.

John