01/04/2021 17:13, Xueming(Steven) Li:
>From: Thomas Monjalon <tho...@monjalon.net>
> >30/03/2021 14:15, Xueming Li:
> >> To use Global Device Syntax as devargs, name is required for device
> >> management.
> >
> >Context is missing.
> >You mean the argument "name" for the vdev bus?
>
> Devargs.name, it is used by probe and device iterator.
I think we could avoid having a name with the new syntax.
In my understanding, this is for compatibility with the legacy syntax.
> To locate a device from a devargs, devargs.name is compared
> agains name of probed devices.
> This not an issue for legacy syntax,
> since the string after "bus:" is saved as name.
It would be interesting to explain where the name is parsed
for the legacy syntax: rte_devargs_parse
and for the new syntax: rte_devargs_layers_parse called
in rte_devargs_parse in the next patch.
> >> In legacy parsing API, devargs name was extracted after bus name:
> >> bus:name,kv_params,,,
> >>
> >> To parse new Global Device Syntax, this patch introduces new bus API
> >> to parse devargs and update name, different bus driver might choose
> >> different keys from parameters with unified formating, example:
> >> -a bus=pci,addr=83:00.0/class=eth/driver=mlx5,...
> >> name: 0000:03:00.0
> >> -a bus=vdev,name=pcap0/class=eth/driver=pcap,...
> >> name:pcap0
> >
> >Only PCI and vdev buses are implemented.
> >What can be the plan for others?
> >We should track the progress somewhere, maybe with TODO comments.
>
> Like legacy parser, how about using "name" as default name key, the new
> syntax parser can resolve it by default.
> Then PCI bus overrides by using "addr" key in new bus API,
> vdev and other bus drivers simply use default implementation, i.e. using
> "name" as key..
Yes, you mean if devargs_parse is not implemented by the bus driver,
the default is to parse the name property,
while the PCI implementation fills the devargs name with the addr property.
> >[...]
> >> +int
> >> +rte_pci_devargs_parse(struct rte_devargs *da) {
> >> + struct rte_kvargs *kvargs;
> >> + const char *addr_str;
> >> + struct rte_pci_addr addr;
> >> + int ret;
> >> +
> >> + if (da == NULL)
> >> + return 0;
> >> + RTE_ASSERT(da->bus_str != NULL);
> >> +
> >> + kvargs = rte_kvargs_parse(da->bus_str, NULL);
> >> + if (kvargs == NULL) {
> >> + RTE_LOG(ERR, EAL, "cannot parse argument list: %s\n",
> >> + da->bus_str);
> >> + ret = -ENODEV;
> >> + goto out;
> >> + }
> >> +
> >> + addr_str = rte_kvargs_get(kvargs, pci_params_keys[RTE_PCI_PARAM_ADDR]);
> >> + if (addr_str == NULL) {
> >> + RTE_LOG(ERR, EAL, "No PCI address specified using '%s=<id>' in:
> >> %s\n",
> >> + pci_params_keys[RTE_PCI_PARAM_ADDR], da->bus_str);
> >> + ret = -ENODEV;
> >> + goto out;
> >> + }
> >> +
> >> + ret = rte_pci_addr_parse(addr_str, &addr);
> >> + if (ret != 0) {
> >> + RTE_LOG(ERR, EAL, "PCI address invalid: %s\n", da->bus_str);
> >> + ret = -EINVAL;
> >> + goto out;
> >> + }
> >> +
> >> + rte_pci_device_name(&addr, da->name, sizeof(da->name));
> >> +
> >> + /* TODO: class parse -> driver parse */
> >
> >Please could you give a longer explanation of what is missing?
>
> Just an option to give class driver and device driver to override name
> parsing.
> But this should happen in new devargs parser, not here if needed.
>
> I'll remove this line, and mention it in commit log.
What would be the benefit of overriding devargs name parsing?
I feel it would be better to not rely on the devargs name
with the new syntax if we have such need.
