Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: add NVGRE encap/decap support

[Ori Kam <orika@mellanox.com>]

Small comment,

> -----Original Message-----
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> ---
>  app/test-pmd/cmdline.c                      | 113 +++++++++++++++++
>  app/test-pmd/cmdline_flow.c                 | 129 ++++++++++++++++++++
>  app/test-pmd/testpmd.c                      |  15 +++
>  app/test-pmd/testpmd.h                      |  15 +++
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  14 +++
>  5 files changed, 286 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 93573606f..711914e53 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> +
> +static void cmd_set_nvgre_parsed(void *parsed_result,
> +	__attribute__((unused)) struct cmdline *cl,
> +	__attribute__((unused)) void *data)
> +{
> +	struct cmd_set_nvgre_result *res = parsed_result;
> +	uint32_t tni = rte_cpu_to_be_32(res->tni) >> 8;

Is this also correct in case of big endian system? 
I think it will  remove part of the tni.

> +
> +	if (strcmp(res->nvgre, "nvgre") == 0)
> +		nvgre_encap_conf.select_vlan = 0;
> +	else if (strcmp(res->nvgre, "nvgre-with-vlan") == 0)
> +		nvgre_encap_conf.select_vlan = 1;
> +	if (strcmp(res->ip_version, "ipv4") == 0)
> +		nvgre_encap_conf.select_ipv4 = 1;
> +	else if (strcmp(res->ip_version, "ipv6") == 0)
> +		nvgre_encap_conf.select_ipv4 = 0;
> +	else
> +		return;
> +	memcpy(nvgre_encap_conf.tni, &tni, 3);

I don't think this will work as expected in big endian system.

> +	if (nvgre_encap_conf.select_ipv4) {
> +		IPV4_ADDR_TO_UINT(res->ip_src,
> nvgre_encap_conf.ipv4_src);
> +		IPV4_ADDR_TO_UINT(res->ip_dst,
> nvgre_encap_conf.ipv4_dst);
> +	} else {
> +		IPV6_ADDR_TO_ARRAY(res->ip_src,
> nvgre_encap_conf.ipv6_src);
> +		IPV6_ADDR_TO_ARRAY(res->ip_dst,
> nvgre_encap_conf.ipv6_dst);
> +	}
> +	if (nvgre_encap_conf.select_vlan)
> +		nvgre_encap_conf.vlan_tci = rte_cpu_to_be_16(res->tci);
> +	memcpy(nvgre_encap_conf.eth_src, res->eth_src.addr_bytes,
> +	       ETHER_ADDR_LEN);
> +	memcpy(nvgre_encap_conf.eth_dst, res->eth_dst.addr_bytes,
> +	       ETHER_ADDR_LEN);
> +}
> +
> --
> 2.17.1


Best,
Ori