From: Matan Azrad <matan@mellanox.com>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: "Thomas Monjalon" <thomas@monjalon.net>,
"dev@dpdk.org" <dev@dpdk.org>,
"Bruce Richardson" <bruce.richardson@intel.com>,
"Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"Gaëtan Rivet" <gaetan.rivet@6wind.com>,
"Wu, Jingjing" <jingjing.wu@intel.com>
Subject: Re: [dpdk-dev] [PATCH 2/5] ethdev: add port ownership
Date: Thu, 21 Dec 2017 21:57:43 +0000 [thread overview]
Message-ID: <HE1PR0502MB3659EA5631A0E14B1248E9B4D20D0@HE1PR0502MB3659.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <20171221201420.GC23958@hmswarspite.think-freely.org>
> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Thursday, December 21, 2017 10:14 PM
> To: Matan Azrad <matan@mellanox.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org; Bruce
> Richardson <bruce.richardson@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Gaëtan Rivet <gaetan.rivet@6wind.com>;
> Wu, Jingjing <jingjing.wu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 2/5] ethdev: add port ownership
>
> On Thu, Dec 21, 2017 at 07:37:06PM +0000, Matan Azrad wrote:
> > Hi
> >
<snip>
> > > > > > I think we need to clearly describe what is the tread-safety
> > > > > > policy in DPDK (especially in ethdev as a first example).
> > > > > > Let's start with obvious things:
> > > > > >
> > > > > > 1/ A queue is not protected for races with multiple Rx or Tx
> > > > > > - no planned change because of performance
> > > purpose
> > > > > > 2/ The list of devices is racy
> > > > > > - to be fixed with atomics
> > > > > > 3/ The configuration of different devices is thread-safe
> > > > > > - the configurations are different per-device
> > > > > > 4/ The configuration of a given device is racy
> > > > > > - can be managed by the owner of the device
> > > > > > 5/ The device ownership is racy
> > > > > > - to be fixed with atomics
> > > > > >
> > > > > > What am I missing?
> > > > > >
> >
> > Thank you Thomas for this order.
> > Actually the port ownership is a good opportunity to redefine the
> > synchronization rules in ethdev :)
> >
> > > > > There is fan out to consider here:
> > > > >
> > > > > 1) Is device configuration racy with ownership? That is to say,
> > > > > can I change ownership of a device safely while another thread
> > > > > that currently owns it modifies its configuration?
> > > >
> > > > If an entity steals ownership to another one, either it is agreed
> > > > earlier, or it is done by a central authority.
> > > > When it is acked that ownership can be moved, there should not be
> > > > any configuration in progress.
> > > > So it is more a communication issue than a race.
> > > >
> > > But if thats the case (specifically that mutual exclusion between
> > > port ownership and configuration is an exercize left to an
> > > application developer), then port ownership itself is largely
> > > meaningless within the dpdk, because the notion of who owns the port
> > > needs to be codified within the application anyway.
> > >
> >
> > Bruce, As I understand it, only the dpdk entity who took ownership of a
> port successfully can configure the device by default, if other dpdk entities
> want to configure it too they must to be synchronized with the port owner
> while it is not recommended after the port ownership integration.
> >
> Can you clarify what you mean by "it is not recommended after the port
> ownership integration"?
Sure,
The new defining of ethdev synchronization doesn't recommend to manage a port by 2 different dpdk entities, it can be done but not recommended.
> I think there is consensus that the port owner must
> be the only entitiy to operate on a port (be that configuration/frame rx/tx, or
> some other operation).
Your question above caused me to think that you don't understand it, How can someone who is not the port owner to change the port owner?
Changing the port owner, like port configuration and port release must be done by the owner itself except the case that there is no owner to the port.
See the API rte_eth_dev_owner_remove.
> Multithreaded operation on a port always means
> some level of synchronization between application threads and the dpdk
> library,
Yes.
>but I'm not sure why that would be different if we introduced a more
> concrete notion of port ownership via a new library.
>
What do you mean by "new library"?, port is an ethdev instance and should be managed by ethdev.
> > So, for example, if the dpdk entity is an application, the application should
>> take ownership of the port and manage the synchronization of this port
>> configuration between the application threads and its EAL host thread
>> callbacks, no other dpdk entity should configure the same port because they
>> should fail when they try to take ownership of the same port too.
> Well, failing is one good approach, yes, blocking on port relenquishment
> could be another. I'd recommend an API with the following interface:
>
> rte_port_ownership_claim(int port_id) - blocks execution of the calling
> thread until the previous owner releases ownership, then claims it and
> returns
>
> rte_port_ownership_release(int port_id) - releases ownership of port, or
> returns error if the port was not owned by this execution context
>
> rte_port_owernship_try_claim(int port_id) - same as
> rte_port_ownership_claim, but fails if the port is already owned.
>
> That would give the option for both semantics.
I think the current APIs are better because of the next reasons:
- It defines well who is the owner.
- The owner structure includes string to allow better debug and printing for humans.
Did you read it?
I can add there an API that wait until the port ownership is released as you suggested in V2.
> > Each dpdk entity which wants to take ownership must to be able to
> >synchronize the port configuration in its level.
> Can you elaborate on what you mean by level here? Are you envisioning a
> scheme in which multiple execution contexts might own a port for various
> non-conflicting purposes?
Sure,
1) Application with 2 threads wanting to configure the same port:
level = application code.
a. The main thread should create owner identifier(rte_eth_dev_owner_new).
b. The main thread should take the port ownership(rte_eth_dev_owner_set).
c. Synchronization between the two threads should be done for the conflicted configurations by the application.
d. when the application finishes the port usage it should release the owner(rte_eth_dev_owner_remove).
2) Fail-safe PMD manages 2 sub-devices (2 ports) and uses alarm for hotplug detections which can configure the 2 ports(by the host thread).
Level = fail-safe code.
a. Application starts the eal and the fail-safe driver probing function is called.
b. Fail-safe probes the 2 sub-devices(2 ports are created) and takes ownership for them.
c. Failsafe creates itself port and leaves it ownerless.
d. Failsafe starts the hotplug alarm mechanism.
e. Application tries to take ownership for all ports and success only for failsafe port.
f. Application start to configure the failsafe port asynchronously to failsafe hotplug alarm.
g. Failsafe must use synchronization between failsafe alarm callback code and failsafe configuration APIs called by the application because they both try to configure the same sub-devices ports.
h. When fail-safe finishes with the two sub devices it should release the ports owner.
> >
> > >
> > > > > 2) Is device configuration racy with device addition/removal?
> > > > > That is to say, can one thread remove a device while another
> configures it.
> > > >
> > > > I think it is the same as two threads configuring the same device
> > > > (item 4/ above). It can be managed with port ownership.
> > > >
> > > Only if you assert that application is required to have the owning
> > > port be responsible for the ports deletion, which we can say, but
> > > that leads to the issue above again.
> > >
> > >
> > As Thomas said in item 2 the port creation must be synchronized by ethdev
> and we need to add it there.
> > I think it is obvious that port removal must to be done only by the port
> owner.
> >
> You say that, but its obvious to you as a developer who has looked
> extensively at the code. It may well be less so to a consumer who is not an
> active member of the community, for instance one who obtains the dpdk via
> pre-built package.
>
Yes I can understand, but new rules should be documented and be adjusted easy easy by the customers, no?
The old way to sync configuration still exists.
> >
> > I think we need to add synchronization for port ownership management in
> this patch V2 and add port creation synchronization in ethdev in separate
> patch to fill the new rules Thomas suggested.
> I think that makes sense, yes.
>
> Neil
>
> >
> > What do you think?
> >
> > > > > There are probably many subsystem interactions that need to be
> > > addressed here.
> > > > >
> > > > > Neil
> > > > >
> > > > > > I am also wondering whether the device ownership can be a
> > > > > > separate library used in several device class interfaces?
> > > >
> > > >
> > > >
> >
next prev parent reply other threads:[~2017-12-21 21:57 UTC|newest]
Thread overview: 214+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-28 11:57 [dpdk-dev] [PATCH 0/5] ethdev: Port ownership Matan Azrad
2017-11-28 11:57 ` [dpdk-dev] [PATCH 1/5] ethdev: free a port by a dedicated API Matan Azrad
2017-11-28 11:57 ` [dpdk-dev] [PATCH 2/5] ethdev: add port ownership Matan Azrad
2017-11-30 12:36 ` Neil Horman
2017-11-30 13:24 ` Gaëtan Rivet
2017-11-30 14:30 ` Matan Azrad
2017-11-30 15:09 ` Gaëtan Rivet
2017-11-30 15:43 ` Matan Azrad
2017-12-01 12:09 ` Neil Horman
2017-12-03 8:04 ` Matan Azrad
2017-12-03 11:10 ` Ananyev, Konstantin
2017-12-03 13:46 ` Matan Azrad
2017-12-04 16:01 ` Neil Horman
2017-12-04 18:10 ` Matan Azrad
2017-12-04 22:30 ` Neil Horman
2017-12-05 6:08 ` Matan Azrad
2017-12-05 10:05 ` Bruce Richardson
2017-12-08 11:35 ` Thomas Monjalon
2017-12-08 12:31 ` Neil Horman
2017-12-21 17:06 ` Thomas Monjalon
2017-12-21 17:43 ` Neil Horman
2017-12-21 19:37 ` Matan Azrad
2017-12-21 20:14 ` Neil Horman
2017-12-21 21:57 ` Matan Azrad [this message]
2017-12-22 14:26 ` Neil Horman
2017-12-23 22:36 ` Matan Azrad
2017-12-29 16:56 ` Neil Horman
2017-12-05 19:26 ` Neil Horman
2017-12-08 11:06 ` Thomas Monjalon
2017-12-05 11:12 ` Ananyev, Konstantin
2017-12-05 11:44 ` Ananyev, Konstantin
2017-12-05 11:53 ` Thomas Monjalon
2017-12-05 14:56 ` Bruce Richardson
2017-12-05 14:57 ` Ananyev, Konstantin
2017-12-05 11:47 ` Thomas Monjalon
2017-12-05 15:13 ` Ananyev, Konstantin
2017-12-05 15:49 ` Thomas Monjalon
2017-11-28 11:57 ` [dpdk-dev] [PATCH 3/5] net/failsafe: free an eth port by a dedicated API Matan Azrad
2017-11-28 11:58 ` [dpdk-dev] [PATCH 4/5] net/failsafe: use ownership mechanism to own ports Matan Azrad
2017-11-28 11:58 ` [dpdk-dev] [PATCH 5/5] app/testpmd: adjust ethdev port ownership Matan Azrad
2018-01-07 9:45 ` [dpdk-dev] [PATCH v2 0/6] ethdev: " Matan Azrad
2018-01-07 9:45 ` [dpdk-dev] [PATCH v2 1/6] ethdev: fix port data reset timing Matan Azrad
2018-01-07 9:45 ` [dpdk-dev] [PATCH v2 2/6] ethdev: add port ownership Matan Azrad
2018-01-10 13:36 ` Ananyev, Konstantin
2018-01-10 16:58 ` Matan Azrad
2018-01-11 12:40 ` Ananyev, Konstantin
2018-01-11 14:51 ` Matan Azrad
2018-01-12 0:02 ` Ananyev, Konstantin
2018-01-12 7:24 ` Matan Azrad
2018-01-15 11:45 ` Ananyev, Konstantin
2018-01-15 13:09 ` Matan Azrad
2018-01-15 18:43 ` Ananyev, Konstantin
2018-01-16 8:04 ` Matan Azrad
2018-01-16 19:11 ` Ananyev, Konstantin
2018-01-16 20:32 ` Matan Azrad
2018-01-17 11:24 ` Ananyev, Konstantin
2018-01-17 12:05 ` Matan Azrad
2018-01-17 12:54 ` Ananyev, Konstantin
2018-01-17 13:10 ` Matan Azrad
2018-01-17 16:52 ` Ananyev, Konstantin
2018-01-17 18:02 ` Matan Azrad
2018-01-17 20:34 ` Matan Azrad
2018-01-18 14:17 ` Ananyev, Konstantin
2018-01-18 14:26 ` Matan Azrad
2018-01-18 14:41 ` Ananyev, Konstantin
2018-01-18 14:45 ` Matan Azrad
2018-01-18 14:51 ` Ananyev, Konstantin
2018-01-18 15:00 ` Matan Azrad
2018-01-17 14:00 ` Neil Horman
2018-01-17 17:01 ` Ananyev, Konstantin
2018-01-18 13:10 ` Neil Horman
2018-01-18 14:00 ` Matan Azrad
2018-01-18 16:54 ` Neil Horman
2018-01-18 17:20 ` Matan Azrad
2018-01-18 18:41 ` Neil Horman
2018-01-18 20:21 ` Matan Azrad
2018-01-19 1:41 ` Neil Horman
2018-01-19 7:14 ` Matan Azrad
2018-01-19 9:30 ` Bruce Richardson
2018-01-19 10:44 ` Matan Azrad
2018-01-19 13:30 ` Neil Horman
2018-01-19 13:57 ` Matan Azrad
2018-01-19 14:13 ` Thomas Monjalon
2018-01-19 15:27 ` Neil Horman
2018-01-19 17:17 ` Thomas Monjalon
2018-01-19 17:43 ` Neil Horman
2018-01-19 18:12 ` Thomas Monjalon
2018-01-19 19:47 ` Neil Horman
2018-01-19 20:19 ` Thomas Monjalon
2018-01-19 22:52 ` Neil Horman
2018-01-20 3:38 ` Tuxdriver
2018-01-20 12:54 ` Ananyev, Konstantin
2018-01-20 14:02 ` Thomas Monjalon
2018-01-19 12:55 ` Neil Horman
2018-01-19 13:52 ` Neil Horman
2018-01-18 16:27 ` Neil Horman
2018-01-17 17:58 ` Matan Azrad
2018-01-18 13:20 ` Neil Horman
2018-01-18 14:52 ` Matan Azrad
2018-01-19 13:57 ` Neil Horman
2018-01-19 14:07 ` Thomas Monjalon
2018-01-19 14:32 ` Neil Horman
2018-01-19 17:09 ` Thomas Monjalon
2018-01-19 17:37 ` Neil Horman
2018-01-19 18:10 ` Thomas Monjalon
2018-01-21 22:12 ` Ferruh Yigit
2018-01-07 9:45 ` [dpdk-dev] [PATCH v2 3/6] ethdev: synchronize port allocation Matan Azrad
2018-01-07 9:58 ` Matan Azrad
2018-01-07 9:45 ` [dpdk-dev] [PATCH v2 4/6] net/failsafe: free an eth port by a dedicated API Matan Azrad
2018-01-07 9:45 ` [dpdk-dev] [PATCH v2 5/6] net/failsafe: use ownership mechanism to own ports Matan Azrad
2018-01-08 10:32 ` Gaëtan Rivet
2018-01-08 11:16 ` Matan Azrad
2018-01-08 11:35 ` Gaëtan Rivet
2018-01-07 9:45 ` [dpdk-dev] [PATCH v2 6/6] app/testpmd: adjust ethdev port ownership Matan Azrad
2018-01-08 11:39 ` Gaëtan Rivet
2018-01-08 12:30 ` Matan Azrad
2018-01-08 13:30 ` Gaëtan Rivet
2018-01-08 13:55 ` Matan Azrad
2018-01-08 14:21 ` Gaëtan Rivet
2018-01-08 14:42 ` Matan Azrad
2018-01-16 5:53 ` Lu, Wenzhuo
2018-01-16 8:15 ` Matan Azrad
2018-01-17 0:46 ` Lu, Wenzhuo
2018-01-17 8:51 ` Matan Azrad
2018-01-18 0:53 ` Lu, Wenzhuo
2018-01-18 16:35 ` [dpdk-dev] [PATCH v3 0/7] Port ownership and syncronization Matan Azrad
2018-01-18 16:35 ` [dpdk-dev] [PATCH v3 1/7] ethdev: fix port data reset timing Matan Azrad
2018-01-18 17:00 ` Thomas Monjalon
2018-01-19 12:38 ` Ananyev, Konstantin
2018-03-05 11:24 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
2018-03-05 14:52 ` Matan Azrad
2018-03-05 15:06 ` Ferruh Yigit
2018-03-05 15:12 ` Matan Azrad
2018-03-27 22:37 ` Ferruh Yigit
2018-03-28 12:07 ` Matan Azrad
2018-03-30 10:39 ` Ferruh Yigit
2018-04-19 11:07 ` Ferruh Yigit
2018-04-25 12:16 ` Matan Azrad
2018-04-25 12:30 ` Ori Kam
2018-04-25 12:54 ` Ferruh Yigit
2018-04-25 14:01 ` Matan Azrad
2018-01-18 16:35 ` [dpdk-dev] [PATCH v3 2/7] ethdev: fix used portid allocation Matan Azrad
2018-01-18 17:00 ` Thomas Monjalon
2018-01-19 12:40 ` Ananyev, Konstantin
2018-01-20 16:48 ` Matan Azrad
2018-01-20 17:26 ` Ananyev, Konstantin
2018-01-18 16:35 ` [dpdk-dev] [PATCH v3 3/7] ethdev: add port ownership Matan Azrad
2018-01-18 21:11 ` Thomas Monjalon
2018-01-19 12:41 ` Ananyev, Konstantin
2018-01-18 16:35 ` [dpdk-dev] [PATCH v3 4/7] ethdev: synchronize port allocation Matan Azrad
2018-01-18 20:43 ` Thomas Monjalon
2018-01-18 20:52 ` Matan Azrad
2018-01-18 21:17 ` Thomas Monjalon
2018-01-19 12:47 ` Ananyev, Konstantin
2018-01-18 16:35 ` [dpdk-dev] [PATCH v3 5/7] net/failsafe: free an eth port by a dedicated API Matan Azrad
2018-01-18 16:35 ` [dpdk-dev] [PATCH v3 6/7] net/failsafe: use ownership mechanism to own ports Matan Azrad
2018-01-18 16:35 ` [dpdk-dev] [PATCH v3 7/7] app/testpmd: adjust ethdev port ownership Matan Azrad
2018-01-19 12:37 ` Ananyev, Konstantin
2018-01-19 12:51 ` Matan Azrad
2018-01-19 13:08 ` Ananyev, Konstantin
2018-01-19 13:35 ` Matan Azrad
2018-01-19 15:00 ` Gaëtan Rivet
2018-01-20 18:14 ` Matan Azrad
2018-01-22 10:17 ` Gaëtan Rivet
2018-01-22 11:22 ` Matan Azrad
2018-01-22 12:28 ` Ananyev, Konstantin
2018-01-22 13:22 ` Matan Azrad
2018-01-22 20:48 ` Ananyev, Konstantin
2018-01-23 8:54 ` Matan Azrad
2018-01-23 12:56 ` Gaëtan Rivet
2018-01-23 14:30 ` Matan Azrad
2018-01-25 9:36 ` Matan Azrad
2018-01-25 10:05 ` Thomas Monjalon
2018-01-25 11:15 ` Ananyev, Konstantin
2018-01-25 11:33 ` Thomas Monjalon
2018-01-25 11:55 ` Ananyev, Konstantin
2018-01-23 13:34 ` Ananyev, Konstantin
2018-01-23 14:18 ` Thomas Monjalon
2018-01-23 15:12 ` Ananyev, Konstantin
2018-01-23 15:18 ` Ananyev, Konstantin
2018-01-23 17:33 ` Thomas Monjalon
2018-01-23 21:18 ` Ananyev, Konstantin
2018-01-24 8:10 ` Thomas Monjalon
2018-01-24 18:30 ` Ananyev, Konstantin
2018-01-25 10:55 ` Thomas Monjalon
2018-01-25 11:09 ` Ananyev, Konstantin
2018-01-25 11:27 ` Thomas Monjalon
2018-01-23 14:43 ` Matan Azrad
2018-01-20 21:24 ` [dpdk-dev] [PATCH v4 0/7] Port ownership and syncronization Matan Azrad
2018-01-20 21:24 ` [dpdk-dev] [PATCH v4 1/7] ethdev: fix port data reset timing Matan Azrad
2018-01-20 21:24 ` [dpdk-dev] [PATCH v4 2/7] ethdev: fix used portid allocation Matan Azrad
2018-01-20 21:24 ` [dpdk-dev] [PATCH v4 3/7] ethdev: add port ownership Matan Azrad
2018-01-21 20:43 ` Ferruh Yigit
2018-01-21 20:46 ` Ferruh Yigit
2018-01-20 21:24 ` [dpdk-dev] [PATCH v4 4/7] ethdev: synchronize port allocation Matan Azrad
2018-01-20 21:24 ` [dpdk-dev] [PATCH v4 5/7] net/failsafe: free an eth port by a dedicated API Matan Azrad
2018-01-20 21:24 ` [dpdk-dev] [PATCH v4 6/7] net/failsafe: use ownership mechanism to own ports Matan Azrad
2018-01-20 21:24 ` [dpdk-dev] [PATCH v4 7/7] app/testpmd: adjust ethdev port ownership Matan Azrad
2018-01-22 16:38 ` [dpdk-dev] [PATCH v5 0/7] Port ownership and synchronization Matan Azrad
2018-01-22 16:38 ` [dpdk-dev] [PATCH v5 1/7] ethdev: fix port data reset timing Matan Azrad
2018-01-22 16:38 ` [dpdk-dev] [PATCH v5 2/7] ethdev: fix used portid allocation Matan Azrad
2018-01-22 16:38 ` [dpdk-dev] [PATCH v5 3/7] ethdev: add port ownership Matan Azrad
2018-01-22 16:38 ` [dpdk-dev] [PATCH v5 4/7] ethdev: synchronize port allocation Matan Azrad
2018-01-22 16:38 ` [dpdk-dev] [PATCH v5 5/7] net/failsafe: free an eth port by a dedicated API Matan Azrad
2018-01-22 16:38 ` [dpdk-dev] [PATCH v5 6/7] net/failsafe: use ownership mechanism to own ports Matan Azrad
2018-01-22 16:38 ` [dpdk-dev] [PATCH v5 7/7] app/testpmd: adjust ethdev port ownership Matan Azrad
2018-01-25 1:47 ` Lu, Wenzhuo
2018-01-25 8:30 ` Matan Azrad
2018-01-26 0:50 ` Lu, Wenzhuo
2018-01-29 11:21 ` [dpdk-dev] [PATCH v5 0/7] Port ownership and synchronization Matan Azrad
2018-01-31 19:53 ` Thomas Monjalon
2018-01-25 14:35 ` [dpdk-dev] [PATCH v3 0/7] Port ownership and syncronization Ferruh Yigit
2017-12-06 0:40 [dpdk-dev] [PATCH 2/5] ethdev: add port ownership Ananyev, Konstantin
2017-12-06 9:22 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=HE1PR0502MB3659EA5631A0E14B1248E9B4D20D0@HE1PR0502MB3659.eurprd05.prod.outlook.com \
--to=matan@mellanox.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=gaetan.rivet@6wind.com \
--cc=jingjing.wu@intel.com \
--cc=konstantin.ananyev@intel.com \
--cc=nhorman@tuxdriver.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).