diff mbox series

[RFC,V1,1/1] net: extend VXLAN header to support more extensions

Message ID 20240130112520.1971315-2-gavinl@nvidia.com (mailing list archive)
State New
Delegated to: Ferruh Yigit
Headers
Series net: extend VXLAN header to support more extensions |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional fail Functional issues

Commit Message

Gavin Li Jan. 30, 2024, 11:25 a.m. UTC 
  Currently, DPDK supports VXLAN and VXLAN-GPE with similar header
structures and we are working on adding support for VXLAN-GBP which is
another extension to VXLAN. More extension of VXLAN may be added in the
future.

VXLAN and VXLAN-GBP use the same UDP port(4789) while VXLAN-GPE uses a
different one, 4790. The three protocols have the same header length and
overall similar header structure as below.
    0                   1                   2                   3
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |R|R|R|R|I|R|R|R|            Reserved                           |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |                VXLAN Network Identifier (VNI) |   Reserved    |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

                           Figure 1: VXLAN Header

    0                   1                   2                   3
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |R|R|Ver|I|P|B|O|       Reserved                |Next Protocol  |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |                VXLAN Network Identifier (VNI) |   Reserved    |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

                         Figure 2: VXLAN-GPE Header

    0                   1                   2                   3
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |G|R|R|R|I|R|R|R|R|D|R|R|A|R|R|R|        Group Policy ID        |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |          VXLAN Network Identifier (VNI)       |   Reserved    |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

                          Figure 3: VXLAN-GBP Extension

Both VXLAN-GPE and VXLAN-GBP extended VXLAN by redefining its reserved
bits, which means the packets can be processed with same pattern and most
of the code can be reused. Instead of adding more new items by
copying/pasting code for the VXLAN extensions in the future, it’s better
to use existing VXLAN infrastructure and add support code in it.

In this patch, all the VXLAN extension header will be merged with VXLAN as
union if the overlapped field has different format among protocols. The
existing VXLAN-GPE will be marked as deprecated and new extensions of
VXLAN should be added to VXLAN instead of a new RTE item.

Signed-off-by: Gavin Li <gavinl@nvidia.com>
---
 doc/guides/rel_notes/deprecation.rst |  5 +++
 lib/ethdev/rte_flow.h                | 13 +++++-
 lib/net/rte_vxlan.h                  | 67 ++++++++++++++++++++++++++--
 3 files changed, 80 insertions(+), 5 deletions(-)

Comments

Thomas Monjalon Feb. 6, 2024, 10:51 p.m. UTC | #1 
30/01/2024 12:25, Gavin Li:
> In this patch, all the VXLAN extension header will be merged with VXLAN as
> union if the overlapped field has different format among protocols. The
> existing VXLAN-GPE will be marked as deprecated and new extensions of
> VXLAN should be added to VXLAN instead of a new RTE item.

So VXLAN GPE, GBP, and original ones will all use the same struct.
Asking confirmation to other reviewers:
- do we want to deprecate specific VXLAN GPE?
- do we want to plan for VXLAN GPE removal?

[...]
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> +* ethdev: The flow item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE`` is replaced with ``RTE_FLOW_ITEM_TYPE_VXLAN``.
> +  The item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE``, the struct ``rte_flow_item_vxlan_gpe``, its mask ``rte_flow_item_vxlan_gpe_mask``,
> +  and the header struct ``rte_vxlan_gpe_hdr`` with the macro ``RTE_ETHER_VXLAN_GPE_HLEN``
> +  will be removed in DPDK 25.11.

[...]
> @@ -38,8 +38,65 @@ struct rte_vxlan_hdr {
>  			rte_be32_t vx_vni;   /**< VNI (24) + Reserved (8). */
>  		};
>  		struct {
> -			uint8_t    flags;    /**< Should be 8 (I flag). */
> -			uint8_t    rsvd0[3]; /**< Reserved. */
> +			union {
> +				uint8_t    flags;    /**< Should be 8 (I flag). */
> +				/* Flag bits defined by GPE */
> +				struct {
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +					uint8_t flag_o:1,
> +						flag_b:1,
> +						flag_p:1,
> +						flag_i_gpe:1,
> +						flag_ver:2,
> +						rsvd_gpe:2;
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +					uint8_t rsvd_gpe:2,
> +						flag_ver:2,
> +						flag_i_gpe:1,
> +						flag_p:1,
> +						flag_b:1,
> +						flag_o:1;
> +#endif
> +				};
> +				/* Flag bits defined by GBP */
> +				struct {
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +					uint8_t rsvd_gbp1:3,
> +						flag_i_gbp:1,
> +						rsvd_gbp2:3,
> +						flag_g:1;
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +					uint8_t flag_g:1,
> +						rsvd_gbp1:3,
> +						flag_i_gbp:1,
> +						rsvd_gbp2:3;
> +#endif
> +				};
> +			};
> +			union {
> +				uint8_t    rsvd0[3]; /**< Reserved. */
> +				/* Overlap with rte_vxlan_gpe_hdr which is deprecated.*/
> +				struct {
> +					uint8_t rsvd0_gpe[2]; /**< Reserved. */
> +					uint8_t proto;	   /**< Next protocol. */
> +				};
> +				struct {
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +					uint8_t rsvd0_gbp1:3,
> +						policy_applied:1,
> +						rsvd0_gbp2:2,
> +						dont_learn:1,
> +						rsvd0_gbp3:1;
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +					uint8_t rsvd0_gbp1:1,
> +						dont_learn:1,
> +						rsvd0_gbp2:2,
> +						policy_applied:1,
> +						rsvd0_gbp3:3;
> +#endif
> +					uint16_t policy_id;
> +				};
> +			};
>  			uint8_t    vni[3];   /**< VXLAN identifier. */
>  			uint8_t    rsvd1;    /**< Reserved. */
>  		};

Naming looks OK.
Any different opinion?
Ajit Khaparde Feb. 7, 2024, 4:49 a.m. UTC | #2 
On Tue, Feb 6, 2024 at 2:51 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 30/01/2024 12:25, Gavin Li:
> > In this patch, all the VXLAN extension header will be merged with VXLAN as
> > union if the overlapped field has different format among protocols. The
> > existing VXLAN-GPE will be marked as deprecated and new extensions of
> > VXLAN should be added to VXLAN instead of a new RTE item.
>
> So VXLAN GPE, GBP, and original ones will all use the same struct.
> Asking confirmation to other reviewers:
> - do we want to deprecate specific VXLAN GPE?
For start, yes.

> - do we want to plan for VXLAN GPE removal?
Once deprecated, we can remove it.

>
> [...]
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > +* ethdev: The flow item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE`` is replaced with ``RTE_FLOW_ITEM_TYPE_VXLAN``.
> > +  The item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE``, the struct ``rte_flow_item_vxlan_gpe``, its mask ``rte_flow_item_vxlan_gpe_mask``,
> > +  and the header struct ``rte_vxlan_gpe_hdr`` with the macro ``RTE_ETHER_VXLAN_GPE_HLEN``
> > +  will be removed in DPDK 25.11.
>
> [...]
> > @@ -38,8 +38,65 @@ struct rte_vxlan_hdr {
> >                       rte_be32_t vx_vni;   /**< VNI (24) + Reserved (8). */
> >               };
> >               struct {
> > -                     uint8_t    flags;    /**< Should be 8 (I flag). */
> > -                     uint8_t    rsvd0[3]; /**< Reserved. */
> > +                     union {
> > +                             uint8_t    flags;    /**< Should be 8 (I flag). */
> > +                             /* Flag bits defined by GPE */
> > +                             struct {
> > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > +                                     uint8_t flag_o:1,
> > +                                             flag_b:1,
> > +                                             flag_p:1,
> > +                                             flag_i_gpe:1,
> > +                                             flag_ver:2,
> > +                                             rsvd_gpe:2;
> > +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> > +                                     uint8_t rsvd_gpe:2,
> > +                                             flag_ver:2,
> > +                                             flag_i_gpe:1,
> > +                                             flag_p:1,
> > +                                             flag_b:1,
> > +                                             flag_o:1;
> > +#endif
> > +                             };
> > +                             /* Flag bits defined by GBP */
> > +                             struct {
> > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > +                                     uint8_t rsvd_gbp1:3,
> > +                                             flag_i_gbp:1,
> > +                                             rsvd_gbp2:3,
> > +                                             flag_g:1;
> > +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> > +                                     uint8_t flag_g:1,
> > +                                             rsvd_gbp1:3,
> > +                                             flag_i_gbp:1,
> > +                                             rsvd_gbp2:3;
> > +#endif
> > +                             };
> > +                     };
> > +                     union {
> > +                             uint8_t    rsvd0[3]; /**< Reserved. */
> > +                             /* Overlap with rte_vxlan_gpe_hdr which is deprecated.*/
> > +                             struct {
> > +                                     uint8_t rsvd0_gpe[2]; /**< Reserved. */
> > +                                     uint8_t proto;     /**< Next protocol. */
> > +                             };
> > +                             struct {
> > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > +                                     uint8_t rsvd0_gbp1:3,
> > +                                             policy_applied:1,
> > +                                             rsvd0_gbp2:2,
> > +                                             dont_learn:1,
> > +                                             rsvd0_gbp3:1;
> > +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> > +                                     uint8_t rsvd0_gbp1:1,
> > +                                             dont_learn:1,
> > +                                             rsvd0_gbp2:2,
> > +                                             policy_applied:1,
> > +                                             rsvd0_gbp3:3;
> > +#endif
> > +                                     uint16_t policy_id;
> > +                             };
> > +                     };
> >                       uint8_t    vni[3];   /**< VXLAN identifier. */
> >                       uint8_t    rsvd1;    /**< Reserved. */
> >               };
>
> Naming looks OK.
+1

> Any different opinion?
>
>
Ferruh Yigit Feb. 8, 2024, 11:54 p.m. UTC | #3 
On 1/30/2024 11:25 AM, Gavin Li wrote:
> Currently, DPDK supports VXLAN and VXLAN-GPE with similar header
> structures and we are working on adding support for VXLAN-GBP which is
> another extension to VXLAN. More extension of VXLAN may be added in the
> future.
> 
> VXLAN and VXLAN-GBP use the same UDP port(4789) while VXLAN-GPE uses a
> different one, 4790. The three protocols have the same header length and
> overall similar header structure as below.
>     0                   1                   2                   3
>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |R|R|R|R|I|R|R|R|            Reserved                           |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |                VXLAN Network Identifier (VNI) |   Reserved    |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
>                            Figure 1: VXLAN Header
> 
>     0                   1                   2                   3
>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |R|R|Ver|I|P|B|O|       Reserved                |Next Protocol  |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |                VXLAN Network Identifier (VNI) |   Reserved    |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
>                          Figure 2: VXLAN-GPE Header
> 
>     0                   1                   2                   3
>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |G|R|R|R|I|R|R|R|R|D|R|R|A|R|R|R|        Group Policy ID        |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |          VXLAN Network Identifier (VNI)       |   Reserved    |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
>                           Figure 3: VXLAN-GBP Extension
> 
> Both VXLAN-GPE and VXLAN-GBP extended VXLAN by redefining its reserved
> bits, which means the packets can be processed with same pattern and most
> of the code can be reused. Instead of adding more new items by
> copying/pasting code for the VXLAN extensions in the future, it’s better
> to use existing VXLAN infrastructure and add support code in it.
> 

