[dpdk-dev] [PATCH] app/testpmd: fix mpls encap ipv4 version and ihl

[dpdk-dev] [PATCH] app/testpmd: fix mpls encap ipv4 version and ihl

[Saleh Alsouqi]

According to ietf rfc791 (see [1])

    "The Version field indicates the format of the
    internet header."

    "Internet Header Length (ihl) is the length of the
    internet header in 32 bit words, and thus points
    to the beginning of the data. Note that
    the minimum value for a correct header is 5."

Having version and ihl set to 0 would result in a
corrupted/incorrect encapsulating packet for MPLS,
this commit sets these fields to 4 and 5 respectively
as a default value.

[1] https://tools.ietf.org/html/rfc791

Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
Fixes: a1191d39cb57 ("app/testpmd: add MPLSoUDP encapsulation")
Cc: stable@dpdk.org

Signed-off-by: Saleh Alsouqi <salehals@mellanox.com>
---
 app/test-pmd/cmdline_flow.c | 2 ++
 app/test-pmd/testpmd.h      | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 5c0108f..c1c5fe3 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -3735,6 +3735,7 @@ static int comp_vc_action_rss_queue(struct context *, const struct token *,
 			.src_addr = mplsogre_encap_conf.ipv4_src,
 			.dst_addr = mplsogre_encap_conf.ipv4_dst,
 			.next_proto_id = IPPROTO_GRE,
+			.version_ihl = IPV4_VHL_DEF,
 		},
 	};
 	struct rte_flow_item_ipv6 ipv6 = {
@@ -3923,6 +3924,7 @@ static int comp_vc_action_rss_queue(struct context *, const struct token *,
 			.src_addr = mplsoudp_encap_conf.ipv4_src,
 			.dst_addr = mplsoudp_encap_conf.ipv4_dst,
 			.next_proto_id = IPPROTO_UDP,
+			.version_ihl = IPV4_VHL_DEF,
 		},
 	};
 	struct rte_flow_item_ipv6 ipv6 = {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 3ff11e6..8b0654f 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -22,6 +22,8 @@
 #define RTE_PORT_CLOSED         (uint16_t)2
 #define RTE_PORT_HANDLING       (uint16_t)3
 
+#define IPV4_VHL_DEF (0x40 | 0x5)
+
 /*
  * It is used to allocate the memory for hash key.
  * The hash key size is NIC dependent.
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH] app/testpmd: fix mpls encap ipv4 version and ihl

[Rami Rosen]

Hi Saleh,
Thanks for your patch.
If you will grep into the DPDK repo you will see that there are several
occurrences of IP_VHL_DEF, with the same functionality.
I would consider putting the IPV4_VHL_DEF you added in a more generic
location and not
under testpmd, so other modules will be able to use it without the need to
include testpmd.h, which is a bit awkard.
Another question is whether it should be IPV4_VHL_DEF or IPV_VHL_DEF as in
other DPDK apps. Probably other apps should be adjusted to use it.

Regards,
Rami Rosen

Re: [dpdk-dev] [PATCH] app/testpmd: fix mpls encap ipv4 version and ihl

[Ferruh Yigit]

On 12/25/2018 7:29 PM, Rami Rosen wrote:
> Hi Saleh,
> Thanks for your patch.
> If you will grep into the DPDK repo you will see that there are several
> occurrences of IP_VHL_DEF, with the same functionality.
> I would consider putting the IPV4_VHL_DEF you added in a more generic
> location and not
> under testpmd, so other modules will be able to use it without the need to
> include testpmd.h, which is a bit awkard.
> Another question is whether it should be IPV4_VHL_DEF or IPV_VHL_DEF as in
> other DPDK apps. Probably other apps should be adjusted to use it.

+1.

Why not put IPV4_VERSION (IPV6_VERSION), IPV4_DEFAULT_HDRLEN & IPV4_VHL_DEF to
'librte_net/rte_ip.h' and update libs/apps to use them? cc'ed Olivier.

And instead of defining ourselves, which we are trying to get rid of, is there
any system ones that we can re-use?

> 
> Regards,
> Rami Rosen
> 

[dpdk-dev] [PATCH v2] app/testpmd: fix mpls encap ipv4 version and ihl

[Saleh Alsouqi]

According to ietf rfc791 (see [1])

    "The Version field indicates the format of the
    internet header."

    "Internet Header Length (ihl) is the length of the
    internet header in 32 bit words, and thus points
    to the beginning of the data. Note that
    the minimum value for a correct header is 5."

Having version and ihl set to 0 would result in a
corrupted/incorrect encapsulating packet for MPLS,
this commit sets these fields to 4 and 5 respectively
as a default value.

[1] https://tools.ietf.org/html/rfc791

Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
Fixes: a1191d39cb57 ("app/testpmd: add MPLSoUDP encapsulation")
Cc: stable@dpdk.org

Signed-off-by: Saleh Alsouqi <salehals@mellanox.com>
---
v2:
        - Include netinet/ip.h in librte_net/rte_ip.h
          to use existing definitions instead of defining
          our own.

        - Move definitions from testpmd.h to librte_net/rte_ip.h
---
 app/test-pmd/cmdline_flow.c | 6 ++++++
 lib/librte_net/rte_ip.h     | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 36659a64a..f536307c9 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -3767,11 +3767,14 @@ parse_vc_action_mplsogre_encap(struct context *ctx, const struct token *token,
 			.src_addr = mplsogre_encap_conf.ipv4_src,
 			.dst_addr = mplsogre_encap_conf.ipv4_dst,
 			.next_proto_id = IPPROTO_GRE,
+			.version_ihl = IPV4_VHL_DEF,
+			.time_to_live = IPDEFTTL,
 		},
 	};
 	struct rte_flow_item_ipv6 ipv6 = {
 		.hdr =  {
 			.proto = IPPROTO_GRE,
+			.hop_limits = IPDEFTTL,
 		},
 	};
 	struct rte_flow_item_gre gre = {
@@ -3955,11 +3958,14 @@ parse_vc_action_mplsoudp_encap(struct context *ctx, const struct token *token,
 			.src_addr = mplsoudp_encap_conf.ipv4_src,
 			.dst_addr = mplsoudp_encap_conf.ipv4_dst,
 			.next_proto_id = IPPROTO_UDP,
+			.version_ihl = IPV4_VHL_DEF,
+			.time_to_live = IPDEFTTL,
 		},
 	};
 	struct rte_flow_item_ipv6 ipv6 = {
 		.hdr =  {
 			.proto = IPPROTO_UDP,
+			.hop_limits = IPDEFTTL,
 		},
 	};
 	struct rte_flow_item_udp udp = {
diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index f9b909090..499a2f79c 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -17,6 +17,7 @@
 
 #include <stdint.h>
 #include <netinet/in.h>
+#include <netinet/ip.h>
 
 #include <rte_byteorder.h>
 #include <rte_mbuf.h>
@@ -89,6 +90,10 @@ struct ipv4_hdr {
 #define IS_IPV4_MCAST(x) \
 	((x) >= IPV4_MIN_MCAST && (x) <= IPV4_MAX_MCAST) /**< check if IPv4 address is multicast */
 
+/* IPv4 default fields values */
+#define IPV4_MIN_IHL    (0x5)
+#define IPV4_VHL_DEF    (IPVERSION | IPV4_MIN_IHL)
+
 /**
  * @internal Calculate a sum of all words in the buffer.
  * Helper routine for the rte_raw_cksum().
-- 
2.17.1

[dpdk-dev] [PATCH v2] app/testpmd: fix mpls encap ipv4 version and ihl

[Saleh Alsouqi]

According to ietf rfc791 (see [1])

    "The Version field indicates the format of the
    internet header."

    "Internet Header Length (ihl) is the length of the
    internet header in 32 bit words, and thus points
    to the beginning of the data. Note that
    the minimum value for a correct header is 5."

Having version and ihl set to 0 would result in a
corrupted/incorrect encapsulating packet for MPLS,
this commit sets these fields to 4 and 5 respectively
as a default value.

[1] https://tools.ietf.org/html/rfc791

Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
Fixes: a1191d39cb57 ("app/testpmd: add MPLSoUDP encapsulation")
Cc: stable@dpdk.org

Signed-off-by: Saleh Alsouqi <salehals@mellanox.com>
---
v2:
        - Include netinet/ip.h in librte_net/rte_ip.h
          to use existing definitions instead of defining
          our own.

        - Move definitions from testpmd.h to librte_net/rte_ip.h
---
 app/test-pmd/cmdline_flow.c | 6 ++++++
 lib/librte_net/rte_ip.h     | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 36659a64a..f536307c9 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -3767,11 +3767,14 @@ parse_vc_action_mplsogre_encap(struct context *ctx, const struct token *token,
 			.src_addr = mplsogre_encap_conf.ipv4_src,
 			.dst_addr = mplsogre_encap_conf.ipv4_dst,
 			.next_proto_id = IPPROTO_GRE,
+			.version_ihl = IPV4_VHL_DEF,
+			.time_to_live = IPDEFTTL,
 		},
 	};
 	struct rte_flow_item_ipv6 ipv6 = {
 		.hdr =  {
 			.proto = IPPROTO_GRE,
+			.hop_limits = IPDEFTTL,
 		},
 	};
 	struct rte_flow_item_gre gre = {
@@ -3955,11 +3958,14 @@ parse_vc_action_mplsoudp_encap(struct context *ctx, const struct token *token,
 			.src_addr = mplsoudp_encap_conf.ipv4_src,
 			.dst_addr = mplsoudp_encap_conf.ipv4_dst,
 			.next_proto_id = IPPROTO_UDP,
+			.version_ihl = IPV4_VHL_DEF,
+			.time_to_live = IPDEFTTL,
 		},
 	};
 	struct rte_flow_item_ipv6 ipv6 = {
 		.hdr =  {
 			.proto = IPPROTO_UDP,
+			.hop_limits = IPDEFTTL,
 		},
 	};
 	struct rte_flow_item_udp udp = {
diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index f9b909090..499a2f79c 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -17,6 +17,7 @@
 
 #include <stdint.h>
 #include <netinet/in.h>
+#include <netinet/ip.h>
 
 #include <rte_byteorder.h>
 #include <rte_mbuf.h>
@@ -89,6 +90,10 @@ struct ipv4_hdr {
 #define IS_IPV4_MCAST(x) \
 	((x) >= IPV4_MIN_MCAST && (x) <= IPV4_MAX_MCAST) /**< check if IPv4 address is multicast */
 
+/* IPv4 default fields values */
+#define IPV4_MIN_IHL    (0x5)
+#define IPV4_VHL_DEF    (IPVERSION | IPV4_MIN_IHL)
+
 /**
  * @internal Calculate a sum of all words in the buffer.
  * Helper routine for the rte_raw_cksum().
-- 
2.17.1

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] app/testpmd: fix mpls encap ipv4 version and ihl

[Ferruh Yigit]

On 3/25/2019 1:01 PM, Saleh Alsouqi wrote:
> According to ietf rfc791 (see [1])
> 
>     "The Version field indicates the format of the
>     internet header."
> 
>     "Internet Header Length (ihl) is the length of the
>     internet header in 32 bit words, and thus points
>     to the beginning of the data. Note that
>     the minimum value for a correct header is 5."
> 
> Having version and ihl set to 0 would result in a
> corrupted/incorrect encapsulating packet for MPLS,
> this commit sets these fields to 4 and 5 respectively
> as a default value.
> 
> [1] https://tools.ietf.org/html/rfc791
> 
> Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
> Fixes: a1191d39cb57 ("app/testpmd: add MPLSoUDP encapsulation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Saleh Alsouqi <salehals@mellanox.com>
> ---
> v2:
>         - Include netinet/ip.h in librte_net/rte_ip.h
>           to use existing definitions instead of defining
>           our own.
> 
>         - Move definitions from testpmd.h to librte_net/rte_ip.h
> ---
>  app/test-pmd/cmdline_flow.c | 6 ++++++
>  lib/librte_net/rte_ip.h     | 5 +++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 36659a64a..f536307c9 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -3767,11 +3767,14 @@ parse_vc_action_mplsogre_encap(struct context *ctx, const struct token *token,
>  			.src_addr = mplsogre_encap_conf.ipv4_src,
>  			.dst_addr = mplsogre_encap_conf.ipv4_dst,
>  			.next_proto_id = IPPROTO_GRE,
> +			.version_ihl = IPV4_VHL_DEF,
> +			.time_to_live = IPDEFTTL,
>  		},
>  	};
>  	struct rte_flow_item_ipv6 ipv6 = {
>  		.hdr =  {
>  			.proto = IPPROTO_GRE,
> +			.hop_limits = IPDEFTTL,
>  		},
>  	};
>  	struct rte_flow_item_gre gre = {
> @@ -3955,11 +3958,14 @@ parse_vc_action_mplsoudp_encap(struct context *ctx, const struct token *token,
>  			.src_addr = mplsoudp_encap_conf.ipv4_src,
>  			.dst_addr = mplsoudp_encap_conf.ipv4_dst,
>  			.next_proto_id = IPPROTO_UDP,
> +			.version_ihl = IPV4_VHL_DEF,
> +			.time_to_live = IPDEFTTL,
>  		},
>  	};
>  	struct rte_flow_item_ipv6 ipv6 = {
>  		.hdr =  {
>  			.proto = IPPROTO_UDP,
> +			.hop_limits = IPDEFTTL,
>  		},
>  	};
>  	struct rte_flow_item_udp udp = {
> diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
> index f9b909090..499a2f79c 100644
> --- a/lib/librte_net/rte_ip.h
> +++ b/lib/librte_net/rte_ip.h
> @@ -17,6 +17,7 @@
>  
>  #include <stdint.h>
>  #include <netinet/in.h>
> +#include <netinet/ip.h>
>  
>  #include <rte_byteorder.h>
>  #include <rte_mbuf.h>
> @@ -89,6 +90,10 @@ struct ipv4_hdr {
>  #define IS_IPV4_MCAST(x) \
>  	((x) >= IPV4_MIN_MCAST && (x) <= IPV4_MAX_MCAST) /**< check if IPv4 address is multicast */
>  
> +/* IPv4 default fields values */
> +#define IPV4_MIN_IHL    (0x5)
> +#define IPV4_VHL_DEF    (IPVERSION | IPV4_MIN_IHL)

There are existing defines for this [1], since we have one in public header, can
you please replace them too,
perhaps in two patches, first one introduces this define and replaces old ones,
second one fixes the mpls issue?

[1]
$ git grep VHL | grep define
app/test-pmd/csumonly.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
app/test-pmd/flowgen.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
app/test-pmd/txonly.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
app/test/packet_burst_generator.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
examples/tep_termination/vxlan_setup.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)


