[dpdk-dev] [PATCH v2 0/4] i40e VXLAN TX checksum rework

[Jijiang Liu]

We have got some feedback about backward compatibility of VXLAN TX checksum 
offload API with 1G/10G NIC after the i40e VXLAN TX checksum codes were 
applied, so we have to rework the APIs on i40e, including the changes of mbuf, 
i40e PMD and csum engine.

The main changes in mbuf are as follows,
In place of removing PKT_TX_VXLAN_CKSUM, we introducing 3 new flags: 
PKT_TX_OUTER_IP_CKSUM,PKT_TX_OUTER_IPV6 and PKT_TX_UDP_TUNNEL_PKT, and a new 
field: l4_tun_len.
Replace the inner_l2_len and the inner_l3_len field with the outer_l2_len and 
outer_l3_len field.

let's use a few examples to demonstrate how to use these new flags and existing 
flags in rte_mbuf.h
Let say we have a tunnel packet: 
eth_hdr_out/ipv4_hdr_out/udp_hdr_out/vxlan_hdr/ehtr_hdr_in/ipv4_hdr_in/tcp_hdr_in.There
 could be several scenarios:

A) User requests HW offload for ipv4_hdr_out checksum.
He doesn't care is it a tunnelled packet or not.
So he sets:

mb->l2_len =  eth_hdr_out;
mb->l3_len = ipv4_hdr_out;
mb->ol_flags |= PKT_TX_IPV4_CSUM;

B) User is aware that it is a tunnelled packet and requests HW offload for 
ipv4_hdr_in and tcp_hdr_in *only*.
He doesn't care about outer IP checksum offload.
In that case, for FVL  he has 2 choices:
   1. Treat that packet as a 'proper' tunnelled packet, and fill all the fields:
 mb->l2_len =  eth_hdr_in;
 mb->l3_len = ipv4_hdr_in;
 mb->outer_l2_len = eth_hdr_out;
 mb->outer_l3_len = ipv4_hdr_out;
 mb->l4tun_len = vxlan_hdr;
 mb->ol_flags |= PKT_TX_UDP_TUNNEL_PKT | PKT_TX_IP_CKSUM |  
PKT_TX_TCP_CKSUM;

   2. As user doesn't care about outer IP hdr checksum, he can treat everything 
before ipv4_hdr_in as L2 header.
   So he knows, that it is a tunnelled packet, but makes HW to treat it as 
ordinary (non-tunnelled) packet:
 mb->l2_len = eth_hdr_out + ipv4_hdr_out + udp_hdr_out + vxlan_hdr + 
ehtr_hdr_in;
 mb->l3_len = ipv4_hdr_in;
 mb->ol_flags |= PKT_TX_IP_CKSUM |  PKT_TX_TCP_CKSUM;

i40e PMD will support both B.1 and B.2.
ixgbe/igb/em PMD supports only B.2.
if HW supports both - it will be up to user app which method to choose.
tespmd will support both methods, and it should be configurable by user which 
approach to use (cmdline parameter).
So the user can try/test both methods and select an appropriate for him.

Now, B.2 is exactly what Oliver suggested.
I think it has few important advantages over B.1:
First of all - compatibility. It works across all HW we currently support 
(i40e/ixgbe/igb/em).
Second - it is probably faster even on FVL, as for it we have to fill only TXD, 
while with approach #2 we have to fill both TCD and TXD.

C) User knows that is a tunnelled packet, and wants HW offload for all 3 
checksums:  outer IP hdr checksum, inner IP checksum, inner TCP checksum.
Then he has to setup all TX checksum fields:
 mb->l2_len =  eth_hdr_in;
 mb->l3_len = ipv4_hdr_in;
 mb->outer_l2_len = eth_hdr_out;
 mb->outer_l3_len = ipv4_hdr_out;
 mb->l4tun_len = vxlan_hdr;
 mb->ol_flags |= PKT_TX_OUT_IP_CKSUM  | PKT_TX_UDP_TUNNEL | PKT_TX_IP_CKSUM 
|  PKT_TX_TCP_CKSUM;

v2 changes:
 remove PKT_TX_IP_CKSUM alias.
 add PKT_TX_OUT_IP_CKSUM and PKT_TX_OUTER_IPV6 in rte_get_tx_ol_flag_name.
 spliting mbuf changes into two patches.
 fix MACLEN caculation issue in i40e driver
 fix some issues in csumonly.c
 change cover letter.

Jijiang Liu (4):
  mbuf change for 3 new flags and 3 fields
  mbuf change for PKT_TX_IPV4 and PKT_TX_IPV6
  i40e PMD change in i40e_rxtx.c
  rework csum forward engine


 app/test-pmd/csumonly.c |   65 ++-
 lib/librte_mbuf/rte_mbuf.c  |6 +++-
 lib/librte_mbuf/rte_mbuf.h  |   22 -
 lib/librte_pmd_i40e/i40e_rxtx.c |   49 -
 4 files changed, 82 insertions(+), 60 deletions(-)

-- 
1.7.7.6

[dpdk-dev] [PATCH v2 3/4] i40e:PMD change for VXLAN TX checksum

[Jijiang Liu]

Rework the i40e PMD codes using the new introduced ol_flags and fields.  

Signed-off-by: Jijiang Liu 
---
 lib/librte_pmd_i40e/i40e_rxtx.c |   49 +-
 1 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
index cce6911..aefec9e 100644
--- a/lib/librte_pmd_i40e/i40e_rxtx.c
+++ b/lib/librte_pmd_i40e/i40e_rxtx.c
@@ -456,48 +456,49 @@ i40e_rxd_build_fdir(volatile union i40e_rx_desc *rxdp, 
struct rte_mbuf *mb)
 #endif
return flags;
 }
+
 static inline void
 i40e_txd_enable_checksum(uint64_t ol_flags,
uint32_t *td_cmd,
uint32_t *td_offset,
uint8_t l2_len,
uint16_t l3_len,
-   uint8_t inner_l2_len,
-   uint16_t inner_l3_len,
+   uint8_t outer_l2_len,
+   uint16_t outer_l3_len,
+   uint8_t l4_tun_len,
uint32_t *cd_tunneling)
 {
if (!l2_len) {
PMD_DRV_LOG(DEBUG, "L2 length set to 0");
return;
}
-   *td_offset |= (l2_len >> 1) << I40E_TX_DESC_LENGTH_MACLEN_SHIFT;

if (!l3_len) {
PMD_DRV_LOG(DEBUG, "L3 length set to 0");
return;
}

-   /* VXLAN packet TX checksum offload */
-   if (unlikely(ol_flags & PKT_TX_VXLAN_CKSUM)) {
-   uint8_t l4tun_len;
-
-   l4tun_len = ETHER_VXLAN_HLEN + inner_l2_len;
+   /* UDP tunneling packet TX checksum offload */
+   if (unlikely(ol_flags & PKT_TX_UDP_TUNNEL_PKT)) {
+   uint8_t l4_tunnel_len = 0;

-   if (ol_flags & PKT_TX_IPV4_CSUM)
+   *td_offset |= (outer_l2_len >> 1)
+   << I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
+   l4_tunnel_len = l4_tun_len + l2_len;
+   if (ol_flags & PKT_TX_OUTER_IP_CKSUM)
*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV4;
-   else if (ol_flags & PKT_TX_IPV6)
+   else if (ol_flags & PKT_TX_OUTER_IPV6)
*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV6;

/* Now set the ctx descriptor fields */
-   *cd_tunneling |= (l3_len >> 2) <<
+   *cd_tunneling |= (outer_l3_len >> 2) <<
I40E_TXD_CTX_QW0_EXT_IPLEN_SHIFT |
I40E_TXD_CTX_UDP_TUNNELING |
-   (l4tun_len >> 1) <<
+   (l4_tunnel_len >> 1) <<
I40E_TXD_CTX_QW0_NATLEN_SHIFT;
-
-   l3_len = inner_l3_len;
}

+   *td_offset |= (l2_len >> 1) << I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
/* Enable L3 checksum offloads */
if (ol_flags & PKT_TX_IPV4_CSUM) {
*td_cmd |= I40E_TX_DESC_CMD_IIPT_IPV4_CSUM;
@@ -1158,8 +1159,8 @@ i40e_calc_context_desc(uint64_t flags)
 {
uint64_t mask = 0ULL;

-   if (flags | PKT_TX_VXLAN_CKSUM)
-   mask |= PKT_TX_VXLAN_CKSUM;
+   if (flags | PKT_TX_UDP_TUNNEL_PKT)
+   mask |= PKT_TX_UDP_TUNNEL_PKT;

 #ifdef RTE_LIBRTE_IEEE1588
mask |= PKT_TX_IEEE1588_TMST;
@@ -1190,8 +1191,9 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, 
uint16_t nb_pkts)
uint64_t ol_flags;
uint8_t l2_len;
uint16_t l3_len;
-   uint8_t inner_l2_len;
-   uint16_t inner_l3_len;
+   uint8_t outer_l2_len;
+   uint16_t outer_l3_len;
+   uint8_t l4_tun_len;
uint16_t nb_used;
uint16_t nb_ctx;
uint16_t tx_last;
@@ -1219,9 +1221,12 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf 
**tx_pkts, uint16_t nb_pkts)

ol_flags = tx_pkt->ol_flags;
l2_len = tx_pkt->l2_len;
-   inner_l2_len = tx_pkt->inner_l2_len;
l3_len = tx_pkt->l3_len;
-   inner_l3_len = tx_pkt->inner_l3_len;
+   outer_l2_len = tx_pkt->outer_l2_len;
+   outer_l3_len = tx_pkt->outer_l3_len;
+
+   /* L4 Tunneling Length */
+   l4_tun_len = tx_pkt->l4_tun_len;

/* Calculate the number of context descriptors needed. */
nb_ctx = i40e_calc_context_desc(ol_flags);
@@ -1271,8 +1276,8 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, 
uint16_t nb_pkts)
/* Enable checksum offloading */
cd_tunneling_params = 0;
i40e_txd_enable_checksum(ol_flags, &td_cmd, &td_offset,
-   l2_len, l3_len, inner_l2_len,
-   inner_l3_len,
+   l2_len, l3_len, outer_l2_len,
+   outer_l3_len, l4_tun_len,
&cd_tunneling_pa

[dpdk-dev] [PATCH v2 1/4] mbuf:add three TX offload flags and change three fields

[Jijiang Liu]

In place of removing the PKT_TX_VXLAN_CKSUM, we introduce 3 new flags: 
PKT_TX_OUTER_IP_CKSUM, PKT_TX_OUTER_IPV6 and PKT_TX_UDP_TUNNEL_PKT, and a new 
field: l4_tun_len.
Replace the inner_l2_len and the inner_l3_len field with the outer_l2_len and 
outer_l3_len field.

PKT_TX_OUTER_IP_CKSUM: is not used for non-tunnelling packet;hardware outer 
checksum for tunnelling packet.
PKT_TX_OUTER_IPV6: Tell the NIC it's an outer IPv6 packet for tunneling packet.
PKT_TX_UDP_TUNNEL_PKT: is used to tell PMD that the transmit packet is a UDP 
tunneling packet.
l4_tun_len: for VXLAN packet, it should be udp header length plus VXLAN header 
length.


Signed-off-by: Jijiang Liu 
---
 lib/librte_mbuf/rte_mbuf.c |6 +-
 lib/librte_mbuf/rte_mbuf.h |   18 +++---
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 87c2963..3c47477 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -240,7 +240,11 @@ const char *rte_get_tx_ol_flag_name(uint64_t mask)
case PKT_TX_SCTP_CKSUM: return "PKT_TX_SCTP_CKSUM";
case PKT_TX_UDP_CKSUM: return "PKT_TX_UDP_CKSUM";
case PKT_TX_IEEE1588_TMST: return "PKT_TX_IEEE1588_TMST";
-   case PKT_TX_VXLAN_CKSUM: return "PKT_TX_VXLAN_CKSUM";
+   case PKT_TX_UDP_TUNNEL_PKT: return "PKT_TX_UDP_TUNNEL_PKT";
+   case PKT_TX_IPV4: return "PKT_TX_IPV4";
+   case PKT_TX_IPV6: return "PKT_TX_IPV6";
+   case PKT_TX_OUTER_IP_CKSUM: return "PKT_TX_OUTER_IP_CKSUM";
+   case PKT_TX_OUTER_IPV6: return "PKT_TX_OUTER_IPV6";
case PKT_TX_TCP_SEG: return "PKT_TX_TCP_SEG";
default: return NULL;
}
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 367fc56..22ee555 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -99,10 +99,9 @@ extern "C" {
 #define PKT_RX_TUNNEL_IPV6_HDR (1ULL << 12) /**< RX tunnel packet with IPv6 
header. */
 #define PKT_RX_FDIR_ID   (1ULL << 13) /**< FD id reported if FDIR match. */
 #define PKT_RX_FDIR_FLX  (1ULL << 14) /**< Flexible bytes reported if FDIR 
match. */
-/* add new RX flags here */

 /* add new TX flags here */
-#define PKT_TX_VXLAN_CKSUM   (1ULL << 50) /**< TX checksum of VXLAN computed 
by NIC */
+#define PKT_TX_UDP_TUNNEL_PKT (1ULL << 50) /**< TX packet is an UDP tunneling 
packet */
 #define PKT_TX_IEEE1588_TMST (1ULL << 51) /**< TX IEEE1588 packet to 
timestamp. */

 /**
@@ -125,13 +124,19 @@ extern "C" {
 #define PKT_TX_IP_CKSUM  (1ULL << 54) /**< IP cksum of TX pkt. computed by 
NIC. */
 #define PKT_TX_IPV4_CSUM PKT_TX_IP_CKSUM /**< Alias of PKT_TX_IP_CKSUM. */

+#define PKT_TX_VLAN_PKT  (1ULL << 55) /**< TX packet is a 802.1q VLAN 
packet. */
+
 /** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or TSO. 
*/
 #define PKT_TX_IPV4  PKT_RX_IPV4_HDR

 /** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or TSO. 
*/
 #define PKT_TX_IPV6  PKT_RX_IPV6_HDR

-#define PKT_TX_VLAN_PKT  (1ULL << 55) /**< TX packet is a 802.1q VLAN 
packet. */
+/** Outer IP cksum of TX pkt. computed by NIC for tunneling packet */
+#define PKT_TX_OUTER_IP_CKSUM   (1ULL << 58)
+
+/** Tell the NIC it's an outer IPv6 packet for tunneling packet.*/
+#define PKT_TX_OUTER_IPV6(1ULL << 59)

 /**
  * TCP segmentation offload. To enable this offload feature for a
@@ -266,10 +271,9 @@ struct rte_mbuf {
uint64_t tso_segsz:16; /**< TCP TSO segment size */

/* fields for TX offloading of tunnels */
-   uint64_t inner_l3_len:9; /**< inner L3 (IP) Hdr Length. 
*/
-   uint64_t inner_l2_len:7; /**< inner L2 (MAC) Hdr 
Length. */
-
-   /* uint64_t unused:8; */
+   uint64_t outer_l3_len:9; /**< outer L3 (IP) Hdr Length. 
*/
+   uint64_t outer_l2_len:7; /**< outer L2 (MAC) Hdr 
Length. */
+   uint64_t l4_tun_len:8; /**< L4 tunnelling header length 
*/
};
};
 } __rte_cache_aligned;
-- 
1.7.7.6

[dpdk-dev] [PATCH v2 2/4] mbuf:change PKT_TX_IPV4 and PKT_TX_IPV6 definition

[Jijiang Liu]

It will avoid to send a packet with a bad info:
  - we receive a Ether/IP6/IP4/L4/data packet
  - the driver sets PKT_RX_IPV6_HDR
  - the stack decapsulates IP6
  - the stack sends the packet, it has the PKT_TX_IPV6 flag but it's an IPv4 
packet.

Signed-off-by: Jijiang Liu 
---
 lib/librte_mbuf/rte_mbuf.h |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 22ee555..f6b3185 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -127,10 +127,10 @@ extern "C" {
 #define PKT_TX_VLAN_PKT  (1ULL << 55) /**< TX packet is a 802.1q VLAN 
packet. */

 /** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or TSO. 
*/
-#define PKT_TX_IPV4  PKT_RX_IPV4_HDR
+#define PKT_TX_IPV4  (1ULL << 56)

 /** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or TSO. 
*/
-#define PKT_TX_IPV6  PKT_RX_IPV6_HDR
+#define PKT_TX_IPV6  (1ULL << 57)

 /** Outer IP cksum of TX pkt. computed by NIC for tunneling packet */
 #define PKT_TX_OUTER_IP_CKSUM   (1ULL << 58)
-- 
1.7.7.6

[dpdk-dev] [PATCH v2 4/4] testpmd:rework csum forward engine

[Jijiang Liu]

The changes include:
1. use the new introduced ol_flags and fields in csumonly.c file;
2. fix an issue of outer UDP checksum check; 
3. change process logic in the process_inner_cksums();

Signed-off-by: Jijiang Liu 
---
 app/test-pmd/csumonly.c |   65 ++
 1 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index d8c080a..f7ad3d8 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -189,11 +189,12 @@ process_inner_cksums(void *l3_hdr, uint16_t ethertype, 
uint16_t l3_len,
} else {
if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_IP_CKSUM)
ol_flags |= PKT_TX_IP_CKSUM;
-   else
+   else {
ipv4_hdr->hdr_checksum =
rte_ipv4_cksum(ipv4_hdr);
+   ol_flags |= PKT_TX_IPV4;
+   }
}
-   ol_flags |= PKT_TX_IPV4;
} else if (ethertype == _htons(ETHER_TYPE_IPv6))
ol_flags |= PKT_TX_IPV6;
else
@@ -257,27 +258,28 @@ process_outer_cksums(void *outer_l3_hdr, uint16_t 
outer_ethertype,
uint64_t ol_flags = 0;

if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM)
-   ol_flags |= PKT_TX_VXLAN_CKSUM;
+   ol_flags |= PKT_TX_UDP_TUNNEL_PKT;

if (outer_ethertype == _htons(ETHER_TYPE_IPv4)) {
ipv4_hdr->hdr_checksum = 0;

-   if ((testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM) == 0)
+   if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM)
+   ol_flags |= PKT_TX_OUTER_IP_CKSUM;
+   else
ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
-   }
+   } else if (outer_ethertype == _htons(ETHER_TYPE_IPv6))
+   ol_flags |= PKT_TX_OUTER_IPV6;

udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + outer_l3_len);
/* do not recalculate udp cksum if it was 0 */
if (udp_hdr->dgram_cksum != 0) {
udp_hdr->dgram_cksum = 0;
-   if ((testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM) == 0) {
-   if (outer_ethertype == _htons(ETHER_TYPE_IPv4))
-   udp_hdr->dgram_cksum =
-   rte_ipv4_udptcp_cksum(ipv4_hdr, 
udp_hdr);
-   else
-   udp_hdr->dgram_cksum =
-   rte_ipv6_udptcp_cksum(ipv6_hdr, 
udp_hdr);
-   }
+   if (outer_ethertype == _htons(ETHER_TYPE_IPv4))
+   udp_hdr->dgram_cksum =
+   rte_ipv4_udptcp_cksum(ipv4_hdr, udp_hdr);
+   else
+   udp_hdr->dgram_cksum =
+   rte_ipv6_udptcp_cksum(ipv6_hdr, udp_hdr);
}

