Thanks, Ferruh

I will perform the suggested recommendations in version 3 of the patch.

On Mon, 20 Sept 2021 at 18:41, Ferruh Yigit [off-list ref] wrote:

On 9/8/2021 9:59 AM, Tudor Cornea wrote:

quoted

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.

[1]

https://github.com/torvalds/linux/commit/bcc6d47903612c3861201cc3a866fb604f26b8b2

quoted

Signed-off-by: Tudor Cornea <redacted>

Hi Tudor,

The concern was unexpected performance degradation (user not setting any
offload
will have performance drop). But since your measurements show no
significant
drop, I think it is fair to make driver behave same as other drivers.
(Until we have a way to describe offloads that can't be disabled by PMDs.)

Can you do a few minor updates:
- Put your performance measurements into to the commit log to record them
- Update the af_packet documentation (doc/guides/nics/af_packet.rst) to
document
PMD behavior with packets with VLAN tag
- Update release note (doc/guides/rel_notes/release_21_11.rst) with a
one/two
sentences to document the change, to notify possible users of the
af_packet with
the change.

Thanks,
ferruh

quoted

---
v2:
* Add DEV_RX_OFFLOAD_VLAN_STRIP to rxmode->offloads
---
 drivers/net/af_packet/rte_eth_af_packet.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/af_packet/rte_eth_af_packet.c

b/drivers/net/af_packet/rte_eth_af_packet.c

quoted

index b73b211..5ed9dd6 100644

--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c

@@ -48,6 +48,7 @@ struct pkt_rx_queue {

      struct rte_mempool *mb_pool;
      uint16_t in_port;
+     uint8_t vlan_strip;

      volatile unsigned long rx_pkts;
      volatile unsigned long rx_bytes;

@@ -78,6 +79,7 @@ struct pmd_internals {

      struct pkt_rx_queue *rx_queue;
      struct pkt_tx_queue *tx_queue;
+     uint8_t vlan_strip;
 };

 static const char *valid_arguments[] = {

@@ -148,6 +150,9 @@ eth_af_packet_rx(void *queue, struct rte_mbuf

**bufs, uint16_t nb_pkts)

quoted

              if (ppd->tp_status & TP_STATUS_VLAN_VALID) {
                      mbuf->vlan_tci = ppd->tp_vlan_tci;
                      mbuf->ol_flags |= (PKT_RX_VLAN |

PKT_RX_VLAN_STRIPPED);

quoted

+
+                     if (!pkt_q->vlan_strip && rte_vlan_insert(&mbuf))
+                             PMD_LOG(ERR, "Failed to reinsert VLAN

tag");

quoted

              }

              /* release incoming frame and advance ring buffer */

@@ -302,6 +307,11 @@ eth_dev_stop(struct rte_eth_dev *dev)
 static int
 eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
 {
+     struct rte_eth_conf *dev_conf = &dev->data->dev_conf;
+     const struct rte_eth_rxmode *rxmode = &dev_conf->rxmode;
+     struct pmd_internals *internals = dev->data->dev_private;
+
+     internals->vlan_strip = !!(rxmode->offloads &

DEV_RX_OFFLOAD_VLAN_STRIP);

quoted

      return 0;
 }

@@ -318,6 +328,7 @@ eth_dev_info(struct rte_eth_dev *dev, struct

rte_eth_dev_info *dev_info)

quoted

      dev_info->min_rx_bufsize = 0;
      dev_info->tx_offload_capa = DEV_TX_OFFLOAD_MULTI_SEGS |
              DEV_TX_OFFLOAD_VLAN_INSERT;
+     dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;

      return 0;
 }

@@ -448,6 +459,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,

      dev->data->rx_queues[rx_queue_id] = pkt_q;
      pkt_q->in_port = dev->data->port_id;
+     pkt_q->vlan_strip = internals->vlan_strip;

      return 0;
 }