> +
>  /**
>   * @internal Calculate a sum of all words in the buffer.
>   * Helper routine for the rte_raw_cksum().
> 

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] app/testpmd: fix mpls encap ipv4 version and ihl

[Ferruh Yigit]

On 3/25/2019 1:01 PM, Saleh Alsouqi wrote:
> According to ietf rfc791 (see [1])
> 
>     "The Version field indicates the format of the
>     internet header."
> 
>     "Internet Header Length (ihl) is the length of the
>     internet header in 32 bit words, and thus points
>     to the beginning of the data. Note that
>     the minimum value for a correct header is 5."
> 
> Having version and ihl set to 0 would result in a
> corrupted/incorrect encapsulating packet for MPLS,
> this commit sets these fields to 4 and 5 respectively
> as a default value.
> 
> [1] https://tools.ietf.org/html/rfc791
> 
> Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
> Fixes: a1191d39cb57 ("app/testpmd: add MPLSoUDP encapsulation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Saleh Alsouqi <salehals@mellanox.com>
> ---
> v2:
>         - Include netinet/ip.h in librte_net/rte_ip.h
>           to use existing definitions instead of defining
>           our own.
> 
>         - Move definitions from testpmd.h to librte_net/rte_ip.h
> ---
>  app/test-pmd/cmdline_flow.c | 6 ++++++
>  lib/librte_net/rte_ip.h     | 5 +++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 36659a64a..f536307c9 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -3767,11 +3767,14 @@ parse_vc_action_mplsogre_encap(struct context *ctx, const struct token *token,
>  			.src_addr = mplsogre_encap_conf.ipv4_src,
>  			.dst_addr = mplsogre_encap_conf.ipv4_dst,
>  			.next_proto_id = IPPROTO_GRE,
> +			.version_ihl = IPV4_VHL_DEF,
> +			.time_to_live = IPDEFTTL,
>  		},
>  	};
>  	struct rte_flow_item_ipv6 ipv6 = {
>  		.hdr =  {
>  			.proto = IPPROTO_GRE,
> +			.hop_limits = IPDEFTTL,
>  		},
>  	};
>  	struct rte_flow_item_gre gre = {
> @@ -3955,11 +3958,14 @@ parse_vc_action_mplsoudp_encap(struct context *ctx, const struct token *token,
>  			.src_addr = mplsoudp_encap_conf.ipv4_src,
>  			.dst_addr = mplsoudp_encap_conf.ipv4_dst,
>  			.next_proto_id = IPPROTO_UDP,
> +			.version_ihl = IPV4_VHL_DEF,
> +			.time_to_live = IPDEFTTL,
>  		},
>  	};
>  	struct rte_flow_item_ipv6 ipv6 = {
>  		.hdr =  {
>  			.proto = IPPROTO_UDP,
> +			.hop_limits = IPDEFTTL,
>  		},
>  	};
>  	struct rte_flow_item_udp udp = {
> diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
> index f9b909090..499a2f79c 100644
> --- a/lib/librte_net/rte_ip.h
> +++ b/lib/librte_net/rte_ip.h
> @@ -17,6 +17,7 @@
>  
>  #include <stdint.h>
>  #include <netinet/in.h>
> +#include <netinet/ip.h>
>  
>  #include <rte_byteorder.h>
>  #include <rte_mbuf.h>
> @@ -89,6 +90,10 @@ struct ipv4_hdr {
>  #define IS_IPV4_MCAST(x) \
>  	((x) >= IPV4_MIN_MCAST && (x) <= IPV4_MAX_MCAST) /**< check if IPv4 address is multicast */
>  
> +/* IPv4 default fields values */
> +#define IPV4_MIN_IHL    (0x5)
> +#define IPV4_VHL_DEF    (IPVERSION | IPV4_MIN_IHL)