Hi Gavin,

The motivation is to prevent code duplication, and the code mentioned is
 the driver code, right?

Overall OK to unify "struct rte_vxlan_hdr" although it makes the struct
a little complex, perhaps we can consider extraction some nested structs
as named struct, no strong opinion.


But not sure about removing the flow item types for VXLAN-GPE, or not
adding for VXLAN-GBP.

Think about a case user adding a rule, which has a item type as VXLAN
and in the protocol header some bits are set, lets say first word, last
byte is set, how driver will know if to take it as GPE "next protocol"
or "group policy id".

And if a specific HW detects VXLAN-GPE and VXLAN-GBP protocols as two
separate protocols, won't only having capability to describe VXLAN
protocol in SW be a limitation.

If the target is to remove code duplication in the driver, can it be
accomplished by extracting code that parse the common fields of these
protocols?


> In this patch, all the VXLAN extension header will be merged with VXLAN as
> union if the overlapped field has different format among protocols. The
> existing VXLAN-GPE will be marked as deprecated and new extensions of
> VXLAN should be added to VXLAN instead of a new RTE item.
> 
> Signed-off-by: Gavin Li <gavinl@nvidia.com>
> ---
>  doc/guides/rel_notes/deprecation.rst |  5 +++
>  lib/ethdev/rte_flow.h                | 13 +++++-
>  lib/net/rte_vxlan.h                  | 67 ++++++++++++++++++++++++++--
>  3 files changed, 80 insertions(+), 5 deletions(-)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 81b93515cb..f9cf931b77 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -95,6 +95,11 @@ Deprecation Notices
>    - ``rte_flow_item_pppoe``
>    - ``rte_flow_item_pppoe_proto_id``
>  
> +* ethdev: The flow item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE`` is replaced with ``RTE_FLOW_ITEM_TYPE_VXLAN``.
> +  The item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE``, the struct ``rte_flow_item_vxlan_gpe``, its mask ``rte_flow_item_vxlan_gpe_mask``,
> +  and the header struct ``rte_vxlan_gpe_hdr`` with the macro ``RTE_ETHER_VXLAN_GPE_HLEN``
> +  will be removed in DPDK 25.11.
> +
>

We have 24.11 in between.