return ol_flags;
@@ -303,7 +305,7 @@ process_outer_cksums(void *outer_l3_hdr, uint16_t 
outer_ethertype,
  * TESTPMD_TX_OFFLOAD_* in ports[tx_port].tx_ol_flags. They control
  * wether a checksum must be calculated in software or in hardware. The
  * IP, UDP, TCP and SCTP flags always concern the inner layer.  The
- * VxLAN flag concerns the outer IP and UDP layer (if packet is
+ * VxLAN flag concerns the outer IP(if packet is
  * recognized as a vxlan packet).
  */
 static void
@@ -320,7 +322,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
uint16_t i;
uint64_t ol_flags;
uint16_t testpmd_ol_flags;
-   uint8_t l4_proto;
+   uint8_t l4_proto, l4_tun_len = 0;
uint16_t ethertype = 0, outer_ethertype = 0;
uint16_t l2_len = 0, l3_len = 0, l4_len = 0;
uint16_t outer_l2_len = 0, outer_l3_len = 0;
@@ -360,6 +362,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)

ol_flags = 0;
tunnel = 0;
+   l4_tun_len = 0;
m = pkts_burst[i];

/* Update the L3/L4 checksum error packet statistics */
@@ -377,15 +380,17 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
/* check if it's a supported tunnel (only vxlan for now) */
if (l4_proto == IPPROTO_UDP) {
udp_hdr = (struct udp_hdr *)((char *)l3_hdr + l3_len);
+
+   /* check udp destination port, 4789 is the default
+* vxlan port (rfc7348) */
+   if (udp_hdr->dst_port == _htons(4789)) {
+   l4_tun_len = ETHER_VXLAN_HLEN;
+   tunnel = 1;

/* currently, this flag is set by i40e only if the
 * packet is vxlan */
-   if (((m->ol_flags & PKT_RX_TUNNEL_IPV4_HDR) ||
-   (m->ol_flags &

[dpdk-dev] [PATCH v3 0/4] i40e VXLAN TX checksum rework

[Jijiang Liu]

We have got some feedback about backward compatibility of VXLAN TX checksum 
offload API with 1G/10G NIC after the i40e VXLAN TX checksum codes were 
applied, so we have to rework the APIs on i40e, including the changes of mbuf, 
i40e PMD and csum engine.

The main changes in mbuf are as follows,
In place of removing PKT_TX_VXLAN_CKSUM, we introducing 3 new flags: 
PKT_TX_OUTER_IP_CKSUM,PKT_TX_OUTER_IPV6 and PKT_TX_UDP_TUNNEL_PKT, and a new 
field: l4_tun_len.
Replace the inner_l2_len and the inner_l3_len field with the outer_l2_len and 
outer_l3_len field.

let's use a few examples to demonstrate how to use these new flags and existing 
flags in rte_mbuf.h
Let say we have a tunnel packet: 
eth_hdr_out/ipv4_hdr_out/udp_hdr_out/vxlan_hdr/ehtr_hdr_in/ipv4_hdr_in/tcp_hdr_in.There
 could be several scenarios:

A) User requests HW offload for ipv4_hdr_out checksum.
He doesn't care is it a tunnelled packet or not.
So he sets:

mb->l2_len =  eth_hdr_out;
mb->l3_len = ipv4_hdr_out;
mb->ol_flags |= PKT_TX_IPV4_CSUM;

B) User is aware that it is a tunnelled packet and requests HW offload for 
ipv4_hdr_in and tcp_hdr_in *only*.
He doesn't care about outer IP checksum offload.
In that case, for FVL  he has 2 choices:
   1. Treat that packet as a 'proper' tunnelled packet, and fill all the fields:
 mb->l2_len =  eth_hdr_in;
 mb->l3_len = ipv4_hdr_in;
 mb->outer_l2_len = eth_hdr_out;
 mb->outer_l3_len = ipv4_hdr_out;
 mb->l4tun_len = vxlan_hdr;
 mb->ol_flags |= PKT_TX_UDP_TUNNEL_PKT | PKT_TX_IP_CKSUM |  
PKT_TX_TCP_CKSUM;

   2. As user doesn't care about outer IP hdr checksum, he can treat everything 
before ipv4_hdr_in as L2 header.
   So he knows, that it is a tunnelled packet, but makes HW to treat it as 
ordinary (non-tunnelled) packet:
 mb->l2_len = eth_hdr_out + ipv4_hdr_out + udp_hdr_out + vxlan_hdr + 
ehtr_hdr_in;
 mb->l3_len = ipv4_hdr_in;
 mb->ol_flags |= PKT_TX_IP_CKSUM |  PKT_TX_TCP_CKSUM;

i40e PMD will support both B.1 and B.2.
ixgbe/igb/em PMD supports only B.2.
if HW supports both - it will be up to user app which method to choose.
tespmd will support both methods, and it should be configurable by user which 
approach to use (cmdline parameter).
So the user can try/test both methods and select an appropriate for him.

Now, B.2 is exactly what Oliver suggested.
I think it has few important advantages over B.1:
First of all - compatibility. It works across all HW we currently support 
(i40e/ixgbe/igb/em).
Second - it is probably faster even on FVL, as for it we have to fill only TXD, 
while with approach #2 we have to fill both TCD and TXD.

C) User knows that is a tunnelled packet, and wants HW offload for all 3 
checksums:  outer IP hdr checksum, inner IP checksum, inner TCP checksum.
Then he has to setup all TX checksum fields:
 mb->l2_len =  eth_hdr_in;
 mb->l3_len = ipv4_hdr_in;
 mb->outer_l2_len = eth_hdr_out;
 mb->outer_l3_len = ipv4_hdr_out;
 mb->l4tun_len = vxlan_hdr;
 mb->ol_flags |= PKT_TX_OUT_IP_CKSUM  | PKT_TX_UDP_TUNNEL | PKT_TX_IP_CKSUM 
|  PKT_TX_TCP_CKSUM;

v2 changes:
 remove PKT_TX_IP_CKSUM alias.
 add PKT_TX_OUT_IP_CKSUM and PKT_TX_OUTER_IPV6 in rte_get_tx_ol_flag_name.
 spliting mbuf changes into two patches.
 fix MACLEN caculation issue in i40e driver
 fix some issues in csumonly.c
 change cover letter.
v3 changes:
 fix MACLEN caculation issue in i40e driver when non-tunneling packet 

Jijiang Liu (4):
  mbuf change for 3 new flags and 3 fields
  mbuf change for PKT_TX_IPV4 and PKT_TX_IPV6
  i40e PMD change in i40e_rxtx.c
  rework csum forward engine


 app/test-pmd/csumonly.c |   65 ++-
 lib/librte_mbuf/rte_mbuf.c  |6 +++-
 lib/librte_mbuf/rte_mbuf.h  |   22 -
 lib/librte_pmd_i40e/i40e_rxtx.c |   49 -
 4 files changed, 82 insertions(+), 60 deletions(-)

-- 
1.7.7.6

[dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and change three fields

[Jijiang Liu]

In place of removing the PKT_TX_VXLAN_CKSUM, we introduce 3 new flags: 
PKT_TX_OUTER_IP_CKSUM, PKT_TX_OUTER_IPV6 and PKT_TX_UDP_TUNNEL_PKT, and a new 
field: l4_tun_len.
Replace the inner_l2_len and the inner_l3_len field with the outer_l2_len and 
outer_l3_len field.

PKT_TX_OUTER_IP_CKSUM: is not used for non-tunnelling packet;hardware outer 
checksum for tunnelling packet.
PKT_TX_OUTER_IPV6: Tell the NIC it's an outer IPv6 packet for tunneling packet.
PKT_TX_UDP_TUNNEL_PKT: is used to tell PMD that the transmit packet is a UDP 
tunneling packet.
l4_tun_len: for VXLAN packet, it should be udp header length plus VXLAN header 
length.


Signed-off-by: Jijiang Liu 
---
 lib/librte_mbuf/rte_mbuf.c |6 +-
 lib/librte_mbuf/rte_mbuf.h |   18 +++---
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 87c2963..3c47477 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -240,7 +240,11 @@ const char *rte_get_tx_ol_flag_name(uint64_t mask)
case PKT_TX_SCTP_CKSUM: return "PKT_TX_SCTP_CKSUM";
case PKT_TX_UDP_CKSUM: return "PKT_TX_UDP_CKSUM";
case PKT_TX_IEEE1588_TMST: return "PKT_TX_IEEE1588_TMST";
-   case PKT_TX_VXLAN_CKSUM: return "PKT_TX_VXLAN_CKSUM";
+   case PKT_TX_UDP_TUNNEL_PKT: return "PKT_TX_UDP_TUNNEL_PKT";
+   case PKT_TX_IPV4: return "PKT_TX_IPV4";
+   case PKT_TX_IPV6: return "PKT_TX_IPV6";
+   case PKT_TX_OUTER_IP_CKSUM: return "PKT_TX_OUTER_IP_CKSUM";
+   case PKT_TX_OUTER_IPV6: return "PKT_TX_OUTER_IPV6";
case PKT_TX_TCP_SEG: return "PKT_TX_TCP_SEG";
default: return NULL;
}
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 367fc56..22ee555 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -99,10 +99,9 @@ extern "C" {
 #define PKT_RX_TUNNEL_IPV6_HDR (1ULL << 12) /**< RX tunnel packet with IPv6 
header. */
 #define PKT_RX_FDIR_ID   (1ULL << 13) /**< FD id reported if FDIR match. */
 #define PKT_RX_FDIR_FLX  (1ULL << 14) /**< Flexible bytes reported if FDIR 
match. */
-/* add new RX flags here */

 /* add new TX flags here */
-#define PKT_TX_VXLAN_CKSUM   (1ULL << 50) /**< TX checksum of VXLAN computed 
by NIC */
+#define PKT_TX_UDP_TUNNEL_PKT (1ULL << 50) /**< TX packet is an UDP tunneling 
packet */
 #define PKT_TX_IEEE1588_TMST (1ULL << 51) /**< TX IEEE1588 packet to 
timestamp. */

 /**
@@ -125,13 +124,19 @@ extern "C" {
 #define PKT_TX_IP_CKSUM  (1ULL << 54) /**< IP cksum of TX pkt. computed by 
NIC. */
 #define PKT_TX_IPV4_CSUM PKT_TX_IP_CKSUM /**< Alias of PKT_TX_IP_CKSUM. */

+#define PKT_TX_VLAN_PKT  (1ULL << 55) /**< TX packet is a 802.1q VLAN 
packet. */
+
 /** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or TSO. 
*/
 #define PKT_TX_IPV4  PKT_RX_IPV4_HDR

 /** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or TSO. 
*/
 #define PKT_TX_IPV6  PKT_RX_IPV6_HDR

-#define PKT_TX_VLAN_PKT  (1ULL << 55) /**< TX packet is a 802.1q VLAN 
packet. */
+/** Outer IP cksum of TX pkt. computed by NIC for tunneling packet */
+#define PKT_TX_OUTER_IP_CKSUM   (1ULL << 58)
+
+/** Tell the NIC it's an outer IPv6 packet for tunneling packet.*/
+#define PKT_TX_OUTER_IPV6(1ULL << 59)

 /**
  * TCP segmentation offload. To enable this offload feature for a
@@ -266,10 +271,9 @@ struct rte_mbuf {
uint64_t tso_segsz:16; /**< TCP TSO segment size */

/* fields for TX offloading of tunnels */
-   uint64_t inner_l3_len:9; /**< inner L3 (IP) Hdr Length. 
*/
-   uint64_t inner_l2_len:7; /**< inner L2 (MAC) Hdr 
Length. */
-
-   /* uint64_t unused:8; */
+   uint64_t outer_l3_len:9; /**< outer L3 (IP) Hdr Length. 
*/
+   uint64_t outer_l2_len:7; /**< outer L2 (MAC) Hdr 
Length. */
+   uint64_t l4_tun_len:8; /**< L4 tunnelling header length 
*/
};
};
 } __rte_cache_aligned;
-- 
1.7.7.6

[dpdk-dev] [PATCH v3 2/4] mbuf:change PKT_TX_IPV4 and PKT_TX_IPV6 definition

[Jijiang Liu]

It will avoid to send a packet with a bad info:
  - we receive a Ether/IP6/IP4/L4/data packet
  - the driver sets PKT_RX_IPV6_HDR
  - the stack decapsulates IP6
  - the stack sends the packet, it has the PKT_TX_IPV6 flag but it's an IPv4 
packet.

Signed-off-by: Jijiang Liu 
---
 lib/librte_mbuf/rte_mbuf.h |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 22ee555..f6b3185 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -127,10 +127,10 @@ extern "C" {
 #define PKT_TX_VLAN_PKT  (1ULL << 55) /**< TX packet is a 802.1q VLAN 
packet. */

 /** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or TSO. 
*/
-#define PKT_TX_IPV4  PKT_RX_IPV4_HDR
+#define PKT_TX_IPV4  (1ULL << 56)

 /** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or TSO. 
*/
-#define PKT_TX_IPV6  PKT_RX_IPV6_HDR
+#define PKT_TX_IPV6  (1ULL << 57)

 /** Outer IP cksum of TX pkt. computed by NIC for tunneling packet */
 #define PKT_TX_OUTER_IP_CKSUM   (1ULL << 58)
-- 
1.7.7.6

[dpdk-dev] [PATCH v3 3/4] i40e:PMD change for VXLAN TX checksum

[Jijiang Liu]

Rework the i40e PMD codes using the new introduced ol_flags and fields.

Signed-off-by: Jijiang Liu 
---
 lib/librte_pmd_i40e/i40e_rxtx.c |   52 +--
 1 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
index cce6911..be06e8f 100644
--- a/lib/librte_pmd_i40e/i40e_rxtx.c
+++ b/lib/librte_pmd_i40e/i40e_rxtx.c
@@ -456,48 +456,48 @@ i40e_rxd_build_fdir(volatile union i40e_rx_desc *rxdp, 
struct rte_mbuf *mb)
 #endif
return flags;
 }
+
 static inline void
 i40e_txd_enable_checksum(uint64_t ol_flags,
uint32_t *td_cmd,
uint32_t *td_offset,
uint8_t l2_len,
uint16_t l3_len,
-   uint8_t inner_l2_len,
-   uint16_t inner_l3_len,
+   uint8_t outer_l2_len,
+   uint16_t outer_l3_len,
+   uint8_t l4_tun_len,
uint32_t *cd_tunneling)
 {
if (!l2_len) {
PMD_DRV_LOG(DEBUG, "L2 length set to 0");
return;
}
-   *td_offset |= (l2_len >> 1) << I40E_TX_DESC_LENGTH_MACLEN_SHIFT;

if (!l3_len) {
PMD_DRV_LOG(DEBUG, "L3 length set to 0");
return;
}

-   /* VXLAN packet TX checksum offload */
-   if (unlikely(ol_flags & PKT_TX_VXLAN_CKSUM)) {
-   uint8_t l4tun_len;
-
-   l4tun_len = ETHER_VXLAN_HLEN + inner_l2_len;
+   /* UDP tunneling packet TX checksum offload */
+   if (unlikely(ol_flags & PKT_TX_UDP_TUNNEL_PKT)) {
+   uint16_t l4_tunnel_len;

-   if (ol_flags & PKT_TX_IPV4_CSUM)
+   *td_offset |= (outer_l2_len >> 1)
+   << I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
+   l4_tunnel_len = l4_tun_len + l2_len;
+   if (ol_flags & PKT_TX_OUTER_IP_CKSUM)
*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV4;
-   else if (ol_flags & PKT_TX_IPV6)
+   else if (ol_flags & PKT_TX_OUTER_IPV6)
*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV6;

/* Now set the ctx descriptor fields */
-   *cd_tunneling |= (l3_len >> 2) <<
+   *cd_tunneling |= (outer_l3_len >> 2) <<
I40E_TXD_CTX_QW0_EXT_IPLEN_SHIFT |
I40E_TXD_CTX_UDP_TUNNELING |
-   (l4tun_len >> 1) <<
+   (l4_tunnel_len >> 1) <<
I40E_TXD_CTX_QW0_NATLEN_SHIFT;
-
-   l3_len = inner_l3_len;
-   }
-
+   } else
+   *td_offset |= (l2_len >> 1) << I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
/* Enable L3 checksum offloads */
if (ol_flags & PKT_TX_IPV4_CSUM) {
*td_cmd |= I40E_TX_DESC_CMD_IIPT_IPV4_CSUM;
@@ -1158,8 +1158,8 @@ i40e_calc_context_desc(uint64_t flags)
 {
uint64_t mask = 0ULL;

-   if (flags | PKT_TX_VXLAN_CKSUM)
-   mask |= PKT_TX_VXLAN_CKSUM;
+   if (flags | PKT_TX_UDP_TUNNEL_PKT)
+   mask |= PKT_TX_UDP_TUNNEL_PKT;

 #ifdef RTE_LIBRTE_IEEE1588
mask |= PKT_TX_IEEE1588_TMST;
@@ -1190,8 +1190,9 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, 
uint16_t nb_pkts)
uint64_t ol_flags;
uint8_t l2_len;
uint16_t l3_len;
-   uint8_t inner_l2_len;
-   uint16_t inner_l3_len;
+   uint8_t outer_l2_len;
+   uint16_t outer_l3_len;
+   uint8_t l4_tun_len;
uint16_t nb_used;
uint16_t nb_ctx;
uint16_t tx_last;
@@ -1219,9 +1220,12 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf 
**tx_pkts, uint16_t nb_pkts)

ol_flags = tx_pkt->ol_flags;
l2_len = tx_pkt->l2_len;
-   inner_l2_len = tx_pkt->inner_l2_len;
l3_len = tx_pkt->l3_len;
-   inner_l3_len = tx_pkt->inner_l3_len;
+   outer_l2_len = tx_pkt->outer_l2_len;
+   outer_l3_len = tx_pkt->outer_l3_len;
+
+   /* L4 Tunneling Length */
+   l4_tun_len = tx_pkt->l4_tun_len;

/* Calculate the number of context descriptors needed. */
nb_ctx = i40e_calc_context_desc(ol_flags);
@@ -1271,8 +1275,8 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, 
uint16_t nb_pkts)
/* Enable checksum offloading */
cd_tunneling_params = 0;
i40e_txd_enable_checksum(ol_flags, &td_cmd, &td_offset,
-   l2_len, l3_len, inner_l2_len,
-   inner_l3_len,
+   l2_len, l3_len, outer_l2_len,
+   outer_l3_len, l4_tun_len,

[dpdk-dev] [PATCH v3 4/4] testpmd:rework csum forward engine

[Jijiang Liu]

The changes include:
1. use the new introduced ol_flags and fields in csumonly.c file;
2. fix an issue of outer UDP checksum check; 
3. change process logic in the process_inner_cksums();

Signed-off-by: Jijiang Liu 
---
 app/test-pmd/csumonly.c |   65 ++
 1 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index d8c080a..f7ad3d8 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -189,11 +189,12 @@ process_inner_cksums(void *l3_hdr, uint16_t ethertype, 
uint16_t l3_len,
} else {
if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_IP_CKSUM)
ol_flags |= PKT_TX_IP_CKSUM;
-   else
+   else {
ipv4_hdr->hdr_checksum =
rte_ipv4_cksum(ipv4_hdr);
+   ol_flags |= PKT_TX_IPV4;
+   }
}
-   ol_flags |= PKT_TX_IPV4;
} else if (ethertype == _htons(ETHER_TYPE_IPv6))
ol_flags |= PKT_TX_IPV6;
else
@@ -257,27 +258,28 @@ process_outer_cksums(void *outer_l3_hdr, uint16_t 
outer_ethertype,
uint64_t ol_flags = 0;

if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM)
-   ol_flags |= PKT_TX_VXLAN_CKSUM;
+   ol_flags |= PKT_TX_UDP_TUNNEL_PKT;

if (outer_ethertype == _htons(ETHER_TYPE_IPv4)) {
ipv4_hdr->hdr_checksum = 0;

-   if ((testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM) == 0)
+   if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM)
+   ol_flags |= PKT_TX_OUTER_IP_CKSUM;
+   else
ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
-   }
+   } else if (outer_ethertype == _htons(ETHER_TYPE_IPv6))
+   ol_flags |= PKT_TX_OUTER_IPV6;

udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + outer_l3_len);
/* do not recalculate udp cksum if it was 0 */
if (udp_hdr->dgram_cksum != 0) {
udp_hdr->dgram_cksum = 0;
-   if ((testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM) == 0) {
-   if (outer_ethertype == _htons(ETHER_TYPE_IPv4))
-   udp_hdr->dgram_cksum =
-   rte_ipv4_udptcp_cksum(ipv4_hdr, 
udp_hdr);
-   else
-   udp_hdr->dgram_cksum =
-   rte_ipv6_udptcp_cksum(ipv6_hdr, 
udp_hdr);
-   }
+   if (outer_ethertype == _htons(ETHER_TYPE_IPv4))
+   udp_hdr->dgram_cksum =
+   rte_ipv4_udptcp_cksum(ipv4_hdr, udp_hdr);
+   else
+   udp_hdr->dgram_cksum =
+   rte_ipv6_udptcp_cksum(ipv6_hdr, udp_hdr);
}

