mbox series

[net-next,v3,0/4] Add option to provide OPT_ID value via cmsg

Message ID 20240904113153.2196238-1-vadfed@meta.com (mailing list archive)
Headers show
Series Add option to provide OPT_ID value via cmsg | expand

Message

Vadim Fedorenko Sept. 4, 2024, 11:31 a.m. UTC
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.

The first patch in the series adds all needed definitions and implements
the function for UDP sockets. The explicit check of socket's type is not
added because subsequent patches in the series will add support for other
types of sockets. The documentation is also included into the first
patch.

Patch 2/4 adds support for TCP sockets. This part is simple and straight
forward.

Patch 3/4 adds support for RAW sockets. It's a bit tricky because
sock_tx_timestamp functions has to be refactored to receive full socket
cookie information to fill in ID. The commit b534dc46c8ae ("net_tstamp:
add SOF_TIMESTAMPING_OPT_ID_TCP") did the conversion of sk_tsflags to
u32 but sock_tx_timestamp functions were not converted and still receive
16b flags. It wasn't a problem because SOF_TIMESTAMPING_OPT_ID_TCP was
not checked in these functions, that's why no backporting is needed.

Patch 4/4 adds selftests for new feature.

Changelog:
v2 -> v3:
- remove SOF_TIMESTAMPING_OPT_ID_CMSG UAPI value and use kernel-internal
  SOCKCM_FLAG_TS_OPT_ID which uses the highest bit of tsflags.
- add support for TCP and RAW sockets
v1 -> v2:
- add more selftests
- add documentation for the feature
- refactor UDP send function
RFC -> v1:
- add selftests
- add SOF_TIMESTAMPING_OPT_ID_CMSG to signal of custom ID provided by
	user-space instead of reserving value of 0 for this.

[1] https://lore.kernel.org/netdev/CALCETrU0jB+kg0mhV6A8mrHfTE1D1pr1SD_B9Eaa9aDPfgHdtA@mail.gmail.com/

Vadim Fedorenko (4):
  net_tstamp: add SCM_TS_OPT_ID to provide OPT_ID in control message
  net_tstamp: add SCM_TS_OPT_ID for TCP sockets
  net_tstamp: add SCM_TS_OPT_ID for RAW sockets
  selftests: txtimestamp: add SCM_TS_OPT_ID test

 Documentation/networking/timestamping.rst  | 13 ++++++
 arch/alpha/include/uapi/asm/socket.h       |  2 +
 arch/mips/include/uapi/asm/socket.h        |  2 +
 arch/parisc/include/uapi/asm/socket.h      |  2 +
 arch/sparc/include/uapi/asm/socket.h       |  2 +
 include/net/inet_sock.h                    |  4 +-
 include/net/sock.h                         | 29 +++++++++----
 include/uapi/asm-generic/socket.h          |  2 +
 include/uapi/linux/net_tstamp.h            |  7 ++++
 net/can/raw.c                              |  2 +-
 net/core/sock.c                            |  9 ++++
 net/ipv4/ip_output.c                       | 20 ++++++---
 net/ipv4/raw.c                             |  2 +-
 net/ipv4/tcp.c                             | 15 ++++---
 net/ipv6/ip6_output.c                      | 20 ++++++---
 net/ipv6/raw.c                             |  2 +-
 net/packet/af_packet.c                     |  6 +--
 net/socket.c                               |  2 +-
 tools/include/uapi/asm-generic/socket.h    |  2 +
 tools/testing/selftests/net/txtimestamp.c  | 48 +++++++++++++++++-----
 tools/testing/selftests/net/txtimestamp.sh | 12 +++---
 21 files changed, 154 insertions(+), 49 deletions(-)

Comments

Vadim Fedorenko Sept. 4, 2024, 11:36 a.m. UTC | #1
On 04/09/2024 12:31, 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.
> 
> The first patch in the series adds all needed definitions and implements
> the function for UDP sockets. The explicit check of socket's type is not
> added because subsequent patches in the series will add support for other
> types of sockets. The documentation is also included into the first
> patch.
> 
> Patch 2/4 adds support for TCP sockets. This part is simple and straight
> forward.
> 
> Patch 3/4 adds support for RAW sockets. It's a bit tricky because
> sock_tx_timestamp functions has to be refactored to receive full socket
> cookie information to fill in ID. The commit b534dc46c8ae ("net_tstamp:
> add SOF_TIMESTAMPING_OPT_ID_TCP") did the conversion of sk_tsflags to
> u32 but sock_tx_timestamp functions were not converted and still receive
> 16b flags. It wasn't a problem because SOF_TIMESTAMPING_OPT_ID_TCP was
> not checked in these functions, that's why no backporting is needed.
> 
> Patch 4/4 adds selftests for new feature.
> 
> Changelog:
> v2 -> v3:
> - remove SOF_TIMESTAMPING_OPT_ID_CMSG UAPI value and use kernel-internal
>    SOCKCM_FLAG_TS_OPT_ID which uses the highest bit of tsflags.
> - add support for TCP and RAW sockets
> v1 -> v2:
> - add more selftests
> - add documentation for the feature
> - refactor UDP send function
> RFC -> v1:
> - add selftests
> - add SOF_TIMESTAMPING_OPT_ID_CMSG to signal of custom ID provided by
> 	user-space instead of reserving value of 0 for this.
> 
> [1] https://lore.kernel.org/netdev/CALCETrU0jB+kg0mhV6A8mrHfTE1D1pr1SD_B9Eaa9aDPfgHdtA@mail.gmail.com/
> 
> Vadim Fedorenko (4):
>    net_tstamp: add SCM_TS_OPT_ID to provide OPT_ID in control message
>    net_tstamp: add SCM_TS_OPT_ID for TCP sockets
>    net_tstamp: add SCM_TS_OPT_ID for RAW sockets
>    selftests: txtimestamp: add SCM_TS_OPT_ID test
> 
>   Documentation/networking/timestamping.rst  | 13 ++++++
>   arch/alpha/include/uapi/asm/socket.h       |  2 +
>   arch/mips/include/uapi/asm/socket.h        |  2 +
>   arch/parisc/include/uapi/asm/socket.h      |  2 +
>   arch/sparc/include/uapi/asm/socket.h       |  2 +
>   include/net/inet_sock.h                    |  4 +-
>   include/net/sock.h                         | 29 +++++++++----
>   include/uapi/asm-generic/socket.h          |  2 +
>   include/uapi/linux/net_tstamp.h            |  7 ++++
>   net/can/raw.c                              |  2 +-
>   net/core/sock.c                            |  9 ++++
>   net/ipv4/ip_output.c                       | 20 ++++++---
>   net/ipv4/raw.c                             |  2 +-
>   net/ipv4/tcp.c                             | 15 ++++---
>   net/ipv6/ip6_output.c                      | 20 ++++++---
>   net/ipv6/raw.c                             |  2 +-
>   net/packet/af_packet.c                     |  6 +--
>   net/socket.c                               |  2 +-
>   tools/include/uapi/asm-generic/socket.h    |  2 +
>   tools/testing/selftests/net/txtimestamp.c  | 48 +++++++++++++++++-----
>   tools/testing/selftests/net/txtimestamp.sh | 12 +++---
>   21 files changed, 154 insertions(+), 49 deletions(-)
> 
Oh, sorry for the mess, patches:

[PATCH v3 2/3] selftests: txtimestamp: add SCM_TS_OPT_ID test
[PATCH v3 3/3] net_tstamp: add SCM_TS_OPT_ID for TCP sockets

should be ignored.

If it too messy I can resend the series.

Sorry again,
Vadim
Willem de Bruijn Sept. 4, 2024, 9:04 p.m. UTC | #2
> > 
> > Vadim Fedorenko (4):
> >    net_tstamp: add SCM_TS_OPT_ID to provide OPT_ID in control message
> >    net_tstamp: add SCM_TS_OPT_ID for TCP sockets
> >    net_tstamp: add SCM_TS_OPT_ID for RAW sockets
> >    selftests: txtimestamp: add SCM_TS_OPT_ID test
> > 
> Oh, sorry for the mess, patches:
> 
> [PATCH v3 2/3] selftests: txtimestamp: add SCM_TS_OPT_ID test
> [PATCH v3 3/3] net_tstamp: add SCM_TS_OPT_ID for TCP sockets
> 
> should be ignored.
> 
> If it too messy I can resend the series.

The series looks good to me. The four patches listed in the cover
letter summary.

Overall looks great.

Perhaps the test can now also test TCP and RAW with a fixed OPT_ID.
Vadim Fedorenko Sept. 4, 2024, 10:50 p.m. UTC | #3
On 04/09/2024 22:04, Willem de Bruijn wrote:
>>>
>>> Vadim Fedorenko (4):
>>>     net_tstamp: add SCM_TS_OPT_ID to provide OPT_ID in control message
>>>     net_tstamp: add SCM_TS_OPT_ID for TCP sockets
>>>     net_tstamp: add SCM_TS_OPT_ID for RAW sockets
>>>     selftests: txtimestamp: add SCM_TS_OPT_ID test
>>>
>> Oh, sorry for the mess, patches:
>>
>> [PATCH v3 2/3] selftests: txtimestamp: add SCM_TS_OPT_ID test
>> [PATCH v3 3/3] net_tstamp: add SCM_TS_OPT_ID for TCP sockets
>>
>> should be ignored.
>>
>> If it too messy I can resend the series.
> 
> The series looks good to me. The four patches listed in the cover
> letter summary.
> 
> Overall looks great.
> 
> Perhaps the test can now also test TCP and RAW with a fixed OPT_ID.

Sure, I'll add these tests. As well as changes to address comments for 
1/4 patch.

Thanks,
Vadim