[dpdk-dev] [PATCH] net/af_packet: try to reinsert the stripped vlan tag

[dpdk-dev] [PATCH] net/af_packet: try to reinsert the stripped vlan tag

[Tudor Cornea]

The af_packet pmd driver binds to a raw socket and allows
sending and receiving of packets through the kernel.

Since commit bcc6d47903 [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.

We would like to have the the vlan tag inside the mbuf.
Let's take a shot at it by trying to reinsert the stripped vlan tag.

As a side note, something similar was done for the netvsc pmd.

[1] https://github.com/torvalds/linux/commit/bcc6d47903612c3861201cc3a866fb604f26b8b2

Signed-off-by: Tudor Cornea <tudor.cornea@gmail.com>
---
 drivers/net/af_packet/rte_eth_af_packet.c | 4 ++++
 1 file changed, 4 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..d116583 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -148,6 +148,10 @@ 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);
+
+			/* the kernel always strips the vlan tag, try to reinsert it */
+			if (rte_vlan_insert(&mbuf))
+				PMD_LOG(ERR, "Failed to reinsert vlan tag");
 		}
 
 		/* release incoming frame and advance ring buffer */
-- 
2.7.4

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

[Ferruh Yigit]

On 8/20/2021 1:46 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 bcc6d47903 [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.
> 
> We would like to have the the vlan tag inside the mbuf.
> Let's take a shot at it by trying to reinsert the stripped vlan tag.
> 

PMD already sets 'mbuf->vlan_tci' and 'PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED'
flags, so application can be aware of the vlan tag and can consume it.

Inserting the vlan tag back to packet is costly, what is the motivation to do so?

> As a side note, something similar was done for the netvsc pmd.
> 
> [1] https://github.com/torvalds/linux/commit/bcc6d47903612c3861201cc3a866fb604f26b8b2
> 
> Signed-off-by: Tudor Cornea <tudor.cornea@gmail.com>
> ---
>  drivers/net/af_packet/rte_eth_af_packet.c | 4 ++++
>  1 file changed, 4 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..d116583 100644
> --- a/drivers/net/af_packet/rte_eth_af_packet.c
> +++ b/drivers/net/af_packet/rte_eth_af_packet.c
> @@ -148,6 +148,10 @@ 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);
> +
> +			/* the kernel always strips the vlan tag, try to reinsert it */
> +			if (rte_vlan_insert(&mbuf))
> +				PMD_LOG(ERR, "Failed to reinsert vlan tag");
>  		}
>  
>  		/* release incoming frame and advance ring buffer */
> 

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

[Tudor Cornea]

Indeed, the vlan insertion could be a costly operation. We should probably
do it only if the user specifically asks to have the vlan tag in the packet.
Otherwise, af_packet PMD users might pay a price in terms of performance
for something they didn't ask for.

I was thinking of avoiding having to change the application in order to
re-insert the vlan tag.
Doing this operation inside the PMD driver seemed like a good fit.

Looking at the netvsc driver (drivers/net/netvsc), the vlan insertion is
guarded by a check to hv->vlan_strip

  if (!hv->vlan_strip && rte_vlan_insert(&m)) {

hv->vlan_strip seems to be initialized in hn_dev_configure() in the
following way

 hv->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);

while 'hv' seems to be stored in rte_eth_dev->data->dev_private

I am thinking of doing something similar for the af_packet PMD.
The 'pmd_internals' structure could potentially hold a field, say
vlan_strip', which could be initialized if the application enables the
DEV_RX_OFFLOAD_VLAN_STRIP in rxmode->offloads

This way, I'm thinking that the application could potentially control the
effect of vlan stripping for the af_packet PMD, in an uniform way, similar
to other PMDs.
Would this be considered an acceptable solution ?




