[dpdk-dev] [PATCH v2 2/6] ethdev: add port ownership

[Matan Azrad]

Hi Bruce
From: Bruce Richardson, Friday, January 19, 2018 11:30 AM
> On Fri, Jan 19, 2018 at 07:14:17AM +0000, Matan Azrad wrote:
> >
> > Hi Neil
> > From: Neil Horman, Friday, January 19, 2018 3:41 AM
> > > On Thu, Jan 18, 2018 at 08:21:34PM +0000, Matan Azrad wrote:
> > > > Hi Neil.
> > > >
> > > > From: Neil Horman, Thursday, January 18, 2018 8:42 PM
> >
> > <snip>
> > > > 1. What exactly do you want to improve?(in details) 2. Which API
> > > > specifically do you want to change(\ part of code)?
> > > > 3. What is the missing in current code(you can answer it in V3 I
> > > > sent if you
> > > want) which should be fixed?
> > > >
> > > >
> > > > <snip> sorry for that, I think it is not relevant continue
> > > > discussion if we are
> > > not fully understand each other. So let's start from the beginning
> > > "with good order :)" by answering the above questions.
> > >
> > >
> > > Sure, this seems like a reasonable way to level set.
> > >
> > > I mentioned in another thread that perhaps some of my issue here is
> > > perception regarding what is meant by ownership.  When I think of an
> > > ownership api I think primarily of mutual exclusion (that is to say,
> > > enforcement of a single execution context having access to a
> > > resource at any given time.  In my mind the simplest form of
> > > ownership is a spinlock or a mutex.  A single execution context
> > > either does or does not hold the resource at any one time.  Those
> > > contexts that attempt to gain excusive access to the resource call
> > > an api that (depending on
> > > implementation) either block continued execution of that thread
> > > until exclusive access to the resource can be granted, or returns
> > > immediately with a success or error indicator to let the caller know if
> access is granted.
> > >
> > > If I were to codify this port ownership api in pseudo code it would
> > > look something like this:
> > >
> > > struct rte_eth_dev {
> > >
> > > 	< eth dev bits >
> > > 	rte_spinlock_t owner_lock;
> > > 	bool locked;
> > > 	pid_t owner_pid;
> > > }
> > >
> As an aside, if you ensure that both locked (or "owned", I think in this
> context) and owner_pid are integer values, you can do away with the lock
> and use a compare-and-set to take ownership, by setting both atomically if
> unmodified from the originally read values.
> 
> > >
> > > bool rte_port_claim_ownership(struct rte_eth_dev *dev) {
> > > 	bool ret = false;
> > >
> > > 	spin_lock(dev->owner_lock);
> > > 	if (dev->locked)
> > > 		goto out;
> > > 	dev->locked = true;
> > > 	dev->owner_pid = getpid();
> > > 	ret = true;
> > > out:
> > > 	spin_unlock(dev->lock)
> > > 	return ret;
> > > }
> > >
> > >
> > > bool rte_port_release_ownership(rte_eth_dev *dev) {
> > >
> > > 	boot ret = false;
> > > 	spin_lock(dev->owner_lock);
> > > 	if (!dev->locked)
> > > 		goto out;
> > > 	if (dev->owner_pid != getpid())
> > > 		goto out;
> > > 	dev->locked = false;
> > > 	dev_owner_pid = 0;
> > > 	ret = true;
> > > out:
> > > 	spin_unlock(dev->owner_lock)
> > > 	return ret;
> > > }
> > >
> > > bool rte_port_is_owned_by(struct rte_eth_dev *dev, pid_t pid) {
> > > 	bool ret = false;
> > >
> > > 	spin_lock(dev->owner_lock);
> > > 	if (pid)
> > > 		ret = (dev->locked && (pid == dev->owner_pid));
> > > 	else
> > > 		ret = dev->locked;
> > > 	spin_unlock(dev->owner_lock);
> > > 	return ret;
> > > }
> > >
> > > The idea here is that lock state is isolated from ownership
> > > information.  Any context has the opportunity to lock the resource
> > > (in this case the eth port) despite its ownership object.
> > >
> > > In comparison, your api, which is in may ways simmilar, separates
> > > the creation of ownership objects to a separate api call, and that
> > > ownership information embodies state that is integral to the ability
> > > to get exclusive access to the resource.  I.E. if thread A calls
> > > your owner_new call, and then thread B calls owner_new, thread A
> > > will never be able to get access to any port unless it calls owner_new
> again.
> > >
> > > Does that help clarify my position?
> This would have been my understanding of what was being looked for too,
> from my minimal understanding of the problem. Thanks for putting that
> forward on behalf of many of us!
> 
> >
> > Now I fully understand you, thanks for your patience.
> >
> > So, you are missing here one of the main ideas of my port ownership
> intention.
> > There are options for X>1 different uncoordinated owners running in the
> same thread.
> 
> Thanks Matan for taking time to try and explain how your idea differs, but I
> for one am still a little confused. Sorry for the late questions.
> 
> Sure, Neil's example above takes the pid or thread id as the owner id
> parameter, but there is no reason we can't use the same scheme with
> arbitrarily assigned owner ids, so long as they are unique. We can even have
> a simple mapping table mapping ids to names of components.
> >
Sorry, don't understand your point here.
My approach asked to allocate unique ID for "any part of code want to manage\use a port".
What is the problem here and how do you suggest to fix it?

Neil approach (with process iD\ thread id ) is wrong because 2 different owners can run in same thread (as I explained a lot below).

> > For example:
> > 1. Think about Testpmd control commands that call to failsafe port devop
> which call to its sub-devices devops, while tespmd is different
> owner(controlling failsafe-port) and failsafe is a different owner(controlling
> all its sub-devices ports), There are both run control commands in the same
> thread and there are uncoordinated!
> >  2. Interrupt callbacks that anyone can register to them and all will run by
> the DPDK host thread.
> 
> Can you provide a little more details here: what is the specific issue or conflict
> in each of these examples and how does your ownership proposal fix it,
> when Neil's simpler approach doesn't?
> 
For the first example:
My approach:
Testpmd want to manage the fail-safe port, therefore it should allocate unique ID(only one time) and use owner set(by its ID) to take ownership of this port.
If it succeed to take ownership it can manage the port.
Failsafe PMD wants to manage its sub-devices ports and does the same process as Testpmd.
Everything is ok.

Neil  approach:
Testpmd want to manage the fail-safe port, therefore it just need to claim ownership(set) and its pid will take as the owner identifier.
Failsafe PMD wants to manage its sub-devices ports and does the same process as Testpmd.
But look these 2 entities run in same threads and there both can set the same pid. -> problem!

The second one just describe more scenario about more than one DPDK entities which run from the same thread.

> >
> > So, no any optional  owner becomes an owner, it depends in the specific
> implementation.
> >
> > So if some "part of code" wants to manage a port exclusively and wants to
> take ownership of it to prevent other "part of code" to use this port :
> > 1. Take ownership.
> > 2. It should ask itself: Am I run in different threads\processes? If yes, it
> should synchronize its port management.
> > 3. Release ownership in the end.
> >
> > Remember that may be different "part of code"s running in the same
> thread\threads\process\processes.
> >
> > Thanks, Matan.
> > >
> > > Regards
> > > Neil
> > >
> > > }