* [dpdk-stable] [PATCH] net: fix Tx VLAN flag for offload emulation
@ 2019-03-25 15:05 Chas Williams
2019-03-26 16:50 ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
2019-04-04 17:07 ` Ferruh Yigit
0 siblings, 2 replies; 5+ messages in thread
From: Chas Williams @ 2019-03-25 15:05 UTC (permalink / raw)
To: dev; +Cc: olivier.matz, Bill Hong, stable, Chas Williams
From: Bill Hong <bhong@brocade.com>
A PMD might use rte_vlan_insert to implement Tx VLAN offload. Typically
the PMD will insert the VLAN header in the transmit path and then attempt
to send the packets. If this fails, the packets are returned to
the application which may attempt to send these packets again. If the
PKT_TX_VLAN flag is not cleared, the transmit path may attempt to insert
the VLAN header again.
Fixes: 47aa48b969f8 ("net: fix stripped VLAN flag for offload emulation");
Cc: stable@dpdk.org
Signed-off-by: Bill Hong <bhong@brocade.com>
Signed-off-by: Chas Williams <chas3@att.com>
---
lib/librte_net/rte_ether.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
index c2c5e249f..e0d831113 100644
--- a/lib/librte_net/rte_ether.h
+++ b/lib/librte_net/rte_ether.h
@@ -408,7 +408,7 @@ static inline int rte_vlan_insert(struct rte_mbuf **m)
vh = (struct vlan_hdr *) (nh + 1);
vh->vlan_tci = rte_cpu_to_be_16((*m)->vlan_tci);
- (*m)->ol_flags &= ~PKT_RX_VLAN_STRIPPED;
+ (*m)->ol_flags &= ~(PKT_RX_VLAN_STRIPPED | PKT_TX_VLAN);
return 0;
}
--
2.17.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH] net: fix Tx VLAN flag for offload emulation
2019-03-25 15:05 [dpdk-stable] [PATCH] net: fix Tx VLAN flag for offload emulation Chas Williams
@ 2019-03-26 16:50 ` Ferruh Yigit
2019-03-26 18:01 ` Chas Williams
2019-04-04 17:07 ` Ferruh Yigit
1 sibling, 1 reply; 5+ messages in thread
From: Ferruh Yigit @ 2019-03-26 16:50 UTC (permalink / raw)
To: Chas Williams, dev, olivier.matz
Cc: Bill Hong, stable, Chas Williams, Andrew Rybchenko
On 3/25/2019 3:05 PM, Chas Williams wrote:
> From: Bill Hong <bhong@brocade.com>
>
> A PMD might use rte_vlan_insert to implement Tx VLAN offload. Typically
> the PMD will insert the VLAN header in the transmit path and then attempt
> to send the packets. If this fails, the packets are returned to
> the application which may attempt to send these packets again. If the
> PKT_TX_VLAN flag is not cleared, the transmit path may attempt to insert
> the VLAN header again.
>
> Fixes: 47aa48b969f8 ("net: fix stripped VLAN flag for offload emulation");
> Cc: stable@dpdk.org
>
> Signed-off-by: Bill Hong <bhong@brocade.com>
> Signed-off-by: Chas Williams <chas3@att.com>
> ---
> lib/librte_net/rte_ether.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
> index c2c5e249f..e0d831113 100644
> --- a/lib/librte_net/rte_ether.h
> +++ b/lib/librte_net/rte_ether.h
> @@ -408,7 +408,7 @@ static inline int rte_vlan_insert(struct rte_mbuf **m)
> vh = (struct vlan_hdr *) (nh + 1);
> vh->vlan_tci = rte_cpu_to_be_16((*m)->vlan_tci);
>
> - (*m)->ol_flags &= ~PKT_RX_VLAN_STRIPPED;
> + (*m)->ol_flags &= ~(PKT_RX_VLAN_STRIPPED | PKT_TX_VLAN);
Hi Chas,
Looks reasonable, AFAIK, PKT_TX_VLAN flag means 'mbuf->vlan_tci' has valid value
and a request from application to driver to insert VLAN information.
After successful insertion flag can be removed.
But in mbuf header, flags documented as:
PKT_TX_VLAN: TX packet is a 802.1q VLAN packet.
PKT_TX_QINQ: TX packet with double VLAN inserted.
Hi Oliver,
Is above comments correct, or I am missing something J
Also here cleaning 'PKT_RX_VLAN_STRIPPED' looks like to say 'vlan' information
is not stripped but in the packet data, but 'RX' in this context can be
confusing, should we have a more generic 'PKT_VLAN_STRIPPED' ?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH] net: fix Tx VLAN flag for offload emulation
2019-03-26 16:50 ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
@ 2019-03-26 18:01 ` Chas Williams
2019-04-01 20:17 ` Ferruh Yigit
0 siblings, 1 reply; 5+ messages in thread
From: Chas Williams @ 2019-03-26 18:01 UTC (permalink / raw)
To: Ferruh Yigit, dev, olivier.matz; +Cc: stable, Chas Williams, Andrew Rybchenko
On 3/26/19 12:50 PM, Ferruh Yigit wrote:
> On 3/25/2019 3:05 PM, Chas Williams wrote:
>> From: Bill Hong <bhong@brocade.com>
>>
>> A PMD might use rte_vlan_insert to implement Tx VLAN offload. Typically
>> the PMD will insert the VLAN header in the transmit path and then attempt
>> to send the packets. If this fails, the packets are returned to
>> the application which may attempt to send these packets again. If the
>> PKT_TX_VLAN flag is not cleared, the transmit path may attempt to insert
>> the VLAN header again.
>>
>> Fixes: 47aa48b969f8 ("net: fix stripped VLAN flag for offload emulation");
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Bill Hong <bhong@brocade.com>
>> Signed-off-by: Chas Williams <chas3@att.com>
>> ---
>> lib/librte_net/rte_ether.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
>> index c2c5e249f..e0d831113 100644
>> --- a/lib/librte_net/rte_ether.h
>> +++ b/lib/librte_net/rte_ether.h
>> @@ -408,7 +408,7 @@ static inline int rte_vlan_insert(struct rte_mbuf **m)
>> vh = (struct vlan_hdr *) (nh + 1);
>> vh->vlan_tci = rte_cpu_to_be_16((*m)->vlan_tci);
>>
>> - (*m)->ol_flags &= ~PKT_RX_VLAN_STRIPPED;
>> + (*m)->ol_flags &= ~(PKT_RX_VLAN_STRIPPED | PKT_TX_VLAN);
>
> Hi Chas,
>
> Looks reasonable, AFAIK, PKT_TX_VLAN flag means 'mbuf->vlan_tci' has valid value
> and a request from application to driver to insert VLAN information.
> After successful insertion flag can be removed.
>
> But in mbuf header, flags documented as:
> PKT_TX_VLAN: TX packet is a 802.1q VLAN packet.
> PKT_TX_QINQ: TX packet with double VLAN inserted.
Note the usage slightly below:
/**
* Bitmask of all supported packet Tx offload features flags,
* which can be set for packet.
*/
#define PKT_TX_OFFLOAD_MASK ( \
PKT_TX_OUTER_IPV6 | \
PKT_TX_OUTER_IPV4 | \
PKT_TX_OUTER_IP_CKSUM | \
PKT_TX_VLAN_PKT | \
So this implies that PKT_TX_VLAN_PKT (which is PKT_TX_VLAN's old name)
is an offload feature, not that the actual packet has an 802.1q header
(offloaded or otherwise).
The same seems to apply for PKT_TX_QINQ.
> Hi Oliver,
>
> Is above comments correct, or I am missing something J
>
>
> Also here cleaning 'PKT_RX_VLAN_STRIPPED' looks like to say 'vlan' information
> is not stripped but in the packet data, but 'RX' in this context can be
> confusing, should we have a more generic 'PKT_VLAN_STRIPPED' ?
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH] net: fix Tx VLAN flag for offload emulation
2019-03-26 18:01 ` Chas Williams
@ 2019-04-01 20:17 ` Ferruh Yigit
0 siblings, 0 replies; 5+ messages in thread
From: Ferruh Yigit @ 2019-04-01 20:17 UTC (permalink / raw)
To: Chas Williams, dev, olivier.matz; +Cc: stable, Chas Williams, Andrew Rybchenko
On 3/26/2019 6:01 PM, Chas Williams wrote:
>
>
> On 3/26/19 12:50 PM, Ferruh Yigit wrote:
>> On 3/25/2019 3:05 PM, Chas Williams wrote:
>>> From: Bill Hong <bhong@brocade.com>
>>>
>>> A PMD might use rte_vlan_insert to implement Tx VLAN offload. Typically
>>> the PMD will insert the VLAN header in the transmit path and then attempt
>>> to send the packets. If this fails, the packets are returned to
>>> the application which may attempt to send these packets again. If the
>>> PKT_TX_VLAN flag is not cleared, the transmit path may attempt to insert
>>> the VLAN header again.
>>>
>>> Fixes: 47aa48b969f8 ("net: fix stripped VLAN flag for offload emulation");
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Bill Hong <bhong@brocade.com>
>>> Signed-off-by: Chas Williams <chas3@att.com>
>>> ---
>>> lib/librte_net/rte_ether.h | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
>>> index c2c5e249f..e0d831113 100644
>>> --- a/lib/librte_net/rte_ether.h
>>> +++ b/lib/librte_net/rte_ether.h
>>> @@ -408,7 +408,7 @@ static inline int rte_vlan_insert(struct rte_mbuf **m)
>>> vh = (struct vlan_hdr *) (nh + 1);
>>> vh->vlan_tci = rte_cpu_to_be_16((*m)->vlan_tci);
>>>
>>> - (*m)->ol_flags &= ~PKT_RX_VLAN_STRIPPED;
>>> + (*m)->ol_flags &= ~(PKT_RX_VLAN_STRIPPED | PKT_TX_VLAN);
>>
>> Hi Chas,
>>
>> Looks reasonable, AFAIK, PKT_TX_VLAN flag means 'mbuf->vlan_tci' has valid value
>> and a request from application to driver to insert VLAN information.
>> After successful insertion flag can be removed.
>>
>> But in mbuf header, flags documented as:
>> PKT_TX_VLAN: TX packet is a 802.1q VLAN packet.
>> PKT_TX_QINQ: TX packet with double VLAN inserted.
Hi Olivier,
Any comment on them, should they be fixed?
>
> Note the usage slightly below:
>
> /**
> * Bitmask of all supported packet Tx offload features flags,
> * which can be set for packet.
> */
> #define PKT_TX_OFFLOAD_MASK ( \
> PKT_TX_OUTER_IPV6 | \
> PKT_TX_OUTER_IPV4 | \
> PKT_TX_OUTER_IP_CKSUM | \
> PKT_TX_VLAN_PKT | \
>
> So this implies that PKT_TX_VLAN_PKT (which is PKT_TX_VLAN's old name)
> is an offload feature, not that the actual packet has an 802.1q header
> (offloaded or otherwise).
>
> The same seems to apply for PKT_TX_QINQ.
Agree, I just would like to confirm, so lgtm:
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
>
>> Hi Oliver,
>>
>> Is above comments correct, or I am missing something J
>>
>>
>> Also here cleaning 'PKT_RX_VLAN_STRIPPED' looks like to say 'vlan' information
>> is not stripped but in the packet data, but 'RX' in this context can be
>> confusing, should we have a more generic 'PKT_VLAN_STRIPPED' ?
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH] net: fix Tx VLAN flag for offload emulation
2019-03-25 15:05 [dpdk-stable] [PATCH] net: fix Tx VLAN flag for offload emulation Chas Williams
2019-03-26 16:50 ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
@ 2019-04-04 17:07 ` Ferruh Yigit
1 sibling, 0 replies; 5+ messages in thread
From: Ferruh Yigit @ 2019-04-04 17:07 UTC (permalink / raw)
To: Chas Williams, dev; +Cc: olivier.matz, Bill Hong, stable, Chas Williams
On 3/25/2019 3:05 PM, Chas Williams wrote:
> From: Bill Hong <bhong@brocade.com>
>
> A PMD might use rte_vlan_insert to implement Tx VLAN offload. Typically
> the PMD will insert the VLAN header in the transmit path and then attempt
> to send the packets. If this fails, the packets are returned to
> the application which may attempt to send these packets again. If the
> PKT_TX_VLAN flag is not cleared, the transmit path may attempt to insert
> the VLAN header again.
>
> Fixes: 47aa48b969f8 ("net: fix stripped VLAN flag for offload emulation");
> Cc: stable@dpdk.org
>
> Signed-off-by: Bill Hong <bhong@brocade.com>
> Signed-off-by: Chas Williams <chas3@att.com>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-04-04 17:07 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-25 15:05 [dpdk-stable] [PATCH] net: fix Tx VLAN flag for offload emulation Chas Williams
2019-03-26 16:50 ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
2019-03-26 18:01 ` Chas Williams
2019-04-01 20:17 ` Ferruh Yigit
2019-04-04 17:07 ` Ferruh Yigit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).