[dpdk-dev,v2,1/5] ethdev: introduce device removal event
Checks
Commit Message
This new API allows reacting to a device removal.
A device removal is the sudden disappearance of a device from its
bus.
PMDs implementing support for this notification guarantee that the removal
of the underlying device does not incur a risk to the application.
In particular, Rx/Tx bursts and all other functions can still be called
(albeit likely returning errors) without triggering a crash, irrespective
of an application handling this event.
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
Signed-off-by: Elad Persiko <eladpe@mellanox.com>
---
doc/guides/nics/features/default.ini | 1 +
doc/guides/prog_guide/env_abstraction_layer.rst | 21 +++++++++++++++++++--
lib/librte_eal/common/include/rte_pci.h | 2 ++
lib/librte_ether/rte_ethdev.c | 11 +++++++----
lib/librte_ether/rte_ethdev.h | 9 +++++++--
lib/librte_ether/rte_ethdev_pci.h | 2 ++
6 files changed, 38 insertions(+), 8 deletions(-)
Comments
On 4/18/2017 1:17 PM, Gaetan Rivet wrote:
> This new API allows reacting to a device removal.
> A device removal is the sudden disappearance of a device from its
> bus.
>
> PMDs implementing support for this notification guarantee that the removal
> of the underlying device does not incur a risk to the application.
>
> In particular, Rx/Tx bursts and all other functions can still be called
> (albeit likely returning errors) without triggering a crash, irrespective
> of an application handling this event.
>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> Signed-off-by: Elad Persiko <eladpe@mellanox.com>
<...>
> diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
> index b1b9114..cafc6c7 100644
> --- a/doc/guides/nics/features/default.ini
> +++ b/doc/guides/nics/features/default.ini
> @@ -10,6 +10,7 @@
> Speed capabilities =
> Link status =
> Link status event =
> +Removal event =
> Queue status event =
> Rx interrupt =
> Free Tx mbuf on demand =
This release a few NIC features added, and it is hard to follow them if
you are particularly looking for these features.
So do you think does it make sense to put those new PMD features into
release notes to make it more visible?
Thanks,
ferruh
Hi Ferruh,
On Fri, Apr 21, 2017 at 03:59:24PM +0100, Ferruh Yigit wrote:
>On 4/18/2017 1:17 PM, Gaetan Rivet wrote:
>> This new API allows reacting to a device removal.
>> A device removal is the sudden disappearance of a device from its
>> bus.
>>
>> PMDs implementing support for this notification guarantee that the removal
>> of the underlying device does not incur a risk to the application.
>>
>> In particular, Rx/Tx bursts and all other functions can still be called
>> (albeit likely returning errors) without triggering a crash, irrespective
>> of an application handling this event.
>>
>> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
>> Signed-off-by: Elad Persiko <eladpe@mellanox.com>
>
><...>
>
>> diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
>> index b1b9114..cafc6c7 100644
>> --- a/doc/guides/nics/features/default.ini
>> +++ b/doc/guides/nics/features/default.ini
>> @@ -10,6 +10,7 @@
>> Speed capabilities =
>> Link status =
>> Link status event =
>> +Removal event =
>> Queue status event =
>> Rx interrupt =
>> Free Tx mbuf on demand =
>
>This release a few NIC features added, and it is hard to follow them if
>you are particularly looking for these features.
>
>So do you think does it make sense to put those new PMD features into
>release notes to make it more visible?
>
Yes it seems like a good idea to announce this evolution. I will
send the relevant patch for this soon.
>Thanks,
>ferruh
Am 25.04.2017 11:06 schrieb "Gaëtan Rivet" <gaetan.rivet@6wind.com>:
Hi Ferruh,
On Fri, Apr 21, 2017 at 03:59:24PM +0100, Ferruh Yigit wrote:
> On 4/18/2017 1:17 PM, Gaetan Rivet wrote:
>
>> This new API allows reacting to a device removal.
>> A device removal is the sudden disappearance of a device from its
>> bus.
>>
>
I don't think this belongs into ethdev. If it is bus related we need to
expose this from it so that apps can register for the low level device
being unplugged.
Jan
>> PMDs implementing support for this notification guarantee that the removal
>> of the underlying device does not incur a risk to the application.
>>
>> In particular, Rx/Tx bursts and all other functions can still be called
>> (albeit likely returning errors) without triggering a crash, irrespective
>> of an application handling this event.
>>
>> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
>> Signed-off-by: Elad Persiko <eladpe@mellanox.com>
>>
>
> <...>
>
> diff --git a/doc/guides/nics/features/default.ini
>> b/doc/guides/nics/features/default.ini
>> index b1b9114..cafc6c7 100644
>> --- a/doc/guides/nics/features/default.ini
>> +++ b/doc/guides/nics/features/default.ini
>> @@ -10,6 +10,7 @@
>> Speed capabilities =
>> Link status =
>> Link status event =
>> +Removal event =
>> Queue status event =
>> Rx interrupt =
>> Free Tx mbuf on demand =
>>
>
> This release a few NIC features added, and it is hard to follow them if
> you are particularly looking for these features.
>
> So do you think does it make sense to put those new PMD features into
> release notes to make it more visible?
>
>
Yes it seems like a good idea to announce this evolution. I will
send the relevant patch for this soon.
Thanks,
> ferruh
>
02/05/2017 09:35, Jan Blunck:
> Am 25.04.2017 11:06 schrieb "Gaëtan Rivet" <gaetan.rivet@6wind.com>:
>
> Hi Ferruh,
>
>
> On Fri, Apr 21, 2017 at 03:59:24PM +0100, Ferruh Yigit wrote:
>
> > On 4/18/2017 1:17 PM, Gaetan Rivet wrote:
> >
> >> This new API allows reacting to a device removal.
> >> A device removal is the sudden disappearance of a device from its
> >> bus.
> >>
> >
> I don't think this belongs into ethdev. If it is bus related we need to
> expose this from it so that apps can register for the low level device
> being unplugged.
Yes it sounds right.
We could work on device notifications.
We need to find a way of notifying the application that there is a
device event and that it affects one or more port at
ethdev/cryptodev/eventdev level.
On Tue, May 02, 2017 at 11:18:06AM +0200, Thomas Monjalon wrote:
>02/05/2017 09:35, Jan Blunck:
>> Am 25.04.2017 11:06 schrieb "Gaëtan Rivet" <gaetan.rivet@6wind.com>:
>>
>> Hi Ferruh,
>>
>>
>> On Fri, Apr 21, 2017 at 03:59:24PM +0100, Ferruh Yigit wrote:
>>
>> > On 4/18/2017 1:17 PM, Gaetan Rivet wrote:
>> >
>> >> This new API allows reacting to a device removal.
>> >> A device removal is the sudden disappearance of a device from its
>> >> bus.
>> >>
>> >
>> I don't think this belongs into ethdev. If it is bus related we need to
>> expose this from it so that apps can register for the low level device
>> being unplugged.
>
>Yes it sounds right.
>We could work on device notifications.
>We need to find a way of notifying the application that there is a
>device event and that it affects one or more port at
>ethdev/cryptodev/eventdev level.
This is interesting.
I developed this event with an easier integration in v17.05 in mind.
It needs a proper generic implementation however (as suggested in [1]).
I tried to have this discussion earlier[2], but without much interest.
However, even with a bus-level event framework, we still need a way for drivers
to advertize their support for specific events, and we still need to
differentiate devices that are ready for specific events from those that
do not.
So I agree that it would be interesting to have a generic rte_device
level interrupt framework to support generic events accross the whole
board, but I'm not sure it would make the dichotomy between the *driver
support* flag and the *device enabled* flag disappear.
Regards,
--
[1]: http://dpdk.org/ml/archives/dev/2017-April/064190.html
[2]: http://dpdk.org/ml/archives/dev/2017-March/060998.html
@@ -10,6 +10,7 @@
Speed capabilities =
Link status =
Link status event =
+Removal event =
Queue status event =
Rx interrupt =
Free Tx mbuf on demand =
@@ -178,8 +178,8 @@ The EAL also allows timed callbacks to be used in the same way as for NIC interr
.. note::
- In DPDK PMD, the only interrupts handled by the dedicated host thread are those for link status change,
- i.e. link up and link down notification.
+ In DPDK PMD, the only interrupts handled by the dedicated host thread are those for link status change
+ (link up and link down notification) and for sudden device removal.
+ RX Interrupt Event
@@ -207,6 +207,23 @@ The eth_dev driver takes responsibility to program the latter mapping.
The RX interrupt are controlled/enabled/disabled by ethdev APIs - 'rte_eth_dev_rx_intr_*'. They return failure if the PMD
hasn't support them yet. The intr_conf.rxq flag is used to turn on the capability of RX interrupt per device.
++ Device Removal Event
+
+This event is triggered by a device being removed at a bus level. Its
+underlying resources may have been made unavailable (i.e. PCI mappings
+unmapped). The PMD must make sure that on such occurrence, the application can
+still safely use its callbacks.
+
+This event can be subscribed to in the same way one would subscribe to a link
+status change event. The execution context is thus the same, i.e. it is the
+dedicated interrupt host thread.
+
+Considering this, it is likely that an application would want to close a
+device having emitted a Device Removal Event. In such case, calling
+``rte_eth_dev_close()`` can trigger it to unregister its own Device Removal Event
+callback. Care must be taken not to close the device from the interrupt handler
+context. It is necessary to reschedule such closing operation.
+
Blacklisting
~~~~~~~~~~~~
@@ -239,6 +239,8 @@ struct rte_pci_bus {
#define RTE_PCI_DRV_NEED_MAPPING 0x0001
/** Device driver supports link state interrupt */
#define RTE_PCI_DRV_INTR_LSC 0x0008
+/** Device driver supports device removal interrupt */
+#define RTE_PCI_DRV_INTR_RMV 0x0010
/**
* A structure describing a PCI mapping.
@@ -749,16 +749,19 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
return -EINVAL;
}
- /*
- * If link state interrupt is enabled, check that the
- * device supports it.
- */
+ /* Check that the device supports requested interrupts */
if ((dev_conf->intr_conf.lsc == 1) &&
(!(dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC))) {
RTE_PMD_DEBUG_TRACE("driver %s does not support lsc\n",
dev->data->drv_name);
return -EINVAL;
}
+ if ((dev_conf->intr_conf.rmv == 1) &&
+ (!(dev->data->dev_flags & RTE_ETH_DEV_INTR_RMV))) {
+ RTE_PMD_DEBUG_TRACE("driver %s does not support rmv\n",
+ dev->data->drv_name);
+ return -EINVAL;
+ }
/*
* If jumbo frames are enabled, check that the maximum RX packet
@@ -815,9 +815,11 @@ struct rte_eth_udp_tunnel {
*/
struct rte_intr_conf {
/** enable/disable lsc interrupt. 0 (default) - disable, 1 enable */
- uint16_t lsc;
+ uint32_t lsc:1;
/** enable/disable rxq interrupt. 0 (default) - disable, 1 enable */
- uint16_t rxq;
+ uint32_t rxq:1;
+ /** enable/disable rmv interrupt. 0 (default) - disable, 1 enable */
+ uint32_t rmv:1;
};
/**
@@ -1737,6 +1739,8 @@ struct rte_eth_dev_data {
#define RTE_ETH_DEV_INTR_LSC 0x0002
/** Device is a bonded slave */
#define RTE_ETH_DEV_BONDED_SLAVE 0x0004
+/** Device supports device removal interrupt */
+#define RTE_ETH_DEV_INTR_RMV 0x0008
/**
* @internal
@@ -3313,6 +3317,7 @@ enum rte_eth_event_type {
/**< reset interrupt event, sent to VF on PF reset */
RTE_ETH_EVENT_VF_MBOX, /**< message from the VF received by PF */
RTE_ETH_EVENT_MACSEC, /**< MACsec offload related event */
+ RTE_ETH_EVENT_INTR_RMV, /**< device removal event */
RTE_ETH_EVENT_MAX /**< max value of this enum */
};
@@ -64,6 +64,8 @@ rte_eth_copy_pci_info(struct rte_eth_dev *eth_dev,
eth_dev->data->dev_flags = 0;
if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)
eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
+ if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_RMV)
+ eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_RMV;
eth_dev->data->kdrv = pci_dev->kdrv;
eth_dev->data->numa_node = pci_dev->device.numa_node;