[dpdk-dev] [PATCH] bond: vlan flags misinterpreted in xmit_slave_hash function

[dpdk-dev] [PATCH] bond: vlan flags misinterpreted in xmit_slave_hash function

[Declan Doherty]

- Split transmit hashing function into separate functions to reduce branching
  and to make code clearer.
- Add IPv4 IHL parameters to rte_ip.h
- Fixed VLAN tag support in hashing functions and add support for TCP
  in layer 4 header hashing.
- Fixed incorrect flag set in test application packet generator.

Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
 app/test/packet_burst_generator.c          |   2 +-
 lib/librte_net/rte_ip.h                    |   2 +
 lib/librte_pmd_bond/rte_eth_bond_api.c     |   8 ++
 lib/librte_pmd_bond/rte_eth_bond_pmd.c     | 161 ++++++++++++++++-------------
 lib/librte_pmd_bond/rte_eth_bond_private.h |  15 +++
 5 files changed, 115 insertions(+), 73 deletions(-)

diff --git a/app/test/packet_burst_generator.c b/app/test/packet_burst_generator.c
index b2824dc..4a89663 100644
--- a/app/test/packet_burst_generator.c
+++ b/app/test/packet_burst_generator.c
@@ -97,7 +97,7 @@ initialize_eth_header(struct ether_hdr *eth_hdr, struct ether_addr *src_mac,
 		vhdr->eth_proto =  rte_cpu_to_be_16(ETHER_TYPE_IPv4);
 		vhdr->vlan_tci = van_id;
 	} else {
-		eth_hdr->ether_type = rte_cpu_to_be_16(ETHER_TYPE_VLAN);
+		eth_hdr->ether_type = rte_cpu_to_be_16(ETHER_TYPE_IPv4);
 	}
 
 }
diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index 46f0497..c97ee0a 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -109,6 +109,8 @@ struct ipv4_hdr {
 					   (((b) & 0xff) << 16) | \
 					   (((c) & 0xff) << 8)  | \
 					   ((d) & 0xff))
+#define IPV4_HDR_IHL_MASK	(0x0f)
+#define IPV4_FIELD_WIDTH	(4)
 
 /* Fragment Offset * Flags. */
 #define	IPV4_HDR_DF_SHIFT	14
diff --git a/lib/librte_pmd_bond/rte_eth_bond_api.c b/lib/librte_pmd_bond/rte_eth_bond_api.c
index ef5ddf4..fb015a8 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_api.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_api.c
@@ -268,6 +268,7 @@ rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id)
 	internals->mode = BONDING_MODE_INVALID;
 	internals->current_primary_port = 0;
 	internals->balance_xmit_policy = BALANCE_XMIT_POLICY_LAYER2;
+	internals->xmit_hash = xmit_l2_hash;
 	internals->user_defined_mac = 0;
 	internals->link_props_set = 0;
 
@@ -710,9 +711,16 @@ rte_eth_bond_xmit_policy_set(uint8_t bonded_port_id, uint8_t policy)
 
 	switch (policy) {
 	case BALANCE_XMIT_POLICY_LAYER2:
+		internals->balance_xmit_policy = policy;
+		internals->xmit_hash = xmit_l2_hash;
+		break;
 	case BALANCE_XMIT_POLICY_LAYER23:
+		internals->balance_xmit_policy = policy;
+		internals->xmit_hash = xmit_l23_hash;
+		break;
 	case BALANCE_XMIT_POLICY_LAYER34:
 		internals->balance_xmit_policy = policy;
+		internals->xmit_hash = xmit_l34_hash;
 		break;
 
 	default:
diff --git a/lib/librte_pmd_bond/rte_eth_bond_pmd.c b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
index 3db473b..dc1a828 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
@@ -31,6 +31,8 @@
  *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 #include <stdlib.h>
+#include <netinet/in.h>
+
 #include <rte_mbuf.h>
 #include <rte_malloc.h>
 #include <rte_ethdev.h>
@@ -48,6 +50,9 @@
 #include "rte_eth_bond_8023ad_private.h"
 
 #define REORDER_PERIOD_MS 10
+
+#define HASH_L4_PORTS(h) ((h)->src_port ^ (h)->dst_port)
+
 /* Table for statistics in mode 5 TLB */
 static uint64_t tlb_last_obytets[RTE_MAX_ETHPORTS];
 
@@ -276,90 +281,104 @@ ipv6_hash(struct ipv6_hdr *ipv6_hdr)
 			(word_src_addr[3] ^ word_dst_addr[3]);
 }
 
-static uint32_t
-udp_hash(struct udp_hdr *hdr)
+static inline size_t
+get_vlan_offset(struct ether_hdr *eth_hdr)
 {
-	return hdr->src_port ^ hdr->dst_port;
+	size_t vlan_offset = 0;
+
+	/* Calculate VLAN offset */
+	if (rte_cpu_to_be_16(ETHER_TYPE_VLAN) == eth_hdr->ether_type) {
+		struct vlan_hdr *vlan_hdr = (struct vlan_hdr *)(eth_hdr + 1);
+		vlan_offset = sizeof(struct vlan_hdr);
+
+		while (rte_cpu_to_be_16(ETHER_TYPE_VLAN) ==
+				vlan_hdr->eth_proto) {
+			vlan_hdr = vlan_hdr + 1;
+			vlan_offset += sizeof(struct vlan_hdr);
+		}
+	}
+	return vlan_offset;
 }
 
