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

2014-12-16 Thread 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 
> ---
>  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


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

2014-12-16 Thread Wodkowski, PawelX
> -Original Message-
> From: Doherty, Declan
> Sent: Tuesday, December 16, 2014 12:16 PM
> To: dev at 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 


Acked-by: Wodkowski, Pawel 





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

2014-12-16 Thread 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 
---
 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. */
 #defineIPV4_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 
+#include 
+
 #include 
 #include 
 #include 
@@ -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