DPDK patches and discussions
 help / color / mirror / Atom feed
From: "De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
To: "Pattan, Reshma" <reshma.pattan@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v6] distributor_app: new sample app
Date: Sun, 2 Nov 2014 20:08:17 +0000
Message-ID: <E115CCD9D858EF4F90C690B0DCB4D897268323A9@IRSMSX108.ger.corp.intel.com> (raw)
In-Reply-To: <1413554380-22809-1-git-send-email-reshma.pattan@intel.com>

Hi Reshma,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Reshma Pattan
[...]

> diff --git a/examples/distributor_app/main.c
> b/examples/distributor_app/main.c
> new file mode 100644
> index 0000000..c9994a6
> --- /dev/null
> +++ b/examples/distributor_app/main.c
> @@ -0,0 +1,642 @@

[...]

> +static const struct rte_eth_rxconf rx_conf_default = {
> +	.rx_thresh = {
> +		.pthresh = RX_PTHRESH,
> +		.hthresh = RX_HTHRESH,
> +		.wthresh = RX_WTHRESH,
> +	},
> +	.rx_free_thresh = RX_FREE_THRESH,
> +	.rx_drop_en = 0,
> +};
> +
> +static const struct rte_eth_txconf tx_conf_default = {
> +	.tx_thresh = {
> +		.pthresh = TX_PTHRESH,
> +		.hthresh = TX_HTHRESH,
> +		.wthresh = TX_WTHRESH,
> +	},
> +	.tx_free_thresh = TX_FREE_THRESH,
> +	.tx_rs_thresh = TX_RSBIT_THRESH,
> +	.txq_flags = TX_Q_FLAGS
> +
> +};

You can remove these two structures (and all the macros involved),
if you pass NULL to rxconf/txconf parameters when calling RX/TX queue setup.

[...]

> +static inline int
> +port_init(uint8_t port, struct rte_mempool *mbuf_pool)
> +{
> +	struct rte_eth_conf port_conf = port_conf_default;
> +	const uint16_t rxRings = 1, txRings = rte_lcore_count() - 1;
> +	int retval;
> +	uint16_t q;
> +
> +	if (port >= rte_eth_dev_count())
> +		return -1;
> +
> +	retval = rte_eth_dev_configure(port, rxRings, txRings, &port_conf);
> +	if (retval != 0)
> +		return retval;
> +
> +	for (q = 0; q < rxRings; q++) {
> +		retval = rte_eth_rx_queue_setup(port, q, RX_RING_SIZE,
> +
> 	rte_eth_dev_socket_id(port),
> +						&rx_conf_default,
> mbuf_pool);

Recently, a new functionality has been added for RX/TX queue setup. 
You can now use NULL as rxconf/txconf parameter, so it will use 
the optimal configuration, so it will save you some lines of code and simplify it.

> +		if (retval < 0)
> +			return retval;
> +	}
> +
> +	for (q = 0; q < txRings; q++) {
> +		retval = rte_eth_tx_queue_setup(port, q, TX_RING_SIZE,
> +
> 	rte_eth_dev_socket_id(port),
> +						&tx_conf_default);
> +		if (retval < 0)
> +			return retval;
> +	}
> +
> +	retval  = rte_eth_dev_start(port);

Remove second space before "=".

> +	if (retval < 0)
> +		return retval;
> +

[...]

> +	rte_distributor_process(d, NULL, 0);
> +	/* flush distributor to bring to known state */
> +	rte_distributor_flush(d);
> +	/* set worker & tx threads quit flag */
> +	quit_signal = 1;
> +	/* worker threads may hang in get packet as
> +	   distributor process is not runnig, just make sure workers
> +	   gets packets till quit_signal is actually been
> +	   received and they gracefully shutdown*/

Typos in "running" and "workers gets". Plus, use standard multi-line comments.

> +	quit_workers(d, mem_pool);
> +	/* rx thread should quit at last*/
> +	return 0;
> +}
> +

 [...]

