sunvnet: set queue mapping when doing packet copies
Subsystems (2)
A message that is not in the archive (sent off-list or not yet received): [email protected]
Sowmini Varadhan
Acked-byQuoted 5 lines · David L Stevens
This patch fixes a bug where
vnet_skb_shape()didn't set the already-selected queue mapping when a packet copy was required. This results in using the wrong queue index for stops/starts, hung tx queues and watchdog timeouts under heavy load.
PATCH 1/1 sunvnet: set queue mapping when doing packet copies
In reply to a message not in the archive[resending this since it never appeared on netdev yesterday]
This patch fixes a bug where vnet_skb_shape() didn't set the already-selected
queue mapping when a packet copy was required. This results in using the wrong queue index for stops/starts, hung tx queues and watchdog timeouts under heavy load.
--- drivers/net/ethernet/sun/sunvnet.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/net/ethernet/sun/sunvnet.c +1 -0
@@ -1123,6 +1123,7 @@ static inline struct sk_buff *vnet_skb_shape(struct sk_buff *skb, int ncookies)skb_shinfo (nskb)->gso_size =skb_shinfo (skb)->gso_size;skb_shinfo (nskb)->gso_type =skb_shinfo (skb)->gso_type; } + nskb->queue_mapping = skb->queue_mapping;dev_kfree_skb (skb); skb = nskb; }
Quoted 18 lines · David L Stevens
[resending this since it never appeared on netdev yesterday]
This patch fixes a bug where
vnet_skb_shape()didn't set the already-selected
queue mapping when a packet copy was required. This results in using the wrong queue index for stops/starts, hung tx queues and watchdog timeouts under heavy load.Signed-off-by David L Stevens <[email protected]>Acked-by Sowmini Varadhan <[email protected]>--- drivers/net/ethernet/sun/sunvnet.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)drivers/net/ethernet/sun/sunvnet.c +0
-0
drivers/net/ethernet/sun/sunvnet.c static inline struct sk_buff *vnet_skb_shape(struct sk_buff *skb, int ncookies) · line 1129
Quoted 7 lines
drivers/net/ethernet/sun/sunvnet.c +1
-0@@ -1123,6 +1123,7 @@ static inline struct sk_buff *vnet_skb_shape(struct sk_buff *skb, int ncookies)skb_shinfo (nskb)->gso_size =skb_shinfo (skb)->gso_size;skb_shinfo (nskb)->gso_type =skb_shinfo (skb)->gso_type; } + nskb->queue_mapping = skb->queue_mapping;dev_kfree_skb (skb); skb = nskb; }
Note that vnet_skb_shape() also drops the skb -> sk assocation.
-> That removes flow control, a single socket can fill TX ring buffer. TCP Small queue is also disabled.
I would try add before the dev_kfree_skb(skb) :
swap(nskb->sk, skb->sk);swap(nskb->truesize, skb->truesize);
Quoted 2 lines · Eric Dumazet
swap(nskb->sk, skb->sk);swap(nskb->truesize, skb->truesize);
and swap(nskb->destructor, skb->destructor);
(check skb_segment() for a similar path)
PATCH 1/1 sunvnet: set queue mapping when doing packet copies
In reply to a message not in the archiveThis patch fixes a bug where vnet_skb_shape() didn't set the already-selected queue mapping when a packet copy was required. This results in using the wrong queue index for stops/starts, hung tx queues and watchdog timeouts under heavy load.
--
I'm resubmitting this in its original form. Since skb_segment() doesn't do TCP flow control either, due to an incorrect destructor check, and those issues raised by Eric are a matter of fairness, while this missing queue_mapping assignment results in crashes, I'd like to get this piece fixed separately.
--- drivers/net/ethernet/sun/sunvnet.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/net/ethernet/sun/sunvnet.c +1 -0
@@ -1123,6 +1123,7 @@ static inline struct sk_buff *vnet_skb_shape(struct sk_buff *skb, int ncookies)skb_shinfo (nskb)->gso_size =skb_shinfo (skb)->gso_size;skb_shinfo (nskb)->gso_type =skb_shinfo (skb)->gso_type; } + nskb->queue_mapping = skb->queue_mapping;dev_kfree_skb (skb); skb = nskb; }
Signature · 1 line
1.7.1
David Miller
maintainerFrom: David L Stevens <[email protected]>
Date: Fri, 30 Jan 2015 12:29:45 -0500
Quoted 7 lines
This patch fixes a bug where
vnet_skb_shape()didn't set the already-selected queue mapping when a packet copy was required. This results in using the wrong queue index for stops/starts, hung tx queues and watchdog timeouts under heavy load.
Quoted 8 lines
Signed-off-by David L Stevens <[email protected]>Acked-by Sowmini Varadhan <[email protected]>--
I'm resubmitting this in its original form. Sinceskb_segment()doesn't do TCP flow control either, due to an incorrect destructor check, and those issues raised by Eric are a matter of fairness, while this missingqueue_mappingassignment results in crashes, I'd like to get this piece fixed separately.
Fair enough, applied.
Please sort out the callback issues with Eric, thanks.