Netdev List
 help / color / mirror / Atom feed
* [PATCH iproute2 net-next v2 2/4] ss: Introduce columns lightweight abstraction
From: Stefano Brivio @ 2017-12-12  0:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Sabrina Dubroca
In-Reply-To: <cover.1513039237.git.sbrivio@redhat.com>

Instead of embedding spacing directly while printing contents,
logically declare columns and functions to buffer their content,
to print left and right spacing around fields, to flush them to
screen, and to print headers.

This makes it a bit easier to handle layout changes and prepares
for full output buffering, needed for optimal spacing in field
output layout.

Columns are currently set up to retain exactly the same output
as before. This needs some slight adjustments of the values
previously calculated in main(), as the width value introduced
here already includes the width of left delimiters and spacing
is not explicitly printed anymore whenever a field is printed.
These calculations will go away altogether once automatic width
calculation is implemented.

We can also remove explicit printing of newlines after the final
content for a given line is printed, flushing the last field on
a line will cause field_flush() to print newlines where
appropriate.

No changes in output expected here.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
---
v2: rebase after conflict with 00ac78d39c29 ("ss: print tcpi_rcv_ssthresh")

 misc/ss.c | 291 ++++++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 198 insertions(+), 93 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index a7d3b89e1478..42310ba4120d 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -103,11 +103,48 @@ int show_header = 1;
 int follow_events;
 int sctp_ino;
 
-int netid_width;
-int state_width;
-int addr_width;
-int serv_width;
-char *odd_width_pad = "";
+enum col_id {
+	COL_NETID,
+	COL_STATE,
+	COL_RECVQ,
+	COL_SENDQ,
+	COL_ADDR,
+	COL_SERV,
+	COL_RADDR,
+	COL_RSERV,
+	COL_EXT,
+	COL_MAX
+};
+
+enum col_align {
+	ALIGN_LEFT,
+	ALIGN_CENTER,
+	ALIGN_RIGHT
+};
+
+struct column {
+	const enum col_align align;
+	const char *header;
+	const char *ldelim;
+	int width;	/* Including delimiter. -1: fit to content, 0: hide */
+	int stored;	/* Characters buffered */
+	int printed;	/* Characters printed so far */
+};
+
+static struct column columns[] = {
+	{ ALIGN_LEFT,	"Netid",		"",	0,	0,	0 },
+	{ ALIGN_LEFT,	"State",		" ",	0,	0,	0 },
+	{ ALIGN_LEFT,	"Recv-Q",		" ",	7,	0,	0 },
+	{ ALIGN_LEFT,	"Send-Q",		" ",	7,	0,	0 },
+	{ ALIGN_RIGHT,	"Local Address:",	" ",	0,	0,	0 },
+	{ ALIGN_LEFT,	"Port",			"",	0,	0,	0 },
+	{ ALIGN_RIGHT,	"Peer Address:",	" ",	0,	0,	0 },
+	{ ALIGN_LEFT,	"Port",			"",	0,	0,	0 },
+	{ ALIGN_LEFT,	"",			"",	-1,	0,	0 },
+};
+
+static struct column *current_field = columns;
+static char field_buf[BUFSIZ];
 
 static const char *TCP_PROTO = "tcp";
 static const char *SCTP_PROTO = "sctp";
@@ -826,13 +863,113 @@ static const char *vsock_netid_name(int type)
 
 static void out(const char *fmt, ...)
 {
+	struct column *f = current_field;
 	va_list args;
 
 	va_start(args, fmt);
-	vfprintf(stdout, fmt, args);
+	f->stored += vsnprintf(field_buf + f->stored, BUFSIZ - f->stored,
+			       fmt, args);
 	va_end(args);
 }
 
+static int print_left_spacing(struct column *f)
+{
+	int s;
+
+	if (f->width < 0 || f->align == ALIGN_LEFT)
+		return 0;
+
+	s = f->width - f->stored - f->printed;
+	if (f->align == ALIGN_CENTER)
+		/* If count of total spacing is odd, shift right by one */
+		s = (s + 1) / 2;
+
+	if (s > 0)
+		return printf("%*c", s, ' ');
+
+	return 0;
+}
+
+static void print_right_spacing(struct column *f)
+{
+	int s;
+
+	if (f->width < 0 || f->align == ALIGN_RIGHT)
+		return;
+
+	s = f->width - f->printed;
+	if (f->align == ALIGN_CENTER)
+		s /= 2;
+
+	if (s > 0)
+		printf("%*c", s, ' ');
+}
+
+static int field_needs_delimiter(struct column *f)
+{
+	if (!f->stored)
+		return 0;
+
+	/* Was another field already printed on this line? */
+	for (f--; f >= columns; f--)
+		if (f->width)
+			return 1;
+
+	return 0;
+}
+
+/* Flush given field to screen together with delimiter and spacing */
+static void field_flush(struct column *f)
+{
+	if (!f->width)
+		return;
+
+	if (field_needs_delimiter(f))
+		f->printed = printf("%s", f->ldelim);
+
+	f->printed += print_left_spacing(f);
+	f->printed += printf("%s", field_buf);
+	print_right_spacing(f);
+
+	*field_buf = 0;
+	f->printed = 0;
+	f->stored = 0;
+}
+
+static int field_is_last(struct column *f)
+{
+	return f - columns == COL_MAX - 1;
+}
+
+static void field_next(void)
+{
+	field_flush(current_field);
+
+	if (field_is_last(current_field)) {
+		printf("\n");
+		current_field = columns;
+	} else {
+		current_field++;
+	}
+}
+
+/* Walk through fields and flush them until we reach the desired one */
+static void field_set(enum col_id id)
+{
+	while (id != current_field - columns)
+		field_next();
+}
+
+/* Print header for all non-empty columns */
+static void print_header(void)
+{
+	while (!field_is_last(current_field)) {
+		if (current_field->width)
+			out(current_field->header);
+		field_next();
+	}
+}
+
 static void sock_state_print(struct sockstat *s)
 {
 	const char *sock_name;
@@ -872,18 +1009,21 @@ static void sock_state_print(struct sockstat *s)
 		sock_name = "unknown";
 	}
 
-	if (netid_width)
-		out("%-*s ", netid_width,
-		    is_sctp_assoc(s, sock_name) ? "" : sock_name);
-	if (state_width) {
-		if (is_sctp_assoc(s, sock_name))
-			out("`- %-*s ", state_width - 3,
-			    sctp_sstate_name[s->state]);
-		else
-			out("%-*s ", state_width, sstate_name[s->state]);
+	if (is_sctp_assoc(s, sock_name)) {
+		field_set(COL_STATE);		/* Empty Netid field */
+		out("`- %s", sctp_sstate_name[s->state]);
+	} else {
+		field_set(COL_NETID);
+		out("%s", sock_name);
+		field_set(COL_STATE);
+		out("%s", sstate_name[s->state]);
 	}
 
-	out("%-6d %-6d %s", s->rq, s->wq, odd_width_pad);
+	field_set(COL_RECVQ);
+	out("%-6d", s->rq);
+	field_set(COL_SENDQ);
+	out("%-6d", s->wq);
+	field_set(COL_ADDR);
 }
 
 static void sock_details_print(struct sockstat *s)
@@ -898,21 +1038,17 @@ static void sock_details_print(struct sockstat *s)
 		out(" fwmark:0x%x", s->mark);
 }
 
-static void sock_addr_print_width(int addr_len, const char *addr, char *delim,
-		int port_len, const char *port, const char *ifname)
-{
-	if (ifname) {
-		out("%*s%%%s%s%-*s ", addr_len, addr, ifname, delim,
-		    port_len, port);
-	} else {
-		out("%*s%s%-*s ", addr_len, addr, delim, port_len, port);
-	}
-}
-
 static void sock_addr_print(const char *addr, char *delim, const char *port,
 		const char *ifname)
 {
-	sock_addr_print_width(addr_width, addr, delim, serv_width, port, ifname);
+	if (ifname)
+		out("%s" "%%" "%s%s", addr, ifname, delim);
+	else
+		out("%s%s", addr, delim);
+
+	field_next();
+	out("%s", port);
+	field_next();
 }
 
 static const char *print_ms_timer(unsigned int timeout)
@@ -1093,7 +1229,6 @@ static void inet_addr_print(const inet_prefix *a, int port,
 {
 	char buf[1024];
 	const char *ap = buf;
-	int est_len = addr_width;
 	const char *ifname = NULL;
 
 	if (a->family == AF_INET) {
@@ -1112,24 +1247,13 @@ static void inet_addr_print(const inet_prefix *a, int port,
 					 "[%s]", ap);
 				ap = buf;
 			}
-
-			est_len = strlen(ap);
-			if (est_len <= addr_width)
-				est_len = addr_width;
-			else
-				est_len = addr_width + ((est_len-addr_width+3)/4)*4;
 		}
 	}
 
-	if (ifindex) {
-		ifname   = ll_index_to_name(ifindex);
-		est_len -= strlen(ifname) + 1;  /* +1 for percent char */
-		if (est_len < 0)
-			est_len = 0;
-	}
+	if (ifindex)
+		ifname = ll_index_to_name(ifindex);
 
-	sock_addr_print_width(est_len, ap, ":", serv_width, resolve_service(port),
-			ifname);
+	sock_addr_print(ap, ":", resolve_service(port), ifname);
 }
 
 struct aafilter {
@@ -2166,7 +2290,6 @@ static int tcp_show_line(char *line, const struct filter *f, int family)
 	if (show_tcpinfo)
 		tcp_stats_print(&s);
 
-	out("\n");
 	return 0;
 }
 
@@ -2547,7 +2670,6 @@ static int inet_show_sock(struct nlmsghdr *nlh,
 	}
 	sctp_ino = s->ino;
 
-	out("\n");
 	return 0;
 }
 
@@ -3005,7 +3127,6 @@ static int dgram_show_line(char *line, const struct filter *f, int family)
 	if (show_details && opt[0])
 		out(" opt:\"%s\"", opt);
 
-	out("\n");
 	return 0;
 }
 
@@ -3184,7 +3305,6 @@ static int unix_show_sock(const struct sockaddr_nl *addr, struct nlmsghdr *nlh,
 			    mask & 1 ? '-' : '<', mask & 2 ? '-' : '>');
 		}
 	}
-	out("\n");
 
 	return 0;
 }
@@ -3341,8 +3461,6 @@ static int unix_show(struct filter *f)
 		if (++cnt > MAX_UNIX_REMEMBER) {
 			while (list) {
 				unix_stats_print(list, f);
-				out("\n");
-
 				unix_list_drop_first(&list);
 			}
 			cnt = 0;
@@ -3351,8 +3469,6 @@ static int unix_show(struct filter *f)
 	fclose(fp);
 	while (list) {
 		unix_stats_print(list, f);
-		out("\n");
-
 		unix_list_drop_first(&list);
 	}
 
@@ -3527,7 +3643,6 @@ static int packet_show_sock(const struct sockaddr_nl *addr,
 			fil++;
 		}
 	}
-	out("\n");
 	return 0;
 }
 
@@ -3570,7 +3685,6 @@ static int packet_show_line(char *buf, const struct filter *f, int fam)
 	if (packet_stats_print(&stat, f))
 		return 0;
 
-	out("\n");
 	return 0;
 }
 
@@ -3690,7 +3804,6 @@ static int netlink_show_one(struct filter *f,
 	if (show_details) {
 		out(" sk=%llx cb=%llx groups=0x%08x", sk, cb, groups);
 	}
-	out("\n");
 
 	return 0;
 }
