[PATCH iproute2] ip link: initial support for bareudp devices

[PATCH iproute2] ip link: initial support for bareudp devices

[Guillaume Nault]

Bareudp devices provide a generic L3 encapsulation for tunnelling
different protocols like MPLS, IP, NSH, etc. inside a UDP tunnel.

This patch is based on original work from Martin Varghese:
https://lore.kernel.org/netdev/1570532361-15163-1-git-send-email-martinvarghesenokia@gmail.com/

Examples:

  - ip link add dev bareudp0 type bareudp dstport 6635 ethertype mpls_uc

This creates a bareudp tunnel device which tunnels L3 traffic with
ethertype 0x8847 (unicast MPLS traffic). The destination port of the
UDP header will be set to 6635. The device will listen on UDP port 6635
to receive traffic.

  - ip link add dev bareudp0 type bareudp dstport 6635 ethertype ipv4 multiproto

Same as the MPLS example, but for IPv4. The "multiproto" keyword allows
the device to also tunnel IPv6 traffic.

Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
 ip/Makefile           |   2 +-
 ip/iplink.c           |   2 +-
 ip/iplink_bareudp.c   | 150 ++++++++++++++++++++++++++++++++++++++++++
 man/man8/ip-link.8.in |  44 +++++++++++++
 4 files changed, 196 insertions(+), 2 deletions(-)
 create mode 100644 ip/iplink_bareudp.c

diff --git a/ip/Makefile b/ip/Makefile
index 8735b8e4..4cad619c 100644
--- a/ip/Makefile
+++ b/ip/Makefile
@@ -11,7 +11,7 @@ IPOBJ=ip.o ipaddress.o ipaddrlabel.o iproute.o iprule.o ipnetns.o \
     iplink_bridge.o iplink_bridge_slave.o ipfou.o iplink_ipvlan.o \
     iplink_geneve.o iplink_vrf.o iproute_lwtunnel.o ipmacsec.o ipila.o \
     ipvrf.o iplink_xstats.o ipseg6.o iplink_netdevsim.o iplink_rmnet.o \
-    ipnexthop.o ipmptcp.o
+    ipnexthop.o ipmptcp.o iplink_bareudp.o
 
 RTMONOBJ=rtmon.o
 
diff --git a/ip/iplink.c b/ip/iplink.c
index 47f73988..7d4b244d 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -124,7 +124,7 @@ void iplink_usage(void)
 			"	   bridge | bond | team | ipoib | ip6tnl | ipip | sit | vxlan |\n"
 			"	   gre | gretap | erspan | ip6gre | ip6gretap | ip6erspan |\n"
 			"	   vti | nlmon | team_slave | bond_slave | bridge_slave |\n"
-			"	   ipvlan | ipvtap | geneve | vrf | macsec | netdevsim | rmnet |\n"
+			"	   ipvlan | ipvtap | geneve | bareudp | vrf | macsec | netdevsim | rmnet |\n"
 			"	   xfrm }\n");
 	}
 	exit(-1);
