| Message ID | 20240314192404.1867189-1-quic_abchauha@quicinc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 35c3e27917568192927c785fc380f139255468b4 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v3] Revert "net: Re-use and set mono_delivery_time bit for userspace tstamp packets" | expand |
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>
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>
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!
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;
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(-)