2020-12-09 16:01:02

by Geva, Erez

[permalink] [raw]
Subject: [PATCH 0/3] Add sending TX hardware timestamp for TC ETF Qdisc

Add support for TX sending hardware timestamp with
Traffic control Earliest TxTime First (ETF) Qdisc.

Why do we need additional timestamp?
Current ETF requires to synchronization the system clock
to the PTP Hardware clock (PHC) we want to send through.
But there are cases that we can not synchronize the system clock with
the desired NIC PHC.
1. We use several NICs with several PTP domains that our device
is not allowed to be PTP master of.
2. We are using another clock source which we need for our system.
Yet our device is not allowed to be PTP master.

Regardless of the exact topology, as the Linux tradition is to allow
the user the freedom to choose,
we propose a patch that will add a hardware timestamp to the packet.
The TC-ETF will use the first timestamp and compare it with
the system clock while send the packet to the network interface driver
with that hardware timestamp that is correlated with the PHC.

Note 1: we do encourage the users to synchronize the system clock with
a PTP clock. Synchronizing the system clock with a PTP clock will
reduce the frequency difference of the PHC and the system clock,
increase the accurecy and may enable the user to reduce the ETF delta.

Note 2: In our network usage models sending a frame has to be very
precise in relation to the PHC. Our user application does have
the exact send time as of PHC perspective so it is able
to provide the hw timestamp.

Note 3: The user can estimate the clocks conversion error done
in the user application and add it to the delta setting of the ETF.

The patches contain:
1. A new flag for the SO_TXTIME socket option.
2. A new cmsg header, SCM_HW_TXTIME to pass the TX hardware timestamp.
3. Add the hardware timestamp to the socket cookie and to the inet cork.
4. As ETF Qdisc is irrelevant to TCP, ignore the TCP.
5. A new flag to the ETF Qdisc setting that mandate
the use of the hardware timestamp in the SKB.
6. The ETF sort packets according to hardware timestamp,
Yet pass the packet to network interface driver based
on the system clock timestamp.

Note 4: The socket buffer hardware timestamp is used by
the network interface driver to send the actual sending timestamp
back to the application. The timestamp is used by the TC ETF
before the socket buffer arrives in the network interface driver.

Erez Geva (3):
Add TX sending hardware timestamp.
Pass TX sending hardware timestamp to a socket's buffer.
The TC ETF Qdisc pass the hardware timestamp to the interface driver.

include/net/inet_sock.h | 1 +
include/net/sock.h | 2 ++
include/uapi/asm-generic/socket.h | 3 ++
include/uapi/linux/net_tstamp.h | 3 +-
include/uapi/linux/pkt_sched.h | 1 +
net/core/sock.c | 9 +++++
net/ipv4/ip_output.c | 2 ++
net/ipv4/raw.c | 1 +
net/ipv6/ip6_output.c | 2 ++
net/ipv6/raw.c | 1 +
net/packet/af_packet.c | 3 ++
net/sched/sch_etf.c | 59 +++++++++++++++++++++++++------
12 files changed, 75 insertions(+), 12 deletions(-)


base-commit: b65054597872ce3aefbc6a666385eabdf9e288da
--
2.20.1


2020-12-09 16:02:14

by Geva, Erez

[permalink] [raw]
Subject: [PATCH 2/3] Pass TX sending hardware timestamp to a socket's buffer.

Pass TX sending hardware timestamp from the socket layer to
a socket's buffer. The TC ETC Qdisc will pass it
to the interface network driver.

- Add the hardware timestamp to the IP cork, to support
the use of IP/UDP with the TX sending hardware timestamp
when sending through the TC ETF Qdisc
- Pass the TX sending hardware timestamp to a socket's buffer
using the hardware timestamp on the socket's buffer shared information.

Note: The socket buffer's hardware timestamp is used by
the network interface driver to send the actual sending timestamp
back to the application.
The timestamp is used by the TC ETF before the buffer
arrives in the network interface driver.

Signed-off-by: Erez Geva <[email protected]>
---
include/net/inet_sock.h | 1 +
net/ipv4/ip_output.c | 2 ++
net/ipv4/raw.c | 1 +
net/ipv6/ip6_output.c | 2 ++
net/ipv6/raw.c | 1 +
net/packet/af_packet.c | 3 +++
6 files changed, 10 insertions(+)

diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index 89163ef8cf4b..e74e8dabf63d 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -160,6 +160,7 @@ struct inet_cork {
char priority;
__u16 gso_size;
u64 transmit_time;
+ u64 transmit_hw_time;
u32 mark;
};

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 879b76ae4435..416598c864e3 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1282,6 +1282,7 @@ 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->transmit_hw_time = ipc->sockc.transmit_hw_time;
cork->tx_flags = 0;
sock_tx_timestamp(sk, ipc->sockc.tsflags, &cork->tx_flags);

