Hi Ferruh,
Will fix it today.
Igor
On Wed, Sep 26, 2018 at 1:41 PM Ferruh Yigit <ferruh.yi...@intel.com> wrote:
> On 9/23/2018 8:12 PM, Igor Ryzhov wrote:
> > Long time ago preallocation of memory for KNI was introduced in commit
> > 0c6bc8e. It was done because of lack of ability to free previously
> > allocated memzones, which led to memzone exhaustion. Currently memzones
> > can be freed and this patch uses this ability for dynamic KNI memory
> > allocation.
>
> Hi Igor,
>
> Good cleanup, thanks.
> +1 to eal_tailq for ctx
>
> A few minor comments below, but they are not significant enough to block
> the
> patch, please let us know if you don't have bandwidth for a new version.
>
> > Signed-off-by: Igor Ryzhov <iryz...@nfware.com>
> <...>
>
> > @@ -294,41 +180,52 @@ rte_kni_alloc(struct rte_mempool *pktmbuf_pool,
> > {
> > int ret;
> > struct rte_kni_device_info dev_info;
> > - struct rte_kni *ctx;
> > - char intf_name[RTE_KNI_NAMESIZE];
> > - const struct rte_memzone *mz;
> > - struct rte_kni_memzone_slot *slot = NULL;
> > + struct rte_kni *kni;
> > + struct rte_tailq_entry *te = NULL;
> > + struct rte_kni_list *kni_list;
> > +
> > + kni_list = RTE_TAILQ_CAST(rte_kni_tailq.head, rte_kni_list);
>
> Can you move this below input validation, no need this assignment if API
> will
> fail because of wrong input.
>
> > if (!pktmbuf_pool || !conf || !conf->name[0])
> > return NULL;
> >
> > /* Check if KNI subsystem has been initialized */
> > - if (kni_memzone_pool.initialized != 1) {
> > + if (kni_fd < 0) {
> > RTE_LOG(ERR, KNI, "KNI subsystem has not been initialized.
> Invoke rte_kni_init() first\n");
> > return NULL;
> > }
> >
> > - /* Get an available slot from the pool */
> > - slot = kni_memzone_pool_alloc();
> > - if (!slot) {
> > - RTE_LOG(ERR, KNI, "Cannot allocate more KNI interfaces;
> increase the number of max_kni_ifaces(current %d) or release unused
> ones.\n",
> > - kni_memzone_pool.max_ifaces);
> > - return NULL;
> > + rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK);
> > +
> > + TAILQ_FOREACH(te, kni_list, next) {
> > + kni = (struct rte_kni *) te->data;
> > + if (strncmp(conf->name, kni->name, RTE_KNI_NAMESIZE) == 0)
> > + break;
> > }
>
> This is rte_kni_get(), why not reuse it. You can create an version of it
> without
> lock.
>
> like _rte_kni_get() which you can call here.
>
> And
> rte_kni_get()
> rte_rwlock_read_lock(RTE_EAL_TAILQ_RWLOCK);
> _rte_kni_get()
> rte_rwlock_read_unlock(RTE_EAL_TAILQ_RWLOCK);
>
> <...>
>
> > +
> > + if (te == NULL) {
> > + goto unlock;
> > + }
>
> No need {} for single line.
> One more below.
>
