netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH iproute2-next 0/3] add interface to TC MPLS actions
@ 2019-07-09 15:59 John Hurley
  2019-07-09 15:59 ` [PATCH iproute2-next 1/3] lib: add mpls_uc and mpls_mc as link layer protocol names John Hurley
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: John Hurley @ 2019-07-09 15:59 UTC (permalink / raw)
  To: netdev
  Cc: davem, jiri, xiyou.wangcong, dsahern, willemdebruijn.kernel,
	simon.horman, jakub.kicinski, oss-drivers, John Hurley

Recent kernel additions to TC allows the manipulation of MPLS headers as
filter actions.

The following patchset creates an iproute2 interface to the new actions
and includes documentation on how to use it.

John Hurley (3):
  lib: add mpls_uc and mpls_mc as link layer protocol names
  tc: add mpls actions
  man: update man pages for TC MPLS actions

 lib/ll_proto.c     |   2 +
 man/man8/tc-mpls.8 | 156 ++++++++++++++++++++++++++++++
 tc/Makefile        |   1 +
 tc/m_mpls.c        | 275 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 434 insertions(+)
 create mode 100644 man/man8/tc-mpls.8
 create mode 100644 tc/m_mpls.c

-- 
2.7.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH iproute2-next 1/3] lib: add mpls_uc and mpls_mc as link layer protocol names
  2019-07-09 15:59 [PATCH iproute2-next 0/3] add interface to TC MPLS actions John Hurley
@ 2019-07-09 15:59 ` John Hurley
  2019-07-09 15:59 ` [PATCH iproute2-next 2/3] tc: add mpls actions John Hurley
  2019-07-09 15:59 ` [PATCH iproute2-next 3/3] man: update man pages for TC MPLS actions John Hurley
  2 siblings, 0 replies; 8+ messages in thread
From: John Hurley @ 2019-07-09 15:59 UTC (permalink / raw)
  To: netdev
  Cc: davem, jiri, xiyou.wangcong, dsahern, willemdebruijn.kernel,
	simon.horman, jakub.kicinski, oss-drivers, John Hurley

Update the llproto_names array to allow users to reference the mpls
protocol ids with the names 'mpls_uc' for unicast MPLS and 'mpls_mc' for
multicast.

Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 lib/ll_proto.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/ll_proto.c b/lib/ll_proto.c
index 78c3961..2a0c1cb 100644
--- a/lib/ll_proto.c
+++ b/lib/ll_proto.c
@@ -78,6 +78,8 @@ __PF(TIPC,tipc)
 __PF(AOE,aoe)
 __PF(8021Q,802.1Q)
 __PF(8021AD,802.1ad)
