From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Jack Min <jackmin@mellanox.com>
Cc: Ori Kam <orika@mellanox.com>, Wenzhuo Lu <wenzhuo.lu@intel.com>,
Jingjing Wu <jingjing.wu@intel.com>,
Bernard Iremonger <bernard.iremonger@intel.com>,
John McNamara <john.mcnamara@intel.com>,
Marko Kovacevic <marko.kovacevic@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v4 4/4] app/testpmd: match GRE's key and present bits
Date: Thu, 4 Jul 2019 11:52:02 +0200 [thread overview]
Message-ID: <20190704095202.GL4512@6wind.com> (raw)
In-Reply-To: <20190704055231.bpwbvbd6g2zosbl6@mellanox.com>
On Thu, Jul 04, 2019 at 05:52:43AM +0000, Jack Min wrote:
> On Wed, 19-07-03, 17:25, Adrien Mazarguil wrote:
> > On Tue, Jul 02, 2019 at 05:45:55PM +0800, Xiaoyu Min wrote:
> > > support matching on GRE key and present bits (C,K,S)
> > >
> > > example testpmd command could be:
> > > testpmd>flow create 0 ingress group 1 pattern eth / ipv4 /
> > > gre crksv is 0x2000 crksv mask 0xb000 /
> > > gre_key key is 0x12345678 / end
> > > actions rss queues 1 0 end / mark id 196 / end
> > >
> > > Which will match GRE packet with k present bit set and key value is
> > > 0x12345678.
> > >
> > > Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> >
> > I'm wondering... Is matching the K bit mandatory if one explicitly matches
> > gre_key already or is this a specific hardware limitation in your case?
> >
>
> If there is gre_key item MLX5 PMD will force set HW matching on K bit,
> From HW perspective it is mandatory. But, from testpmd (user)
> perspective, I agree with you, user needn't set matching on K bit if
> they already explicitly set gre_key item.
OK, makes sense.
> > Perhaps we could document that the K bit is implicitly matched as "1" in the
> > default mask when a gre_key pattern item is present. If a user explicitly
>
> Yes, I should document this.
> So it should be documented in __testpmd_funcs.rst__ ?
No it would be a change in the GRE_KEY item itself at the rte_flow API
level (rte_flow.h) & documentation (rte_flow.rst). The flow rules created by
testpmd must be an exact translation of user input, as a debugging tool it
can't request something that wasn't explicitly written.
> > spec/mask K as "0" and still provides gre_key, the PMD can safely ignore the
> > gre_key item.
> >
>
> Well, actullay, when a user explicitly set spec/mask K as "0" and still
> provide gre_key item, MLX5 PMD will implicitly set match on K bit as
> "1", just ingore the K bit set by user.
Not good then. You should spit an error out if it's an impossible
combination. You can't match both K == 0 *and* a GRE key, unless perhaps if
key mask is also 0, e.g.:
gre crksv is 0x0000 crksv mask 0xb000 /
gre_key value spec 0x00000000 value mask 0x00000000
This is merely an overly complex way for telling the PMD that one wants to
match packets without GRE keys that you could technically support.
> The reason is wanna keep code simple, needn't to get
> information from other item (gre) inside gre_key item, or vice verse.
PMDs typically maintain context as they process the pattern. The GRE pattern
item is guaranteed to come before GRE_KEY, so you already know at this point
whether users want to match K at all, and if so, what value they want it to
have.
> And, I think, when a user provides a gre_key item, most probably, they do
> really wanna match on gre_key. What do you think?
Depends. They may want to match all GRE traffic with a key, doesn't matter
which, in order to process it through a different path. To do so they could
either:
1. Use the GRE item only to match K bit == 1.
2. Use the GRE_KEY item to match a nonspecific key value (mask == 0).
3. Use a combination of both.
I think you can easily support all three of them with mlx5 if you support
partial masks on GRE keys (I haven't checked), even if you're unable to
specifically match the K bit itself.
[...]
> > > @@ -755,6 +759,13 @@ static const enum index item_mpls[] = {
> > >
> > > static const enum index item_gre[] = {
> > > ITEM_GRE_PROTO,
> > > + ITEM_GRE_CRKSV,
> >
> > CRKSV may be unnecessary in this patch if the K bit is documented and
> > implemented as described in my previous comment.
> >
>
> Well, actully, we also wanna testpmd can match on C,S bits with K bit
> together so we can test on gre packet with key only or csum + key, or
> csum + key + sequence.
OK no problem. Perhaps you could make this easier by allowing users to match
individual bits, let me explain:
The flow command in testpmd is a direct interface to manipulate rte_flow's
structures. The "crksv" field doesn't exist in rte_flow_item_gre, its name
is "c_rsvd0_ver". Testpmd must use the same in its command and internal
code.
However since bit-masks are usually a pain to mentally work out, you can
provide extras for convenience. The "types" field of the RSS action
(ACTION_RSS_TYPES) is an extreme example of this approach.
So I suggest adding ITEM_GRE_C_RSVD0_VER taking a 16-bit value like CRKSV,
and complete it with ITEM_GRE_C_BIT, ITEM_GRE_S_BIT and ITEM_GRE_K_BIT
addressing the individual bits you would like to expose for convenience.
[...]
> > You should have named this field "value" then, i.e.:
> >
> > - ``value {unsigned}``: key value.
> >
>
> OK, I'll update it.
Please remember to update it in rte_flow.h and documentation as well,
thanks.
--
Adrien Mazarguil
6WIND
next prev parent reply other threads:[~2019-07-04 9:52 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-24 15:40 [dpdk-dev] [PATCH 0/4] ethdev: add GRE key field to flow API Xiaoyu Min
2019-06-24 15:40 ` [dpdk-dev] [PATCH 1/4] " Xiaoyu Min
2019-06-27 12:36 ` Ori Kam
2019-07-01 5:40 ` Ori Kam
2019-07-01 11:40 ` Jack Min
2019-06-24 15:40 ` [dpdk-dev] [PATCH 2/4] net/mlx5: support match GRE protocol on DR engine Xiaoyu Min
2019-06-24 15:40 ` [dpdk-dev] [PATCH 3/4] net/mlx5: match GRE's key and present bits Xiaoyu Min
2019-06-24 15:40 ` [dpdk-dev] [PATCH 4/4] app/testpmd: " Xiaoyu Min
2019-07-01 13:11 ` [dpdk-dev] [PATCH v2 0/4] ethdev: add GRE key field to flow API Xiaoyu Min
2019-07-01 13:11 ` [dpdk-dev] [PATCH v2 1/4] " Xiaoyu Min
2019-07-01 13:11 ` [dpdk-dev] [PATCH v2 2/4] net/mlx5: support match GRE protocol on DR engine Xiaoyu Min
2019-07-01 13:11 ` [dpdk-dev] [PATCH v2 3/4] net/mlx5: match GRE's key and present bits Xiaoyu Min
2019-07-01 13:11 ` [dpdk-dev] [PATCH v2 4/4] app/testpmd: " Xiaoyu Min
2019-07-02 3:08 ` [dpdk-dev] [PATCH v3 0/4] match on GRE's key Xiaoyu Min
2019-07-02 3:08 ` [dpdk-dev] [PATCH v3 1/4] ethdev: add GRE key field to flow API Xiaoyu Min
2019-07-02 3:08 ` [dpdk-dev] [PATCH v3 2/4] net/mlx5: support match GRE protocol on DR engine Xiaoyu Min
2019-07-02 3:08 ` [dpdk-dev] [PATCH v3 3/4] net/mlx5: match GRE's key and present bits Xiaoyu Min
2019-07-02 3:08 ` [dpdk-dev] [PATCH v3 4/4] app/testpmd: " Xiaoyu Min
2019-07-02 9:45 ` [dpdk-dev] [PATCH v4 0/4] match on GRE's key Xiaoyu Min
2019-07-02 9:45 ` [dpdk-dev] [PATCH v4 1/4] ethdev: add GRE key field to flow API Xiaoyu Min
2019-07-03 14:06 ` Thomas Monjalon
2019-07-04 2:18 ` Jack Min
2019-07-03 15:25 ` Adrien Mazarguil
2019-07-04 2:43 ` Jack Min
2019-07-02 9:45 ` [dpdk-dev] [PATCH v4 2/4] net/mlx5: support match GRE protocol on DR engine Xiaoyu Min
2019-07-02 9:45 ` [dpdk-dev] [PATCH v4 3/4] net/mlx5: match GRE's key and present bits Xiaoyu Min
2019-07-02 9:45 ` [dpdk-dev] [PATCH v4 4/4] app/testpmd: " Xiaoyu Min
2019-07-02 9:53 ` [dpdk-dev] [Suspected-Phishing][PATCH " Ori Kam
2019-07-03 15:25 ` [dpdk-dev] [PATCH " Adrien Mazarguil
2019-07-04 5:52 ` Jack Min
2019-07-04 9:52 ` Adrien Mazarguil [this message]
2019-07-04 11:56 ` Jack Min
2019-07-04 12:13 ` Adrien Mazarguil
2019-07-04 13:01 ` Jack Min
2019-07-04 16:30 ` [dpdk-dev] [PATCH v5 0/4] match on GRE's key Xiaoyu Min
2019-07-04 16:30 ` [dpdk-dev] [PATCH v5 1/4] ethdev: add GRE key field to flow API Xiaoyu Min
2019-07-04 16:30 ` [dpdk-dev] [PATCH v5 2/4] net/mlx5: support match GRE protocol on DR engine Xiaoyu Min
2019-07-04 16:30 ` [dpdk-dev] [PATCH v5 3/4] net/mlx5: match GRE's key and present bits Xiaoyu Min
2019-07-04 16:30 ` [dpdk-dev] [PATCH v5 4/4] app/testpmd: " Xiaoyu Min
2019-07-05 2:14 ` [dpdk-dev] [PATCH v6 0/4] match on GRE's key Xiaoyu Min
2019-07-05 2:14 ` [dpdk-dev] [PATCH v6 1/4] ethdev: add GRE key field to flow API Xiaoyu Min
2019-07-05 8:39 ` Adrien Mazarguil
2019-07-05 2:14 ` [dpdk-dev] [PATCH v6 2/4] net/mlx5: support match GRE protocol on DR engine Xiaoyu Min
2019-07-05 2:14 ` [dpdk-dev] [PATCH v6 3/4] net/mlx5: match GRE's key and present bits Xiaoyu Min
2019-07-05 2:14 ` [dpdk-dev] [PATCH v6 4/4] app/testpmd: " Xiaoyu Min
2019-07-05 8:58 ` Adrien Mazarguil
2019-07-05 9:06 ` Jack Min
2019-07-05 9:54 ` [dpdk-dev] [PATCH v7 0/4] match on GRE's key Xiaoyu Min
2019-07-05 9:54 ` [dpdk-dev] [PATCH v7 1/4] ethdev: add GRE key field to flow API Xiaoyu Min
2019-07-09 5:21 ` [dpdk-dev] [Suspected-Phishing][PATCH " Slava Ovsiienko
2019-07-05 9:54 ` [dpdk-dev] [PATCH v7 2/4] net/mlx5: support match GRE protocol on DR engine Xiaoyu Min
2019-07-09 5:21 ` [dpdk-dev] [Suspected-Phishing][PATCH " Slava Ovsiienko
2019-07-05 9:54 ` [dpdk-dev] [PATCH v7 3/4] net/mlx5: match GRE's key and present bits Xiaoyu Min
2019-07-09 5:21 ` [dpdk-dev] [Suspected-Phishing][PATCH " Slava Ovsiienko
2019-07-05 9:54 ` [dpdk-dev] [PATCH v7 4/4] app/testpmd: " Xiaoyu Min
2019-07-09 5:21 ` [dpdk-dev] [Suspected-Phishing][PATCH " Slava Ovsiienko
2019-07-08 18:00 ` [dpdk-dev] [PATCH v7 0/4] match on GRE's key Ferruh Yigit
2019-07-09 9:02 ` [dpdk-dev] [PATCH v8 0/2] " Xiaoyu Min
2019-07-09 9:02 ` [dpdk-dev] [PATCH v8 1/2] net/mlx5: support match GRE protocol on DR engine Xiaoyu Min
2019-07-09 9:02 ` [dpdk-dev] [PATCH v8 2/2] net/mlx5: match GRE's key and present bits Xiaoyu Min
2019-07-09 9:54 ` Thomas Monjalon
2019-07-09 10:46 ` Jack Min
2019-07-09 10:59 ` [dpdk-dev] [PATCH v9 0/2] match on GRE's key Xiaoyu Min
2019-07-09 10:59 ` [dpdk-dev] [PATCH v9 1/2] net/mlx5: support match GRE protocol on DR engine Xiaoyu Min
2019-07-09 10:59 ` [dpdk-dev] [PATCH v9 2/2] net/mlx5: match GRE's key and present bits Xiaoyu Min
2019-07-11 9:14 ` [dpdk-dev] [PATCH v9 0/2] match on GRE's key Raslan Darawsheh
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=20190704095202.GL4512@6wind.com \
--to=adrien.mazarguil@6wind.com \
--cc=bernard.iremonger@intel.com \
--cc=dev@dpdk.org \
--cc=jackmin@mellanox.com \
--cc=jingjing.wu@intel.com \
--cc=john.mcnamara@intel.com \
--cc=marko.kovacevic@intel.com \
--cc=orika@mellanox.com \
--cc=wenzhuo.lu@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).