diff --git a/ip/iplink_bareudp.c b/ip/iplink_bareudp.c
new file mode 100644
index 00000000..885e1110
--- /dev/null
+++ b/ip/iplink_bareudp.c
@@ -0,0 +1,150 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <stdio.h>
+
+#include "libnetlink.h"
+#include "linux/if_ether.h"
+#include "linux/if_link.h"
+#include "linux/netlink.h"
+#include "linux/rtnetlink.h"
+#include "rt_names.h"
+#include "utils.h"
+#include "ip_common.h"
+#include "json_print.h"
+
+#define BAREUDP_ATTRSET(attrs, type) (((attrs) & (1L << (type))) != 0)
+
+static void print_explain(FILE *f)
+{
+	fprintf(f,
+		"Usage: ... bareudp dstport PORT\n"
+		"		ethertype PROTO\n"
+		"		[ srcportmin PORT ]\n"
+		"		[ [no]multiproto ]\n"
+		"\n"
+		"Where:	PORT       := 0-65535\n"
+		"	PROTO      := NUMBER | ip | mpls\n"
+		"	SRCPORTMIN := 0-65535\n"
+	);
+}
+
+static void explain(void)
+{
+	print_explain(stderr);
+}
+
+static void check_duparg(__u64 *attrs, int type, const char *key,
+			 const char *argv)
+{
+	if (!BAREUDP_ATTRSET(*attrs, type)) {
+		*attrs |= (1L << type);
+		return;
+	}
+	duparg2(key, argv);
+}
+
+static int bareudp_parse_opt(struct link_util *lu, int argc, char **argv,
+			     struct nlmsghdr *n)
+{
+	bool multiproto = false;
+	__u16 srcportmin = 0;
+	__be16 ethertype = 0;
+	__be16 dstport = 0;
+	__u64 attrs = 0;
+
+	while (argc > 0) {
+		if (matches(*argv, "dstport") == 0) {
+			NEXT_ARG();
+			check_duparg(&attrs, IFLA_BAREUDP_PORT, "dstport",
+				     *argv);
+			if (get_be16(&dstport, *argv, 0))
+				invarg("dstport", *argv);
+		} else if (matches(*argv, "ethertype") == 0)  {
+			NEXT_ARG();
+			check_duparg(&attrs, IFLA_BAREUDP_ETHERTYPE,
+				     "ethertype", *argv);
+			if (ll_proto_a2n(&ethertype, *argv))
+				invarg("ethertype", *argv);
+		} else if (matches(*argv, "srcportmin") == 0) {
+			NEXT_ARG();
+			check_duparg(&attrs, IFLA_BAREUDP_SRCPORT_MIN,
+				     "srcportmin", *argv);
+			if (get_u16(&srcportmin, *argv, 0))
+				invarg("srcportmin", *argv);
+		} else if (matches(*argv, "multiproto") == 0) {
+			check_duparg(&attrs, IFLA_BAREUDP_MULTIPROTO_MODE,
+				     *argv, *argv);
+			multiproto = true;
+		} else if (matches(*argv, "nomultiproto") == 0) {
+			check_duparg(&attrs, IFLA_BAREUDP_MULTIPROTO_MODE,
+				     *argv, *argv);
+			multiproto = false;
+		} else if (matches(*argv, "help") == 0) {
+			explain();
+			return -1;
+		} else {
+			fprintf(stderr, "bareudp: unknown command \"%s\"?\n",
+				*argv);
+			explain();
+			return -1;
+		}
+		argc--, argv++;
+	}
+
+	if (!BAREUDP_ATTRSET(attrs, IFLA_BAREUDP_PORT))
+		missarg("dstport");
+	if (!BAREUDP_ATTRSET(attrs, IFLA_BAREUDP_ETHERTYPE))
+		missarg("ethertype");
+
+	addattr16(n, 1024, IFLA_BAREUDP_PORT, dstport);
+	addattr16(n, 1024, IFLA_BAREUDP_ETHERTYPE, ethertype);
+	if (BAREUDP_ATTRSET(attrs, IFLA_BAREUDP_SRCPORT_MIN))
+		addattr16(n, 1024, IFLA_BAREUDP_SRCPORT_MIN, srcportmin);
+	if (multiproto)
+		addattr(n, 1024, IFLA_BAREUDP_MULTIPROTO_MODE);
+
+	return 0;
+}
+
+static void bareudp_print_opt(struct link_util *lu, FILE *f,
+			      struct rtattr *tb[])
+{
+	if (!tb)
+		return;
+
+	if (tb[IFLA_BAREUDP_PORT])
+		print_uint(PRINT_ANY, "dstport", "dstport %u ",
+			   rta_getattr_be16(tb[IFLA_BAREUDP_PORT]));
+
+	if (tb[IFLA_BAREUDP_ETHERTYPE]) {
+		struct rtattr *attr = tb[IFLA_BAREUDP_ETHERTYPE];
+		SPRINT_BUF(ethertype);
+
+		print_string(PRINT_ANY, "ethertype", "ethertype %s ",
+			     ll_proto_n2a(rta_getattr_u16(attr),
+					  ethertype, sizeof(ethertype)));
+	}
+
+	if (tb[IFLA_BAREUDP_SRCPORT_MIN])
+		print_uint(PRINT_ANY, "srcportmin", "srcportmin %u ",
+			   rta_getattr_u16(tb[IFLA_BAREUDP_SRCPORT_MIN]));
+
+	if (tb[IFLA_BAREUDP_MULTIPROTO_MODE])
+		print_bool(PRINT_ANY, "multiproto", "multiproto ", true);
+	else
+		print_bool(PRINT_ANY, "multiproto", "nomultiproto ", false);
+}
+
+static void bareudp_print_help(struct link_util *lu, int argc, char **argv,
+			       FILE *f)
+{
+	print_explain(f);
+}
+
+struct link_util bareudp_link_util = {
+	.id		= "bareudp",
+	.maxattr	= IFLA_BAREUDP_MAX,
+	.parse_opt	= bareudp_parse_opt,
+	.print_opt	= bareudp_print_opt,
+	.print_help	= bareudp_print_help,
+};
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index e8a25451..c6bd2c53 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -223,6 +223,7 @@ ip-link \- network device configuration
 .BR ipvtap " |"
 .BR lowpan " |"
 .BR geneve " |"
+.BR bareudp " |"
 .BR vrf " |"
 .BR macsec " |"
 .BR netdevsim " |"
@@ -356,6 +357,9 @@ Link types:
 .BR geneve
 - GEneric NEtwork Virtualization Encapsulation
 .sp
+.BR bareudp
+- Bare UDP L3 encapsulation support
+.sp
 .BR macsec
 - Interface for IEEE 802.1AE MAC Security (MACsec)
 .sp
@@ -1293,6 +1297,46 @@ options.
 
 .in -8
 
+.TP
+Bareudp Type Support
+For a link of type
+.I Bareudp
+the following additional arguments are supported:
+
+.BI "ip link add " DEVICE
+.BI type " bareudp " dstport " PORT " ethertype " ETHERTYPE"
+[
+.BI srcportmin " SRCPORTMIN "
+] [
+.RB [ no ] multiproto
+]
+
+.in +8
+.sp
+.BI dstport " PORT"
+- specifies the destination port for the UDP tunnel.
+
+.sp
+.BI ethertype " ETHERTYPE"
+- specifies the ethertype of the L3 protocol being tunnelled.
+
+.sp
+.BI srcportmin " SRCPORTMIN"
+- selects the lowest value of the UDP tunnel source port range.
+
+.sp
+.RB [ no ] multiproto
+- activates support for protocols similar to the one
+.RB "specified by " ethertype .
+When
+.I ETHERTYPE
+is "mpls_uc" (that is, unicast MPLS), this allows the tunnel to also handle
+multicast MPLS.
+When
+.I ETHERTYPE
+is "ipv4", this allows the tunnel to also handle IPv6. This option is disabled
+by default.
+
 .TP
 MACVLAN and MACVTAP Type Support
 For a link of type
-- 
2.21.3

Re: [PATCH iproute2] ip link: initial support for bareudp devicesy

[Martin Varghese]

