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 25F588E7C for ; Thu, 19 Apr 2018 16:49:10 +0200 (CEST) Received: by mail-wr0-f194.google.com with SMTP id f14-v6so14818929wre.4 for ; Thu, 19 Apr 2018 07:49:10 -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=7Bx5uMfKVWURGZaMx3ks5ZFdB6Fd7xcW9gxpMgIR84Y=; b=1lEBgF7jZCijvGQTTdN/PaEsicmHfR7A9ZiyWL1PBxhi6QejL4AchLB8CbzFgoIJlY 0Gy6PbSldwRsOb2S5eSxUicA1wZJapHkGAZA0oXz7xSziS/sa2oVavp7UTPAR5KoCaxV a1k9KYr89mqdm9MKoy1omwvZGFW2cKgRxb3/y7+qLS4JfMZ5yOCJ2Z4KJ5ZuFn2Hoa16 GauX2sF8oUC5Wd/ignpyObga3X/6m4/EIuzfLfwyhU8gVsFpTNkaQ6HrL2pyoapTAOeQ KUibPLO1QVMG0dVr6K3vnhTAvjpojPnxqFhEbbxHKC8Dp/HTfqnJVrMICBnbnWxa8oq+ oSLw== 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=7Bx5uMfKVWURGZaMx3ks5ZFdB6Fd7xcW9gxpMgIR84Y=; b=TVrUCbUXTKG3+tnTo5BXj23A7zRPxVZo6gAYuTvDkrr+A8S8KmxzCqLRMFROicTYmC AqHUlylD5+HznfXLqKKl0hL7QFqjztgjdAMY0E5iFsCaSqhwwT3ZPFJe42rQFb7+onLT 8pP0FH2c3flT8ose/rk3uiuM0Dglw7AzBZRwscGVogz69juc5e8xtcvY675hK8sEOBkJ uW1sG9ziakY1jp85TIHV8gqsNBCuTP40DUKx9Ta7axhCLbNuX93C8G7e5+3RWP4M8vVQ ulMk8GqiMM9xQdA12U8af/2KSfNNik6M1aQeNpAqqX32c1sXDTeSUGAvSA35xwZj+L/M P5Cw== X-Gm-Message-State: ALQs6tAH78Td0uW7LZH9E2UMSg4e+GBTeT9bziNU6Qw4jRdOTmQsFvkr Ozv0GPC1wfbXpYjNFB9GIncJ63BXt9w= X-Google-Smtp-Source: AIpwx4975EFjJYEREEIbsObFwgQvjEGvH0ZNNbfaVrU/1QAHNmZFYxNGGTV9WniCu3escZFpN8bYvA== X-Received: by 2002:adf:db90:: with SMTP id u16-v6mr5228143wri.90.1524149349787; Thu, 19 Apr 2018 07:49:09 -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 80sm5947546wmk.46.2018.04.19.07.49.09 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 19 Apr 2018 07:49:09 -0700 (PDT) Date: Thu, 19 Apr 2018 16:48:55 +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: <20180419144855.GG4957@6wind.com> References: <1522279780-34842-1-git-send-email-qi.z.zhang@intel.com> <20180416061042.785-1-qi.z.zhang@intel.com> <20180416061042.785-4-qi.z.zhang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180416061042.785-4-qi.z.zhang@intel.com> 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: Thu, 19 Apr 2018 14:49:10 -0000 On Mon, Apr 16, 2018 at 02:10:41PM +0800, Qi Zhang wrote: > Add couple OpenFlow frienldy TTL change actions. frienldy => friendly > > RTE_FLOW_ACTION_MLPS_TTL_DEC - decrement MPLS TTL. > RTE_FLOW_ACTION_IP_TTL_DEC - decrement IP TTL. > RTE_FLOW_ACTION_TTL_COPY_OUT - copy TTL outwards. > RTE_FLOW_ACTION_TTL_COPY_IN - copy TTL inwards. > > Signed-off-by: Qi Zhang These are OpenFlow actions whose behavior is defined by the OpenFlow specification. They must have "OF" part of their name to make this clear, as in "RTE_FLOW_ACTION_OF_MPLS_TTL_DEC". More below. > --- > app/test-pmd/cmdline_flow.c | 64 +++++++++++++++++++++++ > doc/guides/prog_guide/rte_flow.rst | 80 +++++++++++++++++++++++++++++ > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 12 +++++ > lib/librte_ether/rte_flow.h | 77 +++++++++++++++++++++++++++ > 4 files changed, 233 insertions(+) > > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c > index 7950b1b8d..417cbecc9 100644 > --- a/app/test-pmd/cmdline_flow.c > +++ b/app/test-pmd/cmdline_flow.c > @@ -195,6 +195,12 @@ enum index { > ACTION_VF_ID, > ACTION_METER, > ACTION_METER_ID, > + ACTION_MPLS_TTL_DEC, > + ACTION_MPLS_TTL_DEC_LVL, > + ACTION_IP_TTL_DEC, > + ACTION_IP_TTL_DEC_LVL, > + ACTION_TTL_COPY_OUT, > + ACTION_TTL_COPY_IN, > }; > > /** Size of pattern[] field in struct rte_flow_item_raw. */ > @@ -668,6 +674,10 @@ static const enum index next_action[] = { > ACTION_PF, > ACTION_VF, > ACTION_METER, > + ACTION_MPLS_TTL_DEC, > + ACTION_IP_TTL_DEC, > + ACTION_TTL_COPY_OUT, > + ACTION_TTL_COPY_IN, > ZERO, > }; > > @@ -708,6 +718,16 @@ static const enum index action_meter[] = { > ZERO, > }; > > +static const enum index action_mpls_ttl_dec[] = { > + ACTION_MPLS_TTL_DEC_LVL, > + ZERO, > +}; > + > +static const enum index action_ip_ttl_dec[] = { > + ACTION_IP_TTL_DEC_LVL, > + ZERO, > +}; > + The OpenFlow specification doesn't mention any parameter for these actions. Unless I'm mistaken, it doesn't have a concept of "level", please remove them. > static int parse_init(struct context *, const struct token *, > const char *, unsigned int, > void *, unsigned int); > @@ -1857,6 +1877,50 @@ static const struct token token_list[] = { > .args = ARGS(ARGS_ENTRY(struct rte_flow_action_meter, mtr_id)), > .call = parse_vc_conf, > }, > + [ACTION_MPLS_TTL_DEC] = { > + .name = "mpls_ttl_dec", mpls_ttl_dec => of_mpls_ttl_dec > + .help = "decrement MPLS TTL", > + .priv = PRIV_ACTION(MPLS_TTL_DEC, > + sizeof(struct rte_flow_action_mpls_ttl_dec)), > + .next = NEXT(action_mpls_ttl_dec), > + .call = parse_vc, > + }, > + [ACTION_MPLS_TTL_DEC_LVL] = { > + .name = "level", > + .help = "level of MPLS header", > + .next = NEXT(action_mpls_ttl_dec, NEXT_ENTRY(UNSIGNED)), > + .args = ARGS(ARGS_ENTRY(struct rte_flow_action_queue, index)), > + .call = parse_vc, > + }, > + [ACTION_IP_TTL_DEC] = { > + .name = "ip_ttl_dec", ip_ttl_dec => of_ip_ttl_dec > + .help = "decrement IPv4 TTL or IPv6 Hop Limit", > + .priv = PRIV_ACTION(MPLS_TTL_DEC, > + sizeof(struct rte_flow_action_ip_ttl_dec)), > + .next = NEXT(action_ip_ttl_dec), > + .call = parse_vc, > + }, > + [ACTION_IP_TTL_DEC_LVL] = { > + .name = "level", > + .help = "level of IPv4 or IPv6 header", > + .next = NEXT(action_ip_ttl_dec, NEXT_ENTRY(UNSIGNED)), > + .args = ARGS(ARGS_ENTRY(struct rte_flow_action_queue, index)), > + .call = parse_vc, > + }, > + [ACTION_TTL_COPY_OUT] = { > + .name = "ttl_copy_out", ttl_copy_out => of_ttl_copy_out > + .help = "copy TTL outwards", > + .priv = PRIV_ACTION(DROP, 0), Should be "PRIV_ACTION(ACTION_TTL_COPY_OUT, 0)". > + .next = NEXT(NEXT_ENTRY(ACTION_NEXT)), > + .call = parse_vc, > + }, > + [ACTION_TTL_COPY_IN] = { > + .name = "ttl_copy_in", ttl_copy_in => of_ttl_copy_in > + .help = "copy TTL inwards", > + .priv = PRIV_ACTION(DROP, 0), Should be "PRIV_ACTION(ACTION_TTL_COPY_IN, 0)". > + .next = NEXT(NEXT_ENTRY(ACTION_NEXT)), > + .call = parse_vc, > + }, > }; > > /** Remove and return last entry from argument stack. */ > diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst > index 68deb9812..65f2d4a16 100644 > --- a/doc/guides/prog_guide/rte_flow.rst > +++ b/doc/guides/prog_guide/rte_flow.rst > @@ -1604,6 +1604,86 @@ contain respective protocol layer. > | ``mask`` | Bit-mask applied to new_val | > +---------------+-------------------------------------------------------------------+ > > +Action: ``MPLS_TTL_DEC`` > +^^^^^^^^^^^^^^^^^^^^^^^^ > + > +Decrement MPLS TTL, only applies to packets that contain specific MPLS > +headers. > + > +.. _table_rte_flow_action_mpls_ttl_dec: > + > +.. table:: MPLS_TTL_DEC > + > + +---------------+---------------------------------------------+ > + | Field | Value | > + +===============+=============================================+ > + | ``dir_level`` | Specify the level of MPLS header. | > + | | direction (1b) | > + | | 0: match start from outermost. | > + | | 1: match start from innermost. | > + | | level: (31b) | > + | | 0: outermost or innermost MPLS header | > + | | 1: next to outmost or innermost MPLS header | > + | | 2: and so on ... | > + +---------------+---------------------------------------------+ No need to document dir and level for the above reasons. You must describe how it's an OpenFlow action and provide a link to the relevant OF specification. > + > +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 > + > + +---------------+----------------------------------------------------------+ > + | Field | Value | > + +===============+==========================================================+ > + | ``dir_level`` | Specify the level of IPv4 or IPv6 header. | > + | | direction (1b) | > + | | 0: match start from outermost. | > + | | 1: match start from innermost. | > + | | level: (31b) | > + | | 0: outermost or innermost Ipv4 or IPv6 header | > + | | 1: next to outmost or innermost MPLS IPv4 or IPv6 header | > + | | 2: and so on ... | > + +---------------+----------------------------------------------------------+ Ditto. > + > +Action: ``TTL_COPY_OUT`` > +^^^^^^^^^^^^^^^^^^^^^^^^ > + > +Copy the TTL from next-to-outermost to outermost header with TTL, copy can > +be IP to IP, MPLS to MPLS or IP to MPLS, only applies packets that contain > +required MPLS or IP headers. > + > +.. _table_rte_flow_action_ttl_copy_out: > + > +.. table:: TTL_COPY_OUT > + > + +---------------+ > + | Field | > + +===============+ > + | no properties | > + +---------------+ Same comment as above re linking to OpenFlow. > + > +Action: ``TTL_COPY_IN`` > +^^^^^^^^^^^^^^^^^^^^^^^ > + > +Copy the TTL from outermost to next-to-outermost header with TTL, copy can > +be IP to IP, MPLS to MPLS or MPLS to IP, only applies packets that contain > +required MPLS or IP headers. > + > +.. _table_rte_flow_action_ttl_copy_in: > + > +.. table:: TTL_COPY_IN > + > + +---------------+ > + | Field | > + +===============+ > + | no properties | > + +---------------+ Ditto. > + > Negative types > ~~~~~~~~~~~~~~ > > diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst > index 12d78f10b..d2fe6637b 100644 > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst > @@ -3449,6 +3449,18 @@ This section lists supported actions and their attributes, if any. > - ``original {boolean}``: use original VF ID if possible. > - ``id {unsigned}``: VF ID to redirect packets to. > > +- ``mpls_ttl_dec``: decrement MPLS TTL. > + > + - ``level``: level of MPLS header. > + > +- ``ip_ttl_dec``: decrement IPv4 TTL or IPv6 Hot Limit. > + > + - ``level``: level of IPv4 or IPv6 header. > + > +- ``ttl_copy_out``: copy TTL outwards. > + > +- ``ttl_copy_in``: copy TTL inwards. "level" must be removed from here also. > + > Destroying flow rules > ~~~~~~~~~~~~~~~~~~~~~ > > diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h > index 2dc95b6b8..ab181cd83 100644 > --- a/lib/librte_ether/rte_flow.h > +++ b/lib/librte_ether/rte_flow.h > @@ -1254,6 +1254,37 @@ enum rte_flow_action_type { > * See struct rte_flow_action_field_set. > */ > RTE_FLOW_ACTION_TYPE_FIELD_SET, > + > + /** > + * Decrement MPLS TTL, only applies to packets that contain specific > + * MPLS header. > + * > + * See struct rte_flow_action_mpls_ttl_dec. > + */ > + RTE_FLOW_ACTION_TYPE_MPLS_TTL_DEC, > + > + /** > + * Decrement IPv4 TTL or IPv6 Hop limit field and update the IP > + * checksum, only applies to packets that contain specific IPv4 > + * or IPv6 header. > + * > + * See struct rte_flow_action_ip_ttl_dec. > + */ > + RTE_FLOW_ACTION_TYPE_IP_TTL_DEC, > + > + /** > + * Copy the TTL from next-to-outermost to outermost header with TTL, > + * copy can be IP to IP, MPLS to MPLS or IP to MPLS, only applies > + * packets that contain required protocol headers. As for other actions, please add: No associated configuration structure. > + */ > + RTE_FLOW_ACTION_TYPE_TTL_COPY_OUT, > + > + /** > + * Copy the TTL from outermost to next-to-outermost header with TTL, > + * copy can be IP to IP, MPLS to MPLS or MPLS to IP, only applies > + * packets that contain required protocol headers. Ditto. > + */ > + RTE_FLOW_ACTION_TYPE_TTL_COPY_IN, > }; > > /** > @@ -1424,6 +1455,52 @@ struct rte_flow_action_field_set { > }; > > /** > + * RTE_FLOW_ACTION_TYPE_MPLS_TTL_DEC > + * > + * Decrement MPLS TTL, only applies to packets that contain specific > + * MPLS header. > + */ > +struct rte_flow_action_mpls_ttl_dec { > + /** > + * Specify the level of MPLS header. > + * > + * direction (1b) > + * 0: match start from outermost. > + * 1: match start from innermost. > + * > + * level (31b) > + * 0: outermost|innermost MPLS header. > + * 1: next to outermost|innermost MPLS header. > + * 2: and so on ... > + */ > + uint32_t dir_level; > + > +}; As described above, this structure must be removed. > + > +/** > + * RTE_FLOW_ACTION_TYPE_IP_TTL_DEC > + * > + * Decrement IPv4 TTL or IPv6 hop limit field and update the IP > + * checksum, only applies to packets that contain specific IPv4 > + * or IPv6 header. > + */ > +struct rte_flow_action_ip_ttl_dec { > + /** > + * Specify the level of IPv4 or IPv6 header. > + * > + * direction (1b) > + * 0: match start from outermost. > + * 1: match start from innermost. > + * > + * level (31b) > + * 0: outermost|innermost IPv4 or IPv6 header. > + * 1: next to outermost|innermost IPv4 or IPv6 header. > + * 2: and so on ... > + */ > + uint32_t dir_level; > +}; Ditto. > + > +/** > * Definition of a single action. > * > * A list of actions is terminated by a END action. > -- > 2.13.6 > -- Adrien Mazarguil 6WIND