[v8,03/19] ethdev: enable hotplug on multi-process
Checks
Commit Message
We are going to 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.
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.
This patch covers the implementation of case 1,2,5,6,7,8.
Case 3,4 will be implemented on separate patch as well as handshake
mechanism.
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 5, 6:
Secondary process can attach private device which only visible to itself,
in this case no IPC is involved, primary process is not allowed to have
private device so far.
Case 7, 8:
Secondary process can also temporally to detach a share device "privately"
then attach it back later, this action also not impact other processes.
APIs changes:
rte_eth_dev_attach and rte_eth_dev_attach are extended to support
share device attach/detach in primary-secondary process model, it will
be called in case 1,2,3,4.
New API rte_eth_dev_attach_private and rte_eth_dev_detach_private are
introduced to cover case 5,6,7,8, this API can only be invoked in secondary
process.
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
lib/librte_ethdev/Makefile | 1 +
lib/librte_ethdev/ethdev_mp.c | 261 +++++++++++++++++++++++++++++++
lib/librte_ethdev/ethdev_mp.h | 41 +++++
lib/librte_ethdev/ethdev_private.h | 39 +++++
lib/librte_ethdev/meson.build | 1 +
lib/librte_ethdev/rte_ethdev.c | 210 +++++++++++++++++++++++--
lib/librte_ethdev/rte_ethdev.h | 45 ++++++
lib/librte_ethdev/rte_ethdev_core.h | 5 +
lib/librte_ethdev/rte_ethdev_version.map | 2 +
9 files changed, 588 insertions(+), 17 deletions(-)
create mode 100644 lib/librte_ethdev/ethdev_mp.c
create mode 100644 lib/librte_ethdev/ethdev_mp.h
create mode 100644 lib/librte_ethdev/ethdev_private.h
Comments
Hi,
02/07/2018 07:44, Qi Zhang:
> --- /dev/null
> +++ b/lib/librte_ethdev/ethdev_mp.h
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Intel Corporation
> + */
> +
> +#ifndef _RTE_ETHDEV_MP_H_
> +#define _RTE_ETHDEV_MP_H_
> +
> +#define MAX_DEV_ARGS_LEN 0x80
> +
> +#define ETH_DEV_MP_ACTION_REQUEST "eth_dev_mp_request"
> +#define ETH_DEV_MP_ACTION_RESPONSE "eth_dev_mp_response"
> +
> +enum eth_dev_req_type {
> + REQ_TYPE_ATTACH,
> + REQ_TYPE_PRE_DETACH,
> + REQ_TYPE_DETACH,
> + REQ_TYPE_ATTACH_ROLLBACK,
> +};
These constants are missing an ethdev prefix.
> +
> +struct eth_dev_mp_req {
> + enum eth_dev_req_type t;
> + char devargs[MAX_DEV_ARGS_LEN];
> + uint16_t port_id;
> + int result;
> +};
> +
> +/**
> + * this is a synchronous wrapper for secondary process send
> + * request to primary process, this is invoked when an attach
> + * or detach request issued from primary.
> + */
> +int eth_dev_request_to_primary(struct eth_dev_mp_req *req);
> +
> +/**
> + * this is a synchronous wrapper for primary process send
> + * request to secondary process, this is invoked when an attach
> + * or detach request issued from secondary process.
> + */
> +int eth_dev_request_to_secondary(struct eth_dev_mp_req *req);
Why do we need ethdev functions for IPC (mp request/response)?
How this model can reasonnably scale to other device classes
(crypto, compression, bbdev, eventdev, etc)?
> --- /dev/null
> +++ b/lib/librte_ethdev/ethdev_private.h
What is the purpose of a file ethdev_private.h?
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2010-2018 Intel Corporation
Are you sure about the years?
> +/**
> + * Attach a new Ethernet device in current process.
> + *
> + * @param devargs
> + * A pointer to a strings array describing the new device
> + * to be attached. The strings should be a pci address like
> + * '0000:01:00.0' or virtual device name like 'net_pcap0'.
No, no. The devargs syntax is being changed, so you should not duplicate
its description here. Better to reference an unique source of doc.
> + *
> + * @param port_id
> + * A pointer to a port identifier actually attached.
> + *
> + * @return
> + * 0 on success and port_id is filled, negative on error
> + */
> +int do_eth_dev_attach(const char *devargs, uint16_t *port_id);
So you are duplicating rte_eth_dev_attach which is flawed in its design
and should be deprecated...
As you may have noticed, rte_eth_dev_attach() is calling
rte_eal_hotplug_add() which manages the EAL device.
It is wrong because the relation between an ethdev port and
an EAL device is not a 1:1 mapping.
We must manage the ethdev port as one of the possible abstractions
of a device represented by rte_device.
Hi Thomas:
<...>
> > +enum eth_dev_req_type {
> > + REQ_TYPE_ATTACH,
> > + REQ_TYPE_PRE_DETACH,
> > + REQ_TYPE_DETACH,
> > + REQ_TYPE_ATTACH_ROLLBACK,
> > +};
>
> These constants are missing an ethdev prefix.
OK, will fix.
>
> > +
> > +struct eth_dev_mp_req {
> > + enum eth_dev_req_type t;
> > + char devargs[MAX_DEV_ARGS_LEN];
> > + uint16_t port_id;
> > + int result;
> > +};
> > +
> > +/**
> > + * this is a synchronous wrapper for secondary process send
> > + * request to primary process, this is invoked when an attach
> > + * or detach request issued from primary.
> > + */
> > +int eth_dev_request_to_primary(struct eth_dev_mp_req *req);
> > +
> > +/**
> > + * this is a synchronous wrapper for primary process send
> > + * request to secondary process, this is invoked when an attach
> > + * or detach request issued from secondary process.
> > + */
> > +int eth_dev_request_to_secondary(struct eth_dev_mp_req *req);
>
>
> Why do we need ethdev functions for IPC (mp request/response)?
> How this model can reasonnably scale to other device classes (crypto,
> compression, bbdev, eventdev, etc)?
Yes it will be more generic to more the multi-process device sync mechanism into eal layer.(rte_eal_hotplug_add/rte_eal_hotplug_remove)
I didn't do this is I'm not very sure if all anothers kinds of device type need this, but if you think this is a good direction and we need to enable for all devices,
I think this could be our next step. BTW, I guess ethdev still need some IPC to sync port_id which is specific for itself, and other device type may have similar requirement.
> > --- /dev/null
> > +++ b/lib/librte_ethdev/ethdev_private.h
>
> What is the purpose of a file ethdev_private.h?
>
> > @@ -0,0 +1,39 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2010-2018 Intel Corporation
>
> Are you sure about the years?
NO, will fix:)
>
> > +/**
> > + * Attach a new Ethernet device in current process.
> > + *
> > + * @param devargs
> > + * A pointer to a strings array describing the new device
> > + * to be attached. The strings should be a pci address like
> > + * '0000:01:00.0' or virtual device name like 'net_pcap0'.
>
> No, no. The devargs syntax is being changed, so you should not duplicate its
> description here. Better to reference an unique source of doc.
OK, will check and replace with more correct description.
>
> > + *
> > + * @param port_id
> > + * A pointer to a port identifier actually attached.
> > + *
> > + * @return
> > + * 0 on success and port_id is filled, negative on error */ int
> > +do_eth_dev_attach(const char *devargs, uint16_t *port_id);
>
> So you are duplicating rte_eth_dev_attach which is flawed in its design and
> should be deprecated...
OK, just to know this, but I guess it will not be the issue, if we move the dev sync mechanism into eal layer in future right?
Regards
Qi
>
> As you may have noticed, rte_eth_dev_attach() is calling
> rte_eal_hotplug_add() which manages the EAL device.
> It is wrong because the relation between an ethdev port and an EAL device is
> not a 1:1 mapping.
> We must manage the ethdev port as one of the possible abstractions of a
> device represented by rte_device.
>
03/07/2018 14:59, Zhang, Qi Z:
> > > +/**
> > > + * this is a synchronous wrapper for secondary process send
> > > + * request to primary process, this is invoked when an attach
> > > + * or detach request issued from primary.
> > > + */
> > > +int eth_dev_request_to_primary(struct eth_dev_mp_req *req);
> > > +
> > > +/**
> > > + * this is a synchronous wrapper for primary process send
> > > + * request to secondary process, this is invoked when an attach
> > > + * or detach request issued from secondary process.
> > > + */
> > > +int eth_dev_request_to_secondary(struct eth_dev_mp_req *req);
> >
> >
> > Why do we need ethdev functions for IPC (mp request/response)?
> > How this model can reasonnably scale to other device classes (crypto,
> > compression, bbdev, eventdev, etc)?
>
> Yes it will be more generic to more the multi-process device sync mechanism into eal layer.(rte_eal_hotplug_add/rte_eal_hotplug_remove)
> I didn't do this is I'm not very sure if all anothers kinds of device type need this, but if you think this is a good direction and we need to enable for all devices,
> I think this could be our next step. BTW, I guess ethdev still need some IPC to sync port_id which is specific for itself, and other device type may have similar requirement.
I don't understand what is specific to ethdev in this IPC.
If it is just about a port id, I think it can be done elsewhere (EAL?)
> > > --- /dev/null
> > > +++ b/lib/librte_ethdev/ethdev_private.h
> >
> > What is the purpose of a file ethdev_private.h?
You did not reply this question.
> > > +do_eth_dev_attach(const char *devargs, uint16_t *port_id);
> >
> > So you are duplicating rte_eth_dev_attach which is flawed in its design and
> > should be deprecated...
>
> OK, just to know this, but I guess it will not be the issue, if we move the dev sync mechanism into eal layer in future right?
Future is now :)
We must stop mixing devargs and port id in the same layer.
> > As you may have noticed, rte_eth_dev_attach() is calling
> > rte_eal_hotplug_add() which manages the EAL device.
> > It is wrong because the relation between an ethdev port and an EAL device is
> > not a 1:1 mapping.
> > We must manage the ethdev port as one of the possible abstractions of a
> > device represented by rte_device.
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Tuesday, July 3, 2018 10:11 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Shelton,
> Benjamin H <benjamin.h.shelton@intel.com>; Vangati, Narender
> <narender.vangati@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v8 03/19] ethdev: enable hotplug on
> multi-process
>
> 03/07/2018 14:59, Zhang, Qi Z:
> > > > +/**
> > > > + * this is a synchronous wrapper for secondary process send
> > > > + * request to primary process, this is invoked when an attach
> > > > + * or detach request issued from primary.
> > > > + */
> > > > +int eth_dev_request_to_primary(struct eth_dev_mp_req *req);
> > > > +
> > > > +/**
> > > > + * this is a synchronous wrapper for primary process send
> > > > + * request to secondary process, this is invoked when an attach
> > > > + * or detach request issued from secondary process.
> > > > + */
> > > > +int eth_dev_request_to_secondary(struct eth_dev_mp_req *req);
> > >
> > >
> > > Why do we need ethdev functions for IPC (mp request/response)?
> > > How this model can reasonnably scale to other device classes
> > > (crypto, compression, bbdev, eventdev, etc)?
> >
> > Yes it will be more generic to more the multi-process device sync
> > mechanism into eal layer.(rte_eal_hotplug_add/rte_eal_hotplug_remove)
> > I didn't do this is I'm not very sure if all anothers kinds of device
> > type need this, but if you think this is a good direction and we need to enable
> for all devices, I think this could be our next step. BTW, I guess ethdev still
> need some IPC to sync port_id which is specific for itself, and other device type
> may have similar requirement.
>
> I don't understand what is specific to ethdev in this IPC.
> If it is just about a port id, I think it can be done elsewhere (EAL?)
Yes what I mean is port _id, I'm not sure if it can be done elsewhere, I need to check.
>
> > > > --- /dev/null
> > > > +++ b/lib/librte_ethdev/ethdev_private.h
> > >
> > > What is the purpose of a file ethdev_private.h?
>
> You did not reply this question.
the header file is used to declare the functions only be used by libethdev internally
like eal_private.h, or I don't know where the three functions I should declared.
>
> > > > +do_eth_dev_attach(const char *devargs, uint16_t *port_id);
> > >
> > > So you are duplicating rte_eth_dev_attach which is flawed in its
> > > design and should be deprecated...
> >
> > OK, just to know this, but I guess it will not be the issue, if we move the dev
> sync mechanism into eal layer in future right?
>
> Future is now :)
> We must stop mixing devargs and port id in the same layer.
Ok, is there any RFC I can learn?
>
> > > As you may have noticed, rte_eth_dev_attach() is calling
> > > rte_eal_hotplug_add() which manages the EAL device.
> > > It is wrong because the relation between an ethdev port and an EAL
> > > device is not a 1:1 mapping.
> > > We must manage the ethdev port as one of the possible abstractions
> > > of a device represented by rte_device.
>
>
03/07/2018 17:03, Zhang, Qi Z:
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 03/07/2018 14:59, Zhang, Qi Z:
> > > > > +do_eth_dev_attach(const char *devargs, uint16_t *port_id);
> > > >
> > > > So you are duplicating rte_eth_dev_attach which is flawed in its
> > > > design and should be deprecated...
> > >
> > > OK, just to know this, but I guess it will not be the issue, if we move the dev
> > sync mechanism into eal layer in future right?
> >
> > Future is now :)
> > We must stop mixing devargs and port id in the same layer.
>
> Ok, is there any RFC I can learn?
RFC for what?
It is just a design issue that we must stop propagating.
> > > > As you may have noticed, rte_eth_dev_attach() is calling
> > > > rte_eal_hotplug_add() which manages the EAL device.
> > > > It is wrong because the relation between an ethdev port and an EAL
> > > > device is not a 1:1 mapping.
> > > > We must manage the ethdev port as one of the possible abstractions
> > > > of a device represented by rte_device.
03/07/2018 23:57, Thomas Monjalon:
> 03/07/2018 17:03, Zhang, Qi Z:
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > 03/07/2018 14:59, Zhang, Qi Z:
> > > > > > +do_eth_dev_attach(const char *devargs, uint16_t *port_id);
> > > > >
> > > > > So you are duplicating rte_eth_dev_attach which is flawed in its
> > > > > design and should be deprecated...
> > > >
> > > > OK, just to know this, but I guess it will not be the issue, if we move the dev
> > > sync mechanism into eal layer in future right?
> > >
> > > Future is now :)
> > > We must stop mixing devargs and port id in the same layer.
> >
> > Ok, is there any RFC I can learn?
>
> RFC for what?
> It is just a design issue that we must stop propagating.
Please read at this commit, which is 2 years old:
http://git.dpdk.org/dpdk/commit/?id=b0fb26685570
It was starting to fix early design mistakes, but unfortunately it is not
yet totally fixed today.
> > > > > As you may have noticed, rte_eth_dev_attach() is calling
> > > > > rte_eal_hotplug_add() which manages the EAL device.
> > > > > It is wrong because the relation between an ethdev port and an EAL
> > > > > device is not a 1:1 mapping.
> > > > > We must manage the ethdev port as one of the possible abstractions
> > > > > of a device represented by rte_device.
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Wednesday, July 4, 2018 6:36 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Shelton,
> Benjamin H <benjamin.h.shelton@intel.com>; Vangati, Narender
> <narender.vangati@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v8 03/19] ethdev: enable hotplug on
> multi-process
>
> 03/07/2018 23:57, Thomas Monjalon:
> > 03/07/2018 17:03, Zhang, Qi Z:
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > 03/07/2018 14:59, Zhang, Qi Z:
> > > > > > > +do_eth_dev_attach(const char *devargs, uint16_t *port_id);
> > > > > >
> > > > > > So you are duplicating rte_eth_dev_attach which is flawed in
> > > > > > its design and should be deprecated...
> > > > >
> > > > > OK, just to know this, but I guess it will not be the issue, if
> > > > > we move the dev
> > > > sync mechanism into eal layer in future right?
> > > >
> > > > Future is now :)
> > > > We must stop mixing devargs and port id in the same layer.
> > >
> > > Ok, is there any RFC I can learn?
> >
> > RFC for what?
> > It is just a design issue that we must stop propagating.
>
> Please read at this commit, which is 2 years old:
> http://git.dpdk.org/dpdk/commit/?id=b0fb26685570
> It was starting to fix early design mistakes, but unfortunately it is not yet totally
> fixed today.
OK, rte_eth_dev_attach is going to be deprecated.
Do you mean we will use rte_eal_hotplug_add to attach a device directly,
then the device driver will be responsible for propagating all the ethdev port,
and application could register callback for RTE_ETH_EVENT_NEW to know new ports are created,
is that correct?
>
> > > > > > As you may have noticed, rte_eth_dev_attach() is calling
> > > > > > rte_eal_hotplug_add() which manages the EAL device.
> > > > > > It is wrong because the relation between an ethdev port and an
> > > > > > EAL device is not a 1:1 mapping.
> > > > > > We must manage the ethdev port as one of the possible
> > > > > > abstractions of a device represented by rte_device.
>
>
04/07/2018 04:26, Zhang, Qi Z:
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 03/07/2018 23:57, Thomas Monjalon:
> > > 03/07/2018 17:03, Zhang, Qi Z:
> > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > 03/07/2018 14:59, Zhang, Qi Z:
> > > > > > > > +do_eth_dev_attach(const char *devargs, uint16_t *port_id);
> > > > > > >
> > > > > > > So you are duplicating rte_eth_dev_attach which is flawed in
> > > > > > > its design and should be deprecated...
> > > > > >
> > > > > > OK, just to know this, but I guess it will not be the issue, if
> > > > > > we move the dev
> > > > > sync mechanism into eal layer in future right?
> > > > >
> > > > > Future is now :)
> > > > > We must stop mixing devargs and port id in the same layer.
> > > >
> > > > Ok, is there any RFC I can learn?
> > >
> > > RFC for what?
> > > It is just a design issue that we must stop propagating.
> >
> > Please read at this commit, which is 2 years old:
> > http://git.dpdk.org/dpdk/commit/?id=b0fb26685570
> > It was starting to fix early design mistakes, but unfortunately it is not yet totally
> > fixed today.
>
> OK, rte_eth_dev_attach is going to be deprecated.
> Do you mean we will use rte_eal_hotplug_add to attach a device directly,
> then the device driver will be responsible for propagating all the ethdev port,
> and application could register callback for RTE_ETH_EVENT_NEW to know new ports are created,
> is that correct?
Exact!
All what you describe is already implemented.
To make it clear, EAL and ethdev must stay 2 separate layers.
The bridge between these 2 layers is done only by PMDs.
> > > > > > > As you may have noticed, rte_eth_dev_attach() is calling
> > > > > > > rte_eal_hotplug_add() which manages the EAL device.
> > > > > > > It is wrong because the relation between an ethdev port and an
> > > > > > > EAL device is not a 1:1 mapping.
> > > > > > > We must manage the ethdev port as one of the possible
> > > > > > > abstractions of a device represented by rte_device.
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Wednesday, July 4, 2018 3:34 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Shelton,
> Benjamin H <benjamin.h.shelton@intel.com>; Vangati, Narender
> <narender.vangati@intel.com>; gaetan.rivet@6wind.com
> Subject: Re: [dpdk-dev] [PATCH v8 03/19] ethdev: enable hotplug on
> multi-process
>
> 04/07/2018 04:26, Zhang, Qi Z:
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > 03/07/2018 23:57, Thomas Monjalon:
> > > > 03/07/2018 17:03, Zhang, Qi Z:
> > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > > 03/07/2018 14:59, Zhang, Qi Z:
> > > > > > > > > +do_eth_dev_attach(const char *devargs, uint16_t
> > > > > > > > > +*port_id);
> > > > > > > >
> > > > > > > > So you are duplicating rte_eth_dev_attach which is flawed
> > > > > > > > in its design and should be deprecated...
> > > > > > >
> > > > > > > OK, just to know this, but I guess it will not be the issue,
> > > > > > > if we move the dev
> > > > > > sync mechanism into eal layer in future right?
> > > > > >
> > > > > > Future is now :)
> > > > > > We must stop mixing devargs and port id in the same layer.
> > > > >
> > > > > Ok, is there any RFC I can learn?
> > > >
> > > > RFC for what?
> > > > It is just a design issue that we must stop propagating.
> > >
> > > Please read at this commit, which is 2 years old:
> > > http://git.dpdk.org/dpdk/commit/?id=b0fb26685570
> > > It was starting to fix early design mistakes, but unfortunately it
> > > is not yet totally fixed today.
> >
> > OK, rte_eth_dev_attach is going to be deprecated.
> > Do you mean we will use rte_eal_hotplug_add to attach a device
> > directly, then the device driver will be responsible for propagating
> > all the ethdev port, and application could register callback for
> > RTE_ETH_EVENT_NEW to know new ports are created, is that correct?
>
> Exact!
> All what you describe is already implemented.
>
> To make it clear, EAL and ethdev must stay 2 separate layers.
> The bridge between these 2 layers is done only by PMDs.
OK, I will move the IPC stuff from ethdev layer into eal layer in v9
Thanks!
>
>
> > > > > > > > As you may have noticed, rte_eth_dev_attach() is calling
> > > > > > > > rte_eal_hotplug_add() which manages the EAL device.
> > > > > > > > It is wrong because the relation between an ethdev port
> > > > > > > > and an EAL device is not a 1:1 mapping.
> > > > > > > > We must manage the ethdev port as one of the possible
> > > > > > > > abstractions of a device represented by rte_device.
>
>
@@ -19,6 +19,7 @@ EXPORT_MAP := rte_ethdev_version.map
LIBABIVER := 9
SRCS-y += rte_ethdev.c
+SRCS-y += ethdev_mp.c
SRCS-y += rte_flow.c
SRCS-y += rte_tm.c
SRCS-y += rte_mtr.c
new file mode 100644
@@ -0,0 +1,261 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2010-2018 Intel Corporation
+ */
+#include <rte_string_fns.h>
+#include <rte_alarm.h>
+
+#include "rte_ethdev_driver.h"
+#include "ethdev_mp.h"
+
+#define MP_TIMEOUT_S 5 /**< 5 seconds timeouts */
+
+struct mp_reply_bundle {
+ struct rte_mp_msg msg;
+ void *peer;
+};
+
+static int detach_on_secondary(uint16_t port_id)
+{
+ struct rte_device *dev;
+ struct rte_bus *bus;
+ int ret = 0;
+
+ if (rte_eth_devices[port_id].state == RTE_ETH_DEV_UNUSED) {
+ ethdev_log(ERR, "detach on secondary: invalid port %d\n",
+ port_id);
+ return -ENODEV;
+ }
+
+ dev = rte_eth_devices[port_id].device;
+ if (dev == NULL)
+ return -EINVAL;
+
+ bus = rte_bus_find_by_device(dev);
+ if (bus == NULL)
+ return -ENOENT;
+
+ ret = rte_eal_hotplug_remove(bus->name, dev->name);
+ if (ret) {
+ ethdev_log(ERR, "failed to hot unplug bus: %s, device:%s\n",
+ bus->name, dev->name);
+ return ret;
+ }
+
+ rte_eth_dev_release_port_private(&rte_eth_devices[port_id]);
+ return ret;
+}
+
+static int attach_on_secondary(const char *devargs, uint16_t port_id)
+{
+ struct rte_devargs da;
+ int ret;
+
+ if (rte_eth_devices[port_id].state != RTE_ETH_DEV_UNUSED) {
+ ethdev_log(ERR, "port %d already in used, failed to attach\n",
+ port_id);
+ return -EINVAL;
+ }
+
+ memset(&da, 0, sizeof(da));
+
+ if (rte_devargs_parse(&da, "%s", devargs)) {
+ ethdev_log(ERR, "failed to parse devargs %s\n", devargs);
+ return -EINVAL;
+ }
+
+ ret = rte_eal_hotplug_add(da.bus->name, da.name, "");
+ if (ret) {
+ ethdev_log(ERR, "failed to hotplug bus:%s, device:%s\n",
+ da.bus->name, da.name);
+ free(da.args);
+ return ret;
+ }
+
+ if (rte_eth_devices[port_id].state == RTE_ETH_DEV_UNUSED) {
+ ethdev_log(ERR,
+ "failed to attach to port %d, this is a pmd issue\n",
+ port_id);
+ free(da.args);
+ return -ENODEV;
+ }
+ free(da.args);
+ return 0;
+}
+
+static int
+handle_secondary_request(const struct rte_mp_msg *msg, const void *peer)
+{
+ RTE_SET_USED(msg);
+ RTE_SET_USED(peer);
+ return -ENOTSUP;
+}
+
+static void __handle_primary_request(void *param)
+{
+ struct mp_reply_bundle *bundle = param;
+ struct rte_mp_msg *msg = &bundle->msg;
+ const struct eth_dev_mp_req *req =
+ (const struct eth_dev_mp_req *)msg->param;
+ struct rte_mp_msg mp_resp;
+ struct eth_dev_mp_req *resp =
+ (struct eth_dev_mp_req *)mp_resp.param;
+ int ret = 0;
+
+ memset(&mp_resp, 0, sizeof(mp_resp));
+
+ switch (req->t) {
+ case REQ_TYPE_ATTACH:
+ ret = attach_on_secondary(req->devargs, req->port_id);
+ break;
+ case REQ_TYPE_PRE_DETACH:
+ ret = 0;
+ break;
+ case REQ_TYPE_DETACH:
+ case REQ_TYPE_ATTACH_ROLLBACK:
+ ret = detach_on_secondary(req->port_id);
+ break;
+ default:
+ ret = -EINVAL;
+ }
+
+ strlcpy(mp_resp.name, ETH_DEV_MP_ACTION_REQUEST, sizeof(mp_resp.name));
+ mp_resp.len_param = sizeof(*req);
+ memcpy(resp, req, sizeof(*resp));
+ resp->result = ret;
+ if (rte_mp_reply(&mp_resp, bundle->peer) < 0)
+ ethdev_log(ERR, "failed to send reply to primary request\n");
+
+ free(bundle->peer);
+ free(bundle);
+}
+
+static int
+handle_primary_request(const struct rte_mp_msg *msg, const void *peer)
+{
+
+ struct rte_mp_msg mp_resp;
+ const struct eth_dev_mp_req *req =
+ (const struct eth_dev_mp_req *)msg->param;
+ struct eth_dev_mp_req *resp =
+ (struct eth_dev_mp_req *)mp_resp.param;
+ struct mp_reply_bundle *bundle;
+ int ret = 0;
+
+ memset(&mp_resp, 0, sizeof(mp_resp));
+ strlcpy(mp_resp.name, ETH_DEV_MP_ACTION_REQUEST, sizeof(mp_resp.name));
+ mp_resp.len_param = sizeof(*req);
+ memcpy(resp, req, sizeof(*resp));
+
+ bundle = calloc(1, sizeof(*bundle));
+ if (bundle == NULL) {
+ resp->result = -ENOMEM;
+ ret = rte_mp_reply(&mp_resp, peer);
+ if (ret) {
+ ethdev_log(ERR, "failed to send reply to primary request\n");
+ return ret;
+ }
+ }
+
+ bundle->msg = *msg;
+ /**
+ * We need to send reply on interrupt thread, but peer can't be
+ * parsed directly, so this is a temporal hack, need to be fixed
+ * when it is ready.
+ */
+ bundle->peer = (void *)strdup(peer);
+
+ /**
+ * We are at IPC callback thread, sync IPC is not allowed due to
+ * dead lock, so we delegate the task to interrupt thread.
+ */
+ ret = rte_eal_alarm_set(1, __handle_primary_request, bundle);
+ if (ret) {
+ resp->result = ret;
+ ret = rte_mp_reply(&mp_resp, peer);
+ if (ret) {
+ ethdev_log(ERR, "failed to send reply to primary request\n");
+ return ret;
+ }
+ }
+ return 0;
+}
+
+int eth_dev_request_to_primary(struct eth_dev_mp_req *req)
+{
+ RTE_SET_USED(req);
+ return -ENOTSUP;
+}
+
+/**
+ * Request from primary to secondary.
+ *
+ * Be invoked when try to attach or detach a share device
+ * from primary process.
+ */
+int eth_dev_request_to_secondary(struct eth_dev_mp_req *req)
+{
+ struct rte_mp_msg mp_req;
+ struct rte_mp_reply mp_reply;
+ struct timespec ts = {.tv_sec = MP_TIMEOUT_S, .tv_nsec = 0};
+ int ret;
+ int i;
+
+ memset(&mp_req, 0, sizeof(mp_req));
+ memcpy(mp_req.param, req, sizeof(*req));
+ mp_req.len_param = sizeof(*req);
+ strlcpy(mp_req.name, ETH_DEV_MP_ACTION_REQUEST, sizeof(mp_req.name));
+
+ ret = rte_mp_request_sync(&mp_req, &mp_reply, &ts);
+ if (ret) {
+ ethdev_log(ERR, "rte_mp_request_sync failed\n");
+ return ret;
+ }
+
+ if (mp_reply.nb_sent != mp_reply.nb_received) {
+ ethdev_log(ERR, "not all secondary reply\n");
+ return -1;
+ }
+
+ req->result = 0;
+ for (i = 0; i < mp_reply.nb_received; i++) {
+ struct eth_dev_mp_req *resp =
+ (struct eth_dev_mp_req *)mp_reply.msgs[i].param;
+ if (resp->result) {
+ req->result = resp->result;
+ break;
+ }
+ }
+
+ return 0;
+}
+
+static int on_mp_init(void)
+{
+ int ret;
+
+ if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+ ret = rte_mp_action_register(ETH_DEV_MP_ACTION_REQUEST,
+ handle_secondary_request);
+ if (ret) {
+ RTE_LOG(ERR, EAL, "Couldn't register '%s' action\n",
+ ETH_DEV_MP_ACTION_REQUEST);
+ return ret;
+ }
+ } else {
+ ret = rte_mp_action_register(ETH_DEV_MP_ACTION_REQUEST,
+ handle_primary_request);
+ if (ret) {
+ RTE_LOG(ERR, EAL, "Couldn't register '%s' action\n",
+ ETH_DEV_MP_ACTION_REQUEST);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+RTE_INIT(ethdev_mp_init)
+{
+ if (rte_eal_register_mp_init(on_mp_init))
+ RTE_LOG(ERR, EAL, "ethdev mp channel init failed\n");
+}
new file mode 100644
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation
+ */
+
+#ifndef _RTE_ETHDEV_MP_H_
+#define _RTE_ETHDEV_MP_H_
+
+#define MAX_DEV_ARGS_LEN 0x80
+
+#define ETH_DEV_MP_ACTION_REQUEST "eth_dev_mp_request"
+#define ETH_DEV_MP_ACTION_RESPONSE "eth_dev_mp_response"
+
+enum eth_dev_req_type {
+ REQ_TYPE_ATTACH,
+ REQ_TYPE_PRE_DETACH,
+ REQ_TYPE_DETACH,
+ REQ_TYPE_ATTACH_ROLLBACK,
+};
+
+struct eth_dev_mp_req {
+ enum eth_dev_req_type t;
+ char devargs[MAX_DEV_ARGS_LEN];
+ uint16_t port_id;
+ int result;
+};
+
+/**
+ * this is a synchronous wrapper for secondary process send
+ * request to primary process, this is invoked when an attach
+ * or detach request issued from primary.
+ */
+int eth_dev_request_to_primary(struct eth_dev_mp_req *req);
+
+/**
+ * this is a synchronous wrapper for primary process send
+ * request to secondary process, this is invoked when an attach
+ * or detach request issued from secondary process.
+ */
+int eth_dev_request_to_secondary(struct eth_dev_mp_req *req);
+
+#endif
new file mode 100644
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2010-2018 Intel Corporation
+ */
+
+#ifndef _ETHDEV_PRIVATE_H_
+#define _ETHDEV_PRIVATE_H_
+
+/**
+ * Attach a new Ethernet device in current process.
+ *
+ * @param devargs
+ * A pointer to a strings array describing the new device
+ * to be attached. The strings should be a pci address like
+ * '0000:01:00.0' or virtual device name like 'net_pcap0'.
+ *
+ * @param port_id
+ * A pointer to a port identifier actually attached.
+ *
+ * @return
+ * 0 on success and port_id is filled, negative on error
+ */
+int do_eth_dev_attach(const char *devargs, uint16_t *port_id);
+
+/**
+ * Detach a Ethernet device in current process.
+ *
+ * @param port_id
+ * The port identifier of the device to detach.
+ *
+ * @param devname
+ * A pointer to a buffer that will be filled with the device name.
+ * This buffer must be at least RTE_DEV_NAME_MAX_LEN long.
+ *
+ * @return
+ * 0 on success and devname is filled, negative on error
+ */
+int do_eth_dev_detach(uint16_t port_id);
+
+#endif
@@ -5,6 +5,7 @@ name = 'ethdev'
version = 9
allow_experimental_apis = true
sources = files('ethdev_profile.c',
+ 'ethdev_mp.c'
'rte_ethdev.c',
'rte_flow.c',
'rte_mtr.c',
@@ -41,11 +41,13 @@
#include "rte_ethdev.h"
#include "rte_ethdev_driver.h"
#include "ethdev_profile.h"
+#include "ethdev_mp.h"
+#include "ethdev_private.h"
-static int ethdev_logtype;
+int ethdev_logtype;
-#define ethdev_log(level, fmt, ...) \
- rte_log(RTE_LOG_ ## level, ethdev_logtype, fmt "\n", ## __VA_ARGS__)
+#define RTE_ETH_MP_ACTION_REQUEST "rte_eth_mp_request"
+#define RTE_ETH_MP_ACTION_RESPONSE "rte_eth_mp_response"
static const char *MZ_RTE_ETH_DEV_DATA = "rte_eth_dev_data";
struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS];
@@ -650,9 +652,8 @@ eth_err(uint16_t port_id, int ret)
return ret;
}
-/* attach the new device, then store port_id of the device */
int
-rte_eth_dev_attach(const char *devargs, uint16_t *port_id)
+do_eth_dev_attach(const char *devargs, uint16_t *port_id)
{
int current = rte_eth_dev_count_total();
struct rte_devargs da;
@@ -697,14 +698,125 @@ rte_eth_dev_attach(const char *devargs, uint16_t *port_id)
return ret;
}
-/* detach the device, then store the name of the device */
int
-rte_eth_dev_detach(uint16_t port_id, char *name __rte_unused)
+do_eth_dev_detach(uint16_t port_id)
{
struct rte_device *dev;
struct rte_bus *bus;
+ int solid_release;
+ int ret = 0;
+
+ dev = rte_eth_devices[port_id].device;
+ if (dev == NULL)
+ return -EINVAL;
+
+ bus = rte_bus_find_by_device(dev);
+ if (bus == NULL)
+ return -ENOENT;
+
+ if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+ /**
+ * A private device on secondary need
+ * rte_eth_dev_release_port.
+ * 1) only vdev support private device.
+ * 2) private device has no empty devargs.
+ */
+ if (!strcmp(bus->name, "vdev") &&
+ dev->devargs != NULL &&
+ strlen(dev->devargs->args) > 0)
+ solid_release = 1;
+ else
+ solid_release = 0;
+ } else {
+ solid_release = 0;
+ }
+
+ ret = rte_eal_hotplug_remove(bus->name, dev->name);
+ if (ret < 0)
+ return ret;
+
+ if (solid_release)
+ return rte_eth_dev_release_port(&rte_eth_devices[port_id]);
+ else
+ return rte_eth_dev_release_port_private(
+ &rte_eth_devices[port_id]);
+}
+
+/* attach the new device, then store port_id of the device */
+int
+rte_eth_dev_attach(const char *devargs, uint16_t *port_id)
+{
+ struct eth_dev_mp_req req;
+ int ret;
+
+ if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+
+ /**
+ * If secondary process, we just send request to primary
+ * to start the process.
+ */
+ req.t = REQ_TYPE_ATTACH;
+ strlcpy(req.devargs, devargs, MAX_DEV_ARGS_LEN);
+
+ ret = eth_dev_request_to_primary(&req);
+ if (ret) {
+ ethdev_log(ERR,
+ "Failed to send device attach request to primary\n");
+ return ret;
+ }
+
+ *port_id = req.port_id;
+ return req.result;
+ }
+
+ ret = do_eth_dev_attach(devargs, port_id);
+ if (ret)
+ return ret;
+
+ /* send attach request to seoncary */
+ req.t = REQ_TYPE_ATTACH;
+ strlcpy(req.devargs, devargs, MAX_DEV_ARGS_LEN);
+ req.port_id = *port_id;
+ ret = eth_dev_request_to_secondary(&req);
+ if (ret) {
+ ethdev_log(ERR, "Failed to send device attach request to secondary\n");
+ goto rollback;
+ }
+
+ if (req.result)
+ goto rollback;
+
+ return 0;
+
+rollback:
+ /* send rollback request to secondary since some one fail to attach */
+ req.t = REQ_TYPE_ATTACH_ROLLBACK;
+ req.port_id = *port_id;
+ eth_dev_request_to_secondary(&req);
+
+ do_eth_dev_detach(*port_id);
+
+ return -ENODEV;
+}
+
+/* attach the new device, then store port_id of the device */
+int
+rte_eth_dev_attach_private(const char *devargs, uint16_t *port_id)
+{
+
+ if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+ return -ENOTSUP;
+
+ return do_eth_dev_attach(devargs, port_id);
+}
+
+/* detach the device, then store the name of the device */
+int
+rte_eth_dev_detach(uint16_t port_id, char *name __rte_unused)
+{
+ struct eth_dev_mp_req req = {0};
+ int ret;
uint32_t dev_flags;
- int ret = -1;
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
@@ -715,22 +827,86 @@ rte_eth_dev_detach(uint16_t port_id, char *name __rte_unused)
return -ENOTSUP;
}
- dev = rte_eth_devices[port_id].device;
- if (dev == NULL)
- return -EINVAL;
+ if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+ req.t = REQ_TYPE_DETACH;
+ req.port_id = port_id;
- bus = rte_bus_find_by_device(dev);
- if (bus == NULL)
- return -ENOENT;
+ /**
+ * If secondary process, we just send request to primary
+ * to start the process.
+ */
+ ret = eth_dev_request_to_primary(&req);
+ if (ret) {
+ ethdev_log(ERR,
+ "Failed to send device detach request to primary\n");
+ return ret;
+ }
- ret = rte_eal_hotplug_remove(bus->name, dev->name);
- if (ret < 0)
+ return req.result;
+ }
+
+ /* check pre_detach */
+ req.t = REQ_TYPE_PRE_DETACH;
+ req.port_id = port_id;
+ ret = eth_dev_request_to_secondary(&req);
+ if (ret) {
+ ethdev_log(ERR,
+ "Failed to send device pre-detach request to secondary\n");
+ return ret;
+ }
+
+ if (req.result) {
+ ethdev_log(ERR,
+ "Device is busy on secondary, can't be detached\n");
+ return req.result;
+ }
+
+ /* detach on seconary first */
+ req.t = REQ_TYPE_DETACH;
+ ret = eth_dev_request_to_secondary(&req);
+ if (ret) {
+ ethdev_log(ERR,
+ "Failed to send device detach request to secondary\n");
+ return ret;
+ }
+
+ if (req.result)
+ /**
+ * this should rarely happen, something wrong in secondary
+ * process, will not block primary detach.
+ */
+ ethdev_log(ERR,
+ "Failed to detach device on secondary process\n");
+
+ /* detach on primary */
+ ret = do_eth_dev_detach(port_id);
+ if (ret)
return ret;
- rte_eth_dev_release_port(&rte_eth_devices[port_id]);
return 0;
}
+/* detach the device, then store the name of the device */
+int
+rte_eth_dev_detach_private(uint16_t port_id, char *name __rte_unused)
+{
+ uint32_t dev_flags;
+
+ if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+ return -ENOTSUP;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
+
+ dev_flags = rte_eth_devices[port_id].data->dev_flags;
+ if (dev_flags & RTE_ETH_DEV_BONDED_SLAVE) {
+ ethdev_log(ERR,
+ "Port %" PRIu16 " is bonded, cannot detach", port_id);
+ return -ENOTSUP;
+ }
+
+ return do_eth_dev_detach(port_id);
+}
+
static int
rte_eth_dev_rx_queue_config(struct rte_eth_dev *dev, uint16_t nb_queues)
{
@@ -1462,6 +1462,9 @@ uint16_t __rte_experimental rte_eth_dev_count_total(void);
/**
* Attach a new Ethernet device specified by arguments.
+ * In multi-process mode, it will sync with other process
+ * to make sure all processes attach the device, any
+ * failure on other process will rollback the action.
*
* @param devargs
* A pointer to a strings array describing the new device
@@ -1475,9 +1478,31 @@ uint16_t __rte_experimental rte_eth_dev_count_total(void);
int rte_eth_dev_attach(const char *devargs, uint16_t *port_id);
/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Attach a private Ethernet device specified by arguments.
+ * A private device is invisible to other process.
+ * Can only be invoked in secondary process.
+ *
+ * @param devargs
+ * A pointer to a strings array describing the new device
+ * to be attached. The strings should be a pci address like
+ * '0000:01:00.0' or virtual device name like 'net_pcap0'.
+ * @param port_id
+ * A pointer to a port identifier actually attached.
+ * @return
+ * 0 on success and port_id is filled, negative on error
+ */
+int __rte_experimental
+rte_eth_dev_attach_private(const char *devargs, uint16_t *port_id);
+
+/**
* Detach a Ethernet device specified by port identifier.
* This function must be called when the device is in the
* closed state.
+ * In multi-process mode, it will sync with other process
+ * to detach the device.
*
* @param port_id
* The port identifier of the device to detach.
@@ -1490,6 +1515,26 @@ int rte_eth_dev_attach(const char *devargs, uint16_t *port_id);
int rte_eth_dev_detach(uint16_t port_id, char *devname);
/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Detach a private Ethernet device specified by port identifier.
+ * This function must be called when the device is in the
+ * closed state.
+ * Can only be invoked in secondary process.
+ *
+ * @param port_id
+ * The port identifier of the device to detach.
+ * @param devname
+ * A pointer to a buffer that will be filled with the device name.
+ * This buffer must be at least RTE_DEV_NAME_MAX_LEN long.
+ * @return
+ * 0 on success and devname is filled, negative on error
+ */
+int __rte_experimental
+rte_eth_dev_detach_private(uint16_t port_id, char *devname);
+
+/**
* Convert a numerical speed in Mbps to a bitmap flag that can be used in
* the bitmap link_speeds of the struct rte_eth_conf
*
@@ -622,4 +622,9 @@ struct rte_eth_dev_data {
*/
extern struct rte_eth_dev rte_eth_devices[];
+extern int ethdev_logtype;
+#define ethdev_log(level, fmt, ...) \
+ rte_log(RTE_LOG_ ## level, ethdev_logtype, fmt "\n", ## __VA_ARGS__)
+
+
#endif /* _RTE_ETHDEV_CORE_H_ */
@@ -217,9 +217,11 @@ EXPERIMENTAL {
global:
rte_eth_devargs_parse;
+ rte_eth_dev_attach_private;
rte_eth_dev_count_total;
rte_eth_dev_create;
rte_eth_dev_destroy;
+ rte_eth_dev_detach_private;
rte_eth_dev_get_module_eeprom;
rte_eth_dev_get_module_info;
rte_eth_dev_is_removed;