DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@amd.com>
To: Joyce Kong <Joyce.Kong@arm.com>, Jakub Grajciar <jgrajcia@cisco.com>
Cc: Ruifeng Wang <Ruifeng.Wang@arm.com>,
	"dev@dpdk.org" <dev@dpdk.org>, nd <nd@arm.com>
Subject: Re: [PATCH v1 1/2] net/memif: add a Rx fast path
Date: Thu, 19 May 2022 17:38:31 +0100	[thread overview]
Message-ID: <b52400f9-a847-1a4c-d999-7ea184bda803@amd.com> (raw)
In-Reply-To: <AS4PR08MB77125A8B0A368D9641F4744792D09@AS4PR08MB7712.eurprd08.prod.outlook.com>

On 5/19/2022 4:09 PM, Joyce Kong wrote:
> [CAUTION: External Email]
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@xilinx.com>
>> Sent: Thursday, May 19, 2022 1:06 AM
>> To: Joyce Kong <Joyce.Kong@arm.com>; Jakub Grajciar <jgrajcia@cisco.com>
>> Cc: Ruifeng Wang <Ruifeng.Wang@arm.com>; dev@dpdk.org; nd
>> <nd@arm.com>
>> Subject: Re: [PATCH v1 1/2] net/memif: add a Rx fast path
>>
>> On 5/17/2022 11:51 AM, Joyce Kong wrote:
>>> For memif non-zero-copy mode, there is a branch to compare
>>> the mbuf and memif buffer size during memory copying. Add
>>> a fast memory copy path by removing this branch with mbuf
>>> and memif buffer size defined at compile time. The removal
>>> of the branch leads to considerable performance uplift.
>>>
>>> When memif <= buffer size, Rx chooses the fast memcpy path,
>>> otherwise it would choose the original path.
>>>
>>> Test with 1p1q on Ampere Altra AArch64 server,
>>> --------------------------------------------
>>>     buf size  | memif <= mbuf | memif > mbuf |
>>> --------------------------------------------
>>> non-zc gain |     4.30%     |    -0.52%    |
>>> --------------------------------------------
>>>      zc gain  |     2.46%     |     0.70%    |
>>> --------------------------------------------
>>>
>>> Test with 1p1q on Cascade Lake Xeon X86server,
>>> -------------------------------------------
>>>     buf size  | memif <= mbuf | memif > mbuf |
>>> -------------------------------------------
>>> non-zc gain |     2.13%     |    -1.40%    |
>>> -------------------------------------------
>>>      zc gain  |     0.18%     |     0.48%    |
>>> -------------------------------------------
>>>
>>> Signed-off-by: Joyce Kong <joyce.kong@arm.com>
>>
>> <...>
>>
>>> +   } else {
>>> +           while (n_slots && n_rx_pkts < nb_pkts) {
>>> +                   mbuf_head = rte_pktmbuf_alloc(mq->mempool);
>>> +                   if (unlikely(mbuf_head == NULL))
>>> +                           goto no_free_bufs;
>>> +                   mbuf = mbuf_head;
>>> +                   mbuf->port = mq->in_port;
>>> +
>>> +next_slot2:
>>> +                   s0 = cur_slot & mask;
>>> +                   d0 = &ring->desc[s0];
>>>
>>> -                   rte_memcpy(rte_pktmbuf_mtod_offset(mbuf, void *,
>>> -                                                      dst_off),
>>> -                           (uint8_t *)memif_get_buffer(proc_private, d0)
>> +
>>> -                           src_off, cp_len);
>>> +                   src_len = d0->length;
>>> +                   dst_off = 0;
>>> +                   src_off = 0;
>>
>> Hi Joyce, Jakub,
>>
>> Something doesn't look right in the original code (not in this patch),
>> can you please help me check if I am missing something?
>>
>> For the memif buffer segmented case, first buffer will be copied to
>> mbuf, 'dst_off' increased and jump back to process next memif segment:
>>
>>    + d0
>>    |
>>    v
>> +++  +-+
>> |a+->+b|
>> +-+  +-+
>>
>> +---+
>> |a  |
>> +-+-+
>>     ^
>>     |
>>     + dst_off
>>
>> "
>>       if (d0->flags & MEMIF_DESC_FLAG_NEXT)
>>            goto next_slot;
>> "
>>
>> But here 'dst_off' set back to '0', wont this cause next memif buffer
>> segment to write to beginning of mbuf overwriting previous data?
>>
>> Thanks,
>> Ferruh
> 
> Hi Ferruh,
> 
> Agree with you here, and sorry I didn’t notice it before. Perhaps moving
> 'det_off = 0' to the line above 'next_slot' would solve the overwriting?
> 

Yes, I think this solves the issue.


And I wonder why this is not caught by testing. @Jakub, is the segmented 
memif buffers not a common use case?

I did able to reproduce the issue as following (and confirm suggested 
change fixes it):

server
./build/app/dpdk-testpmd --proc-type=primary --file-prefix=pmd1 
--vdev=net_memif0,role=server,bsize=32 -- -i --txpkts=512
 > set fwd txonly
 > start

client
./build/app/dpdk-testpmd --proc-type=primary --file-prefix=pmd2 
--vdev=net_memif1,bsize=32 -- -i
 > set fwd rxonly
 > set verbose 3
 > start

'client' will display packets info wrong, it will be all '0'. Also it is 
possible to capture packets in client and confirm.

  reply	other threads:[~2022-05-19 16:38 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-12  9:32 [RFC] net/memif: add a fast path for Rx Joyce Kong
2022-05-17 10:51 ` [PATCH v1 0/2] add a fast path for memif Rx/Tx Joyce Kong
2022-05-17 10:51   ` [PATCH v1 1/2] net/memif: add a Rx fast path Joyce Kong
2022-05-18 16:53     ` Ferruh Yigit
2022-05-19  7:00       ` Joyce Kong
2022-05-19  8:44         ` Joyce Kong
2022-05-18 17:06     ` Ferruh Yigit
2022-05-19 15:09       ` Joyce Kong
2022-05-19 16:38         ` Ferruh Yigit [this message]
2022-05-17 10:51   ` [PATCH v1 2/2] net/memif: add a Tx " Joyce Kong
2022-05-17 13:59   ` [PATCH v1 0/2] add a fast path for memif Rx/Tx Morten Brørup
2022-05-18  2:48   ` Ruifeng Wang
2022-07-01 10:28   ` [PATCH v2 " Joyce Kong
2022-07-01 10:28     ` [PATCH v2 1/2] net/memif: add a Rx fast path Joyce Kong
2022-07-01 16:51       ` Stephen Hemminger
2022-08-22  3:47       ` [PATCH v3 0/2] add a fast path for memif Rx/Tx Joyce Kong
2022-08-22  3:47         ` [PATCH v3 1/2] net/memif: add a Rx fast path Joyce Kong
2022-08-31 16:25           ` Stephen Hemminger
2022-09-07  6:06             ` Joyce Kong
2022-08-22  3:47         ` [PATCH v3 2/2] net/memif: add a Tx " Joyce Kong
2022-07-01 10:28     ` [PATCH v2 " Joyce Kong
2022-09-15  6:58 ` [PATCH v4 0/2] add a fast path for memif Rx/Tx Joyce Kong
2022-09-15  6:58   ` [PATCH v4 1/2] net/memif: add a Rx fast path Joyce Kong
2022-09-15  6:58   ` [PATCH v4 2/2] net/memif: add a Tx " Joyce Kong
2022-09-22  9:12   ` [PATCH v4 0/2] add a fast path for memif Rx/Tx Ferruh Yigit
2022-12-09 13:59     ` Ferruh Yigit

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=b52400f9-a847-1a4c-d999-7ea184bda803@amd.com \
    --to=ferruh.yigit@amd.com \
    --cc=Joyce.Kong@arm.com \
    --cc=Ruifeng.Wang@arm.com \
    --cc=dev@dpdk.org \
    --cc=jgrajcia@cisco.com \
    --cc=nd@arm.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).