return ol_flags;
@@ -303,7 +305,7 @@ process_outer_cksums(void *outer_l3_hdr, uint16_t 
outer_ethertype,
  * TESTPMD_TX_OFFLOAD_* in ports[tx_port].tx_ol_flags. They control
  * wether a checksum must be calculated in software or in hardware. The
  * IP, UDP, TCP and SCTP flags always concern the inner layer.  The
- * VxLAN flag concerns the outer IP and UDP layer (if packet is
+ * VxLAN flag concerns the outer IP(if packet is
  * recognized as a vxlan packet).
  */
 static void
@@ -320,7 +322,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
uint16_t i;
uint64_t ol_flags;
uint16_t testpmd_ol_flags;
-   uint8_t l4_proto;
+   uint8_t l4_proto, l4_tun_len = 0;
uint16_t ethertype = 0, outer_ethertype = 0;
uint16_t l2_len = 0, l3_len = 0, l4_len = 0;
uint16_t outer_l2_len = 0, outer_l3_len = 0;
@@ -360,6 +362,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)

ol_flags = 0;
tunnel = 0;
+   l4_tun_len = 0;
m = pkts_burst[i];

/* Update the L3/L4 checksum error packet statistics */
@@ -377,15 +380,17 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
/* check if it's a supported tunnel (only vxlan for now) */
if (l4_proto == IPPROTO_UDP) {
udp_hdr = (struct udp_hdr *)((char *)l3_hdr + l3_len);
+
+   /* check udp destination port, 4789 is the default
+* vxlan port (rfc7348) */
+   if (udp_hdr->dst_port == _htons(4789)) {
+   l4_tun_len = ETHER_VXLAN_HLEN;
+   tunnel = 1;

/* currently, this flag is set by i40e only if the
 * packet is vxlan */
-   if (((m->ol_flags & PKT_RX_TUNNEL_IPV4_HDR) ||
-   (m->ol_flags &

[dpdk-dev] [PATCH v2] i40e: link flow control support

[Zhang, Helin]

Hi Thomas

New version is needed for i40e flow control.

Regards,
Helin

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Thursday, November 27, 2014 11:51 PM
> To: Zang, Zhida
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] i40e: link flow control support
> 
> Hi,
> 
> 2014-11-20 16:58, zhida zang:
> > From: zzang 
> >
> > Add link flow control support for i40e
> >
> > Signed-off-by: zhida zang 
> 
> This patch is pending with open questions.
> Any news to submit a new version and/or acknowledge it?
> 
> --
> Thomas

[dpdk-dev] [PATCH] enicpmd: compilation error during inclusion of vfio.h

[Qiu, Michael]

Hi all,

I have no comments on this issue, but I indeed see many places do have
this kernel issue(before/now/future), so can solve this issue globally?

Thus, we do not need to fix this case by case.

One solution(not sure if it works or not):

1. features and kernel version required list.
2. When config DPDK before build, automatically check this list and if
not mach, just disable this feature in config file even though user set
it manually.

Thus main code may not need to change.

Does this works?

Thanks,
Michael

On 11/28/2014 1:16 AM, Sujith Sankar wrote:
> Inclusion of vfio.h was giving compilation errors if kernel version is less
> than 3.6.0 and if RTE_EAL_VFIO was on in config.
>
> Replaced inclusion of vfio.h with eal_vfio.h and replaced RTE_EAL_VFIO with
> VFIO_PRESENT in enicpmd code.
>
> Signed-off-by: Sujith Sankar 
> ---
>  lib/librte_pmd_enic/Makefile|  1 +
>  lib/librte_pmd_enic/enic_main.c | 13 +
>  2 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/lib/librte_pmd_enic/Makefile b/lib/librte_pmd_enic/Makefile
> index d4c2f66..befc552 100644
> --- a/lib/librte_pmd_enic/Makefile
> +++ b/lib/librte_pmd_enic/Makefile
> @@ -39,6 +39,7 @@ LIB = librte_pmd_enic.a
>  
>  CFLAGS += -I$(RTE_SDK)/lib/librte_hash/ 
> -I$(RTE_SDK)/lib/librte_pmd_enic/vnic/
>  CFLAGS += -I$(RTE_SDK)/lib/librte_pmd_enic/
> +CFLAGS += -I$(RTE_SDK)/lib/librte_eal/linuxapp/eal/
>  CFLAGS += -O3 -Wno-deprecated
>  
>  VPATH += $(RTE_SDK)/lib/librte_pmd_enic/src
> diff --git a/lib/librte_pmd_enic/enic_main.c b/lib/librte_pmd_enic/enic_main.c
> index 4b857bb..f6f00d3 100644
> --- a/lib/librte_pmd_enic/enic_main.c
> +++ b/lib/librte_pmd_enic/enic_main.c
> @@ -39,9 +39,6 @@
>  #include 
>  #include 
>  #include 
> -#ifdef RTE_EAL_VFIO
> -#include 
> -#endif
>  
>  #include 
>  #include 
> @@ -631,7 +628,7 @@ int enic_enable(struct enic *enic)
>  
>   vnic_dev_enable_wait(enic->vdev);
>  
> -#ifndef RTE_EAL_VFIO
> +#ifndef VFIO_PRESENT
>   /* Register and enable error interrupt */
>   rte_intr_callback_register(&(enic->pdev->intr_handle),
>   enic_intr_handler, (void *)enic->rte_dev);
> @@ -995,7 +992,7 @@ int enic_setup_finish(struct enic *enic)
>   return 0;
>  }
>  
> -#ifdef RTE_EAL_VFIO
> +#ifdef VFIO_PRESENT
>  static void enic_eventfd_init(struct enic *enic)
>  {
>   enic->eventfd = enic->pdev->intr_handle.fd;
> @@ -1033,7 +1030,7 @@ int enic_get_link_status(struct enic *enic)
>  }
>  
>  
> -#ifdef RTE_EAL_VFIO
> +#ifdef VFIO_PRESENT
>  static int enic_create_err_intr_thread(struct enic *enic)
>  {
>   pthread_attr_t intr_attr;
> @@ -,7 +1108,7 @@ static void enic_dev_deinit(struct enic *enic)
>   if (eth_dev->data->mac_addrs)
>   rte_free(eth_dev->data->mac_addrs);
>  
> -#ifdef RTE_EAL_VFIO
> +#ifdef VFIO_PRESENT
>   enic_clear_intr_mode(enic);
>  #endif
>  }
> @@ -1167,7 +1164,7 @@ static int enic_dev_init(struct enic *enic)
>   */
>   enic_get_res_counts(enic);
>  
> -#ifdef RTE_EAL_VFIO
> +#ifdef VFIO_PRESENT
>   /* Set interrupt mode based on resource counts and system
>* capabilities
>*/

[dpdk-dev] [PATCH] enicpmd: compilation error during inclusion of vfio.h

[Sujith Sankar (ssujith)]

On 28/11/14 1:54 am, "Thomas Monjalon"  wrote:

>2014-11-27 19:01, Thomas Monjalon:
>> 2014-11-27 22:44, Sujith Sankar:
>> > Inclusion of vfio.h was giving compilation errors if kernel version
>>is less
>> > than 3.6.0 and if RTE_EAL_VFIO was on in config.
>> > 
>> > Replaced inclusion of vfio.h with eal_vfio.h and replaced
>>RTE_EAL_VFIO with
>> > VFIO_PRESENT in enicpmd code.
>> 
>> You should add
>> Reported-by: Pawel Wodkowski 

I shall take care of this from now on.

>> 
>> > Signed-off-by: Sujith Sankar 
>> [...]
>> >  CFLAGS += -I$(RTE_SDK)/lib/librte_hash/
>>-I$(RTE_SDK)/lib/librte_pmd_enic/vnic/
>> >  CFLAGS += -I$(RTE_SDK)/lib/librte_pmd_enic/
>> > +CFLAGS += -I$(RTE_SDK)/lib/librte_eal/linuxapp/eal/
>> >  CFLAGS += -O3 -Wno-deprecated
>> 
>> I think -I$(RTE_SDK)/lib/librte_hash/ and
>>-I$(RTE_SDK)/lib/librte_eal/linuxapp/eal/
>> are not needed.
>> 
>> I'll fix it if you confirm.

You?re right.

>
>Applied with above comments.

Thank you Thomas !

>
>-- 
>Thomas

[dpdk-dev] [PATCH v5 0/7] rte_hash_crc reworked to be platform-independent

[Yerden Zhumabekov]

28.11.2014 3:04, Thomas Monjalon ?:
> 2014-11-20 11:15, Yerden Zhumabekov:
>> These patches bring a fallback mechanism to ensure that CRC32 hash is 
>> calculated regardless of hardware support from CPU (i.e. SSE4.2 intrinsics).
>> Performance is also improved by slicing data in 8 bytes.
>>
>> Patches were tested on machines either with and without SSE4.2 support.
>>
>> Software implementation seems to be about 4-5 times slower than 
>> SSE4.2-enabled one. Of course, they return identical results.
>>
>> Summary of changes:
>> * added CRC32 software implementation, which is used as a fallback in case 
>> SSE4.2 is not available, or if SSE4.2 is intentionally disabled.
>> * added rte_hash_crc_set_alg() function to control availability of SSE4.2.
>> * added rte_hash_crc_8byte() function to calculate CRC32 on 8-byte operand.
>> * reworked rte_hash_crc() function which leverages both versions of CRC32 
>> hash calculation functions with 4 and 8-byte operands.
>> * removed compile-time checks from test_hash_perf and test_hash.
>> * setting default algorithm implementation as a constructor while 
>> application startup.
>> * SSE4.2 intrinsics are implemented through inline assembly code.
>> * added additional run-time check for 64-bit support.
> So you don't want to use the target attribute as suggested by Konstantin?
>
> Why the discussion ended without any acknowledgement?
>

I decided to emit SSE4.2 instruction right from the code, because:
* it is supported by gcc 4.3;
* use of target attribute (in a way suggested by Konstantin) presumably
still requires us to use #ifdef which we want to avoid.

Actually then, I didn't investigate it further. I'm quite happy with
last revision, but I'm open for ideas and discussion.
I made new patch series with solely change of crc32c tables declaration
using 'const' just as Stephen suggested, and I may post it. But I'd like
to see a confirmation for what I've done so far.

-- 
Sincerely,

Yerden Zhumabekov
State Technical Service
Astana, KZ

[dpdk-dev] [PATCH] enicpmd: replace the type u_int* with uint* to remove compilation errors on a few platforms

[Sujith Sankar]

ENIC PMD was giving compilation errors on ppc_64-power8-linuxapp-gcc because 
of types such as u_int32_t.  This patch replaces all those with uint32_t and 
similar ones.

Reported-by: David Marchand 
Signed-off-by: Sujith Sankar 
---
 lib/librte_pmd_enic/enic.h |  2 +-
 lib/librte_pmd_enic/enic_compat.h  | 16 
 lib/librte_pmd_enic/enic_main.c| 18 +-
 lib/librte_pmd_enic/vnic/vnic_devcmd.h |  6 +++---
 4 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/lib/librte_pmd_enic/enic.h b/lib/librte_pmd_enic/enic.h
index 9f80fc0..6400d24 100644
--- a/lib/librte_pmd_enic/enic.h
+++ b/lib/librte_pmd_enic/enic.h
@@ -106,7 +106,7 @@ struct enic {
int iommu_group_fd;
int iommu_groupid;
int eventfd;
-   u_int8_t mac_addr[ETH_ALEN];
+   uint8_t mac_addr[ETH_ALEN];
pthread_t err_intr_thread;
int promisc;
int allmulti;
diff --git a/lib/librte_pmd_enic/enic_compat.h 
b/lib/librte_pmd_enic/enic_compat.h
index d22578e..7c62bf2 100644
--- a/lib/librte_pmd_enic/enic_compat.h
+++ b/lib/librte_pmd_enic/enic_compat.h
@@ -89,9 +89,9 @@ typedef   unsigned intu32;
 typedef unsigned long long  u64;
 typedef unsigned long long  dma_addr_t;

-static inline u_int32_t ioread32(volatile void *addr)
+static inline uint32_t ioread32(volatile void *addr)
 {
-   return *(volatile u_int32_t *)addr;
+   return *(volatile uint32_t *)addr;
 }

 static inline u16 ioread16(volatile void *addr)
@@ -99,14 +99,14 @@ static inline u16 ioread16(volatile void *addr)
return *(volatile u16 *)addr;
 }

-static inline u_int8_t ioread8(volatile void *addr)
+static inline uint8_t ioread8(volatile void *addr)
 {
-   return *(volatile u_int8_t *)addr;
+   return *(volatile uint8_t *)addr;
 }

-static inline void iowrite32(u_int32_t val, volatile void *addr)
+static inline void iowrite32(uint32_t val, volatile void *addr)
 {
-   *(volatile u_int32_t *)addr = val;
+   *(volatile uint32_t *)addr = val;
 }

 static inline void iowrite16(u16 val, volatile void *addr)
@@ -114,9 +114,9 @@ static inline void iowrite16(u16 val, volatile void *addr)
*(volatile u16 *)addr = val;
 }

-static inline void iowrite8(u_int8_t val, volatile void *addr)
+static inline void iowrite8(uint8_t val, volatile void *addr)
 {
-   *(volatile u_int8_t *)addr = val;
+   *(volatile uint8_t *)addr = val;
 }

 static inline unsigned int readl(volatile void __iomem *addr)
diff --git a/lib/librte_pmd_enic/enic_main.c b/lib/librte_pmd_enic/enic_main.c
index f6f00d3..853dd04 100644
--- a/lib/librte_pmd_enic/enic_main.c
+++ b/lib/librte_pmd_enic/enic_main.c
@@ -172,17 +172,17 @@ unsigned int enic_cleanup_wq(struct enic *enic, struct 
vnic_wq *wq)

 int enic_send_pkt(struct enic *enic, struct vnic_wq *wq,
struct rte_mbuf *tx_pkt, unsigned short len,
-   u_int8_t sop, u_int8_t eop,
-   u_int16_t ol_flags, u_int16_t vlan_tag)
+   uint8_t sop, uint8_t eop,
+   uint16_t ol_flags, uint16_t vlan_tag)
 {
struct wq_enet_desc *desc = vnic_wq_next_desc(wq);
-   u_int16_t mss = 0;
-   u_int16_t header_length = 0;
-   u_int8_t cq_entry = eop;
-   u_int8_t vlan_tag_insert = 0;
+   uint16_t mss = 0;
+   uint16_t header_length = 0;
+   uint8_t cq_entry = eop;
+   uint8_t vlan_tag_insert = 0;
unsigned char *buf = (unsigned char *)(tx_pkt->buf_addr) +
RTE_PKTMBUF_HEADROOM;
-   u_int64_t bus_addr = (dma_addr_t)
+   uint64_t bus_addr = (dma_addr_t)
(tx_pkt->buf_physaddr + RTE_PKTMBUF_HEADROOM);

if (sop) {
@@ -342,8 +342,8 @@ static int enic_rq_alloc_buf(struct vnic_rq *rq)
void *buf;
dma_addr_t dma_addr;
struct rq_enet_desc *desc = vnic_rq_next_desc(rq);
-   u_int8_t type = RQ_ENET_TYPE_ONLY_SOP;
-   u_int16_t len = ENIC_MAX_MTU + VLAN_ETH_HLEN;
+   uint8_t type = RQ_ENET_TYPE_ONLY_SOP;
+   uint16_t len = ENIC_MAX_MTU + VLAN_ETH_HLEN;
u16 split_hdr_size = vnic_get_hdr_split_size(enic->vdev);
struct rte_mbuf *mbuf = enic_rxmbuf_alloc(rq->mp);
struct rte_mbuf *hdr_mbuf = NULL;
diff --git a/lib/librte_pmd_enic/vnic/vnic_devcmd.h 
b/lib/librte_pmd_enic/vnic/vnic_devcmd.h
index b4c87c1..e7ecf31 100644
--- a/lib/librte_pmd_enic/vnic/vnic_devcmd.h
+++ b/lib/librte_pmd_enic/vnic/vnic_devcmd.h
@@ -691,9 +691,9 @@ enum {
 #define FILTER_MAX_BUF_SIZE 100  /* Maximum size of buffer to CMD_ADD_FILTER */

 struct filter_tlv {
-   u_int32_t type;
-   u_int32_t length;
-   u_int32_t val[0];
+   uint32_t type;
+   uint32_t length;
+   uint32_t val[0];
 };

 enum {
-- 
1.9.1

[dpdk-dev] [PATCH] i40e: Use one bit flag for all hardware detected RX packet errors

[Zhang, Helin]

Hi Olivier, Konstantin

> -Original Message-
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> Sent: Wednesday, November 26, 2014 10:12 PM
> To: Ananyev, Konstantin; Zhang, Helin; dev at dpdk.org
> Cc: Cao, Waterman; Cao, Min
> Subject: Re: [PATCH] i40e: Use one bit flag for all hardware detected RX 
> packet
> errors
> 
> Hi Konstantin,
> 
> On 11/26/2014 02:38 PM, Ananyev, Konstantin wrote:
> >>> Probably I didn't explain myself clear enough, sorry.
> >>> I didn't suggest to get rid of setting bits that indicate L3/L4 checksum 
> >>> errors:
> >>> PKT_RX_IP_CKSUM_BAD, PKT_RX_L4_CKSUM_BAD,
> PKT_RX_EIP_CKSUM_BAD.
> >>> I think these flags should be set as before.
> >>>
> >>> I was talking only about collapsing only these 4 RX error flags into one:
> >>>
> >>> #define PKT_RX_OVERSIZE  (0ULL << 0)  /**< Num of desc of an RX
> pkt oversize. */
> >>> #define PKT_RX_HBUF_OVERFLOW (0ULL << 0)  /**< Header buffer
> overflow. */
> >>> #define PKT_RX_RECIP_ERR (0ULL << 0)  /**< Hardware processing
> error. */
> >>> #define PKT_RX_MAC_ERR   (0ULL << 0)  /**< MAC error. */
> >>>
> >>>   From my point of view the difference of these 2 groups are:
> >>> First - HW was able to receive whole packet without a problem, but L3/L4
> checksum check failed.
> >>>
> >>> Second - HW was not able to receive whole packet properly by whatever
> reason.
> >>>   From upper layer SW perspective - there it probably makes little
> >>> difference, what caused it, as most likely SW has to throw away erroneous
> packet.
> >>> And for debugging purposes, we can add PMD_LOG(DEBUG, ...) that would
> print what exactly HW error happened.
> >>
> >> I agree with Konstantin that there are 2 different cases:
> >>
> >> a) the packet is properly received by the hardware, but has a bad
> >>  checksum (or another protocol error, for instance an invalid ip len,
> >>  a ip_version == 8 :))
> >>
> >>  in this case, it is useful to the application to have the mbuf with
> >>  the data + an error flag. Then using a tcpdump-like tool could help
> >>  to debug what is the cause of the error and what equipment generates
> >>  a bad packet.
> >>
> >> b) the packet is not properly received by the hardware. In this case
> >>  the data is invalid in the mbuf and not useable by the application.
> >>  I suggest to only have a stats counter in this case, as receiving the
> >>  mbuf is cpu time consuming and the only thing the application can do
> >>  is to drop the packet.
> >
> > So for b) you suggest to drop the packet straight in PMD RX function?
> > Something like:
> > if (unlikely(error_bits & ...)) {
> >  PMD_LOG(DEBUG, ...);
> >   rte_pktmbuf_free(mb);
> > }
> > ?
> 
> Yes
> 
> > That's probably a bit too radical.
> > Yes, mbuf doesn't contain the whole packet, but it may contain at least 
> > part of it,
> let say in case of 'packet oversize'.
> > So for debugging purposes the user may still like to examine the mbuf
> contents.
> 
> As soon as there is some exploitable data in the mbuf, I agree it can be 
> transfered
> to the application (ex: bad header, bad len, bad checksum...).
> 
> But if the hardware is not able to provide any exploitable data, it looks a 
> bit
> overkill to give an mbuf with an error flag.
> 
> But grouping the flags as you suggest is already a good clean-up to me, I 
> don't
> want to be more catholic than the Pope ;)

After I have completed another task, I read the datasheet carefully again. For 
those 5
error bits I introduced for a long time, I'd like to explain one by one as 
below.

#define PKT_RX_EIP_CKSUM_BAD (0ULL << 0)  /**< External IP header checksum 
error. */
[Helin] Nobody complains it, so we will keep it there, and just assign a new 
value to it.

#define PKT_RX_OVERSIZE  (0ULL << 0)  /**< Num of desc of an RX pkt 
oversize. */
[Helin] I don't think it can be merge with other hardware errors. It indicates 
the packet
received needs more descriptors than hardware allowed, and the part of packets 
can
still be stored in the mbufs provided. It is a good hint for users that larger 
size of mbuf
might be needed. If just put it as hardware error, users will lose this 
information. So I
prefer to keep it there, and just assign a new value to it.

#define PKT_RX_HBUF_OVERFLOW (0ULL << 0)  /**< Header buffer overflow. */
[Helin] It indicates the header buff size is not enough, but not means hardware 
cannot
process the packet received. It is a good hint for the users to provide larger 
size of header
buffers. I also prefer to keep it there, and just assign new value to it.

#define PKT_RX_RECIP_ERR (0ULL << 0)  /**< Hardware processing error. */
[Helin] In the latest data sheet, it is not opened to external users. So we can 
just remove
it from here.

#define PKT_RX_MAC_ERR   (0ULL << 0)  /**< MAC error. */
[Helin] This indicates a real hardware error happens.

So my point is to just remove PKT_RX_RECIP_ERR, and we still need other 

[dpdk-dev] [PATCH v2 1/2] igb_uio: compatible with upstream longterm kernel and RHEL6

[Jincheng Miao]

On 11/28/2014 01:01 AM, Thomas Monjalon wrote:
> 2014-10-31 15:37, Jincheng Miao:
>> Function pci_num_vf() is introduced from upstream linux-2.6.34. So
>> this patch make compatible with longterm kernel linux-2.6.32.63.
>>
>> For RHEL6's kernel, although it is based on linux-2.6.32, it has
>> pci_num_vf() implementation. As the same with commit 11ba0426,
>> pci_num_vf() is defined from RHEL6. So we should check the macro
>> RHEL_RELEASE_CODE to consider this situation.
> Please, could you explain in which case CONFIG_PCI_IOV is defined?
> The logic is a bit difficult to understand.

Yep, there is a little confusion for pci_num_vf():
1. it is available when CONFIG_PCI_IOV is defined.
2. it is introduced from upstream kernel v2.6.34 (fb8a0d9)
3. it is implemented from RHEL6.0, although the kernel version is 2.6.32.

The logic of this patch is:
#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 34) && \
(!(defined(RHEL_RELEASE_CODE) && RHEL_RELEASE_CODE >= 
RHEL_RELEASE_VERSION(6, 0) && defined(CONFIG_PCI_IOV)))

Firstly it detects kernel version, if it is less than 2.6.34, and it is 
not RHEL-specified, then define pci_num_vf().

Secondly, it deals with RHEL-specified. If it is RHEL6.0 or later, and 
CONFIG_PCI_IOV is defined. we should not define pci_num_vf(). If any of 
these conditions is not reached, pci_num_vf() should be defined.


Some days ago, I setup dpdk for longterm kernel 2.6.32.63, and got error:
```
CC [M] 
/root/dpdk-source/build/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.o
/root/dpdk-source/build/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.c: 
In function ?show_max_vfs?:
/root/dpdk-source/build/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.c:75: 
error: implicit declaration of function ?pci_num_vf?
```

This problem is introduced by commit 11ba04265

commit 11ba04265cfd2a53c12c030fcaa5dfe7eed39a42
Author: Guillaume Gaudonville 
Date: Wed Sep 3 10:18:23 2014 +0200

igb_uio: fix build on RHEL 6.3

- pci_num_vf() is already defined in RHEL 6
- pci_intx_mask_supported is already defined in RHEL 6.3
- pci_check_and_mask_intx is already defined in RHEL 6.3

Signed-off-by: Guillaume Gaudonville 
Signed-off-by: David Marchand 
Signed-off-by: Thomas Monjalon 

+#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 34) && \
+ !defined(CONFIG_PCI_IOV)

That is because longterm kernel 2.6.32.63 defined CONFIG_PCI_IOV, but it 
lacks pci_num_vf(),
after above processing, pci_num_vf() is still not existed, then build fail.

My patch could work around it, and can deal with RHEL-specified kernel.


>>   #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 34) && \
>> -!defined(CONFIG_PCI_IOV)
>> +   (!(defined(RHEL_RELEASE_CODE) && \
>> +  RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(6, 0) && \
>> +  defined(CONFIG_PCI_IOV)))
>>   
>>   static int pci_num_vf(struct pci_dev *dev)
>>   {

[dpdk-dev] [PATCH] i40e: Use one bit flag for all hardware detected RX packet errors

[Olivier MATZ]

Hi Helin,

On 11/28/2014 09:07 AM, Zhang, Helin wrote:
> After I have completed another task, I read the datasheet carefully again. 
> For those 5
> error bits I introduced for a long time, I'd like to explain one by one as 
> below.
> 
> #define PKT_RX_EIP_CKSUM_BAD (0ULL << 0)  /**< External IP header checksum 
> error. */
> [Helin] Nobody complains it, so we will keep it there, and just assign a new 
> value to it.

ok.

But it would be nice to have a better definition of this flag: does
external mean outer header? For instance, when you receive a
Ether/IP1/UDP/vxlan/Ether/IP2/xxx, does the flag concerns IP1 or IP2?

If it's IP1, it's really strange compared to the current behavior (the
flag PKT_RX_IP_CKSUM_BAD refers to IP1).

> #define PKT_RX_OVERSIZE  (0ULL << 0)  /**< Num of desc of an RX pkt 
> oversize. */
> [Helin] I don't think it can be merge with other hardware errors. It 
> indicates the packet
> received needs more descriptors than hardware allowed, and the part of 
> packets can
> still be stored in the mbufs provided. It is a good hint for users that 
> larger size of mbuf
> might be needed. If just put it as hardware error, users will lose this 
> information. So I
> prefer to keep it there, and just assign a new value to it.

Again, a statistic counter would do the job which if it's just to
provide a hint to the application.

I wonder in which case this flag can happen. If you fill the ring with
mbufs that are large enough compared to your ethernet network, this
should not happen in normal conditions. I really don't believe that
an application receiving an mbuf with this flag would stop the driver,
then refill the rings it with larger mbufs.

Last but not least: If it's really useful, should we have the same
behavior on other drivers like ixgbe? I think we really need to care
about not having different ways to use the different drivers.

To me, the only argument in favor of keeping this flag is when the mbuf
contains a part of the data that could be dumped by a user for debug
purposes.

> #define PKT_RX_HBUF_OVERFLOW (0ULL << 0)  /**< Header buffer overflow. */
> [Helin] It indicates the header buff size is not enough, but not means 
> hardware cannot
> process the packet received. It is a good hint for the users to provide 
> larger size of header
> buffers. I also prefer to keep it there, and just assign new value to it.

Same for this one.

> #define PKT_RX_RECIP_ERR (0ULL << 0)  /**< Hardware processing error. */
> [Helin] In the latest data sheet, it is not opened to external users. So we 
> can just remove
> it from here.

ok

> #define PKT_RX_MAC_ERR   (0ULL << 0)  /**< MAC error. */
> [Helin] This indicates a real hardware error happens.

And what is the content of the mbuf data in this case? Does the
application really need an mbuf?


Regards,
Olivier

[dpdk-dev] [PATCH] enicpmd: replace the type u_int* with uint* to remove compilation errors on a few platforms

[David Marchand]

Hello Sujith,

Why keep those u8, u16, u32 etc... ?
Especially, you can see in this patch that the ioread16 uses u16, while
ioread8, ioread32 uses uint*.
I like consistency and "standard" types (unless there is a reason why we
use different types).

Anyway, this patch builds fine on ppc.

-- 
David Marchand


On Fri, Nov 28, 2014 at 8:02 AM, Sujith Sankar  wrote:

> ENIC PMD was giving compilation errors on ppc_64-power8-linuxapp-gcc
> because
> of types such as u_int32_t.  This patch replaces all those with uint32_t
> and
> similar ones.
>
> Reported-by: David Marchand 
> Signed-off-by: Sujith Sankar 
> ---
>  lib/librte_pmd_enic/enic.h |  2 +-
>  lib/librte_pmd_enic/enic_compat.h  | 16 
>  lib/librte_pmd_enic/enic_main.c| 18 +-
>  lib/librte_pmd_enic/vnic/vnic_devcmd.h |  6 +++---
>  4 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/lib/librte_pmd_enic/enic.h b/lib/librte_pmd_enic/enic.h
> index 9f80fc0..6400d24 100644
> --- a/lib/librte_pmd_enic/enic.h
> +++ b/lib/librte_pmd_enic/enic.h
> @@ -106,7 +106,7 @@ struct enic {
> int iommu_group_fd;
> int iommu_groupid;
> int eventfd;
> -   u_int8_t mac_addr[ETH_ALEN];
> +   uint8_t mac_addr[ETH_ALEN];
> pthread_t err_intr_thread;
> int promisc;
> int allmulti;
> diff --git a/lib/librte_pmd_enic/enic_compat.h
> b/lib/librte_pmd_enic/enic_compat.h
> index d22578e..7c62bf2 100644
> --- a/lib/librte_pmd_enic/enic_compat.h
> +++ b/lib/librte_pmd_enic/enic_compat.h
> @@ -89,9 +89,9 @@ typedef   unsigned intu32;
>  typedef unsigned long long  u64;
>  typedef unsigned long long  dma_addr_t;
>
> -static inline u_int32_t ioread32(volatile void *addr)
> +static inline uint32_t ioread32(volatile void *addr)
>  {
> -   return *(volatile u_int32_t *)addr;
> +   return *(volatile uint32_t *)addr;
>  }
>
>  static inline u16 ioread16(volatile void *addr)
> @@ -99,14 +99,14 @@ static inline u16 ioread16(volatile void *addr)
> return *(volatile u16 *)addr;
>  }
>
> -static inline u_int8_t ioread8(volatile void *addr)
> +static inline uint8_t ioread8(volatile void *addr)
>  {
> -   return *(volatile u_int8_t *)addr;
> +   return *(volatile uint8_t *)addr;
>  }
>
> -static inline void iowrite32(u_int32_t val, volatile void *addr)
> +static inline void iowrite32(uint32_t val, volatile void *addr)
>  {
> -   *(volatile u_int32_t *)addr = val;
> +   *(volatile uint32_t *)addr = val;
>  }
>
>  static inline void iowrite16(u16 val, volatile void *addr)
> @@ -114,9 +114,9 @@ static inline void iowrite16(u16 val, volatile void
> *addr)
> *(volatile u16 *)addr = val;
>  }
>
> -static inline void iowrite8(u_int8_t val, volatile void *addr)
> +static inline void iowrite8(uint8_t val, volatile void *addr)
>  {
> -   *(volatile u_int8_t *)addr = val;
> +   *(volatile uint8_t *)addr = val;
>  }
>
>  static inline unsigned int readl(volatile void __iomem *addr)
> diff --git a/lib/librte_pmd_enic/enic_main.c
> b/lib/librte_pmd_enic/enic_main.c
> index f6f00d3..853dd04 100644
> --- a/lib/librte_pmd_enic/enic_main.c
> +++ b/lib/librte_pmd_enic/enic_main.c
> @@ -172,17 +172,17 @@ unsigned int enic_cleanup_wq(struct enic *enic,
> struct vnic_wq *wq)
>
>  int enic_send_pkt(struct enic *enic, struct vnic_wq *wq,
> struct rte_mbuf *tx_pkt, unsigned short len,
> -   u_int8_t sop, u_int8_t eop,
> -   u_int16_t ol_flags, u_int16_t vlan_tag)
> +   uint8_t sop, uint8_t eop,
> +   uint16_t ol_flags, uint16_t vlan_tag)
>  {
> struct wq_enet_desc *desc = vnic_wq_next_desc(wq);
> -   u_int16_t mss = 0;
> -   u_int16_t header_length = 0;
> -   u_int8_t cq_entry = eop;
> -   u_int8_t vlan_tag_insert = 0;
> +   uint16_t mss = 0;
> +   uint16_t header_length = 0;
> +   uint8_t cq_entry = eop;
> +   uint8_t vlan_tag_insert = 0;
> unsigned char *buf = (unsigned char *)(tx_pkt->buf_addr) +
> RTE_PKTMBUF_HEADROOM;
> -   u_int64_t bus_addr = (dma_addr_t)
> +   uint64_t bus_addr = (dma_addr_t)
> (tx_pkt->buf_physaddr + RTE_PKTMBUF_HEADROOM);
>
> if (sop) {
> @@ -342,8 +342,8 @@ static int enic_rq_alloc_buf(struct vnic_rq *rq)
> void *buf;
> dma_addr_t dma_addr;
> struct rq_enet_desc *desc = vnic_rq_next_desc(rq);
> -   u_int8_t type = RQ_ENET_TYPE_ONLY_SOP;
> -   u_int16_t len = ENIC_MAX_MTU + VLAN_ETH_HLEN;
> +   uint8_t type = RQ_ENET_TYPE_ONLY_SOP;
> +   uint16_t len = ENIC_MAX_MTU + VLAN_ETH_HLEN;
> u16 split_hdr_size = vnic_get_hdr_split_size(enic->vdev);
> struct rte_mbuf *mbuf = enic_rxmbuf_alloc(rq->mp);
> struct rte_mbuf *hdr_mbuf = NULL;
> diff --git a/lib/librte_pmd_enic/vnic/vnic_devcmd.h
> b/lib/librte_pmd_enic/vnic/vnic_devcmd.h
> index b4c87c1..e7ecf31 100644
> --- a/lib/librte_pmd_enic/vnic/vnic_devcmd.h
> +++ b/lib/lib

[dpdk-dev] [PATCH] ixgbe: fix icc issue with mbuf initializer

[Cao, Min]

Tested-by: Min Cao 

Patch name: [dpdk-dev] [PATCH v2] ixgbe: fix icc issue with mbuf 
initializer
Test Flag:  Tested-by
Tester name:min.cao at intel.com
ICC version:13.1.2
ICC package:l_ccompxe_2013.4.183.tgz
Result summary: total 6 cases, 6 passed, 0 failed

Test Case 1:
Name:   l2fwd
Environment:OS: Fedora20 3.11.10-301.fc20.x86_64
CPU: Intel(R) Xeon(R) CPU E5-2680 0 @ 2.70GHz
NIC: Fortville eagle/spirit 
Test result(32bit): PASSED
Test result(64bit): PASSED

Test Case 2:
Name:   l3fwd
Environment:OS: Fedora20 3.11.10-301.fc20.x86_64
CPU: Intel(R) Xeon(R) CPU E5-2680 0 @ 2.70GHz
NIC: Fortville eagle/spirit 
Test result(32bit): PASSED
Test result(64bit): PASSED
Test Case 3:
Name:   pmd
Environment:OS: Fedora20 3.11.10-301.fc20.x86_64
CPU: Intel(R) Xeon(R) CPU E5-2680 0 @ 2.70GHz
NIC: Fortville eagle/spirit 
Test result(32bit): PASSED
Test result(64bit): PASSED

-Original Message-
From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Bruce Richardson
Sent: Monday, November 03, 2014 7:11 PM
To: dev at dpdk.org
Subject: [dpdk-dev] [PATCH] ixgbe: fix icc issue with mbuf initializer

When using Intel C++ compiler(icc) 14.0.1.106 or the older icc 13.x
version, the mbuf initializer variable was not getting configured
correctly, as the mb_def variable was not set correctly. This is due
to an issue with icc (DPD200249565 which already been fixed in
icc 14.0.2 and newer compiler release) where it incorrectly calculates
the field offsets with initializers when zero-sized fields
are used in a structure.
To work around this, the code in ixgbe_rxq_vec_setup does not setup the
fields using an initializer, but instead assigns the values individually
in code
NOTE: There is no performance impact to this change as the queue
setup functions are not data-plane APIs, but are only used at app
initialization.

Signed-off-by: Bruce Richardson 
---
 lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c 
b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
index e813e43..b57c588 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
@@ -730,16 +730,15 @@ static struct ixgbe_txq_ops vec_txq_ops = {
 int
 ixgbe_rxq_vec_setup(struct igb_rx_queue *rxq)
 {
-   struct rte_mbuf mb_def = {
-   .nb_segs = 1,
-   .data_off = RTE_PKTMBUF_HEADROOM,
-#ifdef RTE_MBUF_REFCNT
-   { .refcnt = 1, }
-#endif
-   };
+   struct rte_mbuf mb_def = { .buf_addr = 0 }; /* zeroed mbuf */

+   mb_def.nb_segs = 1;
+   mb_def.data_off = RTE_PKTMBUF_HEADROOM;
mb_def.buf_len = rxq->mb_pool->elt_size - sizeof(struct rte_mbuf);
mb_def.port = rxq->port_id;
+#ifdef RTE_MBUF_REFCNT
+   mb_def.refcnt = 1;
+#endif
rxq->mbuf_initializer = *((uint64_t *)&mb_def.rearm_data);
return 0;
 }
-- 
1.9.3

[dpdk-dev] [PATCH v2] ixgbe: fix icc issue with mbuf initializer

[Cao, Min]

Tested-by: Min Cao 

Patch name: [dpdk-dev] [PATCH v2] ixgbe: fix icc issue with mbuf 
initializer
Test Flag:  Tested-by
Tester name:min.cao at intel.com
ICC version:13.1.2
ICC package:l_ccompxe_2013.4.183.tgz
Result summary: total 6 cases, 6 passed, 0 failed

Test Case 1:
Name:   l2fwd
Environment:OS: Fedora20 3.11.10-301.fc20.x86_64
CPU: Intel(R) Xeon(R) CPU E5-2680 0 @ 2.70GHz
NIC: Fortville eagle/spirit 
Test result(32bit): PASSED
Test result(64bit): PASSED

Test Case 2:
Name:   l3fwd
Environment:OS: Fedora20 3.11.10-301.fc20.x86_64
CPU: Intel(R) Xeon(R) CPU E5-2680 0 @ 2.70GHz
NIC: Fortville eagle/spirit 
Test result(32bit): PASSED
Test result(64bit): PASSED
Test Case 3:
Name:   pmd
Environment:OS: Fedora20 3.11.10-301.fc20.x86_64
CPU: Intel(R) Xeon(R) CPU E5-2680 0 @ 2.70GHz
NIC: Fortville eagle/spirit 
Test result(32bit): PASSED
Test result(64bit): PASSED

-Original Message-
From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Bruce Richardson
Sent: Tuesday, November 04, 2014 1:01 AM
To: dev at dpdk.org
Subject: [dpdk-dev] [PATCH v2] ixgbe: fix icc issue with mbuf initializer

When using Intel C++ compiler(icc) 14.0.1.106 or the older icc 13.x
version, the mbuf initializer variable was not getting configured
correctly, as the mb_def variable was not set correctly. This is due
to an issue with icc (DPD200249565 which already been fixed in
icc 14.0.2 and newer compiler release) where it incorrectly calculates
the field offsets with initializers when zero-sized fields
are used in a structure.
To work around this, the code in ixgbe_rxq_vec_setup does not setup the
fields using an initializer, but instead assigns the values individually
in code
NOTE: There is no performance impact to this change as the queue
setup functions are not data-plane APIs, but are only used at app
initialization.

Signed-off-by: Bruce Richardson 

---
V2 change: use rte_mbuf_refcnt_set to update reference count

---
 lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c 
b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
index e813e43..42c0f60 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
@@ -730,16 +730,13 @@ static struct ixgbe_txq_ops vec_txq_ops = {
 int
 ixgbe_rxq_vec_setup(struct igb_rx_queue *rxq)
 {
-   struct rte_mbuf mb_def = {
-   .nb_segs = 1,
-   .data_off = RTE_PKTMBUF_HEADROOM,
-#ifdef RTE_MBUF_REFCNT
-   { .refcnt = 1, }
-#endif
-   };
+   struct rte_mbuf mb_def = { .buf_addr = 0 }; /* zeroed mbuf */

+   mb_def.nb_segs = 1;
+   mb_def.data_off = RTE_PKTMBUF_HEADROOM;
mb_def.buf_len = rxq->mb_pool->elt_size - sizeof(struct rte_mbuf);
mb_def.port = rxq->port_id;
+   rte_mbuf_refcnt_set(&mb_def, 1);
rxq->mbuf_initializer = *((uint64_t *)&mb_def.rearm_data);
return 0;
 }
-- 
1.9.3

[dpdk-dev] [PATCH v4 08/13] testpmd: rework csum forward engine

[Liu, Jijiang]

Hi Olivier,

> -Original Message-
> From: Ananyev, Konstantin
> Sent: Thursday, November 27, 2014 7:02 PM
> To: Olivier MATZ; dev at dpdk.org
> Cc: Walukiewicz, Miroslaw; Liu, Jijiang; Liu, Yong; jigsaw at gmail.com; 
> Richardson,
> Bruce
> Subject: RE: [PATCH v4 08/13] testpmd: rework csum forward engine
> 
> Hi Oliver,
> 
> > -Original Message-
> > From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> > Sent: Thursday, November 27, 2014 9:11 AM
> > To: Ananyev, Konstantin; dev at dpdk.org
> > Cc: Walukiewicz, Miroslaw; Liu, Jijiang; Liu, Yong; jigsaw at gmail.com;
> > Richardson, Bruce
> > Subject: Re: [PATCH v4 08/13] testpmd: rework csum forward engine
> >
> > Hi Konstantin,
> >
> > On 11/26/2014 09:02 PM, Ananyev, Konstantin wrote:
> > >> +/* if possible, calculate the checksum of a packet in hw or sw,
> > >> + * depending on the testpmd command line configuration */ static
> > >> +uint64_t process_inner_cksums(void *l3_hdr, uint16_t ethertype,
> > >> +uint16_t l3_len,
> > >> +uint8_t l4_proto, uint16_t testpmd_ol_flags) {
> > >> +struct ipv4_hdr *ipv4_hdr = l3_hdr;
> > >> +struct udp_hdr *udp_hdr;
> > >> +struct tcp_hdr *tcp_hdr;
> > >> +struct sctp_hdr *sctp_hdr;
> > >> +uint64_t ol_flags = 0;
> > >> +
> > >> +if (ethertype == _htons(ETHER_TYPE_IPv4)) {
> > >> +ipv4_hdr = l3_hdr;
> > >> +ipv4_hdr->hdr_checksum = 0;
> > >> +
> > >> +if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_IP_CKSUM)
> > >> +ol_flags |= PKT_TX_IP_CKSUM;
> > >> +else
> > >> +ipv4_hdr->hdr_checksum = 
> > >> get_ipv4_cksum(ipv4_hdr);
> > >> +
> > >> +ol_flags |= PKT_TX_IPV4;
> > >
> > > Flags PKT_TX_IP_CKSUM, PKT_TX_IPV4, PKT_TX_IPV6 are all mutually
> exclusive.
> > > So it should be, I think:
> > >
> > > if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_IP_CKSUM) {
> > >  ol_flags |= PKT_TX_IP_CKSUM;
> > >   } else {
> > >   ipv4_hdr->hdr_checksum = get_ipv4_cksum(ipv4_hdr);
> > >   ol_flags |= PKT_TX_IPV4; }
> >
> > It seems normal that PKT_TX_IPV4 are PKT_TX_IPV6 exclusive, but do you
> > mean that PKT_TX_IP_CKSUM and PKT_TX_IPV4 are exclusive too? It looks
> > strange to me.
> >
> > My understanding of the meaning of the flags is:
> >
> >- PKT_TX_IP_CKSUM: tell the NIC to compute IP cksum

> My initial thought:
> It tells the NIC that it is an IPV4 packet for which it has to compute 
> checksum.
> 
> >
> >- PKT_TX_IPV4: tell the NIC it's an IPv4 packet. Required for L4
> >  checksum offload or TSO.
> 
> It tells the NIC that it is an IPV4 packet for which it shouldn't compute 
> checksum.
> 
> >
> >- PKT_TX_IPV6: tell the NIC it's an IPv6 packet. Required for L4
> >  checksum offload or TSO.
> 
> Yes.
> 
> >
> > If it's a i40e driver requirement, don't you think it's better to
> > change the driver?

There should be two logics in csum engine, which is  that either HW computes TX 
checksum (using PKT_TX_IP_CKSUM) or SW compute TX checksum(use PKT_TX_IPV4(or 
another flag) to tell driver no IP checksum offload requirement ),
I think we shouldn't use L3 flag to tell driver what HW need do for L4,  L3 and 
L4 flag should be separated .


> Yes, it could be done in both ways:
> either all 3 flags are mutually exclusive or first two and third one are 
> mutually
> exclusive.
> 
> Current i40e PMD  seems to work correctly with the second way too.
> 
> Though the second way implies a specific order for PMD to check flags.
> Something like:
>  if (ol_flags & PKT_TX_IP_CKSUM) {..} else if (ol_flags & PKT_TX_IPV4) {...} 
> else ...
> would work correctly.

I40e driver use this way.

> But:
> if (ol_flags & PKT_TX_IPV4) {...} else if (ol_flags & PKT_TX_IP_CKSUM) {..} 
> else
> wouldn't.


> >
> > >> +/* Calculate the checksum of outer header (only vxlan is
> > >> +supported,
> > >> + * meaning IP + UDP). The caller already checked that it's a vxlan
> > >> + * packet */
> > >> +static uint64_t
> > >> +process_outer_cksums(void *outer_l3_hdr, uint16_t outer_ethertype,
> > >> +uint16_t outer_l3_len, uint16_t testpmd_ol_flags) {
> > >> +struct ipv4_hdr *ipv4_hdr = outer_l3_hdr;
> > >> +struct ipv6_hdr *ipv6_hdr = outer_l3_hdr;
> > >> +struct udp_hdr *udp_hdr;
> > >> +uint64_t ol_flags = 0;
> > >> +
> > >> +if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM)
> > >> +ol_flags |= PKT_TX_VXLAN_CKSUM;
> > >> +
> > >> +if (outer_ethertype == _htons(ETHER_TYPE_IPv4)) {
> > >> +ipv4_hdr->hdr_checksum = 0;
> > >> +
> > >> +if ((testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM)
> == 0)
> > >> +ipv4_hdr->hdr_checksum = 
> > >> get_ipv4_cksum(ipv4_hdr);
> > >> +}
> > >> +
> > >> +udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + 
> > >> outer_l3_len);
> > >> + 

[dpdk-dev] [PATCH 1/3] mbuf:add two TX offload flags and change three fields

[Olivier MATZ]

Hi Jijiang,

On 11/27/2014 02:14 PM, Liu, Jijiang wrote:
>>> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
>>> index 367fc56..48cd8e1 100644
>>> --- a/lib/librte_mbuf/rte_mbuf.h
>>> +++ b/lib/librte_mbuf/rte_mbuf.h
>>> @@ -99,10 +99,9 @@ extern "C" {
>>>   #define PKT_RX_TUNNEL_IPV6_HDR (1ULL << 12) /**< RX tunnel packet with
>> IPv6 header. */
>>>   #define PKT_RX_FDIR_ID   (1ULL << 13) /**< FD id reported if FDIR 
>>> match.
>> */
>>>   #define PKT_RX_FDIR_FLX  (1ULL << 14) /**< Flexible bytes reported if 
>>> FDIR
>> match. */
>>> -/* add new RX flags here */
>>>
>>
>> We should probably not remove this line.
> 
> Why?
> There are two lines "/* add new RX flags here */" in rte_mbuf.h file.

No, one is RX, the other is TX.


>>> +/** Tell the NIC it's an outer IPv6 packet for tunneling packet.*/
>>> +#define PKT_TX_OUTER_IPV6(1ULL << 59)
>>>
>>
>> This flag is not in the cover letter or commit log. What is its purpose?
>>
> With FVL, if outer L3 header is IPv6, to make HW TX checksum offload work , 
> SW must be responsible to tell hardware this information.


What hardware checksum are you talking about?
I understand that outer L4 checksum is not supported from one of
Konstantin's mail.
And there is no L3 checksum in IPv6.


Regards,
Olivier

[dpdk-dev] [PATCH] ixgbe: Add missing rx_mbuf_alloc_failed statistics for vector PMD

[Balazs Nemeth]

The statistics that is reported through the rx_nombuf fields in struct
rte_eth_stats was not set when the vector PMD was used. The statistics
should report the number of mbufs that could _not_ be allocated during
rearm of the RX queue. The non-vector PMD reports it correctly. The
use of either vector PMD or non-vector PMD depends on runtime
configuration. Hence it is possible that a change in configuration
would disable this statistics. To prevent this from happening, the
statistics should be reported by both implementations.

Signed-off-by: Balazs Nemeth 
---
 lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c 
b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
index 42c0f60..579bc46 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
@@ -71,6 +71,8 @@ ixgbe_rxq_rearm(struct igb_rx_queue *rxq)
dma_addr0);
}
}
+   rte_eth_devices[rxq->port_id].data->rx_mbuf_alloc_failed +=
+   RTE_IXGBE_RXQ_REARM_THRESH;
return;
}

