From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f194.google.com (mail-wr0-f194.google.com [209.85.128.194]) by dpdk.org (Postfix) with ESMTP id 2E526CFFE for ; Mon, 16 Apr 2018 11:59:36 +0200 (CEST) Received: by mail-wr0-f194.google.com with SMTP id u46so24219438wrc.11 for ; Mon, 16 Apr 2018 02:59:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=TXknBfB/7EN3rExNMPmYAbNdQ5fYaAHq/l6mSRxF13k=; b=vABd1gkTi3C61miMHhKiXVtLccSf65YwSDcvV3qGvI1pAdsDArku2dJGphhmIfPbzL hurHC5t0HJ5JUQrKbO5wnEXbE5j9CTEQnno9mn6Pq7NQVeIhu259BQBgMFjneVEPCDZb FTKPVSVPnJm0navMBIPsBUKqiKlzPV+1J8vcjF1Oa/G0EJnNo63nydpq4zWime3Hq9WO 1tZG4YosYt7HMFXlwv4CpD0fWKN7vxlsSFurIlybu82ItsljykHIulYYAaOxKUC2EfHP ikeHiFkGe0Gm2GBeiqp69+sGrsyU5u399HW8DMAa9HfpMqrGOEwhCJ+HFZWxoMYJ4jHp 647w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=TXknBfB/7EN3rExNMPmYAbNdQ5fYaAHq/l6mSRxF13k=; b=mJ/ouvNQBq32qmxXjKzFLstaO5nm1LJVrNlJzR9T6A4cSbznMkicu8oUoqSwBYYHFg ISJsFLjuNeOh0LDy9wd/BEPdLR/E4cHxc2/Wbtx5R+P1sIJI+dMmQpQfEZHCYzEmNFHV bKoyoG6MSmuBZ2d8xVvXOTKIghynccQXuPLZfALDcUD6mlSiD6CZAudX5krWT7Iesfh/ 0fN3GZU7+CEN48f6asPmdXht9LIPXaF7aOMYx2MCmQSatERc2xitcRYJWtt4SZvA1tWu oEqAwSoz3wHEqE1I0yMHaQgSTsxgO14tMWjp4OhLPEQgV+ZVDBXVNWQ1DRltpSrPx+mt Xp1Q== X-Gm-Message-State: ALQs6tDmQtXECqm/EXOAme3UJqcWn5ycxbiE1P0w3n1jbS6KoUtfM8E8 Bp7YcaedwKMEUeoyjocDtzo1tQ== X-Google-Smtp-Source: AIpwx48rs2P+a/z/XxIxFHUssdoSv6+Dxqzm4kSDiXoFcJ71r8aSjzq9JLwTHZz0HEhwWmE/b8SblQ== X-Received: by 10.28.174.79 with SMTP id x76mr10582621wme.41.1523872775866; Mon, 16 Apr 2018 02:59:35 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id f2sm8074525wre.76.2018.04.16.02.59.35 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 16 Apr 2018 02:59:35 -0700 (PDT) Date: Mon, 16 Apr 2018 11:59:21 +0200 From: Adrien Mazarguil To: Shahaf Shuler Cc: Qi Zhang , "dev@dpdk.org" , "declan.doherty@intel.com" , "sugesh.chandran@intel.com" , "michael.j.glynn@intel.com" , "yu.y.liu@intel.com" , "konstantin.ananyev@intel.com" , "bruce.richardson@intel.com" , Thomas Monjalon Message-ID: <20180416095921.GX4957@6wind.com> References: <1522279780-34842-1-git-send-email-qi.z.zhang@intel.com> <20180416051639.188034-1-qi.z.zhang@intel.com> <20180416051639.188034-4-qi.z.zhang@intel.com> <20180416081226.GU4957@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [dpdk-dev] [PATCH v3 3/4] ethdev: add TTL change actions in flow API 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: Mon, 16 Apr 2018 09:59:36 -0000 On Mon, Apr 16, 2018 at 08:56:37AM +0000, Shahaf Shuler wrote: > Monday, April 16, 2018 11:12 AM, Adrien Mazarguil: > > Subject: Re: [dpdk-dev] [PATCH v3 3/4] ethdev: add TTL change actions in > > flow API > > > > Hi Shahaf, > > > > On Mon, Apr 16, 2018 at 05:48:19AM +0000, Shahaf Shuler wrote: > > > Hi Qi, > > > > > > Am wondering if we can make the below more generic and not tailored for > > specific use cases. > > > > Regarding this, please see my previous answer [1] where I asked Qi to make > > his changes more focused on the use case at hand when it became clear all > > this work was targeting OpenFlow. > > OK, > I missed that. Sorry for jumping in late. > > > > > The OF specification [2] defines the behavior associated with each action, for > > instance when a TTL is 0 or decrementing it would yield 0, the packet must be > > dropped. Translating this to a generic decrement action for any packet field is > > not so easy and not convenient. > > I am not sure I understand why. It is to set -1 in the TTL field of the generic action. > We can define the corner cases more carefully as part of the actions. For example - no wrap around. > I did not understood the drop if TTL is 0 is part of the action (it is not described the action description[1]). > Is this the case? I still need to comment the original patch :) Basically I would like to make all these actions point to the OpenFlow action documentation describing them with a disclaimer such as "These are OpenFlow actions, here's a summary of what they do, see linked OF documentation for details". > I think it is wrong approach to introduce a "combo" actions (both decrements and drops if value) in rte_flow. > I would model such operation by a set of (pseudo code) > 1. ACTION_FIELD_DEC_INC , ACTION_GO_TO_GROUP > 2. (in next group) matching on the TTL , ACTION_DROP If a device really implements something that does "check TTL on protocol $FOO, decrement it, re-check TTL, update checksum, drop packet if any of the previous steps failed", then by all means I think a dedicated action is justified. It's also easier to document as "does what OpenFlow specifies" and much more convenient to applications. Another set of actions can be added for devices (or PMDs) with partial support (e.g. to expose a "dumb" decrement capability as in the original series). The real question is are there devices that fully implement OF actions as described by the linked spec? Can any missing bits be handled by PMDs without a noticeable performance impact? > > Therefore my opinion is that if OF actions as defined by this specification are > > supported as hardware capabilities, it makes sense to define dedicated > > rte_flow actions for each of them (although "OF" should be part of their > > name for clarity). > > I still think we may need in the future to support copy/increment/decrement of fields not specifically related to OF. > It is better to have APIs which will not change or have double meaning. We could add them later on a needed basis. Correct me if I'm wrong, but right now OF is the only use case everyone has in mind. Application developers will always favor a set of explicit OF actions over more convoluted means of achieving the expected behavior. > [1] > +Action: ``IP_TTL_DEC`` > +^^^^^^^^^^^^^^^^^^^^^^ > + > +Decrement IPv4 TTL or IPv6 hop limit field and update the IP checksum, > +only applies to packets that contain specific MPLS headers. > + > +.. _table_rte_flow_action_ip_ttl_dec: > + > +.. table:: IP_TTL_DEC > > > > > > I'll comment the patch proper in a separate message. > > > > [1] > > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdpd > > k.org%2Fml%2Farchives%2Fdev%2F2018- > > April%2F096857.html&data=02%7C01%7Cshahafs%40mellanox.com%7C6d2b > > 747ae47841bc55e508d5a371d2f4%7Ca652971c7d2e4d9ba6a4d149256f461b%7 > > C0%7C0%7C636594631626247567&sdata=3oTbKT6QwS1WiAIrkF885dEU76ep4 > > xreuHoHiwDA2Ec%3D&reserved=0 > > [2] > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fw > > ww.opennetworking.org%2Fimages%2Fstories%2Fdownloads%2Fsdn- > > resources%2Fonf-specifications%2Fopenflow%2Fopenflow-spec- > > v1.3.0.pdf&data=02%7C01%7Cshahafs%40mellanox.com%7C6d2b747ae4784 > > 1bc55e508d5a371d2f4%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C1% > > 7C636594631626247567&sdata=e6uelVwIu1poE2uIvEJELuIzela8H%2B8HclQE5 > > EdKEaM%3D&reserved=0 > > > > -- > > Adrien Mazarguil > > 6WIND -- Adrien Mazarguil 6WIND