[dpdk-dev] [PATCH] l3fwd: fix incorrect size for destination port values

[dpdk-dev] [PATCH] l3fwd: fix incorrect size for destination port values

[Konstantin Ananyev]

Fixes: dc81ebbacaeb ("lpm: extend IPv4 next hop field")

Originally l3fwd used 16-bit value to store dest_port value.
To accommodate 24-bit nexthop dest_port was increased to 32-bit,
though some further packet processing code remained unchanged and
still expects dest_port to be 16-bit.
That is not correct and can cause l3fwd invalid behaviour or even
process crash/hang on some input packet patterns.
For the fix, I choose the simplest approach and restored dest_port
as 16-bit value, plus necessary conversions from 32 to 16 bit values
after lpm_lookupx4.

Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
 examples/l3fwd/l3fwd_em_hlm_sse.h |  6 +++---
 examples/l3fwd/l3fwd_em_sse.h     |  2 +-
 examples/l3fwd/l3fwd_lpm.h        |  4 ++--
 examples/l3fwd/l3fwd_lpm_sse.h    | 12 ++++++++----
 examples/l3fwd/l3fwd_sse.h        |  8 ++++----
 5 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/examples/l3fwd/l3fwd_em_hlm_sse.h b/examples/l3fwd/l3fwd_em_hlm_sse.h