@@ -1545,6 +1546,7 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
skb->priority = (cork->tos != -1) ? cork->priority: sk->sk_priority;
skb->mark = cork->mark;
skb->tstamp = cork->transmit_time;
+ skb_hwtstamps(skb)->hwtstamp = ns_to_ktime(cork->transmit_hw_time);
/*
* 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 7d26e0f8bdae..213a47fb2df8 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -378,6 +378,7 @@ static int raw_send_hdrinc(struct sock *sk, struct flowi4 *fl4,
skb->priority = sk->sk_priority;
skb->mark = sockc->mark;
skb->tstamp = sockc->transmit_time;
+ skb_hwtstamps(skb)->hwtstamp = ns_to_ktime(sockc->transmit_hw_time);
skb_dst_set(skb, &rt->dst);
*rtp = NULL;

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 749ad72386b2..8cff05f5aa8a 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1375,6 +1375,7 @@ static int ip6_setup_cork(struct sock *sk, struct inet_cork_full *cork,
cork->base.length = 0;

cork->base.transmit_time = ipc6->sockc.transmit_time;
+ cork->base.transmit_hw_time = ipc6->sockc.transmit_hw_time;

return 0;
}
@@ -1841,6 +1842,7 @@ struct sk_buff *__ip6_make_skb(struct sock *sk,
skb->mark = cork->base.mark;

skb->tstamp = cork->base.transmit_time;
+ skb_hwtstamps(skb)->hwtstamp = ns_to_ktime(cork->base.transmit_hw_time);

skb_dst_set(skb, dst_clone(&rt->dst));
IP6_UPD_PO_STATS(net, rt->rt6i_idev, IPSTATS_MIB_OUT, skb->len);
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 6e4ab80a3b94..417f21867782 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -648,6 +648,7 @@ static int rawv6_send_hdrinc(struct sock *sk, struct msghdr *msg, int length,
skb->priority = sk->sk_priority;
skb->mark = sockc->mark;
skb->tstamp = sockc->transmit_time;
+ skb_hwtstamps(skb)->hwtstamp = ns_to_ktime(sockc->transmit_hw_time);

skb_put(skb, length);
skb_reset_network_header(skb);
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 7a18ffff8551..c762d5bcecc2 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1986,6 +1986,7 @@ static int packet_sendmsg_spkt(struct socket *sock, struct msghdr *msg,
skb->priority = sk->sk_priority;
skb->mark = sk->sk_mark;
skb->tstamp = sockc.transmit_time;
+ skb_hwtstamps(skb)->hwtstamp = ns_to_ktime(sockc.transmit_hw_time);

skb_setup_tx_timestamp(skb, sockc.tsflags);

@@ -2500,6 +2501,7 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
skb->priority = po->sk.sk_priority;
skb->mark = po->sk.sk_mark;
skb->tstamp = sockc->transmit_time;
+ skb_hwtstamps(skb)->hwtstamp = ns_to_ktime(sockc->transmit_hw_time);
skb_setup_tx_timestamp(skb, sockc->tsflags);
skb_zcopy_set_nouarg(skb, ph.raw);

@@ -2975,6 +2977,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
skb->priority = sk->sk_priority;
skb->mark = sockc.mark;
skb->tstamp = sockc.transmit_time;
+ skb_hwtstamps(skb)->hwtstamp = ns_to_ktime(sockc.transmit_hw_time);

if (has_vnet_hdr) {
err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le());
--
2.20.1

2020-12-09 19:56:38

by Geva, Erez

[permalink] [raw]
Subject: [PATCH 1/3] Add TX sending hardware timestamp.

Configure and send TX sending hardware timestamp from
user space application to the socket layer,
to provide to the TC ETC Qdisc, and pass it to
the interface network driver.

- New flag for the SO_TXTIME socket option.
- New access auxiliary data header to pass the
TX sending hardware timestamp.
- Add the hardware timestamp to the socket cookie.
- Copy the TX sending hardware timestamp to the socket cookie.

Signed-off-by: Erez Geva <[email protected]>
---
include/net/sock.h | 2 ++
include/uapi/asm-generic/socket.h | 3 +++
include/uapi/linux/net_tstamp.h | 3 ++-
net/core/sock.c | 9 +++++++++
4 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index a5c6ae78df77..dd5bfd42b4e2 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -859,6 +859,7 @@ enum sock_flags {
SOCK_SELECT_ERR_QUEUE, /* Wake select on error queue */
SOCK_RCU_FREE, /* wait rcu grace period in sk_destruct() */
SOCK_TXTIME,
+ SOCK_HW_TXTIME,
SOCK_XDP, /* XDP is attached */
SOCK_TSTAMP_NEW, /* Indicates 64 bit timestamps always */
};
@@ -1690,6 +1691,7 @@ void sk_send_sigurg(struct sock *sk);

