From: Jesus Sanchez-Palencia <hidden> Date: 2018-01-17 23:07:41
This series is the v2 of the Time based packet transmission RFC, which was
originally proposed by Richard Cochran: https://lwn.net/Articles/733962/ .
It introduces a new socket option (SO_TXTIME), a new qdisc (tbs) and implements
support for hw offloading on the igb driver for the Intel i210 NIC. The tbs
qdisc also supports SW best effort that can be used as a fallback.
The main changes since v1 are:
- the tstamp field from sk_buffs is now used;
- ktime_t is the type used for the field added to struct sockcm_cookie instead
of u64;
- the tbs qdisc is introduced with SW best effort and hw offloading;
- the igb implementation for HW offloading was re-written, allowing both tbs
and cbs qdiscs to co-exist with proper driver support.
The tbs qdisc is designed so it buffers packets until a configurable time before
their deadline (tx times). It uses a rbtree internally, thus the buffered
packets are always 'ordered' by the earliest deadline.
The other configurable parameter from the tbs qdisc is the clockid to be used.
In order to provide that, this series adds a new API to pkt_sched.h (i.e.
qdisc_watchdog_init_clockid()).
As an usage example:
$ tc qdisc replace dev enp2s0 parent root handle 100 mqprio num_tc 3 \
map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 queues 1@0 1@1 2@2 hw 0
$ tc qdisc add dev enp2s0 parent 100:1 tbs delta 60000 clockid 11 \
offload 1
In this example first the mqprio qdisc is setup, then the tbs qdisc is
configured onto the first hw Tx queue and tries to enable HW offloading (i.e.
offload 1). Also, it is configured so the timestamps on each packet are in
reference to the clockid '11' (CLOCK_TAI) and so packets are dequeued from
the qdisc 60000 nanoseconds before their transmission time.
The tbs qdisc will drop any packets with a transmission time in the past or
when a deadline is missed. Queueing packets in advance plus configuring the
delta parameter for the system correctly makes all the difference in reducing
the number of drops. Moreover, note that the delta parameter ends up defining
the Tx time when SW best effort is used given that the timestamps won't be used
by the NIC on this case.
For testing, we've followed a similar approach from the v1 testing:
1. Prepared a PC and the Device Under Test (DUT) each with an Intel
i210 card connected.
2. The DUT was a Intel(R) Core(TM) i5-7600 CPU @ 3.50GHz running on top of
kernel 4.15.0-rc8+ with about 50 usec maximum latency under cyclictest.
3. Synchronized the DUT's PHC to the PC's PHC using ptp4l.
4. Synchronized the DUT's system clock to its PHC using phc2sys.
5. Measured the arrival time of the packets at the PC's PHC using
hardware time stamping.
First, a baseline test was ran for 10 minutes with the plain kernel only:
| | plain kernel @ 1ms |
|-----------------+--------------------+
| min (ns): | +4.820000e+02 |
| max (ns): | +9.999300e+05 |
| pk-pk: | +9.994480e+05 |
| mean (ns): | +3.464421e+04 |
| stddev: | +1.305947e+05 |
| count: | 600000 |
Tests were then ran for 10 minutes with a period of 1 millisecond using both
SW best effort and HW offloading. For last, we repeated the HW offloading test
with a 250 microsecond period. The measured offset from the expected period is
shown below, plus the tbs delta parameter that was used in each case.
| | tbs SW @ 1ms | tbs HW @ 1ms | tbs HW @ 250 us |
|-----------------+-------------------+----------------+-----------------|
| min (ns): | +1.510000e+02 | +4.420000e+02 | +4.260000e+02 |
| max (ns): | +9.977030e+05 | +5.060000e+02 | +5.060000e+02 |
| pk-pk: | +9.975520e+05 | +6.400000e+01 | +8.000000e+01 |
| mean (ns): | +1.416511e+04 | +4.687228e+02 | +4.600596e+02 |
| stddev: | +5.750639e+04 | +9.868569e+00 | +1.287626e+01 |
| count: | 600000 | 600000 | 2400000 |
| dropped: | 3 | 0 | 0 |
| tbs delta (ns): | 130000 | 130000 | 130000 |
The code used for testing is appended below. The wake_tx parameter (-d) used
for all tests was 600000 ns and the priority parameter was 90 (-p). The
baseline test (plain kernel) used a wake_tx parameter of 130000 ns.
Our main questions at this stage are related to the qdisc:
- does the proposed design attend all use cases?
- should the qdisc really drop packets that expired after being queued even
for the SW best effort mode?
- once one expired packet is found and dropped during a dequeue, should we
traverse the rbtree and drop other expired packets if any, or should we
keep deferring that to the next dequeue call?
For last, most of the To Dos we still have before a final patchset are related
to further testing the igb support:
- testing with L2 only talkers + AF_PACKET sockets;
- testing tbs in conjunction with cbs;
Thanks,
Jesus
Jesus Sanchez-Palencia (4):
igb: Refactor igb_configure_cbs()
igb: Only change Tx arbitration when CBS is on
igb: Refactor igb_offload_cbs()
igb: Add support for TBS offload
Richard Cochran (4):
net: Add a new socket option for a future transmit time.
net: ipv4: raw: Hook into time based transmission.
net: ipv4: udp: Hook into time based transmission.
net: packet: Hook into time based transmission.
Vinicius Costa Gomes (2):
net/sched: Allow creating a Qdisc watchdog with other clocks
net/sched: Introduce the TBS Qdisc
arch/alpha/include/uapi/asm/socket.h | 3 +
arch/frv/include/uapi/asm/socket.h | 3 +
arch/ia64/include/uapi/asm/socket.h | 3 +
arch/m32r/include/uapi/asm/socket.h | 3 +
arch/mips/include/uapi/asm/socket.h | 3 +
arch/mn10300/include/uapi/asm/socket.h | 3 +
arch/parisc/include/uapi/asm/socket.h | 3 +
arch/s390/include/uapi/asm/socket.h | 3 +
arch/sparc/include/uapi/asm/socket.h | 3 +
arch/xtensa/include/uapi/asm/socket.h | 3 +
drivers/net/ethernet/intel/igb/e1000_defines.h | 16 +
drivers/net/ethernet/intel/igb/igb.h | 1 +
drivers/net/ethernet/intel/igb/igb_main.c | 239 +++++++++++----
include/linux/netdevice.h | 1 +
include/net/pkt_sched.h | 7 +
include/net/sock.h | 2 +
include/uapi/asm-generic/socket.h | 3 +
include/uapi/linux/pkt_sched.h | 17 ++
net/core/sock.c | 16 +
net/ipv4/raw.c | 2 +
net/ipv4/udp.c | 5 +-
net/packet/af_packet.c | 6 +
net/sched/Kconfig | 11 +
net/sched/Makefile | 1 +
net/sched/sch_api.c | 11 +-
net/sched/sch_tbs.c | 392 +++++++++++++++++++++++++
26 files changed, 699 insertions(+), 61 deletions(-)
create mode 100644 net/sched/sch_tbs.c
--
2.15.1
---8<---
/*
* This program demonstrates transmission of UDP packets using the
* system TAI timer.
*
* Copyright (C) 2017 linutronix GmbH
*
* Large portions taken from the linuxptp stack.
* Copyright (C) 2011, 2012 Richard Cochran [off-list ref]
*
* Some portions taken from the sgd test program.
* Copyright (C) 2015 linutronix GmbH
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; if not, write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/
#define _GNU_SOURCE /*for CPU_SET*/
#include <arpa/inet.h>
#include <errno.h>
#include <fcntl.h>
#include <ifaddrs.h>
#include <linux/ethtool.h>
#include <linux/net_tstamp.h>
#include <linux/sockios.h>
#include <net/if.h>
#include <netinet/in.h>
#include <poll.h>
#include <pthread.h>
#include <sched.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
#define DEFAULT_PERIOD 1000000
#define DEFAULT_DELAY 500000
#define MCAST_IPADDR "239.1.1.1"
#define UDP_PORT 7788
#ifndef SO_TXTIME
#define SO_TXTIME 61
#endif
#define pr_err(s) fprintf(stderr, s "\n")
#define pr_info(s) fprintf(stdout, s "\n")
static int running = 1, use_so_txtime = 1;
static int period_nsec = DEFAULT_PERIOD;
static int waketx_delay = DEFAULT_DELAY;
static struct in_addr mcast_addr;
static int mcast_bind(int fd, int index)
{
int err;
struct ip_mreqn req;
memset(&req, 0, sizeof(req));
req.imr_ifindex = index;
err = setsockopt(fd, IPPROTO_IP, IP_MULTICAST_IF, &req, sizeof(req));
if (err) {
pr_err("setsockopt IP_MULTICAST_IF failed: %m");
return -1;
}
return 0;
}
static int mcast_join(int fd, int index, const struct sockaddr *grp,
socklen_t grplen)
{
int err, off = 0;
struct ip_mreqn req;
struct sockaddr_in *sa = (struct sockaddr_in *) grp;
memset(&req, 0, sizeof(req));
memcpy(&req.imr_multiaddr, &sa->sin_addr, sizeof(struct in_addr));
req.imr_ifindex = index;
err = setsockopt(fd, IPPROTO_IP, IP_ADD_MEMBERSHIP, &req, sizeof(req));
if (err) {
pr_err("setsockopt IP_ADD_MEMBERSHIP failed: %m");
return -1;
}
err = setsockopt(fd, IPPROTO_IP, IP_MULTICAST_LOOP, &off, sizeof(off));
if (err) {
pr_err("setsockopt IP_MULTICAST_LOOP failed: %m");
return -1;
}
return 0;
}
static void normalize(struct timespec *ts)
{
while (ts->tv_nsec > 999999999) {
ts->tv_sec += 1;
ts->tv_nsec -= 1000000000;
}
}
static int sk_interface_index(int fd, const char *name)
{
struct ifreq ifreq;
int err;
memset(&ifreq, 0, sizeof(ifreq));
strncpy(ifreq.ifr_name, name, sizeof(ifreq.ifr_name) - 1);
err = ioctl(fd, SIOCGIFINDEX, &ifreq);
if (err < 0) {
pr_err("ioctl SIOCGIFINDEX failed: %m");
return err;
}
return ifreq.ifr_ifindex;
}
static int open_socket(const char *name, struct in_addr mc_addr, short port)
{
struct sockaddr_in addr;
int fd, index, on = 1;
int priority = 3;
memset(&addr, 0, sizeof(addr));
addr.sin_family = AF_INET;
addr.sin_addr.s_addr = htonl(INADDR_ANY);
addr.sin_port = htons(port);
fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
if (fd < 0) {
pr_err("socket failed: %m");
goto no_socket;
}
index = sk_interface_index(fd, name);
if (index < 0)
goto no_option;
if (setsockopt(fd, SOL_SOCKET, SO_PRIORITY, &priority, sizeof(priority))) {
pr_err("Couldn't set priority");
goto no_option;
}
if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on))) {
pr_err("setsockopt SO_REUSEADDR failed: %m");
goto no_option;
}
if (bind(fd, (struct sockaddr *) &addr, sizeof(addr))) {
pr_err("bind failed: %m");
goto no_option;
}
if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, name, strlen(name))) {
pr_err("setsockopt SO_BINDTODEVICE failed: %m");
goto no_option;
}
addr.sin_addr = mc_addr;
if (mcast_join(fd, index, (struct sockaddr *) &addr, sizeof(addr))) {
pr_err("mcast_join failed");
goto no_option;
}
if (mcast_bind(fd, index)) {
goto no_option;
}
if (use_so_txtime && setsockopt(fd, SOL_SOCKET, SO_TXTIME, &on, sizeof(on))) {
pr_err("setsockopt SO_TXTIME failed: %m");
goto no_option;
}
return fd;
no_option:
close(fd);
no_socket:
return -1;
}
static int udp_open(const char *name)
{
int fd;
if (!inet_aton(MCAST_IPADDR, &mcast_addr))
return -1;
fd = open_socket(name, mcast_addr, UDP_PORT);
return fd;
}
static int udp_send(int fd, void *buf, int len, __u64 txtime)
{
union {
char buf[CMSG_SPACE(sizeof(__u64))];
struct cmsghdr align;
} u;
struct sockaddr_in sin;
struct cmsghdr *cmsg;
struct msghdr msg;
struct iovec iov;
ssize_t cnt;
memset(&sin, 0, sizeof(sin));
sin.sin_family = AF_INET;
sin.sin_addr = mcast_addr;
sin.sin_port = htons(UDP_PORT);
iov.iov_base = buf;
iov.iov_len = len;
memset(&msg, 0, sizeof(msg));
msg.msg_name = &sin;
msg.msg_namelen = sizeof(sin);
msg.msg_iov = &iov;
msg.msg_iovlen = 1;
/*
* We specify the transmission time in the CMSG.
*/
if (use_so_txtime) {
msg.msg_control = u.buf;
msg.msg_controllen = sizeof(u.buf);
cmsg = CMSG_FIRSTHDR(&msg);
cmsg->cmsg_level = SOL_SOCKET;
cmsg->cmsg_type = SO_TXTIME;
cmsg->cmsg_len = CMSG_LEN(sizeof(__u64));
*((__u64 *) CMSG_DATA(cmsg)) = txtime;
}
cnt = sendmsg(fd, &msg, 0);
if (cnt < 1) {
pr_err("sendmsg failed: %m");
return cnt;
}
return cnt;
}
static unsigned char tx_buffer[256];
static int marker;
static int run_nanosleep(clockid_t clkid, int fd)
{
struct timespec ts;
int cnt, err;
__u64 txtime;
clock_gettime(clkid, &ts);
/* Start one to two seconds in the future. */
ts.tv_sec += 1;
ts.tv_nsec = 1000000000 - waketx_delay;
normalize(&ts);
txtime = ts.tv_sec * 1000000000ULL + ts.tv_nsec;
txtime += waketx_delay;
while (running) {
err = clock_nanosleep(clkid, TIMER_ABSTIME, &ts, NULL);
switch (err) {
case 0:
cnt = udp_send(fd, tx_buffer, sizeof(tx_buffer), txtime);
if (cnt != sizeof(tx_buffer)) {
pr_err("udp_send failed");
}
memset(tx_buffer, marker++, sizeof(tx_buffer));
ts.tv_nsec += period_nsec;
normalize(&ts);
txtime += period_nsec;
break;
case EINTR:
continue;
default:
fprintf(stderr, "clock_nanosleep returned %d: %s",
err, strerror(err));
return err;
}
}
return 0;
}
static int set_realtime(pthread_t thread, int priority, int cpu)
{
cpu_set_t cpuset;
struct sched_param sp;
int err, policy;
int min = sched_get_priority_min(SCHED_FIFO);
int max = sched_get_priority_max(SCHED_FIFO);
fprintf(stderr, "min %d max %d\n", min, max);
if (priority < 0) {
return 0;
}
err = pthread_getschedparam(thread, &policy, &sp);
if (err) {
fprintf(stderr, "pthread_getschedparam: %s\n", strerror(err));
return -1;
}
sp.sched_priority = priority;
err = pthread_setschedparam(thread, SCHED_FIFO, &sp);
if (err) {
fprintf(stderr, "pthread_setschedparam: %s\n", strerror(err));
return -1;
}
if (cpu < 0) {
return 0;
}
CPU_ZERO(&cpuset);
CPU_SET(cpu, &cpuset);
err = pthread_setaffinity_np(thread, sizeof(cpu_set_t), &cpuset);
if (err) {
fprintf(stderr, "pthread_setaffinity_np: %s\n", strerror(err));
return -1;
}
return 0;
}
static void usage(char *progname)
{
fprintf(stderr,
"\n"
"usage: %s [options]\n"
"\n"
" -c [num] run on CPU 'num'\n"
" -d [num] delay from wake up to transmission in nanoseconds (default %d)\n"
" -h prints this message and exits\n"
" -i [name] use network interface 'name'\n"
" -p [num] run with RT priorty 'num'\n"
" -P [num] period in nanoseconds (default %d)\n"
" -u do not use SO_TXTIME\n"
"\n",
progname, DEFAULT_DELAY, DEFAULT_PERIOD);
}
int main(int argc, char *argv[])
{
int c, cpu = -1, err, fd, priority = -1;
clockid_t clkid = CLOCK_TAI;
char *iface = NULL, *progname;
/* Process the command line arguments. */
progname = strrchr(argv[0], '/');
progname = progname ? 1 + progname : argv[0];
while (EOF != (c = getopt(argc, argv, "c:d:hi:p:P:u"))) {
switch (c) {
case 'c':
cpu = atoi(optarg);
break;
case 'd':
waketx_delay = atoi(optarg);
break;
case 'h':
usage(progname);
return 0;
case 'i':
iface = optarg;
break;
case 'p':
priority = atoi(optarg);
break;
case 'P':
period_nsec = atoi(optarg);
break;
case 'u':
use_so_txtime = 0;
break;
case '?':
usage(progname);
return -1;
}
}
if (waketx_delay > 999999999 || waketx_delay < 0) {
pr_err("Bad wake up to transmission delay.");
usage(progname);
return -1;
}
if (period_nsec < 1000) {
pr_err("Bad period.");
usage(progname);
return -1;
}
if (!iface) {
pr_err("Need a network interface.");
usage(progname);
return -1;
}
if (set_realtime(pthread_self(), priority, cpu)) {
return -1;
}
fd = udp_open(iface);
if (fd < 0) {
return -1;
}
err = run_nanosleep(clkid, fd);
close(fd);
return err;
}
From: Jesus Sanchez-Palencia <hidden> Date: 2018-01-17 23:07:40
From: Richard Cochran <redacted>
For raw layer-2 packets, copy the desired future transmit time from
the CMSG cookie into the skb.
Signed-off-by: Richard Cochran <redacted>
Signed-off-by: Jesus Sanchez-Palencia <redacted>
---
net/packet/af_packet.c | 6 ++++++
1 file changed, 6 insertions(+)
From: Jesus Sanchez-Palencia <hidden> Date: 2018-01-17 23:07:42
From: Richard Cochran <redacted>
For udp packets, copy the desired future transmit time from the CMSG
cookie into the skb.
Signed-off-by: Richard Cochran <redacted>
Signed-off-by: Jesus Sanchez-Palencia <redacted>
---
net/ipv4/udp.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
From: Jesus Sanchez-Palencia <hidden> Date: 2018-01-17 23:07:42
From: Richard Cochran <redacted>
For raw packets, copy the desired future transmit time from the CMSG
cookie into the skb.
Signed-off-by: Richard Cochran <redacted>
Signed-off-by: Jesus Sanchez-Palencia <redacted>
---
net/ipv4/raw.c | 2 ++
1 file changed, 2 insertions(+)
From: Jesus Sanchez-Palencia <hidden> Date: 2018-01-17 23:07:42
From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
This adds 'qdisc_watchdog_init_clockid()' that allows a clockid to be
passed, this allows other time references to be used when scheduling
the Qdisc to run.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
include/net/pkt_sched.h | 2 ++
net/sched/sch_api.c | 11 +++++++++--
2 files changed, 11 insertions(+), 2 deletions(-)
From: Jesus Sanchez-Palencia <hidden> Date: 2018-01-17 23:07:43
Make this function retrieve what it needs from the Tx ring being
addressed since it already relies on what had been saved on it before.
Also, since this function will be used by the upcoming Launchtime
patches rename it to better reflect its intention. Note that
Launchtime is not part of what 802.1Qav specifies, but the i210
datasheet refers to this set of functionality as "Qav Transmission
Mode".
Here we also perform a tiny refactor at is_any_cbs_enabled(), and add
further documentation to igb_setup_tx_mode().
Signed-off-by: Jesus Sanchez-Palencia <redacted>
---
drivers/net/ethernet/intel/igb/igb_main.c | 54 ++++++++++++++-----------------
1 file changed, 25 insertions(+), 29 deletions(-)
From: Jesus Sanchez-Palencia <hidden> Date: 2018-01-17 23:07:43
Currently the data transmission arbitration algorithm - DataTranARB
field on TQAVCTRL reg - is always set to CBS when the Tx mode is
changed from legacy to 'Qav' mode.
Make that configuration a bit more granular in preparation for the
upcoming Launchtime enabling patches, since CBS and Launchtime can be
enabled separately. That is achieved by moving the DataTranARB setup
to igb_config_tx_modes() instead.
Similarly, when disabling CBS we must check if it has been disabled
for all queues, and clear the DataTranARB accordingly.
Signed-off-by: Jesus Sanchez-Palencia <redacted>
---
drivers/net/ethernet/intel/igb/igb_main.c | 49 +++++++++++++++++++++----------
1 file changed, 33 insertions(+), 16 deletions(-)
@@ -1696,6 +1708,14 @@ static void igb_config_tx_modes(struct igb_adapter *adapter, int queue)set_tx_desc_fetch_prio(hw,queue,TX_QUEUE_PRIO_HIGH);set_queue_mode(hw,queue,QUEUE_MODE_STREAM_RESERVATION);+/* Always set data transfer arbitration to credit-based+*shaperalgorithmonTQAVCTRLifCBSisenabledforanyof+*thequeues.+*/+tqavctrl=rd32(E1000_I210_TQAVCTRL);+tqavctrl|=E1000_TQAVCTRL_DATATRANARB;+wr32(E1000_I210_TQAVCTRL,tqavctrl);+/* According to i210 datasheet section 7.2.7.7, we should set*the'idleSlope'fieldfromTQAVCCregisterfollowingthe*equation:
@@ -1759,6 +1779,16 @@ static void igb_config_tx_modes(struct igb_adapter *adapter, int queue)/* Set hiCredit to zero. */wr32(E1000_I210_TQAVHC(queue),0);++/* If CBS is not enabled for any queues anymore, then return to+*thedefaultstateofDataTransmissionArbitrationon+*TQAVCTRL.+*/+if(!is_any_cbs_enabled(adapter)){+tqavctrl=rd32(E1000_I210_TQAVCTRL);+tqavctrl&=~E1000_TQAVCTRL_DATATRANARB;+wr32(E1000_I210_TQAVCTRL,tqavctrl);+}}/* XXX: In i210 controller the sendSlope and loCredit parameters from
@@ -1792,18 +1822,6 @@ static int igb_save_cbs_params(struct igb_adapter *adapter, int queue,return0;}-staticboolis_any_cbs_enabled(structigb_adapter*adapter)-{-inti;--for(i=0;i<adapter->num_tx_queues;i++){-if(adapter->tx_ring[i]->cbs_enable)-returntrue;-}--returnfalse;-}-/***igb_setup_tx_mode-Switchto/fromQavTxmodewhenapplicable*@adapter:pointertoadapterstruct
@@ -1827,11 +1845,10 @@ static void igb_setup_tx_mode(struct igb_adapter *adapter)inti,max_queue;/* Configure TQAVCTRL register: set transmit mode to 'Qav',-*setdatafetcharbitrationto'roundrobin'andsetdata-*transferarbitrationto'creditshaperalgorithm.+*setdatafetcharbitrationto'roundrobin'.*/val=rd32(E1000_I210_TQAVCTRL);-val|=E1000_TQAVCTRL_XMIT_MODE|E1000_TQAVCTRL_DATATRANARB;+val|=E1000_TQAVCTRL_XMIT_MODE;val&=~E1000_TQAVCTRL_DATAFETCHARB;wr32(E1000_I210_TQAVCTRL,val);
From: Jesus Sanchez-Palencia <hidden> Date: 2018-01-17 23:07:43
From: Richard Cochran <redacted>
This patch introduces SO_TXTIME. User space enables this option in
order to pass a desired future transmit time in a CMSG when calling
sendmsg(2).
A new field is added to struct sockcm_cookie, and the tstamp from
skbuffs will be used later on.
Signed-off-by: Richard Cochran <redacted>
Signed-off-by: Jesus Sanchez-Palencia <redacted>
---
arch/alpha/include/uapi/asm/socket.h | 3 +++
arch/frv/include/uapi/asm/socket.h | 3 +++
arch/ia64/include/uapi/asm/socket.h | 3 +++
arch/m32r/include/uapi/asm/socket.h | 3 +++
arch/mips/include/uapi/asm/socket.h | 3 +++
arch/mn10300/include/uapi/asm/socket.h | 3 +++
arch/parisc/include/uapi/asm/socket.h | 3 +++
arch/s390/include/uapi/asm/socket.h | 3 +++
arch/sparc/include/uapi/asm/socket.h | 3 +++
arch/xtensa/include/uapi/asm/socket.h | 3 +++
include/net/sock.h | 2 ++
include/uapi/asm-generic/socket.h | 3 +++
net/core/sock.c | 16 ++++++++++++++++
13 files changed, 51 insertions(+)
@@ -1061,6 +1061,13 @@ int sock_setsockopt(struct socket *sock, int level, int optname,sock_valbool_flag(sk,SOCK_ZEROCOPY,valbool);break;+caseSO_TXTIME:+if(ns_capable(sock_net(sk)->user_ns,CAP_NET_ADMIN))+sock_valbool_flag(sk,SOCK_TXTIME,valbool);+else+ret=-EPERM;+break;+default:ret=-ENOPROTOOPT;break;
@@ -2130,6 +2137,15 @@ int __sock_cmsg_send(struct sock *sk, struct msghdr *msg, struct cmsghdr *cmsg,sockc->tsflags&=~SOF_TIMESTAMPING_TX_RECORD_MASK;sockc->tsflags|=tsflags;break;+caseSO_TXTIME:+if(!ns_capable(sock_net(sk)->user_ns,CAP_NET_ADMIN))+return-EPERM;+if(!sock_flag(sk,SOCK_TXTIME))+return-EINVAL;+if(cmsg->cmsg_len!=CMSG_LEN(sizeof(ktime_t)))+return-EINVAL;+sockc->transmit_time=*(ktime_t*)CMSG_DATA(cmsg);+break;/* SCM_RIGHTS and SCM_CREDENTIALS are semantically in SOL_UNIX. */caseSCM_RIGHTS:caseSCM_CREDENTIALS:
From: Jesus Sanchez-Palencia <hidden> Date: 2018-01-17 23:07:43
From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
TBS (Time Based Scheduler) uses the information added earlier in this
series (the socket option SO_TXTIME and the new role of
sk_buff->tstamp) to schedule traffic transmission based on absolute
time.
For some workloads, just bandwidth enforcement is not enough, and
precise control of the transmission of packets is necessary.
Example:
$ tc qdisc replace dev enp2s0 parent root handle 100 mqprio num_tc 3 \
map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 queues 1@0 1@1 2@2 hw 0
$ tc qdisc add dev enp2s0 parent 100:1 tbs delta 60000 clockid 11 offload 1
In this example, the Qdisc will try to enable offloading (offload 1)
the control of the transmission time to the network adapter, the
time stamp in socket are in reference to the clockid '11' (CLOCK_TAI)
and packets leave the Qdisc "delta" (60000) nanoseconds before its
transmission time.
When offloading is disabled, the network adapter will ignore the
sk_buff time stamp, and so, the transmission time will be only "best
effort" from the Qdisc.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Signed-off-by: Jesus Sanchez-Palencia <redacted>
---
include/linux/netdevice.h | 1 +
include/net/pkt_sched.h | 5 +
include/uapi/linux/pkt_sched.h | 17 ++
net/sched/Kconfig | 11 ++
net/sched/Makefile | 1 +
net/sched/sch_tbs.c | 392 +++++++++++++++++++++++++++++++++++++++++
6 files changed, 427 insertions(+)
create mode 100644 net/sched/sch_tbs.c
@@ -781,6 +781,7 @@ enum tc_setup_type {TC_SETUP_QDISC_CBS,TC_SETUP_QDISC_RED,TC_SETUP_QDISC_PRIO,+TC_SETUP_QDISC_TBS,};/* These structures hold the attributes of bpf state that are being passed
@@ -183,6 +183,17 @@ config NET_SCH_CBSTocompilethiscodeasamodule,chooseMhere:themodulewillbecalledsch_cbs.+configNET_SCH_TBS+tristate"Time Based Scheduler (TBS)"+---help---+SayYhereifyouwanttousetheTimeBasedScheduler(TBS)packet+schedulingalgorithm.++Seethetopof<file:net/sched/sch_tbs.c>formoredetails.++Tocompilethiscodeasamodule,chooseMhere:the+modulewillbecalledsch_tbs.+configNET_SCH_GREDtristate"Generic Random Early Detection (GRED)"---help---
@@ -0,0 +1,392 @@+/*+*net/sched/sch_tbs.cTimeBasedShaper+*+*Thisprogramisfreesoftware;youcanredistributeitand/or+*modifyitunderthetermsoftheGNUGeneralPublicLicense+*aspublishedbytheFreeSoftwareFoundation;eitherversion+*2oftheLicense,or(atyouroption)anylaterversion.+*+*Authors:ViniciusCostaGomes<vinicius.gomes@intel.com>+*JesusSanchez-Palencia<jesus.sanchez-palencia@intel.com>+*+*/++#include<linux/module.h>+#include<linux/types.h>+#include<linux/kernel.h>+#include<linux/string.h>+#include<linux/errno.h>+#include<linux/rbtree.h>+#include<linux/skbuff.h>+#include<net/netlink.h>+#include<net/sch_generic.h>+#include<net/pkt_sched.h>+#include<net/pkt_sched.h>+#include<net/sock.h>++structtbs_sched_data{+booloffload;+intclockid;+intqueue;+s32delta;/* in ns */+ktime_tlast;/* The txtime of the last skb sent to the netdevice. */+structrb_roothead;+structqdisc_watchdogwatchdog;+structQdisc*qdisc;+};++staticconststructnla_policytbs_policy[TCA_TBS_MAX+1]={+[TCA_TBS_PARMS]={.len=sizeof(structtc_tbs_qopt)},+};++typedefktime_t(*get_time_func_t)(void);++staticconstget_time_func_tclockid_to_get_time[MAX_CLOCKS]={+[CLOCK_MONOTONIC]=ktime_get,+[CLOCK_REALTIME]=ktime_get_real,+[CLOCK_BOOTTIME]=ktime_get_boottime,+[CLOCK_TAI]=ktime_get_clocktai,+};++staticktime_tget_time_by_clockid(clockid_tclockid)+{+get_time_func_tfunc=clockid_to_get_time[clockid];++if(!func)+return0;++returnfunc();+}++staticstructsk_buff*tbs_peek(structQdisc*sch)+{+structtbs_sched_data*q=qdisc_priv(sch);+structrb_node*p;++p=rb_first(&q->head);+if(!p)+returnNULL;++returnrb_to_skb(p);+}++staticvoidreset_watchdog(structQdisc*sch)+{+structtbs_sched_data*q=qdisc_priv(sch);+structsk_buff*skb=tbs_peek(sch);+ktime_tnext;++if(!skb)+return;++next=ktime_sub_ns(skb->tstamp,q->delta);+qdisc_watchdog_schedule_ns(&q->watchdog,ktime_to_ns(next));+}++staticinttbs_enqueue(structsk_buff*nskb,structQdisc*sch,+structsk_buff**to_free)+{+structtbs_sched_data*q=qdisc_priv(sch);+structrb_node**p=&q->head.rb_node,*parent=NULL;+ktime_ttxtime=nskb->tstamp;+structsock*sk=nskb->sk;+ktime_tnow;++if(sk&&!sock_flag(sk,SOCK_TXTIME))+gotodrop;++now=get_time_by_clockid(q->clockid);++if(ktime_before(txtime,now)||ktime_before(txtime,q->last))+gotodrop;++while(*p){+structsk_buff*skb;++parent=*p;+skb=rb_to_skb(parent);+if(ktime_after(txtime,skb->tstamp))+p=&parent->rb_right;+else+p=&parent->rb_left;+}+rb_link_node(&nskb->rbnode,parent,p);+rb_insert_color(&nskb->rbnode,&q->head);++qdisc_qstats_backlog_inc(sch,nskb);+sch->q.qlen++;++/* Now we may need to re-arm the qdisc watchdog for the next packet. */+reset_watchdog(sch);++returnNET_XMIT_SUCCESS;++drop:+returnqdisc_drop(nskb,sch,to_free);+}++staticstructsk_buff*timerqueue_erase(structQdisc*sch,+structsk_buff*skb,booldrop)+{+structtbs_sched_data*q=qdisc_priv(sch);++rb_erase(&skb->rbnode,&q->head);++if(drop){+structsk_buff*to_free=NULL;++qdisc_drop(skb,sch,&to_free);+kfree_skb_list(to_free);+}else{+qdisc_qstats_backlog_dec(sch,skb);+qdisc_bstats_update(sch,skb);++q->last=skb->tstamp;+}++sch->q.qlen--;++/* The rbnode field in the skb re-uses these fields, now that+*wearedonewiththerbnode,resetthem.+*/+skb->next=NULL;+skb->prev=NULL;+skb->dev=qdisc_dev(sch);++returnskb;+}++staticstructsk_buff*tbs_dequeue(structQdisc*sch)+{+structtbs_sched_data*q=qdisc_priv(sch);+structsk_buff*skb;+ktime_tnow,next;++skb=tbs_peek(sch);+if(!skb)+returnNULL;++now=get_time_by_clockid(q->clockid);++/* If packet has expired while in queue, drop it. */+if(ktime_before(skb->tstamp,now)){+timerqueue_erase(sch,skb,true);+skb=NULL;+gotoout;+}++next=ktime_sub_ns(skb->tstamp,q->delta);++/* Dequeue only if now is within the [txtime - delta, txtime] range. */+if(ktime_after(now,next))+timerqueue_erase(sch,skb,false);+else+skb=NULL;++out:+/* Now we may need to re-arm the qdisc watchdog for the next packet. */+reset_watchdog(sch);++returnskb;+}++staticvoidtbs_disable_offload(structnet_device*dev,+structtbs_sched_data*q)+{+structtc_tbs_qopt_offloadtbs={};+conststructnet_device_ops*ops;+interr;++if(!q->offload)+return;++ops=dev->netdev_ops;+if(!ops->ndo_setup_tc)+return;++tbs.queue=q->queue;+tbs.enable=0;++err=ops->ndo_setup_tc(dev,TC_SETUP_QDISC_TBS,&tbs);+if(err<0)+pr_warn("Couldn't disable TBS offload for queue %d\n",+tbs.queue);+}++staticinttbs_enable_offload(structnet_device*dev,structtbs_sched_data*q,+structnetlink_ext_ack*extack)+{+conststructnet_device_ops*ops=dev->netdev_ops;+structtc_tbs_qopt_offloadtbs={};+interr;++if(q->offload)+return0;++if(!ops->ndo_setup_tc){+NL_SET_ERR_MSG(extack,"Specified device does not support TBS offload");+return-EOPNOTSUPP;+}++tbs.queue=q->queue;+tbs.enable=1;++err=ops->ndo_setup_tc(dev,TC_SETUP_QDISC_TBS,&tbs);+if(err<0){+NL_SET_ERR_MSG(extack,"Specified device failed to setup TBS hardware offload");+returnerr;+}++return0;+}++staticinttbs_change(structQdisc*sch,structnlattr*opt,+structnetlink_ext_ack*extack)+{+structtbs_sched_data*q=qdisc_priv(sch);+structnet_device*dev=qdisc_dev(sch);+structnlattr*tb[TCA_CBS_MAX+1];+structtc_tbs_qopt*qopt;+interr;++err=nla_parse_nested(tb,TCA_TBS_MAX,opt,tbs_policy,extack);+if(err<0)+returnerr;++if(!tb[TCA_TBS_PARMS]){+NL_SET_ERR_MSG(extack,"Missing mandatory TBS parameters");+return-EINVAL;+}++qopt=nla_data(tb[TCA_TBS_PARMS]);++if(qopt->clockid<0||qopt->clockid>=MAX_CLOCKS||+!clockid_to_get_time[qopt->clockid]){+NL_SET_ERR_MSG(extack,"Invalid clockid");+return-EINVAL;+}++pr_debug("delta %d clockid %d offload %d\n",+qopt->delta,qopt->clockid,qopt->offload);++if(!qopt->offload){+tbs_disable_offload(dev,q);+}else{+err=tbs_enable_offload(dev,q,extack);+if(err<0)+returnerr;+}++/* Everything went OK, save the parameters used. */+q->delta=qopt->delta;+q->clockid=qopt->clockid;+q->offload=qopt->offload;++qdisc_watchdog_init_clockid(&q->watchdog,sch,q->clockid);++return0;+}++staticinttbs_init(structQdisc*sch,structnlattr*opt,+structnetlink_ext_ack*extack)+{+structtbs_sched_data*q=qdisc_priv(sch);+structnet_device*dev=qdisc_dev(sch);++if(!opt){+NL_SET_ERR_MSG(extack,"Missing TBS qdisc options which are mandatory");+return-EINVAL;+}++q->queue=sch->dev_queue-netdev_get_tx_queue(dev,0);++returntbs_change(sch,opt,extack);+}++staticvoidtimerqueue_clear(structQdisc*sch)+{+structtbs_sched_data*q=qdisc_priv(sch);+structrb_node*p=rb_first(&q->head);++while(p){+structsk_buff*skb=rb_to_skb(p);++p=rb_next(p);+rb_erase(&skb->rbnode,&q->head);+rtnl_kfree_skbs(skb,skb);+}+}++staticvoidtbs_reset(structQdisc*sch)+{+structtbs_sched_data*q=qdisc_priv(sch);++qdisc_watchdog_cancel(&q->watchdog);+timerqueue_clear(sch);++sch->qstats.backlog=0;+sch->q.qlen=0;++q->last=0;+}++staticvoidtbs_destroy(structQdisc*sch)+{+structtbs_sched_data*q=qdisc_priv(sch);+structnet_device*dev=qdisc_dev(sch);++qdisc_watchdog_cancel(&q->watchdog);+timerqueue_clear(sch);+tbs_disable_offload(dev,q);+}++staticinttbs_dump(structQdisc*sch,structsk_buff*skb)+{+structtbs_sched_data*q=qdisc_priv(sch);+structtc_tbs_qoptopt={};+structnlattr*nest;++nest=nla_nest_start(skb,TCA_OPTIONS);+if(!nest)+gotonla_put_failure;++opt.delta=q->delta;+opt.clockid=q->clockid;+opt.offload=q->offload;++if(nla_put(skb,TCA_TBS_PARMS,sizeof(opt),&opt))+gotonla_put_failure;++returnnla_nest_end(skb,nest);++nla_put_failure:+nla_nest_cancel(skb,nest);+return-1;+}++staticstructQdisc_opstbs_qdisc_ops__read_mostly={+.id="tbs",+.priv_size=sizeof(structtbs_sched_data),+.enqueue=tbs_enqueue,+.dequeue=tbs_dequeue,+.peek=tbs_peek,+.init=tbs_init,+.reset=tbs_reset,+.destroy=tbs_destroy,+.change=tbs_change,+.dump=tbs_dump,+.owner=THIS_MODULE,+};++staticint__inittbs_module_init(void)+{+returnregister_qdisc(&tbs_qdisc_ops);+}++staticvoid__exittbs_module_exit(void)+{+unregister_qdisc(&tbs_qdisc_ops);+}+module_init(tbs_module_init)+module_exit(tbs_module_exit)+MODULE_LICENSE("GPL");
From: Jesus Sanchez-Palencia <hidden> Date: 2018-01-17 23:07:44
Implement HW offload support for SO_TXTIME through igb's Launchtime
feature. This is done by extending igb_setup_tc() so it supports
TC_SETUP_QDISC_TBS and configuring i210 so time based transmit
arbitration is enabled.
The FQTSS transmission mode added before is extended so strict
priority (SP) queues wait for stream reservation (SR) ones.
igb_config_tx_modes() is extended so it can support enabling/disabling
Launchtime following the previous approach used for the credit-based
shaper (CBS).
As the previous flow, FQTSS transmission mode is enabled automatically
by the driver once Launchtime (or CBS, as before) is enabled.
Similarly, it's automatically disabled when the feature is disabled
for the last queue that had it setup on.
The driver just consumes the transmit times from the skbuffs directly,
so no special handling is done in case an 'invalid' time is provided.
We assume this has been handled by the TBS qdisc already.
Signed-off-by: Jesus Sanchez-Palencia <redacted>
---
drivers/net/ethernet/intel/igb/e1000_defines.h | 16 +++
drivers/net/ethernet/intel/igb/igb.h | 1 +
drivers/net/ethernet/intel/igb/igb_main.c | 135 ++++++++++++++++++++++---
3 files changed, 137 insertions(+), 15 deletions(-)
@@ -281,6 +281,7 @@ struct igb_ring {u16count;/* number of desc. in the ring */u8queue_index;/* logical index of the ring*/u8reg_idx;/* physical index of the ring */+boollaunchtime_enable;/* true if LaunchTime is enabled */boolcbs_enable;/* indicates if CBS is enabled */s32idleslope;/* idleSlope in kbps */s32sendslope;/* sendSlope in kbps */
@@ -1704,10 +1717,20 @@ static void igb_config_tx_modes(struct igb_adapter *adapter, int queue)WARN_ON(hw->mac.type!=e1000_i210);WARN_ON(queue<0||queue>1);-if(ring->cbs_enable){+/* If any of the Qav features is enabled, configure queues as SR and+*withHIGHPRIO.Ifnoneis,thenconfigurethemwithLOWPRIOand+*asSP.+*/+if(ring->cbs_enable||ring->launchtime_enable){set_tx_desc_fetch_prio(hw,queue,TX_QUEUE_PRIO_HIGH);set_queue_mode(hw,queue,QUEUE_MODE_STREAM_RESERVATION);+}else{+set_tx_desc_fetch_prio(hw,queue,TX_QUEUE_PRIO_LOW);+set_queue_mode(hw,queue,QUEUE_MODE_STRICT_PRIORITY);+}+/* If CBS is enabled, set DataTranARB and config its parameters. */+if(ring->cbs_enable){/* Always set data transfer arbitration to credit-based*shaperalgorithmonTQAVCTRLifCBSisenabledforanyof*thequeues.
@@ -1769,8 +1792,6 @@ static void igb_config_tx_modes(struct igb_adapter *adapter, int queue)wr32(E1000_I210_TQAVHC(queue),0x80000000+ring->hicredit*0x7735);}else{-set_tx_desc_fetch_prio(hw,queue,TX_QUEUE_PRIO_LOW);-set_queue_mode(hw,queue,QUEUE_MODE_STRICT_PRIORITY);/* Set idleSlope to zero. */tqavcc=rd32(E1000_I210_TQAVCC(queue));
@@ -1791,17 +1812,61 @@ static void igb_config_tx_modes(struct igb_adapter *adapter, int queue)}}+/* If LaunchTime is enabled, set DataTranTIM. */+if(ring->launchtime_enable){+/* Always set DataTranTIM on TQAVCTRL if LaunchTime is enabled+*foranyoftheSRqueues,andconfigurefetchtimedelta.+*XXXNOTE:+*-LaunchTimewillbeenabledforallSRqueues.+*-Afixedoffsetcanbeaddedrelativetothelaunch+*timeofallpacketsifconfiguredatregLAUNCH_OS0.+*Wearekeepingitas0fornow(defaultvalue).+*/+tqavctrl=rd32(E1000_I210_TQAVCTRL);+tqavctrl|=E1000_TQAVCTRL_DATATRANTIM|+E1000_TQAVCTRL_FETCHTIME_DELTA;+wr32(E1000_I210_TQAVCTRL,tqavctrl);+}else{+/* If Launchtime is not enabled for any SR queues anymore,+*thenclearDataTranTIMonTQAVCTRLandclearfetchtimedelta,+*effectivelydisablingLaunchtime.+*/+if(!is_any_txtime_enabled(adapter)){+tqavctrl=rd32(E1000_I210_TQAVCTRL);+tqavctrl&=~E1000_TQAVCTRL_DATATRANTIM;+tqavctrl&=~E1000_TQAVCTRL_FETCHTIME_DELTA;+wr32(E1000_I210_TQAVCTRL,tqavctrl);+}+}+/* XXX: In i210 controller the sendSlope and loCredit parameters from*CBSarenotconfigurablebysoftwaresowedon'tdoany'controller*configuration'inrespecttotheseparameters.*/-netdev_dbg(netdev,"CBS %s: queue %d idleslope %d sendslope %d hiCredit %d locredit %d\n",-(ring->cbs_enable)?"enabled":"disabled",queue,+netdev_dbg(netdev,"Qav Tx mode: cbs %s, launchtime %s, queue %d \+idleslope%dsendslope%dhiCredit%d\+locredit%d\n",+(ring->cbs_enable)?"enabled":"disabled",+(ring->launchtime_enable)?"enabled":"disabled",queue,ring->idleslope,ring->sendslope,ring->hicredit,ring->locredit);}+staticintigb_save_txtime_params(structigb_adapter*adapter,intqueue,+boolenable)+{+structigb_ring*ring;++if(queue<0||queue>adapter->num_tx_queues)+return-EINVAL;++ring=adapter->tx_ring[queue];+ring->launchtime_enable=enable;++return0;+}+staticintigb_save_cbs_params(structigb_adapter*adapter,intqueue,boolenable,intidleslope,intsendslope,inthicredit,intlocredit)
@@ -1845,10 +1910,11 @@ static void igb_setup_tx_mode(struct igb_adapter *adapter)inti,max_queue;/* Configure TQAVCTRL register: set transmit mode to 'Qav',-*setdatafetcharbitrationto'roundrobin'.+*setdatafetcharbitrationto'roundrobin',setSP_WAIT_SR+*soSPqueueswaitforSRones.*/val=rd32(E1000_I210_TQAVCTRL);-val|=E1000_TQAVCTRL_XMIT_MODE;+val|=E1000_TQAVCTRL_XMIT_MODE|E1000_TQAVCTRL_SP_WAIT_SR;val&=~E1000_TQAVCTRL_DATAFETCHARB;wr32(E1000_I210_TQAVCTRL,val);
@@ -2500,6 +2566,30 @@ static int igb_offload_cbs(struct igb_adapter *adapter,return0;}+staticintigb_offload_txtime(structigb_adapter*adapter,+structtc_tbs_qopt_offload*qopt)+{+structe1000_hw*hw=&adapter->hw;+interr;++/* Launchtime offloading is only supported by i210 controller. */+if(hw->mac.type!=e1000_i210)+return-EOPNOTSUPP;++/* Launchtime offloading is only supported by queues 0 and 1. */+if(qopt->queue<0||qopt->queue>1)+return-EINVAL;++err=igb_save_txtime_params(adapter,qopt->queue,qopt->enable);++if(err)+returnerr;++igb_offload_apply(adapter,qopt->queue);++return0;+}+staticintigb_setup_tc(structnet_device*dev,enumtc_setup_typetype,void*type_data){
@@ -5332,9 +5427,18 @@ static void igb_tx_ctxtdesc(struct igb_ring *tx_ring, u32 vlan_macip_lens,mss_l4len_idx|=tx_ring->reg_idx<<4;context_desc->vlan_macip_lens=cpu_to_le32(vlan_macip_lens);-context_desc->seqnum_seed=0;context_desc->type_tucmd_mlhl=cpu_to_le32(type_tucmd);context_desc->mss_l4len_idx=cpu_to_le32(mss_l4len_idx);++/* We assume there is always a valid tx time available. Invalid times+*shouldhavebeenhandledbytheupperlayers.+*/+if(tx_ring->launchtime_enable){+ts=ns_to_timespec64(first->skb->tstamp);+context_desc->seqnum_seed=cpu_to_le32(ts.tv_nsec/32);+}else{+context_desc->seqnum_seed=0;+}}staticintigb_tso(structigb_ring*tx_ring,
@@ -5417,7 +5521,8 @@ static int igb_tso(struct igb_ring *tx_ring,vlan_macip_lens|=(ip.hdr-skb->data)<<E1000_ADVTXD_MACLEN_SHIFT;vlan_macip_lens|=first->tx_flags&IGB_TX_FLAGS_VLAN_MASK;-igb_tx_ctxtdesc(tx_ring,vlan_macip_lens,type_tucmd,mss_l4len_idx);+igb_tx_ctxtdesc(tx_ring,first,vlan_macip_lens,+type_tucmd,mss_l4len_idx);return1;}
From: Jesus Sanchez-Palencia <hidden> Date: 2018-01-17 23:07:44
Split code into a separate function (igb_offload_apply()) that will be
used by TBS offload implementation.
Signed-off-by: Jesus Sanchez-Palencia <redacted>
---
drivers/net/ethernet/intel/igb/igb_main.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
From: Eric Dumazet <hidden> Date: 2018-01-18 00:28:20
On Wed, 2018-01-17 at 15:06 -0800, Jesus Sanchez-Palencia wrote:
quoted hunk
From: Richard Cochran <redacted>
For raw packets, copy the desired future transmit time from the CMSG
cookie into the skb.
Signed-off-by: Richard Cochran <redacted>
Signed-off-by: Jesus Sanchez-Palencia <redacted>
---
net/ipv4/raw.c | 2 ++
1 file changed, 2 insertions(+)
It seems that skb_scrub_packet() will clear skb->tstamp, meaning that
going through a tunnel will break your feature.
Maybe we need to remove skb->tstamp clear from skb_scrub_packet() and
do the cleaning only in forwarding path.
From: Miroslav Lichvar <hidden> Date: 2018-01-18 08:42:54
On Wed, Jan 17, 2018 at 03:06:12PM -0800, Jesus Sanchez-Palencia wrote:
From: Richard Cochran <redacted>
This patch introduces SO_TXTIME. User space enables this option in
order to pass a desired future transmit time in a CMSG when calling
sendmsg(2).
A new field is added to struct sockcm_cookie, and the tstamp from
skbuffs will be used later on.
In the discussion about the v1 patchset, there was a question if the
cmsg should include a clockid_t. Without that, how can an application
prevent the packet from being sent using an incorrect clock, e.g.
the system clock when it expects it to be a PHC, or a different PHC
when the socket is not bound to a specific interface?
At least in some applications it would be preferred to not sent a
packet at all instead of sending it at a wrong time.
Please keep in mind that the PHCs and the system clock don't have to
be synchronized to each other. If I understand the rest of the series
correctly, there is an assumption that the PHCs are keeping time in
TAI and CLOCK_TAI can be used as a fallback.
--
Miroslav Lichvar
From: Jamal Hadi Salim <jhs@mojatatu.com> Date: 2018-01-18 13:35:25
On 18-01-17 06:06 PM, Jesus Sanchez-Palencia wrote:
From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
TBS (Time Based Scheduler) uses the information added earlier in this
series (the socket option SO_TXTIME and the new role of
sk_buff->tstamp) to schedule traffic transmission based on absolute
time.
For some workloads, just bandwidth enforcement is not enough, and
precise control of the transmission of packets is necessary.
Example:
$ tc qdisc replace dev enp2s0 parent root handle 100 mqprio num_tc 3 \
handle 100:0 ?
map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 queues 1@0 1@1 2@2 hw 0
$ tc qdisc add dev enp2s0 parent 100:1 tbs delta 60000 clockid 11 offload 1
In this example, the Qdisc will try to enable offloading (offload 1)
the control of the transmission time to the network adapter, the
time stamp in socket are in reference to the clockid '11' (CLOCK_TAI)
and packets leave the Qdisc "delta" (60000) nanoseconds before its
transmission time.
When offloading is disabled, the network adapter will ignore the
sk_buff time stamp, and so, the transmission time will be only "best
effort" from the Qdisc.
General comments:
1) iproute2: Avoid magic numbers like 1 or 11 please; "offload"
(without 1) and "TAI" will be more human friendly.
2) Experience shows that adding padding fields in the control structs
implies they will _never ever_ be used. That was not design intent
for netlink but over years shit like that has happened.
Maybe look at using a 32 bitmap? It is more "future proof".
You seem to only have 2-3 flags but it gives you opportunity
to add more changes later. If you are 100% sure youll never need
it - then maybe just move the tc_tbs_qopt::offload to the end of
of the struct.
3)It would be helpful for debugging to increment some stats other
than drop counters on enqueu/dequeue obsolete packet drop. Maybe use
overlimits for the dequeu drops (in addition)?
4) I may be misreading things - but did you need to reset the
watchdog on dequeue? It is already being kicked for every incoming packet.
cheers,
jamal
From: Jamal Hadi Salim <jhs@mojatatu.com> Date: 2018-01-18 13:44:03
One more comment:
Probably try to run a test with a very small delta with
no offload (probably using something like prio as the root qdisc)
and dump the stats.
My gut feeling is your accounting of the backlog in particular is off.
cheers,
jamal
On 18-01-18 08:35 AM, Jamal Hadi Salim wrote:
On 18-01-17 06:06 PM, Jesus Sanchez-Palencia wrote:
quoted
From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
TBS (Time Based Scheduler) uses the information added earlier in this
series (the socket option SO_TXTIME and the new role of
sk_buff->tstamp) to schedule traffic transmission based on absolute
time.
For some workloads, just bandwidth enforcement is not enough, and
precise control of the transmission of packets is necessary.
Example:
$ tc qdisc replace dev enp2s0 parent root handle 100 mqprio num_tc 3 \
handle 100:0 ?
quoted
map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 queues 1@0 1@1 2@2 hw 0
$ tc qdisc add dev enp2s0 parent 100:1 tbs delta 60000 clockid 11
offload 1
In this example, the Qdisc will try to enable offloading (offload 1)
the control of the transmission time to the network adapter, the
time stamp in socket are in reference to the clockid '11' (CLOCK_TAI)
and packets leave the Qdisc "delta" (60000) nanoseconds before its
transmission time.
quoted
When offloading is disabled, the network adapter will ignore the
sk_buff time stamp, and so, the transmission time will be only "best
effort" from the Qdisc.
General comments:
1) iproute2: Avoid magic numbers like 1 or 11 please; "offload"
(without 1) and "TAI" will be more human friendly.
2) Experience shows that adding padding fields in the control structs
implies they will _never ever_ be used. That was not design intent
for netlink but over years shit like that has happened.
Maybe look at using a 32 bitmap? It is more "future proof".
You seem to only have 2-3 flags but it gives you opportunity
to add more changes later. If you are 100% sure youll never need
it - then maybe just move the tc_tbs_qopt::offload to the end of
of the struct.
3)It would be helpful for debugging to increment some stats other
than drop counters on enqueu/dequeue obsolete packet drop. Maybe use
overlimits for the dequeu drops (in addition)?
4) I may be misreading things - but did you need to reset the
watchdog on dequeue? It is already being kicked for every incoming packet.
cheers,
jamal
From: Richard Cochran <richardcochran@gmail.com> Date: 2018-01-18 17:13:38
On Thu, Jan 18, 2018 at 09:42:27AM +0100, Miroslav Lichvar wrote:
In the discussion about the v1 patchset, there was a question if the
cmsg should include a clockid_t. Without that, how can an application
prevent the packet from being sent using an incorrect clock, e.g.
the system clock when it expects it to be a PHC, or a different PHC
when the socket is not bound to a specific interface?
Right, the clockid_t should be passed in through the CMSG along with
the time.
Thanks,
Richard
From: Richard Cochran <richardcochran@gmail.com> Date: 2018-01-18 17:18:38
On Thu, Jan 18, 2018 at 08:35:18AM -0500, Jamal Hadi Salim wrote:
1) iproute2: Avoid magic numbers like 1 or 11 please; "offload"
(without 1) and "TAI" will be more human friendly.
Yes, and for the clockid, the program should accept CLOCK_REALTIME or
CLOCK_TAI for the hard coded SYS-V IDs or /dev/ptp0 for dynamic IDs.
Thanks,
Richard
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2018-01-19 21:16:27
On Wed, Jan 17, 2018 at 6:06 PM, Jesus Sanchez-Palencia
[off-list ref] wrote:
From: Richard Cochran <redacted>
This patch introduces SO_TXTIME. User space enables this option in
order to pass a desired future transmit time in a CMSG when calling
sendmsg(2).
A new field is added to struct sockcm_cookie, and the tstamp from
skbuffs will be used later on.
Signed-off-by: Richard Cochran <redacted>
Signed-off-by: Jesus Sanchez-Palencia <redacted>
---
@@ -1061,6 +1061,13 @@ int sock_setsockopt(struct socket *sock, int level, int optname,sock_valbool_flag(sk,SOCK_ZEROCOPY,valbool);break;+caseSO_TXTIME:+if(ns_capable(sock_net(sk)->user_ns,CAP_NET_ADMIN))+sock_valbool_flag(sk,SOCK_TXTIME,valbool);+else+ret=-EPERM;+break;+default:ret=-ENOPROTOOPT;break;
Please add getsockopt alongside setsockopt.
I would also restrict input to [0, 1] exactly to allow for future extensions.
If using ns_capable, skb->tstamp must continue to be scrubbed when traversing
network namespaces.
quoted hunk
@@ -2130,6 +2137,15 @@ int __sock_cmsg_send(struct sock *sk, struct msghdr *msg, struct cmsghdr *cmsg, sockc->tsflags &= ~SOF_TIMESTAMPING_TX_RECORD_MASK; sockc->tsflags |= tsflags; break;+ case SO_TXTIME:+ if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN))+ return -EPERM;+ if (!sock_flag(sk, SOCK_TXTIME))+ return -EINVAL;
No need for ns_capable check on each packet when already required to
toggle socket option.
+ if (cmsg->cmsg_len != CMSG_LEN(sizeof(ktime_t)))
+ return -EINVAL;
I don't see any existing reference to ktime_t in include/uapi. Just use a s64?
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2018-01-19 21:19:24
On Wed, Jan 17, 2018 at 6:06 PM, Jesus Sanchez-Palencia
[off-list ref] wrote:
From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
TBS (Time Based Scheduler) uses the information added earlier in this
series (the socket option SO_TXTIME and the new role of
sk_buff->tstamp) to schedule traffic transmission based on absolute
time.
For some workloads, just bandwidth enforcement is not enough, and
precise control of the transmission of packets is necessary.
Example:
$ tc qdisc replace dev enp2s0 parent root handle 100 mqprio num_tc 3 \
map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 queues 1@0 1@1 2@2 hw 0
$ tc qdisc add dev enp2s0 parent 100:1 tbs delta 60000 clockid 11 offload 1
In this example, the Qdisc will try to enable offloading (offload 1)
the control of the transmission time to the network adapter, the
time stamp in socket are in reference to the clockid '11' (CLOCK_TAI)
and packets leave the Qdisc "delta" (60000) nanoseconds before its
transmission time.
When offloading is disabled, the network adapter will ignore the
sk_buff time stamp, and so, the transmission time will be only "best
effort" from the Qdisc.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Signed-off-by: Jesus Sanchez-Palencia <redacted>
---
+static struct sk_buff *timerqueue_erase(struct Qdisc *sch,
+ struct sk_buff *skb, bool drop)
+{
+ struct tbs_sched_data *q = qdisc_priv(sch);
+
+ rb_erase(&skb->rbnode, &q->head);
+
+ if (drop) {
+ struct sk_buff *to_free = NULL;
+
+ qdisc_drop(skb, sch, &to_free);
+ kfree_skb_list(to_free);
+ } else {
+ qdisc_qstats_backlog_dec(sch, skb);
+ qdisc_bstats_update(sch, skb);
+
+ q->last = skb->tstamp;
+ }
+
+ sch->q.qlen--;
+
+ /* The rbnode field in the skb re-uses these fields, now that
+ * we are done with the rbnode, reset them.
+ */
+ skb->next = NULL;
+ skb->prev = NULL;
+ skb->dev = qdisc_dev(sch);
+
+ return skb;
+}
From: Richard Cochran <richardcochran@gmail.com> Date: 2018-01-23 05:23:31
On Wed, Jan 17, 2018 at 03:06:11PM -0800, Jesus Sanchez-Palencia wrote:
First, a baseline test was ran for 10 minutes with the plain kernel only:
| | plain kernel @ 1ms |
|-----------------+--------------------+
| min (ns): | +4.820000e+02 |
| max (ns): | +9.999300e+05 |
| pk-pk: | +9.994480e+05 |
I wonder about these worst case measurements of 999 and 998
milliseconds. It almost looks like you missed one entire period.
Could this simply be a bug in the test setup?
Thanks,
Richard
From: Richard Cochran <richardcochran@gmail.com> Date: 2018-01-23 05:26:04
On Mon, Jan 22, 2018 at 09:23:27PM -0800, Richard Cochran wrote:
On Wed, Jan 17, 2018 at 03:06:11PM -0800, Jesus Sanchez-Palencia wrote:
quoted
First, a baseline test was ran for 10 minutes with the plain kernel only:
| | plain kernel @ 1ms |
|-----------------+--------------------+
| min (ns): | +4.820000e+02 |
| max (ns): | +9.999300e+05 |
| pk-pk: | +9.994480e+05 |
From: Jesus Sanchez-Palencia <hidden> Date: 2018-01-23 18:08:48
Hi,
On 01/22/2018 09:26 PM, Richard Cochran wrote:
On Mon, Jan 22, 2018 at 09:23:27PM -0800, Richard Cochran wrote:
quoted
On Wed, Jan 17, 2018 at 03:06:11PM -0800, Jesus Sanchez-Palencia wrote:
quoted
First, a baseline test was ran for 10 minutes with the plain kernel only:
| | plain kernel @ 1ms |
|-----------------+--------------------+
| min (ns): | +4.820000e+02 |
| max (ns): | +9.999300e+05 |
| pk-pk: | +9.994480e+05 |
I wonder about these worst case measurements of 999 and 998
milliseconds. It almost looks like you missed one entire period.
^^^^
microseconds
quoted
Could this simply be a bug in the test setup?
Yes. From the data set of the tbs SW:
offset | timestamp
-------+---------------------
(...) |
10639 | 1516117448.058010639
9503 | 1516117448.059009503
10167 | 1516117448.060010167
9823 | 1516117448.061009823
9567 | 1516117448.062009567
997703 | 1516117448.062997703 ****
911719 | 1516117448.063911719
12655 | 1516117448.065012655
12399 | 1516117448.066012399
(...)
Since the period was 1ms, the highlighted entry should have arrived within the
[1516117448.063000000, 1516117448.063999999] range, so in this case it was
early. For the next runs, I will modify the test setup so the txtime is sent as
part of the packet payload and later taken into account by the post-processing
script that is calculating the offsets.
Thanks,
Jesus
From: Jesus Sanchez-Palencia <hidden> Date: 2018-01-23 18:25:30
Hi,
On 01/19/2018 01:15 PM, Willem de Bruijn wrote:
On Wed, Jan 17, 2018 at 6:06 PM, Jesus Sanchez-Palencia
[off-list ref] wrote:
quoted
From: Richard Cochran <redacted>
This patch introduces SO_TXTIME. User space enables this option in
order to pass a desired future transmit time in a CMSG when calling
sendmsg(2).
A new field is added to struct sockcm_cookie, and the tstamp from
skbuffs will be used later on.
Signed-off-by: Richard Cochran <redacted>
Signed-off-by: Jesus Sanchez-Palencia <redacted>
---
@@ -1061,6 +1061,13 @@ int sock_setsockopt(struct socket *sock, int level, int optname,sock_valbool_flag(sk,SOCK_ZEROCOPY,valbool);break;+caseSO_TXTIME:+if(ns_capable(sock_net(sk)->user_ns,CAP_NET_ADMIN))+sock_valbool_flag(sk,SOCK_TXTIME,valbool);+else+ret=-EPERM;+break;+default:ret=-ENOPROTOOPT;break;
Please add getsockopt alongside setsockopt.
I would also restrict input to [0, 1] exactly to allow for future extensions.
Ok, will do.
If using ns_capable, skb->tstamp must continue to be scrubbed when traversing
network namespaces.
I was planning to follow Eric's suggestion and move the tstamp scrubbing out of
skb_scrub_packet() into ____dev_forward_skb() instead. Would that break when
traversing namespaces?
quoted
@@ -2130,6 +2137,15 @@ int __sock_cmsg_send(struct sock *sk, struct msghdr *msg, struct cmsghdr *cmsg, sockc->tsflags &= ~SOF_TIMESTAMPING_TX_RECORD_MASK; sockc->tsflags |= tsflags; break;+ case SO_TXTIME:+ if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN))+ return -EPERM;+ if (!sock_flag(sk, SOCK_TXTIME))+ return -EINVAL;
No need for ns_capable check on each packet when already required to
toggle socket option.
Ok. SO_MARK is doing the same so it might have "mis-inspired" me. I should
probably fix both.
quoted
+ if (cmsg->cmsg_len != CMSG_LEN(sizeof(ktime_t)))
+ return -EINVAL;
I don't see any existing reference to ktime_t in include/uapi. Just use a s64?
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2018-01-23 20:03:29
quoted
If using ns_capable, skb->tstamp must continue to be scrubbed when traversing
network namespaces.
I was planning to follow Eric's suggestion and move the tstamp scrubbing out of
skb_scrub_packet() into ____dev_forward_skb() instead. Would that break when
traversing namespaces?
That implies namespace traversal, so sounds perfect for this purpose.
quoted
quoted
@@ -2130,6 +2137,15 @@ int __sock_cmsg_send(struct sock *sk, struct msghdr *msg, struct cmsghdr *cmsg, sockc->tsflags &= ~SOF_TIMESTAMPING_TX_RECORD_MASK; sockc->tsflags |= tsflags; break;+ case SO_TXTIME:+ if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN))+ return -EPERM;+ if (!sock_flag(sk, SOCK_TXTIME))+ return -EINVAL;
No need for ns_capable check on each packet when already required to
toggle socket option.
Ok. SO_MARK is doing the same so it might have "mis-inspired" me. I should
probably fix both.
The SO_MARK cmsg does need a check on each invocation,
because it is not conditional on a sock_flag like SO_TXTIME.
From: Vinicius Costa Gomes <vinicius.gomes@intel.com> Date: 2018-01-23 21:22:39
Hi,
Miroslav Lichvar [off-list ref] writes:
On Wed, Jan 17, 2018 at 03:06:12PM -0800, Jesus Sanchez-Palencia wrote:
quoted
From: Richard Cochran <redacted>
This patch introduces SO_TXTIME. User space enables this option in
order to pass a desired future transmit time in a CMSG when calling
sendmsg(2).
A new field is added to struct sockcm_cookie, and the tstamp from
skbuffs will be used later on.
In the discussion about the v1 patchset, there was a question if the
cmsg should include a clockid_t. Without that, how can an application
prevent the packet from being sent using an incorrect clock, e.g.
the system clock when it expects it to be a PHC, or a different PHC
when the socket is not bound to a specific interface?
At least in some applications it would be preferred to not sent a
packet at all instead of sending it at a wrong time.
Including the clockid in a CMSG field does make sense. Will add it in
the next version of this series.
What I think would be the ideal scenario would be if the clockid
parameter to the TBS Qdisc would not be necessary (if offload was
enabled), but that's not quite possible right now, because there's no
support for using the hrtimer infrastructure with dynamic clocks
(/dev/ptp*).
What I am thinking is to keep the clockid parameter for the Qdisc (and
add support for expressing the clockid in friendlier ways, as requested
later in this thread), but I can't think of a way to add support for
using /dev/ptp* clocks without first having hrtimer support them.
And the behavior would be to drop any packets with a clockid not
matching the Qdisc clockid.
How does this sound?
Please keep in mind that the PHCs and the system clock don't have to
be synchronized to each other. If I understand the rest of the series
correctly, there is an assumption that the PHCs are keeping time in
TAI and CLOCK_TAI can be used as a fallback.
You understand correctly, that's because of whole hrtimer issue.
From: Jesus Sanchez-Palencia <hidden> Date: 2018-01-23 21:46:42
Hi,
On 01/18/2018 05:44 AM, Jamal Hadi Salim wrote:
One more comment:
Probably try to run a test with a very small delta with
no offload (probably using something like prio as the root qdisc)
and dump the stats.
My gut feeling is your accounting of the backlog in particular is off.
You were right, thanks. It'll be fixed on our next version.
Regards,
Jesus
cheers,
jamal
On 18-01-18 08:35 AM, Jamal Hadi Salim wrote:
quoted
On 18-01-17 06:06 PM, Jesus Sanchez-Palencia wrote:
quoted
From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
TBS (Time Based Scheduler) uses the information added earlier in this
series (the socket option SO_TXTIME and the new role of
sk_buff->tstamp) to schedule traffic transmission based on absolute
time.
For some workloads, just bandwidth enforcement is not enough, and
precise control of the transmission of packets is necessary.
Example:
$ tc qdisc replace dev enp2s0 parent root handle 100 mqprio num_tc 3 \
handle 100:0 ?
quoted
map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 queues 1@0 1@1 2@2 hw 0
$ tc qdisc add dev enp2s0 parent 100:1 tbs delta 60000 clockid 11 offload 1
In this example, the Qdisc will try to enable offloading (offload 1)
the control of the transmission time to the network adapter, the
time stamp in socket are in reference to the clockid '11' (CLOCK_TAI)
and packets leave the Qdisc "delta" (60000) nanoseconds before its
transmission time.
quoted
When offloading is disabled, the network adapter will ignore the
sk_buff time stamp, and so, the transmission time will be only "best
effort" from the Qdisc.
General comments:
1) iproute2: Avoid magic numbers like 1 or 11 please; "offload"
(without 1) and "TAI" will be more human friendly.
2) Experience shows that adding padding fields in the control structs
implies they will _never ever_ be used. That was not design intent
for netlink but over years shit like that has happened.
Maybe look at using a 32 bitmap? It is more "future proof".
You seem to only have 2-3 flags but it gives you opportunity
to add more changes later. If you are 100% sure youll never need
it - then maybe just move the tc_tbs_qopt::offload to the end of
of the struct.
3)It would be helpful for debugging to increment some stats other
than drop counters on enqueu/dequeue obsolete packet drop. Maybe use
overlimits for the dequeu drops (in addition)?
4) I may be misreading things - but did you need to reset the
watchdog on dequeue? It is already being kicked for every incoming packet.
cheers,
jamal
From: Jesus Sanchez-Palencia <hidden> Date: 2018-01-23 22:03:05
Hi,
On 01/18/2018 05:35 AM, Jamal Hadi Salim wrote:
On 18-01-17 06:06 PM, Jesus Sanchez-Palencia wrote:
quoted
From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
TBS (Time Based Scheduler) uses the information added earlier in this
series (the socket option SO_TXTIME and the new role of
sk_buff->tstamp) to schedule traffic transmission based on absolute
time.
For some workloads, just bandwidth enforcement is not enough, and
precise control of the transmission of packets is necessary.
Example:
$ tc qdisc replace dev enp2s0 parent root handle 100 mqprio num_tc 3 \
handle 100:0 ?
quoted
map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 queues 1@0 1@1 2@2 hw 0
$ tc qdisc add dev enp2s0 parent 100:1 tbs delta 60000 clockid 11 offload 1
In this example, the Qdisc will try to enable offloading (offload 1)
the control of the transmission time to the network adapter, the
time stamp in socket are in reference to the clockid '11' (CLOCK_TAI)
and packets leave the Qdisc "delta" (60000) nanoseconds before its
transmission time.
quoted
When offloading is disabled, the network adapter will ignore the
sk_buff time stamp, and so, the transmission time will be only "best
effort" from the Qdisc.
General comments:
1) iproute2: Avoid magic numbers like 1 or 11 please; "offload"
(without 1) and "TAI" will be more human friendly.
Sure, we'll change both parameters.
2) Experience shows that adding padding fields in the control structs
implies they will _never ever_ be used. That was not design intent
for netlink but over years shit like that has happened.
Maybe look at using a 32 bitmap? It is more "future proof".
You seem to only have 2-3 flags but it gives you opportunity
to add more changes later. If you are 100% sure youll never need
it - then maybe just move the tc_tbs_qopt::offload to the end of
of the struct.
Ok, we are looking into it.
3)It would be helpful for debugging to increment some stats other
than drop counters on enqueu/dequeue obsolete packet drop. Maybe use
overlimits for the dequeu drops (in addition)?
Yes, sure, that's a good idea.
4) I may be misreading things - but did you need to reset the
watchdog on dequeue? It is already being kicked for every incoming packet.
The watchdog timer must always be set to the next deadline. Since both enqueue()
and dequeue() modify the tree structure and may have it rebalanced, we need to
make sure the watchdog has been 're-set' to the next deadline. i.e. After a
dequeue, we need to re-arm the timer for the next head of the queue, so that is
why. Does that make sense?
I'm now thinking that naming that helper as reset_watchdog() can be misleading,
so we are considering naming it as rearm_watchdog() instead.
Thanks,
Jesus
From: Levi Pearson <hidden> Date: 2018-01-24 01:43:23
On Wed, Jan 17, 2018 at 4:06 PM, Jesus Sanchez-Palencia
[off-list ref] wrote:
This series is the v2 of the Time based packet transmission RFC, which was
originally proposed by Richard Cochran: https://lwn.net/Articles/733962/ .
Great to see you carrying on with this!
Our main questions at this stage are related to the qdisc:
- does the proposed design attend all use cases?
- should the qdisc really drop packets that expired after being queued even
for the SW best effort mode?
I don't think that being "expired" is necessarily cause for dropping.
The semantic of a launch time is "launch no earlier than this point"
after all, not a deadline. To keep the hardware working, we must only
enforce the invariant that we never queue a packet with an earlier
timestamp than one we previously enqueued that has not launched yet.
Just checking for expiration is going to rule out some potential uses
and also won't necessarily prevent enqueuing out-of-order packets.
Here is an example:
A group of applications enqueue packets to be sent at 1 second
intervals, and share a 5ms window in which they can send them. Due to
scheduling variation, they may finish executing in a different order
per interval, and occasionally some may not finish preparing their
packet before the window opens, although they always will present
their packet before the window closes.
If they all pick different times within the launch window, it is
possible that two of them might pick times very close to one another.
If they present their frames out-of-order to the qdisc, but close
enough to the launch time that the qdisc doesn't hold on to them (i.e.
in the [txtime - delta, txtime] range mentioned in tbs_dequeue), then
they will get enqueued out of order and the invariant will be
violated. Reordering within some time window only works if all frames
for that window are scheduled well in advance of the first launch
time, and that's not great for applications that need to to calculate
right up to the time they need to send their data.
If they each schedule their packet for the beginning of the window, on
the other hand, everything will be fine for those that complete before
the window opens; there's no longer any order requirement that needs
to be maintained between those in the group, since they all use the
same timestamp. But those that finish after the window opens will be
"late" and have their frames dropped in the current scheme. There's no
technical reason to regard them as late until sending would exceed the
window bounds, but we also don't want to delay any of them once the
window opens.
To maintain the hardware ordering invariant, you need to keep track of
the most recent timestamp you have enqueued in the hardware. Anything
that hits tbs_enqueue with a timestamp earlier than that must be
either dropped or have its timestamp adjusted.
The one remaining question is how late can a timestamped frame be
before it should be dropped instead of enqueued, assuming it is to be
allowed at all? The qdisc could track the allowed window based on user
configuration. I believe the i210 hardware will launch any frame at
the head of queue with a launch time set at or before the present
time, but not so far before that it wraps and interprets the time as a
future time. The qdisc would need to be able query the driver about
how large that window is if it wants to pass in-the-past timestamps
through as-is, but it could also just update timestamps still within
the user-configured window to be set at the current time.
My understanding of reservations for industrial TSN use cases is that
applications will present their working period and their scheduling
accuracy to the central manager, which will take into account the
worst case timing bounds when creating the window that the application
will use on the network. It will then give back an assignment for a
start time offset from the period base time (UTC_time values that are
multiples of interval_time) at which the application's transmit window
starts, and it will remain open long enough to account for the
scheduling jitter in the application.
I think putting the window concept in the qdisc makes for a nice
mapping to how the TSN scheduling works as well as resolving some of
the tricky details around ensuring that you don't jam the hardware
with out-of-order timestamps or unnecessarily delay scheduling packets
to reorder them.
--Levi
From: Richard Cochran <richardcochran@gmail.com> Date: 2018-01-24 03:04:39
On Tue, Jan 23, 2018 at 01:22:37PM -0800, Vinicius Costa Gomes wrote:
What I think would be the ideal scenario would be if the clockid
parameter to the TBS Qdisc would not be necessary (if offload was
enabled), but that's not quite possible right now, because there's no
support for using the hrtimer infrastructure with dynamic clocks
(/dev/ptp*).
We don't need hrtimer for HW offloading. Just enqueue the packets. I
thought we agreed that user space get the ordering correct. In fact,
davem insisted on it, IIRC.
Thanks,
Richard
From: Vinicius Costa Gomes <vinicius.gomes@intel.com> Date: 2018-01-24 22:46:26
Hi Richard,
Richard Cochran [off-list ref] writes:
On Tue, Jan 23, 2018 at 01:22:37PM -0800, Vinicius Costa Gomes wrote:
quoted
What I think would be the ideal scenario would be if the clockid
parameter to the TBS Qdisc would not be necessary (if offload was
enabled), but that's not quite possible right now, because there's no
support for using the hrtimer infrastructure with dynamic clocks
(/dev/ptp*).
We don't need hrtimer for HW offloading. Just enqueue the packets. I
thought we agreed that user space get the ordering correct. In fact,
davem insisted on it, IIRC.
About the ordering of packets, From here [1], there are 3 clear points
(in my understanding):
1. Re-ordering of TX descriptors on the device queue should/must not
happen;
2. Out of order requests are an error;
3. Timestamps in the past are an error;
The only robust way that we could think of about keeping the the packets
in order for the device queue is re-ordering packets in the Qdisc.
We tried to reach out for confirmation [2] of this understanding but
didn't receive any word.
Even if we reach a decision that the Qdisc should not re-order packets
(we wouldn't have any dependency on hrtimers in the offload case, as you
pointed out), we still need hrtimers for the software implementation.
So, I guess, the problem remains, if it's possible for the user to
express a /dev/ptp* clock, what should we do?
From: Miroslav Lichvar <hidden> Date: 2018-01-25 09:12:30
On Fri, Jan 19, 2018 at 06:09:15PM -0800, Richard Cochran wrote:
On Fri, Jan 19, 2018 at 04:15:46PM -0500, Willem de Bruijn wrote:
quoted
quoted
+ if (cmsg->cmsg_len != CMSG_LEN(sizeof(ktime_t)))
+ return -EINVAL;
I don't see any existing reference to ktime_t in include/uapi. Just use a s64?
Agreed. I didn't see the point of switching to ktime, either.
Do I understand it correctly that no other interface is using
nanoseconds since 1970? We probably don't have to worry about year
2262 yet, but wouldn't it be better to make it consistent with the
timestamping API using timespec? Or is it just better to avoid the
64/32-bit mess of time_t?
--
Miroslav Lichvar
From: Richard Cochran <richardcochran@gmail.com> Date: 2018-01-25 16:52:25
On Thu, Jan 25, 2018 at 10:12:25AM +0100, Miroslav Lichvar wrote:
Do I understand it correctly that no other interface is using
nanoseconds since 1970? We probably don't have to worry about year
2262 yet, but wouldn't it be better to make it consistent with the
timestamping API using timespec? Or is it just better to avoid the
64/32-bit mess of time_t?
I prefer a single 64 bit nanoseconds field:
- Applications won't have to convert to timespec.
- Avoids the time_t issue.
Thanks,
Richard
From: Richard Cochran <richardcochran@gmail.com> Date: 2018-01-26 02:12:28
On Wed, Jan 24, 2018 at 02:46:24PM -0800, Vinicius Costa Gomes wrote:
The only robust way that we could think of about keeping the the packets
in order for the device queue is re-ordering packets in the Qdisc.
Right, but you cannot afford the overhead of the timerqueue when using
HW offload, when the HW device sits on a PCIe bus. Many serious TSN
applications (like industrial controls) will want to have just one
packet queued, readying the next one just in time for the next
deadline. The control loops are sensitive to the feedback interval.
Even if we reach a decision that the Qdisc should not re-order packets
(we wouldn't have any dependency on hrtimers in the offload case, as you
pointed out), we still need hrtimers for the software implementation.
Fine.
So, I guess, the problem remains, if it's possible for the user to
express a /dev/ptp* clock, what should we do?
Thinking a bit more, it doesn't make sense to have a user choice for
the HW offloading case. The clock should implicitly be the device
clock, always. Using any other clock would make no sense.
Thanks,
Richard
From: Jesus Sanchez-Palencia <hidden> Date: 2018-01-27 00:06:02
Hi Levi,
On 01/23/2018 05:43 PM, Levi Pearson wrote:
On Wed, Jan 17, 2018 at 4:06 PM, Jesus Sanchez-Palencia
[off-list ref] wrote:
quoted
This series is the v2 of the Time based packet transmission RFC, which was
originally proposed by Richard Cochran: https://lwn.net/Articles/733962/ .
Great to see you carrying on with this!
quoted
Our main questions at this stage are related to the qdisc:
- does the proposed design attend all use cases?
- should the qdisc really drop packets that expired after being queued even
for the SW best effort mode?
I don't think that being "expired" is necessarily cause for dropping.
The semantic of a launch time is "launch no earlier than this point"
after all, not a deadline. To keep the hardware working, we must only
enforce the invariant that we never queue a packet with an earlier
timestamp than one we previously enqueued that has not launched yet.
Just checking for expiration is going to rule out some potential uses
and also won't necessarily prevent enqueuing out-of-order packets.
Let me just split this a bit to make sure we don’t mix things up.
Currently, as discussed during the RFC v1 thread, on tbs_enqueue() we drop
packets if they are expired or if they have an earlier timestamp than the last
dequeued packet.
On tbs_dequeue(), we drop packets if they have expired while sitting at our
timerqueue. That is done because our current semantic for txtime is “no later
than this point”. Are you suggesting that we change that to “no earlier than
this point” instead? The delta parameter would then be defining how early is
acceptable for dequeuing a packet, but we’ll need another parameter that can
define how late it should be when we decide to drop it.
Here is an example:
A group of applications enqueue packets to be sent at 1 second
intervals, and share a 5ms window in which they can send them. Due to
scheduling variation, they may finish executing in a different order
per interval, and occasionally some may not finish preparing their
packet before the window opens, although they always will present
their packet before the window closes.
If they all pick different times within the launch window, it is
possible that two of them might pick times very close to one another.
If they present their frames out-of-order to the qdisc, but close
enough to the launch time that the qdisc doesn't hold on to them (i.e.
in the [txtime - delta, txtime] range mentioned in tbs_dequeue), then
they will get enqueued out of order and the invariant will be
violated. Reordering within some time window only works if all frames
for that window are scheduled well in advance of the first launch
time, and that's not great for applications that need to to calculate
right up to the time they need to send their data.
I like the example, but due to the data structure that we use internally,
tbs_enqueue() will always enqueue packets onto their correct position, i.e. the
rbtree will always be ‘sorted’. If a dequeue() happens before the next enqueue,
then yes we may get to the situation you are describing, but that will always be
true regardless of the applications that are running, right? If that can’t be
fixed in userspace, then I’m afraid that either using a per-packet txtime is not
the right strategy for this system or tbs might not be the correct qdisc for it.
(...)
To maintain the hardware ordering invariant, you need to keep track of
the most recent timestamp you have enqueued in the hardware. Anything
that hits tbs_enqueue with a timestamp earlier than that must be
either dropped or have its timestamp adjusted.
Yes, and we currently drop them there (that’s what the ktime_before(txtime, q->last)
check is doing). Adjusting timestamps is a can-of-worms, in my opinion, and I
don’t think we should go down that route.
The one remaining question is how late can a timestamped frame be
before it should be dropped instead of enqueued, assuming it is to be
allowed at all? The qdisc could track the allowed window based on user
configuration. I believe the i210 hardware will launch any frame at
the head of queue with a launch time set at or before the present
time, but not so far before that it wraps and interprets the time as a
future time. The qdisc would need to be able query the driver about
how large that window is if it wants to pass in-the-past timestamps
through as-is, but it could also just update timestamps still within
the user-configured window to be set at the current time.
I believe I have tackled the question here already. For the rest, we don’t think
a qdisc should fetch any information from the driver. The information flow
should be kept as is, from qdisc to the driver, not the other way around.
My understanding of reservations for industrial TSN use cases is that
applications will present their working period and their scheduling
accuracy to the central manager, which will take into account the
worst case timing bounds when creating the window that the application
will use on the network. It will then give back an assignment for a
start time offset from the period base time (UTC_time values that are
multiples of interval_time) at which the application's transmit window
starts, and it will remain open long enough to account for the
scheduling jitter in the application.
I think putting the window concept in the qdisc makes for a nice
mapping to how the TSN scheduling works as well as resolving some of
the tricky details around ensuring that you don't jam the hardware
with out-of-order timestamps or unnecessarily delay scheduling packets
to reorder them.
I follow what you are saying, but I think we will either have those fixed at
userspace or through a separate mechanism. The tbs qdisc should be kept simple
and concise:
* it provides a per-packet tx time;
* it holds packets (in order) until at least their txtime minus a configurable
delta;
* it drops packets if they are expired (or perhaps if they are after a
configurable threshold);
* it avoids out-of-order timestamps and it allows you to turn a hw feature on
to increase accuracy for per-packet time based transmission.
Even though the concept of per-queue tx windows (with periods and offsets)
should be implementable on top of tbs, our opinion is that it might better fit
under yet another qdisc that should be designed specifically for that. We have
shared some ideas before, but that is definitely out-of-scope here.
Thanks for the feedback so far.
Regards,
Jesus
From: Jesus Sanchez-Palencia <hidden> Date: 2018-02-01 00:51:22
Hi,
On 01/18/2018 09:13 AM, Richard Cochran wrote:
On Thu, Jan 18, 2018 at 09:42:27AM +0100, Miroslav Lichvar wrote:
quoted
In the discussion about the v1 patchset, there was a question if the
cmsg should include a clockid_t. Without that, how can an application
prevent the packet from being sent using an incorrect clock, e.g.
the system clock when it expects it to be a PHC, or a different PHC
when the socket is not bound to a specific interface?
Right, the clockid_t should be passed in through the CMSG along with
the time.
While implementing this today it crossed my mind that why don't we have the
clockid_t set per socket (e.g. as an argument to SO_TXTIME) instead of per packet?
The only use-case that we could think of that would be 'blocked' was using
sendmmsg() to send a packet to different interfaces with a single syscall, but
I'm not sure how common that is.
What do you think?
Thanks,
Jesus
From: Richard Cochran <richardcochran@gmail.com> Date: 2018-02-01 04:16:06
On Wed, Jan 31, 2018 at 04:49:36PM -0800, Jesus Sanchez-Palencia wrote:
While implementing this today it crossed my mind that why don't we have the
clockid_t set per socket (e.g. as an argument to SO_TXTIME) instead of per packet?
From: Miroslav Lichvar <hidden> Date: 2018-02-01 09:27:31
On Wed, Jan 31, 2018 at 04:49:36PM -0800, Jesus Sanchez-Palencia wrote:
On 01/18/2018 09:13 AM, Richard Cochran wrote:
quoted
Right, the clockid_t should be passed in through the CMSG along with
the time.
While implementing this today it crossed my mind that why don't we have the
clockid_t set per socket (e.g. as an argument to SO_TXTIME) instead of per packet?
I suspect that might have an impact on the performance. Even if the
application doesn't use sendmmsg(), it would possibly have to call
setsockopt() before each sendmsg() to change the clockid_t, right?
If clockid_t could be set per packet, a special value could be used
to allow sending on interfaces that don't support it.
The only use-case that we could think of that would be 'blocked' was using
sendmmsg() to send a packet to different interfaces with a single syscall, but
I'm not sure how common that is.
The SO_TXTIME option will make sendmmsg() useful in applications where
it wasn't before. For instance, an NTP server will be able to batch
multiple responses as their transmit timestamps can be set accurately
in advance and it's no longer necessary to send the responses as soon
as they are assembled.
I think it would be nice the sendmmsg() calls didn't have to be split
by clockid_t.
--
Miroslav Lichvar
From: Jesus Sanchez-Palencia <hidden> Date: 2018-02-01 20:57:46
Hi,
On 02/01/2018 01:27 AM, Miroslav Lichvar wrote:
On Wed, Jan 31, 2018 at 04:49:36PM -0800, Jesus Sanchez-Palencia wrote:
quoted
On 01/18/2018 09:13 AM, Richard Cochran wrote:
quoted
Right, the clockid_t should be passed in through the CMSG along with
the time.
While implementing this today it crossed my mind that why don't we have the
clockid_t set per socket (e.g. as an argument to SO_TXTIME) instead of per packet?
I suspect that might have an impact on the performance. Even if the
application doesn't use sendmmsg(), it would possibly have to call
setsockopt() before each sendmsg() to change the clockid_t, right?
Yes. On the other hand, for applications that will be using only 1 clockid_t,
keeping it per packet will also have an impact as we'll be copying the same
value from the cmsg cookie into sk_buffs over and over.
If clockid_t could be set per packet, a special value could be used
to allow sending on interfaces that don't support it.
quoted
The only use-case that we could think of that would be 'blocked' was using
sendmmsg() to send a packet to different interfaces with a single syscall, but
I'm not sure how common that is.
The SO_TXTIME option will make sendmmsg() useful in applications where
it wasn't before. For instance, an NTP server will be able to batch
multiple responses as their transmit timestamps can be set accurately
in advance and it's no longer necessary to send the responses as soon
as they are assembled.
I think it would be nice the sendmmsg() calls didn't have to be split
by clockid_t.
OK, fair enough. I will keep it per-packet for now as initially agreed and we
can revisit this later if needed.
Thanks,
Jesus
From: Jesus Sanchez-Palencia <hidden> Date: 2018-02-12 22:41:06
Hi,
On 01/18/2018 12:42 AM, Miroslav Lichvar wrote:
On Wed, Jan 17, 2018 at 03:06:12PM -0800, Jesus Sanchez-Palencia wrote:
quoted
From: Richard Cochran <redacted>
This patch introduces SO_TXTIME. User space enables this option in
order to pass a desired future transmit time in a CMSG when calling
sendmsg(2).
A new field is added to struct sockcm_cookie, and the tstamp from
skbuffs will be used later on.
In the discussion about the v1 patchset, there was a question if the
cmsg should include a clockid_t. Without that, how can an application
prevent the packet from being sent using an incorrect clock, e.g.
the system clock when it expects it to be a PHC, or a different PHC
when the socket is not bound to a specific interface?
At least in some applications it would be preferred to not sent a
packet at all instead of sending it at a wrong time.
Please keep in mind that the PHCs and the system clock don't have to
be synchronized to each other. If I understand the rest of the series
correctly, there is an assumption that the PHCs are keeping time in
TAI and CLOCK_TAI can be used as a fallback.
Just to double-check, imagine that I've configured the qdisc for
SW best-effort and with clockid CLOCK_REALTIME. When it receives a
packet with the clockid of a /dev/ptpX, the qdisc should just drop that
packet, right?
Or would this block any use-cases that I couldn't think of ?
Thanks,
Jesus
From: Miroslav Lichvar <hidden> Date: 2018-02-13 09:56:57
On Mon, Feb 12, 2018 at 02:39:06PM -0800, Jesus Sanchez-Palencia wrote:
On 01/18/2018 12:42 AM, Miroslav Lichvar wrote:
quoted
Please keep in mind that the PHCs and the system clock don't have to
be synchronized to each other. If I understand the rest of the series
correctly, there is an assumption that the PHCs are keeping time in
TAI and CLOCK_TAI can be used as a fallback.
Just to double-check, imagine that I've configured the qdisc for
SW best-effort and with clockid CLOCK_REALTIME. When it receives a
packet with the clockid of a /dev/ptpX, the qdisc should just drop that
packet, right?
Yes, I think it should drop it. The kernel does not know the offset
between the two clocks (they don't even have to be synchronized), so
it cannot convert a PHC-based TX time to the system time.
--
Miroslav Lichvar