@@ -3728,7 +3841,6 @@ static int netlink_show_sock(const struct sockaddr_nl *addr,
 	if (show_mem) {
 		out("\t");
 		print_skmeminfo(tb, NETLINK_DIAG_MEMINFO);
-		out("\n");
 	}
 
 	return 0;
@@ -3818,8 +3930,6 @@ static void vsock_stats_print(struct sockstat *s, struct filter *f)
 	vsock_addr_print(&s->remote, s->rport);
 
 	proc_ctx_print(s);
-
-	out("\n");
 }
 
 static int vsock_show_sock(const struct sockaddr_nl *addr,
@@ -4539,13 +4649,17 @@ int main(int argc, char *argv[])
 	if (ssfilter_parse(&current_filter.f, argc, argv, filter_fp))
 		usage();
 
-	netid_width = 0;
 	if (current_filter.dbs&(current_filter.dbs-1))
-		netid_width = 5;
+		columns[COL_NETID].width = 6;
 
-	state_width = 0;
 	if (current_filter.states&(current_filter.states-1))
-		state_width = 10;
+		columns[COL_STATE].width = 10;
+
+	/* If Netid or State are hidden, no delimiter before next column */
+	if (!columns[COL_NETID].width)
+		columns[COL_STATE].width--;
+	else if (!columns[COL_STATE].width)
+		columns[COL_RECVQ].width--;
 
 	if (isatty(STDOUT_FILENO)) {
 		struct winsize w;
@@ -4556,49 +4670,38 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	addrp_width = screen_width;
-	if (netid_width)
-		addrp_width -= netid_width + 1;
-	if (state_width)
-		addrp_width -= state_width + 1;
-	addrp_width -= 14;
+	addrp_width = screen_width -
+		      columns[COL_NETID].width -
+		      columns[COL_STATE].width -
+		      columns[COL_RECVQ].width -
+		      columns[COL_SENDQ].width;
 
 	if (addrp_width&1) {
-		if (netid_width)
-			netid_width++;
-		else if (state_width)
-			state_width++;
+		if (columns[COL_NETID].width)
+			columns[COL_NETID].width++;
+		else if (columns[COL_STATE].width)
+			columns[COL_STATE].width++;
 		else
-			odd_width_pad = " ";
+			columns[COL_SENDQ].width++;
 	}
 
 	addrp_width /= 2;
-	addrp_width--;
 
-	serv_width = resolve_services ? 7 : 5;
+	columns[COL_SERV].width = resolve_services ? 8 : 6;
+	if (addrp_width < 15 + columns[COL_SERV].width)
+		addrp_width = 15 + columns[COL_SERV].width;
 
-	if (addrp_width < 15+serv_width+1)
-		addrp_width = 15+serv_width+1;
-
-	addr_width = addrp_width - serv_width - 1;
-
-	if (show_header) {
-		if (netid_width)
-			out("%-*s ", netid_width, "Netid");
-		if (state_width)
-			out("%-*s ", state_width, "State");
-		out("%-6s %-6s %s", "Recv-Q", "Send-Q", odd_width_pad);
-	}
+	columns[COL_ADDR].width = addrp_width - columns[COL_SERV].width;
 
 	/* Make enough space for the local/remote port field */
-	addr_width -= 13;
-	serv_width += 13;
+	columns[COL_ADDR].width -= 13;
+	columns[COL_SERV].width += 13;
 
-	if (show_header) {
-		out("%*s:%-*s %*s:%-*s\n",
-		    addr_width, "Local Address", serv_width, "Port",
-		    addr_width, "Peer Address", serv_width, "Port");
-	}
+	columns[COL_RADDR].width = columns[COL_ADDR].width;
+	columns[COL_RSERV].width = columns[COL_SERV].width;
+
+	if (show_header)
+		print_header();
 
 	fflush(stdout);
 
@@ -4627,5 +4730,7 @@ int main(int argc, char *argv[])
 	if (show_users || show_proc_ctx || show_sock_ctx)
 		user_ent_destroy();
 
+	field_next();
+
 	return 0;
 }
-- 
2.9.4

^ permalink raw reply related

* [PATCH iproute2 net-next v2 3/4] ss: Buffer raw fields first, then render them as a table
From: Stefano Brivio @ 2017-12-12  0:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Sabrina Dubroca
In-Reply-To: <cover.1513039237.git.sbrivio@redhat.com>

This allows us to measure the maximum field length for each
column before printing fields and will permit us to apply
optimal field spacing and distribution. Structure of the output
buffer with chunked allocation is described in comments.

Output is still unchanged, original spacing is used.

Running over one million sockets with -tul options by simply
modifying main() to loop 50,000 times over the *_show()
functions, buffering the whole output and rendering it at the
end, with 10 UDP sockets, 10 TCP sockets, while throwing
output away, doesn't show significant changes in execution time
on my laptop with an Intel i7-6600U CPU:

- before this patch:
$ time ./ss -tul > /dev/null
real	0m29.899s
user	0m2.017s
sys	0m27.801s

- after this patch:
$ time ./ss -tul > /dev/null
real	0m29.827s
user	0m1.942s
sys	0m27.812s

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
---
v2: rebase after conflict with 00ac78d39c29 ("ss: print tcpi_rcv_ssthresh")

 misc/ss.c | 271 +++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 225 insertions(+), 46 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 42310ba4120d..166267974c36 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -47,6 +47,8 @@
 #include <linux/vm_sockets_diag.h>
 
 #define MAGIC_SEQ 123456
+#define BUF_CHUNK (1024 * 1024)
+#define LEN_ALIGN(x) (((x) + 1) & ~1)
 
 #define DIAG_REQUEST(_req, _r)						    \
 	struct {							    \
@@ -127,24 +129,45 @@ struct column {
 	const char *header;
 	const char *ldelim;
 	int width;	/* Including delimiter. -1: fit to content, 0: hide */
-	int stored;	/* Characters buffered */
-	int printed;	/* Characters printed so far */
 };
 
 static struct column columns[] = {
-	{ ALIGN_LEFT,	"Netid",		"",	0,	0,	0 },
-	{ ALIGN_LEFT,	"State",		" ",	0,	0,	0 },
-	{ ALIGN_LEFT,	"Recv-Q",		" ",	7,	0,	0 },
-	{ ALIGN_LEFT,	"Send-Q",		" ",	7,	0,	0 },
-	{ ALIGN_RIGHT,	"Local Address:",	" ",	0,	0,	0 },
-	{ ALIGN_LEFT,	"Port",			"",	0,	0,	0 },
-	{ ALIGN_RIGHT,	"Peer Address:",	" ",	0,	0,	0 },
-	{ ALIGN_LEFT,	"Port",			"",	0,	0,	0 },
-	{ ALIGN_LEFT,	"",			"",	-1,	0,	0 },
+	{ ALIGN_LEFT,	"Netid",		"",	0 },
+	{ ALIGN_LEFT,	"State",		" ",	0 },
+	{ ALIGN_LEFT,	"Recv-Q",		" ",	7 },
+	{ ALIGN_LEFT,	"Send-Q",		" ",	7 },
+	{ ALIGN_RIGHT,	"Local Address:",	" ",	0 },
+	{ ALIGN_LEFT,	"Port",			"",	0 },
+	{ ALIGN_RIGHT,	"Peer Address:",	" ",	0 },
+	{ ALIGN_LEFT,	"Port",			"",	0 },
+	{ ALIGN_LEFT,	"",			"",	-1 },
 };
 
 static struct column *current_field = columns;
-static char field_buf[BUFSIZ];
+
+/* Output buffer: chained chunks of BUF_CHUNK bytes. Each field is written to
+ * the buffer as a variable size token. A token consists of a 16 bits length
+ * field, followed by a string which is not NULL-terminated.
+ *
+ * A new chunk is allocated and linked when the current chunk doesn't have
+ * enough room to store the current token as a whole.
+ */
+struct buf_chunk {
+	struct buf_chunk *next;	/* Next chained chunk */
+	char *end;		/* Current end of content */
+	char data[0];
+};
+
+struct buf_token {
+	uint16_t len;		/* Data length, excluding length descriptor */
+	char data[0];
+};
+
+static struct {
+	struct buf_token *cur;	/* Position of current token in chunk */
+	struct buf_chunk *head;	/* First chunk */
+	struct buf_chunk *tail;	/* Current chunk */
+} buffer;
 
 static const char *TCP_PROTO = "tcp";
 static const char *SCTP_PROTO = "sctp";
@@ -861,25 +884,109 @@ static const char *vsock_netid_name(int type)
 	}
 }
 
+/* Allocate and initialize a new buffer chunk */
+static struct buf_chunk *buf_chunk_new(void)
+{
+	struct buf_chunk *new = malloc(BUF_CHUNK);
+
+	if (!new)
+		abort();
+
+	new->next = NULL;
+
+	/* This is also the last block */
+	buffer.tail = new;
+
+	/* Next token will be stored at the beginning of chunk data area, and
+	 * its initial length is zero.
+	 */
+	buffer.cur = (struct buf_token *)new->data;
+	buffer.cur->len = 0;
+
+	new->end = buffer.cur->data;
+
+	return new;
+}
+
+/* Return available tail room in given chunk */
+static int buf_chunk_avail(struct buf_chunk *chunk)
+{
+	return BUF_CHUNK - offsetof(struct buf_chunk, data) -
+	       (chunk->end - chunk->data);
+}
+
+/* Update end pointer and token length, link new chunk if we hit the end of the
+ * current one. Return -EAGAIN if we got a new chunk, caller has to print again.
+ */
+static int buf_update(int len)
+{
+	struct buf_chunk *chunk = buffer.tail;
+	struct buf_token *t = buffer.cur;
+
+	/* Claim success if new content fits in the current chunk, and anyway
+	 * if this is the first token in the chunk: in the latter case,
+	 * allocating a new chunk won't help, so we'll just cut the output.
+	 */
+	if ((len < buf_chunk_avail(chunk) && len != -1 /* glibc < 2.0.6 */) ||
+	    t == (struct buf_token *)chunk->data) {
+		len = min(len, buf_chunk_avail(chunk));
+
+		/* Total field length can't exceed 2^16 bytes, cut as needed */
+		len = min(len, USHRT_MAX - t->len);
+
+		chunk->end += len;
+		t->len += len;
+		return 0;
+	}
+
+	/* Content truncated, time to allocate more */
+	chunk->next = buf_chunk_new();
+
+	/* Copy current token over to new chunk, including length descriptor */
+	memcpy(chunk->next->data, t, sizeof(t->len) + t->len);
+	chunk->next->end += t->len;
+
+	/* Discard partially written field in old chunk */
+	chunk->end -= t->len + sizeof(t->len);
+
+	return -EAGAIN;
+}
+
+/* Append content to buffer as part of the current field */
 static void out(const char *fmt, ...)
 {
 	struct column *f = current_field;
 	va_list args;
+	char *pos;
+	int len;
+
+	if (!f->width)
+		return;
+
+	if (!buffer.head)
+		buffer.head = buf_chunk_new();
+
+again:	/* Append to buffer: if we have a new chunk, print again */
 
+	pos = buffer.cur->data + buffer.cur->len;
 	va_start(args, fmt);
-	f->stored += vsnprintf(field_buf + f->stored, BUFSIZ - f->stored,
-			       fmt, args);
+
+	/* Limit to tail room. If we hit the limit, buf_update() will tell us */
+	len = vsnprintf(pos, buf_chunk_avail(buffer.tail), fmt, args);
 	va_end(args);
+
+	if (buf_update(len))
+		goto again;
 }
 
-static int print_left_spacing(struct column *f)
+static int print_left_spacing(struct column *f, int stored, int printed)
 {
 	int s;
 
 	if (f->width < 0 || f->align == ALIGN_LEFT)
 		return 0;
 
-	s = f->width - f->stored - f->printed;
+	s = f->width - stored - printed;
 	if (f->align == ALIGN_CENTER)
 		/* If count of total spacing is odd, shift right by one */
 		s = (s + 1) / 2;
@@ -890,14 +997,14 @@ static int print_left_spacing(struct column *f)
 	return 0;
 }
 
-static void print_right_spacing(struct column *f)
+static void print_right_spacing(struct column *f, int printed)
 {
 	int s;
 
 	if (f->width < 0 || f->align == ALIGN_RIGHT)
 		return;
 
-	s = f->width - f->printed;
+	s = f->width - printed;
 	if (f->align == ALIGN_CENTER)
 		s /= 2;
 
@@ -905,35 +1012,29 @@ static void print_right_spacing(struct column *f)
 		printf("%*c", s, ' ');
 }
 
-static int field_needs_delimiter(struct column *f)
-{
-	if (!f->stored)
-		return 0;
-
-	/* Was another field already printed on this line? */
-	for (f--; f >= columns; f--)
-		if (f->width)
-			return 1;
-
-	return 0;
-}
-
-/* Flush given field to screen together with delimiter and spacing */
+/* Done with field: update buffer pointer, start new token after current one */
 static void field_flush(struct column *f)
 {
+	struct buf_chunk *chunk = buffer.tail;
+	unsigned int pad = buffer.cur->len % 2;
+
 	if (!f->width)
 		return;
 
-	if (field_needs_delimiter(f))
-		f->printed = printf("%s", f->ldelim);
-
-	f->printed += print_left_spacing(f);
-	f->printed += printf("%s", field_buf);
-	print_right_spacing(f);
+	/* We need a new chunk if we can't store the next length descriptor.
+	 * Mind the gap between end of previous token and next aligned position
+	 * for length descriptor.
+	 */
+	if (buf_chunk_avail(chunk) - pad < sizeof(buffer.cur->len)) {
+		chunk->end += pad;
+		chunk->next = buf_chunk_new();
+		return;
+	}
 
-	*field_buf = 0;
-	f->printed = 0;
-	f->stored = 0;
+	buffer.cur = (struct buf_token *)(buffer.cur->data +
+					  LEN_ALIGN(buffer.cur->len));
+	buffer.cur->len = 0;
+	buffer.tail->end = buffer.cur->data;
 }
 
 static int field_is_last(struct column *f)
@@ -945,12 +1046,10 @@ static void field_next(void)
 {
 	field_flush(current_field);
 
-	if (field_is_last(current_field)) {
-		printf("\n");
+	if (field_is_last(current_field))
 		current_field = columns;
-	} else {
+	else
 		current_field++;
-	}
 }
 
 /* Walk through fields and flush them until we reach the desired one */
@@ -970,6 +1069,86 @@ static void print_header(void)
 	}
 }
 
