Re: [dpdk-dev] incorrect vlan_tci in rte mbuf

[Jeff Guo <jia.guo@intel.com>]

hi, xiaoping

Base on the HW behaviors, either i40e normal recv or i40e vector recv 
should use the offload flags of the last rxd to resign to the first 
segment, then restore the mbuf to the recv packets, i am not sure if 
this could answer your question, Anyway, you could use below quick check 
to see if it could help.

+++ b/drivers/net/i40e/i40e_rxtx_vec_common.h
@@ -33,6 +33,7 @@ reassemble_packets(struct i40e_rx_queue *rxq, struct rte_mbuf **rx_bufs,
                         if (!split_flags[buf_idx]) {
                                 /* it's the last packet of the set */
                                 start->hash = end->hash;
+                               start->vlan_tci = end->vlan_tci;
                                 start->ol_flags = end->ol_flags;
                                 /* we need to strip crc for the whole packet */
                                 start->pkt_len -= rxq->crc_len;

On 5/19/2020 4:31 PM, Yan, Xiaoping (NSB - CN/Hangzhou) wrote:
> Hi,
>
> I tried to use gdb to print the rx descriptor, and it turned out that only the rxd for the last segment has correct l2tag1 value.
> Test step: ping xx.xx.xx.xx -s 2500,   from a vlan interface with vlan id 1901, MTU 9000. (this will sends 1 packet, and needs two mbuf segments to receive, length of 1st mbuf segment is 2176(mbuf size of my dpdk application), length of 2nd mbuf segment is 366)
>
> Rx descriptor printed with gdb:
> Breakpoint 2 at 0x7ffff5efa4bb: file /usr/src/debug/dpdk-18.11.1-2.nok6.wf31.x86_64/package/dpdk/dpdk-18.11/drivers/net/i40e/i40e_rxtx_vec_avx2.c, line 496.
> (gdb) print *(rxq->rx_ring + rxq->rx_tail+1)
> $4 = {read = {pkt_addr = 14858615587319906304, hdr_addr = 598164390293517, rsvd1 = 0,
>      rsvd2 = 0}, wb = {qword0 = {lo_dword = {mirr_fcoe = {mirroring_status = 0,
>            fcoe_ctx_id = 0}, l2tag1 = 73}, hi_dword = {rss = 3459541031,
>          fcoe_param = 3459541031, fd_id = 3459541031}}, qword1 = {
>        status_error_len = 598164390293517}, qword2 = {ext_status = 0, rsvd = 0,
>        l2tag2_1 = 0, l2tag2_2 = 0}, qword3 = {lo_dword = {flex_bytes_lo = 0,
>          pe_status = 0}, hi_dword = {flex_bytes_hi = 0, fd_id = 0}}}}
> (gdb) print *(rxq->rx_ring + rxq->rx_tail+2)
> $5 = {read = {pkt_addr = 14858615587439706112, hdr_addr = 100635378724879, rsvd1 = 0,
>      rsvd2 = 0}, wb = {qword0 = {lo_dword = {mirr_fcoe = {mirroring_status = 0,
>            fcoe_ctx_id = 0}, l2tag1 = 1901}, hi_dword = {rss = 3459541031,
>          fcoe_param = 3459541031, fd_id = 3459541031}}, qword1 = {
>        status_error_len = 100635378724879}, qword2 = {ext_status = 0, rsvd = 0,
>        l2tag2_1 = 0, l2tag2_2 = 0}, qword3 = {lo_dword = {flex_bytes_lo = 0,
>          pe_status = 0}, hi_dword = {flex_bytes_hi = 0, fd_id = 0}}}}
>
> With formula:		length = (qword1 & I40E_RXD_QW1_LENGTH_PBUF_MASK) >> I40E_RXD_QW1_LENGTH_PBUF_SHIFT;  (qword1 is status_error_len in gdb printout)
> I can get length of first segment(rxq->rx_ring + rxq->rx_tail+1) is 2176, and length of second segment(rxq->rx_ring + rxq->rx_tail+2) is 366, which proves they are the mbuf segments to receive.
> However we can see the l2tag1 of first segment is 73(invalid), l2tag1 of second segment is 1901(valid).
>
> It means either HW should be modified to fill l2tag1 correctly for each every segment, or _recv_raw_pkts_vec_avx2 should be modified to read l2tag1 of last segment?
> PS: it seems i40e_recv_scattered_pkts use the l2tag1 of last rxd (to set vlan tci of first segment), don't know it is by accident(convenience) or by purpose.
>
> Do you agree on my analysis?
> If my analysis is correct, is it possible for you to help make a sw patch, because I'm not familiar with this vec avx code.
>
> Best regards
> Yan Xiaoping
>
> -----Original Message-----
> From: Jeff Guo <jia.guo@intel.com>
> Sent: 2020年5月19日 15:00
> To: Yan, Xiaoping (NSB - CN/Hangzhou) <xiaoping.yan@nokia-sbell.com>; beilei.xing@intel.com
> Cc: dev@dpdk.org; Olivier Matz <olivier.matz@6wind.com>
> Subject: Re: [dpdk-dev] incorrect vlan_tci in rte mbuf
>
> hi, xiaoping
>
> On 5/18/2020 4:31 PM, Yan, Xiaoping (NSB - CN/Hangzhou) wrote:
>> Hi Beilei & Jia,
>>
>> I got your name from the MAINTAINERS  for Intel i40e.
>> Could you help to have a look at the issue, described in my previous mail?
>>
>> Thank you.
>>
>> Best regards
>> Yan Xiaoping
>>
>> -----Original Message-----
>> From: Olivier Matz <olivier.matz@6wind.com>
>> Sent: 2020年5月18日 15:24
>> To: Yan, Xiaoping (NSB - CN/Hangzhou) <xiaoping.yan@nokia-sbell.com>
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] incorrect vlan_tci in rte mbuf
>>
>> Hi,
>>
>> On Fri, May 15, 2020 at 11:12:27AM +0000, Yan, Xiaoping (NSB - CN/Hangzhou) wrote:
>>> Hi,
>>>
>>> I'm using i40e vf, dpdk 18.11, x86_64 CPU (rx function in use is
>>> i40e_recv_scattered_pkts_vec_avx2)
>>> When enable hw vlan strip:
>>>
>>>     *   If packet fit in one mbuf segment, the vlan_tci field is correct
>>>     *   If packets are stored in several mbuf segment, the vlan_tci of last segment is correct, vlan_tci of other segments are invalid
>>>
>>> It seems i40e_recv_scattered_pkts has correctly set the vlan_tci, by
>>> calling i40e_rxd_to_vlan_tci(first_seg, &rxd);
>>>
>>> Is this a bug in i40e_recv_scattered_pkts_vec_avx2?
>>> (I didn't find setting vlan_tci for first segment, but it's a bit
>>> difficult for me to understand codes in
>>> i40e_recv_scattered_pkts_vec_avx2, so I'm not quite sure) I checked
>>> the latest dpdk version 20.02
>>> http://lxr.dpdk.org/dpdk/latest/source/drivers/net/i40e/i40e_rxtx_vec
>>> _
>>> avx2.c#L791
>>> But seems no change for this.
>>>
>>> Any comment please?
>>> [...]
>> I don't know about the i40e driver, but I can confirm that the vlan tci flag should be in the first segment, and not in the next ones.
>>
>> I suggest to CC i40e driver maintainers.
>>
>> Regards,
>> Olivier
>
> On one hand, as Olivier said that vlan tci flag should be in the first segment, and on the other hands, vlan_tci do process in "_recv_raw_pkts_vec_avx2" when use 256 instinct
>
> to process the descriptor with mbuf,  you could check how to process the vlan_flags_shuf and blend it into the mbuf.
>