On 9/8/2021 9:59 AM, 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.
>
> [1]
> https://github.com/torvalds/linux/commit/bcc6d47903612c3861201cc3a866fb604f26b8b2
>
> Signed-off-by: Tudor Cornea <tudor.cor...@gmail.com>
>
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
> ---
> 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
> 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)
> 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);
> +
> + if (!pkt_q->vlan_strip && rte_vlan_insert(&mbuf))
> + PMD_LOG(ERR, "Failed to reinsert VLAN tag");
> }
>
> /* 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);
> return 0;
> }
>
> @@ -318,6 +328,7 @@ eth_dev_info(struct rte_eth_dev *dev, struct
> rte_eth_dev_info *dev_info)
> 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;
> }
>
