DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: Ori Kam <orika@mellanox.com>
Cc: Andrew Rybchenko <arybchenko@solarflare.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	Ferruh Yigit <ferruh.yigit@intel.com>,
	"jingjing.wu@intel.com" <jingjing.wu@intel.com>,
	"stephen@networkplumber.org" <stephen@networkplumber.org>
Subject: Re: [dpdk-dev] [PATCH v5 02/15] ethdev: add support for hairpin queue
Date: Sat, 26 Oct 2019 00:16:28 +0200	[thread overview]
Message-ID: <98253428.ADkZfdayxY@xps> (raw)
In-Reply-To: <AM4PR05MB3425C3EF92979EDF3F428D1FDB650@AM4PR05MB3425.eurprd05.prod.outlook.com>

25/10/2019 21:01, Ori Kam:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 24/10/2019 17:30, Andrew Rybchenko:
> > > On 10/24/19 6:17 PM, Thomas Monjalon wrote:
> > > > 24/10/2019 16:47, Andrew Rybchenko:
> > > >> On 10/24/19 11:29 AM, Ori Kam wrote:
> > > >>> Hi Andrew,
> > > >>>
> > > >>> When writing the new function I thought about using bool, but
> > > >>> I decided against it for the following reasons:
> > > >>> 1. There is no use of bool any where in the code, and there is not special
> > reason to add it now.
> > > >> rte_ethdev.c includes stdbool.h and uses bool
> > > >>
> > > >>> 2. Other functions of this kind already returns int. for example
> > (rte_eth_dev_is_valid_port / rte_eth_is_valid_owner_id)
> > > > I agree with Ori here for 2 reasons:
> > > > 1. It is better to be consistent in the API
> > > > 2. I remember having some issues with some drivers when introducing
> > stdbool in the API.
> > > >
> > > > I think it may be nice to convert all such API to bool in one patch,
> > > > and check if there are some remaining issues with bool usage in drivers or
> > with PPC.
> > > > But I suggest to do such API change in DPDK 20.11.
> > >
> > > OK, no problem. Does it prevent to avoid comparison == 1? Just to
> > > avoid changes in these lines in the future.
> > 
> > Yes probably better to avoid explicit comparison, but prefer boolean operator
> > (!).
> > 
> > 
> 
> Thomas I understand your comments but from Andrew comment on my V2-01 patch:
> "
> >>> +	ret = (*dev->dev_ops->rx_hairpin_queue_setup)(dev, rx_queue_id,
> >>> +						      nb_rx_desc, conf);
> >>> +	if (!ret)
> >> Please, compare with 0
> >>
> > Will do, but again just for my knowledge why?
> 
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdoc.dpdk.org%2Fguides%2Fcontributing%2Fcoding_style.html%23function-calls&amp;data=02%7C01%7Corika%40mellanox.com%7C022cd953964f4a20d50508d7508a259f%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637066426584029629&amp;sdata=G8ZxEWFFsv1kLWc6L7sKT8O6CSiBcj5ZuwQqmK0Q6nY%3D&amp;reserved=0
> 
> "
> I don't see any relevant info in the link, but maybe I'm missing something .
> What are the rules? 
> Thomas also keep in mind that in most cases the condition that is tested is the positive, meaning  it will look something like this:
> if (rte_eth_dev_is_rx_hairpin_queue(dev, queue_id)) {     
>         rte_errno = EINVAL;                               
>         return NULL;                                      
> }                                       
> 
> What do you think?

I think for normal functions with error codes,
we should compare explictly with a value.
But for boolean-type functions like "is_hairpin_queue",
we should have implicit (natural) comparison. So yes, this is correct:
	if (rte_eth_dev_is_rx_hairpin_queue(dev, queue_id))



  reply	other threads:[~2019-10-25 22:16 UTC|newest]