struct sockcm_cookie {
u64 transmit_time;
+ u64 transmit_hw_time;
u32 mark;
u16 tsflags;
};
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index 77f7c1638eb1..16265b00c25a 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -119,6 +119,9 @@

#define SO_DETACH_REUSEPORT_BPF 68

+#define SO_HW_TXTIME 69
+#define SCM_HW_TXTIME SO_HW_TXTIME
+
#if !defined(__KERNEL__)

#if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h
index 7ed0b3d1c00a..dd51c9a99b1f 100644
--- a/include/uapi/linux/net_tstamp.h
+++ b/include/uapi/linux/net_tstamp.h
@@ -162,8 +162,9 @@ struct scm_ts_pktinfo {
enum txtime_flags {
SOF_TXTIME_DEADLINE_MODE = (1 << 0),
SOF_TXTIME_REPORT_ERRORS = (1 << 1),
+ SOF_TXTIME_USE_HW_TIMESTAMP = (1 << 2),

- SOF_TXTIME_FLAGS_LAST = SOF_TXTIME_REPORT_ERRORS,
+ SOF_TXTIME_FLAGS_LAST = SOF_TXTIME_USE_HW_TIMESTAMP,
SOF_TXTIME_FLAGS_MASK = (SOF_TXTIME_FLAGS_LAST - 1) |
SOF_TXTIME_FLAGS_LAST
};
diff --git a/net/core/sock.c b/net/core/sock.c
index 727ea1cc633c..317dce54321b 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1227,6 +1227,8 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
break;
}
sock_valbool_flag(sk, SOCK_TXTIME, true);
+ sock_valbool_flag(sk, SOCK_HW_TXTIME,
+ sk_txtime.flags & SOF_TXTIME_USE_HW_TIMESTAMP);
sk->sk_clockid = sk_txtime.clockid;
sk->sk_txtime_deadline_mode =
!!(sk_txtime.flags & SOF_TXTIME_DEADLINE_MODE);
@@ -2378,6 +2380,13 @@ int __sock_cmsg_send(struct sock *sk, struct msghdr *msg, struct cmsghdr *cmsg,
return -EINVAL;
sockc->transmit_time = get_unaligned((u64 *)CMSG_DATA(cmsg));
break;
+ case SCM_HW_TXTIME:
+ if (!sock_flag(sk, SOCK_HW_TXTIME))
+ return -EINVAL;
+ if (cmsg->cmsg_len != CMSG_LEN(sizeof(u64)))
+ return -EINVAL;
+ sockc->transmit_hw_time = get_unaligned((u64 *)CMSG_DATA(cmsg));
+ break;
/* SCM_RIGHTS and SCM_CREDENTIALS are semantically in SOL_UNIX. */
case SCM_RIGHTS:
case SCM_CREDENTIALS:
--
2.20.1

2020-12-10 03:15:09

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/3] Add TX sending hardware timestamp.

Hi Erez,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on b65054597872ce3aefbc6a666385eabdf9e288da]

url: https://github.com/0day-ci/linux/commits/Erez-Geva/Add-sending-TX-hardware-timestamp-for-TC-ETF-Qdisc/20201210-000521
base: b65054597872ce3aefbc6a666385eabdf9e288da
config: mips-randconfig-r026-20201209 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 1968804ac726e7674d5de22bc2204b45857da344)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
# https://github.com/0day-ci/linux/commit/8a8f634bc74db16dc551cfcf3b63c1183f98eaac
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Erez-Geva/Add-sending-TX-hardware-timestamp-for-TC-ETF-Qdisc/20201210-000521
git checkout 8a8f634bc74db16dc551cfcf3b63c1183f98eaac
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> net/core/sock.c:2383:7: error: use of undeclared identifier 'SCM_HW_TXTIME'; did you mean 'SOCK_HW_TXTIME'?
case SCM_HW_TXTIME:
^~~~~~~~~~~~~
SOCK_HW_TXTIME
include/net/sock.h:862:2: note: 'SOCK_HW_TXTIME' declared here
SOCK_HW_TXTIME,
^
1 error generated.

vim +2383 net/core/sock.c