-static inline uint16_t
-xmit_slave_hash(const struct rte_mbuf *buf, uint8_t slave_count, uint8_t policy)
+uint16_t
+xmit_l2_hash(const struct rte_mbuf *buf, uint8_t slave_count)
 {
-	struct ether_hdr *eth_hdr;
-	struct udp_hdr *udp_hdr;
-	size_t eth_offset = 0;
-	uint32_t hash = 0;
-
-	if (slave_count == 1)
-		return 0;
+	struct ether_hdr *eth_hdr = rte_pktmbuf_mtod(buf, struct ether_hdr *);
 
-	switch (policy) {
-	case BALANCE_XMIT_POLICY_LAYER2:
-		eth_hdr = rte_pktmbuf_mtod(buf, struct ether_hdr *);
+	uint32_t hash = ether_hash(eth_hdr);
 
-		hash = ether_hash(eth_hdr);
-		hash ^= hash >> 8;
-		return hash % slave_count;
+	return (hash ^= hash >> 8) % slave_count;
+}
 
-	case BALANCE_XMIT_POLICY_LAYER23:
-		eth_hdr = rte_pktmbuf_mtod(buf, struct ether_hdr *);
+uint16_t
+xmit_l23_hash(const struct rte_mbuf *buf, uint8_t slave_count)
+{
+	struct ether_hdr *eth_hdr = rte_pktmbuf_mtod(buf, struct ether_hdr *);
+	size_t vlan_offset = get_vlan_offset(eth_hdr);
+	uint32_t hash, l3hash = 0;
 
-		if (buf->ol_flags & PKT_RX_VLAN_PKT)
-			eth_offset = sizeof(struct ether_hdr) + sizeof(struct vlan_hdr);
-		else
-			eth_offset = sizeof(struct ether_hdr);
+	hash = ether_hash(eth_hdr);
 
-		if (buf->ol_flags & PKT_RX_IPV4_HDR) {
-			struct ipv4_hdr *ipv4_hdr;
-			ipv4_hdr = (struct ipv4_hdr *)(rte_pktmbuf_mtod(buf,
-					unsigned char *) + eth_offset);
+	if (buf->ol_flags & PKT_RX_IPV4_HDR) {
+		struct ipv4_hdr *ipv4_hdr = (struct ipv4_hdr *)
+				((char *)(eth_hdr + 1) + vlan_offset);
+		l3hash = ipv4_hash(ipv4_hdr);
 
-			hash = ether_hash(eth_hdr) ^ ipv4_hash(ipv4_hdr);
+	} else if  (buf->ol_flags & PKT_RX_IPV6_HDR) {
+		struct ipv6_hdr *ipv6_hdr = (struct ipv6_hdr *)
+				((char *)(eth_hdr + 1) + vlan_offset);
+		l3hash = ipv6_hash(ipv6_hdr);
+	}
 
-		} else {
-			struct ipv6_hdr *ipv6_hdr;
+	hash = hash ^ l3hash;
+	hash ^= hash >> 16;
+	hash ^= hash >> 8;
 
-			ipv6_hdr = (struct ipv6_hdr *)(rte_pktmbuf_mtod(buf,
-					unsigned char *) + eth_offset);
+	return hash % slave_count;
+}
 
-			hash = ether_hash(eth_hdr) ^ ipv6_hash(ipv6_hdr);
+uint16_t
+xmit_l34_hash(const struct rte_mbuf *buf, uint8_t slave_count)
+{
+	struct ether_hdr *eth_hdr = rte_pktmbuf_mtod(buf, struct ether_hdr *);
+	size_t vlan_offset = get_vlan_offset(eth_hdr);
+	struct udp_hdr *udp_hdr = NULL;
+	struct tcp_hdr *tcp_hdr = NULL;
+	uint32_t hash, l3hash = 0, l4hash = 0;
+
+	if (buf->ol_flags & PKT_RX_IPV4_HDR) {
+		struct ipv4_hdr *ipv4_hdr = (struct ipv4_hdr *)
+				((char *)(eth_hdr + 1) + vlan_offset);
+		size_t ip_hdr_offset;
+
+		l3hash = ipv4_hash(ipv4_hdr);
+
+		ip_hdr_offset = (ipv4_hdr->version_ihl & IPV4_HDR_IHL_MASK) * IPV4_FIELD_WIDTH;
+
+		if (ipv4_hdr->next_proto_id == IPPROTO_TCP) {
+			tcp_hdr = (struct tcp_hdr *)((char *)ipv4_hdr +
+					ip_hdr_offset);
+			l4hash = HASH_L4_PORTS(tcp_hdr);
+		} else if (ipv4_hdr->next_proto_id == IPPROTO_UDP) {
+			udp_hdr = (struct udp_hdr *)((char *)ipv4_hdr +
+					ip_hdr_offset);
+			l4hash = HASH_L4_PORTS(udp_hdr);
 		}
-		break;
-
-	case BALANCE_XMIT_POLICY_LAYER34:
-		if (buf->ol_flags & PKT_RX_VLAN_PKT)
-			eth_offset = sizeof(struct ether_hdr) + sizeof(struct vlan_hdr);
-		else
-			eth_offset = sizeof(struct ether_hdr);
-
-		if (buf->ol_flags & PKT_RX_IPV4_HDR) {
-			struct ipv4_hdr *ipv4_hdr = (struct ipv4_hdr *)
-					(rte_pktmbuf_mtod(buf, unsigned char *) + eth_offset);
-
-			if (ipv4_hdr->next_proto_id == IPPROTO_UDP) {
-				udp_hdr = (struct udp_hdr *)
-						(rte_pktmbuf_mtod(buf, unsigned char *) + eth_offset +
-								sizeof(struct ipv4_hdr));
-				hash = ipv4_hash(ipv4_hdr) ^ udp_hash(udp_hdr);
-			} else {
-				hash = ipv4_hash(ipv4_hdr);
-			}
-		} else {
-			struct ipv6_hdr *ipv6_hdr = (struct ipv6_hdr *)
-					(rte_pktmbuf_mtod(buf, unsigned char *) + eth_offset);
-
-			if (ipv6_hdr->proto == IPPROTO_UDP) {
-				udp_hdr = (struct udp_hdr *)
-						(rte_pktmbuf_mtod(buf, unsigned char *) + eth_offset +
-								sizeof(struct ipv6_hdr));
-				hash = ipv6_hash(ipv6_hdr) ^ udp_hash(udp_hdr);
-			} else {
-				hash = ipv6_hash(ipv6_hdr);
-			}
+	} else if  (buf->ol_flags & PKT_RX_IPV6_HDR) {
+		struct ipv6_hdr *ipv6_hdr = (struct ipv6_hdr *)
+				((char *)(eth_hdr + 1) + vlan_offset);
+		l3hash = ipv6_hash(ipv6_hdr);
+
+		if (ipv6_hdr->proto == IPPROTO_TCP) {
+			tcp_hdr = (struct tcp_hdr *)(ipv6_hdr + 1);
+			l4hash = HASH_L4_PORTS(tcp_hdr);
+		} else if (ipv6_hdr->proto == IPPROTO_UDP) {
+			udp_hdr = (struct udp_hdr *)(ipv6_hdr + 1);
+			l4hash = HASH_L4_PORTS(udp_hdr);
 		}
-		break;
 	}
 
+	hash = l3hash ^ l4hash;
 	hash ^= hash >> 16;
 	hash ^= hash >> 8;
 
@@ -536,8 +555,7 @@ bond_ethdev_tx_burst_balance(void *queue, struct rte_mbuf **bufs,
 	/* Populate slaves mbuf with the packets which are to be sent on it  */
 	for (i = 0; i < nb_pkts; i++) {
 		/* Select output slave using hash based on xmit policy */
-		op_slave_id = xmit_slave_hash(bufs[i], num_of_slaves,
-				internals->balance_xmit_policy);
+		op_slave_id = internals->xmit_hash(bufs[i], num_of_slaves);
 
 		/* Populate slave mbuf arrays with mbufs for that slave */
 		slave_bufs[op_slave_id][slave_nb_pkts[op_slave_id]++] = bufs[i];
@@ -575,7 +593,7 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
 
 	uint8_t num_of_slaves;
 	uint8_t slaves[RTE_MAX_ETHPORTS];
-	 /* possitions in slaves, not ID */
+	 /* positions in slaves, not ID */
 	uint8_t distributing_offsets[RTE_MAX_ETHPORTS];
 	uint8_t distributing_count;
 
@@ -622,8 +640,7 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
 		/* Populate slaves mbuf with the packets which are to be sent on it */
 		for (i = 0; i < nb_pkts; i++) {
 			/* Select output slave using hash based on xmit policy */
-			op_slave_idx = xmit_slave_hash(bufs[i], distributing_count,
-					internals->balance_xmit_policy);
+			op_slave_idx = internals->xmit_hash(bufs[i], distributing_count);
 
 			/* Populate slave mbuf arrays with mbufs for that slave. Use only
 			 * slaves that are currently distributing. */
diff --git a/lib/librte_pmd_bond/rte_eth_bond_private.h b/lib/librte_pmd_bond/rte_eth_bond_private.h
index f913c5b..e01e66b 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_private.h
+++ b/lib/librte_pmd_bond/rte_eth_bond_private.h
@@ -108,6 +108,9 @@ struct bond_slave_details {
 	struct ether_addr persisted_mac_addr;
 };
 
+
+typedef uint16_t (*xmit_hash_t)(const struct rte_mbuf *buf, uint8_t slave_count);
+
 /** Link Bonding PMD device private configuration Structure */
 struct bond_dev_private {
 	uint8_t port_id;					/**< Port Id of Bonded Port */
@@ -122,6 +125,9 @@ struct bond_dev_private {
 
 	uint8_t balance_xmit_policy;
 	/**< Transmit policy - l2 / l23 / l34 for operation in balance mode */
+	xmit_hash_t xmit_hash;
+	/**< Transmit policy hash function */
+
 	uint8_t user_defined_mac;
 	/**< Flag for whether MAC address is user defined or not */
 	uint8_t promiscuous_en;
@@ -225,6 +231,15 @@ void
 slave_add(struct bond_dev_private *internals,
 		struct rte_eth_dev *slave_eth_dev);
 
+uint16_t
+xmit_l2_hash(const struct rte_mbuf *buf, uint8_t slave_count);
+
+uint16_t
+xmit_l23_hash(const struct rte_mbuf *buf, uint8_t slave_count);
+
+uint16_t
+xmit_l34_hash(const struct rte_mbuf *buf, uint8_t slave_count);
+
 void
 bond_ethdev_primary_set(struct bond_dev_private *internals,
 		uint8_t slave_port_id);
-- 
1.7.12.2

Re: [dpdk-dev] [PATCH] bond: vlan flags misinterpreted in xmit_slave_hash function

[Thomas Monjalon]

Hi Declan,

2014-12-16 11:15, Declan Doherty:
> - Split transmit hashing function into separate functions to reduce branching
>   and to make code clearer.
> - Add IPv4 IHL parameters to rte_ip.h
> - Fixed VLAN tag support in hashing functions and add support for TCP
>   in layer 4 header hashing.
> - Fixed incorrect flag set in test application packet generator.

You forgot to describe the problem you are solving.

You seem fixing something but I'm afraid this patch is too big to be safely
integrated in 1.8.0.
Was it your goal?

> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> ---
>  app/test/packet_burst_generator.c          |   2 +-
>  lib/librte_net/rte_ip.h                    |   2 +
>  lib/librte_pmd_bond/rte_eth_bond_api.c     |   8 ++
>  lib/librte_pmd_bond/rte_eth_bond_pmd.c     | 161 ++++++++++++++++-------------
>  lib/librte_pmd_bond/rte_eth_bond_private.h |  15 +++
>  5 files changed, 115 insertions(+), 73 deletions(-)
> 
[...]
> --- a/lib/librte_net/rte_ip.h
> +++ b/lib/librte_net/rte_ip.h
> @@ -109,6 +109,8 @@ struct ipv4_hdr {
>  					   (((b) & 0xff) << 16) | \
>  					   (((c) & 0xff) << 8)  | \
>  					   ((d) & 0xff))
> +#define IPV4_HDR_IHL_MASK	(0x0f)
> +#define IPV4_FIELD_WIDTH	(4)

These new definitions require some doxygen comments.

Thanks
-- 
Thomas

Re: [dpdk-dev] [PATCH] bond: vlan flags misinterpreted in xmit_slave_hash function

[Wodkowski, PawelX]

> -----Original Message-----
> From: Doherty, Declan
> Sent: Tuesday, December 16, 2014 12:16 PM
> To: dev@dpdk.org
> Cc: Wodkowski, PawelX; Doherty, Declan
> Subject: [PATCH] bond: vlan flags misinterpreted in xmit_slave_hash function
> 
> - Split transmit hashing function into separate functions to reduce branching
>   and to make code clearer.
> - Add IPv4 IHL parameters to rte_ip.h
> - Fixed VLAN tag support in hashing functions and add support for TCP
>   in layer 4 header hashing.
> - Fixed incorrect flag set in test application packet generator.
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>


Acked-by: Wodkowski, Pawel <pawelx.wodkowski@intel.com>

[dpdk-dev] [PATCH v2] bond: vlan flags misinterpreted in xmit_slave_hash function

[Declan Doherty]

This patch contains a fix for link bonding handling of vlan tagged packets in mode 3 and 5.
Currently xmit_slave_hash function misinterprets the PKT_RX_VLAN_PKT flag to mean that
there is a vlan tag within the packet when in actually means that there is a valid entry
in the vlan_tci field in the mbuf.

-v2:
doxygen comments for rte_ip.h

- Fixed VLAN tag support in hashing functions.
- Adds support for TCP in layer 4 header hashing.
- Splits transmit hashing function into separate functions for each policy to
  reduce branching and to make the code clearer.
- Fixed incorrect flag set in test application packet generator.

Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
 app/test/packet_burst_generator.c          |   2 +-
 lib/librte_net/rte_ip.h                    |   8 ++
 lib/librte_pmd_bond/rte_eth_bond_api.c     |   8 ++
 lib/librte_pmd_bond/rte_eth_bond_pmd.c     | 162 ++++++++++++++++-------------
 lib/librte_pmd_bond/rte_eth_bond_private.h |  15 +++
 5 files changed, 122 insertions(+), 73 deletions(-)

diff --git a/app/test/packet_burst_generator.c b/app/test/packet_burst_generator.c
index b2824dc..4a89663 100644
--- a/app/test/packet_burst_generator.c
+++ b/app/test/packet_burst_generator.c
@@ -97,7 +97,7 @@ initialize_eth_header(struct ether_hdr *eth_hdr, struct ether_addr *src_mac,
 		vhdr->eth_proto =  rte_cpu_to_be_16(ETHER_TYPE_IPv4);
 		vhdr->vlan_tci = van_id;
 	} else {
-		eth_hdr->ether_type = rte_cpu_to_be_16(ETHER_TYPE_VLAN);
+		eth_hdr->ether_type = rte_cpu_to_be_16(ETHER_TYPE_IPv4);
 	}
 
 }
diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index 46f0497..3446e1c 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -110,6 +110,14 @@ struct ipv4_hdr {
 					   (((c) & 0xff) << 8)  | \
 					   ((d) & 0xff))
 
+/** Internet header length mask for version_ihl field */
+#define IPV4_HDR_IHL_MASK	(0x0f)
+/**
+ * Internet header length field multiplier (IHL field specifies overall header
+ * length in number of 4-byte words)
+ */
+#define IPV4_IHL_MULTIPLIER	(4)
+
 /* Fragment Offset * Flags. */
 #define	IPV4_HDR_DF_SHIFT	14
 #define	IPV4_HDR_MF_SHIFT	13
diff --git a/lib/librte_pmd_bond/rte_eth_bond_api.c b/lib/librte_pmd_bond/rte_eth_bond_api.c
index ef5ddf4..fb015a8 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_api.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_api.c
@@ -268,6 +268,7 @@ rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id)
 	internals->mode = BONDING_MODE_INVALID;
 	internals->current_primary_port = 0;
 	internals->balance_xmit_policy = BALANCE_XMIT_POLICY_LAYER2;
+	internals->xmit_hash = xmit_l2_hash;
 	internals->user_defined_mac = 0;
 	internals->link_props_set = 0;
 
@@ -710,9 +711,16 @@ rte_eth_bond_xmit_policy_set(uint8_t bonded_port_id, uint8_t policy)
 
 	switch (policy) {
 	case BALANCE_XMIT_POLICY_LAYER2:
+		internals->balance_xmit_policy = policy;
+		internals->xmit_hash = xmit_l2_hash;
+		break;
 	case BALANCE_XMIT_POLICY_LAYER23:
+		internals->balance_xmit_policy = policy;
+		internals->xmit_hash = xmit_l23_hash;
+		break;
 	case BALANCE_XMIT_POLICY_LAYER34:
 		internals->balance_xmit_policy = policy;
+		internals->xmit_hash = xmit_l34_hash;
 		break;
 
 	default:
diff --git a/lib/librte_pmd_bond/rte_eth_bond_pmd.c b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
index 3db473b..c055911 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
@@ -31,6 +31,8 @@
  *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 #include <stdlib.h>
+#include <netinet/in.h>
+
 #include <rte_mbuf.h>
 #include <rte_malloc.h>
 #include <rte_ethdev.h>
@@ -48,6 +50,9 @@
 #include "rte_eth_bond_8023ad_private.h"
 
 #define REORDER_PERIOD_MS 10
+
+#define HASH_L4_PORTS(h) ((h)->src_port ^ (h)->dst_port)
+
 /* Table for statistics in mode 5 TLB */
 static uint64_t tlb_last_obytets[RTE_MAX_ETHPORTS];
 
@@ -276,90 +281,105 @@ ipv6_hash(struct ipv6_hdr *ipv6_hdr)
 			(word_src_addr[3] ^ word_dst_addr[3]);
 }
 
-static uint32_t
-udp_hash(struct udp_hdr *hdr)
+static inline size_t
+get_vlan_offset(struct ether_hdr *eth_hdr)
 {
-	return hdr->src_port ^ hdr->dst_port;
+	size_t vlan_offset = 0;
+
+	/* Calculate VLAN offset */
+	if (rte_cpu_to_be_16(ETHER_TYPE_VLAN) == eth_hdr->ether_type) {
+		struct vlan_hdr *vlan_hdr = (struct vlan_hdr *)(eth_hdr + 1);
+		vlan_offset = sizeof(struct vlan_hdr);
+
+		while (rte_cpu_to_be_16(ETHER_TYPE_VLAN) ==
+				vlan_hdr->eth_proto) {
+			vlan_hdr = vlan_hdr + 1;
+			vlan_offset += sizeof(struct vlan_hdr);
+		}
+	}
+	return vlan_offset;
 }
 
-static inline uint16_t
-xmit_slave_hash(const struct rte_mbuf *buf, uint8_t slave_count, uint8_t policy)
+uint16_t
+xmit_l2_hash(const struct rte_mbuf *buf, uint8_t slave_count)
 {
-	struct ether_hdr *eth_hdr;
-	struct udp_hdr *udp_hdr;
-	size_t eth_offset = 0;
-	uint32_t hash = 0;
-
-	if (slave_count == 1)
-		return 0;
+	struct ether_hdr *eth_hdr = rte_pktmbuf_mtod(buf, struct ether_hdr *);
 
-	switch (policy) {
-	case BALANCE_XMIT_POLICY_LAYER2:
-		eth_hdr = rte_pktmbuf_mtod(buf, struct ether_hdr *);
+	uint32_t hash = ether_hash(eth_hdr);
 
-		hash = ether_hash(eth_hdr);
-		hash ^= hash >> 8;
-		return hash % slave_count;
+	return (hash ^= hash >> 8) % slave_count;
+}
 
-	case BALANCE_XMIT_POLICY_LAYER23:
-		eth_hdr = rte_pktmbuf_mtod(buf, struct ether_hdr *);
+uint16_t
+xmit_l23_hash(const struct rte_mbuf *buf, uint8_t slave_count)
+{
+	struct ether_hdr *eth_hdr = rte_pktmbuf_mtod(buf, struct ether_hdr *);
+	size_t vlan_offset = get_vlan_offset(eth_hdr);
+	uint32_t hash, l3hash = 0;
 
-		if (buf->ol_flags & PKT_RX_VLAN_PKT)
-			eth_offset = sizeof(struct ether_hdr) + sizeof(struct vlan_hdr);
-		else
-			eth_offset = sizeof(struct ether_hdr);
+	hash = ether_hash(eth_hdr);
 
-		if (buf->ol_flags & PKT_RX_IPV4_HDR) {
-			struct ipv4_hdr *ipv4_hdr;
-			ipv4_hdr = (struct ipv4_hdr *)(rte_pktmbuf_mtod(buf,
-					unsigned char *) + eth_offset);
+	if (buf->ol_flags & PKT_RX_IPV4_HDR) {
+		struct ipv4_hdr *ipv4_hdr = (struct ipv4_hdr *)
+				((char *)(eth_hdr + 1) + vlan_offset);
+		l3hash = ipv4_hash(ipv4_hdr);
 
-			hash = ether_hash(eth_hdr) ^ ipv4_hash(ipv4_hdr);
+	} else if  (buf->ol_flags & PKT_RX_IPV6_HDR) {
+		struct ipv6_hdr *ipv6_hdr = (struct ipv6_hdr *)
+				((char *)(eth_hdr + 1) + vlan_offset);
+		l3hash = ipv6_hash(ipv6_hdr);
+	}
 
-		} else {
-			struct ipv6_hdr *ipv6_hdr;
+	hash = hash ^ l3hash;
+	hash ^= hash >> 16;
+	hash ^= hash >> 8;
 
-			ipv6_hdr = (struct ipv6_hdr *)(rte_pktmbuf_mtod(buf,
-					unsigned char *) + eth_offset);
+	return hash % slave_count;
+}
 
-			hash = ether_hash(eth_hdr) ^ ipv6_hash(ipv6_hdr);
+uint16_t
+xmit_l34_hash(const struct rte_mbuf *buf, uint8_t slave_count)
+{
+	struct ether_hdr *eth_hdr = rte_pktmbuf_mtod(buf, struct ether_hdr *);
+	size_t vlan_offset = get_vlan_offset(eth_hdr);
+	struct udp_hdr *udp_hdr = NULL;
+	struct tcp_hdr *tcp_hdr = NULL;
+	uint32_t hash, l3hash = 0, l4hash = 0;
+
+	if (buf->ol_flags & PKT_RX_IPV4_HDR) {
+		struct ipv4_hdr *ipv4_hdr = (struct ipv4_hdr *)
+				((char *)(eth_hdr + 1) + vlan_offset);
+		size_t ip_hdr_offset;
+
+		l3hash = ipv4_hash(ipv4_hdr);
+
+		ip_hdr_offset = (ipv4_hdr->version_ihl & IPV4_HDR_IHL_MASK) *
+				IPV4_IHL_MULTIPLIER;
+
+		if (ipv4_hdr->next_proto_id == IPPROTO_TCP) {
+			tcp_hdr = (struct tcp_hdr *)((char *)ipv4_hdr +
+					ip_hdr_offset);
+			l4hash = HASH_L4_PORTS(tcp_hdr);
+		} else if (ipv4_hdr->next_proto_id == IPPROTO_UDP) {
+			udp_hdr = (struct udp_hdr *)((char *)ipv4_hdr +
+					ip_hdr_offset);
+			l4hash = HASH_L4_PORTS(udp_hdr);
 		}
-		break;
-
-	case BALANCE_XMIT_POLICY_LAYER34:
-		if (buf->ol_flags & PKT_RX_VLAN_PKT)
-			eth_offset = sizeof(struct ether_hdr) + sizeof(struct vlan_hdr);
-		else
-			eth_offset = sizeof(struct ether_hdr);
-
-		if (buf->ol_flags & PKT_RX_IPV4_HDR) {
-			struct ipv4_hdr *ipv4_hdr = (struct ipv4_hdr *)
-					(rte_pktmbuf_mtod(buf, unsigned char *) + eth_offset);
-
-			if (ipv4_hdr->next_proto_id == IPPROTO_UDP) {
-				udp_hdr = (struct udp_hdr *)
-						(rte_pktmbuf_mtod(buf, unsigned char *) + eth_offset +
-								sizeof(struct ipv4_hdr));
-				hash = ipv4_hash(ipv4_hdr) ^ udp_hash(udp_hdr);
-			} else {
-				hash = ipv4_hash(ipv4_hdr);
-			}
-		} else {
-			struct ipv6_hdr *ipv6_hdr = (struct ipv6_hdr *)
-					(rte_pktmbuf_mtod(buf, unsigned char *) + eth_offset);
-
-			if (ipv6_hdr->proto == IPPROTO_UDP) {
-				udp_hdr = (struct udp_hdr *)
-						(rte_pktmbuf_mtod(buf, unsigned char *) + eth_offset +
-								sizeof(struct ipv6_hdr));
-				hash = ipv6_hash(ipv6_hdr) ^ udp_hash(udp_hdr);
-			} else {
-				hash = ipv6_hash(ipv6_hdr);
-			}
+	} else if  (buf->ol_flags & PKT_RX_IPV6_HDR) {
+		struct ipv6_hdr *ipv6_hdr = (struct ipv6_hdr *)
+				((char *)(eth_hdr + 1) + vlan_offset);
+		l3hash = ipv6_hash(ipv6_hdr);
+
+		if (ipv6_hdr->proto == IPPROTO_TCP) {
+			tcp_hdr = (struct tcp_hdr *)(ipv6_hdr + 1);
+			l4hash = HASH_L4_PORTS(tcp_hdr);
+		} else if (ipv6_hdr->proto == IPPROTO_UDP) {
+			udp_hdr = (struct udp_hdr *)(ipv6_hdr + 1);
+			l4hash = HASH_L4_PORTS(udp_hdr);
 		}
-		break;
 	}
 
+	hash = l3hash ^ l4hash;
 	hash ^= hash >> 16;
 	hash ^= hash >> 8;
 
@@ -536,8 +556,7 @@ bond_ethdev_tx_burst_balance(void *queue, struct rte_mbuf **bufs,
 	/* Populate slaves mbuf with the packets which are to be sent on it  */
 	for (i = 0; i < nb_pkts; i++) {
 		/* Select output slave using hash based on xmit policy */
-		op_slave_id = xmit_slave_hash(bufs[i], num_of_slaves,
-				internals->balance_xmit_policy);
+		op_slave_id = internals->xmit_hash(bufs[i], num_of_slaves);
 
 		/* Populate slave mbuf arrays with mbufs for that slave */
 		slave_bufs[op_slave_id][slave_nb_pkts[op_slave_id]++] = bufs[i];
@@ -575,7 +594,7 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
 
 	uint8_t num_of_slaves;
 	uint8_t slaves[RTE_MAX_ETHPORTS];
-	 /* possitions in slaves, not ID */
+	 /* positions in slaves, not ID */
 	uint8_t distributing_offsets[RTE_MAX_ETHPORTS];
 	uint8_t distributing_count;
 
@@ -622,8 +641,7 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
 		/* Populate slaves mbuf with the packets which are to be sent on it */
 		for (i = 0; i < nb_pkts; i++) {
 			/* Select output slave using hash based on xmit policy */
-			op_slave_idx = xmit_slave_hash(bufs[i], distributing_count,
-					internals->balance_xmit_policy);
+			op_slave_idx = internals->xmit_hash(bufs[i], distributing_count);
 
 			/* Populate slave mbuf arrays with mbufs for that slave. Use only
 			 * slaves that are currently distributing. */
diff --git a/lib/librte_pmd_bond/rte_eth_bond_private.h b/lib/librte_pmd_bond/rte_eth_bond_private.h
index f913c5b..e01e66b 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_private.h
+++ b/lib/librte_pmd_bond/rte_eth_bond_private.h
@@ -108,6 +108,9 @@ struct bond_slave_details {
 	struct ether_addr persisted_mac_addr;
 };
 
+
+typedef uint16_t (*xmit_hash_t)(const struct rte_mbuf *buf, uint8_t slave_count);
+
 /** Link Bonding PMD device private configuration Structure */
 struct bond_dev_private {
 	uint8_t port_id;					/**< Port Id of Bonded Port */
@@ -122,6 +125,9 @@ struct bond_dev_private {
 
 	uint8_t balance_xmit_policy;
 	/**< Transmit policy - l2 / l23 / l34 for operation in balance mode */
+	xmit_hash_t xmit_hash;
+	/**< Transmit policy hash function */
+
 	uint8_t user_defined_mac;
 	/**< Flag for whether MAC address is user defined or not */
 	uint8_t promiscuous_en;
@@ -225,6 +231,15 @@ void
 slave_add(struct bond_dev_private *internals,
 		struct rte_eth_dev *slave_eth_dev);
 
+uint16_t
+xmit_l2_hash(const struct rte_mbuf *buf, uint8_t slave_count);
+
+uint16_t
+xmit_l23_hash(const struct rte_mbuf *buf, uint8_t slave_count);
+
+uint16_t
+xmit_l34_hash(const struct rte_mbuf *buf, uint8_t slave_count);
+
 void
 bond_ethdev_primary_set(struct bond_dev_private *internals,
 		uint8_t slave_port_id);
-- 
1.7.12.2

Re: [dpdk-dev] [PATCH v2] bond: vlan flags misinterpreted in xmit_slave_hash function

[Jiajia, SunX]

Tested-by: Jiajia, SunX <sunx.jiajia@intel.com>
- Tested Commit: 6fb3161060fc894295a27f9304c56ef34492799d
- OS: Fedora20 3.11.10-301.fc20.x86_64
- GCC: gcc version 4.8.3
- CPU: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
- NIC: Intel Corporation 82599ES 10-Gigabit SFI/SFP+ Network Connection [8086:10fb]
- Target x86_64-native-linuxapp-gcc and i686-native-linuxapp-gcc
- Total 44 cases, 44 passed, 0 failed

TOPO:
* Connections ports between tester/ixia and DUT
  - TESTER(Or IXIA)-------DUT
  - portA------------------port0
  - portB------------------port1
  - portC------------------port2
  - portD------------------port3


Test Setup#1 for Functional test
================================

Tester has 4 ports(portA--portD), and DUT has 4 ports(port0--port3), then connect portA to port0, portB to port1, portC to port2, portD to port3. 



- Case: Basic bonding--Create bonded devices and slaves
  Description: 
		Use Setup#1.
		Create bonded device and add some ports as salve of bonded device,
            Then removed slaves or added slaves or change the bonding primary slave
            Or change bonding mode and so on.
  Expected test result:
            Verify the basic functions are normal.

- Case: Basic bonding--MAC Address Test
  Description: 
		Use Setup#1.
		Create bonded device and add some ports as slaves of bonded device,
            Check that the changes of  the bonded device and slave MAC
  Expected test result:
            Verify the behavior of bonded device and slave according to the mode.

- Case: Basic bonding--Device Promiscuous Mode Test
  Description: 
		Use Setup#1.
		Create bonded device and add some ports as slaves of bonded device,
            Set promiscuous mode on or off, then send packets to the bonded device
            Or slaves.
  Expected test result:
            Verify the RX/TX status of bonded device and slaves according to the mode.

- Case: Mode 0(Round Robin) TX/RX test
  Description: 
		Use Setup#1.
		Create bonded device with mode 0 and add 3 ports as slaves of bonded device,
            Forward packets between bonded device and unbounded device, start to forward,
            And send packets to unbound device or slaves.
  Expected test result:
            Verify the RX/TX status of bonded device and slaves in mode 0.

- Case: Mode 0(Round Robin) Bring one slave link down
  Description: 
		Use Setup#1.
		Create bonded device with mode 0 and add 3 ports as slaves of bonded device,
            Forward packets between bonded device and unbounded device, start to forward,
            Bring the link on either port 0, 1 or 2 down. And send packets to unbound 
            device or slaves.
  Expected test result:
            Verify the RX/TX status of bonded device and slaves in mode 0.

- Case: Mode 0(Round Robin) Bring all slave links down
  Description: 
		Use Setup#1.
		Create bonded device with mode 0 and add 3 ports as slaves of bonded device,
            Forward packets between bonded device and unbounded device, start to forward,
            Bring the links down on all bonded ports. And send packets to unbound 
            device or slaves.
  Expected test result:
            Verify the RX/TX status of bonded device and slaves in mode 0.

- Case: Mode 1(Active Backup) TX/RX Test
  Description: 
		Use Setup#1.
		Create bonded device with mode 1 and add 3 ports as slaves of bonded device,
            Forward packets between bonded device and unbounded device, start to forward,
            And send packets to unbound device or slaves.
  Expected test result:
            Verify the RX/TX status of bonded device and slaves in mode 1.

- Case: Mode 1(Active Backup) Change active slave, RX/TX test
  Description: 
		Use Setup#1.
		Continuing from previous test case.Change the active slave port from port0 
            to port1.Verify that the bonded device's MAC has changed to slave1's MAC.

            testpmd> set bonding primary 1 4 

           Repeat the transmission and reception(TX/RX) test verify that data is now 
           transmitted and received through the new active slave and no longer through
           port0
  Expected test result:
            Verify the RX/TX status of bonded device and slaves in mode 1.

- Case: Mode 1(Active Backup) Link up/down active eth dev
  Description: 
		Use Setup#1.

           Bring link between port A and port0 down. If tester is ixia, can use 
           IxExplorer to set the "Simulate Cable Disconnect" at the port property.  
           Verify that the active slave has been changed from port0. Repeat the 
           transmission and reception test verify that data is now transmitted and
           received through the new active slave and no longer through port0

           Bring port0 to link down at the remote end.Verify the active slave has been changed from port0.
           send 100 packets to port3.

           testpmd> show port stats 4----(Verify port3 have 100 rx packets,meanwhile
          port4 have 100 tx packets,the current primary port have 100 tx packets,and port0
          doesn`t have any packet)
  Expected test result:
            Verify the RX/TX status of bonded device and slaves in mode 1. 

- Case: Mode 1(Active Backup) Bring all slave links down
  Description: 
		Use Setup#1.
            Bring all slave ports of bonded port down.
            Verify that bonded callback for link down is called and no active slaves. 
            Verify that data cannot be sent or received through bonded port. Send 100 packets 
            to port3 and verify that bonded port can't TX 100 packets.

            Bring port 0-2 to link down at the remote end. Verify port4 has been link down and has no active slave.
            Send 100 rx packets to port3. 

         testpmd> show port stats 4----(Verify port3 have 100 rx packets,meanwhile 
                                        port4 doesn`t have any packet)
  Expected test result:
            Verify the RX/TX status of bonded device and slaves in mode 1.

- Case: Mode 2(Balance XOR) TX Load Balance test
  Description: 
		Use Setup#1.
            Bonded port will activate each slave eth dev based on the following hash function:

             ((dst_mac XOR src_mac) % (number of slave ports))

            Send 300 packets from IXIA port D to non-bonded port(port3),and verify these packets 
            will be forwarded to bonded device. The bonded device will transimit these packets to all slaves.
            Verify that each slave receive correct number of packets according to the policy. The total number
            of packets which received by slave should be equal as 300 packets. 

            Create 3 streams in the IXIA,which streams source MAC are 00:00:03:00:0F:00,
            00:00:03:00:0F:01,00:00:03:00:0F:02 respectively (make sure generate different value by
            the policy ((dst_mac XOR src_mac) % (number of slave ports)) ) and dest MAC all is 
            90:E2:BA:4A:54:80(this value should been your unbonded port address). Respectively
            send 100 packets to port3 by the 3 streams.

            testpmd> show port stats all----(Verify port3 have 300 rx packets,
            port4 have 300 tx packets,and port 0-2 have 100 tx packets respectively)

  Expected test result:
            Verify the RX/TX status of bonded device and slaves in mode 2.

- Case: Mode 2(Balance XOR) TX Load Balance Link down
  Description: 
		Use Setup#1.
		ring link down of one slave.
            Send 300 packets from non-bonded port(port3), and verify these 
		packets will be forwarded to bonded device. 
            Verify that each active slave receive correct number of packets
            (according to the mode policy), and the down slave will not receive packets.
		Link down slave 0 at the remote end.Create 3 streams in the IXIA like case13.
		Then respectively send 100 packets to port3 by the 3 streams.

  Expected test result:
            Verify the RX/TX status of bonded device and slaves in mode 2.

- Case: Mode 2(Balance XOR) Bring all slave links down
  Description: 
		Use Setup#1.
		Bring all slave links down.
		Verify that bonded callback for link down is called.
		Verify no packet can be sent.

		Bring all slaves to link down at the remote end.Create 
		3 streams in the IXIA like case13.Then respectively send 100 packets 
		to port3 by the 3 streams.

		testpmd> show port info 4----(Verify bonding port has been linked down)
   	      testpmd> show port stats all----(Verify port3 have 300 rx packets,meanwhile
							 port 0,1,2 and 4 don`t have any packet)
  Expected test result:
            Verify the RX/TX status of bonded device and slaves in mode 2.


- Case: Mode 2(Balance XOR) Layer 3+4 forwarding 	
  Description: 
		Use Setup#1.
		Use "xmit_hash_policy()" to change to this forwarding mode
		Create a stream of traffic which will exercise all slave ports using the transmit policy 

   		 ((SRC_PORT XOR DST_PORT) XOR ((SRC_IP XOR DST_IP) AND 0xffff) % # of Slaves

		Transmit data through bonded device, verify TX packet count for each slave port is as expected

		Create 3 streams on a port which is mapped with unbonded port 3 at the IXIA end.
	      And set different IP by the transmit policy.Then send 100 packets on each stream.

           testpmd> show port stats all----(Verify port3 have 300 rx packets,
				meanwhile port4 have 300 rx packets, port 0-2 have 100 tx packets respectively)

		Add Vlan tag in the previouse 3 streams. Everthing else remains the same. 
		Then send 100 packets on each stream. Verify the packet statics is the same as above.  

   	      testpmd> show port stats all----(Verify port3  
		have 300 rx packets,meanwhile port4 have 300 rx packets,port 0-2 have 100 tx packets respecitvely)
  Expected test result:
            Verify the RX/TX status of bonded device and slaves in mode 2.

- Case: Mode 2(Balance XOR) RX test
  Description: 
		Use Setup#1.
		Send 100 packets to each bonded slaves(port0,1,2)
		Verify that each slave receives 100 packets and the bonded device receive a total 300 packets.
		Verify that the bonded device forwards 300 packets to the non-bonded port(port4).

		Send 100 packets to port0,1,2 respectively.

    		testpmd> show port stats all----(Verify port0,1,2 have 100 tx packets respectively 
				and port4 have 300 rx packets,and port3 have 300 tx packets)

  Expected test result:
            Verify the RX/TX status of bonded device and slaves in mode 2.

- Case: Mode 3(Broadcast) TX/RX Test
  Description: 
		Use Setup#1.
		Add ports 0-2 as slave devices to the bonded port 4. Make all slaves
	      to be active slaves on bonded device. 

            RX: Send a packet stream(100 packets) from port A on the traffic
		    generator to be forwarded through the bonded port4 to port3.
		    Verify the sum of the packets transmitted from the traffic generator
		    portA is equal the total received packets on port0, port4 and portD(Traffic generator).

               testpmd> show port stats all---(Verify port0 receive 100 packets, 
				and port4 receive 100 packets, and port3 transmit 100 packets)

           TX: Send a packet stream(100 packets) from portD on the traffic generator 
               to be forwarded through port3 to the bonded port4. Verify the sum of 
               the packets(100packets) transmitted from the traffic generator port is equal the total 
               received packets on port4, portA and transmitted to port0.
    
          testpmd> show port stats all---(Verify port3 RX 100 packets, 
                    and port0,1,2 TX 100 packets,port4 has 300 TX packets)
  Expected test result:
            Verify the RX/TX status of bonded device and slaves in mode 3.


- Case: Mode 3(Broadcast) Bring one slave link down
  Description: 
		Use Setup#1.
		Bring one slave port link down. Send 100 packets through portD to port3,
	      then port3 forwards to bondede device(port4), verify that the bonded device 
		and other slaves TX the correct number of packets(100 packets for each port).
  Expected test result:
            Verify the RX/TX status of bonded device and slaves in mode 3.

- Case: Mode 3(Broadcast) Bring all slave links down
  Description: 
		Use Setup#1.
		Bring all slave ports of bonded port down
		Verify that bonded callback for link down is called
		Verify that data cannot be sent or received through bonded port.
  Expected test result:
            Verify the RX/TX status of bonded device and slaves in mode 3.

- Case: Mode 5(Transmit load balance) basic test  ============================================================================

Use Setup#1

Create bonded device. Add first slave - port 0. Verify default bonded device has default mode 5. Verify bonded device MAC address is that of primary slave. Add another slaves port 1 and 2 to the bonded device. Verify that their MAC are different.

Bring the primary slave down. Verify if bonding interface reconfigured itself. The next port (in this scenario port 1) should became a primary one but the MAC should not be changed - should still be the MAC of port 0.

Bring a slave of bonding device down. Verify if bonding interface reconfigured itself.

Bring the portA, portB and portC down. Verify the bonded device will link down.

Set bonded device promiscuous mode to be off. Verify only the promiscuous state of primary interface will be off.


- Case: Mode 5(Transmit load balance)  TX/RX test ============================================================================

Use Setup#1

Create a bonded device(port4 in this scenario). Add port 0-2 as slaves of bonding port4. Make packets to transmit between port3 and port4. Then start to forward streams.

TX: Prepare a packet stream which will send packets in 5 minutes. Send this stream from portD to port3. Verify that port3 will receive all packets---no missed packets and no error packets. Port 0-2 transmitting packets will be represented as packet0, packet1 and packet2 respectively and their average packets will be represented as mean.  Then verify port 0-2 will meet the situation:
mean = (packet0 + packet1 + packet2)/3, packet0, packet1 and packet2 will be higher than 90% of mean and lower than 110% of mean. Then the test of port0, port1 and port2 balancing are finished positive.

RX: A continuation of TX testing setup. Prepare 3 packet streams and each stream has 100 packets. Respectively, send these streams from portA, portB and portC. Verify only the primary slave port0 will receive 100 packets, and port3 will transmit 100 packets.


- Case: Mode 5(Transmit load balance)  Bring one slave link down ============================================================================

Use Setup#1

Create a bonded device(port4 in this scenario). Add port0, port1 and port2 as slaves of bonding port4. Make packets to transmit between port3 and port4. Then start to forward streams.

TX: Prepare a packet stream which will send packets in 5 minutes. Bringing port0 link down in the remote end. Send this stream from portD to port3. After the time of 5 minutes is out, verify that port3 will receive all packets---no missed packets and no error packets. Port 1-2 transmitting packets will be represented as packet1 and packet2 respectively and their average packets will be represented as mean. Then verify port1 and port2 will meet the situation:
mean = (packet1 + packet2)/2, packet1 and packet2 will be higher than 90% of mean and lower than 110% of mean. Then the test of port1 and port2 balancing are finished positive.

RX: Prepare 3 packet streams and each stream has 100 packets. Respectively, send these streams from portA, portB and portC. Verify the primary port(port0 this scenario) will receive 100 packets, and port1 and port2 will receive nothing, and port3 will transmit 100 packets.


- Case: Mode 5(Transmit load balance)  Bring all slaves link down =============================================================================

Use Setup#1

First, set the same setup as case 30. Second, bring all slaves link down.

TX: Prepare 1 packet stream which will have 300 packets. Send this stream from portD to port3. Verify port0, port1 and port2 will transmit nothing.

RX: Prepare 3 packet streams and each stream has 100 packets. Respectively, send these streams from portA, portB and portC. Verify port0, port1 and port2 will receive nothing.

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Declan Doherty
> Sent: Tuesday, December 16, 2014 8:45 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] bond: vlan flags misinterpreted in
> xmit_slave_hash function
> 
> This patch contains a fix for link bonding handling of vlan tagged
> packets in mode 3 and 5.
> Currently xmit_slave_hash function misinterprets the PKT_RX_VLAN_PKT
> flag to mean that
> there is a vlan tag within the packet when in actually means that there
> is a valid entry
> in the vlan_tci field in the mbuf.
> 
> -v2:
> doxygen comments for rte_ip.h
> 
> - Fixed VLAN tag support in hashing functions.
> - Adds support for TCP in layer 4 header hashing.
> - Splits transmit hashing function into separate functions for each
> policy to
>   reduce branching and to make the code clearer.
> - Fixed incorrect flag set in test application packet generator.
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> ---
>  app/test/packet_burst_generator.c          |   2 +-
>  lib/librte_net/rte_ip.h                    |   8 ++
>  lib/librte_pmd_bond/rte_eth_bond_api.c     |   8 ++
>  lib/librte_pmd_bond/rte_eth_bond_pmd.c     | 162 ++++++++++++++++-----
> --------
>  lib/librte_pmd_bond/rte_eth_bond_private.h |  15 +++
>  5 files changed, 122 insertions(+), 73 deletions(-)
> 
> diff --git a/app/test/packet_burst_generator.c
> b/app/test/packet_burst_generator.c
> index b2824dc..4a89663 100644
> --- a/app/test/packet_burst_generator.c
> +++ b/app/test/packet_burst_generator.c
> @@ -97,7 +97,7 @@ initialize_eth_header(struct ether_hdr *eth_hdr,
> struct ether_addr *src_mac,
>  		vhdr->eth_proto =  rte_cpu_to_be_16(ETHER_TYPE_IPv4);
>  		vhdr->vlan_tci = van_id;
>  	} else {
> -		eth_hdr->ether_type = rte_cpu_to_be_16(ETHER_TYPE_VLAN);
> +		eth_hdr->ether_type = rte_cpu_to_be_16(ETHER_TYPE_IPv4);
>  	}
> 
>  }
> diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
> index 46f0497..3446e1c 100644
> --- a/lib/librte_net/rte_ip.h
> +++ b/lib/librte_net/rte_ip.h
> @@ -110,6 +110,14 @@ struct ipv4_hdr {
>  					   (((c) & 0xff) << 8)  | \
>  					   ((d) & 0xff))
> 
> +/** Internet header length mask for version_ihl field */
> +#define IPV4_HDR_IHL_MASK	(0x0f)
> +/**
> + * Internet header length field multiplier (IHL field specifies
> overall header
> + * length in number of 4-byte words)
> + */
> +#define IPV4_IHL_MULTIPLIER	(4)
> +
>  /* Fragment Offset * Flags. */
>  #define	IPV4_HDR_DF_SHIFT	14
>  #define	IPV4_HDR_MF_SHIFT	13
> diff --git a/lib/librte_pmd_bond/rte_eth_bond_api.c
> b/lib/librte_pmd_bond/rte_eth_bond_api.c
> index ef5ddf4..fb015a8 100644
> --- a/lib/librte_pmd_bond/rte_eth_bond_api.c
> +++ b/lib/librte_pmd_bond/rte_eth_bond_api.c
> @@ -268,6 +268,7 @@ rte_eth_bond_create(const char *name, uint8_t mode,
> uint8_t socket_id)
>  	internals->mode = BONDING_MODE_INVALID;
>  	internals->current_primary_port = 0;
>  	internals->balance_xmit_policy = BALANCE_XMIT_POLICY_LAYER2;
> +	internals->xmit_hash = xmit_l2_hash;
>  	internals->user_defined_mac = 0;
>  	internals->link_props_set = 0;
> 
> @@ -710,9 +711,16 @@ rte_eth_bond_xmit_policy_set(uint8_t
> bonded_port_id, uint8_t policy)
> 
>  	switch (policy) {
>  	case BALANCE_XMIT_POLICY_LAYER2:
> +		internals->balance_xmit_policy = policy;
> +		internals->xmit_hash = xmit_l2_hash;
> +		break;
>  	case BALANCE_XMIT_POLICY_LAYER23:
> +		internals->balance_xmit_policy = policy;
> +		internals->xmit_hash = xmit_l23_hash;
> +		break;
>  	case BALANCE_XMIT_POLICY_LAYER34:
>  		internals->balance_xmit_policy = policy;
> +		internals->xmit_hash = xmit_l34_hash;
>  		break;
> 
>  	default:
> diff --git a/lib/librte_pmd_bond/rte_eth_bond_pmd.c
> b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
> index 3db473b..c055911 100644
> --- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c
> +++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
> @@ -31,6 +31,8 @@
>   *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
>   */
>  #include <stdlib.h>
> +#include <netinet/in.h>
> +
>  #include <rte_mbuf.h>
>  #include <rte_malloc.h>
>  #include <rte_ethdev.h>
> @@ -48,6 +50,9 @@
>  #include "rte_eth_bond_8023ad_private.h"
> 
>  #define REORDER_PERIOD_MS 10
> +
> +#define HASH_L4_PORTS(h) ((h)->src_port ^ (h)->dst_port)
> +
>  /* Table for statistics in mode 5 TLB */
>  static uint64_t tlb_last_obytets[RTE_MAX_ETHPORTS];
> 
> @@ -276,90 +281,105 @@ ipv6_hash(struct ipv6_hdr *ipv6_hdr)
>  			(word_src_addr[3] ^ word_dst_addr[3]);
>  }
> 
> -static uint32_t
> -udp_hash(struct udp_hdr *hdr)
> +static inline size_t
> +get_vlan_offset(struct ether_hdr *eth_hdr)
>  {
> -	return hdr->src_port ^ hdr->dst_port;
> +	size_t vlan_offset = 0;
> +
> +	/* Calculate VLAN offset */
> +	if (rte_cpu_to_be_16(ETHER_TYPE_VLAN) == eth_hdr->ether_type) {
> +		struct vlan_hdr *vlan_hdr = (struct vlan_hdr *)(eth_hdr +
> 1);
> +		vlan_offset = sizeof(struct vlan_hdr);
> +
> +		while (rte_cpu_to_be_16(ETHER_TYPE_VLAN) ==
> +				vlan_hdr->eth_proto) {
> +			vlan_hdr = vlan_hdr + 1;
> +			vlan_offset += sizeof(struct vlan_hdr);
> +		}
> +	}
> +	return vlan_offset;
>  }
> 
> -static inline uint16_t
> -xmit_slave_hash(const struct rte_mbuf *buf, uint8_t slave_count,
> uint8_t policy)
> +uint16_t
> +xmit_l2_hash(const struct rte_mbuf *buf, uint8_t slave_count)
>  {
> -	struct ether_hdr *eth_hdr;
> -	struct udp_hdr *udp_hdr;
> -	size_t eth_offset = 0;
> -	uint32_t hash = 0;
> -
> -	if (slave_count == 1)
> -		return 0;
> +	struct ether_hdr *eth_hdr = rte_pktmbuf_mtod(buf, struct
> ether_hdr *);
> 
> -	switch (policy) {
> -	case BALANCE_XMIT_POLICY_LAYER2:
> -		eth_hdr = rte_pktmbuf_mtod(buf, struct ether_hdr *);
> +	uint32_t hash = ether_hash(eth_hdr);
> 
> -		hash = ether_hash(eth_hdr);
> -		hash ^= hash >> 8;
> -		return hash % slave_count;
> +	return (hash ^= hash >> 8) % slave_count;
> +}
> 
> -	case BALANCE_XMIT_POLICY_LAYER23:
> -		eth_hdr = rte_pktmbuf_mtod(buf, struct ether_hdr *);
> +uint16_t
> +xmit_l23_hash(const struct rte_mbuf *buf, uint8_t slave_count)
> +{
> +	struct ether_hdr *eth_hdr = rte_pktmbuf_mtod(buf, struct
> ether_hdr *);
> +	size_t vlan_offset = get_vlan_offset(eth_hdr);
> +	uint32_t hash, l3hash = 0;
> 
> -		if (buf->ol_flags & PKT_RX_VLAN_PKT)
> -			eth_offset = sizeof(struct ether_hdr) + sizeof(struct
> vlan_hdr);
> -		else
> -			eth_offset = sizeof(struct ether_hdr);
> +	hash = ether_hash(eth_hdr);
> 
> -		if (buf->ol_flags & PKT_RX_IPV4_HDR) {
> -			struct ipv4_hdr *ipv4_hdr;
> -			ipv4_hdr = (struct ipv4_hdr *)(rte_pktmbuf_mtod(buf,
> -					unsigned char *) + eth_offset);
> +	if (buf->ol_flags & PKT_RX_IPV4_HDR) {
> +		struct ipv4_hdr *ipv4_hdr = (struct ipv4_hdr *)
> +				((char *)(eth_hdr + 1) + vlan_offset);
> +		l3hash = ipv4_hash(ipv4_hdr);
> 
> -			hash = ether_hash(eth_hdr) ^ ipv4_hash(ipv4_hdr);
> +	} else if  (buf->ol_flags & PKT_RX_IPV6_HDR) {
> +		struct ipv6_hdr *ipv6_hdr = (struct ipv6_hdr *)
> +				((char *)(eth_hdr + 1) + vlan_offset);
> +		l3hash = ipv6_hash(ipv6_hdr);
> +	}
> 
> -		} else {
> -			struct ipv6_hdr *ipv6_hdr;
> +	hash = hash ^ l3hash;
> +	hash ^= hash >> 16;
> +	hash ^= hash >> 8;
> 
> -			ipv6_hdr = (struct ipv6_hdr *)(rte_pktmbuf_mtod(buf,
> -					unsigned char *) + eth_offset);
> +	return hash % slave_count;
> +}
> 
> -			hash = ether_hash(eth_hdr) ^ ipv6_hash(ipv6_hdr);
> +uint16_t
> +xmit_l34_hash(const struct rte_mbuf *buf, uint8_t slave_count)
> +{
> +	struct ether_hdr *eth_hdr = rte_pktmbuf_mtod(buf, struct
> ether_hdr *);
> +	size_t vlan_offset = get_vlan_offset(eth_hdr);
> +	struct udp_hdr *udp_hdr = NULL;
> +	struct tcp_hdr *tcp_hdr = NULL;
> +	uint32_t hash, l3hash = 0, l4hash = 0;
> +
> +	if (buf->ol_flags & PKT_RX_IPV4_HDR) {
> +		struct ipv4_hdr *ipv4_hdr = (struct ipv4_hdr *)
> +				((char *)(eth_hdr + 1) + vlan_offset);
> +		size_t ip_hdr_offset;
> +
> +		l3hash = ipv4_hash(ipv4_hdr);
> +
> +		ip_hdr_offset = (ipv4_hdr->version_ihl & IPV4_HDR_IHL_MASK)
> *
> +				IPV4_IHL_MULTIPLIER;
> +
> +		if (ipv4_hdr->next_proto_id == IPPROTO_TCP) {
> +			tcp_hdr = (struct tcp_hdr *)((char *)ipv4_hdr +
> +					ip_hdr_offset);
> +			l4hash = HASH_L4_PORTS(tcp_hdr);
> +		} else if (ipv4_hdr->next_proto_id == IPPROTO_UDP) {
> +			udp_hdr = (struct udp_hdr *)((char *)ipv4_hdr +
> +					ip_hdr_offset);
> +			l4hash = HASH_L4_PORTS(udp_hdr);
>  		}
> -		break;
> -
> -	case BALANCE_XMIT_POLICY_LAYER34:
> -		if (buf->ol_flags & PKT_RX_VLAN_PKT)
> -			eth_offset = sizeof(struct ether_hdr) + sizeof(struct
> vlan_hdr);
> -		else
> -			eth_offset = sizeof(struct ether_hdr);
> -
> -		if (buf->ol_flags & PKT_RX_IPV4_HDR) {
> -			struct ipv4_hdr *ipv4_hdr = (struct ipv4_hdr *)
> -					(rte_pktmbuf_mtod(buf, unsigned char *) +
> eth_offset);
> -
> -			if (ipv4_hdr->next_proto_id == IPPROTO_UDP) {
> -				udp_hdr = (struct udp_hdr *)
> -						(rte_pktmbuf_mtod(buf, unsigned
> char *) + eth_offset +
> -								sizeof(struct
> ipv4_hdr));
> -				hash = ipv4_hash(ipv4_hdr) ^ udp_hash(udp_hdr);
> -			} else {
> -				hash = ipv4_hash(ipv4_hdr);
> -			}
> -		} else {
> -			struct ipv6_hdr *ipv6_hdr = (struct ipv6_hdr *)
> -					(rte_pktmbuf_mtod(buf, unsigned char *) +
> eth_offset);
> -
> -			if (ipv6_hdr->proto == IPPROTO_UDP) {
> -				udp_hdr = (struct udp_hdr *)
> -						(rte_pktmbuf_mtod(buf, unsigned
> char *) + eth_offset +
> -								sizeof(struct
> ipv6_hdr));
> -				hash = ipv6_hash(ipv6_hdr) ^ udp_hash(udp_hdr);
> -			} else {
> -				hash = ipv6_hash(ipv6_hdr);
> -			}
> +	} else if  (buf->ol_flags & PKT_RX_IPV6_HDR) {
> +		struct ipv6_hdr *ipv6_hdr = (struct ipv6_hdr *)
> +				((char *)(eth_hdr + 1) + vlan_offset);
> +		l3hash = ipv6_hash(ipv6_hdr);
> +
> +		if (ipv6_hdr->proto == IPPROTO_TCP) {
> +			tcp_hdr = (struct tcp_hdr *)(ipv6_hdr + 1);
> +			l4hash = HASH_L4_PORTS(tcp_hdr);
> +		} else if (ipv6_hdr->proto == IPPROTO_UDP) {
> +			udp_hdr = (struct udp_hdr *)(ipv6_hdr + 1);
> +			l4hash = HASH_L4_PORTS(udp_hdr);
>  		}
> -		break;
>  	}
> 
> +	hash = l3hash ^ l4hash;
>  	hash ^= hash >> 16;
>  	hash ^= hash >> 8;
> 
> @@ -536,8 +556,7 @@ bond_ethdev_tx_burst_balance(void *queue, struct
> rte_mbuf **bufs,
>  	/* Populate slaves mbuf with the packets which are to be sent on
> it  */
>  	for (i = 0; i < nb_pkts; i++) {
>  		/* Select output slave using hash based on xmit policy */
> -		op_slave_id = xmit_slave_hash(bufs[i], num_of_slaves,
> -				internals->balance_xmit_policy);
> +		op_slave_id = internals->xmit_hash(bufs[i], num_of_slaves);
> 
>  		/* Populate slave mbuf arrays with mbufs for that slave */
>  		slave_bufs[op_slave_id][slave_nb_pkts[op_slave_id]++] =
> bufs[i];
> @@ -575,7 +594,7 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct
> rte_mbuf **bufs,
> 
>  	uint8_t num_of_slaves;
>  	uint8_t slaves[RTE_MAX_ETHPORTS];
> -	 /* possitions in slaves, not ID */
> +	 /* positions in slaves, not ID */
>  	uint8_t distributing_offsets[RTE_MAX_ETHPORTS];
>  	uint8_t distributing_count;
> 
> @@ -622,8 +641,7 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct
> rte_mbuf **bufs,
>  		/* Populate slaves mbuf with the packets which are to be
> sent on it */
>  		for (i = 0; i < nb_pkts; i++) {
>  			/* Select output slave using hash based on xmit
> policy */
> -			op_slave_idx = xmit_slave_hash(bufs[i],
> distributing_count,
> -					internals->balance_xmit_policy);
> +			op_slave_idx = internals->xmit_hash(bufs[i],
> distributing_count);
> 
>  			/* Populate slave mbuf arrays with mbufs for that
> slave. Use only
>  			 * slaves that are currently distributing. */
> diff --git a/lib/librte_pmd_bond/rte_eth_bond_private.h
> b/lib/librte_pmd_bond/rte_eth_bond_private.h
> index f913c5b..e01e66b 100644
> --- a/lib/librte_pmd_bond/rte_eth_bond_private.h
> +++ b/lib/librte_pmd_bond/rte_eth_bond_private.h
> @@ -108,6 +108,9 @@ struct bond_slave_details {
>  	struct ether_addr persisted_mac_addr;
>  };
> 
> +
> +typedef uint16_t (*xmit_hash_t)(const struct rte_mbuf *buf, uint8_t
> slave_count);
> +
>  /** Link Bonding PMD device private configuration Structure */
>  struct bond_dev_private {
>  	uint8_t port_id;					/**< Port Id of Bonded
> Port */
> @@ -122,6 +125,9 @@ struct bond_dev_private {
> 
>  	uint8_t balance_xmit_policy;
>  	/**< Transmit policy - l2 / l23 / l34 for operation in balance
> mode */
> +	xmit_hash_t xmit_hash;
> +	/**< Transmit policy hash function */
> +
>  	uint8_t user_defined_mac;
>  	/**< Flag for whether MAC address is user defined or not */
>  	uint8_t promiscuous_en;
> @@ -225,6 +231,15 @@ void
>  slave_add(struct bond_dev_private *internals,
>  		struct rte_eth_dev *slave_eth_dev);
> 
> +uint16_t
> +xmit_l2_hash(const struct rte_mbuf *buf, uint8_t slave_count);
> +
> +uint16_t
> +xmit_l23_hash(const struct rte_mbuf *buf, uint8_t slave_count);
> +
> +uint16_t
> +xmit_l34_hash(const struct rte_mbuf *buf, uint8_t slave_count);
> +
>  void
>  bond_ethdev_primary_set(struct bond_dev_private *internals,
>  		uint8_t slave_port_id);
> --
> 1.7.12.2

Re: [dpdk-dev] [PATCH v2] bond: vlan flags misinterpreted in xmit_slave_hash function

[Thomas Monjalon]

> This patch contains a fix for link bonding handling of vlan tagged packets in mode 3 and 5.
> Currently xmit_slave_hash function misinterprets the PKT_RX_VLAN_PKT flag to mean that
> there is a vlan tag within the packet when in actually means that there is a valid entry
> in the vlan_tci field in the mbuf.
> 
> -v2:
> doxygen comments for rte_ip.h
> 
> - Fixed VLAN tag support in hashing functions.
> - Adds support for TCP in layer 4 header hashing.
> - Splits transmit hashing function into separate functions for each policy to
>   reduce branching and to make the code clearer.
> - Fixed incorrect flag set in test application packet generator.
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
Acked-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Tested-by: SunX Jiajia <sunx.jiajia@intel.com>

Applied

Thanks
-- 
Thomas