+__PF(MPLS_UC,mpls_uc)
+__PF(MPLS_MC,mpls_mc)
 
 { 0x8100, "802.1Q" },
 { 0x88cc, "LLDP" },
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH iproute2-next 2/3] tc: add mpls actions
  2019-07-09 15:59 [PATCH iproute2-next 0/3] add interface to TC MPLS actions John Hurley
  2019-07-09 15:59 ` [PATCH iproute2-next 1/3] lib: add mpls_uc and mpls_mc as link layer protocol names John Hurley
@ 2019-07-09 15:59 ` John Hurley
  2019-07-09 17:00   ` Stephen Hemminger
  2019-07-09 20:39   ` David Ahern
  2019-07-09 15:59 ` [PATCH iproute2-next 3/3] man: update man pages for TC MPLS actions John Hurley
  2 siblings, 2 replies; 8+ messages in thread
From: John Hurley @ 2019-07-09 15:59 UTC (permalink / raw)
  To: netdev
  Cc: davem, jiri, xiyou.wangcong, dsahern, willemdebruijn.kernel,
	simon.horman, jakub.kicinski, oss-drivers, John Hurley

Create a new action type for TC that allows the pushing, popping, and
modifying of MPLS headers.

Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tc/Makefile |   1 +
 tc/m_mpls.c | 275 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 276 insertions(+)
 create mode 100644 tc/m_mpls.c

diff --git a/tc/Makefile b/tc/Makefile
index 60abdde..09ff369 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -39,6 +39,7 @@ TCMODULES += q_drr.o
 TCMODULES += q_qfq.o
 TCMODULES += m_gact.o
 TCMODULES += m_mirred.o
+TCMODULES += m_mpls.o
 TCMODULES += m_nat.o
 TCMODULES += m_pedit.o
 TCMODULES += m_ife.o
diff --git a/tc/m_mpls.c b/tc/m_mpls.c
new file mode 100644
index 0000000..d2700ec
--- /dev/null
+++ b/tc/m_mpls.c
@@ -0,0 +1,275 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/* Copyright (C) 2019 Netronome Systems, Inc. */
+
+#include <linux/if_ether.h>
+#include <linux/tc_act/tc_mpls.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "utils.h"
+#include "rt_names.h"
+#include "tc_util.h"
+
+static const char * const action_names[] = {
+	[TCA_MPLS_ACT_POP] = "pop",
+	[TCA_MPLS_ACT_PUSH] = "push",
+	[TCA_MPLS_ACT_MODIFY] = "modify",
+	[TCA_MPLS_ACT_DEC_TTL] = "dec_ttl",
+};
+
+static void explain(void)
+{
+	fprintf(stderr,
+		"Usage: mpls pop [ protocol MPLS_PROTO ]\n"
+		"       mpls push [ protocol MPLS_PROTO ] [ label MPLS_LABEL ] [ tc MPLS_TC ] [ ttl MPLS_TTL ] [ bos MPLS_BOS ] [CONTROL]\n"
+		"       mpls modify [ label MPLS_LABEL ] [ tc MPLS_TC ] [ ttl MPLS_TTL ] [CONTROL]\n"
+		"	for pop MPLS_PROTO is next header of packet - e.g. ip or mpls_uc\n"
+		"       for push MPLS_PROTO is one of mpls_uc or mpls_mc\n"
+		"            with default: mpls_uc\n"
+		"       CONTROL := reclassify | pipe | drop | continue | pass |\n"
+		"                  goto chain <CHAIN_INDEX>\n");
+}
+
+static void usage(void)
+{
+	explain();
+	exit(-1);
+}
+
+static bool can_modify_mpls_fields(unsigned int action)
+{
+	return action == TCA_MPLS_ACT_PUSH || action == TCA_MPLS_ACT_MODIFY;
+}
+
+static bool can_modify_ethtype(unsigned int action)
+{
+	return action == TCA_MPLS_ACT_PUSH || action == TCA_MPLS_ACT_POP;
+}
+
+static bool is_valid_label(__u32 label)
+{
+	return label <= 0xfffff;
+}
+
+static bool check_double_action(unsigned int action, const char *arg)
+{
+	if (!action)
+		return false;
+
+	fprintf(stderr,
+		"Error: got \"%s\" but action already set to \"%s\"\n",
+		arg, action_names[action]);
+	explain();
+	return true;
+}
+
+static int parse_mpls(struct action_util *a, int *argc_p, char ***argv_p,
+		      int tca_id, struct nlmsghdr *n)
+{
+	struct tc_mpls parm = {};
+	__u32 label = 0xffffffff;
+	unsigned int action = 0;
+	char **argv = *argv_p;
+	struct rtattr *tail;
+	int argc = *argc_p;
+	__u16 proto = 0;
+	__u8 bos = 0xff;
+	__u8 tc = 0xff;
+	__u8 ttl = 0;
+
+	if (matches(*argv, "mpls") != 0)
+		return -1;
+
+	NEXT_ARG();
+
+	while (argc > 0) {
+		if (matches(*argv, "pop") == 0) {
+			if (check_double_action(action, *argv))
+				return -1;
+			action = TCA_MPLS_ACT_POP;
+		} else if (matches(*argv, "push") == 0) {
+			if (check_double_action(action, *argv))
+				return -1;
+			action = TCA_MPLS_ACT_PUSH;
+		} else if (matches(*argv, "modify") == 0) {
+			if (check_double_action(action, *argv))
+				return -1;
+			action = TCA_MPLS_ACT_MODIFY;
+		} else if (matches(*argv, "dec_ttl") == 0) {
+			if (check_double_action(action, *argv))
+				return -1;
+			action = TCA_MPLS_ACT_DEC_TTL;
+		} else if (matches(*argv, "label") == 0) {
+			if (!can_modify_mpls_fields(action))
+				invarg("only valid for push/modify", *argv);
+			NEXT_ARG();
+			if (get_u32(&label, *argv, 0) || !is_valid_label(label))
+				invarg("label must be <=0xFFFFF", *argv);
+		} else if (matches(*argv, "tc") == 0) {
+			if (!can_modify_mpls_fields(action))
+				invarg("only valid for push/modify", *argv);
+			NEXT_ARG();
+			if (get_u8(&tc, *argv, 0) || (tc & ~0x7))
+				invarg("tc field is 3 bits max", *argv);
+		} else if (matches(*argv, "ttl") == 0) {
+			if (!can_modify_mpls_fields(action))
+				invarg("only valid for push/modify", *argv);
+			NEXT_ARG();
+			if (get_u8(&ttl, *argv, 0) || !ttl)
+				invarg("ttl must be >0 and <=255", *argv);
+		} else if (matches(*argv, "bos") == 0) {
+			if (!can_modify_mpls_fields(action))
+				invarg("only valid for push/modify", *argv);
+			NEXT_ARG();
+			if (get_u8(&bos, *argv, 0) || (bos & ~0x1))
+				invarg("bos must be 0 or 1", *argv);
+		} else if (matches(*argv, "protocol") == 0) {
+			if (!can_modify_ethtype(action))
+				invarg("only valid for push/pop", *argv);
+			NEXT_ARG();
+			if (ll_proto_a2n(&proto, *argv))
+				invarg("protocol is invalid", *argv);
+		} else if (matches(*argv, "help") == 0) {
+			usage();
+		} else {
+			break;
+		}
+
+		NEXT_ARG_FWD();
+	}
+
+	if (!action)
+		incomplete_command();
+
+	parse_action_control_dflt(&argc, &argv, &parm.action,
+				  false, TC_ACT_PIPE);
+
+	if (argc) {
+		if (matches(*argv, "index") == 0) {
+			NEXT_ARG();
+			if (get_u32(&parm.index, *argv, 10))
+				invarg("illegal index", *argv);
+			NEXT_ARG_FWD();
+		}
+	}
+
+	if (action == TCA_MPLS_ACT_PUSH && !label)
+		missarg("label");
+
+	if (action == TCA_MPLS_ACT_PUSH && proto &&
+	    proto != htons(ETH_P_MPLS_UC) && proto != htons(ETH_P_MPLS_MC)) {
+		fprintf(stderr,
+			"invalid push protocol \"0x%04x\" - use mpls_(uc|mc)\n",
+			ntohs(proto));
+		return -1;
+	}
+
+	if (action == TCA_MPLS_ACT_POP && !proto)
+		missarg("protocol");
+
+	parm.m_action = action;
+	tail = addattr_nest(n, MAX_MSG, tca_id | NLA_F_NESTED);
+	addattr_l(n, MAX_MSG, TCA_MPLS_PARMS, &parm, sizeof(parm));
+	if (label != 0xffffffff)
+		addattr_l(n, MAX_MSG, TCA_MPLS_LABEL, &label, sizeof(label));
+	if (proto)
+		addattr_l(n, MAX_MSG, TCA_MPLS_PROTO, &proto, sizeof(proto));
+	if (tc != 0xff)
+		addattr8(n, MAX_MSG, TCA_MPLS_TC, tc);
+	if (ttl)
+		addattr8(n, MAX_MSG, TCA_MPLS_TTL, ttl);
+	if (bos != 0xff)
+		addattr8(n, MAX_MSG, TCA_MPLS_BOS, bos);
+	addattr_nest_end(n, tail);
+
+	*argc_p = argc;
+	*argv_p = argv;
+	return 0;
+}
+
+static int print_mpls(struct action_util *au, FILE *f, struct rtattr *arg)
+{
+	struct rtattr *tb[TCA_MPLS_MAX + 1];
+	struct tc_mpls *parm;
+	SPRINT_BUF(b1);
+	__u32 val;
+
+	if (!arg)
+		return -1;
+
+	parse_rtattr_nested(tb, TCA_MPLS_MAX, arg);
+
+	if (!tb[TCA_MPLS_PARMS]) {
+		print_string(PRINT_FP, NULL, "%s", "[NULL mpls parameters]");
+		return -1;
+	}
+	parm = RTA_DATA(tb[TCA_MPLS_PARMS]);
+
+	print_string(PRINT_ANY, "kind", "%s ", "mpls");
+	print_string(PRINT_ANY, "mpls_action", " %s",
+		     action_names[parm->m_action]);
+
+	switch (parm->m_action) {
+	case TCA_MPLS_ACT_POP:
+		if (tb[TCA_MPLS_PROTO]) {
+			__u16 proto;
+
+			proto = rta_getattr_u16(tb[TCA_MPLS_PROTO]);
+			print_string(PRINT_ANY, "protocol", " protocol %s",
+				     ll_proto_n2a(proto, b1, sizeof(b1)));
+		}
+		break;
+	case TCA_MPLS_ACT_PUSH:
+		if (tb[TCA_MPLS_PROTO]) {
+			__u16 proto;
+
+			proto = rta_getattr_u16(tb[TCA_MPLS_PROTO]);
+			print_string(PRINT_ANY, "protocol", " protocol %s",
+				     ll_proto_n2a(proto, b1, sizeof(b1)));
+		}
+		/* Fallthrough */
+	case TCA_MPLS_ACT_MODIFY:
+		if (tb[TCA_MPLS_LABEL]) {
+			val = rta_getattr_u32(tb[TCA_MPLS_LABEL]);
+			print_uint(PRINT_ANY, "label", " label %u", val);
+		}
+		if (tb[TCA_MPLS_TC]) {
+			val = rta_getattr_u8(tb[TCA_MPLS_TC]);
+			print_uint(PRINT_ANY, "tc", " tc %u", val);
+		}
+		if (tb[TCA_MPLS_BOS]) {
+			val = rta_getattr_u8(tb[TCA_MPLS_BOS]);
+			print_uint(PRINT_ANY, "bos", " bos %u", val);
+		}
+		if (tb[TCA_MPLS_TTL]) {
+			val = rta_getattr_u8(tb[TCA_MPLS_TTL]);
+			print_uint(PRINT_ANY, "ttl", " ttl %u", val);
+		}
+		break;
+	}
+	print_action_control(f, " ", parm->action, "");
+
+	print_uint(PRINT_ANY, "index", "\n\t index %u", parm->index);
+	print_int(PRINT_ANY, "ref", " ref %d", parm->refcnt);
+	print_int(PRINT_ANY, "bind", " bind %d", parm->bindcnt);
+
+	if (show_stats) {
+		if (tb[TCA_MPLS_TM]) {
+			struct tcf_t *tm = RTA_DATA(tb[TCA_MPLS_TM]);
+
+			print_tm(f, tm);
+		}
+	}
+
+	print_string(PRINT_FP, NULL, "%s", "\n");
+
+	return 0;
+}
+
+struct action_util mpls_action_util = {
+	.id = "mpls",
+	.parse_aopt = parse_mpls,
+	.print_aopt = print_mpls,
+};
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH iproute2-next 3/3] man: update man pages for TC MPLS actions
  2019-07-09 15:59 [PATCH iproute2-next 0/3] add interface to TC MPLS actions John Hurley
  2019-07-09 15:59 ` [PATCH iproute2-next 1/3] lib: add mpls_uc and mpls_mc as link layer protocol names John Hurley
  2019-07-09 15:59 ` [PATCH iproute2-next 2/3] tc: add mpls actions John Hurley
@ 2019-07-09 15:59 ` John Hurley
  2 siblings, 0 replies; 8+ messages in thread