2351
2352 int __sock_cmsg_send(struct sock *sk, struct msghdr *msg, struct cmsghdr *cmsg,
2353 struct sockcm_cookie *sockc)
2354 {
2355 u32 tsflags;
2356
2357 switch (cmsg->cmsg_type) {
2358 case SO_MARK:
2359 if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN))
2360 return -EPERM;
2361 if (cmsg->cmsg_len != CMSG_LEN(sizeof(u32)))
2362 return -EINVAL;
2363 sockc->mark = *(u32 *)CMSG_DATA(cmsg);
2364 break;
2365 case SO_TIMESTAMPING_OLD:
2366 if (cmsg->cmsg_len != CMSG_LEN(sizeof(u32)))
2367 return -EINVAL;
2368
2369 tsflags = *(u32 *)CMSG_DATA(cmsg);
2370 if (tsflags & ~SOF_TIMESTAMPING_TX_RECORD_MASK)
2371 return -EINVAL;
2372
2373 sockc->tsflags &= ~SOF_TIMESTAMPING_TX_RECORD_MASK;
2374 sockc->tsflags |= tsflags;
2375 break;
2376 case SCM_TXTIME:
2377 if (!sock_flag(sk, SOCK_TXTIME))
2378 return -EINVAL;
2379 if (cmsg->cmsg_len != CMSG_LEN(sizeof(u64)))
2380 return -EINVAL;
2381 sockc->transmit_time = get_unaligned((u64 *)CMSG_DATA(cmsg));
2382 break;
> 2383 case SCM_HW_TXTIME:
2384 if (!sock_flag(sk, SOCK_HW_TXTIME))
2385 return -EINVAL;
2386 if (cmsg->cmsg_len != CMSG_LEN(sizeof(u64)))
2387 return -EINVAL;
2388 sockc->transmit_hw_time = get_unaligned((u64 *)CMSG_DATA(cmsg));
2389 break;
2390 /* SCM_RIGHTS and SCM_CREDENTIALS are semantically in SOL_UNIX. */
2391 case SCM_RIGHTS:
2392 case SCM_CREDENTIALS:
2393 break;
2394 default:
2395 return -EINVAL;
2396 }
2397 return 0;
2398 }
2399 EXPORT_SYMBOL(__sock_cmsg_send);
2400

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (3.50 kB)
.config.gz (26.26 kB)
Download all attachments

2020-12-11 00:53:47

by Geva, Erez

[permalink] [raw]
Subject: Re: [PATCH 1/3] Add TX sending hardware timestamp.


On 10/12/2020 04:11, kernel test robot wrote:
> Hi Erez,
>
> Thank you for the patch! Yet something to improve:
>
Thanks for the robot,
as we rarely use clang for kernel. It is very helpful.

> [auto build test ERROR on b65054597872ce3aefbc6a666385eabdf9e288da]
>
> url: https://github.com/0day-ci/linux/commits/Erez-Geva/Add-sending-TX-hardware-timestamp-for-TC-ETF-Qdisc/20201210-000521
I can not find this commit

> base: b65054597872ce3aefbc6a666385eabdf9e288da
> config: mips-randconfig-r026-20201209 (attached as .config)
> compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 1968804ac726e7674d5de22bc2204b45857da344)
However the clang in
https://download.01.org/0day-ci/cross-package/clang-latest/clang.tar.xz is version 11

> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
Your make cross script tries to download the clang every time.
Please separate the download (which is ~400 MB and 2 GB after open) from the compilation.

Please use "wget" follow your own instructions in this email.

> chmod +x ~/bin/make.cross
> # install mips cross compiling tool for clang build
> # apt-get install binutils-mips-linux-gnu
> # https://github.com/0day-ci/linux/commit/8a8f634bc74db16dc551cfcf3b63c1183f98eaac
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Erez-Geva/Add-sending-TX-hardware-timestamp-for-TC-ETF-Qdisc/20201210-000521
This branch is absent

> git checkout 8a8f634bc74db16dc551cfcf3b63c1183f98eaac
This commit as well

> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips
>
I use Debian 10.7.
I usually compile with GCC. I have not see any errors.

When I use clang 11 from download.01.org I get a crash right away.
Please add a proper instructions how to use clang on Debian or provide
a Docker container with updated clang for testing.

> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> All errors (new ones prefixed by >>):
>
>>> net/core/sock.c:2383:7: error: use of undeclared identifier 'SCM_HW_TXTIME'; did you mean 'SOCK_HW_TXTIME'?
> case SCM_HW_TXTIME:
> ^~~~~~~~~~~~~
> SOCK_HW_TXTIME
> include/net/sock.h:862:2: note: 'SOCK_HW_TXTIME' declared here
> SOCK_HW_TXTIME,
> ^
> 1 error generated.
>
> vim +2383 net/core/sock.c
>
> 2351
> 2352 int __sock_cmsg_send(struct sock *sk, struct msghdr *msg, struct cmsghdr *cmsg,
> 2353 struct sockcm_cookie *sockc)
> 2354 {
> 2355 u32 tsflags;
> 2356
> 2357 switch (cmsg->cmsg_type) {
> 2358 case SO_MARK:
> 2359 if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN))
> 2360 return -EPERM;
> 2361 if (cmsg->cmsg_len != CMSG_LEN(sizeof(u32)))
> 2362 return -EINVAL;
> 2363 sockc->mark = *(u32 *)CMSG_DATA(cmsg);
> 2364 break;
> 2365 case SO_TIMESTAMPING_OLD:
> 2366 if (cmsg->cmsg_len != CMSG_LEN(sizeof(u32)))
> 2367 return -EINVAL;
> 2368
> 2369 tsflags = *(u32 *)CMSG_DATA(cmsg);
> 2370 if (tsflags & ~SOF_TIMESTAMPING_TX_RECORD_MASK)
> 2371 return -EINVAL;
> 2372
> 2373 sockc->tsflags &= ~SOF_TIMESTAMPING_TX_RECORD_MASK;
> 2374 sockc->tsflags |= tsflags;
> 2375 break;
> 2376 case SCM_TXTIME:
> 2377 if (!sock_flag(sk, SOCK_TXTIME))
> 2378 return -EINVAL;
> 2379 if (cmsg->cmsg_len != CMSG_LEN(sizeof(u64)))
> 2380 return -EINVAL;
> 2381 sockc->transmit_time = get_unaligned((u64 *)CMSG_DATA(cmsg));
> 2382 break;
>> 2383 case SCM_HW_TXTIME:
> 2384 if (!sock_flag(sk, SOCK_HW_TXTIME))
> 2385 return -EINVAL;
> 2386 if (cmsg->cmsg_len != CMSG_LEN(sizeof(u64)))
> 2387 return -EINVAL;
> 2388 sockc->transmit_hw_time = get_unaligned((u64 *)CMSG_DATA(cmsg));
> 2389 break;
> 2390 /* SCM_RIGHTS and SCM_CREDENTIALS are semantically in SOL_UNIX. */
> 2391 case SCM_RIGHTS:
> 2392 case SCM_CREDENTIALS:
> 2393 break;
> 2394 default:
> 2395 return -EINVAL;
> 2396 }
> 2397 return 0;
> 2398 }
> 2399 EXPORT_SYMBOL(__sock_cmsg_send);
> 2400
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/[email protected]
>

Please improve the robot, so we can comply and properly support clang compilation.

Thanks
Erez

2020-12-11 14:53:01

by Geva, Erez

[permalink] [raw]
Subject: Re: [PATCH 1/3] Add TX sending hardware timestamp.


On 11/12/2020 01:27, Vinicius Costa Gomes wrote:
> Willem de Bruijn <[email protected]> writes:
>
>>>> If I understand correctly, you are trying to achieve a single delivery time.
>>>> The need for two separate timestamps passed along is only because the
>>>> kernel is unable to do the time base conversion.
>>>
>>> Yes, a correct point.
>>>
>>>>
>>>> Else, ETF could program the qdisc watchdog in system time and later,
>>>> on dequeue, convert skb->tstamp to the h/w time base before
>>>> passing it to the device.
>>>
>>> Or the skb->tstamp is HW time-stamp and the ETF convert it to system clock based.
>>>
>>>>
>>>> It's still not entirely clear to me why the packet has to be held by
>>>> ETF initially first, if it is held until delivery time by hardware
>>>> later. But more on that below.
>>>
>>> Let plot a simple scenario.
>>> App A send a packet with time-stamp 100.
>>> After arrive a second packet from App B with time-stamp 90.
>>> Without ETF, the second packet will have to wait till the interface hardware send the first packet on 100.
>>> Making the second packet late by 10 + first packet send time.
>>> Obviously other "normal" packets are send to the non-ETF queue, though they do not block ETF packets
>>> The ETF delta is a barrier that the application have to send the packet before to ensure the packet do not tossed.
>>
>> Got it. The assumption here is that devices are FIFO. That is not
>> necessarily the case, but I do not know whether it is in practice,
>> e.g., on the i210.
>
> On the i210 and i225, that's indeed the case, i.e. only the launch time
> of the packet at the front of the queue is considered.
>
> [...]
>
>>>>>>>> It only requires that pacing qdiscs, both sch_etf and sch_fq,
>>>>>>>> optionally skip queuing in their .enqueue callback and instead allow
>>>>>>>> the skb to pass to the device driver as is, with skb->tstamp set. Only
>>>>>>>> to devices that advertise support for h/w pacing offload.
>>>>>>>>
>>>>>>> I did not use "Fair Queue traffic policing".
>>>>>>> As for ETF, it is all about ordering packets from different applications.
>>>>>>> How can we achive it with skiping queuing?
>>>>>>> Could you elaborate on this point?
>>>>>>
>>>>>> The qdisc can only defer pacing to hardware if hardware can ensure the
>>>>>> same invariants on ordering, of course.
>>>>>
>>>>> Yes, this is why we suggest ETF order packets using the hardware time-stamp.
>>>>> And pass the packet based on system time.
>>>>> So ETF query the system clock only and not the PHC.
>>>>
>>>> On which note: with this patch set all applications have to agree to
>>>> use h/w time base in etf_enqueue_timesortedlist. In practice that
>>>> makes this h/w mode a qdisc used by a single process?
>>>
>>> A single process theoretically does not need ETF, just set the skb-> tstamp and use a pass through queue.
>>> However the only way now to set TC_SETUP_QDISC_ETF in the driver is using ETF.
>>
>> Yes, and I'd like to eventually get rid of this constraint.
>>
>
> I'm interested in these kind of ideas :-)
>
> What would be your end goal? Something like:
> - Any application is able to set SO_TXTIME;
> - We would have a best effort support for scheduling packets based on
> their transmission time enabled by default;
> - If the hardware supports, there would be a "offload" flag that could
> be enabled;
>
> More or less this?