+/* Get the next available token in the buffer starting from the current token */
+static struct buf_token *buf_token_next(struct buf_token *cur)
+{
+	struct buf_chunk *chunk = buffer.tail;
+
+	/* If we reached the end of chunk contents, get token from next chunk */
+	if (cur->data + LEN_ALIGN(cur->len) == chunk->end) {
+		buffer.tail = chunk = chunk->next;
+		return chunk ? (struct buf_token *)chunk->data : NULL;
+	}
+
+	return (struct buf_token *)(cur->data + LEN_ALIGN(cur->len));
+}
+
+/* Free up all allocated buffer chunks */
+static void buf_free_all(void)
+{
+	struct buf_chunk *tmp;
+
+	for (buffer.tail = buffer.head; buffer.tail; ) {
+		tmp = buffer.tail;
+		buffer.tail = buffer.tail->next;
+		free(tmp);
+	}
+	buffer.head = NULL;
+}
+
+/* Render buffered output with spacing and delimiters, then free up buffers */
+static void render(void)
+{
+	struct buf_token *token = (struct buf_token *)buffer.head->data;
+	int printed, line_started = 0, need_newline = 0;
+	struct column *f;
+
+	/* Ensure end alignment of last token, it wasn't necessarily flushed */
+	buffer.tail->end += buffer.cur->len % 2;
+
+	/* Rewind and replay */
+	buffer.tail = buffer.head;
+
+	f = columns;
+	while (!f->width)
+		f++;
+
+	while (token) {
+		/* Print left delimiter only if we already started a line */
+		if (line_started++)
+			printed = printf("%s", current_field->ldelim);
+		else
+			printed = 0;
+
+		/* Print field content from token data with spacing */
+		printed += print_left_spacing(f, token->len, printed);
+		printed += fwrite(token->data, 1, token->len, stdout);
+		print_right_spacing(f, printed);
+
+		/* Variable field size or overflow, won't align to screen */
+		if (printed > f->width)
+			need_newline = 1;
+
+		/* Go to next non-empty field, deal with end-of-line */
+		do {
+			if (field_is_last(f)) {
+				if (need_newline) {
+					printf("\n");
+					need_newline = 0;
+				}
+				f = columns;
+				line_started = 0;
+			} else {
+				f++;
+			}
+		} while (!f->width);
+
+		token = buf_token_next(token);
+	}
+
+	buf_free_all();
+}
+
 static void sock_state_print(struct sockstat *s)
 {
 	const char *sock_name;
@@ -4730,7 +4909,7 @@ int main(int argc, char *argv[])
 	if (show_users || show_proc_ctx || show_sock_ctx)
 		user_ent_destroy();
 
-	field_next();
+	render();
 
 	return 0;
 }
-- 
2.9.4

^ permalink raw reply related

* [PATCH iproute2 net-next v2 4/4] ss: Implement automatic column width calculation
From: Stefano Brivio @ 2017-12-12  0:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Sabrina Dubroca
In-Reply-To: <cover.1513039237.git.sbrivio@redhat.com>

Group fitting fields into lines and space them equally using the
remaining screen width for each line. If columns don't fit on
one line, break them into the least possible amount of lines and
keep them aligned across lines.

This is done by:
 - recording the length of the longest item in each column during
   formatting and buffering (which was added in the previous patch)
 - fitting as many fields as possible on each line of output
 - distributing the remaining padding space equally between the
   columns

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
---
v2: rebase after conflict with 00ac78d39c29 ("ss: print tcpi_rcv_ssthresh")

 misc/ss.c | 188 +++++++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 120 insertions(+), 68 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 166267974c36..9d21ed7a0705 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -128,19 +128,21 @@ struct column {
 	const enum col_align align;
 	const char *header;
 	const char *ldelim;
-	int width;	/* Including delimiter. -1: fit to content, 0: hide */
+	int disabled;
+	int width;	/* Calculated, including additional layout spacing */
+	int max_len;	/* Measured maximum field length in this column */
 };
 
 static struct column columns[] = {
-	{ ALIGN_LEFT,	"Netid",		"",	0 },
-	{ ALIGN_LEFT,	"State",		" ",	0 },
-	{ ALIGN_LEFT,	"Recv-Q",		" ",	7 },
-	{ ALIGN_LEFT,	"Send-Q",		" ",	7 },
-	{ ALIGN_RIGHT,	"Local Address:",	" ",	0 },
-	{ ALIGN_LEFT,	"Port",			"",	0 },
-	{ ALIGN_RIGHT,	"Peer Address:",	" ",	0 },
-	{ ALIGN_LEFT,	"Port",			"",	0 },
-	{ ALIGN_LEFT,	"",			"",	-1 },
+	{ ALIGN_LEFT,	"Netid",		"",	0, 0, 0 },
+	{ ALIGN_LEFT,	"State",		" ",	0, 0, 0 },
+	{ ALIGN_LEFT,	"Recv-Q",		" ",	0, 0, 0 },
+	{ ALIGN_LEFT,	"Send-Q",		" ",	0, 0, 0 },
+	{ ALIGN_RIGHT,	"Local Address:",	" ",	0, 0, 0 },
+	{ ALIGN_LEFT,	"Port",			"",	0, 0, 0 },
+	{ ALIGN_RIGHT,	"Peer Address:",	" ",	0, 0, 0 },
+	{ ALIGN_LEFT,	"Port",			"",	0, 0, 0 },
+	{ ALIGN_LEFT,	"",			"",	0, 0, 0 },
 };
 
 static struct column *current_field = columns;
@@ -960,7 +962,7 @@ static void out(const char *fmt, ...)
 	char *pos;
 	int len;
 
-	if (!f->width)
+	if (f->disabled)
 		return;
 
 	if (!buffer.head)
