> -----Original Message----- > From: dev [mailto:dev-bounces at 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 at bisdn.de> wrote: > > > > > On 25/02/15 13:24, Hemant at freescale.com wrote: > > > >> Hi OIivier > >> Comments inline. > >> Regards, > >> Hemant > >> > >> -----Original Message----- > >>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Olivier Deme > >>> Sent: 25/Feb/2015 5:44 PM > >>> To: dev at 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
