[dpdk-dev] [PATCH 7/7] net/mlx5: add parameter for port representors

Xueming(Steven) Li xuemingl at mellanox.com
Tue Jun 12 15:43:18 CEST 2018



> -----Original Message-----
> From: Adrien Mazarguil <adrien.mazarguil at 6wind.com>
> Sent: Tuesday, June 12, 2018 9:21 PM
> To: Xueming(Steven) Li <xuemingl at mellanox.com>
> Cc: Shahaf Shuler <shahafs at mellanox.com>; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 7/7] net/mlx5: add parameter for port representors
> 
> On Tue, Jun 12, 2018 at 08:02:17AM +0000, Xueming(Steven) Li wrote:
> > Hi Adrien,
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces at dpdk.org> On Behalf Of Adrien Mazarguil
> > > Sent: Saturday, May 26, 2018 12:35 AM
> > > To: Shahaf Shuler <shahafs at mellanox.com>
> > > Cc: dev at dpdk.org
> > > Subject: [dpdk-dev] [PATCH 7/7] net/mlx5: add parameter for port
> > > representors
> > >
> > > Prior to this patch, all port representors detected on a given
> > > device were probed and Ethernet devices instantiated for each of them.
> > >
> > > This patch adds support for the standard "representor" parameter,
> > > which implies that port representors are not probed by default anymore, except for the list
> provided through device arguments.
> > >
> > > (Patch based on prior work from Yuanhan Liu)
> > >
> > > Signed-off-by: Adrien Mazarguil <adrien.mazarguil at 6wind.com>
> <snip>
> > > @@ -1142,13 +1149,30 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
> > >  	struct rte_eth_dev **eth_list = NULL;
> > >  	struct ibv_context *ctx;
> > >  	struct ibv_device_attr_ex attr;
> > > +	struct rte_eth_devargs eth_da;
> >
> > Not related to this patch, from this data structure, maximum
> > representor count is 32, customer might use VF on container
> > environment, 32 is far from requirement. We need additional work here.
> > A workaround is that users call this api multiple times with different representor IDs.
> 
> 32 ought to be enough for anybody!
> 
> Not sure I understand your concern actually. One can't instantiate more representors than there are
> DPDK ports because the limit for both is RTE_MAX_ETHPORTS (i.e. 1 representor = 1 DPDK port). Users
> who want to spawn more than 32 DPDK ports overall must increase RTE_MAX_ETHPORTS regardless.

ConnectX-5 support 127 VFs, but as you said, increasing RTE_MAX_ETHPORTS should work.

> 
> > >  	void *tmp;
> > >  	unsigned int i;
> > >  	unsigned int j = 0;
> > >  	unsigned int n = 0;
> > >  	int ret;
> > >
> > > +	if (dpdk_dev->devargs) {
> > > +		ret = rte_eth_devargs_parse(dpdk_dev->devargs->args, &eth_da);
> > > +		if (ret)
> > > +			goto error;
> > > +	} else {
> > > +		memset(&eth_da, 0, sizeof(eth_da));
> > > +	}
> > >  next:
> > > +	if (j) {
> > > +		unsigned int k;
> > > +
> > > +		for (k = 0; k < eth_da.nb_representor_ports; ++k)
> > > +			if (eth_da.representor_ports[k] == j - 1)
> > > +				break;
> > > +		if (k == eth_da.nb_representor_ports)
> > > +			goto skip;
> > > +	}
> > >  	errno = 0;
> > >  	ctx = mlx5_glue->open_device(ibv_dev[j]);
> >
> > Need a range check for j here.
> 
> I think it's properly checked. j == 0 stands for "master device", always found at index 0 and probed.
> Representors devices, if any, start at index 1 which triggers the previous block. This block makes
> sure that a given representor is indeed enabled before either spawning the related device (pass
> through with a valid "j") or skipping it altogether (goto skip).

Yes, this code looks good. What I wanted to ask what if dev args specify an invalid rep id, e.g. 33.
This code walk through silently w/o warning, it works, but it better to have a warning if input id out of range.

> 
> I intend to leave this patch as is for v2.
> 
> --
> Adrien Mazarguil
> 6WIND


More information about the dev mailing list