From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-we0-f170.google.com (mail-we0-f170.google.com [74.125.82.170]) by dpdk.org (Postfix) with ESMTP id C386DB3B7 for ; Tue, 12 Aug 2014 16:15:37 +0200 (CEST) Received: by mail-we0-f170.google.com with SMTP id w62so10083267wes.15 for ; Tue, 12 Aug 2014 07:18:33 -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 :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=UA7mbRLC292o9U1z/5JPRnWXtYlnPM4MEr8wj35RArI=; b=VdiwNcru4MazFyYv2PBYpHlKZLfshArAFuw9gYOr37VerSpflu80chg829Mo1NVNv0 MX/GJy0GQLVRxoS0vPyhsNaenBdwYLoxjM7SzMFMkyQy9puhO3mjEKrB4ZlQ1PYhuZP9 NfJlGwOWU+xcVAJmi06QDxeG94WzNzgBIUmwQqhUJXELIVdJJc62mkzHrW+pe//1XkXr he3LsIVswcptLQqUlfw3LdGpE6CSqziLNsv1TZ/+ZxwwWOqzwgev4ybq6zUrBwHgmVPN 4Zf6aCAWY+B7PWB1tr2zn5sEWFM8U+Y6Nem6f8fyPHBQPh0naYWiTu0VPP1gD+kSmMVZ 5pBQ== X-Gm-Message-State: ALoCoQkp1Hy4jPFnAuC1pQF8qVobEet70s31du+aoNZIP2UfDGvhc6qAJxuel1CMLDhIiLVyCF8i X-Received: by 10.194.119.193 with SMTP id kw1mr5577246wjb.82.1407853113442; Tue, 12 Aug 2014 07:18:33 -0700 (PDT) Received: from [10.16.0.195] (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by mx.google.com with ESMTPSA id ga2sm9535973wjb.44.2014.08.12.07.18.31 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 12 Aug 2014 07:18:32 -0700 (PDT) Message-ID: <53EA2237.50502@6wind.com> Date: Tue, 12 Aug 2014 16:18:31 +0200 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Icedove/24.5.0 MIME-Version: 1.0 To: Bruce Richardson , dev@dpdk.org References: <1407789890-17355-1-git-send-email-bruce.richardson@intel.com> <1407789890-17355-14-git-send-email-bruce.richardson@intel.com> In-Reply-To: <1407789890-17355-14-git-send-email-bruce.richardson@intel.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [RFC PATCH 13/14] mbuf: cleanup + added in additional mbuf fields. X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Aug 2014 14:15:37 -0000 Hi Bruce, On 08/11/2014 10:44 PM, Bruce Richardson wrote: > Cleanups: > * use typedefs for markers within mbuf struct > * split up vlan_macip field as the l2/l3 lengths are for TX so go on the > second cache line. > * created a tx_ol field in second cache line for data used for tx > offloads > * rename the hash field to the filter field as it contains more than > just a hash value. > > Added in the extra mbuf fields needed: > * fdir flex bytes for i40e driver, i.e. extra 32-bits for filters > * field to be used for a sequence number, extra 32-bit field > * field for a second vlan tag, extra 16-bits, using space freed by > moving out the l2 l3 lengths. > * userdata field for general application use. > * added inner_l3 and l4 length fields to allow tunneling. > > Signed-off-by: Bruce Richardson I think it would be clearer if splitted in several patches. First 2 patches for: - rename hash in filter - rename vlan_macip in rte_tx_offloads (and change to 64 bits ?) By the way, the modification of ol_flags in 64 bits probably requires more modifications in driver and testpmd. See me initial patch "mbuf: change ol_flags to 32 bits". Why not directly adding the MARKER typedef in previous patches? Also, I'm wondering if the markers really need to by typed. An empty struct would do the job, just requiring the users to cast it into the proper type. About sequence, double vlan id, userdata: they are not used now. I think we should add them one by one in separate patches, with code really that really requires the added field to be present. By the way, there is already a way to reserve a zone in mbuf for the application at mbuf pool creation. What is the purpose of the userdata field? About fdir_i40e field, I'm not sure that we should have driver-specific info in the generic mbuf structure. In addition, the application would need to know which hash type is present in the mbuf. Each time we add a new field in the mbuf, I think we should ask ourselves how the application will use it, especially if the application manages several kind of ports (virtual and physical for instance), which may not support all features. For instance, who will fill the "sequence" field ? Is it the driver? If it's application-specific Regards, Olivier