sunvnet: set queue mapping when doing packet copies

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. It also transfers the destructor, truesize, and the owning socket for flow control (code adapted from skb_segment).

Signed-off-by David L Stevens <[email protected]>

Acked-by Sowmini Varadhan <[email protected]>

--
Changes since v1:
move truesize, destructor and socket per Eric Dumazet <[email protected]>

---
 drivers/net/ethernet/sun/sunvnet.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

drivers/net/ethernet/sun/sunvnet.c +0 -0

index 2b719cc..0a1d3eb 100644

drivers/net/ethernet/sun/sunvnet.c +4 -0

+			swap(nskb->truesize, skb->truesize);
+			swap(nskb->destructor, skb->destructor);
+			swap(nskb->sk, skb->sk);
+		}
 		dev_kfree_skb(skb);
 		skb = nskb;
 	}

Sorry, but you should remove this test.

(TCP uses another destructor, not sock_wfree())

All sent packets will support these swap() operations, regardless of destructor.

In general the destructor should match the allocator, right? So it bothers me here that we'd be replacing it in an skb allocated with alloc_and_align_skb(), with an an arbitrary destructor from an skb NOT allocated with alloc_and_align_skb(). If it has a different destructor that does special handling related to the allocator, it is the original skb, not the new one, that needs the old destructor. This TCP accounting has less to do with the buffer destructor than with the freeing of the contents of the buffer, but that isn't necessarily true for all destructors.

Checking for a known, specific destructor is less troubling, so I don't want to remove the test entirely.

Since the concern here is specifically TCP flow control, do you think it's sufficient to substitute tcp_wfree for the sock_wfree here?

I guess you can extend the test to sock_wfree and tcp_wfree, but :

You have to EXPORT_SYMBOL(tcp_wfree)

Add an #ifdef CONFIG_INET (take a look at skb_orphan_partial())

drivers/net/ethernet/sun/sunvnet.c static inline struct sk_buff *vnet_skb_shape(struct sk_buff *skb, int ncookies) · line 1131

Quoted 10 lines

drivers/net/ethernet/sun/sunvnet.c +6 -0

@@ -1123,6 +1123,16 @@ 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;
+		/* Following permits correct back-pressure, for protocols
+		 * using skb_set_owner_w().
+		 * Idea is to transfer ownership from skb to nskb.
+		 */
+		if (skb->destructor == sock_wfree) {

Sorry, but you should remove this test.

(TCP uses another destructor, not sock_wfree())

All sent packets will support these swap() operations, regardless of destructor.

Then we need to fix skb_segment() too.