* [PATCH net 1/2] net-timestamp: fix missing ACK timestamp
@ 2014-08-12 18:53 Willem de Bruijn
2014-08-12 18:53 ` [PATCH net 2/2] net-timestamp: fix missing tcp fragmentation cases Willem de Bruijn
2014-08-12 22:16 ` [PATCH net 1/2] net-timestamp: fix missing ACK timestamp David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Willem de Bruijn @ 2014-08-12 18:53 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, richardcochran, Willem de Bruijn
ACK timestamps are generated in tcp_clean_rtx_queue. The TSO datapath
can break out early, causing the timestamp code to be skipped. Move
the code up before the break.
Reported-by: David S. Miller <davem@davemloft.net>
Also fix a boundary condition: tp->snd_una is the next unacknowledged
byte and between tests inclusive (a <= b <= c), so generate a an ACK
timestamp if (prior_snd_una <= tskey <= tp->snd_una - 1).
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/ipv4/tcp_input.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index a3d47af..1a8e89f 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3050,10 +3050,15 @@ static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets,
first_ackt.v64 = 0;
while ((skb = tcp_write_queue_head(sk)) && skb != tcp_send_head(sk)) {
+ struct skb_shared_info *shinfo = skb_shinfo(skb);
struct tcp_skb_cb *scb = TCP_SKB_CB(skb);
u8 sacked = scb->sacked;
u32 acked_pcount;
+ if (unlikely(shinfo->tx_flags & SKBTX_ACK_TSTAMP) &&
+ between(shinfo->tskey, prior_snd_una, tp->snd_una - 1))
+ __skb_tstamp_tx(skb, NULL, sk, SCM_TSTAMP_ACK);
+
/* Determine how many packets and what bytes were acked, tso and else */
if (after(scb->end_seq, tp->snd_una)) {
if (tcp_skb_pcount(skb) == 1 ||
@@ -3107,11 +3112,6 @@ static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets,
tp->retrans_stamp = 0;
}
- if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_ACK_TSTAMP) &&
- between(skb_shinfo(skb)->tskey, prior_snd_una,
- tp->snd_una + 1))
- __skb_tstamp_tx(skb, NULL, sk, SCM_TSTAMP_ACK);
-
if (!fully_acked)
break;
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net 2/2] net-timestamp: fix missing tcp fragmentation cases
2014-08-12 18:53 [PATCH net 1/2] net-timestamp: fix missing ACK timestamp Willem de Bruijn
@ 2014-08-12 18:53 ` Willem de Bruijn
2014-08-12 19:00 ` Willem de Bruijn
2014-08-12 22:16 ` [PATCH net 1/2] net-timestamp: fix missing ACK timestamp David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Willem de Bruijn @ 2014-08-12 18:53 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, richardcochran, Willem de Bruijn
Bytestream timestamps are correlated with a single byte in the skbuff,
recorded in skb_shinfo(skb)->tskey. When fragmenting skbuffs, ensure
that the tskey is set for the fragment in which the tskey falls
(seqno <= tskey < end_seqno).
The original implementation did not address fragmentation in
tcp_fragment or tso_fragment. Add code to inspect the sequence numbers
and move both tskey and the relevant tx_flags if necessary.
Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/ipv4/tcp_output.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 8fcfc91..c07064e 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1069,6 +1069,21 @@ static void tcp_adjust_pcount(struct sock *sk, const struct sk_buff *skb, int de
tcp_verify_left_out(tp);
}
+static void tcp_fragment_tstamp(struct sk_buff *skb, struct sk_buff *skb2)
+{
+ struct skb_shared_info *shinfo = skb_shinfo(skb);
+
+ if (unlikely(shinfo->tx_flags & SKBTX_ANY_TSTAMP) &&
+ after(shinfo->tskey, TCP_SKB_CB(skb)->end_seq)) {
+ struct skb_shared_info *shinfo2 = skb_shinfo(skb2);
+ u8 tsflags = shinfo->tx_flags & SKBTX_ANY_TSTAMP;
+
+ shinfo->tx_flags &= ~tsflags;
+ shinfo2->tx_flags |= tsflags;
+ swap(shinfo->tskey, shinfo2->tskey);
+ }
+}
+
/* Function to create two new TCP segments. Shrinks the given segment
* to the specified size and appends a new segment with the rest of the
* packet to the list. This won't be called frequently, I hope.
@@ -1136,6 +1151,7 @@ int tcp_fragment(struct sock *sk, struct sk_buff *skb, u32 len,
*/
TCP_SKB_CB(buff)->when = TCP_SKB_CB(skb)->when;
buff->tstamp = skb->tstamp;
+ tcp_fragment_tstamp(skb, buff);
old_factor = tcp_skb_pcount(skb);
@@ -1652,6 +1668,7 @@ static int tso_fragment(struct sock *sk, struct sk_buff *skb, unsigned int len,
buff->ip_summed = skb->ip_summed = CHECKSUM_PARTIAL;
skb_split(skb, buff, len);
+ tcp_fragment_tstamp(skb, buff);
/* Fix up tso_factor for both original and new SKB. */
tcp_set_skb_tso_segs(sk, skb, mss_now);
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] net-timestamp: fix missing tcp fragmentation cases
2014-08-12 18:53 ` [PATCH net 2/2] net-timestamp: fix missing tcp fragmentation cases Willem de Bruijn
@ 2014-08-12 19:00 ` Willem de Bruijn
2014-08-12 19:08 ` [PATCH net] " Willem de Bruijn
0 siblings, 1 reply; 6+ messages in thread
From: Willem de Bruijn @ 2014-08-12 19:00 UTC (permalink / raw)
To: Network Development
Cc: David Miller, Eric Dumazet, Richard Cochran, Willem de Bruijn
On Tue, Aug 12, 2014 at 11:53 AM, Willem de Bruijn <willemb@google.com> wrote:
> Bytestream timestamps are correlated with a single byte in the skbuff,
> recorded in skb_shinfo(skb)->tskey. When fragmenting skbuffs, ensure
> that the tskey is set for the fragment in which the tskey falls
> (seqno <= tskey < end_seqno).
>
> The original implementation did not address fragmentation in
> tcp_fragment or tso_fragment. Add code to inspect the sequence numbers
> and move both tskey and the relevant tx_flags if necessary.
>
> Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
> net/ipv4/tcp_output.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 8fcfc91..c07064e 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -1069,6 +1069,21 @@ static void tcp_adjust_pcount(struct sock *sk, const struct sk_buff *skb, int de
> tcp_verify_left_out(tp);
> }
>
> +static void tcp_fragment_tstamp(struct sk_buff *skb, struct sk_buff *skb2)
> +{
> + struct skb_shared_info *shinfo = skb_shinfo(skb);
> +
> + if (unlikely(shinfo->tx_flags & SKBTX_ANY_TSTAMP) &&
> + after(shinfo->tskey, TCP_SKB_CB(skb)->end_seq)) {
This still has an off by one error, when tskey == end_seq. I will resubmit.
> + struct skb_shared_info *shinfo2 = skb_shinfo(skb2);
> + u8 tsflags = shinfo->tx_flags & SKBTX_ANY_TSTAMP;
> +
> + shinfo->tx_flags &= ~tsflags;
> + shinfo2->tx_flags |= tsflags;
> + swap(shinfo->tskey, shinfo2->tskey);
> + }
> +}
> +
> /* Function to create two new TCP segments. Shrinks the given segment
> * to the specified size and appends a new segment with the rest of the
> * packet to the list. This won't be called frequently, I hope.
> @@ -1136,6 +1151,7 @@ int tcp_fragment(struct sock *sk, struct sk_buff *skb, u32 len,
> */
> TCP_SKB_CB(buff)->when = TCP_SKB_CB(skb)->when;
> buff->tstamp = skb->tstamp;
> + tcp_fragment_tstamp(skb, buff);
>
> old_factor = tcp_skb_pcount(skb);
>
> @@ -1652,6 +1668,7 @@ static int tso_fragment(struct sock *sk, struct sk_buff *skb, unsigned int len,
>
> buff->ip_summed = skb->ip_summed = CHECKSUM_PARTIAL;
> skb_split(skb, buff, len);
> + tcp_fragment_tstamp(skb, buff);
>
> /* Fix up tso_factor for both original and new SKB. */
> tcp_set_skb_tso_segs(sk, skb, mss_now);
> --
> 2.1.0.rc2.206.gedb03e5
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net] net-timestamp: fix missing tcp fragmentation cases
2014-08-12 19:00 ` Willem de Bruijn
@ 2014-08-12 19:08 ` Willem de Bruijn
2014-08-12 22:16 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Willem de Bruijn @ 2014-08-12 19:08 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, richardcochran, Willem de Bruijn
Bytestream timestamps are correlated with a single byte in the skbuff,
recorded in skb_shinfo(skb)->tskey. When fragmenting skbuffs, ensure
that the tskey is set for the fragment in which the tskey falls
(seqno <= tskey < end_seqno).
The original implementation did not address fragmentation in
tcp_fragment or tso_fragment. Add code to inspect the sequence numbers
and move both tskey and the relevant tx_flags if necessary.
Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/ipv4/tcp_output.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 8fcfc91..ef4a051 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1069,6 +1069,21 @@ static void tcp_adjust_pcount(struct sock *sk, const struct sk_buff *skb, int de
tcp_verify_left_out(tp);
}
+static void tcp_fragment_tstamp(struct sk_buff *skb, struct sk_buff *skb2)
+{
+ struct skb_shared_info *shinfo = skb_shinfo(skb);
+
+ if (unlikely(shinfo->tx_flags & SKBTX_ANY_TSTAMP) &&
+ !before(shinfo->tskey, TCP_SKB_CB(skb2)->seq)) {
+ struct skb_shared_info *shinfo2 = skb_shinfo(skb2);
+ u8 tsflags = shinfo->tx_flags & SKBTX_ANY_TSTAMP;
+
+ shinfo->tx_flags &= ~tsflags;
+ shinfo2->tx_flags |= tsflags;
+ swap(shinfo->tskey, shinfo2->tskey);
+ }
+}
+
/* Function to create two new TCP segments. Shrinks the given segment
* to the specified size and appends a new segment with the rest of the
* packet to the list. This won't be called frequently, I hope.
@@ -1136,6 +1151,7 @@ int tcp_fragment(struct sock *sk, struct sk_buff *skb, u32 len,
*/
TCP_SKB_CB(buff)->when = TCP_SKB_CB(skb)->when;
buff->tstamp = skb->tstamp;
+ tcp_fragment_tstamp(skb, buff);
old_factor = tcp_skb_pcount(skb);
@@ -1652,6 +1668,7 @@ static int tso_fragment(struct sock *sk, struct sk_buff *skb, unsigned int len,
buff->ip_summed = skb->ip_summed = CHECKSUM_PARTIAL;
skb_split(skb, buff, len);
+ tcp_fragment_tstamp(skb, buff);
/* Fix up tso_factor for both original and new SKB. */
tcp_set_skb_tso_segs(sk, skb, mss_now);
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] net-timestamp: fix missing ACK timestamp
2014-08-12 18:53 [PATCH net 1/2] net-timestamp: fix missing ACK timestamp Willem de Bruijn
2014-08-12 18:53 ` [PATCH net 2/2] net-timestamp: fix missing tcp fragmentation cases Willem de Bruijn
@ 2014-08-12 22:16 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2014-08-12 22:16 UTC (permalink / raw)
To: willemb; +Cc: netdev, eric.dumazet, richardcochran
From: Willem de Bruijn <willemb@google.com>
Date: Tue, 12 Aug 2014 14:53:16 -0400
> ACK timestamps are generated in tcp_clean_rtx_queue. The TSO datapath
> can break out early, causing the timestamp code to be skipped. Move
> the code up before the break.
>
> Reported-by: David S. Miller <davem@davemloft.net>
>
> Also fix a boundary condition: tp->snd_una is the next unacknowledged
> byte and between tests inclusive (a <= b <= c), so generate a an ACK
> timestamp if (prior_snd_una <= tskey <= tp->snd_una - 1).
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] net-timestamp: fix missing tcp fragmentation cases
2014-08-12 19:08 ` [PATCH net] " Willem de Bruijn
@ 2014-08-12 22:16 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2014-08-12 22:16 UTC (permalink / raw)
To: willemb; +Cc: netdev, eric.dumazet, richardcochran
From: Willem de Bruijn <willemb@google.com>
Date: Tue, 12 Aug 2014 15:08:12 -0400
> Bytestream timestamps are correlated with a single byte in the skbuff,
> recorded in skb_shinfo(skb)->tskey. When fragmenting skbuffs, ensure
> that the tskey is set for the fragment in which the tskey falls
> (seqno <= tskey < end_seqno).
>
> The original implementation did not address fragmentation in
> tcp_fragment or tso_fragment. Add code to inspect the sequence numbers
> and move both tskey and the relevant tx_flags if necessary.
>
> Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
Also applied, thanks Willem.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-08-12 22:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-12 18:53 [PATCH net 1/2] net-timestamp: fix missing ACK timestamp Willem de Bruijn
2014-08-12 18:53 ` [PATCH net 2/2] net-timestamp: fix missing tcp fragmentation cases Willem de Bruijn
2014-08-12 19:00 ` Willem de Bruijn
2014-08-12 19:08 ` [PATCH net] " Willem de Bruijn
2014-08-12 22:16 ` David Miller
2014-08-12 22:16 ` [PATCH net 1/2] net-timestamp: fix missing ACK timestamp David Miller
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).