From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by inbox.dpdk.org (Postfix) with ESMTP id 3BE98A0487
for <public@inbox.dpdk.org>; Wed, 3 Jul 2019 17:26:13 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id 750581B970;
Wed, 3 Jul 2019 17:25:18 +0200 (CEST)
Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com
[209.85.128.66]) by dpdk.org (Postfix) with ESMTP id D820E7CBC
for <dev@dpdk.org>; Wed, 3 Jul 2019 17:25:10 +0200 (CEST)
Received: by mail-wm1-f66.google.com with SMTP id x15so2648306wmj.3
for <dev@dpdk.org>; Wed, 03 Jul 2019 08:25:10 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google;
h=date:from:to:cc:subject:message-id:references:mime-version
:content-disposition:in-reply-to;
bh=TXn9vORKnHryYC02aF/2pSe2QAyMA1fORskRVjt5jSw=;
b=ZR7q6VHHM/ta47LN6puMKOXRiQjfohhCuMEGeYeUmQ7yszXeh/LZIppcIofrQ9+TnU
qhhrXRvns/usx+dQ0iEAIfqr57o1xk+EIE6UCEyfLqWWlNQ+KPVJYUV+DNDKIB4+mfVr
yedH0OI1fnCSjm4KpWibRz71+gJNxKNqKrvTGWaIoBR5nsQpxWGWe/RdxDkiOdxAX3R8
roMRtpoEWe3C14eTspmtShQOFkTjiLPHq3j7k6xX6inE3w7eFhjo2ZltlVbrDa86KMqk
y5FZ8xNxjFyaPjzGx2ya85oGNRrFhY9Wsw944dVTdUdAfQlV50Mg1f7Qr3QEXWvXEwE2
Kevg==
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=TXn9vORKnHryYC02aF/2pSe2QAyMA1fORskRVjt5jSw=;
b=X+n998sw/SbofErnfW3CFCqeqVqacBvnDb2k/neNmvLcm7T1F9jeEVAWFS2/woarSj
IiJ3aNf3x1npYSZUwQzNNIwixsmxEPEWNgL1/zWmXeWB8h8sVFsAa4DHo4jXWOVGlkeS
PY3pL8mwNRkgkrdJeh88SftaieFe1zyGOYXvQyFP5OAVlAh7dX5gyqwXB0w/QESCqk8J
fFZhnohnqk3MXGiI+7hn9ZMFoMfnT3UMK0Kk4wpva2+Qu9gd++sTX3iFOR+wqYRADOvE
Uu/DXFmUYUugjEplpsktzbBWnFaMCinWGVdUfiH9vuFZFOb61v8HM/oxdIap9mL6abfJ
jQfw==
X-Gm-Message-State: APjAAAWSwyDNg3zO4ULURVKevW0zwMhaU5CzP+fxi45J6qiJAcXiZ74q
5cMZOjoNqioNNrTZOzw4Zy02uNB3e6DFqA==
X-Google-Smtp-Source: APXvYqws6ftIGHIuRCskTWZGfsT8vlHAdSR5XLYghimoEk6pXjGUY+gIeRPA008uIi2URvUyTyJxaA==
X-Received: by 2002:a1c:e108:: with SMTP id y8mr8315285wmg.65.1562167510029;
Wed, 03 Jul 2019 08:25:10 -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 s11sm304126wrr.59.2019.07.03.08.25.08
(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
Wed, 03 Jul 2019 08:25:09 -0700 (PDT)
Date: Wed, 3 Jul 2019 17:25:07 +0200
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Xiaoyu Min <jackmin@mellanox.com>
Cc: orika@mellanox.com, John McNamara <john.mcnamara@intel.com>,
Marko Kovacevic <marko.kovacevic@intel.com>,
Thomas Monjalon <thomas@monjalon.net>,
Ferruh Yigit <ferruh.yigit@intel.com>,
Andrew Rybchenko <arybchenko@solarflare.com>, dev@dpdk.org
Message-ID: <20190703152507.GH4512@6wind.com>
References: <20190624154018.128379-1-jackmin@mellanox.com>
<cover.1562058723.git.jackmin@mellanox.com>
<f639cc93de14d44a1f213aba59a7e4991ca10bb7.1562058723.git.jackmin@mellanox.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f639cc93de14d44a1f213aba59a7e4991ca10bb7.1562058723.git.jackmin@mellanox.com>
Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add GRE key field to flow API
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
On Tue, Jul 02, 2019 at 05:45:52PM +0800, Xiaoyu Min wrote:
> Add new rte_flow_item_gre_key in order to match the optional key field.
>
> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
OK with adding this feature, however I still have a bunch of comments below.
> ---
> doc/guides/prog_guide/rte_flow.rst | 8 ++++++++
> lib/librte_ethdev/rte_flow.c | 1 +
> lib/librte_ethdev/rte_flow.h | 7 +++++++
> 3 files changed, 16 insertions(+)
>
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index a34d012e55..f4b7baa3c3 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -980,6 +980,14 @@ Matches a GRE header.
> - ``protocol``: protocol type.
> - Default ``mask`` matches protocol only.
>
> +Item: ``GRE_KEY``
> +^^^^^^^^^^^^^^^^^
> +
> +Matches a GRE key field.
> +This should be preceded by item ``GRE``
Nit: missing ending "."
> +
> +- Value to be matched is a big-endian 32 bit integer
> +
> Item: ``FUZZY``
> ^^^^^^^^^^^^^^^
>
> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> index 3277be1edb..f3e56d0bbe 100644
> --- a/lib/librte_ethdev/rte_flow.c
> +++ b/lib/librte_ethdev/rte_flow.c
> @@ -55,6 +55,7 @@ static const struct rte_flow_desc_data rte_flow_desc_item[] = {
> MK_FLOW_ITEM(NVGRE, sizeof(struct rte_flow_item_nvgre)),
> MK_FLOW_ITEM(MPLS, sizeof(struct rte_flow_item_mpls)),
> MK_FLOW_ITEM(GRE, sizeof(struct rte_flow_item_gre)),
> + MK_FLOW_ITEM(GRE_KEY, sizeof(rte_be32_t)),
Hmm? Adding a new item in the middle?
> MK_FLOW_ITEM(FUZZY, sizeof(struct rte_flow_item_fuzzy)),
> MK_FLOW_ITEM(GTP, sizeof(struct rte_flow_item_gtp)),
> MK_FLOW_ITEM(GTPC, sizeof(struct rte_flow_item_gtp)),
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index f3a8fb103f..5d3702a44c 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -289,6 +289,13 @@ enum rte_flow_item_type {
> */
> RTE_FLOW_ITEM_TYPE_GRE,
>
> + /**
> + * Matches a GRE optional key field.
> + *
> + * The value should a big-endian 32bit integer.
> + */
> + RTE_FLOW_ITEM_TYPE_GRE_KEY,
> +
Same comment. While I understand the intent to group GRE and GRE_KEY, doing
so causes ABI breakage by shifting the value of all subsequent pattern
items (see IPV6 and IPV6_EXT for instance).
We could later decide to sort them while knowingly breaking ABI on purpose,
however right now there's no choice but adding new pattern items and actions
at the end of their respective enums, please do that.
--
Adrien Mazarguil
6WIND