From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mta113.f1.k8.com.br (mta113.f1.k8.com.br [187.73.32.185]) by dpdk.org (Postfix) with ESMTP id 7471D326C for ; Thu, 6 Sep 2018 15:35:00 +0200 (CEST) Received: from [192.168.1.4] (pool-173-48-214-200.bstnma.fios.verizon.net [173.48.214.200]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtpz.f1.k8.com.br (Postfix) with ESMTPSA id AF5EC800ED; Thu, 6 Sep 2018 13:34:53 +0000 (UTC) X-DKIM: OpenDKIM Filter v2.6.8 smtpz.f1.k8.com.br AF5EC800ED DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=digirati.com.br; s=default; t=1536240898; bh=XJ+XhESgVjsCq2W1Rwjs+ZQyC4LTuawrOI72iciRrL8=; h=Subject:To:From:Date:Feedback-ID; b=t7TAluemFDIWVdYoNBQLI2T/BfaUogBQAyVQxoESxLjXKa9VJDix4WzeCy/6vWUMB zswk+2gQ3AfLlnwd3X7IGD829BiX++cDKDti6bHrpK5Z0VORVd2kpELaGQIGDN6LFm IJaIZm740m5JkiMID2fQ4RqeN57pTSmfV51GdD2I= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=k8.com.br; s=default; t=1536240898; bh=XJ+XhESgVjsCq2W1Rwjs+ZQyC4LTuawrOI72iciRrL8=; h=Subject:To:From:Date:Feedback-ID; b=nNaUZdFXM3WviFXEys7NqhMFTTKg+U2ClRg7OcS3scilmGRCgjTc/CRwfpOYD9eDy pVST2+VlrDV7DqpVsQYGcegkCKNABMnUoeaNDGkhIo6R5Q3rlJt3/lLMfNqR1NmPbl /ZWTdDHuGQQLFhDSrLf9sJcebYqnUuxsF+NMB8zk= To: "Wang, Yipeng1" , Qiaobin Fu , "Richardson, Bruce" , "De Lara Guarch, Pablo" Cc: "dev@dpdk.org" , "doucette@bu.edu" , "Wiles, Keith" , "Gobriel, Sameh" , "Tai, Charlie" , "stephen@networkplumber.org" , "nd@arm.com" , "honnappa.nagarahalli@arm.com" References: <20180831165101.20026-1-qiaobinf@bu.edu> <037eca0c-a1e5-a7fa-8b5e-d7a35006e852@digirati.com.br> From: Michel Machado Organization: Digirati Internet LTDA. Message-ID: <8e1e18a4-1110-aea9-ba7d-5b0a2b8fa9c4@digirati.com.br> Date: Thu, 6 Sep 2018 09:34:50 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-HN-S: bWljaGVsQGRpZ2lyYXRpLmNvbS5icg== X-HN-R: aG9ubmFwcGEubmFnYXJhaGFsbGlAYXJtLmNvbSxuZEBhcm0uY29tLHN0ZXBoZW5AbmV0d29ya3BsdW1iZXIub3JnLGNoYXJsaWUudGFpQGludGVsLmNvbSxzYW1laC5nb2JyaWVsQGludGVsLmNvbSxrZWl0aC53aWxlc0BpbnRlbC5jb20sZG91Y2V0dGVAYnUuZWR1LGRldkBkcGRrLm9yZyxwYWJsby5kZS5sYXJhLmd1YXJjaEBpbnRlbC5jb20sYnJ1Y2UucmljaGFyZHNvbkBpbnRlbC5jb20scWlhb2JpbmZAYnUuZWR1LHlpcGVuZzEud2FuZ0BpbnRlbC5jb20= Feedback-ID: MjAxODA5MDY=:bWljaGVsQGRpZ2lyYXRpLmNvbS5icg==:ZGlnaXJhdGkuY29tLmJy:k8networks X-Mailman-Approved-At: Fri, 07 Sep 2018 13:28:17 +0200 Subject: Re: [dpdk-dev] [PATCH v3] hash table: add an iterator over conflicting entries 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: Thu, 06 Sep 2018 13:35:01 -0000 On 09/05/2018 04:27 PM, Wang, Yipeng1 wrote: > Hmm I see, it falls back to my original thought to have malloc inside the init function.. > Thanks for the explanation. :) > > So I guess with your implementation, in future if we change the internal state to be larger, > the ABI will be broken. If that happens, yes, the ABI would need to change again. But this concern is overblown for two reasons. First, this event is unlikely to happen because struct rte_hash_iterator_state is already allocating 64 bytes while struct rte_hash_iterator_istate and struct rte_hash_iterator_conflict_entries_istate consume 16 and 20 bytes, respectively. Thus, the complexity of the underlying hash algorithm would need to grow substantially to force the necessary state of these iterators to grow more than 4x and 3x, respectively. This is unlikely to happen, and, if it does, it would likely break the ABI somewhere else and have a high impact on applications anyway. Second, even if the unlikely event happens, all one would need to do is to increase the size of struct rte_hash_iterator_state, mark the new API as a new version, and applications would be ready for the new ABI just recompiling. > BTW, this patch set also changes API so proper notice is needed. > People more familiar with API/ABI change policies may be able to help here. We'd be happy to get feedback on this aspect. > Just to confirm, is there anyway like I said for your application to have some long-live states > and reuse them throughout the application so that you don’t have to have short-lived ones in stack? Two things would need to happen for this to be possible. The init functions would need to accept previously allocated iterator states, that is, the init function would act as a reset of the state when acting on a previous allocated state. And, applications would now need to carry these pre-allocated state to avoid a malloc. In order words, we'll increase the complexity of the API. To emphasize that the cost of a malloc is not negligible, rte_malloc() needs to get a spinlock (see heap_alloc_on_socket()), do its thing to allocate memory, and, if the first attempt fails, try to allocate the memory on other sockets (see end of malloc_heap_alloc()). For an iterator that goes through the whole hash table, this cost may be okay, but for an iterator that goes through a couple entries, this cost is a lot to add. This memory allocation concern is not new. Function rte_pktmbuf_read(), for example, let applications pass buffers, which are often allocated in the execution stack, to avoid the malloc cost. [ ]'s Michel Machado