index ee0211f..5001c72 100644
--- a/examples/l3fwd/l3fwd_em_hlm_sse.h
+++ b/examples/l3fwd/l3fwd_em_hlm_sse.h
@@ -38,7 +38,7 @@
 
 static inline __attribute__((always_inline)) void
 em_get_dst_port_ipv4x8(struct lcore_conf *qconf, struct rte_mbuf *m[8],
-		uint8_t portid, uint32_t dst_port[8])
+		uint8_t portid, uint16_t dst_port[8])
 {
 	int32_t ret[8];
 	union ipv4_5tuple_host key[8];
@@ -162,7 +162,7 @@ get_ipv6_5tuple(struct rte_mbuf *m0, __m128i mask0,
 
 static inline __attribute__((always_inline)) void
 em_get_dst_port_ipv6x8(struct lcore_conf *qconf, struct rte_mbuf *m[8],
-		uint8_t portid, uint32_t dst_port[8])
+		uint8_t portid, uint16_t dst_port[8])
 {
 	int32_t ret[8];
 	union ipv6_5tuple_host key[8];
@@ -289,7 +289,7 @@ l3fwd_em_send_packets(int nb_rx, struct rte_mbuf **pkts_burst,
 		uint8_t portid, struct lcore_conf *qconf)
 {
 	int32_t j;
-	uint32_t dst_port[MAX_PKT_BURST];
+	uint16_t dst_port[MAX_PKT_BURST];
 
 	/*
 	 * Send nb_rx - nb_rx%8 packets
diff --git a/examples/l3fwd/l3fwd_em_sse.h b/examples/l3fwd/l3fwd_em_sse.h
index e2fe932..c0a9725 100644
--- a/examples/l3fwd/l3fwd_em_sse.h
+++ b/examples/l3fwd/l3fwd_em_sse.h
@@ -102,7 +102,7 @@ l3fwd_em_send_packets(int nb_rx, struct rte_mbuf **pkts_burst,
 			uint8_t portid, struct lcore_conf *qconf)
 {
 	int32_t j;
-	uint32_t dst_port[MAX_PKT_BURST];
+	uint16_t dst_port[MAX_PKT_BURST];
 
 	for (j = 0; j < nb_rx; j++)
 		dst_port[j] = em_get_dst_port(qconf, pkts_burst[j], portid);
diff --git a/examples/l3fwd/l3fwd_lpm.h b/examples/l3fwd/l3fwd_lpm.h
index fc10235..a43c507 100644
--- a/examples/l3fwd/l3fwd_lpm.h
+++ b/examples/l3fwd/l3fwd_lpm.h
@@ -34,14 +34,14 @@
 #ifndef __L3FWD_LPM_H__
 #define __L3FWD_LPM_H__
 
-static inline uint32_t
+static inline uint8_t
 lpm_get_ipv4_dst_port(void *ipv4_hdr,  uint8_t portid, void *lookup_struct)
 {
 	uint32_t next_hop;
 	struct rte_lpm *ipv4_l3fwd_lookup_struct =
 		(struct rte_lpm *)lookup_struct;
 
-	return (uint32_t) ((rte_lpm_lookup(ipv4_l3fwd_lookup_struct,
+	return (uint8_t) ((rte_lpm_lookup(ipv4_l3fwd_lookup_struct,
 		rte_be_to_cpu_32(((struct ipv4_hdr *)ipv4_hdr)->dst_addr),
 		&next_hop) == 0) ? next_hop : portid);
 }
diff --git a/examples/l3fwd/l3fwd_lpm_sse.h b/examples/l3fwd/l3fwd_lpm_sse.h
index d64d6d2..538fe3d 100644
--- a/examples/l3fwd/l3fwd_lpm_sse.h
+++ b/examples/l3fwd/l3fwd_lpm_sse.h
@@ -145,9 +145,9 @@ static inline void
 processx4_step2(const struct lcore_conf *qconf,
 		__m128i dip,
 		uint32_t ipv4_flag,
-		uint32_t portid,
+		uint8_t portid,
 		struct rte_mbuf *pkt[FWDSTEP],
-		uint32_t dprt[FWDSTEP])
+		uint16_t dprt[FWDSTEP])
 {
 	rte_xmm_t dst;
 	const  __m128i bswap_mask = _mm_set_epi8(12, 13, 14, 15, 8, 9, 10, 11,
@@ -158,7 +158,11 @@ processx4_step2(const struct lcore_conf *qconf,
 
 	/* if all 4 packets are IPV4. */
 	if (likely(ipv4_flag)) {
-		rte_lpm_lookupx4(qconf->ipv4_lookup_struct, dip, dprt, portid);
+		rte_lpm_lookupx4(qconf->ipv4_lookup_struct, dip, dst.u32,
+			portid);
+		/* get rid of unused upper 16 bit for each dport. */
+		dst.x = _mm_packs_epi32(dst.x, dst.x);
+		*(uint64_t *)dprt = dst.u64[0];
 	} else {
 		dst.x = dip;
 		dprt[0] = lpm_get_dst_port_with_ipv4(qconf, pkt[0], dst.u32[0], portid);
@@ -177,7 +181,7 @@ l3fwd_lpm_send_packets(int nb_rx, struct rte_mbuf **pkts_burst,
 			uint8_t portid, struct lcore_conf *qconf)
 {
 	int32_t j;
-	uint32_t dst_port[MAX_PKT_BURST];
+	uint16_t dst_port[MAX_PKT_BURST];
 	__m128i dip[MAX_PKT_BURST / FWDSTEP];
 	uint32_t ipv4_flag[MAX_PKT_BURST / FWDSTEP];
 	const int32_t k = RTE_ALIGN_FLOOR(nb_rx, FWDSTEP);
diff --git a/examples/l3fwd/l3fwd_sse.h b/examples/l3fwd/l3fwd_sse.h
index 3d344d0..f9cf50a 100644
--- a/examples/l3fwd/l3fwd_sse.h
+++ b/examples/l3fwd/l3fwd_sse.h
@@ -58,7 +58,7 @@
  * to BAD_PORT value.
  */
 static inline __attribute__((always_inline)) void
-rfc1812_process(struct ipv4_hdr *ipv4_hdr, uint32_t *dp, uint32_t ptype)
+rfc1812_process(struct ipv4_hdr *ipv4_hdr, uint16_t *dp, uint32_t ptype)
 {
 	uint8_t ihl;
 
@@ -85,7 +85,7 @@ rfc1812_process(struct ipv4_hdr *ipv4_hdr, uint32_t *dp, uint32_t ptype)
  * Perform RFC1812 checks and updates for IPV4 packets.
  */
 static inline void
-processx4_step3(struct rte_mbuf *pkt[FWDSTEP], uint32_t dst_port[FWDSTEP])
+processx4_step3(struct rte_mbuf *pkt[FWDSTEP], uint16_t dst_port[FWDSTEP])
 {
 	__m128i te[FWDSTEP];
 	__m128i ve[FWDSTEP];
@@ -297,7 +297,7 @@ port_groupx4(uint16_t pn[FWDSTEP + 1], uint16_t *lp, __m128i dp1, __m128i dp2)
  * Perform RFC1812 checks and updates for IPV4 packets.
  */
 static inline void
-process_packet(struct rte_mbuf *pkt, uint32_t *dst_port)
+process_packet(struct rte_mbuf *pkt, uint16_t *dst_port)
 {
 	struct ether_hdr *eth_hdr;
 	__m128i te, ve;
@@ -397,7 +397,7 @@ send_packetsx4(struct lcore_conf *qconf, uint8_t port, struct rte_mbuf *m[],
  */
 static inline __attribute__((always_inline)) void
 send_packets_multi(struct lcore_conf *qconf, struct rte_mbuf **pkts_burst,
-		uint32_t dst_port[MAX_PKT_BURST], int nb_rx)
+		uint16_t dst_port[MAX_PKT_BURST], int nb_rx)
 {
 	int32_t k;
 	int j = 0;
-- 
2.5.0

Re: [dpdk-dev] [PATCH] l3fwd: fix incorrect size for destination port values

[Thomas Monjalon]

2016-03-31 14:07, Konstantin Ananyev:
> Fixes: dc81ebbacaeb ("lpm: extend IPv4 next hop field")
> 
> Originally l3fwd used 16-bit value to store dest_port value.
> To accommodate 24-bit nexthop dest_port was increased to 32-bit,
> though some further packet processing code remained unchanged and
> still expects dest_port to be 16-bit.
> That is not correct and can cause l3fwd invalid behaviour or even
> process crash/hang on some input packet patterns.
> For the fix, I choose the simplest approach and restored dest_port
> as 16-bit value, plus necessary conversions from 32 to 16 bit values
> after lpm_lookupx4.
> 
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Applied, thanks