Thread overview: 186+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-26  6:28 [dpdk-dev] [PATCH 00/13] add hairpin feature Ori Kam
2019-09-26  6:28 ` [dpdk-dev] [PATCH 01/13] ethdev: support setup function for hairpin queue Ori Kam
2019-09-26 12:18   ` Andrew Rybchenko
     [not found]     ` <AM0PR0502MB4019A2FEADE5F9DCD0D9DDFED2860@AM0PR0502MB4019.eurprd05.prod.outlook.com>
2019-09-26 15:58       ` Ori Kam
2019-09-26 17:24         ` Andrew Rybchenko
2019-09-28 15:19           ` Ori Kam
2019-09-29 12:10             ` Andrew Rybchenko
2019-10-02 12:19               ` Ori Kam
2019-10-03 13:26                 ` Andrew Rybchenko
2019-10-03 17:46                   ` Ori Kam
2019-10-03 18:39     ` Ray Kinsella
2019-09-26  6:28 ` [dpdk-dev] [PATCH 02/13] net/mlx5: query hca hairpin capabilities Ori Kam
2019-09-26  9:31   ` Slava Ovsiienko
2019-09-26  6:28 ` [dpdk-dev] [PATCH 03/13] net/mlx5: support Rx hairpin queues Ori Kam
2019-09-26  9:32   ` Slava Ovsiienko
2019-09-26  6:29 ` [dpdk-dev] [PATCH 04/13] net/mlx5: prepare txq to work with different types Ori Kam
2019-09-26  9:32   ` Slava Ovsiienko
2019-09-26  6:29 ` [dpdk-dev] [PATCH 05/13] net/mlx5: support Tx hairpin queues Ori Kam
2019-09-26  9:32   ` Slava Ovsiienko
2019-09-26  6:29 ` [dpdk-dev] [PATCH 06/13] app/testpmd: add hairpin support Ori Kam
2019-09-26  9:32   ` Slava Ovsiienko
2019-09-26  6:29 ` [dpdk-dev] [PATCH 07/13] net/mlx5: add hairpin binding function Ori Kam
2019-09-26  9:33   ` Slava Ovsiienko
2019-09-26  6:29 ` [dpdk-dev] [PATCH 08/13] net/mlx5: add support for hairpin hrxq Ori Kam
2019-09-26  9:33   ` Slava Ovsiienko
2019-09-26  6:29 ` [dpdk-dev] [PATCH 09/13] net/mlx5: add internal tag item and action Ori Kam
2019-09-26  9:33   ` Slava Ovsiienko
2019-09-26  6:29 ` [dpdk-dev] [PATCH 10/13] net/mlx5: add id generation function Ori Kam
2019-09-26  9:34   ` Slava Ovsiienko
2019-09-26  6:29 ` [dpdk-dev] [PATCH 11/13] net/mlx5: add default flows for hairpin Ori Kam
2019-09-26  9:34   ` Slava Ovsiienko
2019-09-26  6:29 ` [dpdk-dev] [PATCH 12/13] net/mlx5: split hairpin flows Ori Kam
2019-09-26  9:34   ` Slava Ovsiienko
2019-09-26  6:29 ` [dpdk-dev] [PATCH 13/13] doc: add hairpin feature Ori Kam
2019-09-26  9:34   ` Slava Ovsiienko
2019-09-26 12:32 ` [dpdk-dev] [PATCH 00/13] " Andrew Rybchenko
2019-09-26 15:22   ` Ori Kam
2019-09-26 15:48     ` Andrew Rybchenko
2019-09-26 16:11       ` Ori Kam
2019-10-04 19:54 ` [dpdk-dev] [PATCH v2 00/14] " Ori Kam
2019-10-04 19:54   ` [dpdk-dev] [PATCH v2 01/14] ethdev: add support for hairpin queue Ori Kam
2019-10-08 16:11     ` Andrew Rybchenko
2019-10-10 21:07       ` Ori Kam
2019-10-14  9:37         ` Andrew Rybchenko
2019-10-14 10:19           ` Ori Kam
2019-10-04 19:54   ` [dpdk-dev] [PATCH v2 02/14] net/mlx5: query hca hairpin capabilities Ori Kam
2019-10-04 19:54   ` [dpdk-dev] [PATCH v2 03/14] net/mlx5: support Rx hairpin queues Ori Kam
2019-10-04 19:54   ` [dpdk-dev] [PATCH v2 04/14] net/mlx5: prepare txq to work with different types Ori Kam
2019-10-04 19:54   ` [dpdk-dev] [PATCH v2 05/14] net/mlx5: support Tx hairpin queues Ori Kam
2019-10-04 19:54   ` [dpdk-dev] [PATCH v2 06/14] net/mlx5: add get hairpin capabilities Ori Kam
2019-10-04 19:54   ` [dpdk-dev] [PATCH v2 07/14] app/testpmd: add hairpin support Ori Kam
2019-10-04 19:54   ` [dpdk-dev] [PATCH v2 08/14] net/mlx5: add hairpin binding function Ori Kam
2019-10-04 19:54   ` [dpdk-dev] [PATCH v2 09/14] net/mlx5: add support for hairpin hrxq Ori Kam
2019-10-04 19:54   ` [dpdk-dev] [PATCH v2 10/14] net/mlx5: add internal tag item and action Ori Kam
2019-10-04 19:54   ` [dpdk-dev] [PATCH v2 11/14] net/mlx5: add id generation function Ori Kam
2019-10-04 19:54   ` [dpdk-dev] [PATCH v2 12/14] net/mlx5: add default flows for hairpin Ori Kam
2019-10-04 19:54   ` [dpdk-dev] [PATCH v2 13/14] net/mlx5: split hairpin flows Ori Kam
2019-10-04 19:54   ` [dpdk-dev] [PATCH v2 14/14] doc: add hairpin feature Ori Kam
2019-10-08 14:55     ` Andrew Rybchenko
2019-10-10  8:24       ` Ori Kam
2019-10-15  9:04 ` [dpdk-dev] [PATCH v3 00/14] " Ori Kam
2019-10-15  9:04   ` [dpdk-dev] [PATCH v3 01/14] ethdev: add support for hairpin queue Ori Kam
2019-10-15 10:12     ` Andrew Rybchenko
2019-10-16 19:36       ` Ori Kam
2019-10-17 10:41         ` Andrew Rybchenko
2019-10-15  9:04   ` [dpdk-dev] [PATCH v3 02/14] net/mlx5: query hca hairpin capabilities Ori Kam
2019-10-15  9:04   ` [dpdk-dev] [PATCH v3 03/14] net/mlx5: support Rx hairpin queues Ori Kam
2019-10-15  9:04   ` [dpdk-dev] [PATCH v3 04/14] net/mlx5: prepare txq to work with different types Ori Kam
2019-10-15  9:04   ` [dpdk-dev] [PATCH v3 05/14] net/mlx5: support Tx hairpin queues Ori Kam
2019-10-15  9:04   ` [dpdk-dev] [PATCH v3 06/14] net/mlx5: add get hairpin capabilities Ori Kam
2019-10-15  9:04   ` [dpdk-dev] [PATCH v3 07/14] app/testpmd: add hairpin support Ori Kam
2019-10-15  9:04   ` [dpdk-dev] [PATCH v3 08/14] net/mlx5: add hairpin binding function Ori Kam
2019-10-15  9:04   ` [dpdk-dev] [PATCH v3 09/14] net/mlx5: add support for hairpin hrxq Ori Kam
2019-10-15  9:04   ` [dpdk-dev] [PATCH v3 10/14] net/mlx5: add internal tag item and action Ori Kam
2019-10-15  9:04   ` [dpdk-dev] [PATCH v3 11/14] net/mlx5: add id generation function Ori Kam
2019-10-15  9:04   ` [dpdk-dev] [PATCH v3 12/14] net/mlx5: add default flows for hairpin Ori Kam
2019-10-15  9:04   ` [dpdk-dev] [PATCH v3 13/14] net/mlx5: split hairpin flows Ori Kam
2019-10-15  9:04   ` [dpdk-dev] [PATCH v3 14/14] doc: add hairpin feature Ori Kam
2019-10-17 15:32 ` [dpdk-dev] [PATCH v4 00/15] " Ori Kam
2019-10-17 15:32   ` [dpdk-dev] [PATCH v4 01/15] ethdev: move queue state defines to private file Ori Kam
2019-10-17 15:37     ` Stephen Hemminger
2019-10-22 10:59     ` Andrew Rybchenko
2019-10-17 15:32   ` [dpdk-dev] [PATCH v4 02/15] ethdev: add support for hairpin queue Ori Kam
2019-10-17 21:01     ` Thomas Monjalon
2019-10-22 11:37     ` Andrew Rybchenko
2019-10-23  6:23       ` Ori Kam
2019-10-23  7:04     ` Thomas Monjalon
2019-10-23 10:09       ` Ori Kam
2019-10-23 10:18         ` Bruce Richardson
2019-10-17 15:32   ` [dpdk-dev] [PATCH v4 03/15] net/mlx5: query hca hairpin capabilities Ori Kam
2019-10-17 15:32   ` [dpdk-dev] [PATCH v4 04/15] net/mlx5: support Rx hairpin queues Ori Kam
2019-10-17 15:32   ` [dpdk-dev] [PATCH v4 05/15] net/mlx5: prepare txq to work with different types Ori Kam
2019-10-17 15:32   ` [dpdk-dev] [PATCH v4 06/15] net/mlx5: support Tx hairpin queues Ori Kam
2019-10-17 15:32   ` [dpdk-dev] [PATCH v4 07/15] net/mlx5: add get hairpin capabilities Ori Kam
2019-10-17 15:32   ` [dpdk-dev] [PATCH v4 08/15] app/testpmd: add hairpin support Ori Kam
2019-10-17 15:32   ` [dpdk-dev] [PATCH v4 09/15] net/mlx5: add hairpin binding function Ori Kam
2019-10-17 15:32   ` [dpdk-dev] [PATCH v4 10/15] net/mlx5: add support for hairpin hrxq Ori Kam
2019-10-17 15:32   ` [dpdk-dev] [PATCH v4 11/15] net/mlx5: add internal tag item and action Ori Kam
2019-10-17 15:32   ` [dpdk-dev] [PATCH v4 12/15] net/mlx5: add id generation function Ori Kam
2019-10-17 15:32   ` [dpdk-dev] [PATCH v4 13/15] net/mlx5: add default flows for hairpin Ori Kam
2019-10-17 15:32   ` [dpdk-dev] [PATCH v4 14/15] net/mlx5: split hairpin flows Ori Kam
2019-10-17 15:32   ` [dpdk-dev] [PATCH v4 15/15] doc: add hairpin feature Ori Kam
2019-10-18 19:07   ` [dpdk-dev] [PATCH v4 00/15] " Ferruh Yigit
2019-10-23 13:37 ` [dpdk-dev] [PATCH v5 " Ori Kam
2019-10-23 13:37   ` [dpdk-dev] [PATCH v5 01/15] ethdev: move queue state defines to private file Ori Kam
2019-10-23 13:37   ` [dpdk-dev] [PATCH v5 02/15] ethdev: add support for hairpin queue Ori Kam
2019-10-24  7:54     ` Andrew Rybchenko
2019-10-24  8:29       ` Ori Kam
2019-10-24 14:47         ` Andrew Rybchenko
2019-10-24 15:17           ` Thomas Monjalon
2019-10-24 15:30             ` Andrew Rybchenko
2019-10-24 15:34               ` Thomas Monjalon
2019-10-25 19:01                 ` Ori Kam
2019-10-25 22:16                   ` Thomas Monjalon [this message]
2019-10-23 13:37   ` [dpdk-dev] [PATCH v5 03/15] net/mlx5: query hca hairpin capabilities Ori Kam
2019-10-23 13:37   ` [dpdk-dev] [PATCH v5 04/15] net/mlx5: support Rx hairpin queues Ori Kam
2019-10-23 13:37   ` [dpdk-dev] [PATCH v5 05/15] net/mlx5: prepare txq to work with different types Ori Kam
2019-10-23 13:37   ` [dpdk-dev] [PATCH v5 06/15] net/mlx5: support Tx hairpin queues Ori Kam
2019-10-23 13:37   ` [dpdk-dev] [PATCH v5 07/15] net/mlx5: add get hairpin capabilities Ori Kam
2019-10-23 13:37   ` [dpdk-dev] [PATCH v5 08/15] app/testpmd: add hairpin support Ori Kam
2019-10-23 13:37   ` [dpdk-dev] [PATCH v5 09/15] net/mlx5: add hairpin binding function Ori Kam
2019-10-23 13:37   ` [dpdk-dev] [PATCH v5 10/15] net/mlx5: add support for hairpin hrxq Ori Kam
2019-10-23 13:37   ` [dpdk-dev] [PATCH v5 11/15] net/mlx5: add internal tag item and action Ori Kam
2019-10-23 13:37   ` [dpdk-dev] [PATCH v5 12/15] net/mlx5: add id generation function Ori Kam
2019-10-23 13:37   ` [dpdk-dev] [PATCH v5 13/15] net/mlx5: add default flows for hairpin Ori Kam
2019-10-23 13:37   ` [dpdk-dev] [PATCH v5 14/15] net/mlx5: split hairpin flows Ori Kam
2019-10-23 13:37   ` [dpdk-dev] [PATCH v5 15/15] doc: add hairpin feature Ori Kam
2019-10-24  8:11     ` Thomas Monjalon
2019-10-25 18:49   ` [dpdk-dev] [PATCH v5 00/15] " Ferruh Yigit
2019-10-27 12:24 ` [dpdk-dev] [PATCH v6 00/14] " Ori Kam
2019-10-27 12:24   ` [dpdk-dev] [PATCH v6 01/14] ethdev: move queue state defines to private file Ori Kam
2019-10-27 12:24   ` [dpdk-dev] [PATCH v6 02/14] ethdev: add support for hairpin queue Ori Kam
2019-10-28 15:16     ` Andrew Rybchenko
2019-10-28 18:44       ` Ori Kam
2019-10-29  7:38         ` Andrew Rybchenko
2019-10-29 19:39           ` Ori Kam
2019-10-30  6:39             ` Andrew Rybchenko
2019-10-30  6:56               ` Ori Kam
2019-10-27 12:24   ` [dpdk-dev] [PATCH v6 03/14] net/mlx5: query hca hairpin capabilities Ori Kam
2019-10-27 12:24   ` [dpdk-dev] [PATCH v6 04/14] net/mlx5: support Rx hairpin queues Ori Kam
2019-10-27 12:24   ` [dpdk-dev] [PATCH v6 05/14] net/mlx5: prepare txq to work with different types Ori Kam
2019-10-27 12:24   ` [dpdk-dev] [PATCH v6 06/14] net/mlx5: support Tx hairpin queues Ori Kam
2019-10-27 12:24   ` [dpdk-dev] [PATCH v6 07/14] net/mlx5: add get hairpin capabilities Ori Kam
2019-10-27 12:24   ` [dpdk-dev] [PATCH v6 08/14] app/testpmd: add hairpin support Ori Kam
2019-10-27 12:24   ` [dpdk-dev] [PATCH v6 09/14] net/mlx5: add hairpin binding function Ori Kam
2019-10-27 12:24   ` [dpdk-dev] [PATCH v6 10/14] net/mlx5: add support for hairpin hrxq Ori Kam
2019-10-27 12:24   ` [dpdk-dev] [PATCH v6 11/14] net/mlx5: add internal tag item and action Ori Kam
2019-10-27 12:24   ` [dpdk-dev] [PATCH v6 12/14] net/mlx5: add id generation function Ori Kam
2019-10-27 12:25   ` [dpdk-dev] [PATCH v6 13/14] net/mlx5: add default flows for hairpin Ori Kam
2019-10-27 12:25   ` [dpdk-dev] [PATCH v6 14/14] net/mlx5: split hairpin flows Ori Kam
2019-10-30 23:53 ` [dpdk-dev] [PATCH v7 00/14] add hairpin feature Ori Kam
2019-10-30 23:53   ` [dpdk-dev] [PATCH v7 01/14] ethdev: move queue state defines to private file Ori Kam
2019-10-30 23:53   ` [dpdk-dev] [PATCH v7 02/14] ethdev: add support for hairpin queue Ori Kam
2019-10-31  8:25     ` Andrew Rybchenko
2019-11-05 11:24     ` Ferruh Yigit
2019-11-05 11:36       ` Ori Kam
2019-11-05 11:49         ` Andrew Rybchenko
2019-11-05 12:00           ` Ori Kam
2019-11-05 12:05           ` Ferruh Yigit
2019-11-05 12:12             ` Andrew Rybchenko
2019-11-05 12:23               ` Ferruh Yigit
2019-11-05 12:27                 ` Andrew Rybchenko
2019-11-05 12:51                   ` Thomas Monjalon
2019-11-05 12:53                     ` Andrew Rybchenko
2019-11-05 13:02                       ` Thomas Monjalon
2019-11-05 13:23                         ` Ori Kam
2019-11-05 13:27                           ` Thomas Monjalon
2019-11-05 13:34                             ` Andrew Rybchenko
2019-11-05 13:41                         ` Andrew Rybchenko
2019-10-30 23:53   ` [dpdk-dev] [PATCH v7 03/14] net/mlx5: query hca hairpin capabilities Ori Kam
2019-10-30 23:53   ` [dpdk-dev] [PATCH v7 04/14] net/mlx5: support Rx hairpin queues Ori Kam
2019-10-30 23:53   ` [dpdk-dev] [PATCH v7 05/14] net/mlx5: prepare txq to work with different types Ori Kam
2019-10-30 23:53   ` [dpdk-dev] [PATCH v7 06/14] net/mlx5: support Tx hairpin queues Ori Kam
2019-10-30 23:53   ` [dpdk-dev] [PATCH v7 07/14] net/mlx5: add get hairpin capabilities Ori Kam
2019-10-30 23:53   ` [dpdk-dev] [PATCH v7 08/14] app/testpmd: add hairpin support Ori Kam
2019-10-31 17:11     ` Ferruh Yigit
2019-10-31 17:36       ` Ori Kam
2019-10-31 17:54         ` Ferruh Yigit
2019-10-31 18:59           ` Ori Kam
2019-10-30 23:53   ` [dpdk-dev] [PATCH v7 09/14] net/mlx5: add hairpin binding function Ori Kam
2019-10-30 23:53   ` [dpdk-dev] [PATCH v7 10/14] net/mlx5: add support for hairpin hrxq Ori Kam
2019-10-30 23:53   ` [dpdk-dev] [PATCH v7 11/14] net/mlx5: add internal tag item and action Ori Kam
2019-10-30 23:53   ` [dpdk-dev] [PATCH v7 12/14] net/mlx5: add id generation function Ori Kam
2019-10-30 23:53   ` [dpdk-dev] [PATCH v7 13/14] net/mlx5: add default flows for hairpin Ori Kam
2019-10-30 23:53   ` [dpdk-dev] [PATCH v7 14/14] net/mlx5: split hairpin flows Ori Kam
2019-10-31 17:13   ` [dpdk-dev] [PATCH v7 00/14] add hairpin feature Ferruh Yigit

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=98253428.ADkZfdayxY@xps \
    --to=thomas@monjalon.net \
    --cc=arybchenko@solarflare.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=jingjing.wu@intel.com \
    --cc=orika@mellanox.com \
    --cc=stephen@networkplumber.org \
    /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).