tun: fix multiqueue rx
Subsystems (3)
PATCH 1/1 tun: fix multiqueue rx
When writing packets to a descriptor associated with a combined queue, the packets should end up on that queue.
Before this change all packets written to any descriptor associated with a tap interface end up on rx-0, even when the descriptor is associated with a different queue.
The rx traffic can be generated by either of the following.
1. a simple tap program which spins up multiple queues and writes packets to each of the file descriptors
2. tx from a qemu vm with a tap multiqueue netdev
The queue for rx traffic can be observed by either of the following (done on the hypervisor in the qemu case).
1. a simple netmap program which opens and reads from per-queue descriptors
2. configuring RPS and doing per-cpu captures with rxtxcpu
Alternatively, if you printk() the return value of skb_get_rx_queue() just before each instance of netif_receive_skb() in tun.c, you will get 65535 for every skb.
Calling skb_record_rx_queue() to set the rx queue to the queue_index fixes the association between descriptor and rx queue.
--- drivers/net/tun.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/net/tun.c +5 -1
@@ -1536,6 +1536,7 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,if (!rx_batched || (!more &&skb_queue_empty (queue))) {local_bh_disable (); +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb);local_bh_enable ();return ;
@@ -1555,8 +1556,11 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,struct sk_buff *nskb;local_bh_disable (); -while ((nskb =__skb_dequeue (&process_queue))) +while ((nskb =__skb_dequeue (&process_queue))) { +skb_record_rx_queue (nskb, tfile->queue_index);netif_receive_skb (nskb); + } +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb);local_bh_enable (); }
Signature · 1 line
2.15.2 (Apple Git-101.1)
Quoted 45 lines · Matthew Cover
When writing packets to a descriptor associated with a combined queue, the packets should end up on that queue.
Before this change all packets written to any descriptor associated with a tap interface end up on rx-0, even when the descriptor is associated with a different queue.
The rx traffic can be generated by either of the following.
1. a simple tap program which spins up multiple queues and writes packets to each of the file descriptors
2. tx from a qemu vm with a tap multiqueue netdevThe queue for rx traffic can be observed by either of the following (done on the hypervisor in the qemu case).
1. a simple netmap program which opens and reads from per-queue descriptors
2. configuring RPS and doing per-cpu captures with rxtxcpuAlternatively, if you
printk()the return value ofskb_get_rx_queue()just before each instance ofnetif_receive_skb()intun.c, you will get 65535 for every skb.Calling
skb_record_rx_queue()to set the rx queue to thequeue_indexfixes the association between descriptor and rx queue.Signed-off-by Matthew Cover <[email protected]>--- drivers/net/tun.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)drivers/net/tun.c +1
-0@@ -1536,6 +1536,7 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,if (!rx_batched || (!more &&skb_queue_empty (queue))) {local_bh_disable (); +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb);local_bh_enable ();return ;@@ -1555,8 +1556,11 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,struct sk_buff *nskb;
drivers/net/tun.c static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile, · line 1566
Quoted 10 lines
drivers/net/tun.c +4
-1local_bh_disable (); -while ((nskb =__skb_dequeue (&process_queue))) +while ((nskb =__skb_dequeue (&process_queue))) { +skb_record_rx_queue (nskb, tfile->queue_index);netif_receive_skb (nskb); + } +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb);local_bh_enable (); }
Thanks for the fix. Actually, there's another path which needs to be fixed as well in tun_xdp_one(). This path is used for vhost to pass a batched of packets.
PATCH 1/1 tun: fix multiqueue rx
In reply to Matthew CoverWhen writing packets to a descriptor associated with a combined queue, the packets should end up on that queue.
Before this change all packets written to any descriptor associated with a tap interface end up on rx-0, even when the descriptor is associated with a different queue.
The rx traffic can be generated by either of the following.
1. a simple tap program which spins up multiple queues and writes packets to each of the file descriptors
2. tx from a qemu vm with a tap multiqueue netdev
The queue for rx traffic can be observed by either of the following (done on the hypervisor in the qemu case).
1. a simple netmap program which opens and reads from per-queue descriptors
2. configuring RPS and doing per-cpu captures with rxtxcpu
Alternatively, if you printk() the return value of skb_get_rx_queue() just before each instance of netif_receive_skb() in tun.c, you will get 65535 for every skb.
Calling skb_record_rx_queue() to set the rx queue to the queue_index fixes the association between descriptor and rx queue.
--- drivers/net/tun.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/net/tun.c +6 -1
@@ -1536,6 +1536,7 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,if (!rx_batched || (!more &&skb_queue_empty (queue))) {local_bh_disable (); +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb);local_bh_enable ();return ;
@@ -1555,8 +1556,11 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,struct sk_buff *nskb;local_bh_disable (); -while ((nskb =__skb_dequeue (&process_queue))) +while ((nskb =__skb_dequeue (&process_queue))) { +skb_record_rx_queue (nskb, tfile->queue_index);netif_receive_skb (nskb); + } +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb);local_bh_enable (); }
@@ -2452,6 +2456,7 @@ static int tun_xdp_one(struct tun_struct *tun, !tfile->detached) rxhash =__skb_get_hash_symmetric (skb); +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb); stats =get_cpu_ptr (tun->pcpu_stats);
Signature · 1 line
2.15.2 (Apple Git-101.1)
Quoted 56 lines · Matthew Cover
When writing packets to a descriptor associated with a combined queue, the packets should end up on that queue.
Before this change all packets written to any descriptor associated with a tap interface end up on rx-0, even when the descriptor is associated with a different queue.
The rx traffic can be generated by either of the following.
1. a simple tap program which spins up multiple queues and writes packets to each of the file descriptors
2. tx from a qemu vm with a tap multiqueue netdevThe queue for rx traffic can be observed by either of the following (done on the hypervisor in the qemu case).
1. a simple netmap program which opens and reads from per-queue descriptors
2. configuring RPS and doing per-cpu captures with rxtxcpuAlternatively, if you
printk()the return value ofskb_get_rx_queue()just before each instance ofnetif_receive_skb()intun.c, you will get 65535 for every skb.Calling
skb_record_rx_queue()to set the rx queue to thequeue_indexfixes the association between descriptor and rx queue.Signed-off-by Matthew Cover <[email protected]>--- drivers/net/tun.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)drivers/net/tun.c +5
-1@@ -1536,6 +1536,7 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,if (!rx_batched || (!more &&skb_queue_empty (queue))) {local_bh_disable (); +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb);local_bh_enable ();return ;@@ -1555,8 +1556,11 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,struct sk_buff *nskb;local_bh_disable (); -while ((nskb =__skb_dequeue (&process_queue))) +while ((nskb =__skb_dequeue (&process_queue))) { +skb_record_rx_queue (nskb, tfile->queue_index);netif_receive_skb (nskb); + } +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb);local_bh_enable (); }
drivers/net/tun.c static int tun_xdp_one(struct tun_struct *tun, · line 2462
Quoted 7 lines
drivers/net/tun.c +1
-0@@ -2452,6 +2456,7 @@ static int tun_xdp_one(struct tun_struct *tun, !tfile->detached) rxhash =__skb_get_hash_symmetric (skb); +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb); stats =get_cpu_ptr (tun->pcpu_stats);
Quoted 18 lines · Matthew Cover
When writing packets to a descriptor associated with a combined queue, the packets should end up on that queue.
Before this change all packets written to any descriptor associated with a tap interface end up on rx-0, even when the descriptor is associated with a different queue.
The rx traffic can be generated by either of the following.
1. a simple tap program which spins up multiple queues and writes packets to each of the file descriptors
2. tx from a qemu vm with a tap multiqueue netdevThe queue for rx traffic can be observed by either of the following (done on the hypervisor in the qemu case).
1. a simple netmap program which opens and reads from per-queue descriptors
2. configuring RPS and doing per-cpu captures with rxtxcpu
Quoted 8 lines
Alternatively, if you
printk()the return value ofskb_get_rx_queue()just before each instance ofnetif_receive_skb()intun.c, you will get 65535 for every skb.Calling
skb_record_rx_queue()to set the rx queue to thequeue_indexfixes the association between descriptor and rx queue.Signed-off-by Matthew Cover <[email protected]>
stable material?
Quoted 39 lines
--- drivers/net/tun.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)drivers/net/tun.c +6
-1@@ -1536,6 +1536,7 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,if (!rx_batched || (!more &&skb_queue_empty (queue))) {local_bh_disable (); +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb);local_bh_enable ();return ;@@ -1555,8 +1556,11 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,struct sk_buff *nskb;local_bh_disable (); -while ((nskb =__skb_dequeue (&process_queue))) +while ((nskb =__skb_dequeue (&process_queue))) { +skb_record_rx_queue (nskb, tfile->queue_index);netif_receive_skb (nskb); + } +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb);local_bh_enable (); }@@ -2452,6 +2456,7 @@ static int tun_xdp_one(struct tun_struct *tun, !tfile->detached) rxhash =__skb_get_hash_symmetric (skb); +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb); stats =get_cpu_ptr (tun->pcpu_stats);Signature · 1 line
2.15.2 (Apple Git-101.1)
Quoted 24 lines · Michael S. Tsirkin
Quoted 22 lines · Matthew Cover
When writing packets to a descriptor associated with a combined queue, the packets should end up on that queue.
Before this change all packets written to any descriptor associated with a tap interface end up on rx-0, even when the descriptor is associated with a different queue.
The rx traffic can be generated by either of the following.
1. a simple tap program which spins up multiple queues and writes packets to each of the file descriptors
2. tx from a qemu vm with a tap multiqueue netdevThe queue for rx traffic can be observed by either of the following (done on the hypervisor in the qemu case).
1. a simple netmap program which opens and reads from per-queue descriptors
2. configuring RPS and doing per-cpu captures with rxtxcpuAlternatively, if you
printk()the return value ofskb_get_rx_queue()just before each instance ofnetif_receive_skb()intun.c, you will get 65535 for every skb.
Quoted 9 lines
Quoted 4 lines
Calling
skb_record_rx_queue()to set the rx queue to thequeue_indexfixes the association between descriptor and rx queue.Signed-off-by Matthew Cover <[email protected]>stable material?
Yes, I believe so.
The documentation below I think justifies classifying this as a fix.
https://github.com/torvalds/linux/blob/v4.19/Documentation/networking/tuntap.txt#L111
Quoted 39 lines
Quoted 39 lines
--- drivers/net/tun.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)drivers/net/tun.c +6
-2@@ -1536,6 +1536,7 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,if (!rx_batched || (!more &&skb_queue_empty (queue))) {local_bh_disable (); +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb);local_bh_enable ();return ;@@ -1555,8 +1556,11 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,struct sk_buff *nskb;local_bh_disable (); -while ((nskb =__skb_dequeue (&process_queue))) +while ((nskb =__skb_dequeue (&process_queue))) { +skb_record_rx_queue (nskb, tfile->queue_index);netif_receive_skb (nskb); + } +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb);local_bh_enable (); }@@ -2452,6 +2456,7 @@ static int tun_xdp_one(struct tun_struct *tun, !tfile->detached) rxhash =__skb_get_hash_symmetric (skb); +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb); stats =get_cpu_ptr (tun->pcpu_stats); --2.15.2 (Apple Git-101.1)
David Miller
maintainerFrom: Matthew Cover <[email protected]>
Date: Thu, 15 Nov 2018 21:10:16 -0700
Quoted 18 lines
When writing packets to a descriptor associated with a combined queue, the packets should end up on that queue.
Before this change all packets written to any descriptor associated with a tap interface end up on rx-0, even when the descriptor is associated with a different queue.
The rx traffic can be generated by either of the following.
1. a simple tap program which spins up multiple queues and writes packets to each of the file descriptors
2. tx from a qemu vm with a tap multiqueue netdevThe queue for rx traffic can be observed by either of the following (done on the hypervisor in the qemu case).
1. a simple netmap program which opens and reads from per-queue descriptors
2. configuring RPS and doing per-cpu captures with rxtxcpu
Quoted 8 lines
Alternatively, if you
printk()the return value ofskb_get_rx_queue()just before each instance ofnetif_receive_skb()intun.c, you will get 65535 for every skb.Calling
skb_record_rx_queue()to set the rx queue to thequeue_indexfixes the association between descriptor and rx queue.Signed-off-by Matthew Cover <[email protected]>
As noted by Jason, tun_xdp_one() needs this fix too.
Can you please add that to your patch and resubmit?
Thanks.
David Miller
maintainerFrom: Matthew Cover <[email protected]>
Date: Fri, 16 Nov 2018 00:00:15 -0700
Quoted 18 lines
When writing packets to a descriptor associated with a combined queue, the packets should end up on that queue.
Before this change all packets written to any descriptor associated with a tap interface end up on rx-0, even when the descriptor is associated with a different queue.
The rx traffic can be generated by either of the following.
1. a simple tap program which spins up multiple queues and writes packets to each of the file descriptors
2. tx from a qemu vm with a tap multiqueue netdevThe queue for rx traffic can be observed by either of the following (done on the hypervisor in the qemu case).
1. a simple netmap program which opens and reads from per-queue descriptors
2. configuring RPS and doing per-cpu captures with rxtxcpu
Quoted 8 lines
Alternatively, if you
printk()the return value ofskb_get_rx_queue()just before each instance ofnetif_receive_skb()intun.c, you will get 65535 for every skb.Calling
skb_record_rx_queue()to set the rx queue to thequeue_indexfixes the association between descriptor and rx queue.Signed-off-by Matthew Cover <[email protected]>
If this is intended to target -stable as well, which some responses seem to indicate, you need to respin and submit this against 'net'.
Thanks.
PATCH 1/1 tuntap: fix multiqueue rx
When writing packets to a descriptor associated with a combined queue, the packets should end up on that queue.
Before this change all packets written to any descriptor associated with a tap interface end up on rx-0, even when the descriptor is associated with a different queue.
The rx traffic can be generated by either of the following.
1. a simple tap program which spins up multiple queues and writes packets to each of the file descriptors
2. tx from a qemu vm with a tap multiqueue netdev
The queue for rx traffic can be observed by either of the following (done on the hypervisor in the qemu case).
1. a simple netmap program which opens and reads from per-queue descriptors
2. configuring RPS and doing per-cpu captures with rxtxcpu
Alternatively, if you printk() the return value of skb_get_rx_queue() just before each instance of netif_receive_skb() in tun.c, you will get 65535 for every skb.
Calling skb_record_rx_queue() to set the rx queue to the queue_index fixes the association between descriptor and rx queue.
--- drivers/net/tun.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/net/tun.c +6 -1
@@ -1536,6 +1536,7 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,if (!rx_batched || (!more &&skb_queue_empty (queue))) {local_bh_disable (); +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb);local_bh_enable ();return ;
@@ -1555,8 +1556,11 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,struct sk_buff *nskb;local_bh_disable (); -while ((nskb =__skb_dequeue (&process_queue))) +while ((nskb =__skb_dequeue (&process_queue))) { +skb_record_rx_queue (nskb, tfile->queue_index);netif_receive_skb (nskb); + } +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb);local_bh_enable (); }
@@ -2451,6 +2455,7 @@ static int tun_xdp_one(struct tun_struct *tun,if (!rcu_dereference (tun->steering_prog)) rxhash =__skb_get_hash_symmetric (skb); +skb_record_rx_queue (skb, tfile->queue_index);netif_receive_skb (skb); stats =get_cpu_ptr (tun->pcpu_stats);
Signature · 1 line
2.15.2 (Apple Git-101.1)
David Miller
maintainerRe: [PATCH net] tuntap: fix multiqueue rx
From: Matthew Cover <[email protected]>
Date: Sun, 18 Nov 2018 00:46:00 -0700
Quoted 18 lines
When writing packets to a descriptor associated with a combined queue, the packets should end up on that queue.
Before this change all packets written to any descriptor associated with a tap interface end up on rx-0, even when the descriptor is associated with a different queue.
The rx traffic can be generated by either of the following.
1. a simple tap program which spins up multiple queues and writes packets to each of the file descriptors
2. tx from a qemu vm with a tap multiqueue netdevThe queue for rx traffic can be observed by either of the following (done on the hypervisor in the qemu case).
1. a simple netmap program which opens and reads from per-queue descriptors
2. configuring RPS and doing per-cpu captures with rxtxcpu
Quoted 8 lines
Alternatively, if you
printk()the return value ofskb_get_rx_queue()just before each instance ofnetif_receive_skb()intun.c, you will get 65535 for every skb.Calling
skb_record_rx_queue()to set the rx queue to thequeue_indexfixes the association between descriptor and rx queue.Signed-off-by Matthew Cover <[email protected]>
Applied and queued up for -stable, thanks.