* [PATCH net-next v3] Revert "net: Re-use and set mono_delivery_time bit for userspace tstamp packets"
@ 2024-03-14 19:24 Abhishek Chauhan
2024-03-14 21:14 ` Daniel Borkmann
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Abhishek Chauhan @ 2024-03-14 19:24 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev, linux-kernel, Andrew Halaney, Willem de Bruijn,
Martin KaFai Lau, Martin KaFai Lau, Daniel Borkmann
Cc: kernel
This reverts commit 885c36e59f46375c138de18ff1692f18eff67b7f.
The patch currently broke the bpf selftest test_tc_dtime because
uapi field __sk_buff->tstamp_type depends on skb->mono_delivery_time which
does not necessarily mean mono with the original fix as the bit was re-used
for userspace timestamp as well to avoid tstamp reset in the forwarding
path. To solve this we need to keep mono_delivery_time as is and
introduce another bit called user_delivery_time and fall back to the
initial proposal of setting the user_delivery_time bit based on
sk_clockid set from userspace.
Fixes: 885c36e59f46 ("net: Re-use and set mono_delivery_time bit for userspace tstamp packets")
Link: https://lore.kernel.org/netdev/bc037db4-58bb-4861-ac31-a361a93841d3@linux.dev/
Signed-off-by: Abhishek Chauhan <quic_abchauha@quicinc.com>
---
Changes since v2
- Fixed minor comments given by Willem
- Added Fixes tag as mentioned by Jakub
- Corrected the SHA
Changes since v1
- Took care of Jakub's comment to explain more about the
revert commit
- Added Link to the discussion of the problem found in the
original commit.
include/linux/skbuff.h | 6 +++---
net/ipv4/ip_output.c | 1 -
net/ipv4/raw.c | 1 -
net/ipv6/ip6_output.c | 2 +-
net/ipv6/raw.c | 2 +-
net/packet/af_packet.c | 4 +---
6 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 3023bc2be6a1..7d56ce195120 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -822,9 +822,9 @@ typedef unsigned char *sk_buff_data_t;
* @decrypted: Decrypted SKB
* @slow_gro: state present at GRO time, slower prepare step required
* @mono_delivery_time: When set, skb->tstamp has the
- * delivery_time in mono clock base (i.e., EDT) or a clock base chosen
- * by SO_TXTIME. If zero, skb->tstamp has the (rcv) timestamp at
- * ingress.
+ * delivery_time in mono clock base (i.e. EDT). Otherwise, the
+ * skb->tstamp has the (rcv) timestamp at ingress and
+ * delivery_time at egress.
* @napi_id: id of the NAPI struct this skb came from
* @sender_cpu: (aka @napi_id) source CPU in XPS
* @alloc_cpu: CPU which did the skb allocation.
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 33f93dc730a3..1fe794967211 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1458,7 +1458,6 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
skb->priority = (cork->tos != -1) ? cork->priority: READ_ONCE(sk->sk_priority);
skb->mark = cork->mark;
skb->tstamp = cork->transmit_time;
- skb->mono_delivery_time = !!skb->tstamp;
/*
* Steal rt from cork.dst to avoid a pair of atomic_inc/atomic_dec
* on dst refcount
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 42ac434cfcfa..12b3740393ba 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -360,7 +360,6 @@ static int raw_send_hdrinc(struct sock *sk, struct flowi4 *fl4,
skb->priority = READ_ONCE(sk->sk_priority);
skb->mark = sockc->mark;
skb->tstamp = sockc->transmit_time;
- skb->mono_delivery_time = !!skb->tstamp;
skb_dst_set(skb, &rt->dst);
*rtp = NULL;
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 02eeca5492cd..b9dd3a66e423 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1925,7 +1925,7 @@ struct sk_buff *__ip6_make_skb(struct sock *sk,
skb->priority = READ_ONCE(sk->sk_priority);
skb->mark = cork->base.mark;
skb->tstamp = cork->base.transmit_time;
- skb->mono_delivery_time = !!skb->tstamp;
+
ip6_cork_steal_dst(skb, cork);
IP6_INC_STATS(net, rt->rt6i_idev, IPSTATS_MIB_OUTREQUESTS);
if (proto == IPPROTO_ICMPV6) {
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index ca49e6617afa..0d896ca7b589 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -622,7 +622,7 @@ static int rawv6_send_hdrinc(struct sock *sk, struct msghdr *msg, int length,
skb->priority = READ_ONCE(sk->sk_priority);
skb->mark = sockc->mark;
skb->tstamp = sockc->transmit_time;
- skb->mono_delivery_time = !!skb->tstamp;
+
skb_put(skb, length);
skb_reset_network_header(skb);
iph = ipv6_hdr(skb);
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 61270826b9ac..4c4fbdd2f96f 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2057,7 +2057,7 @@ static int packet_sendmsg_spkt(struct socket *sock, struct msghdr *msg,
skb->priority = READ_ONCE(sk->sk_priority);
skb->mark = READ_ONCE(sk->sk_mark);
skb->tstamp = sockc.transmit_time;
- skb->mono_delivery_time = !!skb->tstamp;
+
skb_setup_tx_timestamp(skb, sockc.tsflags);
if (unlikely(extra_len == 4))
@@ -2586,7 +2586,6 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
skb->priority = READ_ONCE(po->sk.sk_priority);
skb->mark = READ_ONCE(po->sk.sk_mark);
skb->tstamp = sockc->transmit_time;
- skb->mono_delivery_time = !!skb->tstamp;
skb_setup_tx_timestamp(skb, sockc->tsflags);
skb_zcopy_set_nouarg(skb, ph.raw);
@@ -3065,7 +3064,6 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
skb->priority = READ_ONCE(sk->sk_priority);
skb->mark = sockc.mark;
skb->tstamp = sockc.transmit_time;
- skb->mono_delivery_time = !!skb->tstamp;
if (unlikely(extra_len == 4))
skb->no_fcs = 1;
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next v3] Revert "net: Re-use and set mono_delivery_time bit for userspace tstamp packets"
2024-03-14 19:24 [PATCH net-next v3] Revert "net: Re-use and set mono_delivery_time bit for userspace tstamp packets" Abhishek Chauhan
@ 2024-03-14 21:14 ` Daniel Borkmann
2024-03-14 21:59 ` Martin KaFai Lau
2024-03-18 12:40 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: Daniel Borkmann @ 2024-03-14 21:14 UTC (permalink / raw)
To: Abhishek Chauhan, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, linux-kernel, Andrew Halaney,
Willem de Bruijn, Martin KaFai Lau, Martin KaFai Lau
Cc: kernel, bpf
On 3/14/24 8:24 PM, Abhishek Chauhan wrote:
> This reverts commit 885c36e59f46375c138de18ff1692f18eff67b7f.
>
> The patch currently broke the bpf selftest test_tc_dtime because
> uapi field __sk_buff->tstamp_type depends on skb->mono_delivery_time which
> does not necessarily mean mono with the original fix as the bit was re-used
> for userspace timestamp as well to avoid tstamp reset in the forwarding
> path. To solve this we need to keep mono_delivery_time as is and
> introduce another bit called user_delivery_time and fall back to the
> initial proposal of setting the user_delivery_time bit based on
> sk_clockid set from userspace.
>
> Fixes: 885c36e59f46 ("net: Re-use and set mono_delivery_time bit for userspace tstamp packets")
> Link: https://lore.kernel.org/netdev/bc037db4-58bb-4861-ac31-a361a93841d3@linux.dev/
> Signed-off-by: Abhishek Chauhan <quic_abchauha@quicinc.com>
[ Side note: target tree for this fix is net not net-next, the follow-up
work would be net-next then. ]
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next v3] Revert "net: Re-use and set mono_delivery_time bit for userspace tstamp packets"
2024-03-14 19:24 [PATCH net-next v3] Revert "net: Re-use and set mono_delivery_time bit for userspace tstamp packets" Abhishek Chauhan
2024-03-14 21:14 ` Daniel Borkmann
@ 2024-03-14 21:59 ` Martin KaFai Lau
2024-03-18 12:40 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: Martin KaFai Lau @ 2024-03-14 21:59 UTC (permalink / raw)
To: Abhishek Chauhan
Cc: kernel, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, linux-kernel, Andrew Halaney,
Willem de Bruijn, Martin KaFai Lau, Daniel Borkmann
On 3/14/24 12:24 PM, Abhishek Chauhan wrote:
> This reverts commit 885c36e59f46375c138de18ff1692f18eff67b7f.
>
> The patch currently broke the bpf selftest test_tc_dtime because
> uapi field __sk_buff->tstamp_type depends on skb->mono_delivery_time which
> does not necessarily mean mono with the original fix as the bit was re-used
> for userspace timestamp as well to avoid tstamp reset in the forwarding
> path. To solve this we need to keep mono_delivery_time as is and
> introduce another bit called user_delivery_time and fall back to the
> initial proposal of setting the user_delivery_time bit based on
> sk_clockid set from userspace.
>
> Fixes: 885c36e59f46 ("net: Re-use and set mono_delivery_time bit for userspace tstamp packets")
> Link: https://lore.kernel.org/netdev/bc037db4-58bb-4861-ac31-a361a93841d3@linux.dev/
> Signed-off-by: Abhishek Chauhan <quic_abchauha@quicinc.com>
Acked-by: Martin KaFai Lau <martin.lau@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next v3] Revert "net: Re-use and set mono_delivery_time bit for userspace tstamp packets"
2024-03-14 19:24 [PATCH net-next v3] Revert "net: Re-use and set mono_delivery_time bit for userspace tstamp packets" Abhishek Chauhan
2024-03-14 21:14 ` Daniel Borkmann
2024-03-14 21:59 ` Martin KaFai Lau
@ 2024-03-18 12:40 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-03-18 12:40 UTC (permalink / raw)
To: Abhishek Chauhan
Cc: davem, edumazet, kuba, pabeni, netdev, linux-kernel, ahalaney,
willemdebruijn.kernel, martin.lau, martin.lau, daniel, kernel
Hello:
This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:
On Thu, 14 Mar 2024 12:24:04 -0700 you wrote:
> This reverts commit 885c36e59f46375c138de18ff1692f18eff67b7f.
>
> The patch currently broke the bpf selftest test_tc_dtime because
> uapi field __sk_buff->tstamp_type depends on skb->mono_delivery_time which
> does not necessarily mean mono with the original fix as the bit was re-used
> for userspace timestamp as well to avoid tstamp reset in the forwarding
> path. To solve this we need to keep mono_delivery_time as is and
> introduce another bit called user_delivery_time and fall back to the
> initial proposal of setting the user_delivery_time bit based on
> sk_clockid set from userspace.
>
> [...]
Here is the summary with links:
- [net-next,v3] Revert "net: Re-use and set mono_delivery_time bit for userspace tstamp packets"
https://git.kernel.org/netdev/net/c/35c3e2791756
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-03-18 12:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-14 19:24 [PATCH net-next v3] Revert "net: Re-use and set mono_delivery_time bit for userspace tstamp packets" Abhishek Chauhan
2024-03-14 21:14 ` Daniel Borkmann
2024-03-14 21:59 ` Martin KaFai Lau
2024-03-18 12:40 ` patchwork-bot+netdevbpf
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).