[PATCH RFC net-next] netlink: Add support for timestamping messages

[PATCH RFC net-next] netlink: Add support for timestamping messages

[David Ahern]

From: David Ahern <dsahern@gmail.com>

Add support for timestamping netlink messages. If a socket wants a
timestamp, it is added when the skb clone is queued to the socket.

Allow userspace to know the actual time an event happened. In a
busy system there can be a long lag between when the event happened
and when the message is read from the socket. Further, this allows
separate netlink sockets for various RTNLGRP's where the timestamp
can be used to sort the messages if needed.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
one question I have is whether it would be better to add the timestamp
when the skb is created so it is the same for all sockets as opposed to
setting the time per socket.

 net/netlink/af_netlink.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 216ab915dd54..5e29ebfc701e 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1343,6 +1343,8 @@ int netlink_unicast(struct sock *ssk, struct sk_buff *skb,
 		return err;
 	}
 
+	if (sock_flag(sk, SOCK_RCVTSTAMP))
+		__net_timestamp(skb);
 	err = netlink_attachskb(sk, skb, &timeo, ssk);
 	if (err == 1)
 		goto retry;
@@ -1469,6 +1471,9 @@ static void do_one_broadcast(struct sock *sk,
 		p->skb2 = NULL;
 		goto out;
 	}
+
+	if (sock_flag(sk, SOCK_RCVTSTAMP))
+		__net_timestamp(p->skb2);
 	NETLINK_CB(p->skb2).nsid = peernet2id(sock_net(sk), p->net);
 	if (NETLINK_CB(p->skb2).nsid != NETNSA_NSID_NOT_ASSIGNED)
 		NETLINK_CB(p->skb2).nsid_is_set = true;
@@ -1848,6 +1853,47 @@ static void netlink_cmsg_listen_all_nsid(struct sock *sk, struct msghdr *msg,
 		 &NETLINK_CB(skb).nsid);
 }
 
+/* based on tcp_recv_timestamp */
+static void netlink_cmsg_timestamp(struct msghdr *msg, struct sk_buff *skb,
+				   struct sock *sk)
+{
+	int new_tstamp;
+
+	if (!skb_get_ktime(skb))
+		return;
+
+	new_tstamp = sock_flag(sk, SOCK_TSTAMP_NEW);
+	if (sock_flag(sk, SOCK_RCVTSTAMPNS)) {
+		if (new_tstamp) {
+			struct __kernel_timespec kts;
+
+			skb_get_new_timestampns(skb, &kts);
+			put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMPNS_NEW,
+				 sizeof(kts), &kts);
+		} else {
+			struct timespec ts;
+
+			skb_get_timestampns(skb, &ts);
+			put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMPNS_OLD,
+				 sizeof(ts), &ts);
+		}
+	} else {
+		if (new_tstamp) {
+			struct __kernel_sock_timeval stv;
+
+			skb_get_new_timestamp(skb, &stv);
+			put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMP_NEW,
+				 sizeof(stv), &stv);
+		} else {
+			struct __kernel_old_timeval tv;
+
+			skb_get_timestamp(skb, &tv);
+			put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMP_OLD,
+				 sizeof(tv), &tv);
+		}
+	}
+}
+
 static int netlink_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 {
 	struct sock *sk = sock->sk;
@@ -1996,6 +2042,8 @@ static int netlink_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 		netlink_cmsg_recv_pktinfo(msg, skb);
 	if (nlk->flags & NETLINK_F_LISTEN_ALL_NSID)
 		netlink_cmsg_listen_all_nsid(sk, msg, skb);
+	if (sock_flag(sk, SOCK_RCVTSTAMP))
+		netlink_cmsg_timestamp(msg, skb, sk);
 
 	memset(&scm, 0, sizeof(scm));
 	scm.creds = *NETLINK_CREDS(skb);
-- 
2.11.0

Re: [PATCH RFC net-next] netlink: Add support for timestamping messages

[David Miller]

From: David Ahern <dsahern@kernel.org>
Date: Thu,  9 May 2019 08:55:42 -0700

> From: David Ahern <dsahern@gmail.com>
> 
> Add support for timestamping netlink messages. If a socket wants a
> timestamp, it is added when the skb clone is queued to the socket.
> 
> Allow userspace to know the actual time an event happened. In a
> busy system there can be a long lag between when the event happened
> and when the message is read from the socket. Further, this allows
> separate netlink sockets for various RTNLGRP's where the timestamp
> can be used to sort the messages if needed.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
> one question I have is whether it would be better to add the timestamp
> when the skb is created so it is the same for all sockets as opposed to
> setting the time per socket.

If the importance is that the timestamp is when the "event" occurs
then you should set it at skb creation time.

Re: [PATCH RFC net-next] netlink: Add support for timestamping messages

[Willem de Bruijn]

On Thu, May 9, 2019 at 11:57 AM David Ahern <dsahern@kernel.org> wrote:
>
> From: David Ahern <dsahern@gmail.com>
>
> Add support for timestamping netlink messages. If a socket wants a
> timestamp, it is added when the skb clone is queued to the socket.
>
> Allow userspace to know the actual time an event happened. In a
> busy system there can be a long lag between when the event happened
> and when the message is read from the socket. Further, this allows
> separate netlink sockets for various RTNLGRP's where the timestamp
> can be used to sort the messages if needed.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
> one question I have is whether it would be better to add the timestamp
> when the skb is created so it is the same for all sockets as opposed to
> setting the time per socket.
>
>  net/netlink/af_netlink.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> +/* based on tcp_recv_timestamp */

Which itself is based on __sock_recv_timestamp. Which this resembles
even more closely, as both pass an skb. Instead of duplicating the
core code yet again, we can probably factor out and reuse it. Netlink
only does not need the SOF_TIMESTAMPING part.


> +static void netlink_cmsg_timestamp(struct msghdr *msg, struct sk_buff *skb,
> +                                  struct sock *sk)
> +{
> +       int new_tstamp;
> +
> +       if (!skb_get_ktime(skb))
> +               return;
> +
> +       new_tstamp = sock_flag(sk, SOCK_TSTAMP_NEW);
> +       if (sock_flag(sk, SOCK_RCVTSTAMPNS)) {
> +               if (new_tstamp) {
> +                       struct __kernel_timespec kts;
> +
> +                       skb_get_new_timestampns(skb, &kts);
> +                       put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMPNS_NEW,
> +                                sizeof(kts), &kts);
> +               } else {
> +                       struct timespec ts;
> +
> +                       skb_get_timestampns(skb, &ts);
> +                       put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMPNS_OLD,
> +                                sizeof(ts), &ts);
> +               }
> +       } else {
> +               if (new_tstamp) {
> +                       struct __kernel_sock_timeval stv;
> +
> +                       skb_get_new_timestamp(skb, &stv);
> +                       put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMP_NEW,
> +                                sizeof(stv), &stv);
> +               } else {
> +                       struct __kernel_old_timeval tv;
> +
> +                       skb_get_timestamp(skb, &tv);
> +                       put_cmsg(msg, SOL_SOCKET, SO_TIMESTAMP_OLD,
> +                                sizeof(tv), &tv);
> +               }
> +       }
> +}

