DPDK patches and discussions
 help / color / mirror / Atom feed
From: Shreyansh Jain <shreyansh.jain@nxp.com>
To: "Trahe, Fiona" <fiona.trahe@intel.com>
Cc: Hemant Agrawal <hemant.agrawal@nxp.com>,
	"Xu, Rosen" <rosen.xu@intel.com>,  "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v1 1/5] rawdev: introduce raw device library support
Date: Mon, 8 Jan 2018 19:39:36 +0530	[thread overview]
Message-ID: <f6cb3545-49d5-acf6-65c8-53c52dc1c520@nxp.com> (raw)
In-Reply-To: <348A99DA5F5B7549AA880327E580B435892EFE92@IRSMSX101.ger.corp.intel.com>

Hello Fiona,

On Saturday 06 January 2018 07:10 PM, Trahe, Fiona wrote:
> Hi Shreyansh,
> 
> This looks like a useful generic device, thanks. Some comments below.

Thanks for taking interest and sending your review.
I have some responses inline....
(And I have shortened the original email)

[...]

>> +#include "rte_rawdev.h"
>> +#include "rte_rawdev_pmd.h"
>> +
>> +/* dynamic log identifier */
>> +int librawdev_logtype;
>> +
>> +/* Maximum rawdevices supported by system.
>> + */
>> +#define RTE_MAX_RAWDEVPORTS	10
> [Fiona] Typo in comment above? There's RTE_RAWDEV_MAX_DEVS, RTE_MAX_RAWDEVS and RTE_MAX_RAWDEVPORTS. Are all 3 necessary and what's the relationship between ports and devs?

This is a stupid mistake by me. It should be only RTE_RAWDEV_MAX_DEVS.
RTE_MAX_RAWDEVS is useless and I will remove RTE_MAX_RAWDEVPORTS.
They are intend the same thing - number of max devices supported.

>   

[...]

>> +
>> +/**
>> + * Allocate and set up a raw queue for a raw device.
>> + *
>> + * @param dev_id
>> + *   The identifier of the device.
>> + * @param queue_id
>> + *   The index of the raw queue to setup. The value must be in the range
>> + *   [0, nb_raw_queues - 1] previously supplied to rte_rawdev_configure().
>> + *
>> + * @see rte_rawdev_queue_conf_get()
>> + *
>> + * @return
>> + *   - 0: Success, raw queue correctly set up.
>> + *   - <0: raw queue configuration failed
>> + */
> [Fiona] cut and paste error above - should be release.

Indeed. Thanks for pointing out.
I will fix this.

> 
>> +int
>> +rte_rawdev_queue_release(uint16_t dev_id, uint16_t queue_id);
>> +/**
>> + * Get the number of raw queues on a specific raw device
>> + *
>> + * @param dev_id
>> + *   Raw device identifier.
>> + * @return
>> + *   - The number of configured raw queues
>> + */
>> +uint16_t

[...]

>> +
>> +/**
>> + * Allocates a new rawdev slot for an raw device and returns the pointer
>> + * to that slot for the driver to use.
>> + *
>> + * @param name
>> + *   Unique identifier name for each device
>> + * @dev_priv_size
>> + *   Private data allocated within rte_rawdev object.
>> + * @param socket_id
>> + *   Socket to allocate resources on.
>> + * @return
>> + *   - Slot in the rte_dev_devices array for a new device;
>> + */
>> +struct rte_rawdev *
>> +rte_rawdev_pmd_allocate(const char *name, size_t dev_private_size,
>> +			int socket_id);
> [Fiona] The driver must allocate a unique name for each device, and the application presumably must search through all devices using dev_count and dev_info_get for each
> until it finds a name it expects? But will the application always know the name chosen by the PMD? e.g. driver type xyz might find 10 devices and call them xyz_0, xyz_1, xyz_2, etc
> The application wants to look for any or all xyz devices so must know the naming format used by the PMD.
> Would it be useful to have 2 parts to the name, a type and an instance, to facilitate finding all devices of a specific type?

let me state what I have understood:

There are two types of devices:
1. which are scanned through a bus (PCI ...)
2. which are created through vdev (devargs, vdev_init)

for those which are scanned through a bus, it is easy to append a 
"type_" string during device naming.
for those which are added through command line, this pattern would have 
to be choosen by the application/user.