There are existing defines for this [1], since we have one in public header, can
you please replace them too,
perhaps in two patches, first one introduces this define and replaces old ones,
second one fixes the mpls issue?

[1]
$ git grep VHL | grep define
app/test-pmd/csumonly.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
app/test-pmd/flowgen.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
app/test-pmd/txonly.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
app/test/packet_burst_generator.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
examples/tep_termination/vxlan_setup.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)


> +
>  /**
>   * @internal Calculate a sum of all words in the buffer.
>   * Helper routine for the rte_raw_cksum().
> 

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] app/testpmd: fix mpls encap ipv4 version and ihl

[Ferruh Yigit]

On 3/25/2019 1:30 PM, Ferruh Yigit wrote:
> On 3/25/2019 1:01 PM, Saleh Alsouqi wrote:
>> According to ietf rfc791 (see [1])
>>
>>     "The Version field indicates the format of the
>>     internet header."
>>
>>     "Internet Header Length (ihl) is the length of the
>>     internet header in 32 bit words, and thus points
>>     to the beginning of the data. Note that
>>     the minimum value for a correct header is 5."
>>
>> Having version and ihl set to 0 would result in a
>> corrupted/incorrect encapsulating packet for MPLS,
>> this commit sets these fields to 4 and 5 respectively
>> as a default value.
>>
>> [1] https://tools.ietf.org/html/rfc791
>>
>> Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
>> Fixes: a1191d39cb57 ("app/testpmd: add MPLSoUDP encapsulation")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Saleh Alsouqi <salehals@mellanox.com>
>> ---
>> v2:
>>         - Include netinet/ip.h in librte_net/rte_ip.h
>>           to use existing definitions instead of defining
>>           our own.
>>
>>         - Move definitions from testpmd.h to librte_net/rte_ip.h
>> ---
>>  app/test-pmd/cmdline_flow.c | 6 ++++++
>>  lib/librte_net/rte_ip.h     | 5 +++++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
>> index 36659a64a..f536307c9 100644
>> --- a/app/test-pmd/cmdline_flow.c
>> +++ b/app/test-pmd/cmdline_flow.c
>> @@ -3767,11 +3767,14 @@ parse_vc_action_mplsogre_encap(struct context *ctx, const struct token *token,
>>  			.src_addr = mplsogre_encap_conf.ipv4_src,
>>  			.dst_addr = mplsogre_encap_conf.ipv4_dst,
>>  			.next_proto_id = IPPROTO_GRE,
>> +			.version_ihl = IPV4_VHL_DEF,
>> +			.time_to_live = IPDEFTTL,
>>  		},
>>  	};
>>  	struct rte_flow_item_ipv6 ipv6 = {
>>  		.hdr =  {
>>  			.proto = IPPROTO_GRE,
>> +			.hop_limits = IPDEFTTL,
>>  		},
>>  	};
>>  	struct rte_flow_item_gre gre = {
>> @@ -3955,11 +3958,14 @@ parse_vc_action_mplsoudp_encap(struct context *ctx, const struct token *token,
>>  			.src_addr = mplsoudp_encap_conf.ipv4_src,
>>  			.dst_addr = mplsoudp_encap_conf.ipv4_dst,
>>  			.next_proto_id = IPPROTO_UDP,
>> +			.version_ihl = IPV4_VHL_DEF,
>> +			.time_to_live = IPDEFTTL,
>>  		},
>>  	};
>>  	struct rte_flow_item_ipv6 ipv6 = {
>>  		.hdr =  {
>>  			.proto = IPPROTO_UDP,
>> +			.hop_limits = IPDEFTTL,
>>  		},
>>  	};
>>  	struct rte_flow_item_udp udp = {
>> diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
>> index f9b909090..499a2f79c 100644
>> --- a/lib/librte_net/rte_ip.h
>> +++ b/lib/librte_net/rte_ip.h
>> @@ -17,6 +17,7 @@
>>  
>>  #include <stdint.h>
>>  #include <netinet/in.h>
>> +#include <netinet/ip.h>
>>  
>>  #include <rte_byteorder.h>
>>  #include <rte_mbuf.h>
>> @@ -89,6 +90,10 @@ struct ipv4_hdr {
>>  #define IS_IPV4_MCAST(x) \
>>  	((x) >= IPV4_MIN_MCAST && (x) <= IPV4_MAX_MCAST) /**< check if IPv4 address is multicast */
>>  
>> +/* IPv4 default fields values */
>> +#define IPV4_MIN_IHL    (0x5)
>> +#define IPV4_VHL_DEF    (IPVERSION | IPV4_MIN_IHL)
> 
> There are existing defines for this [1], since we have one in public header, can
> you please replace them too,
> perhaps in two patches, first one introduces this define and replaces old ones,
> second one fixes the mpls issue?
> 
> [1]
> $ git grep VHL | grep define
> app/test-pmd/csumonly.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
> app/test-pmd/flowgen.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
> app/test-pmd/txonly.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
> app/test/packet_burst_generator.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
> examples/tep_termination/vxlan_setup.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)