>  * ethdev: Queue specific stats fields will be removed from ``struct rte_eth_stats``.
>    Mentioned fields are: ``q_ipackets``, ``q_opackets``, ``q_ibytes``, ``q_obytes``,
>    ``q_errors``.
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index 1267c146e5..a9943106d9 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -355,6 +355,7 @@ enum rte_flow_item_type {
>  	RTE_FLOW_ITEM_TYPE_GENEVE,
>  
>  	/**
> +	 * @deprecated Replaced with ``RTE_FLOW_ITEM_TYPE_VXLAN``.
>  	 * Matches a VXLAN-GPE header.
>  	 *
>  	 * See struct rte_flow_item_vxlan_gpe.
> @@ -1096,7 +1097,11 @@ static const struct rte_flow_item_sctp rte_flow_item_sctp_mask = {
>  /**
>   * RTE_FLOW_ITEM_TYPE_VXLAN.
>   *
> - * Matches a VXLAN header (RFC 7348).
> + * Matches a VXLAN header (RFC 7348), including GPE (draft-ietf-nvo3-vxlan-gpe-13.txt)
> + * and GBP (draft-smith-vxlan-group-policy-05.txt).
> + *
> + * GPE is distinguished with its UDP port.
> + * UDP port may be specified with ``rte_eth_dev_udp_tunnel_port_add()``.
>   */
>  struct rte_flow_item_vxlan {
>  	union {
> @@ -1339,6 +1344,7 @@ static const struct rte_flow_item_geneve rte_flow_item_geneve_mask = {
>  #endif
>  
>  /**
> + * @deprecated Replaced with ``rte_flow_item_vxlan``.
>   * RTE_FLOW_ITEM_TYPE_VXLAN_GPE (draft-ietf-nvo3-vxlan-gpe-05).
>   *
>   * Matches a VXLAN-GPE header.
> @@ -1360,7 +1366,10 @@ struct rte_flow_item_vxlan_gpe {
>  	};
>  };
>  
> -/** Default mask for RTE_FLOW_ITEM_TYPE_VXLAN_GPE. */
> +/**
> + * @deprecated Replaced with ``rte_flow_item_vxlan_mask``.
> + * Default mask for RTE_FLOW_ITEM_TYPE_VXLAN_GPE.
> + */
>  #ifndef __cplusplus
>  static const struct rte_flow_item_vxlan_gpe rte_flow_item_vxlan_gpe_mask = {
>  	.hdr.vni = "\xff\xff\xff",
> diff --git a/lib/net/rte_vxlan.h b/lib/net/rte_vxlan.h
> index 997fc784fc..255c30f71d 100644
> --- a/lib/net/rte_vxlan.h
> +++ b/lib/net/rte_vxlan.h
> @@ -38,8 +38,65 @@ struct rte_vxlan_hdr {
>  			rte_be32_t vx_vni;   /**< VNI (24) + Reserved (8). */
>  		};
>  		struct {
> -			uint8_t    flags;    /**< Should be 8 (I flag). */
> -			uint8_t    rsvd0[3]; /**< Reserved. */
> +			union {
> +				uint8_t    flags;    /**< Should be 8 (I flag). */
> +				/* Flag bits defined by GPE */
> +				struct {
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +					uint8_t flag_o:1,
> +						flag_b:1,
> +						flag_p:1,
> +						flag_i_gpe:1,
> +						flag_ver:2,
> +						rsvd_gpe:2;
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +					uint8_t rsvd_gpe:2,
> +						flag_ver:2,
> +						flag_i_gpe:1,
> +						flag_p:1,
> +						flag_b:1,
> +						flag_o:1;
> +#endif
> +				};
> +				/* Flag bits defined by GBP */
> +				struct {
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +					uint8_t rsvd_gbp1:3,
> +						flag_i_gbp:1,
> +						rsvd_gbp2:3,
> +						flag_g:1;
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +					uint8_t flag_g:1,
> +						rsvd_gbp1:3,
> +						flag_i_gbp:1,
> +						rsvd_gbp2:3;
> +#endif
> +				};
> +			};
> +			union {
> +				uint8_t    rsvd0[3]; /**< Reserved. */
> +				/* Overlap with rte_vxlan_gpe_hdr which is deprecated.*/
> +				struct {
> +					uint8_t rsvd0_gpe[2]; /**< Reserved. */
> +					uint8_t proto;	   /**< Next protocol. */
> +				};
> +				struct {
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +					uint8_t rsvd0_gbp1:3,
> +						policy_applied:1,
> +						rsvd0_gbp2:2,
> +						dont_learn:1,
> +						rsvd0_gbp3:1;
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +					uint8_t rsvd0_gbp1:1,
> +						dont_learn:1,
> +						rsvd0_gbp2:2,
> +						policy_applied:1,
> +						rsvd0_gbp3:3;
> +#endif
> +					uint16_t policy_id;
> +				};
> +			};
>  			uint8_t    vni[3];   /**< VXLAN identifier. */
>  			uint8_t    rsvd1;    /**< Reserved. */
>  		};
> @@ -52,6 +109,7 @@ struct rte_vxlan_hdr {
>  
>  
>  /**
> + * @deprecated Replaced with ``rte_vxlan_hdr``.
>   * VXLAN-GPE protocol header (draft-ietf-nvo3-vxlan-gpe-05).
>   * Contains the 8-bit flag, 8-bit next-protocol, 24-bit VXLAN Network
>   * Identifier and Reserved fields (16 bits and 8 bits).
> @@ -75,7 +133,10 @@ struct rte_vxlan_gpe_hdr {
>  	};
>  } __rte_packed;
>  
> -/** VXLAN-GPE tunnel header length. */
> +/**
> + * @deprecated Replaced with ``RTE_ETHER_VXLAN_HLEN``.
> + * VXLAN-GPE tunnel header length.
> + */
>  #define RTE_ETHER_VXLAN_GPE_HLEN (sizeof(struct rte_udp_hdr) + \
>  		sizeof(struct rte_vxlan_gpe_hdr))
>
Thomas Monjalon Feb. 9, 2024, 10:12 a.m. UTC | #4 
09/02/2024 00:54, Ferruh Yigit:
> On 1/30/2024 11:25 AM, Gavin Li wrote:
> > Currently, DPDK supports VXLAN and VXLAN-GPE with similar header
> > structures and we are working on adding support for VXLAN-GBP which is
> > another extension to VXLAN. More extension of VXLAN may be added in the
> > future.
> > 
> > VXLAN and VXLAN-GBP use the same UDP port(4789) while VXLAN-GPE uses a
> > different one, 4790. The three protocols have the same header length and
> > overall similar header structure as below.
> >     0                   1                   2                   3
> >     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> >    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >    |R|R|R|R|I|R|R|R|            Reserved                           |
> >    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >    |                VXLAN Network Identifier (VNI) |   Reserved    |
> >    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> > 
> >                            Figure 1: VXLAN Header
> > 
> >     0                   1                   2                   3
> >     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> >    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >    |R|R|Ver|I|P|B|O|       Reserved                |Next Protocol  |
> >    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >    |                VXLAN Network Identifier (VNI) |   Reserved    |
> >    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> > 
> >                          Figure 2: VXLAN-GPE Header
> > 
> >     0                   1                   2                   3
> >     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> >    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >    |G|R|R|R|I|R|R|R|R|D|R|R|A|R|R|R|        Group Policy ID        |
> >    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >    |          VXLAN Network Identifier (VNI)       |   Reserved    |
> >    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> > 
> >                           Figure 3: VXLAN-GBP Extension
> > 
> > Both VXLAN-GPE and VXLAN-GBP extended VXLAN by redefining its reserved
> > bits, which means the packets can be processed with same pattern and most
> > of the code can be reused. Instead of adding more new items by
> > copying/pasting code for the VXLAN extensions in the future, it’s better
> > to use existing VXLAN infrastructure and add support code in it.
> > 
> 
> Hi Gavin,
> 
> The motivation is to prevent code duplication, and the code mentioned is
>  the driver code, right?

The motivation is mainly to provide a unified and more explicit API.

> Overall OK to unify "struct rte_vxlan_hdr" although it makes the struct
> a little complex, perhaps we can consider extraction some nested structs
> as named struct, no strong opinion.
> 
> 
> But not sure about removing the flow item types for VXLAN-GPE, or not
> adding for VXLAN-GBP.
> 
> Think about a case user adding a rule, which has a item type as VXLAN
> and in the protocol header some bits are set, lets say first word, last
> byte is set, how driver will know if to take it as GPE "next protocol"
> or "group policy id".

The driver may decide depending on the UDP port and some distinguishing flags.
If you want to match on GBP, you should includes the GBP flag in your pattern,
no need to use a separate item.

> And if a specific HW detects VXLAN-GPE and VXLAN-GBP protocols as two
> separate protocols, won't only having capability to describe VXLAN
> protocol in SW be a limitation.

I think the driver may know based on the flow rule.

That's a good question about the item granularity.
What is the separation between a different protocol and protocol options?
How flow item should match protocol variations?
My current thinking is that we should minimize the number of items.

> If the target is to remove code duplication in the driver, can it be
> accomplished by extracting code that parse the common fields of these
> protocols?

Driver code is not a concern as far as any driver can implement the API.


> > In this patch, all the VXLAN extension header will be merged with VXLAN as
> > union if the overlapped field has different format among protocols. The
> > existing VXLAN-GPE will be marked as deprecated and new extensions of
> > VXLAN should be added to VXLAN instead of a new RTE item.
> > 
> > Signed-off-by: Gavin Li <gavinl@nvidia.com>
> > ---
> >  doc/guides/rel_notes/deprecation.rst |  5 +++
> >  lib/ethdev/rte_flow.h                | 13 +++++-
> >  lib/net/rte_vxlan.h                  | 67 ++++++++++++++++++++++++++--
> >  3 files changed, 80 insertions(+), 5 deletions(-)
> > 
> > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> > index 81b93515cb..f9cf931b77 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -95,6 +95,11 @@ Deprecation Notices
> >    - ``rte_flow_item_pppoe``
> >    - ``rte_flow_item_pppoe_proto_id``
> >  
> > +* ethdev: The flow item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE`` is replaced with ``RTE_FLOW_ITEM_TYPE_VXLAN``.
> > +  The item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE``, the struct ``rte_flow_item_vxlan_gpe``, its mask ``rte_flow_item_vxlan_gpe_mask``,
> > +  and the header struct ``rte_vxlan_gpe_hdr`` with the macro ``RTE_ETHER_VXLAN_GPE_HLEN``
> > +  will be removed in DPDK 25.11.
> > +
> >
> 
> We have 24.11 in between.

Isn't it too soon?
Should we remove at all?
Ferruh Yigit Feb. 9, 2024, 12:11 p.m. UTC | #5 
On 2/9/2024 10:12 AM, Thomas Monjalon wrote:
> 09/02/2024 00:54, Ferruh Yigit:
>> On 1/30/2024 11:25 AM, Gavin Li wrote:
>>> Currently, DPDK supports VXLAN and VXLAN-GPE with similar header
>>> structures and we are working on adding support for VXLAN-GBP which is
>>> another extension to VXLAN. More extension of VXLAN may be added in the
>>> future.
>>>
>>> VXLAN and VXLAN-GBP use the same UDP port(4789) while VXLAN-GPE uses a
>>> different one, 4790. The three protocols have the same header length and
>>> overall similar header structure as below.
>>>     0                   1                   2                   3
>>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>    |R|R|R|R|I|R|R|R|            Reserved                           |
>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>    |                VXLAN Network Identifier (VNI) |   Reserved    |
>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>
>>>                            Figure 1: VXLAN Header
>>>
>>>     0                   1                   2                   3
>>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>    |R|R|Ver|I|P|B|O|       Reserved                |Next Protocol  |
>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>    |                VXLAN Network Identifier (VNI) |   Reserved    |
>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>
>>>                          Figure 2: VXLAN-GPE Header
>>>
>>>     0                   1                   2                   3
>>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>    |G|R|R|R|I|R|R|R|R|D|R|R|A|R|R|R|        Group Policy ID        |
>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>    |          VXLAN Network Identifier (VNI)       |   Reserved    |
>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>
>>>                           Figure 3: VXLAN-GBP Extension
>>>
>>> Both VXLAN-GPE and VXLAN-GBP extended VXLAN by redefining its reserved
>>> bits, which means the packets can be processed with same pattern and most
>>> of the code can be reused. Instead of adding more new items by
>>> copying/pasting code for the VXLAN extensions in the future, it’s better
>>> to use existing VXLAN infrastructure and add support code in it.
>>>
>>
>> Hi Gavin,
>>
>> The motivation is to prevent code duplication, and the code mentioned is
>>  the driver code, right?
> 
> The motivation is mainly to provide a unified and more explicit API.
> 

From user perspective, I think existing approach is more explicit,
because it sets VXLAN or VXLAN_GPE flow types.

I am trying to understand the benefit, how unifying flow type in the API
helps to the user?

>> Overall OK to unify "struct rte_vxlan_hdr" although it makes the struct
>> a little complex, perhaps we can consider extraction some nested structs
>> as named struct, no strong opinion.
>>
>>
>> But not sure about removing the flow item types for VXLAN-GPE, or not
>> adding for VXLAN-GBP.
>>
>> Think about a case user adding a rule, which has a item type as VXLAN
>> and in the protocol header some bits are set, lets say first word, last
>> byte is set, how driver will know if to take it as GPE "next protocol"
>> or "group policy id".
> 
> The driver may decide depending on the UDP port and some distinguishing flags.
> If you want to match on GBP, you should includes the GBP flag in your pattern,
> no need to use a separate item.
> 

Why not be more explicit?
It helps to driver to know more about the pattern to be able to create
proper flow rule, if there is an obvious way for driver to differentiate
these protocol extensions, and flow item type is redundant, I can
understand the proposal, but otherwise I think better to keep flow items
for extensions.

When a rule is set in HW, HW may not care about the protocol, as long as
bits in the rule match with the packet, HW can apply the action.
But for driver to be able to set the rule properly, it needs more
explicit information.


Lets assume driver API get a pattern with 'RTE_FLOW_ITEM_TYPE_VXLAN'
type and "struct rte_flow_item_vxlan", at this point driver doesn't know
if it is VXLAN or any of the extensions.
Should driver go and check GBP flags to deduce if it is GBP flag, and
what if they are all zero, how can driver can tell if this is GBP flag
that is zero or is it VXLAN header and bit is reserved.


Or I will just make up a sample, it may not be accurate but please take
it as a case to prove a point.
Lets assume driver API again get "struct rte_flow_item_vxlan" whose
first word's last bit is set, which union in the struct will driver
check to detect this, GPE one or GBP one?
This can be GPE-"next protocol" or GBP-"Group policy id", if driver
knows this it can set the mask better for the field in the rule.
Driver may try to deduce this extension information from other
information, but why not proper specific flow type for each extension
instead?



>> And if a specific HW detects VXLAN-GPE and VXLAN-GBP protocols as two
>> separate protocols, won't only having capability to describe VXLAN
>> protocol in SW be a limitation.
> 
> I think the driver may know based on the flow rule.
> 
> That's a good question about the item granularity.
> What is the separation between a different protocol and protocol options?
> How flow item should match protocol variations?
> My current thinking is that we should minimize the number of items.
> 
>> If the target is to remove code duplication in the driver, can it be
>> accomplished by extracting code that parse the common fields of these
>> protocols?
> 
> Driver code is not a concern as far as any driver can implement the API.
> 

Got it, I want to clarify this one.
Where the code duplication via copy/paste that commit log mention occurs?

> 
>>> In this patch, all the VXLAN extension header will be merged with VXLAN as
>>> union if the overlapped field has different format among protocols. The
>>> existing VXLAN-GPE will be marked as deprecated and new extensions of
>>> VXLAN should be added to VXLAN instead of a new RTE item.
>>>
>>> Signed-off-by: Gavin Li <gavinl@nvidia.com>
>>> ---
>>>  doc/guides/rel_notes/deprecation.rst |  5 +++
>>>  lib/ethdev/rte_flow.h                | 13 +++++-
>>>  lib/net/rte_vxlan.h                  | 67 ++++++++++++++++++++++++++--
>>>  3 files changed, 80 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
>>> index 81b93515cb..f9cf931b77 100644
>>> --- a/doc/guides/rel_notes/deprecation.rst
>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>> @@ -95,6 +95,11 @@ Deprecation Notices
>>>    - ``rte_flow_item_pppoe``
>>>    - ``rte_flow_item_pppoe_proto_id``
>>>  
>>> +* ethdev: The flow item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE`` is replaced with ``RTE_FLOW_ITEM_TYPE_VXLAN``.
>>> +  The item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE``, the struct ``rte_flow_item_vxlan_gpe``, its mask ``rte_flow_item_vxlan_gpe_mask``,
>>> +  and the header struct ``rte_vxlan_gpe_hdr`` with the macro ``RTE_ETHER_VXLAN_GPE_HLEN``
>>> +  will be removed in DPDK 25.11.
>>> +
>>>
>>
>> We have 24.11 in between.
> 
> Isn't it too soon?
> Should we remove at all?
>
Thomas Monjalon Feb. 9, 2024, 1:44 p.m. UTC | #6 
09/02/2024 13:11, Ferruh Yigit:
> On 2/9/2024 10:12 AM, Thomas Monjalon wrote:
> > 09/02/2024 00:54, Ferruh Yigit:
> >> On 1/30/2024 11:25 AM, Gavin Li wrote:
> >>> Currently, DPDK supports VXLAN and VXLAN-GPE with similar header
> >>> structures and we are working on adding support for VXLAN-GBP which is
> >>> another extension to VXLAN. More extension of VXLAN may be added in the
> >>> future.
> >>>
> >>> VXLAN and VXLAN-GBP use the same UDP port(4789) while VXLAN-GPE uses a
> >>> different one, 4790. The three protocols have the same header length and
> >>> overall similar header structure as below.
> >>>     0                   1                   2                   3
> >>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> >>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>    |R|R|R|R|I|R|R|R|            Reserved                           |
> >>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>    |                VXLAN Network Identifier (VNI) |   Reserved    |
> >>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>
> >>>                            Figure 1: VXLAN Header
> >>>
> >>>     0                   1                   2                   3
> >>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> >>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>    |R|R|Ver|I|P|B|O|       Reserved                |Next Protocol  |
> >>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>    |                VXLAN Network Identifier (VNI) |   Reserved    |
> >>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>
> >>>                          Figure 2: VXLAN-GPE Header
> >>>
> >>>     0                   1                   2                   3
> >>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> >>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>    |G|R|R|R|I|R|R|R|R|D|R|R|A|R|R|R|        Group Policy ID        |
> >>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>    |          VXLAN Network Identifier (VNI)       |   Reserved    |
> >>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>
> >>>                           Figure 3: VXLAN-GBP Extension
> >>>
> >>> Both VXLAN-GPE and VXLAN-GBP extended VXLAN by redefining its reserved
> >>> bits, which means the packets can be processed with same pattern and most
> >>> of the code can be reused. Instead of adding more new items by
> >>> copying/pasting code for the VXLAN extensions in the future, it’s better
> >>> to use existing VXLAN infrastructure and add support code in it.
> >>>
> >>
> >> Hi Gavin,
> >>
> >> The motivation is to prevent code duplication, and the code mentioned is
> >>  the driver code, right?
> > 
> > The motivation is mainly to provide a unified and more explicit API.
> > 
> 
> From user perspective, I think existing approach is more explicit,
> because it sets VXLAN or VXLAN_GPE flow types.
> 
> I am trying to understand the benefit, how unifying flow type in the API
> helps to the user?
> 
> >> Overall OK to unify "struct rte_vxlan_hdr" although it makes the struct
> >> a little complex, perhaps we can consider extraction some nested structs
> >> as named struct, no strong opinion.
> >>
> >>
> >> But not sure about removing the flow item types for VXLAN-GPE, or not
> >> adding for VXLAN-GBP.
> >>
> >> Think about a case user adding a rule, which has a item type as VXLAN
> >> and in the protocol header some bits are set, lets say first word, last
> >> byte is set, how driver will know if to take it as GPE "next protocol"
> >> or "group policy id".
> > 
> > The driver may decide depending on the UDP port and some distinguishing flags.
> > If you want to match on GBP, you should includes the GBP flag in your pattern,
> > no need to use a separate item.
> > 
> 
> Why not be more explicit?
> It helps to driver to know more about the pattern to be able to create
> proper flow rule, if there is an obvious way for driver to differentiate
> these protocol extensions, and flow item type is redundant, I can
> understand the proposal, but otherwise I think better to keep flow items
> for extensions.

In any case we need the simple VXLAN item.
If we have GPE and GBP specialized items,
what means a match on the simple VXLAN?
Does it include packets with other extensions or exclude them?
Matching the bits in the protocol make such decision explicit.

> When a rule is set in HW, HW may not care about the protocol, as long as
> bits in the rule match with the packet, HW can apply the action.
> But for driver to be able to set the rule properly, it needs more
> explicit information.

Yes information is in the pattern to match.

> Lets assume driver API get a pattern with 'RTE_FLOW_ITEM_TYPE_VXLAN'
> type and "struct rte_flow_item_vxlan", at this point driver doesn't know
> if it is VXLAN or any of the extensions.

Yes it knows because of the matched bits in the pattern.
If the rule specify a match on GBP flag = 1, it is GBP only.
If the rule specify a match on GBP flag = 0, it excludes GBP.
If the rule does not mask GBP flag, it includes GBP.

> Should driver go and check GBP flags to deduce if it is GBP flag, and

Yes

> what if they are all zero, how can driver can tell if this is GBP flag
> that is zero or is it VXLAN header and bit is reserved.

As explained above, value & mask does the matching.

> Or I will just make up a sample, it may not be accurate but please take
> it as a case to prove a point.
> Lets assume driver API again get "struct rte_flow_item_vxlan" whose
> first word's last bit is set, which union in the struct will driver
> check to detect this, GPE one or GBP one?

It depends on the G flag.

> This can be GPE-"next protocol" or GBP-"Group policy id", if driver
> knows this it can set the mask better for the field in the rule.
> Driver may try to deduce this extension information from other
> information, but why not proper specific flow type for each extension
> instead?

I believe relying on bit matching is a cleaner approach
than creating a new item each time a protocol has a variation.
And it makes include/exclude policy easier to understand
thanks to the value/mask logic.


> >> And if a specific HW detects VXLAN-GPE and VXLAN-GBP protocols as two
> >> separate protocols, won't only having capability to describe VXLAN
> >> protocol in SW be a limitation.
> > 
> > I think the driver may know based on the flow rule.
> > 
> > That's a good question about the item granularity.
> > What is the separation between a different protocol and protocol options?
> > How flow item should match protocol variations?
> > My current thinking is that we should minimize the number of items.
> > 
> >> If the target is to remove code duplication in the driver, can it be
> >> accomplished by extracting code that parse the common fields of these
> >> protocols?
> > 
> > Driver code is not a concern as far as any driver can implement the API.
> > 
> 
> Got it, I want to clarify this one.
> Where the code duplication via copy/paste that commit log mention occurs?
> 
> > 
> >>> In this patch, all the VXLAN extension header will be merged with VXLAN as
> >>> union if the overlapped field has different format among protocols. The
> >>> existing VXLAN-GPE will be marked as deprecated and new extensions of
> >>> VXLAN should be added to VXLAN instead of a new RTE item.
> >>>
> >>> Signed-off-by: Gavin Li <gavinl@nvidia.com>
> >>> ---
> >>>  doc/guides/rel_notes/deprecation.rst |  5 +++
> >>>  lib/ethdev/rte_flow.h                | 13 +++++-
> >>>  lib/net/rte_vxlan.h                  | 67 ++++++++++++++++++++++++++--
> >>>  3 files changed, 80 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> >>> index 81b93515cb..f9cf931b77 100644
> >>> --- a/doc/guides/rel_notes/deprecation.rst
> >>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>> @@ -95,6 +95,11 @@ Deprecation Notices
> >>>    - ``rte_flow_item_pppoe``
> >>>    - ``rte_flow_item_pppoe_proto_id``
> >>>  
> >>> +* ethdev: The flow item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE`` is replaced with ``RTE_FLOW_ITEM_TYPE_VXLAN``.
> >>> +  The item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE``, the struct ``rte_flow_item_vxlan_gpe``, its mask ``rte_flow_item_vxlan_gpe_mask``,
> >>> +  and the header struct ``rte_vxlan_gpe_hdr`` with the macro ``RTE_ETHER_VXLAN_GPE_HLEN``
> >>> +  will be removed in DPDK 25.11.
> >>> +
> >>>
> >>
> >> We have 24.11 in between.
> > 
> > Isn't it too soon?
> > Should we remove at all?
Ferruh Yigit Feb. 9, 2024, 2:58 p.m. UTC | #7 
On 2/9/2024 1:44 PM, Thomas Monjalon wrote:
> 09/02/2024 13:11, Ferruh Yigit:
>> On 2/9/2024 10:12 AM, Thomas Monjalon wrote:
>>> 09/02/2024 00:54, Ferruh Yigit:
>>>> On 1/30/2024 11:25 AM, Gavin Li wrote:
>>>>> Currently, DPDK supports VXLAN and VXLAN-GPE with similar header
>>>>> structures and we are working on adding support for VXLAN-GBP which is
>>>>> another extension to VXLAN. More extension of VXLAN may be added in the
>>>>> future.
>>>>>
>>>>> VXLAN and VXLAN-GBP use the same UDP port(4789) while VXLAN-GPE uses a
>>>>> different one, 4790. The three protocols have the same header length and
>>>>> overall similar header structure as below.
>>>>>     0                   1                   2                   3
>>>>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>    |R|R|R|R|I|R|R|R|            Reserved                           |
>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>    |                VXLAN Network Identifier (VNI) |   Reserved    |
>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>
>>>>>                            Figure 1: VXLAN Header
>>>>>
>>>>>     0                   1                   2                   3
>>>>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>    |R|R|Ver|I|P|B|O|       Reserved                |Next Protocol  |
>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>    |                VXLAN Network Identifier (VNI) |   Reserved    |
>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>
>>>>>                          Figure 2: VXLAN-GPE Header
>>>>>
>>>>>     0                   1                   2                   3
>>>>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>    |G|R|R|R|I|R|R|R|R|D|R|R|A|R|R|R|        Group Policy ID        |
>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>    |          VXLAN Network Identifier (VNI)       |   Reserved    |
>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>
>>>>>                           Figure 3: VXLAN-GBP Extension
>>>>>
>>>>> Both VXLAN-GPE and VXLAN-GBP extended VXLAN by redefining its reserved
>>>>> bits, which means the packets can be processed with same pattern and most
>>>>> of the code can be reused. Instead of adding more new items by
>>>>> copying/pasting code for the VXLAN extensions in the future, it’s better
>>>>> to use existing VXLAN infrastructure and add support code in it.
>>>>>
>>>>
>>>> Hi Gavin,
>>>>
>>>> The motivation is to prevent code duplication, and the code mentioned is
>>>>  the driver code, right?
>>>
>>> The motivation is mainly to provide a unified and more explicit API.
>>>
>>
>> From user perspective, I think existing approach is more explicit,
>> because it sets VXLAN or VXLAN_GPE flow types.
>>
>> I am trying to understand the benefit, how unifying flow type in the API
>> helps to the user?
>>
>>>> Overall OK to unify "struct rte_vxlan_hdr" although it makes the struct
>>>> a little complex, perhaps we can consider extraction some nested structs
>>>> as named struct, no strong opinion.
>>>>
>>>>
>>>> But not sure about removing the flow item types for VXLAN-GPE, or not
>>>> adding for VXLAN-GBP.
>>>>
>>>> Think about a case user adding a rule, which has a item type as VXLAN
>>>> and in the protocol header some bits are set, lets say first word, last
>>>> byte is set, how driver will know if to take it as GPE "next protocol"
>>>> or "group policy id".
>>>
>>> The driver may decide depending on the UDP port and some distinguishing flags.
>>> If you want to match on GBP, you should includes the GBP flag in your pattern,
>>> no need to use a separate item.
>>>
>>
>> Why not be more explicit?
>> It helps to driver to know more about the pattern to be able to create
>> proper flow rule, if there is an obvious way for driver to differentiate
>> these protocol extensions, and flow item type is redundant, I can
>> understand the proposal, but otherwise I think better to keep flow items
>> for extensions.
> 
> In any case we need the simple VXLAN item.
> If we have GPE and GBP specialized items,
> what means a match on the simple VXLAN?
> Does it include packets with other extensions or exclude them?
> Matching the bits in the protocol make such decision explicit.
> 
>> When a rule is set in HW, HW may not care about the protocol, as long as
>> bits in the rule match with the packet, HW can apply the action.
>> But for driver to be able to set the rule properly, it needs more
>> explicit information.
> 
> Yes information is in the pattern to match.
> 
>> Lets assume driver API get a pattern with 'RTE_FLOW_ITEM_TYPE_VXLAN'
>> type and "struct rte_flow_item_vxlan", at this point driver doesn't know
>> if it is VXLAN or any of the extensions.
> 
> Yes it knows because of the matched bits in the pattern.
> If the rule specify a match on GBP flag = 1, it is GBP only.
> If the rule specify a match on GBP flag = 0, it excludes GBP.
> If the rule does not mask GBP flag, it includes GBP.
> 


OK, VXLAN-GBP protocol has a GBP flag that gives a way to differentiate
the extension, so flow item for it becomes redundant and we can get rid
of it.

Is it same for the other extensions?
If we use VXLAN flow item and by setting specific field in pattern can
we differentiate VXLAN and any other extension?
Or in some cases other information, like UDP port, needs to be taken
into account to differentiate protocol/extension?


I found a spec for VXLAN-GBP, but it shows as sub-header for VXLAN-GPE,
different than what this RFC describes:
https://datatracker.ietf.org/doc/html/draft-lemon-vxlan-gpe-gbp

Can you please share link for VXLAN-GBP Extension spec?
Thomas Monjalon Feb. 9, 2024, 3:32 p.m. UTC | #8 
09/02/2024 15:58, Ferruh Yigit:
> On 2/9/2024 1:44 PM, Thomas Monjalon wrote:
> > 09/02/2024 13:11, Ferruh Yigit:
> >> On 2/9/2024 10:12 AM, Thomas Monjalon wrote:
> >>> 09/02/2024 00:54, Ferruh Yigit:
> >>>> On 1/30/2024 11:25 AM, Gavin Li wrote:
> >>>>> Currently, DPDK supports VXLAN and VXLAN-GPE with similar header
> >>>>> structures and we are working on adding support for VXLAN-GBP which is
> >>>>> another extension to VXLAN. More extension of VXLAN may be added in the
> >>>>> future.
> >>>>>
> >>>>> VXLAN and VXLAN-GBP use the same UDP port(4789) while VXLAN-GPE uses a
> >>>>> different one, 4790. The three protocols have the same header length and
> >>>>> overall similar header structure as below.
> >>>>>     0                   1                   2                   3
> >>>>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> >>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>>>    |R|R|R|R|I|R|R|R|            Reserved                           |
> >>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>>>    |                VXLAN Network Identifier (VNI) |   Reserved    |
> >>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>>>
> >>>>>                            Figure 1: VXLAN Header
> >>>>>
> >>>>>     0                   1                   2                   3
> >>>>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> >>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>>>    |R|R|Ver|I|P|B|O|       Reserved                |Next Protocol  |
> >>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>>>    |                VXLAN Network Identifier (VNI) |   Reserved    |
> >>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>>>
> >>>>>                          Figure 2: VXLAN-GPE Header
> >>>>>
> >>>>>     0                   1                   2                   3
> >>>>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> >>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>>>    |G|R|R|R|I|R|R|R|R|D|R|R|A|R|R|R|        Group Policy ID        |
> >>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>>>    |          VXLAN Network Identifier (VNI)       |   Reserved    |
> >>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >>>>>
> >>>>>                           Figure 3: VXLAN-GBP Extension
> >>>>>
> >>>>> Both VXLAN-GPE and VXLAN-GBP extended VXLAN by redefining its reserved
> >>>>> bits, which means the packets can be processed with same pattern and most
> >>>>> of the code can be reused. Instead of adding more new items by
> >>>>> copying/pasting code for the VXLAN extensions in the future, it’s better
> >>>>> to use existing VXLAN infrastructure and add support code in it.
> >>>>>
> >>>>
> >>>> Hi Gavin,
> >>>>
> >>>> The motivation is to prevent code duplication, and the code mentioned is
> >>>>  the driver code, right?
> >>>
> >>> The motivation is mainly to provide a unified and more explicit API.
> >>>
> >>
> >> From user perspective, I think existing approach is more explicit,
> >> because it sets VXLAN or VXLAN_GPE flow types.
> >>
> >> I am trying to understand the benefit, how unifying flow type in the API
> >> helps to the user?
> >>
> >>>> Overall OK to unify "struct rte_vxlan_hdr" although it makes the struct
> >>>> a little complex, perhaps we can consider extraction some nested structs
> >>>> as named struct, no strong opinion.
> >>>>
> >>>>
> >>>> But not sure about removing the flow item types for VXLAN-GPE, or not
> >>>> adding for VXLAN-GBP.
> >>>>
> >>>> Think about a case user adding a rule, which has a item type as VXLAN
> >>>> and in the protocol header some bits are set, lets say first word, last
> >>>> byte is set, how driver will know if to take it as GPE "next protocol"
> >>>> or "group policy id".
> >>>
> >>> The driver may decide depending on the UDP port and some distinguishing flags.
> >>> If you want to match on GBP, you should includes the GBP flag in your pattern,
> >>> no need to use a separate item.
> >>>
> >>
> >> Why not be more explicit?
> >> It helps to driver to know more about the pattern to be able to create
> >> proper flow rule, if there is an obvious way for driver to differentiate
> >> these protocol extensions, and flow item type is redundant, I can
> >> understand the proposal, but otherwise I think better to keep flow items
> >> for extensions.
> > 
> > In any case we need the simple VXLAN item.
> > If we have GPE and GBP specialized items,
> > what means a match on the simple VXLAN?
> > Does it include packets with other extensions or exclude them?
> > Matching the bits in the protocol make such decision explicit.
> > 
> >> When a rule is set in HW, HW may not care about the protocol, as long as
> >> bits in the rule match with the packet, HW can apply the action.
> >> But for driver to be able to set the rule properly, it needs more
> >> explicit information.
> > 
> > Yes information is in the pattern to match.
> > 
> >> Lets assume driver API get a pattern with 'RTE_FLOW_ITEM_TYPE_VXLAN'
> >> type and "struct rte_flow_item_vxlan", at this point driver doesn't know
> >> if it is VXLAN or any of the extensions.
> > 
> > Yes it knows because of the matched bits in the pattern.
> > If the rule specify a match on GBP flag = 1, it is GBP only.
> > If the rule specify a match on GBP flag = 0, it excludes GBP.
> > If the rule does not mask GBP flag, it includes GBP.
> > 
> 
> 
> OK, VXLAN-GBP protocol has a GBP flag that gives a way to differentiate
> the extension, so flow item for it becomes redundant and we can get rid
> of it.

Yes I think so.

> Is it same for the other extensions?
> If we use VXLAN flow item and by setting specific field in pattern can
> we differentiate VXLAN and any other extension?
> Or in some cases other information, like UDP port, needs to be taken
> into account to differentiate protocol/extension?

For VXLAN-GPE, differentiation is on UDP port.
Remember we have an API to fill some UDP ports:
rte_eth_dev_udp_tunnel_port_add with RTE_ETH_TUNNEL_TYPE_VXLAN_GPE

The UDP port value/mask may be part of the flow rule pattern.


> I found a spec for VXLAN-GBP, but it shows as sub-header for VXLAN-GPE,
> different than what this RFC describes:
> https://datatracker.ietf.org/doc/html/draft-lemon-vxlan-gpe-gbp
> 
> Can you please share link for VXLAN-GBP Extension spec?

I will let Gavin explain here, I'm not an expert.
Ferruh Yigit Feb. 9, 2024, 3:58 p.m. UTC | #9 
On 2/9/2024 3:32 PM, Thomas Monjalon wrote:
> 09/02/2024 15:58, Ferruh Yigit:
>> On 2/9/2024 1:44 PM, Thomas Monjalon wrote:
>>> 09/02/2024 13:11, Ferruh Yigit:
>>>> On 2/9/2024 10:12 AM, Thomas Monjalon wrote:
>>>>> 09/02/2024 00:54, Ferruh Yigit:
>>>>>> On 1/30/2024 11:25 AM, Gavin Li wrote:
>>>>>>> Currently, DPDK supports VXLAN and VXLAN-GPE with similar header
>>>>>>> structures and we are working on adding support for VXLAN-GBP which is
>>>>>>> another extension to VXLAN. More extension of VXLAN may be added in the
>>>>>>> future.
>>>>>>>
>>>>>>> VXLAN and VXLAN-GBP use the same UDP port(4789) while VXLAN-GPE uses a
>>>>>>> different one, 4790. The three protocols have the same header length and
>>>>>>> overall similar header structure as below.
>>>>>>>     0                   1                   2                   3
>>>>>>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>    |R|R|R|R|I|R|R|R|            Reserved                           |
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>    |                VXLAN Network Identifier (VNI) |   Reserved    |
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>
>>>>>>>                            Figure 1: VXLAN Header
>>>>>>>
>>>>>>>     0                   1                   2                   3
>>>>>>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>    |R|R|Ver|I|P|B|O|       Reserved                |Next Protocol  |
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>    |                VXLAN Network Identifier (VNI) |   Reserved    |
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>
>>>>>>>                          Figure 2: VXLAN-GPE Header
>>>>>>>
>>>>>>>     0                   1                   2                   3
>>>>>>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>    |G|R|R|R|I|R|R|R|R|D|R|R|A|R|R|R|        Group Policy ID        |
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>    |          VXLAN Network Identifier (VNI)       |   Reserved    |
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>
>>>>>>>                           Figure 3: VXLAN-GBP Extension
>>>>>>>
>>>>>>> Both VXLAN-GPE and VXLAN-GBP extended VXLAN by redefining its reserved
>>>>>>> bits, which means the packets can be processed with same pattern and most
>>>>>>> of the code can be reused. Instead of adding more new items by
>>>>>>> copying/pasting code for the VXLAN extensions in the future, it’s better
>>>>>>> to use existing VXLAN infrastructure and add support code in it.
>>>>>>>
>>>>>>
>>>>>> Hi Gavin,
>>>>>>
>>>>>> The motivation is to prevent code duplication, and the code mentioned is
>>>>>>  the driver code, right?
>>>>>
>>>>> The motivation is mainly to provide a unified and more explicit API.
>>>>>
>>>>
>>>> From user perspective, I think existing approach is more explicit,
>>>> because it sets VXLAN or VXLAN_GPE flow types.
>>>>
>>>> I am trying to understand the benefit, how unifying flow type in the API
>>>> helps to the user?
>>>>
>>>>>> Overall OK to unify "struct rte_vxlan_hdr" although it makes the struct
>>>>>> a little complex, perhaps we can consider extraction some nested structs
>>>>>> as named struct, no strong opinion.
>>>>>>
>>>>>>
>>>>>> But not sure about removing the flow item types for VXLAN-GPE, or not
>>>>>> adding for VXLAN-GBP.
>>>>>>
>>>>>> Think about a case user adding a rule, which has a item type as VXLAN
>>>>>> and in the protocol header some bits are set, lets say first word, last
>>>>>> byte is set, how driver will know if to take it as GPE "next protocol"
>>>>>> or "group policy id".
>>>>>
>>>>> The driver may decide depending on the UDP port and some distinguishing flags.
>>>>> If you want to match on GBP, you should includes the GBP flag in your pattern,
>>>>> no need to use a separate item.
>>>>>
>>>>
>>>> Why not be more explicit?
>>>> It helps to driver to know more about the pattern to be able to create
>>>> proper flow rule, if there is an obvious way for driver to differentiate
>>>> these protocol extensions, and flow item type is redundant, I can
>>>> understand the proposal, but otherwise I think better to keep flow items
>>>> for extensions.
>>>
>>> In any case we need the simple VXLAN item.
>>> If we have GPE and GBP specialized items,
>>> what means a match on the simple VXLAN?
>>> Does it include packets with other extensions or exclude them?
>>> Matching the bits in the protocol make such decision explicit.
>>>
>>>> When a rule is set in HW, HW may not care about the protocol, as long as
>>>> bits in the rule match with the packet, HW can apply the action.
>>>> But for driver to be able to set the rule properly, it needs more
>>>> explicit information.
>>>
>>> Yes information is in the pattern to match.
>>>
>>>> Lets assume driver API get a pattern with 'RTE_FLOW_ITEM_TYPE_VXLAN'
>>>> type and "struct rte_flow_item_vxlan", at this point driver doesn't know
>>>> if it is VXLAN or any of the extensions.
>>>
>>> Yes it knows because of the matched bits in the pattern.
>>> If the rule specify a match on GBP flag = 1, it is GBP only.
>>> If the rule specify a match on GBP flag = 0, it excludes GBP.
>>> If the rule does not mask GBP flag, it includes GBP.
>>>
>>
>>
>> OK, VXLAN-GBP protocol has a GBP flag that gives a way to differentiate
>> the extension, so flow item for it becomes redundant and we can get rid
>> of it.
> 
> Yes I think so.
> 
>> Is it same for the other extensions?
>> If we use VXLAN flow item and by setting specific field in pattern can
>> we differentiate VXLAN and any other extension?
>> Or in some cases other information, like UDP port, needs to be taken
>> into account to differentiate protocol/extension?
> 
> For VXLAN-GPE, differentiation is on UDP port.
> Remember we have an API to fill some UDP ports:
> rte_eth_dev_udp_tunnel_port_add with RTE_ETH_TUNNEL_TYPE_VXLAN_GPE
> 
> The UDP port value/mask may be part of the flow rule pattern.
> 

So one option is use vxlan extension specific flow item, which is
current case.

Other option is use generic vxlan flow item, and detect the extension
using other fields, which is this proposal.

Both works, but specially if driver needs other protocol information,
like UDP port, to detect the vxlan protocol extension, why not just keep
continue with existing explicit flow item.


OK for merging vxlan struct in net library.
But I am failing to see benefit to change the flow item and structs.
No strong opinion, it can be good to get more comments.


> 
>> I found a spec for VXLAN-GBP, but it shows as sub-header for VXLAN-GPE,
>> different than what this RFC describes:
>> https://datatracker.ietf.org/doc/html/draft-lemon-vxlan-gpe-gbp
>>
>> Can you please share link for VXLAN-GBP Extension spec?
> 
> I will let Gavin explain here, I'm not an expert.
> 
>
Gavin Li Feb. 19, 2024, 3:16 a.m. UTC | #10 
On 2/9/2024 11:32 PM, Thomas Monjalon wrote:
> 09/02/2024 15:58, Ferruh Yigit:
>> On 2/9/2024 1:44 PM, Thomas Monjalon wrote:
>>> 09/02/2024 13:11, Ferruh Yigit:
>>>> On 2/9/2024 10:12 AM, Thomas Monjalon wrote:
>>>>> 09/02/2024 00:54, Ferruh Yigit:
>>>>>> On 1/30/2024 11:25 AM, Gavin Li wrote:
>>>>>>> Currently, DPDK supports VXLAN and VXLAN-GPE with similar header
>>>>>>> structures and we are working on adding support for VXLAN-GBP which is
>>>>>>> another extension to VXLAN. More extension of VXLAN may be added in the
>>>>>>> future.
>>>>>>>
>>>>>>> VXLAN and VXLAN-GBP use the same UDP port(4789) while VXLAN-GPE uses a
>>>>>>> different one, 4790. The three protocols have the same header length and
>>>>>>> overall similar header structure as below.
>>>>>>>      0                   1                   2                   3
>>>>>>>      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>     |R|R|R|R|I|R|R|R|            Reserved                           |
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>     |                VXLAN Network Identifier (VNI) |   Reserved    |
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>
>>>>>>>                             Figure 1: VXLAN Header
>>>>>>>
>>>>>>>      0                   1                   2                   3
>>>>>>>      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>     |R|R|Ver|I|P|B|O|       Reserved                |Next Protocol  |
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>     |                VXLAN Network Identifier (VNI) |   Reserved    |
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>
>>>>>>>                           Figure 2: VXLAN-GPE Header
>>>>>>>
>>>>>>>      0                   1                   2                   3
>>>>>>>      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>     |G|R|R|R|I|R|R|R|R|D|R|R|A|R|R|R|        Group Policy ID        |
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>     |          VXLAN Network Identifier (VNI)       |   Reserved    |
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>
>>>>>>>                            Figure 3: VXLAN-GBP Extension
>>>>>>>
>>>>>>> Both VXLAN-GPE and VXLAN-GBP extended VXLAN by redefining its reserved
>>>>>>> bits, which means the packets can be processed with same pattern and most
>>>>>>> of the code can be reused. Instead of adding more new items by
>>>>>>> copying/pasting code for the VXLAN extensions in the future, it’s better
>>>>>>> to use existing VXLAN infrastructure and add support code in it.
>>>>>>>
>>>>>>
>>>>>> Hi Gavin,
>>>>>>
>>>>>> The motivation is to prevent code duplication, and the code mentioned is
>>>>>>   the driver code, right?
>>>>>
>>>>> The motivation is mainly to provide a unified and more explicit API.
>>>>>
>>>>
>>>>  From user perspective, I think existing approach is more explicit,
>>>> because it sets VXLAN or VXLAN_GPE flow types.
>>>>
>>>> I am trying to understand the benefit, how unifying flow type in the API
>>>> helps to the user?
>>>>
>>>>>> Overall OK to unify "struct rte_vxlan_hdr" although it makes the struct
>>>>>> a little complex, perhaps we can consider extraction some nested structs
>>>>>> as named struct, no strong opinion.
>>>>>>
>>>>>>
>>>>>> But not sure about removing the flow item types for VXLAN-GPE, or not
>>>>>> adding for VXLAN-GBP.
>>>>>>
>>>>>> Think about a case user adding a rule, which has a item type as VXLAN
>>>>>> and in the protocol header some bits are set, lets say first word, last
>>>>>> byte is set, how driver will know if to take it as GPE "next protocol"
>>>>>> or "group policy id".
>>>>>
>>>>> The driver may decide depending on the UDP port and some distinguishing flags.
>>>>> If you want to match on GBP, you should includes the GBP flag in your pattern,
>>>>> no need to use a separate item.
>>>>>
>>>>
>>>> Why not be more explicit?
>>>> It helps to driver to know more about the pattern to be able to create
>>>> proper flow rule, if there is an obvious way for driver to differentiate
>>>> these protocol extensions, and flow item type is redundant, I can
>>>> understand the proposal, but otherwise I think better to keep flow items
>>>> for extensions.
>>>
>>> In any case we need the simple VXLAN item.
>>> If we have GPE and GBP specialized items,
>>> what means a match on the simple VXLAN?
>>> Does it include packets with other extensions or exclude them?
>>> Matching the bits in the protocol make such decision explicit.
>>>
>>>> When a rule is set in HW, HW may not care about the protocol, as long as
>>>> bits in the rule match with the packet, HW can apply the action.
>>>> But for driver to be able to set the rule properly, it needs more
>>>> explicit information.
>>>
>>> Yes information is in the pattern to match.
>>>
>>>> Lets assume driver API get a pattern with 'RTE_FLOW_ITEM_TYPE_VXLAN'
>>>> type and "struct rte_flow_item_vxlan", at this point driver doesn't know
>>>> if it is VXLAN or any of the extensions.
>>>
>>> Yes it knows because of the matched bits in the pattern.
>>> If the rule specify a match on GBP flag = 1, it is GBP only.
>>> If the rule specify a match on GBP flag = 0, it excludes GBP.
>>> If the rule does not mask GBP flag, it includes GBP.
>>>
>>
>>
>> OK, VXLAN-GBP protocol has a GBP flag that gives a way to differentiate
>> the extension, so flow item for it becomes redundant and we can get rid
>> of it.
> 
> Yes I think so.
> 
>> Is it same for the other extensions?
>> If we use VXLAN flow item and by setting specific field in pattern can
>> we differentiate VXLAN and any other extension?
>> Or in some cases other information, like UDP port, needs to be taken
>> into account to differentiate protocol/extension?
> 
> For VXLAN-GPE, differentiation is on UDP port.
> Remember we have an API to fill some UDP ports:
> rte_eth_dev_udp_tunnel_port_add with RTE_ETH_TUNNEL_TYPE_VXLAN_GPE
> 
> The UDP port value/mask may be part of the flow rule pattern.
> 
> 
>> I found a spec for VXLAN-GBP, but it shows as sub-header for VXLAN-GPE,
>> different than what this RFC describes:
>> https://datatracker.ietf.org/doc/html/draft-lemon-vxlan-gpe-gbp
>>
>> Can you please share link for VXLAN-GBP Extension spec?
> 
> I will let Gavin explain here, I'm not an expertThe RFC mentioned, aka draft-lemon-vxlan-gpe-gbp was to define VXLAN-GBP 
as a sub header of VXLAN-GPE by assigning a next protocol num of 
VXLAN-GPE for VXLAN-GBP, which has been obsoleted since 
draft-ietf-nvo3-vxlan-gpe-09, the latest is 
draft-ietf-nvo3-vxlan-gpe-13. It was temporarily valid from 
draft-ietf-nvo3-vxlan-gpe-05 to draft-ietf-nvo3-vxlan-gpe-08.

The VXLAN-GBP we are discussing in this thread is an VXLAN extension 
parallel to VXLAN-GPE.
> 
>
Gavin Li Feb. 19, 2024, 3:44 a.m. UTC | #11 
On 2/9/2024 11:32 PM, Thomas Monjalon wrote:
> 09/02/2024 15:58, Ferruh Yigit:
>> On 2/9/2024 1:44 PM, Thomas Monjalon wrote:
>>> 09/02/2024 13:11, Ferruh Yigit:
>>>> On 2/9/2024 10:12 AM, Thomas Monjalon wrote:
>>>>> 09/02/2024 00:54, Ferruh Yigit:
>>>>>> On 1/30/2024 11:25 AM, Gavin Li wrote:
>>>>>>> Currently, DPDK supports VXLAN and VXLAN-GPE with similar header
>>>>>>> structures and we are working on adding support for VXLAN-GBP which is
>>>>>>> another extension to VXLAN. More extension of VXLAN may be added in the
>>>>>>> future.
>>>>>>>
>>>>>>> VXLAN and VXLAN-GBP use the same UDP port(4789) while VXLAN-GPE uses a
>>>>>>> different one, 4790. The three protocols have the same header length and
>>>>>>> overall similar header structure as below.
>>>>>>>      0                   1                   2                   3
>>>>>>>      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>     |R|R|R|R|I|R|R|R|            Reserved                           |
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>     |                VXLAN Network Identifier (VNI) |   Reserved    |
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>
>>>>>>>                             Figure 1: VXLAN Header
>>>>>>>
>>>>>>>      0                   1                   2                   3
>>>>>>>      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>     |R|R|Ver|I|P|B|O|       Reserved                |Next Protocol  |
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>     |                VXLAN Network Identifier (VNI) |   Reserved    |
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>
>>>>>>>                           Figure 2: VXLAN-GPE Header
>>>>>>>
>>>>>>>      0                   1                   2                   3
>>>>>>>      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>     |G|R|R|R|I|R|R|R|R|D|R|R|A|R|R|R|        Group Policy ID        |
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>     |          VXLAN Network Identifier (VNI)       |   Reserved    |
>>>>>>>     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>
>>>>>>>                            Figure 3: VXLAN-GBP Extension
>>>>>>>
>>>>>>> Both VXLAN-GPE and VXLAN-GBP extended VXLAN by redefining its reserved
>>>>>>> bits, which means the packets can be processed with same pattern and most
>>>>>>> of the code can be reused. Instead of adding more new items by
>>>>>>> copying/pasting code for the VXLAN extensions in the future, it’s better
>>>>>>> to use existing VXLAN infrastructure and add support code in it.
>>>>>>>
>>>>>>
>>>>>> Hi Gavin,
>>>>>>
>>>>>> The motivation is to prevent code duplication, and the code mentioned is
>>>>>>   the driver code, right?
>>>>>
>>>>> The motivation is mainly to provide a unified and more explicit API.
>>>>>
>>>>
>>>>  From user perspective, I think existing approach is more explicit,
>>>> because it sets VXLAN or VXLAN_GPE flow types.
>>>>
>>>> I am trying to understand the benefit, how unifying flow type in the API
>>>> helps to the user?
>>>>
>>>>>> Overall OK to unify "struct rte_vxlan_hdr" although it makes the struct
>>>>>> a little complex, perhaps we can consider extraction some nested structs
>>>>>> as named struct, no strong opinion.
>>>>>>
>>>>>>
>>>>>> But not sure about removing the flow item types for VXLAN-GPE, or not
>>>>>> adding for VXLAN-GBP.
>>>>>>
>>>>>> Think about a case user adding a rule, which has a item type as VXLAN
>>>>>> and in the protocol header some bits are set, lets say first word, last
>>>>>> byte is set, how driver will know if to take it as GPE "next protocol"
>>>>>> or "group policy id".
>>>>>
>>>>> The driver may decide depending on the UDP port and some distinguishing flags.
>>>>> If you want to match on GBP, you should includes the GBP flag in your pattern,
>>>>> no need to use a separate item.
>>>>>
>>>>
>>>> Why not be more explicit?
>>>> It helps to driver to know more about the pattern to be able to create
>>>> proper flow rule, if there is an obvious way for driver to differentiate
>>>> these protocol extensions, and flow item type is redundant, I can
>>>> understand the proposal, but otherwise I think better to keep flow items
>>>> for extensions.
>>>
>>> In any case we need the simple VXLAN item.
>>> If we have GPE and GBP specialized items,
>>> what means a match on the simple VXLAN?
>>> Does it include packets with other extensions or exclude them?
>>> Matching the bits in the protocol make such decision explicit.
>>>
>>>> When a rule is set in HW, HW may not care about the protocol, as long as
>>>> bits in the rule match with the packet, HW can apply the action.
>>>> But for driver to be able to set the rule properly, it needs more
>>>> explicit information.
>>>
>>> Yes information is in the pattern to match.
>>>
>>>> Lets assume driver API get a pattern with 'RTE_FLOW_ITEM_TYPE_VXLAN'
>>>> type and "struct rte_flow_item_vxlan", at this point driver doesn't know
>>>> if it is VXLAN or any of the extensions.
>>>
>>> Yes it knows because of the matched bits in the pattern.
>>> If the rule specify a match on GBP flag = 1, it is GBP only.
>>> If the rule specify a match on GBP flag = 0, it excludes GBP.
>>> If the rule does not mask GBP flag, it includes GBP.
>>>
>>
>>
>> OK, VXLAN-GBP protocol has a GBP flag that gives a way to differentiate
>> the extension, so flow item for it becomes redundant and we can get rid
>> of it.
> 
> Yes I think so.
> 
>> Is it same for the other extensions?
>> If we use VXLAN flow item and by setting specific field in pattern can
>> we differentiate VXLAN and any other extension?
>> Or in some cases other information, like UDP port, needs to be taken
>> into account to differentiate protocol/extension?
> 
> For VXLAN-GPE, differentiation is on UDP port.
> Remember we have an API to fill some UDP ports:
> rte_eth_dev_udp_tunnel_port_add with RTE_ETH_TUNNEL_TYPE_VXLAN_GPE
> 
> The UDP port value/mask may be part of the flow rule pattern.
> 
> 
>> I found a spec for VXLAN-GBP, but it shows as sub-header for VXLAN-GPE,
>> different than what this RFC describes:
>> https://datatracker.ietf.org/doc/html/draft-lemon-vxlan-gpe-gbp
>>
>> Can you please share link for VXLAN-GBP Extension spec?
> 
> I will let Gavin explain here, I'm not an expert.

Sorry for the wrong format of my last mail. Let me try again.

The RFC mentioned, aka draft-lemon-vxlan-gpe-gbp was to define VXLAN-GBP
as a sub header of VXLAN-GPE by assigning a next protocol num of 
VXLAN-GPE for VXLAN-GBP, which has been obsoleted since 
draft-ietf-nvo3-vxlan-gpe-09, the latest is 
draft-ietf-nvo3-vxlan-gpe-13. It was temporarily valid from 
draft-ietf-nvo3-vxlan-gpe-05 to draft-ietf-nvo3-vxlan-gpe-08.

The VXLAN-GBP we are discussing in this thread is an VXLAN extension 
parallel to VXLAN-GPE.

The RFC introduced VXLAN-GBP sub header to VXLAN-GPE is 
https://datatracker.ietf.org/doc/html/draft-ietf-nvo3-vxlan-gpe-05.

The RFC obsoleted the sub header is 
https://datatracker.ietf.org/doc/html/draft-ietf-nvo3-vxlan-gpe-09.

The latest RFC of VXLAN-GPE is 
https://datatracker.ietf.org/doc/html/draft-ietf-nvo3-vxlan-gpe-13.


> 
>
Gavin Li Feb. 19, 2024, 4:03 a.m. UTC | #12 
On 2/19/2024 11:44 AM, Gavin Li wrote:
> 
> 
> On 2/9/2024 11:32 PM, Thomas Monjalon wrote:
>> 09/02/2024 15:58, Ferruh Yigit:
>>> On 2/9/2024 1:44 PM, Thomas Monjalon wrote:
>>>> 09/02/2024 13:11, Ferruh Yigit:
>>>>> On 2/9/2024 10:12 AM, Thomas Monjalon wrote:
>>>>>> 09/02/2024 00:54, Ferruh Yigit:
>>>>>>> On 1/30/2024 11:25 AM, Gavin Li wrote:
>>>>>>>> Currently, DPDK supports VXLAN and VXLAN-GPE with similar header
>>>>>>>> structures and we are working on adding support for VXLAN-GBP 
>>>>>>>> which is
>>>>>>>> another extension to VXLAN. More extension of VXLAN may be added 
>>>>>>>> in the
>>>>>>>> future.
>>>>>>>>
>>>>>>>> VXLAN and VXLAN-GBP use the same UDP port(4789) while VXLAN-GPE 
>>>>>>>> uses a
>>>>>>>> different one, 4790. The three protocols have the same header 
>>>>>>>> length and
>>>>>>>> overall similar header structure as below.
>>>>>>>>      0                   1                   2                   3
>>>>>>>>      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 
>>>>>>>> 0 1
>>>>>>>>     
>>>>>>>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>>     |R|R|R|R|I|R|R|R|            
>>>>>>>> Reserved                           |
>>>>>>>>     
>>>>>>>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>>     |                VXLAN Network Identifier (VNI) |   
>>>>>>>> Reserved    |
>>>>>>>>     
>>>>>>>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>>
>>>>>>>>                             Figure 1: VXLAN Header
>>>>>>>>
>>>>>>>>      0                   1                   2                   3
>>>>>>>>      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 
>>>>>>>> 0 1
>>>>>>>>     
>>>>>>>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>>     |R|R|Ver|I|P|B|O|       Reserved                |Next 
>>>>>>>> Protocol  |
>>>>>>>>     
>>>>>>>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>>     |                VXLAN Network Identifier (VNI) |   
>>>>>>>> Reserved    |
>>>>>>>>     
>>>>>>>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>>
>>>>>>>>                           Figure 2: VXLAN-GPE Header
>>>>>>>>
>>>>>>>>      0                   1                   2                   3
>>>>>>>>      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 
>>>>>>>> 0 1
>>>>>>>>     
>>>>>>>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>>     |G|R|R|R|I|R|R|R|R|D|R|R|A|R|R|R|        Group Policy 
>>>>>>>> ID        |
>>>>>>>>     
>>>>>>>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>>     |          VXLAN Network Identifier (VNI)       |   
>>>>>>>> Reserved    |
>>>>>>>>     
>>>>>>>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>>
>>>>>>>>                            Figure 3: VXLAN-GBP Extension
>>>>>>>>
>>>>>>>> Both VXLAN-GPE and VXLAN-GBP extended VXLAN by redefining its 
>>>>>>>> reserved
>>>>>>>> bits, which means the packets can be processed with same pattern 
>>>>>>>> and most
>>>>>>>> of the code can be reused. Instead of adding more new items by
>>>>>>>> copying/pasting code for the VXLAN extensions in the future, 
>>>>>>>> it’s better
>>>>>>>> to use existing VXLAN infrastructure and add support code in it.
>>>>>>>>
>>>>>>>
>>>>>>> Hi Gavin,
>>>>>>>
>>>>>>> The motivation is to prevent code duplication, and the code 
>>>>>>> mentioned is
>>>>>>>   the driver code, right?
>>>>>>
>>>>>> The motivation is mainly to provide a unified and more explicit API.
>>>>>>
>>>>>
>>>>>  From user perspective, I think existing approach is more explicit,
>>>>> because it sets VXLAN or VXLAN_GPE flow types.
>>>>>
>>>>> I am trying to understand the benefit, how unifying flow type in 
>>>>> the API
>>>>> helps to the user?
>>>>>
>>>>>>> Overall OK to unify "struct rte_vxlan_hdr" although it makes the 
>>>>>>> struct
>>>>>>> a little complex, perhaps we can consider extraction some nested 
>>>>>>> structs
>>>>>>> as named struct, no strong opinion.
>>>>>>>
>>>>>>>
>>>>>>> But not sure about removing the flow item types for VXLAN-GPE, or 
>>>>>>> not
>>>>>>> adding for VXLAN-GBP.
>>>>>>>
>>>>>>> Think about a case user adding a rule, which has a item type as 
>>>>>>> VXLAN
>>>>>>> and in the protocol header some bits are set, lets say first 
>>>>>>> word, last
>>>>>>> byte is set, how driver will know if to take it as GPE "next 
>>>>>>> protocol"
>>>>>>> or "group policy id".
>>>>>>
>>>>>> The driver may decide depending on the UDP port and some 
>>>>>> distinguishing flags.
>>>>>> If you want to match on GBP, you should includes the GBP flag in 
>>>>>> your pattern,
>>>>>> no need to use a separate item.
>>>>>>
>>>>>
>>>>> Why not be more explicit?
>>>>> It helps to driver to know more about the pattern to be able to create
>>>>> proper flow rule, if there is an obvious way for driver to 
>>>>> differentiate
>>>>> these protocol extensions, and flow item type is redundant, I can
>>>>> understand the proposal, but otherwise I think better to keep flow 
>>>>> items
>>>>> for extensions.
>>>>
>>>> In any case we need the simple VXLAN item.
>>>> If we have GPE and GBP specialized items,
>>>> what means a match on the simple VXLAN?
>>>> Does it include packets with other extensions or exclude them?
>>>> Matching the bits in the protocol make such decision explicit.
>>>>
>>>>> When a rule is set in HW, HW may not care about the protocol, as 
>>>>> long as
>>>>> bits in the rule match with the packet, HW can apply the action.
>>>>> But for driver to be able to set the rule properly, it needs more
>>>>> explicit information.
>>>>
>>>> Yes information is in the pattern to match.
>>>>
>>>>> Lets assume driver API get a pattern with 'RTE_FLOW_ITEM_TYPE_VXLAN'
>>>>> type and "struct rte_flow_item_vxlan", at this point driver doesn't 
>>>>> know
>>>>> if it is VXLAN or any of the extensions.
>>>>
>>>> Yes it knows because of the matched bits in the pattern.
>>>> If the rule specify a match on GBP flag = 1, it is GBP only.
>>>> If the rule specify a match on GBP flag = 0, it excludes GBP.
>>>> If the rule does not mask GBP flag, it includes GBP.
>>>>
>>>
>>>
>>> OK, VXLAN-GBP protocol has a GBP flag that gives a way to differentiate
>>> the extension, so flow item for it becomes redundant and we can get rid
>>> of it.
>>
>> Yes I think so.
>>
>>> Is it same for the other extensions?
>>> If we use VXLAN flow item and by setting specific field in pattern can
>>> we differentiate VXLAN and any other extension?
>>> Or in some cases other information, like UDP port, needs to be taken
>>> into account to differentiate protocol/extension?
>>
>> For VXLAN-GPE, differentiation is on UDP port.
>> Remember we have an API to fill some UDP ports:
>> rte_eth_dev_udp_tunnel_port_add with RTE_ETH_TUNNEL_TYPE_VXLAN_GPE
>>
>> The UDP port value/mask may be part of the flow rule pattern.
>>
>>
>>> I found a spec for VXLAN-GBP, but it shows as sub-header for VXLAN-GPE,
>>> different than what this RFC describes:
>>> https://datatracker.ietf.org/doc/html/draft-lemon-vxlan-gpe-gbp
>>>
>>> Can you please share link for VXLAN-GBP Extension spec?
>>
>> I will let Gavin explain here, I'm not an expert.
> 
> Sorry for the wrong format of my last mail. Let me try again.
> 
> The RFC mentioned, aka draft-lemon-vxlan-gpe-gbp was to define VXLAN-GBP
> as a sub header of VXLAN-GPE by assigning a next protocol num of 
> VXLAN-GPE for VXLAN-GBP, which has been obsoleted since 
> draft-ietf-nvo3-vxlan-gpe-09, the latest is 
> draft-ietf-nvo3-vxlan-gpe-13. It was temporarily valid from 
> draft-ietf-nvo3-vxlan-gpe-05 to draft-ietf-nvo3-vxlan-gpe-08.
> 
> The VXLAN-GBP we are discussing in this thread is an VXLAN extension 
> parallel to VXLAN-GPE.
> 
> The RFC introduced VXLAN-GBP sub header to VXLAN-GPE is 
> https://datatracker.ietf.org/doc/html/draft-ietf-nvo3-vxlan-gpe-05.
> 
> The RFC obsoleted the sub header is 
> https://datatracker.ietf.org/doc/html/draft-ietf-nvo3-vxlan-gpe-09.
> 
> The latest RFC of VXLAN-GPE is 
> https://datatracker.ietf.org/doc/html/draft-ietf-nvo3-vxlan-gpe-13.

And the latest RFC of VXLAN-GPB is 
https://datatracker.ietf.org/doc/html/draft-smith-vxlan-group-policy-05.

> 
> 
>>
>>
diff mbox series

Patch

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 81b93515cb..f9cf931b77 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -95,6 +95,11 @@  Deprecation Notices
   - ``rte_flow_item_pppoe``
   - ``rte_flow_item_pppoe_proto_id``
 
+* ethdev: The flow item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE`` is replaced with ``RTE_FLOW_ITEM_TYPE_VXLAN``.
+  The item ``RTE_FLOW_ITEM_TYPE_VXLAN_GPE``, the struct ``rte_flow_item_vxlan_gpe``, its mask ``rte_flow_item_vxlan_gpe_mask``,
+  and the header struct ``rte_vxlan_gpe_hdr`` with the macro ``RTE_ETHER_VXLAN_GPE_HLEN``
+  will be removed in DPDK 25.11.
+
 * ethdev: Queue specific stats fields will be removed from ``struct rte_eth_stats``.
   Mentioned fields are: ``q_ipackets``, ``q_opackets``, ``q_ibytes``, ``q_obytes``,
   ``q_errors``.
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 1267c146e5..a9943106d9 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -355,6 +355,7 @@  enum rte_flow_item_type {
 	RTE_FLOW_ITEM_TYPE_GENEVE,
 
 	/**
+	 * @deprecated Replaced with ``RTE_FLOW_ITEM_TYPE_VXLAN``.
 	 * Matches a VXLAN-GPE header.
 	 *
 	 * See struct rte_flow_item_vxlan_gpe.
@@ -1096,7 +1097,11 @@  static const struct rte_flow_item_sctp rte_flow_item_sctp_mask = {
 /**
  * RTE_FLOW_ITEM_TYPE_VXLAN.
  *
- * Matches a VXLAN header (RFC 7348).
+ * Matches a VXLAN header (RFC 7348), including GPE (draft-ietf-nvo3-vxlan-gpe-13.txt)
+ * and GBP (draft-smith-vxlan-group-policy-05.txt).
+ *
+ * GPE is distinguished with its UDP port.
+ * UDP port may be specified with ``rte_eth_dev_udp_tunnel_port_add()``.
  */
 struct rte_flow_item_vxlan {
 	union {
@@ -1339,6 +1344,7 @@  static const struct rte_flow_item_geneve rte_flow_item_geneve_mask = {
 #endif
 
 /**
+ * @deprecated Replaced with ``rte_flow_item_vxlan``.
  * RTE_FLOW_ITEM_TYPE_VXLAN_GPE (draft-ietf-nvo3-vxlan-gpe-05).
  *
  * Matches a VXLAN-GPE header.
@@ -1360,7 +1366,10 @@  struct rte_flow_item_vxlan_gpe {
 	};
 };
 
-/** Default mask for RTE_FLOW_ITEM_TYPE_VXLAN_GPE. */
+/**
+ * @deprecated Replaced with ``rte_flow_item_vxlan_mask``.
+ * Default mask for RTE_FLOW_ITEM_TYPE_VXLAN_GPE.
+ */
 #ifndef __cplusplus
 static const struct rte_flow_item_vxlan_gpe rte_flow_item_vxlan_gpe_mask = {
 	.hdr.vni = "\xff\xff\xff",
diff --git a/lib/net/rte_vxlan.h b/lib/net/rte_vxlan.h
index 997fc784fc..255c30f71d 100644
--- a/lib/net/rte_vxlan.h
+++ b/lib/net/rte_vxlan.h
@@ -38,8 +38,65 @@  struct rte_vxlan_hdr {
 			rte_be32_t vx_vni;   /**< VNI (24) + Reserved (8). */
 		};
 		struct {
-			uint8_t    flags;    /**< Should be 8 (I flag). */
-			uint8_t    rsvd0[3]; /**< Reserved. */
+			union {
+				uint8_t    flags;    /**< Should be 8 (I flag). */
+				/* Flag bits defined by GPE */
+				struct {
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+					uint8_t flag_o:1,
+						flag_b:1,
+						flag_p:1,
+						flag_i_gpe:1,
+						flag_ver:2,
+						rsvd_gpe:2;
+#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+					uint8_t rsvd_gpe:2,
+						flag_ver:2,
+						flag_i_gpe:1,
+						flag_p:1,
+						flag_b:1,
+						flag_o:1;
+#endif
+				};
+				/* Flag bits defined by GBP */
+				struct {
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+					uint8_t rsvd_gbp1:3,
+						flag_i_gbp:1,
+						rsvd_gbp2:3,
+						flag_g:1;
+#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+					uint8_t flag_g:1,
+						rsvd_gbp1:3,
+						flag_i_gbp:1,
+						rsvd_gbp2:3;
+#endif
+				};
+			};
+			union {
+				uint8_t    rsvd0[3]; /**< Reserved. */
+				/* Overlap with rte_vxlan_gpe_hdr which is deprecated.*/
+				struct {
+					uint8_t rsvd0_gpe[2]; /**< Reserved. */
+					uint8_t proto;	   /**< Next protocol. */
+				};
+				struct {
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+					uint8_t rsvd0_gbp1:3,
+						policy_applied:1,
+						rsvd0_gbp2:2,
+						dont_learn:1,
+						rsvd0_gbp3:1;
+#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+					uint8_t rsvd0_gbp1:1,
+						dont_learn:1,
+						rsvd0_gbp2:2,
+						policy_applied:1,
+						rsvd0_gbp3:3;
+#endif
+					uint16_t policy_id;
+				};
+			};
 			uint8_t    vni[3];   /**< VXLAN identifier. */
 			uint8_t    rsvd1;    /**< Reserved. */
 		};
@@ -52,6 +109,7 @@  struct rte_vxlan_hdr {
 
 
 /**
+ * @deprecated Replaced with ``rte_vxlan_hdr``.
  * VXLAN-GPE protocol header (draft-ietf-nvo3-vxlan-gpe-05).
  * Contains the 8-bit flag, 8-bit next-protocol, 24-bit VXLAN Network
  * Identifier and Reserved fields (16 bits and 8 bits).
@@ -75,7 +133,10 @@  struct rte_vxlan_gpe_hdr {
 	};
 } __rte_packed;
 
-/** VXLAN-GPE tunnel header length. */
+/**
+ * @deprecated Replaced with ``RTE_ETHER_VXLAN_HLEN``.
+ * VXLAN-GPE tunnel header length.
+ */
 #define RTE_ETHER_VXLAN_GPE_HLEN (sizeof(struct rte_udp_hdr) + \
 		sizeof(struct rte_vxlan_gpe_hdr))