Re: [PATCH net-next v3 1/4] net_tstamp: add SCM_TS_OPT_ID to provide OPT_ID in control message

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Vadim Fedorenko wrote:
> On 04/09/2024 21:54, Willem de Bruijn wrote:
> > Vadim Fedorenko wrote:
> >> SOF_TIMESTAMPING_OPT_ID socket option flag gives a way to correlate TX
> >> timestamps and packets sent via socket. Unfortunately, there is no way
> >> to reliably predict socket timestamp ID value in case of error returned
> >> by sendmsg. For UDP sockets it's impossible because of lockless
> >> nature of UDP transmit, several threads may send packets in parallel. In
> >> case of RAW sockets MSG_MORE option makes things complicated. More
> >> details are in the conversation [1].
> >> This patch adds new control message type to give user-space
> >> software an opportunity to control the mapping between packets and
> >> values by providing ID with each sendmsg for UDP sockets.
> >> The documentation is also added in this patch.
> >>
> >> [1] https://lore.kernel.org/netdev/CALCETrU0jB+kg0mhV6A8mrHfTE1D1pr1SD_B9Eaa9aDPfgHdtA@mail.gmail.com/
> >>
> >> Signed-off-by: Vadim Fedorenko <vadfed@meta.com>

> >> @@ -1325,8 +1330,11 @@ static int ip_setup_cork(struct sock *sk, struct inet_cork *cork,
> >>   	cork->mark = ipc->sockc.mark;
> >>   	cork->priority = ipc->priority;
> >>   	cork->transmit_time = ipc->sockc.transmit_time;
> >> +	cork->ts_opt_id = ipc->sockc.ts_opt_id;
> >>   	cork->tx_flags = 0;
> >>   	sock_tx_timestamp(sk, ipc->sockc.tsflags, &cork->tx_flags);
> >> +	if (ipc->sockc.tsflags & SOCKCM_FLAG_TS_OPT_ID)
> >> +		cork->flags |= IPCORK_TS_OPT_ID;
> > 
> > We can move initialization of ts_opt_id into the branch.
> > 
> > Or conversely avoid the branch with some convoluted shift operations
> > to have the rval be either 1 << 1 or 0 << 1. But let's do the simpler
> > thing.
> 
> What is the reason to move initialization behind the flag? We are not
> doing this for transmit_time even though it's also used with flag only.
> 
> It's not a big deal to change, I just wonder what are the benefits?

Just avoid one assignment in the hot path that does not use this
feature.

cork->ts_opt_id is only valuid if cork->flags & IPCORK_TS_OPT_ID.