On Wed, Jul 01, 2020 at 09:45:04PM +0200, Guillaume Nault wrote:
> Bareudp devices provide a generic L3 encapsulation for tunnelling
> different protocols like MPLS, IP, NSH, etc. inside a UDP tunnel.
> 
> This patch is based on original work from Martin Varghese:
> https://lore.kernel.org/netdev/1570532361-15163-1-git-send-email-martinvarghesenokia@gmail.com/
> 
> Examples:
> 
>   - ip link add dev bareudp0 type bareudp dstport 6635 ethertype mpls_uc
> 
> This creates a bareudp tunnel device which tunnels L3 traffic with
> ethertype 0x8847 (unicast MPLS traffic). The destination port of the
> UDP header will be set to 6635. The device will listen on UDP port 6635
> to receive traffic.
> 
>   - ip link add dev bareudp0 type bareudp dstport 6635 ethertype ipv4 multiproto
> 
> Same as the MPLS example, but for IPv4. The "multiproto" keyword allows
> the device to also tunnel IPv6 traffic.
> 
> Signed-off-by: Guillaume Nault <gnault@redhat.com>
> ---
>  ip/Makefile           |   2 +-
>  ip/iplink.c           |   2 +-
>  ip/iplink_bareudp.c   | 150 ++++++++++++++++++++++++++++++++++++++++++
>  man/man8/ip-link.8.in |  44 +++++++++++++
>  4 files changed, 196 insertions(+), 2 deletions(-)
>  create mode 100644 ip/iplink_bareudp.c
> 
> diff --git a/ip/Makefile b/ip/Makefile
> index 8735b8e4..4cad619c 100644
> --- a/ip/Makefile
> +++ b/ip/Makefile
> @@ -11,7 +11,7 @@ IPOBJ=ip.o ipaddress.o ipaddrlabel.o iproute.o iprule.o ipnetns.o \
>      iplink_bridge.o iplink_bridge_slave.o ipfou.o iplink_ipvlan.o \
>      iplink_geneve.o iplink_vrf.o iproute_lwtunnel.o ipmacsec.o ipila.o \
>      ipvrf.o iplink_xstats.o ipseg6.o iplink_netdevsim.o iplink_rmnet.o \
> -    ipnexthop.o ipmptcp.o
> +    ipnexthop.o ipmptcp.o iplink_bareudp.o
>  
>  RTMONOBJ=rtmon.o
>  
> diff --git a/ip/iplink.c b/ip/iplink.c
> index 47f73988..7d4b244d 100644
> --- a/ip/iplink.c
> +++ b/ip/iplink.c
> @@ -124,7 +124,7 @@ void iplink_usage(void)
>  			"	   bridge | bond | team | ipoib | ip6tnl | ipip | sit | vxlan |\n"
>  			"	   gre | gretap | erspan | ip6gre | ip6gretap | ip6erspan |\n"
>  			"	   vti | nlmon | team_slave | bond_slave | bridge_slave |\n"
> -			"	   ipvlan | ipvtap | geneve | vrf | macsec | netdevsim | rmnet |\n"
> +			"	   ipvlan | ipvtap | geneve | bareudp | vrf | macsec | netdevsim | rmnet |\n"
>  			"	   xfrm }\n");
>  	}
>  	exit(-1);
> diff --git a/ip/iplink_bareudp.c b/ip/iplink_bareudp.c
> new file mode 100644
> index 00000000..885e1110
> --- /dev/null
> +++ b/ip/iplink_bareudp.c
> @@ -0,0 +1,150 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <stdio.h>
> +
> +#include "libnetlink.h"
> +#include "linux/if_ether.h"
> +#include "linux/if_link.h"
> +#include "linux/netlink.h"
> +#include "linux/rtnetlink.h"
> +#include "rt_names.h"
> +#include "utils.h"
> +#include "ip_common.h"
> +#include "json_print.h"
> +
> +#define BAREUDP_ATTRSET(attrs, type) (((attrs) & (1L << (type))) != 0)
> +
> +static void print_explain(FILE *f)
> +{
> +	fprintf(f,
> +		"Usage: ... bareudp dstport PORT\n"
> +		"		ethertype PROTO\n"
> +		"		[ srcportmin PORT ]\n"
> +		"		[ [no]multiproto ]\n"
> +		"\n"
> +		"Where:	PORT       := 0-65535\n"
> +		"	PROTO      := NUMBER | ip | mpls\n"
> +		"	SRCPORTMIN := 0-65535\n"
> +	);
> +}
> +
> +static void explain(void)
> +{
> +	print_explain(stderr);
> +}
> +
> +static void check_duparg(__u64 *attrs, int type, const char *key,
> +			 const char *argv)
> +{
> +	if (!BAREUDP_ATTRSET(*attrs, type)) {
> +		*attrs |= (1L << type);
> +		return;
> +	}
> +	duparg2(key, argv);
> +}
> +
> +static int bareudp_parse_opt(struct link_util *lu, int argc, char **argv,
> +			     struct nlmsghdr *n)
> +{
> +	bool multiproto = false;
> +	__u16 srcportmin = 0;
> +	__be16 ethertype = 0;
> +	__be16 dstport = 0;
> +	__u64 attrs = 0;
> +
> +	while (argc > 0) {
> +		if (matches(*argv, "dstport") == 0) {
> +			NEXT_ARG();
> +			check_duparg(&attrs, IFLA_BAREUDP_PORT, "dstport",
> +				     *argv);
> +			if (get_be16(&dstport, *argv, 0))
> +				invarg("dstport", *argv);
> +		} else if (matches(*argv, "ethertype") == 0)  {
> +			NEXT_ARG();
> +			check_duparg(&attrs, IFLA_BAREUDP_ETHERTYPE,
> +				     "ethertype", *argv);
> +			if (ll_proto_a2n(&ethertype, *argv))
Does this function takes care of mpls proto names ?

The original idea of bareudp is to allow even reserved ethertypes.Hence i think we
must take ethertype in hex aswell
> +				invarg("ethertype", *argv);
> +		} else if (matches(*argv, "srcportmin") == 0) {
> +			NEXT_ARG();
> +			check_duparg(&attrs, IFLA_BAREUDP_SRCPORT_MIN,
> +				     "srcportmin", *argv);
> +			if (get_u16(&srcportmin, *argv, 0))
> +				invarg("srcportmin", *argv);
> +		} else if (matches(*argv, "multiproto") == 0) {
> +			check_duparg(&attrs, IFLA_BAREUDP_MULTIPROTO_MODE,
> +				     *argv, *argv);
> +			multiproto = true;
> +		} else if (matches(*argv, "nomultiproto") == 0) {
do we need nomultiproto flag. Is it redundant
> +			check_duparg(&attrs, IFLA_BAREUDP_MULTIPROTO_MODE,
> +				     *argv, *argv);
> +			multiproto = false;
> +		} else if (matches(*argv, "help") == 0) {
> +			explain();
> +			return -1;
> +		} else {
> +			fprintf(stderr, "bareudp: unknown command \"%s\"?\n",
> +				*argv);
> +			explain();
> +			return -1;
> +		}
> +		argc--, argv++;
> +	}
> +
> +	if (!BAREUDP_ATTRSET(attrs, IFLA_BAREUDP_PORT))
> +		missarg("dstport");
> +	if (!BAREUDP_ATTRSET(attrs, IFLA_BAREUDP_ETHERTYPE))
> +		missarg("ethertype");
> +
> +	addattr16(n, 1024, IFLA_BAREUDP_PORT, dstport);
> +	addattr16(n, 1024, IFLA_BAREUDP_ETHERTYPE, ethertype);
> +	if (BAREUDP_ATTRSET(attrs, IFLA_BAREUDP_SRCPORT_MIN))
> +		addattr16(n, 1024, IFLA_BAREUDP_SRCPORT_MIN, srcportmin);
> +	if (multiproto)
> +		addattr(n, 1024, IFLA_BAREUDP_MULTIPROTO_MODE);
> +
> +	return 0;
> +}
> +
> +static void bareudp_print_opt(struct link_util *lu, FILE *f,
> +			      struct rtattr *tb[])
> +{
> +	if (!tb)
> +		return;
> +
> +	if (tb[IFLA_BAREUDP_PORT])
> +		print_uint(PRINT_ANY, "dstport", "dstport %u ",
> +			   rta_getattr_be16(tb[IFLA_BAREUDP_PORT]));
> +
> +	if (tb[IFLA_BAREUDP_ETHERTYPE]) {
> +		struct rtattr *attr = tb[IFLA_BAREUDP_ETHERTYPE];
> +		SPRINT_BUF(ethertype);
> +
> +		print_string(PRINT_ANY, "ethertype", "ethertype %s ",
> +			     ll_proto_n2a(rta_getattr_u16(attr),
> +					  ethertype, sizeof(ethertype)));
> +	}
> +
> +	if (tb[IFLA_BAREUDP_SRCPORT_MIN])
> +		print_uint(PRINT_ANY, "srcportmin", "srcportmin %u ",
> +			   rta_getattr_u16(tb[IFLA_BAREUDP_SRCPORT_MIN]));
> +
> +	if (tb[IFLA_BAREUDP_MULTIPROTO_MODE])
> +		print_bool(PRINT_ANY, "multiproto", "multiproto ", true);
> +	else
> +		print_bool(PRINT_ANY, "multiproto", "nomultiproto ", false);
Comments from stephen@networkplumber.org on the first version patch is given below

One of the unwritten rules of ip commands is that the show format
should match the command line arguments.  In this case extmode is
really a presence flag not a boolean. best to print that with
json null command.

Thanks
Martin

> +}
> +
> +static void bareudp_print_help(struct link_util *lu, int argc, char **argv,
> +			       FILE *f)
> +{
> +	print_explain(f);
> +}
> +
> +struct link_util bareudp_link_util = {
> +	.id		= "bareudp",
> +	.maxattr	= IFLA_BAREUDP_MAX,
> +	.parse_opt	= bareudp_parse_opt,
> +	.print_opt	= bareudp_print_opt,
> +	.print_help	= bareudp_print_help,
> +};
> diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
> index e8a25451..c6bd2c53 100644
> --- a/man/man8/ip-link.8.in
> +++ b/man/man8/ip-link.8.in
> @@ -223,6 +223,7 @@ ip-link \- network device configuration
>  .BR ipvtap " |"
>  .BR lowpan " |"
>  .BR geneve " |"
> +.BR bareudp " |"
>  .BR vrf " |"
>  .BR macsec " |"
>  .BR netdevsim " |"
> @@ -356,6 +357,9 @@ Link types:
>  .BR geneve
>  - GEneric NEtwork Virtualization Encapsulation
>  .sp
> +.BR bareudp
> +- Bare UDP L3 encapsulation support
> +.sp
>  .BR macsec
>  - Interface for IEEE 802.1AE MAC Security (MACsec)
>  .sp
> @@ -1293,6 +1297,46 @@ options.
>  
>  .in -8
>  
> +.TP
> +Bareudp Type Support
> +For a link of type
> +.I Bareudp
> +the following additional arguments are supported:
> +
> +.BI "ip link add " DEVICE
> +.BI type " bareudp " dstport " PORT " ethertype " ETHERTYPE"
> +[
> +.BI srcportmin " SRCPORTMIN "
> +] [
> +.RB [ no ] multiproto
> +]
> +
> +.in +8
> +.sp
> +.BI dstport " PORT"
> +- specifies the destination port for the UDP tunnel.
> +
> +.sp
> +.BI ethertype " ETHERTYPE"
> +- specifies the ethertype of the L3 protocol being tunnelled.
> +
> +.sp
> +.BI srcportmin " SRCPORTMIN"
> +- selects the lowest value of the UDP tunnel source port range.
> +
> +.sp
> +.RB [ no ] multiproto
> +- activates support for protocols similar to the one
> +.RB "specified by " ethertype .
> +When
> +.I ETHERTYPE
> +is "mpls_uc" (that is, unicast MPLS), this allows the tunnel to also handle
> +multicast MPLS.
> +When
> +.I ETHERTYPE
> +is "ipv4", this allows the tunnel to also handle IPv6. This option is disabled
> +by default.
> +
>  .TP
>  MACVLAN and MACVTAP Type Support
>  For a link of type
> -- 
> 2.21.3
> 