From: John Hurley @ 2019-07-09 15:59 UTC (permalink / raw)
  To: netdev
  Cc: davem, jiri, xiyou.wangcong, dsahern, willemdebruijn.kernel,
	simon.horman, jakub.kicinski, oss-drivers, John Hurley

Add a man page describing the newly added TC mpls manipulation actions.

Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 man/man8/tc-mpls.8 | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 156 insertions(+)
 create mode 100644 man/man8/tc-mpls.8

diff --git a/man/man8/tc-mpls.8 b/man/man8/tc-mpls.8
new file mode 100644
index 0000000..84ef2ef
--- /dev/null
+++ b/man/man8/tc-mpls.8
@@ -0,0 +1,156 @@
+.TH "MPLS manipulation action in tc" 8 "22 May 2019" "iproute2" "Linux"
+
+.SH NAME
+mpls - mpls manipulation module
+.SH SYNOPSIS
+.in +8
+.ti -8
+.BR tc " ... " "action mpls" " { "
+.IR POP " | " PUSH " | " MODIFY " | "
+.BR dec_ttl " } [ "
+.IR CONTROL " ]"
+
+.ti -8
+.IR POP " := "
+.BR pop " " protocol
+.IR MPLS_PROTO
+
+.ti -8
+.IR PUSH " := "
+.BR push " [ " protocol
+.IR MPLS_PROTO " ]"
+.RB " [ " tc
+.IR MPLS_TC " ] "
+.RB " [ " ttl
+.IR MPLS_TTL " ] "
+.RB " [ " bos
+.IR MPLS_BOS " ] "
+.BI label " MPLS_LABEL"
+
+.ti -8
+.IR MODIFY " := "
+.BR modify " [ " label
+.IR MPLS_LABEL " ]"
+.RB " [ " tc
+.IR MPLS_TC " ] "
+.RB " [ " ttl
+.IR MPLS_TTL " ] "
+
+.ti -8
+.IR CONTROL " := { "
+.BR reclassify " | " pipe " | " drop " | " continue " | " pass " | " goto " " chain " " CHAIN_INDEX " }"
+.SH DESCRIPTION
+The
+.B mpls
+action performs mpls encapsulation or decapsulation on a packet, reflected by the
+operation modes
+.IR POP ", " PUSH ", " MODIFY " and " DEC_TTL .
+The
+.I POP
+mode requires the ethertype of the header that follows the MPLS header (e.g.
+IPv4 or another MPLS). It will remove the outer MPLS header and replace the
+ethertype in the MAC header with that passed. The
+.IR PUSH " and " MODIFY
+modes update the current MPLS header information or add a new header.
+.IR PUSH
+requires at least an
+.IR MPLS_LABEL ". "
+.I DEC_TTL
+requires no arguments and simply subtracts 1 from the MPLS header TTL field.
+
+.SH OPTIONS
+.TP
+.B pop
+Decapsulation mode. Requires the protocol of the next header.
+.TP
+.B push
+Encapsulation mode. Requires at least the
+.B label
+option.
+.TP
+.B modify
+Replace mode. Existing MPLS tag is replaced.
+.BR label ", "
+.BR tc ", "
+and
+.B ttl
+are all optional.
+.TP
+.B dec_ttl
+Decrement the TTL field on the outer most MPLS header.
+.TP
+.BI label " MPLS_LABEL"
+Specify the MPLS LABEL for the outer MPLS header.
+.I MPLS_LABEL
+is an unsigned 20bit integer, the format is detected automatically (e.g. prefix
+with
+.RB ' 0x '
+for hexadecimal interpretation, etc.).
+.TP
+.BI protocol " MPLS_PROTO"
+Choose the protocol to use. For push actions this must be
+.BR mpls_uc " or " mpls_mc " (" mpls_uc
+is the default). For pop actions it should be the protocol of the next header.
+This option cannot be used with modify.
+.TP
+.BI tc " MPLS_TC"
+Choose the TC value for the outer MPLS header. Decimal number in range of 0-7.
+Defaults to 0.
+.TP
+.BI ttl " MPLS_TTL"
+Choose the TTL value for the outer MPLS header. Number in range of 0-255. A
+non-zero default value will be selected if this is not explicitly set.
+.TP
+.BI bos " MPLS_BOS"
+Manually configure the bottom of stack bit for an MPLS header push. The default
+is for TC to automatically set (or unset) the bit based on the next header of
+the packet.
+.TP
+.I CONTROL
+How to continue after executing this action.
+.RS
+.TP
+.B reclassify
+Restarts classification by jumping back to the first filter attached to this
+action's parent.
+.TP
+.B pipe
+Continue with the next action, this is the default.
+.TP
+.B drop
+Packet will be dropped without running further actions.
+.TP
+.B continue
+Continue classification with next filter in line.
+.TP
+.B pass
+Return to calling qdisc for packet processing. This ends the classification
+process.
+.RE
+.SH EXAMPLES
+The following example encapsulates incoming IP packets on eth0 into MPLS with
+a label 123 and sends them out eth1:
+
+.RS
+.EX
+#tc qdisc add dev eth0 handle ffff: ingress
+#tc filter add dev eth0 protocol ip parent ffff: flower \\
+	action mpls push protocol mpls_uc label 123  \\
+	action mirred egress redirect dev eth1
+.EE
+.RE
+
+In this example, incoming MPLS unicast packets on eth0 are decapsulated and to
+ip packets and output to eth1:
+
+.RS
+.EX
+#tc qdisc add dev eth0 handle ffff: ingress
+#tc filter add dev eth0 protocol mpls_uc parent ffff: flower \\
+	action mpls pop protocol ipv4  \\
+	action mirred egress redirect dev eth0
+.EE
+.RE
+
+.SH SEE ALSO
+.BR tc (8)
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH iproute2-next 2/3] tc: add mpls actions
  2019-07-09 15:59 ` [PATCH iproute2-next 2/3] tc: add mpls actions John Hurley
@ 2019-07-09 17:00   ` Stephen Hemminger
  2019-07-09 20:36     ` David Ahern
  2019-07-09 20:39   ` David Ahern
  1 sibling, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2019-07-09 17:00 UTC (permalink / raw)
  To: John Hurley
  Cc: netdev, davem, jiri, xiyou.wangcong, dsahern,
	willemdebruijn.kernel, simon.horman, jakub.kicinski, oss-drivers

