From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f196.google.com (mail-wr0-f196.google.com [209.85.128.196]) by dpdk.org (Postfix) with ESMTP id 510E38D97 for ; Thu, 19 Apr 2018 16:48:58 +0200 (CEST) Received: by mail-wr0-f196.google.com with SMTP id u11-v6so14786512wri.12 for ; Thu, 19 Apr 2018 07:48:58 -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=PuKh98Y7lZlhYWoztugDFxzj8Uu6DbISYOkPyk9HYyE=; b=z91WAmZM9aYynRsdFtRvkX5gvkM39h6v7P7cfmppDaV3HrU8lwEMof0RFa54P7SDQQ NaodadMifl1+PRbvw02lwtBGJy39KvOatu4Mb/e9w9chAe8y020dRhF1qt3KGccdlvX5 imW195bqFHKLb7baTrQ4LLjb8OI2zXKpzSCpOXZ2VRv3vfdyNkXic7uPfiOqbxbpJHTY Igp8/zTIQBG1zlBRXL+wneSXMYZd/+iHHBgj4CcpfxRPvTZdbSBOES5HeQHvPqopzrXS k4c0wFRvj9aKsWTFChDLyBz0YYWVWBJ0vda8c/3Me32GGJVtx4ZnA+zsujmPpDlsRYdJ d3bw== 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=PuKh98Y7lZlhYWoztugDFxzj8Uu6DbISYOkPyk9HYyE=; b=ZqI1bF5dUmo5+SPObHIkpK0QaJhfuemIKDu+BHGnuN2Nxkik2+SWvLOPOpqWv4XlbF wFI3R8ScE+Lt3d6ALh7zLI258bjMVRzTUTWmqhybueVcFYISQiH97XXwv4R01hHVXZIu BpH5OrFoLuvcb/ieu7BZ9FkO1/rinBxOS8atuI3TQ098HEvYepq0HTYY7hsHcU/+qacA lCAEcNJ5wy3fHZ+MX0fOTByrF5OC0mT/pCYEbuenRjrgWJLkU0BB/im2m50+scqeF1Ef rR81GeqIuUmSrrAmLxc7Mg1XIn+GEXKpy7F8BAwDkiRfiL+sUN2yDlX8pGUpzxZmi2jE Sfkw== X-Gm-Message-State: ALQs6tDJFNzZWG62ASHCwtkdonknswvqSFbI666IE8fCXTlMjitdSARr D5ggVfrYHdU5/62TnHf4A/Ypzw== X-Google-Smtp-Source: AIpwx4/00CDsxWsdYiCUcmEotfgyW/ufRxlrzIiDJgAT2X7iNJEfRPmrzmbMEofuTvho6xW+fDKFuw== X-Received: by 2002:adf:b595:: with SMTP id c21-v6mr5216646wre.233.1524149337955; Thu, 19 Apr 2018 07:48:57 -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 63-v6sm4570153wrh.70.2018.04.19.07.48.57 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 19 Apr 2018 07:48:57 -0700 (PDT) Date: Thu, 19 Apr 2018 16:48:43 +0200 From: Adrien Mazarguil To: Qi Zhang Cc: 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 Message-ID: <20180419144843.GF4957@6wind.com> References: <1522279780-34842-1-git-send-email-qi.z.zhang@intel.com> <20180416061042.785-1-qi.z.zhang@intel.com> <20180416061042.785-3-qi.z.zhang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180416061042.785-3-qi.z.zhang@intel.com> Subject: Re: [dpdk-dev] [PATCH v3 2/4] ethdev: add packet field set aciton 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: Thu, 19 Apr 2018 14:48:58 -0000 Typo in commit title: aciton => action On Mon, Apr 16, 2018 at 02:10:40PM +0800, Qi Zhang wrote: > Add new action RTE_FLOW_ACTION_TYPE_FIELD_SET, it is used to > modify fields of specific protocol layer of the packet, the > action only apply on packets that contain the requireds protocol > layer. requireds => required > > Signed-off-by: Qi Zhang (more below) > --- > doc/guides/prog_guide/rte_flow.rst | 30 +++++++++++++++++++++++++++ > lib/librte_ether/rte_flow.h | 42 +++++++++++++++++++++++++++++++++++++- > 2 files changed, 71 insertions(+), 1 deletion(-) > > diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst > index 99468bf60..68deb9812 100644 > --- a/doc/guides/prog_guide/rte_flow.rst > +++ b/doc/guides/prog_guide/rte_flow.rst > @@ -1574,6 +1574,36 @@ fields in the pattern items. > | 1 | END | > +-------+----------+ > > +Action: ``FILED_SET`` > +^^^^^^^^^^^^^^^^^^^^^ FILED_SET => FIELD_SET > + > +Modify the value of fields in a protocol layer, only applies to packets that > +contain respective protocol layer. > + > +.. _table_rte_flow_action_field_set: > + > +.. table:: FIELD_SET > + > + +---------------+-------------------------------------------------------------------+ > + | Field | Value | > + +===============+===================================================================+ > + | ``type`` | Specify the type of a protocol layer. (see RTE_FLOW_ITEM_TYPE_*) | > + +---------------+-------------------------------------------------------------------+ > + | ``dir_level`` | Specify the level of matched protocol layer. | > + | | direction (1b) | > + | | 0: match start from outermost. | > + | | 1: match start from innermost. | Please remove the direction part. What devices can match is always outermost up to the point where they can't recognize an inner header. "innermost" is almost guaranteed to never have the desired effect. > + | | level: (31b) | > + | | 0: outermost or innermost protocol layer that matched @type | > + | | 1: next to outmost or innermost protocol layer that matched @type | > + | | 2: and so on ... | Then you can remove any reference to dir_level from here. > + +---------------+-------------------------------------------------------------------+ > + | ``new_val`` | Pointer to specific data structure according to protocol type, | > + | | the content is the new value to updtae. | updtae => update > + +---------------+-------------------------------------------------------------------+ > + | ``mask`` | Bit-mask applied to new_val | > + +---------------+-------------------------------------------------------------------+ > + > Negative types > ~~~~~~~~~~~~~~ > > diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h > index f84bbfda5..2dc95b6b8 100644 > --- a/lib/librte_ether/rte_flow.h > +++ b/lib/librte_ether/rte_flow.h > @@ -1245,7 +1245,15 @@ enum rte_flow_action_type { > * > * See struct rte_flow_action_security. > */ > - RTE_FLOW_ACTION_TYPE_SECURITY > + RTE_FLOW_ACTION_TYPE_SECURITY, > + > + /** > + * Modify the value of fields in a protocol layer, only applies to > + * packets that contain respective protocol layer. > + * > + * See struct rte_flow_action_field_set. > + */ > + RTE_FLOW_ACTION_TYPE_FIELD_SET, > }; > > /** > @@ -1384,6 +1392,38 @@ struct rte_flow_action_security { > }; > > /** > + * RTE_FLOW_ACTION_TYPE_FIELD_SET > + * > + * Modify the value of fields in a protocol layer, only applies to > + * packets that contain respective protocol layer. > + */ > +struct rte_flow_action_field_set { > + /** > + * Specify the type of a protocol layer. > + */ > + enum rte_flow_item_type type; > + /** > + * Specify the level of matched protocol layer. > + * > + * direction (1b) > + * 0: match start from outermost. > + * 1: match start from innermost. > + * > + * level (31b) > + * 0: outermost|innermost protocol layer that matched @type. > + * 1: next to outermost|innermost protocol layer that matched @type. > + * 2: and so on ... > + */ > + uint32_t dir_level; See above regarding this field. > + /** > + * Pointer to specific data structure according to protocol type, > + * the content is the new value to update. > + */ > + const void *new_val; > + const void *mask; /**< Bit-mask applied to new_val. */ > +}; > + > +/** > * Definition of a single action. > * > * A list of actions is terminated by a END action. > -- > 2.13.6 > Testpmd implementation and documentation update are also missing, however I'm still not convinced by the definition of this new action, it seems too generic to be useful (e.g. compare this with a dedicated "update destination IPv4 address" action for instance). What existing HW capabilities do you intend to expose through this, what kind of fields can be updated at this point? If it's still unclear, I suggest to remove this patch from the series or at the very least mark it as experimental. You can even provide a forward declaration without the contents of struct rte_flow_action_field_set to prevent applications from using it before it's finalized. -- Adrien Mazarguil 6WIND