diff mbox

[dpdk-dev] bond: remove ol flags from transmit policy checks

Message ID 1422637337-6382-1-git-send-email-declan.doherty@intel.com (mailing list archive)
State Accepted, archived
Headers show

Commit Message

Doherty, Declan Jan. 30, 2015, 5:02 p.m. UTC
The Link bonding library is incorrectly using receive packet type flags
in the transmit policy hashing functions, which would cause packets
generated locally to be incorrectly distributed across the slave
devices. This patch completely removes the dependency on the packet
type flags and uses the ether_type from either the Ethernet header or
the VLAN headers for branching.

This patch also includes the associate changes in the test suite and in
the packet_burst_generator code to remove the dependences on the packet
type flags.

Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
 app/test/packet_burst_generator.c      | 24 +++++++++++-------------
 app/test/packet_burst_generator.h      |  3 ++-
 app/test/test_link_bonding.c           | 16 ++++++++--------
 app/test/test_pmd_perf.c               |  2 +-
 lib/librte_pmd_bond/rte_eth_bond_pmd.c | 26 +++++++++++++++-----------
 5 files changed, 37 insertions(+), 34 deletions(-)

Comments

Wodkowski, PawelX Feb. 2, 2015, 9:57 a.m. UTC | #1
On 2015-01-30 18:02, Declan Doherty wrote:
> The Link bonding library is incorrectly using receive packet type flags
> in the transmit policy hashing functions, which would cause packets
> generated locally to be incorrectly distributed across the slave
> devices. This patch completely removes the dependency on the packet
> type flags and uses the ether_type from either the Ethernet header or
> the VLAN headers for branching.
>
> This patch also includes the associate changes in the test suite and in
> the packet_burst_generator code to remove the dependences on the packet
> type flags.
>
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> ---
>   app/test/packet_burst_generator.c      | 24 +++++++++++-------------
>   app/test/packet_burst_generator.h      |  3 ++-
>   app/test/test_link_bonding.c           | 16 ++++++++--------
>   app/test/test_pmd_perf.c               |  2 +-
>   lib/librte_pmd_bond/rte_eth_bond_pmd.c | 26 +++++++++++++++-----------
>   5 files changed, 37 insertions(+), 34 deletions(-)
>

Reviewed-by: Wodkowski Pawel <pawelx.wodkowski@intel.com>
Thomas Monjalon Feb. 2, 2015, 11:37 a.m. UTC | #2
2015-02-02 10:57, Pawel Wodkowski:
> On 2015-01-30 18:02, Declan Doherty wrote:
> > The Link bonding library is incorrectly using receive packet type flags
> > in the transmit policy hashing functions, which would cause packets
> > generated locally to be incorrectly distributed across the slave
> > devices. This patch completely removes the dependency on the packet
> > type flags and uses the ether_type from either the Ethernet header or
> > the VLAN headers for branching.
> >
> > This patch also includes the associate changes in the test suite and in
> > the packet_burst_generator code to remove the dependences on the packet
> > type flags.
> >
> > Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> > ---
> >   app/test/packet_burst_generator.c      | 24 +++++++++++-------------
> >   app/test/packet_burst_generator.h      |  3 ++-
> >   app/test/test_link_bonding.c           | 16 ++++++++--------
> >   app/test/test_pmd_perf.c               |  2 +-
> >   lib/librte_pmd_bond/rte_eth_bond_pmd.c | 26 +++++++++++++++-----------
> >   5 files changed, 37 insertions(+), 34 deletions(-)
> >
> 
> Reviewed-by: Wodkowski Pawel <pawelx.wodkowski@intel.com>

Applied

Thanks
diff mbox

Patch

