app/testpmd: fix mpls encap ipv4 version and ihl
Checks
Commit Message
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(+)
Comments
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
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
>
@@ -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 = {
@@ -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.