On Tue, 31 Aug 2021 at 18:31, Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 8/20/2021 1:46 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 bcc6d47903 [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.
> >
> > We would like to have the the vlan tag inside the mbuf.
> > Let's take a shot at it by trying to reinsert the stripped vlan tag.
> >
>
> PMD already sets 'mbuf->vlan_tci' and 'PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED'
> flags, so application can be aware of the vlan tag and can consume it.
>
> Inserting the vlan tag back to packet is costly, what is the motivation to
> do so?
>
> > As a side note, something similar was done for the netvsc pmd.
> >
> > [1]
> https://github.com/torvalds/linux/commit/bcc6d47903612c3861201cc3a866fb604f26b8b2
> >
> > Signed-off-by: Tudor Cornea <tudor.cornea@gmail.com>
> > ---
> >  drivers/net/af_packet/rte_eth_af_packet.c | 4 ++++
> >  1 file changed, 4 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..d116583 100644
> > --- a/drivers/net/af_packet/rte_eth_af_packet.c
> > +++ b/drivers/net/af_packet/rte_eth_af_packet.c
> > @@ -148,6 +148,10 @@ 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);
> > +
> > +                     /* the kernel always strips the vlan tag, try to
> reinsert it */
> > +                     if (rte_vlan_insert(&mbuf))
> > +                             PMD_LOG(ERR, "Failed to reinsert vlan
> tag");
> >               }
> >
> >               /* release incoming frame and advance ring buffer */
> >
>
>

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

[Stephen Hemminger]

On Wed, 1 Sep 2021 22:07:22 +0300
Tudor Cornea <tudor.cornea@gmail.com> wrote:

