20/01/2018 13:54, Ananyev, Konstantin:

Hi Neil,

quoted

----- Message-----
From: Neil Horman [mailto:nhorman@tuxdriver.com]
Sent: Friday, January 19, 2018 7:48 PM
To: Thomas Monjalon <redacted>
Cc: dev@dpdk.org; Matan Azrad <redacted>; Richardson, Bruce <redacted>; Ananyev, Konstantin
[off-list ref]; Gaetan Rivet [off-list ref]; Wu, Jingjing [off-list ref]
Subject: Re: [dpdk-dev] [PATCH v2 2/6] ethdev: add port ownership

On Fri, Jan 19, 2018 at 07:12:36PM +0100, Thomas Monjalon wrote:

quoted

19/01/2018 18:43, Neil Horman:

quoted

On Fri, Jan 19, 2018 at 06:17:51PM +0100, Thomas Monjalon wrote:

quoted

19/01/2018 16:27, Neil Horman:

quoted

On Fri, Jan 19, 2018 at 03:13:47PM +0100, Thomas Monjalon wrote:

quoted

19/01/2018 14:30, Neil Horman:

quoted

So it seems like the real point of contention that we need to settle here is,
what codifies an 'owner'.  Must it be a specific execution context, or can we
define any arbitrary section of code as being an owner?  I would agrue against
the latter.

This is the first thing explained in the cover letter:
"2. The port usage synchronization will be managed by the port owner."
There is no intent to manage the threads synchronization for a given port.
It is the responsibility of the owner (a code object) to configure its
port via only one thread.
It is consistent with not trying to manage threads synchronization
for Rx/Tx on a given queue.

Yes, in his cover letter, and I contend that notion is an invalid design point.
By codifying an area of code as an 'owner', rather than an execution context,
you're defining the notion of heirarchy, not ownership. That is to say,
you want to codify the notion that there are top level ports that the
application might see, and some of those top level ports are parents to
subordinate ports, which only the parent port should access directly.  If thats
all you want to encode, there are far easier ways to do it:

struct rte_eth_shared_data {
	< existing bits >
	struct rte_eth_port_list {
		struct rte_eth_port_list *children;
		struct rte_eth_port_list *parent;
	};
};


Build an api around a structure like that, so that the parent/child relationship
is globally clear, and this would be much easier, especially if you want to
continue asserting that the notion of synchronization/exclusion is an exercise
left to the application.

Not only Neil.
An owner can be something else than a port.
An owner can be an app process (multi-processes).
An owner can be a library.
The intent is really to solve the generic problem of which code
is managing a port.

I don't see how this precludes any part of what you just said.  Define the
rte_eth_port_list externally to the shared_data struct and allow any object you
want to allocate it, then anything you want to control a heirarchy of ports can
do so without issue, and the structure is far more clear than an opaque id that
carries subtle semantic ordering with it.

Sorry, I don't understand. Please could you rephrase?

Sure, I'm saying the fact that you want an owner to be an object
(library/port/process) rather than strictly an execution context
(process/thread) doesn't preclude what I'm proposing above.  You can create a
generic version of the strcture I propose above like so:

struct rte_obj_heirarchy {
	struct rte_obj_heirarchy *children;
	struct rte_obj_heirarchy *parent;
	void *owner_data; /* optional */
};

And embed that structure in any object you would like to give a representative
heirarchy to, you then have a fairly simple api

struct rte_obj_heirarchy *heirarchy_alloc();
bool heirarchy_set(struct rte_obj_heirarchy *parent, struct rte_obj_heirarcy *child)
void heirarchy_release(struct rte_obj_heirarchy *obj)

That gives you the privately held list relationship I think you are in part
looking for (i.e. the ability for a failsafe device to iterate over the ports it
is in control of), without the awkwardness of the ordinal priority that the
current implementation imposes.

In summary, if what you want is ownership in the strictest sense of the word
(i.e. mutually exclusive access, which I think makes sense), then using a lock
and flag is really the simplest way to go.  If instead what you want is a
heirarchical relationship where you can iterate over a limited set of objects
(the failsafe child port example), then the above is what you want.


The soution Matan is providing does some of each of these things, but comes with
very odd side effects

It offers a level of mutual exclusion, in that only one
object can own another at a time, but does so in a way that introduces this very
atypical ordinality (once an ownership object is created with owner_new, any
previously created ownership object will be denied the ability to take ownership
of a port)

Why is that?
As I understand current code: any owner id between 1 and next_owner_id 
is considered as valid.

Yes, Neil sent another email to explain it was a review mistake.