[PATCH 1/1] ip: add rmnet initial support

[PATCH 1/1] ip: add rmnet initial support

[Daniele Palmas]

This patch adds basic support for Qualcomm rmnet devices.

Signed-off-by: Daniele Palmas <dnlplm@gmail.com>
---
 ip/Makefile       |  2 +-
 ip/iplink.c       |  2 +-
 ip/iplink_rmnet.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+), 2 deletions(-)
 create mode 100644 ip/iplink_rmnet.c

diff --git a/ip/Makefile b/ip/Makefile
index 77fadee..a88f936 100644
--- a/ip/Makefile
+++ b/ip/Makefile
@@ -10,7 +10,7 @@ IPOBJ=ip.o ipaddress.o ipaddrlabel.o iproute.o iprule.o ipnetns.o \
     link_iptnl.o link_gre6.o iplink_bond.o iplink_bond_slave.o iplink_hsr.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
+    ipvrf.o iplink_xstats.o ipseg6.o iplink_netdevsim.o iplink_rmnet.o
 
 RTMONOBJ=rtmon.o
 
diff --git a/ip/iplink.c b/ip/iplink.c
index e4d4da9..f0f8fb8 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -121,7 +121,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 | ipvlan | geneve |\n"
-			"          bridge_slave | vrf | macsec | netdevsim }\n");
+			"          bridge_slave | vrf | macsec | netdevsim | rmnet}\n");
 	}
 	exit(-1);
 }
