From: Joyce Kong <Joyce.Kong@arm.com>
To: Ferruh Yigit <ferruh.yigit@xilinx.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 08:44:53 +0000 [thread overview]
Message-ID: <AS4PR08MB7712D6D827651AC1D105DAD992D09@AS4PR08MB7712.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <AS4PR08MB77120499D7FC03A54A1393D292D09@AS4PR08MB7712.eurprd08.prod.outlook.com>
> -----Original Message-----
> From: Joyce Kong
> Sent: Thursday, May 19, 2022 3:00 PM
> To: Ferruh Yigit <ferruh.yigit@xilinx.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
>
> Hi Ferruh,
>
> > -----Original Message-----
> > From: Ferruh Yigit <ferruh.yigit@xilinx.com>
> > Sent: Thursday, May 19, 2022 12:53 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% |
> > > -------------------------------------------
> > >
> >
> >
> > Hi Joyce,
> >
> > I have multiple questions,
> >
> > 1) The patch updates only non-zero-copy mode Rx path ('eth_memif_rx'),
> > why zero-copy path performance also impacted?
> >
> For memif zero-copy mode, only client runs 'eth_memif_rx_zc', and server still
> runs 'eth_memif_rx', so the patch would impacts zero-copy mode.
>
> > 2) As far as I can see there is a behavior change, more details below
> >
> > 3) patch talking about memif buffer size being defined in compile
> > time, is the big "memif <= mbuf" if block optimized out?
> > Since 'pkt_buffer_size' is a devarg, so it can change from run to run
> > and it is not known in compile time, I doubt that it is optimized out.
> > Is having 'pkt_buffer_size' as devarg breaks your logic?
> >
> From memif run to run, run.pkt_buffer_size would change, and
> cfg.pkt_buffer_size which is the reserved max buffer size would not change.
> For patch details, I use cfg.pkt_buffer_size to implement the logic.
>
> > 4) One option gains performance and other loose performance, do you
> > think gain performance case is more common use case? Is there any data
> around it?
> >
> Yes, I think the gain performance case is more common case, as the default
> memif buffer size equals to mbuf size. In theory, when memif buf size >= mbuf
> size, the Rx runs the original path, it would not lead to obvious impact.
>
> >
> > Jakub,
> >
> > Do you want to test this patch first before progressing with it?
> >
> > > Signed-off-by: Joyce Kong <joyce.kong@arm.com>
> > > ---
> > > drivers/net/memif/rte_eth_memif.c | 124 ++++++++++++++++++++----------
> > > 1 file changed, 84 insertions(+), 40 deletions(-)
> > >
> > > diff --git a/drivers/net/memif/rte_eth_memif.c
> > > b/drivers/net/memif/rte_eth_memif.c
> > > index 587ad45576..f55776ca46 100644
> > > --- a/drivers/net/memif/rte_eth_memif.c
> > > +++ b/drivers/net/memif/rte_eth_memif.c
> > > @@ -342,66 +342,111 @@ eth_memif_rx(void *queue, struct rte_mbuf
> > **bufs, uint16_t nb_pkts)
> > > goto refill;
> > > n_slots = last_slot - cur_slot;
> > >
> > > - 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;
> > > + if (likely(mbuf_size >= pmd->cfg.pkt_buffer_size)) {
> > > + 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_slot1:
> > > + s0 = cur_slot & mask;
> > > + d0 = &ring->desc[s0];
> > >
> > > -next_slot:
> > > - s0 = cur_slot & mask;
> > > - d0 = &ring->desc[s0];
> > > + cp_len = d0->length;
> > >
> > > - src_len = d0->length;
> > > - dst_off = 0;
> > > - src_off = 0;
> > > + rte_pktmbuf_data_len(mbuf) = cp_len;
> > > + rte_pktmbuf_pkt_len(mbuf) = cp_len;
> > > + if (mbuf != mbuf_head)
> > > + rte_pktmbuf_pkt_len(mbuf_head) += cp_len;
> > >
> > > - do {
> > > - dst_len = mbuf_size - dst_off;
> > > - if (dst_len == 0) {
> > > - dst_off = 0;
> > > - dst_len = mbuf_size;
> > > + rte_memcpy(rte_pktmbuf_mtod(mbuf, void *),
> > > + (uint8_t *)memif_get_buffer(proc_private, d0),
> > cp_len);
> > > +
> > > + cur_slot++;
> > > + n_slots--;
> > >
> > > - /* store pointer to tail */
> > > + if (d0->flags & MEMIF_DESC_FLAG_NEXT) {
> > > mbuf_tail = mbuf;
> > > mbuf = rte_pktmbuf_alloc(mq->mempool);
> > > if (unlikely(mbuf == NULL))
> > > goto no_free_bufs;
> > > - mbuf->port = mq->in_port;
> > > ret = memif_pktmbuf_chain(mbuf_head,
> > mbuf_tail, mbuf);
> > > if (unlikely(ret < 0)) {
> > > MIF_LOG(ERR, "number-of-segments-
> > overflow");
> > > rte_pktmbuf_free(mbuf);
> > > goto no_free_bufs;
> > > }
> > > + goto next_slot1;
> > > }
> >
> > It is very hard to comment on the correct part of the patch, since it
> > is mixed a lot, but
> > - previously when memif buffer is segmented, and its size is less than
> > mbuf; mbuf is filled with as much memif data as possible and later
> > switched to next mbuf, like:
> >
> > memif buffer
> > +-+ +-+ +-+ +-+
> > |a|->|b|->|c|->|d|
> > +-+ +-+ +-+ +-+
> >
> > +---+ +---+
> > |abc|->|d |
> > +---+ +---+
> > mbuf
> >
> >
> > - Now each memif segment is a mbuf,
> >
> > memif buffer
> > +-+ +-+ +-+ +-+
> > |a|->|b|->|c|->|d|
> > +-+ +-+ +-+ +-+
> >
> > +---+ +---+ +---+ +---+
> > |a |->|b |->|c |->|d |
> > +---+ +---+ +---+ +---+
> > mbuf
> >
> > Can you please confirm this behavior change? If so can you please
> > highlight is more in the commit log?
> > And is this tradeoff something preferred?
Yes, the patch leads to the behavior change, and I will highlight more in the commit
log for next version.
This change is the same as zero-copy mode does, reducing complexed comparation
with more memory space. I am also looking forward to get some feedback from the
community about the tradeoff.
Thanks,
Joyce
next prev parent reply other threads:[~2022-05-19 8:45 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 [this message]
2022-05-18 17:06 ` Ferruh Yigit
2022-05-19 15:09 ` Joyce Kong
2022-05-19 16:38 ` Ferruh Yigit
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=AS4PR08MB7712D6D827651AC1D105DAD992D09@AS4PR08MB7712.eurprd08.prod.outlook.com \
--to=joyce.kong@arm.com \
--cc=Ruifeng.Wang@arm.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@xilinx.com \
--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).