> +static int
> +lcore_worker(struct lcore_params *p)
> +{
> +	struct rte_distributor *d = p->d;
> +	const unsigned id = p->worker_id;
> +	/* for single port, xor_val will be zero so we won't modify the output
> +	 * port, otherwise we send traffic from 0 to 1, 2 to 3, and vice versa
> +	 */

Move "/*" to the first line, to be consistent with other multi-line comments.
 
> +	const unsigned xor_val = (rte_eth_dev_count() > 1);
> +	struct rte_mbuf *buf = NULL;
> +
> +	printf("\nCore %u acting as worker core.\n", rte_lcore_id());
> +	while (!quit_signal) {
> +		buf = rte_distributor_get_pkt(d, id, buf);
> +		buf->port ^= xor_val;
> +	}
> +	return 0;

[...]

> +			default:
> +				print_usage(prgname);
> +				return -1;
> +		}
> +	}
> +
> +if (optind <= 1) {
> +	print_usage(prgname);
> +	return -1;
> +}

Missing indentation.

> +
> +	argv[optind-1] = prgname;
> +
> +	optind = 0; /* reset getopt lib */
> +	return 0;
> +}

Thanks,
Pablo

  reply	other threads:[~2014-11-02 19:59 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-16 12:13 [dpdk-dev] [PATCH 0/3] distributor_app: new sample application for distributor library reshmapa
2014-09-16 12:13 ` [dpdk-dev] [PATCH 1/3] distributor_app: new sample app reshmapa
2014-09-16 12:13 ` [dpdk-dev] [PATCH 2/3] distributor_app: code review comments implementation reshmapa
2014-09-16 12:13 ` [dpdk-dev] [PATCH 3/3] distributor_app: removed extra spaces reshmapa
2014-09-23 12:55 ` [dpdk-dev] [PATCH 0/3] distributor_app: new sample application for distributor library Bruce Richardson
2014-09-24 14:16 ` [dpdk-dev] [PATCH v2] distributor_app: new sample app reshmapa
2014-09-26 15:11   ` De Lara Guarch, Pablo
2014-09-26 15:51     ` Ananyev, Konstantin
2014-09-29 12:39       ` Pattan, Reshma
2014-09-29 13:06         ` Ananyev, Konstantin
2014-09-29 13:35           ` De Lara Guarch, Pablo
2014-09-29 14:35             ` Neil Horman
2014-09-30  8:02             ` Pattan, Reshma
2014-09-30  9:21               ` Ananyev, Konstantin
2014-09-30 10:39   ` [dpdk-dev] [PATCH v3] " reshmapa
2014-09-30 11:34     ` Neil Horman
2014-09-30 12:18       ` Bruce Richardson
2014-09-30 13:39         ` Neil Horman
2014-10-01 14:47           ` Pattan, Reshma
2014-10-01 14:56             ` Neil Horman
2014-10-01 15:37               ` Bruce Richardson
2014-10-01 16:07                 ` Neil Horman
2014-10-06 14:16                   ` Pattan, Reshma
2014-10-06 14:44                     ` Neil Horman
2014-10-06 17:34                       ` Pattan, Reshma
2014-10-06 19:02                         ` Neil Horman
2014-10-02  9:04                 ` Ananyev, Konstantin
2014-10-01 13:33     ` [dpdk-dev] [PATCH v4] distributor_app: gracefull shutdown of tx/rx threads on SIGINT reshmapa
2014-10-01 13:46       ` Pattan, Reshma
2014-10-01 13:49       ` Thomas Monjalon
2014-10-01 14:33       ` [dpdk-dev] [PATCH v5] distributor_app: new sample app reshmapa
2014-10-17 13:59         ` [dpdk-dev] [PATCH v6] " Reshma Pattan
2014-11-02 20:08           ` De Lara Guarch, Pablo [this message]
2014-11-03 15:49           ` [dpdk-dev] [PATCH v7] " Reshma Pattan
2014-11-03 16:03             ` De Lara Guarch, Pablo
2014-11-13 21:30               ` Thomas Monjalon
2014-11-14  8:44                 ` Pattan, Reshma
2014-11-16 21:59                   ` Thomas Monjalon

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=E115CCD9D858EF4F90C690B0DCB4D897268323A9@IRSMSX108.ger.corp.intel.com \
    --to=pablo.de.lara.guarch@intel.com \
    --cc=dev@dpdk.org \
    --cc=reshma.pattan@intel.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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git