Re: [PATCH iproute2] ip link: initial support for bareudp devicesy

[Guillaume Nault]

On Thu, Jul 02, 2020 at 02:45:39PM +0530, Martin Varghese wrote:
> On Wed, Jul 01, 2020 at 09:45:04PM +0200, Guillaume Nault wrote:
> > +		} else if (matches(*argv, "ethertype") == 0)  {
> > +			NEXT_ARG();
> > +			check_duparg(&attrs, IFLA_BAREUDP_ETHERTYPE,
> > +				     "ethertype", *argv);
> > +			if (ll_proto_a2n(&ethertype, *argv))
> Does this function takes care of mpls proto names ?
> 
> The original idea of bareudp is to allow even reserved ethertypes.Hence i think we
> must take ethertype in hex aswell

ll_proto_a2n() handles both symbolic and numeric ethertypes.

> > +				invarg("ethertype", *argv);
> > +		} else if (matches(*argv, "srcportmin") == 0) {
> > +			NEXT_ARG();
> > +			check_duparg(&attrs, IFLA_BAREUDP_SRCPORT_MIN,
> > +				     "srcportmin", *argv);
> > +			if (get_u16(&srcportmin, *argv, 0))
> > +				invarg("srcportmin", *argv);
> > +		} else if (matches(*argv, "multiproto") == 0) {
> > +			check_duparg(&attrs, IFLA_BAREUDP_MULTIPROTO_MODE,
> > +				     *argv, *argv);
> > +			multiproto = true;
> > +		} else if (matches(*argv, "nomultiproto") == 0) {
> do we need nomultiproto flag. Is it redundant

It allows users to exlicitely disable multiproto without having to rely
on default values. Also nomultiproto appears in the detailed output, so
it should be usable as input.

> > +	if (tb[IFLA_BAREUDP_MULTIPROTO_MODE])
> > +		print_bool(PRINT_ANY, "multiproto", "multiproto ", true);
> > +	else
> > +		print_bool(PRINT_ANY, "multiproto", "nomultiproto ", false);
> Comments from stephen@networkplumber.org on the first version patch is given below
> 
> One of the unwritten rules of ip commands is that the show format
> should match the command line arguments.  In this case extmode is
> really a presence flag not a boolean. best to print that with
> json null command.

The detailed output prints either "multiproto" or "nomultiproto". Both
keywords are accepted as configuration input. I can't see any deviation
from the unwritten rule here.

Re: [PATCH iproute2] ip link: initial support for bareudp devices

[Martin Varghese]

On Wed, Jul 01, 2020 at 09:45:04PM +0200, Guillaume Nault wrote:
> Bareudp devices provide a generic L3 encapsulation for tunnelling
> different protocols like MPLS, IP, NSH, etc. inside a UDP tunnel.
> 
> This patch is based on original work from Martin Varghese:
> https://lore.kernel.org/netdev/1570532361-15163-1-git-send-email-martinvarghesenokia@gmail.com/
> 
> Examples:
> 
>   - ip link add dev bareudp0 type bareudp dstport 6635 ethertype mpls_uc
> 
> This creates a bareudp tunnel device which tunnels L3 traffic with
> ethertype 0x8847 (unicast MPLS traffic). The destination port of the
> UDP header will be set to 6635. The device will listen on UDP port 6635
> to receive traffic.
> 
>   - ip link add dev bareudp0 type bareudp dstport 6635 ethertype ipv4 multiproto
> 
> Same as the MPLS example, but for IPv4. The "multiproto" keyword allows
> the device to also tunnel IPv6 traffic.
> 
> Signed-off-by: Guillaume Nault <gnault@redhat.com>
> ---
>  ip/Makefile           |   2 +-
>  ip/iplink.c           |   2 +-
>  ip/iplink_bareudp.c   | 150 ++++++++++++++++++++++++++++++++++++++++++
>  man/man8/ip-link.8.in |  44 +++++++++++++
>  4 files changed, 196 insertions(+), 2 deletions(-)
>  create mode 100644 ip/iplink_bareudp.c
> 
> diff --git a/ip/Makefile b/ip/Makefile
> index 8735b8e4..4cad619c 100644
> --- a/ip/Makefile
> +++ b/ip/Makefile
> @@ -11,7 +11,7 @@ IPOBJ=ip.o ipaddress.o ipaddrlabel.o iproute.o iprule.o ipnetns.o \
>      iplink_bridge.o iplink_bridge_slave.o ipfou.o iplink_ipvlan.o \
>      iplink_geneve.o iplink_vrf.o iproute_lwtunnel.o ipmacsec.o ipila.o \
>      ipvrf.o iplink_xstats.o ipseg6.o iplink_netdevsim.o iplink_rmnet.o \
> -    ipnexthop.o ipmptcp.o
> +    ipnexthop.o ipmptcp.o iplink_bareudp.o
>  
>  RTMONOBJ=rtmon.o
>  
> diff --git a/ip/iplink.c b/ip/iplink.c
> index 47f73988..7d4b244d 100644
> --- a/ip/iplink.c
> +++ b/ip/iplink.c
> @@ -124,7 +124,7 @@ void iplink_usage(void)
>  			"	   bridge | bond | team | ipoib | ip6tnl | ipip | sit | vxlan |\n"
>  			"	   gre | gretap | erspan | ip6gre | ip6gretap | ip6erspan |\n"
>  			"	   vti | nlmon | team_slave | bond_slave | bridge_slave |\n"
> -			"	   ipvlan | ipvtap | geneve | vrf | macsec | netdevsim | rmnet |\n"
> +			"	   ipvlan | ipvtap | geneve | bareudp | vrf | macsec | netdevsim | rmnet |\n"
>  			"	   xfrm }\n");
>  	}
>  	exit(-1);
> diff --git a/ip/iplink_bareudp.c b/ip/iplink_bareudp.c
> new file mode 100644
> index 00000000..885e1110
> --- /dev/null
> +++ b/ip/iplink_bareudp.c
> @@ -0,0 +1,150 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <stdio.h>
> +
> +#include "libnetlink.h"
> +#include "linux/if_ether.h"
> +#include "linux/if_link.h"
> +#include "linux/netlink.h"
> +#include "linux/rtnetlink.h"
> +#include "rt_names.h"
> +#include "utils.h"
> +#include "ip_common.h"
> +#include "json_print.h"
> +
> +#define BAREUDP_ATTRSET(attrs, type) (((attrs) & (1L << (type))) != 0)
> +
> +static void print_explain(FILE *f)
> +{
> +	fprintf(f,
> +		"Usage: ... bareudp dstport PORT\n"
> +		"		ethertype PROTO\n"
> +		"		[ srcportmin PORT ]\n"
> +		"		[ [no]multiproto ]\n"
> +		"\n"
> +		"Where:	PORT       := 0-65535\n"
> +		"	PROTO      := NUMBER | ip | mpls\n"
> +		"	SRCPORTMIN := 0-65535\n"
> +	);
> +}
> +
> +static void explain(void)
> +{
> +	print_explain(stderr);
> +}
> +
> +static void check_duparg(__u64 *attrs, int type, const char *key,
> +			 const char *argv)
> +{
> +	if (!BAREUDP_ATTRSET(*attrs, type)) {
> +		*attrs |= (1L << type);
> +		return;
> +	}
> +	duparg2(key, argv);
> +}
> +
> +static int bareudp_parse_opt(struct link_util *lu, int argc, char **argv,
> +			     struct nlmsghdr *n)
> +{
> +	bool multiproto = false;
> +	__u16 srcportmin = 0;
> +	__be16 ethertype = 0;
> +	__be16 dstport = 0;
> +	__u64 attrs = 0;
> +
> +	while (argc > 0) {
> +		if (matches(*argv, "dstport") == 0) {
> +			NEXT_ARG();
> +			check_duparg(&attrs, IFLA_BAREUDP_PORT, "dstport",
> +				     *argv);
> +			if (get_be16(&dstport, *argv, 0))
> +				invarg("dstport", *argv);
> +		} else if (matches(*argv, "ethertype") == 0)  {
> +			NEXT_ARG();
> +			check_duparg(&attrs, IFLA_BAREUDP_ETHERTYPE,
> +				     "ethertype", *argv);
> +			if (ll_proto_a2n(&ethertype, *argv))
> +				invarg("ethertype", *argv);
> +		} else if (matches(*argv, "srcportmin") == 0) {
> +			NEXT_ARG();
> +			check_duparg(&attrs, IFLA_BAREUDP_SRCPORT_MIN,
> +				     "srcportmin", *argv);
> +			if (get_u16(&srcportmin, *argv, 0))
> +				invarg("srcportmin", *argv);
> +		} else if (matches(*argv, "multiproto") == 0) {
> +			check_duparg(&attrs, IFLA_BAREUDP_MULTIPROTO_MODE,
> +				     *argv, *argv);
> +			multiproto = true;
> +		} else if (matches(*argv, "nomultiproto") == 0) {
> +			check_duparg(&attrs, IFLA_BAREUDP_MULTIPROTO_MODE,
> +				     *argv, *argv);
> +			multiproto = false;
> +		} else if (matches(*argv, "help") == 0) {
> +			explain();
> +			return -1;
> +		} else {
> +			fprintf(stderr, "bareudp: unknown command \"%s\"?\n",
> +				*argv);
> +			explain();
> +			return -1;
> +		}
> +		argc--, argv++;
> +	}
> +
> +	if (!BAREUDP_ATTRSET(attrs, IFLA_BAREUDP_PORT))
> +		missarg("dstport");
> +	if (!BAREUDP_ATTRSET(attrs, IFLA_BAREUDP_ETHERTYPE))
> +		missarg("ethertype");
> +
> +	addattr16(n, 1024, IFLA_BAREUDP_PORT, dstport);
> +	addattr16(n, 1024, IFLA_BAREUDP_ETHERTYPE, ethertype);
> +	if (BAREUDP_ATTRSET(attrs, IFLA_BAREUDP_SRCPORT_MIN))
> +		addattr16(n, 1024, IFLA_BAREUDP_SRCPORT_MIN, srcportmin);
> +	if (multiproto)
> +		addattr(n, 1024, IFLA_BAREUDP_MULTIPROTO_MODE);
> +
> +	return 0;
> +}
> +
> +static void bareudp_print_opt(struct link_util *lu, FILE *f,
> +			      struct rtattr *tb[])
> +{
> +	if (!tb)
> +		return;
> +
> +	if (tb[IFLA_BAREUDP_PORT])
> +		print_uint(PRINT_ANY, "dstport", "dstport %u ",
> +			   rta_getattr_be16(tb[IFLA_BAREUDP_PORT]));
> +
> +	if (tb[IFLA_BAREUDP_ETHERTYPE]) {
> +		struct rtattr *attr = tb[IFLA_BAREUDP_ETHERTYPE];
> +		SPRINT_BUF(ethertype);
> +
> +		print_string(PRINT_ANY, "ethertype", "ethertype %s ",
> +			     ll_proto_n2a(rta_getattr_u16(attr),
> +					  ethertype, sizeof(ethertype)));
> +	}
> +
> +	if (tb[IFLA_BAREUDP_SRCPORT_MIN])
> +		print_uint(PRINT_ANY, "srcportmin", "srcportmin %u ",
> +			   rta_getattr_u16(tb[IFLA_BAREUDP_SRCPORT_MIN]));
> +
> +	if (tb[IFLA_BAREUDP_MULTIPROTO_MODE])
> +		print_bool(PRINT_ANY, "multiproto", "multiproto ", true);
> +	else
> +		print_bool(PRINT_ANY, "multiproto", "nomultiproto ", false);
> +}
> +
> +static void bareudp_print_help(struct link_util *lu, int argc, char **argv,
> +			       FILE *f)
> +{
> +	print_explain(f);
> +}
> +
> +struct link_util bareudp_link_util = {
> +	.id		= "bareudp",
> +	.maxattr	= IFLA_BAREUDP_MAX,
> +	.parse_opt	= bareudp_parse_opt,
> +	.print_opt	= bareudp_print_opt,
> +	.print_help	= bareudp_print_help,
> +};
> diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
> index e8a25451..c6bd2c53 100644
> --- a/man/man8/ip-link.8.in
> +++ b/man/man8/ip-link.8.in
> @@ -223,6 +223,7 @@ ip-link \- network device configuration
>  .BR ipvtap " |"
>  .BR lowpan " |"
>  .BR geneve " |"
> +.BR bareudp " |"
>  .BR vrf " |"
>  .BR macsec " |"
>  .BR netdevsim " |"
> @@ -356,6 +357,9 @@ Link types:
>  .BR geneve
>  - GEneric NEtwork Virtualization Encapsulation
>  .sp
> +.BR bareudp
> +- Bare UDP L3 encapsulation support
> +.sp
>  .BR macsec
>  - Interface for IEEE 802.1AE MAC Security (MACsec)
>  .sp
> @@ -1293,6 +1297,46 @@ options.
>  
>  .in -8
>  
> +.TP
> +Bareudp Type Support
> +For a link of type
> +.I Bareudp
> +the following additional arguments are supported:
> +
> +.BI "ip link add " DEVICE
> +.BI type " bareudp " dstport " PORT " ethertype " ETHERTYPE"
> +[
> +.BI srcportmin " SRCPORTMIN "
> +] [
> +.RB [ no ] multiproto
> +]
> +
> +.in +8
> +.sp
> +.BI dstport " PORT"
> +- specifies the destination port for the UDP tunnel.
> +
> +.sp
> +.BI ethertype " ETHERTYPE"
> +- specifies the ethertype of the L3 protocol being tunnelled.
> +
> +.sp
> +.BI srcportmin " SRCPORTMIN"
> +- selects the lowest value of the UDP tunnel source port range.
> +
> +.sp
> +.RB [ no ] multiproto
> +- activates support for protocols similar to the one
> +.RB "specified by " ethertype .
> +When
> +.I ETHERTYPE
> +is "mpls_uc" (that is, unicast MPLS), this allows the tunnel to also handle
> +multicast MPLS.
> +When
> +.I ETHERTYPE
> +is "ipv4", this allows the tunnel to also handle IPv6. This option is disabled
> +by default.
> +
>  .TP
>  MACVLAN and MACVTAP Type Support
>  For a link of type
> -- 


