On 6/7/2017 4:31 AM, Jerin Jacob wrote:
-----Original Message-----
Date: Wed, 5 Jul 2017 12:15:51 +0100
From: "Hunt, David" <david.h...@intel.com>
To: Jerin Jacob <jerin.ja...@caviumnetworks.com>
CC: dev@dpdk.org, harry.van.haa...@intel.com, Gage Eads
<gage.e...@intel.com>, Bruce Richardson <bruce.richard...@intel.com>
Subject: Re: [PATCH v5 1/3] examples/eventdev_pipeline: added sample app
User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101
Thunderbird/45.8.0
Hi Jerin,
On 5/7/2017 6:30 AM, Jerin Jacob wrote:
-----Original Message-----
Date: Tue, 4 Jul 2017 08:55:25 +0100
From: "Hunt, David" <david.h...@intel.com>
To: Jerin Jacob <jerin.ja...@caviumnetworks.com>
CC: dev@dpdk.org, harry.van.haa...@intel.com, Gage Eads
<gage.e...@intel.com>, Bruce Richardson <bruce.richard...@intel.com>
Subject: Re: [PATCH v5 1/3] examples/eventdev_pipeline: added sample app
User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101
Thunderbird/45.8.0
Hi Jerin,
Hi David,
I have checked the v6. Some comments below.
On 3/7/2017 4:57 AM, Jerin Jacob wrote:
-----Original Message-----
--snip--
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+#
+
+static unsigned int active_cores;
+static unsigned int num_workers;
+static long num_packets = (1L << 25); /* do ~32M packets */
+static unsigned int num_fids = 512;
+static unsigned int num_stages = 1;
+static unsigned int worker_cq_depth = 16;
Any reason not move this to struct config_data?
Sure. All vars now moved to either config_data or fastpath_data.
+static int queue_type = RTE_EVENT_QUEUE_CFG_ATOMIC_ONLY;
+static int16_t next_qid[MAX_NUM_STAGES+1] = {-1};
+static int16_t qid[MAX_NUM_STAGES] = {-1};
+static int worker_cycles;
used in fastpath move to struct fastpath_data.
Performance drops by ~20% when I put these in fastpath_data. No performace
drop when in config_data.
I think we need to leaving in config_data for now.
I checked v7 it looks to OK to merge. Can you fix following minor issue with
check patch and check-git-log.sh
check-git-log.sh
-----------------
Wrong headline lowercase:
doc: add sw eventdev pipeline to sample app ug
Will Do. Change sw to SW
### examples/eventdev_pipeline: added sample app
Will Do. Add _sw_pmd
Note:
Change application to new name.
checkpatch.sh
-----------------
WARNING:EMBEDDED_FUNCTION_NAME: Prefer using '"%s...", __func__' to
using 'consumer', this function's name, in a string
#294: FILE: examples/eventdev_pipeline_sw_pmd/main.c:178:
+ printf("# consumer RX=%"PRIu64", time %"PRIu64 "ms, "
WARNING:EMBEDDED_FUNCTION_NAME: Prefer using '"%s...", __func__' to
using 'worker', this function's name, in a string
#453: FILE: examples/eventdev_pipeline_sw_pmd/main.c:337:
+ printf(" worker %u thread done. RX=%zu TX=%zu\n",
total: 0 errors, 2 warnings, 1078 lines checked
These are false positives. The text in the messages are not meant to be
the function name.
If anything, I would prefer to change the function names to have " _thread"?
I will give pull request Thomas on Friday morning. I will include this change
set
in the pull request.
Regarding the performance drop, Can you add __rte_cache_aligned on those
variable which creates regression in moving to rte_malloc area. The
cache line could be shared? If not fixing then its fine we will look into that
latter.
I will investigate and post a new patch in a few hours.
and
Can you share your command to test for this regression, I will also try
on x86 box if I get time?
Sure. I'm using this:
./examples/eventdev_pipeline_sw_pmd/build/app/eventdev_pipeline_sw_pmd
-c ff0 -w 0000:18:00.0 -w 0000:18:00.1 -w 0000:18:00.2 -w 0000:18:00.3
--vdev=event_sw0 -- -s 4 -r 10 -t 10 -e 20 -w f00
+static int enable_queue_priorities;
struct config_data ?
Done.
+
+struct prod_data {
+ uint8_t dev_id;
+ uint8_t port_id;
+ int32_t qid;
+ unsigned int num_nic_ports;
+} __rte_cache_aligned;
+
+struct cons_data {
+ uint8_t dev_id;
+ uint8_t port_id;
+} __rte_cache_aligned;
+
+static struct prod_data prod_data;
+static struct cons_data cons_data;
+
+struct worker_data {
+ uint8_t dev_id;
+ uint8_t port_id;
+} __rte_cache_aligned;
+
+static unsigned int *enqueue_cnt;
+static unsigned int *dequeue_cnt;
Not been consumed. Remove it.
Done.
+
+ return 0;
+}
+
+
+static inline void
+work(struct rte_mbuf *m)
+{
+ struct ether_hdr *eth;
+ struct ether_addr addr;
+
+ /* change mac addresses on packet (to use mbuf data) */
+ eth = rte_pktmbuf_mtod(m, struct ether_hdr *);
+ ether_addr_copy(ð->d_addr, &addr);
+ ether_addr_copy(ð->s_addr, ð->d_addr);
+ ether_addr_copy(&addr, ð->s_addr);
If it is even number of stages(say 2), Will mac swap be negated? as we are
swapping on each stage NOT in consumer?
The mac swap is just to touch the mbuf. It does not matter if it is negated.
Are you sure or I am missing something here? for example,If I add following
piece
of code, irrespective number of stages it should send the same packet if
source packet is same. Right ?
But not happening as expected(Check the src and dest mac address)
stage == 1
00000000: 00 0F B7 11 27 2B 00 0F B7 11 27 2C 08 00 45 00 |....'+....',..E.
00000010: 00 2E 00 00 00 00 04 11 D5 B1 C0 A8 12 02 0E 01 |................
00000020: 00 63 10 00 10 01 00 1A 00 00 61 62 63 64 65 66 |.c........abcdef
00000030: 67 68 69 6A 6B 6C 6D 6E 6F 70 71 72 | | | | | ghijklmnopqr
stage == 2
00000000: 00 0F B7 11 27 2C 00 0F B7 11 27 2B 08 00 45 00 |....',....'+..E.
00000010: 00 2E 00 00 00 00 04 11 D5 B0 C0 A8 12 03 0E 01 |................
00000020: 00 63 10 00 10 01 00 1A 00 00 61 62 63 64 65 66 |.c........abcdef
00000030: 67 68 69 6A 6B 6C 6D 6E 6F 70 71 72 | | | | | ghijklmnopqr
diff --git a/examples/eventdev_pipeline_sw_pmd/main.c
b/examples/eventdev_pipeline_sw_pmd/main.c
index c62cba2..a7aaf37 100644
--- a/examples/eventdev_pipeline_sw_pmd/main.c
+++ b/examples/eventdev_pipeline_sw_pmd/main.c
@@ -147,8 +147,9 @@ consumer(void)
if (start_time == 0)
@@ -157,6 +158,7 @@ consumer(void)
received += n;
for (i = 0; i < n; i++) {
uint8_t outport = packets[i].mbuf->port;
+ rte_pktmbuf_dump(stdout, packets[i].mbuf, 64);
rte_eth_tx_buffer(outport, 0, fdata->tx_buf[outport],
packets[i].mbuf);
Either fix the mac swap properly or remove it.
Temporary fix added. Now reading in addr and writing it back without
swapping. Not ideal,
will probably need more work in the future. Added a FIXME in the code with
agreement from Jerin.
On a agreement that, Before moving to generic application it has to be
fixed in SW PMD driver or if its specific behavior of SW PMD then it has
to abstracted in proper way in fastpath.
