[v6,4/4] app/testpmd: match GRE's key and present bits
Checks
Commit Message
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 / gre_key value 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.
Acked-by: Ori Kam <orika@mellanox.com>
Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
---
app/test-pmd/cmdline_flow.c | 61 +++++++++++++++++++++
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 4 ++
2 files changed, 65 insertions(+)
Comments
On Fri, Jul 05, 2019 at 10:14:45AM +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 / gre_key value 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.
>
> Acked-by: Ori Kam <orika@mellanox.com>
> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
A few more nits below.
[...]
> @@ -1898,6 +1915,50 @@ static const struct token token_list[] = {
> .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_gre,
> protocol)),
> },
> + [ITEM_GRE_C_RSVD0_VER] = {
> + .name = "c_rsvd0_ver",
> + .help = "GRE's first word (bit0 - bit15)",
Help strings on existing fields should ideally be the same as their
counterparts in rte_flow.h (shortened if necessary, not starting with a cap
and not ending "."), in this case for instance:
.help =
"checksum (1b), undefined (1b), key bit (1b),"
" sequence number (1b), reserved 0 (9b),"
" version (3b)",
> + .next = NEXT(item_gre, NEXT_ENTRY(UNSIGNED), item_param),
> + .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_gre,
> + c_rsvd0_ver)),
> + },
> + [ITEM_GRE_C_BIT] = {
> + .name = "c_bit",
> + .help = "GRE's C present bit",
A bit odd, here's a suggestion:
"checksum bit (C)".
> + .next = NEXT(item_gre, NEXT_ENTRY(BOOLEAN), item_param),
> + .args = ARGS(ARGS_ENTRY_MASK_HTON(struct rte_flow_item_gre,
> + c_rsvd0_ver,
> + "\x80\x00\x00\x00")),
> + },
> + [ITEM_GRE_S_BIT] = {
> + .name = "s_bit",
> + .help = "GRE's S present bit",
Ditto:
"sequence number bit (S)"
> + .next = NEXT(item_gre, NEXT_ENTRY(BOOLEAN), item_param),
> + .args = ARGS(ARGS_ENTRY_MASK_HTON(struct rte_flow_item_gre,
> + c_rsvd0_ver,
> + "\x10\x00\x00\x00")),
> + },
> + [ITEM_GRE_K_BIT] = {
> + .name = "k_bit",
> + .help = "GRE's K present bit",
Ditto:
"key bit (K)"
> + .next = NEXT(item_gre, NEXT_ENTRY(BOOLEAN), item_param),
> + .args = ARGS(ARGS_ENTRY_MASK_HTON(struct rte_flow_item_gre,
> + c_rsvd0_ver,
> + "\x20\x00\x00\x00")),
> + },
> + [ITEM_GRE_KEY] = {
> + .name = "gre_key",
> + .help = "match GRE Key",
Nit: no caps for "Key" => "match GRE key"
> + .priv = PRIV_ITEM(GRE_KEY, sizeof(rte_be32_t)),
> + .next = NEXT(item_gre_key),
> + .call = parse_vc,
> + },
> + [ITEM_GRE_KEY_VALUE] = {
> + .name = "value",
> + .help = "GRE key value",
No need to repeat "GRE" here since it's already in GRE context:
"key value"
> + .next = NEXT(item_gre_key, NEXT_ENTRY(UNSIGNED), item_param),
> + .args = ARGS(ARG_ENTRY_HTON(rte_be32_t)),
> + },
Also ITEM_GRE_KEY and ITEM_GRE_KEY_VALUE should come after ITEM_META_DATA to
keep the same order as everywhere else.
Then assuming all the suggested changes are made:
Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Note I did not look at mlx5 patches, please make sure someone has reviewed
them. Thanks.
On Fri, 19-07-05, 10:58, Adrien Mazarguil wrote:
> On Fri, Jul 05, 2019 at 10:14:45AM +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 / gre_key value 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.
> >
> > Acked-by: Ori Kam <orika@mellanox.com>
> > Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
>
> A few more nits below.
>
> [...]
> > @@ -1898,6 +1915,50 @@ static const struct token token_list[] = {
> > .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_gre,
> > protocol)),
> > },
> > + [ITEM_GRE_C_RSVD0_VER] = {
> > + .name = "c_rsvd0_ver",
> > + .help = "GRE's first word (bit0 - bit15)",
>
> Help strings on existing fields should ideally be the same as their
> counterparts in rte_flow.h (shortened if necessary, not starting with a cap
> and not ending "."), in this case for instance:
>
Didn't know this before.
> .help =
> "checksum (1b), undefined (1b), key bit (1b),"
> " sequence number (1b), reserved 0 (9b),"
> " version (3b)",
>
I'll update.
> > + .next = NEXT(item_gre, NEXT_ENTRY(UNSIGNED), item_param),
> > + .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_gre,
> > + c_rsvd0_ver)),
> > + },
> > + [ITEM_GRE_C_BIT] = {
> > + .name = "c_bit",
> > + .help = "GRE's C present bit",
>
> A bit odd, here's a suggestion:
>
> "checksum bit (C)".
>
I'll update.
> > + .next = NEXT(item_gre, NEXT_ENTRY(BOOLEAN), item_param),
> > + .args = ARGS(ARGS_ENTRY_MASK_HTON(struct rte_flow_item_gre,
> > + c_rsvd0_ver,
> > + "\x80\x00\x00\x00")),
> > + },
> > + [ITEM_GRE_S_BIT] = {
> > + .name = "s_bit",
> > + .help = "GRE's S present bit",
>
> Ditto:
>
> "sequence number bit (S)"
>
Ditto.
> > + .next = NEXT(item_gre, NEXT_ENTRY(BOOLEAN), item_param),
> > + .args = ARGS(ARGS_ENTRY_MASK_HTON(struct rte_flow_item_gre,
> > + c_rsvd0_ver,
> > + "\x10\x00\x00\x00")),
> > + },
> > + [ITEM_GRE_K_BIT] = {
> > + .name = "k_bit",
> > + .help = "GRE's K present bit",
>
> Ditto:
>
> "key bit (K)"
>
Ditto.
> > + .next = NEXT(item_gre, NEXT_ENTRY(BOOLEAN), item_param),
> > + .args = ARGS(ARGS_ENTRY_MASK_HTON(struct rte_flow_item_gre,
> > + c_rsvd0_ver,
> > + "\x20\x00\x00\x00")),
> > + },
> > + [ITEM_GRE_KEY] = {
> > + .name = "gre_key",
> > + .help = "match GRE Key",
>
> Nit: no caps for "Key" => "match GRE key"
>
OK.
> > + .priv = PRIV_ITEM(GRE_KEY, sizeof(rte_be32_t)),
> > + .next = NEXT(item_gre_key),
> > + .call = parse_vc,
> > + },
> > + [ITEM_GRE_KEY_VALUE] = {
> > + .name = "value",
> > + .help = "GRE key value",
>
> No need to repeat "GRE" here since it's already in GRE context:
>
> "key value"
>
OK.
> > + .next = NEXT(item_gre_key, NEXT_ENTRY(UNSIGNED), item_param),
> > + .args = ARGS(ARG_ENTRY_HTON(rte_be32_t)),
> > + },
>
> Also ITEM_GRE_KEY and ITEM_GRE_KEY_VALUE should come after ITEM_META_DATA to
> keep the same order as everywhere else.
>
Yes, it should be.
> Then assuming all the suggested changes are made:
>
> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
>
Thank you!
> Note I did not look at mlx5 patches, please make sure someone has reviewed
> them. Thanks.
>
Yes, Slava will review them.
> --
> Adrien Mazarguil
> 6WIND
@@ -148,6 +148,10 @@ enum index {
ITEM_MPLS_LABEL,
ITEM_GRE,
ITEM_GRE_PROTO,
+ ITEM_GRE_C_RSVD0_VER,
+ ITEM_GRE_C_BIT,
+ ITEM_GRE_K_BIT,
+ ITEM_GRE_S_BIT,
ITEM_FUZZY,
ITEM_FUZZY_THRESH,
ITEM_GTP,
@@ -181,6 +185,8 @@ enum index {
ITEM_ICMP6_ND_OPT_TLA_ETH_TLA,
ITEM_META,
ITEM_META_DATA,
+ ITEM_GRE_KEY,
+ ITEM_GRE_KEY_VALUE,
/* Validate/create actions. */
ACTIONS,
@@ -610,6 +616,7 @@ static const enum index next_item[] = {
ITEM_ICMP6_ND_OPT_SLA_ETH,
ITEM_ICMP6_ND_OPT_TLA_ETH,
ITEM_META,
+ ITEM_GRE_KEY,
ZERO,
};
@@ -755,6 +762,16 @@ static const enum index item_mpls[] = {
static const enum index item_gre[] = {
ITEM_GRE_PROTO,
+ ITEM_GRE_C_RSVD0_VER,
+ ITEM_GRE_C_BIT,
+ ITEM_GRE_K_BIT,
+ ITEM_GRE_S_BIT,
+ ITEM_NEXT,
+ ZERO,
+};
+
+static const enum index item_gre_key[] = {
+ ITEM_GRE_KEY_VALUE,
ITEM_NEXT,
ZERO,
};
@@ -1898,6 +1915,50 @@ static const struct token token_list[] = {
.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_gre,
protocol)),
},
+ [ITEM_GRE_C_RSVD0_VER] = {
+ .name = "c_rsvd0_ver",
+ .help = "GRE's first word (bit0 - bit15)",
+ .next = NEXT(item_gre, NEXT_ENTRY(UNSIGNED), item_param),
+ .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_gre,
+ c_rsvd0_ver)),
+ },
+ [ITEM_GRE_C_BIT] = {
+ .name = "c_bit",
+ .help = "GRE's C present bit",
+ .next = NEXT(item_gre, NEXT_ENTRY(BOOLEAN), item_param),
+ .args = ARGS(ARGS_ENTRY_MASK_HTON(struct rte_flow_item_gre,
+ c_rsvd0_ver,
+ "\x80\x00\x00\x00")),
+ },
+ [ITEM_GRE_S_BIT] = {
+ .name = "s_bit",
+ .help = "GRE's S present bit",
+ .next = NEXT(item_gre, NEXT_ENTRY(BOOLEAN), item_param),
+ .args = ARGS(ARGS_ENTRY_MASK_HTON(struct rte_flow_item_gre,
+ c_rsvd0_ver,
+ "\x10\x00\x00\x00")),
+ },
+ [ITEM_GRE_K_BIT] = {
+ .name = "k_bit",
+ .help = "GRE's K present bit",
+ .next = NEXT(item_gre, NEXT_ENTRY(BOOLEAN), item_param),
+ .args = ARGS(ARGS_ENTRY_MASK_HTON(struct rte_flow_item_gre,
+ c_rsvd0_ver,
+ "\x20\x00\x00\x00")),
+ },
+ [ITEM_GRE_KEY] = {
+ .name = "gre_key",
+ .help = "match GRE Key",
+ .priv = PRIV_ITEM(GRE_KEY, sizeof(rte_be32_t)),
+ .next = NEXT(item_gre_key),
+ .call = parse_vc,
+ },
+ [ITEM_GRE_KEY_VALUE] = {
+ .name = "value",
+ .help = "GRE key value",
+ .next = NEXT(item_gre_key, NEXT_ENTRY(UNSIGNED), item_param),
+ .args = ARGS(ARG_ENTRY_HTON(rte_be32_t)),
+ },
[ITEM_FUZZY] = {
.name = "fuzzy",
.help = "fuzzy pattern match, expect faster than default",
@@ -3804,6 +3804,10 @@ This section lists supported pattern items and their attributes, if any.
- ``protocol {unsigned}``: protocol type.
+- ``gre_key``: match GRE optional key field.
+
+ - ``value {unsigned}``: key value.
+
- ``fuzzy``: fuzzy pattern match, expect faster than default.
- ``thresh {unsigned}``: accuracy threshold.