DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Zhang, Qi Z" <qi.z.zhang@intel.com>
To: "Yan,
	Xiaoping (NSB - CN/Hangzhou)" <xiaoping.yan@nokia-sbell.com>,
	"Guo,  Jia" <jia.guo@intel.com>,
	"Xing, Beilei" <beilei.xing@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Olivier Matz <olivier.matz@6wind.com>
Subject: Re: [dpdk-dev] incorrect vlan_tci in rte mbuf
Date: Wed, 20 May 2020 07:48:42 +0000	[thread overview]
Message-ID: <039ED4275CED7440929022BC67E706115482349E@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <b1b2cc8cb2cf44c3b089803a4c80951f@nokia-sbell.com>



> -----Original Message-----
> From: Yan, Xiaoping (NSB - CN/Hangzhou) <xiaoping.yan@nokia-sbell.com>
> Sent: Wednesday, May 20, 2020 3:42 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Guo, Jia <jia.guo@intel.com>; Xing,
> Beilei <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,
> 
> I tried the change, and it solved the problem.
> Should I submit the change to github? Or you will do that?
> Thank you.

No worry, I saw Guo Jia also reply this on another thread with the same fix, and he will submit the fix, thanks to report this!

> 
> Best regards
> Yan Xiaoping
> 
> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: 2020年5月19日 20:42
> To: Yan, Xiaoping (NSB - CN/Hangzhou) <xiaoping.yan@nokia-sbell.com>;
> Guo, Jia <jia.guo@intel.com>; Xing, Beilei <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:
> 
> 	According to i40e datasheet, for packet with multiple buffer, the L2TAG
> should only be valid on last Rx descriptor.
> 	So it is identical with what you observed and it looks like a bug in the
> vector path.
> 	Could you try if below change can fix the issue?
> 
> --- a/drivers/net/i40e/i40e_rxtx_vec_common.h
> +++ b/drivers/net/i40e/i40e_rxtx_vec_common.h
> @@ -34,6 +34,7 @@ reassemble_packets(struct i40e_rx_queue *rxq, struct
> rte_mbuf **rx_bufs,
>                                 /* it's the last packet of the set */
>                                 start->hash = end->hash;
>                                 start->ol_flags = end->ol_flags;
> +                               start->vlan_tci = end->vlan_tci;
>                                 /* we need to strip crc for the whole
> packet */
>                                 start->pkt_len -= rxq->crc_len;
>                                 if (end->data_len > rxq->crc_len
> 
> Thanks
> Qi
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Yan, Xiaoping (NSB -
> > CN/Hangzhou)
> > Sent: Tuesday, May 19, 2020 4:32 PM
> > To: Guo, Jia <jia.guo@intel.com>; Xing, Beilei <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,
> >
> > 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.1
> > 1/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_v
> > >> ec
> > >> _
> > >> 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.


      reply	other threads:[~2020-05-20  7:48 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-15 11:12 Yan, Xiaoping (NSB - CN/Hangzhou)
2020-05-18  7:24 ` Olivier Matz
2020-05-18  8:31   ` Yan, Xiaoping (NSB - CN/Hangzhou)
2020-05-19  7:00     ` Jeff Guo
2020-05-19  8:31       ` Yan, Xiaoping (NSB - CN/Hangzhou)
2020-05-19 12:27         ` Jeff Guo
2020-05-19 12:41         ` Zhang, Qi Z
2020-05-20  7:42           ` Yan, Xiaoping (NSB - CN/Hangzhou)
2020-05-20  7:48             ` Zhang, Qi Z [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=039ED4275CED7440929022BC67E706115482349E@SHSMSX103.ccr.corp.intel.com \
    --to=qi.z.zhang@intel.com \
    --cc=beilei.xing@intel.com \
    --cc=dev@dpdk.org \
    --cc=jia.guo@intel.com \
    --cc=olivier.matz@6wind.com \
    --cc=xiaoping.yan@nokia-sbell.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).