diff --git a/app/test/packet_burst_generator.c b/app/test/packet_burst_generator.c
index 4a89663..e9d059c 100644
--- a/app/test/packet_burst_generator.c
+++ b/app/test/packet_burst_generator.c
@@ -83,7 +83,8 @@  copy_buf_to_pkt(void *buf, unsigned len, struct rte_mbuf *pkt, unsigned offset)
 
 void
 initialize_eth_header(struct ether_hdr *eth_hdr, struct ether_addr *src_mac,
-		struct ether_addr *dst_mac, uint8_t vlan_enabled, uint16_t van_id)
+		struct ether_addr *dst_mac, uint8_t ipv4, uint8_t vlan_enabled,
+		uint16_t van_id)
 {
 	ether_addr_copy(dst_mac, &eth_hdr->d_addr);
 	ether_addr_copy(src_mac, &eth_hdr->s_addr);
@@ -94,10 +95,17 @@  initialize_eth_header(struct ether_hdr *eth_hdr, struct ether_addr *src_mac,
 
 		eth_hdr->ether_type = rte_cpu_to_be_16(ETHER_TYPE_VLAN);
 
-		vhdr->eth_proto =  rte_cpu_to_be_16(ETHER_TYPE_IPv4);
+		if (ipv4)
+			vhdr->eth_proto =  rte_cpu_to_be_16(ETHER_TYPE_IPv4);
+		else
+			vhdr->eth_proto =  rte_cpu_to_be_16(ETHER_TYPE_IPv6);
+
 		vhdr->vlan_tci = van_id;
 	} else {
-		eth_hdr->ether_type = rte_cpu_to_be_16(ETHER_TYPE_IPv4);
+		if (ipv4)
+			eth_hdr->ether_type = rte_cpu_to_be_16(ETHER_TYPE_IPv4);
+		else
+			eth_hdr->ether_type = rte_cpu_to_be_16(ETHER_TYPE_IPv6);
 	}
 
 }
@@ -257,19 +265,9 @@  nomore_mbuf:
 		if (ipv4) {
 			pkt->vlan_tci  = ETHER_TYPE_IPv4;
 			pkt->l3_len = sizeof(struct ipv4_hdr);
-
-			if (vlan_enabled)
-				pkt->ol_flags = PKT_RX_IPV4_HDR | PKT_RX_VLAN_PKT;
-			else
-				pkt->ol_flags = PKT_RX_IPV4_HDR;
 		} else {
 			pkt->vlan_tci  = ETHER_TYPE_IPv6;
 			pkt->l3_len = sizeof(struct ipv6_hdr);
-
-			if (vlan_enabled)
-				pkt->ol_flags = PKT_RX_IPV6_HDR | PKT_RX_VLAN_PKT;
-			else
-				pkt->ol_flags = PKT_RX_IPV6_HDR;
 		}
 
 		pkts_burst[nb_pkt] = pkt;
diff --git a/app/test/packet_burst_generator.h b/app/test/packet_burst_generator.h
index f86589e..666cc8e 100644
--- a/app/test/packet_burst_generator.h
+++ b/app/test/packet_burst_generator.h
@@ -53,7 +53,8 @@  extern "C" {
 
 void
 initialize_eth_header(struct ether_hdr *eth_hdr, struct ether_addr *src_mac,
-		struct ether_addr *dst_mac, uint8_t vlan_enabled, uint16_t van_id);
+		struct ether_addr *dst_mac, uint8_t ipv4, uint8_t vlan_enabled,
+		uint16_t van_id);
 
 uint16_t
 initialize_udp_header(struct udp_hdr *udp_hdr, uint16_t src_port,
diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c
index 4523de6..579ebbf 100644
--- a/app/test/test_link_bonding.c
+++ b/app/test/test_link_bonding.c
@@ -1319,11 +1319,11 @@  generate_test_burst(struct rte_mbuf **pkts_burst, uint16_t burst_size,
 	if (toggle_dst_mac)
 		initialize_eth_header(test_params->pkt_eth_hdr,
 				(struct ether_addr *)src_mac, (struct ether_addr *)dst_mac_1,
-				vlan, vlan_id);
+				ipv4, vlan, vlan_id);
 	else
 		initialize_eth_header(test_params->pkt_eth_hdr,
 				(struct ether_addr *)src_mac, (struct ether_addr *)dst_mac_0,
-				vlan, vlan_id);
+				ipv4, vlan, vlan_id);
 
 
 	if (toggle_udp_port)