Let me get this as it is, and I will send another patch to update existing defines.

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] app/testpmd: fix mpls encap ipv4 version and ihl

[Ferruh Yigit]

On 3/25/2019 1:30 PM, Ferruh Yigit wrote:
> On 3/25/2019 1:01 PM, Saleh Alsouqi wrote:
>> According to ietf rfc791 (see [1])
>>
>>     "The Version field indicates the format of the
>>     internet header."
>>
>>     "Internet Header Length (ihl) is the length of the
>>     internet header in 32 bit words, and thus points
>>     to the beginning of the data. Note that
>>     the minimum value for a correct header is 5."
>>
>> Having version and ihl set to 0 would result in a
>> corrupted/incorrect encapsulating packet for MPLS,
>> this commit sets these fields to 4 and 5 respectively
>> as a default value.
>>
>> [1] https://tools.ietf.org/html/rfc791
>>
>> Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
>> Fixes: a1191d39cb57 ("app/testpmd: add MPLSoUDP encapsulation")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Saleh Alsouqi <salehals@mellanox.com>
>> ---
>> v2:
>>         - Include netinet/ip.h in librte_net/rte_ip.h
>>           to use existing definitions instead of defining
>>           our own.
>>
>>         - Move definitions from testpmd.h to librte_net/rte_ip.h
>> ---
>>  app/test-pmd/cmdline_flow.c | 6 ++++++
>>  lib/librte_net/rte_ip.h     | 5 +++++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
>> index 36659a64a..f536307c9 100644
>> --- a/app/test-pmd/cmdline_flow.c
>> +++ b/app/test-pmd/cmdline_flow.c
>> @@ -3767,11 +3767,14 @@ parse_vc_action_mplsogre_encap(struct context *ctx, const struct token *token,
>>  			.src_addr = mplsogre_encap_conf.ipv4_src,
>>  			.dst_addr = mplsogre_encap_conf.ipv4_dst,
>>  			.next_proto_id = IPPROTO_GRE,
>> +			.version_ihl = IPV4_VHL_DEF,
>> +			.time_to_live = IPDEFTTL,
>>  		},
>>  	};
>>  	struct rte_flow_item_ipv6 ipv6 = {
>>  		.hdr =  {
>>  			.proto = IPPROTO_GRE,
>> +			.hop_limits = IPDEFTTL,
>>  		},
>>  	};
>>  	struct rte_flow_item_gre gre = {
>> @@ -3955,11 +3958,14 @@ parse_vc_action_mplsoudp_encap(struct context *ctx, const struct token *token,
>>  			.src_addr = mplsoudp_encap_conf.ipv4_src,
>>  			.dst_addr = mplsoudp_encap_conf.ipv4_dst,
>>  			.next_proto_id = IPPROTO_UDP,
>> +			.version_ihl = IPV4_VHL_DEF,
>> +			.time_to_live = IPDEFTTL,
>>  		},
>>  	};
>>  	struct rte_flow_item_ipv6 ipv6 = {
>>  		.hdr =  {
>>  			.proto = IPPROTO_UDP,
>> +			.hop_limits = IPDEFTTL,
>>  		},
>>  	};
>>  	struct rte_flow_item_udp udp = {
>> diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
>> index f9b909090..499a2f79c 100644
>> --- a/lib/librte_net/rte_ip.h
>> +++ b/lib/librte_net/rte_ip.h
>> @@ -17,6 +17,7 @@
>>  
>>  #include <stdint.h>
>>  #include <netinet/in.h>
>> +#include <netinet/ip.h>
>>  
>>  #include <rte_byteorder.h>
>>  #include <rte_mbuf.h>
>> @@ -89,6 +90,10 @@ struct ipv4_hdr {
>>  #define IS_IPV4_MCAST(x) \
>>  	((x) >= IPV4_MIN_MCAST && (x) <= IPV4_MAX_MCAST) /**< check if IPv4 address is multicast */
>>  
>> +/* IPv4 default fields values */
>> +#define IPV4_MIN_IHL    (0x5)
>> +#define IPV4_VHL_DEF    (IPVERSION | IPV4_MIN_IHL)
> 
> There are existing defines for this [1], since we have one in public header, can
> you please replace them too,
> perhaps in two patches, first one introduces this define and replaces old ones,
> second one fixes the mpls issue?
> 
> [1]
> $ git grep VHL | grep define
> app/test-pmd/csumonly.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
> app/test-pmd/flowgen.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
> app/test-pmd/txonly.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
> app/test/packet_burst_generator.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
> examples/tep_termination/vxlan_setup.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)

Let me get this as it is, and I will send another patch to update existing defines.

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] app/testpmd: fix mpls encap ipv4 version and ihl

