From: "Liang, Cunming" <cunming.liang@intel.com>
To: "Richardson, Bruce" <bruce.richardson@intel.com>,
"Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
Neil Horman <nhorman@tuxdriver.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2 1/4] app/test: unit test for rx and tx cycles/packet
Date: Fri, 24 Oct 2014 03:06:36 +0000 [thread overview]
Message-ID: <D0158A423229094DA7ABF71CF2FA0DA311851A5D@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <59AF69C657FD0841A61C55336867B5B034421C3A@IRSMSX103.ger.corp.intel.com>
It's reasonable to me.
I'll make a patch for rte_ethdev.c.
> -----Original Message-----
> From: Richardson, Bruce
> Sent: Wednesday, October 22, 2014 11:10 PM
> To: Ananyev, Konstantin; Neil Horman; Liang, Cunming
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 1/4] app/test: unit test for rx and tx
> cycles/packet
>
>
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konstantin
> > Sent: Wednesday, October 22, 2014 3:53 PM
> > To: Neil Horman; Liang, Cunming
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2 1/4] app/test: unit test for rx and tx
> > cycles/packet
> >
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> > > Sent: Wednesday, October 22, 2014 3:03 PM
> > > To: Liang, Cunming
> > > Cc: dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v2 1/4] app/test: unit test for rx and tx
> > cycles/packet
> > >
> > > On Tue, Oct 21, 2014 at 01:17:01PM +0000, Liang, Cunming wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > > > > Sent: Tuesday, October 21, 2014 6:33 PM
> > > > > To: Liang, Cunming
> > > > > Cc: dev@dpdk.org
> > > > > Subject: Re: [dpdk-dev] [PATCH v2 1/4] app/test: unit test for rx and tx
> > > > > cycles/packet
> > > > >
> > > > > On Sun, Oct 12, 2014 at 11:10:39AM +0000, Liang, Cunming wrote:
> > > > > > Hi Neil,
> > > > > >
> > > > > > Very appreciate your comments.
> > > > > > I add inline reply, will send v3 asap when we get alignment.
> > > > > >
> > > > > > BRs,
> > > > > > Liang Cunming
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > > > > > > Sent: Saturday, October 11, 2014 1:52 AM
> > > > > > > To: Liang, Cunming
> > > > > > > Cc: dev@dpdk.org
> > > > > > > Subject: Re: [dpdk-dev] [PATCH v2 1/4] app/test: unit test for rx and tx
> > > > > cycles/packet
> > > > > > >
> <...snip...>
> > > > > > >
> > > > > > > > + printf("Force Stop!\n");
> > > > > > > > + stop = 1;
> > > > > > > > + }
> > > > > > > > + if (signum == SIGUSR2)
> > > > > > > > + stats_display(0);
> > > > > > > > +}
> > > > > > > > +/* main processing loop */
> > > > > > > > +static int
> > > > > > > > +main_loop(__rte_unused void *args)
> > > > > > > > +{
> > > > > > > > +#define PACKET_SIZE 64
> > > > > > > > +#define FRAME_GAP 12
> > > > > > > > +#define MAC_PREAMBLE 8
> > > > > > > > + struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
> > > > > > > > + unsigned lcore_id;
> > > > > > > > + unsigned i, portid, nb_rx = 0, nb_tx = 0;
> > > > > > > > + struct lcore_conf *conf;
> > > > > > > > + uint64_t prev_tsc, cur_tsc;
> > > > > > > > + int pkt_per_port;
> > > > > > > > + uint64_t packets_per_second, total_packets;
> > > > > > > > +
> > > > > > > > + lcore_id = rte_lcore_id();
> > > > > > > > + conf = &lcore_conf[lcore_id];
> > > > > > > > + if (conf->status != LCORE_USED)
> > > > > > > > + return 0;
> > > > > > > > +
> > > > > > > > + pkt_per_port = MAX_TRAFIC_BURST / conf->nb_ports;
> > > > > > > > +
> > > > > > > > + int idx = 0;
> > > > > > > > + for (i = 0; i < conf->nb_ports; i++) {
> > > > > > > > + int num = pkt_per_port;
> > > > > > > > + portid = conf->portlist[i];
> > > > > > > > + printf("inject %d packet to port %d\n", num, portid);
> > > > > > > > + while (num) {
> > > > > > > > + nb_tx = RTE_MIN(MAX_PKT_BURST, num);
> > > > > > > > + nb_tx = rte_eth_tx_burst(portid, 0,
> > > > > > > > + &tx_burst[idx], nb_tx);
> > > > > > > > + num -= nb_tx;
> > > > > > > > + idx += nb_tx;
> > > > > > > > + }
> > > > > > > > + }
> > > > > > > > + printf("Total packets inject to prime ports = %u\n", idx);
> > > > > > > > +
> > > > > > > > + packets_per_second = (link_mbps * 1000 * 1000) /
> > > > > > > > + +((PACKET_SIZE + FRAME_GAP + MAC_PREAMBLE) *
> > CHAR_BIT);
> > > > > > > > + printf("Each port will do %"PRIu64" packets per second\n",
> > > > > > > > + +packets_per_second);
> > > > > > > > +
> > > > > > > > + total_packets = RTE_TEST_DURATION * conf->nb_ports *
> > > > > > > packets_per_second;
> > > > > > > > + printf("Test will stop after at least %"PRIu64" packets
> > received\n",
> > > > > > > > + + total_packets);
> > > > > > > > +
> > > > > > > > + prev_tsc = rte_rdtsc();
> > > > > > > > +
> > > > > > > > + while (likely(!stop)) {
> > > > > > > > + for (i = 0; i < conf->nb_ports; i++) {
> > > > > > > > + portid = conf->portlist[i];
> > > > > > > > + nb_rx = rte_eth_rx_burst((uint8_t) portid, 0,
> > > > > > > > + pkts_burst,
> > MAX_PKT_BURST);
> > > > > > > > + if (unlikely(nb_rx == 0)) {
> > > > > > > > + idle++;
> > > > > > > > + continue;
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + count += nb_rx;
> > > > > > > Doesn't take into consideration error conditions. rte_eth_rx_burst
> can
> > > > > return
> > > > > > > -ENOTSUP
> > > > > > [Liang, Cunming] It returns -ENOTSUP when turning on ETHDEV_DEBUG
> > > > > CONFIG.
> > > > > > The error is used to avoid no function call register.
> > > > > > When ETHDEV_DEBUG turn off, the NULL function call cause segfault
> > directly.
> > > > > > So I think it's a library internal error.
> > > > > No, look at FUNC_PTR_OR_ERR_RET. If the passed in function pointer is
> > null,
> > > > > -ENOTSUPP will be returned to the application, you need to handle the
> > error
> > > > > condition.
> > > > [Liang, Cunming] The runtime rte_eth_rx_burst is a inline function in
> > rte_ethdev.h.
> > > > The one you're talking about is the one defined in rte_ethdev.c which is a
> > extern non-inline function.
> > > > It works only when RTE_LIBRTE_ETHDEV_DEBUG turns on.
> > > > If we always turns such library internal checking on, it lose performance.
> > > > So I insist it's a library internal error checking, doesn't need to take care in
> > application level.
> > > I'm really flored that you would respond this way.
> > >
> > > What you have is two versions of a function, one returns errors and one
> > doesn't,
> > > and the version used is based on compile time configuration. You've written
> > > your application such that it can only gracefully handle one of the two
> > > versions, and you're happy to allow the other version, the one thats
> supposed
> > to
> > > be in use when you are debugging applications, to create silent accounting
> > > failures in your application. And it completely ignores the fact that the
> > > non-debug version might one day return errors as well (even if it doesn't
> > > currently). Your assertion above that we can ignore it because its debug code
> > > is the most short sighted thing I've read in a long time.
> >
> > Actually looking at rte_eth_rx_burst(): it returns uint16_t.
> > Which is sort of a strange if it expects to return a negative value as error code.
> > Also reading though 'formal' comments of rte_eth_rx_burst() - it seems that it
> > not supposed to return negative values:
> > "...
> > The rte_eth_rx_burst() function does not provide any error
> > notification to avoid the corresponding overhead. As a hint, the
> > upper-level application might check the status of the device link once
> > being systematically returned a 0 value for a given number of tries.
> > ...
> > @return
> > * The number of packets actually retrieved, which is the number
> > * of pointers to *rte_mbuf* structures effectively supplied to the
> > * *rx_pkts* array."
> >
> > Again, if you looks at rte_eth_rx_burst() implementation, when
> > RTE_LIBRTE_ETHDEV_DEBUG is on -
> > For some error cases it returns '0', foor other's: -ENOTSUP:
> >
> > FUNC_PTR_OR_ERR_RET(*dev->rx_pkt_burst, -ENOTSUP);
> > if (queue_id >= dev->data->nb_rx_queues) {
> > PMD_DEBUG_TRACE("Invalid RX queue_id=%d\n", queue_id);
> > return 0;
> > }
> >
> > Which makes me thing that we just have errnoneous implementation of
> > rte_eth_rx_burst()
> > when RTE_LIBRTE_ETHDEV_DEBUG is on.
> > Probably just a mechanical mistake, when FUNC_PTR_OR_ERR_RET() was
> > added.
> > I'd say we change rte_eth_rx_burst() to always return 0 (as was initially
> > planned).
> >
> > Konstantin
>
> Agreed. For any errors other than no-packets available yet, we can also consider
> setting rte_errno when returning 0.
>
> /Bruce
next prev parent reply other threads:[~2014-10-24 2:59 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-25 6:12 [dpdk-dev] [PATCH 0/5] app/test: unit test to measure cycles per packet Cunming Liang
2014-08-25 6:12 ` [dpdk-dev] [PATCH 1/5] app/test: unit test for rx and tx cycles/packet Cunming Liang
2014-08-25 6:12 ` [dpdk-dev] [PATCH 2/5] app/test: measure standalone rx or " Cunming Liang
2014-08-25 6:12 ` [dpdk-dev] [PATCH 3/5] ixgbe/vpmd: add fix to store unaligned mbuf point array Cunming Liang
2014-08-25 6:12 ` [dpdk-dev] [PATCH 4/5] app/test: add unit test to measure RX burst cycles Cunming Liang
2014-08-25 6:12 ` [dpdk-dev] [PATCH 5/5] app/test: allow to create packets in different sizes Cunming Liang
2014-09-03 1:46 ` [dpdk-dev] [PATCH 0/5] app/test: unit test to measure cycles per packet Zhan, Zhaochen
2014-10-10 12:29 ` [dpdk-dev] [PATCH v2 0/4] " Cunming Liang
2014-10-10 12:29 ` [dpdk-dev] [PATCH v2 1/4] app/test: unit test for rx and tx cycles/packet Cunming Liang
2014-10-10 17:52 ` Neil Horman
2014-10-12 11:10 ` Liang, Cunming
[not found] ` <E115CCD9D858EF4F90C690B0DCB4D8972262B720@IRSMSX108.ger.corp.intel.com>
2014-10-14 0:54 ` Liang, Cunming
2014-10-21 10:33 ` Neil Horman
2014-10-21 10:43 ` Richardson, Bruce
2014-10-21 13:37 ` Neil Horman
2014-10-21 22:43 ` Ananyev, Konstantin
2014-10-21 13:17 ` Liang, Cunming
2014-10-22 14:03 ` Neil Horman
2014-10-22 14:48 ` Liang, Cunming
2014-10-22 14:53 ` Ananyev, Konstantin
2014-10-22 15:09 ` Richardson, Bruce
2014-10-24 3:06 ` Liang, Cunming [this message]
2014-10-10 12:29 ` [dpdk-dev] [PATCH v2 2/4] app/test: measure standalone rx or " Cunming Liang
2014-10-10 12:30 ` [dpdk-dev] [PATCH v2 3/4] app/test: add unit test to measure RX burst cycles Cunming Liang
2014-10-10 12:30 ` [dpdk-dev] [PATCH v2 4/4] app/test: allow to create packets in different sizes Cunming Liang
2014-10-20 8:13 ` [dpdk-dev] [PATCH v3 0/2] app/test: unit test to measure cycles per packet Cunming Liang
2014-10-20 8:13 ` [dpdk-dev] [PATCH v3 1/2] app/test: allow to create packets in different sizes Cunming Liang
2014-10-20 8:13 ` [dpdk-dev] [PATCH v3 2/2] app/test: measure the cost of rx/tx routines by cycle number Cunming Liang
2014-10-21 2:40 ` [dpdk-dev] [PATCH v3 0/2] app/test: unit test to measure cycles per packet Liu, Yong
2014-10-24 5:39 ` [dpdk-dev] [PATCH v4 0/3] " Cunming Liang
2014-10-24 5:39 ` [dpdk-dev] [PATCH v4 1/3] app/test: allow to create packets in different sizes Cunming Liang
2014-10-24 5:39 ` [dpdk-dev] [PATCH v4 2/3] app/test: measure the cost of rx/tx routines by cycle number Cunming Liang
2014-10-24 5:39 ` [dpdk-dev] [PATCH v4 3/3] ethdev: fix wrong error return refer to API definition Cunming Liang
2014-10-24 5:57 ` [dpdk-dev] [PATCH v5 0/3] app/test: unit test to measure cycles per packet Cunming Liang
2014-10-24 5:58 ` [dpdk-dev] [PATCH v5 1/3] app/test: allow to create packets in different sizes Cunming Liang
2014-10-24 5:58 ` [dpdk-dev] [PATCH v5 2/3] app/test: measure the cost of rx/tx routines by cycle number Cunming Liang
2014-10-24 5:58 ` [dpdk-dev] [PATCH v5 3/3] ethdev: fix wrong error return refere to API definition Cunming Liang
2014-10-27 1:20 ` [dpdk-dev] [PATCH v6 0/3] app/test: unit test to measure cycles per packet Cunming Liang
2014-10-27 1:20 ` [dpdk-dev] [PATCH v6 1/3] app/test: allow to create packets in different sizes Cunming Liang
2014-10-27 1:20 ` [dpdk-dev] [PATCH v6 2/3] app/test: measure the cost of rx/tx routines by cycle number Cunming Liang
2014-11-11 23:28 ` Thomas Monjalon
2014-11-12 6:32 ` Liang, Cunming
2014-10-27 1:20 ` [dpdk-dev] [PATCH v6 3/3] ethdev: fix wrong error return refere to API definition Cunming Liang
2014-10-27 16:03 ` Ananyev, Konstantin
2014-10-27 1:45 ` [dpdk-dev] [PATCH v6 0/3] app/test: unit test to measure cycles per packet Liu, Yong
2014-10-29 5:06 ` Liang, Cunming
2014-11-12 6:24 ` [dpdk-dev] [PATCH v7 0/7] " Cunming Liang
2014-11-12 6:24 ` [dpdk-dev] [PATCH v7 1/7] app/test: allow to create packets in different sizes Cunming Liang
2014-11-12 6:24 ` [dpdk-dev] [PATCH v7 2/7] ixgbe:clean scattered_rx configure in dev_stop Cunming Liang
2014-11-12 7:53 ` Thomas Monjalon
2014-11-12 8:21 ` Liang, Cunming
2014-11-12 9:24 ` Thomas Monjalon
2014-11-12 10:29 ` Liang, Cunming
2014-11-12 10:32 ` Thomas Monjalon
2014-11-12 10:42 ` Liang, Cunming
2014-11-12 6:24 ` [dpdk-dev] [PATCH v7 3/7] ether: new API to format eth_addr in string Cunming Liang
2014-11-12 6:24 ` [dpdk-dev] [PATCH v7 4/7] app/testpmd: cleanup eth_addr print Cunming Liang
2014-11-12 6:24 ` [dpdk-dev] [PATCH v7 5/7] examples: " Cunming Liang
2014-11-12 6:24 ` [dpdk-dev] [PATCH v7 6/7] app/test: measure the cost of rx/tx routines by cycle number Cunming Liang
2014-11-12 6:24 ` [dpdk-dev] [PATCH v7 7/7] ethdev: fix wrong error return refere to API definition Cunming Liang
2014-11-12 23:50 ` [dpdk-dev] [PATCH v7 0/7] app/test: unit test to measure cycles per packet Thomas Monjalon
2014-10-24 5:59 ` [dpdk-dev] [PATCH v4 0/3] " Liang, Cunming
[not found] ` <1414130090-17910-1-git-send-email-y>
[not found] ` <1414130090-17910-4-git-send-email-y>
2014-10-24 11:04 ` [dpdk-dev] [PATCH v5 3/3] ethdev: fix wrong error return refere to API definition Ananyev, Konstantin
2014-10-27 0:58 ` Liang, Cunming
2014-10-28 12:21 ` [dpdk-dev] [PATCH v2 0/4] app/test: unit test to measure cycles per packet Neil Horman
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=D0158A423229094DA7ABF71CF2FA0DA311851A5D@shsmsx102.ccr.corp.intel.com \
--to=cunming.liang@intel.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=konstantin.ananyev@intel.com \
--cc=nhorman@tuxdriver.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).