On 10/8/18 11:07 AM, Thomas Monjalon wrote:
08/10/2018 09:20, Andrew Rybchenko:
On 10/8/18 1:25 AM, Thomas Monjalon wrote:
If no rte_device is given in the iterator,
eth_dev_match() is looking at all ports without any restriction,
except the ethdev kvargs filter.
It allows to iterate with a devargs filter referencing only
some ethdev parameters. The format (from the new devargs syntax) is:
class=eth,paramY=Y
Fixes: e815a7f69371 ("ethdev: register as a class")
Signed-off-by: Thomas Monjalon <tho...@monjalon.net>
---
--- a/lib/librte_ethdev/rte_class_eth.c
+++ b/lib/librte_ethdev/rte_class_eth.c
@@ -42,7 +42,7 @@ eth_dev_match(const struct rte_eth_dev *edev,
if (edev->state == RTE_ETH_DEV_UNUSED)
return -1;
- if (edev->device != arg->device)
+ if (arg->device != NULL && arg->device != edev->device)
return -1;
if (kvlist == NULL)
/* Empty string matches everything. */
It looks like it is the only hunk which
Fixes: e815a7f69371 ("ethdev: register as a class")
Yes this hunk is fixing above commit.
everything else adjusts the previous patch.
Yes but the whole goal of this patch is to allow ethdev pure filter.
All is related in this patch.
I think this fix should go before and the rest should be squashed
in the previous patch. It was really questionable why it is safe
to dereference iter->bus without checking that it is not NULL.
No iter->bus was safe because iter->cls was checked before and
implied that iter->bus was successfully set.
OK, I see now.
I still think it is better for understanding to split different
kind of filters in 2 patches.
I may remove the Fixes line however. Opinion?
Yes, it makes sense. If so, I think it is better to remove Fixes line.
With Fixes I would expect to be able to apply it before previous patch.
[...]
+ /* Handle a case from future syntax, without any bus-level argument. */
+ if (strncmp(devargs_str, iter_anybus_str,
+ strlen(iter_anybus_str)) == 0) {
+ iter->cls_str = devargs_str + strlen(iter_anybus_str);
+ goto end;
+ }
+
It looks like a hack, but I guess we need it since rte_devargs_parse()
cannot handle the case. May be it is acceptable if we have no time
to solve it, but it would be good to highlight it better in the comments.
This function is a mix of old and new syntax.
When only new syntax will be supported, it will be better.
It is a work in progress.
I do not know how to better explain it.
I'd suggest to highligh rte_devargs_parse() limitations here to
explain why special handling is required here.
