DPDK patches and discussions
 help / color / mirror / Atom feed
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Beilei Xing <beilei.xing@intel.com>
Cc: jingjing.wu@intel.com, dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 2/7] ethdev: add GTP item
Date: Wed, 6 Sep 2017 18:02:55 +0200	[thread overview]
Message-ID: <20170906160255.GB4301@6wind.com> (raw)
In-Reply-To: <1503647430-93905-3-git-send-email-beilei.xing@intel.com>

Hi Beilei,

On Fri, Aug 25, 2017 at 03:50:25PM +0800, Beilei Xing wrote:
> This patch adds GTP items to generic rte flow.
> 
> Signed-off-by: Beilei Xing <beilei.xing@intel.com>

Thanks, several comments below.

> ---
>  doc/guides/prog_guide/rte_flow.rst | 12 ++++++++++++
>  lib/librte_ether/rte_flow.h        | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 662a912..2843b71 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -955,6 +955,18 @@ Usage example, fuzzy match a TCPv4 packets:
>     | 4     | END      |
>     +-------+----------+
>  
> +Item: ``GTP``
> +^^^^^^^^^^^^^^
> +
> +Matches a GTP header.
> +
> +- ``v_pt_rsv_flags``: version (3b), protocol type (1b), reserved (1b),
> +  extension header flag (1b), sequence number flag (1b), N-PDU number flag(1b).

Missing space after "flag", this line is also too long, you should add a
line break after the last comma.

> +- ``msg_type``: message type.
> +- ``msg_len``: message length.
> +- ``teid``: TEID.
> +- Default ``mask`` matches teid only.
> +
>  Actions
>  ~~~~~~~
>  
> diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
> index bba6169..73305aa 100644
> --- a/lib/librte_ether/rte_flow.h
> +++ b/lib/librte_ether/rte_flow.h
> @@ -309,6 +309,13 @@ enum rte_flow_item_type {
>  	 * See struct rte_flow_item_fuzzy.
>  	 */
>  	RTE_FLOW_ITEM_TYPE_FUZZY,
> +
> +	/**
> +	 * Matches a GTP header

Missing "." (pedantic mode enabled).

> +	 *
> +	 * See struct rte_flow_item.

Wrong structure, should be rte_flow_item_gtp.

> +	 */
> +	RTE_FLOW_ITEM_TYPE_GTP,
>  };
>  
>  /**
> @@ -735,6 +742,30 @@ static const struct rte_flow_item_fuzzy rte_flow_item_fuzzy_mask = {
>  #endif
>  
>  /**
> + * RTE_FLOW_ITEM_TYPE_GTP.
> + *
> + * Matches a GTP header.
> + */
> +struct rte_flow_item_gtp {
> +	/**
> +	 * Version(2b), Protocol type(1b), Reserved(1b),
> +	 * Extension header flag(1b),
> +	 * Sequqnce number flag(1b),
> +	 * N-PDU number flag(1b).
> +	 */

No need to capitalize everything, only the first one. Several missing spaces
betwen name and bit widths ("foo(42b)" => "foo (42b)").

There's also a typo on "Sequqnce".

> +	uint8_t v_pt_rsv_flags;
> +	uint8_t msg_type; /**< Message type */
> +	rte_be16_t msg_len; /**< Message length */

These comments lack ending ".".

> +	rte_be32_t teid;

This field lack a comment, even if obvious, please add it for
consistency. You can use this opportunity to expand the acronym as well.

> +};
> +
> +/** Default mask for RTE_FLOW_ITEM_TYPE_GTP. */
> +#ifndef __cplusplus
> +	static const struct rte_flow_item_gtp rte_flow_item_gtp_mask = {
> +		.teid = RTE_BE32(0xffffffff),
> +	};
> +#endif

Indentation is wrong.