Activate the SO_TXTIME is what cause the SKB to enter the matching ETF QDISC.
If the ETF QDISC is not set the SKB will pass directly to the driver.
Or if the SO_TXTIME Clock ID is not TAI.
So application can use the SO_TXTIME as is and set the skb-> tstamp.
No need to change anything for SO_TXTIME.

As for setting TC_SETUP_QDISC_ETF on a driver queue.
We can add net-link message using the net-link protocol.
How about other TC_SETUP_QDISC_XXX like CBS?

>
>
> Cheers.
>

2020-12-11 21:06:48

by Geva, Erez

[permalink] [raw]
Subject: Re: [PATCH 1/3] Add TX sending hardware timestamp.


On 10/12/2020 13:33, Geva, Erez (ext) (DI PA CI R&D 3) wrote:
>
> On 10/12/2020 04:11, kernel test robot wrote:
>> Hi Erez,
>>
>> Thank you for the patch! Yet something to improve:
>>
> Thanks for the robot,
> as we rarely use clang for kernel. It is very helpful.
>
>> [auto build test ERROR on b65054597872ce3aefbc6a666385eabdf9e288da]
>>
>> url: https://github.com/0day-ci/linux/commits/Erez-Geva/Add-sending-TX-hardware-timestamp-for-TC-ETF-Qdisc/20201210-000521
> I can not find this commit
>
>> base: b65054597872ce3aefbc6a666385eabdf9e288da
>> config: mips-randconfig-r026-20201209 (attached as .config)
>> compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 1968804ac726e7674d5de22bc2204b45857da344)
> However the clang in
> https://download.01.org/0day-ci/cross-package/clang-latest/clang.tar.xz is version 11
>
>> reproduce (this is a W=1 build):
>> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> Your make cross script tries to download the clang every time.
> Please separate the download (which is ~400 MB and 2 GB after open) from the compilation.
>
> Please use "wget" follow your own instructions in this email.
>
>> chmod +x ~/bin/make.cross
>> # install mips cross compiling tool for clang build
>> # apt-get install binutils-mips-linux-gnu
>> # https://github.com/0day-ci/linux/commit/8a8f634bc74db16dc551cfcf3b63c1183f98eaac
>> git remote add linux-review https://github.com/0day-ci/linux
>> git fetch --no-tags linux-review Erez-Geva/Add-sending-TX-hardware-timestamp-for-TC-ETF-Qdisc/20201210-000521
> This branch is absent
>
>> git checkout 8a8f634bc74db16dc551cfcf3b63c1183f98eaac
> This commit as well
>
>> # save the attached .config to linux build tree
>> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips
>>
> I use Debian 10.7.
> I usually compile with GCC. I have not see any errors.
>
> When I use clang 11 from download.01.org I get a crash right away.
> Please add a proper instructions how to use clang on Debian or provide
> a Docker container with updated clang for testing.
>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <[email protected]>
>>
>> All errors (new ones prefixed by >>):
>>
>>>> net/core/sock.c:2383:7: error: use of undeclared identifier 'SCM_HW_TXTIME'; did you mean 'SOCK_HW_TXTIME'?
>> case SCM_HW_TXTIME:
>> ^~~~~~~~~~~~~
>> SOCK_HW_TXTIME
>> include/net/sock.h:862:2: note: 'SOCK_HW_TXTIME' declared here
>> SOCK_HW_TXTIME,
>> ^
>> 1 error generated.
>>
>> vim +2383 net/core/sock.c
>>
>> 2351
>> 2352 int __sock_cmsg_send(struct sock *sk, struct msghdr *msg, struct cmsghdr *cmsg,
>> 2353 struct sockcm_cookie *sockc)
>> 2354 {
>> 2355 u32 tsflags;
>> 2356
>> 2357 switch (cmsg->cmsg_type) {
>> 2358 case SO_MARK:
>> 2359 if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN))
>> 2360 return -EPERM;
>> 2361 if (cmsg->cmsg_len != CMSG_LEN(sizeof(u32)))
>> 2362 return -EINVAL;
>> 2363 sockc->mark = *(u32 *)CMSG_DATA(cmsg);
>> 2364 break;
>> 2365 case SO_TIMESTAMPING_OLD:
>> 2366 if (cmsg->cmsg_len != CMSG_LEN(sizeof(u32)))
>> 2367 return -EINVAL;
>> 2368
>> 2369 tsflags = *(u32 *)CMSG_DATA(cmsg);
>> 2370 if (tsflags & ~SOF_TIMESTAMPING_TX_RECORD_MASK)
>> 2371 return -EINVAL;
>> 2372
>> 2373 sockc->tsflags &= ~SOF_TIMESTAMPING_TX_RECORD_MASK;
>> 2374 sockc->tsflags |= tsflags;
>> 2375 break;
>> 2376 case SCM_TXTIME:
>> 2377 if (!sock_flag(sk, SOCK_TXTIME))
>> 2378 return -EINVAL;
>> 2379 if (cmsg->cmsg_len != CMSG_LEN(sizeof(u64)))
>> 2380 return -EINVAL;
>> 2381 sockc->transmit_time = get_unaligned((u64 *)CMSG_DATA(cmsg));
>> 2382 break;
>>> 2383 case SCM_HW_TXTIME:
>> 2384 if (!sock_flag(sk, SOCK_HW_TXTIME))
>> 2385 return -EINVAL;
>> 2386 if (cmsg->cmsg_len != CMSG_LEN(sizeof(u64)))
>> 2387 return -EINVAL;
>> 2388 sockc->transmit_hw_time = get_unaligned((u64 *)CMSG_DATA(cmsg));
>> 2389 break;
>> 2390 /* SCM_RIGHTS and SCM_CREDENTIALS are semantically in SOL_UNIX. */
>> 2391 case SCM_RIGHTS:
>> 2392 case SCM_CREDENTIALS:
>> 2393 break;
>> 2394 default:
>> 2395 return -EINVAL;
>> 2396 }
>> 2397 return 0;
>> 2398 }
>> 2399 EXPORT_SYMBOL(__sock_cmsg_send);
>> 2400
>>
>> ---
>> 0-DAY CI Kernel Test Service, Intel Corporation
>> https://lists.01.org/hyperkitty/list/[email protected]
>>
>
> Please improve the robot, so we can comply and properly support clang compilation.
>
> Thanks
> Erez
>