Re: [PATCH RFC net-next] netlink: Add support for timestamping messages

[David Ahern]

On 5/9/19 10:51 AM, David Miller wrote:
> From: David Ahern <dsahern@kernel.org>
> Date: Thu,  9 May 2019 08:55:42 -0700
> 
>> From: David Ahern <dsahern@gmail.com>
>>
>> Add support for timestamping netlink messages. If a socket wants a
>> timestamp, it is added when the skb clone is queued to the socket.
>>
>> Allow userspace to know the actual time an event happened. In a
>> busy system there can be a long lag between when the event happened
>> and when the message is read from the socket. Further, this allows
>> separate netlink sockets for various RTNLGRP's where the timestamp
>> can be used to sort the messages if needed.
>>
>> Signed-off-by: David Ahern <dsahern@gmail.com>
>> ---
>> one question I have is whether it would be better to add the timestamp
>> when the skb is created so it is the same for all sockets as opposed to
>> setting the time per socket.
> 
> If the importance is that the timestamp is when the "event" occurs
> then you should set it at skb creation time.
> 

The overhead of adding the timestamp is why I was thinking of setting it
based on a socket request.

If I defer setting the timestamp to do_one_broadcast only systems where
a process / socket wanting a timestamp takes the overhead and all
processes / sockets wanting the timestamp see the same the one. Seems
like a good trade-off. It is a very small time gap between the skb
allocation and do_one_broadcast.

Worst case scenario is a notification storm such as a huge route dump
into the kernel. Enabling the timestamp does have a measurable overhead
(~15% for a notification storm of ~240,000/sec). Given all of the other
improvements the end result is still a huge gain, but to defer the
overhead only to users who want it seems like the right thing to do.

Re: [PATCH RFC net-next] netlink: Add support for timestamping messages

[David Ahern]

On 5/13/19 9:31 AM, Willem de Bruijn wrote:
>>
>> +/* based on tcp_recv_timestamp */
> 
> Which itself is based on __sock_recv_timestamp. Which this resembles
> even more closely, as both pass an skb. Instead of duplicating the
> core code yet again, we can probably factor out and reuse it. Netlink
> only does not need the SOF_TIMESTAMPING part.
> 
> 

agreed. My original version went down that path. I'll take another look
for v2.