@@ -983,7 +985,7 @@ static int print_left_spacing(struct column *f, int stored, int printed)
 {
 	int s;
 
-	if (f->width < 0 || f->align == ALIGN_LEFT)
+	if (!f->width || f->align == ALIGN_LEFT)
 		return 0;
 
 	s = f->width - stored - printed;
@@ -1001,7 +1003,7 @@ static void print_right_spacing(struct column *f, int printed)
 {
 	int s;
 
-	if (f->width < 0 || f->align == ALIGN_RIGHT)
+	if (!f->width || f->align == ALIGN_RIGHT)
 		return;
 
 	s = f->width - printed;
@@ -1018,9 +1020,12 @@ static void field_flush(struct column *f)
 	struct buf_chunk *chunk = buffer.tail;
 	unsigned int pad = buffer.cur->len % 2;
 
-	if (!f->width)
+	if (f->disabled)
 		return;
 
+	if (buffer.cur->len > f->max_len)
+		f->max_len = buffer.cur->len;
+
 	/* We need a new chunk if we can't store the next length descriptor.
 	 * Mind the gap between end of previous token and next aligned position
 	 * for length descriptor.
@@ -1063,7 +1068,7 @@ static void field_set(enum col_id id)
 static void print_header(void)
 {
 	while (!field_is_last(current_field)) {
-		if (current_field->width)
+		if (!current_field->disabled)
 			out(current_field->header);
 		field_next();
 	}
@@ -1096,16 +1101,106 @@ static void buf_free_all(void)
 	buffer.head = NULL;
 }
 
+/* Calculate column width from contents length. If columns don't fit on one
+ * line, break them into the least possible amount of lines and keep them
+ * aligned across lines. Available screen space is equally spread between fields
+ * as additional spacing.
+ */
+static void render_calc_width(int screen_width)
+{
+	int first, len = 0, linecols = 0;
+	struct column *c, *eol = columns - 1;
+
+	/* First pass: set width for each column to measured content length */
+	for (first = 1, c = columns; c - columns < COL_MAX; c++) {
+		if (c->disabled)
+			continue;
+
+		if (!first && c->max_len)
+			c->width = c->max_len + strlen(c->ldelim);
+		else
+			c->width = c->max_len;
+
+		/* But don't exceed screen size. If we exceed the screen size
+		 * for even a single field, it will just start on a line of its
+		 * own and then naturally wrap.
+		 */
+		c->width = min(c->width, screen_width);
+
+		if (c->width)
+			first = 0;
+	}
+
+	/* Second pass: find out newlines and distribute available spacing */
+	for (c = columns; c - columns < COL_MAX; c++) {
+		int pad, spacing, rem, last;
+		struct column *tmp;
+
+		if (!c->width)
+			continue;
+
+		linecols++;
+		len += c->width;
+
+		for (last = 1, tmp = c + 1; tmp - columns < COL_MAX; tmp++) {
+			if (tmp->width) {
+				last = 0;
+				break;
+			}
+		}
+
+		if (!last && len < screen_width) {
+			/* Columns fit on screen so far, nothing to do yet */
+			continue;
+		}
+
+		if (len == screen_width) {
+			/* Exact fit, just start with new line */
+			goto newline;
+		}
+
+		if (len > screen_width) {
+			/* Screen width exceeded: go back one column */
+			len -= c->width;
+			c--;
+			linecols--;
+		}
+
+		/* Distribute remaining space to columns on this line */
+		pad = screen_width - len;
+		spacing = pad / linecols;
+		rem = pad % linecols;
+		for (tmp = c; tmp > eol; tmp--) {
+			if (!tmp->width)
+				continue;
+
+			tmp->width += spacing;
+			if (rem) {
+				tmp->width++;
+				rem--;
+			}
+		}
+
+newline:
+		/* Line break: reset line counters, mark end-of-line */
+		eol = c;
+		len = 0;
+		linecols = 0;
+	}
+}
+
 /* Render buffered output with spacing and delimiters, then free up buffers */
-static void render(void)
+static void render(int screen_width)
 {
 	struct buf_token *token = (struct buf_token *)buffer.head->data;
-	int printed, line_started = 0, need_newline = 0;
+	int printed, line_started = 0;
 	struct column *f;
 
 	/* Ensure end alignment of last token, it wasn't necessarily flushed */
 	buffer.tail->end += buffer.cur->len % 2;
 
+	render_calc_width(screen_width);
+
 	/* Rewind and replay */
 	buffer.tail = buffer.head;
 
@@ -1125,23 +1220,16 @@ static void render(void)
 		printed += fwrite(token->data, 1, token->len, stdout);
 		print_right_spacing(f, printed);
 
-		/* Variable field size or overflow, won't align to screen */
-		if (printed > f->width)
-			need_newline = 1;
-
 		/* Go to next non-empty field, deal with end-of-line */
 		do {
 			if (field_is_last(f)) {
-				if (need_newline) {
-					printf("\n");
-					need_newline = 0;
-				}
+				printf("\n");
 				f = columns;
 				line_started = 0;
 			} else {
 				f++;
 			}
-		} while (!f->width);
+		} while (f->disabled);
 
 		token = buf_token_next(token);
 	}
@@ -4532,7 +4620,7 @@ int main(int argc, char *argv[])
 	FILE *filter_fp = NULL;
 	int ch;
 	int state_filter = 0;
-	int addrp_width, screen_width = 80;
+	int screen_width = 80;
 
 	while ((ch = getopt_long(argc, argv,
 				 "dhaletuwxnro460spbEf:miA:D:F:vVzZN:KHS",
@@ -4828,17 +4916,11 @@ int main(int argc, char *argv[])
 	if (ssfilter_parse(&current_filter.f, argc, argv, filter_fp))
 		usage();
 
-	if (current_filter.dbs&(current_filter.dbs-1))
-		columns[COL_NETID].width = 6;
-
-	if (current_filter.states&(current_filter.states-1))
-		columns[COL_STATE].width = 10;
+	if (!(current_filter.dbs & (current_filter.dbs - 1)))
+		columns[COL_NETID].disabled = 1;
 
-	/* If Netid or State are hidden, no delimiter before next column */
-	if (!columns[COL_NETID].width)
-		columns[COL_STATE].width--;
-	else if (!columns[COL_STATE].width)
-		columns[COL_RECVQ].width--;
+	if (!(current_filter.states & (current_filter.states - 1)))
+		columns[COL_STATE].disabled = 1;
 
 	if (isatty(STDOUT_FILENO)) {
 		struct winsize w;
@@ -4849,36 +4931,6 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	addrp_width = screen_width -
-		      columns[COL_NETID].width -
-		      columns[COL_STATE].width -
-		      columns[COL_RECVQ].width -
-		      columns[COL_SENDQ].width;
-
-	if (addrp_width&1) {
-		if (columns[COL_NETID].width)
-			columns[COL_NETID].width++;
-		else if (columns[COL_STATE].width)
-			columns[COL_STATE].width++;
-		else
-			columns[COL_SENDQ].width++;
-	}
-
-	addrp_width /= 2;
-
-	columns[COL_SERV].width = resolve_services ? 8 : 6;
-	if (addrp_width < 15 + columns[COL_SERV].width)
-		addrp_width = 15 + columns[COL_SERV].width;
-
-	columns[COL_ADDR].width = addrp_width - columns[COL_SERV].width;
-
-	/* Make enough space for the local/remote port field */
-	columns[COL_ADDR].width -= 13;
-	columns[COL_SERV].width += 13;
-
-	columns[COL_RADDR].width = columns[COL_ADDR].width;
-	columns[COL_RSERV].width = columns[COL_SERV].width;
-
 	if (show_header)
 		print_header();
 
@@ -4909,7 +4961,7 @@ int main(int argc, char *argv[])
 	if (show_users || show_proc_ctx || show_sock_ctx)
 		user_ent_destroy();
 
-	render();
+	render(screen_width);
 
 	return 0;
 }
-- 
2.9.4

^ permalink raw reply related

* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2017-12-12  1:07 UTC (permalink / raw)
  To: David Miller, Networking
  Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Jerome Brunet,
	Heiner Kallweit

Hi all,

Today's linux-next merge of the net-next tree got a conflict in:

  drivers/net/phy/meson-gxl.c

between commit:

  f1e2400a80ff ("net: phy: meson-gxl: detect LPA corruption")

from the net tree and commit:

  80274abafc60 ("net: phy: remove generic settings for callbacks config_aneg and read_status from drivers")

from the net-next tree.

I fixed it up (I just used the former) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging.  You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/net/phy/meson-gxl.c
index 700007dd4be5,401e3234be58..000000000000
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@@ -130,9 -58,7 +130,8 @@@ static struct phy_driver meson_gxl_phy[
  		.features	= PHY_BASIC_FEATURES,
  		.flags		= PHY_IS_INTERNAL,
  		.config_init	= meson_gxl_config_init,
- 		.config_aneg	= genphy_config_aneg,
  		.aneg_done      = genphy_aneg_done,
 +		.read_status	= meson_gxl_read_status,
  		.suspend        = genphy_suspend,
  		.resume         = genphy_resume,
  	},

^ permalink raw reply

* [PATCH bpf 0/3] Misc BPF fixes
From: Daniel Borkmann @ 2017-12-12  1:25 UTC (permalink / raw)
  To: ast; +Cc: netdev, Daniel Borkmann

Couple of outstanding fixes for BPF tree: 1) fixes a perf RB
corruption, 2) and 3) fixes a few build issues from the recent
bpf_perf_event.h uapi corrections. Thanks!

Daniel Borkmann (3):
  bpf: fix corruption on concurrent perf_event_output calls
  bpf: fix build issues on um due to mising bpf_perf_event.h
  bpf: fix broken BPF selftest build

 arch/um/include/asm/Kbuild              |  1 +
 kernel/trace/bpf_trace.c                | 19 ++++++++++++-------
 tools/include/uapi/asm/bpf_perf_event.h |  7 +++++++
 tools/testing/selftests/bpf/Makefile    | 13 +------------
 4 files changed, 21 insertions(+), 19 deletions(-)
 create mode 100644 tools/include/uapi/asm/bpf_perf_event.h

-- 
2.9.5

^ permalink raw reply

* [PATCH bpf 1/3] bpf: fix corruption on concurrent perf_event_output calls
From: Daniel Borkmann @ 2017-12-12  1:25 UTC (permalink / raw)
  To: ast; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20171212012532.30268-1-daniel@iogearbox.net>

When tracing and networking programs are both attached in the
system and both use event-output helpers that eventually call
into perf_event_output(), then we could end up in a situation
where the tracing attached program runs in user context while
a cls_bpf program is triggered on that same CPU out of softirq
context.

Since both rely on the same per-cpu perf_sample_data, we could
potentially corrupt it. This can only ever happen in a combination
of the two types; all tracing programs use a bpf_prog_active
counter to bail out in case a program is already running on
that CPU out of a different context. XDP and cls_bpf programs
by themselves don't have this issue as they run in the same
context only. Therefore, split both perf_sample_data so they
cannot be accessed from each other.

Fixes: 20b9d7ac4852 ("bpf: avoid excessive stack usage for perf_sample_data")
Reported-by: Alexei Starovoitov <ast@fb.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Tested-by: Song Liu <songliubraving@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/trace/bpf_trace.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 0ce99c3..40207c2 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -343,14 +343,13 @@ static const struct bpf_func_proto bpf_perf_event_read_value_proto = {
 	.arg4_type	= ARG_CONST_SIZE,
 };
 
-static DEFINE_PER_CPU(struct perf_sample_data, bpf_sd);
+static DEFINE_PER_CPU(struct perf_sample_data, bpf_trace_sd);
 
 static __always_inline u64
 __bpf_perf_event_output(struct pt_regs *regs, struct bpf_map *map,
-			u64 flags, struct perf_raw_record *raw)
+			u64 flags, struct perf_sample_data *sd)
 {
 	struct bpf_array *array = container_of(map, struct bpf_array, map);
-	struct perf_sample_data *sd = this_cpu_ptr(&bpf_sd);
 	unsigned int cpu = smp_processor_id();
 	u64 index = flags & BPF_F_INDEX_MASK;
 	struct bpf_event_entry *ee;
@@ -373,8 +372,6 @@ __bpf_perf_event_output(struct pt_regs *regs, struct bpf_map *map,
 	if (unlikely(event->oncpu != cpu))
 		return -EOPNOTSUPP;
 
-	perf_sample_data_init(sd, 0, 0);
-	sd->raw = raw;
 	perf_event_output(event, sd, regs);
 	return 0;
 }
@@ -382,6 +379,7 @@ __bpf_perf_event_output(struct pt_regs *regs, struct bpf_map *map,
 BPF_CALL_5(bpf_perf_event_output, struct pt_regs *, regs, struct bpf_map *, map,
 	   u64, flags, void *, data, u64, size)
 {
+	struct perf_sample_data *sd = this_cpu_ptr(&bpf_trace_sd);
 	struct perf_raw_record raw = {
 		.frag = {
 			.size = size,
@@ -392,7 +390,10 @@ BPF_CALL_5(bpf_perf_event_output, struct pt_regs *, regs, struct bpf_map *, map,
 	if (unlikely(flags & ~(BPF_F_INDEX_MASK)))
 		return -EINVAL;
 
-	return __bpf_perf_event_output(regs, map, flags, &raw);
+	perf_sample_data_init(sd, 0, 0);
+	sd->raw = &raw;
+
+	return __bpf_perf_event_output(regs, map, flags, sd);
 }
 
 static const struct bpf_func_proto bpf_perf_event_output_proto = {
@@ -407,10 +408,12 @@ static const struct bpf_func_proto bpf_perf_event_output_proto = {
 };
 
 static DEFINE_PER_CPU(struct pt_regs, bpf_pt_regs);
+static DEFINE_PER_CPU(struct perf_sample_data, bpf_misc_sd);
 
 u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
 		     void *ctx, u64 ctx_size, bpf_ctx_copy_t ctx_copy)
 {
+	struct perf_sample_data *sd = this_cpu_ptr(&bpf_misc_sd);
 	struct pt_regs *regs = this_cpu_ptr(&bpf_pt_regs);
 	struct perf_raw_frag frag = {
 		.copy		= ctx_copy,
@@ -428,8 +431,10 @@ u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
 	};
 
 	perf_fetch_caller_regs(regs);
+	perf_sample_data_init(sd, 0, 0);
+	sd->raw = &raw;
 
-	return __bpf_perf_event_output(regs, map, flags, &raw);
+	return __bpf_perf_event_output(regs, map, flags, sd);
 }
 
 BPF_CALL_0(bpf_get_current_task)
-- 
2.9.5

^ permalink raw reply related

* [PATCH bpf 3/3] bpf: fix broken BPF selftest build
From: Daniel Borkmann @ 2017-12-12  1:25 UTC (permalink / raw)
  To: ast; +Cc: netdev, Daniel Borkmann, Hendrik Brueckner,
	Arnaldo Carvalho de Melo
In-Reply-To: <20171212012532.30268-1-daniel@iogearbox.net>

At least on x86_64, the kernel's BPF selftests seemed to have stopped
to build due to 618e165b2a8e ("selftests/bpf: sync kernel headers and
introduce arch support in Makefile"):

  [...]
  In file included from test_verifier.c:29:0:
  ../../../include/uapi/linux/bpf_perf_event.h:11:32:
     fatal error: asm/bpf_perf_event.h: No such file or directory
   #include <asm/bpf_perf_event.h>
                                ^
  compilation terminated.
  [...]

While pulling in tools/arch/*/include/uapi/asm/bpf_perf_event.h seems
to work fine, there's no automated fall-back logic right now that would
do the same out of tools/include/uapi/asm-generic/bpf_perf_event.h. The
usual convention today is to add a include/[uapi/]asm/ equivalent that
would pull in the correct arch header or generic one as fall-back, all
ifdef'ed based on compiler target definition. It's similarly done also
in other cases such as tools/include/asm/barrier.h, thus adapt the same
here.

Fixes: 618e165b2a8e ("selftests/bpf: sync kernel headers and introduce arch support in Makefile")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 tools/include/uapi/asm/bpf_perf_event.h |  7 +++++++
 tools/testing/selftests/bpf/Makefile    | 13 +------------
 2 files changed, 8 insertions(+), 12 deletions(-)
 create mode 100644 tools/include/uapi/asm/bpf_perf_event.h

diff --git a/tools/include/uapi/asm/bpf_perf_event.h b/tools/include/uapi/asm/bpf_perf_event.h
new file mode 100644
index 0000000..13a5853
--- /dev/null
+++ b/tools/include/uapi/asm/bpf_perf_event.h
@@ -0,0 +1,7 @@
+#if defined(__aarch64__)
+#include "../../arch/arm64/include/uapi/asm/bpf_perf_event.h"
+#elif defined(__s390__)
+#include "../../arch/s390/include/uapi/asm/bpf_perf_event.h"
+#else
+#include <uapi/asm-generic/bpf_perf_event.h>
+#endif
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 21a2d76..792af7c 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -1,19 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 
-ifeq ($(srctree),)
-srctree := $(patsubst %/,%,$(dir $(CURDIR)))
-srctree := $(patsubst %/,%,$(dir $(srctree)))
-srctree := $(patsubst %/,%,$(dir $(srctree)))
-srctree := $(patsubst %/,%,$(dir $(srctree)))
-endif
-include $(srctree)/tools/scripts/Makefile.arch
-
-$(call detected_var,SRCARCH)
-
 LIBDIR := ../../../lib
 BPFDIR := $(LIBDIR)/bpf
 APIDIR := ../../../include/uapi
-ASMDIR:= ../../../arch/$(ARCH)/include/uapi
 GENDIR := ../../../../include/generated
 GENHDR := $(GENDIR)/autoconf.h
 
@@ -21,7 +10,7 @@ ifneq ($(wildcard $(GENHDR)),)
   GENFLAGS := -DHAVE_GENHDR
 endif
 
-CFLAGS += -Wall -O2 -I$(APIDIR) -I$(ASMDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include
+CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include
 LDLIBS += -lcap -lelf
 
 TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \
-- 
2.9.5

^ permalink raw reply related

* [PATCH bpf 2/3] bpf: fix build issues on um due to mising bpf_perf_event.h
From: Daniel Borkmann @ 2017-12-12  1:25 UTC (permalink / raw)
  To: ast; +Cc: netdev, Daniel Borkmann, Hendrik Brueckner, Richard Weinberger
In-Reply-To: <20171212012532.30268-1-daniel@iogearbox.net>

Since c895f6f703ad ("bpf: correct broken uapi for
BPF_PROG_TYPE_PERF_EVENT program type") um (uml) won't build
on i386 or x86_64:

  [...]
    CC      init/main.o
  In file included from ../include/linux/perf_event.h:18:0,
                   from ../include/linux/trace_events.h:10,
                   from ../include/trace/syscall.h:7,
                   from ../include/linux/syscalls.h:82,
                   from ../init/main.c:20:
  ../include/uapi/linux/bpf_perf_event.h:11:32: fatal error:
  asm/bpf_perf_event.h: No such file or directory #include
  <asm/bpf_perf_event.h>
  [...]

Lets add missing bpf_perf_event.h also to um arch. This seems
to be the only one still missing.

Fixes: c895f6f703ad ("bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program type")
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Suggested-by: Richard Weinberger <richard@sigma-star.at>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Tested-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Cc: Richard Weinberger <richard@sigma-star.at>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 arch/um/include/asm/Kbuild | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/um/include/asm/Kbuild b/arch/um/include/asm/Kbuild
index 50a32c3..73c57f6 100644
--- a/arch/um/include/asm/Kbuild
+++ b/arch/um/include/asm/Kbuild
@@ -1,4 +1,5 @@
 generic-y += barrier.h
+generic-y += bpf_perf_event.h
 generic-y += bug.h
 generic-y += clkdev.h
 generic-y += current.h
-- 
2.9.5

^ permalink raw reply related

* Re: [PATCH] selftests: bpf: Adding config fragment CONFIG_CGROUP_BPF=y
From: Daniel Borkmann @ 2017-12-12  1:34 UTC (permalink / raw)
  To: Naresh Kamboju, netdev; +Cc: guro, shuahkh, shuah, linux-kselftest
In-Reply-To: <1513020323-29591-1-git-send-email-naresh.kamboju@linaro.org>

On 12/11/2017 08:25 PM, Naresh Kamboju wrote:
> CONFIG_CGROUP_BPF=y is required for test_dev_cgroup test case.
> 
> Signed-off-by: Naresh Kamboju <naresh.kamboju@linaro.org>

Applied to bpf-next, thanks Naresh!

^ permalink raw reply

* [PATCH v2] igb: Free IRQs when device is hotplugged
From: Lyude Paul @ 2017-12-12  1:34 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: Todd Fujinaka, Stephen Hemminger, stable, Jeff Kirsher, netdev,
	linux-kernel

Recently I got a Caldigit TS3 Thunderbolt 3 dock, and noticed that upon
hotplugging my kernel would immediately crash due to igb:

[  680.825801] kernel BUG at drivers/pci/msi.c:352!
[  680.828388] invalid opcode: 0000 [#1] SMP
[  680.829194] Modules linked in: igb(O) thunderbolt i2c_algo_bit joydev vfat fat btusb btrtl btbcm btintel bluetooth ecdh_generic hp_wmi sparse_keymap rfkill wmi_bmof iTCO_wdt intel_rapl x86_pkg_temp_thermal coretemp crc32_pclmul snd_pcm rtsx_pci_ms mei_me snd_timer memstick snd pcspkr mei soundcore i2c_i801 tpm_tis psmouse shpchp wmi tpm_tis_core tpm video hp_wireless acpi_pad rtsx_pci_sdmmc mmc_core crc32c_intel serio_raw rtsx_pci mfd_core xhci_pci xhci_hcd i2c_hid i2c_core [last unloaded: igb]
[  680.831085] CPU: 1 PID: 78 Comm: kworker/u16:1 Tainted: G           O     4.15.0-rc3Lyude-Test+ #6
[  680.831596] Hardware name: HP HP ZBook Studio G4/826B, BIOS P71 Ver. 01.03 06/09/2017
[  680.832168] Workqueue: kacpi_hotplug acpi_hotplug_work_fn
[  680.832687] RIP: 0010:free_msi_irqs+0x180/0x1b0
[  680.833271] RSP: 0018:ffffc9000030fbf0 EFLAGS: 00010286
[  680.833761] RAX: ffff8803405f9c00 RBX: ffff88033e3d2e40 RCX: 000000000000002c
[  680.834278] RDX: 0000000000000000 RSI: 00000000000000ac RDI: ffff880340be2178
[  680.834832] RBP: 0000000000000000 R08: ffff880340be1ff0 R09: ffff8803405f9c00
[  680.835342] R10: 0000000000000000 R11: 0000000000000040 R12: ffff88033d63a298
[  680.835822] R13: ffff88033d63a000 R14: 0000000000000060 R15: ffff880341959000
[  680.836332] FS:  0000000000000000(0000) GS:ffff88034f440000(0000) knlGS:0000000000000000
[  680.836817] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  680.837360] CR2: 000055e64044afdf CR3: 0000000001c09002 CR4: 00000000003606e0
[  680.837954] Call Trace:
[  680.838853]  pci_disable_msix+0xce/0xf0
[  680.839616]  igb_reset_interrupt_capability+0x5d/0x60 [igb]
[  680.840278]  igb_remove+0x9d/0x110 [igb]
[  680.840764]  pci_device_remove+0x36/0xb0
[  680.841279]  device_release_driver_internal+0x157/0x220
[  680.841739]  pci_stop_bus_device+0x7d/0xa0
[  680.842255]  pci_stop_bus_device+0x2b/0xa0
[  680.842722]  pci_stop_bus_device+0x3d/0xa0
[  680.843189]  pci_stop_and_remove_bus_device+0xe/0x20
[  680.843627]  trim_stale_devices+0xf3/0x140
[  680.844086]  trim_stale_devices+0x94/0x140
[  680.844532]  trim_stale_devices+0xa6/0x140
[  680.845031]  ? get_slot_status+0x90/0xc0
[  680.845536]  acpiphp_check_bridge.part.5+0xfe/0x140
[  680.846021]  acpiphp_hotplug_notify+0x175/0x200
[  680.846581]  ? free_bridge+0x100/0x100
[  680.847113]  acpi_device_hotplug+0x8a/0x490
[  680.847535]  acpi_hotplug_work_fn+0x1a/0x30
[  680.848076]  process_one_work+0x182/0x3a0
[  680.848543]  worker_thread+0x2e/0x380
[  680.848963]  ? process_one_work+0x3a0/0x3a0
[  680.849373]  kthread+0x111/0x130
[  680.849776]  ? kthread_create_worker_on_cpu+0x50/0x50
[  680.850188]  ret_from_fork+0x1f/0x30
[  680.850601] Code: 43 14 85 c0 0f 84 d5 fe ff ff 31 ed eb 0f 83 c5 01 39 6b 14 0f 86 c5 fe ff ff 8b 7b 10 01 ef e8 b7 e4 d2 ff 48 83 78 70 00 74 e3 <0f> 0b 49 8d b5 a0 00 00 00 e8 62 6f d3 ff e9 c7 fe ff ff 48 8b
[  680.851497] RIP: free_msi_irqs+0x180/0x1b0 RSP: ffffc9000030fbf0

As it turns out, normally the freeing of IRQs that would fix this is called
inside of the scope of __igb_close(). However, since the device is
already gone by the point we try to unregister the netdevice from the
driver due to a hotplug we end up seeing that the netif isn't present
and thus, forget to free any of the device IRQs.

So: make sure that if we're in the process of dismantling the netdev, we
always allow __igb_close() to be called so that IRQs may be freed
normally. Additionally, only allow igb_close() to be called from
__igb_close() if it hasn't already been called for the given adapter.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Fixes: 9474933caf21 ("igb: close/suspend race in netif_device_detach")
Cc: Todd Fujinaka <todd.fujinaka@intel.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: stable@vger.kernel.org
---
Changes since v1:
  - Remove code for freeing IRQs from igb_remove(), unbreak
    __igb_close() instead (re: Stephen Hemminger)

 drivers/net/ethernet/intel/igb/igb_main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index c208753ff5b7..a1083fd074dd 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3663,7 +3663,9 @@ static int __igb_close(struct net_device *netdev, bool suspending)
 	if (!suspending)
 		pm_runtime_get_sync(&pdev->dev);
 
-	igb_down(adapter);
+	if (!test_bit(__IGB_DOWN, &adapter->state))
+		igb_down(adapter);
+
 	igb_free_irq(adapter);
 
 	igb_free_all_tx_resources(adapter);
@@ -3676,7 +3678,7 @@ static int __igb_close(struct net_device *netdev, bool suspending)
 
 int igb_close(struct net_device *netdev)
 {
-	if (netif_device_present(netdev))
+	if (netif_device_present(netdev) || netdev->dismantle)
 		return __igb_close(netdev, false);
 	return 0;
 }
-- 
2.14.3

^ permalink raw reply related

* linux-next: build failure after merge of the mac80211-next tree
From: Stephen Rothwell @ 2017-12-12  1:58 UTC (permalink / raw)
  To: Johannes Berg, Kalle Valo, Wireless
  Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Felix Fietkau,
	Lorenzo Bianconi, Toke Høiland-Jørgensen, David Miller,
	Networking

Hi Johannes,

After merging the mac80211-next tree, today's linux-next build (x86_64
allmodconfig) failed like this:

drivers/net/wireless/mediatek/mt76/mt76x2_main.c:539:19: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
  .wake_tx_queue = mt76_wake_tx_queue,
                   ^
drivers/net/wireless/mediatek/mt76/mt76x2_main.c:539:19: note: (near initialization for 'mt76x2_ops.wake_tx_queue')

Caused by commits

  17f1de56df05 ("mt76: add common code shared between multiple chipsets")
  7bc04215a66b ("mt76: add driver code for MT76x2e")

from the wireless-drivers-next tree interacting with commit

  e937b8da5a59 ("mac80211: Add TXQ scheduling API")

from the mac80211-next tree.

I applied the below hack merge fix ... please let me know if something
more/better is required.  Someone needs to remember to tell Dave when
these trees meet in his tree.

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 12 Dec 2017 12:50:40 +1100
Subject: [PATCH] mt76: fix up for "mac80211: Add TXQ scheduling API"

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/net/wireless/mediatek/mt76/mt76.h |  2 +-
 drivers/net/wireless/mediatek/mt76/tx.c   | 10 +++++++---
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index aa0880bbea7f..e395d3859212 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -338,7 +338,7 @@ void mt76_tx(struct mt76_dev *dev, struct ieee80211_sta *sta,
 	     struct mt76_wcid *wcid, struct sk_buff *skb);
 void mt76_txq_init(struct mt76_dev *dev, struct ieee80211_txq *txq);
 void mt76_txq_remove(struct mt76_dev *dev, struct ieee80211_txq *txq);
-void mt76_wake_tx_queue(struct ieee80211_hw *hw, struct ieee80211_txq *txq);
+void mt76_wake_tx_queue(struct ieee80211_hw *hw);
 void mt76_stop_tx_queues(struct mt76_dev *dev, struct ieee80211_sta *sta,
 			 bool send_bar);
 void mt76_txq_schedule(struct mt76_dev *dev, struct mt76_queue *hwq);
diff --git a/drivers/net/wireless/mediatek/mt76/tx.c b/drivers/net/wireless/mediatek/mt76/tx.c
index 4eef69bd8a9e..ad414af0750f 100644
--- a/drivers/net/wireless/mediatek/mt76/tx.c
+++ b/drivers/net/wireless/mediatek/mt76/tx.c
@@ -463,12 +463,16 @@ void mt76_stop_tx_queues(struct mt76_dev *dev, struct ieee80211_sta *sta,
 }
 EXPORT_SYMBOL_GPL(mt76_stop_tx_queues);
 
-void mt76_wake_tx_queue(struct ieee80211_hw *hw, struct ieee80211_txq *txq)
+void mt76_wake_tx_queue(struct ieee80211_hw *hw)
 {
+	struct ieee80211_txq *txq;
 	struct mt76_dev *dev = hw->priv;
-	struct mt76_txq *mtxq = (struct mt76_txq *) txq->drv_priv;
-	struct mt76_queue *hwq = mtxq->hwq;
+	struct mt76_txq *mtxq;
+	struct mt76_queue *hwq;
 
+	txq = ieee80211_next_txq(hw);
+	mtxq = (struct mt76_txq *) txq->drv_priv;
+	hwq = mtxq->hwq;
 	spin_lock_bh(&hwq->lock);
 	if (list_empty(&mtxq->list))
 		list_add_tail(&mtxq->list, &hwq->swq);
-- 
2.15.0

-- 
Cheers,
Stephen Rothwell

^ permalink raw reply related

* Re: [PATCH v3 17/33] nds32: VDSO support
From: Vincent Chen @ 2017-12-12  1:58 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Greentime Hu, Greentime, Linux Kernel Mailing List, Arnd Bergmann,
	linux-arch, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Rob Herring, netdev, DTML, Al Viro, David Howells, Will Deacon,
	Daniel Lezcano, linux-serial, Geert Uytterhoeven, Linus Walleij,
	Greg KH, Vincent Chen
In-Reply-To: <20171208121405.dy6raczdlbxtbnfo@lakrids.cambridge.arm.com>

2017-12-08 20:14 GMT+08:00 Mark Rutland <mark.rutland@arm.com>:
> On Fri, Dec 08, 2017 at 07:54:42PM +0800, Greentime Hu wrote:
>> 2017-12-08 18:21 GMT+08:00 Mark Rutland <mark.rutland@arm.com>:
>> > On Fri, Dec 08, 2017 at 05:12:00PM +0800, Greentime Hu wrote:
>> >> +static int grab_timer_node_info(void)
>> >> +{
>> >> +     struct device_node *timer_node;
>> >> +
>> >> +     timer_node = of_find_node_by_name(NULL, "timer");
>> >
>> > Please use a compatible string, rather than matching the timer by name.
>> >
>> > It's plausible that you have multiple nodes called "timer" in the DT,
>> > under different parent nodes, and this might not be the device you
>> > think it is. I see your dt in patch 24 has two timer nodes.
>> >
>> > It would be best if your clocksource driver exposed some stuct that you
>> > looked at here, so that you're guaranteed to user the same device.
>>
>> We'd like to use "timer" here because there are 2 different timer IPs
>> and we are sure that they won't be in the same SoC.
>> We think this implementation in VDSO should be platform independent to
>> get cycle-count register.
>> Our customer or other SoC provider who can use "timer" and define
>> cycle-count-offset or cycle-count-down then we can get the correct
>> cycle-count.
>
> This is not the right way to do things.
>
> So from a DT perspective, NAK.
>
> You should not add properties to arbitrary DT bindings to handle a Linux
> implementation detail.
>
> Please remove this DT code, and have the drivers for those timer blocks
> export this information to your vdso code somehow.
>

Hi, Mark:
Based on your suggestion, we define a new sturct timer_info to let
timer driver record the value
of cycle-count-offset and cycle-count-down in timer_init function. The
above code in timer driver
is validate only when CONFIG_NDS32 is defined.

>> We sent atcpit100 patch last time along with our arch, however we'd
>> like to send it to its sub system this time and my colleague is still
>> working on it.
>> He may send the timer patch next week.
>
> I think that it would make sense for that patch to be part of the arch
> port, especially given that (AFAICT) there is no dirver for the other
> timer IP that you mention.
>
> [...]
>
>> >> +int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
>> >> +{
>> >
>> >> +     /*Map timer to user space */
>> >> +     vdso_base += PAGE_SIZE;
>> >> +     prot = __pgprot(_PAGE_V | _PAGE_M_UR_KR | _PAGE_D |
>> >> +                     _PAGE_G | _PAGE_C_DEV);
>> >> +     ret = io_remap_pfn_range(vma, vdso_base, timer_res.start >> PAGE_SHIFT,
>> >> +                              PAGE_SIZE, prot);
>> >> +     if (ret)
>> >> +             goto up_fail;
>> >
>> > Maybe this is fine, but it looks a bit suspicious.
>> >
>> > Is it safe to map IO memory to a userspace process like this?
>> >
>> > In general that isn't safe, since userspace could access other registers
>> > (if those exist), perform accesses that change the state of hardware, or
>> > make unsupported access types (e.g. unaligned, atomic) that result in
>> > errors the kernel can't handle.
>> >
>> > Does none of that apply here?
>>
>> We only provide read permission to this page so hareware state won't
>> be chagned. It will trigger exception if we try to write.
>> We will check about the alignment/atomic issue of this region.
>

For alignment issue, we intentionally make an un-alignment read to
access this region and we
got "Segmentation fault" as expected.


Thanks,
Vincent

> Ok, thanks.
>
> This is another reason to only do this for devices/drivers that we have
> drivers for, since we can't know that this is safe in general.
>
> Thanks,
> Mark.

^ permalink raw reply

* Re: [PATCH v3 net-next 0/9] net: Generic network resolver backend and ILA resolver
From: David Miller @ 2017-12-12  2:16 UTC (permalink / raw)
  To: tom; +Cc: netdev, roopa, rohit
In-Reply-To: <CAPDqMer_F=uFZ08LytAbU7hwT4d98iYFGfHHFKKtL1mB4a+6qw@mail.gmail.com>

From: Tom Herbert <tom@quantonium.net>
Date: Mon, 11 Dec 2017 14:16:17 -0800

> How can we build a system that allows an unlimited number of
> resolutions without drop?

IPV4 routing solves this with a prefixed trie, for example.

The fundamental backing datastructure for the switching
or whatever operation must be in-memory, in the kernel,
scalable, and without a fronting "cache".

^ permalink raw reply

* Re: [PATCH net-next v4 0/2] bpf/tracing: allow user space to query prog array on the same tp
From: Daniel Borkmann @ 2017-12-12  2:17 UTC (permalink / raw)
  To: Yonghong Song, peterz, ast, netdev; +Cc: kernel-team
In-Reply-To: <20171211193903.2428317-1-yhs@fb.com>

On 12/11/2017 08:39 PM, Yonghong Song wrote:
> Commit e87c6bc3852b ("bpf: permit multiple bpf attachments
> for a single perf event") added support to attach multiple
> bpf programs to a single perf event. Given a perf event
> (kprobe, uprobe, or kernel tracepoint), the perf ioctl interface
> is used to query bpf programs attached to the same trace event.
> 
> There already exists a BPF_PROG_QUERY command for introspection
> currently used by cgroup+bpf. We did have an implementation for
> querying tracepoint+bpf through the same interface. However, it
> looks cleaner to use ioctl() style of api here, since attaching
> bpf prog to tracepoint/kuprobe is also done via ioctl.
> 
> Patch #1 had the core implementation and patch #2 added
> a test case in tools bpf selftests suite.
> 
> Changelogs:
> v3 -> v4:
>   - Fix a compilation error with newer gcc like 6.3.1 while
>     old gcc 4.8.5 is okay. I was using &uquery->ids to represent
>     the address to the ids array to make it explicit that the
>     address is passed, and this syntax is rightly rejected
>     by gcc 6.3.1.

Series applied to bpf-next, thanks Yonghong.

^ permalink raw reply

* Re: [PATCH net-next] libbpf: add function to setup XDP
From: Daniel Borkmann @ 2017-12-12  2:23 UTC (permalink / raw)
  To: David Ahern, Eric Leblond, Jakub Kicinski; +Cc: netdev, linux-kernel, ast
In-Reply-To: <49b37939-ebd4-fe92-eeab-e5d2713cadb8@gmail.com>

On 12/10/2017 10:07 PM, David Ahern wrote:
> On 12/10/17 1:34 PM, Eric Leblond wrote:
>>> Would it be possible to print out or preferably return to the caller
>>> the ext ack error message?  A couple of drivers are using it for XDP
>>> mis-configuration reporting instead of printks.  We should encourage
>>> other to do the same and support it in all user space since ext ack 
>>> msgs lead to much better user experience.
>>
>> I've seen the kind of messages displayed by reading at kernel log. They
>> are really useful and it looks almost mandatory to be able to display
>> them.
>>
>> Kernel code seems to not have a parser for the ext ack error message.
>> Did I miss something here ?
>>  
>> Looking at tc code, it seems it is using libmnl to parse them and I
>> doubt it is a good idea to use that in libbpf as it is introducing a
>> dependency.
>>
>> Does someone has an existing parsing code or should I write on my own ?
> 
> I had worked on extack for libbpf but seem to have lost the changes.
> 
> Look at the commits here:
>     https://github.com/dsahern/iproute2/commits/ext-ack
> 
> I suggest using this:
> 
> https://github.com/dsahern/iproute2/commit/b61e4c7dd54a5d3ff98640da4b480441cee497b2
> 
> to bring in nlattr from lib/nlattr (as I recall lib/nlattr can not be
> used directly). From there, use this one:
> 
> https://github.com/dsahern/iproute2/commit/261f7251e6704d565b91e310faabbbb7e18d14a1
> 
> to see what is needed for extack support.
> 
> Really not that much code to add.

+1, ext ack support would improve troubleshooting a lot here; please
add and respin. Thanks, Eric!

^ permalink raw reply

* Re: [PATCH net-next v5 2/2] net: ethernet: socionext: add AVE ethernet driver
From: Masami Hiramatsu @ 2017-12-12  2:29 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Kunihiko Hayashi, Mark Rutland, Andrew Lunn, Florian Fainelli,
	devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	Linux kernel mailing list, Masahiro Yamada, Rob Herring,
	Philippe Ombredanne, Jassi Brar, David S. Miller,
	linux-arm-kernel
In-Reply-To: <20171211134627.GU10595-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org>

Hi Russell,

2017-12-11 22:46 GMT+09:00 Russell King - ARM Linux <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>:
> On Mon, Dec 11, 2017 at 10:34:17PM +0900, Masami Hiramatsu wrote:
>> IMHO, even if we use SPDX license identifier, I recommend to use
>> C-style comments as many other files do, since it is C code.
>> If SPDX identifier requires C++ style, that is SPDX parser's issue
>> and should be fixed to get it from C-style comment.
>
> See the numerous emails on this subject already.  The issue of C
> vs C++ comments has come up many times by many different people, but
> the result is the same.  That's not going to happen.  Linux kernel
> C files are required to use "//" for the SPDX identifier by order
> of Linus Torvalds.

OK, I got it.

>
> Linus has also revealed in that discussion that he has a preference
> for "//" style commenting for single comments, so it seems that the
> kernel coding style may change - but there is no desire for patches
> to "clean up" single line comments to use "//".

Thank you for making it clear.

Then what I'm considering is copyright notice lines. Those are usually
treat as the header lines, not single line. So

> +// SDPX-License-Identifier: GPL-2.0
> +// sni_ave.c - Socionext UniPhier AVE ethernet driver
> +// Copyright 2014 Panasonic Corporation
> +// Copyright 2015-2017 Socionext Inc.

is acceptable? or should we keep C-style header lines for new drivers?

> +// SDPX-License-Identifier: GPL-2.0
> +/*
> + * sni_ave.c - Socionext UniPhier AVE ethernet driver
> + * Copyright 2014 Panasonic Corporation
> + * Copyright 2015-2017 Socionext Inc.
> + */

I just concern that those lines are not "single". that's all. :)

>
> For further information, and to see the discussion that has already
> happened, the arguments that have been made about style, see the
> threads for the patch series that tglx has been posting wrt documenting
> the SPDX stuff for the kernel.

OK, got it.

https://lkml.org/lkml/2017/11/16/663


Thanks,

>
> Thanks (let's stop rehashing the same arguments.)
>


-- 
Masami Hiramatsu
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Setting large MTU size on slave interfaces may stall the whole system
From: Qing Huang @ 2017-12-12  3:21 UTC (permalink / raw)
  To: netdev, linux-kernel; +Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek
In-Reply-To: <9f95c2a0-e4fe-270d-790a-beeb6b3e7690@oracle.com>

(resend this email in text format)


Hi,

We found an issue with the bonding driver when testing Mellanox devices.
The following test commands will stall the whole system sometimes, with 
serial console
flooded with log messages from the bond_miimon_inspect() function. 
Setting mtu size
to be 1500 seems okay but very rarely it may hit the same problem too.

ip address flush dev ens3f0
ip link set dev ens3f0 down
ip address flush dev ens3f1
ip link set dev ens3f1 down
[root@ca-hcl629 etc]# modprobe bonding mode=0 miimon=250 use_carrier=1
updelay=500 downdelay=500
[root@ca-hcl629 etc]# ifconfig bond0 up
[root@ca-hcl629 etc]# ifenslave bond0 ens3f0 ens3f1
[root@ca-hcl629 etc]# ip link set bond0 mtu 4500 up


Seiral console output:

** 4 printk messages dropped ** [ 3717.743761] bond0: link status down for
interface ens3f0, disabling it in 500 ms

** 5 printk messages dropped ** [ 3717.755737] bond0: link status down for
interface ens3f0, disabling it in 500 ms

** 5 printk messages dropped ** [ 3717.767758] bond0: link status down for
interface ens3f0, disabling it in 500 ms

** 4 printk messages dropped ** [ 3717.777737] bond0: link status down for
interface ens3f0, disabling it in 500 ms

or

** 4 printk messages dropped ** [274743.297863] bond0: link status down 
again
after 500 ms for interface enp48s0f1
** 4 printk messages dropped ** [274743.307866] bond0: link status down 
again
after 500 ms for interface enp48s0f1
** 4 printk messages dropped ** [274743.317857] bond0: link status down 
again
after 500 ms for interface enp48s0f1
** 4 printk messages dropped ** [274743.327823] bond0: link status down 
again
after 500 ms for interface enp48s0f1
** 4 printk messages dropped ** [274743.337817] bond0: link status down 
again
after 500 ms for interface enp48s0f1


The root cause is the combined affect from commit 
1f2cd845d3827412e82bf26dde0abca332ede402(Revert
"Merge branch 'bonding_monitor_locking'") and commit 
de77ecd4ef02ca783f7762e04e92b3d0964be66b
("bonding: improve link-status update in mii-monitoring"). E.g. 
reverting the second commit, we don't
see the problem.

It seems that when setting a large mtu size on an RoCE interface, the 
RTNL mutex may be held too long by the slave
interface, causing bond_mii_monitor() to be called repeatedly at an 
interval of 1 tick (1K HZ kernel configuration)
and kernel to become unresponsive.


We found two possible solutions:

#1, don't re-arm the mii monitor thread too quick if we cannot get RTNL 
lock:
index b2db581..8fd587a 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2266,7 +2266,6 @@ static void bond_mii_monitor(struct work_struct 
*work)

                 /* Race avoidance with bond_close cancel of workqueue */
                 if (!rtnl_trylock()) {
-                       delay = 1;
                         should_notify_peers = false;
                         goto re_arm;
                 }

#2, we use printk_ratelimit() to avoid flooding log messages generated 
by bond_miimon_inspect().

index b2db581..0183b7f 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2054,7 +2054,7 @@ static int bond_miimon_inspect(struct bonding *bond)
                         bond_propose_link_state(slave, BOND_LINK_FAIL);
                         commit++;
                         slave->delay = bond->params.downdelay;
-                       if (slave->delay) {
+                       if (slave->delay && printk_ratelimit()) {
                                 netdev_info(bond->dev, "link status 
down for
%sinterface %s, disabling it in %d ms\n",
                                             (BOND_MODE(bond) ==
BOND_MODE_ACTIVEBACKUP) ?
@@ -2105,7 +2105,8 @@ static int bond_miimon_inspect(struct bonding *bond)
                 case BOND_LINK_BACK:
                         if (!link_state) {
                                 bond_propose_link_state(slave,
BOND_LINK_DOWN);
-                               netdev_info(bond->dev, "link status down
again after %d ms for interface %s\n",
+                               if(printk_ratelimit())
+                                       netdev_info(bond->dev, "link status
down again after %d ms for interface %s\n",
(bond->params.updelay -
slave->delay) *
bond->params.miimon,
slave->dev->name);


Regarding the flooding messages, the netdev_info output is misleading 
anyway
when bond_mii_monitor() is called at 1 tick interval due to lock 
contention.


Solution #1 looks simpler and cleaner to me. Any side affect of doing that?


Thanks,
Qing

^ permalink raw reply related

* [BUG] 3com/3c59x: two possible sleep-in-atomic bugs
From: Jia-Ju Bai @ 2017-12-12  3:34 UTC (permalink / raw)
  To: klassert, vortex, becker, David Miller; +Cc: netdev, Linux Kernel Mailing List

According to drivers/net/ethernet/3com/3c59x.c, the kernel module may 
sleep in the interrupt handler.
The function call paths are:
boomerang_interrupt (interrupt handler)
   vortex_error
     vortex_up
       pci_set_power_state --> may sleep
       pci_enable_device --> may sleep

vortex_interrupt (interrupt handler)
   vortex_error
     vortex_up
       pci_set_power_state --> may sleep
       pci_enable_device --> may sleep

I do not find a good way to fix them, so I only report.
These possible bugs are found by my static analysis tool (DSAC) and 
checked by my code review.


Thanks,
Jia-Ju Bai

^ permalink raw reply

* Re: [RFC][PATCH] new byteorder primitives - ..._{replace,get}_bits()
From: Jakub Kicinski @ 2017-12-12  4:02 UTC (permalink / raw)
  To: Al Viro; +Cc: Linus Torvalds, netdev, linux-kernel
In-Reply-To: <20171211155422.GA12326@ZenIV.linux.org.uk>

On Mon, 11 Dec 2017 15:54:22 +0000, Al Viro wrote:
> Essentially, it gives helpers for work with bitfields in fixed-endian.
> Suppose we have e.g. a little-endian 32bit value with fixed layout;
> expressing that as a bitfield would go like
> 	struct foo {
> 		unsigned foo:4;		/* bits 0..3 */
> 		unsigned :2;
> 		unsigned bar:12;	/* bits 6..17 */
> 		unsigned baz:14;	/* bits 18..31 */
> 	}
> Even for host-endian it doesn't work all that well - you end up with
> ifdefs in structure definition and generated code stinks.  For fixed-endian
> it gets really painful, and people tend to use explicit shift-and-mask
> kind of macros for accessing the fields (and often enough get the
> endianness conversions wrong, at that).  With these primitives
> 
> struct foo v		<=>	__le32 v
> v.foo = i ? 1 : 2	<=>	v = le32_replace_bits(v, i ? 1 : 2, 0, 4)
> f(4 + v.baz)		<=>	f(4 + le32_get_bits(v, 18, 14))

Looks very useful.  The [start bit, size] pair may not land itself
too nicely to creating defines, though.  Which is why in
include/linux/bitfield.h we tried to use a shifted mask and work
backwards from that single value what the start and size are.  commit
3e9b3112ec74 ("add basic register-field manipulation macros") has the
description.  Could a similar trick perhaps be applicable here?

^ permalink raw reply

* Re: [PATCH net] tcp md5sig: Use skb's saddr when replying to an incoming segment
From: Eric Dumazet @ 2017-12-12  4:51 UTC (permalink / raw)
  To: Christoph Paasch, David Miller; +Cc: netdev
In-Reply-To: <20171211080546.89418-1-cpaasch@apple.com>

On Mon, 2017-12-11 at 00:05 -0800, Christoph Paasch wrote:
> The MD5-key that belongs to a connection is identified by the peer's
> IP-address. When we are in tcp_v4(6)_reqsk_send_ack(), we are
> replying
> to an incoming segment from tcp_check_req() that failed the seq-
> number
> checks.
> 
> Thus, to find the correct key, we need to use the skb's saddr and not
> the daddr.
> 
> This bug seems to have been there since quite a while, but probably
> got
> unnoticed because the consequences are not catastrophic. We will call
> tcp_v4_reqsk_send_ack only to send a challenge-ACK back to the peer,
> thus the connection doesn't really fail.
> 
> Fixes: 9501f9722922 ("tcp md5sig: Let the caller pass appropriate key
> for tcp_v{4,6}_do_calc_md5_hash().")
> Signed-off-by: Christoph Paasch <cpaasch@apple.com>
> ---
>  net/ipv4/tcp_ipv4.c | 2 +-
>  net/ipv6/tcp_ipv6.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Eric Dumazet <edumazet@google.com>

Thanks !

^ permalink raw reply

* [PATCH net-next] tcp/dccp: avoid one atomic operation for timewait hashdance
From: Eric Dumazet @ 2017-12-12  5:25 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

First, rename __inet_twsk_hashdance() to inet_twsk_hashdance()

Then, remove one inet_twsk_put() by setting tw_refcnt to 3 instead
of 4, but adding a fat warning that we do not have the right to access
tw anymore after inet_twsk_hashdance()

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/inet_timewait_sock.h |    4 ++--
 net/dccp/minisocks.c             |    7 ++++---
 net/ipv4/inet_timewait_sock.c    |   27 +++++++++++++--------------
 net/ipv4/tcp_minisocks.c         |    7 ++++---
 4 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/include/net/inet_timewait_sock.h b/include/net/inet_timewait_sock.h
index 1356fa6a7566bf8b53632215ef8de4b153848f9b..899495589a7ea2bf693cdda42f83cec160e861b5 100644
--- a/include/net/inet_timewait_sock.h
+++ b/include/net/inet_timewait_sock.h
@@ -93,8 +93,8 @@ struct inet_timewait_sock *inet_twsk_alloc(const struct sock *sk,
 					   struct inet_timewait_death_row *dr,
 					   const int state);
 
-void __inet_twsk_hashdance(struct inet_timewait_sock *tw, struct sock *sk,
-			   struct inet_hashinfo *hashinfo);
+void inet_twsk_hashdance(struct inet_timewait_sock *tw, struct sock *sk,
+			 struct inet_hashinfo *hashinfo);
 
 void __inet_twsk_schedule(struct inet_timewait_sock *tw, int timeo,
 			  bool rearm);
diff --git a/net/dccp/minisocks.c b/net/dccp/minisocks.c
index 178bb9833311f83205317b07fe64cb2e45a9f734..37ccbe62eb1af3f9dffbf63323c008cc96cd8ea1 100644
--- a/net/dccp/minisocks.c
+++ b/net/dccp/minisocks.c
@@ -63,9 +63,10 @@ void dccp_time_wait(struct sock *sk, int state, int timeo)
 		 */
 		local_bh_disable();
 		inet_twsk_schedule(tw, timeo);
-		/* Linkage updates. */
-		__inet_twsk_hashdance(tw, sk, &dccp_hashinfo);
-		inet_twsk_put(tw);
+		/* Linkage updates.
+		 * Note that access to tw after this point is illegal.
+		 */
+		inet_twsk_hashdance(tw, sk, &dccp_hashinfo);
 		local_bh_enable();
 	} else {
 		/* Sorry, if we're out of memory, just CLOSE this
diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
index b563e0c46bac2362acccf38495546a8b6b726384..277ff69a312dca1d0bc04be4b0b36db133aaf63b 100644
--- a/net/ipv4/inet_timewait_sock.c
+++ b/net/ipv4/inet_timewait_sock.c
@@ -97,7 +97,7 @@ static void inet_twsk_add_bind_node(struct inet_timewait_sock *tw,
  * Essentially we whip up a timewait bucket, copy the relevant info into it
  * from the SK, and mess with hash chains and list linkage.
  */
-void __inet_twsk_hashdance(struct inet_timewait_sock *tw, struct sock *sk,
+void inet_twsk_hashdance(struct inet_timewait_sock *tw, struct sock *sk,
 			   struct inet_hashinfo *hashinfo)
 {
 	const struct inet_sock *inet = inet_sk(sk);
@@ -119,18 +119,6 @@ void __inet_twsk_hashdance(struct inet_timewait_sock *tw, struct sock *sk,
 
 	spin_lock(lock);
 
-	/*
-	 * Step 2: Hash TW into tcp ehash chain.
-	 * Notes :
-	 * - tw_refcnt is set to 4 because :
-	 * - We have one reference from bhash chain.
-	 * - We have one reference from ehash chain.
-	 * - We have one reference from timer.
-	 * - One reference for ourself (our caller will release it).
-	 * We can use atomic_set() because prior spin_lock()/spin_unlock()
-	 * committed into memory all tw fields.
-	 */
-	refcount_set(&tw->tw_refcnt, 4);
 	inet_twsk_add_node_rcu(tw, &ehead->chain);
 
 	/* Step 3: Remove SK from hash chain */
@@ -138,8 +126,19 @@ void __inet_twsk_hashdance(struct inet_timewait_sock *tw, struct sock *sk,
 		sock_prot_inuse_add(sock_net(sk), sk->sk_prot, -1);
 
 	spin_unlock(lock);
+
+	/* tw_refcnt is set to 3 because we have :
+	 * - one reference for bhash chain.
+	 * - one reference for ehash chain.
+	 * - one reference for timer.
+	 * We can use atomic_set() because prior spin_lock()/spin_unlock()
+	 * committed into memory all tw fields.
+	 * Also note that after this point, we lost our implicit reference
+	 * so we are not allowed to use tw anymore.
+	 */
+	refcount_set(&tw->tw_refcnt, 3);
 }
-EXPORT_SYMBOL_GPL(__inet_twsk_hashdance);
+EXPORT_SYMBOL_GPL(inet_twsk_hashdance);
 
 static void tw_timer_handler(struct timer_list *t)
 {
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index b079b619b60ca577d5ef20a5065fce87acecd96c..a8384b0c11f8fa589e2ed5311899b62c80a269f8 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -316,9 +316,10 @@ void tcp_time_wait(struct sock *sk, int state, int timeo)
 		 */
 		local_bh_disable();
 		inet_twsk_schedule(tw, timeo);
-		/* Linkage updates. */
-		__inet_twsk_hashdance(tw, sk, &tcp_hashinfo);
-		inet_twsk_put(tw);
+		/* Linkage updates.
+		 * Note that access to tw after this point is illegal.
+		 */
+		inet_twsk_hashdance(tw, sk, &tcp_hashinfo);
 		local_bh_enable();
 	} else {
 		/* Sorry, if we're out of memory, just CLOSE this

^ permalink raw reply related

* [PATCH v5 0/3] Add andestech atcpit100 timer
From: Rick Chen @ 2017-12-12  5:46 UTC (permalink / raw)
  To: rickchen36-Re5JQEeQqe8AvxtiuMwx3w, rick-MUIXKm3Oiri1Z/+hSey0Gg,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4,
	linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
	daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
	linux-arch-u79uwXL29TY76Z2rM5mHXA, tglx-hfZtesqFncYOwBW4kG4KsQ,
	jason-NLaQJdtUoK4Be96aLqz0jA, marc.zyngier-5wv7dgnIgG8,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
	deanbo422-Re5JQEeQqe8AvxtiuMwx3w,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn,
	dhowells-H+wXaHxf7aLQT0dZR+AlfA, will.deacon-5wv7dgnIgG8,
	linux-serial-u79uwXL29TY76Z2rM5mHXA

Changelog v5:
 - Patch 1/3: Changes
 - Patch 2/3: New
 - Patch 3/3: Changes

[Patch 1/3] clocksource/drivers/atcpit100: Add andestech atcpit100 timer
 1 No need to split out the Makefile patch from the actual driver.
 	 Suggested by Arnd Bergmann
 2 Add of_clk.name = "PCLK" to be explicit on what we use.
   Suggested by Linus Walleij
 3 Remove the GENERIC_CLOCKEVENTS from Kconfig.
   Suggested by Daniel Lezcano
 4 Add depends on NDS32 || COMPILE_TEST in Kconfig
   Suggested by Greentime Hu

[Patch 2/3] clocksource/drivers/atcpit100: VDSO support
	 Why implemented in timer driver, please see details from
	 https://lkml.org/lkml/2017/12/8/362
	 [PATCH v3 17/33] nds32: VDSO support.
	 Suggested by Mark Rutland
	 Here Mark Rutlan suggested as below:
	 You should not add properties to arbitrary DT bindings to
	 handle a Linux implementation detail.
	 Please remove this DT code, and have the drivers for those
	 timer blocks export this information to your vdso code somehow.

[Patch 3/3] dt-bindings: timer: Add andestech atcpit100 timer binding doc
   Fix incorrect description about PCLK.
	 Suggested by Linus Walleij

Rick Chen (3):
  clocksource/drivers/atcpit100: Add andestech atcpit100 timer
  clocksource/drivers/atcpit100: VDSO support
  dt-bindings: timer: Add andestech atcpit100 timer binding doc

 .../bindings/timer/andestech,atcpit100-timer.txt   |  33 +++
 drivers/clocksource/Kconfig                        |   7 +
 drivers/clocksource/Makefile                       |   1 +
 drivers/clocksource/timer-atcpit100.c              | 270 +++++++++++++++++++++
 4 files changed, 311 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/andestech,atcpit100-timer.txt
 create mode 100644 drivers/clocksource/timer-atcpit100.c

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v5 1/3] clocksource/drivers/atcpit100: Add andestech atcpit100 timer
From: Rick Chen @ 2017-12-12  5:46 UTC (permalink / raw)
  To: rickchen36, rick, linux-kernel, arnd, linus.walleij,
	daniel.lezcano, linux-arch, tglx, jason, marc.zyngier, robh+dt,
	netdev, deanbo422, devicetree, viro, dhowells, will.deacon,
	linux-serial
  Cc: Greentime Hu
In-Reply-To: <1513057621-19084-1-git-send-email-rickchen36@gmail.com>

ATCPIT100 is often used on the Andes architecture,
This timer provide 4 PIT channels. Each PIT channel is a
multi-function timer, can be configured as 32,16,8 bit timers
or PWM as well.

For system timer it will set channel 1 32-bit timer0 as clock
source and count downwards until underflow and restart again.

It also set channel 0 32-bit timer0 as clock event and count
downwards until condition match. It will generate an interrupt
for handling periodically.

Signed-off-by: Rick Chen <rickchen36@gmail.com>
Signed-off-by: Greentime Hu <green.hu@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/clocksource/Kconfig           |   7 +
 drivers/clocksource/Makefile          |   1 +
 drivers/clocksource/timer-atcpit100.c | 255 ++++++++++++++++++++++++++++++++++
 3 files changed, 263 insertions(+)
 create mode 100644 drivers/clocksource/timer-atcpit100.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index cc60620..8c57ef2 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -615,4 +615,11 @@ config CLKSRC_ST_LPC
 	  Enable this option to use the Low Power controller timer
 	  as clocksource.
 
+config CLKSRC_ATCPIT100
+	bool "Clocksource for AE3XX platform"
+	depends on NDS32 || COMPILE_TEST
+	depends on HAS_IOMEM
+	help
+	  This option enables support for the Andestech AE3XX platform timers.
+
 endmenu
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 72711f1..7d072f5 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -75,3 +75,4 @@ obj-$(CONFIG_H8300_TMR16)		+= h8300_timer16.o
 obj-$(CONFIG_H8300_TPU)			+= h8300_tpu.o
 obj-$(CONFIG_CLKSRC_ST_LPC)		+= clksrc_st_lpc.o
 obj-$(CONFIG_X86_NUMACHIP)		+= numachip.o
+obj-$(CONFIG_CLKSRC_ATCPIT100)		+= timer-atcpit100.o
diff --git a/drivers/clocksource/timer-atcpit100.c b/drivers/clocksource/timer-atcpit100.c
new file mode 100644
index 0000000..0077fdb
--- /dev/null
+++ b/drivers/clocksource/timer-atcpit100.c
@@ -0,0 +1,255 @@
+/*
+ *  Andestech ATCPIT100 Timer Device Driver Implementation
+ *
+ * Copyright (C) 2017 Andes Technology Corporation
+ * Rick Chen, Andes Technology Corporation <rick@andestech.com>
+ *
+ * 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.
+ *
+ *
+ */
+
+#include <linux/irq.h>
+#include <linux/clocksource.h>
+#include <linux/clockchips.h>
+#include <linux/interrupt.h>
+#include <linux/ioport.h>
+#include <linux/cpufreq.h>
+#include <linux/sched.h>
+#include <linux/sched_clock.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include "timer-of.h"
+
+/*
+ * Definition of register offsets
+ */
+
+/* ID and Revision Register */
+#define ID_REV		0x0
+
+/* Configuration Register */
+#define CFG		0x10
+
+/* Interrupt Enable Register */
+#define INT_EN		0x14
+#define CH_INT_EN(c, i)	((1<<i)<<(4*c))
+#define CH0INT0EN	0x01
+
+/* Interrupt Status Register */
+#define INT_STA		0x18
+#define CH0INT0		0x01
+
+/* Channel Enable Register */
+#define CH_EN		0x1C
+#define CH0TMR0EN	0x1
+#define CH1TMR0EN	0x10
+
+/* Channel 0 , 1 Control Register */
+#define CH0_CTL		(0x20)
+#define CH1_CTL		(0x20 + 0x10)
+
+/* Channel clock source , bit 3 , 0:External clock , 1:APB clock */
+#define APB_CLK		BIT(3)
+
+/* Channel mode , bit 0~2 */
+#define TMR_32		0x1
+#define TMR_16		0x2
+#define TMR_8		0x3
+
+/* Channel 0 , 1 Reload Register */
+#define CH0_REL		(0x24)
+#define CH1_REL		(0x24 + 0x10)
+
+/* Channel 0 , 1 Counter Register */
+#define CH0_CNT		(0x28)
+#define CH1_CNT		(0x28 + 0x10)
+
+#define TIMER_SYNC_TICKS	3
+
+static void atcpit100_ch1_tmr0_en(void __iomem *base)
+{
+	writel(~0, base + CH1_REL);
+	writel(APB_CLK|TMR_32, base + CH1_CTL);
+}
+
+static void atcpit100_ch0_tmr0_en(void __iomem *base)
+{
+	writel(APB_CLK|TMR_32, base + CH0_CTL);
+}
+
+static void atcpit100_clkevt_time_setup(void __iomem *base, unsigned long delay)
+{
+	writel(delay, base + CH0_CNT);
+	writel(delay, base + CH0_REL);
+}
+
+static void atcpit100_timer_clear_interrupt(void __iomem *base)
+{
+	u32 val;
+
+	val = readl(base + INT_STA);
+	writel(val | CH0INT0, base + INT_STA);
+}
+
+static void atcpit100_clocksource_start(void __iomem *base)
+{
+	u32 val;
+
+	val = readl(base + CH_EN);
+	writel(val | CH1TMR0EN, base + CH_EN);
+}
+
+static void atcpit100_clkevt_time_start(void __iomem *base)
+{
+	u32 val;
+
+	val = readl(base + CH_EN);
+	writel(val | CH0TMR0EN, base + CH_EN);
+}
+
+static void atcpit100_clkevt_time_stop(void __iomem *base)
+{
+	u32 val;
+
+	atcpit100_timer_clear_interrupt(base);
+	val = readl(base + CH_EN);
+	writel(val & ~CH0TMR0EN, base + CH_EN);
+}
+
+static int atcpit100_clkevt_next_event(unsigned long evt,
+	struct clock_event_device *clkevt)
+{
+	struct timer_of *to = to_timer_of(clkevt);
+
+	writel(evt, timer_of_base(to) + CH0_REL);
+
+	return 0;
+}
+
+static int atcpit100_clkevt_set_periodic(struct clock_event_device *evt)
+{
+	struct timer_of *to = to_timer_of(evt);
+
+	atcpit100_clkevt_time_setup(timer_of_base(to), timer_of_period(to));
+	atcpit100_clkevt_time_start(timer_of_base(to));
+
+	return 0;
+}
+static int atcpit100_clkevt_shutdown(struct clock_event_device *evt)
+{
+	struct timer_of *to = to_timer_of(evt);
+
+	atcpit100_clkevt_time_stop(timer_of_base(to));
+
+	return 0;
+}
+static int atcpit100_clkevt_set_oneshot(struct clock_event_device *evt)
+{
+	struct timer_of *to = to_timer_of(evt);
+	u32 val;
+
+	writel(~0x0, timer_of_base(to) + CH0_REL);
+	val = readl(timer_of_base(to) + CH_EN);
+	writel(val | CH0TMR0EN, timer_of_base(to) + CH_EN);
+
+	return 0;
+}
+
+static irqreturn_t atcpit100_timer_interrupt(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = (struct clock_event_device *)dev_id;
+	struct timer_of *to = to_timer_of(evt);
+
+	atcpit100_timer_clear_interrupt(timer_of_base(to));
+
+	evt->event_handler(evt);
+
+	return IRQ_HANDLED;
+}
+
+static struct timer_of to = {
+	.flags = TIMER_OF_IRQ | TIMER_OF_CLOCK | TIMER_OF_BASE,
+
+	.clkevt = {
+		.name = "atcpit100_tick",
+		.rating = 300,
+		.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
+		.set_state_shutdown = atcpit100_clkevt_shutdown,
+		.set_state_periodic = atcpit100_clkevt_set_periodic,
+		.set_state_oneshot = atcpit100_clkevt_set_oneshot,
+		.tick_resume = atcpit100_clkevt_shutdown,
+		.set_next_event = atcpit100_clkevt_next_event,
+		.cpumask = cpu_all_mask,
+	},
+
+	.of_irq = {
+		.handler = atcpit100_timer_interrupt,
+		.flags = IRQF_TIMER | IRQF_IRQPOLL,
+	},
+
+	/*
+	 * FIXME: we currently only support clocking using PCLK
+	 * and using EXTCLK is not supported in the driver.
+	 */
+	.of_clk = {
+		.name = "PCLK",
+	}
+};
+
+static u64 notrace atcpit100_timer_sched_read(void)
+{
+	return ~readl(timer_of_base(&to) + CH1_CNT);
+}
+
+static int __init atcpit100_timer_init(struct device_node *node)
+{
+	int ret;
+	u32 val;
+	void __iomem *base;
+
+	ret = timer_of_init(node, &to);
+	if (ret)
+		return ret;
+
+	base = timer_of_base(&to);
+
+	sched_clock_register(atcpit100_timer_sched_read, 32,
+		timer_of_rate(&to));
+
+	ret = clocksource_mmio_init(base + CH1_CNT,
+		node->name, timer_of_rate(&to), 300, 32,
+		clocksource_mmio_readl_down);
+
+	if (ret) {
+		pr_err("Failed to register clocksource\n");
+		return ret;
+	}
+
+	/* clear channel 0 timer0 interrupt */
+	atcpit100_timer_clear_interrupt(base);
+
+	clockevents_config_and_register(&to.clkevt, timer_of_rate(&to),
+					TIMER_SYNC_TICKS, 0xffffffff);
+	atcpit100_ch0_tmr0_en(base);
+	atcpit100_ch1_tmr0_en(base);
+	atcpit100_clocksource_start(base);
+	atcpit100_clkevt_time_start(base);
+
+	/* Enable channel 0 timer0 interrupt */
+	val = readl(base + INT_EN);
+	writel(val | CH0INT0EN, base + INT_EN);
+
+	return ret;
+}
+
+TIMER_OF_DECLARE(atcpit100, "andestech,atcpit100", atcpit100_timer_init);
-- 
2.7.4

^ permalink raw reply related

* [PATCH v5 2/3] clocksource/drivers/atcpit100: VDSO support
From: Rick Chen @ 2017-12-12  5:47 UTC (permalink / raw)
  To: rickchen36, rick, linux-kernel, arnd, linus.walleij,
	daniel.lezcano, linux-arch, tglx, jason, marc.zyngier, robh+dt,
	netdev, deanbo422, devicetree, viro, dhowells, will.deacon,
	linux-serial
  Cc: Vincent Chen, Greentime Hu
In-Reply-To: <1513057621-19084-1-git-send-email-rickchen36@gmail.com>

VDSO needs real-time cycle count to ensure the time accuracy.
Unlike others, nds32 architecture does not define clock source,
hence VDSO needs atcpit100 offering real-time cycle count
to derive the correct time.

Signed-off-by: Vincent Chen <vincentc@andestech.com>
Signed-off-by: Rick Chen <rickchen36@gmail.com>
Signed-off-by: Greentime Hu <green.hu@gmail.com>
---
 drivers/clocksource/timer-atcpit100.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/clocksource/timer-atcpit100.c b/drivers/clocksource/timer-atcpit100.c
index 0077fdb..1be6c0a 100644
--- a/drivers/clocksource/timer-atcpit100.c
+++ b/drivers/clocksource/timer-atcpit100.c
@@ -29,6 +29,9 @@
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include "timer-of.h"
+#ifdef CONFIG_NDS32
+#include <asm/vdso_timer_info.h>
+#endif
 
 /*
  * Definition of register offsets
@@ -211,6 +214,14 @@ static u64 notrace atcpit100_timer_sched_read(void)
 	return ~readl(timer_of_base(&to) + CH1_CNT);
 }
 
+#ifdef CONFIG_NDS32
+static void fill_vdso_need_info(void)
+{
+	timer_info.cycle_count_down = true;
+	timer_info.cycle_count_reg_offset = CH1_CNT;
+}
+#endif
+
 static int __init atcpit100_timer_init(struct device_node *node)
 {
 	int ret;
@@ -249,6 +260,10 @@ static int __init atcpit100_timer_init(struct device_node *node)
 	val = readl(base + INT_EN);
 	writel(val | CH0INT0EN, base + INT_EN);
 
+#ifdef CONFIG_NDS32
+	fill_vdso_need_info();
+#endif
+
 	return ret;
 }
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH v5 3/3] dt-bindings: timer: Add andestech atcpit100 timer binding doc
From: Rick Chen @ 2017-12-12  5:47 UTC (permalink / raw)
  To: rickchen36, rick, linux-kernel, arnd, linus.walleij,
	daniel.lezcano, linux-arch, tglx, jason, marc.zyngier, robh+dt,
	netdev, deanbo422, devicetree, viro, dhowells, will.deacon,
	linux-serial
  Cc: Greentime Hu
In-Reply-To: <1513057621-19084-1-git-send-email-rickchen36@gmail.com>

Add a document to describe Andestech atcpit100 timer and
binding information.

Signed-off-by: Rick Chen <rickchen36@gmail.com>
Signed-off-by: Greentime Hu <green.hu@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../bindings/timer/andestech,atcpit100-timer.txt   | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/andestech,atcpit100-timer.txt

diff --git a/Documentation/devicetree/bindings/timer/andestech,atcpit100-timer.txt b/Documentation/devicetree/bindings/timer/andestech,atcpit100-timer.txt
new file mode 100644
index 0000000..14812f68
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/andestech,atcpit100-timer.txt
@@ -0,0 +1,33 @@
+Andestech ATCPIT100 timer
+------------------------------------------------------------------
+ATCPIT100 is a generic IP block from Andes Technology, embedded in
+Andestech AE3XX platforms and other designs.
+
+This timer is a set of compact multi-function timers, which can be
+used as pulse width modulators (PWM) as well as simple timers.
+
+It supports up to 4 PIT channels. Each PIT channel is a
+multi-function timer and provide the following usage scenarios:
+One 32-bit timer
+Two 16-bit timers
+Four 8-bit timers
+One 16-bit PWM
+One 16-bit timer and one 8-bit PWM
+Two 8-bit timer and one 8-bit PWM
+
+Required properties:
+- compatible	: Should be "andestech,atcpit100"
+- reg		: Address and length of the register set
+- interrupts	: Reference to the timer interrupt
+- clocks : a clock to provide the tick rate for "andestech,atcpit100"
+- clock-names : should be "PCLK" for the peripheral clock source.
+
+Examples:
+
+timer0: timer@f0400000 {
+	compatible = "andestech,atcpit100";
+	reg = <0xf0400000 0x1000>;
+	interrupts = <2 4>;
+	clocks = <&apb>;
+	clock-names = "PCLK";
+};
-- 
2.7.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox