From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <olivier.matz@6wind.com> Received: from mail-wi0-f179.google.com (mail-wi0-f179.google.com [209.85.212.179]) by dpdk.org (Postfix) with ESMTP id A23652E7B for <dev@dpdk.org>; Wed, 8 Apr 2015 11:44:24 +0200 (CEST) Received: by wiaa2 with SMTP id a2so51297440wia.0 for <dev@dpdk.org>; Wed, 08 Apr 2015 02:44:24 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=g9taF3K2weWD0wYakkbZ/JdRDQmA66uXIkqv5ENW7Go=; b=GH+XAPum4Ekmg7O3H6meIQWKXS/AN4YugOofqKiO7tS27ldK9dI/UgLuhbJyvvme+4 PlY69Lca9m34wVmsfjuqdTpSNabma+CqDMcebLGzXaN1ZaqMZOXBk/e5GleTO9wd5+iG 3zqnbXfGdoi5hoq+mZhuHQstpekIESrBMSct+nImlD8B5+muncyc/sMu1bh/lmaPls1g Vr/OmKxWykzAZi7smTFNH8ydfijaDxZPuSvVVa6lxP7uJ5rEkKixyztpWzhekDjaqL1W d8HkvbSvGDq4s1EGWp8bWlfEyOnZbW9uoIxD72dKgMmd/KYb/bF4TSZflB1KKdMIlk6L ueBQ== X-Gm-Message-State: ALoCoQlFFhfqwpE+4ai/sjmu8iYB8NQxkUosmwy7T+C8X0D2LootLxQ2kVxw0nxTMJ48Db8jtS7S X-Received: by 10.180.88.131 with SMTP id bg3mr12777867wib.71.1428486264502; Wed, 08 Apr 2015 02:44:24 -0700 (PDT) Received: from [10.16.0.195] (6wind.net2.nerim.net. [213.41.180.237]) by mx.google.com with ESMTPSA id ad7sm14841778wid.21.2015.04.08.02.44.23 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 08 Apr 2015 02:44:23 -0700 (PDT) Message-ID: <5524F876.8090900@6wind.com> Date: Wed, 08 Apr 2015 11:44:22 +0200 From: Olivier MATZ <olivier.matz@6wind.com> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.3.0 MIME-Version: 1.0 To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, "dev@dpdk.org" <dev@dpdk.org> References: <1427385595-15011-1-git-send-email-olivier.matz@6wind.com> <1427829784-12323-1-git-send-email-zer0@droids-corp.org> <1427829784-12323-2-git-send-email-zer0@droids-corp.org> <2601191342CEEE43887BDE71AB97725821413A2D@irsmsx105.ger.corp.intel.com> <5522FF6B.1030503@6wind.com> <2601191342CEEE43887BDE71AB97725821414310@irsmsx105.ger.corp.intel.com> <5523FB9B.2060508@6wind.com> <2601191342CEEE43887BDE71AB9772582141451F@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB9772582141451F@irsmsx105.ger.corp.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3 1/5] mbuf: fix clone support when application uses private mbuf data X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> X-List-Received-Date: Wed, 08 Apr 2015 09:44:24 -0000 Hi Konstantin, On 04/07/2015 07:17 PM, Ananyev, Konstantin wrote: >> Just to be sure we're on the same line: >> >> - before the patch series >> >> - private area was working before that patch series if clones were not >> used. To use a private are, the user had to provide another >> function derived from pktmbuf_init() to change m->buf_addr and >> m->buf_len. >> - using both private area + clones was broken >> >> - after the patch series >> >> - private area is working with or without clone. But yo use it, >> the user still has to provide another function to change >> m->buf_addr, m->buf_len *and m->priv_size*. >> >> The series just fixes the fact that "clones + priv" was not working. >> It does not address the problem that providing a new pktmbuf_init() >> function is required to use privata area. To fix this, I think it >> could require a API evolution that should be part of another series. > > I don't think we need new pktmbuf_init(). > We just need to update it, so both pktmbuf_init() and detach() setup > buf_addr, buf_len (and priv_size) to exactly the same values. > If they don't do that, it means that you can't use attach/detach with > mempools created with pktmbuf_init() any more. > > BTW, another thing that I just realised: > examples/ipv4_multicast and examples/ip_fragmentation/ - > both create a pool of mbufs with elem_size < 2K and don't populate mempool's private area - > so mbp_priv->mbuf_data_room_size == 0, for them. > > So that code in detach(): > > + mbp_priv = rte_mempool_get_priv(mp); > + m->priv_size = mp->elt_size - RTE_PKTMBUF_HEADROOM - > + mbp_priv->mbuf_data_room_size - > + sizeof(struct rte_mbuf); > > > Would break both these samples. > I suppose we need to handle situation when mp->elt_size < RTE_PKTMBUF_HEADROOM + sizeof(struct rte_mbuf), > (and probably also when mbuf_data_room_size == 0) correctly. Indeed. I think a mbuf pool (even with buf_len == 0 like in ip_fragmentation example) should have a pool with a private area and should call rte_pktmbuf_pool_init() to populate it. So rte_pktmbuf_pool_init() has to be fixed first to use elt_size and support the buf_len < RTE_PKTMBUF_HEADROOM, then we can update frag/multicast examples. Unfortunately, we don't know the size of the mbuf private area in rte_pktmbuf_pool_init() if the opaque arg (data_room_size) is 0, which is the default. I think it should be replaced by a structure containing data_room_size and mbuf_priv_size, but it would break applications that are setting data_room_size. I don't see any good solution to do that while keeping a backward compatibility for rte_pktmbuf_pool_init(), but as the current API is not ideal, I think it's worth changing it and add something in the release note. We may also want to introduce a new helper as discussed previously: struct rte_mempool * rte_pktmbuf_pool_create(const char *name, unsigned n, unsigned elt_size, unsigned cache_size, size_t mbuf_priv_size, rte_mempool_obj_ctor_t *obj_init, void *obj_init_arg, int socket_id, unsigned flags) Any comment? > > Konstantin