On Tue,  9 Jul 2019 16:59:31 +0100
John Hurley <john.hurley@netronome.com> wrote:

> 	if (!tb[TCA_MPLS_PARMS]) {
> +		print_string(PRINT_FP, NULL, "%s", "[NULL mpls parameters]");

This is an error message please just use fprintf(stderr instead

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH iproute2-next 2/3] tc: add mpls actions
  2019-07-09 17:00   ` Stephen Hemminger
@ 2019-07-09 20:36     ` David Ahern
  2019-07-09 21:05       ` Stephen Hemminger
  0 siblings, 1 reply; 8+ messages in thread
From: David Ahern @ 2019-07-09 20:36 UTC (permalink / raw)
  To: Stephen Hemminger, John Hurley
  Cc: netdev, davem, jiri, xiyou.wangcong, willemdebruijn.kernel,
	simon.horman, jakub.kicinski, oss-drivers

On 7/9/19 11:00 AM, Stephen Hemminger wrote:
> On Tue,  9 Jul 2019 16:59:31 +0100
> John Hurley <john.hurley@netronome.com> wrote:
> 
>> 	if (!tb[TCA_MPLS_PARMS]) {
>> +		print_string(PRINT_FP, NULL, "%s", "[NULL mpls parameters]");
> 
> This is an error message please just use fprintf(stderr instead
> 

skbedit, nat as 2 examples (and the only 2 I checked) do the print_string.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH iproute2-next 2/3] tc: add mpls actions
  2019-07-09 15:59 ` [PATCH iproute2-next 2/3] tc: add mpls actions John Hurley
  2019-07-09 17:00   ` Stephen Hemminger
@ 2019-07-09 20:39   ` David Ahern
  1 sibling, 0 replies; 8+ messages in thread
From: David Ahern @ 2019-07-09 20:39 UTC (permalink / raw)
  To: John Hurley, netdev
  Cc: davem, jiri, xiyou.wangcong, willemdebruijn.kernel, simon.horman,
	jakub.kicinski, oss-drivers


On 7/9/19 9:59 AM, John Hurley wrote:
> +static void explain(void)
> +{
> +	fprintf(stderr,
> +		"Usage: mpls pop [ protocol MPLS_PROTO ]\n"
> +		"       mpls push [ protocol MPLS_PROTO ] [ label MPLS_LABEL ] [ tc MPLS_TC ] [ ttl MPLS_TTL ] [ bos MPLS_BOS ] [CONTROL]\n"

that makes for a very long line to the user. Break at the ttl option and
make a newline:

mpls push [ protocol MPLS_PROTO ] [ label MPLS_LABEL ] [ tc MPLS_TC ]
          [ ttl MPLS_TTL ] [ bos MPLS_BOS ] [CONTROL]


> +		"       mpls modify [ label MPLS_LABEL ] [ tc MPLS_TC ] [ ttl MPLS_TTL ] [CONTROL]\n"
> +		"	for pop MPLS_PROTO is next header of packet - e.g. ip or mpls_uc\n"
> +		"       for push MPLS_PROTO is one of mpls_uc or mpls_mc\n"
> +		"            with default: mpls_uc\n"
> +		"       CONTROL := reclassify | pipe | drop | continue | pass |\n"
> +		"                  goto chain <CHAIN_INDEX>\n");
> +}
> +


...

> +static int print_mpls(struct action_util *au, FILE *f, struct rtattr *arg)
> +{
> +	struct rtattr *tb[TCA_MPLS_MAX + 1];
> +	struct tc_mpls *parm;
> +	SPRINT_BUF(b1);
> +	__u32 val;
> +
> +	if (!arg)
> +		return -1;
> +
> +	parse_rtattr_nested(tb, TCA_MPLS_MAX, arg);
> +
> +	if (!tb[TCA_MPLS_PARMS]) {
> +		print_string(PRINT_FP, NULL, "%s", "[NULL mpls parameters]");
> +		return -1;
> +	}
> +	parm = RTA_DATA(tb[TCA_MPLS_PARMS]);
> +
> +	print_string(PRINT_ANY, "kind", "%s ", "mpls");
> +	print_string(PRINT_ANY, "mpls_action", " %s",
> +		     action_names[parm->m_action]);
> +
> +	switch (parm->m_action) {
> +	case TCA_MPLS_ACT_POP:
> +		if (tb[TCA_MPLS_PROTO]) {
> +			__u16 proto;
> +
> +			proto = rta_getattr_u16(tb[TCA_MPLS_PROTO]);
> +			print_string(PRINT_ANY, "protocol", " protocol %s",
> +				     ll_proto_n2a(proto, b1, sizeof(b1)));
> +		}
> +		break;
> +	case TCA_MPLS_ACT_PUSH:
> +		if (tb[TCA_MPLS_PROTO]) {
> +			__u16 proto;
> +
> +			proto = rta_getattr_u16(tb[TCA_MPLS_PROTO]);
> +			print_string(PRINT_ANY, "protocol", " protocol %s",
> +				     ll_proto_n2a(proto, b1, sizeof(b1)));
> +		}
> +		/* Fallthrough */
> +	case TCA_MPLS_ACT_MODIFY:
> +		if (tb[TCA_MPLS_LABEL]) {
> +			val = rta_getattr_u32(tb[TCA_MPLS_LABEL]);
> +			print_uint(PRINT_ANY, "label", " label %u", val);
> +		}
> +		if (tb[TCA_MPLS_TC]) {
> +			val = rta_getattr_u8(tb[TCA_MPLS_TC]);
> +			print_uint(PRINT_ANY, "tc", " tc %u", val);
> +		}
> +		if (tb[TCA_MPLS_BOS]) {
> +			val = rta_getattr_u8(tb[TCA_MPLS_BOS]);
> +			print_uint(PRINT_ANY, "bos", " bos %u", val);
> +		}
> +		if (tb[TCA_MPLS_TTL]) {
> +			val = rta_getattr_u8(tb[TCA_MPLS_TTL]);
> +			print_uint(PRINT_ANY, "ttl", " ttl %u", val);
> +		}
> +		break;
> +	}
> +	print_action_control(f, " ", parm->action, "");
> +
> +	print_uint(PRINT_ANY, "index", "\n\t index %u", parm->index);
> +	print_int(PRINT_ANY, "ref", " ref %d", parm->refcnt);
> +	print_int(PRINT_ANY, "bind", " bind %d", parm->bindcnt);
> +
> +	if (show_stats) {
> +		if (tb[TCA_MPLS_TM]) {
> +			struct tcf_t *tm = RTA_DATA(tb[TCA_MPLS_TM]);
> +
> +			print_tm(f, tm);
> +		}
> +	}
> +
> +	print_string(PRINT_FP, NULL, "%s", "\n");

s/"\n"/_SL_/ ?


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH iproute2-next 2/3] tc: add mpls actions
  2019-07-09 20:36     ` David Ahern
@ 2019-07-09 21:05       ` Stephen Hemminger
  0 siblings, 0 replies; 8+ messages in thread
From: Stephen Hemminger @ 2019-07-09 21:05 UTC (permalink / raw)
  To: David Ahern
  Cc: John Hurley, netdev, davem, jiri, xiyou.wangcong,
	willemdebruijn.kernel, simon.horman, jakub.kicinski, oss-drivers

On Tue, 9 Jul 2019 14:36:34 -0600
David Ahern <dsahern@gmail.com> wrote:

> On 7/9/19 11:00 AM, Stephen Hemminger wrote:
> > On Tue,  9 Jul 2019 16:59:31 +0100
> > John Hurley <john.hurley@netronome.com> wrote:
> >   
> >> 	if (!tb[TCA_MPLS_PARMS]) {
> >> +		print_string(PRINT_FP, NULL, "%s", "[NULL mpls parameters]");  
> > 
> > This is an error message please just use fprintf(stderr instead
> >   
> 
> skbedit, nat as 2 examples (and the only 2 I checked) do the print_string.

Thanks I will fix those.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-07-09 21:05 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-09 15:59 [PATCH iproute2-next 0/3] add interface to TC MPLS actions John Hurley
2019-07-09 15:59 ` [PATCH iproute2-next 1/3] lib: add mpls_uc and mpls_mc as link layer protocol names John Hurley
2019-07-09 15:59 ` [PATCH iproute2-next 2/3] tc: add mpls actions John Hurley
2019-07-09 17:00   ` Stephen Hemminger
2019-07-09 20:36     ` David Ahern
2019-07-09 21:05       ` Stephen Hemminger
2019-07-09 20:39   ` David Ahern
2019-07-09 15:59 ` [PATCH iproute2-next 3/3] man: update man pages for TC MPLS actions John Hurley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).