@@ -2094,7 +2094,7 @@  test_activebackup_tx_burst(void)
 			"Failed to initialize bonded device with slaves");
 
 	initialize_eth_header(test_params->pkt_eth_hdr,
-			(struct ether_addr *)src_mac, (struct ether_addr *)dst_mac_0, 0, 0);
+			(struct ether_addr *)src_mac, (struct ether_addr *)dst_mac_0, 1, 0, 0);
 	pktlen = initialize_udp_header(test_params->pkt_udp_hdr, src_port,
 			dst_port_0, 16);
 	pktlen = initialize_ipv4_header(test_params->pkt_ipv4_hdr, src_addr,
@@ -2637,7 +2637,7 @@  test_balance_l2_tx_burst(void)
 			"Failed to set balance xmit policy.");
 
 	initialize_eth_header(test_params->pkt_eth_hdr,
-			(struct ether_addr *)src_mac, (struct ether_addr *)dst_mac_0, 0, 0);
+			(struct ether_addr *)src_mac, (struct ether_addr *)dst_mac_0, 1, 0, 0);
 	pktlen = initialize_udp_header(test_params->pkt_udp_hdr, src_port,
 			dst_port_0, 16);
 	pktlen = initialize_ipv4_header(test_params->pkt_ipv4_hdr, src_addr,
@@ -2651,7 +2651,7 @@  test_balance_l2_tx_burst(void)
 			"failed to generate packet burst");
 
 	initialize_eth_header(test_params->pkt_eth_hdr,
-			(struct ether_addr *)src_mac, (struct ether_addr *)dst_mac_1, 0, 0);
+			(struct ether_addr *)src_mac, (struct ether_addr *)dst_mac_1, 1, 0, 0);
 
 	/* Generate a burst 2 of packets to transmit */
 	TEST_ASSERT_EQUAL(generate_packet_burst(test_params->mbuf_pool, &pkts_burst[1][0],
@@ -3488,7 +3488,7 @@  test_broadcast_tx_burst(void)
 			"Failed to intialise bonded device");
 
 	initialize_eth_header(test_params->pkt_eth_hdr,
-			(struct ether_addr *)src_mac, (struct ether_addr *)dst_mac_0, 0, 0);
+			(struct ether_addr *)src_mac, (struct ether_addr *)dst_mac_0, 1, 0, 0);
 
 	pktlen = initialize_udp_header(test_params->pkt_udp_hdr, src_port,
 			dst_port_0, 16);
@@ -4068,11 +4068,11 @@  test_tlb_tx_burst(void)
 		if (i % 2 == 0) {
 			initialize_eth_header(test_params->pkt_eth_hdr,
 					(struct ether_addr *)src_mac,
-					(struct ether_addr *)dst_mac_0, 0, 0);
+					(struct ether_addr *)dst_mac_0, 1, 0, 0);
 		} else {
 			initialize_eth_header(test_params->pkt_eth_hdr,
 					(struct ether_addr *)test_params->default_slave_mac,
-					(struct ether_addr *)dst_mac_0, 0, 0);
+					(struct ether_addr *)dst_mac_0, 1, 0, 0);
 		}
 		pktlen = initialize_udp_header(test_params->pkt_udp_hdr, src_port,
 				dst_port_0, 16);
