-----Original Message-----
From: Shreyansh Jain [mailto:shreyansh.j...@nxp.com]
Sent: Monday, January 8, 2018 2:10 PM
To: Trahe, Fiona <fiona.tr...@intel.com>
Cc: Hemant Agrawal <hemant.agra...@nxp.com>; Xu, Rosen <rosen...@intel.com>;
dev@dpdk.org
Subject: Re: [PATCH v1 1/5] rawdev: introduce raw device library support
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?
