HI Andrew:
BR
Rongwei
> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
> Sent: Wednesday, February 1, 2023 17:21
> To: Rongwei Liu <rongw...@nvidia.com>; dev@dpdk.org; Matan Azrad
> <ma...@nvidia.com>; Slava Ovsiienko <viachesl...@nvidia.com>; Ori Kam
> <or...@nvidia.com>; NBU-Contact-Thomas Monjalon (EXTERNAL)
> <tho...@monjalon.net>
> Cc: Raslan Darawsheh <rasl...@nvidia.com>; Aman Singh
> <aman.deep.si...@intel.com>; Yuying Zhang <yuying.zh...@intel.com>;
> Ferruh Yigit <ferruh.yi...@amd.com>; Olivier Matz <olivier.m...@6wind.com>
> Subject: Re: [PATCH v4 1/3] ethdev: add IPv6 routing extension header
> definition
>
> External email: Use caution opening links or attachments
>
>
> On 1/31/23 12:36, Rongwei Liu wrote:
> > Add IPv6 routing extension header definition and no TLV support for
> > now.
> >
> > At rte_flow layer, there are new items defined for matching
> > type/nexthdr/segments_left field.
> >
> > Add command line support for IPv6 routing extension header
> > matching: type/nexthdr/segment_list.
> >
> > Signed-off-by: Rongwei Liu <rongw...@nvidia.com>
> > Acked-by: Ori Kam <or...@nvidia.com>
> > ---
> > app/test-pmd/cmdline_flow.c | 46 ++++++++++++++++++++++++++
> > doc/guides/prog_guide/rte_flow.rst | 9 +++++
> > doc/guides/rel_notes/release_23_03.rst | 9 +++++
> > lib/ethdev/rte_flow.c | 19 +++++++++++
> > lib/ethdev/rte_flow.h | 21 ++++++++++++
> > lib/net/rte_ip.h | 19 +++++++++++
> > 6 files changed, 123 insertions(+)
> >
> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > index 88108498e0..7a8516829c 100644
> > --- a/app/test-pmd/cmdline_flow.c
> > +++ b/app/test-pmd/cmdline_flow.c
> > @@ -298,6 +298,10 @@ enum index {
> > ITEM_IPV6_SRC,
> > ITEM_IPV6_DST,
> > ITEM_IPV6_HAS_FRAG_EXT,
> > + ITEM_IPV6_ROUTING_EXT,
> > + ITEM_IPV6_ROUTING_EXT_TYPE,
> > + ITEM_IPV6_ROUTING_EXT_NEXT_HDR,
> > + ITEM_IPV6_ROUTING_EXT_SEG_LEFT,
> > ITEM_ICMP,
> > ITEM_ICMP_TYPE,
> > ITEM_ICMP_CODE,
> > @@ -1326,6 +1330,7 @@ static const enum index next_item[] = {
> > ITEM_ARP_ETH_IPV4,
> > ITEM_IPV6_EXT,
> > ITEM_IPV6_FRAG_EXT,
> > + ITEM_IPV6_ROUTING_EXT,
> > ITEM_ICMP6,
> > ITEM_ICMP6_ND_NS,
> > ITEM_ICMP6_ND_NA,
> > @@ -1435,6 +1440,15 @@ static const enum index item_ipv6[] = {
> > ITEM_IPV6_SRC,
> > ITEM_IPV6_DST,
> > ITEM_IPV6_HAS_FRAG_EXT,
> > + ITEM_IPV6_ROUTING_EXT,
> > + ITEM_NEXT,
> > + ZERO,
> > +};
> > +
> > +static const enum index item_ipv6_routing_ext[] = {
> > + ITEM_IPV6_ROUTING_EXT_TYPE,
> > + ITEM_IPV6_ROUTING_EXT_NEXT_HDR,
> > + ITEM_IPV6_ROUTING_EXT_SEG_LEFT,
> > ITEM_NEXT,
> > ZERO,
> > };
> > @@ -3844,6 +3858,38 @@ static const struct token token_list[] = {
> > .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_item_ipv6,
> > has_frag_ext, 1)),
> > },
> > + [ITEM_IPV6_ROUTING_EXT] = {
> > + .name = "ipv6_routing_ext",
> > + .help = "match IPv6 routing extension header",
> > + .priv = PRIV_ITEM(IPV6_ROUTING_EXT,
> > + sizeof(struct
> > rte_flow_item_ipv6_routing_ext)),
> > + .next = NEXT(item_ipv6_routing_ext),
> > + .call = parse_vc,
> > + },
> > + [ITEM_IPV6_ROUTING_EXT_TYPE] = {
> > + .name = "ext_type",
> > + .help = "match IPv6 routing extension header type",
> > + .next = NEXT(item_ipv6_routing_ext,
> NEXT_ENTRY(COMMON_UNSIGNED),
> > + item_param),
> > + .args = ARGS(ARGS_ENTRY_HTON(struct
> rte_flow_item_ipv6_routing_ext,
> > + hdr.type)),
> > + },
> > + [ITEM_IPV6_ROUTING_EXT_NEXT_HDR] = {
> > + .name = "ext_next_hdr",
> > + .help = "match IPv6 routing extension header next header
> > type",
> > + .next = NEXT(item_ipv6_routing_ext,
> NEXT_ENTRY(COMMON_UNSIGNED),
> > + item_param),
> > + .args = ARGS(ARGS_ENTRY_HTON(struct
> rte_flow_item_ipv6_routing_ext,
> > + hdr.next_hdr)),
> > + },
> > + [ITEM_IPV6_ROUTING_EXT_SEG_LEFT] = {
> > + .name = "ext_seg_left",
> > + .help = "match IPv6 routing extension header segment left",
> > + .next = NEXT(item_ipv6_routing_ext,
> NEXT_ENTRY(COMMON_UNSIGNED),
> > + item_param),
> > + .args = ARGS(ARGS_ENTRY_HTON(struct
> rte_flow_item_ipv6_routing_ext,
> > + hdr.segments_left)),
> > + },
> > [ITEM_ICMP] = {
> > .name = "icmp",
> > .help = "match ICMP header", diff --git
> > a/doc/guides/prog_guide/rte_flow.rst
> > b/doc/guides/prog_guide/rte_flow.rst
> > index 3e6242803d..602fab29d3 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -1544,6 +1544,15 @@ Matches Color Marker set by a Meter.
> >
> > - ``color``: Metering color marker.
> >
> > +Item: ``IPV6_ROUTING_EXT``
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Matches IPv6 routing extension header.
> > +
> > +- ``next_hdr``: Next layer header type.
> > +- ``type``: IPv6 routing extension header type.
> > +- ``segments_left``: How many IPv6 destination addresses carries on.
> > +
> > Actions
> > ~~~~~~~
> >
> > diff --git a/doc/guides/rel_notes/release_23_03.rst
> > b/doc/guides/rel_notes/release_23_03.rst
> > index c15f6fbb9f..1337da73b8 100644
> > --- a/doc/guides/rel_notes/release_23_03.rst
> > +++ b/doc/guides/rel_notes/release_23_03.rst
> > @@ -69,6 +69,11 @@ New Features
> > ``rte_event_dev_config::nb_single_link_event_port_queues`` parameter
> > required for eth_rx, eth_tx, crypto and timer eventdev adapters.
> >
> > +* **Added rte_flow support for matching IPv6 routing extension header
> > +fields.**
> > +
> > + Added ``ipv6_routing_ext`` items in rte_flow to match IPv6 routing
> > + extension header.
> > +
> >
> > Removed Items
> > -------------
> > @@ -98,6 +103,10 @@ API Changes
> > Also, make sure to start the actual text at the margin.
> > =======================================================
> >
> > +* net: added a new structure:
> > +
> > + - IPv6 routing extension header ``rte_ipv6_routing_ext``.
> > +
> >
> > ABI Changes
> > -----------
> > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index
> > 7d0c24366c..833382c466 100644
> > --- a/lib/ethdev/rte_flow.c
> > +++ b/lib/ethdev/rte_flow.c
> > @@ -76,6 +76,23 @@ rte_flow_item_flex_conv(void *buf, const void *data)
> > return src->length;
> > }
> >
> > +static size_t
> > +rte_flow_item_ipv6_routing_ext_conv(void *buf, const void *data) {
> > + struct rte_flow_item_ipv6_routing_ext *dst = buf;
> > + const struct rte_flow_item_ipv6_routing_ext *src = data;
> > + size_t len;
> > +
> > + if (src->hdr.hdr_len)
>
> Please, compare vs 0
>
> > + len = src->hdr.hdr_len << 3;
> > + else
> > + len = src->hdr.segments_left << 4;
> > + if (dst == NULL)
> > + return 0;
> > + memcpy(dst->segments, src->segments, len);
> > + return len;
> > +}
> > +
> > /** Generate flow_item[] entry. */
> > #define MK_FLOW_ITEM(t, s) \
> > [RTE_FLOW_ITEM_TYPE_ ## t] = { \ @@ -157,6 +174,8 @@ static
> > const struct rte_flow_desc_data rte_flow_desc_item[] = {
> > MK_FLOW_ITEM(L2TPV2, sizeof(struct rte_flow_item_l2tpv2)),
> > MK_FLOW_ITEM(PPP, sizeof(struct rte_flow_item_ppp)),
> > MK_FLOW_ITEM(METER_COLOR, sizeof(struct
> > rte_flow_item_meter_color)),
> > + MK_FLOW_ITEM_FN(IPV6_ROUTING_EXT, sizeof(struct
> rte_flow_item_ipv6_routing_ext),
> > + rte_flow_item_ipv6_routing_ext_conv),
> > };
> >
> > /** Generate flow_action[] entry. */ diff --git
> > a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
> > b60987db4b..ff9270690c 100644
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -624,6 +624,13 @@ enum rte_flow_item_type {
> > * See struct rte_flow_item_meter_color.
> > */
> > RTE_FLOW_ITEM_TYPE_METER_COLOR,
> > +
> > + /**
> > + * Matches the presence of IPv6 routing extension header.
> > + *
> > + * @see struct rte_flow_item_ipv6_routing_ext.
> > + */
> > + RTE_FLOW_ITEM_TYPE_IPV6_ROUTING_EXT,
> > };
> >
> > /**
> > @@ -873,6 +880,20 @@ struct rte_flow_item_ipv6 {
> > uint32_t reserved:23;
> > };
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this structure may change without prior notice
> > + *
> > + * RTE_FLOW_ITEM_TYPE_IPV6_ROUTING_EXT.
> > + *
> > + * Matches an IPv6 routing extension header.
> > + */
> > +struct rte_flow_item_ipv6_routing_ext {
> > + struct rte_ipv6_routing_ext hdr;
> > + __extension__
> > + rte_be32_t segments[]; /**< Each hop IPv6 address. */
>
> Do we really need it? Are you going to support it?
> Will testpmd and flow conf work correctly since it uses size of the structure?
>
In the rte_flow.c and testpmd raw_encap() function, it relies on the hdr_len or
segment_left fild not sizeof this structure.
> IMHO we should just remove it right now if we're not going to support it.
>
In matching, we don't support segments field. I am ok to remove it.
> > +};
> > +
> > /** Default mask for RTE_FLOW_ITEM_TYPE_IPV6. */
> > #ifndef __cplusplus
> > static const struct rte_flow_item_ipv6 rte_flow_item_ipv6_mask = {
> > diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h index
> > 9c8e8206f0..778fb5ef83 100644
> > --- a/lib/net/rte_ip.h
> > +++ b/lib/net/rte_ip.h
> > @@ -539,6 +539,25 @@ struct rte_ipv6_hdr {
> > uint8_t dst_addr[16]; /**< IP address of destination host(s). */
> > } __rte_packed;
> >
> > +/**
> > + * IPv6 Routing Extension Header
> > + */
> > +struct rte_ipv6_routing_ext {
> > + uint8_t next_hdr; /**< Protocol, next header. */
> > + uint8_t hdr_len; /**< Header length. */
> > + uint8_t type; /**< Extension header type. */
> > + uint8_t segments_left; /**< Valid segments number. */
> > + __extension__
> > + union {
> > + rte_be32_t flags;
>
> flags should be documented as well.
Sure.
>
> > + struct {
> > + uint8_t last_entry; /**< The last_entry field of
> > SRH */
> > + uint8_t flag; /**< Packet flag. */
> > + rte_be16_t tag; /**< Packet tag. */
> > + };
> > + };
>
> May be we should add a comment here that segments follow?
Sure.
>
> > +} __rte_packed;
> > +
> > /* IPv6 vtc_flow: IPv / TC / flow_label */
> > #define RTE_IPV6_HDR_FL_SHIFT 0
> > #define RTE_IPV6_HDR_TC_SHIFT 20