diff --git a/app/test/test_pmd_perf.c b/app/test/test_pmd_perf.c
index 941d099..bad9503 100644
--- a/app/test/test_pmd_perf.c
+++ b/app/test/test_pmd_perf.c
@@ -235,7 +235,7 @@  init_traffic(struct rte_mempool *mp,
 
 	initialize_eth_header(&pkt_eth_hdr,
 		(struct ether_addr *)src_mac,
-		(struct ether_addr *)dst_mac, 0, 0);
+		(struct ether_addr *)dst_mac, 1, 0, 0);
 	pkt_eth_hdr.ether_type = rte_cpu_to_be_16(ETHER_TYPE_IPv4);
 
 	pktlen = initialize_ipv4_header(&pkt_ipv4_hdr,
diff --git a/lib/librte_pmd_bond/rte_eth_bond_pmd.c b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
index 8b80297..09b0f30 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
@@ -282,18 +282,19 @@  ipv6_hash(struct ipv6_hdr *ipv6_hdr)
 }
 
 static inline size_t
-get_vlan_offset(struct ether_hdr *eth_hdr)
+get_vlan_offset(struct ether_hdr *eth_hdr, uint16_t *proto)
 {
 	size_t vlan_offset = 0;
 
-	/* Calculate VLAN offset */
-	if (rte_cpu_to_be_16(ETHER_TYPE_VLAN) == eth_hdr->ether_type) {
+	if (rte_cpu_to_be_16(ETHER_TYPE_VLAN) == *proto) {
 		struct vlan_hdr *vlan_hdr = (struct vlan_hdr *)(eth_hdr + 1);
 		vlan_offset = sizeof(struct vlan_hdr);
+		*proto = vlan_hdr->eth_proto;
 
-		while (rte_cpu_to_be_16(ETHER_TYPE_VLAN) ==
-				vlan_hdr->eth_proto) {
+		if (rte_cpu_to_be_16(ETHER_TYPE_VLAN) == *proto) {
 			vlan_hdr = vlan_hdr + 1;
+
+			*proto = vlan_hdr->eth_proto;
 			vlan_offset += sizeof(struct vlan_hdr);
 		}
 	}
@@ -314,17 +315,18 @@  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);
+	uint16_t proto = eth_hdr->ether_type;
+	size_t vlan_offset = get_vlan_offset(eth_hdr, &proto);
 	uint32_t hash, l3hash = 0;
 
 	hash = ether_hash(eth_hdr);
 
-	if (buf->ol_flags & PKT_RX_IPV4_HDR) {
+	if (rte_cpu_to_be_16(ETHER_TYPE_IPv4) == proto) {
 		struct ipv4_hdr *ipv4_hdr = (struct ipv4_hdr *)
 				((char *)(eth_hdr + 1) + vlan_offset);
 		l3hash = ipv4_hash(ipv4_hdr);
 
-	} else if  (buf->ol_flags & PKT_RX_IPV6_HDR) {
+	} else if (rte_cpu_to_be_16(ETHER_TYPE_IPv6) == proto) {
 		struct ipv6_hdr *ipv6_hdr = (struct ipv6_hdr *)
 				((char *)(eth_hdr + 1) + vlan_offset);
 		l3hash = ipv6_hash(ipv6_hdr);
@@ -341,12 +343,14 @@  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);
+	uint16_t proto = eth_hdr->ether_type;
+	size_t vlan_offset = get_vlan_offset(eth_hdr, &proto);
+
 	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) {
+	if (rte_cpu_to_be_16(ETHER_TYPE_IPv4) == proto) {
 		struct ipv4_hdr *ipv4_hdr = (struct ipv4_hdr *)
 				((char *)(eth_hdr + 1) + vlan_offset);
 		size_t ip_hdr_offset;
@@ -365,7 +369,7 @@  xmit_l34_hash(const struct rte_mbuf *buf, uint8_t slave_count)
 					ip_hdr_offset);
 			l4hash = HASH_L4_PORTS(udp_hdr);
 		}
-	} else if  (buf->ol_flags & PKT_RX_IPV6_HDR) {
+	} else if  (rte_cpu_to_be_16(ETHER_TYPE_IPv6) == proto) {
 		struct ipv6_hdr *ipv6_hdr = (struct ipv6_hdr *)
 				((char *)(eth_hdr + 1) + vlan_offset);
 		l3hash = ipv6_hash(ipv6_hdr);