Update,

I use the same .config from the Intel robot test.

I was trying to compile v5.10-rc6 with GCC cross compiler for mips.

# apt-get install -t sid gcc-mips-linux-gnu

kernel-test ((v5.10-rc6))$ /usr/bin/mips-linux-gnu-gcc --version
mips-linux-gnu-gcc (Debian 10.2.0-17) 10.2.0
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

kernel-test ((v5.10-rc6))$ cp ../intel_robot.config .config
kernel-test ((v5.10-rc6))$ make ARCH=mips CROSS_COMPILE=/usr/bin/mips-linux-gnu- olddefconfig
...

kernel-test ((v5.10-rc6))$ make ARCH=mips CROSS_COMPILE=/usr/bin/mips-linux-gnu- all
...
CC init/main.o
{standard input}: Assembler messages:
{standard input}:9103: Error: found '(', expected: ')'
{standard input}:9103: Error: found '(', expected: ')'
{standard input}:9103: Error: non-constant expression in ".if" statement
{standard input}:9103: Error: junk at end of line, first unrecognized character is `('
make[1]: *** [scripts/Makefile.build:283: init/main.o] Error 1
make: *** [Makefile:1797: init] Error 2

Erez

2020-12-13 14:31:05

by Philip Li

[permalink] [raw]
Subject: Re: [kbuild-all] Re: [PATCH 1/3] Add TX sending hardware timestamp.

On Thu, Dec 10, 2020 at 12:41:32PM +0000, Geva, Erez wrote:
>
> On 10/12/2020 04:11, kernel test robot wrote:
> > Hi Erez,
> >
> > Thank you for the patch! Yet something to improve:
> >
> Thanks for the robot,
> as we rarely use clang for kernel. It is very helpful.
>
> > [auto build test ERROR on b65054597872ce3aefbc6a666385eabdf9e288da]
> >
> > url: https://github.com/0day-ci/linux/commits/Erez-Geva/Add-sending-TX-hardware-timestamp-for-TC-ETF-Qdisc/20201210-000521
> I can not find this commit
>
> > base: b65054597872ce3aefbc6a666385eabdf9e288da
> > config: mips-randconfig-r026-20201209 (attached as .config)
> > compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 1968804ac726e7674d5de22bc2204b45857da344)
> However the clang in
> https://download.01.org/0day-ci/cross-package/clang-latest/clang.tar.xz is version 11
Sorry that these are issues at our side, including the branch/commit missing.
The push to download.01.org failed and did not really work, we will look for
recovering them.

>
> > reproduce (this is a W=1 build):
> > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> Your make cross script tries to download the clang every time.
> Please separate the download (which is ~400 MB and 2 GB after open) from the compilation.
Hi Erez, thanks for your feedback, we will improve the reproduction
side per these suggestions.

>
> Please use "wget" follow your own instructions in this email.
>
> > chmod +x ~/bin/make.cross
> > # install mips cross compiling tool for clang build
> > # apt-get install binutils-mips-linux-gnu
> > # https://github.com/0day-ci/linux/commit/8a8f634bc74db16dc551cfcf3b63c1183f98eaac
> > git remote add linux-review https://github.com/0day-ci/linux
> > git fetch --no-tags linux-review Erez-Geva/Add-sending-TX-hardware-timestamp-for-TC-ETF-Qdisc/20201210-000521
> This branch is absent
>
> > git checkout 8a8f634bc74db16dc551cfcf3b63c1183f98eaac
> This commit as well
>
> > # save the attached .config to linux build tree
> > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips
> >
> I use Debian 10.7.
> I usually compile with GCC. I have not see any errors.
>
> When I use clang 11 from download.01.org I get a crash right away.
> Please add a proper instructions how to use clang on Debian or provide
> a Docker container with updated clang for testing.
>
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <[email protected]>
> >
> > All errors (new ones prefixed by >>):
> >
> >>> net/core/sock.c:2383:7: error: use of undeclared identifier 'SCM_HW_TXTIME'; did you mean 'SOCK_HW_TXTIME'?
> > case SCM_HW_TXTIME:
> > ^~~~~~~~~~~~~
> > SOCK_HW_TXTIME
> > include/net/sock.h:862:2: note: 'SOCK_HW_TXTIME' declared here
> > SOCK_HW_TXTIME,
> > ^
> > 1 error generated.
> >
> > vim +2383 net/core/sock.c
> >
> > 2351
> > 2352 int __sock_cmsg_send(struct sock *sk, struct msghdr *msg, struct cmsghdr *cmsg,
> > 2353 struct sockcm_cookie *sockc)
> > 2354 {
> > 2355 u32 tsflags;
> > 2356
> > 2357 switch (cmsg->cmsg_type) {
> > 2358 case SO_MARK:
> > 2359 if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN))
> > 2360 return -EPERM;
> > 2361 if (cmsg->cmsg_len != CMSG_LEN(sizeof(u32)))
> > 2362 return -EINVAL;
> > 2363 sockc->mark = *(u32 *)CMSG_DATA(cmsg);
> > 2364 break;
> > 2365 case SO_TIMESTAMPING_OLD:
> > 2366 if (cmsg->cmsg_len != CMSG_LEN(sizeof(u32)))
> > 2367 return -EINVAL;
> > 2368
> > 2369 tsflags = *(u32 *)CMSG_DATA(cmsg);
> > 2370 if (tsflags & ~SOF_TIMESTAMPING_TX_RECORD_MASK)
> > 2371 return -EINVAL;
> > 2372
> > 2373 sockc->tsflags &= ~SOF_TIMESTAMPING_TX_RECORD_MASK;
> > 2374 sockc->tsflags |= tsflags;
> > 2375 break;
> > 2376 case SCM_TXTIME:
> > 2377 if (!sock_flag(sk, SOCK_TXTIME))
> > 2378 return -EINVAL;
> > 2379 if (cmsg->cmsg_len != CMSG_LEN(sizeof(u64)))
> > 2380 return -EINVAL;
> > 2381 sockc->transmit_time = get_unaligned((u64 *)CMSG_DATA(cmsg));
> > 2382 break;
> >> 2383 case SCM_HW_TXTIME:
> > 2384 if (!sock_flag(sk, SOCK_HW_TXTIME))
> > 2385 return -EINVAL;
> > 2386 if (cmsg->cmsg_len != CMSG_LEN(sizeof(u64)))
> > 2387 return -EINVAL;
> > 2388 sockc->transmit_hw_time = get_unaligned((u64 *)CMSG_DATA(cmsg));
> > 2389 break;
> > 2390 /* SCM_RIGHTS and SCM_CREDENTIALS are semantically in SOL_UNIX. */
> > 2391 case SCM_RIGHTS:
> > 2392 case SCM_CREDENTIALS:
> > 2393 break;
> > 2394 default:
> > 2395 return -EINVAL;
> > 2396 }
> > 2397 return 0;
> > 2398 }
> > 2399 EXPORT_SYMBOL(__sock_cmsg_send);
> > 2400
> >
> > ---
> > 0-DAY CI Kernel Test Service, Intel Corporation
> > https://lists.01.org/hyperkitty/list/[email protected]
> >
>
> Please improve the robot, so we can comply and properly support clang compilation.
Got it, we will keep improving the bot.

>
> Thanks
> Erez
> _______________________________________________
> kbuild-all mailing list -- [email protected]
> To unsubscribe send an email to [email protected]