| Message ID | 20240313235003.1354814-1-quic_abchauha@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v1] Revert "net: Re-use and set mono_delivery_time bit for userspace tstamp packets" | expand |
On Wed, 13 Mar 2024 16:50:03 -0700 Abhishek Chauhan wrote: > Subject: [PATCH net-next v1] Revert "net: Re-use and set mono_delivery_time bit for userspace tstamp packets" > Date: Wed, 13 Mar 2024 16:50:03 -0700 > X-Mailer: git-send-email 2.25.1 > > This reverts commit 30bb896b98fce7d823a96fc02cd69be30384a5cc. > > Signed-off-by: Abhishek Chauhan <quic_abchauha@quicinc.com> > --- > Reason for revert - > https://lore.kernel.org/netdev/bc037db4-58bb-4861-ac31-a361a93841d3@linux.dev/ > 1. Fallback to design proposed in version 1 to have a seperate user_delivery_time > bit > 2. Keep mono_delivery_time as ease > 3. Add user_delivery_time as a new bitfield will be a seperate patch > 4. do not reset the time if either mono_delivery_time or user_delivery_time is set > 5. we will need a user_delivery_time bit and use that to look at sk_clockid Could you please spell this out a bit more in the commit message and add the link as a: Link: https://lore.kernel.org/netdev/bc037db4-58bb-4861-ac31-a361a93841d3@linux.dev/ in the commit message rather than the "throw away notes" ? (feel free to repost asap, without usual netdev's 24h)
On 3/13/2024 5:16 PM, Jakub Kicinski wrote: > On Wed, 13 Mar 2024 16:50:03 -0700 Abhishek Chauhan wrote: >> Subject: [PATCH net-next v1] Revert "net: Re-use and set mono_delivery_time bit for userspace tstamp packets" >> Date: Wed, 13 Mar 2024 16:50:03 -0700 >> X-Mailer: git-send-email 2.25.1 >> >> This reverts commit 30bb896b98fce7d823a96fc02cd69be30384a5cc. >> >> Signed-off-by: Abhishek Chauhan <quic_abchauha@quicinc.com> >> --- >> Reason for revert - >> https://lore.kernel.org/netdev/bc037db4-58bb-4861-ac31-a361a93841d3@linux.dev/ > >> 1. Fallback to design proposed in version 1 to have a seperate user_delivery_time >> bit >> 2. Keep mono_delivery_time as ease >> 3. Add user_delivery_time as a new bitfield will be a seperate patch >> 4. do not reset the time if either mono_delivery_time or user_delivery_time is set >> 5. we will need a user_delivery_time bit and use that to look at sk_clockid > > Could you please spell this out a bit more in the commit message and > add the link as a: > Certainly i can do that and explain more in the Revert commit message and point a link as mentioned below as why we need to fall back to design 1 proposal for adding an extra bit. > Link: https://lore.kernel.org/netdev/bc037db4-58bb-4861-ac31-a361a93841d3@linux.dev/ > > in the commit message rather than the "throw away notes" ? > > (feel free to repost asap, without usual netdev's 24h)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 4726298d4ed4..2dde34c29203 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -817,9 +817,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 ff1df64c5697..5b5a0adb927f 100644 --- a/net/ipv4/ip_output.c +++ b/net/ipv4/ip_output.c @@ -1455,7 +1455,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 c4c29fc5b73f..aea89326c697 100644 --- a/net/ipv4/raw.c +++ b/net/ipv4/raw.c @@ -353,7 +353,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 2fc1d03dc07d..a722a43dd668 100644 --- a/net/ipv6/ip6_output.c +++ b/net/ipv6/ip6_output.c @@ -1922,7 +1922,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 13f54f8eea35..03dbb874c363 100644 --- a/net/ipv6/raw.c +++ b/net/ipv6/raw.c @@ -616,7 +616,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 0db31ca4982d..c9bbc2686690 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 30bb896b98fce7d823a96fc02cd69be30384a5cc. Signed-off-by: Abhishek Chauhan <quic_abchauha@quicinc.com> --- Reason for revert - https://lore.kernel.org/netdev/bc037db4-58bb-4861-ac31-a361a93841d3@linux.dev/ 1. Fallback to design proposed in version 1 to have a seperate user_delivery_time bit 2. Keep mono_delivery_time as ease 3. Add user_delivery_time as a new bitfield will be a seperate patch 4. do not reset the time if either mono_delivery_time or user_delivery_time is set 5. we will need a user_delivery_time bit and use that to look at sk_clockid 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(-)