[Ferruh Yigit]

On 4/16/2019 10:59 AM, Ferruh Yigit wrote:
> On 3/25/2019 1:30 PM, Ferruh Yigit wrote:
>> On 3/25/2019 1:01 PM, Saleh Alsouqi wrote:
>>> According to ietf rfc791 (see [1])
>>>
>>>     "The Version field indicates the format of the
>>>     internet header."
>>>
>>>     "Internet Header Length (ihl) is the length of the
>>>     internet header in 32 bit words, and thus points
>>>     to the beginning of the data. Note that
>>>     the minimum value for a correct header is 5."
>>>
>>> Having version and ihl set to 0 would result in a
>>> corrupted/incorrect encapsulating packet for MPLS,
>>> this commit sets these fields to 4 and 5 respectively
>>> as a default value.
>>>
>>> [1] https://tools.ietf.org/html/rfc791
>>>
>>> Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
>>> Fixes: a1191d39cb57 ("app/testpmd: add MPLSoUDP encapsulation")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Saleh Alsouqi <salehals@mellanox.com>
>>> ---
>>> v2:
>>>         - Include netinet/ip.h in librte_net/rte_ip.h
>>>           to use existing definitions instead of defining
>>>           our own.
>>>
>>>         - Move definitions from testpmd.h to librte_net/rte_ip.h
>>> ---
>>>  app/test-pmd/cmdline_flow.c | 6 ++++++
>>>  lib/librte_net/rte_ip.h     | 5 +++++
>>>  2 files changed, 11 insertions(+)
>>>
>>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
>>> index 36659a64a..f536307c9 100644
>>> --- a/app/test-pmd/cmdline_flow.c
>>> +++ b/app/test-pmd/cmdline_flow.c
>>> @@ -3767,11 +3767,14 @@ parse_vc_action_mplsogre_encap(struct context *ctx, const struct token *token,
>>>  			.src_addr = mplsogre_encap_conf.ipv4_src,
>>>  			.dst_addr = mplsogre_encap_conf.ipv4_dst,
>>>  			.next_proto_id = IPPROTO_GRE,
>>> +			.version_ihl = IPV4_VHL_DEF,
>>> +			.time_to_live = IPDEFTTL,
>>>  		},
>>>  	};
>>>  	struct rte_flow_item_ipv6 ipv6 = {
>>>  		.hdr =  {
>>>  			.proto = IPPROTO_GRE,
>>> +			.hop_limits = IPDEFTTL,
>>>  		},
>>>  	};
>>>  	struct rte_flow_item_gre gre = {
>>> @@ -3955,11 +3958,14 @@ parse_vc_action_mplsoudp_encap(struct context *ctx, const struct token *token,
>>>  			.src_addr = mplsoudp_encap_conf.ipv4_src,
>>>  			.dst_addr = mplsoudp_encap_conf.ipv4_dst,
>>>  			.next_proto_id = IPPROTO_UDP,
>>> +			.version_ihl = IPV4_VHL_DEF,
>>> +			.time_to_live = IPDEFTTL,
>>>  		},
>>>  	};
>>>  	struct rte_flow_item_ipv6 ipv6 = {
>>>  		.hdr =  {
>>>  			.proto = IPPROTO_UDP,
>>> +			.hop_limits = IPDEFTTL,
>>>  		},
>>>  	};
>>>  	struct rte_flow_item_udp udp = {
>>> diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
>>> index f9b909090..499a2f79c 100644
>>> --- a/lib/librte_net/rte_ip.h
>>> +++ b/lib/librte_net/rte_ip.h
>>> @@ -17,6 +17,7 @@
>>>  
>>>  #include <stdint.h>
>>>  #include <netinet/in.h>
>>> +#include <netinet/ip.h>
>>>  
>>>  #include <rte_byteorder.h>
>>>  #include <rte_mbuf.h>
>>> @@ -89,6 +90,10 @@ struct ipv4_hdr {
>>>  #define IS_IPV4_MCAST(x) \
>>>  	((x) >= IPV4_MIN_MCAST && (x) <= IPV4_MAX_MCAST) /**< check if IPv4 address is multicast */
>>>  
>>> +/* IPv4 default fields values */
>>> +#define IPV4_MIN_IHL    (0x5)
>>> +#define IPV4_VHL_DEF    (IPVERSION | IPV4_MIN_IHL)
>>
>> There are existing defines for this [1], since we have one in public header, can
>> you please replace them too,
>> perhaps in two patches, first one introduces this define and replaces old ones,
>> second one fixes the mpls issue?
>>
>> [1]
>> $ git grep VHL | grep define
>> app/test-pmd/csumonly.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
>> app/test-pmd/flowgen.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
>> app/test-pmd/txonly.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
>> app/test/packet_burst_generator.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
>> examples/tep_termination/vxlan_setup.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
> 
> Let me get this as it is, and I will send another patch to update existing defines.
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 

Ahh, this seems causing build errors in FreeBSD:
http://mails.dpdk.org/archives/test-report/2019-March/077811.html

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] app/testpmd: fix mpls encap ipv4 version and ihl

[Ferruh Yigit]

On 4/16/2019 10:59 AM, Ferruh Yigit wrote:
> On 3/25/2019 1:30 PM, Ferruh Yigit wrote:
>> On 3/25/2019 1:01 PM, Saleh Alsouqi wrote:
>>> According to ietf rfc791 (see [1])
>>>
>>>     "The Version field indicates the format of the
>>>     internet header."
>>>
>>>     "Internet Header Length (ihl) is the length of the
>>>     internet header in 32 bit words, and thus points
>>>     to the beginning of the data. Note that
>>>     the minimum value for a correct header is 5."
>>>
>>> Having version and ihl set to 0 would result in a
>>> corrupted/incorrect encapsulating packet for MPLS,
>>> this commit sets these fields to 4 and 5 respectively
>>> as a default value.
>>>
>>> [1] https://tools.ietf.org/html/rfc791
>>>
>>> Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
>>> Fixes: a1191d39cb57 ("app/testpmd: add MPLSoUDP encapsulation")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Saleh Alsouqi <salehals@mellanox.com>
>>> ---
>>> v2:
>>>         - Include netinet/ip.h in librte_net/rte_ip.h
>>>           to use existing definitions instead of defining
>>>           our own.
>>>
>>>         - Move definitions from testpmd.h to librte_net/rte_ip.h
>>> ---
>>>  app/test-pmd/cmdline_flow.c | 6 ++++++
>>>  lib/librte_net/rte_ip.h     | 5 +++++
>>>  2 files changed, 11 insertions(+)
>>>
>>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
>>> index 36659a64a..f536307c9 100644
>>> --- a/app/test-pmd/cmdline_flow.c
>>> +++ b/app/test-pmd/cmdline_flow.c
>>> @@ -3767,11 +3767,14 @@ parse_vc_action_mplsogre_encap(struct context *ctx, const struct token *token,
>>>  			.src_addr = mplsogre_encap_conf.ipv4_src,
>>>  			.dst_addr = mplsogre_encap_conf.ipv4_dst,
>>>  			.next_proto_id = IPPROTO_GRE,
>>> +			.version_ihl = IPV4_VHL_DEF,
>>> +			.time_to_live = IPDEFTTL,
>>>  		},
>>>  	};
>>>  	struct rte_flow_item_ipv6 ipv6 = {
>>>  		.hdr =  {
>>>  			.proto = IPPROTO_GRE,
>>> +			.hop_limits = IPDEFTTL,
>>>  		},
>>>  	};
>>>  	struct rte_flow_item_gre gre = {
>>> @@ -3955,11 +3958,14 @@ parse_vc_action_mplsoudp_encap(struct context *ctx, const struct token *token,
>>>  			.src_addr = mplsoudp_encap_conf.ipv4_src,
>>>  			.dst_addr = mplsoudp_encap_conf.ipv4_dst,
>>>  			.next_proto_id = IPPROTO_UDP,
>>> +			.version_ihl = IPV4_VHL_DEF,
>>> +			.time_to_live = IPDEFTTL,
>>>  		},
>>>  	};
>>>  	struct rte_flow_item_ipv6 ipv6 = {
>>>  		.hdr =  {
>>>  			.proto = IPPROTO_UDP,
>>> +			.hop_limits = IPDEFTTL,
>>>  		},
>>>  	};
>>>  	struct rte_flow_item_udp udp = {
>>> diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
>>> index f9b909090..499a2f79c 100644
>>> --- a/lib/librte_net/rte_ip.h
>>> +++ b/lib/librte_net/rte_ip.h
>>> @@ -17,6 +17,7 @@
>>>  
>>>  #include <stdint.h>
>>>  #include <netinet/in.h>
>>> +#include <netinet/ip.h>
>>>  
>>>  #include <rte_byteorder.h>
>>>  #include <rte_mbuf.h>
>>> @@ -89,6 +90,10 @@ struct ipv4_hdr {
>>>  #define IS_IPV4_MCAST(x) \
>>>  	((x) >= IPV4_MIN_MCAST && (x) <= IPV4_MAX_MCAST) /**< check if IPv4 address is multicast */
>>>  
>>> +/* IPv4 default fields values */
>>> +#define IPV4_MIN_IHL    (0x5)
>>> +#define IPV4_VHL_DEF    (IPVERSION | IPV4_MIN_IHL)
>>
>> There are existing defines for this [1], since we have one in public header, can
>> you please replace them too,
>> perhaps in two patches, first one introduces this define and replaces old ones,
>> second one fixes the mpls issue?
>>
>> [1]
>> $ git grep VHL | grep define
>> app/test-pmd/csumonly.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
>> app/test-pmd/flowgen.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
>> app/test-pmd/txonly.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
>> app/test/packet_burst_generator.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
>> examples/tep_termination/vxlan_setup.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
> 
> Let me get this as it is, and I will send another patch to update existing defines.
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 

Ahh, this seems causing build errors in FreeBSD:
http://mails.dpdk.org/archives/test-report/2019-March/077811.html

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] app/testpmd: fix mpls encap ipv4 version and ihl

[Yigit, Ferruh]

On 4/16/2019 11:02 AM, Ferruh Yigit wrote:
> On 4/16/2019 10:59 AM, Ferruh Yigit wrote:
>> On 3/25/2019 1:30 PM, Ferruh Yigit wrote:
>>> On 3/25/2019 1:01 PM, Saleh Alsouqi wrote:
>>>> According to ietf rfc791 (see [1])
>>>>
>>>>     "The Version field indicates the format of the
>>>>     internet header."
>>>>
>>>>     "Internet Header Length (ihl) is the length of the
>>>>     internet header in 32 bit words, and thus points
>>>>     to the beginning of the data. Note that
>>>>     the minimum value for a correct header is 5."
>>>>
>>>> Having version and ihl set to 0 would result in a
>>>> corrupted/incorrect encapsulating packet for MPLS,
>>>> this commit sets these fields to 4 and 5 respectively
>>>> as a default value.
>>>>
>>>> [1] https://tools.ietf.org/html/rfc791
>>>>
>>>> Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
>>>> Fixes: a1191d39cb57 ("app/testpmd: add MPLSoUDP encapsulation")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Saleh Alsouqi <salehals@mellanox.com>
>>>> ---
>>>> v2:
>>>>         - Include netinet/ip.h in librte_net/rte_ip.h
>>>>           to use existing definitions instead of defining
>>>>           our own.
>>>>
>>>>         - Move definitions from testpmd.h to librte_net/rte_ip.h
>>>> ---
>>>>  app/test-pmd/cmdline_flow.c | 6 ++++++
>>>>  lib/librte_net/rte_ip.h     | 5 +++++
>>>>  2 files changed, 11 insertions(+)
>>>>
>>>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
>>>> index 36659a64a..f536307c9 100644
>>>> --- a/app/test-pmd/cmdline_flow.c
>>>> +++ b/app/test-pmd/cmdline_flow.c
>>>> @@ -3767,11 +3767,14 @@ parse_vc_action_mplsogre_encap(struct context *ctx, const struct token *token,
>>>>  			.src_addr = mplsogre_encap_conf.ipv4_src,
>>>>  			.dst_addr = mplsogre_encap_conf.ipv4_dst,
>>>>  			.next_proto_id = IPPROTO_GRE,
>>>> +			.version_ihl = IPV4_VHL_DEF,
>>>> +			.time_to_live = IPDEFTTL,
>>>>  		},
>>>>  	};
>>>>  	struct rte_flow_item_ipv6 ipv6 = {
>>>>  		.hdr =  {
>>>>  			.proto = IPPROTO_GRE,
>>>> +			.hop_limits = IPDEFTTL,
>>>>  		},
>>>>  	};
>>>>  	struct rte_flow_item_gre gre = {
>>>> @@ -3955,11 +3958,14 @@ parse_vc_action_mplsoudp_encap(struct context *ctx, const struct token *token,
>>>>  			.src_addr = mplsoudp_encap_conf.ipv4_src,
>>>>  			.dst_addr = mplsoudp_encap_conf.ipv4_dst,
>>>>  			.next_proto_id = IPPROTO_UDP,
>>>> +			.version_ihl = IPV4_VHL_DEF,
>>>> +			.time_to_live = IPDEFTTL,
>>>>  		},
>>>>  	};
>>>>  	struct rte_flow_item_ipv6 ipv6 = {
>>>>  		.hdr =  {
>>>>  			.proto = IPPROTO_UDP,
>>>> +			.hop_limits = IPDEFTTL,
>>>>  		},
>>>>  	};
>>>>  	struct rte_flow_item_udp udp = {
>>>> diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
>>>> index f9b909090..499a2f79c 100644
>>>> --- a/lib/librte_net/rte_ip.h
>>>> +++ b/lib/librte_net/rte_ip.h
>>>> @@ -17,6 +17,7 @@
>>>>  
>>>>  #include <stdint.h>
>>>>  #include <netinet/in.h>
>>>> +#include <netinet/ip.h>
>>>>  
>>>>  #include <rte_byteorder.h>
>>>>  #include <rte_mbuf.h>
>>>> @@ -89,6 +90,10 @@ struct ipv4_hdr {
>>>>  #define IS_IPV4_MCAST(x) \
>>>>  	((x) >= IPV4_MIN_MCAST && (x) <= IPV4_MAX_MCAST) /**< check if IPv4 address is multicast */
>>>>  
>>>> +/* IPv4 default fields values */
>>>> +#define IPV4_MIN_IHL    (0x5)
>>>> +#define IPV4_VHL_DEF    (IPVERSION | IPV4_MIN_IHL)
>>>
>>> There are existing defines for this [1], since we have one in public header, can
>>> you please replace them too,
>>> perhaps in two patches, first one introduces this define and replaces old ones,
>>> second one fixes the mpls issue?
>>>
>>> [1]
>>> $ git grep VHL | grep define
>>> app/test-pmd/csumonly.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
>>> app/test-pmd/flowgen.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
>>> app/test-pmd/txonly.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
>>> app/test/packet_burst_generator.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
>>> examples/tep_termination/vxlan_setup.c:#define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
>>
>> Let me get this as it is, and I will send another patch to update existing defines.
>>
>> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>
> 
> Ahh, this seems causing build errors in FreeBSD:
> http://mails.dpdk.org/archives/test-report/2019-March/077811.html
> 
It seems it is easy to fix the BSD with following change:
 +#include <sys/types.h>

And now with the recent changes DPDK define should have RTE_ prefix, so new
defines "IPV4_MIN_IHL" && "IPV4_VHL_DEF" should have RTE_ prefix.

Can you able to send a new version with these changes, please let us know if you
can't, perhaps we take care of this since changes looks trivial.

[dpdk-dev] [PATCH v3 0/2] define ipv4 ihl and vhl

[Saleh Alsouqi]

v3:
        - Include sys/types.h in librte_net/rte_ip.h,
          to fix undefined types in freebsd systems.

        - Split commits into 2, definition in librte_net/rte_ip,
          and the usage in app/testpmd.

        - Add "RTE" prefix to the definitions names.

v2:
        - Include netinet/ip.h in librte_net/rte_ip.h
          to use existing definitions instead of defining
          our own.

        - Move definitions from testpmd.h to librte_net/rte_ip.h

Saleh AlSouqi (2):
  librte_net/rte_ip: define ipv4 ihl and vhl
  app/testpmd: fix mpls ipv4 encap fields

 app/test-pmd/cmdline_flow.c | 6 ++++++
 lib/librte_net/rte_ip.h     | 6 ++++++
 2 files changed, 12 insertions(+)

-- 
2.7.4

[dpdk-dev] [PATCH v3 1/2] librte_net/rte_ip: define ipv4 ihl and vhl

[Saleh Alsouqi]

Define IPv4 Minimum IHL and VHL according to rfc791 (see [1])

    "The Version field indicates the format of the
    internet header."

    "Internet Header Length (ihl) is the length of the
    internet header in 32 bit words, and thus points
    to the beginning of the data. Note that
    the minimum value for a correct header is 5."

[1] https://tools.ietf.org/html/rfc791

Signed-off-by: Saleh AlSouqi <salehals@mellanox.com>
---
 lib/librte_net/rte_ip.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index e69b863..26b089e 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -16,7 +16,9 @@
  */
 
 #include <stdint.h>
+#include <sys/types.h>
 #include <netinet/in.h>
+#include <netinet/ip.h>
 
 #include <rte_byteorder.h>
 #include <rte_mbuf.h>
@@ -96,6 +98,10 @@ struct rte_ipv4_hdr {
 	((x) >= RTE_IPV4_MIN_MCAST && (x) <= RTE_IPV4_MAX_MCAST)
 	/**< check if IPv4 address is multicast */
 
+/* IPv4 default fields values */
+#define RTE_IPV4_MIN_IHL    (0x5)
+#define RTE_IPV4_VHL_DEF    (IPVERSION | RTE_IPV4_MIN_IHL)
+
 /**
  * @internal Calculate a sum of all words in the buffer.
  * Helper routine for the rte_raw_cksum().
-- 
2.7.4

[dpdk-dev] [PATCH v3 2/2] app/testpmd: fix mpls ipv4 encap fields

[Saleh Alsouqi]

Having version and ihl set to 0 would result in a
corrupted/incorrect encapsulating packet for MPLS,
this commit sets these fields to 4 and 5 respectively
as defined in librte_net/rte_ip.h.

Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
Fixes: a1191d39cb57 ("app/testpmd: add MPLSoUDP encapsulation")
Cc: stable@dpdk.org

Signed-off-by: Saleh AlSouqi <salehals@mellanox.com>
---
 app/test-pmd/cmdline_flow.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index e644efa..4e5fa7a 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -3878,11 +3878,14 @@ parse_vc_action_mplsogre_encap(struct context *ctx, const struct token *token,
 			.src_addr = mplsogre_encap_conf.ipv4_src,
 			.dst_addr = mplsogre_encap_conf.ipv4_dst,
 			.next_proto_id = IPPROTO_GRE,
+			.version_ihl = RTE_IPV4_VHL_DEF,
+			.time_to_live = IPDEFTTL,
 		},
 	};
 	struct rte_flow_item_ipv6 ipv6 = {
 		.hdr =  {
 			.proto = IPPROTO_GRE,
+			.hop_limits = IPDEFTTL,
 		},
 	};
 	struct rte_flow_item_gre gre = {
@@ -4066,11 +4069,14 @@ parse_vc_action_mplsoudp_encap(struct context *ctx, const struct token *token,
 			.src_addr = mplsoudp_encap_conf.ipv4_src,
 			.dst_addr = mplsoudp_encap_conf.ipv4_dst,
 			.next_proto_id = IPPROTO_UDP,
+			.version_ihl = RTE_IPV4_VHL_DEF,
+			.time_to_live = IPDEFTTL,
 		},
 	};
 	struct rte_flow_item_ipv6 ipv6 = {
 		.hdr =  {
 			.proto = IPPROTO_UDP,
+			.hop_limits = IPDEFTTL,
 		},
 	};
 	struct rte_flow_item_udp udp = {
-- 
2.7.4

Re: [dpdk-dev] [dpdk-stable] [PATCH v3 0/2] define ipv4 ihl and vhl

[Ferruh Yigit]

On 7/4/2019 8:33 AM, Saleh Alsouqi wrote:
> v3:
>         - Include sys/types.h in librte_net/rte_ip.h,
>           to fix undefined types in freebsd systems.
> 
>         - Split commits into 2, definition in librte_net/rte_ip,
>           and the usage in app/testpmd.
> 
>         - Add "RTE" prefix to the definitions names.
> 
> v2:
>         - Include netinet/ip.h in librte_net/rte_ip.h
>           to use existing definitions instead of defining
>           our own.
> 
>         - Move definitions from testpmd.h to librte_net/rte_ip.h
> 
> Saleh AlSouqi (2):
>   librte_net/rte_ip: define ipv4 ihl and vhl
>   app/testpmd: fix mpls ipv4 encap fields

For series,
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Series applied to dpdk-next-net/master, thanks.