[Linuxptp-devel] [PATCH] sk: Modify poll to poll on ERRQUEUE

[Joe Schaack]

Previously the poll in sk_receive would "timeout" and when it did so
would check the ERRQUEUE for data and set POLLERR. This meant that if
sk_tx_timeout was set to 100 each poll would wait for 100ms rather than
exiting immediately when ERRQUEUE data was available.

Implement the SO_SELECT_ERR_QUEUE socket option that enables ERRQUEUE
messages to be polled for under the POLLPRI flag, greatly increasing the
number of packets per second that can be sent from linuxptp.

Signed-off-by: Joe Schaack <***@xes-inc.com>
---
sk.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/sk.c b/sk.c
index c48cf45..7573e56 100644
--- a/sk.c
+++ b/sk.c
@@ -230,7 +230,7 @@ int sk_receive(int fd, void *buf, int buflen,
msg.msg_controllen = sizeof(control);

if (flags == MSG_ERRQUEUE) {
- struct pollfd pfd = { fd, 0, 0 };
+ struct pollfd pfd = { fd, POLLPRI, 0 };
res = poll(&pfd, 1, sk_tx_timeout);
if (res < 1) {
pr_err(res ? "poll for tx timestamp failed: %m" :
@@ -238,8 +238,8 @@ int sk_receive(int fd, void *buf, int buflen,
pr_err("increasing tx_timestamp_timeout may correct "
"this issue, but it is likely caused by a driver bug");
return res;
- } else if (!(pfd.revents & POLLERR)) {
- pr_err("poll for tx timestamp woke up on non ERR event");
+ } else if (!(pfd.revents & POLLPRI)) {
+ pr_err("poll for tx timestamp woke up on non ERRQUEUE event");
return -1;
}
}
@@ -352,6 +352,13 @@ int sk_timestamping_init(int fd, const char *device, enum timestamp_type type,
return -1;
}

+ flags = 1;
+ if (setsockopt(fd, SOL_SOCKET, SO_SELECT_ERR_QUEUE,
+ &flags, sizeof(flags)) < 0) {
+ pr_err("ioctl SO_SELECT_ERR_QUEUE failed: %m");
+ return -1;
+ }
+
/* Enable the sk_check_fupsync option, perhaps. */
if (sk_general_init(fd)) {
return -1;

[Richard Cochran]

Post by Joe Schaack
Implement the SO_SELECT_ERR_QUEUE socket option that enables ERRQUEUE
messages to be polled for under the POLLPRI flag, greatly increasing the
number of packets per second that can be sent from linuxptp.

SO_SELECT_ERR_QUEUE is only available since Linux 3.10. This patch
breaks the program when running (or even compiling for) older kernels.

Thanks,
Richard

------------------------------------------------------------------------------

[Richard Cochran]

Joe,

The improved performance would be really nice to have. How about this
backward compatible version?

With your premission, I would like to give you credit for the idea by
adding the Suggested-by tag.

Thanks,
Richard
---

From 411573c5a361a916a7b3765b7791eedccedabacc Mon Sep 17 00:00:00 2001

From: Richard Cochran <***@gmail.com>
Date: Sat, 1 Nov 2014 12:27:14 +0100
Subject: [PATCH] Use SO_SELECT_ERR_QUEUE when available.

The current implementation fetches a transmit time stamp by polling on the
socket with pollfd.events set to zero, and then checking if POLLERR has
been returned by the kernel in pollfd.revents. This has the unfortunate
side effect of sleeping in poll() for the entire time out duration,
regardless of when the error queue becomes readable.

Linux kernel version 3.10 introduced a new socket option that allows
polling for transmit time stamps explicitly, waking the process as soon as
a time stamp becomes available. This patch enables the socket option,
falling back to the old behavior if necessary.

Suggested-by: Joe Schaack <***@xes-inc.com>
Signed-off-by: Richard Cochran <***@gmail.com>
---
missing.h | 4 ++++
sk.c | 16 ++++++++++++++--
2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/missing.h b/missing.h
index 43ac6cf..f7efd92 100644
--- a/missing.h
+++ b/missing.h
@@ -58,6 +58,10 @@ enum _missing_hwtstamp_tx_types {
#define SIOCGHWTSTAMP 0x89b1
#endif

+#ifndef SO_SELECT_ERR_QUEUE
+#define SO_SELECT_ERR_QUEUE 45
+#endif
+
#ifndef HAVE_CLOCK_ADJTIME
static inline int clock_adjtime(clockid_t id, struct timex *tx)
{
diff --git a/sk.c b/sk.c
index c48cf45..847855a 100644
--- a/sk.c
+++ b/sk.c
@@ -32,6 +32,7 @@

#include "address.h"
#include "ether.h"
+#include "missing.h"
#include "print.h"
#include "sk.h"

@@ -208,6 +209,9 @@ int sk_interface_addr(const char *name, int family, struct address *addr)
return result;
}

+static short sk_events = POLLPRI;
+static short sk_revents = POLLPRI;
+
int sk_receive(int fd, void *buf, int buflen,
struct address *addr, struct hw_timestamp *hwts, int flags)
{
@@ -230,7 +234,7 @@ int sk_receive(int fd, void *buf, int buflen,
msg.msg_controllen = sizeof(control);

if (flags == MSG_ERRQUEUE) {
- struct pollfd pfd = { fd, 0, 0 };
+ struct pollfd pfd = { fd, sk_events, 0 };
res = poll(&pfd, 1, sk_tx_timeout);
if (res < 1) {
pr_err(res ? "poll for tx timestamp failed: %m" :
@@ -238,7 +242,7 @@ int sk_receive(int fd, void *buf, int buflen,
pr_err("increasing tx_timestamp_timeout may correct "
"this issue, but it is likely caused by a driver bug");
return res;
- } else if (!(pfd.revents & POLLERR)) {
+ } else if (!(pfd.revents & sk_revents)) {
pr_err("poll for tx timestamp woke up on non ERR event");
return -1;
}
@@ -352,6 +356,14 @@ int sk_timestamping_init(int fd, const char *device, enum timestamp_type type,
return -1;
}

+ flags = 1;
+ if (setsockopt(fd, SOL_SOCKET, SO_SELECT_ERR_QUEUE,
+ &flags, sizeof(flags)) < 0) {
+ pr_warning("%s: SO_SELECT_ERR_QUEUE: %m", device);
+ sk_events = 0;
+ sk_revents = POLLERR;
+ }
+
/* Enable the sk_check_fupsync option, perhaps. */
if (sk_general_init(fd)) {
return -1;

[Joe Schaack]

Post by Richard Cochran
The improved performance would be really nice to have. How about this
backward compatible version?
With your premission, I would like to give you credit for the idea by
adding the Suggested-by tag.

That patch looks good to me, and preserves the compatibility that I missed.

Thanks,
Joe Schaack

------------------------------------------------------------------------------