I couldnt apply the patch to master.Could you please confirm if it was rebased to the master
> 2.21.3
> 

Re: [PATCH iproute2] ip link: initial support for bareudp devices

[Guillaume Nault]

On Thu, Jul 02, 2020 at 09:32:21PM +0530, Martin Varghese wrote:
> On Wed, Jul 01, 2020 at 09:45:04PM +0200, Guillaume Nault wrote:
> 
> I couldnt apply the patch to master.Could you please confirm if it was rebased to the master

Hi Martin,

Yes, it's based on the latest iproute2 master branch.
I can apply it with "git am" without any problem.

Which command did you use?

Re: [PATCH iproute2] ip link: initial support for bareudp devices

[Martin Varghese]

On Thu, Jul 02, 2020 at 07:03:53PM +0200, Guillaume Nault wrote:
> On Thu, Jul 02, 2020 at 09:32:21PM +0530, Martin Varghese wrote:
> > On Wed, Jul 01, 2020 at 09:45:04PM +0200, Guillaume Nault wrote:
> > 
> > I couldnt apply the patch to master.Could you please confirm if it was rebased to the master
> 
> Hi Martin,
> 
> Yes, it's based on the latest iproute2 master branch.
> I can apply it with "git am" without any problem.
> 
> Which command did you use?
>
git apply --check was failing.I assume my git remote repo is not correct.Could you please paste yours.
I can do one round of testing and give you feedback

