diff mbox series

[iproute2-next,2/2] m_mpls: add mac_push action

Message ID 622d70e7bb6158c6f207661dea8c47e129f16107.1602598178.git.gnault@redhat.com
State Changes Requested
Delegated to: David Ahern
Headers show
Series tc: support for new MPLS L2 VPN actions | expand

Commit Message

Guillaume Nault Oct. 13, 2020, 2:32 p.m. UTC 
Add support for the new TCA_MPLS_ACT_MAC_PUSH action (kernel commit
a45294af9e96 ("net/sched: act_mpls: Add action to push MPLS LSE before
Ethernet header")). This action let TC push an MPLS header before the
MAC header of a frame.

Example (encapsulate all outgoing frames with label 20, then add an
outer Ethernet header):
 # tc filter add dev ethX matchall \
       action mpls mac_push label 20 ttl 64 \
       action vlan push_eth dst_mac 0a:00:00:00:00:02 \
                            src_mac 0a:00:00:00:00:01

This patch also adds an alias for ETH_P_TEB, since it is useful when
decapsulating MPLS packets that contain an Ethernet frame.

With MAC_PUSH, there's no previous Ethertype to modify. However, the
"protocol" option is still needed, because the kernel uses it to set
skb->protocol. So rename can_modify_ethtype() to can_set_ethtype().

Also add a test suite for m_mpls, which covers the new action and the
pre-existing ones.

Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
 lib/ll_proto.c            |  1 +
 man/man8/tc-mpls.8        | 44 +++++++++++++++++++++++--
 man/man8/tc-vlan.8        |  5 ++-
 tc/m_mpls.c               | 41 +++++++++++++++--------
 testsuite/tests/tc/mpls.t | 69 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 143 insertions(+), 17 deletions(-)
 create mode 100755 testsuite/tests/tc/mpls.t

Comments

David Ahern Oct. 17, 2020, 3:23 p.m. UTC | #1 
On 10/13/20 8:32 AM, Guillaume Nault wrote:
> @@ -41,12 +44,12 @@ static void usage(void)
>  
>  static bool can_modify_mpls_fields(unsigned int action)
>  {
> -	return action == TCA_MPLS_ACT_PUSH || action == TCA_MPLS_ACT_MODIFY;
> +	return action == TCA_MPLS_ACT_PUSH || action == TCA_MPLS_ACT_MAC_PUSH || action == TCA_MPLS_ACT_MODIFY;
>  }
>  
> -static bool can_modify_ethtype(unsigned int action)
> +static bool can_set_ethtype(unsigned int action)
>  {
> -	return action == TCA_MPLS_ACT_PUSH || action == TCA_MPLS_ACT_POP;
> +	return action == TCA_MPLS_ACT_PUSH || action == TCA_MPLS_ACT_MAC_PUSH || action == TCA_MPLS_ACT_POP;
>  }
>  
>  static bool is_valid_label(__u32 label)

nit: please wrap the lines with the new action.

Besides the nit, very nice and complete change set - man page, help, and
 tests.
Guillaume Nault Oct. 19, 2020, 1:39 p.m. UTC | #2 
On Sat, Oct 17, 2020 at 09:23:52AM -0600, David Ahern wrote:
> On 10/13/20 8:32 AM, Guillaume Nault wrote:
> > @@ -41,12 +44,12 @@ static void usage(void)
> >  
> >  static bool can_modify_mpls_fields(unsigned int action)
> >  {
> > -	return action == TCA_MPLS_ACT_PUSH || action == TCA_MPLS_ACT_MODIFY;
> > +	return action == TCA_MPLS_ACT_PUSH || action == TCA_MPLS_ACT_MAC_PUSH || action == TCA_MPLS_ACT_MODIFY;
> >  }
> >  
> > -static bool can_modify_ethtype(unsigned int action)
> > +static bool can_set_ethtype(unsigned int action)
> >  {
> > -	return action == TCA_MPLS_ACT_PUSH || action == TCA_MPLS_ACT_POP;
> > +	return action == TCA_MPLS_ACT_PUSH || action == TCA_MPLS_ACT_MAC_PUSH || action == TCA_MPLS_ACT_POP;
> >  }
> >  
> >  static bool is_valid_label(__u32 label)
> 
> nit: please wrap the lines with the new action.

Will do.

> Besides the nit, very nice and complete change set - man page, help, and
>  tests.

Thanks!
diff mbox series

Patch

diff --git a/lib/ll_proto.c b/lib/ll_proto.c
index 2a0c1cb3..78179311 100644
--- a/lib/ll_proto.c
+++ b/lib/ll_proto.c
@@ -80,6 +80,7 @@  __PF(8021Q,802.1Q)
 __PF(8021AD,802.1ad)
 __PF(MPLS_UC,mpls_uc)
 __PF(MPLS_MC,mpls_mc)
+__PF(TEB,teb)
 
 { 0x8100, "802.1Q" },
 { 0x88cc, "LLDP" },
diff --git a/man/man8/tc-mpls.8 b/man/man8/tc-mpls.8
index 84ef2ef1..9e563e98 100644
--- a/man/man8/tc-mpls.8
+++ b/man/man8/tc-mpls.8
@@ -17,7 +17,7 @@  mpls - mpls manipulation module
 
 .ti -8
 .IR PUSH " := "
-.BR push " [ " protocol
+.RB "{ " push " | " mac_push " } [ " protocol
 .IR MPLS_PROTO " ]"
 .RB " [ " tc
 .IR MPLS_TC " ] "
@@ -64,7 +64,14 @@  requires no arguments and simply subtracts 1 from the MPLS header TTL field.
 Decapsulation mode. Requires the protocol of the next header.
 .TP
 .B push
-Encapsulation mode. Requires at least the
+Encapsulation mode. Adds the MPLS header between the MAC and the network
+headers. Requires at least the
+.B label
+option.
+.TP
+.B mac_push
+Encapsulation mode. Adds the MPLS header before the MAC header. Requires at
+least the
 .B label
 option.
 .TP
@@ -152,5 +159,36 @@  ip packets and output to eth1:
 .EE
 .RE
 
+Here is another example, where incoming Ethernet frames are encapsulated into
+MPLS with label 123 and TTL 64. Then, an outer Ethernet header is added and the
+resulting frame is finally sent on eth1:
+
+.RS
+.EX
+#tc qdisc add dev eth0 ingress
+#tc filter add dev eth0 ingress matchall \\
+	action mpls mac_push label 123 ttl 64 \\
+	action vlan push_eth \\
+		dst_mac 02:00:00:00:00:02 \\
+		src_mac 02:00:00:00:00:01 \\
+	action mirred egress redirect dev eth1
+.EE
+.RE
+
+The following example assumes that incoming MPLS packets with label 123
+transport Ethernet frames. The outer Ethernet and the MPLS headers are
+stripped, then the inner Ethernet frame is sent on eth1:
+
+.RS
+.EX
+#tc qdisc add dev eth0 ingress
+#tc filter add dev eth0 ingress protocol mpls_uc \\
+	flower mpls_label 123 mpls_bos 1 \\
+	action vlan pop_eth \\
+	action mpls pop protocol teb \\
+	action mirred egress redirect dev eth1
+.EE
+.RE
+
 .SH SEE ALSO
-.BR tc (8)
+.BR tc "(8), " tc-mirred "(8), " tc-vlan (8)
diff --git a/man/man8/tc-vlan.8 b/man/man8/tc-vlan.8
index 5c2808b1..264053d3 100644
--- a/man/man8/tc-vlan.8
+++ b/man/man8/tc-vlan.8
@@ -157,5 +157,8 @@  process then restarted for the plain packet:
 .EE
 .RE
 
+For an example of the
+.BR pop_eth " and " push_eth " modes, see " tc-mpls (8).
+
 .SH SEE ALSO
-.BR tc (8)
+.BR tc "(8), " tc-mpls (8)
diff --git a/tc/m_mpls.c b/tc/m_mpls.c
index 3d5d9b25..56977fa9 100644
--- a/tc/m_mpls.c
+++ b/tc/m_mpls.c
@@ -17,6 +17,7 @@  static const char * const action_names[] = {
 	[TCA_MPLS_ACT_PUSH] = "push",
 	[TCA_MPLS_ACT_MODIFY] = "modify",
 	[TCA_MPLS_ACT_DEC_TTL] = "dec_ttl",
+	[TCA_MPLS_ACT_MAC_PUSH] = "mac_push",
 };
 
 static void explain(void)
@@ -25,9 +26,11 @@  static void explain(void)
 		"Usage: mpls pop [ protocol MPLS_PROTO ]\n"
 		"       mpls push [ protocol MPLS_PROTO ] [ label MPLS_LABEL ] [ tc MPLS_TC ]\n"
 		"                 [ ttl MPLS_TTL ] [ bos MPLS_BOS ] [CONTROL]\n"
+		"       mpls mac_push [ protocol MPLS_PROTO ] [ label MPLS_LABEL ] [ tc MPLS_TC ]\n"
+		"                     [ 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"
+		"           for pop, MPLS_PROTO is next header of packet - e.g. ip or mpls_uc\n"
+		"           for push and mac_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");
@@ -41,12 +44,12 @@  static void usage(void)
 
 static bool can_modify_mpls_fields(unsigned int action)
 {
-	return action == TCA_MPLS_ACT_PUSH || action == TCA_MPLS_ACT_MODIFY;
+	return action == TCA_MPLS_ACT_PUSH || action == TCA_MPLS_ACT_MAC_PUSH || action == TCA_MPLS_ACT_MODIFY;
 }
 
-static bool can_modify_ethtype(unsigned int action)
+static bool can_set_ethtype(unsigned int action)
 {
-	return action == TCA_MPLS_ACT_PUSH || action == TCA_MPLS_ACT_POP;
+	return action == TCA_MPLS_ACT_PUSH || action == TCA_MPLS_ACT_MAC_PUSH || action == TCA_MPLS_ACT_POP;
 }
 
 static bool is_valid_label(__u32 label)
@@ -94,6 +97,10 @@  static int parse_mpls(struct action_util *a, int *argc_p, char ***argv_p,
 			if (check_double_action(action, *argv))
 				return -1;
 			action = TCA_MPLS_ACT_PUSH;
+		} else if (matches(*argv, "mac_push") == 0) {
+			if (check_double_action(action, *argv))
+				return -1;
+			action = TCA_MPLS_ACT_MAC_PUSH;
 		} else if (matches(*argv, "modify") == 0) {
 			if (check_double_action(action, *argv))
 				return -1;
@@ -104,31 +111,36 @@  static int parse_mpls(struct action_util *a, int *argc_p, char ***argv_p,
 			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);
+				invarg("only valid for push, mac_push and 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);
+				invarg("only valid for push, mac_push and 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);
+				invarg("only valid for push, mac_push and 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);
+				invarg("only valid for push, mac_push and 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);
+			if (!can_set_ethtype(action))
+				invarg("only valid for push, mac_push and pop",
+				       *argv);
 			NEXT_ARG();
 			if (ll_proto_a2n(&proto, *argv))
 				invarg("protocol is invalid", *argv);
@@ -159,10 +171,12 @@  static int parse_mpls(struct action_util *a, int *argc_p, char ***argv_p,
 	if (action == TCA_MPLS_ACT_PUSH && label == 0xffffffff)
 		missarg("label");
 
-	if (action == TCA_MPLS_ACT_PUSH && proto &&
+	if ((action == TCA_MPLS_ACT_PUSH || action == TCA_MPLS_ACT_MAC_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",
+			"invalid %spush protocol \"0x%04x\" - use mpls_(uc|mc)\n",
+			action == TCA_MPLS_ACT_MAC_PUSH ? "mac_" : "",
 			ntohs(proto));
 		return -1;
 	}
@@ -223,6 +237,7 @@  static int print_mpls(struct action_util *au, FILE *f, struct rtattr *arg)
 		}
 		break;
 	case TCA_MPLS_ACT_PUSH:
+	case TCA_MPLS_ACT_MAC_PUSH:
 		if (tb[TCA_MPLS_PROTO]) {
 			__u16 proto;
 
diff --git a/testsuite/tests/tc/mpls.t b/testsuite/tests/tc/mpls.t
new file mode 100755
index 00000000..cb25f361
--- /dev/null
+++ b/testsuite/tests/tc/mpls.t
@@ -0,0 +1,69 @@ 
+#!/bin/sh
+
+. lib/generic.sh
+
+DEV="$(rand_dev)"
+ts_ip "$0" "Add $DEV dummy interface" link add dev $DEV up type dummy
+ts_tc "$0" "Add ingress qdisc" qdisc add dev $DEV ingress
+
+reset_qdisc()
+{
+	ts_tc "$0" "Remove ingress qdisc" qdisc del dev $DEV ingress
+	ts_tc "$0" "Add ingress qdisc" qdisc add dev $DEV ingress
+}
+
+ts_tc "$0" "Add mpls action pop"                              \
+	filter add dev $DEV ingress protocol mpls_uc matchall \
+	action mpls pop protocol ip
+ts_tc "$0" "Show ingress filters" filter show dev $DEV ingress
+test_on "mpls"
+test_on "pop protocol ip pipe"
+
+reset_qdisc
+ts_tc "$0" "Add mpls action push"                        \
+	filter add dev $DEV ingress protocol ip matchall \
+	action mpls push protocol mpls_uc label 20 tc 3 bos 1 ttl 64
+ts_tc "$0" "Show ingress filters" filter show dev $DEV ingress
+test_on "mpls"
+test_on "push"
+test_on "protocol mpls_uc"
+test_on "label 20"
+test_on "tc 3"
+test_on "bos 1"
+test_on "ttl 64"
+test_on "pipe"
+
+reset_qdisc
+ts_tc "$0" "Add mpls action mac_push"        \
+	filter add dev $DEV ingress matchall \
+	action mpls mac_push protocol mpls_uc label 20 tc 3 bos 1 ttl 64
+ts_tc "$0" "Show ingress filters" filter show dev $DEV ingress
+test_on "mpls"
+test_on "mac_push"
+test_on "protocol mpls_uc"
+test_on "label 20"
+test_on "tc 3"
+test_on "bos 1"
+test_on "ttl 64"
+test_on "pipe"
+
+reset_qdisc
+ts_tc "$0" "Add mpls action modify"                           \
+	filter add dev $DEV ingress protocol mpls_uc matchall \
+	action mpls modify label 20 tc 3 ttl 64
+ts_tc "$0" "Show ingress filters" filter show dev $DEV ingress
+test_on "mpls"
+test_on "modify"
+test_on "label 20"
+test_on "tc 3"
+test_on "ttl 64"
+test_on "pipe"
+
+reset_qdisc
+ts_tc "$0" "Add mpls action dec_ttl"                          \
+	filter add dev $DEV ingress protocol mpls_uc matchall \
+	action mpls dec_ttl
+ts_tc "$0" "Show ingress filters" filter show dev $DEV ingress
+test_on "mpls"
+test_on "dec_ttl"
+test_on "pipe"