From: "Hemant@freescale.com" <Hemant@freescale.com>
To: Jay Rolette <rolette@infiniteio.com>, Marc Sune <marc.sune@bisdn.de>
Cc: DPDK <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] kni:optimization of rte_kni_rx_burst
Date: Thu, 26 Feb 2015 07:00:20 +0000 [thread overview]
Message-ID: <BY2PR0301MB06939B503E040A8173F21408C2140@BY2PR0301MB0693.namprd03.prod.outlook.com> (raw)
In-Reply-To: <CADNuJVqw4hBD1kk1Y4A2+8-5yi0A0K9_5zBQaO3zOnnwXC0nog@mail.gmail.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jay Rolette
> Sent: 25/Feb/2015 7:00 PM
> To: Marc Sune
> Cc: DPDK
> Subject: Re: [dpdk-dev] [PATCH] kni:optimization of rte_kni_rx_burst
>
> On Wed, Feb 25, 2015 at 6:38 AM, Marc Sune <marc.sune@bisdn.de> wrote:
>
> >
> > On 25/02/15 13:24, Hemant@freescale.com wrote:
> >
> >> Hi OIivier
> >> Comments inline.
> >> Regards,
> >> Hemant
> >>
> >> -----Original Message-----
> >>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Deme
> >>> Sent: 25/Feb/2015 5:44 PM
> >>> To: dev@dpdk.org
> >>> Subject: Re: [dpdk-dev] [PATCH] kni:optimization of rte_kni_rx_burst
> >>>
> >>> Thank you Hemant, I think there might be one issue left with the
> >>> patch though.
> >>> The alloc_q must initially be filled with mbufs before getting mbuf
> >>> back on the tx_q.
> >>>
> >>> So the patch should allow rte_kni_rx_burst to check if alloc_q is empty.
> >>> If so, it should invoke kni_allocate_mbufs(kni, 0) (to fill the
> >>> alloc_q with MAX_MBUF_BURST_NUM mbufs)
> >>>
> >>> The patch for rte_kni_rx_burst would then look like:
> >>>
> >>> @@ -575,7 +575,7 @@ rte_kni_rx_burst(struct rte_kni *kni, struct
> >>> rte_mbuf **mbufs, unsigned num)
> >>>
> >>> /* If buffers removed, allocate mbufs and then put them into
> >>> alloc_q */
> >>> if (ret)
> >>> - kni_allocate_mbufs(kni);
> >>> + kni_allocate_mbufs(kni, ret); else if
> >>> + (unlikely(kni->alloc_q->write == kni->alloc_q->read))
> >>> + kni_allocate_mbufs(kni, 0);
> >>>
> >>> [hemant] This will introduce a run-time check.
> >>
> >> I missed to include the other change in the patch.
> >> I am doing it in kni_alloc i.e. initiate the alloc_q with default
> >> burst size.
> >> kni_allocate_mbufs(ctx, 0);
> >>
> >> In a way, we are now suggesting to reduce the size of alloc_q to only
> >> default burst size.
> >>
> >
> > As an aside comment here, I think that we should allow to tweak the
> > userspace <-> kernel queue sizes (rx_q, tx_q, free_q and alloc_q) .
> > Whether this should be a build configuration option or a parameter to
> > rte_kni_init(), it is not completely clear to me, but I guess
> > rte_kni_init() is a better option.
> >
>
> rte_kni_init() is definitely a better option. It allows things to be tuned based on
> individual system config rather than requiring different builds.
>
>
> > Having said that, the original mail from Hemant was describing that
> > KNI was giving an out-of-memory. This to me indicates that the pool is
> > incorrectly dimensioned. Even if KNI will not pre-allocate in the
> > alloc_q, or not completely, in the event of high load, you will get
> > this same "out of memory".
> >
> > We can reduce the usage of buffers by the KNI subsystem in kernel
> > space and in userspace, but the kernel will always need a small cache
> > of pre-allocated buffers (coming from user-space), since the KNI
> > kernel module does not know where to grab the packets from (which
> > pool). So my guess is that the dimensioning problem experienced by
> > Hemant would be the same, even with the proposed changes.
> >
> >
> >> Can we reach is situation, when the kernel is adding packets faster
> >> in tx_q than the application is able to dequeue?
> >>
> >
> > I think so. We cannot control much how the kernel will schedule the
> > KNI thread(s), specially if the # of threads in relation to the cores
> > is incorrect (not enough), hence we need at least a reasonable amount
> > of buffering to prevent early dropping to those "internal" burst side effects.
> >
> > Marc
>
>
> Strongly agree with Marc here. We *really* don't want just a single burst worth
> of mbufs available to the kernel in alloc_q. That's just asking for congestion
> when there's no need for it.
>
> The original problem reported by Olivier is more of a resource tuning problem
> than anything else. The number of mbufs you need in the system has to take
> into account internal queue depths.
[hemant] Following are my suggestions for the time being.
1. The existing code allocates X buffers and try to add them to alloc_q. If alloc_q is not having space, it frees them. This is not optimized at all. In the rx_burst, we shall only add the numbers of packets, as removed from tx_q.
2. During the kni_alloc, we can set kni_allocate_mbufs X*Y buffers initially for alloc_q. We can further improve it to make it configurable in future enhancements. Currently we can have the value of Y as 2.
3. kni_allocate_mbufs will allocate as many buffer are requested in function parameter.
>
> Jay
next prev parent reply other threads:[~2015-02-26 7:00 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-25 11:48 Hemant Agrawal
2015-02-25 12:13 ` Olivier Deme
2015-02-25 12:24 ` Hemant
2015-02-25 12:28 ` Olivier Deme
2015-02-25 12:38 ` Marc Sune
2015-02-25 12:51 ` Olivier Deme
2015-02-25 13:29 ` Jay Rolette
2015-02-26 7:00 ` Hemant [this message]
2015-02-26 12:56 ` Marc Sune
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=BY2PR0301MB06939B503E040A8173F21408C2140@BY2PR0301MB0693.namprd03.prod.outlook.com \
--to=hemant@freescale.com \
--cc=dev@dpdk.org \
--cc=marc.sune@bisdn.de \
--cc=rolette@infiniteio.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).