[dpdk-dev,v3,2/3] net/ring: create vdev from PMD specific API
Checks
Commit Message
When ring PMD created via PMD specific API instead of EAL abstraction
it is missing the virtual device creation done by EAL vdev.
And this makes eth_dev unusable exact same as other PMDs used, because
of some missing fields, like rte_device->name.
Now API creates a virtual device and sets proper fields, not all, and it
still won't be linked in the virtual device list eal keeps track. But
makes PMD usable in usual manner.
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
drivers/net/ring/rte_eth_ring.c | 49 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 48 insertions(+), 1 deletion(-)
Comments
On Fri, Jun 09, 2017 at 06:51:19PM +0100, Ferruh Yigit wrote:
> When ring PMD created via PMD specific API instead of EAL abstraction
> it is missing the virtual device creation done by EAL vdev.
>
> And this makes eth_dev unusable exact same as other PMDs used, because
> of some missing fields, like rte_device->name.
>
> Now API creates a virtual device and sets proper fields, not all, and it
> still won't be linked in the virtual device list eal keeps track. But
> makes PMD usable in usual manner.
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
Is a better fix not to have this API call into the EAL to create the
vdev and add it to the lists as with other vdevs? [If it makes it easier,
the extra parameters passed in to the library-local function can be
saved in a context that can be accessed when the EAL calls back into the
driver, rather than having to flatten them out into devargs and re-parsed
again.]
Regards,
/Bruce
On 6/12/2017 2:25 PM, Bruce Richardson wrote:
> On Fri, Jun 09, 2017 at 06:51:19PM +0100, Ferruh Yigit wrote:
>> When ring PMD created via PMD specific API instead of EAL abstraction
>> it is missing the virtual device creation done by EAL vdev.
>>
>> And this makes eth_dev unusable exact same as other PMDs used, because
>> of some missing fields, like rte_device->name.
>>
>> Now API creates a virtual device and sets proper fields, not all, and it
>> still won't be linked in the virtual device list eal keeps track. But
>> makes PMD usable in usual manner.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>
> Is a better fix not to have this API call into the EAL to create the
> vdev and add it to the lists as with other vdevs? [If it makes it easier,
> the extra parameters passed in to the library-local function can be
> saved in a context that can be accessed when the EAL calls back into the
> driver, rather than having to flatten them out into devargs and re-parsed
> again.]
Let me send the patch as suggested.
Using EAL API is better idea I think, but overall this ring PMD looked
like hack after changes.
Please check the latest patch, if we want to keep ring PMD API, perhaps
we should postpone removing drv_name patch.
>
> Regards,
> /Bruce
>
On Mon, Jun 12, 2017 at 03:08:31PM +0100, Ferruh Yigit wrote:
> On 6/12/2017 2:25 PM, Bruce Richardson wrote:
> > On Fri, Jun 09, 2017 at 06:51:19PM +0100, Ferruh Yigit wrote:
> >> When ring PMD created via PMD specific API instead of EAL abstraction
> >> it is missing the virtual device creation done by EAL vdev.
> >>
> >> And this makes eth_dev unusable exact same as other PMDs used, because
> >> of some missing fields, like rte_device->name.
> >>
> >> Now API creates a virtual device and sets proper fields, not all, and it
> >> still won't be linked in the virtual device list eal keeps track. But
> >> makes PMD usable in usual manner.
> >>
> >> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >> ---
> >
> > Is a better fix not to have this API call into the EAL to create the
> > vdev and add it to the lists as with other vdevs? [If it makes it easier,
> > the extra parameters passed in to the library-local function can be
> > saved in a context that can be accessed when the EAL calls back into the
> > driver, rather than having to flatten them out into devargs and re-parsed
> > again.]
>
> Let me send the patch as suggested.
>
> Using EAL API is better idea I think, but overall this ring PMD looked
> like hack after changes.
>
> Please check the latest patch, if we want to keep ring PMD API, perhaps
> we should postpone removing drv_name patch.
>
The new patch looks ok to me. I actually don't think it looks that
hacked together. :-)
/Bruce
On 6/12/2017 3:19 PM, Bruce Richardson wrote:
> On Mon, Jun 12, 2017 at 03:08:31PM +0100, Ferruh Yigit wrote:
>> On 6/12/2017 2:25 PM, Bruce Richardson wrote:
>>> On Fri, Jun 09, 2017 at 06:51:19PM +0100, Ferruh Yigit wrote:
>>>> When ring PMD created via PMD specific API instead of EAL abstraction
>>>> it is missing the virtual device creation done by EAL vdev.
>>>>
>>>> And this makes eth_dev unusable exact same as other PMDs used, because
>>>> of some missing fields, like rte_device->name.
>>>>
>>>> Now API creates a virtual device and sets proper fields, not all, and it
>>>> still won't be linked in the virtual device list eal keeps track. But
>>>> makes PMD usable in usual manner.
>>>>
>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> ---
>>>
>>> Is a better fix not to have this API call into the EAL to create the
>>> vdev and add it to the lists as with other vdevs? [If it makes it easier,
>>> the extra parameters passed in to the library-local function can be
>>> saved in a context that can be accessed when the EAL calls back into the
>>> driver, rather than having to flatten them out into devargs and re-parsed
>>> again.]
>>
>> Let me send the patch as suggested.
>>
>> Using EAL API is better idea I think, but overall this ring PMD looked
>> like hack after changes.
>>
>> Please check the latest patch, if we want to keep ring PMD API, perhaps
>> we should postpone removing drv_name patch.
>>
> The new patch looks ok to me. I actually don't think it looks that
> hacked together. :-)
That is good, if you are happy with the patch, I guess we can get the
patchset.
Thanks,
ferruh
>
> /Bruce
>
@@ -365,6 +365,41 @@ do_eth_dev_ring_create(const char *name,
return -1;
}
+static struct rte_vdev_device *
+alloc_vdev(const char *name)
+{
+ struct rte_devargs *devargs = NULL;
+ struct rte_vdev_device *vdev = NULL;
+ int ret;
+
+ devargs = calloc(1, sizeof(*devargs));
+ if (!devargs)
+ goto out_free;
+
+ vdev = calloc(1, sizeof(*vdev));
+ if (!vdev)
+ goto out_free;
+
+ devargs->type = RTE_DEVTYPE_VIRTUAL;
+ ret = snprintf(devargs->virt.drv_name, sizeof(devargs->virt.drv_name),
+ "%s", name);
+ if (ret < 0)
+ goto out_free;
+
+ vdev->device.devargs = devargs;
+ vdev->device.numa_node = SOCKET_ID_ANY;
+ vdev->device.name = devargs->virt.drv_name;
+
+ vdev->device.driver = &pmd_ring_drv.driver;
+
+ return vdev;
+
+out_free:
+ free(devargs);
+ free(vdev);
+ return NULL;
+}
+
int
rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
const unsigned nb_rx_queues,
@@ -373,6 +408,8 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
const unsigned numa_node)
{
struct rte_eth_dev *eth_dev = NULL;
+ struct rte_vdev_device *vdev;
+ int ret;
/* do some parameter checking */
if (rx_queues == NULL && nb_rx_queues > 0) {
@@ -388,9 +425,19 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
return -1;
}
- return do_eth_dev_ring_create(name, rx_queues, nb_rx_queues,
+ vdev = alloc_vdev(name);
+ if (!vdev) {
+ rte_errno = EINVAL;
+ return -1;
+ }
+
+ ret = do_eth_dev_ring_create(name, rx_queues, nb_rx_queues,
tx_queues, nb_tx_queues, numa_node, DEV_ATTACH,
ð_dev);
+
+ eth_dev->device = &vdev->device;
+
+ return ret;
}
int