* [dpdk-dev] [PATCH] kni:optimization of rte_kni_rx_burst
@ 2015-02-25 11:48 Hemant Agrawal
2015-02-25 12:13 ` Olivier Deme
0 siblings, 1 reply; 9+ messages in thread
From: Hemant Agrawal @ 2015-02-25 11:48 UTC (permalink / raw)
To: dev
From: Hemant Agrawal <hemant@freescale.com>
if any buffer is read from the tx_q, MAX_BURST buffers will be allocated and attempted to be added to to the alloc_q.
This seems terribly inefficient and it also looks like the alloc_q will quickly fill to its maximum capacity. If the system buffers are low in number, it will reach "out of memory" situation.
This patch allocates the number of buffers as many dequeued from tx_q.
Signed-off-by: Hemant Agrawal <hemant@freescale.com>
---
lib/librte_kni/rte_kni.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index 4e70fa0..4cf8e30 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -128,7 +128,7 @@ struct rte_kni_memzone_pool {
static void kni_free_mbufs(struct rte_kni *kni);
-static void kni_allocate_mbufs(struct rte_kni *kni);
+static void kni_allocate_mbufs(struct rte_kni *kni, int num);
static volatile int kni_fd = -1;
static struct rte_kni_memzone_pool kni_memzone_pool = {
@@ -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);
return ret;
}
@@ -594,7 +594,7 @@ kni_free_mbufs(struct rte_kni *kni)
}
static void
-kni_allocate_mbufs(struct rte_kni *kni)
+kni_allocate_mbufs(struct rte_kni *kni, int num)
{
int i, ret;
struct rte_mbuf *pkts[MAX_MBUF_BURST_NUM];
@@ -620,7 +620,10 @@ kni_allocate_mbufs(struct rte_kni *kni)
return;
}
- for (i = 0; i < MAX_MBUF_BURST_NUM; i++) {
+ if (num == 0 || num > MAX_MBUF_BURST_NUM)
+ num = MAX_MBUF_BURST_NUM;
+
+ for (i = 0; i < num; i++) {
pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool);
if (unlikely(pkts[i] == NULL)) {
/* Out of memory */
@@ -636,7 +639,7 @@ kni_allocate_mbufs(struct rte_kni *kni)
ret = kni_fifo_put(kni->alloc_q, (void **)pkts, i);
/* Check if any mbufs not put into alloc_q, and then free them */
- if (ret >= 0 && ret < i && ret < MAX_MBUF_BURST_NUM) {
+ if (ret >= 0 && ret < i && ret < num) {
int j;
for (j = ret; j < i; j++)
--
1.9.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] kni:optimization of rte_kni_rx_burst
2015-02-25 11:48 [dpdk-dev] [PATCH] kni:optimization of rte_kni_rx_burst Hemant Agrawal
@ 2015-02-25 12:13 ` Olivier Deme
2015-02-25 12:24 ` Hemant
0 siblings, 1 reply; 9+ messages in thread
From: Olivier Deme @ 2015-02-25 12:13 UTC (permalink / raw)
To: dev
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);
Olivier.
On 25/02/15 11:48, Hemant Agrawal wrote:
> From: Hemant Agrawal <hemant@freescale.com>
>
> if any buffer is read from the tx_q, MAX_BURST buffers will be allocated and attempted to be added to to the alloc_q.
> This seems terribly inefficient and it also looks like the alloc_q will quickly fill to its maximum capacity. If the system buffers are low in number, it will reach "out of memory" situation.
>
> This patch allocates the number of buffers as many dequeued from tx_q.
>
> Signed-off-by: Hemant Agrawal <hemant@freescale.com>
> ---
> lib/librte_kni/rte_kni.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
> index 4e70fa0..4cf8e30 100644
> --- a/lib/librte_kni/rte_kni.c
> +++ b/lib/librte_kni/rte_kni.c
> @@ -128,7 +128,7 @@ struct rte_kni_memzone_pool {
>
>
> static void kni_free_mbufs(struct rte_kni *kni);
> -static void kni_allocate_mbufs(struct rte_kni *kni);
> +static void kni_allocate_mbufs(struct rte_kni *kni, int num);
>
> static volatile int kni_fd = -1;
> static struct rte_kni_memzone_pool kni_memzone_pool = {
> @@ -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);
>
> return ret;
> }
> @@ -594,7 +594,7 @@ kni_free_mbufs(struct rte_kni *kni)
> }
>
> static void
> -kni_allocate_mbufs(struct rte_kni *kni)
> +kni_allocate_mbufs(struct rte_kni *kni, int num)
> {
> int i, ret;
> struct rte_mbuf *pkts[MAX_MBUF_BURST_NUM];
> @@ -620,7 +620,10 @@ kni_allocate_mbufs(struct rte_kni *kni)
> return;
> }
>
> - for (i = 0; i < MAX_MBUF_BURST_NUM; i++) {
> + if (num == 0 || num > MAX_MBUF_BURST_NUM)
> + num = MAX_MBUF_BURST_NUM;
> +
> + for (i = 0; i < num; i++) {
> pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool);
> if (unlikely(pkts[i] == NULL)) {
> /* Out of memory */
> @@ -636,7 +639,7 @@ kni_allocate_mbufs(struct rte_kni *kni)
> ret = kni_fifo_put(kni->alloc_q, (void **)pkts, i);
>
> /* Check if any mbufs not put into alloc_q, and then free them */
> - if (ret >= 0 && ret < i && ret < MAX_MBUF_BURST_NUM) {MAX_MBUF_BURST_NUM
>
> + if (ret >= 0 && ret < i && ret < num) {
> int j;
>
> for (j = ret; j < i; j++)
--
*Olivier Demé*
*Druid Software Ltd.*
*Tel: +353 1 202 1831*
*Email: odeme@druidsoftware.com <mailto:odeme@druidsoftware.com>*
*URL: http://www.druidsoftware.com*
*Hall 7, stand 7F70.*
Druid Software: Monetising enterprise small cells solutions.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] kni:optimization of rte_kni_rx_burst
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
0 siblings, 2 replies; 9+ messages in thread
From: Hemant @ 2015-02-25 12:24 UTC (permalink / raw)
To: Olivier Deme, dev
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.
Can we reach is situation, when the kernel is adding packets faster in tx_q than the application is able to dequeue?
alloc_q can be empty in this case and kernel will be striving.
>
> Olivier.
>
> On 25/02/15 11:48, Hemant Agrawal wrote:
> > From: Hemant Agrawal <hemant@freescale.com>
> >
> > if any buffer is read from the tx_q, MAX_BURST buffers will be allocated and
> attempted to be added to to the alloc_q.
> > This seems terribly inefficient and it also looks like the alloc_q will quickly fill
> to its maximum capacity. If the system buffers are low in number, it will reach
> "out of memory" situation.
> >
> > This patch allocates the number of buffers as many dequeued from tx_q.
> >
> > Signed-off-by: Hemant Agrawal <hemant@freescale.com>
> > ---
> > lib/librte_kni/rte_kni.c | 13 ++++++++-----
> > 1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c index
> > 4e70fa0..4cf8e30 100644
> > --- a/lib/librte_kni/rte_kni.c
> > +++ b/lib/librte_kni/rte_kni.c
> > @@ -128,7 +128,7 @@ struct rte_kni_memzone_pool {
> >
> >
> > static void kni_free_mbufs(struct rte_kni *kni); -static void
> > kni_allocate_mbufs(struct rte_kni *kni);
> > +static void kni_allocate_mbufs(struct rte_kni *kni, int num);
> >
> > static volatile int kni_fd = -1;
> > static struct rte_kni_memzone_pool kni_memzone_pool = { @@ -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);
> >
> > return ret;
> > }
> > @@ -594,7 +594,7 @@ kni_free_mbufs(struct rte_kni *kni)
> > }
> >
> > static void
> > -kni_allocate_mbufs(struct rte_kni *kni)
> > +kni_allocate_mbufs(struct rte_kni *kni, int num)
> > {
> > int i, ret;
> > struct rte_mbuf *pkts[MAX_MBUF_BURST_NUM]; @@ -620,7 +620,10
> @@
> > kni_allocate_mbufs(struct rte_kni *kni)
> > return;
> > }
> >
> > - for (i = 0; i < MAX_MBUF_BURST_NUM; i++) {
> > + if (num == 0 || num > MAX_MBUF_BURST_NUM)
> > + num = MAX_MBUF_BURST_NUM;
> > +
> > + for (i = 0; i < num; i++) {
> > pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool);
> > if (unlikely(pkts[i] == NULL)) {
> > /* Out of memory */
> > @@ -636,7 +639,7 @@ kni_allocate_mbufs(struct rte_kni *kni)
> > ret = kni_fifo_put(kni->alloc_q, (void **)pkts, i);
> >
> > /* Check if any mbufs not put into alloc_q, and then free them */
> > - if (ret >= 0 && ret < i && ret < MAX_MBUF_BURST_NUM)
> {MAX_MBUF_BURST_NUM
> >
> > + if (ret >= 0 && ret < i && ret < num) {
> > int j;
> >
> > for (j = ret; j < i; j++)
>
> --
> *Olivier Demé*
> *Druid Software Ltd.*
> *Tel: +353 1 202 1831*
> *Email: odeme@druidsoftware.com <mailto:odeme@druidsoftware.com>*
> *URL: http://www.druidsoftware.com*
> *Hall 7, stand 7F70.*
> Druid Software: Monetising enterprise small cells solutions.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] kni:optimization of rte_kni_rx_burst
2015-02-25 12:24 ` Hemant
@ 2015-02-25 12:28 ` Olivier Deme
2015-02-25 12:38 ` Marc Sune
1 sibling, 0 replies; 9+ messages in thread
From: Olivier Deme @ 2015-02-25 12:28 UTC (permalink / raw)
To: Hemant, dev
I guess it would be unusual but possible for the kernel to enqueue
faster to tx_q than the application dequeues.
But that would also be possible with a real NIC, so I think it is
acceptable for the kernel to have to drop egress packets in that case.
On 25/02/15 12: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.
>
> Can we reach is situation, when the kernel is adding packets faster in tx_q than the application is able to dequeue?
> alloc_q can be empty in this case and kernel will be striving.
>
>> Olivier.
>>
>> On 25/02/15 11:48, Hemant Agrawal wrote:
>>> From: Hemant Agrawal <hemant@freescale.com>
>>>
>>> if any buffer is read from the tx_q, MAX_BURST buffers will be allocated and
>> attempted to be added to to the alloc_q.
>>> This seems terribly inefficient and it also looks like the alloc_q will quickly fill
>> to its maximum capacity. If the system buffers are low in number, it will reach
>> "out of memory" situation.
>>> This patch allocates the number of buffers as many dequeued from tx_q.
>>>
>>> Signed-off-by: Hemant Agrawal <hemant@freescale.com>
>>> ---
>>> lib/librte_kni/rte_kni.c | 13 ++++++++-----
>>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c index
>>> 4e70fa0..4cf8e30 100644
>>> --- a/lib/librte_kni/rte_kni.c
>>> +++ b/lib/librte_kni/rte_kni.c
>>> @@ -128,7 +128,7 @@ struct rte_kni_memzone_pool {
>>>
>>>
>>> static void kni_free_mbufs(struct rte_kni *kni); -static void
>>> kni_allocate_mbufs(struct rte_kni *kni);
>>> +static void kni_allocate_mbufs(struct rte_kni *kni, int num);
>>>
>>> static volatile int kni_fd = -1;
>>> static struct rte_kni_memzone_pool kni_memzone_pool = { @@ -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);
>>>
>>> return ret;
>>> }
>>> @@ -594,7 +594,7 @@ kni_free_mbufs(struct rte_kni *kni)
>>> }
>>>
>>> static void
>>> -kni_allocate_mbufs(struct rte_kni *kni)
>>> +kni_allocate_mbufs(struct rte_kni *kni, int num)
>>> {
>>> int i, ret;
>>> struct rte_mbuf *pkts[MAX_MBUF_BURST_NUM]; @@ -620,7 +620,10
>> @@
>>> kni_allocate_mbufs(struct rte_kni *kni)
>>> return;
>>> }
>>>
>>> - for (i = 0; i < MAX_MBUF_BURST_NUM; i++) {
>>> + if (num == 0 || num > MAX_MBUF_BURST_NUM)
>>> + num = MAX_MBUF_BURST_NUM;
>>> +
>>> + for (i = 0; i < num; i++) {
>>> pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool);
>>> if (unlikely(pkts[i] == NULL)) {
>>> /* Out of memory */
>>> @@ -636,7 +639,7 @@ kni_allocate_mbufs(struct rte_kni *kni)
>>> ret = kni_fifo_put(kni->alloc_q, (void **)pkts, i);
>>>
>>> /* Check if any mbufs not put into alloc_q, and then free them */
>>> - if (ret >= 0 && ret < i && ret < MAX_MBUF_BURST_NUM)
>> {MAX_MBUF_BURST_NUM
>>> + if (ret >= 0 && ret < i && ret < num) {
>>> int j;
>>>
>>> for (j = ret; j < i; j++)
>> --
>> *Olivier Demé*
>> *Druid Software Ltd.*
>> *Tel: +353 1 202 1831*
>> *Email: odeme@druidsoftware.com <mailto:odeme@druidsoftware.com>*
>> *URL: http://www.druidsoftware.com*
>> *Hall 7, stand 7F70.*
>> Druid Software: Monetising enterprise small cells solutions.
--
*Olivier Demé*
*Druid Software Ltd.*
*Tel: +353 1 202 1831*
*Email: odeme@druidsoftware.com <mailto:odeme@druidsoftware.com>*
*URL: http://www.druidsoftware.com*
*Hall 7, stand 7F70.*
Druid Software: Monetising enterprise small cells solutions.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] kni:optimization of rte_kni_rx_burst
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
1 sibling, 2 replies; 9+ messages in thread
From: Marc Sune @ 2015-02-25 12:38 UTC (permalink / raw)
To: dev
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.
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
> alloc_q can be empty in this case and kernel will be striving.
>
>> Olivier.
>>
>> On 25/02/15 11:48, Hemant Agrawal wrote:
>>> From: Hemant Agrawal <hemant@freescale.com>
>>>
>>> if any buffer is read from the tx_q, MAX_BURST buffers will be allocated and
>> attempted to be added to to the alloc_q.
>>> This seems terribly inefficient and it also looks like the alloc_q will quickly fill
>> to its maximum capacity. If the system buffers are low in number, it will reach
>> "out of memory" situation.
>>> This patch allocates the number of buffers as many dequeued from tx_q.
>>>
>>> Signed-off-by: Hemant Agrawal <hemant@freescale.com>
>>> ---
>>> lib/librte_kni/rte_kni.c | 13 ++++++++-----
>>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c index
>>> 4e70fa0..4cf8e30 100644
>>> --- a/lib/librte_kni/rte_kni.c
>>> +++ b/lib/librte_kni/rte_kni.c
>>> @@ -128,7 +128,7 @@ struct rte_kni_memzone_pool {
>>>
>>>
>>> static void kni_free_mbufs(struct rte_kni *kni); -static void
>>> kni_allocate_mbufs(struct rte_kni *kni);
>>> +static void kni_allocate_mbufs(struct rte_kni *kni, int num);
>>>
>>> static volatile int kni_fd = -1;
>>> static struct rte_kni_memzone_pool kni_memzone_pool = { @@ -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);
>>>
>>> return ret;
>>> }
>>> @@ -594,7 +594,7 @@ kni_free_mbufs(struct rte_kni *kni)
>>> }
>>>
>>> static void
>>> -kni_allocate_mbufs(struct rte_kni *kni)
>>> +kni_allocate_mbufs(struct rte_kni *kni, int num)
>>> {
>>> int i, ret;
>>> struct rte_mbuf *pkts[MAX_MBUF_BURST_NUM]; @@ -620,7 +620,10
>> @@
>>> kni_allocate_mbufs(struct rte_kni *kni)
>>> return;
>>> }
>>>
>>> - for (i = 0; i < MAX_MBUF_BURST_NUM; i++) {
>>> + if (num == 0 || num > MAX_MBUF_BURST_NUM)
>>> + num = MAX_MBUF_BURST_NUM;
>>> +
>>> + for (i = 0; i < num; i++) {
>>> pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool);
>>> if (unlikely(pkts[i] == NULL)) {
>>> /* Out of memory */
>>> @@ -636,7 +639,7 @@ kni_allocate_mbufs(struct rte_kni *kni)
>>> ret = kni_fifo_put(kni->alloc_q, (void **)pkts, i);
>>>
>>> /* Check if any mbufs not put into alloc_q, and then free them */
>>> - if (ret >= 0 && ret < i && ret < MAX_MBUF_BURST_NUM)
>> {MAX_MBUF_BURST_NUM
>>> + if (ret >= 0 && ret < i && ret < num) {
>>> int j;
>>>
>>> for (j = ret; j < i; j++)
>> --
>> *Olivier Demé*
>> *Druid Software Ltd.*
>> *Tel: +353 1 202 1831*
>> *Email: odeme@druidsoftware.com <mailto:odeme@druidsoftware.com>*
>> *URL: http://www.druidsoftware.com*
>> *Hall 7, stand 7F70.*
>> Druid Software: Monetising enterprise small cells solutions.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] kni:optimization of rte_kni_rx_burst
2015-02-25 12:38 ` Marc Sune
@ 2015-02-25 12:51 ` Olivier Deme
2015-02-25 13:29 ` Jay Rolette
1 sibling, 0 replies; 9+ messages in thread
From: Olivier Deme @ 2015-02-25 12:51 UTC (permalink / raw)
To: dev
Hi Marc,
I think one of the observations is that currently the alloc_q grows very
quickly to the maximum fifo size (1024).
The patch suggests fixing the alloc_q to a fix size and maybe make that
size configurable in rte_kni_alloc or rte_kni_init.
It should then be up to the application to provision the mempool
accordingly.
Currently the out of memory problem shows up if the mempool doesn't have
1024 buffers per KNI.
Olivier.
On 25/02/15 12:38, Marc Sune 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.
>
> 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
>
>> alloc_q can be empty in this case and kernel will be striving.
>>
>>> Olivier.
>>>
>>> On 25/02/15 11:48, Hemant Agrawal wrote:
>>>> From: Hemant Agrawal <hemant@freescale.com>
>>>>
>>>> if any buffer is read from the tx_q, MAX_BURST buffers will be
>>>> allocated and
>>> attempted to be added to to the alloc_q.
>>>> This seems terribly inefficient and it also looks like the alloc_q
>>>> will quickly fill
>>> to its maximum capacity. If the system buffers are low in number, it
>>> will reach
>>> "out of memory" situation.
>>>> This patch allocates the number of buffers as many dequeued from tx_q.
>>>>
>>>> Signed-off-by: Hemant Agrawal <hemant@freescale.com>
>>>> ---
>>>> lib/librte_kni/rte_kni.c | 13 ++++++++-----
>>>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c index
>>>> 4e70fa0..4cf8e30 100644
>>>> --- a/lib/librte_kni/rte_kni.c
>>>> +++ b/lib/librte_kni/rte_kni.c
>>>> @@ -128,7 +128,7 @@ struct rte_kni_memzone_pool {
>>>>
>>>>
>>>> static void kni_free_mbufs(struct rte_kni *kni); -static void
>>>> kni_allocate_mbufs(struct rte_kni *kni);
>>>> +static void kni_allocate_mbufs(struct rte_kni *kni, int num);
>>>>
>>>> static volatile int kni_fd = -1;
>>>> static struct rte_kni_memzone_pool kni_memzone_pool = { @@ -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);
>>>>
>>>> return ret;
>>>> }
>>>> @@ -594,7 +594,7 @@ kni_free_mbufs(struct rte_kni *kni)
>>>> }
>>>>
>>>> static void
>>>> -kni_allocate_mbufs(struct rte_kni *kni)
>>>> +kni_allocate_mbufs(struct rte_kni *kni, int num)
>>>> {
>>>> int i, ret;
>>>> struct rte_mbuf *pkts[MAX_MBUF_BURST_NUM]; @@ -620,7 +620,10
>>> @@
>>>> kni_allocate_mbufs(struct rte_kni *kni)
>>>> return;
>>>> }
>>>>
>>>> - for (i = 0; i < MAX_MBUF_BURST_NUM; i++) {
>>>> + if (num == 0 || num > MAX_MBUF_BURST_NUM)
>>>> + num = MAX_MBUF_BURST_NUM;
>>>> +
>>>> + for (i = 0; i < num; i++) {
>>>> pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool);
>>>> if (unlikely(pkts[i] == NULL)) {
>>>> /* Out of memory */
>>>> @@ -636,7 +639,7 @@ kni_allocate_mbufs(struct rte_kni *kni)
>>>> ret = kni_fifo_put(kni->alloc_q, (void **)pkts, i);
>>>>
>>>> /* Check if any mbufs not put into alloc_q, and then free
>>>> them */
>>>> - if (ret >= 0 && ret < i && ret < MAX_MBUF_BURST_NUM)
>>> {MAX_MBUF_BURST_NUM
>>>> + if (ret >= 0 && ret < i && ret < num) {
>>>> int j;
>>>>
>>>> for (j = ret; j < i; j++)
>>> --
>>> *Olivier Demé*
>>> *Druid Software Ltd.*
>>> *Tel: +353 1 202 1831*
>>> *Email: odeme@druidsoftware.com <mailto:odeme@druidsoftware.com>*
>>> *URL: http://www.druidsoftware.com*
>>> *Hall 7, stand 7F70.*
>>> Druid Software: Monetising enterprise small cells solutions.
>
--
*Olivier Demé*
*Druid Software Ltd.*
*Tel: +353 1 202 1831*
*Email: odeme@druidsoftware.com <mailto:odeme@druidsoftware.com>*
*URL: http://www.druidsoftware.com*
*Hall 7, stand 7F70.*
Druid Software: Monetising enterprise small cells solutions.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] kni:optimization of rte_kni_rx_burst
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
1 sibling, 1 reply; 9+ messages in thread
From: Jay Rolette @ 2015-02-25 13:29 UTC (permalink / raw)
To: Marc Sune; +Cc: DPDK
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.
Jay
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] kni:optimization of rte_kni_rx_burst
2015-02-25 13:29 ` Jay Rolette
@ 2015-02-26 7:00 ` Hemant
2015-02-26 12:56 ` Marc Sune
0 siblings, 1 reply; 9+ messages in thread
From: Hemant @ 2015-02-26 7:00 UTC (permalink / raw)
To: Jay Rolette, Marc Sune; +Cc: DPDK
> -----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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] kni:optimization of rte_kni_rx_burst
2015-02-26 7:00 ` Hemant
@ 2015-02-26 12:56 ` Marc Sune
0 siblings, 0 replies; 9+ messages in thread
From: Marc Sune @ 2015-02-26 12:56 UTC (permalink / raw)
To: Hemant, Jay Rolette; +Cc: DPDK
On 26/02/15 08:00, Hemant@freescale.com wrote:
>
>> -----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.
Agree
> 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.
Provided that the dimensioning (X*Y), if defined in runtime it is set
during rte_kni_init(), in principle I agree.
However it is not clear to me if you wantg to call
kni_allocate_mbufs(X*Y) for every kni_alloc or just in the first one (in
other words, if X*Y == size of alloc_q). Since alloc_q is shared and
assuming X*Y == size of alloc_q, I think doing it that in the first
kni_alloc() would be sufficient, and then it will get refilled once
RX/TX events happen.
A different approach, that would require more refactor, since it changes
slightly the current strategy, would be to pre-alloc the alloc_q based
of the number of KNI interfaces created (kni_alloc). In this sense,
rte_kni_init() would get then 2 parameters: the length of the entire
shared alloc_q (actually all the queues in the KNI subsystem, with the
current impl.) and the number of buffers / KNI interface. This approach
could lower the mbuf consumption in certain configurations.
> 3. kni_allocate_mbufs will allocate as many buffer are requested in function parameter.
Agree
Marc
>
>> Jay
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-02-26 12:56 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-25 11:48 [dpdk-dev] [PATCH] kni:optimization of rte_kni_rx_burst 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
2015-02-26 12:56 ` Marc Sune
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).