further, a rawdevice doesn't have a specific type. So, type would be 
purely be defined by the driver (scan) or the device name itself 
(vdev_init).

So, eventually the "type_" field would be left out for driver or 
application to decide. framework (lib/librte_rawdev) would never 
override/append to it.

Is this understanding correct?

I will send a v2 shortly with your comments. I will also try and think 
through your suggestion about name containing "type_" - I do think it is 
useful but not really sure how would it define semantics between driver 
and application.

-
Shreyansh

  reply	other threads:[~2018-01-08 13:55 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-02 12:57 [dpdk-dev] [PATCH v1 0/5] Introduce generic 'rawdevice' support Shreyansh Jain
2018-01-02 12:57 ` [dpdk-dev] [PATCH v1 1/5] rawdev: introduce raw device library support Shreyansh Jain
2018-01-06 13:40   ` Trahe, Fiona
2018-01-08 14:09     ` Shreyansh Jain [this message]
2018-01-08 14:51       ` Trahe, Fiona
2018-01-12 15:00         ` Shreyansh Jain
2018-01-12 19:35           ` Trahe, Fiona
2018-01-14 22:42   ` Thomas Monjalon
2018-01-14 22:50   ` Thomas Monjalon
2018-01-02 12:57 ` [dpdk-dev] [PATCH v1 2/5] config: enable compilation of rawdev library Shreyansh Jain
2018-01-14 22:50   ` Thomas Monjalon
2018-01-02 12:57 ` [dpdk-dev] [PATCH v1 3/5] drivers/raw: introduce skeleton rawdev driver Shreyansh Jain
2018-01-14 22:54   ` Thomas Monjalon
2018-01-16 10:21     ` Shreyansh Jain
2018-01-16 10:34       ` Thomas Monjalon
2018-01-02 12:57 ` [dpdk-dev] [PATCH v1 4/5] config: enable compilation of rawdev skeleton driver Shreyansh Jain
2018-01-14 22:55   ` Thomas Monjalon
2018-01-02 12:57 ` [dpdk-dev] [PATCH v1 5/5] test: support for rawdev testcases Shreyansh Jain
2018-01-14 22:58   ` Thomas Monjalon
2018-01-16 10:07     ` Shreyansh Jain
2018-01-16 10:32       ` Thomas Monjalon
2018-01-14 23:00 ` [dpdk-dev] [PATCH v1 0/5] Introduce generic 'rawdevice' support Thomas Monjalon
2018-01-15  5:30   ` Shreyansh Jain
2018-01-23 13:59 ` [dpdk-dev] [PATCH v2 00/10] " Shreyansh Jain
2018-01-23 13:59   ` [dpdk-dev] [PATCH v2 01/10] rawdev: introduce raw device library support Shreyansh Jain
2018-01-23 13:59   ` [dpdk-dev] [PATCH v2 02/10] rawdev: add attribute get and set support Shreyansh Jain
2018-01-23 13:59   ` [dpdk-dev] [PATCH v2 03/10] rawdev: add buffer stream IO support Shreyansh Jain
2018-01-23 13:59   ` [dpdk-dev] [PATCH v2 04/10] rawdev: support for extended stats Shreyansh Jain
2018-01-23 13:59   ` [dpdk-dev] [PATCH v2 05/10] rawdev: support for firmware management Shreyansh Jain
2018-01-23 13:59   ` [dpdk-dev] [PATCH v2 06/10] rawdev: add self test support Shreyansh Jain
2018-01-23 13:59   ` [dpdk-dev] [PATCH v2 07/10] drivers/raw: introduce skeleton rawdev driver Shreyansh Jain
2018-01-23 13:59   ` [dpdk-dev] [PATCH v2 08/10] drivers/raw: support for rawdev testcases Shreyansh Jain
2018-01-23 13:59   ` [dpdk-dev] [PATCH v2 09/10] test: enable rawdev skeleton test Shreyansh Jain
2018-01-23 13:59   ` [dpdk-dev] [PATCH v2 10/10] maintainers: claim ownership of rawdev Shreyansh Jain
2018-01-25 22:21   ` [dpdk-dev] [PATCH v2 00/10] Introduce generic 'rawdevice' support Thomas Monjalon
2018-01-29 23:49     ` Thomas Monjalon
2018-01-30  6:31       ` Shreyansh Jain
2018-01-30 14:56   ` [dpdk-dev] [PATCH v3 00/11] " Shreyansh Jain
2018-01-30 14:57     ` [dpdk-dev] [PATCH v3 01/11] rawdev: introduce raw device library support Shreyansh Jain
2018-01-30 14:57     ` [dpdk-dev] [PATCH v3 02/11] rawdev: add attribute get and set support Shreyansh Jain
2018-01-30 14:57     ` [dpdk-dev] [PATCH v3 03/11] rawdev: add buffer stream IO support Shreyansh Jain
2018-01-30 14:57     ` [dpdk-dev] [PATCH v3 04/11] rawdev: support for extended stats Shreyansh Jain
2018-01-30 14:57     ` [dpdk-dev] [PATCH v3 05/11] rawdev: support for firmware management Shreyansh Jain
2018-01-30 14:57     ` [dpdk-dev] [PATCH v3 06/11] rawdev: add self test support Shreyansh Jain
2018-01-30 14:57     ` [dpdk-dev] [PATCH v3 07/11] drivers/raw: introduce skeleton rawdev driver Shreyansh Jain
2018-01-30 16:48       ` Thomas Monjalon
2018-01-30 14:57     ` [dpdk-dev] [PATCH v3 08/11] drivers/raw: support for rawdev testcases Shreyansh Jain
2018-01-30 14:57     ` [dpdk-dev] [PATCH v3 09/11] test: enable rawdev skeleton test Shreyansh Jain
2018-01-30 14:57     ` [dpdk-dev] [PATCH v3 10/11] maintainers: claim ownership of rawdev Shreyansh Jain
2018-01-30 16:50       ` Thomas Monjalon
2018-01-30 14:57     ` [dpdk-dev] [PATCH v3 11/11] doc: add rawdev library page Shreyansh Jain
2018-01-30 16:55     ` [dpdk-dev] [PATCH v3 00/11] Introduce generic 'rawdevice' support Thomas Monjalon
2018-01-31  9:13     ` [dpdk-dev] [PATCH v4 00/10] " Shreyansh Jain
2018-01-31  9:13       ` [dpdk-dev] [PATCH v4 01/10] rawdev: introduce raw device library support Shreyansh Jain
2018-01-31  9:13       ` [dpdk-dev] [PATCH v4 02/10] rawdev: add attribute get and set support Shreyansh Jain
2018-01-31  9:13       ` [dpdk-dev] [PATCH v4 03/10] rawdev: add buffer stream IO support Shreyansh Jain
2018-01-31  9:13       ` [dpdk-dev] [PATCH v4 04/10] rawdev: support for extended stats Shreyansh Jain
2018-01-31  9:13       ` [dpdk-dev] [PATCH v4 05/10] rawdev: support for firmware management Shreyansh Jain
2018-01-31  9:13       ` [dpdk-dev] [PATCH v4 06/10] rawdev: add self test support Shreyansh Jain
2018-01-31  9:13       ` [dpdk-dev] [PATCH v4 07/10] drivers/raw: introduce skeleton rawdev driver Shreyansh Jain
2018-01-31 13:20         ` Thomas Monjalon
2018-01-31 13:31           ` Thomas Monjalon
2018-01-31  9:13       ` [dpdk-dev] [PATCH v4 08/10] drivers/raw: support for rawdev testcases Shreyansh Jain
2018-01-31 14:01         ` Thomas Monjalon
2018-01-31  9:13       ` [dpdk-dev] [PATCH v4 09/10] test: enable rawdev skeleton test Shreyansh Jain
2018-01-31  9:13       ` [dpdk-dev] [PATCH v4 10/10] doc: add rawdev library page and support Doxygen Shreyansh Jain
2018-01-31 14:45       ` [dpdk-dev] [PATCH v4 00/10] Introduce generic 'rawdevice' support 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=f6cb3545-49d5-acf6-65c8-53c52dc1c520@nxp.com \
    --to=shreyansh.jain@nxp.com \
    --cc=dev@dpdk.org \
    --cc=fiona.trahe@intel.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=rosen.xu@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
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).