From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 0D802689B for ; Wed, 11 Jan 2017 19:41:31 +0100 (CET) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga101.fm.intel.com with ESMTP; 11 Jan 2017 10:41:30 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,346,1477983600"; d="scan'208";a="51892785" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.38]) ([10.237.220.38]) by orsmga005.jf.intel.com with ESMTP; 11 Jan 2017 10:41:29 -0800 To: "Ananyev, Konstantin" , Stephen Hemminger References: <1483048216-2936-1-git-send-email-s.vyazmitinov@brain4net.com> <20170111081759.7b1ee146@xeon-e3> <2601191342CEEE43887BDE71AB9772583F103F8F@irsmsx105.ger.corp.intel.com> <20170111093559.753a0fc9@xeon-e3> <2601191342CEEE43887BDE71AB9772583F103FCA@irsmsx105.ger.corp.intel.com> <63819ae1-f056-0ad7-b7dd-041fe1fe08fa@intel.com> <2601191342CEEE43887BDE71AB9772583F104048@irsmsx105.ger.corp.intel.com> Cc: Sergey Vyazmitinov , "olivier.matz@6wind.com" , "dev@dpdk.org" From: Ferruh Yigit Message-ID: Date: Wed, 11 Jan 2017 18:41:28 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <2601191342CEEE43887BDE71AB9772583F104048@irsmsx105.ger.corp.intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs 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: Wed, 11 Jan 2017 18:41:32 -0000 On 1/11/2017 6:25 PM, Ananyev, Konstantin wrote: > > >> -----Original Message----- >> From: Yigit, Ferruh >> Sent: Wednesday, January 11, 2017 5:48 PM >> To: Ananyev, Konstantin ; Stephen Hemminger >> Cc: Sergey Vyazmitinov ; olivier.matz@6wind.com; dev@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs >> >> On 1/11/2017 5:43 PM, Ananyev, Konstantin wrote: >>> >>> >>>> -----Original Message----- >>>> From: Stephen Hemminger [mailto:stephen@networkplumber.org] >>>> Sent: Wednesday, January 11, 2017 5:36 PM >>>> To: Ananyev, Konstantin >>>> Cc: Sergey Vyazmitinov ; olivier.matz@6wind.com; Yigit, Ferruh ; >> dev@dpdk.org >>>> Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs >>>> >>>> On Wed, 11 Jan 2017 17:28:21 +0000 >>>> "Ananyev, Konstantin" wrote: >>>> >>>>>> -----Original Message----- >>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger >>>>>> Sent: Wednesday, January 11, 2017 4:18 PM >>>>>> To: Sergey Vyazmitinov >>>>>> Cc: olivier.matz@6wind.com; Yigit, Ferruh ; dev@dpdk.org >>>>>> Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs >>>>>> >>>>>> On Fri, 30 Dec 2016 04:50:16 +0700 >>>>>> Sergey Vyazmitinov wrote: >>>>>> >>>>>>> /** >>>>>>> + * Free n packets mbuf back into its original mempool. >>>>>>> + * >>>>>>> + * Free each mbuf, and all its segments in case of chained buffers. Each >>>>>>> + * segment is added back into its original mempool. >>>>>>> + * >>>>>>> + * @param mp >>>>>>> + * The packets mempool. >>>>>>> + * @param mbufs >>>>>>> + * The packets mbufs array to be freed. >>>>>>> + * @param n >>>>>>> + * Number of packets. >>>>>>> + */ >>>>>>> +static inline void rte_pktmbuf_free_bulk(struct rte_mempool *mp, >>>>>>> + struct rte_mbuf **mbufs, unsigned n) >>>>>>> +{ >>>>>>> + struct rte_mbuf *mbuf, *m_next; >>>>>>> + unsigned i; >>>>>>> + for (i = 0; i < n; ++i) { >>>>>>> + mbuf = mbufs[i]; >>>>>>> + __rte_mbuf_sanity_check(mbuf, 1); >>>>>>> + >>>>>>> + mbuf = mbuf->next; >>>>>>> + while (mbuf != NULL) { >>>>>>> + m_next = mbuf->next; >>>>>>> + rte_pktmbuf_free_seg(mbuf); >>>>>>> + mbuf = m_next; >>>>>>> + } >>>>>>> + } >>>>>>> + rte_mempool_put_bulk(mp, (void * const *)mbufs, n); >>>>>>> +} >>>>>> >>>>>> The mbufs may come from different pools. You need to handle that. >>>>> >>>>> I suppose both stituations are possible: >>>>> 1) user knows off-hand that all mbufs in the group are from the same mempool >>>>> 2) user can't guarantee that all mbufs in the group are from same mempool. >>>>> >>>>> As I understand that patch is for case 1) only. >>>>> For 2) it could be a separate function and separate patch. >>>>> >>>>> Konstantin >>>>> >>>>> >>>> >>>> Please don't make unnecessary assumptions in pursuit of minor optimizations. >>> >>> I don't suggest to make *any* assumptions. >>> What I am saying we can have 2 functions for two different cases. >> >> kni_free_mbufs() is static function. Even user knows if all mbufs are >> some same pool or not, can't pass this information to the free function. >> >> Of course this information can be passed via new API, or as an update to >> exiting API, but I think it is better to update free function to cover >> both cases instead of getting this information from user. > > I suppose misunderstanding came from the fact that kni_free_mbufs() > is modified to use rte_pktmbuf_free_bulk(mp, mbufs, n). > I am not talking about kni part of the patch > (to be honest I didn't pay much attention to it). > What I am saying there are many situations when user knows off-hand > that all mbufs in that group are from the same mempool and such > function will be useful too. > BTW, for my own curiosity, how it could happen with KNI that > kni_fifo_get() would return mbufs not from kni->pktmbuf_pool > (I am not really familiar with KNI and its use-cases)? It gets packets from free queue: kni_fifo_get(kni->free_q, ...) DPDK app may send a mbuf (from any pool, like another port's mempool) to kernel, kernel puts buf back to kni->free_q when done with it. > Konstantin > >> >>> Obviously we'll have to document it properly. >>> Konstantin >>> >>>> It is trivial to write a correct free bulk that handles pool changing. >>>> Also the free_seg could be bulked as well. >