> +/**
>   * Matching pattern item definition.
>   *
>   * A pattern is formed by stacking items starting from the lowest protocol
> -- 
> 2.5.5
> 

It's not a problem if you want to keep them separate, however note that you
could make the testpmd update part of this commit. The API change can be
considered useless without an implementation counterpart.

-- 
Adrien Mazarguil
6WIND

  reply	other threads:[~2017-09-06 16:03 UTC|newest]

Thread overview: 116+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-25  7:50 [dpdk-dev] [PATCH 0/7] GTP enabling Beilei Xing
2017-08-25  7:50 ` [dpdk-dev] [PATCH 1/7] net/i40e: support RSS for GTP-C and GTP-U Beilei Xing
2017-09-07 11:20   ` [dpdk-dev] [PATCH v2 0/6] GPT-C and GTP-U enabling Beilei Xing
2017-09-07 11:20     ` [dpdk-dev] [PATCH v2 1/6] net/i40e: support RSS for GTP-C and GTP-U Beilei Xing
2017-09-18 14:17       ` Bruce Richardson
2017-09-18 14:21         ` Bruce Richardson
2017-09-07 11:20     ` [dpdk-dev] [PATCH v2 2/6] ethdev: add GTPC and GTPU items Beilei Xing
2017-09-07 12:19       ` Adrien Mazarguil
2017-09-12  6:40         ` Xing, Beilei
2017-09-12 10:46           ` Adrien Mazarguil
2017-09-13  3:09             ` Xing, Beilei
2017-09-07 11:21     ` [dpdk-dev] [PATCH v2 3/6] net/i40e: finish integration FDIR with generic flow API Beilei Xing
2017-09-07 11:21     ` [dpdk-dev] [PATCH v2 4/6] net/i40e: add FDIR support for GTP-C and GTP-U Beilei Xing
2017-09-07 11:21     ` [dpdk-dev] [PATCH v2 5/6] net/i40e: add cloud filter parsing function for GTP Beilei Xing
2017-09-07 11:21     ` [dpdk-dev] [PATCH v2 6/6] net/i40e: enable cloud filter for GTP-C and GTP-U Beilei Xing
2017-09-22 22:35     ` [dpdk-dev] [PATCH v3 0/8] GPT-C and GTP-U enabling Beilei Xing
2017-09-22 22:35       ` [dpdk-dev] [PATCH v3 1/8] mbuf: support GTP in software packet type parser Beilei Xing
2017-09-25  9:21         ` Olivier MATZ
2017-09-28  2:17         ` [dpdk-dev] [PATCH v4 0/8] GPT-C and GTP-U enabling Beilei Xing
2017-09-28  2:17           ` [dpdk-dev] [PATCH v4 1/8] mbuf: support GTP in software packet type parser Beilei Xing
2017-09-28  2:17           ` [dpdk-dev] [PATCH v4 2/8] net/i40e: update ptype and pctype info Beilei Xing
2017-09-28  2:17           ` [dpdk-dev] [PATCH v4 3/8] net/i40e: support RSS for new pctype Beilei Xing
2017-09-28  2:17           ` [dpdk-dev] [PATCH v4 4/8] ethdev: add GTP items to support flow API Beilei Xing
2017-09-28  2:17           ` [dpdk-dev] [PATCH v4 5/8] net/i40e: finish integration FDIR with generic " Beilei Xing
2017-09-28  2:17           ` [dpdk-dev] [PATCH v4 6/8] net/i40e: add FDIR support for GTP-C and GTP-U Beilei Xing
2017-09-28  2:17           ` [dpdk-dev] [PATCH v4 7/8] net/i40e: add cloud filter parsing function for GTP Beilei Xing
2017-09-28  2:17           ` [dpdk-dev] [PATCH v4 8/8] net/i40e: enable cloud filter for GTP-C and GTP-U Beilei Xing
2017-09-28  8:13           ` [dpdk-dev] [PATCH v5 0/8] GPT-C and GTP-U enabling Beilei Xing
2017-09-28  8:13             ` [dpdk-dev] [PATCH v5 1/8] mbuf: support GTP in software packet type parser Beilei Xing
2017-09-28  8:13             ` [dpdk-dev] [PATCH v5 2/8] net/i40e: update ptype and pctype info Beilei Xing
2017-09-28  8:13             ` [dpdk-dev] [PATCH v5 3/8] net/i40e: support RSS for new pctype Beilei Xing
2017-09-28  8:13             ` [dpdk-dev] [PATCH v5 4/8] ethdev: add GTP items to support flow API Beilei Xing
2017-09-28 13:43               ` Sean Harte
2017-09-29  2:12                 ` Xing, Beilei
2017-09-28  8:13             ` [dpdk-dev] [PATCH v5 5/8] net/i40e: finish integration FDIR with generic " Beilei Xing
2017-09-28  8:13             ` [dpdk-dev] [PATCH v5 6/8] net/i40e: add FDIR support for GTP-C and GTP-U Beilei Xing
2017-09-28  8:13             ` [dpdk-dev] [PATCH v5 7/8] net/i40e: add cloud filter parsing function for GTP Beilei Xing
2017-09-28  8:13             ` [dpdk-dev] [PATCH v5 8/8] net/i40e: enable cloud filter for GTP-C and GTP-U Beilei Xing
2017-09-29  5:18           ` [dpdk-dev] [PATCH v6 0/8] GPT-C and GTP-U enabling Beilei Xing
2017-09-29  5:18             ` [dpdk-dev] [PATCH v6 1/8] mbuf: support GTP in software packet type parser Beilei Xing
2017-09-29  8:15               ` Sean Harte
2017-09-29  8:41                 ` Xing, Beilei
2017-09-29  5:18             ` [dpdk-dev] [PATCH v6 2/8] net/i40e: update ptype and pctype info Beilei Xing
2017-09-29 13:22               ` Wu, Jingjing
2017-09-29 13:24                 ` Xing, Beilei
2017-09-29  5:18             ` [dpdk-dev] [PATCH v6 3/8] net/i40e: support RSS for new pctype Beilei Xing
2017-09-29 13:24               ` Wu, Jingjing
2017-09-29  5:18             ` [dpdk-dev] [PATCH v6 4/8] ethdev: add GTP items to support flow API Beilei Xing
2017-09-29  8:15               ` Sean Harte
2017-09-29  8:54                 ` Xing, Beilei
2017-09-29  9:29                   ` Sean Harte
2017-09-29  9:37                     ` Xing, Beilei
2017-10-02 12:27                     ` Adrien Mazarguil
2017-10-03  8:56                       ` Sean Harte
2017-10-05  8:06                         ` Wu, Jingjing
2017-10-05  8:30                           ` Adrien Mazarguil
2017-10-05  8:39                             ` Wu, Jingjing
2017-09-29  5:18             ` [dpdk-dev] [PATCH v6 5/8] net/i40e: finish integration FDIR with generic " Beilei Xing
2017-09-29 13:28               ` Wu, Jingjing
2017-09-29  5:19             ` [dpdk-dev] [PATCH v6 6/8] net/i40e: add FDIR support for GTP-C and GTP-U Beilei Xing
2017-09-29  8:15               ` Sean Harte
2017-09-29  9:33                 ` Xing, Beilei
2017-09-29 10:09                   ` Sean Harte
2017-09-29 11:30                     ` Xing, Beilei
2017-09-29 11:39                       ` Xing, Beilei
2017-09-29 13:14                     ` Xing, Beilei
2017-09-29 15:15                     ` Xing, Beilei
2017-09-29  5:19             ` [dpdk-dev] [PATCH v6 7/8] net/i40e: add cloud filter parsing function for GTP Beilei Xing
2017-09-29  5:19             ` [dpdk-dev] [PATCH v6 8/8] net/i40e: enable cloud filter for GTP-C and GTP-U Beilei Xing
2017-09-29 15:50             ` [dpdk-dev] [PATCH v7 0/8] net/i40e: GPT-C and GTP-U enabling Beilei Xing
2017-09-29 15:50               ` [dpdk-dev] [PATCH v7 1/8] mbuf: support GTP in software packet type parser Beilei Xing
2017-09-29 15:50               ` [dpdk-dev] [PATCH v7 2/8] net/i40e: update ptype and pctype info Beilei Xing
2017-10-05  2:51                 ` Wu, Jingjing
2017-09-29 15:50               ` [dpdk-dev] [PATCH v7 3/8] net/i40e: support RSS for new pctype Beilei Xing
2017-09-29 15:50               ` [dpdk-dev] [PATCH v7 4/8] ethdev: add GTP items to support flow API Beilei Xing
2017-10-05  8:01                 ` Wu, Jingjing
2017-09-29 15:50               ` [dpdk-dev] [PATCH v7 5/8] net/i40e: finish integration FDIR with generic " Beilei Xing
2017-10-05  2:52                 ` Wu, Jingjing
2017-09-29 15:50               ` [dpdk-dev] [PATCH v7 6/8] net/i40e: add FDIR support for GTP-C and GTP-U Beilei Xing
2017-10-05  3:09                 ` Wu, Jingjing
2017-09-29 15:50               ` [dpdk-dev] [PATCH v7 7/8] net/i40e: add cloud filter parsing function for GTP Beilei Xing
2017-10-05  3:13                 ` Wu, Jingjing
2017-09-29 15:50               ` [dpdk-dev] [PATCH v7 8/8] net/i40e: enable cloud filter for GTP-C and GTP-U Beilei Xing
2017-10-05  8:03                 ` Wu, Jingjing
2017-10-04 22:43               ` [dpdk-dev] [PATCH v7 0/8] net/i40e: GPT-C and GTP-U enabling Ferruh Yigit
2017-10-05  8:14               ` [dpdk-dev] [PATCH v8 0/7] " Beilei Xing
2017-10-05  8:14                 ` [dpdk-dev] [PATCH v8 1/7] mbuf: support GTP in software packet type parser Beilei Xing
2017-10-05 11:50                   ` Sean Harte
2017-10-05  8:14                 ` [dpdk-dev] [PATCH v8 2/7] net/i40e: update ptype and pctype info Beilei Xing
2017-10-05  8:14                 ` [dpdk-dev] [PATCH v8 3/7] ethdev: add GTP items to support flow API Beilei Xing
2017-10-05 11:50                   ` Sean Harte
2017-10-05  8:14                 ` [dpdk-dev] [PATCH v8 4/7] net/i40e: finish integration FDIR with generic " Beilei Xing
2017-10-05  8:14                 ` [dpdk-dev] [PATCH v8 5/7] net/i40e: add FDIR support for GTP-C and GTP-U Beilei Xing
2017-10-05 11:50                   ` Sean Harte
2017-10-05  8:14                 ` [dpdk-dev] [PATCH v8 6/7] net/i40e: add cloud filter parsing function for GTP Beilei Xing
2017-10-05  8:14                 ` [dpdk-dev] [PATCH v8 7/7] net/i40e: enable cloud filter for GTP-C and GTP-U Beilei Xing
2017-10-05  8:23                 ` [dpdk-dev] [PATCH v8 0/7] net/i40e: GPT-C and GTP-U enabling Wu, Jingjing
2017-10-05 21:13                 ` Ferruh Yigit
2017-09-22 22:35       ` [dpdk-dev] [PATCH v3 2/8] net/i40e: update ptype and pctype info Beilei Xing
2017-09-23  2:58         ` Wu, Jingjing
2017-09-22 22:35       ` [dpdk-dev] [PATCH v3 3/8] net/i40e: support RSS for new pctype Beilei Xing
2017-09-22 22:35       ` [dpdk-dev] [PATCH v3 4/8] ethdev: add GTP items to support flow API Beilei Xing
2017-09-22 13:39         ` Adrien Mazarguil
2017-09-22 22:35       ` [dpdk-dev] [PATCH v3 5/8] net/i40e: finish integration FDIR with generic " Beilei Xing
2017-09-22 22:35       ` [dpdk-dev] [PATCH v3 6/8] net/i40e: add FDIR support for GTP-C and GTP-U Beilei Xing
2017-09-22 22:35       ` [dpdk-dev] [PATCH v3 7/8] net/i40e: add cloud filter parsing function for GTP Beilei Xing
2017-09-22 22:35       ` [dpdk-dev] [PATCH v3 8/8] net/i40e: enable cloud filter for GTP-C and GTP-U Beilei Xing
2017-08-25  7:50 ` [dpdk-dev] [PATCH 2/7] ethdev: add GTP item Beilei Xing
2017-09-06 16:02   ` Adrien Mazarguil [this message]
2017-09-07  6:31     ` Xing, Beilei
2017-08-25  7:50 ` [dpdk-dev] [PATCH 3/7] app/testpmd: add GTP fields to flow command Beilei Xing
2017-09-06 16:03   ` Adrien Mazarguil
2017-08-25  7:50 ` [dpdk-dev] [PATCH 4/7] net/i40e: finish integration FDIR with generic flow API Beilei Xing
2017-08-25  7:50 ` [dpdk-dev] [PATCH 5/7] net/i40e: add FDIR support for GTP-C and GTP-U Beilei Xing
2017-08-25  7:50 ` [dpdk-dev] [PATCH 6/7] net/i40e: add cloud filter parsing function for GTP Beilei Xing
2017-08-25  7:50 ` [dpdk-dev] [PATCH 7/7] net/i40e: enable cloud filter for GTP-C and GTP-U Beilei Xing

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170906160255.GB4301@6wind.com \
    --to=adrien.mazarguil@6wind.com \
    --cc=beilei.xing@intel.com \
    --cc=dev@dpdk.org \
    --cc=jingjing.wu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).