-- 
2.1.2

[dpdk-dev] [PATCH] enicpmd: replace the type u_int* with uint* to remove compilation errors on a few platforms

[Sujith Sankar (ssujith)]

Hi David,

The u8, u16, etc are there because enicpmd shares a lot of code with kernel 
mode driver and these are used at a lot of places.
I agree with you on bringing in consistency.  Let me change ioread16 with 
uint16 and upload a new patch.

Hope this is ok with you.

Thanks,
-Sujith

From: David Marchand mailto:david.march...@6wind.com>>
Date: Friday, 28 November 2014 2:18 pm
To: "Sujith Sankar (ssujith)" mailto:ssujith at 
cisco.com>>
Cc: "dev at dpdk.org" mailto:dev at 
dpdk.org>>
Subject: Re: [dpdk-dev] [PATCH] enicpmd: replace the type u_int* with uint* to 
remove compilation errors on a few platforms

Hello Sujith,

Why keep those u8, u16, u32 etc... ?
Especially, you can see in this patch that the ioread16 uses u16, while 
ioread8, ioread32 uses uint*.
I like consistency and "standard" types (unless there is a reason why we use 
different types).

Anyway, this patch builds fine on ppc.

--
David Marchand


On Fri, Nov 28, 2014 at 8:02 AM, Sujith Sankar mailto:ssujith at cisco.com>> wrote:
ENIC PMD was giving compilation errors on ppc_64-power8-linuxapp-gcc because
of types such as u_int32_t.  This patch replaces all those with uint32_t and
similar ones.

Reported-by: David Marchand mailto:david.marchand 
at 6wind.com>>
Signed-off-by: Sujith Sankar mailto:ssujith at cisco.com>>
---
 lib/librte_pmd_enic/enic.h |  2 +-
 lib/librte_pmd_enic/enic_compat.h  | 16 
 lib/librte_pmd_enic/enic_main.c| 18 +-
 lib/librte_pmd_enic/vnic/vnic_devcmd.h |  6 +++---
 4 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/lib/librte_pmd_enic/enic.h b/lib/librte_pmd_enic/enic.h
index 9f80fc0..6400d24 100644
--- a/lib/librte_pmd_enic/enic.h
+++ b/lib/librte_pmd_enic/enic.h
@@ -106,7 +106,7 @@ struct enic {
int iommu_group_fd;
int iommu_groupid;
int eventfd;
-   u_int8_t mac_addr[ETH_ALEN];
+   uint8_t mac_addr[ETH_ALEN];
pthread_t err_intr_thread;
int promisc;
int allmulti;
diff --git a/lib/librte_pmd_enic/enic_compat.h 
b/lib/librte_pmd_enic/enic_compat.h
index d22578e..7c62bf2 100644
--- a/lib/librte_pmd_enic/enic_compat.h
+++ b/lib/librte_pmd_enic/enic_compat.h
@@ -89,9 +89,9 @@ typedef   unsigned intu32;
 typedef unsigned long long  u64;
 typedef unsigned long long  dma_addr_t;

-static inline u_int32_t ioread32(volatile void *addr)
+static inline uint32_t ioread32(volatile void *addr)
 {
-   return *(volatile u_int32_t *)addr;
+   return *(volatile uint32_t *)addr;
 }

 static inline u16 ioread16(volatile void *addr)
@@ -99,14 +99,14 @@ static inline u16 ioread16(volatile void *addr)
return *(volatile u16 *)addr;
 }

-static inline u_int8_t ioread8(volatile void *addr)
+static inline uint8_t ioread8(volatile void *addr)
 {
-   return *(volatile u_int8_t *)addr;
+   return *(volatile uint8_t *)addr;
 }

-static inline void iowrite32(u_int32_t val, volatile void *addr)
+static inline void iowrite32(uint32_t val, volatile void *addr)
 {
-   *(volatile u_int32_t *)addr = val;
+   *(volatile uint32_t *)addr = val;
 }

 static inline void iowrite16(u16 val, volatile void *addr)
@@ -114,9 +114,9 @@ static inline void iowrite16(u16 val, volatile void *addr)
*(volatile u16 *)addr = val;
 }

-static inline void iowrite8(u_int8_t val, volatile void *addr)
+static inline void iowrite8(uint8_t val, volatile void *addr)
 {
-   *(volatile u_int8_t *)addr = val;
+   *(volatile uint8_t *)addr = val;
 }

 static inline unsigned int readl(volatile void __iomem *addr)
