[dpdk-dev] [PATCH] net: introduce IPv4 ihl and version fields

[Morten Brørup]

> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Gregory Etelson
> Sent: Thursday, 27 May 2021 17.29
and version fields
> 
> RTE IPv4 header definition combines the `version' and `ihl'  fields
> into a single structure member.
> This patch introduces dedicated structure members for both `version'
> and `ihl' IPv4 fields. Separated header fields definitions allow to
> create simplified code to match on the IHL value in a flow rule.
> The original `version_ihl' structure member is kept for backward
> compatibility.
> 
> Signed-off-by: Gregory Etelson <getelson at nvidia.com>
> ---
>  app/test/test_flow_classify.c |  8 ++++----
>  lib/net/rte_ip.h              | 16 +++++++++++++++-
>  2 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/app/test/test_flow_classify.c
> b/app/test/test_flow_classify.c
> index 951606f248..4f64be5357 100644
> --- a/app/test/test_flow_classify.c
> +++ b/app/test/test_flow_classify.c
> @@ -95,7 +95,7 @@ static struct rte_acl_field_def
> ipv4_defs[NUM_FIELDS_IPV4] = {
>   *  dst mask 255.255.255.00 / udp src is 32 dst is 33 / end"
>   */
>  static struct rte_flow_item_ipv4 ipv4_udp_spec_1 = {
> -	{ 0, 0, 0, 0, 0, 0, IPPROTO_UDP, 0,
> +	{ { .version_ihl = 0}, 0, 0, 0, 0, 0, IPPROTO_UDP, 0,
>  	  RTE_IPV4(2, 2, 2, 3), RTE_IPV4(2, 2, 2, 7)}
>  };
>  static const struct rte_flow_item_ipv4 ipv4_mask_24 = {
> @@ -131,7 +131,7 @@ static struct rte_flow_item  end_item = {
> RTE_FLOW_ITEM_TYPE_END,
>   *  dst mask 255.255.255.00 / tcp src is 16 dst is 17 / end"
>   */
>  static struct rte_flow_item_ipv4 ipv4_tcp_spec_1 = {
> -	{ 0, 0, 0, 0, 0, 0, IPPROTO_TCP, 0,
> +	{ { .version_ihl = 0}, 0, 0, 0, 0, 0, IPPROTO_TCP, 0,
>  	  RTE_IPV4(1, 2, 3, 4), RTE_IPV4(5, 6, 7, 8)}
>  };
> 
> @@ -150,8 +150,8 @@ static struct rte_flow_item  tcp_item_1 = {
> RTE_FLOW_ITEM_TYPE_TCP,
>   *  dst mask 255.255.255.00 / sctp src is 16 dst is 17/ end"
>   */
>  static struct rte_flow_item_ipv4 ipv4_sctp_spec_1 = {
> -	{ 0, 0, 0, 0, 0, 0, IPPROTO_SCTP, 0, RTE_IPV4(11, 12, 13, 14),
> -	RTE_IPV4(15, 16, 17, 18)}
> +	{ { .version_ihl = 0}, 0, 0, 0, 0, 0, IPPROTO_SCTP, 0,
> +	RTE_IPV4(11, 12, 13, 14), RTE_IPV4(15, 16, 17, 18)}
>  };
> 
>  static struct rte_flow_item_sctp sctp_spec_1 = {
> diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h
> index 4b728969c1..684bb028b2 100644
> --- a/lib/net/rte_ip.h
> +++ b/lib/net/rte_ip.h
> @@ -38,7 +38,21 @@ extern "C" {
>   * IPv4 Header
>   */
>  struct rte_ipv4_hdr {
> -	uint8_t  version_ihl;		/**< version and header length */
> +	__extension__
> +	union {
> +		uint8_t version_ihl;    /**< version and header length */
> +		struct {
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +			uint8_t ihl:4;
> +			uint8_t version:4;
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +			uint8_t version:4;
> +			uint8_t ihl:4;
> +#else
> +#error "setup endian definition"
> +#endif
> +		};
> +	};
>  	uint8_t  type_of_service;	/**< type of service */
>  	rte_be16_t total_length;	/**< length of packet */
>  	rte_be16_t packet_id;		/**< packet ID */
> --
> 2.31.1
> 

This does not break the ABI, but it could be discussed if it breaks the API due to the required structure initialization changes shown in test_flow_classify.c. I think this patch is an improvement, and that such structure modifications should be generally accepted, so:

Acked-by: Morten Brørup <mb at smartsharesystems.com>