> Indeed, the vlan insertion could be a costly operation. We should probably
> do it only if the user specifically asks to have the vlan tag in the packet.
> Otherwise, af_packet PMD users might pay a price in terms of performance
> for something they didn't ask for.
> 
> I was thinking of avoiding having to change the application in order to
> re-insert the vlan tag.
> Doing this operation inside the PMD driver seemed like a good fit.
> 
> Looking at the netvsc driver (drivers/net/netvsc), the vlan insertion is
> guarded by a check to hv->vlan_strip
> 
>   if (!hv->vlan_strip && rte_vlan_insert(&m)) {
> 
> hv->vlan_strip seems to be initialized in hn_dev_configure() in the
> following way
> 
>  hv->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
> 
> while 'hv' seems to be stored in rte_eth_dev->data->dev_private
> 
> I am thinking of doing something similar for the af_packet PMD.
> The 'pmd_internals' structure could potentially hold a field, say
> vlan_strip', which could be initialized if the application enables the
> DEV_RX_OFFLOAD_VLAN_STRIP in rxmode->offloads
> 
> This way, I'm thinking that the application could potentially control the
> effect of vlan stripping for the af_packet PMD, in an uniform way, similar
> to other PMDs.
> Would this be considered an acceptable solution ?
> 
> 
> 
> 
> On Tue, 31 Aug 2021 at 18:31, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
> > On 8/20/2021 1:46 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 bcc6d47903 [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.
> > >
> > > We would like to have the the vlan tag inside the mbuf.
> > > Let's take a shot at it by trying to reinsert the stripped vlan tag.
> > >  
> >
> > PMD already sets 'mbuf->vlan_tci' and 'PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED'
> > flags, so application can be aware of the vlan tag and can consume it.
> >
> > Inserting the vlan tag back to packet is costly, what is the motivation to
> > do so?
> >  
> > > As a side note, something similar was done for the netvsc pmd.
> > >
> > > [1]  
> > https://github.com/torvalds/linux/commit/bcc6d47903612c3861201cc3a866fb604f26b8b2  
> > >
> > > Signed-off-by: Tudor Cornea <tudor.cornea@gmail.com>

The netvsc PMD has to handle some subtle cases where VLAN stripping
is done by the VF but the slow path over vmbus does not.
Since most traffic goes over the VF path, it makes sense for the
netvsc PMD to advertise and handle VLAN stripping even if it has
to do it in software.

Ferruh is right the mbuf generated by current AF_PACKET PMD is
valid with VLAN stripped correctly. I think you are also correct
that the stripping needs to be controllable by the application.
And yes the kernel always strips the VLAN; there is no option
to tell socket(AF_PACKET) not to do that.

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

[Ferruh Yigit]

On 9/1/2021 10:34 PM, Stephen Hemminger wrote:
> On Wed, 1 Sep 2021 22:07:22 +0300
> Tudor Cornea <tudor.cornea@gmail.com> wrote:
> 
>> Indeed, the vlan insertion could be a costly operation. We should probably
>> do it only if the user specifically asks to have the vlan tag in the packet.
>> Otherwise, af_packet PMD users might pay a price in terms of performance
>> for something they didn't ask for.
>>
>> I was thinking of avoiding having to change the application in order to
>> re-insert the vlan tag.
>> Doing this operation inside the PMD driver seemed like a good fit.
>>
>> Looking at the netvsc driver (drivers/net/netvsc), the vlan insertion is
>> guarded by a check to hv->vlan_strip
>>
>>   if (!hv->vlan_strip && rte_vlan_insert(&m)) {
>>
>> hv->vlan_strip seems to be initialized in hn_dev_configure() in the
>> following way
>>
>>  hv->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
>>
>> while 'hv' seems to be stored in rte_eth_dev->data->dev_private
>>
>> I am thinking of doing something similar for the af_packet PMD.
>> The 'pmd_internals' structure could potentially hold a field, say
>> vlan_strip', which could be initialized if the application enables the
>> DEV_RX_OFFLOAD_VLAN_STRIP in rxmode->offloads
>>
>> This way, I'm thinking that the application could potentially control the
>> effect of vlan stripping for the af_packet PMD, in an uniform way, similar
>> to other PMDs.
>> Would this be considered an acceptable solution ?
>>
>>
>>
>>
>> On Tue, 31 Aug 2021 at 18:31, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>>> On 8/20/2021 1:46 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 bcc6d47903 [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.
>>>>
>>>> We would like to have the the vlan tag inside the mbuf.
>>>> Let's take a shot at it by trying to reinsert the stripped vlan tag.
>>>>  
>>>
>>> PMD already sets 'mbuf->vlan_tci' and 'PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED'
>>> flags, so application can be aware of the vlan tag and can consume it.
>>>
>>> Inserting the vlan tag back to packet is costly, what is the motivation to
>>> do so?
>>>  
>>>> As a side note, something similar was done for the netvsc pmd.
>>>>
>>>> [1]  
>>> https://github.com/torvalds/linux/commit/bcc6d47903612c3861201cc3a866fb604f26b8b2  
>>>>
>>>> Signed-off-by: Tudor Cornea <tudor.cornea@gmail.com>
> 
> The netvsc PMD has to handle some subtle cases where VLAN stripping
> is done by the VF but the slow path over vmbus does not.
> Since most traffic goes over the VF path, it makes sense for the
> netvsc PMD to advertise and handle VLAN stripping even if it has
> to do it in software.
> 
> Ferruh is right the mbuf generated by current AF_PACKET PMD is
> valid with VLAN stripped correctly. I think you are also correct
> that the stripping needs to be controllable by the application.
> And yes the kernel always strips the VLAN; there is no option
> to tell socket(AF_PACKET) not to do that.
> 

When application doesn't set VLAN_STRIP offload, expectation is VLAN tag to be
in the packet and no additional work is done.

But that is not the case for af_packet.
If your change is applied, not requesting any offload, default confing, may
cause unintended work for af_packet, since it will insert the already stripped
vlan tag back.

And we don't have a way to say any specific offload can't be disabled by the
PMD/device, although we hit this case a few times previously.
Proper fix can be adding this support to offloads, but it is more invasive
change. + Andrew, Thomas, Jerin for this discussion.


For short term at least 'DEV_RX_OFFLOAD_VLAN_STRIP' offload should be added to
the af_packet capability.
It is also possible to set this offload in the config by PMD itself even
application doesn't request it, to be correct in the config. Not sure how much
it helps to applications (there is a new API proposed this release to get config
to application, perhaps after configuration step app can request the config and
recognize that VLAN_STRIP offload is set by PMD, but this is some overhead).

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

[Tudor Cornea]

Thanks,

I hope I understood correctly the above comments.

I'm thinking of adding DEV_RX_OFFLOAD_VLAN_STRIP to
dev_info->rx_offload_capa

eth_dev_info()
+dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;

then populating pmd_internals->vlan_strip with the vlan stripping
option that the application requests

eth_dev_configure()
+internals->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);

From the internals structure, we could populate a newly-added field in the
pkt_rx_queue structure 'vlan_strip'

eth_rx_queue_setup()
+pkt_q->vlan_strip = internals->vlan_strip;

And attempt to re-insert the vlan only if required in eth_af_packet_rx

eth_af_packet_rx()
+if (!pkt_q->vlan_strip && rte_vlan_insert(&mbuf))

I've attempted a simple benchmark to understand if the change could cause a
sizable performance hit.

Setup:
Tx: vmxnet3 PMD
Rx: af_packet (running on top of a 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

I don't seem to have a large degradation in terms of packet rate at first
glance, but maybe the experiment could be repeated on different setups as
I'm using a virtual environment.

Would it be reasonable if I send v2 of the patch for review, with the above
changes ?


On Thu, 2 Sept 2021 at 13:49, Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 9/1/2021 10:34 PM, Stephen Hemminger wrote:
> > On Wed, 1 Sep 2021 22:07:22 +0300
> > Tudor Cornea <tudor.cornea@gmail.com> wrote:
> >
> >> Indeed, the vlan insertion could be a costly operation. We should
> probably
> >> do it only if the user specifically asks to have the vlan tag in the
> packet.
> >> Otherwise, af_packet PMD users might pay a price in terms of performance
> >> for something they didn't ask for.
> >>
> >> I was thinking of avoiding having to change the application in order to
> >> re-insert the vlan tag.
> >> Doing this operation inside the PMD driver seemed like a good fit.
> >>
> >> Looking at the netvsc driver (drivers/net/netvsc), the vlan insertion is
> >> guarded by a check to hv->vlan_strip
> >>
> >>   if (!hv->vlan_strip && rte_vlan_insert(&m)) {
> >>
> >> hv->vlan_strip seems to be initialized in hn_dev_configure() in the
> >> following way
> >>
> >>  hv->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
> >>
> >> while 'hv' seems to be stored in rte_eth_dev->data->dev_private
> >>
> >> I am thinking of doing something similar for the af_packet PMD.
> >> The 'pmd_internals' structure could potentially hold a field, say
> >> vlan_strip', which could be initialized if the application enables the
> >> DEV_RX_OFFLOAD_VLAN_STRIP in rxmode->offloads
> >>
> >> This way, I'm thinking that the application could potentially control
> the
> >> effect of vlan stripping for the af_packet PMD, in an uniform way,
> similar
> >> to other PMDs.
> >> Would this be considered an acceptable solution ?
> >>
> >>
> >>
> >>
> >> On Tue, 31 Aug 2021 at 18:31, Ferruh Yigit <ferruh.yigit@intel.com>
> wrote:
> >>
> >>> On 8/20/2021 1:46 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 bcc6d47903 [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.
> >>>>
> >>>> We would like to have the the vlan tag inside the mbuf.
> >>>> Let's take a shot at it by trying to reinsert the stripped vlan tag.
> >>>>
> >>>
> >>> PMD already sets 'mbuf->vlan_tci' and 'PKT_RX_VLAN |
> PKT_RX_VLAN_STRIPPED'
> >>> flags, so application can be aware of the vlan tag and can consume it.
> >>>
> >>> Inserting the vlan tag back to packet is costly, what is the
> motivation to
> >>> do so?
> >>>
> >>>> As a side note, something similar was done for the netvsc pmd.
> >>>>
> >>>> [1]
> >>>
> https://github.com/torvalds/linux/commit/bcc6d47903612c3861201cc3a866fb604f26b8b2
>
> >>>>
> >>>> Signed-off-by: Tudor Cornea <tudor.cornea@gmail.com>
> >
> > The netvsc PMD has to handle some subtle cases where VLAN stripping
> > is done by the VF but the slow path over vmbus does not.
> > Since most traffic goes over the VF path, it makes sense for the
> > netvsc PMD to advertise and handle VLAN stripping even if it has
> > to do it in software.
> >
> > Ferruh is right the mbuf generated by current AF_PACKET PMD is
> > valid with VLAN stripped correctly. I think you are also correct
> > that the stripping needs to be controllable by the application.
> > And yes the kernel always strips the VLAN; there is no option
> > to tell socket(AF_PACKET) not to do that.
> >
>
> When application doesn't set VLAN_STRIP offload, expectation is VLAN tag
> to be
> in the packet and no additional work is done.
>
> But that is not the case for af_packet.
> If your change is applied, not requesting any offload, default confing, may
> cause unintended work for af_packet, since it will insert the already
> stripped
> vlan tag back.
>
> And we don't have a way to say any specific offload can't be disabled by
> the
> PMD/device, although we hit this case a few times previously.
> Proper fix can be adding this support to offloads, but it is more invasive
> change. + Andrew, Thomas, Jerin for this discussion.
>
>
> For short term at least 'DEV_RX_OFFLOAD_VLAN_STRIP' offload should be
> added to
> the af_packet capability.
> It is also possible to set this offload in the config by PMD itself even
> application doesn't request it, to be correct in the config. Not sure how
> much
> it helps to applications (there is a new API proposed this release to get
> config
> to application, perhaps after configuration step app can request the
> config and
> recognize that VLAN_STRIP offload is set by PMD, but this is some
> overhead).
>

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

[Tudor Cornea]

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(+)

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;
 }
-- 
2.7.4

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

[Ferruh Yigit]

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;
>  }
> 

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

[Tudor Cornea]

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;
> >  }
> >
>
>

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

[Tudor Cornea]

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>

---
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             | 38 +++++++++++++++++++++++++++++++
 doc/guides/rel_notes/release_21_11.rst    |  4 ++++
 drivers/net/af_packet/rte_eth_af_packet.c | 12 ++++++++++
 3 files changed, 54 insertions(+)

diff --git a/doc/guides/nics/af_packet.rst b/doc/guides/nics/af_packet.rst
index efd6f1c..97d5502 100644
--- a/doc/guides/nics/af_packet.rst
+++ b/doc/guides/nics/af_packet.rst
@@ -65,3 +65,41 @@ 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
+---------------------------------------------
+
+Since the following commit, the Linux kernel strips the vlan tag
+
+.. code-block:: console
+
+    commit bcc6d47903612c3861201cc3a866fb604f26b8b2
+    Author: Jiri Pirko <jpirko@xxxxxxxxxx>
+    Date:   Thu Apr 7 19:48:33 2011 +0000
+
+     net: vlan: make non-hw-accel rx path similar to hw-accel
+
+Running on such a kernel results in receiving untagged frames while using
+the af_packet PMD. Fortunately, the stripped information is still available
+for use in ``mbuf->vlan_tci``, and applications could check ``PKT_RX_VLAN_STRIPPED``.
+
+However, we currently don't have a way to describe offloads which can't be
+disabled by PMDs, and this creates an inconsistency with the way applications
+expect the PMD offloads to work, and requires them to be aware of which
+underlying driver they use.
+
+Since release 21.11 the af_packet PMD will implement support for the
+``DEV_RX_OFFLOAD_VLAN_STRIP`` offload, and users can control the desired vlan
+stripping behavior.
+
+It's important to note that the default case will change. If previously,
+the vlan tag was stripped, if the application now requires the same behavior,
+it will need to configure ``rxmode.offloads`` with ``DEV_RX_OFFLOAD_VLAN_STRIP``.
+
+The PMD driver will re-insert the vlan tag transparently to the application
+if the kernel strips it, as long as the ``DEV_RX_OFFLOAD_VLAN_STRIP`` is not
+enabled.
+
+.. code-block:: console
+
+    port_conf.rxmode.offloads |= DEV_RX_OFFLOAD_VLAN_STRIP
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.
+
 * **Updated Marvell cnxk crypto PMD.**
 
   * Added AES-CBC SHA1-HMAC support in lookaside protocol (IPsec) for CN10K.
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;
 }
-- 
2.7.4

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

[Stephen Hemminger]

On Fri, 24 Sep 2021 14:44:45 +0300
Tudor Cornea <tudor.cornea@gmail.com> wrote:

> +Features and Limitations of the af_packet PMD
> +---------------------------------------------
> +
> +Since the following commit, the Linux kernel strips the vlan tag
> +
> +.. code-block:: console
> +
> +    commit bcc6d47903612c3861201cc3a866fb604f26b8b2
> +    Author: Jiri Pirko <jpirko@xxxxxxxxxx>
> +    Date:   Thu Apr 7 19:48:33 2011 +0000
> +
> +     net: vlan: make non-hw-accel rx path similar to hw-accel
> +
> +Running on such a kernel results in receiving untagged frames while using
> +the af_packet PMD. Fortunately, the stripped information is still available
> +for use in ``mbuf->vlan_tci``, and applications could check ``PKT_RX_VLAN_STRIPPED``.
> +
> +However, we currently don't have a way to describe offloads which can't be
> +disabled by PMDs, and this creates an inconsistency with the way applications
> +expect the PMD offloads to work, and requires them to be aware of which
> +underlying driver they use.
> +
> +Since release 21.11 the af_packet PMD will implement support for the
> +``DEV_RX_OFFLOAD_VLAN_STRIP`` offload, and users can control the desired vlan
> +stripping behavior.
> +
> +It's important to note that the default case will change. If previously,
> +the vlan tag was stripped, if the application now requires the same behavior,
> +it will need to configure ``rxmode.offloads`` with ``DEV_RX_OFFLOAD_VLAN_STRIP``.
> +
> +The PMD driver will re-insert the vlan tag transparently to the application
> +if the kernel strips it, as long as the ``DEV_RX_OFFLOAD_VLAN_STRIP`` is not
> +enabled.

This seems like the wrong place for this text.
It is not a new feature, it is a bug fix.

If you want to describe it as an enhancement, the text should be succinct and describe
the situation from user point of view. Something like:

    Af_packet PMD now works with VLAN's on Linux

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

[Tudor Cornea]

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
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.
+
 * **Updated Marvell cnxk crypto PMD.**
 
   * Added AES-CBC SHA1-HMAC support in lookaside protocol (IPsec) for CN10K.
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;
 }
-- 
2.7.4

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

[Tudor Cornea]

Thanks Stephen, for the suggestion

I've sent v4 of the patch, which adds the succinct description in the
af_packet documentation..


On Fri, 24 Sept 2021 at 18:11, Stephen Hemminger <stephen@networkplumber.org>
wrote:

> On Fri, 24 Sep 2021 14:44:45 +0300
> Tudor Cornea <tudor.cornea@gmail.com> wrote:
>
> > +Features and Limitations of the af_packet PMD
> > +---------------------------------------------
> > +
> > +Since the following commit, the Linux kernel strips the vlan tag
> > +
> > +.. code-block:: console
> > +
> > +    commit bcc6d47903612c3861201cc3a866fb604f26b8b2
> > +    Author: Jiri Pirko <jpirko@xxxxxxxxxx>
> > +    Date:   Thu Apr 7 19:48:33 2011 +0000
> > +
> > +     net: vlan: make non-hw-accel rx path similar to hw-accel
> > +
> > +Running on such a kernel results in receiving untagged frames while
> using
> > +the af_packet PMD. Fortunately, the stripped information is still
> available
> > +for use in ``mbuf->vlan_tci``, and applications could check
> ``PKT_RX_VLAN_STRIPPED``.
> > +
> > +However, we currently don't have a way to describe offloads which can't
> be
> > +disabled by PMDs, and this creates an inconsistency with the way
> applications
> > +expect the PMD offloads to work, and requires them to be aware of which
> > +underlying driver they use.
> > +
> > +Since release 21.11 the af_packet PMD will implement support for the
> > +``DEV_RX_OFFLOAD_VLAN_STRIP`` offload, and users can control the
> desired vlan
> > +stripping behavior.
> > +
> > +It's important to note that the default case will change. If previously,
> > +the vlan tag was stripped, if the application now requires the same
> behavior,
> > +it will need to configure ``rxmode.offloads`` with
> ``DEV_RX_OFFLOAD_VLAN_STRIP``.
> > +
> > +The PMD driver will re-insert the vlan tag transparently to the
> application
> > +if the kernel strips it, as long as the ``DEV_RX_OFFLOAD_VLAN_STRIP``
> is not
> > +enabled.
>
> This seems like the wrong place for this text.
> It is not a new feature, it is a bug fix.
>
> If you want to describe it as an enhancement, the text should be succinct
> and describe
> the situation from user point of view. Something like:
>
>     Af_packet PMD now works with VLAN's on Linux
>
>

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

[Ferruh Yigit]

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``.
"

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

[Tudor Cornea]

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>

---
v5:
* Updated the af_packet documentation
* Updated the af_packet release notes
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             |  7 +++++++
 doc/guides/rel_notes/release_21_11.rst    |  5 +++++
 drivers/net/af_packet/rte_eth_af_packet.c | 12 ++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/doc/guides/nics/af_packet.rst b/doc/guides/nics/af_packet.rst
index efd6f1c..168a946 100644
--- a/doc/guides/nics/af_packet.rst
+++ b/doc/guides/nics/af_packet.rst
@@ -65,3 +65,10 @@ 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
+---------------------------------------------
+
+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 the application.
diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index ad7c1af..3315703 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -66,6 +66,11 @@ New Features
 
   * Added rte_flow support for dual VLAN insert and strip actions.
 
+* **Updated af_packet ethdev driver.**
+
+  * Default VLAN strip behavior changed.
+    If previously, the VLAN tag was stripped by the kernel, if the application now requires the same behavior, it will need to enable ``DEV_RX_OFFLOAD_VLAN_STRIP``.
+
 * **Updated Marvell cnxk crypto PMD.**
 
   * Added AES-CBC SHA1-HMAC support in lookaside protocol (IPsec) for CN10K.
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;
 }
-- 
2.7.4

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

[Tudor Cornea]

Hi Ferruh,

 Also driver was already working with VLAN, the change is VLAN is not
> force stripped anymore.


 Agreed. It makes more sense to inform the users that the default behavior
of the PMD has changed w.r.t VLAN stripping.

I have updated the patch

On Thu, 30 Sept 2021 at 11:14, Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> 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``.
> "
>

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

[Stephen Hemminger]

On Fri,  1 Oct 2021 11:35:01 +0300
Tudor Cornea <tudor.cornea@gmail.com> 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>


Acked-by: Stephen Hemminger <stephen@networkplumber.org>

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

[Ferruh Yigit]

On 10/1/2021 4:02 PM, Stephen Hemminger wrote:
> On Fri,  1 Oct 2021 11:35:01 +0300
> Tudor Cornea <tudor.cornea@gmail.com> 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>
> 
> 
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> 

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk-next-net/main, thanks.
Release notes slightly updated, to simplify the sentences, while merging.