From patchwork Wed Dec 17 12:52:59 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Savolainen, Petri \(NSN - FI/Espoo\)" X-Patchwork-Id: 42376 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-la0-f69.google.com (mail-la0-f69.google.com [209.85.215.69]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id DD7BC26C76 for ; Wed, 17 Dec 2014 12:53:19 +0000 (UTC) Received: by mail-la0-f69.google.com with SMTP id gd6sf10035135lab.4 for ; Wed, 17 Dec 2014 04:53:18 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:from:to:thread-topic:thread-index :date:message-id:references:in-reply-to:accept-language :content-language:mime-version:cc:subject:precedence:list-id :list-unsubscribe:list-archive:list-post:list-help:list-subscribe :content-type:errors-to:sender:x-original-sender :x-original-authentication-results:mailing-list; bh=8Y8p/rNUVRuykdOySaBVnUSJlCmKqT7wDFV0xjkxdcI=; b=CZ+9qSi4Sl8o0QmgeQx69XdAoDcdsnqQxtcX5C55p28SSyXlybMJ2LP+/H8ELoYA4r GxRftanM+TM4h/hioYIYFXvHcaoWQm4g/ERYhzEYhh4z5Pb1ZzZaZF9ZX4+Bf66c1AoQ b9dB5gfQQOS1D8obEFeKW0ElHuXRGYO9bF7e6cPwg/rB/Fom7YBtBz0yfJSgFALTXRqU mjENNJEKjKYcn6jiP02oy7uT6md5MCJXFtoUirhs/RJAoINwPvFyU4D1JAQvk3Tarken X0nPo9RXq2bRsqYbNDulrm04Q+Dtjf7fJWZmo7uhgNUIjuwA2dEH1/EeutHyEYWQBAcx 3Ilg== X-Gm-Message-State: ALoCoQmX/KCNDJY8fCoQtQtrzPh8K5FE2X/VcW4pCVHGOcyAtJE3EVImwSptqEq1+x+2HCXBSB7X X-Received: by 10.180.105.97 with SMTP id gl1mr1313536wib.7.1418820798866; Wed, 17 Dec 2014 04:53:18 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.43.41 with SMTP id t9ls1051994lal.51.gmail; Wed, 17 Dec 2014 04:53:18 -0800 (PST) X-Received: by 10.112.168.164 with SMTP id zx4mr32791215lbb.28.1418820798483; Wed, 17 Dec 2014 04:53:18 -0800 (PST) Received: from mail-la0-f45.google.com (mail-la0-f45.google.com. [209.85.215.45]) by mx.google.com with ESMTPS id bb5si3817771lab.36.2014.12.17.04.53.18 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 17 Dec 2014 04:53:18 -0800 (PST) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.45 as permitted sender) client-ip=209.85.215.45; Received: by mail-la0-f45.google.com with SMTP id gq15so13218313lab.18 for ; Wed, 17 Dec 2014 04:53:18 -0800 (PST) X-Received: by 10.112.135.197 with SMTP id pu5mr40685983lbb.22.1418820798350; Wed, 17 Dec 2014 04:53:18 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.142.69 with SMTP id ru5csp1352671lbb; Wed, 17 Dec 2014 04:53:17 -0800 (PST) X-Received: by 10.224.122.15 with SMTP id j15mr13869623qar.68.1418820796616; Wed, 17 Dec 2014 04:53:16 -0800 (PST) Received: from ip-10-35-177-41.ec2.internal (lists.linaro.org. [54.225.227.206]) by mx.google.com with ESMTPS id e34si4401142qge.99.2014.12.17.04.53.15 (version=TLSv1 cipher=RC4-SHA bits=128/128); Wed, 17 Dec 2014 04:53:16 -0800 (PST) Received-SPF: none (google.com: lng-odp-bounces@lists.linaro.org does not designate permitted sender hosts) client-ip=54.225.227.206; Received: from localhost ([127.0.0.1] helo=ip-10-35-177-41.ec2.internal) by ip-10-35-177-41.ec2.internal with esmtp (Exim 4.76) (envelope-from ) id 1Y1E6L-0002kB-JP; Wed, 17 Dec 2014 12:53:13 +0000 Received: from demumfd002.nsn-inter.net ([93.183.12.31]) by ip-10-35-177-41.ec2.internal with esmtp (Exim 4.76) (envelope-from ) id 1Y1E6F-0002hA-5U for lng-odp@lists.linaro.org; Wed, 17 Dec 2014 12:53:07 +0000 Received: from demuprx017.emea.nsn-intra.net ([10.150.129.56]) by demumfd002.nsn-inter.net (8.14.3/8.14.3) with ESMTP id sBHCr0KB006777 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 17 Dec 2014 12:53:00 GMT Received: from DEMUHTC002.nsn-intra.net ([10.159.42.33]) by demuprx017.emea.nsn-intra.net (8.12.11.20060308/8.12.11) with ESMTP id sBHCqxij007419 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=FAIL); Wed, 17 Dec 2014 13:53:00 +0100 Received: from DEMUMBX012.nsn-intra.net ([169.254.12.71]) by DEMUHTC002.nsn-intra.net ([10.159.42.33]) with mapi id 14.03.0195.001; Wed, 17 Dec 2014 13:52:59 +0100 From: "Savolainen, Petri (NSN - FI/Espoo)" To: ext Bill Fischofer Thread-Topic: [lng-odp] [PATCH] Remove packet l2/l3/l4 offset adjustments Thread-Index: AQHQGe0kQk8qzeFbaUmN3aF6jXpi/JyTp3Aw///3e4CAABN3AP//9hKAgAARaQA= Date: Wed, 17 Dec 2014 12:52:59 +0000 Message-ID: References: <1418807557-17790-1-git-send-email-petri.savolainen@linaro.org> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.159.42.112] MIME-Version: 1.0 X-purgate-type: clean X-purgate-Ad: Categorized by eleven eXpurgate (R) http://www.eleven.de X-purgate: clean X-purgate: This mail is considered clean (visit http://www.eleven.de for further information) X-purgate-size: 38052 X-purgate-ID: 151667::1418820780-0000677A-1C5F2A3A/0/0 X-Topics: patch Cc: lng-odp-forward Subject: Re: [lng-odp] [PATCH] Remove packet l2/l3/l4 offset adjustments X-BeenThere: lng-odp@lists.linaro.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: , List-Help: , List-Subscribe: , Errors-To: lng-odp-bounces@lists.linaro.org Sender: lng-odp-bounces@lists.linaro.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: petri.savolainen@nsn.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.45 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 From: ext Bill Fischofer [mailto:bill.fischofer@linaro.org] Sent: Wednesday, December 17, 2014 2:40 PM To: Savolainen, Petri (NSN - FI/Espoo) Cc: Petri Savolainen; lng-odp-forward Subject: Re: [lng-odp] [PATCH] Remove packet l2/l3/l4 offset adjustments While ODP may not know why an application is choosing to make a specific push/pull request, the implications of any given call are precise. After pushing the head by X bytes, all of the other packet offset metadata are now in error until and unless they are adjusted. If it is part of the API definition that the implementation MUST NOT assist in these adjustments, then there are several implications: 1. Other ODP APIs that reference these now-erroneous metadata are invalidated and will silently return incorrect values. Does this help promote good programming? We have to specify (per ODP API) anyway which metadata fields are required to have valid values. If application creates a packet and passes it to e.g. a crypto IPSEC API, it has to first fill in the correct metadata (e.g. l3 offset). 2. The application takes responsibility for knowing which metadata items have been invalidated, even if additional metadata is added to ODP in subsequent releases. How does it know this? It’s then a new API version. An application is coded against an API version. 3. As has been pointed out, since the API does not provide relative offset adjustment APIs, for an application to adjust a given offset value correctly, it must read/modify/write each individually. Hardly efficient. Application likely read the offsets anyway before push/pull. Also that data is likely on same cache with all other packet metadata. There’s really no difference if application increments offsets by ‘len’ or if implementation does it. The real difference is that is an offset is written twice vs only once, OR once vs not at all. 4. Implementations that support push/pull operations with HW assistance may need to add special code to undo any automatic metadata adjustments to comply with the API. Is there any HW like this? Typically these are just fields in the packet descriptor (in the memory) and HW does not see it before application passes the packet to e.g. the output HW. -Petri So it's not at all clear how the application benefits from this stipulation or how it would be harmed if it was the implementations responsibility to maintain the system metadata in a consistent state. On Wed, Dec 17, 2014 at 6:24 AM, Savolainen, Petri (NSN - FI/Espoo) > wrote: Once again. There’s no point of implementation speculating what will happen to the packet after the pull/push operation. Which L2/L3/L4/… offsets are going to be changed and in which direction? Between which headers the application is going to add/remove layers? What header are moved? What happens next in the chain? What offsets really need an update? Application has all that knowledge and can do only those updates that are needed. We keep it simple – if application modifies the packet it is responsible to maintain metadata offsets accurate. -Petri From: ext Bill Fischofer [mailto:bill.fischofer@linaro.org] Sent: Wednesday, December 17, 2014 2:06 PM To: Savolainen, Petri (NSN - FI/Espoo) Cc: Petri Savolainen; lng-odp-forward Subject: Re: [lng-odp] [PATCH] Remove packet l2/l3/l4 offset adjustments You are correct that the API states this. What others are pointing out are the non-intutive implications of choosing to define the API this way. You've not provided a convincing rationale for making this choice. While you can assert your right not to do so, it would be helpful to others if you did. On Wed, Dec 17, 2014 at 5:51 AM, Savolainen, Petri (NSN - FI/Espoo) > wrote: The API says already: “Operation does not modify packet segmentation or move data. Handles and pointers remain valid. User is responsible to update packet meta-data offsets when needed.” If application modifies packet headers (adds/removes/changes them), it must update those offsets that it or some other block after it in the chain cares about (reloads from the packet). It may leave some offsets unchanged because it knows that those are not needed any more. Some more words can be added, if the current text is not clear enough. The current implementation does not implement the spec. This patch fixes that issue. -Petri From: ext Bill Fischofer [mailto:bill.fischofer@linaro.org] Sent: Wednesday, December 17, 2014 1:33 PM To: Petri Savolainen Cc: lng-odp-forward Subject: Re: [lng-odp] [PATCH] Remove packet l2/l3/l4 offset adjustments If we do this we really need a documentation note that says: Note: After doing a head push/pull, applications cannot use routines like odp_packet_l3_ptr(), etc., until they make appropriate calls to odp_packet_l3_offset_set(), etc., to reflect the previous operation. Otherwise they will receive pointers to incorrect areas of the packet. On Wed, Dec 17, 2014 at 3:12 AM, Petri Savolainen > wrote: Packet head push/pull calls do not automatically adjust metadata offsets, only data pointer and headroom. Signed-off-by: Petri Savolainen > --- platform/linux-generic/include/odp_packet_internal.h | 8 -------- 1 file changed, 8 deletions(-) -- 2.2.0 _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp diff --git a/platform/linux-generic/include/odp_packet_internal.h b/platform/linux-generic/include/odp_packet_internal.h index a0eff30..18e69b3 100644 --- a/platform/linux-generic/include/odp_packet_internal.h +++ b/platform/linux-generic/include/odp_packet_internal.h @@ -192,24 +192,16 @@ static inline void *packet_map(odp_packet_hdr_t *pkt_hdr, pkt_hdr->headroom + pkt_hdr->frame_len); } -#define pull_offset(x, len) (x = x < len ? 0 : x - len) - static inline void push_head(odp_packet_hdr_t *pkt_hdr, size_t len) { pkt_hdr->headroom -= len; pkt_hdr->frame_len += len; - pkt_hdr->l2_offset += len; - pkt_hdr->l3_offset += len; - pkt_hdr->l4_offset += len; } static inline void pull_head(odp_packet_hdr_t *pkt_hdr, size_t len) { pkt_hdr->headroom += len; pkt_hdr->frame_len -= len; - pull_offset(pkt_hdr->l2_offset, len); - pull_offset(pkt_hdr->l3_offset, len); - pull_offset(pkt_hdr->l4_offset, len); } static inline void push_tail(odp_packet_hdr_t *pkt_hdr, size_t len)