Thanks
Martin

Re: [PATCH iproute2] ip link: initial support for bareudp devices

[Guillaume Nault]

On Thu, Jul 02, 2020 at 10:41:03PM +0530, Martin Varghese wrote:
> On Thu, Jul 02, 2020 at 07:03:53PM +0200, Guillaume Nault wrote:
> > On Thu, Jul 02, 2020 at 09:32:21PM +0530, Martin Varghese wrote:
> > > On Wed, Jul 01, 2020 at 09:45:04PM +0200, Guillaume Nault wrote:
> > > 
> > > I couldnt apply the patch to master.Could you please confirm if it was rebased to the master
> > 
> > Hi Martin,
> > 
> > Yes, it's based on the latest iproute2 master branch.
> > I can apply it with "git am" without any problem.
> > 
> > Which command did you use?
> >
> git apply --check was failing.I assume my git remote repo is not correct.Could you please paste yours.

Here it is:
$ git remote -v
origin  https://git.kernel.org/pub/scm/network/iproute2/iproute2.git (fetch)
origin  https://git.kernel.org/pub/scm/network/iproute2/iproute2.git (push)

> I can do one round of testing and give you feedback

Nice, thanks!

Re: [PATCH iproute2] ip link: initial support for bareudp devicesy

[Martin Varghese]