diff --git a/lib/librte_pmd_enic/enic_main.c b/lib/librte_pmd_enic/enic_main.c
index f6f00d3..853dd04 100644
--- a/lib/librte_pmd_enic/enic_main.c
+++ b/lib/librte_pmd_enic/enic_main.c
@@ -172,17 +172,17 @@ unsigned int enic_cleanup_wq(struct enic *enic, struct 
vnic_wq *wq)

 int enic_send_pkt(struct enic *enic, struct vnic_wq *wq,
struct rte_mbuf *tx_pkt, unsigned short len,
-   u_int8_t sop, u_int8_t eop,
-   u_int16_t ol_flags, u_int16_t vlan_tag)
+   uint8_t sop, uint8_t eop,
+   uint16_t ol_flags, uint16_t vlan_tag)
 {
struct wq_enet_desc *desc = vnic_wq_next_desc(wq);
-   u_int16_t mss = 0;
-   u_int16_t header_length = 0;
-   u_int8_t cq_entry = eop;
-   u_int8_t vlan_tag_insert = 0;
+   uint16_t mss = 0;
+   uint16_t header_length = 0;
+   uint8_t cq_entry = eop;
+   uint8_t vlan_tag_insert = 0;
unsigned char *buf = (unsigned char *)(tx_pkt->buf_addr) +
RTE_PKTMBUF_HEADROOM;
-   u_int64_t bus_addr = (dma_addr_t)
+   uint64_t bus_addr = (dma_addr_t)
(tx_pkt->buf_physaddr + RTE_PKTMBUF_HEADROOM);

if (sop) {
@@ -342,8 +342,8 @@ static int enic_rq_alloc_buf(struct vnic_rq *rq)
void *buf;
dma_addr_t dma_addr;
struct rq_enet_desc *desc = vnic_rq_next_desc(rq);
-   u_int8_t type = RQ_ENET_TYPE_ONLY_SOP

[dpdk-dev] [PATCH 0/3] i40e VXLAN TX checksum rework

[Olivier MATZ]

Hi Konstantin,

On 11/27/2014 04:29 PM, Ananyev, Konstantin wrote:
>> As I suggested in the TSO thread, I think the following semantics
>> is easier to understand for the user:
>>
>>- PKT_TX_IP_CKSUM: tell the NIC to compute IP cksum
>>
>>- PKT_TX_IPV4: tell the NIC it's an IPv4 packet. Required for L4
>>  checksum offload or TSO.
>>
>>- PKT_TX_IPV6: tell the NIC it's an IPv6 packet. Required for L4
>>  checksum offload or TSO.
>>
>> I think it won't make a big difference in the FVL driver.
> 
> No, no big difference here, but I still think it will be a bit cleaner if all 
> 3 flags would be nutually exclusive.
> In fact,  we can unite all 3 of them them into 2 bits,same as we doing 
> for L4 checksum flags.

In case of TSO, you need to set the PKT_TX_IPV4 flag.
But as suggested by Yong Wang from Vmware [1], the vmxnet3 driver could
support TSO without offloading IP checksum, so I think it's better to
have flags for (is_ipv4 or is_ipv6), and another one to ask the
ip_checksum.


> You mean a new DEV_TX_OFFLOAD_* value, right?
> Something like:  DEV_TX_OFFLOAD_UDP_TUNNEL?
> And make i40e_dev_info_get() to return it?
> Yes, forgot about it, sounds like a proper thing to do. 

Yes. I've seen that Jijiang is planning to add it in a future bug fix
patch. That's fine to me.


[1] http://dpdk.org/ml/archives/dev/2014-November/007775.html

Regards,
Olivier

[dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and change three fields

[Olivier MATZ]

Hi Jijiang,

On 11/27/2014 06:03 PM, Jijiang Liu wrote:
>  /** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or 
> TSO. */
>  #define PKT_TX_IPV4  PKT_RX_IPV4_HDR
>  
>  /** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or 
> TSO. */
>  #define PKT_TX_IPV6  PKT_RX_IPV6_HDR

The description still does not match what we discussed. Either we
have PKT_TX_IPV4 meaning "packet is IPv4 without requiring IP cksum
offload", or  "packet is IPv4". I prefer the second one, but whatever
the choice is, the comments must be coherent.

> -#define PKT_TX_VLAN_PKT  (1ULL << 55) /**< TX packet is a 802.1q VLAN 
> packet. */
> +/** Outer IP cksum of TX pkt. computed by NIC for tunneling packet */
> +#define PKT_TX_OUTER_IP_CKSUM   (1ULL << 58)
> +
> +/** Tell the NIC it's an outer IPv6 packet for tunneling packet.*/
> +#define PKT_TX_OUTER_IPV6(1ULL << 59)

I think we should have the same flags with the same meanings for
inner and outer:

- PKT_TX_IPV4, PKT_TX_IP_CKSUM, PKT_TX_IPV6
- PKT_TX_OUTER_IPV4, PKT_TX_OUTER_IP_CKSUM, PKT_TX_OUTER_IPV6

In your patch there is no PKT_TX_OUTER_IPV4 flag.

Regards,
Olivier

[dpdk-dev] [PATCH v3 2/4] mbuf:change PKT_TX_IPV4 and PKT_TX_IPV6 definition

[Olivier MATZ]

On 11/27/2014 06:03 PM, Jijiang Liu wrote:
> It will avoid to send a packet with a bad info:
>   - we receive a Ether/IP6/IP4/L4/data packet
>   - the driver sets PKT_RX_IPV6_HDR
>   - the stack decapsulates IP6
>   - the stack sends the packet, it has the PKT_TX_IPV6 flag but it's an IPv4 
> packet.
> 
> Signed-off-by: Jijiang Liu 
> ---
>  lib/librte_mbuf/rte_mbuf.h |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 22ee555..f6b3185 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -127,10 +127,10 @@ extern "C" {
>  #define PKT_TX_VLAN_PKT  (1ULL << 55) /**< TX packet is a 802.1q VLAN 
> packet. */
>  
>  /** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or 
> TSO. */
> -#define PKT_TX_IPV4  PKT_RX_IPV4_HDR
> +#define PKT_TX_IPV4  (1ULL << 56)
>  
>  /** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or 
> TSO. */
> -#define PKT_TX_IPV6  PKT_RX_IPV6_HDR
> +#define PKT_TX_IPV6  (1ULL << 57)
>  
>  /** Outer IP cksum of TX pkt. computed by NIC for tunneling packet */
>  #define PKT_TX_OUTER_IP_CKSUM   (1ULL << 58)
> 

Acked-by: Olivier Matz 

[dpdk-dev] [PATCH v2] enicpmd: replace the type u_int* with uint* to remove compilation errors on a few platforms

[Sujith Sankar]

ENIC PMD was giving compilation errors on ppc_64-power8-linuxapp-gcc because
of types such as u_int32_t.  This patch replaces all those with uint32_t and
similar ones.

Reported-by: David Marchand 
Signed-off-by: Sujith Sankar 
---
 lib/librte_pmd_enic/enic.h |  2 +-
 lib/librte_pmd_enic/enic_compat.h  | 24 
 lib/librte_pmd_enic/enic_main.c| 18 +-
 lib/librte_pmd_enic/vnic/vnic_devcmd.h |  6 +++---
 4 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/lib/librte_pmd_enic/enic.h b/lib/librte_pmd_enic/enic.h
index 9f80fc0..6400d24 100644
--- a/lib/librte_pmd_enic/enic.h
+++ b/lib/librte_pmd_enic/enic.h
@@ -106,7 +106,7 @@ struct enic {
int iommu_group_fd;
int iommu_groupid;
int eventfd;
-   u_int8_t mac_addr[ETH_ALEN];
+   uint8_t mac_addr[ETH_ALEN];
pthread_t err_intr_thread;
int promisc;
int allmulti;
diff --git a/lib/librte_pmd_enic/enic_compat.h 
b/lib/librte_pmd_enic/enic_compat.h
index d22578e..b3738ee 100644
--- a/lib/librte_pmd_enic/enic_compat.h
+++ b/lib/librte_pmd_enic/enic_compat.h
@@ -89,34 +89,34 @@ typedef unsigned intu32;
 typedef unsigned long long  u64;
 typedef unsigned long long  dma_addr_t;

-static inline u_int32_t ioread32(volatile void *addr)
+static inline uint32_t ioread32(volatile void *addr)
 {
-   return *(volatile u_int32_t *)addr;
+   return *(volatile uint32_t *)addr;
 }

-static inline u16 ioread16(volatile void *addr)
+static inline uint16_t ioread16(volatile void *addr)
 {
-   return *(volatile u16 *)addr;
+   return *(volatile uint16_t *)addr;
 }

-static inline u_int8_t ioread8(volatile void *addr)
+static inline uint8_t ioread8(volatile void *addr)
 {
-   return *(volatile u_int8_t *)addr;
+   return *(volatile uint8_t *)addr;
 }

-static inline void iowrite32(u_int32_t val, volatile void *addr)
+static inline void iowrite32(uint32_t val, volatile void *addr)
 {
-   *(volatile u_int32_t *)addr = val;
+   *(volatile uint32_t *)addr = val;
 }

-static inline void iowrite16(u16 val, volatile void *addr)
+static inline void iowrite16(uint16_t val, volatile void *addr)
 {
-   *(volatile u16 *)addr = val;
+   *(volatile uint16_t *)addr = val;
 }

-static inline void iowrite8(u_int8_t val, volatile void *addr)
+static inline void iowrite8(uint8_t val, volatile void *addr)
 {
-   *(volatile u_int8_t *)addr = val;
+   *(volatile uint8_t *)addr = val;
 }

 static inline unsigned int readl(volatile void __iomem *addr)
diff --git a/lib/librte_pmd_enic/enic_main.c b/lib/librte_pmd_enic/enic_main.c
index f6f00d3..853dd04 100644
--- a/lib/librte_pmd_enic/enic_main.c
+++ b/lib/librte_pmd_enic/enic_main.c
@@ -172,17 +172,17 @@ unsigned int enic_cleanup_wq(struct enic *enic, struct 
vnic_wq *wq)

 int enic_send_pkt(struct enic *enic, struct vnic_wq *wq,
struct rte_mbuf *tx_pkt, unsigned short len,
-   u_int8_t sop, u_int8_t eop,
-   u_int16_t ol_flags, u_int16_t vlan_tag)
+   uint8_t sop, uint8_t eop,
+   uint16_t ol_flags, uint16_t vlan_tag)
 {
struct wq_enet_desc *desc = vnic_wq_next_desc(wq);
-   u_int16_t mss = 0;
-   u_int16_t header_length = 0;
-   u_int8_t cq_entry = eop;
-   u_int8_t vlan_tag_insert = 0;
+   uint16_t mss = 0;
+   uint16_t header_length = 0;
+   uint8_t cq_entry = eop;
+   uint8_t vlan_tag_insert = 0;
unsigned char *buf = (unsigned char *)(tx_pkt->buf_addr) +
RTE_PKTMBUF_HEADROOM;
-   u_int64_t bus_addr = (dma_addr_t)
+   uint64_t bus_addr = (dma_addr_t)
(tx_pkt->buf_physaddr + RTE_PKTMBUF_HEADROOM);

if (sop) {
@@ -342,8 +342,8 @@ static int enic_rq_alloc_buf(struct vnic_rq *rq)
void *buf;
dma_addr_t dma_addr;
struct rq_enet_desc *desc = vnic_rq_next_desc(rq);
-   u_int8_t type = RQ_ENET_TYPE_ONLY_SOP;
-   u_int16_t len = ENIC_MAX_MTU + VLAN_ETH_HLEN;
+   uint8_t type = RQ_ENET_TYPE_ONLY_SOP;
+   uint16_t len = ENIC_MAX_MTU + VLAN_ETH_HLEN;
u16 split_hdr_size = vnic_get_hdr_split_size(enic->vdev);
struct rte_mbuf *mbuf = enic_rxmbuf_alloc(rq->mp);
struct rte_mbuf *hdr_mbuf = NULL;
diff --git a/lib/librte_pmd_enic/vnic/vnic_devcmd.h 
b/lib/librte_pmd_enic/vnic/vnic_devcmd.h
index b4c87c1..e7ecf31 100644
--- a/lib/librte_pmd_enic/vnic/vnic_devcmd.h
+++ b/lib/librte_pmd_enic/vnic/vnic_devcmd.h
@@ -691,9 +691,9 @@ enum {
 #define FILTER_MAX_BUF_SIZE 100  /* Maximum size of buffer to CMD_ADD_FILTER */

 struct filter_tlv {
-   u_int32_t type;
-   u_int32_t length;
-   u_int32_t val[0];
+   uint32_t type;
+   uint32_t length;
+   uint32_t val[0];
 };

 enum {
-- 
1.9.1

[dpdk-dev] [PATCH v3 4/4] testpmd:rework csum forward engine

[Olivier MATZ]

Hi Jijiang,

On 11/27/2014 06:03 PM, Jijiang Liu wrote:
> @@ -303,7 +305,7 @@ process_outer_cksums(void *outer_l3_hdr, uint16_t 
> outer_ethertype,
>   * TESTPMD_TX_OFFLOAD_* in ports[tx_port].tx_ol_flags. They control
>   * wether a checksum must be calculated in software or in hardware. The
>   * IP, UDP, TCP and SCTP flags always concern the inner layer.  The
> - * VxLAN flag concerns the outer IP and UDP layer (if packet is
> + * VxLAN flag concerns the outer IP(if packet is
>   * recognized as a vxlan packet).
>   */

Please reindent the comment properly, taking care of the space before
the parenthesis.

Another comment that concerns the whole patchset. It's maybe a question
for Thomas, but I think that having patches that don't break compilation
between each other would help when bissecting.

In this case, I wonder if it's possible to split in something like:

- change PKT_TX_IPV4 and PKT_TX_IPV6 definition
- add + rename flags (update them in mbuf + testpmd + i40)
- rename inner_l{23}, l{23} in l{23}, outer_l{23} (update them in mbuf
  + testpmd + i40)

I'm not sure it's feasible, I let Thomas give his mind about this.

Regards,
Olivier

[dpdk-dev] [PATCH v4 08/13] testpmd: rework csum forward engine

[Olivier MATZ]

Hi Jijiang,

On 11/28/2014 09:54 AM, Liu, Jijiang wrote:
>>> My understanding of the meaning of the flags is:
>>>
>>>- PKT_TX_IP_CKSUM: tell the NIC to compute IP cksum
> 
>> My initial thought:
>> It tells the NIC that it is an IPV4 packet for which it has to compute 
>> checksum.
>>
>>>
>>>- PKT_TX_IPV4: tell the NIC it's an IPv4 packet. Required for L4
>>>  checksum offload or TSO.
>>
>> It tells the NIC that it is an IPV4 packet for which it shouldn't compute 
>> checksum.
>>
>>>
>>>- PKT_TX_IPV6: tell the NIC it's an IPv6 packet. Required for L4
>>>  checksum offload or TSO.
>>
>> Yes.
>>
>>>
>>> If it's a i40e driver requirement, don't you think it's better to
>>> change the driver?
> 
> There should be two logics in csum engine, which is  that either HW computes 
> TX checksum (using PKT_TX_IP_CKSUM) or SW compute TX checksum(use 
> PKT_TX_IPV4(or another flag) to tell driver no IP checksum offload 
> requirement ),

To me it's strange to have a flag saying "it's ipv4 and do the
checksum" and another one saying "it's ipv4 and don't do the checksum".

We can clearly see we have 2 different things to know about the
packet:

- is it ipv4?
- do we want the hw checksum?

I think that mapping the flags on these 2 questions is the most logical.

> I think we shouldn't use L3 flag to tell driver what HW need do for L4,  L3 
> and L4 flag should be separated .

What L4 operation are you talking about?

Regards,
Olivier

[dpdk-dev] [PATCH v3 4/4] testpmd:rework csum forward engine

[Thomas Monjalon]

2014-11-28 10:50, Olivier MATZ:
> Hi Jijiang,
> 
> On 11/27/2014 06:03 PM, Jijiang Liu wrote:
> > @@ -303,7 +305,7 @@ process_outer_cksums(void *outer_l3_hdr, uint16_t 
> > outer_ethertype,
> >   * TESTPMD_TX_OFFLOAD_* in ports[tx_port].tx_ol_flags. They control
> >   * wether a checksum must be calculated in software or in hardware. The
> >   * IP, UDP, TCP and SCTP flags always concern the inner layer.  The
> > - * VxLAN flag concerns the outer IP and UDP layer (if packet is
> > + * VxLAN flag concerns the outer IP(if packet is
> >   * recognized as a vxlan packet).
> >   */
> 
> Please reindent the comment properly, taking care of the space before
> the parenthesis.
> 
> Another comment that concerns the whole patchset. It's maybe a question
> for Thomas, but I think that having patches that don't break compilation
> between each other would help when bissecting.
> 
> In this case, I wonder if it's possible to split in something like:
> 
> - change PKT_TX_IPV4 and PKT_TX_IPV6 definition
> - add + rename flags (update them in mbuf + testpmd + i40)
> - rename inner_l{23}, l{23} in l{23}, outer_l{23} (update them in mbuf
>   + testpmd + i40)
> 
> I'm not sure it's feasible, I let Thomas give his mind about this.

Exact, compilation must not be broken between patches.
The proposal from Olivier seems OK.

-- 
Thomas

[dpdk-dev] [PATCH v2] enicpmd: replace the type u_int* with uint* to remove compilation errors on a few platforms

[Thomas Monjalon]

Hi Sujith,

Some tips when sending a v2:
- use --in-reply-to to keep all the versions in the same thread
- use --annotate to write a changelog

These guidelines are explained in this page:
http://dpdk.org/dev#send

Thanks
-- 
Thomas

[dpdk-dev] [PATCH v2] enicpmd: replace the type u_int* with uint* to remove compilation errors on a few platforms

[Sujith Sankar (ssujith)]

Sure Thomas.  Thanks !
Do you want me to send this V2 again with these?

-Sujith

On 28/11/14 3:47 pm, "Thomas Monjalon"  wrote:

>Hi Sujith,
>
>Some tips when sending a v2:
>- use --in-reply-to to keep all the versions in the same thread
>- use --annotate to write a changelog
>
>These guidelines are explained in this page:
>   http://dpdk.org/dev#send
>
>Thanks
>-- 
>Thomas

[dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and change three fields

[Ananyev, Konstantin]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Olivier MATZ
> Sent: Friday, November 28, 2014 9:37 AM
> To: Liu, Jijiang; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and 
> change three fields
> 
> Hi Jijiang,
> 
> On 11/27/2014 06:03 PM, Jijiang Liu wrote:
> >  /** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or 
> > TSO. */
> >  #define PKT_TX_IPV4  PKT_RX_IPV4_HDR
> >
> >  /** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or 
> > TSO. */
> >  #define PKT_TX_IPV6  PKT_RX_IPV6_HDR
> 
> The description still does not match what we discussed. Either we
> have PKT_TX_IPV4 meaning "packet is IPv4 without requiring IP cksum
> offload", or  "packet is IPv4". I prefer the second one, but whatever
> the choice is, the comments must be coherent.
> 
> > -#define PKT_TX_VLAN_PKT  (1ULL << 55) /**< TX packet is a 802.1q VLAN 
> > packet. */
> > +/** Outer IP cksum of TX pkt. computed by NIC for tunneling packet */
> > +#define PKT_TX_OUTER_IP_CKSUM   (1ULL << 58)
> > +
> > +/** Tell the NIC it's an outer IPv6 packet for tunneling packet.*/
> > +#define PKT_TX_OUTER_IPV6(1ULL << 59)
> 
> I think we should have the same flags with the same meanings for
> inner and outer:
> 
> - PKT_TX_IPV4, PKT_TX_IP_CKSUM, PKT_TX_IPV6
> - PKT_TX_OUTER_IPV4, PKT_TX_OUTER_IP_CKSUM, PKT_TX_OUTER_IPV6

Good point.
I still think: should we squeeze each of these triple in 2 bits?
Same as we doing for L4 checksum flags:

#define PKT_TX_IP_CKSUM   (1ULL << X)
#define PKT_TX_IPV6(2ULL << X)
#define PKT_TX_IPV4(3ULL << X)

Same of outer flags.
Of course it means that they need to be mutually exclusive.

> 
> In your patch there is no PKT_TX_OUTER_IPV4 flag.
> 
> Regards,
> Olivier

[dpdk-dev] [PATCH v2] enicpmd: replace the type u_int* with uint* to remove compilation errors on a few platforms

[Thomas Monjalon]

2014-11-28 10:21, Sujith Sankar:
> Sure Thomas.  Thanks !
> Do you want me to send this V2 again with these?

No, not needed. Just a reminder for next time in the hope you'll
send hundreds of good patches ;)

Note: avoid top-posting (there's also some guidelines for emails).

> On 28/11/14 3:47 pm, "Thomas Monjalon"  wrote:
> >Hi Sujith,
> >
> >Some tips when sending a v2:
> >- use --in-reply-to to keep all the versions in the same thread
> >- use --annotate to write a changelog
> >
> >These guidelines are explained in this page:
> > http://dpdk.org/dev#send
> >
> >Thanks

[dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and change three fields

[Liu, Jijiang]

Hi Olivier,

> -Original Message-
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> Sent: Friday, November 28, 2014 5:37 PM
> To: Liu, Jijiang; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and
> change three fields
> 
> Hi Jijiang,
> 
> On 11/27/2014 06:03 PM, Jijiang Liu wrote:
> >  /** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or 
> > TSO.
> */
> >  #define PKT_TX_IPV4  PKT_RX_IPV4_HDR
> >
> >  /** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or 
> > TSO.
> */
> >  #define PKT_TX_IPV6  PKT_RX_IPV6_HDR
> 
> The description still does not match what we discussed. Either we have
> PKT_TX_IPV4 meaning "packet is IPv4 without requiring IP cksum offload", or
> "packet is IPv4". I prefer the second one, but whatever the choice is, the
> comments must be coherent.
>
I agree.
 "packet is IPv4" is ok for me, too.
The comment "Required for L4 checksum offload or TSO" is not added by me,  I 
should have thought you added it during developing TSO.
Anyway, we came to an agreement for  PKT_TX_IPV6/4 meaning, I will change  the 
two flags comments.

[dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and change three fields

[Ananyev, Konstantin]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Liu, Jijiang
> Sent: Friday, November 28, 2014 10:33 AM
> To: Olivier MATZ; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and 
> change three fields
> 
> Hi Olivier,
> 
> > -Original Message-
> > From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> > Sent: Friday, November 28, 2014 5:37 PM
> > To: Liu, Jijiang; dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and
> > change three fields
> >
> > Hi Jijiang,
> >
> > On 11/27/2014 06:03 PM, Jijiang Liu wrote:
> > >  /** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload 
> > > or TSO.
> > */
> > >  #define PKT_TX_IPV4  PKT_RX_IPV4_HDR
> > >
> > >  /** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload 
> > > or TSO.
> > */
> > >  #define PKT_TX_IPV6  PKT_RX_IPV6_HDR
> >
> > The description still does not match what we discussed. Either we have
> > PKT_TX_IPV4 meaning "packet is IPv4 without requiring IP cksum offload", or
> > "packet is IPv4". I prefer the second one, but whatever the choice is, the
> > comments must be coherent.
> >
> I agree.
>  "packet is IPv4" is ok for me, too.
> The comment "Required for L4 checksum offload or TSO" is not added by me,  I 
> should have thought you added it during developing
> TSO.
> Anyway, we came to an agreement for  PKT_TX_IPV6/4 meaning, I will change  
> the two flags comments.
> 
> 

Well, I still prefer them to be mutually exclusive.
Even better, if we can squeeze these 3 flags into 2 bits.
Would save us 2 bits, plus might be handy, as in the PMD you can do:

switch (ol_flags & TX_L3_MASK) {
case TX_IPV4:
   ...
   break;
case TX_IPV6:
   ...
   break;
case TX_IP_CKSUM:
   ...
   break;
}

For the upper layer, I think there would be no big difference, what ways we 
will choose.

Konstantin

[dpdk-dev] [PATCH 1/3] mbuf:add two TX offload flags and change three fields

[Olivier MATZ]

Hi Konstantin,

On 11/27/2014 06:01 PM, Ananyev, Konstantin wrote:
>> Yes, I think it could be done that way too.
>> Though I still prefer to keep l4tun_len - it makes things a bit cleaner (at 
>> least to me).
>> After all  we do have space for it in mbuf's tx_offload.
> 
> As one more thing in favour of separate l4tun_len field:
> l2_len is 7 bit long, so in theory it might be not enough, as for FVL:
> 12:18 L4TUNLEN L4 Tunneling Length (Teredo / GRE header / VXLAN header) 
> defined in Words. 

The l2_len field is 7 bits long because it was mapped to ixgbe hardware.
If it's not enough (although I'm not sure it's possible to have a header
larger than 128 bytes in this case), it's probably because we should
not have been doing that.
Maybe these generic fields should be generic :)
If it's not enough, what about changing l2_len to 8 bits?

Often in the recent discussions, I see as an argument "fortville needs
this so we need to add it in the mbuf". I agree that currently
fortville is the only hardware supported for the new features, so it
can make sense to act like this, but we have to accept to come back
to this API in the future if it makes less sense with other drivers.

Also, application developers can be annoyed to see that the mbuf flags
and meta data are just duplicating information that is already present
in the packet.

About the l4tun_len, it's another field the application has to fill,
but it's maybe cleaner. I just wanted to clarify why I'm discussing
these points.

Regards,
Olivier

[dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and change three fields

[Olivier MATZ]

Hi Konstantin,

On 11/28/2014 11:40 AM, Ananyev, Konstantin wrote:
> 
> Well, I still prefer them to be mutually exclusive.
> Even better, if we can squeeze these 3 flags into 2 bits.
> Would save us 2 bits, plus might be handy, as in the PMD you can do:
> 
> switch (ol_flags & TX_L3_MASK) {
> case TX_IPV4:
>...
>break;
> case TX_IPV6:
>...
>break;
> case TX_IP_CKSUM:
>...
>break;
> }
> 
> For the upper layer, I think there would be no big difference, what ways we 
> will choose.

I think the 2 informations are transversal, and that's why I would
prefer 2 flags. Also, having 2 separate flags would also help to keep
backward compatibility with previous versions.

It may help to have other points of view to make the good decision.
I'll follow the majority.

Regards,
Olivier

[dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and change three fields

[Ananyev, Konstantin]

> -Original Message-
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> Sent: Friday, November 28, 2014 11:00 AM
> To: Ananyev, Konstantin; Liu, Jijiang; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and 
> change three fields
> 
> Hi Konstantin,
> 
> On 11/28/2014 11:40 AM, Ananyev, Konstantin wrote:
> >
> > Well, I still prefer them to be mutually exclusive.
> > Even better, if we can squeeze these 3 flags into 2 bits.
> > Would save us 2 bits, plus might be handy, as in the PMD you can do:
> >
> > switch (ol_flags & TX_L3_MASK) {
> > case TX_IPV4:
> >...
> >break;
> > case TX_IPV6:
> >...
> >break;
> > case TX_IP_CKSUM:
> >...
> >break;
> > }
> >
> > For the upper layer, I think there would be no big difference, what ways we 
> > will choose.
> 
> I think the 2 informations are transversal, and that's why I would
> prefer 2 flags. Also, having 2 separate flags would also help to keep
> backward compatibility with previous versions.

Hmm, not sure how we will break compatibility in that case?
If we'll make TX_IP_CKSUM to be 1 bit value (1 << X) then for current drivers 
nothing should change, no?

> 
> It may help to have other points of view to make the good decision.
> I'll follow the majority.
> 
> Regards,
> Olivier

[dpdk-dev] [PATCH 1/3] mbuf:add two TX offload flags and change three fields

[Ananyev, Konstantin]

Hi Olver,

> -Original Message-
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> Sent: Friday, November 28, 2014 10:46 AM
> To: Ananyev, Konstantin; Liu, Jijiang
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/3] mbuf:add two TX offload flags and change 
> three fields
> 
> Hi Konstantin,
> 
> On 11/27/2014 06:01 PM, Ananyev, Konstantin wrote:
> >> Yes, I think it could be done that way too.
> >> Though I still prefer to keep l4tun_len - it makes things a bit cleaner 
> >> (at least to me).
> >> After all  we do have space for it in mbuf's tx_offload.
> >
> > As one more thing in favour of separate l4tun_len field:
> > l2_len is 7 bit long, so in theory it might be not enough, as for FVL:
> > 12:18 L4TUNLEN L4 Tunneling Length (Teredo / GRE header / VXLAN header) 
> > defined in Words.
> 
> The l2_len field is 7 bits long because it was mapped to ixgbe hardware.

Yes.

> If it's not enough (although I'm not sure it's possible to have a header
> larger than 128 bytes in this case), it's probably because we should
> not have been doing that.

I also can't imagine the L2 header being that long.
>From other side - I just don't like an idea of PMD stripping off HW 
>capabilities.

> Maybe these generic fields should be generic :)
> If it's not enough, what about changing l2_len to 8 bits?

Yes, was thinking about the same thing.
Maybe instead of introducing l4tun_len, we should increase l2_len and 
outer_l2_len sizes to 8bits each? 
Though that would break the pair:
l2_len : 7
l3_len :9 
Which is quite useful, as it fits into 2B, and maps exactly to ixgbe TCD layout.
Wonder if we change it, would be there any performance penalty for ixgbe, and 
if yes how big.

> 
> Often in the recent discussions, I see as an argument "fortville needs
> this so we need to add it in the mbuf". I agree that currently
> fortville is the only hardware supported for the new features, so it
> can make sense to act like this, but we have to accept to come back
> to this API in the future if it makes less sense with other drivers.

Yes, it is sometime hard to keep a balance.
>From one side people would like to have a  clean and generic API, from other 
>side
people would like to have ab ability to use all features that HW supports. 

> 
> Also, application developers can be annoyed to see that the mbuf flags
> and meta data are just duplicating information that is already present
> in the packet.
> 
> About the l4tun_len, it's another field the application has to fill,
> but it's maybe cleaner. I just wanted to clarify why I'm discussing
> these points.
> 
> Regards,
> Olivier

[dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and change three fields

[Olivier MATZ]

Hi Konstantin,

On 11/28/2014 12:13 PM, Ananyev, Konstantin wrote:
>>> For the upper layer, I think there would be no big difference, what ways we 
>>> will choose.
>>
>> I think the 2 informations are transversal, and that's why I would
>> prefer 2 flags. Also, having 2 separate flags would also help to keep
>> backward compatibility with previous versions.
> 
> Hmm, not sure how we will break compatibility in that case?
> If we'll make TX_IP_CKSUM to be 1 bit value (1 << X) then for current drivers 
> nothing should change, no?

Yes, you're right, sorry.

Olivier

[dpdk-dev] [PATCH] scripts: fix symbol overriding in configuration files

[Bruce Richardson]

On Thu, Nov 27, 2014 at 12:29:05PM +0100, David Marchand wrote:
> When redefining the same symbol in configuration (basically after an 
> inclusion),
> we need to undefine the previous symbol to avoid "redefined" errors.
> 
> Signed-off-by: David Marchand 

Though I see this patch is already replied, my comment on the below is that
it may be best as two separate patches, since you are doing two things there
that makes the actual change hard to see. One patch should move the "|" from the
start of the next line to end of the previous, and the second patch should then
add the undef statements. As it is, I had to stare at this for a while to work
out why the grep lines were changing to undefine previously defined values. :-)

/Bruce

> ---
>  scripts/gen-config-h.sh |   12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/scripts/gen-config-h.sh b/scripts/gen-config-h.sh
> index efd7667..2fac08c 100755
> --- a/scripts/gen-config-h.sh
> +++ b/scripts/gen-config-h.sh
> @@ -33,11 +33,11 @@
>  
>  echo "#ifndef __RTE_CONFIG_H"
>  echo "#define __RTE_CONFIG_H"
> -grep CONFIG_ $1   \
> -| grep -v '^[ \t]*#'  \
> -| sed 's,CONFIG_\(.*\)=y.*$,#define \1 1,'\
> -| sed 's,CONFIG_\(.*\)=n.*$,#undef \1,'   \
> -| sed 's,CONFIG_\(.*\)=\(.*\)$,#define \1 \2,'\
> -| sed 's,\# CONFIG_\(.*\) is not set$,#undef \1,'
> +grep CONFIG_ $1 |
> +grep -v '^[ \t]*#' |
> +sed 's,CONFIG_\(.*\)=y.*$,#undef \1\n#define \1 1,' |
> +sed 's,CONFIG_\(.*\)=n.*$,#undef \1,' |
> +sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\n#define \1 \2,' |
> +sed 's,\# CONFIG_\(.*\) is not set$,#undef \1,'
>  echo "#endif /* __RTE_CONFIG_H */"
>  
> -- 
> 1.7.10.4
> 

[dpdk-dev] [PATCH v7 0/4] Support configuring hash functions

[Helin Zhang]

These patches mainly support configuring hash functions. In detail,
 - It can get/set global hash configurations.
  * Get/set symmetric hash enable per flow type.
  * Get/set hash function type.
 - It can get/set symmetric hash enable per port.
 - Four commands have been implemented in testpmd to support
   testing above.
   * get_sym_hash_ena_per_port
   * set_sym_hash_ena_per_port
   * get_hash_global_config
   * set_hash_global_config

It also uses constant hash keys to replace runtime generating hash keys. Global 
initialization is added to correctly put registers to an initial state.

v3 changes:
* Removed renamings in rte_ethdev.h.
* Redesigned filter control API and its relevant structures/enums.
* Renamed header file from rte_eth_features.h to rte_eth_ctrol.h.
* Remove public header file of rte_i40e.h specific for i40e.
* Added hardware initialization function during port init.
* Used constant random hash keys in i40e PF.
* renamed the commands in testpmd based on the redesigned filter
  control API.

v4 changes:
* Fixed a bug in testpmd to support 'set_sym_hash_ena_per_port'.

v5 changes:
* Integrated with filter API defined recently.
* Remove all for filter API definition, as it has already defined
  and merged recently.

v6 changes:
* Flow type strings are used to replace Packet Classification
  Types, to isolate hardware specific things.
* Implemented the mapping function to convert RSS offload types to
  Packet Classification Types, to isolate the real hardware
  specific things.
* Removed initialization of global registers in i40e PMD, as global
  registers shouldn't be initialized per port.
* Added more annotations to get code more understandable.
* Corrected annotation format for documenation.

v7 changes:
* Remove swap configurations, as it is not allowed by hardware design.
* Put symmetric hash per flow type and hash function type into
  'RTE_ETH_HASH_FILTER_GLOBAL_CONFIG', as they are controlling global
  registers which will affects all the ports of the same NIC.

Helin Zhang (4):
  ethdev: code style fixes
  i40e: use constant as the default hash keys
  i40e: support of controlling hash functions
  app/testpmd: app/testpmd: add commands to support hash functions

 app/test-pmd/cmdline.c| 334 ++
 lib/librte_ether/rte_eth_ctrl.h   |  72 +++-
 lib/librte_pmd_i40e/i40e_ethdev.c | 312 ++-
 3 files changed, 704 insertions(+), 14 deletions(-)

-- 
1.8.1.4

[dpdk-dev] [PATCH v7 1/4] ethdev: code style fixes

[Helin Zhang]

Added code style fixes.

Signed-off-by: Helin Zhang 
---
 lib/librte_ether/rte_eth_ctrl.h | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
index 7088d8d..6ab16c2 100644
--- a/lib/librte_ether/rte_eth_ctrl.h
+++ b/lib/librte_ether/rte_eth_ctrl.h
@@ -62,8 +62,8 @@ enum rte_filter_type {
  * Generic operations on filters
  */
 enum rte_filter_op {
+   /** used to check whether the type filter is supported */
RTE_ETH_FILTER_NOP = 0,
-   /**< used to check whether the type filter is supported */
RTE_ETH_FILTER_ADD,  /**< add filter entry */
RTE_ETH_FILTER_UPDATE,   /**< update filter entry */
RTE_ETH_FILTER_DELETE,   /**< delete filter entry */
@@ -75,16 +75,15 @@ enum rte_filter_op {
RTE_ETH_FILTER_OP_MAX
 };

-/**
+/*
  * MAC filter type
  */
 enum rte_mac_filter_type {
RTE_MAC_PERFECT_MATCH = 1, /**< exact match of MAC addr. */
-   RTE_MACVLAN_PERFECT_MATCH,
-   /**< exact match of MAC addr and VLAN ID. */
+   RTE_MACVLAN_PERFECT_MATCH, /**< exact match of MAC addr and VLAN ID. */
RTE_MAC_HASH_MATCH, /**< hash match of MAC addr. */
+   /** hash match of MAC addr and exact match of VLAN ID. */
RTE_MACVLAN_HASH_MATCH,
-   /**< hash match of MAC addr and exact match of VLAN ID. */
 };

 /**
-- 
1.8.1.4

[dpdk-dev] [PATCH v7 4/4] app/testpmd: app/testpmd: add commands to support hash functions

[Helin Zhang]

To demonstrate the hash filter control, commands are added.
They are,
- get_sym_hash_ena_per_port
- set_sym_hash_ena_per_port
- get_hash_global_config
- set_hash_global_config

Signed-off-by: Helin Zhang 
---
 app/test-pmd/cmdline.c | 334 +
 1 file changed, 334 insertions(+)

v6 changes:
* Flow type strings are used to replace Packet Classification
  Types, to isolate hardware specific things.

v7 changes:
* Removed commands of,
  get_sym_hash_ena_per_pctype
  set_sym_hash_ena_per_pctype
  get_filter_swap
  set_filter_swap
  get_hash_function
  set_hash_function.
* Added new commands of,
  get_hash_global_config
  set_hash_global_config

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index c61c3a0..9027f47 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -75,6 +75,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -740,6 +741,21 @@ static void cmd_help_long_parsed(void *parsed_result,
"flow_director_flex_payload (port_id)"
" (l2|l3|l4) (config)\n"
"Configure flex payload selection.\n\n"
+
+   "get_sym_hash_ena_per_port (port_id)\n"
+   "get symmetric hash enable configuration per 
port.\n\n"
+
+   "set_sym_hash_ena_per_port (port_id) (enable|disable)\n"
+   "set symmetric hash enable configuration per port"
+   " to enable or disable.\n\n"
+
+   "get_hash_global_config (port_id)\n"
+   "Get the global configurations of hash filters.\n\n"
+
+   "set_hash_global_config (port_id) 
(toeplitz|simple_xor|default)"
+   " 
(ip4|ip4-frag|tcp4|udp4|#sctp4|ip6|ip6-frag|tcp6|udp6|sctp6)"
+   " (enable|disable)\n"
+   "Set the global configurations of hash filters.\n\n"
);
}
 }
@@ -8697,6 +8713,320 @@ cmdline_parse_inst_t cmd_set_flow_director_flex_payload 
= {
},
 };

+/* *** Classification Filters Control *** */
+/* *** Get symmetric hash enable per port *** */
+struct cmd_get_sym_hash_ena_per_port_result {
+   cmdline_fixed_string_t get_sym_hash_ena_per_port;
+   uint8_t port_id;
+};
+
+static void
+cmd_get_sym_hash_per_port_parsed(void *parsed_result,
+__rte_unused struct cmdline *cl,
+__rte_unused void *data)
+{
+   struct cmd_get_sym_hash_ena_per_port_result *res = parsed_result;
+   struct rte_eth_hash_filter_info info;
+   int ret;
+
+   if (rte_eth_dev_filter_supported(res->port_id,
+   RTE_ETH_FILTER_HASH) < 0) {
+   printf("RTE_ETH_FILTER_HASH not supported on port: %d\n",
+   res->port_id);
+   return;
+   }
+
+   memset(&info, 0, sizeof(info));
+   info.info_type = RTE_ETH_HASH_FILTER_SYM_HASH_ENA_PER_PORT;
+   ret = rte_eth_dev_filter_ctrl(res->port_id, RTE_ETH_FILTER_HASH,
+   RTE_ETH_FILTER_GET, &info);
+
+   if (ret < 0) {
+   printf("Cannot get symmetric hash enable per port "
+   "on port %u\n", res->port_id);
+   return;
+   }
+
+   printf("Symmetric hash is %s on port %u\n", info.info.enable ?
+   "enabled" : "disabled", res->port_id);
+}
+
+cmdline_parse_token_string_t cmd_get_sym_hash_ena_per_port_all =
+   TOKEN_STRING_INITIALIZER(struct cmd_get_sym_hash_ena_per_port_result,
+   get_sym_hash_ena_per_port, "get_sym_hash_ena_per_port");
+cmdline_parse_token_num_t cmd_get_sym_hash_ena_per_port_port_id =
+   TOKEN_NUM_INITIALIZER(struct cmd_get_sym_hash_ena_per_port_result,
+   port_id, UINT8);
+
+cmdline_parse_inst_t cmd_get_sym_hash_ena_per_port = {
+   .f = cmd_get_sym_hash_per_port_parsed,
+   .data = NULL,
+   .help_str = "get_sym_hash_ena_per_port port_id",
+   .tokens = {
+   (void *)&cmd_get_sym_hash_ena_per_port_all,
+   (void *)&cmd_get_sym_hash_ena_per_port_port_id,
+   NULL,
+   },
+};
+
+/* *** Set symmetric hash enable per port *** */
+struct cmd_set_sym_hash_ena_per_port_result {
+   cmdline_fixed_string_t set_sym_hash_ena_per_port;
+   cmdline_fixed_string_t enable;
+   uint8_t port_id;
+};
+
+static void
+cmd_set_sym_hash_per_port_parsed(void *parsed_result,
+__rte_unused struct cmdline *cl,
+__rte_unused void *data)
+{
+   struct cmd_set_sym_hash_ena_per_port_result *res = parsed_result;
+   struct rte_eth_hash_filter_info info;
+   int ret;
+
+   if (rte_eth_dev_filter_supported(res->port_id,
+ 

[dpdk-dev] [PATCH v7 2/4] i40e: use constant as the default hash keys

[Helin Zhang]

Calculating the default RSS hash keys at run time is not needed
at all, and may have race conditions. The alternative is to use
array of random values which were generated manually as the
default hash keys.

Signed-off-by: Helin Zhang 
---
 lib/librte_pmd_i40e/i40e_ethdev.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c 
b/lib/librte_pmd_i40e/i40e_ethdev.c
index 87e750a..8fbe25f 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -73,7 +73,7 @@
 /* Maximun number of VSI */
 #define I40E_MAX_NUM_VSIS  (384UL)

-/* Default queue interrupt throttling time in microseconds*/
+/* Default queue interrupt throttling time in microseconds */
 #define I40E_ITR_INDEX_DEFAULT  0
 #define I40E_QUEUE_ITR_INTERVAL_DEFAULT 32 /* 32 us */
 #define I40E_QUEUE_ITR_INTERVAL_MAX 8160 /* 8160 us */
@@ -199,9 +199,6 @@ static int i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
enum rte_filter_op filter_op,
void *arg);

-/* Default hash key buffer for RSS */
-static uint32_t rss_key_default[I40E_PFQF_HKEY_MAX_INDEX + 1];
-
 static struct rte_pci_id pci_id_i40e_map[] = {
 #define RTE_PCI_DEV_ID_DECL_I40E(vend, dev) {RTE_PCI_DEVICE(vend, dev)},
 #include "rte_pci_dev_ids.h"
@@ -5023,9 +5020,12 @@ i40e_pf_config_rss(struct i40e_pf *pf)
}
if (rss_conf.rss_key == NULL || rss_conf.rss_key_len <
(I40E_PFQF_HKEY_MAX_INDEX + 1) * sizeof(uint32_t)) {
-   /* Calculate the default hash key */
-   for (i = 0; i <= I40E_PFQF_HKEY_MAX_INDEX; i++)
-   rss_key_default[i] = (uint32_t)rte_rand();
+   /* Random default keys */
+   static uint32_t rss_key_default[] = {0x6b793944,
+   0x23504cb5, 0x5bea75b6, 0x309f4f12, 0x3dc0a2b8,
+   0x024ddcdf, 0x339b8ca0, 0x4c4af64a, 0x34fac605,
+   0x55d85839, 0x3a58997d, 0x2ec938e1, 0x66031581};
+
rss_conf.rss_key = (uint8_t *)rss_key_default;
rss_conf.rss_key_len = (I40E_PFQF_HKEY_MAX_INDEX + 1) *
sizeof(uint32_t);
-- 
1.8.1.4

[dpdk-dev] [PATCH v7 3/4] i40e: support of controlling hash functions

[Helin Zhang]

Hash filter control has been implemented for i40e. It includes
getting/setting,
- global hash configurations (hash function type, and symmetric
  hash enable per flow type)
- symmetric hash enable per port

Signed-off-by: Helin Zhang 
---
 lib/librte_ether/rte_eth_ctrl.h   |  63 
 lib/librte_pmd_i40e/i40e_ethdev.c | 298 +-
 2 files changed, 359 insertions(+), 2 deletions(-)

v5 changes:
* Integrated with filter API defined recently.

v6 changes:
* Implemented the mapping function to convert RSS offload types to
  Packet Classification Types, to isolate the real hardware
  specific things.
* Removed initialization of global registers in i40e PMD, as global
  registers shouldn't be initialized per port.
* Added more annotations to get code more understandable.
* Corrected annotation format for documenation.

v7 changes:
* Remove swap configurations, as it is not allowed by hardware design.
* Put symmetric hash per flow type and hash function type into
  'RTE_ETH_HASH_FILTER_GLOBAL_CONFIG', as they are controlling global
  registers which will affects all the ports of the same NIC.

diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
index 6ab16c2..827d7ba 100644
--- a/lib/librte_ether/rte_eth_ctrl.h
+++ b/lib/librte_ether/rte_eth_ctrl.h
@@ -55,6 +55,7 @@ enum rte_filter_type {
RTE_ETH_FILTER_ETHERTYPE,
RTE_ETH_FILTER_TUNNEL,
RTE_ETH_FILTER_FDIR,
+   RTE_ETH_FILTER_HASH,
RTE_ETH_FILTER_MAX
 };

@@ -449,6 +450,68 @@ struct rte_eth_fdir_stats {
uint32_t best_cnt; /**< Number of filters in best effort spaces. */
 };

+/**
+ * Hash filter information types.
+ * - RTE_ETH_HASH_FILTER_SYM_HASH_ENA_PER_PORT is for getting/setting the
+ *   information/configuration of 'symmetric hash enable' per port.
+ * - RTE_ETH_HASH_FILTER_GLOBAL_CONFIG is for getting/setting the global
+ *   configurations of hash filters. Those global configurations are valid
+ *   for all ports of the same NIC.
+ */
+enum rte_eth_hash_filter_info_type {
+   RTE_ETH_HASH_FILTER_INFO_TYPE_UNKNOWN = 0,
+   /** Symmetric hash enable per port */
+   RTE_ETH_HASH_FILTER_SYM_HASH_ENA_PER_PORT,
+   /** Configure globally for hash filter */
+   RTE_ETH_HASH_FILTER_GLOBAL_CONFIG,
+   RTE_ETH_HASH_FILTER_INFO_TYPE_MAX,
+};
+
+/**
+ * Hash function types.
+ */
+enum rte_eth_hash_function {
+   RTE_ETH_HASH_FUNCTION_DEFAULT = 0,
+   RTE_ETH_HASH_FUNCTION_TOEPLITZ, /**< Toeplitz */
+   RTE_ETH_HASH_FUNCTION_SIMPLE_XOR, /**< Simple XOR */
+   RTE_ETH_HASH_FUNCTION_MAX,
+};
+
+#define UINT32_BIT (CHAR_BIT * sizeof(uint32_t))
+#define RTE_SYM_HASH_MASK_ARRAY_SIZE \
+   (RTE_ALIGN(RTE_ETH_FLOW_TYPE_MAX, UINT32_BIT)/UINT32_BIT)
+/**
+ * A structure used to set or get global hash function configurations which
+ * include symmetric hash enable per flow type and hash function type.
+ * Each bit in sym_hash_enable_mask[] indicates if the symmetric hash of the
+ * coresponding flow type is enabled or not.
+ * Each bit in valid_bit_mask[] indicates if the coresponding bit in
+ * sym_hash_enable_mask[] is valid or not. For the configurations gotten, it
+ * also means if the flow type is supported by hardware or not.
+ */
+struct rte_eth_hash_global_conf {
+   enum rte_eth_hash_function hash_func; /**< Hash function type */
+   /** Bit mask for symmetric hash enable per flow type */
+   uint32_t sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
+   /** Bit mask indicates if the coresponding bit is valid */
+   uint32_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
+};
+
+/**
+ * A structure used to set or get hash filter information, to support filter
+ * type of 'RTE_ETH_FILTER_HASH' and its operations.
+ */
+struct rte_eth_hash_filter_info {
+   enum rte_eth_hash_filter_info_type info_type; /**< Information type. */
+   /** Details of hash filter infomation */
+   union {
+   /* For RTE_ETH_HASH_FILTER_SYM_HASH_ENA_PER_PORT */
+   uint8_t enable;
+   /* Global configurations of hash filter */
+   struct rte_eth_hash_global_conf global_conf;
+   } info;
+};
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c 
b/lib/librte_pmd_i40e/i40e_ethdev.c
index 8fbe25f..ef8edd4 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -93,6 +93,18 @@
I40E_PFINT_ICR0_ENA_VFLR_MASK | \
I40E_PFINT_ICR0_ENA_ADMINQ_MASK)

+#define I40E_FLOW_TYPES ( \
+   (1UL << RTE_ETH_FLOW_TYPE_UDPV4) | \
+   (1UL << RTE_ETH_FLOW_TYPE_TCPV4) | \
+   (1UL << RTE_ETH_FLOW_TYPE_SCTPV4) | \
+   (1UL << RTE_ETH_FLOW_TYPE_IPV4_OTHER) | \
+   (1UL << RTE_ETH_FLOW_TYPE_FRAG_IPV4) | \
+   (1UL << RTE_ETH_FLOW_TYPE_UDPV6) | \
+   (1UL << RTE_ETH_FLOW_TYPE_TCPV6) | \
+   (1UL << RTE_ETH_FLOW_TYPE_SCTPV6) | \
+   (1UL << RTE_ETH_FLOW_TYPE_I

[dpdk-dev] [PATCH v7 3/4] i40e: support of controlling hash functions

[Ananyev, Konstantin]

Hi Helin,
Few nits from me.
Konstantin


> -Original Message-
> From: Zhang, Helin
> Sent: Friday, November 28, 2014 12:14 PM
> To: dev at dpdk.org
> Cc: Cao, Waterman; Cao, Min; Ananyev, Konstantin; Zhang, Helin
> Subject: [PATCH v7 3/4] i40e: support of controlling hash functions
> 
> Hash filter control has been implemented for i40e. It includes
> getting/setting,
> - global hash configurations (hash function type, and symmetric
>   hash enable per flow type)
> - symmetric hash enable per port
> 
> Signed-off-by: Helin Zhang 
> ---
>  lib/librte_ether/rte_eth_ctrl.h   |  63 
>  lib/librte_pmd_i40e/i40e_ethdev.c | 298 
> +-
>  2 files changed, 359 insertions(+), 2 deletions(-)
> 
> v5 changes:
> * Integrated with filter API defined recently.
> 
> v6 changes:
> * Implemented the mapping function to convert RSS offload types to
>   Packet Classification Types, to isolate the real hardware
>   specific things.
> * Removed initialization of global registers in i40e PMD, as global
>   registers shouldn't be initialized per port.
> * Added more annotations to get code more understandable.
> * Corrected annotation format for documenation.
> 
> v7 changes:
> * Remove swap configurations, as it is not allowed by hardware design.
> * Put symmetric hash per flow type and hash function type into
>   'RTE_ETH_HASH_FILTER_GLOBAL_CONFIG', as they are controlling global
>   registers which will affects all the ports of the same NIC.
> 
> diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
> index 6ab16c2..827d7ba 100644
> --- a/lib/librte_ether/rte_eth_ctrl.h
> +++ b/lib/librte_ether/rte_eth_ctrl.h
> @@ -55,6 +55,7 @@ enum rte_filter_type {
>   RTE_ETH_FILTER_ETHERTYPE,
>   RTE_ETH_FILTER_TUNNEL,
>   RTE_ETH_FILTER_FDIR,
> + RTE_ETH_FILTER_HASH,
>   RTE_ETH_FILTER_MAX
>  };
> 
> @@ -449,6 +450,68 @@ struct rte_eth_fdir_stats {
>   uint32_t best_cnt; /**< Number of filters in best effort spaces. */
>  };
> 
> +/**
> + * Hash filter information types.
> + * - RTE_ETH_HASH_FILTER_SYM_HASH_ENA_PER_PORT is for getting/setting the
> + *   information/configuration of 'symmetric hash enable' per port.
> + * - RTE_ETH_HASH_FILTER_GLOBAL_CONFIG is for getting/setting the global
> + *   configurations of hash filters. Those global configurations are valid
> + *   for all ports of the same NIC.
> + */
> +enum rte_eth_hash_filter_info_type {
> + RTE_ETH_HASH_FILTER_INFO_TYPE_UNKNOWN = 0,
> + /** Symmetric hash enable per port */
> + RTE_ETH_HASH_FILTER_SYM_HASH_ENA_PER_PORT,
> + /** Configure globally for hash filter */
> + RTE_ETH_HASH_FILTER_GLOBAL_CONFIG,
> + RTE_ETH_HASH_FILTER_INFO_TYPE_MAX,
> +};
> +
> +/**
> + * Hash function types.
> + */
> +enum rte_eth_hash_function {
> + RTE_ETH_HASH_FUNCTION_DEFAULT = 0,
> + RTE_ETH_HASH_FUNCTION_TOEPLITZ, /**< Toeplitz */
> + RTE_ETH_HASH_FUNCTION_SIMPLE_XOR, /**< Simple XOR */
> + RTE_ETH_HASH_FUNCTION_MAX,
> +};
> +
> +#define UINT32_BIT (CHAR_BIT * sizeof(uint32_t))
> +#define RTE_SYM_HASH_MASK_ARRAY_SIZE \
> + (RTE_ALIGN(RTE_ETH_FLOW_TYPE_MAX, UINT32_BIT)/UINT32_BIT)
> +/**
> + * A structure used to set or get global hash function configurations which
> + * include symmetric hash enable per flow type and hash function type.
> + * Each bit in sym_hash_enable_mask[] indicates if the symmetric hash of the
> + * coresponding flow type is enabled or not.
> + * Each bit in valid_bit_mask[] indicates if the coresponding bit in
> + * sym_hash_enable_mask[] is valid or not. For the configurations gotten, it
> + * also means if the flow type is supported by hardware or not.
> + */
> +struct rte_eth_hash_global_conf {
> + enum rte_eth_hash_function hash_func; /**< Hash function type */
> + /** Bit mask for symmetric hash enable per flow type */
> + uint32_t sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> + /** Bit mask indicates if the coresponding bit is valid */
> + uint32_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE];
> +};
> +
> +/**
> + * A structure used to set or get hash filter information, to support filter
> + * type of 'RTE_ETH_FILTER_HASH' and its operations.
> + */
> +struct rte_eth_hash_filter_info {
> + enum rte_eth_hash_filter_info_type info_type; /**< Information type. */
> + /** Details of hash filter infomation */
> + union {
> + /* For RTE_ETH_HASH_FILTER_SYM_HASH_ENA_PER_PORT */
> + uint8_t enable;
> + /* Global configurations of hash filter */
> + struct rte_eth_hash_global_conf global_conf;
> + } info;
> +};
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c 
> b/lib/librte_pmd_i40e/i40e_ethdev.c
> index 8fbe25f..ef8edd4 100644
> --- a/lib/librte_pmd_i40e/i40e_ethdev.c
> +++ b/lib/librte_pmd_i40e/i40e_ethdev.c
> @@ -93,6 +93,18 @@
>   I40E_PFINT_ICR0_ENA_VFLR_MASK | \
> 

[dpdk-dev] [PATCH] ixgbe: Add missing rx_mbuf_alloc_failed statistics for vector PMD

[Bruce Richardson]

On Fri, Nov 28, 2014 at 09:21:45AM +, Balazs Nemeth wrote:
> The statistics that is reported through the rx_nombuf fields in struct
> rte_eth_stats was not set when the vector PMD was used. The statistics
> should report the number of mbufs that could _not_ be allocated during
> rearm of the RX queue. The non-vector PMD reports it correctly. The
> use of either vector PMD or non-vector PMD depends on runtime
> configuration. Hence it is possible that a change in configuration
> would disable this statistics. To prevent this from happening, the
> statistics should be reported by both implementations.
> 
> Signed-off-by: Balazs Nemeth 

Acked-by: Bruce Richardson 

> ---
>  lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c 
> b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
> index 42c0f60..579bc46 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
> @@ -71,6 +71,8 @@ ixgbe_rxq_rearm(struct igb_rx_queue *rxq)
>   dma_addr0);
>   }
>   }
> + rte_eth_devices[rxq->port_id].data->rx_mbuf_alloc_failed +=
> + RTE_IXGBE_RXQ_REARM_THRESH;
>   return;
>   }
>  
> -- 
> 2.1.2
> 

[dpdk-dev] [PATCH v2] enicpmd: replace the type u_int* with uint* to remove compilation errors on a few platforms

[Bruce Richardson]

On Fri, Nov 28, 2014 at 03:08:19PM +0530, Sujith Sankar wrote:
> ENIC PMD was giving compilation errors on ppc_64-power8-linuxapp-gcc because
> of types such as u_int32_t.  This patch replaces all those with uint32_t and
> similar ones.
> 
> Reported-by: David Marchand 
> Signed-off-by: Sujith Sankar 
> ---

Thanks for the patch to change these. Can I also suggest that you try 
compilation
with the clang compiler too, as I get a number of errors reported by that 
compiler
when I try compiling up the librte_pmd_enic folder.

Regards,
/Bruce

[dpdk-dev] [PATCH] scripts: fix symbol overriding in configuration files

[Bruce Richardson]

On Thu, Nov 27, 2014 at 10:17:22AM -0800, Thomas Monjalon wrote:
> > When redefining the same symbol in configuration (basically after an 
> > inclusion),
> > we need to undefine the previous symbol to avoid "redefined" errors.
> > 
> > Signed-off-by: David Marchand 
> 
> Acked-by: Thomas Monjalon 
> 
> Applied
>
This patch doesn't seem to work on FreeBSD. I'm still investigating how to fix
it but right now compilation with gcc/clang on FreeBSD is broken due to the
config.h file having lines like the below in it :-(

/usr/home/bruce/dpdk.org/x86_64-native-bsdapp-clang/include/rte_config.h:3:21: 
fatal error: extra tokens at end of #undef directive [-Wextra-tokens]
#undef RTE_EXEC_ENVn#define RTE_EXEC_ENV "bsdapp"

I'll send on a fix as soon as I can.

Regards,
/Bruce

[dpdk-dev] [PATCH] scripts: fix symbol overriding in configuration files

[David Marchand]

Hello Bruce,

On Fri, Nov 28, 2014 at 2:56 PM, Bruce Richardson <
bruce.richardson at intel.com> wrote:

> On Thu, Nov 27, 2014 at 10:17:22AM -0800, Thomas Monjalon wrote:
> > > When redefining the same symbol in configuration (basically after an
> inclusion),
> > > we need to undefine the previous symbol to avoid "redefined" errors.
> > >
> > > Signed-off-by: David Marchand 
> >
> > Acked-by: Thomas Monjalon 
> >
> > Applied
> >
> This patch doesn't seem to work on FreeBSD. I'm still investigating how to
> fix
> it but right now compilation with gcc/clang on FreeBSD is broken due to the
> config.h file having lines like the below in it :-(
>
> /usr/home/bruce/
> dpdk.org/x86_64-native-bsdapp-clang/include/rte_config.h:3:21: fatal
> error: extra tokens at end of #undef directive [-Wextra-tokens]
> #undef RTE_EXEC_ENVn#define RTE_EXEC_ENV "bsdapp"
>

This is probably because of the \n in the sed.

Can you try something like this (I did not like this version of my patch at
first because it is not that readable) ?


diff --git a/scripts/gen-config-h.sh b/scripts/gen-config-h.sh
index 2fac08c..d36efd6 100755
--- a/scripts/gen-config-h.sh
+++ b/scripts/gen-config-h.sh
@@ -35,9 +35,11 @@ echo "#ifndef __RTE_CONFIG_H"
 echo "#define __RTE_CONFIG_H"
 grep CONFIG_ $1 |
 grep -v '^[ \t]*#' |
-sed 's,CONFIG_\(.*\)=y.*$,#undef \1\n#define \1 1,' |
+sed 's,CONFIG_\(.*\)=y.*$,#undef \1\
+#define \1 1,' |
 sed 's,CONFIG_\(.*\)=n.*$,#undef \1,' |
-sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\n#define \1 \2,' |
+sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\
+#define \1 \2,' |
 sed 's,\# CONFIG_\(.*\) is not set$,#undef \1,'
 echo "#endif /* __RTE_CONFIG_H */"



-- 
David Marchand

[dpdk-dev] [PATCH] enicpmd: compilation error during inclusion of vfio.h

[Thomas Monjalon]

2014-11-28 02:09, Qiu, Michael:
> I have no comments on this issue, but I indeed see many places do have
> this kernel issue(before/now/future), so can solve this issue globally?
> 
> Thus, we do not need to fix this case by case.
> 
> One solution(not sure if it works or not):
> 
> 1. features and kernel version required list.
> 2. When config DPDK before build, automatically check this list and if
> not mach, just disable this feature in config file even though user set
> it manually.
> 
> Thus main code may not need to change.
> 
> Does this works?

If configuration system was different, we could have a list of constraint
to satisfy before enabling a feature.

-- 
Thomas

[dpdk-dev] [PATCH] scripts: fix merged lines on FreeBSD in config.h

[Bruce Richardson]

Since commit 0a91453d, "Fix symbol overriding in configuration", the
rte_config.h can have two lines generated for a single directive to
enable a feature - one line to undef the feature value, and a second
to enable or set the new value. On FreeBSD, sed inserts an "n" char
instead of the "\n" carriage return, leading to compiler errors.
This patch fixes that by having sed insert a "$" character instead
of attempting a "\n", and then using tr subsequently to turn "$"
characters into real "\n" characters.

Signed-off-by: Bruce Richardson 
---
 scripts/gen-config-h.sh | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/scripts/gen-config-h.sh b/scripts/gen-config-h.sh
index 2fac08c..647bf4b 100755
--- a/scripts/gen-config-h.sh
+++ b/scripts/gen-config-h.sh
@@ -35,9 +35,10 @@ echo "#ifndef __RTE_CONFIG_H"
 echo "#define __RTE_CONFIG_H"
 grep CONFIG_ $1 |
 grep -v '^[ \t]*#' |
-sed 's,CONFIG_\(.*\)=y.*$,#undef \1\n#define \1 1,' |
+sed 's,CONFIG_\(.*\)=y.*$,#undef \1$#define \1 1,' |
 sed 's,CONFIG_\(.*\)=n.*$,#undef \1,' |
-sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\n#define \1 \2,' |
-sed 's,\# CONFIG_\(.*\) is not set$,#undef \1,'
+sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1$#define \1 \2,' |
+sed 's,\# CONFIG_\(.*\) is not set$,#undef \1,' |
+tr '$' '\n'
 echo "#endif /* __RTE_CONFIG_H */"

-- 
2.1.0

[dpdk-dev] [PATCH] scripts: fix symbol overriding in configuration files

[Bruce Richardson]

On Fri, Nov 28, 2014 at 03:06:11PM +0100, David Marchand wrote:
> Hello Bruce,
> 
> On Fri, Nov 28, 2014 at 2:56 PM, Bruce Richardson <
> bruce.richardson at intel.com> wrote:
> 
> > On Thu, Nov 27, 2014 at 10:17:22AM -0800, Thomas Monjalon wrote:
> > > > When redefining the same symbol in configuration (basically after an
> > inclusion),
> > > > we need to undefine the previous symbol to avoid "redefined" errors.
> > > >
> > > > Signed-off-by: David Marchand 
> > >
> > > Acked-by: Thomas Monjalon 
> > >
> > > Applied
> > >
> > This patch doesn't seem to work on FreeBSD. I'm still investigating how to
> > fix
> > it but right now compilation with gcc/clang on FreeBSD is broken due to the
> > config.h file having lines like the below in it :-(
> >
> > /usr/home/bruce/
> > dpdk.org/x86_64-native-bsdapp-clang/include/rte_config.h:3:21: fatal
> > error: extra tokens at end of #undef directive [-Wextra-tokens]
> > #undef RTE_EXEC_ENVn#define RTE_EXEC_ENV "bsdapp"
> >
> 
> This is probably because of the \n in the sed.
> 
> Can you try something like this (I did not like this version of my patch at
> first because it is not that readable) ?
> 
> 
> diff --git a/scripts/gen-config-h.sh b/scripts/gen-config-h.sh
> index 2fac08c..d36efd6 100755
> --- a/scripts/gen-config-h.sh
> +++ b/scripts/gen-config-h.sh
> @@ -35,9 +35,11 @@ echo "#ifndef __RTE_CONFIG_H"
>  echo "#define __RTE_CONFIG_H"
>  grep CONFIG_ $1 |
>  grep -v '^[ \t]*#' |
> -sed 's,CONFIG_\(.*\)=y.*$,#undef \1\n#define \1 1,' |
> +sed 's,CONFIG_\(.*\)=y.*$,#undef \1\
> +#define \1 1,' |
>  sed 's,CONFIG_\(.*\)=n.*$,#undef \1,' |
> -sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\n#define \1 \2,' |
> +sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\
> +#define \1 \2,' |
>  sed 's,\# CONFIG_\(.*\) is not set$,#undef \1,'
>  echo "#endif /* __RTE_CONFIG_H */"
>
I tried a number of different things to get sed to introduce "\n" characters,
but I missed trying that one. I've sent on an alternative fix now, which uses
tr instead of sed to insert the "\n"'s. It's not a fix I'm terribly happy with,
but having seen the above (but not tried it yet), it actually doesn't seem that
bad in comparison :-)

/Bruce

[dpdk-dev] [PATCH] scripts: fix symbol overriding in configuration files

[Bruce Richardson]

On Fri, Nov 28, 2014 at 03:06:11PM +0100, David Marchand wrote:
> Hello Bruce,
> 
> On Fri, Nov 28, 2014 at 2:56 PM, Bruce Richardson <
> bruce.richardson at intel.com> wrote:
> 
> > On Thu, Nov 27, 2014 at 10:17:22AM -0800, Thomas Monjalon wrote:
> > > > When redefining the same symbol in configuration (basically after an
> > inclusion),
> > > > we need to undefine the previous symbol to avoid "redefined" errors.
> > > >
> > > > Signed-off-by: David Marchand 
> > >
> > > Acked-by: Thomas Monjalon 
> > >
> > > Applied
> > >
> > This patch doesn't seem to work on FreeBSD. I'm still investigating how to
> > fix
> > it but right now compilation with gcc/clang on FreeBSD is broken due to the
> > config.h file having lines like the below in it :-(
> >
> > /usr/home/bruce/
> > dpdk.org/x86_64-native-bsdapp-clang/include/rte_config.h:3:21: fatal
> > error: extra tokens at end of #undef directive [-Wextra-tokens]
> > #undef RTE_EXEC_ENVn#define RTE_EXEC_ENV "bsdapp"
> >
> 
> This is probably because of the \n in the sed.
> 
> Can you try something like this (I did not like this version of my patch at
> first because it is not that readable) ?
> 
> 
> diff --git a/scripts/gen-config-h.sh b/scripts/gen-config-h.sh
> index 2fac08c..d36efd6 100755
> --- a/scripts/gen-config-h.sh
> +++ b/scripts/gen-config-h.sh
> @@ -35,9 +35,11 @@ echo "#ifndef __RTE_CONFIG_H"
>  echo "#define __RTE_CONFIG_H"
>  grep CONFIG_ $1 |
>  grep -v '^[ \t]*#' |
> -sed 's,CONFIG_\(.*\)=y.*$,#undef \1\n#define \1 1,' |
> +sed 's,CONFIG_\(.*\)=y.*$,#undef \1\
> +#define \1 1,' |
>  sed 's,CONFIG_\(.*\)=n.*$,#undef \1,' |
> -sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\n#define \1 \2,' |
> +sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\
> +#define \1 \2,' |
>  sed 's,\# CONFIG_\(.*\) is not set$,#undef \1,'
>  echo "#endif /* __RTE_CONFIG_H */"
> 
>
This proposed fix also works. I don't mind whether my patch proposal or this
fix gets applied - gen-config-h.sh is not a commonly modified script anyway.

/Bruce

[dpdk-dev] [PATCH] scripts: fix symbol overriding in configuration files

[David Marchand]

On Fri, Nov 28, 2014 at 3:43 PM, Bruce Richardson <
bruce.richardson at intel.com> wrote:

> This proposed fix also works. I don't mind whether my patch proposal or
> this
> fix gets applied - gen-config-h.sh is not a commonly modified script
> anyway.
>

I prefer my ugliness ;-)
But Thomas just proposed me something that could work.
Trying ...


-- 
David Marchand

[dpdk-dev] [PATCH] enicpmd: compilation error during inclusion of vfio.h

[Neil Horman]

On Fri, Nov 28, 2014 at 02:09:40AM +, Qiu, Michael wrote:
> Hi all,
> 
> I have no comments on this issue, but I indeed see many places do have
> this kernel issue(before/now/future), so can solve this issue globally?
> 
> Thus, we do not need to fix this case by case.
> 
> One solution(not sure if it works or not):
> 
> 1. features and kernel version required list.
> 2. When config DPDK before build, automatically check this list and if
> not mach, just disable this feature in config file even though user set
> it manually.
> 
Instead of enumerating a kernel version, this should be designed as an
enumeration of feature sets.  That way you can enable DPDK features based on
kernel feature availability (i.e. kernel version doesn't necessecarily imply
feature set, like when a distribution backports a given feature to an older
kernel.

What would really make the most sense I think is to convert the configuration
system to autoconf/automake, so that these tests can be preformed at config
time.

Neil

> Thus main code may not need to change.
> 
> Does this works?
> 
> Thanks,
> Michael
> 
> On 11/28/2014 1:16 AM, Sujith Sankar wrote:
> > Inclusion of vfio.h was giving compilation errors if kernel version is less
> > than 3.6.0 and if RTE_EAL_VFIO was on in config.
> >
> > Replaced inclusion of vfio.h with eal_vfio.h and replaced RTE_EAL_VFIO with
> > VFIO_PRESENT in enicpmd code.
> >
> > Signed-off-by: Sujith Sankar 
> > ---
> >  lib/librte_pmd_enic/Makefile|  1 +
> >  lib/librte_pmd_enic/enic_main.c | 13 +
> >  2 files changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/lib/librte_pmd_enic/Makefile b/lib/librte_pmd_enic/Makefile
> > index d4c2f66..befc552 100644
> > --- a/lib/librte_pmd_enic/Makefile
> > +++ b/lib/librte_pmd_enic/Makefile
> > @@ -39,6 +39,7 @@ LIB = librte_pmd_enic.a
> >  
> >  CFLAGS += -I$(RTE_SDK)/lib/librte_hash/ 
> > -I$(RTE_SDK)/lib/librte_pmd_enic/vnic/
> >  CFLAGS += -I$(RTE_SDK)/lib/librte_pmd_enic/
> > +CFLAGS += -I$(RTE_SDK)/lib/librte_eal/linuxapp/eal/
> >  CFLAGS += -O3 -Wno-deprecated
> >  
> >  VPATH += $(RTE_SDK)/lib/librte_pmd_enic/src
> > diff --git a/lib/librte_pmd_enic/enic_main.c 
> > b/lib/librte_pmd_enic/enic_main.c
> > index 4b857bb..f6f00d3 100644
> > --- a/lib/librte_pmd_enic/enic_main.c
> > +++ b/lib/librte_pmd_enic/enic_main.c
> > @@ -39,9 +39,6 @@
> >  #include 
> >  #include 
> >  #include 
> > -#ifdef RTE_EAL_VFIO
> > -#include 
> > -#endif
> >  
> >  #include 
> >  #include 
> > @@ -631,7 +628,7 @@ int enic_enable(struct enic *enic)
> >  
> > vnic_dev_enable_wait(enic->vdev);
> >  
> > -#ifndef RTE_EAL_VFIO
> > +#ifndef VFIO_PRESENT
> > /* Register and enable error interrupt */
> > rte_intr_callback_register(&(enic->pdev->intr_handle),
> > enic_intr_handler, (void *)enic->rte_dev);
> > @@ -995,7 +992,7 @@ int enic_setup_finish(struct enic *enic)
> > return 0;
> >  }
> >  
> > -#ifdef RTE_EAL_VFIO
> > +#ifdef VFIO_PRESENT
> >  static void enic_eventfd_init(struct enic *enic)
> >  {
> > enic->eventfd = enic->pdev->intr_handle.fd;
> > @@ -1033,7 +1030,7 @@ int enic_get_link_status(struct enic *enic)
> >  }
> >  
> >  
> > -#ifdef RTE_EAL_VFIO
> > +#ifdef VFIO_PRESENT
> >  static int enic_create_err_intr_thread(struct enic *enic)
> >  {
> > pthread_attr_t intr_attr;
> > @@ -,7 +1108,7 @@ static void enic_dev_deinit(struct enic *enic)
> > if (eth_dev->data->mac_addrs)
> > rte_free(eth_dev->data->mac_addrs);
> >  
> > -#ifdef RTE_EAL_VFIO
> > +#ifdef VFIO_PRESENT
> > enic_clear_intr_mode(enic);
> >  #endif
> >  }
> > @@ -1167,7 +1164,7 @@ static int enic_dev_init(struct enic *enic)
> > */
> > enic_get_res_counts(enic);
> >  
> > -#ifdef RTE_EAL_VFIO
> > +#ifdef VFIO_PRESENT
> > /* Set interrupt mode based on resource counts and system
> >  * capabilities
> >  */
> 
> 

[dpdk-dev] [PATCH] scripts: fix symbol overriding in configuration files

[Bruce Richardson]

On Fri, Nov 28, 2014 at 03:49:29PM +0100, David Marchand wrote:
> On Fri, Nov 28, 2014 at 3:43 PM, Bruce Richardson <
> bruce.richardson at intel.com> wrote:
> 
> > This proposed fix also works. I don't mind whether my patch proposal or
> > this
> > fix gets applied - gen-config-h.sh is not a commonly modified script
> > anyway.
> >
> 
> I prefer my ugliness ;-)
> But Thomas just proposed me something that could work.
> Trying ...
>
Yes, it's ugly, but it's probably more resilient. I'm looking forward to getting
an option C.

/Bruce

[dpdk-dev] [PATCH] bond: Fixed compilation issue on gcc 4.3, due to uninitialized array

[Pablo de Lara]

gcc 4.3 complains that slow_pkts array in bond_ethdev_tx_burst_8023ad may be 
used uninitialized, so it has been initialized to NULL

Signed-off-by: Pablo de Lara 
---
 lib/librte_pmd_bond/rte_eth_bond_pmd.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/librte_pmd_bond/rte_eth_bond_pmd.c 
b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
index bb2b909..539baa4 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
@@ -585,7 +585,7 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf 
**bufs,

/* Allocate additional packets in case 8023AD mode. */
struct rte_mbuf *slave_bufs[RTE_MAX_ETHPORTS][buffs_size];
-   void *slow_pkts[BOND_MODE_8023AX_SLAVE_TX_PKTS];
+   void *slow_pkts[BOND_MODE_8023AX_SLAVE_TX_PKTS] = { NULL };

/* Total amount of packets in slave_bufs */
uint16_t slave_nb_pkts[RTE_MAX_ETHPORTS] = { 0 };
-- 
1.7.4.1

[dpdk-dev] [PATCH] bond: Fixed compilation issue on gcc 4.3, due to uninitialized array

[Thomas Monjalon]

2014-11-28 15:10, Pablo de Lara:
> gcc 4.3 complains that slow_pkts array in bond_ethdev_tx_burst_8023ad may be 
> used uninitialized, so it has been initialized to NULL
> 
> Signed-off-by: Pablo de Lara 

Acked-by: Thomas Monjalon 

Applied

Thanks
-- 
Thomas

[dpdk-dev] [PATCH] ixgbe: Add missing rx_mbuf_alloc_failed statistics for vector PMD

[Thomas Monjalon]

> > The statistics that is reported through the rx_nombuf fields in struct
> > rte_eth_stats was not set when the vector PMD was used. The statistics
> > should report the number of mbufs that could _not_ be allocated during
> > rearm of the RX queue. The non-vector PMD reports it correctly. The
> > use of either vector PMD or non-vector PMD depends on runtime
> > configuration. Hence it is possible that a change in configuration
> > would disable this statistics. To prevent this from happening, the
> > statistics should be reported by both implementations.
> > 
> > Signed-off-by: Balazs Nemeth 
> 
> Acked-by: Bruce Richardson 

Applied

Thanks
-- 
Thomas

[dpdk-dev] [PATCH] ixgbe: fix clang compile - remove truncation errors

[Bruce Richardson]

When compiling with clang, errors were being emitted due to truncation
of values when assigning to the tx_offload_mask bit fields.

dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c:404:27: fatal error: implicit 
truncation from 'int' to bitfield changes value from -1 to 127 
[-Wbitfield-constant-conversion]
tx_offload_mask.l2_len = ~0;

The fix proposed here is to define a static const value of the same type
with all fields set to 1s, and use that instead of constants for assigning to.

Other options would be to explicitily define the suitable constants that
would not truncate for each individual field e.g. 0x7f for l2_len, 0x1FF
for l3_len, etc., but this solution here has the advantage that it works
without any changes to values if the field sizes are ever modified.

Signed-off-by: Bruce Richardson 
---
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 29 +++--
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c 
b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 8559ef6..4f71194 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -367,6 +367,7 @@ ixgbe_set_xmit_ctx(struct igb_tx_queue* txq,
volatile struct ixgbe_adv_tx_context_desc *ctx_txd,
uint64_t ol_flags, union ixgbe_tx_offload tx_offload)
 {
+   static const union ixgbe_tx_offload offload_allones = { .data = ~0 };
uint32_t type_tucmd_mlhl;
uint32_t mss_l4len_idx = 0;
uint32_t ctx_idx;
@@ -381,7 +382,7 @@ ixgbe_set_xmit_ctx(struct igb_tx_queue* txq,
mss_l4len_idx |= (ctx_idx << IXGBE_ADVTXD_IDX_SHIFT);

if (ol_flags & PKT_TX_VLAN_PKT) {
-   tx_offload_mask.vlan_tci = ~0;
+   tx_offload_mask.vlan_tci = offload_allones.vlan_tci;
}

/* check if TCP segmentation required for this packet */
@@ -391,17 +392,17 @@ ixgbe_set_xmit_ctx(struct igb_tx_queue* txq,
IXGBE_ADVTXD_TUCMD_L4T_TCP |
IXGBE_ADVTXD_DTYP_CTXT | IXGBE_ADVTXD_DCMD_DEXT;

-   tx_offload_mask.l2_len = ~0;
-   tx_offload_mask.l3_len = ~0;
-   tx_offload_mask.l4_len = ~0;
-   tx_offload_mask.tso_segsz = ~0;
+   tx_offload_mask.l2_len = offload_allones.l2_len;
+   tx_offload_mask.l3_len = offload_allones.l3_len;
+   tx_offload_mask.l4_len = offload_allones.l4_len;
+   tx_offload_mask.tso_segsz = offload_allones.tso_segsz;
mss_l4len_idx |= tx_offload.tso_segsz << IXGBE_ADVTXD_MSS_SHIFT;
mss_l4len_idx |= tx_offload.l4_len << IXGBE_ADVTXD_L4LEN_SHIFT;
} else { /* no TSO, check if hardware checksum is needed */
if (ol_flags & PKT_TX_IP_CKSUM) {
type_tucmd_mlhl = IXGBE_ADVTXD_TUCMD_IPV4;
-   tx_offload_mask.l2_len = ~0;
-   tx_offload_mask.l3_len = ~0;
+   tx_offload_mask.l2_len = offload_allones.l2_len;
+   tx_offload_mask.l3_len = offload_allones.l3_len;
}

switch (ol_flags & PKT_TX_L4_MASK) {
@@ -409,23 +410,23 @@ ixgbe_set_xmit_ctx(struct igb_tx_queue* txq,
type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_L4T_UDP |
IXGBE_ADVTXD_DTYP_CTXT | IXGBE_ADVTXD_DCMD_DEXT;
mss_l4len_idx |= sizeof(struct udp_hdr) << 
IXGBE_ADVTXD_L4LEN_SHIFT;
-   tx_offload_mask.l2_len = ~0;
-   tx_offload_mask.l3_len = ~0;
+   tx_offload_mask.l2_len = offload_allones.l2_len;
+   tx_offload_mask.l3_len = offload_allones.l3_len;
break;
case PKT_TX_TCP_CKSUM:
type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_L4T_TCP |
IXGBE_ADVTXD_DTYP_CTXT | IXGBE_ADVTXD_DCMD_DEXT;
mss_l4len_idx |= sizeof(struct tcp_hdr) << 
IXGBE_ADVTXD_L4LEN_SHIFT;
-   tx_offload_mask.l2_len = ~0;
-   tx_offload_mask.l3_len = ~0;
-   tx_offload_mask.l4_len = ~0;
+   tx_offload_mask.l2_len = offload_allones.l2_len;
+   tx_offload_mask.l3_len = offload_allones.l3_len;
+   tx_offload_mask.l4_len = offload_allones.l4_len;
break;
case PKT_TX_SCTP_CKSUM:
type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_L4T_SCTP |
IXGBE_ADVTXD_DTYP_CTXT | IXGBE_ADVTXD_DCMD_DEXT;
mss_l4len_idx |= sizeof(struct sctp_hdr) << 
IXGBE_ADVTXD_L4LEN_SHIFT;
-   tx_offload_mask.l2_len = ~0;
-   tx_offload_mask.l3_len = ~0;
+   tx_offload_mask.l2_len = offload_allones.l2_len;
+   tx_offload_ma

[dpdk-dev] [PATCH] mk: --no-as-needed by default for linux exec-env

[Thomas Monjalon]

Hi Sergio, Neil,

Do you agree to add this comment (before applying this *needed* patch)?

+# Workaround lack of DT_NEEDED entry

> -EXECENV_LDFLAGS =
> +EXECENV_LDFLAGS = --no-as-needed

-- 
Thomas

[dpdk-dev] [PATCH] scripts: fix symbol overriding in configuration files

[David Marchand]

On Fri, Nov 28, 2014 at 3:59 PM, Bruce Richardson <
bruce.richardson at intel.com> wrote:

> Yes, it's ugly, but it's probably more resilient. I'm looking forward to
> getting
> an option C.
>

Option C and D are ugly as well (using some bashism like $' ' or using an
intermediate variable with a newline in it).
Our "posix" guy told me that he prefers the initial version of the patch.
I will send a patch with this.

Thomas ?

-- 
David Marchand

[dpdk-dev] [PATCH] scripts: fix newline character in sed expression

[David Marchand]

Use of \n in sed expression is not portable and triggered an invalid
configuration on BSD (at least).
Replace with an explicit newline.

Reported-by: Bruce Richardson 
Signed-off-by: David Marchand 
---
 scripts/gen-config-h.sh |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/gen-config-h.sh b/scripts/gen-config-h.sh
index 2fac08c..d36efd6 100755
--- a/scripts/gen-config-h.sh
+++ b/scripts/gen-config-h.sh
@@ -35,9 +35,11 @@ echo "#ifndef __RTE_CONFIG_H"
 echo "#define __RTE_CONFIG_H"
 grep CONFIG_ $1 |
 grep -v '^[ \t]*#' |
-sed 's,CONFIG_\(.*\)=y.*$,#undef \1\n#define \1 1,' |
+sed 's,CONFIG_\(.*\)=y.*$,#undef \1\
+#define \1 1,' |
 sed 's,CONFIG_\(.*\)=n.*$,#undef \1,' |
-sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\n#define \1 \2,' |
+sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\
+#define \1 \2,' |
 sed 's,\# CONFIG_\(.*\) is not set$,#undef \1,'
 echo "#endif /* __RTE_CONFIG_H */"

-- 
1.7.10.4

[dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and change three fields

[Ananyev, Konstantin]

> -Original Message-
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> Sent: Friday, November 28, 2014 11:19 AM
> To: Ananyev, Konstantin; Liu, Jijiang; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and 
> change three fields
> 
> Hi Konstantin,
> 
> On 11/28/2014 12:13 PM, Ananyev, Konstantin wrote:
> >>> For the upper layer, I think there would be no big difference, what ways 
> >>> we will choose.
> >>
> >> I think the 2 informations are transversal, and that's why I would
> >> prefer 2 flags. Also, having 2 separate flags would also help to keep
> >> backward compatibility with previous versions.
> >
> > Hmm, not sure how we will break compatibility in that case?
> > If we'll make TX_IP_CKSUM to be 1 bit value (1 << X) then for current 
> > drivers nothing should change, no?
> 
> Yes, you're right, sorry.

Actually, you right - squeezing these 3 flags into 2 bits  would break backward 
compatibility.
Sorry, didn't think properly.
Konstantin

> 
> Olivier

[dpdk-dev] [PATCH] mk: fix build 32bits shared libs on 64bits system

[Thomas Monjalon]

Hi Sergio,

2014-10-22 17:36, Sergio Gonzalez Monroy:
> Incompatible libraries error when building shared libraries for 32bits on
> a 64bits system.
> Fix issue by passing CPU_CFLAGS to CC when LINK_USING_CC is enabled.

This issue looks really strange. If that's the only way to fix it,
it would be better to have a comment in the makefile and a detailed
explanation in the commit log.

Thanks
-- 
Thomas

[dpdk-dev] [PATCH] scripts: fix newline character in sed expression

[Bruce Richardson]

On Fri, Nov 28, 2014 at 04:42:44PM +0100, David Marchand wrote:
> Use of \n in sed expression is not portable and triggered an invalid
> configuration on BSD (at least).
> Replace with an explicit newline.
> 
> Reported-by: Bruce Richardson 
> Signed-off-by: David Marchand 

Acked-by: Bruce Richardson 

> ---
>  scripts/gen-config-h.sh |6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/gen-config-h.sh b/scripts/gen-config-h.sh
> index 2fac08c..d36efd6 100755
> --- a/scripts/gen-config-h.sh
> +++ b/scripts/gen-config-h.sh
> @@ -35,9 +35,11 @@ echo "#ifndef __RTE_CONFIG_H"
>  echo "#define __RTE_CONFIG_H"
>  grep CONFIG_ $1 |
>  grep -v '^[ \t]*#' |
> -sed 's,CONFIG_\(.*\)=y.*$,#undef \1\n#define \1 1,' |
> +sed 's,CONFIG_\(.*\)=y.*$,#undef \1\
> +#define \1 1,' |
>  sed 's,CONFIG_\(.*\)=n.*$,#undef \1,' |
> -sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\n#define \1 \2,' |
> +sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\
> +#define \1 \2,' |
>  sed 's,\# CONFIG_\(.*\) is not set$,#undef \1,'
>  echo "#endif /* __RTE_CONFIG_H */"
>  
> -- 
> 1.7.10.4
> 

[dpdk-dev] [PATCH v2] enicpmd: replace the type u_int* with uint* to remove compilation errors on a few platforms

[David Marchand]

On Fri, Nov 28, 2014 at 10:38 AM, Sujith Sankar  wrote:

> ENIC PMD was giving compilation errors on ppc_64-power8-linuxapp-gcc
> because
> of types such as u_int32_t.  This patch replaces all those with uint32_t
> and
> similar ones.
>
> Reported-by: David Marchand 
> Signed-off-by: Sujith Sankar 
> ---
>  lib/librte_pmd_enic/enic.h |  2 +-
>  lib/librte_pmd_enic/enic_compat.h  | 24 
>  lib/librte_pmd_enic/enic_main.c| 18 +-
>  lib/librte_pmd_enic/vnic/vnic_devcmd.h |  6 +++---
>  4 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/lib/librte_pmd_enic/enic.h b/lib/librte_pmd_enic/enic.h
> index 9f80fc0..6400d24 100644
> --- a/lib/librte_pmd_enic/enic.h
> +++ b/lib/librte_pmd_enic/enic.h
> @@ -106,7 +106,7 @@ struct enic {
> int iommu_group_fd;
> int iommu_groupid;
> int eventfd;
> -   u_int8_t mac_addr[ETH_ALEN];
> +   uint8_t mac_addr[ETH_ALEN];
> pthread_t err_intr_thread;
> int promisc;
> int allmulti;
> diff --git a/lib/librte_pmd_enic/enic_compat.h
> b/lib/librte_pmd_enic/enic_compat.h
> index d22578e..b3738ee 100644
> --- a/lib/librte_pmd_enic/enic_compat.h
> +++ b/lib/librte_pmd_enic/enic_compat.h
> @@ -89,34 +89,34 @@ typedef unsigned intu32;
>  typedef unsigned long long  u64;
>  typedef unsigned long long  dma_addr_t;
>
> -static inline u_int32_t ioread32(volatile void *addr)
> +static inline uint32_t ioread32(volatile void *addr)
>  {
> -   return *(volatile u_int32_t *)addr;
> +   return *(volatile uint32_t *)addr;
>  }
>
> -static inline u16 ioread16(volatile void *addr)
> +static inline uint16_t ioread16(volatile void *addr)
>  {
> -   return *(volatile u16 *)addr;
> +   return *(volatile uint16_t *)addr;
>  }
>
> -static inline u_int8_t ioread8(volatile void *addr)
> +static inline uint8_t ioread8(volatile void *addr)
>  {
> -   return *(volatile u_int8_t *)addr;
> +   return *(volatile uint8_t *)addr;
>  }
>
> -static inline void iowrite32(u_int32_t val, volatile void *addr)
> +static inline void iowrite32(uint32_t val, volatile void *addr)
>  {
> -   *(volatile u_int32_t *)addr = val;
> +   *(volatile uint32_t *)addr = val;
>  }
>
> -static inline void iowrite16(u16 val, volatile void *addr)
> +static inline void iowrite16(uint16_t val, volatile void *addr)
>  {
> -   *(volatile u16 *)addr = val;
> +   *(volatile uint16_t *)addr = val;
>  }
>
> -static inline void iowrite8(u_int8_t val, volatile void *addr)
> +static inline void iowrite8(uint8_t val, volatile void *addr)
>  {
> -   *(volatile u_int8_t *)addr = val;
> +   *(volatile uint8_t *)addr = val;
>  }
>
>  static inline unsigned int readl(volatile void __iomem *addr)
> diff --git a/lib/librte_pmd_enic/enic_main.c
> b/lib/librte_pmd_enic/enic_main.c
> index f6f00d3..853dd04 100644
> --- a/lib/librte_pmd_enic/enic_main.c
> +++ b/lib/librte_pmd_enic/enic_main.c
> @@ -172,17 +172,17 @@ unsigned int enic_cleanup_wq(struct enic *enic,
> struct vnic_wq *wq)
>
>  int enic_send_pkt(struct enic *enic, struct vnic_wq *wq,
> struct rte_mbuf *tx_pkt, unsigned short len,
> -   u_int8_t sop, u_int8_t eop,
> -   u_int16_t ol_flags, u_int16_t vlan_tag)
> +   uint8_t sop, uint8_t eop,
> +   uint16_t ol_flags, uint16_t vlan_tag)
>  {
> struct wq_enet_desc *desc = vnic_wq_next_desc(wq);
> -   u_int16_t mss = 0;
> -   u_int16_t header_length = 0;
> -   u_int8_t cq_entry = eop;
> -   u_int8_t vlan_tag_insert = 0;
> +   uint16_t mss = 0;
> +   uint16_t header_length = 0;
> +   uint8_t cq_entry = eop;
> +   uint8_t vlan_tag_insert = 0;
> unsigned char *buf = (unsigned char *)(tx_pkt->buf_addr) +
> RTE_PKTMBUF_HEADROOM;
> -   u_int64_t bus_addr = (dma_addr_t)
> +   uint64_t bus_addr = (dma_addr_t)
> (tx_pkt->buf_physaddr + RTE_PKTMBUF_HEADROOM);
>
> if (sop) {
> @@ -342,8 +342,8 @@ static int enic_rq_alloc_buf(struct vnic_rq *rq)
> void *buf;
> dma_addr_t dma_addr;
> struct rq_enet_desc *desc = vnic_rq_next_desc(rq);
> -   u_int8_t type = RQ_ENET_TYPE_ONLY_SOP;
> -   u_int16_t len = ENIC_MAX_MTU + VLAN_ETH_HLEN;
> +   uint8_t type = RQ_ENET_TYPE_ONLY_SOP;
> +   uint16_t len = ENIC_MAX_MTU + VLAN_ETH_HLEN;
> u16 split_hdr_size = vnic_get_hdr_split_size(enic->vdev);
> struct rte_mbuf *mbuf = enic_rxmbuf_alloc(rq->mp);
> struct rte_mbuf *hdr_mbuf = NULL;
> diff --git a/lib/librte_pmd_enic/vnic/vnic_devcmd.h
> b/lib/librte_pmd_enic/vnic/vnic_devcmd.h
> index b4c87c1..e7ecf31 100644
> --- a/lib/librte_pmd_enic/vnic/vnic_devcmd.h
> +++ b/lib/librte_pmd_enic/vnic/vnic_devcmd.h
> @@ -691,9 +691,9 @@ enum {
>  #define FILTER_MAX_BUF_SIZE 100  /* Maximum size of buffer to
> CMD_ADD_FILTER */
>
>  struct filter_tlv {
> -   u_int32_t type;
> -   u_int32_t length;
> - 

[dpdk-dev] [PATCH v2] enicpmd: replace the type u_int* with uint* to remove compilation errors on a few platforms

[Bruce Richardson]

On Fri, Nov 28, 2014 at 03:08:19PM +0530, Sujith Sankar wrote:
> ENIC PMD was giving compilation errors on ppc_64-power8-linuxapp-gcc because
> of types such as u_int32_t.  This patch replaces all those with uint32_t and
> similar ones.
> 
> Reported-by: David Marchand 
> Signed-off-by: Sujith Sankar 

Acked-by: Bruce Richardson 

This patch helps out with getting a clang compile on BSD. However, one error
and a number of warnings remain that should be looked at in another patch.
The error is:

dpdk.org/lib/librte_pmd_enic/enic_main.c:435:3: fatal error: non-void function 
'enic_rq_indicate_buf' should return a value [-Wreturn-type]
return;

> ---
>  lib/librte_pmd_enic/enic.h |  2 +-
>  lib/librte_pmd_enic/enic_compat.h  | 24 
>  lib/librte_pmd_enic/enic_main.c| 18 +-
>  lib/librte_pmd_enic/vnic/vnic_devcmd.h |  6 +++---
>  4 files changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/lib/librte_pmd_enic/enic.h b/lib/librte_pmd_enic/enic.h
> index 9f80fc0..6400d24 100644
> --- a/lib/librte_pmd_enic/enic.h
> +++ b/lib/librte_pmd_enic/enic.h
> @@ -106,7 +106,7 @@ struct enic {
>   int iommu_group_fd;
>   int iommu_groupid;
>   int eventfd;
> - u_int8_t mac_addr[ETH_ALEN];
> + uint8_t mac_addr[ETH_ALEN];
>   pthread_t err_intr_thread;
>   int promisc;
>   int allmulti;
> diff --git a/lib/librte_pmd_enic/enic_compat.h 
> b/lib/librte_pmd_enic/enic_compat.h
> index d22578e..b3738ee 100644
> --- a/lib/librte_pmd_enic/enic_compat.h
> +++ b/lib/librte_pmd_enic/enic_compat.h
> @@ -89,34 +89,34 @@ typedef   unsigned intu32;
>  typedef unsigned long long  u64;
>  typedef unsigned long long  dma_addr_t;
>  
> -static inline u_int32_t ioread32(volatile void *addr)
> +static inline uint32_t ioread32(volatile void *addr)
>  {
> - return *(volatile u_int32_t *)addr;
> + return *(volatile uint32_t *)addr;
>  }
>  
> -static inline u16 ioread16(volatile void *addr)
> +static inline uint16_t ioread16(volatile void *addr)
>  {
> - return *(volatile u16 *)addr;
> + return *(volatile uint16_t *)addr;
>  }
>  
> -static inline u_int8_t ioread8(volatile void *addr)
> +static inline uint8_t ioread8(volatile void *addr)
>  {
> - return *(volatile u_int8_t *)addr;
> + return *(volatile uint8_t *)addr;
>  }
>  
> -static inline void iowrite32(u_int32_t val, volatile void *addr)
> +static inline void iowrite32(uint32_t val, volatile void *addr)
>  {
> - *(volatile u_int32_t *)addr = val;
> + *(volatile uint32_t *)addr = val;
>  }
>  
> -static inline void iowrite16(u16 val, volatile void *addr)
> +static inline void iowrite16(uint16_t val, volatile void *addr)
>  {
> - *(volatile u16 *)addr = val;
> + *(volatile uint16_t *)addr = val;
>  }
>  
> -static inline void iowrite8(u_int8_t val, volatile void *addr)
> +static inline void iowrite8(uint8_t val, volatile void *addr)
>  {
> - *(volatile u_int8_t *)addr = val;
> + *(volatile uint8_t *)addr = val;
>  }
>  
>  static inline unsigned int readl(volatile void __iomem *addr)
> diff --git a/lib/librte_pmd_enic/enic_main.c b/lib/librte_pmd_enic/enic_main.c
> index f6f00d3..853dd04 100644
> --- a/lib/librte_pmd_enic/enic_main.c
> +++ b/lib/librte_pmd_enic/enic_main.c
> @@ -172,17 +172,17 @@ unsigned int enic_cleanup_wq(struct enic *enic, struct 
> vnic_wq *wq)
>  
>  int enic_send_pkt(struct enic *enic, struct vnic_wq *wq,
>   struct rte_mbuf *tx_pkt, unsigned short len,
> - u_int8_t sop, u_int8_t eop,
> - u_int16_t ol_flags, u_int16_t vlan_tag)
> + uint8_t sop, uint8_t eop,
> + uint16_t ol_flags, uint16_t vlan_tag)
>  {
>   struct wq_enet_desc *desc = vnic_wq_next_desc(wq);
> - u_int16_t mss = 0;
> - u_int16_t header_length = 0;
> - u_int8_t cq_entry = eop;
> - u_int8_t vlan_tag_insert = 0;
> + uint16_t mss = 0;
> + uint16_t header_length = 0;
> + uint8_t cq_entry = eop;
> + uint8_t vlan_tag_insert = 0;
>   unsigned char *buf = (unsigned char *)(tx_pkt->buf_addr) +
>   RTE_PKTMBUF_HEADROOM;
> - u_int64_t bus_addr = (dma_addr_t)
> + uint64_t bus_addr = (dma_addr_t)
>   (tx_pkt->buf_physaddr + RTE_PKTMBUF_HEADROOM);
>  
>   if (sop) {
> @@ -342,8 +342,8 @@ static int enic_rq_alloc_buf(struct vnic_rq *rq)
>   void *buf;
>   dma_addr_t dma_addr;
>   struct rq_enet_desc *desc = vnic_rq_next_desc(rq);
> - u_int8_t type = RQ_ENET_TYPE_ONLY_SOP;
> - u_int16_t len = ENIC_MAX_MTU + VLAN_ETH_HLEN;
> + uint8_t type = RQ_ENET_TYPE_ONLY_SOP;
> + uint16_t len = ENIC_MAX_MTU + VLAN_ETH_HLEN;
>   u16 split_hdr_size = vnic_get_hdr_split_size(enic->vdev);
>   struct rte_mbuf *mbuf = enic_rxmbuf_alloc(rq->mp);
>   struct rte_mbuf *hdr_mbuf = NULL;
> diff --git a/lib/librte_pmd_enic/vnic/vnic_devcmd.h 
> b/lib/librte_pmd_enic/vnic/vnic_devcmd.h
> index b4c87c1..e7ecf31 100644
> --- a/lib

[dpdk-dev] [PATCH] mk: fix app linking for combined libs

[Thomas Monjalon]

2014-10-23 16:36, Sergio Gonzalez Monroy:
> Building combined shared libraries results in applications being linked
> against separeted/individual and combined libs altogether.
> 
> Link only against combined lib when the config option is enabled.
> 
> Signed-off-by: Sergio Gonzalez Monroy 
[...]
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -217,6 +217,12 @@ endif
>  
>  endif # plugins
>  
> +ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> +LDLIBS = --whole-archive

You are resetting LDLIBS here.
It's not easy to read and probably not desired.
I think it would be better to explicitly disable separated libs
in this case.

> +LDLIBS += --start-group
> +LDLIBS += -l$(RTE_LIBNAME)
> +endif

Thanks
-- 
Thomas

[dpdk-dev] [PATCH v2] enicpmd: replace the type u_int* with uint* to remove compilation errors on a few platforms

[Thomas Monjalon]

> > ENIC PMD was giving compilation errors on ppc_64-power8-linuxapp-gcc because
> > of types such as u_int32_t.  This patch replaces all those with uint32_t and
> > similar ones.
> > 
> > Reported-by: David Marchand 
> > Signed-off-by: Sujith Sankar 
> 
> Acked-by: Bruce Richardson 
> 
> This patch helps out with getting a clang compile on BSD. However, one error
> and a number of warnings remain that should be looked at in another patch.

Applied

Thanks
-- 
Thomas

[dpdk-dev] [PATCH v2] enicpmd: replace the type u_int* with uint* to remove compilation errors on a few platforms

[Sujith Sankar (ssujith)]

On 28/11/14 9:22 pm, "Bruce Richardson"  wrote:

>On Fri, Nov 28, 2014 at 03:08:19PM +0530, Sujith Sankar wrote:
>> ENIC PMD was giving compilation errors on ppc_64-power8-linuxapp-gcc
>>because
>> of types such as u_int32_t.  This patch replaces all those with
>>uint32_t and
>> similar ones.
>> 
>> Reported-by: David Marchand 
>> Signed-off-by: Sujith Sankar 
>
>Acked-by: Bruce Richardson 
>
>This patch helps out with getting a clang compile on BSD. However, one
>error
>and a number of warnings remain that should be looked at in another patch.
>The error is:
>
>dpdk.org/lib/librte_pmd_enic/enic_main.c:435:3: fatal error: non-void
>function 'enic_rq_indicate_buf' should return a value [-Wreturn-type]
>return;

Bruce, thanks for the comment.  I?ve not built enicpmd using clang
compiler.
I shall setup a BSD build with clang compiler and send a patch with
necessary fixes.
Could you please let me know the versions and flavours that you are using?

Thanks,
-Sujith

>
>> ---
>>  lib/librte_pmd_enic/enic.h |  2 +-
>>  lib/librte_pmd_enic/enic_compat.h  | 24 
>>  lib/librte_pmd_enic/enic_main.c| 18 +-
>>  lib/librte_pmd_enic/vnic/vnic_devcmd.h |  6 +++---
>>  4 files changed, 25 insertions(+), 25 deletions(-)
>> 
>> diff --git a/lib/librte_pmd_enic/enic.h b/lib/librte_pmd_enic/enic.h
>> index 9f80fc0..6400d24 100644
>> --- a/lib/librte_pmd_enic/enic.h
>> +++ b/lib/librte_pmd_enic/enic.h
>> @@ -106,7 +106,7 @@ struct enic {
>>  int iommu_group_fd;
>>  int iommu_groupid;
>>  int eventfd;
>> -u_int8_t mac_addr[ETH_ALEN];
>> +uint8_t mac_addr[ETH_ALEN];
>>  pthread_t err_intr_thread;
>>  int promisc;
>>  int allmulti;
>> diff --git a/lib/librte_pmd_enic/enic_compat.h
>>b/lib/librte_pmd_enic/enic_compat.h
>> index d22578e..b3738ee 100644
>> --- a/lib/librte_pmd_enic/enic_compat.h
>> +++ b/lib/librte_pmd_enic/enic_compat.h
>> @@ -89,34 +89,34 @@ typedef  unsigned intu32;
>>  typedef unsigned long long  u64;
>>  typedef unsigned long long  dma_addr_t;
>>  
>> -static inline u_int32_t ioread32(volatile void *addr)
>> +static inline uint32_t ioread32(volatile void *addr)
>>  {
>> -return *(volatile u_int32_t *)addr;
>> +return *(volatile uint32_t *)addr;
>>  }
>>  
>> -static inline u16 ioread16(volatile void *addr)
>> +static inline uint16_t ioread16(volatile void *addr)
>>  {
>> -return *(volatile u16 *)addr;
>> +return *(volatile uint16_t *)addr;
>>  }
>>  
>> -static inline u_int8_t ioread8(volatile void *addr)
>> +static inline uint8_t ioread8(volatile void *addr)
>>  {
>> -return *(volatile u_int8_t *)addr;
>> +return *(volatile uint8_t *)addr;
>>  }
>>  
>> -static inline void iowrite32(u_int32_t val, volatile void *addr)
>> +static inline void iowrite32(uint32_t val, volatile void *addr)
>>  {
>> -*(volatile u_int32_t *)addr = val;
>> +*(volatile uint32_t *)addr = val;
>>  }
>>  
>> -static inline void iowrite16(u16 val, volatile void *addr)
>> +static inline void iowrite16(uint16_t val, volatile void *addr)
>>  {
>> -*(volatile u16 *)addr = val;
>> +*(volatile uint16_t *)addr = val;
>>  }
>>  
>> -static inline void iowrite8(u_int8_t val, volatile void *addr)
>> +static inline void iowrite8(uint8_t val, volatile void *addr)
>>  {
>> -*(volatile u_int8_t *)addr = val;
>> +*(volatile uint8_t *)addr = val;
>>  }
>>  
>>  static inline unsigned int readl(volatile void __iomem *addr)
>> diff --git a/lib/librte_pmd_enic/enic_main.c
>>b/lib/librte_pmd_enic/enic_main.c
>> index f6f00d3..853dd04 100644
>> --- a/lib/librte_pmd_enic/enic_main.c
>> +++ b/lib/librte_pmd_enic/enic_main.c
>> @@ -172,17 +172,17 @@ unsigned int enic_cleanup_wq(struct enic *enic,
>>struct vnic_wq *wq)
>>  
>>  int enic_send_pkt(struct enic *enic, struct vnic_wq *wq,
>>  struct rte_mbuf *tx_pkt, unsigned short len,
>> -u_int8_t sop, u_int8_t eop,
>> -u_int16_t ol_flags, u_int16_t vlan_tag)
>> +uint8_t sop, uint8_t eop,
>> +uint16_t ol_flags, uint16_t vlan_tag)
>>  {
>>  struct wq_enet_desc *desc = vnic_wq_next_desc(wq);
>> -u_int16_t mss = 0;
>> -u_int16_t header_length = 0;
>> -u_int8_t cq_entry = eop;
>> -u_int8_t vlan_tag_insert = 0;
>> +uint16_t mss = 0;
>> +uint16_t header_length = 0;
>> +uint8_t cq_entry = eop;
>> +uint8_t vlan_tag_insert = 0;
>>  unsigned char *buf = (unsigned char *)(tx_pkt->buf_addr) +
>>  RTE_PKTMBUF_HEADROOM;
>> -u_int64_t bus_addr = (dma_addr_t)
>> +uint64_t bus_addr = (dma_addr_t)
>>  (tx_pkt->buf_physaddr + RTE_PKTMBUF_HEADROOM);
>>  
>>  if (sop) {
>> @@ -342,8 +342,8 @@ static int enic_rq_alloc_buf(struct vnic_rq *rq)
>>  void *buf;
>>  dma_addr_t dma_addr;
>>  struct rq_enet_desc *desc = vnic_rq_next_desc(rq);
>> -u_int8_t type = RQ_ENET_TYPE_ONLY_SOP;
>> -u_int16_t len = ENIC_MAX_MTU + VLAN_ETH_HLEN;
>> +uint8_t ty

[dpdk-dev] [PATCH] scripts: fix newline character in sed expression

[Thomas Monjalon]

> > Use of \n in sed expression is not portable and triggered an invalid
> > configuration on BSD (at least).
> > Replace with an explicit newline.
> > 
> > Reported-by: Bruce Richardson 
> > Signed-off-by: David Marchand 
> 
> Acked-by: Bruce Richardson 

Applied

Thanks
-- 
Thomas

[dpdk-dev] [PATCH v2] enicpmd: replace the type u_int* with uint* to remove compilation errors on a few platforms

[Bruce Richardson]

On Fri, Nov 28, 2014 at 04:01:00PM +, Sujith Sankar (ssujith) wrote:
> 
> 
> On 28/11/14 9:22 pm, "Bruce Richardson"  wrote:
> 
> >On Fri, Nov 28, 2014 at 03:08:19PM +0530, Sujith Sankar wrote:
> >> ENIC PMD was giving compilation errors on ppc_64-power8-linuxapp-gcc
> >>because
> >> of types such as u_int32_t.  This patch replaces all those with
> >>uint32_t and
> >> similar ones.
> >> 
> >> Reported-by: David Marchand 
> >> Signed-off-by: Sujith Sankar 
> >
> >Acked-by: Bruce Richardson 
> >
> >This patch helps out with getting a clang compile on BSD. However, one
> >error
> >and a number of warnings remain that should be looked at in another patch.
> >The error is:
> >
> >dpdk.org/lib/librte_pmd_enic/enic_main.c:435:3: fatal error: non-void
> >function 'enic_rq_indicate_buf' should return a value [-Wreturn-type]
> >return;
> 
> Bruce, thanks for the comment.  I?ve not built enicpmd using clang
> compiler.
> I shall setup a BSD build with clang compiler and send a patch with
> necessary fixes.
> Could you please let me know the versions and flavours that you are using?
> 
> Thanks,
> -Sujith
>
Clang on a linux install gives the same errors, as far as I can see. What
I'm using is clang 3.4 on Fedora 20, and clang 3.3 on FreeBSD 10. 

If you like, if you just fix clang on Linux compilation, I can check for any
additional errors on FreeBSD and get back to you on them, rather than you having
to install a FreeBSD system right away.

/Bruce

[dpdk-dev] [PATCH v2] enicpmd: replace the type u_int* with uint* to remove compilation errors on a few platforms

[Sujith Sankar (ssujith)]

On 28/11/14 9:36 pm, "Bruce Richardson"  wrote:

>On Fri, Nov 28, 2014 at 04:01:00PM +, Sujith Sankar (ssujith) wrote:
>> 
>> 
>> On 28/11/14 9:22 pm, "Bruce Richardson" 
>>wrote:
>> 
>> >On Fri, Nov 28, 2014 at 03:08:19PM +0530, Sujith Sankar wrote:
>> >> ENIC PMD was giving compilation errors on ppc_64-power8-linuxapp-gcc
>> >>because
>> >> of types such as u_int32_t.  This patch replaces all those with
>> >>uint32_t and
>> >> similar ones.
>> >> 
>> >> Reported-by: David Marchand 
>> >> Signed-off-by: Sujith Sankar 
>> >
>> >Acked-by: Bruce Richardson 
>> >
>> >This patch helps out with getting a clang compile on BSD. However, one
>> >error
>> >and a number of warnings remain that should be looked at in another
>>patch.
>> >The error is:
>> >
>> >dpdk.org/lib/librte_pmd_enic/enic_main.c:435:3: fatal error: non-void
>> >function 'enic_rq_indicate_buf' should return a value [-Wreturn-type]
>> >return;
>> 
>> Bruce, thanks for the comment.  I?ve not built enicpmd using clang
>> compiler.
>> I shall setup a BSD build with clang compiler and send a patch with
>> necessary fixes.
>> Could you please let me know the versions and flavours that you are
>>using?
>> 
>> Thanks,
>> -Sujith
>>
>Clang on a linux install gives the same errors, as far as I can see. What
>I'm using is clang 3.4 on Fedora 20, and clang 3.3 on FreeBSD 10.
>
>If you like, if you just fix clang on Linux compilation, I can check for
>any
>additional errors on FreeBSD and get back to you on them, rather than you
>having
>to install a FreeBSD system right away.

Thank you Bruce.
I shall start with Clang on Linux and send the patch soon.

>
>/Bruce

[dpdk-dev] [PATCH v2 1/2] igb_uio: compatible with upstream longterm kernel and RHEL6

[Thomas Monjalon]

2014-11-28 16:13, Jincheng Miao:
> 
> On 11/28/2014 01:01 AM, Thomas Monjalon wrote:
> > 2014-10-31 15:37, Jincheng Miao:
> >> Function pci_num_vf() is introduced from upstream linux-2.6.34. So
> >> this patch make compatible with longterm kernel linux-2.6.32.63.
> >>
> >> For RHEL6's kernel, although it is based on linux-2.6.32, it has
> >> pci_num_vf() implementation. As the same with commit 11ba0426,
> >> pci_num_vf() is defined from RHEL6. So we should check the macro
> >> RHEL_RELEASE_CODE to consider this situation.
> > 
> > Please, could you explain in which case CONFIG_PCI_IOV is defined?
> > The logic is a bit difficult to understand.
> 
> Yep, there is a little confusion for pci_num_vf():
> 1. it is available when CONFIG_PCI_IOV is defined.
> 2. it is introduced from upstream kernel v2.6.34 (fb8a0d9)
> 3. it is implemented from RHEL6.0, although the kernel version is 2.6.32.

Sorry, you didn't described when CONFIG_PCI_IOV is defined.
Is it defined since 2.6.34 upstream? In lower stable versions?
Is it defined since RHEL 6.0?
Why checking CONFIG_PCI_IOV is not sufficient?

When pci_num_vf will be backported in other distributions, we will have to
tune this check and clearly understand what was the situation.

> The logic of this patch is:
> #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 34) && \
> (!(defined(RHEL_RELEASE_CODE) && RHEL_RELEASE_CODE >= 
> RHEL_RELEASE_VERSION(6, 0) && defined(CONFIG_PCI_IOV)))
> 
> Firstly it detects kernel version, if it is less than 2.6.34, and it is 
> not RHEL-specified, then define pci_num_vf().
> 
> Secondly, it deals with RHEL-specified. If it is RHEL6.0 or later, and 
> CONFIG_PCI_IOV is defined. we should not define pci_num_vf(). If any of 
> these conditions is not reached, pci_num_vf() should be defined.

I can read the check but I don't know why CONFIG_PCI_IOV is checked in the
RHEL case.

> Some days ago, I setup dpdk for longterm kernel 2.6.32.63, and got error:
> ```
> CC [M] 
> /root/dpdk-source/build/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.o
> /root/dpdk-source/build/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.c: 
> In function ?show_max_vfs?:
> /root/dpdk-source/build/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.c:75: 
> error: implicit declaration of function ?pci_num_vf?
> ```

Thank you. Describing the problem is helpful for the commit log.

> This problem is introduced by commit 11ba04265
> 
> commit 11ba04265cfd2a53c12c030fcaa5dfe7eed39a42
> Author: Guillaume Gaudonville 
> Date: Wed Sep 3 10:18:23 2014 +0200
> 
> igb_uio: fix build on RHEL 6.3
> 
> - pci_num_vf() is already defined in RHEL 6
> - pci_intx_mask_supported is already defined in RHEL 6.3
> - pci_check_and_mask_intx is already defined in RHEL 6.3
> 
> Signed-off-by: Guillaume Gaudonville 
> Signed-off-by: David Marchand 
> Signed-off-by: Thomas Monjalon 
> 
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 34) && \
> + !defined(CONFIG_PCI_IOV)
> 
> That is because longterm kernel 2.6.32.63 defined CONFIG_PCI_IOV, but it 
> lacks pci_num_vf(),
> after above processing, pci_num_vf() is still not existed, then build fail.
> 
> My patch could work around it, and can deal with RHEL-specified kernel.

Thanks, we just need to understand the matrix of combinations to be sure
it will be well maintained.

-- 
Thomas

[dpdk-dev] [PATCH v2 2/2] eal: replace strict_strtoul with kstrtoul

[Thomas Monjalon]

2014-10-31 15:37, Jincheng Miao:
> From upstream kernel commit 3db2e9cd, strict_strto* serial functions
> are removed. So that we should directly used kstrtoul instead.
> 
> For compatible with old kernel and RHEL6, add some logic to
> igb_uio/compat.h, same as what we do for pci_num_vf().
> 
> Signed-off-by: Jincheng Miao 
> ---
>  lib/librte_eal/linuxapp/igb_uio/compat.h|8 
>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c   |4 ++--
>  lib/librte_eal/linuxapp/kni/kni_vhost.c |2 +-
>  lib/librte_eal/linuxapp/xen_dom0/dom0_mm_misc.c |2 +-

You are replacing strict_strtoul in kni and xen_dom0 without providing
a compatibility fallback.

-- 
Thomas

[dpdk-dev] [PATCH v2] doc: new sample app UG for VM power management

[Pablo de Lara]

This patch adds a new sample app UG, contaning explanation
of the new two sample apps added in the VM power management
patchset

Changes in v2:

Corrected svg files

Signed-off-by: Pablo de Lara 
---
 .../sample_app_ug/img/vm_power_mgr_highlevel.svg   | 1173 
 .../img/vm_power_mgr_vm_request_seq.svg|  548 +
 doc/guides/sample_app_ug/index.rst |5 +
 doc/guides/sample_app_ug/vm_power_management.rst   |  274 +
 4 files changed, 2000 insertions(+), 0 deletions(-)
 create mode 100644 doc/guides/sample_app_ug/img/vm_power_mgr_highlevel.svg
 create mode 100644 doc/guides/sample_app_ug/img/vm_power_mgr_vm_request_seq.svg
 create mode 100644 doc/guides/sample_app_ug/vm_power_management.rst

diff --git a/doc/guides/sample_app_ug/img/vm_power_mgr_highlevel.svg 
b/doc/guides/sample_app_ug/img/vm_power_mgr_highlevel.svg
new file mode 100644
index 000..4b0b3b8
--- /dev/null
+++ b/doc/guides/sample_app_ug/img/vm_power_mgr_highlevel.svg
@@ -0,0 +1,1173 @@
+
+http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd";>
+
+http://www.w3.org/2000/svg"; 
xmlns:xlink="http://www.w3.org/1999/xlink"; 
xmlns:ev="http://www.w3.org/2001/xml-events";
+   
xmlns:v="http://schemas.microsoft.com/visio/2003/SVGExtensions/"; 
width="7.96928in" height="6.37479in"
+   viewBox="0 0 573.788 458.985" xml:space="preserve" 
color-interpolation-filters="sRGB" class="st28">
+   
+   
+   
+   
+   
+   
+   
+
+   
+   
+   
+
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+   
+ 

[dpdk-dev] RTE mempool "used count" steadily goes down to zero despite steady packet throughput

[Kamraan Nasim]

Hello,

I have ~15Gbps of traffic flowing through two 10GE ports and been profiling
the rte mempool(or rather the pktmbuf mempool) memory consumption:

I have per lcore caching disabled(cache_size is 0)

I have noticed that:
- Mempool FREE cnt(as given byt rte_mempool_free_count()) increases
- Mempool USED cnt(as given by rte_mempool_used_count() decreases and
eventually drops to 0. When this happens, mempool reports itself as EMPTY
- rx_nombuf stats for the eth ports start climbing
- Valgrind Memcheck does not indicate any obvious leaks in RTE mempool or
my application.


I was wondering if others have come across this issue?  Or if people here
have used ways, besides Valgrind to profile the mempool or the pkt mbuf
pool?

Thanks,
Kam