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.
