-----Original Message-----
From: Thomas Monjalon [mailto:thomas@monjalon.net]
Sent: Thursday, January 25, 2018 10:55 AM
To: Ananyev, Konstantin <redacted>
Cc: Matan Azrad <redacted>; Gaëtan Rivet <redacted>; Wu, Jingjing <redacted>;
dev@dpdk.org; Neil Horman [off-list ref]; Richardson, Bruce [off-list ref]
Subject: Re: [dpdk-dev] [PATCH v3 7/7] app/testpmd: adjust ethdev port ownership

24/01/2018 19:30, Ananyev, Konstantin:

quoted

From: Thomas Monjalon [mailto:thomas@monjalon.net]

quoted

23/01/2018 22:18, Ananyev, Konstantin:

quoted

quoted

23/01/2018 16:18, Ananyev, Konstantin:

quoted

From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konstantin

quoted

From: Thomas Monjalon [mailto:thomas@monjalon.net]

quoted

23/01/2018 14:34, Ananyev, Konstantin:

quoted

If that' s the use case, then I think you need to set device ownership at creation time -
inside dev_allocate().
Again that would avoid such racing conditions inside testpmd.

The devices must be allocated at a low level layer.

No one arguing about that.
But we can provide owner id information to the low level.

Sorry, you did not get it.

Might be.

quoted

We cannot provide owner id at the low level
because it is not yet decided who will be the owner
before the port is allocated.

Why is that?
What prevents us decide who will manage that device *before* allocating port of it?
IMO we do have all needed information at that stage.

We don't have the information.

At that point we do have dev name and all parameters, right?

We just have the PCI id.

quoted

Plus we do have blacklist/whitelist, etc.
So what else are we missing to make the decision at that point?

It depends on the ownership policy.
Example: we can decide to take ownership based on a MAC address.

That's sounds a bit articificial (mac address can be changed on the fly), but ok -
for such devices user can decide to use default id first and change
it later after port is allocated and dev_init() is passed.
Though as I understand there situations (like in failsafe PMD) when we do 
know for sure owner_id before calling dev_allocate().

Another example: it can be decided to take ownership of a given driver.
We don't have these informations with the PCI id.

quoted

quoted

It is a new device, it is matched by a driver which allocates a port.
I don't see where the higher level can interact here.
And even if you manage a trick, the higher level needs to read the port
informations to decide the ownership.

Could you specify what particular port information it needs?

Replied to the same question above :)

quoted

quoted

quoted

quoted

quoted

quoted

quoted

When a new device appears (hotplug), an ethdev port should be allocated
automatically if it passes the whitelist/blacklist policy test.
Then we must decide who will manage this device.
I suggest notifying the DPDK libs first.
So a DPDK lib or PMD like failsafe can have the priority to take the
ownership in its notification callback.

Possible, but seems a bit overcomplicated.
Why not just:

Have a global variable process_default_owner_id, that would be init once at startup.
Have an LTS variable default_owner_id.
It will be used by rte_eth_dev_allocate() caller can set dev->owner_id at creation time,
so port allocation and setting ownership - will be an atomic operation.
At the exit rte_eth_dev_allocate() will always reset default_owner_id=0:

rte_eth_dev_allocate(...)
{
   lock(owner_lock);
   <allocate_port>
   owner = RTE_PER_LCORE(default_owner_id);
   if (owner == 0)
       owner = process_default_owner_id;
  set_owner(port, ..., owner);
 unlock(owner_lock);
 RTE_PER_LCORE(default_owner_id) = 0;

Or probably better to leave default_owner_id reset to the caller.
Another thing - we can use same LTS variable in all control ops to
allow/disallow changing of port configuration based on ownership.
Konstantin

quoted

}

So callers who don't need any special ownership - don't need to do anything.
Special callers (like failsafe) can set default_owenr_id just before calling hotplug
handling routine.

No, hotplug will not be a routine.
I am talking about real hotplug, like a device which appears in the VM.
This new device must be handled by EAL and probed automatically if
comply with whitelist/blacklist policy given by the application or user.
Real hotplug is asynchronous.

By 'asynchronous' here you mean it would be handled in the EAL interrupt thread
or something different?

Yes, we receive an hotplug event which is processed in the event thread.

quoted

Anyway, I suppose  you do need a function inside DPDK that will do the actual work in response
on hotplug event, right?

Yes

Ok, btw why that function has to be always called from interrupt thread?
Why not to allow user to decide?

Absolutely, the user must decide.
In the example of failsafe, the user instructs a policy to decide
which devices will be owned, so failsafe takes the decision based
on user inputs.

quoted

Some apps have their own thread dedicated for control ops (like testpmd)
For them it might be plausible to call that function from their own control thread context.
Konstantin

quoted

quoted

That's what I refer to as 'hotplug routine' above.

quoted

We will just receive notifications that it appeared.

Note: there is some temporary code in failsafe to manage some hotplug.
This code must be removed when it will be properly handled in EAL.

Ok, if it is just a temporary code, that would be removed soon -
then it definitely seems wrong to modify tespmd (or any other user app)
to comply with that temporary solution.

It will be modified when EAL hotplug will be implemented.

However, the ownership issue will be the same:
we don't know the owner when allocating a port.