* [dpdk-dev] [Pktgen] [PATCH] pktgen_setup_packets: fix race for packet header
@ 2015-09-09 14:22 Ilya Maximets
2015-09-16 9:09 ` Ilya Maximets
0 siblings, 1 reply; 7+ messages in thread
From: Ilya Maximets @ 2015-09-09 14:22 UTC (permalink / raw)
To: dev, Keith Wiles; +Cc: Ilya Maximets, Dyasly Sergey
While pktgen_setup_packets() all threads of one port uses same
info->seq_pkt. This leads to constructing packets in the same memory region
(&pkt->hdr). As a result, pktgen_setup_packets generates random headers.
Fix that by making a local copy of info->seq_pkt and using it for
constructing of packets.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
app/pktgen-arp.c | 2 +-
app/pktgen-cmds.c | 40 ++++++++++++++++++++--------------------
app/pktgen-ipv4.c | 2 +-
app/pktgen.c | 39 +++++++++++++++++++++++++++------------
app/pktgen.h | 4 ++--
app/t/pktgen.t.c | 6 +++---
6 files changed, 54 insertions(+), 39 deletions(-)
diff --git a/app/pktgen-arp.c b/app/pktgen-arp.c
index c378880..b7040d7 100644
--- a/app/pktgen-arp.c
+++ b/app/pktgen-arp.c
@@ -190,7 +190,7 @@ pktgen_process_arp( struct rte_mbuf * m, uint32_t pid, uint32_t vlan )
rte_memcpy(&pkt->eth_dst_addr, &arp->sha, 6);
for (i = 0; i < info->seqCnt; i++)
- pktgen_packet_ctor(info, i, -1);
+ pktgen_packet_ctor(info, i, -1, NULL);
}
// Swap the two MAC addresses
diff --git a/app/pktgen-cmds.c b/app/pktgen-cmds.c
index da040e5..a6abb41 100644
--- a/app/pktgen-cmds.c
+++ b/app/pktgen-cmds.c
@@ -931,7 +931,7 @@ pktgen_set_proto(port_info_t * info, char type)
if ( type == 'i' )
info->seq_pkt[SINGLE_PKT].ethType = ETHER_TYPE_IPv4;
- pktgen_packet_ctor(info, SINGLE_PKT, -1);
+ pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
}
/**************************************************************************//**
@@ -1067,7 +1067,7 @@ pktgen_set_pkt_type(port_info_t * info, const char * type)
(type[3] == '6') ? ETHER_TYPE_IPv6 :
/* TODO print error: unknown type */ ETHER_TYPE_IPv4;
- pktgen_packet_ctor(info, SINGLE_PKT, -1);
+ pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
}
/**************************************************************************//**
@@ -1092,7 +1092,7 @@ pktgen_set_vlan(port_info_t * info, uint32_t onOff)
}
else
pktgen_clr_port_flags(info, SEND_VLAN_ID);
- pktgen_packet_ctor(info, SINGLE_PKT, -1);
+ pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
}
/**************************************************************************//**
@@ -1112,7 +1112,7 @@ pktgen_set_vlanid(port_info_t * info, uint16_t vlanid)
{
info->vlanid = vlanid;
info->seq_pkt[SINGLE_PKT].vlanid = info->vlanid;
- pktgen_packet_ctor(info, SINGLE_PKT, -1);
+ pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
}
/**************************************************************************//**
@@ -1137,7 +1137,7 @@ pktgen_set_mpls(port_info_t * info, uint32_t onOff)
}
else
pktgen_clr_port_flags(info, SEND_MPLS_LABEL);
- pktgen_packet_ctor(info, SINGLE_PKT, -1);
+ pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
}
/**************************************************************************//**
@@ -1157,7 +1157,7 @@ pktgen_set_mpls_entry(port_info_t * info, uint32_t mpls_entry)
{
info->mpls_entry = mpls_entry;
info->seq_pkt[SINGLE_PKT].mpls_entry = info->mpls_entry;
- pktgen_packet_ctor(info, SINGLE_PKT, -1);
+ pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
}
/**************************************************************************//**
@@ -1182,7 +1182,7 @@ pktgen_set_qinq(port_info_t * info, uint32_t onOff)
}
else
pktgen_clr_port_flags(info, SEND_Q_IN_Q_IDS);
- pktgen_packet_ctor(info, SINGLE_PKT, -1);
+ pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
}
/**************************************************************************//**
@@ -1204,7 +1204,7 @@ pktgen_set_qinqids(port_info_t * info, uint16_t outerid, uint16_t innerid)
info->seq_pkt[SINGLE_PKT].qinq_outerid = info->qinq_outerid;
info->qinq_innerid = innerid;
info->seq_pkt[SINGLE_PKT].qinq_innerid = info->qinq_innerid;
- pktgen_packet_ctor(info, SINGLE_PKT, -1);
+ pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
}
/**************************************************************************//**
@@ -1228,7 +1228,7 @@ pktgen_set_gre(port_info_t * info, uint32_t onOff)
}
else
pktgen_clr_port_flags(info, SEND_GRE_IPv4_HEADER);
- pktgen_packet_ctor(info, SINGLE_PKT, -1);
+ pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
}
/**************************************************************************//**
@@ -1252,7 +1252,7 @@ pktgen_set_gre_eth(port_info_t * info, uint32_t onOff)
}
else
pktgen_clr_port_flags(info, SEND_GRE_ETHER_HEADER);
- pktgen_packet_ctor(info, SINGLE_PKT, -1);
+ pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
}
/**************************************************************************//**
@@ -1272,7 +1272,7 @@ pktgen_set_gre_key(port_info_t * info, uint32_t gre_key)
{
info->gre_key = gre_key;
info->seq_pkt[SINGLE_PKT].gre_key = info->gre_key;
- pktgen_packet_ctor(info, SINGLE_PKT, -1);
+ pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
}
/**************************************************************************//**
@@ -1401,7 +1401,7 @@ void pktgen_port_defaults(uint32_t pid, uint8_t seq)
memset(&pkt->eth_dst_addr, 0, sizeof (pkt->eth_dst_addr));
- pktgen_packet_ctor(info, seq, -1);
+ pktgen_packet_ctor(info, seq, -1, NULL);
pktgen.flags |= PRINT_LABELS_FLAG;
}
@@ -1423,7 +1423,7 @@ void pktgen_ping4(port_info_t * info)
memcpy(&info->seq_pkt[PING_PKT],
&info->seq_pkt[SINGLE_PKT], sizeof(pkt_seq_t));
info->seq_pkt[PING_PKT].ipProto = PG_IPPROTO_ICMP;
- pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO);
+ pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO, NULL);
pktgen_set_port_flags(info, SEND_PING4_REQUEST);
}
@@ -1445,7 +1445,7 @@ void pktgen_ping6(port_info_t * info)
memcpy(&info->pkt[PING_PKT],
&info->pkt[SINGLE_PKT], sizeof(pkt_seq_t));
info->pkt[PING_PKT].ipProto = PG_IPPROTO_ICMP;
- pktgen_packet_ctor(info, PING_PKT, ICMP6_ECHO);
+ pktgen_packet_ctor(info, PING_PKT, ICMP6_ECHO, NULL);
pktgen_set_port_flags(info, SEND_PING6_REQUEST);
}
#endif
@@ -1645,7 +1645,7 @@ void pktgen_set_pkt_size(port_info_t * info, uint32_t size)
else if ( (size - FCS_SIZE) > MAX_PKT_SIZE)
size = MAX_PKT_SIZE + FCS_SIZE;
info->seq_pkt[SINGLE_PKT].pktSize = (size - FCS_SIZE);
- pktgen_packet_ctor(info, SINGLE_PKT, -1);
+ pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
pktgen_packet_rate(info);
}
@@ -1667,7 +1667,7 @@ void pktgen_set_port_value(port_info_t * info, char type, uint32_t portValue)
info->seq_pkt[SINGLE_PKT].dport = (uint16_t)portValue;
else
info->seq_pkt[SINGLE_PKT].sport = (uint16_t)portValue;
- pktgen_packet_ctor(info, SINGLE_PKT, -1);
+ pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
}
/**************************************************************************//**
@@ -1711,7 +1711,7 @@ void pktgen_set_ipaddr(port_info_t * info, char type, cmdline_ipaddr_t * ip)
info->seq_pkt[SINGLE_PKT].ip_src_addr = ntohl(ip->addr.ipv4.s_addr);
} else
info->seq_pkt[SINGLE_PKT].ip_dst_addr = ntohl(ip->addr.ipv4.s_addr);
- pktgen_packet_ctor(info, SINGLE_PKT, -1);
+ pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
}
/**************************************************************************//**
@@ -1729,7 +1729,7 @@ void pktgen_set_ipaddr(port_info_t * info, char type, cmdline_ipaddr_t * ip)
void pktgen_set_dst_mac(port_info_t * info, cmdline_etheraddr_t * mac)
{
memcpy(&info->seq_pkt[SINGLE_PKT].eth_dst_addr, mac->mac, 6);
- pktgen_packet_ctor(info, SINGLE_PKT, -1);
+ pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
}
/**************************************************************************//**
@@ -2111,7 +2111,7 @@ void pktgen_set_seq(port_info_t * info, uint32_t seqnum,
type = '4';
pkt->ethType = (type == '6')? ETHER_TYPE_IPv6 : ETHER_TYPE_IPv4;
pkt->vlanid = vlanid;
- pktgen_packet_ctor(info, seqnum, -1);
+ pktgen_packet_ctor(info, seqnum, -1, NULL);
}
/**************************************************************************//**
@@ -2156,7 +2156,7 @@ void pktgen_compile_pkt(port_info_t * info, uint32_t seqnum,
(type == '6')? ETHER_TYPE_IPv6 :
(type == 'n')? ETHER_TYPE_VLAN : ETHER_TYPE_IPv4;
pkt->vlanid = vlanid;
- pktgen_packet_ctor(info, seqnum, -1);
+ pktgen_packet_ctor(info, seqnum, -1, NULL);
}
/**************************************************************************//**
diff --git a/app/pktgen-ipv4.c b/app/pktgen-ipv4.c
index 7813c36..a8b6be2 100644
--- a/app/pktgen-ipv4.c
+++ b/app/pktgen-ipv4.c
@@ -138,7 +138,7 @@ pktgen_send_ping4( uint32_t pid, uint8_t seq_idx )
return;
}
*ppkt = *spkt; // Copy the sequence setup to the ping setup.
- pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO);
+ pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO, NULL);
rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t *)&ppkt->hdr, ppkt->pktSize);
m->pkt_len = ppkt->pktSize;
diff --git a/app/pktgen.c b/app/pktgen.c
index 2dcdbfd..ecd4560 100644
--- a/app/pktgen.c
+++ b/app/pktgen.c
@@ -401,8 +401,8 @@ static __inline__ int pktgen_has_work(void) {
*/
void
-pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t type) {
- pkt_seq_t * pkt = &info->seq_pkt[seq_idx];
+pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t type, pkt_seq_t * seq_pkt) {
+ pkt_seq_t * pkt = (seq_pkt == NULL) ? &info->seq_pkt[seq_idx] : &seq_pkt[seq_idx];
struct ether_hdr * eth = (struct ether_hdr *)&pkt->hdr.eth;
uint16_t tlen;
@@ -812,22 +812,35 @@ static __inline__ void
pktgen_setup_packets(port_info_t * info, struct rte_mempool * mp, uint16_t qid)
{
struct rte_mbuf * m, * mm;
- pkt_seq_t * pkt;
+ pkt_seq_t * pkt, * seq_pkt = NULL;
+ uint16_t seqIdx;
+ char buff[RTE_MEMZONE_NAMESIZE];
pktgen_clr_q_flags(info, qid, CLEAR_FAST_ALLOC_FLAG);
if ( mp == info->q[qid].pcap_mp )
return;
+ snprintf(buff, sizeof(buff), "tmp_seq_pkt_%d_%d", info->pid, qid);
+ seq_pkt = (pkt_seq_t *)rte_zmalloc(buff,
+ (sizeof(pkt_seq_t) * NUM_TOTAL_PKTS), RTE_CACHE_LINE_SIZE);
+ if (seq_pkt == NULL)
+ pktgen_log_panic("Unable to allocate %d pkt_seq_t headers",
+ NUM_TOTAL_PKTS);
+ /* Copy global configuration to construct new packets locally */
+ rte_memcpy((uint8_t *)seq_pkt, (uint8_t *)info->seq_pkt,
+ sizeof(pkt_seq_t) * NUM_TOTAL_PKTS);
+ seqIdx = info->seqIdx;
+
mm = NULL;
pkt = NULL;
if ( mp == info->q[qid].tx_mp )
- pkt = &info->seq_pkt[SINGLE_PKT];
+ pkt = &seq_pkt[SINGLE_PKT];
else if ( mp == info->q[qid].range_mp )
- pkt = &info->seq_pkt[RANGE_PKT];
+ pkt = &seq_pkt[RANGE_PKT];
else if ( mp == info->q[qid].seq_mp )
- pkt = &info->seq_pkt[info->seqIdx];
+ pkt = &seq_pkt[seqIdx];
// allocate each mbuf and put them on a list to be freed.
for(;;) {
@@ -840,7 +853,7 @@ pktgen_setup_packets(port_info_t * info, struct rte_mempool * mp, uint16_t qid)
mm = m;
if ( mp == info->q[qid].tx_mp ) {
- pktgen_packet_ctor(info, SINGLE_PKT, -1);
+ pktgen_packet_ctor(info, SINGLE_PKT, -1, seq_pkt);
rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t *)&pkt->hdr, MAX_PKT_SIZE);
@@ -848,16 +861,16 @@ pktgen_setup_packets(port_info_t * info, struct rte_mempool * mp, uint16_t qid)
m->data_len = pkt->pktSize;
} else if ( mp == info->q[qid].range_mp ) {
pktgen_range_ctor(&info->range, pkt);
- pktgen_packet_ctor(info, RANGE_PKT, -1);
+ pktgen_packet_ctor(info, RANGE_PKT, -1, seq_pkt);
rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t *)&pkt->hdr, MAX_PKT_SIZE);
m->pkt_len = pkt->pktSize;
m->data_len = pkt->pktSize;
} else if ( mp == info->q[qid].seq_mp ) {
- pktgen_packet_ctor(info, info->seqIdx++, -1);
- if ( unlikely(info->seqIdx >= info->seqCnt) )
- info->seqIdx = 0;
+ pktgen_packet_ctor(info, seqIdx++, -1, seq_pkt);
+ if ( unlikely(seqIdx >= info->seqCnt) )
+ seqIdx = 0;
rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t *)&pkt->hdr, MAX_PKT_SIZE);
@@ -865,12 +878,14 @@ pktgen_setup_packets(port_info_t * info, struct rte_mempool * mp, uint16_t qid)
m->data_len = pkt->pktSize;
// move to the next packet in the sequence.
- pkt = &info->seq_pkt[info->seqIdx];
+ pkt = &seq_pkt[seqIdx];
}
}
if ( mm != NULL )
rte_pktmbuf_free(mm);
+
+ rte_free(seq_pkt);
}
/**************************************************************************//**
diff --git a/app/pktgen.h b/app/pktgen.h
index 2c5c98c..f3f76f2 100644
--- a/app/pktgen.h
+++ b/app/pktgen.h
@@ -151,7 +151,7 @@
#include "pktgen-port-cfg.h"
#include "pktgen-capture.h"
#include "pktgen-log.h"
-
+#include "pktgen-seq.h"
#define PKTGEN_VERSION "2.9.2"
#define PKTGEN_APP_NAME "Pktgen"
@@ -386,7 +386,7 @@ extern pktgen_t pktgen;
extern void pktgen_page_display(__attribute__((unused)) struct rte_timer *tim, __attribute__((unused)) void *arg);
-extern void pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t type);
+extern void pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t type, pkt_seq_t * seq_pkt);
extern void pktgen_packet_rate(port_info_t * info);
extern void pktgen_send_mbuf(struct rte_mbuf *m, uint8_t pid, uint16_t qid);
diff --git a/app/t/pktgen.t.c b/app/t/pktgen.t.c
index 0eca4c4..77bf300 100644
--- a/app/t/pktgen.t.c
+++ b/app/t/pktgen.t.c
@@ -77,7 +77,7 @@ void test_pktgen_packet_ctor_IPv4_UDP(void)
};
- lives_ok( { pktgen_packet_ctor(&info, 0, 0); }, "pktgen_packet_ctor must generate IPv4/UDP");
+ lives_ok( { pktgen_packet_ctor(&info, 0, 0, NULL); }, "pktgen_packet_ctor must generate IPv4/UDP");
note("... with Ethernet header");
cmp_mem_lit_incr(data, (0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd), " ... with correct dest MAC");
@@ -133,9 +133,9 @@ void test_pktgen_packet_ctor_IPv4_GRE_Ether(void)
.pktSize = 102, // Subtract 4 for FCS
};
- pktgen_packet_ctor(&info, 0, 0);
+ pktgen_packet_ctor(&info, 0, 0, NULL);
- lives_ok( { pktgen_packet_ctor(&info, 0, 0); }, "pktgen_packet_ctor must generate IPv4 GRE Ethernet frame");
+ lives_ok( { pktgen_packet_ctor(&info, 0, 0, NULL); }, "pktgen_packet_ctor must generate IPv4 GRE Ethernet frame");
note("... with outer Ethernet header");
cmp_mem_lit_incr(data, (0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd), " ... with correct dest MAC");
--
2.1.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [Pktgen] [PATCH] pktgen_setup_packets: fix race for packet header
2015-09-09 14:22 [dpdk-dev] [Pktgen] [PATCH] pktgen_setup_packets: fix race for packet header Ilya Maximets
@ 2015-09-16 9:09 ` Ilya Maximets
2015-09-16 15:37 ` Wiles, Keith
0 siblings, 1 reply; 7+ messages in thread
From: Ilya Maximets @ 2015-09-16 9:09 UTC (permalink / raw)
To: dev, Keith Wiles; +Cc: Dyasly Sergey
Ping.
On 09.09.2015 17:22, Ilya Maximets wrote:
> While pktgen_setup_packets() all threads of one port uses same
> info->seq_pkt. This leads to constructing packets in the same memory region
> (&pkt->hdr). As a result, pktgen_setup_packets generates random headers.
>
> Fix that by making a local copy of info->seq_pkt and using it for
> constructing of packets.
>
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
> app/pktgen-arp.c | 2 +-
> app/pktgen-cmds.c | 40 ++++++++++++++++++++--------------------
> app/pktgen-ipv4.c | 2 +-
> app/pktgen.c | 39 +++++++++++++++++++++++++++------------
> app/pktgen.h | 4 ++--
> app/t/pktgen.t.c | 6 +++---
> 6 files changed, 54 insertions(+), 39 deletions(-)
>
> diff --git a/app/pktgen-arp.c b/app/pktgen-arp.c
> index c378880..b7040d7 100644
> --- a/app/pktgen-arp.c
> +++ b/app/pktgen-arp.c
> @@ -190,7 +190,7 @@ pktgen_process_arp( struct rte_mbuf * m, uint32_t pid, uint32_t vlan )
>
> rte_memcpy(&pkt->eth_dst_addr, &arp->sha, 6);
> for (i = 0; i < info->seqCnt; i++)
> - pktgen_packet_ctor(info, i, -1);
> + pktgen_packet_ctor(info, i, -1, NULL);
> }
>
> // Swap the two MAC addresses
> diff --git a/app/pktgen-cmds.c b/app/pktgen-cmds.c
> index da040e5..a6abb41 100644
> --- a/app/pktgen-cmds.c
> +++ b/app/pktgen-cmds.c
> @@ -931,7 +931,7 @@ pktgen_set_proto(port_info_t * info, char type)
> if ( type == 'i' )
> info->seq_pkt[SINGLE_PKT].ethType = ETHER_TYPE_IPv4;
>
> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
> }
>
> /**************************************************************************//**
> @@ -1067,7 +1067,7 @@ pktgen_set_pkt_type(port_info_t * info, const char * type)
> (type[3] == '6') ? ETHER_TYPE_IPv6 :
> /* TODO print error: unknown type */ ETHER_TYPE_IPv4;
>
> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
> }
>
> /**************************************************************************//**
> @@ -1092,7 +1092,7 @@ pktgen_set_vlan(port_info_t * info, uint32_t onOff)
> }
> else
> pktgen_clr_port_flags(info, SEND_VLAN_ID);
> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
> }
>
> /**************************************************************************//**
> @@ -1112,7 +1112,7 @@ pktgen_set_vlanid(port_info_t * info, uint16_t vlanid)
> {
> info->vlanid = vlanid;
> info->seq_pkt[SINGLE_PKT].vlanid = info->vlanid;
> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
> }
>
> /**************************************************************************//**
> @@ -1137,7 +1137,7 @@ pktgen_set_mpls(port_info_t * info, uint32_t onOff)
> }
> else
> pktgen_clr_port_flags(info, SEND_MPLS_LABEL);
> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
> }
>
> /**************************************************************************//**
> @@ -1157,7 +1157,7 @@ pktgen_set_mpls_entry(port_info_t * info, uint32_t mpls_entry)
> {
> info->mpls_entry = mpls_entry;
> info->seq_pkt[SINGLE_PKT].mpls_entry = info->mpls_entry;
> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
> }
>
> /**************************************************************************//**
> @@ -1182,7 +1182,7 @@ pktgen_set_qinq(port_info_t * info, uint32_t onOff)
> }
> else
> pktgen_clr_port_flags(info, SEND_Q_IN_Q_IDS);
> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
> }
>
> /**************************************************************************//**
> @@ -1204,7 +1204,7 @@ pktgen_set_qinqids(port_info_t * info, uint16_t outerid, uint16_t innerid)
> info->seq_pkt[SINGLE_PKT].qinq_outerid = info->qinq_outerid;
> info->qinq_innerid = innerid;
> info->seq_pkt[SINGLE_PKT].qinq_innerid = info->qinq_innerid;
> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
> }
>
> /**************************************************************************//**
> @@ -1228,7 +1228,7 @@ pktgen_set_gre(port_info_t * info, uint32_t onOff)
> }
> else
> pktgen_clr_port_flags(info, SEND_GRE_IPv4_HEADER);
> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
> }
>
> /**************************************************************************//**
> @@ -1252,7 +1252,7 @@ pktgen_set_gre_eth(port_info_t * info, uint32_t onOff)
> }
> else
> pktgen_clr_port_flags(info, SEND_GRE_ETHER_HEADER);
> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
> }
>
> /**************************************************************************//**
> @@ -1272,7 +1272,7 @@ pktgen_set_gre_key(port_info_t * info, uint32_t gre_key)
> {
> info->gre_key = gre_key;
> info->seq_pkt[SINGLE_PKT].gre_key = info->gre_key;
> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
> }
>
> /**************************************************************************//**
> @@ -1401,7 +1401,7 @@ void pktgen_port_defaults(uint32_t pid, uint8_t seq)
> memset(&pkt->eth_dst_addr, 0, sizeof (pkt->eth_dst_addr));
>
>
> - pktgen_packet_ctor(info, seq, -1);
> + pktgen_packet_ctor(info, seq, -1, NULL);
>
> pktgen.flags |= PRINT_LABELS_FLAG;
> }
> @@ -1423,7 +1423,7 @@ void pktgen_ping4(port_info_t * info)
> memcpy(&info->seq_pkt[PING_PKT],
> &info->seq_pkt[SINGLE_PKT], sizeof(pkt_seq_t));
> info->seq_pkt[PING_PKT].ipProto = PG_IPPROTO_ICMP;
> - pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO);
> + pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO, NULL);
> pktgen_set_port_flags(info, SEND_PING4_REQUEST);
> }
>
> @@ -1445,7 +1445,7 @@ void pktgen_ping6(port_info_t * info)
> memcpy(&info->pkt[PING_PKT],
> &info->pkt[SINGLE_PKT], sizeof(pkt_seq_t));
> info->pkt[PING_PKT].ipProto = PG_IPPROTO_ICMP;
> - pktgen_packet_ctor(info, PING_PKT, ICMP6_ECHO);
> + pktgen_packet_ctor(info, PING_PKT, ICMP6_ECHO, NULL);
> pktgen_set_port_flags(info, SEND_PING6_REQUEST);
> }
> #endif
> @@ -1645,7 +1645,7 @@ void pktgen_set_pkt_size(port_info_t * info, uint32_t size)
> else if ( (size - FCS_SIZE) > MAX_PKT_SIZE)
> size = MAX_PKT_SIZE + FCS_SIZE;
> info->seq_pkt[SINGLE_PKT].pktSize = (size - FCS_SIZE);
> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
> pktgen_packet_rate(info);
> }
>
> @@ -1667,7 +1667,7 @@ void pktgen_set_port_value(port_info_t * info, char type, uint32_t portValue)
> info->seq_pkt[SINGLE_PKT].dport = (uint16_t)portValue;
> else
> info->seq_pkt[SINGLE_PKT].sport = (uint16_t)portValue;
> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
> }
>
> /**************************************************************************//**
> @@ -1711,7 +1711,7 @@ void pktgen_set_ipaddr(port_info_t * info, char type, cmdline_ipaddr_t * ip)
> info->seq_pkt[SINGLE_PKT].ip_src_addr = ntohl(ip->addr.ipv4.s_addr);
> } else
> info->seq_pkt[SINGLE_PKT].ip_dst_addr = ntohl(ip->addr.ipv4.s_addr);
> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
> }
>
> /**************************************************************************//**
> @@ -1729,7 +1729,7 @@ void pktgen_set_ipaddr(port_info_t * info, char type, cmdline_ipaddr_t * ip)
> void pktgen_set_dst_mac(port_info_t * info, cmdline_etheraddr_t * mac)
> {
> memcpy(&info->seq_pkt[SINGLE_PKT].eth_dst_addr, mac->mac, 6);
> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
> }
>
> /**************************************************************************//**
> @@ -2111,7 +2111,7 @@ void pktgen_set_seq(port_info_t * info, uint32_t seqnum,
> type = '4';
> pkt->ethType = (type == '6')? ETHER_TYPE_IPv6 : ETHER_TYPE_IPv4;
> pkt->vlanid = vlanid;
> - pktgen_packet_ctor(info, seqnum, -1);
> + pktgen_packet_ctor(info, seqnum, -1, NULL);
> }
>
> /**************************************************************************//**
> @@ -2156,7 +2156,7 @@ void pktgen_compile_pkt(port_info_t * info, uint32_t seqnum,
> (type == '6')? ETHER_TYPE_IPv6 :
> (type == 'n')? ETHER_TYPE_VLAN : ETHER_TYPE_IPv4;
> pkt->vlanid = vlanid;
> - pktgen_packet_ctor(info, seqnum, -1);
> + pktgen_packet_ctor(info, seqnum, -1, NULL);
> }
>
> /**************************************************************************//**
> diff --git a/app/pktgen-ipv4.c b/app/pktgen-ipv4.c
> index 7813c36..a8b6be2 100644
> --- a/app/pktgen-ipv4.c
> +++ b/app/pktgen-ipv4.c
> @@ -138,7 +138,7 @@ pktgen_send_ping4( uint32_t pid, uint8_t seq_idx )
> return;
> }
> *ppkt = *spkt; // Copy the sequence setup to the ping setup.
> - pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO);
> + pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO, NULL);
> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t *)&ppkt->hdr, ppkt->pktSize);
>
> m->pkt_len = ppkt->pktSize;
> diff --git a/app/pktgen.c b/app/pktgen.c
> index 2dcdbfd..ecd4560 100644
> --- a/app/pktgen.c
> +++ b/app/pktgen.c
> @@ -401,8 +401,8 @@ static __inline__ int pktgen_has_work(void) {
> */
>
> void
> -pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t type) {
> - pkt_seq_t * pkt = &info->seq_pkt[seq_idx];
> +pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t type, pkt_seq_t * seq_pkt) {
> + pkt_seq_t * pkt = (seq_pkt == NULL) ? &info->seq_pkt[seq_idx] : &seq_pkt[seq_idx];
> struct ether_hdr * eth = (struct ether_hdr *)&pkt->hdr.eth;
> uint16_t tlen;
>
> @@ -812,22 +812,35 @@ static __inline__ void
> pktgen_setup_packets(port_info_t * info, struct rte_mempool * mp, uint16_t qid)
> {
> struct rte_mbuf * m, * mm;
> - pkt_seq_t * pkt;
> + pkt_seq_t * pkt, * seq_pkt = NULL;
> + uint16_t seqIdx;
> + char buff[RTE_MEMZONE_NAMESIZE];
>
> pktgen_clr_q_flags(info, qid, CLEAR_FAST_ALLOC_FLAG);
>
> if ( mp == info->q[qid].pcap_mp )
> return;
>
> + snprintf(buff, sizeof(buff), "tmp_seq_pkt_%d_%d", info->pid, qid);
> + seq_pkt = (pkt_seq_t *)rte_zmalloc(buff,
> + (sizeof(pkt_seq_t) * NUM_TOTAL_PKTS), RTE_CACHE_LINE_SIZE);
> + if (seq_pkt == NULL)
> + pktgen_log_panic("Unable to allocate %d pkt_seq_t headers",
> + NUM_TOTAL_PKTS);
> + /* Copy global configuration to construct new packets locally */
> + rte_memcpy((uint8_t *)seq_pkt, (uint8_t *)info->seq_pkt,
> + sizeof(pkt_seq_t) * NUM_TOTAL_PKTS);
> + seqIdx = info->seqIdx;
> +
> mm = NULL;
> pkt = NULL;
>
> if ( mp == info->q[qid].tx_mp )
> - pkt = &info->seq_pkt[SINGLE_PKT];
> + pkt = &seq_pkt[SINGLE_PKT];
> else if ( mp == info->q[qid].range_mp )
> - pkt = &info->seq_pkt[RANGE_PKT];
> + pkt = &seq_pkt[RANGE_PKT];
> else if ( mp == info->q[qid].seq_mp )
> - pkt = &info->seq_pkt[info->seqIdx];
> + pkt = &seq_pkt[seqIdx];
>
> // allocate each mbuf and put them on a list to be freed.
> for(;;) {
> @@ -840,7 +853,7 @@ pktgen_setup_packets(port_info_t * info, struct rte_mempool * mp, uint16_t qid)
> mm = m;
>
> if ( mp == info->q[qid].tx_mp ) {
> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
> + pktgen_packet_ctor(info, SINGLE_PKT, -1, seq_pkt);
>
> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t *)&pkt->hdr, MAX_PKT_SIZE);
>
> @@ -848,16 +861,16 @@ pktgen_setup_packets(port_info_t * info, struct rte_mempool * mp, uint16_t qid)
> m->data_len = pkt->pktSize;
> } else if ( mp == info->q[qid].range_mp ) {
> pktgen_range_ctor(&info->range, pkt);
> - pktgen_packet_ctor(info, RANGE_PKT, -1);
> + pktgen_packet_ctor(info, RANGE_PKT, -1, seq_pkt);
>
> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t *)&pkt->hdr, MAX_PKT_SIZE);
>
> m->pkt_len = pkt->pktSize;
> m->data_len = pkt->pktSize;
> } else if ( mp == info->q[qid].seq_mp ) {
> - pktgen_packet_ctor(info, info->seqIdx++, -1);
> - if ( unlikely(info->seqIdx >= info->seqCnt) )
> - info->seqIdx = 0;
> + pktgen_packet_ctor(info, seqIdx++, -1, seq_pkt);
> + if ( unlikely(seqIdx >= info->seqCnt) )
> + seqIdx = 0;
>
> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t *)&pkt->hdr, MAX_PKT_SIZE);
>
> @@ -865,12 +878,14 @@ pktgen_setup_packets(port_info_t * info, struct rte_mempool * mp, uint16_t qid)
> m->data_len = pkt->pktSize;
>
> // move to the next packet in the sequence.
> - pkt = &info->seq_pkt[info->seqIdx];
> + pkt = &seq_pkt[seqIdx];
> }
> }
>
> if ( mm != NULL )
> rte_pktmbuf_free(mm);
> +
> + rte_free(seq_pkt);
> }
>
> /**************************************************************************//**
> diff --git a/app/pktgen.h b/app/pktgen.h
> index 2c5c98c..f3f76f2 100644
> --- a/app/pktgen.h
> +++ b/app/pktgen.h
> @@ -151,7 +151,7 @@
> #include "pktgen-port-cfg.h"
> #include "pktgen-capture.h"
> #include "pktgen-log.h"
> -
> +#include "pktgen-seq.h"
>
> #define PKTGEN_VERSION "2.9.2"
> #define PKTGEN_APP_NAME "Pktgen"
> @@ -386,7 +386,7 @@ extern pktgen_t pktgen;
>
> extern void pktgen_page_display(__attribute__((unused)) struct rte_timer *tim, __attribute__((unused)) void *arg);
>
> -extern void pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t type);
> +extern void pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t type, pkt_seq_t * seq_pkt);
> extern void pktgen_packet_rate(port_info_t * info);
>
> extern void pktgen_send_mbuf(struct rte_mbuf *m, uint8_t pid, uint16_t qid);
> diff --git a/app/t/pktgen.t.c b/app/t/pktgen.t.c
> index 0eca4c4..77bf300 100644
> --- a/app/t/pktgen.t.c
> +++ b/app/t/pktgen.t.c
> @@ -77,7 +77,7 @@ void test_pktgen_packet_ctor_IPv4_UDP(void)
> };
>
>
> - lives_ok( { pktgen_packet_ctor(&info, 0, 0); }, "pktgen_packet_ctor must generate IPv4/UDP");
> + lives_ok( { pktgen_packet_ctor(&info, 0, 0, NULL); }, "pktgen_packet_ctor must generate IPv4/UDP");
>
> note("... with Ethernet header");
> cmp_mem_lit_incr(data, (0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd), " ... with correct dest MAC");
> @@ -133,9 +133,9 @@ void test_pktgen_packet_ctor_IPv4_GRE_Ether(void)
> .pktSize = 102, // Subtract 4 for FCS
> };
>
> - pktgen_packet_ctor(&info, 0, 0);
> + pktgen_packet_ctor(&info, 0, 0, NULL);
>
> - lives_ok( { pktgen_packet_ctor(&info, 0, 0); }, "pktgen_packet_ctor must generate IPv4 GRE Ethernet frame");
> + lives_ok( { pktgen_packet_ctor(&info, 0, 0, NULL); }, "pktgen_packet_ctor must generate IPv4 GRE Ethernet frame");
>
> note("... with outer Ethernet header");
> cmp_mem_lit_incr(data, (0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd), " ... with correct dest MAC");
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [Pktgen] [PATCH] pktgen_setup_packets: fix race for packet header
2015-09-16 9:09 ` Ilya Maximets
@ 2015-09-16 15:37 ` Wiles, Keith
2015-09-17 5:55 ` Ilya Maximets
0 siblings, 1 reply; 7+ messages in thread
From: Wiles, Keith @ 2015-09-16 15:37 UTC (permalink / raw)
To: Ilya Maximets, dev; +Cc: Dyasly Sergey
Thanks the patch looks fine, but I have not had a lot of time to review it
detail. I hope to get to it next week after I return back home.
On 9/16/15, 2:09 AM, "Ilya Maximets" <i.maximets@samsung.com> wrote:
>Ping.
>
>On 09.09.2015 17:22, Ilya Maximets wrote:
>> While pktgen_setup_packets() all threads of one port uses same
>> info->seq_pkt. This leads to constructing packets in the same memory
>>region
>> (&pkt->hdr). As a result, pktgen_setup_packets generates random headers.
>>
>> Fix that by making a local copy of info->seq_pkt and using it for
>> constructing of packets.
>>
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>> ---
>> app/pktgen-arp.c | 2 +-
>> app/pktgen-cmds.c | 40 ++++++++++++++++++++--------------------
>> app/pktgen-ipv4.c | 2 +-
>> app/pktgen.c | 39 +++++++++++++++++++++++++++------------
>> app/pktgen.h | 4 ++--
>> app/t/pktgen.t.c | 6 +++---
>> 6 files changed, 54 insertions(+), 39 deletions(-)
>>
>> diff --git a/app/pktgen-arp.c b/app/pktgen-arp.c
>> index c378880..b7040d7 100644
>> --- a/app/pktgen-arp.c
>> +++ b/app/pktgen-arp.c
>> @@ -190,7 +190,7 @@ pktgen_process_arp( struct rte_mbuf * m, uint32_t
>>pid, uint32_t vlan )
>>
>> rte_memcpy(&pkt->eth_dst_addr, &arp->sha, 6);
>> for (i = 0; i < info->seqCnt; i++)
>> - pktgen_packet_ctor(info, i, -1);
>> + pktgen_packet_ctor(info, i, -1, NULL);
>> }
>>
>> // Swap the two MAC addresses
>> diff --git a/app/pktgen-cmds.c b/app/pktgen-cmds.c
>> index da040e5..a6abb41 100644
>> --- a/app/pktgen-cmds.c
>> +++ b/app/pktgen-cmds.c
>> @@ -931,7 +931,7 @@ pktgen_set_proto(port_info_t * info, char type)
>> if ( type == 'i' )
>> info->seq_pkt[SINGLE_PKT].ethType = ETHER_TYPE_IPv4;
>>
>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>> }
>>
>>
>>/************************************************************************
>>**//**
>> @@ -1067,7 +1067,7 @@ pktgen_set_pkt_type(port_info_t * info, const
>>char * type)
>> (type[3] == '6') ? ETHER_TYPE_IPv6 :
>> /* TODO print error: unknown type */ ETHER_TYPE_IPv4;
>>
>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>> }
>>
>>
>>/************************************************************************
>>**//**
>> @@ -1092,7 +1092,7 @@ pktgen_set_vlan(port_info_t * info, uint32_t
>>onOff)
>> }
>> else
>> pktgen_clr_port_flags(info, SEND_VLAN_ID);
>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>> }
>>
>>
>>/************************************************************************
>>**//**
>> @@ -1112,7 +1112,7 @@ pktgen_set_vlanid(port_info_t * info, uint16_t
>>vlanid)
>> {
>> info->vlanid = vlanid;
>> info->seq_pkt[SINGLE_PKT].vlanid = info->vlanid;
>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>> }
>>
>>
>>/************************************************************************
>>**//**
>> @@ -1137,7 +1137,7 @@ pktgen_set_mpls(port_info_t * info, uint32_t
>>onOff)
>> }
>> else
>> pktgen_clr_port_flags(info, SEND_MPLS_LABEL);
>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>> }
>>
>>
>>/************************************************************************
>>**//**
>> @@ -1157,7 +1157,7 @@ pktgen_set_mpls_entry(port_info_t * info,
>>uint32_t mpls_entry)
>> {
>> info->mpls_entry = mpls_entry;
>> info->seq_pkt[SINGLE_PKT].mpls_entry = info->mpls_entry;
>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>> }
>>
>>
>>/************************************************************************
>>**//**
>> @@ -1182,7 +1182,7 @@ pktgen_set_qinq(port_info_t * info, uint32_t
>>onOff)
>> }
>> else
>> pktgen_clr_port_flags(info, SEND_Q_IN_Q_IDS);
>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>> }
>>
>>
>>/************************************************************************
>>**//**
>> @@ -1204,7 +1204,7 @@ pktgen_set_qinqids(port_info_t * info, uint16_t
>>outerid, uint16_t innerid)
>> info->seq_pkt[SINGLE_PKT].qinq_outerid = info->qinq_outerid;
>> info->qinq_innerid = innerid;
>> info->seq_pkt[SINGLE_PKT].qinq_innerid = info->qinq_innerid;
>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>> }
>>
>>
>>/************************************************************************
>>**//**
>> @@ -1228,7 +1228,7 @@ pktgen_set_gre(port_info_t * info, uint32_t onOff)
>> }
>> else
>> pktgen_clr_port_flags(info, SEND_GRE_IPv4_HEADER);
>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>> }
>>
>>
>>/************************************************************************
>>**//**
>> @@ -1252,7 +1252,7 @@ pktgen_set_gre_eth(port_info_t * info, uint32_t
>>onOff)
>> }
>> else
>> pktgen_clr_port_flags(info, SEND_GRE_ETHER_HEADER);
>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>> }
>>
>>
>>/************************************************************************
>>**//**
>> @@ -1272,7 +1272,7 @@ pktgen_set_gre_key(port_info_t * info, uint32_t
>>gre_key)
>> {
>> info->gre_key = gre_key;
>> info->seq_pkt[SINGLE_PKT].gre_key = info->gre_key;
>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>> }
>>
>>
>>/************************************************************************
>>**//**
>> @@ -1401,7 +1401,7 @@ void pktgen_port_defaults(uint32_t pid, uint8_t
>>seq)
>> memset(&pkt->eth_dst_addr, 0, sizeof (pkt->eth_dst_addr));
>>
>>
>> - pktgen_packet_ctor(info, seq, -1);
>> + pktgen_packet_ctor(info, seq, -1, NULL);
>>
>> pktgen.flags |= PRINT_LABELS_FLAG;
>> }
>> @@ -1423,7 +1423,7 @@ void pktgen_ping4(port_info_t * info)
>> memcpy(&info->seq_pkt[PING_PKT],
>> &info->seq_pkt[SINGLE_PKT], sizeof(pkt_seq_t));
>> info->seq_pkt[PING_PKT].ipProto = PG_IPPROTO_ICMP;
>> - pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO);
>> + pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO, NULL);
>> pktgen_set_port_flags(info, SEND_PING4_REQUEST);
>> }
>>
>> @@ -1445,7 +1445,7 @@ void pktgen_ping6(port_info_t * info)
>> memcpy(&info->pkt[PING_PKT],
>> &info->pkt[SINGLE_PKT], sizeof(pkt_seq_t));
>> info->pkt[PING_PKT].ipProto = PG_IPPROTO_ICMP;
>> - pktgen_packet_ctor(info, PING_PKT, ICMP6_ECHO);
>> + pktgen_packet_ctor(info, PING_PKT, ICMP6_ECHO, NULL);
>> pktgen_set_port_flags(info, SEND_PING6_REQUEST);
>> }
>> #endif
>> @@ -1645,7 +1645,7 @@ void pktgen_set_pkt_size(port_info_t * info,
>>uint32_t size)
>> else if ( (size - FCS_SIZE) > MAX_PKT_SIZE)
>> size = MAX_PKT_SIZE + FCS_SIZE;
>> info->seq_pkt[SINGLE_PKT].pktSize = (size - FCS_SIZE);
>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>> pktgen_packet_rate(info);
>> }
>>
>> @@ -1667,7 +1667,7 @@ void pktgen_set_port_value(port_info_t * info,
>>char type, uint32_t portValue)
>> info->seq_pkt[SINGLE_PKT].dport = (uint16_t)portValue;
>> else
>> info->seq_pkt[SINGLE_PKT].sport = (uint16_t)portValue;
>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>> }
>>
>>
>>/************************************************************************
>>**//**
>> @@ -1711,7 +1711,7 @@ void pktgen_set_ipaddr(port_info_t * info, char
>>type, cmdline_ipaddr_t * ip)
>> info->seq_pkt[SINGLE_PKT].ip_src_addr = ntohl(ip->addr.ipv4.s_addr);
>> } else
>> info->seq_pkt[SINGLE_PKT].ip_dst_addr = ntohl(ip->addr.ipv4.s_addr);
>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>> }
>>
>>
>>/************************************************************************
>>**//**
>> @@ -1729,7 +1729,7 @@ void pktgen_set_ipaddr(port_info_t * info, char
>>type, cmdline_ipaddr_t * ip)
>> void pktgen_set_dst_mac(port_info_t * info, cmdline_etheraddr_t * mac)
>> {
>> memcpy(&info->seq_pkt[SINGLE_PKT].eth_dst_addr, mac->mac, 6);
>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>> }
>>
>>
>>/************************************************************************
>>**//**
>> @@ -2111,7 +2111,7 @@ void pktgen_set_seq(port_info_t * info, uint32_t
>>seqnum,
>> type = '4';
>> pkt->ethType = (type == '6')? ETHER_TYPE_IPv6 : ETHER_TYPE_IPv4;
>> pkt->vlanid = vlanid;
>> - pktgen_packet_ctor(info, seqnum, -1);
>> + pktgen_packet_ctor(info, seqnum, -1, NULL);
>> }
>>
>>
>>/************************************************************************
>>**//**
>> @@ -2156,7 +2156,7 @@ void pktgen_compile_pkt(port_info_t * info,
>>uint32_t seqnum,
>> (type == '6')? ETHER_TYPE_IPv6 :
>> (type == 'n')? ETHER_TYPE_VLAN : ETHER_TYPE_IPv4;
>> pkt->vlanid = vlanid;
>> - pktgen_packet_ctor(info, seqnum, -1);
>> + pktgen_packet_ctor(info, seqnum, -1, NULL);
>> }
>>
>>
>>/************************************************************************
>>**//**
>> diff --git a/app/pktgen-ipv4.c b/app/pktgen-ipv4.c
>> index 7813c36..a8b6be2 100644
>> --- a/app/pktgen-ipv4.c
>> +++ b/app/pktgen-ipv4.c
>> @@ -138,7 +138,7 @@ pktgen_send_ping4( uint32_t pid, uint8_t seq_idx )
>> return;
>> }
>> *ppkt = *spkt; // Copy the sequence setup to the ping setup.
>> - pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO);
>> + pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO, NULL);
>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>*)&ppkt->hdr, ppkt->pktSize);
>>
>> m->pkt_len = ppkt->pktSize;
>> diff --git a/app/pktgen.c b/app/pktgen.c
>> index 2dcdbfd..ecd4560 100644
>> --- a/app/pktgen.c
>> +++ b/app/pktgen.c
>> @@ -401,8 +401,8 @@ static __inline__ int pktgen_has_work(void) {
>> */
>>
>> void
>> -pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t type) {
>> - pkt_seq_t * pkt = &info->seq_pkt[seq_idx];
>> +pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t type,
>>pkt_seq_t * seq_pkt) {
>> + pkt_seq_t * pkt = (seq_pkt == NULL) ? &info->seq_pkt[seq_idx] :
>>&seq_pkt[seq_idx];
>> struct ether_hdr * eth = (struct ether_hdr *)&pkt->hdr.eth;
>> uint16_t tlen;
>>
>> @@ -812,22 +812,35 @@ static __inline__ void
>> pktgen_setup_packets(port_info_t * info, struct rte_mempool * mp,
>>uint16_t qid)
>> {
>> struct rte_mbuf * m, * mm;
>> - pkt_seq_t * pkt;
>> + pkt_seq_t * pkt, * seq_pkt = NULL;
>> + uint16_t seqIdx;
>> + char buff[RTE_MEMZONE_NAMESIZE];
>>
>> pktgen_clr_q_flags(info, qid, CLEAR_FAST_ALLOC_FLAG);
>>
>> if ( mp == info->q[qid].pcap_mp )
>> return;
>>
>> + snprintf(buff, sizeof(buff), "tmp_seq_pkt_%d_%d", info->pid, qid);
>> + seq_pkt = (pkt_seq_t *)rte_zmalloc(buff,
>> + (sizeof(pkt_seq_t) * NUM_TOTAL_PKTS), RTE_CACHE_LINE_SIZE);
>> + if (seq_pkt == NULL)
>> + pktgen_log_panic("Unable to allocate %d pkt_seq_t headers",
>> + NUM_TOTAL_PKTS);
>> + /* Copy global configuration to construct new packets locally */
>> + rte_memcpy((uint8_t *)seq_pkt, (uint8_t *)info->seq_pkt,
>> + sizeof(pkt_seq_t) * NUM_TOTAL_PKTS);
>> + seqIdx = info->seqIdx;
>> +
>> mm = NULL;
>> pkt = NULL;
>>
>> if ( mp == info->q[qid].tx_mp )
>> - pkt = &info->seq_pkt[SINGLE_PKT];
>> + pkt = &seq_pkt[SINGLE_PKT];
>> else if ( mp == info->q[qid].range_mp )
>> - pkt = &info->seq_pkt[RANGE_PKT];
>> + pkt = &seq_pkt[RANGE_PKT];
>> else if ( mp == info->q[qid].seq_mp )
>> - pkt = &info->seq_pkt[info->seqIdx];
>> + pkt = &seq_pkt[seqIdx];
>>
>> // allocate each mbuf and put them on a list to be freed.
>> for(;;) {
>> @@ -840,7 +853,7 @@ pktgen_setup_packets(port_info_t * info, struct
>>rte_mempool * mp, uint16_t qid)
>> mm = m;
>>
>> if ( mp == info->q[qid].tx_mp ) {
>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, seq_pkt);
>>
>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>*)&pkt->hdr, MAX_PKT_SIZE);
>>
>> @@ -848,16 +861,16 @@ pktgen_setup_packets(port_info_t * info, struct
>>rte_mempool * mp, uint16_t qid)
>> m->data_len = pkt->pktSize;
>> } else if ( mp == info->q[qid].range_mp ) {
>> pktgen_range_ctor(&info->range, pkt);
>> - pktgen_packet_ctor(info, RANGE_PKT, -1);
>> + pktgen_packet_ctor(info, RANGE_PKT, -1, seq_pkt);
>>
>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>*)&pkt->hdr, MAX_PKT_SIZE);
>>
>> m->pkt_len = pkt->pktSize;
>> m->data_len = pkt->pktSize;
>> } else if ( mp == info->q[qid].seq_mp ) {
>> - pktgen_packet_ctor(info, info->seqIdx++, -1);
>> - if ( unlikely(info->seqIdx >= info->seqCnt) )
>> - info->seqIdx = 0;
>> + pktgen_packet_ctor(info, seqIdx++, -1, seq_pkt);
>> + if ( unlikely(seqIdx >= info->seqCnt) )
>> + seqIdx = 0;
>>
>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>*)&pkt->hdr, MAX_PKT_SIZE);
>>
>> @@ -865,12 +878,14 @@ pktgen_setup_packets(port_info_t * info, struct
>>rte_mempool * mp, uint16_t qid)
>> m->data_len = pkt->pktSize;
>>
>> // move to the next packet in the sequence.
>> - pkt = &info->seq_pkt[info->seqIdx];
>> + pkt = &seq_pkt[seqIdx];
>> }
>> }
>>
>> if ( mm != NULL )
>> rte_pktmbuf_free(mm);
>> +
>> + rte_free(seq_pkt);
>> }
>>
>>
>>/************************************************************************
>>**//**
>> diff --git a/app/pktgen.h b/app/pktgen.h
>> index 2c5c98c..f3f76f2 100644
>> --- a/app/pktgen.h
>> +++ b/app/pktgen.h
>> @@ -151,7 +151,7 @@
>> #include "pktgen-port-cfg.h"
>> #include "pktgen-capture.h"
>> #include "pktgen-log.h"
>> -
>> +#include "pktgen-seq.h"
>>
>> #define PKTGEN_VERSION "2.9.2"
>> #define PKTGEN_APP_NAME "Pktgen"
>> @@ -386,7 +386,7 @@ extern pktgen_t pktgen;
>>
>> extern void pktgen_page_display(__attribute__((unused)) struct
>>rte_timer *tim, __attribute__((unused)) void *arg);
>>
>> -extern void pktgen_packet_ctor(port_info_t * info, int32_t seq_idx,
>>int32_t type);
>> +extern void pktgen_packet_ctor(port_info_t * info, int32_t seq_idx,
>>int32_t type, pkt_seq_t * seq_pkt);
>> extern void pktgen_packet_rate(port_info_t * info);
>>
>> extern void pktgen_send_mbuf(struct rte_mbuf *m, uint8_t pid, uint16_t
>>qid);
>> diff --git a/app/t/pktgen.t.c b/app/t/pktgen.t.c
>> index 0eca4c4..77bf300 100644
>> --- a/app/t/pktgen.t.c
>> +++ b/app/t/pktgen.t.c
>> @@ -77,7 +77,7 @@ void test_pktgen_packet_ctor_IPv4_UDP(void)
>> };
>>
>>
>> - lives_ok( { pktgen_packet_ctor(&info, 0, 0); }, "pktgen_packet_ctor
>>must generate IPv4/UDP");
>> + lives_ok( { pktgen_packet_ctor(&info, 0, 0, NULL); },
>>"pktgen_packet_ctor must generate IPv4/UDP");
>>
>> note("... with Ethernet header");
>> cmp_mem_lit_incr(data, (0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd), " ...
>>with correct dest MAC");
>> @@ -133,9 +133,9 @@ void test_pktgen_packet_ctor_IPv4_GRE_Ether(void)
>> .pktSize = 102, // Subtract 4 for FCS
>> };
>>
>> - pktgen_packet_ctor(&info, 0, 0);
>> + pktgen_packet_ctor(&info, 0, 0, NULL);
>>
>> - lives_ok( { pktgen_packet_ctor(&info, 0, 0); }, "pktgen_packet_ctor
>>must generate IPv4 GRE Ethernet frame");
>> + lives_ok( { pktgen_packet_ctor(&info, 0, 0, NULL); },
>>"pktgen_packet_ctor must generate IPv4 GRE Ethernet frame");
>>
>> note("... with outer Ethernet header");
>> cmp_mem_lit_incr(data, (0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd), " ...
>>with correct dest MAC");
>>
>
‹
Regards,
++Keith
Intel Corporation
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [Pktgen] [PATCH] pktgen_setup_packets: fix race for packet header
2015-09-16 15:37 ` Wiles, Keith
@ 2015-09-17 5:55 ` Ilya Maximets
2015-10-02 9:23 ` Ilya Maximets
0 siblings, 1 reply; 7+ messages in thread
From: Ilya Maximets @ 2015-09-17 5:55 UTC (permalink / raw)
To: Wiles, Keith, dev; +Cc: Dyasly Sergey
Ok. Thank you. I'll wait.
On 16.09.2015 18:37, Wiles, Keith wrote:
> Thanks the patch looks fine, but I have not had a lot of time to review it
> detail. I hope to get to it next week after I return back home.
>
> On 9/16/15, 2:09 AM, "Ilya Maximets" <i.maximets@samsung.com> wrote:
>
>> Ping.
>>
>> On 09.09.2015 17:22, Ilya Maximets wrote:
>>> While pktgen_setup_packets() all threads of one port uses same
>>> info->seq_pkt. This leads to constructing packets in the same memory
>>> region
>>> (&pkt->hdr). As a result, pktgen_setup_packets generates random headers.
>>>
>>> Fix that by making a local copy of info->seq_pkt and using it for
>>> constructing of packets.
>>>
>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>>> ---
>>> app/pktgen-arp.c | 2 +-
>>> app/pktgen-cmds.c | 40 ++++++++++++++++++++--------------------
>>> app/pktgen-ipv4.c | 2 +-
>>> app/pktgen.c | 39 +++++++++++++++++++++++++++------------
>>> app/pktgen.h | 4 ++--
>>> app/t/pktgen.t.c | 6 +++---
>>> 6 files changed, 54 insertions(+), 39 deletions(-)
>>>
>>> diff --git a/app/pktgen-arp.c b/app/pktgen-arp.c
>>> index c378880..b7040d7 100644
>>> --- a/app/pktgen-arp.c
>>> +++ b/app/pktgen-arp.c
>>> @@ -190,7 +190,7 @@ pktgen_process_arp( struct rte_mbuf * m, uint32_t
>>> pid, uint32_t vlan )
>>>
>>> rte_memcpy(&pkt->eth_dst_addr, &arp->sha, 6);
>>> for (i = 0; i < info->seqCnt; i++)
>>> - pktgen_packet_ctor(info, i, -1);
>>> + pktgen_packet_ctor(info, i, -1, NULL);
>>> }
>>>
>>> // Swap the two MAC addresses
>>> diff --git a/app/pktgen-cmds.c b/app/pktgen-cmds.c
>>> index da040e5..a6abb41 100644
>>> --- a/app/pktgen-cmds.c
>>> +++ b/app/pktgen-cmds.c
>>> @@ -931,7 +931,7 @@ pktgen_set_proto(port_info_t * info, char type)
>>> if ( type == 'i' )
>>> info->seq_pkt[SINGLE_PKT].ethType = ETHER_TYPE_IPv4;
>>>
>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>> }
>>>
>>>
>>> /************************************************************************
>>> **//**
>>> @@ -1067,7 +1067,7 @@ pktgen_set_pkt_type(port_info_t * info, const
>>> char * type)
>>> (type[3] == '6') ? ETHER_TYPE_IPv6 :
>>> /* TODO print error: unknown type */ ETHER_TYPE_IPv4;
>>>
>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>> }
>>>
>>>
>>> /************************************************************************
>>> **//**
>>> @@ -1092,7 +1092,7 @@ pktgen_set_vlan(port_info_t * info, uint32_t
>>> onOff)
>>> }
>>> else
>>> pktgen_clr_port_flags(info, SEND_VLAN_ID);
>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>> }
>>>
>>>
>>> /************************************************************************
>>> **//**
>>> @@ -1112,7 +1112,7 @@ pktgen_set_vlanid(port_info_t * info, uint16_t
>>> vlanid)
>>> {
>>> info->vlanid = vlanid;
>>> info->seq_pkt[SINGLE_PKT].vlanid = info->vlanid;
>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>> }
>>>
>>>
>>> /************************************************************************
>>> **//**
>>> @@ -1137,7 +1137,7 @@ pktgen_set_mpls(port_info_t * info, uint32_t
>>> onOff)
>>> }
>>> else
>>> pktgen_clr_port_flags(info, SEND_MPLS_LABEL);
>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>> }
>>>
>>>
>>> /************************************************************************
>>> **//**
>>> @@ -1157,7 +1157,7 @@ pktgen_set_mpls_entry(port_info_t * info,
>>> uint32_t mpls_entry)
>>> {
>>> info->mpls_entry = mpls_entry;
>>> info->seq_pkt[SINGLE_PKT].mpls_entry = info->mpls_entry;
>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>> }
>>>
>>>
>>> /************************************************************************
>>> **//**
>>> @@ -1182,7 +1182,7 @@ pktgen_set_qinq(port_info_t * info, uint32_t
>>> onOff)
>>> }
>>> else
>>> pktgen_clr_port_flags(info, SEND_Q_IN_Q_IDS);
>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>> }
>>>
>>>
>>> /************************************************************************
>>> **//**
>>> @@ -1204,7 +1204,7 @@ pktgen_set_qinqids(port_info_t * info, uint16_t
>>> outerid, uint16_t innerid)
>>> info->seq_pkt[SINGLE_PKT].qinq_outerid = info->qinq_outerid;
>>> info->qinq_innerid = innerid;
>>> info->seq_pkt[SINGLE_PKT].qinq_innerid = info->qinq_innerid;
>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>> }
>>>
>>>
>>> /************************************************************************
>>> **//**
>>> @@ -1228,7 +1228,7 @@ pktgen_set_gre(port_info_t * info, uint32_t onOff)
>>> }
>>> else
>>> pktgen_clr_port_flags(info, SEND_GRE_IPv4_HEADER);
>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>> }
>>>
>>>
>>> /************************************************************************
>>> **//**
>>> @@ -1252,7 +1252,7 @@ pktgen_set_gre_eth(port_info_t * info, uint32_t
>>> onOff)
>>> }
>>> else
>>> pktgen_clr_port_flags(info, SEND_GRE_ETHER_HEADER);
>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>> }
>>>
>>>
>>> /************************************************************************
>>> **//**
>>> @@ -1272,7 +1272,7 @@ pktgen_set_gre_key(port_info_t * info, uint32_t
>>> gre_key)
>>> {
>>> info->gre_key = gre_key;
>>> info->seq_pkt[SINGLE_PKT].gre_key = info->gre_key;
>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>> }
>>>
>>>
>>> /************************************************************************
>>> **//**
>>> @@ -1401,7 +1401,7 @@ void pktgen_port_defaults(uint32_t pid, uint8_t
>>> seq)
>>> memset(&pkt->eth_dst_addr, 0, sizeof (pkt->eth_dst_addr));
>>>
>>>
>>> - pktgen_packet_ctor(info, seq, -1);
>>> + pktgen_packet_ctor(info, seq, -1, NULL);
>>>
>>> pktgen.flags |= PRINT_LABELS_FLAG;
>>> }
>>> @@ -1423,7 +1423,7 @@ void pktgen_ping4(port_info_t * info)
>>> memcpy(&info->seq_pkt[PING_PKT],
>>> &info->seq_pkt[SINGLE_PKT], sizeof(pkt_seq_t));
>>> info->seq_pkt[PING_PKT].ipProto = PG_IPPROTO_ICMP;
>>> - pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO);
>>> + pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO, NULL);
>>> pktgen_set_port_flags(info, SEND_PING4_REQUEST);
>>> }
>>>
>>> @@ -1445,7 +1445,7 @@ void pktgen_ping6(port_info_t * info)
>>> memcpy(&info->pkt[PING_PKT],
>>> &info->pkt[SINGLE_PKT], sizeof(pkt_seq_t));
>>> info->pkt[PING_PKT].ipProto = PG_IPPROTO_ICMP;
>>> - pktgen_packet_ctor(info, PING_PKT, ICMP6_ECHO);
>>> + pktgen_packet_ctor(info, PING_PKT, ICMP6_ECHO, NULL);
>>> pktgen_set_port_flags(info, SEND_PING6_REQUEST);
>>> }
>>> #endif
>>> @@ -1645,7 +1645,7 @@ void pktgen_set_pkt_size(port_info_t * info,
>>> uint32_t size)
>>> else if ( (size - FCS_SIZE) > MAX_PKT_SIZE)
>>> size = MAX_PKT_SIZE + FCS_SIZE;
>>> info->seq_pkt[SINGLE_PKT].pktSize = (size - FCS_SIZE);
>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>> pktgen_packet_rate(info);
>>> }
>>>
>>> @@ -1667,7 +1667,7 @@ void pktgen_set_port_value(port_info_t * info,
>>> char type, uint32_t portValue)
>>> info->seq_pkt[SINGLE_PKT].dport = (uint16_t)portValue;
>>> else
>>> info->seq_pkt[SINGLE_PKT].sport = (uint16_t)portValue;
>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>> }
>>>
>>>
>>> /************************************************************************
>>> **//**
>>> @@ -1711,7 +1711,7 @@ void pktgen_set_ipaddr(port_info_t * info, char
>>> type, cmdline_ipaddr_t * ip)
>>> info->seq_pkt[SINGLE_PKT].ip_src_addr = ntohl(ip->addr.ipv4.s_addr);
>>> } else
>>> info->seq_pkt[SINGLE_PKT].ip_dst_addr = ntohl(ip->addr.ipv4.s_addr);
>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>> }
>>>
>>>
>>> /************************************************************************
>>> **//**
>>> @@ -1729,7 +1729,7 @@ void pktgen_set_ipaddr(port_info_t * info, char
>>> type, cmdline_ipaddr_t * ip)
>>> void pktgen_set_dst_mac(port_info_t * info, cmdline_etheraddr_t * mac)
>>> {
>>> memcpy(&info->seq_pkt[SINGLE_PKT].eth_dst_addr, mac->mac, 6);
>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>> }
>>>
>>>
>>> /************************************************************************
>>> **//**
>>> @@ -2111,7 +2111,7 @@ void pktgen_set_seq(port_info_t * info, uint32_t
>>> seqnum,
>>> type = '4';
>>> pkt->ethType = (type == '6')? ETHER_TYPE_IPv6 : ETHER_TYPE_IPv4;
>>> pkt->vlanid = vlanid;
>>> - pktgen_packet_ctor(info, seqnum, -1);
>>> + pktgen_packet_ctor(info, seqnum, -1, NULL);
>>> }
>>>
>>>
>>> /************************************************************************
>>> **//**
>>> @@ -2156,7 +2156,7 @@ void pktgen_compile_pkt(port_info_t * info,
>>> uint32_t seqnum,
>>> (type == '6')? ETHER_TYPE_IPv6 :
>>> (type == 'n')? ETHER_TYPE_VLAN : ETHER_TYPE_IPv4;
>>> pkt->vlanid = vlanid;
>>> - pktgen_packet_ctor(info, seqnum, -1);
>>> + pktgen_packet_ctor(info, seqnum, -1, NULL);
>>> }
>>>
>>>
>>> /************************************************************************
>>> **//**
>>> diff --git a/app/pktgen-ipv4.c b/app/pktgen-ipv4.c
>>> index 7813c36..a8b6be2 100644
>>> --- a/app/pktgen-ipv4.c
>>> +++ b/app/pktgen-ipv4.c
>>> @@ -138,7 +138,7 @@ pktgen_send_ping4( uint32_t pid, uint8_t seq_idx )
>>> return;
>>> }
>>> *ppkt = *spkt; // Copy the sequence setup to the ping setup.
>>> - pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO);
>>> + pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO, NULL);
>>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>> *)&ppkt->hdr, ppkt->pktSize);
>>>
>>> m->pkt_len = ppkt->pktSize;
>>> diff --git a/app/pktgen.c b/app/pktgen.c
>>> index 2dcdbfd..ecd4560 100644
>>> --- a/app/pktgen.c
>>> +++ b/app/pktgen.c
>>> @@ -401,8 +401,8 @@ static __inline__ int pktgen_has_work(void) {
>>> */
>>>
>>> void
>>> -pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t type) {
>>> - pkt_seq_t * pkt = &info->seq_pkt[seq_idx];
>>> +pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t type,
>>> pkt_seq_t * seq_pkt) {
>>> + pkt_seq_t * pkt = (seq_pkt == NULL) ? &info->seq_pkt[seq_idx] :
>>> &seq_pkt[seq_idx];
>>> struct ether_hdr * eth = (struct ether_hdr *)&pkt->hdr.eth;
>>> uint16_t tlen;
>>>
>>> @@ -812,22 +812,35 @@ static __inline__ void
>>> pktgen_setup_packets(port_info_t * info, struct rte_mempool * mp,
>>> uint16_t qid)
>>> {
>>> struct rte_mbuf * m, * mm;
>>> - pkt_seq_t * pkt;
>>> + pkt_seq_t * pkt, * seq_pkt = NULL;
>>> + uint16_t seqIdx;
>>> + char buff[RTE_MEMZONE_NAMESIZE];
>>>
>>> pktgen_clr_q_flags(info, qid, CLEAR_FAST_ALLOC_FLAG);
>>>
>>> if ( mp == info->q[qid].pcap_mp )
>>> return;
>>>
>>> + snprintf(buff, sizeof(buff), "tmp_seq_pkt_%d_%d", info->pid, qid);
>>> + seq_pkt = (pkt_seq_t *)rte_zmalloc(buff,
>>> + (sizeof(pkt_seq_t) * NUM_TOTAL_PKTS), RTE_CACHE_LINE_SIZE);
>>> + if (seq_pkt == NULL)
>>> + pktgen_log_panic("Unable to allocate %d pkt_seq_t headers",
>>> + NUM_TOTAL_PKTS);
>>> + /* Copy global configuration to construct new packets locally */
>>> + rte_memcpy((uint8_t *)seq_pkt, (uint8_t *)info->seq_pkt,
>>> + sizeof(pkt_seq_t) * NUM_TOTAL_PKTS);
>>> + seqIdx = info->seqIdx;
>>> +
>>> mm = NULL;
>>> pkt = NULL;
>>>
>>> if ( mp == info->q[qid].tx_mp )
>>> - pkt = &info->seq_pkt[SINGLE_PKT];
>>> + pkt = &seq_pkt[SINGLE_PKT];
>>> else if ( mp == info->q[qid].range_mp )
>>> - pkt = &info->seq_pkt[RANGE_PKT];
>>> + pkt = &seq_pkt[RANGE_PKT];
>>> else if ( mp == info->q[qid].seq_mp )
>>> - pkt = &info->seq_pkt[info->seqIdx];
>>> + pkt = &seq_pkt[seqIdx];
>>>
>>> // allocate each mbuf and put them on a list to be freed.
>>> for(;;) {
>>> @@ -840,7 +853,7 @@ pktgen_setup_packets(port_info_t * info, struct
>>> rte_mempool * mp, uint16_t qid)
>>> mm = m;
>>>
>>> if ( mp == info->q[qid].tx_mp ) {
>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, seq_pkt);
>>>
>>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>> *)&pkt->hdr, MAX_PKT_SIZE);
>>>
>>> @@ -848,16 +861,16 @@ pktgen_setup_packets(port_info_t * info, struct
>>> rte_mempool * mp, uint16_t qid)
>>> m->data_len = pkt->pktSize;
>>> } else if ( mp == info->q[qid].range_mp ) {
>>> pktgen_range_ctor(&info->range, pkt);
>>> - pktgen_packet_ctor(info, RANGE_PKT, -1);
>>> + pktgen_packet_ctor(info, RANGE_PKT, -1, seq_pkt);
>>>
>>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>> *)&pkt->hdr, MAX_PKT_SIZE);
>>>
>>> m->pkt_len = pkt->pktSize;
>>> m->data_len = pkt->pktSize;
>>> } else if ( mp == info->q[qid].seq_mp ) {
>>> - pktgen_packet_ctor(info, info->seqIdx++, -1);
>>> - if ( unlikely(info->seqIdx >= info->seqCnt) )
>>> - info->seqIdx = 0;
>>> + pktgen_packet_ctor(info, seqIdx++, -1, seq_pkt);
>>> + if ( unlikely(seqIdx >= info->seqCnt) )
>>> + seqIdx = 0;
>>>
>>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>> *)&pkt->hdr, MAX_PKT_SIZE);
>>>
>>> @@ -865,12 +878,14 @@ pktgen_setup_packets(port_info_t * info, struct
>>> rte_mempool * mp, uint16_t qid)
>>> m->data_len = pkt->pktSize;
>>>
>>> // move to the next packet in the sequence.
>>> - pkt = &info->seq_pkt[info->seqIdx];
>>> + pkt = &seq_pkt[seqIdx];
>>> }
>>> }
>>>
>>> if ( mm != NULL )
>>> rte_pktmbuf_free(mm);
>>> +
>>> + rte_free(seq_pkt);
>>> }
>>>
>>>
>>> /************************************************************************
>>> **//**
>>> diff --git a/app/pktgen.h b/app/pktgen.h
>>> index 2c5c98c..f3f76f2 100644
>>> --- a/app/pktgen.h
>>> +++ b/app/pktgen.h
>>> @@ -151,7 +151,7 @@
>>> #include "pktgen-port-cfg.h"
>>> #include "pktgen-capture.h"
>>> #include "pktgen-log.h"
>>> -
>>> +#include "pktgen-seq.h"
>>>
>>> #define PKTGEN_VERSION "2.9.2"
>>> #define PKTGEN_APP_NAME "Pktgen"
>>> @@ -386,7 +386,7 @@ extern pktgen_t pktgen;
>>>
>>> extern void pktgen_page_display(__attribute__((unused)) struct
>>> rte_timer *tim, __attribute__((unused)) void *arg);
>>>
>>> -extern void pktgen_packet_ctor(port_info_t * info, int32_t seq_idx,
>>> int32_t type);
>>> +extern void pktgen_packet_ctor(port_info_t * info, int32_t seq_idx,
>>> int32_t type, pkt_seq_t * seq_pkt);
>>> extern void pktgen_packet_rate(port_info_t * info);
>>>
>>> extern void pktgen_send_mbuf(struct rte_mbuf *m, uint8_t pid, uint16_t
>>> qid);
>>> diff --git a/app/t/pktgen.t.c b/app/t/pktgen.t.c
>>> index 0eca4c4..77bf300 100644
>>> --- a/app/t/pktgen.t.c
>>> +++ b/app/t/pktgen.t.c
>>> @@ -77,7 +77,7 @@ void test_pktgen_packet_ctor_IPv4_UDP(void)
>>> };
>>>
>>>
>>> - lives_ok( { pktgen_packet_ctor(&info, 0, 0); }, "pktgen_packet_ctor
>>> must generate IPv4/UDP");
>>> + lives_ok( { pktgen_packet_ctor(&info, 0, 0, NULL); },
>>> "pktgen_packet_ctor must generate IPv4/UDP");
>>>
>>> note("... with Ethernet header");
>>> cmp_mem_lit_incr(data, (0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd), " ...
>>> with correct dest MAC");
>>> @@ -133,9 +133,9 @@ void test_pktgen_packet_ctor_IPv4_GRE_Ether(void)
>>> .pktSize = 102, // Subtract 4 for FCS
>>> };
>>>
>>> - pktgen_packet_ctor(&info, 0, 0);
>>> + pktgen_packet_ctor(&info, 0, 0, NULL);
>>>
>>> - lives_ok( { pktgen_packet_ctor(&info, 0, 0); }, "pktgen_packet_ctor
>>> must generate IPv4 GRE Ethernet frame");
>>> + lives_ok( { pktgen_packet_ctor(&info, 0, 0, NULL); },
>>> "pktgen_packet_ctor must generate IPv4 GRE Ethernet frame");
>>>
>>> note("... with outer Ethernet header");
>>> cmp_mem_lit_incr(data, (0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd), " ...
>>> with correct dest MAC");
>>>
>>
>
>
> ‹
> Regards,
> ++Keith
> Intel Corporation
>
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [Pktgen] [PATCH] pktgen_setup_packets: fix race for packet header
2015-09-17 5:55 ` Ilya Maximets
@ 2015-10-02 9:23 ` Ilya Maximets
2015-10-02 11:25 ` Wiles, Keith
0 siblings, 1 reply; 7+ messages in thread
From: Ilya Maximets @ 2015-10-02 9:23 UTC (permalink / raw)
To: Wiles, Keith, dev; +Cc: Dyasly Sergey
Ping.
On 17.09.2015 08:55, Ilya Maximets wrote:
> Ok. Thank you. I'll wait.
>
> On 16.09.2015 18:37, Wiles, Keith wrote:
>> Thanks the patch looks fine, but I have not had a lot of time to review it
>> detail. I hope to get to it next week after I return back home.
>>
>> On 9/16/15, 2:09 AM, "Ilya Maximets" <i.maximets@samsung.com> wrote:
>>
>>> Ping.
>>>
>>> On 09.09.2015 17:22, Ilya Maximets wrote:
>>>> While pktgen_setup_packets() all threads of one port uses same
>>>> info->seq_pkt. This leads to constructing packets in the same memory
>>>> region
>>>> (&pkt->hdr). As a result, pktgen_setup_packets generates random headers.
>>>>
>>>> Fix that by making a local copy of info->seq_pkt and using it for
>>>> constructing of packets.
>>>>
>>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>>>> ---
>>>> app/pktgen-arp.c | 2 +-
>>>> app/pktgen-cmds.c | 40 ++++++++++++++++++++--------------------
>>>> app/pktgen-ipv4.c | 2 +-
>>>> app/pktgen.c | 39 +++++++++++++++++++++++++++------------
>>>> app/pktgen.h | 4 ++--
>>>> app/t/pktgen.t.c | 6 +++---
>>>> 6 files changed, 54 insertions(+), 39 deletions(-)
>>>>
>>>> diff --git a/app/pktgen-arp.c b/app/pktgen-arp.c
>>>> index c378880..b7040d7 100644
>>>> --- a/app/pktgen-arp.c
>>>> +++ b/app/pktgen-arp.c
>>>> @@ -190,7 +190,7 @@ pktgen_process_arp( struct rte_mbuf * m, uint32_t
>>>> pid, uint32_t vlan )
>>>>
>>>> rte_memcpy(&pkt->eth_dst_addr, &arp->sha, 6);
>>>> for (i = 0; i < info->seqCnt; i++)
>>>> - pktgen_packet_ctor(info, i, -1);
>>>> + pktgen_packet_ctor(info, i, -1, NULL);
>>>> }
>>>>
>>>> // Swap the two MAC addresses
>>>> diff --git a/app/pktgen-cmds.c b/app/pktgen-cmds.c
>>>> index da040e5..a6abb41 100644
>>>> --- a/app/pktgen-cmds.c
>>>> +++ b/app/pktgen-cmds.c
>>>> @@ -931,7 +931,7 @@ pktgen_set_proto(port_info_t * info, char type)
>>>> if ( type == 'i' )
>>>> info->seq_pkt[SINGLE_PKT].ethType = ETHER_TYPE_IPv4;
>>>>
>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>> }
>>>>
>>>>
>>>> /************************************************************************
>>>> **//**
>>>> @@ -1067,7 +1067,7 @@ pktgen_set_pkt_type(port_info_t * info, const
>>>> char * type)
>>>> (type[3] == '6') ? ETHER_TYPE_IPv6 :
>>>> /* TODO print error: unknown type */ ETHER_TYPE_IPv4;
>>>>
>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>> }
>>>>
>>>>
>>>> /************************************************************************
>>>> **//**
>>>> @@ -1092,7 +1092,7 @@ pktgen_set_vlan(port_info_t * info, uint32_t
>>>> onOff)
>>>> }
>>>> else
>>>> pktgen_clr_port_flags(info, SEND_VLAN_ID);
>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>> }
>>>>
>>>>
>>>> /************************************************************************
>>>> **//**
>>>> @@ -1112,7 +1112,7 @@ pktgen_set_vlanid(port_info_t * info, uint16_t
>>>> vlanid)
>>>> {
>>>> info->vlanid = vlanid;
>>>> info->seq_pkt[SINGLE_PKT].vlanid = info->vlanid;
>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>> }
>>>>
>>>>
>>>> /************************************************************************
>>>> **//**
>>>> @@ -1137,7 +1137,7 @@ pktgen_set_mpls(port_info_t * info, uint32_t
>>>> onOff)
>>>> }
>>>> else
>>>> pktgen_clr_port_flags(info, SEND_MPLS_LABEL);
>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>> }
>>>>
>>>>
>>>> /************************************************************************
>>>> **//**
>>>> @@ -1157,7 +1157,7 @@ pktgen_set_mpls_entry(port_info_t * info,
>>>> uint32_t mpls_entry)
>>>> {
>>>> info->mpls_entry = mpls_entry;
>>>> info->seq_pkt[SINGLE_PKT].mpls_entry = info->mpls_entry;
>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>> }
>>>>
>>>>
>>>> /************************************************************************
>>>> **//**
>>>> @@ -1182,7 +1182,7 @@ pktgen_set_qinq(port_info_t * info, uint32_t
>>>> onOff)
>>>> }
>>>> else
>>>> pktgen_clr_port_flags(info, SEND_Q_IN_Q_IDS);
>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>> }
>>>>
>>>>
>>>> /************************************************************************
>>>> **//**
>>>> @@ -1204,7 +1204,7 @@ pktgen_set_qinqids(port_info_t * info, uint16_t
>>>> outerid, uint16_t innerid)
>>>> info->seq_pkt[SINGLE_PKT].qinq_outerid = info->qinq_outerid;
>>>> info->qinq_innerid = innerid;
>>>> info->seq_pkt[SINGLE_PKT].qinq_innerid = info->qinq_innerid;
>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>> }
>>>>
>>>>
>>>> /************************************************************************
>>>> **//**
>>>> @@ -1228,7 +1228,7 @@ pktgen_set_gre(port_info_t * info, uint32_t onOff)
>>>> }
>>>> else
>>>> pktgen_clr_port_flags(info, SEND_GRE_IPv4_HEADER);
>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>> }
>>>>
>>>>
>>>> /************************************************************************
>>>> **//**
>>>> @@ -1252,7 +1252,7 @@ pktgen_set_gre_eth(port_info_t * info, uint32_t
>>>> onOff)
>>>> }
>>>> else
>>>> pktgen_clr_port_flags(info, SEND_GRE_ETHER_HEADER);
>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>> }
>>>>
>>>>
>>>> /************************************************************************
>>>> **//**
>>>> @@ -1272,7 +1272,7 @@ pktgen_set_gre_key(port_info_t * info, uint32_t
>>>> gre_key)
>>>> {
>>>> info->gre_key = gre_key;
>>>> info->seq_pkt[SINGLE_PKT].gre_key = info->gre_key;
>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>> }
>>>>
>>>>
>>>> /************************************************************************
>>>> **//**
>>>> @@ -1401,7 +1401,7 @@ void pktgen_port_defaults(uint32_t pid, uint8_t
>>>> seq)
>>>> memset(&pkt->eth_dst_addr, 0, sizeof (pkt->eth_dst_addr));
>>>>
>>>>
>>>> - pktgen_packet_ctor(info, seq, -1);
>>>> + pktgen_packet_ctor(info, seq, -1, NULL);
>>>>
>>>> pktgen.flags |= PRINT_LABELS_FLAG;
>>>> }
>>>> @@ -1423,7 +1423,7 @@ void pktgen_ping4(port_info_t * info)
>>>> memcpy(&info->seq_pkt[PING_PKT],
>>>> &info->seq_pkt[SINGLE_PKT], sizeof(pkt_seq_t));
>>>> info->seq_pkt[PING_PKT].ipProto = PG_IPPROTO_ICMP;
>>>> - pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO);
>>>> + pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO, NULL);
>>>> pktgen_set_port_flags(info, SEND_PING4_REQUEST);
>>>> }
>>>>
>>>> @@ -1445,7 +1445,7 @@ void pktgen_ping6(port_info_t * info)
>>>> memcpy(&info->pkt[PING_PKT],
>>>> &info->pkt[SINGLE_PKT], sizeof(pkt_seq_t));
>>>> info->pkt[PING_PKT].ipProto = PG_IPPROTO_ICMP;
>>>> - pktgen_packet_ctor(info, PING_PKT, ICMP6_ECHO);
>>>> + pktgen_packet_ctor(info, PING_PKT, ICMP6_ECHO, NULL);
>>>> pktgen_set_port_flags(info, SEND_PING6_REQUEST);
>>>> }
>>>> #endif
>>>> @@ -1645,7 +1645,7 @@ void pktgen_set_pkt_size(port_info_t * info,
>>>> uint32_t size)
>>>> else if ( (size - FCS_SIZE) > MAX_PKT_SIZE)
>>>> size = MAX_PKT_SIZE + FCS_SIZE;
>>>> info->seq_pkt[SINGLE_PKT].pktSize = (size - FCS_SIZE);
>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>> pktgen_packet_rate(info);
>>>> }
>>>>
>>>> @@ -1667,7 +1667,7 @@ void pktgen_set_port_value(port_info_t * info,
>>>> char type, uint32_t portValue)
>>>> info->seq_pkt[SINGLE_PKT].dport = (uint16_t)portValue;
>>>> else
>>>> info->seq_pkt[SINGLE_PKT].sport = (uint16_t)portValue;
>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>> }
>>>>
>>>>
>>>> /************************************************************************
>>>> **//**
>>>> @@ -1711,7 +1711,7 @@ void pktgen_set_ipaddr(port_info_t * info, char
>>>> type, cmdline_ipaddr_t * ip)
>>>> info->seq_pkt[SINGLE_PKT].ip_src_addr = ntohl(ip->addr.ipv4.s_addr);
>>>> } else
>>>> info->seq_pkt[SINGLE_PKT].ip_dst_addr = ntohl(ip->addr.ipv4.s_addr);
>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>> }
>>>>
>>>>
>>>> /************************************************************************
>>>> **//**
>>>> @@ -1729,7 +1729,7 @@ void pktgen_set_ipaddr(port_info_t * info, char
>>>> type, cmdline_ipaddr_t * ip)
>>>> void pktgen_set_dst_mac(port_info_t * info, cmdline_etheraddr_t * mac)
>>>> {
>>>> memcpy(&info->seq_pkt[SINGLE_PKT].eth_dst_addr, mac->mac, 6);
>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>> }
>>>>
>>>>
>>>> /************************************************************************
>>>> **//**
>>>> @@ -2111,7 +2111,7 @@ void pktgen_set_seq(port_info_t * info, uint32_t
>>>> seqnum,
>>>> type = '4';
>>>> pkt->ethType = (type == '6')? ETHER_TYPE_IPv6 : ETHER_TYPE_IPv4;
>>>> pkt->vlanid = vlanid;
>>>> - pktgen_packet_ctor(info, seqnum, -1);
>>>> + pktgen_packet_ctor(info, seqnum, -1, NULL);
>>>> }
>>>>
>>>>
>>>> /************************************************************************
>>>> **//**
>>>> @@ -2156,7 +2156,7 @@ void pktgen_compile_pkt(port_info_t * info,
>>>> uint32_t seqnum,
>>>> (type == '6')? ETHER_TYPE_IPv6 :
>>>> (type == 'n')? ETHER_TYPE_VLAN : ETHER_TYPE_IPv4;
>>>> pkt->vlanid = vlanid;
>>>> - pktgen_packet_ctor(info, seqnum, -1);
>>>> + pktgen_packet_ctor(info, seqnum, -1, NULL);
>>>> }
>>>>
>>>>
>>>> /************************************************************************
>>>> **//**
>>>> diff --git a/app/pktgen-ipv4.c b/app/pktgen-ipv4.c
>>>> index 7813c36..a8b6be2 100644
>>>> --- a/app/pktgen-ipv4.c
>>>> +++ b/app/pktgen-ipv4.c
>>>> @@ -138,7 +138,7 @@ pktgen_send_ping4( uint32_t pid, uint8_t seq_idx )
>>>> return;
>>>> }
>>>> *ppkt = *spkt; // Copy the sequence setup to the ping setup.
>>>> - pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO);
>>>> + pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO, NULL);
>>>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>>> *)&ppkt->hdr, ppkt->pktSize);
>>>>
>>>> m->pkt_len = ppkt->pktSize;
>>>> diff --git a/app/pktgen.c b/app/pktgen.c
>>>> index 2dcdbfd..ecd4560 100644
>>>> --- a/app/pktgen.c
>>>> +++ b/app/pktgen.c
>>>> @@ -401,8 +401,8 @@ static __inline__ int pktgen_has_work(void) {
>>>> */
>>>>
>>>> void
>>>> -pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t type) {
>>>> - pkt_seq_t * pkt = &info->seq_pkt[seq_idx];
>>>> +pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t type,
>>>> pkt_seq_t * seq_pkt) {
>>>> + pkt_seq_t * pkt = (seq_pkt == NULL) ? &info->seq_pkt[seq_idx] :
>>>> &seq_pkt[seq_idx];
>>>> struct ether_hdr * eth = (struct ether_hdr *)&pkt->hdr.eth;
>>>> uint16_t tlen;
>>>>
>>>> @@ -812,22 +812,35 @@ static __inline__ void
>>>> pktgen_setup_packets(port_info_t * info, struct rte_mempool * mp,
>>>> uint16_t qid)
>>>> {
>>>> struct rte_mbuf * m, * mm;
>>>> - pkt_seq_t * pkt;
>>>> + pkt_seq_t * pkt, * seq_pkt = NULL;
>>>> + uint16_t seqIdx;
>>>> + char buff[RTE_MEMZONE_NAMESIZE];
>>>>
>>>> pktgen_clr_q_flags(info, qid, CLEAR_FAST_ALLOC_FLAG);
>>>>
>>>> if ( mp == info->q[qid].pcap_mp )
>>>> return;
>>>>
>>>> + snprintf(buff, sizeof(buff), "tmp_seq_pkt_%d_%d", info->pid, qid);
>>>> + seq_pkt = (pkt_seq_t *)rte_zmalloc(buff,
>>>> + (sizeof(pkt_seq_t) * NUM_TOTAL_PKTS), RTE_CACHE_LINE_SIZE);
>>>> + if (seq_pkt == NULL)
>>>> + pktgen_log_panic("Unable to allocate %d pkt_seq_t headers",
>>>> + NUM_TOTAL_PKTS);
>>>> + /* Copy global configuration to construct new packets locally */
>>>> + rte_memcpy((uint8_t *)seq_pkt, (uint8_t *)info->seq_pkt,
>>>> + sizeof(pkt_seq_t) * NUM_TOTAL_PKTS);
>>>> + seqIdx = info->seqIdx;
>>>> +
>>>> mm = NULL;
>>>> pkt = NULL;
>>>>
>>>> if ( mp == info->q[qid].tx_mp )
>>>> - pkt = &info->seq_pkt[SINGLE_PKT];
>>>> + pkt = &seq_pkt[SINGLE_PKT];
>>>> else if ( mp == info->q[qid].range_mp )
>>>> - pkt = &info->seq_pkt[RANGE_PKT];
>>>> + pkt = &seq_pkt[RANGE_PKT];
>>>> else if ( mp == info->q[qid].seq_mp )
>>>> - pkt = &info->seq_pkt[info->seqIdx];
>>>> + pkt = &seq_pkt[seqIdx];
>>>>
>>>> // allocate each mbuf and put them on a list to be freed.
>>>> for(;;) {
>>>> @@ -840,7 +853,7 @@ pktgen_setup_packets(port_info_t * info, struct
>>>> rte_mempool * mp, uint16_t qid)
>>>> mm = m;
>>>>
>>>> if ( mp == info->q[qid].tx_mp ) {
>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, seq_pkt);
>>>>
>>>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>>> *)&pkt->hdr, MAX_PKT_SIZE);
>>>>
>>>> @@ -848,16 +861,16 @@ pktgen_setup_packets(port_info_t * info, struct
>>>> rte_mempool * mp, uint16_t qid)
>>>> m->data_len = pkt->pktSize;
>>>> } else if ( mp == info->q[qid].range_mp ) {
>>>> pktgen_range_ctor(&info->range, pkt);
>>>> - pktgen_packet_ctor(info, RANGE_PKT, -1);
>>>> + pktgen_packet_ctor(info, RANGE_PKT, -1, seq_pkt);
>>>>
>>>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>>> *)&pkt->hdr, MAX_PKT_SIZE);
>>>>
>>>> m->pkt_len = pkt->pktSize;
>>>> m->data_len = pkt->pktSize;
>>>> } else if ( mp == info->q[qid].seq_mp ) {
>>>> - pktgen_packet_ctor(info, info->seqIdx++, -1);
>>>> - if ( unlikely(info->seqIdx >= info->seqCnt) )
>>>> - info->seqIdx = 0;
>>>> + pktgen_packet_ctor(info, seqIdx++, -1, seq_pkt);
>>>> + if ( unlikely(seqIdx >= info->seqCnt) )
>>>> + seqIdx = 0;
>>>>
>>>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>>> *)&pkt->hdr, MAX_PKT_SIZE);
>>>>
>>>> @@ -865,12 +878,14 @@ pktgen_setup_packets(port_info_t * info, struct
>>>> rte_mempool * mp, uint16_t qid)
>>>> m->data_len = pkt->pktSize;
>>>>
>>>> // move to the next packet in the sequence.
>>>> - pkt = &info->seq_pkt[info->seqIdx];
>>>> + pkt = &seq_pkt[seqIdx];
>>>> }
>>>> }
>>>>
>>>> if ( mm != NULL )
>>>> rte_pktmbuf_free(mm);
>>>> +
>>>> + rte_free(seq_pkt);
>>>> }
>>>>
>>>>
>>>> /************************************************************************
>>>> **//**
>>>> diff --git a/app/pktgen.h b/app/pktgen.h
>>>> index 2c5c98c..f3f76f2 100644
>>>> --- a/app/pktgen.h
>>>> +++ b/app/pktgen.h
>>>> @@ -151,7 +151,7 @@
>>>> #include "pktgen-port-cfg.h"
>>>> #include "pktgen-capture.h"
>>>> #include "pktgen-log.h"
>>>> -
>>>> +#include "pktgen-seq.h"
>>>>
>>>> #define PKTGEN_VERSION "2.9.2"
>>>> #define PKTGEN_APP_NAME "Pktgen"
>>>> @@ -386,7 +386,7 @@ extern pktgen_t pktgen;
>>>>
>>>> extern void pktgen_page_display(__attribute__((unused)) struct
>>>> rte_timer *tim, __attribute__((unused)) void *arg);
>>>>
>>>> -extern void pktgen_packet_ctor(port_info_t * info, int32_t seq_idx,
>>>> int32_t type);
>>>> +extern void pktgen_packet_ctor(port_info_t * info, int32_t seq_idx,
>>>> int32_t type, pkt_seq_t * seq_pkt);
>>>> extern void pktgen_packet_rate(port_info_t * info);
>>>>
>>>> extern void pktgen_send_mbuf(struct rte_mbuf *m, uint8_t pid, uint16_t
>>>> qid);
>>>> diff --git a/app/t/pktgen.t.c b/app/t/pktgen.t.c
>>>> index 0eca4c4..77bf300 100644
>>>> --- a/app/t/pktgen.t.c
>>>> +++ b/app/t/pktgen.t.c
>>>> @@ -77,7 +77,7 @@ void test_pktgen_packet_ctor_IPv4_UDP(void)
>>>> };
>>>>
>>>>
>>>> - lives_ok( { pktgen_packet_ctor(&info, 0, 0); }, "pktgen_packet_ctor
>>>> must generate IPv4/UDP");
>>>> + lives_ok( { pktgen_packet_ctor(&info, 0, 0, NULL); },
>>>> "pktgen_packet_ctor must generate IPv4/UDP");
>>>>
>>>> note("... with Ethernet header");
>>>> cmp_mem_lit_incr(data, (0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd), " ...
>>>> with correct dest MAC");
>>>> @@ -133,9 +133,9 @@ void test_pktgen_packet_ctor_IPv4_GRE_Ether(void)
>>>> .pktSize = 102, // Subtract 4 for FCS
>>>> };
>>>>
>>>> - pktgen_packet_ctor(&info, 0, 0);
>>>> + pktgen_packet_ctor(&info, 0, 0, NULL);
>>>>
>>>> - lives_ok( { pktgen_packet_ctor(&info, 0, 0); }, "pktgen_packet_ctor
>>>> must generate IPv4 GRE Ethernet frame");
>>>> + lives_ok( { pktgen_packet_ctor(&info, 0, 0, NULL); },
>>>> "pktgen_packet_ctor must generate IPv4 GRE Ethernet frame");
>>>>
>>>> note("... with outer Ethernet header");
>>>> cmp_mem_lit_incr(data, (0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd), " ...
>>>> with correct dest MAC");
>>>>
>>>
>>
>>
>> ‹
>> Regards,
>> ++Keith
>> Intel Corporation
>>
>>
>>
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [Pktgen] [PATCH] pktgen_setup_packets: fix race for packet header
2015-10-02 9:23 ` Ilya Maximets
@ 2015-10-02 11:25 ` Wiles, Keith
2015-10-12 14:09 ` Ilya Maximets
0 siblings, 1 reply; 7+ messages in thread
From: Wiles, Keith @ 2015-10-02 11:25 UTC (permalink / raw)
To: Ilya Maximets, dev; +Cc: Dyasly Sergey
I looked at the code and everything looks good. I will try to merge the
code next week as I am traveling again :-(
Thanks for the patch, I am glad you found this problem as I believe
someone else reported something odd in that area, but was not able to give
me many details.
—
Regards,
++Keith Wiles
Intel Corporation
On 10/2/15, 10:23 AM, "Ilya Maximets" <i.maximets@samsung.com> wrote:
>Ping.
>
>On 17.09.2015 08:55, Ilya Maximets wrote:
>> Ok. Thank you. I'll wait.
>>
>> On 16.09.2015 18:37, Wiles, Keith wrote:
>>> Thanks the patch looks fine, but I have not had a lot of time to
>>>review it
>>> detail. I hope to get to it next week after I return back home.
>>>
>>> On 9/16/15, 2:09 AM, "Ilya Maximets" <i.maximets@samsung.com> wrote:
>>>
>>>> Ping.
>>>>
>>>> On 09.09.2015 17:22, Ilya Maximets wrote:
>>>>> While pktgen_setup_packets() all threads of one port uses same
>>>>> info->seq_pkt. This leads to constructing packets in the same memory
>>>>> region
>>>>> (&pkt->hdr). As a result, pktgen_setup_packets generates random
>>>>>headers.
>>>>>
>>>>> Fix that by making a local copy of info->seq_pkt and using it for
>>>>> constructing of packets.
>>>>>
>>>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>>>>> ---
>>>>> app/pktgen-arp.c | 2 +-
>>>>> app/pktgen-cmds.c | 40 ++++++++++++++++++++--------------------
>>>>> app/pktgen-ipv4.c | 2 +-
>>>>> app/pktgen.c | 39 +++++++++++++++++++++++++++------------
>>>>> app/pktgen.h | 4 ++--
>>>>> app/t/pktgen.t.c | 6 +++---
>>>>> 6 files changed, 54 insertions(+), 39 deletions(-)
>>>>>
>>>>> diff --git a/app/pktgen-arp.c b/app/pktgen-arp.c
>>>>> index c378880..b7040d7 100644
>>>>> --- a/app/pktgen-arp.c
>>>>> +++ b/app/pktgen-arp.c
>>>>> @@ -190,7 +190,7 @@ pktgen_process_arp( struct rte_mbuf * m, uint32_t
>>>>> pid, uint32_t vlan )
>>>>>
>>>>> rte_memcpy(&pkt->eth_dst_addr, &arp->sha, 6);
>>>>> for (i = 0; i < info->seqCnt; i++)
>>>>> - pktgen_packet_ctor(info, i, -1);
>>>>> + pktgen_packet_ctor(info, i, -1, NULL);
>>>>> }
>>>>>
>>>>> // Swap the two MAC addresses
>>>>> diff --git a/app/pktgen-cmds.c b/app/pktgen-cmds.c
>>>>> index da040e5..a6abb41 100644
>>>>> --- a/app/pktgen-cmds.c
>>>>> +++ b/app/pktgen-cmds.c
>>>>> @@ -931,7 +931,7 @@ pktgen_set_proto(port_info_t * info, char type)
>>>>> if ( type == 'i' )
>>>>> info->seq_pkt[SINGLE_PKT].ethType = ETHER_TYPE_IPv4;
>>>>>
>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>/*********************************************************************
>>>>>***
>>>>> **//**
>>>>> @@ -1067,7 +1067,7 @@ pktgen_set_pkt_type(port_info_t * info, const
>>>>> char * type)
>>>>> (type[3] == '6') ? ETHER_TYPE_IPv6 :
>>>>> /* TODO print error: unknown type */ ETHER_TYPE_IPv4;
>>>>>
>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>/*********************************************************************
>>>>>***
>>>>> **//**
>>>>> @@ -1092,7 +1092,7 @@ pktgen_set_vlan(port_info_t * info, uint32_t
>>>>> onOff)
>>>>> }
>>>>> else
>>>>> pktgen_clr_port_flags(info, SEND_VLAN_ID);
>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>/*********************************************************************
>>>>>***
>>>>> **//**
>>>>> @@ -1112,7 +1112,7 @@ pktgen_set_vlanid(port_info_t * info, uint16_t
>>>>> vlanid)
>>>>> {
>>>>> info->vlanid = vlanid;
>>>>> info->seq_pkt[SINGLE_PKT].vlanid = info->vlanid;
>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>/*********************************************************************
>>>>>***
>>>>> **//**
>>>>> @@ -1137,7 +1137,7 @@ pktgen_set_mpls(port_info_t * info, uint32_t
>>>>> onOff)
>>>>> }
>>>>> else
>>>>> pktgen_clr_port_flags(info, SEND_MPLS_LABEL);
>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>/*********************************************************************
>>>>>***
>>>>> **//**
>>>>> @@ -1157,7 +1157,7 @@ pktgen_set_mpls_entry(port_info_t * info,
>>>>> uint32_t mpls_entry)
>>>>> {
>>>>> info->mpls_entry = mpls_entry;
>>>>> info->seq_pkt[SINGLE_PKT].mpls_entry = info->mpls_entry;
>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>/*********************************************************************
>>>>>***
>>>>> **//**
>>>>> @@ -1182,7 +1182,7 @@ pktgen_set_qinq(port_info_t * info, uint32_t
>>>>> onOff)
>>>>> }
>>>>> else
>>>>> pktgen_clr_port_flags(info, SEND_Q_IN_Q_IDS);
>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>/*********************************************************************
>>>>>***
>>>>> **//**
>>>>> @@ -1204,7 +1204,7 @@ pktgen_set_qinqids(port_info_t * info, uint16_t
>>>>> outerid, uint16_t innerid)
>>>>> info->seq_pkt[SINGLE_PKT].qinq_outerid = info->qinq_outerid;
>>>>> info->qinq_innerid = innerid;
>>>>> info->seq_pkt[SINGLE_PKT].qinq_innerid = info->qinq_innerid;
>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>/*********************************************************************
>>>>>***
>>>>> **//**
>>>>> @@ -1228,7 +1228,7 @@ pktgen_set_gre(port_info_t * info, uint32_t
>>>>>onOff)
>>>>> }
>>>>> else
>>>>> pktgen_clr_port_flags(info, SEND_GRE_IPv4_HEADER);
>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>/*********************************************************************
>>>>>***
>>>>> **//**
>>>>> @@ -1252,7 +1252,7 @@ pktgen_set_gre_eth(port_info_t * info, uint32_t
>>>>> onOff)
>>>>> }
>>>>> else
>>>>> pktgen_clr_port_flags(info, SEND_GRE_ETHER_HEADER);
>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>/*********************************************************************
>>>>>***
>>>>> **//**
>>>>> @@ -1272,7 +1272,7 @@ pktgen_set_gre_key(port_info_t * info, uint32_t
>>>>> gre_key)
>>>>> {
>>>>> info->gre_key = gre_key;
>>>>> info->seq_pkt[SINGLE_PKT].gre_key = info->gre_key;
>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>/*********************************************************************
>>>>>***
>>>>> **//**
>>>>> @@ -1401,7 +1401,7 @@ void pktgen_port_defaults(uint32_t pid, uint8_t
>>>>> seq)
>>>>> memset(&pkt->eth_dst_addr, 0, sizeof (pkt->eth_dst_addr));
>>>>>
>>>>>
>>>>> - pktgen_packet_ctor(info, seq, -1);
>>>>> + pktgen_packet_ctor(info, seq, -1, NULL);
>>>>>
>>>>> pktgen.flags |= PRINT_LABELS_FLAG;
>>>>> }
>>>>> @@ -1423,7 +1423,7 @@ void pktgen_ping4(port_info_t * info)
>>>>> memcpy(&info->seq_pkt[PING_PKT],
>>>>> &info->seq_pkt[SINGLE_PKT], sizeof(pkt_seq_t));
>>>>> info->seq_pkt[PING_PKT].ipProto = PG_IPPROTO_ICMP;
>>>>> - pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO);
>>>>> + pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO, NULL);
>>>>> pktgen_set_port_flags(info, SEND_PING4_REQUEST);
>>>>> }
>>>>>
>>>>> @@ -1445,7 +1445,7 @@ void pktgen_ping6(port_info_t * info)
>>>>> memcpy(&info->pkt[PING_PKT],
>>>>> &info->pkt[SINGLE_PKT], sizeof(pkt_seq_t));
>>>>> info->pkt[PING_PKT].ipProto = PG_IPPROTO_ICMP;
>>>>> - pktgen_packet_ctor(info, PING_PKT, ICMP6_ECHO);
>>>>> + pktgen_packet_ctor(info, PING_PKT, ICMP6_ECHO, NULL);
>>>>> pktgen_set_port_flags(info, SEND_PING6_REQUEST);
>>>>> }
>>>>> #endif
>>>>> @@ -1645,7 +1645,7 @@ void pktgen_set_pkt_size(port_info_t * info,
>>>>> uint32_t size)
>>>>> else if ( (size - FCS_SIZE) > MAX_PKT_SIZE)
>>>>> size = MAX_PKT_SIZE + FCS_SIZE;
>>>>> info->seq_pkt[SINGLE_PKT].pktSize = (size - FCS_SIZE);
>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>> pktgen_packet_rate(info);
>>>>> }
>>>>>
>>>>> @@ -1667,7 +1667,7 @@ void pktgen_set_port_value(port_info_t * info,
>>>>> char type, uint32_t portValue)
>>>>> info->seq_pkt[SINGLE_PKT].dport = (uint16_t)portValue;
>>>>> else
>>>>> info->seq_pkt[SINGLE_PKT].sport = (uint16_t)portValue;
>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>/*********************************************************************
>>>>>***
>>>>> **//**
>>>>> @@ -1711,7 +1711,7 @@ void pktgen_set_ipaddr(port_info_t * info, char
>>>>> type, cmdline_ipaddr_t * ip)
>>>>> info->seq_pkt[SINGLE_PKT].ip_src_addr =
>>>>>ntohl(ip->addr.ipv4.s_addr);
>>>>> } else
>>>>> info->seq_pkt[SINGLE_PKT].ip_dst_addr =
>>>>>ntohl(ip->addr.ipv4.s_addr);
>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>/*********************************************************************
>>>>>***
>>>>> **//**
>>>>> @@ -1729,7 +1729,7 @@ void pktgen_set_ipaddr(port_info_t * info, char
>>>>> type, cmdline_ipaddr_t * ip)
>>>>> void pktgen_set_dst_mac(port_info_t * info, cmdline_etheraddr_t *
>>>>>mac)
>>>>> {
>>>>> memcpy(&info->seq_pkt[SINGLE_PKT].eth_dst_addr, mac->mac, 6);
>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>/*********************************************************************
>>>>>***
>>>>> **//**
>>>>> @@ -2111,7 +2111,7 @@ void pktgen_set_seq(port_info_t * info,
>>>>>uint32_t
>>>>> seqnum,
>>>>> type = '4';
>>>>> pkt->ethType = (type == '6')? ETHER_TYPE_IPv6 : ETHER_TYPE_IPv4;
>>>>> pkt->vlanid = vlanid;
>>>>> - pktgen_packet_ctor(info, seqnum, -1);
>>>>> + pktgen_packet_ctor(info, seqnum, -1, NULL);
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>/*********************************************************************
>>>>>***
>>>>> **//**
>>>>> @@ -2156,7 +2156,7 @@ void pktgen_compile_pkt(port_info_t * info,
>>>>> uint32_t seqnum,
>>>>> (type == '6')? ETHER_TYPE_IPv6 :
>>>>> (type == 'n')? ETHER_TYPE_VLAN : ETHER_TYPE_IPv4;
>>>>> pkt->vlanid = vlanid;
>>>>> - pktgen_packet_ctor(info, seqnum, -1);
>>>>> + pktgen_packet_ctor(info, seqnum, -1, NULL);
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>/*********************************************************************
>>>>>***
>>>>> **//**
>>>>> diff --git a/app/pktgen-ipv4.c b/app/pktgen-ipv4.c
>>>>> index 7813c36..a8b6be2 100644
>>>>> --- a/app/pktgen-ipv4.c
>>>>> +++ b/app/pktgen-ipv4.c
>>>>> @@ -138,7 +138,7 @@ pktgen_send_ping4( uint32_t pid, uint8_t seq_idx
>>>>>)
>>>>> return;
>>>>> }
>>>>> *ppkt = *spkt; // Copy the sequence setup to the ping setup.
>>>>> - pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO);
>>>>> + pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO, NULL);
>>>>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>>>> *)&ppkt->hdr, ppkt->pktSize);
>>>>>
>>>>> m->pkt_len = ppkt->pktSize;
>>>>> diff --git a/app/pktgen.c b/app/pktgen.c
>>>>> index 2dcdbfd..ecd4560 100644
>>>>> --- a/app/pktgen.c
>>>>> +++ b/app/pktgen.c
>>>>> @@ -401,8 +401,8 @@ static __inline__ int pktgen_has_work(void) {
>>>>> */
>>>>>
>>>>> void
>>>>> -pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t
>>>>>type) {
>>>>> - pkt_seq_t * pkt = &info->seq_pkt[seq_idx];
>>>>> +pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t
>>>>>type,
>>>>> pkt_seq_t * seq_pkt) {
>>>>> + pkt_seq_t * pkt = (seq_pkt == NULL) ? &info->seq_pkt[seq_idx] :
>>>>> &seq_pkt[seq_idx];
>>>>> struct ether_hdr * eth = (struct ether_hdr *)&pkt->hdr.eth;
>>>>> uint16_t tlen;
>>>>>
>>>>> @@ -812,22 +812,35 @@ static __inline__ void
>>>>> pktgen_setup_packets(port_info_t * info, struct rte_mempool * mp,
>>>>> uint16_t qid)
>>>>> {
>>>>> struct rte_mbuf * m, * mm;
>>>>> - pkt_seq_t * pkt;
>>>>> + pkt_seq_t * pkt, * seq_pkt = NULL;
>>>>> + uint16_t seqIdx;
>>>>> + char buff[RTE_MEMZONE_NAMESIZE];
>>>>>
>>>>> pktgen_clr_q_flags(info, qid, CLEAR_FAST_ALLOC_FLAG);
>>>>>
>>>>> if ( mp == info->q[qid].pcap_mp )
>>>>> return;
>>>>>
>>>>> + snprintf(buff, sizeof(buff), "tmp_seq_pkt_%d_%d", info->pid, qid);
>>>>> + seq_pkt = (pkt_seq_t *)rte_zmalloc(buff,
>>>>> + (sizeof(pkt_seq_t) * NUM_TOTAL_PKTS), RTE_CACHE_LINE_SIZE);
>>>>> + if (seq_pkt == NULL)
>>>>> + pktgen_log_panic("Unable to allocate %d pkt_seq_t headers",
>>>>> + NUM_TOTAL_PKTS);
>>>>> + /* Copy global configuration to construct new packets locally */
>>>>> + rte_memcpy((uint8_t *)seq_pkt, (uint8_t *)info->seq_pkt,
>>>>> + sizeof(pkt_seq_t) * NUM_TOTAL_PKTS);
>>>>> + seqIdx = info->seqIdx;
>>>>> +
>>>>> mm = NULL;
>>>>> pkt = NULL;
>>>>>
>>>>> if ( mp == info->q[qid].tx_mp )
>>>>> - pkt = &info->seq_pkt[SINGLE_PKT];
>>>>> + pkt = &seq_pkt[SINGLE_PKT];
>>>>> else if ( mp == info->q[qid].range_mp )
>>>>> - pkt = &info->seq_pkt[RANGE_PKT];
>>>>> + pkt = &seq_pkt[RANGE_PKT];
>>>>> else if ( mp == info->q[qid].seq_mp )
>>>>> - pkt = &info->seq_pkt[info->seqIdx];
>>>>> + pkt = &seq_pkt[seqIdx];
>>>>>
>>>>> // allocate each mbuf and put them on a list to be freed.
>>>>> for(;;) {
>>>>> @@ -840,7 +853,7 @@ pktgen_setup_packets(port_info_t * info, struct
>>>>> rte_mempool * mp, uint16_t qid)
>>>>> mm = m;
>>>>>
>>>>> if ( mp == info->q[qid].tx_mp ) {
>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, seq_pkt);
>>>>>
>>>>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>>>> *)&pkt->hdr, MAX_PKT_SIZE);
>>>>>
>>>>> @@ -848,16 +861,16 @@ pktgen_setup_packets(port_info_t * info, struct
>>>>> rte_mempool * mp, uint16_t qid)
>>>>> m->data_len = pkt->pktSize;
>>>>> } else if ( mp == info->q[qid].range_mp ) {
>>>>> pktgen_range_ctor(&info->range, pkt);
>>>>> - pktgen_packet_ctor(info, RANGE_PKT, -1);
>>>>> + pktgen_packet_ctor(info, RANGE_PKT, -1, seq_pkt);
>>>>>
>>>>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>>>> *)&pkt->hdr, MAX_PKT_SIZE);
>>>>>
>>>>> m->pkt_len = pkt->pktSize;
>>>>> m->data_len = pkt->pktSize;
>>>>> } else if ( mp == info->q[qid].seq_mp ) {
>>>>> - pktgen_packet_ctor(info, info->seqIdx++, -1);
>>>>> - if ( unlikely(info->seqIdx >= info->seqCnt) )
>>>>> - info->seqIdx = 0;
>>>>> + pktgen_packet_ctor(info, seqIdx++, -1, seq_pkt);
>>>>> + if ( unlikely(seqIdx >= info->seqCnt) )
>>>>> + seqIdx = 0;
>>>>>
>>>>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>>>> *)&pkt->hdr, MAX_PKT_SIZE);
>>>>>
>>>>> @@ -865,12 +878,14 @@ pktgen_setup_packets(port_info_t * info, struct
>>>>> rte_mempool * mp, uint16_t qid)
>>>>> m->data_len = pkt->pktSize;
>>>>>
>>>>> // move to the next packet in the sequence.
>>>>> - pkt = &info->seq_pkt[info->seqIdx];
>>>>> + pkt = &seq_pkt[seqIdx];
>>>>> }
>>>>> }
>>>>>
>>>>> if ( mm != NULL )
>>>>> rte_pktmbuf_free(mm);
>>>>> +
>>>>> + rte_free(seq_pkt);
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>/*********************************************************************
>>>>>***
>>>>> **//**
>>>>> diff --git a/app/pktgen.h b/app/pktgen.h
>>>>> index 2c5c98c..f3f76f2 100644
>>>>> --- a/app/pktgen.h
>>>>> +++ b/app/pktgen.h
>>>>> @@ -151,7 +151,7 @@
>>>>> #include "pktgen-port-cfg.h"
>>>>> #include "pktgen-capture.h"
>>>>> #include "pktgen-log.h"
>>>>> -
>>>>> +#include "pktgen-seq.h"
>>>>>
>>>>> #define PKTGEN_VERSION "2.9.2"
>>>>> #define PKTGEN_APP_NAME "Pktgen"
>>>>> @@ -386,7 +386,7 @@ extern pktgen_t pktgen;
>>>>>
>>>>> extern void pktgen_page_display(__attribute__((unused)) struct
>>>>> rte_timer *tim, __attribute__((unused)) void *arg);
>>>>>
>>>>> -extern void pktgen_packet_ctor(port_info_t * info, int32_t seq_idx,
>>>>> int32_t type);
>>>>> +extern void pktgen_packet_ctor(port_info_t * info, int32_t seq_idx,
>>>>> int32_t type, pkt_seq_t * seq_pkt);
>>>>> extern void pktgen_packet_rate(port_info_t * info);
>>>>>
>>>>> extern void pktgen_send_mbuf(struct rte_mbuf *m, uint8_t pid,
>>>>>uint16_t
>>>>> qid);
>>>>> diff --git a/app/t/pktgen.t.c b/app/t/pktgen.t.c
>>>>> index 0eca4c4..77bf300 100644
>>>>> --- a/app/t/pktgen.t.c
>>>>> +++ b/app/t/pktgen.t.c
>>>>> @@ -77,7 +77,7 @@ void test_pktgen_packet_ctor_IPv4_UDP(void)
>>>>> };
>>>>>
>>>>>
>>>>> - lives_ok( { pktgen_packet_ctor(&info, 0, 0); }, "pktgen_packet_ctor
>>>>> must generate IPv4/UDP");
>>>>> + lives_ok( { pktgen_packet_ctor(&info, 0, 0, NULL); },
>>>>> "pktgen_packet_ctor must generate IPv4/UDP");
>>>>>
>>>>> note("... with Ethernet header");
>>>>> cmp_mem_lit_incr(data, (0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd), "
>>>>>...
>>>>> with correct dest MAC");
>>>>> @@ -133,9 +133,9 @@ void test_pktgen_packet_ctor_IPv4_GRE_Ether(void)
>>>>> .pktSize = 102, // Subtract 4 for FCS
>>>>> };
>>>>>
>>>>> - pktgen_packet_ctor(&info, 0, 0);
>>>>> + pktgen_packet_ctor(&info, 0, 0, NULL);
>>>>>
>>>>> - lives_ok( { pktgen_packet_ctor(&info, 0, 0); }, "pktgen_packet_ctor
>>>>> must generate IPv4 GRE Ethernet frame");
>>>>> + lives_ok( { pktgen_packet_ctor(&info, 0, 0, NULL); },
>>>>> "pktgen_packet_ctor must generate IPv4 GRE Ethernet frame");
>>>>>
>>>>> note("... with outer Ethernet header");
>>>>> cmp_mem_lit_incr(data, (0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd), "
>>>>>...
>>>>> with correct dest MAC");
>>>>>
>>>>
>>>
>>>
>>> ‹
>>> Regards,
>>> ++Keith
>>> Intel Corporation
>>>
>>>
>>>
>>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [Pktgen] [PATCH] pktgen_setup_packets: fix race for packet header
2015-10-02 11:25 ` Wiles, Keith
@ 2015-10-12 14:09 ` Ilya Maximets
0 siblings, 0 replies; 7+ messages in thread
From: Ilya Maximets @ 2015-10-12 14:09 UTC (permalink / raw)
To: Wiles, Keith, dev; +Cc: Dyasly Sergey
Sorry for pinging, but have you tried to merge this patch?
Best regards, Ilya Maximets.
On 02.10.2015 14:25, Wiles, Keith wrote:
> I looked at the code and everything looks good. I will try to merge the
> code next week as I am traveling again :-(
>
> Thanks for the patch, I am glad you found this problem as I believe
> someone else reported something odd in that area, but was not able to give
> me many details.
> —
> Regards,
> ++Keith Wiles
>
> Intel Corporation
>
>
>
>
> On 10/2/15, 10:23 AM, "Ilya Maximets" <i.maximets@samsung.com> wrote:
>
>> Ping.
>>
>> On 17.09.2015 08:55, Ilya Maximets wrote:
>>> Ok. Thank you. I'll wait.
>>>
>>> On 16.09.2015 18:37, Wiles, Keith wrote:
>>>> Thanks the patch looks fine, but I have not had a lot of time to
>>>> review it
>>>> detail. I hope to get to it next week after I return back home.
>>>>
>>>> On 9/16/15, 2:09 AM, "Ilya Maximets" <i.maximets@samsung.com> wrote:
>>>>
>>>>> Ping.
>>>>>
>>>>> On 09.09.2015 17:22, Ilya Maximets wrote:
>>>>>> While pktgen_setup_packets() all threads of one port uses same
>>>>>> info->seq_pkt. This leads to constructing packets in the same memory
>>>>>> region
>>>>>> (&pkt->hdr). As a result, pktgen_setup_packets generates random
>>>>>> headers.
>>>>>>
>>>>>> Fix that by making a local copy of info->seq_pkt and using it for
>>>>>> constructing of packets.
>>>>>>
>>>>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>>>>>> ---
>>>>>> app/pktgen-arp.c | 2 +-
>>>>>> app/pktgen-cmds.c | 40 ++++++++++++++++++++--------------------
>>>>>> app/pktgen-ipv4.c | 2 +-
>>>>>> app/pktgen.c | 39 +++++++++++++++++++++++++++------------
>>>>>> app/pktgen.h | 4 ++--
>>>>>> app/t/pktgen.t.c | 6 +++---
>>>>>> 6 files changed, 54 insertions(+), 39 deletions(-)
>>>>>>
>>>>>> diff --git a/app/pktgen-arp.c b/app/pktgen-arp.c
>>>>>> index c378880..b7040d7 100644
>>>>>> --- a/app/pktgen-arp.c
>>>>>> +++ b/app/pktgen-arp.c
>>>>>> @@ -190,7 +190,7 @@ pktgen_process_arp( struct rte_mbuf * m, uint32_t
>>>>>> pid, uint32_t vlan )
>>>>>>
>>>>>> rte_memcpy(&pkt->eth_dst_addr, &arp->sha, 6);
>>>>>> for (i = 0; i < info->seqCnt; i++)
>>>>>> - pktgen_packet_ctor(info, i, -1);
>>>>>> + pktgen_packet_ctor(info, i, -1, NULL);
>>>>>> }
>>>>>>
>>>>>> // Swap the two MAC addresses
>>>>>> diff --git a/app/pktgen-cmds.c b/app/pktgen-cmds.c
>>>>>> index da040e5..a6abb41 100644
>>>>>> --- a/app/pktgen-cmds.c
>>>>>> +++ b/app/pktgen-cmds.c
>>>>>> @@ -931,7 +931,7 @@ pktgen_set_proto(port_info_t * info, char type)
>>>>>> if ( type == 'i' )
>>>>>> info->seq_pkt[SINGLE_PKT].ethType = ETHER_TYPE_IPv4;
>>>>>>
>>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> /*********************************************************************
>>>>>> ***
>>>>>> **//**
>>>>>> @@ -1067,7 +1067,7 @@ pktgen_set_pkt_type(port_info_t * info, const
>>>>>> char * type)
>>>>>> (type[3] == '6') ? ETHER_TYPE_IPv6 :
>>>>>> /* TODO print error: unknown type */ ETHER_TYPE_IPv4;
>>>>>>
>>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> /*********************************************************************
>>>>>> ***
>>>>>> **//**
>>>>>> @@ -1092,7 +1092,7 @@ pktgen_set_vlan(port_info_t * info, uint32_t
>>>>>> onOff)
>>>>>> }
>>>>>> else
>>>>>> pktgen_clr_port_flags(info, SEND_VLAN_ID);
>>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> /*********************************************************************
>>>>>> ***
>>>>>> **//**
>>>>>> @@ -1112,7 +1112,7 @@ pktgen_set_vlanid(port_info_t * info, uint16_t
>>>>>> vlanid)
>>>>>> {
>>>>>> info->vlanid = vlanid;
>>>>>> info->seq_pkt[SINGLE_PKT].vlanid = info->vlanid;
>>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> /*********************************************************************
>>>>>> ***
>>>>>> **//**
>>>>>> @@ -1137,7 +1137,7 @@ pktgen_set_mpls(port_info_t * info, uint32_t
>>>>>> onOff)
>>>>>> }
>>>>>> else
>>>>>> pktgen_clr_port_flags(info, SEND_MPLS_LABEL);
>>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> /*********************************************************************
>>>>>> ***
>>>>>> **//**
>>>>>> @@ -1157,7 +1157,7 @@ pktgen_set_mpls_entry(port_info_t * info,
>>>>>> uint32_t mpls_entry)
>>>>>> {
>>>>>> info->mpls_entry = mpls_entry;
>>>>>> info->seq_pkt[SINGLE_PKT].mpls_entry = info->mpls_entry;
>>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> /*********************************************************************
>>>>>> ***
>>>>>> **//**
>>>>>> @@ -1182,7 +1182,7 @@ pktgen_set_qinq(port_info_t * info, uint32_t
>>>>>> onOff)
>>>>>> }
>>>>>> else
>>>>>> pktgen_clr_port_flags(info, SEND_Q_IN_Q_IDS);
>>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> /*********************************************************************
>>>>>> ***
>>>>>> **//**
>>>>>> @@ -1204,7 +1204,7 @@ pktgen_set_qinqids(port_info_t * info, uint16_t
>>>>>> outerid, uint16_t innerid)
>>>>>> info->seq_pkt[SINGLE_PKT].qinq_outerid = info->qinq_outerid;
>>>>>> info->qinq_innerid = innerid;
>>>>>> info->seq_pkt[SINGLE_PKT].qinq_innerid = info->qinq_innerid;
>>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> /*********************************************************************
>>>>>> ***
>>>>>> **//**
>>>>>> @@ -1228,7 +1228,7 @@ pktgen_set_gre(port_info_t * info, uint32_t
>>>>>> onOff)
>>>>>> }
>>>>>> else
>>>>>> pktgen_clr_port_flags(info, SEND_GRE_IPv4_HEADER);
>>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> /*********************************************************************
>>>>>> ***
>>>>>> **//**
>>>>>> @@ -1252,7 +1252,7 @@ pktgen_set_gre_eth(port_info_t * info, uint32_t
>>>>>> onOff)
>>>>>> }
>>>>>> else
>>>>>> pktgen_clr_port_flags(info, SEND_GRE_ETHER_HEADER);
>>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> /*********************************************************************
>>>>>> ***
>>>>>> **//**
>>>>>> @@ -1272,7 +1272,7 @@ pktgen_set_gre_key(port_info_t * info, uint32_t
>>>>>> gre_key)
>>>>>> {
>>>>>> info->gre_key = gre_key;
>>>>>> info->seq_pkt[SINGLE_PKT].gre_key = info->gre_key;
>>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> /*********************************************************************
>>>>>> ***
>>>>>> **//**
>>>>>> @@ -1401,7 +1401,7 @@ void pktgen_port_defaults(uint32_t pid, uint8_t
>>>>>> seq)
>>>>>> memset(&pkt->eth_dst_addr, 0, sizeof (pkt->eth_dst_addr));
>>>>>>
>>>>>>
>>>>>> - pktgen_packet_ctor(info, seq, -1);
>>>>>> + pktgen_packet_ctor(info, seq, -1, NULL);
>>>>>>
>>>>>> pktgen.flags |= PRINT_LABELS_FLAG;
>>>>>> }
>>>>>> @@ -1423,7 +1423,7 @@ void pktgen_ping4(port_info_t * info)
>>>>>> memcpy(&info->seq_pkt[PING_PKT],
>>>>>> &info->seq_pkt[SINGLE_PKT], sizeof(pkt_seq_t));
>>>>>> info->seq_pkt[PING_PKT].ipProto = PG_IPPROTO_ICMP;
>>>>>> - pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO);
>>>>>> + pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO, NULL);
>>>>>> pktgen_set_port_flags(info, SEND_PING4_REQUEST);
>>>>>> }
>>>>>>
>>>>>> @@ -1445,7 +1445,7 @@ void pktgen_ping6(port_info_t * info)
>>>>>> memcpy(&info->pkt[PING_PKT],
>>>>>> &info->pkt[SINGLE_PKT], sizeof(pkt_seq_t));
>>>>>> info->pkt[PING_PKT].ipProto = PG_IPPROTO_ICMP;
>>>>>> - pktgen_packet_ctor(info, PING_PKT, ICMP6_ECHO);
>>>>>> + pktgen_packet_ctor(info, PING_PKT, ICMP6_ECHO, NULL);
>>>>>> pktgen_set_port_flags(info, SEND_PING6_REQUEST);
>>>>>> }
>>>>>> #endif
>>>>>> @@ -1645,7 +1645,7 @@ void pktgen_set_pkt_size(port_info_t * info,
>>>>>> uint32_t size)
>>>>>> else if ( (size - FCS_SIZE) > MAX_PKT_SIZE)
>>>>>> size = MAX_PKT_SIZE + FCS_SIZE;
>>>>>> info->seq_pkt[SINGLE_PKT].pktSize = (size - FCS_SIZE);
>>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>>> pktgen_packet_rate(info);
>>>>>> }
>>>>>>
>>>>>> @@ -1667,7 +1667,7 @@ void pktgen_set_port_value(port_info_t * info,
>>>>>> char type, uint32_t portValue)
>>>>>> info->seq_pkt[SINGLE_PKT].dport = (uint16_t)portValue;
>>>>>> else
>>>>>> info->seq_pkt[SINGLE_PKT].sport = (uint16_t)portValue;
>>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> /*********************************************************************
>>>>>> ***
>>>>>> **//**
>>>>>> @@ -1711,7 +1711,7 @@ void pktgen_set_ipaddr(port_info_t * info, char
>>>>>> type, cmdline_ipaddr_t * ip)
>>>>>> info->seq_pkt[SINGLE_PKT].ip_src_addr =
>>>>>> ntohl(ip->addr.ipv4.s_addr);
>>>>>> } else
>>>>>> info->seq_pkt[SINGLE_PKT].ip_dst_addr =
>>>>>> ntohl(ip->addr.ipv4.s_addr);
>>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> /*********************************************************************
>>>>>> ***
>>>>>> **//**
>>>>>> @@ -1729,7 +1729,7 @@ void pktgen_set_ipaddr(port_info_t * info, char
>>>>>> type, cmdline_ipaddr_t * ip)
>>>>>> void pktgen_set_dst_mac(port_info_t * info, cmdline_etheraddr_t *
>>>>>> mac)
>>>>>> {
>>>>>> memcpy(&info->seq_pkt[SINGLE_PKT].eth_dst_addr, mac->mac, 6);
>>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, NULL);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> /*********************************************************************
>>>>>> ***
>>>>>> **//**
>>>>>> @@ -2111,7 +2111,7 @@ void pktgen_set_seq(port_info_t * info,
>>>>>> uint32_t
>>>>>> seqnum,
>>>>>> type = '4';
>>>>>> pkt->ethType = (type == '6')? ETHER_TYPE_IPv6 : ETHER_TYPE_IPv4;
>>>>>> pkt->vlanid = vlanid;
>>>>>> - pktgen_packet_ctor(info, seqnum, -1);
>>>>>> + pktgen_packet_ctor(info, seqnum, -1, NULL);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> /*********************************************************************
>>>>>> ***
>>>>>> **//**
>>>>>> @@ -2156,7 +2156,7 @@ void pktgen_compile_pkt(port_info_t * info,
>>>>>> uint32_t seqnum,
>>>>>> (type == '6')? ETHER_TYPE_IPv6 :
>>>>>> (type == 'n')? ETHER_TYPE_VLAN : ETHER_TYPE_IPv4;
>>>>>> pkt->vlanid = vlanid;
>>>>>> - pktgen_packet_ctor(info, seqnum, -1);
>>>>>> + pktgen_packet_ctor(info, seqnum, -1, NULL);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> /*********************************************************************
>>>>>> ***
>>>>>> **//**
>>>>>> diff --git a/app/pktgen-ipv4.c b/app/pktgen-ipv4.c
>>>>>> index 7813c36..a8b6be2 100644
>>>>>> --- a/app/pktgen-ipv4.c
>>>>>> +++ b/app/pktgen-ipv4.c
>>>>>> @@ -138,7 +138,7 @@ pktgen_send_ping4( uint32_t pid, uint8_t seq_idx
>>>>>> )
>>>>>> return;
>>>>>> }
>>>>>> *ppkt = *spkt; // Copy the sequence setup to the ping setup.
>>>>>> - pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO);
>>>>>> + pktgen_packet_ctor(info, PING_PKT, ICMP4_ECHO, NULL);
>>>>>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>>>>> *)&ppkt->hdr, ppkt->pktSize);
>>>>>>
>>>>>> m->pkt_len = ppkt->pktSize;
>>>>>> diff --git a/app/pktgen.c b/app/pktgen.c
>>>>>> index 2dcdbfd..ecd4560 100644
>>>>>> --- a/app/pktgen.c
>>>>>> +++ b/app/pktgen.c
>>>>>> @@ -401,8 +401,8 @@ static __inline__ int pktgen_has_work(void) {
>>>>>> */
>>>>>>
>>>>>> void
>>>>>> -pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t
>>>>>> type) {
>>>>>> - pkt_seq_t * pkt = &info->seq_pkt[seq_idx];
>>>>>> +pktgen_packet_ctor(port_info_t * info, int32_t seq_idx, int32_t
>>>>>> type,
>>>>>> pkt_seq_t * seq_pkt) {
>>>>>> + pkt_seq_t * pkt = (seq_pkt == NULL) ? &info->seq_pkt[seq_idx] :
>>>>>> &seq_pkt[seq_idx];
>>>>>> struct ether_hdr * eth = (struct ether_hdr *)&pkt->hdr.eth;
>>>>>> uint16_t tlen;
>>>>>>
>>>>>> @@ -812,22 +812,35 @@ static __inline__ void
>>>>>> pktgen_setup_packets(port_info_t * info, struct rte_mempool * mp,
>>>>>> uint16_t qid)
>>>>>> {
>>>>>> struct rte_mbuf * m, * mm;
>>>>>> - pkt_seq_t * pkt;
>>>>>> + pkt_seq_t * pkt, * seq_pkt = NULL;
>>>>>> + uint16_t seqIdx;
>>>>>> + char buff[RTE_MEMZONE_NAMESIZE];
>>>>>>
>>>>>> pktgen_clr_q_flags(info, qid, CLEAR_FAST_ALLOC_FLAG);
>>>>>>
>>>>>> if ( mp == info->q[qid].pcap_mp )
>>>>>> return;
>>>>>>
>>>>>> + snprintf(buff, sizeof(buff), "tmp_seq_pkt_%d_%d", info->pid, qid);
>>>>>> + seq_pkt = (pkt_seq_t *)rte_zmalloc(buff,
>>>>>> + (sizeof(pkt_seq_t) * NUM_TOTAL_PKTS), RTE_CACHE_LINE_SIZE);
>>>>>> + if (seq_pkt == NULL)
>>>>>> + pktgen_log_panic("Unable to allocate %d pkt_seq_t headers",
>>>>>> + NUM_TOTAL_PKTS);
>>>>>> + /* Copy global configuration to construct new packets locally */
>>>>>> + rte_memcpy((uint8_t *)seq_pkt, (uint8_t *)info->seq_pkt,
>>>>>> + sizeof(pkt_seq_t) * NUM_TOTAL_PKTS);
>>>>>> + seqIdx = info->seqIdx;
>>>>>> +
>>>>>> mm = NULL;
>>>>>> pkt = NULL;
>>>>>>
>>>>>> if ( mp == info->q[qid].tx_mp )
>>>>>> - pkt = &info->seq_pkt[SINGLE_PKT];
>>>>>> + pkt = &seq_pkt[SINGLE_PKT];
>>>>>> else if ( mp == info->q[qid].range_mp )
>>>>>> - pkt = &info->seq_pkt[RANGE_PKT];
>>>>>> + pkt = &seq_pkt[RANGE_PKT];
>>>>>> else if ( mp == info->q[qid].seq_mp )
>>>>>> - pkt = &info->seq_pkt[info->seqIdx];
>>>>>> + pkt = &seq_pkt[seqIdx];
>>>>>>
>>>>>> // allocate each mbuf and put them on a list to be freed.
>>>>>> for(;;) {
>>>>>> @@ -840,7 +853,7 @@ pktgen_setup_packets(port_info_t * info, struct
>>>>>> rte_mempool * mp, uint16_t qid)
>>>>>> mm = m;
>>>>>>
>>>>>> if ( mp == info->q[qid].tx_mp ) {
>>>>>> - pktgen_packet_ctor(info, SINGLE_PKT, -1);
>>>>>> + pktgen_packet_ctor(info, SINGLE_PKT, -1, seq_pkt);
>>>>>>
>>>>>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>>>>> *)&pkt->hdr, MAX_PKT_SIZE);
>>>>>>
>>>>>> @@ -848,16 +861,16 @@ pktgen_setup_packets(port_info_t * info, struct
>>>>>> rte_mempool * mp, uint16_t qid)
>>>>>> m->data_len = pkt->pktSize;
>>>>>> } else if ( mp == info->q[qid].range_mp ) {
>>>>>> pktgen_range_ctor(&info->range, pkt);
>>>>>> - pktgen_packet_ctor(info, RANGE_PKT, -1);
>>>>>> + pktgen_packet_ctor(info, RANGE_PKT, -1, seq_pkt);
>>>>>>
>>>>>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>>>>> *)&pkt->hdr, MAX_PKT_SIZE);
>>>>>>
>>>>>> m->pkt_len = pkt->pktSize;
>>>>>> m->data_len = pkt->pktSize;
>>>>>> } else if ( mp == info->q[qid].seq_mp ) {
>>>>>> - pktgen_packet_ctor(info, info->seqIdx++, -1);
>>>>>> - if ( unlikely(info->seqIdx >= info->seqCnt) )
>>>>>> - info->seqIdx = 0;
>>>>>> + pktgen_packet_ctor(info, seqIdx++, -1, seq_pkt);
>>>>>> + if ( unlikely(seqIdx >= info->seqCnt) )
>>>>>> + seqIdx = 0;
>>>>>>
>>>>>> rte_memcpy((uint8_t *)m->buf_addr + m->data_off, (uint8_t
>>>>>> *)&pkt->hdr, MAX_PKT_SIZE);
>>>>>>
>>>>>> @@ -865,12 +878,14 @@ pktgen_setup_packets(port_info_t * info, struct
>>>>>> rte_mempool * mp, uint16_t qid)
>>>>>> m->data_len = pkt->pktSize;
>>>>>>
>>>>>> // move to the next packet in the sequence.
>>>>>> - pkt = &info->seq_pkt[info->seqIdx];
>>>>>> + pkt = &seq_pkt[seqIdx];
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> if ( mm != NULL )
>>>>>> rte_pktmbuf_free(mm);
>>>>>> +
>>>>>> + rte_free(seq_pkt);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>> /*********************************************************************
>>>>>> ***
>>>>>> **//**
>>>>>> diff --git a/app/pktgen.h b/app/pktgen.h
>>>>>> index 2c5c98c..f3f76f2 100644
>>>>>> --- a/app/pktgen.h
>>>>>> +++ b/app/pktgen.h
>>>>>> @@ -151,7 +151,7 @@
>>>>>> #include "pktgen-port-cfg.h"
>>>>>> #include "pktgen-capture.h"
>>>>>> #include "pktgen-log.h"
>>>>>> -
>>>>>> +#include "pktgen-seq.h"
>>>>>>
>>>>>> #define PKTGEN_VERSION "2.9.2"
>>>>>> #define PKTGEN_APP_NAME "Pktgen"
>>>>>> @@ -386,7 +386,7 @@ extern pktgen_t pktgen;
>>>>>>
>>>>>> extern void pktgen_page_display(__attribute__((unused)) struct
>>>>>> rte_timer *tim, __attribute__((unused)) void *arg);
>>>>>>
>>>>>> -extern void pktgen_packet_ctor(port_info_t * info, int32_t seq_idx,
>>>>>> int32_t type);
>>>>>> +extern void pktgen_packet_ctor(port_info_t * info, int32_t seq_idx,
>>>>>> int32_t type, pkt_seq_t * seq_pkt);
>>>>>> extern void pktgen_packet_rate(port_info_t * info);
>>>>>>
>>>>>> extern void pktgen_send_mbuf(struct rte_mbuf *m, uint8_t pid,
>>>>>> uint16_t
>>>>>> qid);
>>>>>> diff --git a/app/t/pktgen.t.c b/app/t/pktgen.t.c
>>>>>> index 0eca4c4..77bf300 100644
>>>>>> --- a/app/t/pktgen.t.c
>>>>>> +++ b/app/t/pktgen.t.c
>>>>>> @@ -77,7 +77,7 @@ void test_pktgen_packet_ctor_IPv4_UDP(void)
>>>>>> };
>>>>>>
>>>>>>
>>>>>> - lives_ok( { pktgen_packet_ctor(&info, 0, 0); }, "pktgen_packet_ctor
>>>>>> must generate IPv4/UDP");
>>>>>> + lives_ok( { pktgen_packet_ctor(&info, 0, 0, NULL); },
>>>>>> "pktgen_packet_ctor must generate IPv4/UDP");
>>>>>>
>>>>>> note("... with Ethernet header");
>>>>>> cmp_mem_lit_incr(data, (0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd), "
>>>>>> ...
>>>>>> with correct dest MAC");
>>>>>> @@ -133,9 +133,9 @@ void test_pktgen_packet_ctor_IPv4_GRE_Ether(void)
>>>>>> .pktSize = 102, // Subtract 4 for FCS
>>>>>> };
>>>>>>
>>>>>> - pktgen_packet_ctor(&info, 0, 0);
>>>>>> + pktgen_packet_ctor(&info, 0, 0, NULL);
>>>>>>
>>>>>> - lives_ok( { pktgen_packet_ctor(&info, 0, 0); }, "pktgen_packet_ctor
>>>>>> must generate IPv4 GRE Ethernet frame");
>>>>>> + lives_ok( { pktgen_packet_ctor(&info, 0, 0, NULL); },
>>>>>> "pktgen_packet_ctor must generate IPv4 GRE Ethernet frame");
>>>>>>
>>>>>> note("... with outer Ethernet header");
>>>>>> cmp_mem_lit_incr(data, (0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd), "
>>>>>> ...
>>>>>> with correct dest MAC");
>>>>>>
>>>>>
>>>>
>>>>
>>>> ‹
>>>> Regards,
>>>> ++Keith
>>>> Intel Corporation
>>>>
>>>>
>>>>
>>>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-10-12 14:09 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-09 14:22 [dpdk-dev] [Pktgen] [PATCH] pktgen_setup_packets: fix race for packet header Ilya Maximets
2015-09-16 9:09 ` Ilya Maximets
2015-09-16 15:37 ` Wiles, Keith
2015-09-17 5:55 ` Ilya Maximets
2015-10-02 9:23 ` Ilya Maximets
2015-10-02 11:25 ` Wiles, Keith
2015-10-12 14:09 ` Ilya Maximets
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).