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

[Lu, Wenzhuo]

Hi Matan,

> -----Original Message-----
> From: Matan Azrad [mailto:matan at mellanox.com]
> Sent: Wednesday, January 17, 2018 4:51 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, 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 for the explanation. Sounds good to me :)

> 
> >
> > >
> > > Thanks, Matan.