Re: [dpdk-dev] [PATCH v4] net/af_packet: reinsert the stripped vlan tag

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 9/29/2021 3:08 PM, Tudor Cornea wrote:
> The af_packet pmd driver binds to a raw socket and allows
> sending and receiving of packets through the kernel.
> 
> Since commit [1], the kernel strips the vlan tags early in
> __netif_receive_skb_core(), so we receive untagged packets while
> running with the af_packet pmd.
> 
> Luckily for us, the skb vlan-related fields are still populated from the
> stripped vlan tags, so we end up having all the information
> that we need in the mbuf.
> 
> Having the pmd driver support DEV_RX_OFFLOAD_VLAN_STRIP allows the
> application to control the desired vlan stripping behavior,
> until we have a way to describe offloads that can't be disabled by
> pmd drivers.
> 
> This patch will cause a change in the default way that the af_packet
> pmd treats received vlan-tagged frames. While previously, the
> application was required to check the PKT_RX_VLAN_STRIPPED flag, after
> this patch, the pmd will re-insert the vlan tag transparently to the
> user, unless the DEV_RX_OFFLOAD_VLAN_STRIP is enabled in
> rxmode.offloads.
> 
> I've attempted a preliminary benchmark to understand if the change could
> cause a sizable performance hit.
> 
> Setup:
> Two virtual machines running on top of an ESXi hypervisor
> 
> Tx: DPDK app (running on top of vmxnet3 PMD)
> Rx: af_packet (running on top of a kernel vmxnet3 interface)
> Packet size :68 (packet contains a vlan tag)
> 
> Rates:
> Tx - 1.419 Mpps
> Rx (without vlan insertion) - 1227636 pps
> Rx (with vlan insertion)    - 1220081 pps
> 
> At a first glance, we don't seem to have a large degradation in terms
> of packet rate.
> 
> [1] https://github.com/torvalds/linux/commit/bcc6d47903612c3861201cc3a866fb604f26b8b2
> 
> Signed-off-by: Tudor Cornea <tudor.cornea@gmail.com>
> 
> ---
> v4:
> * Updated the af_packet documentation
> v3:
> * Updated release note and documentation
> * Updated commit with performance measurements
> v2:
> * Added DEV_RX_OFFLOAD_VLAN_STRIP to rxmode->offloads
> ---
>  doc/guides/nics/af_packet.rst             |  5 +++++
>  doc/guides/rel_notes/release_21_11.rst    |  4 ++++
>  drivers/net/af_packet/rte_eth_af_packet.c | 12 ++++++++++++
>  3 files changed, 21 insertions(+)
> 
> diff --git a/doc/guides/nics/af_packet.rst b/doc/guides/nics/af_packet.rst
> index efd6f1c..c87310b 100644
> --- a/doc/guides/nics/af_packet.rst
> +++ b/doc/guides/nics/af_packet.rst
> @@ -65,3 +65,8 @@ framecnt=512):
>  .. code-block:: console
>  
>      --vdev=eth_af_packet0,iface=tap0,blocksz=4096,framesz=2048,framecnt=512,qpairs=1,qdisc_bypass=0
> +
> +Features and Limitations of the af_packet PMD
> +---------------------------------------------
> +
> +Af_packet PMD now works with VLAN's on Linux

Thanks Tudor.

I see this is suggested by Stephen, but I think 'now' is confusing in the driver
guide. Also driver was already working with VLAN, the change is VLAN is not
force stripped anymore.

I think following part from your previous version is better, what do you think
something like following?
"
The PMD will re-insert the VLAN tag transparently to the packet
if the kernel strips it, as long as the ``DEV_RX_OFFLOAD_VLAN_STRIP`` is not
enabled by application.
"

> diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
> index ad7c1af..095fd5b 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -66,6 +66,10 @@ New Features
>  
>    * Added rte_flow support for dual VLAN insert and strip actions.
>  
> +* **Updated af_packet ethdev driver.**
> +
> +  * Added DEV_RX_OFFLOAD_VLAN_STRIP capability.
> +

I think change in the default behavior is more important in the release notes,
again what do you think to have your following update here:

"
Default VLAN strip behavior changed. If previously,
the vlan tag was stripped, if the application now requires the same behavior,
it will need to configure ``DEV_RX_OFFLOAD_VLAN_STRIP``.
"