2018-08-05 10:26:45

by Yafang Shao

Subject: [PATCH net-next v2] tcp: propagate gso_segs to the new skb built in tcp collapse

The gso_segs of the new built SKB in tcp collapse is inited to 0,
that makes us hard to know the accurate segments number of this new SKB.
We'd better propagate the gso_segs of the collapsed SKB to the new built
one, so when this SKB is dropped (for example when doing tcp prune) the
sk_drops will be added to the correct value.

If the collapsed SKB is fully copied to the new built one, we just add its
gso_segs to the new SKB.
While if the collapsed SKB is partially copied to the new built SKB,
we have to caculate how many segments are copied.
Furthemore, we have to reset the gso_segs of this SKB if is is partially
copied, so in the next round when the left segments are copied it could
propagate the correct value.

Signed-off-by: Yafang Shao <[email protected]>
---
net/ipv4/tcp_input.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 715d541..72d4fc8 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4915,12 +4915,23 @@ void tcp_rbtree_insert(struct rb_root *root, struct sk_buff *skb)

/* Copy data, releasing collapsed skbs. */
while (copy > 0) {
- int offset = start - TCP_SKB_CB(skb)->seq;
int size = TCP_SKB_CB(skb)->end_seq - start;
+ int offset = start - TCP_SKB_CB(skb)->seq;

BUG_ON(offset < 0);
if (size > 0) {
size = min(copy, size);
+ if (copy >= size) {
+ skb_shinfo(nskb)->gso_segs +=
+ max_t(u16, 1, skb_shinfo(skb)->gso_segs);
+ } else {
+ skb_shinfo(nskb)->gso_segs +=
+ DIV_ROUND_UP(copy, skb_shinfo(skb)->gso_size);
+ skb_shinfo(skb)->gso_segs =
+ DIV_ROUND_UP(size - copy, skb_shinfo(skb)->gso_size);
+ size = copy;
+ }
+
if (skb_copy_bits(skb, offset, skb_put(nskb, size), size))
BUG();
TCP_SKB_CB(nskb)->end_seq += size;
--
1.8.3.1

2018-08-07 20:10:21

by David Miller

Subject: Re: [PATCH net-next v2] tcp: propagate gso_segs to the new skb built in tcp collapse

From: Yafang Shao <[email protected]>
Date: Sun, 5 Aug 2018 18:25:07 +0800

> The gso_segs of the new built SKB in tcp collapse is inited to 0,
> that makes us hard to know the accurate segments number of this new SKB.
> We'd better propagate the gso_segs of the collapsed SKB to the new built
> one, so when this SKB is dropped (for example when doing tcp prune) the
> sk_drops will be added to the correct value.
>
> If the collapsed SKB is fully copied to the new built one, we just add its
> gso_segs to the new SKB.
> While if the collapsed SKB is partially copied to the new built SKB,
> we have to caculate how many segments are copied.
> Furthemore, we have to reset the gso_segs of this SKB if is is partially
> copied, so in the next round when the left segments are copied it could
> propagate the correct value.
>
> Signed-off-by: Yafang Shao <[email protected]>

Eric, please review.

If partial copies are common, I wonder how much sense it makes to try to
avoid the DIV_ROUND_UP() operations.