[v2] net/af_packet: reinsert the stripped vlan tag
Checks
Commit Message
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.cornea@gmail.com>
---
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(+)
Comments
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.cornea@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;
> }
>
Thanks, Ferruh
I will perform the suggested recommendations in version 3 of the patch.
On Mon, 20 Sept 2021 at 18:41, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 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.cornea@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;
> > }
> >
>
>
@@ -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;
}