Hi Andrew, Can you share some comments on the modified version v3? BR Rongwei
> -----Original Message----- > From: Rongwei Liu <rongw...@nvidia.com> > Sent: Monday, January 30, 2023 21:20 > To: Matan Azrad <ma...@nvidia.com>; Slava Ovsiienko > <viachesl...@nvidia.com>; Ori Kam <or...@nvidia.com>; NBU-Contact- > Thomas Monjalon (EXTERNAL) <tho...@monjalon.net>; Aman Singh > <aman.deep.si...@intel.com>; Yuying Zhang <yuying.zh...@intel.com>; > Ferruh Yigit <ferruh.yi...@amd.com>; Andrew Rybchenko > <andrew.rybche...@oktetlabs.ru> > Cc: dev@dpdk.org; Raslan Darawsheh <rasl...@nvidia.com> > Subject: [PATCH v3 01/11] ethdev: add flex item modify field support > > External email: Use caution opening links or attachments > > > Add flex item as modify field destination. > Add "struct rte_flow_item_flex_handle *flex_handle" into "struct > rte_flow_action_modify_data" as union with existed "level" member. This new > member is dedicated for modifying flex item. > > Add flex item modify field cmdline support. Now user can use testpmd cli to > specify which flex item to be modified, either source or destination. > > Syntax is as below: > modify_field op set dst_type flex_item dst_level 0 dst_offset 16 src_type > value > src_value 0x123456781020 width 8 > > Signed-off-by: Rongwei Liu <rongw...@nvidia.com> > Acked-by: Ori Kam <or...@nvidia.com> > --- > app/test-pmd/cmdline_flow.c | 89 ++++++++++++++++++++++++-- > doc/guides/prog_guide/rte_flow.rst | 41 +++++++----- > doc/guides/rel_notes/release_23_03.rst | 4 ++ > lib/ethdev/rte_flow.h | 8 ++- > 4 files changed, 116 insertions(+), 26 deletions(-) > > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c > index 88108498e0..7c12d63cbc 100644 > --- a/app/test-pmd/cmdline_flow.c > +++ b/app/test-pmd/cmdline_flow.c > @@ -601,10 +601,12 @@ enum index { > ACTION_MODIFY_FIELD_DST_TYPE, > ACTION_MODIFY_FIELD_DST_TYPE_VALUE, > ACTION_MODIFY_FIELD_DST_LEVEL, > + ACTION_MODIFY_FIELD_DST_LEVEL_VALUE, > ACTION_MODIFY_FIELD_DST_OFFSET, > ACTION_MODIFY_FIELD_SRC_TYPE, > ACTION_MODIFY_FIELD_SRC_TYPE_VALUE, > ACTION_MODIFY_FIELD_SRC_LEVEL, > + ACTION_MODIFY_FIELD_SRC_LEVEL_VALUE, > ACTION_MODIFY_FIELD_SRC_OFFSET, > ACTION_MODIFY_FIELD_SRC_VALUE, > ACTION_MODIFY_FIELD_SRC_POINTER, @@ -807,7 +809,8 @@ static > const char *const modify_field_ids[] = { > "udp_port_src", "udp_port_dst", > "vxlan_vni", "geneve_vni", "gtp_teid", > "tag", "mark", "meta", "pointer", "value", > - "ipv4_ecn", "ipv6_ecn", "gtp_psc_qfi", "meter_color", NULL > + "ipv4_ecn", "ipv6_ecn", "gtp_psc_qfi", "meter_color", > + "flex_item", NULL > }; > > static const char *const meter_colors[] = { @@ -2282,6 +2285,10 @@ > parse_vc_modify_field_id(struct context *ctx, const struct token *token, > const char *str, unsigned int len, void *buf, > unsigned int size); static int > +parse_vc_modify_field_level(struct context *ctx, const struct token *token, > + const char *str, unsigned int len, void *buf, > + unsigned int size); static int > parse_vc_action_conntrack_update(struct context *ctx, const struct token > *token, > const char *str, unsigned int len, void *buf, > unsigned int size); @@ -5976,11 +5983,15 @@ static > const > struct token token_list[] = { > .name = "dst_level", > .help = "destination field level", > .next = NEXT(action_modify_field_dst, > - NEXT_ENTRY(COMMON_UNSIGNED)), > - .args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field, > - dst.level)), > + > + NEXT_ENTRY(ACTION_MODIFY_FIELD_DST_LEVEL_VALUE)), > .call = parse_vc_conf, > }, > + [ACTION_MODIFY_FIELD_DST_LEVEL_VALUE] = { > + .name = "{dst_level}", > + .help = "destination field level value", > + .call = parse_vc_modify_field_level, > + .comp = comp_none, > + }, > [ACTION_MODIFY_FIELD_DST_OFFSET] = { > .name = "dst_offset", > .help = "destination field bit offset", @@ -6007,11 +6018,15 > @@ > static const struct token token_list[] = { > .name = "src_level", > .help = "source field level", > .next = NEXT(action_modify_field_src, > - NEXT_ENTRY(COMMON_UNSIGNED)), > - .args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field, > - src.level)), > + > + NEXT_ENTRY(ACTION_MODIFY_FIELD_SRC_LEVEL_VALUE)), > .call = parse_vc_conf, > }, > + [ACTION_MODIFY_FIELD_SRC_LEVEL_VALUE] = { > + .name = "{src_level}", > + .help = "source field level value", > + .call = parse_vc_modify_field_level, > + .comp = comp_none, > + }, > [ACTION_MODIFY_FIELD_SRC_OFFSET] = { > .name = "src_offset", > .help = "source field bit offset", @@ -8477,6 +8492,66 @@ > parse_vc_modify_field_id(struct context *ctx, const struct token *token, > return len; > } > > +/** Parse level for modify_field command. */ static int > +parse_vc_modify_field_level(struct context *ctx, const struct token *token, > + const char *str, unsigned int len, void *buf, > + unsigned int size) { > + struct rte_flow_action_modify_field *action; > + struct flex_item *fp; > + uint32_t val; > + struct buffer *out = buf; > + char *end; > + > + (void)token; > + (void)size; > + if (ctx->curr != ACTION_MODIFY_FIELD_DST_LEVEL_VALUE && > + ctx->curr != ACTION_MODIFY_FIELD_SRC_LEVEL_VALUE) > + return -1; > + if (!ctx->object) > + return len; > + action = ctx->object; > + errno = 0; > + val = strtoumax(str, &end, 0); > + if (errno || (size_t)(end - str) != len) > + return -1; > + /* No need to validate action template mask value */ > + if (out->args.vc.masks) { > + if (ctx->curr == ACTION_MODIFY_FIELD_DST_LEVEL_VALUE) > + action->dst.level = val; > + else > + action->src.level = val; > + return len; > + } > + if ((ctx->curr == ACTION_MODIFY_FIELD_DST_LEVEL_VALUE && > + action->dst.field == RTE_FLOW_FIELD_FLEX_ITEM) || > + (ctx->curr == ACTION_MODIFY_FIELD_SRC_LEVEL_VALUE && > + action->src.field == RTE_FLOW_FIELD_FLEX_ITEM)) { > + if (val >= FLEX_MAX_PARSERS_NUM) { > + printf("Bad flex item handle\n"); > + return -1; > + } > + fp = flex_items[ctx->port][val]; > + if (!fp) { > + printf("Bad flex item handle\n"); > + return -1; > + } > + } > + if (ctx->curr == ACTION_MODIFY_FIELD_DST_LEVEL_VALUE) { > + if (action->dst.field != RTE_FLOW_FIELD_FLEX_ITEM) > + action->dst.level = val; > + else > + action->dst.flex_handle = fp->flex_handle; > + } else if (ctx->curr == ACTION_MODIFY_FIELD_SRC_LEVEL_VALUE) { > + if (action->src.field != RTE_FLOW_FIELD_FLEX_ITEM) > + action->src.level = val; > + else > + action->src.flex_handle = fp->flex_handle; > + } > + return len; > +} > + > /** Parse the conntrack update, not a rte_flow_action. */ static int > parse_vc_action_conntrack_update(struct context *ctx, const struct token > *token, diff --git a/doc/guides/prog_guide/rte_flow.rst > b/doc/guides/prog_guide/rte_flow.rst > index 3e6242803d..9ffd7baa7a 100644 > --- a/doc/guides/prog_guide/rte_flow.rst > +++ b/doc/guides/prog_guide/rte_flow.rst > @@ -2905,6 +2905,9 @@ encapsulation level, from outermost to innermost > (lower to higher values). > For the tag array (in case of multiple tags are supported and present) > ``level`` translates directly into the array index. > > +``flex_handle`` is used to specify the flex item pointer which is being > +modified. ``flex_handle`` and ``level`` are mutually exclusive. > + > ``offset`` specifies the number of bits to skip from a field's start. > That allows performing a partial copy of the needed part or to divide a big > packet field into multiple smaller fields. Alternatively, ``offset`` allows > @@ - > 2952,23 +2955,27 @@ value as sequence of bytes {xxx, xxx, 0x85, xxx, xxx, > xxx}. > > .. table:: destination/source field definition > > - > +---------------+----------------------------------------------------------+ > - | Field | Value > | > - > +===============+=============================================== > ===========+ > - | ``field`` | ID: packet field, mark, meta, tag, immediate, pointer > | > - > +---------------+----------------------------------------------------------+ > - | ``level`` | encapsulation level of a packet field or tag array > index | > - > +---------------+----------------------------------------------------------+ > - | ``offset`` | number of bits to skip at the beginning > | > - > +---------------+----------------------------------------------------------+ > - | ``value`` | immediate value buffer (source field only, not > | > - | | applicable to destination) for RTE_FLOW_FIELD_VALUE > | > - | | field type > | > - > +---------------+----------------------------------------------------------+ > - | ``pvalue`` | pointer to immediate value data (source field only, not > | > - | | applicable to destination) for RTE_FLOW_FIELD_POINTER > | > - | | field type > | > - > +---------------+----------------------------------------------------------+ > + > +-----------------+----------------------------------------------------------+ > + | Field | Value > | > + > +=================+============================================= > =============+ > + | ``field`` | ID: packet field, mark, meta, tag, immediate, pointer > | > + > +-----------------+----------------------------------------------------------+ > + | ``level`` | encapsulation level of a packet field or tag array > index | > + > +-----------------+----------------------------------------------------------+ > + | ``flex_handle`` | flex item handle of a packet field > | > + > +-----------------+----------------------------------------------------------+ > + | ``offset`` | number of bits to skip at the beginning > | > + > +-----------------+----------------------------------------------------------+ > + | ``value`` | immediate value buffer (source field only, not > | > + | | applicable to destination) for RTE_FLOW_FIELD_VALUE > | > + | | field type > | > + | | This field is only 16 bytes, maybe not big enough for > | > + | | all NICs' flex item > | > + > +-----------------+----------------------------------------------------------+ > + | ``pvalue`` | pointer to immediate value data (source field only, > not | > + | | applicable to destination) for RTE_FLOW_FIELD_POINTER > | > + | | field type > | > + > + +-----------------+--------------------------------------------------- > + -------+ > > Action: ``CONNTRACK`` > ^^^^^^^^^^^^^^^^^^^^^ > diff --git a/doc/guides/rel_notes/release_23_03.rst > b/doc/guides/rel_notes/release_23_03.rst > index c15f6fbb9f..3fb6e738e2 100644 > --- a/doc/guides/rel_notes/release_23_03.rst > +++ b/doc/guides/rel_notes/release_23_03.rst > @@ -69,6 +69,10 @@ New Features > ``rte_event_dev_config::nb_single_link_event_port_queues`` parameter > required for eth_rx, eth_tx, crypto and timer eventdev adapters. > > +* ethdev: added a new field: > + > + - modify flex item: ``rte_flow_action_modify_data.flex_handle``. > + > > Removed Items > ------------- > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index > b60987db4b..ba9f3d50a3 100644 > --- a/lib/ethdev/rte_flow.h > +++ b/lib/ethdev/rte_flow.h > @@ -3528,6 +3528,7 @@ enum rte_flow_field_id { > RTE_FLOW_FIELD_IPV6_ECN, /**< IPv6 ECN. */ > RTE_FLOW_FIELD_GTP_PSC_QFI, /**< GTP QFI. */ > RTE_FLOW_FIELD_METER_COLOR, /**< Meter color marker. */ > + RTE_FLOW_FIELD_FLEX_ITEM, /**< Flex item. */ > }; > > /** > @@ -3541,8 +3542,11 @@ struct rte_flow_action_modify_data { > RTE_STD_C11 > union { > struct { > - /** Encapsulation level or tag index. */ > - uint32_t level; > + /** Encapsulation level or tag index or flex item > handle. */ > + union { > + uint32_t level; > + struct rte_flow_item_flex_handle *flex_handle; > + }; > /** Number of bits to skip from a field. */ > uint32_t offset; > }; > -- > 2.27.0