On Thu, Jul 02, 2020 at 11:57:46AM +0200, Guillaume Nault wrote:
> On Thu, Jul 02, 2020 at 02:45:39PM +0530, Martin Varghese wrote:
> > On Wed, Jul 01, 2020 at 09:45:04PM +0200, Guillaume Nault wrote:
> > > +		} else if (matches(*argv, "ethertype") == 0)  {
> > > +			NEXT_ARG();
> > > +			check_duparg(&attrs, IFLA_BAREUDP_ETHERTYPE,
> > > +				     "ethertype", *argv);
> > > +			if (ll_proto_a2n(&ethertype, *argv))
> > Does this function takes care of mpls proto names ?
> > 
> > The original idea of bareudp is to allow even reserved ethertypes.Hence i think we
> > must take ethertype in hex aswell
> 
> ll_proto_a2n() handles both symbolic and numeric ethertypes.
>
Yes it works. 
> > > +				invarg("ethertype", *argv);
> > > +		} else if (matches(*argv, "srcportmin") == 0) {
> > > +			NEXT_ARG();
> > > +			check_duparg(&attrs, IFLA_BAREUDP_SRCPORT_MIN,
> > > +				     "srcportmin", *argv);
> > > +			if (get_u16(&srcportmin, *argv, 0))
> > > +				invarg("srcportmin", *argv);
> > > +		} else if (matches(*argv, "multiproto") == 0) {
> > > +			check_duparg(&attrs, IFLA_BAREUDP_MULTIPROTO_MODE,
> > > +				     *argv, *argv);
> > > +			multiproto = true;
> > > +		} else if (matches(*argv, "nomultiproto") == 0) {
> > do we need nomultiproto flag. Is it redundant
> 
> It allows users to exlicitely disable multiproto without having to rely
> on default values. Also nomultiproto appears in the detailed output, so
> it should be usable as input.
> 
ok
> > > +	if (tb[IFLA_BAREUDP_MULTIPROTO_MODE])
> > > +		print_bool(PRINT_ANY, "multiproto", "multiproto ", true);
> > > +	else
> > > +		print_bool(PRINT_ANY, "multiproto", "nomultiproto ", false);
> > Comments from stephen@networkplumber.org on the first version patch is given below
> > 
> > One of the unwritten rules of ip commands is that the show format
> > should match the command line arguments.  In this case extmode is
> > really a presence flag not a boolean. best to print that with
> > json null command.
> 
> The detailed output prints either "multiproto" or "nomultiproto". Both
> keywords are accepted as configuration input. I can't see any deviation
> from the unwritten rule here.
> 
ok

Re: [PATCH iproute2] ip link: initial support for bareudp devices

[Martin Varghese]

On Thu, Jul 02, 2020 at 07:32:31PM +0200, Guillaume Nault wrote:
> On Thu, Jul 02, 2020 at 10:41:03PM +0530, Martin Varghese wrote:
> > On Thu, Jul 02, 2020 at 07:03:53PM +0200, Guillaume Nault wrote:
> > > On Thu, Jul 02, 2020 at 09:32:21PM +0530, Martin Varghese wrote:
> > > > On Wed, Jul 01, 2020 at 09:45:04PM +0200, Guillaume Nault wrote:
> > > > 
> > > > I couldnt apply the patch to master.Could you please confirm if it was rebased to the master
> > > 
> > > Hi Martin,
> > > 
> > > Yes, it's based on the latest iproute2 master branch.
> > > I can apply it with "git am" without any problem.
> > > 
> > > Which command did you use?
> > >
> > git apply --check was failing.I assume my git remote repo is not correct.Could you please paste yours.
> 
> Here it is:
> $ git remote -v
> origin  https://git.kernel.org/pub/scm/network/iproute2/iproute2.git (fetch)
> origin  https://git.kernel.org/pub/scm/network/iproute2/iproute2.git (push)
> 
> > I can do one round of testing and give you feedback
> 
> Nice, thanks!
> 
I did one round of testing.It looks good to me. Thanks 

Re: [PATCH iproute2] ip link: initial support for bareudp devices

[Stephen Hemminger]

On Wed, 1 Jul 2020 21:45:04 +0200
Guillaume Nault <gnault@redhat.com> wrote:

> Bareudp devices provide a generic L3 encapsulation for tunnelling
> different protocols like MPLS, IP, NSH, etc. inside a UDP tunnel.
> 
> This patch is based on original work from Martin Varghese:
> https://lore.kernel.org/netdev/1570532361-15163-1-git-send-email-martinvarghesenokia@gmail.com/
> 
> Examples:
> 
>   - ip link add dev bareudp0 type bareudp dstport 6635 ethertype mpls_uc
> 
> This creates a bareudp tunnel device which tunnels L3 traffic with
> ethertype 0x8847 (unicast MPLS traffic). The destination port of the
> UDP header will be set to 6635. The device will listen on UDP port 6635
> to receive traffic.
> 
>   - ip link add dev bareudp0 type bareudp dstport 6635 ethertype ipv4 multiproto
> 
> Same as the MPLS example, but for IPv4. The "multiproto" keyword allows
> the device to also tunnel IPv6 traffic.
> 
> Signed-off-by: Guillaume Nault <gnault@redhat.com>

Applied. 
Would it be possible to add a test for this in the iproute2 testsuite?

Re: [PATCH iproute2] ip link: initial support for bareudp devices

[Guillaume Nault]

On Mon, Jul 06, 2020 at 11:11:09AM -0700, Stephen Hemminger wrote:
> On Wed, 1 Jul 2020 21:45:04 +0200
> Guillaume Nault <gnault@redhat.com> wrote:
> 
> > Bareudp devices provide a generic L3 encapsulation for tunnelling
> > different protocols like MPLS, IP, NSH, etc. inside a UDP tunnel.
> > 
> > This patch is based on original work from Martin Varghese:
> > https://lore.kernel.org/netdev/1570532361-15163-1-git-send-email-martinvarghesenokia@gmail.com/
> > 
> > Examples:
> > 
> >   - ip link add dev bareudp0 type bareudp dstport 6635 ethertype mpls_uc
> > 
> > This creates a bareudp tunnel device which tunnels L3 traffic with
> > ethertype 0x8847 (unicast MPLS traffic). The destination port of the
> > UDP header will be set to 6635. The device will listen on UDP port 6635
> > to receive traffic.
> > 
> >   - ip link add dev bareudp0 type bareudp dstport 6635 ethertype ipv4 multiproto
> > 
> > Same as the MPLS example, but for IPv4. The "multiproto" keyword allows
> > the device to also tunnel IPv6 traffic.
> > 
> > Signed-off-by: Guillaume Nault <gnault@redhat.com>
> 
> Applied. 
> Would it be possible to add a test for this in the iproute2 testsuite?

No problem, I'm working on it.