Skip to content

Commit 916f6ef

Browse files
Florian Westphalummakynes
authored andcommitted
netfilter: never get/set skb->tstamp
setting net.netfilter.nf_conntrack_timestamp=1 breaks xmit with fq scheduler. skb->tstamp might be "refreshed" using ktime_get_real(), but fq expects CLOCK_MONOTONIC. This patch removes all places in netfilter that check/set skb->tstamp: 1. To fix the bogus "start" time seen with conntrack timestamping for outgoing packets, never use skb->tstamp and always use current time. 2. In nfqueue and nflog, only use skb->tstamp for incoming packets, as determined by current hook (prerouting, input, forward). 3. xt_time has to use system clock as well rather than skb->tstamp. We could still use skb->tstamp for prerouting/input/foward, but I see no advantage to make this conditional. Fixes: fb420d5 ("tcp/fq: move back to CLOCK_MONOTONIC") Cc: Eric Dumazet <[email protected]> Reported-by: Michal Soltys <[email protected]> Signed-off-by: Florian Westphal <[email protected]> Acked-by: Eric Dumazet <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]>
1 parent 7caa56f commit 916f6ef

File tree

4 files changed

+18
-16
lines changed

4 files changed

+18
-16
lines changed

net/netfilter/nf_conntrack_core.c

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,12 +1017,9 @@ __nf_conntrack_confirm(struct sk_buff *skb)
10171017

10181018
/* set conntrack timestamp, if enabled. */
10191019
tstamp = nf_conn_tstamp_find(ct);
1020-
if (tstamp) {
1021-
if (skb->tstamp == 0)
1022-
__net_timestamp(skb);
1020+
if (tstamp)
1021+
tstamp->start = ktime_get_real_ns();
10231022

1024-
tstamp->start = ktime_to_ns(skb->tstamp);
1025-
}
10261023
/* Since the lookup is lockless, hash insertion must be done after
10271024
* starting the timer and setting the CONFIRMED bit. The RCU barriers
10281025
* guarantee that no other CPU can find the conntrack before the above

net/netfilter/nfnetlink_log.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ __build_packet_message(struct nfnl_log_net *log,
540540
goto nla_put_failure;
541541
}
542542

543-
if (skb->tstamp) {
543+
if (hooknum <= NF_INET_FORWARD && skb->tstamp) {
544544
struct nfulnl_msg_packet_timestamp ts;
545545
struct timespec64 kts = ktime_to_timespec64(skb->tstamp);
546546
ts.sec = cpu_to_be64(kts.tv_sec);

net/netfilter/nfnetlink_queue.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
582582
if (nfqnl_put_bridge(entry, skb) < 0)
583583
goto nla_put_failure;
584584

585-
if (entskb->tstamp) {
585+
if (entry->state.hook <= NF_INET_FORWARD && entskb->tstamp) {
586586
struct nfqnl_msg_packet_timestamp ts;
587587
struct timespec64 kts = ktime_to_timespec64(entskb->tstamp);
588588

net/netfilter/xt_time.c

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -163,19 +163,24 @@ time_mt(const struct sk_buff *skb, struct xt_action_param *par)
163163
s64 stamp;
164164

165165
/*
166-
* We cannot use get_seconds() instead of __net_timestamp() here.
166+
* We need real time here, but we can neither use skb->tstamp
167+
* nor __net_timestamp().
168+
*
169+
* skb->tstamp and skb->skb_mstamp_ns overlap, however, they
170+
* use different clock types (real vs monotonic).
171+
*
167172
* Suppose you have two rules:
168-
* 1. match before 13:00
169-
* 2. match after 13:00
173+
* 1. match before 13:00
174+
* 2. match after 13:00
175+
*
170176
* If you match against processing time (get_seconds) it
171177
* may happen that the same packet matches both rules if
172-
* it arrived at the right moment before 13:00.
178+
* it arrived at the right moment before 13:00, so it would be
179+
* better to check skb->tstamp and set it via __net_timestamp()
180+
* if needed. This however breaks outgoing packets tx timestamp,
181+
* and causes them to get delayed forever by fq packet scheduler.
173182
*/
174-
if (skb->tstamp == 0)
175-
__net_timestamp((struct sk_buff *)skb);
176-
177-
stamp = ktime_to_ns(skb->tstamp);
178-
stamp = div_s64(stamp, NSEC_PER_SEC);
183+
stamp = get_seconds();
179184

180185
if (info->flags & XT_TIME_LOCAL_TZ)
181186
/* Adjust for local timezone */

0 commit comments

Comments
 (0)