diff --git a/ip/iplink_rmnet.c b/ip/iplink_rmnet.c
new file mode 100644
index 0000000..2367754
--- /dev/null
+++ b/ip/iplink_rmnet.c
@@ -0,0 +1,70 @@
+/*
+ * iplink_rmnet.c	RMNET device support
+ *
+ *              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.
+ *
+ * Authors:     Daniele Palmas <dnlplm@gmail.com>
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "rt_names.h"
+#include "utils.h"
+#include "ip_common.h"
+
+static void print_explain(FILE *f)
+{
+	fprintf(f,
+		"Usage: ... rmnet mux_id MUXID\n"
+		"\n"
+		"MUXID := 1-127\n"
+	);
+}
+
+static void explain(void)
+{
+	print_explain(stderr);
+}
+
+static int rmnet_parse_opt(struct link_util *lu, int argc, char **argv,
+			   struct nlmsghdr *n)
+{
+	__u16 mux_id;
+
+	while (argc > 0) {
+		if (matches(*argv, "mux_id") == 0) {
+			NEXT_ARG();
+			if (get_u16(&mux_id, *argv, 0))
+				invarg("mux_id is invalid", *argv);
+			addattr_l(n, 1024, IFLA_RMNET_MUX_ID, &mux_id, 2);
+		} else if (matches(*argv, "help") == 0) {
+			explain();
+			return -1;
+		} else {
+			fprintf(stderr, "rmnet: unknown command \"%s\"?\n", *argv);
+			explain();
+			return -1;
+		}
+		argc--, argv++;
+	}
+
+	return 0;
+}
+
+static void rmnet_print_help(struct link_util *lu, int argc, char **argv,
+			     FILE *f)
+{
+	print_explain(f);
+}
+
+struct link_util rmnet_link_util = {
+	.id		= "rmnet",
+	.maxattr	= IFLA_RMNET_MAX,
+	.parse_opt	= rmnet_parse_opt,
+	.print_help	= rmnet_print_help,
+};
-- 
2.7.4

Re: [PATCH 1/1] ip: add rmnet initial support

[Subash Abhinov Kasiviswanathan]

> +
> +static void print_explain(FILE *f)
> +{
> +	fprintf(f,
> +		"Usage: ... rmnet mux_id MUXID\n"
> +		"\n"
> +		"MUXID := 1-127\n"
> +	);
> +}

Hi Daniele

This range can be from 1-254.

> +
> +static void explain(void)
> +{
> +	print_explain(stderr);
> +}
> +
> +static int rmnet_parse_opt(struct link_util *lu, int argc, char 
> **argv,
> +			   struct nlmsghdr *n)
> +{
> +	__u16 mux_id;
> +
> +	while (argc > 0) {
> +		if (matches(*argv, "mux_id") == 0) {
> +			NEXT_ARG();
> +			if (get_u16(&mux_id, *argv, 0))
> +				invarg("mux_id is invalid", *argv);
> +			addattr_l(n, 1024, IFLA_RMNET_MUX_ID, &mux_id, 2);

You could use addattr16() instead since it is __u16.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/1] ip: add rmnet initial support

[Stephen Hemminger]

On Tue, 12 Jun 2018 16:12:57 +0200
Daniele Palmas <dnlplm@gmail.com> wrote:

> This patch adds basic support for Qualcomm rmnet devices.
> 
> Signed-off-by: Daniele Palmas <dnlplm@gmail.com>
> ---
>  ip/Makefile       |  2 +-
>  ip/iplink.c       |  2 +-
>  ip/iplink_rmnet.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 72 insertions(+), 2 deletions(-)
>  create mode 100644 ip/iplink_rmnet.c
};

I am glad to see integrated tool support, but this needs to be targeted at
the iproute2-next since it is a new feature.

Some things that I would like to see changed:
  1. All of iproute2 is now using SPDX license identifiers, you should not
     include GPL boilerplate
  2. You should provide dump (print_opt) as well as parse routine.
     Output format should use the print_uint (json print) routines.
  3. Please update manual page (man/man8/ip-link.8.in) to include the new
     option.

Re: [PATCH 1/1] ip: add rmnet initial support

[Daniele Palmas]

2018-06-13 1:06 GMT+02:00 Subash Abhinov Kasiviswanathan
<subashab@codeaurora.org>:
>> +
>> +static void print_explain(FILE *f)
>> +{
>> +       fprintf(f,
>> +               "Usage: ... rmnet mux_id MUXID\n"
>> +               "\n"
>> +               "MUXID := 1-127\n"
>> +       );
>> +}
>
>
> Hi Daniele
>
> This range can be from 1-254.
>
>> +
>> +static void explain(void)
>> +{
>> +       print_explain(stderr);
>> +}
>> +
>> +static int rmnet_parse_opt(struct link_util *lu, int argc, char **argv,
>> +                          struct nlmsghdr *n)
>> +{
>> +       __u16 mux_id;
>> +
>> +       while (argc > 0) {
>> +               if (matches(*argv, "mux_id") == 0) {
>> +                       NEXT_ARG();
>> +                       if (get_u16(&mux_id, *argv, 0))
>> +                               invarg("mux_id is invalid", *argv);
>> +                       addattr_l(n, 1024, IFLA_RMNET_MUX_ID, &mux_id, 2);
>
>
> You could use addattr16() instead since it is __u16.
>

Thanks Subash, I'll fix those in v2.

Regards,
Daniele

> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

Re: [PATCH 1/1] ip: add rmnet initial support

[Daniele Palmas]

2018-06-13 2:22 GMT+02:00 Stephen Hemminger <stephen@networkplumber.org>:
> On Tue, 12 Jun 2018 16:12:57 +0200
> Daniele Palmas <dnlplm@gmail.com> wrote:
>
>> This patch adds basic support for Qualcomm rmnet devices.
>>
>> Signed-off-by: Daniele Palmas <dnlplm@gmail.com>
>> ---
>>  ip/Makefile       |  2 +-
>>  ip/iplink.c       |  2 +-
>>  ip/iplink_rmnet.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 72 insertions(+), 2 deletions(-)
>>  create mode 100644 ip/iplink_rmnet.c
> };
>
> I am glad to see integrated tool support, but this needs to be targeted at
> the iproute2-next since it is a new feature.
>
> Some things that I would like to see changed:
>   1. All of iproute2 is now using SPDX license identifiers, you should not
>      include GPL boilerplate
>   2. You should provide dump (print_opt) as well as parse routine.
>      Output format should use the print_uint (json print) routines.
>   3. Please update manual page (man/man8/ip-link.8.in) to include the new
>      option.
>

Thanks for the review Stephen, I'll submit a v2 for fixing those issues.

Regards,
Daniele