On 19-Jun-18 4:22 AM, Zhang, Qi Z wrote:

quoted

-----Original Message-----
From: Burakov, Anatoly
Sent: Monday, June 18, 2018 4:18 PM
To: Zhang, Qi Z <redacted>; thomas@monjalon.net
Cc: Ananyev, Konstantin <redacted>; dev@dpdk.org;
Richardson, Bruce [off-list ref]; Yigit, Ferruh
[off-list ref]; Shelton, Benjamin H
[off-list ref]; Vangati, Narender
[off-list ref]
Subject: Re: [PATCH 04/22] ethdev: enable hotplug on multi-process

On 07-Jun-18 1:38 PM, Qi Zhang wrote:

quoted

The patch introduce the solution to handle different hotplug cases in
multi-process situation, it include below scenario:

1. Attach a share device from primary
2. Detach a share device from primary
3. Attach a share device from secondary 4. Detach a share device from
secondary 5. Attach a private device from secondary 6. Detach a
private device from secondary 7. Detach a share device from secondary
privately 8. Attach a share device from secondary privately

In primary-secondary process model, we assume device is shared by

default.

quoted

that means attach or detach a device on any process will broadcast to
all other processes through mp channel then device information will be
synchronized on all processes.

Any failure during attaching process will cause inconsistent status
between processes, so proper rollback action should be considered.
Also it is not safe to detach a share device when other process still
use it, so a handshake mechanism is introduced, it will be implemented
in following separate patch.

Scenario for Case 1, 2:

attach device
a) primary attach the new device if failed goto h).
b) primary send attach sync request to all secondary.
c) secondary receive request and attach device and send reply.
d) primary check the reply if all success go to i).
e) primary send attach rollback sync request to all secondary.
f) secondary receive the request and detach device and send reply.
g) primary receive the reply and detach device as rollback action.
h) attach fail
i) attach success

detach device
a) primary perform pre-detach check, if device is locked, goto i).
b) primary send pre-detach sync request to all secondary.
c) secondary perform pre-detach check and send reply.
d) primary check the reply if any fail goto i).
e) primary send detach sync request to all secondary
f) secondary detach the device and send reply (assume no fail)
g) primary detach the device.
h) detach success
i) detach failed

Case 3, 4:
This will be implemented in following patch.

If these will be implemented in following patch, why spend half the commit
message talking about it? :)

Sorry, I didn't get your point about "see half commit to talk about it" :)
This patch covered an overview, and also the implementation of case 1,2,5,6,7,8

For case 3, 4, just below 4 lines to describe it

3. Attach a share device from secondary.
4. Detach a share device from secondary.
Case 3, 4:
This will be implemented in following patch.

quoted

is commit doesn't implement secondary
process functionality at all, so the commit message should probably be
reworded to only include primary process logic, no?

OK, I will reword it to highlight the patch's scope as description at above.

Thanks!

<snip>

The return value of rte_eth_dev_request_to_primary only means communication fail,
(message not able to send, or not get reply in time).
but not the fail on attach/detach itself. (which comes from req->result)

Ah, yes, my apologies, you're right! The log message is fine then.

<snip>

quoted

Do we have to do a similar check for failsafe devices?

Just keep it same logic as before, it could be a separate patch to fix I guess.

Sure.

<snip>

quoted

Here and in other places: rte_strlcpy?

OK

Apologies, this should read strlcpy, not rte_strlcpy.

-- 
Thanks,
Anatoly