* [dpdk-stable] [PATCH v1] net/tap: fix keep-alive queue not detached
@ 2018-05-24 23:10 Ophir Munk
2018-05-25 0:09 ` Wiles, Keith
2018-05-25 8:29 ` Thomas Monjalon
0 siblings, 2 replies; 6+ messages in thread
From: Ophir Munk @ 2018-05-24 23:10 UTC (permalink / raw)
To: dev, Pascal Mazon, Keith Wiles
Cc: Thomas Monjalon, Olga Shern, Ophir Munk, Shahaf Shuler, stable
The TAP keep-alive queue was created in order to keep the TAP device
in Linux even in case all of its Rx/Tx queues are released (in Linux
terminology: even in case all of the TAP device file descriptors are
closed), however, the keep-alive queue itself is attached to the TAP
device like all other Rx/Tx queues and therefore the kernel will
enqueue to it some Rx packets based on the kernel RSS distribution
rules. Those packets are unknown to the application and will remain
lost in the keep-alive queue.
All queues are attached by default to the TAP device after they are
created though TUNSETIFF ioctl call.
The fix is to detach the keep-alive queue after its creation through
TUNSETQUEUE ioctl call.
Fixes: 3101191c63ab ("net/tap: fix device removal when no queue exist")
Cc: stable@dpdk.org
Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
---
drivers/net/tap/rte_eth_tap.c | 33 +++++++++++++++++++++++++++------
1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 310c7d8..c3af608 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -95,13 +95,20 @@ enum ioctl_mode {
static int tap_intr_handle_set(struct rte_eth_dev *dev, int set);
-/* Tun/Tap allocation routine
+/**
+ * Tun/Tap allocation routine
+ *
+ * @param[in] pmd
+ * Pointer to private structure.
+ *
+ * @param[in] is_keepalive
+ * Keepliave flag
*
- * name is the number of the interface to use, unless NULL to take the host
- * supplied name.
+ * @return
+ * -1 on failure, fd on success
*/
static int
-tun_alloc(struct pmd_internals *pmd)
+tun_alloc(struct pmd_internals *pmd, int is_keepalive)
{
struct ifreq ifr;
#ifdef IFF_MULTI_QUEUE
@@ -154,6 +161,20 @@ tun_alloc(struct pmd_internals *pmd)
goto error;
}
+ if (is_keepalive) {
+ /*
+ * Detach the TUN/TAP keep-alive queue
+ * to avoid traffic through it
+ */
+ ifr.ifr_flags = IFF_DETACH_QUEUE;
+ if (ioctl(fd, TUNSETQUEUE, (void *)&ifr) < 0) {
+ TAP_LOG(WARNING,
+ "Unable to detach keep-alive queue for %s: %s",
+ ifr.ifr_name, strerror(errno));
+ goto error;
+ }
+ }
+
/* Always set the file descriptor to non-blocking */
if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
TAP_LOG(WARNING,
@@ -1020,7 +1041,7 @@ tap_setup_queue(struct rte_eth_dev *dev,
pmd->name, *other_fd, dir, qid, *fd);
} else {
/* Both RX and TX fds do not exist (equal -1). Create fd */
- *fd = tun_alloc(pmd);
+ *fd = tun_alloc(pmd, 0);
if (*fd < 0) {
*fd = -1; /* restore original value */
TAP_LOG(ERR, "%s: tun_alloc() failed.", pmd->name);
@@ -1425,7 +1446,7 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
* This keep-alive file descriptor will guarantee that the TUN device
* exists even when all of its queues are closed
*/
- pmd->ka_fd = tun_alloc(pmd);
+ pmd->ka_fd = tun_alloc(pmd, 1);
if (pmd->ka_fd == -1) {
TAP_LOG(ERR, "Unable to create %s interface", tuntap_name);
goto error_exit;
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-stable] [PATCH v1] net/tap: fix keep-alive queue not detached
2018-05-24 23:10 [dpdk-stable] [PATCH v1] net/tap: fix keep-alive queue not detached Ophir Munk
@ 2018-05-25 0:09 ` Wiles, Keith
2018-05-25 8:29 ` Thomas Monjalon
1 sibling, 0 replies; 6+ messages in thread
From: Wiles, Keith @ 2018-05-25 0:09 UTC (permalink / raw)
To: Ophir Munk
Cc: dev, Pascal Mazon, Thomas Monjalon, Olga Shern, Shahaf Shuler, stable
> On May 24, 2018, at 6:10 PM, Ophir Munk <ophirmu@mellanox.com> wrote:
>
> The TAP keep-alive queue was created in order to keep the TAP device
> in Linux even in case all of its Rx/Tx queues are released (in Linux
> terminology: even in case all of the TAP device file descriptors are
> closed), however, the keep-alive queue itself is attached to the TAP
> device like all other Rx/Tx queues and therefore the kernel will
> enqueue to it some Rx packets based on the kernel RSS distribution
> rules. Those packets are unknown to the application and will remain
> lost in the keep-alive queue.
> All queues are attached by default to the TAP device after they are
> created though TUNSETIFF ioctl call.
> The fix is to detach the keep-alive queue after its creation through
> TUNSETQUEUE ioctl call.
>
> Fixes: 3101191c63ab ("net/tap: fix device removal when no queue exist")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> ---
> drivers/net/tap/rte_eth_tap.c | 33 +++++++++++++++++++++++++++------
> 1 file changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 310c7d8..c3af608 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -95,13 +95,20 @@ enum ioctl_mode {
>
> static int tap_intr_handle_set(struct rte_eth_dev *dev, int set);
>
> -/* Tun/Tap allocation routine
> +/**
> + * Tun/Tap allocation routine
> + *
> + * @param[in] pmd
> + * Pointer to private structure.
> + *
> + * @param[in] is_keepalive
> + * Keepliave flag
> *
> - * name is the number of the interface to use, unless NULL to take the host
> - * supplied name.
> + * @return
> + * -1 on failure, fd on success
> */
> static int
> -tun_alloc(struct pmd_internals *pmd)
> +tun_alloc(struct pmd_internals *pmd, int is_keepalive)
This poor tun_alloc() routine did have a flag, then it was taken out and now it is back again :-(
> {
> struct ifreq ifr;
> #ifdef IFF_MULTI_QUEUE
> @@ -154,6 +161,20 @@ tun_alloc(struct pmd_internals *pmd)
> goto error;
> }
>
> + if (is_keepalive) {
> + /*
> + * Detach the TUN/TAP keep-alive queue
> + * to avoid traffic through it
> + */
> + ifr.ifr_flags = IFF_DETACH_QUEUE;
> + if (ioctl(fd, TUNSETQUEUE, (void *)&ifr) < 0) {
> + TAP_LOG(WARNING,
> + "Unable to detach keep-alive queue for %s: %s",
> + ifr.ifr_name, strerror(errno));
> + goto error;
> + }
> + }
> +
> /* Always set the file descriptor to non-blocking */
> if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
> TAP_LOG(WARNING,
> @@ -1020,7 +1041,7 @@ tap_setup_queue(struct rte_eth_dev *dev,
> pmd->name, *other_fd, dir, qid, *fd);
> } else {
> /* Both RX and TX fds do not exist (equal -1). Create fd */
> - *fd = tun_alloc(pmd);
> + *fd = tun_alloc(pmd, 0);
> if (*fd < 0) {
> *fd = -1; /* restore original value */
> TAP_LOG(ERR, "%s: tun_alloc() failed.", pmd->name);
> @@ -1425,7 +1446,7 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
> * This keep-alive file descriptor will guarantee that the TUN device
> * exists even when all of its queues are closed
> */
> - pmd->ka_fd = tun_alloc(pmd);
> + pmd->ka_fd = tun_alloc(pmd, 1);
> if (pmd->ka_fd == -1) {
> TAP_LOG(ERR, "Unable to create %s interface", tuntap_name);
> goto error_exit;
> --
> 2.7.4
>
Looks good to me.
Acked-by: Keith Wiles<keith.wiles@intel.com>
Regards,
Keith
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-stable] [PATCH v1] net/tap: fix keep-alive queue not detached
2018-05-24 23:10 [dpdk-stable] [PATCH v1] net/tap: fix keep-alive queue not detached Ophir Munk
2018-05-25 0:09 ` Wiles, Keith
@ 2018-05-25 8:29 ` Thomas Monjalon
2018-05-25 8:37 ` Ferruh Yigit
2018-05-25 15:06 ` Ferruh Yigit
1 sibling, 2 replies; 6+ messages in thread
From: Thomas Monjalon @ 2018-05-25 8:29 UTC (permalink / raw)
To: Ophir Munk, ferruh.yigit
Cc: stable, dev, Pascal Mazon, Keith Wiles, Olga Shern, Shahaf Shuler
25/05/2018 01:10, Ophir Munk:
> The TAP keep-alive queue was created in order to keep the TAP device
> in Linux even in case all of its Rx/Tx queues are released (in Linux
> terminology: even in case all of the TAP device file descriptors are
> closed), however, the keep-alive queue itself is attached to the TAP
> device like all other Rx/Tx queues and therefore the kernel will
> enqueue to it some Rx packets based on the kernel RSS distribution
> rules. Those packets are unknown to the application and will remain
> lost in the keep-alive queue.
> All queues are attached by default to the TAP device after they are
> created though TUNSETIFF ioctl call.
> The fix is to detach the keep-alive queue after its creation through
> TUNSETQUEUE ioctl call.
>
> Fixes: 3101191c63ab ("net/tap: fix device removal when no queue exist")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> ---
> drivers/net/tap/rte_eth_tap.c | 33 +++++++++++++++++++++++++++------
> 1 file changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 310c7d8..c3af608 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -95,13 +95,20 @@ enum ioctl_mode {
>
> static int tap_intr_handle_set(struct rte_eth_dev *dev, int set);
>
> -/* Tun/Tap allocation routine
> +/**
> + * Tun/Tap allocation routine
> + *
> + * @param[in] pmd
> + * Pointer to private structure.
> + *
> + * @param[in] is_keepalive
> + * Keepliave flag
Typo: Keepliave -> Keepalive
Ferruh, please can you fix it?
Or just remove all the doxygen comments?
Note: this fix is critical. Without it, some flows are lost.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-stable] [PATCH v1] net/tap: fix keep-alive queue not detached
2018-05-25 8:29 ` Thomas Monjalon
@ 2018-05-25 8:37 ` Ferruh Yigit
2018-05-25 15:06 ` Ferruh Yigit
1 sibling, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2018-05-25 8:37 UTC (permalink / raw)
To: Thomas Monjalon, Ophir Munk
Cc: stable, dev, Pascal Mazon, Keith Wiles, Olga Shern, Shahaf Shuler
On 5/25/2018 9:29 AM, Thomas Monjalon wrote:
> 25/05/2018 01:10, Ophir Munk:
>> The TAP keep-alive queue was created in order to keep the TAP device
>> in Linux even in case all of its Rx/Tx queues are released (in Linux
>> terminology: even in case all of the TAP device file descriptors are
>> closed), however, the keep-alive queue itself is attached to the TAP
>> device like all other Rx/Tx queues and therefore the kernel will
>> enqueue to it some Rx packets based on the kernel RSS distribution
>> rules. Those packets are unknown to the application and will remain
>> lost in the keep-alive queue.
>> All queues are attached by default to the TAP device after they are
>> created though TUNSETIFF ioctl call.
>> The fix is to detach the keep-alive queue after its creation through
>> TUNSETQUEUE ioctl call.
>>
>> Fixes: 3101191c63ab ("net/tap: fix device removal when no queue exist")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
>> ---
>> drivers/net/tap/rte_eth_tap.c | 33 +++++++++++++++++++++++++++------
>> 1 file changed, 27 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
>> index 310c7d8..c3af608 100644
>> --- a/drivers/net/tap/rte_eth_tap.c
>> +++ b/drivers/net/tap/rte_eth_tap.c
>> @@ -95,13 +95,20 @@ enum ioctl_mode {
>>
>> static int tap_intr_handle_set(struct rte_eth_dev *dev, int set);
>>
>> -/* Tun/Tap allocation routine
>> +/**
>> + * Tun/Tap allocation routine
>> + *
>> + * @param[in] pmd
>> + * Pointer to private structure.
>> + *
>> + * @param[in] is_keepalive
>> + * Keepliave flag
>
> Typo: Keepliave -> Keepalive
> Ferruh, please can you fix it?
> Or just remove all the doxygen comments?
I will fix the typo while merging.
>
> Note: this fix is critical. Without it, some flows are lost.
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-stable] [PATCH v1] net/tap: fix keep-alive queue not detached
2018-05-25 8:29 ` Thomas Monjalon
2018-05-25 8:37 ` Ferruh Yigit
@ 2018-05-25 15:06 ` Ferruh Yigit
1 sibling, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2018-05-25 15:06 UTC (permalink / raw)
To: Thomas Monjalon, Ophir Munk
Cc: stable, dev, Pascal Mazon, Keith Wiles, Olga Shern, Shahaf Shuler
On 5/25/2018 9:29 AM, Thomas Monjalon wrote:
> 25/05/2018 01:10, Ophir Munk:
>> The TAP keep-alive queue was created in order to keep the TAP device
>> in Linux even in case all of its Rx/Tx queues are released (in Linux
>> terminology: even in case all of the TAP device file descriptors are
>> closed), however, the keep-alive queue itself is attached to the TAP
>> device like all other Rx/Tx queues and therefore the kernel will
>> enqueue to it some Rx packets based on the kernel RSS distribution
>> rules. Those packets are unknown to the application and will remain
>> lost in the keep-alive queue.
>> All queues are attached by default to the TAP device after they are
>> created though TUNSETIFF ioctl call.
>> The fix is to detach the keep-alive queue after its creation through
>> TUNSETQUEUE ioctl call.
>>
>> Fixes: 3101191c63ab ("net/tap: fix device removal when no queue exist")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
>> ---
>> drivers/net/tap/rte_eth_tap.c | 33 +++++++++++++++++++++++++++------
>> 1 file changed, 27 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
>> index 310c7d8..c3af608 100644
>> --- a/drivers/net/tap/rte_eth_tap.c
>> +++ b/drivers/net/tap/rte_eth_tap.c
>> @@ -95,13 +95,20 @@ enum ioctl_mode {
>>
>> static int tap_intr_handle_set(struct rte_eth_dev *dev, int set);
>>
>> -/* Tun/Tap allocation routine
>> +/**
>> + * Tun/Tap allocation routine
>> + *
>> + * @param[in] pmd
>> + * Pointer to private structure.
>> + *
>> + * @param[in] is_keepalive
>> + * Keepliave flag
>
> Typo: Keepliave -> Keepalive
> Ferruh, please can you fix it?
> Or just remove all the doxygen comments?
>
> Note: this fix is critical. Without it, some flows are lost.
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-stable] [PATCH v1] net/tap: fix keep-alive queue not detached
@ 2018-05-24 23:08 Ophir Munk
0 siblings, 0 replies; 6+ messages in thread
From: Ophir Munk @ 2018-05-24 23:08 UTC (permalink / raw)
To: Ophir Munk; +Cc: stable
The TAP keep-alive queue was created in order to keep the TAP device
in Linux even in case all of its Rx/Tx queues are released (in Linux
terminology: even in case all of the TAP device file descriptors are
closed), however, the keep-alive queue itself is attached to the TAP
device like all other Rx/Tx queues and therefore the kernel will
enqueue to it some Rx packets based on the kernel RSS distribution
rules. Those packets are unknown to the application and will remain
lost in the keep-alive queue.
All queues are attached by default to the TAP device after they are
created though TUNSETIFF ioctl call.
The fix is to detach the keep-alive queue after its creation through
TUNSETQUEUE ioctl call.
Fixes: 3101191c63ab ("net/tap: fix device removal when no queue exist")
Cc: stable@dpdk.org
Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
---
drivers/net/tap/rte_eth_tap.c | 33 +++++++++++++++++++++++++++------
1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 310c7d8..c3af608 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -95,13 +95,20 @@ enum ioctl_mode {
static int tap_intr_handle_set(struct rte_eth_dev *dev, int set);
-/* Tun/Tap allocation routine
+/**
+ * Tun/Tap allocation routine
+ *
+ * @param[in] pmd
+ * Pointer to private structure.
+ *
+ * @param[in] is_keepalive
+ * Keepliave flag
*
- * name is the number of the interface to use, unless NULL to take the host
- * supplied name.
+ * @return
+ * -1 on failure, fd on success
*/
static int
-tun_alloc(struct pmd_internals *pmd)
+tun_alloc(struct pmd_internals *pmd, int is_keepalive)
{
struct ifreq ifr;
#ifdef IFF_MULTI_QUEUE
@@ -154,6 +161,20 @@ tun_alloc(struct pmd_internals *pmd)
goto error;
}
+ if (is_keepalive) {
+ /*
+ * Detach the TUN/TAP keep-alive queue
+ * to avoid traffic through it
+ */
+ ifr.ifr_flags = IFF_DETACH_QUEUE;
+ if (ioctl(fd, TUNSETQUEUE, (void *)&ifr) < 0) {
+ TAP_LOG(WARNING,
+ "Unable to detach keep-alive queue for %s: %s",
+ ifr.ifr_name, strerror(errno));
+ goto error;
+ }
+ }
+
/* Always set the file descriptor to non-blocking */
if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
TAP_LOG(WARNING,
@@ -1020,7 +1041,7 @@ tap_setup_queue(struct rte_eth_dev *dev,
pmd->name, *other_fd, dir, qid, *fd);
} else {
/* Both RX and TX fds do not exist (equal -1). Create fd */
- *fd = tun_alloc(pmd);
+ *fd = tun_alloc(pmd, 0);
if (*fd < 0) {
*fd = -1; /* restore original value */
TAP_LOG(ERR, "%s: tun_alloc() failed.", pmd->name);
@@ -1425,7 +1446,7 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
* This keep-alive file descriptor will guarantee that the TUN device
* exists even when all of its queues are closed
*/
- pmd->ka_fd = tun_alloc(pmd);
+ pmd->ka_fd = tun_alloc(pmd, 1);
if (pmd->ka_fd == -1) {
TAP_LOG(ERR, "Unable to create %s interface", tuntap_name);
goto error_exit;
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-05-25 15:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-24 23:10 [dpdk-stable] [PATCH v1] net/tap: fix keep-alive queue not detached Ophir Munk
2018-05-25 0:09 ` Wiles, Keith
2018-05-25 8:29 ` Thomas Monjalon
2018-05-25 8:37 ` Ferruh Yigit
2018-05-25 15:06 ` Ferruh Yigit
-- strict thread matches above, loose matches on Subject: below --
2018-05-24 23:08 Ophir Munk
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).