From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 381E04CA1 for ; Mon, 26 Mar 2018 18:44:41 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Mar 2018 09:44:40 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,365,1517904000"; d="scan'208";a="42306276" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.63]) ([10.237.221.63]) by orsmga001.jf.intel.com with ESMTP; 26 Mar 2018 09:44:39 -0700 To: zhouyangchao Cc: dev@dpdk.org References: From: Ferruh Yigit Openpgp: preference=signencrypt Autocrypt: addr=ferruh.yigit@intel.com; keydata= xsFNBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABzSVGZXJydWggWWln aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+wsF+BBMBAgAoAhsDBgsJCAcDAgYVCAIJCgsE FgIDAQIeAQIXgAUCWZR1xgUJB33UawAKCRD5M+tD3xNhH3TID/wNjq1doFXg49WaM7ZXez/1 qwj7U6FQ2eUhlEPX8dXc52cu+iQk8/mssXQtANEx+jndxfvlkikBNgI+mr3m/ho54uQWFZd7 Bv0DVVkLHNkLWK6tT6iISdCgXrQuOv8y5rucEGkJ8dDPsp8Mqr4sBjavRBpczphoa3miKusA HzUEo0SYuHUPmBlbmvKv4PkA5quWtXpkgV2HZ1bW9X0dQkAqEVioAPm5Q6pXJODfV8kaZUtJ z4swEtBnK9XAWm2HccF7KmFh0vv+Zh9lEMnEFt7UPXngY6+xCTo6xV1IVc8EZfDjOip8I4h2 ALMaLgrZwA9VVmHCVOrnO3UZqfGehrwz4O3sUrcmIzxjk3gfBmVRCBfuWGchSpWtZ99U6V1a OEuG+ymyjBDtyymW+KSvmCBl2gIxZHvZFvSRRdDObkkUIskSJ95f/6HBOESRZBOiV9GEAhC5 gI9OAKKF8HQaN/r5KJrkzscjQcjIdV1jXhTkfQ4wH3GJiVM5JxsKsEjjRw7yfSJo2GBEGqMe KMwMLWg4f1DgVtjOuGf10isu+29MvapW2IxKYaHcVc2vHfWbDi2AvBj/VAzKILWbTEgI1VL5 zKpo5p6X2O55oEyeflDiAzrUfvLqB4vmTyqXtW6PdLyZC7kXIzmNu6EBVx9oSgy3CADw5saN 0La9OoCAc7Tn+s7BTQRX1ky+ARAApzQNvXvE2q1LAS+Z+ni2R13Bb1cDS1ZYq1jgpR13+OKN ipzd8MPngRJilXxBaPTErhgzR0vGcNTYhjGMSyFIHVOoBq1VbP1a0Fi/NqWzJOowo/fDfgVy K4vuitc/gCJs+2se4hdZA4EQJxVlNM51lgYDNpjPGIA43MX15OLAip73+ho6NPBMuc5qse3X pAClNhBKfENRCWN428pi3WVkT+ABRTE0taxjJNP7bb+9TQYNRqGwnGzX5/XISv44asWIQCaq vOkXSUJLd//cdVNTqtL1wreCVVR5pMXj7VIrlk07fmmJVALCmGbFr53BMb8O+8dgK2A5mitM n44d+8KdJWOwziRxcaMk/LclmZS3Iv1TERtiWt98Y9AjeAtcgYPkA3ld0BcUKONogP8pHVz1 Ed3s5rDQ91yr1S0wuAzW91fxGUO4wY+uPmxCtFVuBgd9VT9NAKTUL0qHM7CDgCnZPe0TW6Zj 8OqtdCCyAfvU9cW5xWM7Icxhde6AtPxhDSBwE8fL2ZmrDmaA4jmUKXp3i4JxRPSX84S08b+s DWXHPxy10UFU5A7EK/BEbZAKBwn9ROfm+WK+6X5xOGLoRE++OqNuUudxC1GDyLOPaqCbBCS9 +P6HsTHzxsjyJa27n4jcrcuY3P9TEcFJYSZSeSDh8mVGvugi0exnSJrrBZDyVCcAEQEAAcLB ZQQYAQIADwIbDAUCWZR1ZwUJA59cIQAKCRD5M+tD3xNhH5b+D/9XG44Ci6STdcA5RO/ur05J EE3Ux1DCHZ5V7vNAtX/8Wg4l4GZfweauXwuJ1w7Sp7fklwcNC6wsceI+EmNjGMqfIaukGetG +jBGqsQ7moOZodfXUoCK98gblKgt/BPYMVidzlGC8Q/+lZg1+o29sPnwImW+MXt/Z5az/Z17 Qc265g+p5cqJHzq6bpQdnF7Fu6btKU/kv6wJghENvgMXBuyThqsyFReJWFh2wfaKyuix3Zyj ccq7/blkhzIKmtFWgDcgaSc2UAuJU+x9nuYjihW6WobpKP/nlUDu3BIsbIq09UEke+uE/QK+ FJ8PTJkAsXOf1Bc2C0XbW4Y2hf103+YY6L8weUCBsWC5VH5VtVmeuh26ENURclwfeXhWQ9Og 77yzpTXWr5g1Z0oLpYpWPv745J4bE7pv+dzxOrFdM1xNkzY2pvXph/A8OjxZNQklDkHQ7PIB Lki5L2F4XkEOddUUQchJwzMqTPsggPDmGjgLZrqgO+s4ECZK5+nLD3HEpAbPa3JLDaScy+90 Nu1lAqPUHSnP3vYZVw85ZYm6UCxHE4VLMnnJsN09ZhsOSVR+GyP5Nyw9rT1V3lcsuH7M5Naa 2Xobn9m7l9bRCD/Ji8kG15eV1WTxx1HXVQGjdUYDI7UwegBNbwMLh17XDy+3sn/6SgcqtECA Q6pZKA2mTQxEKA== Message-ID: Date: Mon, 26 Mar 2018 17:44:38 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH] kni: optimize the kni release speed X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 26 Mar 2018 16:44:41 -0000 On 2/6/2018 10:33 AM, zhouyangchao wrote: > Physical addresses in the fifo named alloc_q need to be traversed to > release in user space. The physical address to the virtual address > conversion in kernel space is much better. Yes current approach should be slower but this is not in data path, this is when a kni interface released, I expect no recognizable difference. > > Signed-off-by: Yangchao Zhou > --- > lib/librte_eal/linuxapp/kni/kni_dev.h | 1 + > lib/librte_eal/linuxapp/kni/kni_misc.c | 1 + > lib/librte_eal/linuxapp/kni/kni_net.c | 15 +++++++++++++++ > lib/librte_kni/rte_kni.c | 26 +------------------------- > 4 files changed, 18 insertions(+), 25 deletions(-) > > diff --git a/lib/librte_eal/linuxapp/kni/kni_dev.h b/lib/librte_eal/linuxapp/kni/kni_dev.h > index c9393d8..7cd9bf8 100644 > --- a/lib/librte_eal/linuxapp/kni/kni_dev.h > +++ b/lib/librte_eal/linuxapp/kni/kni_dev.h > @@ -92,6 +92,7 @@ struct kni_dev { > void *alloc_va[MBUF_BURST_SZ]; > }; > > +void kni_net_fifo_pa2va(struct kni_dev *kni); > void kni_net_rx(struct kni_dev *kni); > void kni_net_init(struct net_device *dev); > void kni_net_config_lo_mode(char *lo_str); > diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c > index 01574ec..668488b 100644 > --- a/lib/librte_eal/linuxapp/kni/kni_misc.c > +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c > @@ -507,6 +507,7 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num, > dev->pthread = NULL; > } > > + kni_net_fifo_pa2va(dev); > kni_dev_remove(dev); > list_del(&dev->list); > ret = 0; > diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c b/lib/librte_eal/linuxapp/kni/kni_net.c > index 9f9b798..662a527 100644 > --- a/lib/librte_eal/linuxapp/kni/kni_net.c > +++ b/lib/librte_eal/linuxapp/kni/kni_net.c > @@ -73,6 +73,21 @@ va2pa(void *va, struct rte_kni_mbuf *m) > return pa; > } > > +/* convert physical addresses to virtual addresses in fifo for kni release */ > +void > +kni_net_fifo_pa2va(struct kni_dev *kni) > +{ > + void *fifo = kni->alloc_q; > + int i, count = kni_fifo_count(fifo); > + void *pa = NULL, *kva, *va; > + for (i = 0; i < count; ++i) { > + (void)kni_fifo_get(fifo, &pa, 1); > + kva = pa2kva(pa); > + va = pa2va(pa, kva); > + (void)kni_fifo_put(fifo, &va, 1); kni fifo are single producer, single consumer. For alloc_q kernel side is consumer, I aware at this stage applications should stop the traffic, but still I am not comfortable mixing producer/consumer roles here. Also alloc_q should have physical addresses this logic stores virtual addresses in it and not sure about this either to mix addressing logic in the queue. Instead of this conversion, what about moving from alloc_q to free_q? free_q already has virtual addresses and freed by userspace, so this will be safe. I suggest keeping alloc_q free logic in the userspace in any case, if alloc_q is free it won't cost anyway. And while checking for this I may found something else. We have same problem with rx_q, it has physical addresses which makes hard to free in userspace. The existing intention is to give some time to kernel to consume the rx_q so that it won't be an issue for userspace. But that logic can be wrong. During the time userspace waits the netdev may be already destroyed and there is nothing to receive the packet, perhaps we should move wait above the ioctl. Since you are already checking these parts perhaps you would like to comment :) > + } > +} > + > /* > * It can be called to process the request. > */ > diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c > index 2867411..f8398a9 100644 > --- a/lib/librte_kni/rte_kni.c > +++ b/lib/librte_kni/rte_kni.c > @@ -435,30 +435,6 @@ va2pa(struct rte_mbuf *m) > (unsigned long)m->buf_iova)); > } > > -static void > -obj_free(struct rte_mempool *mp __rte_unused, void *opaque, void *obj, > - unsigned obj_idx __rte_unused) > -{ > - struct rte_mbuf *m = obj; > - void *mbuf_phys = opaque; > - > - if (va2pa(m) == mbuf_phys) > - rte_pktmbuf_free(m); > -} > - > -static void > -kni_free_fifo_phy(struct rte_mempool *mp, struct rte_kni_fifo *fifo) > -{ > - void *mbuf_phys; > - int ret; > - > - do { > - ret = kni_fifo_get(fifo, &mbuf_phys, 1); > - if (ret) > - rte_mempool_obj_iter(mp, obj_free, mbuf_phys); > - } while (ret); > -} > - > int > rte_kni_release(struct rte_kni *kni) > { > @@ -484,7 +460,7 @@ rte_kni_release(struct rte_kni *kni) > if (kni_fifo_count(kni->rx_q)) > RTE_LOG(ERR, KNI, "Fail to free all Rx-q items\n"); > > - kni_free_fifo_phy(kni->pktmbuf_pool, kni->alloc_q); > + kni_free_fifo(kni->alloc_q); > kni_free_fifo(kni->tx_q); > kni_free_fifo(kni->free_q); > >