[dpdk-dev] [PATCH v2 6/6] app/testpmd: adjust ethdev port ownership

[Matan Azrad]

Hi Lu

From: Lu, Wenzhuo, Wednesday, January 17, 2018 2:47 AM
> Hi Matan,
> 
> > -----Original Message-----
> > From: Matan Azrad [mailto:matan at mellanox.com]
> > Sent: Tuesday, January 16, 2018 4:16 PM
> > To: Lu, Wenzhuo <wenzhuo.lu at intel.com>; Thomas Monjalon
> > <thomas at monjalon.net>; Gaetan Rivet <gaetan.rivet at 6wind.com>; Wu,
> > Jingjing <jingjing.wu at intel.com>
> > Cc: dev at dpdk.org; Neil Horman <nhorman at tuxdriver.com>; Richardson,
> > Bruce <bruce.richardson at intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev at intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 6/6] app/testpmd: adjust ethdev port
> > ownership
> >
> > Hi Lu
> > From: Lu, Wenzhuo, Tuesday, January 16, 2018 7:54 AM
> > > Hi Matan,
> > >
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Matan Azrad
> > > > Sent: Sunday, January 7, 2018 5:46 PM
> > > > To: Thomas Monjalon <thomas at monjalon.net>; Gaetan Rivet
> > > > <gaetan.rivet at 6wind.com>; Wu, Jingjing <jingjing.wu at intel.com>
> > > > Cc: dev at dpdk.org; Neil Horman <nhorman at tuxdriver.com>;
> Richardson,
> > > > Bruce <bruce.richardson at intel.com>; Ananyev, Konstantin
> > > > <konstantin.ananyev at intel.com>
> > > > Subject: [dpdk-dev] [PATCH v2 6/6] app/testpmd: adjust ethdev port
> > > > ownership
> > > >
> > > > Testpmd should not use ethdev ports which are managed by other
> > > > DPDK entities.
> > > >
> > > > Set Testpmd ownership to each port which is not used by other
> > > > entity and prevent any usage of ethdev ports which are not owned by
> Testpmd.
> > > Sorry I don't follow all the discussion as there's too much. So it
> > > may be a silly question.
> >
> > No problem, I'm here for any question :)
> >
> > > Testpmd already has the parameter " --pci-whitelist" to only use the
> > > assigned devices.
> >
> > It is an EAL parameter. No? just say to EAL which devices to create..
> >
> > > When using this parameter, all the devices are owned by the current
> > > APP.
> >
> > No, what's about vdev? vdevs may manage devices(even whitlist PCI
> > devices) by themselves and want to prevent any app to use these
> > devices(see fail- safe PMD).
> I'm not an expert of EAL and vdev. Suppose this would be discussed in other
> patches.
> I don't want to bother you again here as testpmd is only used to show the
> result.
> So I think if this patch is needed just depends on if other patches are
> accepted :)
> 
Sure!

> >
> >  > So I don't know why need to set/check the ownership.
> > > BTW, in this patch, seem all the change is for ownership checking. I
> > > don't find the setting code. Do I miss something?
> >
> > Yes, see in main function (the first FOREACH).
> I think you mean this change,
> 
> @@ -2394,7 +2406,12 @@  uint8_t port_is_bonding_slave(portid_t
> slave_pid)
>  	rte_pdump_init(NULL);
>  #endif
> 
> -	nb_ports = (portid_t) rte_eth_dev_count();
> +	if (rte_eth_dev_owner_new(&my_owner.id))
> +		rte_panic("Failed to get unique owner identifier\n");
> +	snprintf(my_owner.name, sizeof(my_owner.name),
> TESTPMD_OWNER_NAME);
> +	RTE_ETH_FOREACH_DEV_OWNED_BY(port_id,
> RTE_ETH_DEV_NO_OWNER)
> +		if (rte_eth_dev_owner_set(port_id, &my_owner) == 0)
> +			nb_ports++;
>  	if (nb_ports == 0)
>  		RTE_LOG(WARNING, EAL, "No probed ethernet devices\n");

Yes.

> But I thought about some code to assign a specific device to a specific APP
> explicitly.
> This code looks like just occupying the devices with no owner. So, it means
> the first APP will occupy all the devices? It makes me confused as I don't see
> the benefit or the difference than before.

Remember that in EAL init some drivers may create ports and take ownership of them, so this code owns all the ownerless ports.
So, ports which are already owned by another DPDK entity will not success to take ownership here.
Since Testpmd wanted to manage all the ports before this feature( by mistake ), I guess this is the right behavior now, just use the ports which are not used.
 
> 
> >
> > Thanks, Matan.