On Sun, Jun 24, 2018 at 01:33:31PM +0000, Shahaf Shuler wrote:
> One more input,
>
> Sunday, June 17, 2018 1:15 PM, Shahaf Shuler:
> > Subject: RE: [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port representors
>
> [...]
>
> > > > + eth_list = tmp;
> > > > for (i = 0; i < attr.orig_attr.phys_port_cnt; ++i) {
> > > > - eth_list[i] = mlx5_dev_spawn_one(dpdk_dev, ibv_dev, vf,
> > > > - &attr, i + 1);
> > > > - if (eth_list[i])
> > > > - continue;
> > > > - /* Save rte_errno and roll back in case of failure. */
> > > > - ret = rte_errno;
> > > > - while (i--) {
> > > > - mlx5_dev_close(eth_list[i]);
> > > > - if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> > > > -
> > > > rte_free(eth_list[i]->data->dev_private);
> > > > -
> > > > claim_zero(rte_eth_dev_release_port(eth_list[i]));
> > > > - }
> > > > - free(eth_list);
> > > > - rte_errno = ret;
> > > > - return NULL;
> > > > + eth_list[n] = mlx5_dev_spawn_one(dpdk_dev, ibv_dev[j],
> > > vf,
> > > > + &attr, i + 1,
> > > > + j ? eth_list[0] : NULL,
> > > > + j - 1);
> >
> > The representor id is according to the sort made by qsort (based on device
> > names).
> > A better way may be to set it according to the sysfs information, like you
> > do
> > in the mlx5_get_ifname function.
> > What do you think?
>
> In fact relaying on linear increasing port numbers is dangerous. In may break
> on special scenarios like BlueField.
> In BlueField there are representors between the x86 and the ARM cores. Those
> are not VF representors. The phys_port_name of those is -1 and each of them
> belongs to different phys_switch_id.
>
> We can argue whether it is correct/not to assign them w/ -1 value, but I
> think the suggested approach above can detect the right "vf_id" for those and
> not break the current behavior on x86.
> Let me know if you have other suggestions.
I didn't know that. Assuming that with these, there is exactly only one
representor per device, I think we can manage, the main issue being that
"-1" will be difficult to parse as a valid "representor" argument which uses
"-" for ranges.
Anyway, I suggest to deal with Bluefield specifics in a subsequent series.
This one focuses on and is validated with VF representors only.
--
Adrien Mazarguil
6WIND
