Re: [dpdk-dev] [PATCH v7 2/2] net: introduce IPv4 ihl and version fields

[Ajit Khaparde <ajit.khaparde@broadcom.com>]

On Thu, Oct 14, 2021 at 10:42 AM Gregory Etelson <getelson@nvidia.com> wrote:
>
> 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@nvidia.com>
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
> Acked-by: Ray Kinsella <mdr@ashroe.eu>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

> ---
> v2: Add dependency.
> v3: Add comments.
> v4: Update release notes.
> v5: Remove deprecation notice.
>     Update the patch comment.
> v6: split the patch into 2 parts:
>     1: update the announce.
>     2: implement the IPv4 header changes.
> v7: Remove `#else`.
> ---
>  app/test/test_flow_classify.c          |  8 ++++----
>  doc/guides/rel_notes/deprecation.rst   |  3 ---
>  doc/guides/rel_notes/release_21_11.rst |  3 +++
>  lib/net/rte_ip.h                       | 14 +++++++++++++-
>  4 files changed, 20 insertions(+), 8 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/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 841653fe30..d1f3faac39 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -158,9 +158,6 @@ Deprecation Notices
>    consistent with existing outer header checksum status flag naming, which
>    should help in reducing confusion about its usage.
>
> -* net: The structure ``rte_ipv4_hdr`` will have a union for
> -  existing ``version_ihl`` byte and new bitfield for ``version`` and ``ihl``.
> -
>  * vhost: ``rte_vdpa_register_device``, ``rte_vdpa_unregister_device``,
>    ``rte_vhost_host_notifier_ctrl`` and ``rte_vdpa_relay_vring_used`` vDPA
>    driver interface will be marked as internal in DPDK v21.11.
> diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
> index e1726774ba..fa60fd9a85 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -216,6 +216,9 @@ API Changes
>    the crypto/security operation. This field will be used to communicate
>    events such as soft expiry with IPsec in lookaside mode.
>
> +* net: Add ``version`` and ``ihl`` bit-fields to ``struct rte_ipv4_hdr``.
> +  Existing ``version_ihl`` field was kept for backward compatibility.
> +
>
>  ABI Changes
>  -----------
> diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h
> index b3d45e85db..0ef2430607 100644
> --- a/lib/net/rte_ip.h
> +++ b/lib/net/rte_ip.h
> @@ -39,7 +39,19 @@ 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;     /**< header length */
> +                       uint8_t version:4; /**< version */
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +                       uint8_t version:4; /**< version */
> +                       uint8_t ihl:4;     /**< header length */
> +#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.33.0
>