From: "Hunt, David" <david.hunt@intel.com>
To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Cc: dev@dpdk.org, harry.van.haaren@intel.com,
Gage Eads <gage.eads@intel.com>,
Bruce Richardson <bruce.richardson@intel.com>
Subject: Re: [dpdk-dev] [PATCH v5 1/3] examples/eventdev_pipeline: added sample app
Date: Wed, 5 Jul 2017 12:15:51 +0100 [thread overview]
Message-ID: <dc8aa64b-a5ad-f52a-1173-0be9a5af8ccb@intel.com> (raw)
In-Reply-To: <20170705052853.GA8031@jerin>
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.hunt@intel.com>
>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>> CC: dev@dpdk.org, harry.van.haaren@intel.com, Gage Eads
>> <gage.eads@intel.com>, Bruce Richardson <bruce.richardson@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.
>>>> +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.
>
>>>> +
>>>> + if (!quiet) {
>>>> + printf("\nPort Workload distribution:\n");
>>>> + uint32_t i;
>>>> + uint64_t tot_pkts = 0;
>>>> + uint64_t pkts_per_wkr[RTE_MAX_LCORE] = {0};
>>>> + for (i = 0; i < num_workers; i++) {
>>>> + char statname[64];
>>>> + snprintf(statname, sizeof(statname), "port_%u_rx",
>>>> + worker_data[i].port_id);
>>>> + pkts_per_wkr[i] = rte_event_dev_xstats_by_name_get(
>>>> + dev_id, statname, NULL);
>>> As discussed, Check the the given xstat supported on the PMD first.
>> Checking has now been implemented. It'd done by calling
>> rte_event_dev_xstats_by_name_get()
>> and seeing if the result is -ENOTSUP. However there is a bug in the function
>> in that it is declared
>> as a uint64_t, but then returns a -ENOTSUP, so I have to cast the -ENOTSUP
>> as a uint64_t for
>> comparison. This will need to be fixed when the function is patched.
>>
>> retval = rte_event_dev_xstats_by_name_get(
>> dev_id, statname, NULL);
>> if (retval != (uint64_t)-ENOTSUP) {
>> pkts_per_wkr[i] = retval;
>> tot_pkts += pkts_per_wkr[i];
>> }
>>
>>
>>
>>>> + tot_pkts += pkts_per_wkr[i];
>>>> + }
>>>> + for (i = 0; i < num_workers; i++) {
>>>> + float pc = pkts_per_wkr[i] * 100 /
>>>> + ((float)tot_pkts);
> This will print NAN.
>
> How about, move the specific xstat as static inline function.
> port_stat(...)
> {
> char statname[64];
> uint64_t retval;
> snprintf(statname, sizeof(statname),"port_%u_rx",
> worker_data[i].port_id);
> retval = rte_event_dev_xstats_by_name_get(
> dev_id, statname, NULL);
> }
> and add check in the beginning of the "if" condition.
> ie.
>
> if (!cdata.quiet && port_stat(,,) != (uint64_t)-ENOTSUP) {
> printf("\nPort Workload distribution:\n");
> ...
> }
>
Sure. Adding the port_stat simplifies a lot. Will do.
>>>> + printf("worker %i :\t%.1f %% (%"PRIu64" pkts)\n",
>>>> + i, pc, pkts_per_wkr[i]);
>>>> + }
>>>> +
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
next prev parent reply other threads:[~2017-07-05 11:16 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-21 9:51 [dpdk-dev] [PATCH 0/3] next-eventdev: RFC evendev pipeline " Harry van Haaren
2017-04-21 9:51 ` [dpdk-dev] [PATCH 1/3] examples/eventdev_pipeline: added " Harry van Haaren
2017-05-10 14:12 ` Jerin Jacob
2017-05-10 16:40 ` Eads, Gage
2017-05-10 20:16 ` Eads, Gage
2017-06-26 14:46 ` Hunt, David
2017-06-27 9:35 ` Jerin Jacob
2017-06-27 13:12 ` Hunt, David
2017-06-29 7:17 ` Jerin Jacob
2017-06-29 12:51 ` Hunt, David
2017-05-17 18:03 ` Jerin Jacob
2017-05-18 10:13 ` Bruce Richardson
2017-06-26 14:41 ` [dpdk-dev] [PATCH v2 0/3] next-eventdev: evendev pipeline " David Hunt
2017-06-26 14:41 ` [dpdk-dev] [PATCH v2 1/3] examples/eventdev_pipeline: added " David Hunt
2017-06-27 12:54 ` [dpdk-dev] [PATCH v3 0/3] next-eventdev: evendev pipeline " David Hunt
2017-06-27 12:54 ` [dpdk-dev] [PATCH v3 1/3] examples/eventdev_pipeline: added " David Hunt
2017-06-29 15:49 ` [dpdk-dev] [PATCH v4 0/3] next-eventdev: evendev pipeline " David Hunt
2017-06-29 15:49 ` [dpdk-dev] [PATCH v4 1/3] examples/eventdev_pipeline: added " David Hunt
2017-06-30 13:51 ` [dpdk-dev] [PATCH v5 0/3] next-eventdev: evendev pipeline " David Hunt
2017-06-30 13:51 ` [dpdk-dev] [PATCH v5 1/3] examples/eventdev_pipeline: added " David Hunt
2017-07-03 3:57 ` Jerin Jacob
2017-07-04 7:55 ` Hunt, David
2017-07-05 5:30 ` Jerin Jacob
2017-07-05 11:15 ` Hunt, David [this message]
2017-07-06 3:31 ` Jerin Jacob
2017-07-06 10:04 ` Hunt, David
2017-07-06 10:39 ` Hunt, David
2017-07-06 13:26 ` Hunt, David
2017-07-06 13:38 ` Jerin Jacob
2017-07-04 8:14 ` [dpdk-dev] [PATCH v6 0/3] next-eventdev: evendev pipeline " David Hunt
2017-07-04 8:14 ` [dpdk-dev] [PATCH v6 1/3] examples/eventdev_pipeline: added " David Hunt
2017-07-05 12:52 ` [dpdk-dev] [PATCH v7 0/3] next-eventdev: evendev pipeline " David Hunt
2017-07-05 12:52 ` [dpdk-dev] [PATCH v7 1/3] examples/eventdev_pipeline: added " David Hunt
2017-07-06 14:35 ` [dpdk-dev] [PATCH v8 0/3] next-eventdev: evendev pipeline " David Hunt
2017-07-06 14:35 ` [dpdk-dev] [PATCH v8 1/3] examples/eventdev_pipeline_sw_pmd: add " David Hunt
2017-07-06 14:35 ` [dpdk-dev] [PATCH v8 2/3] doc: add SW eventdev pipeline to sample app ug David Hunt
2017-07-06 14:35 ` [dpdk-dev] [PATCH v8 3/3] doc: add eventdev library to programmers guide David Hunt
2017-07-07 4:50 ` [dpdk-dev] [PATCH v8 0/3] next-eventdev: evendev pipeline sample app Jerin Jacob
2017-07-05 12:52 ` [dpdk-dev] [PATCH v7 2/3] doc: add sw eventdev pipeline to sample app ug David Hunt
2017-07-05 12:52 ` [dpdk-dev] [PATCH v7 3/3] doc: add eventdev library to programmers guide David Hunt
2017-07-04 8:14 ` [dpdk-dev] [PATCH v6 2/3] doc: add sw eventdev pipeline to sample app ug David Hunt
2017-07-05 4:30 ` Jerin Jacob
2017-07-04 8:14 ` [dpdk-dev] [PATCH v6 3/3] doc: add eventdev library to programmers guide David Hunt
2017-06-30 13:51 ` [dpdk-dev] [PATCH v5 2/3] doc: add sw eventdev pipeline to sample app ug David Hunt
2017-06-30 14:37 ` Mcnamara, John
2017-07-03 5:37 ` Jerin Jacob
2017-07-03 9:25 ` Hunt, David
2017-07-03 9:32 ` Jerin Jacob
2017-07-04 8:20 ` Hunt, David
2017-06-30 13:51 ` [dpdk-dev] [PATCH v5 3/3] doc: add eventdev library to programmers guide David Hunt
2017-06-30 14:38 ` Mcnamara, John
2017-07-02 12:08 ` Jerin Jacob
2017-06-29 15:49 ` [dpdk-dev] [PATCH v4 2/3] doc: add sw eventdev pipeline to sample app ug David Hunt
2017-06-30 12:25 ` Mcnamara, John
2017-06-29 15:49 ` [dpdk-dev] [PATCH v4 3/3] doc: add eventdev library to programmers guide David Hunt
2017-06-30 12:26 ` Mcnamara, John
2017-06-27 12:54 ` [dpdk-dev] [PATCH v3 2/3] doc: add eventdev pipeline to sample app ug David Hunt
2017-06-27 12:54 ` [dpdk-dev] [PATCH v3 3/3] doc: add eventdev library to programmers guide David Hunt
2017-06-26 14:41 ` [dpdk-dev] [PATCH v2 2/3] doc: add eventdev pipeline to sample app ug David Hunt
2017-06-26 14:41 ` [dpdk-dev] [PATCH v2 3/3] doc: add eventdev library to programmers guide David Hunt
2017-04-21 9:51 ` [dpdk-dev] [PATCH 2/3] doc: add eventdev pipeline to sample app ug Harry van Haaren
2017-04-21 9:51 ` [dpdk-dev] [PATCH 3/3] doc: add eventdev library to programmers guide Harry van Haaren
2017-04-21 11:14 ` Bruce Richardson
2017-04-21 14:00 ` Jerin Jacob
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=dc8aa64b-a5ad-f52a-1173-0be9a5af8ccb@intel.com \
--to=david.hunt@intel.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=gage.eads@intel.com \
--cc=harry.van.haaren@intel.com \
--cc=jerin.jacob@caviumnetworks.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).