All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iproute2-next] ip link: add support to display extended tun attributes
@ 2018-02-16 10:03 Sabrina Dubroca
  2018-02-19 22:19 ` Serhey Popovych
  0 siblings, 1 reply; 4+ messages in thread
From: Sabrina Dubroca @ 2018-02-16 10:03 UTC (permalink / raw)
  To: netdev; +Cc: dsahern, sbrivio, Sabrina Dubroca

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
---

I included the uapi changes from the kernel patch. Let me know if I
need to resend without the header changes.

 include/uapi/linux/if_link.h | 18 ++++++++++
 ip/iptuntap.c                | 85 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 1726e49fbc6b..e2af6588b50d 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -936,4 +936,22 @@ enum {
 	IFLA_EVENT_BONDING_OPTIONS,	/* change in bonding options */
 };
 
+/* tun section */
+
+enum {
+	IFLA_TUN_UNSPEC,
+	IFLA_TUN_OWNER,
+	IFLA_TUN_GROUP,
+	IFLA_TUN_TYPE,
+	IFLA_TUN_PI,
+	IFLA_TUN_VNET_HDR,
+	IFLA_TUN_PERSIST,
+	IFLA_TUN_MULTI_QUEUE,
+	IFLA_TUN_NUM_QUEUES,
+	IFLA_TUN_NUM_DISABLED_QUEUES,
+	__IFLA_TUN_MAX,
+};
+
+#define IFLA_TUN_MAX (__IFLA_TUN_MAX - 1)
+
 #endif /* _LINUX_IF_LINK_H */
diff --git a/ip/iptuntap.c b/ip/iptuntap.c
index 4628db2832b4..520244c31b89 100644
--- a/ip/iptuntap.c
+++ b/ip/iptuntap.c
@@ -469,3 +469,88 @@ int do_iptuntap(int argc, char **argv)
 		*argv);
 	exit(-1);
 }
+
+static void print_owner(FILE *f, uid_t uid)
+{
+	struct passwd *pw = getpwuid(uid);
+
+	if (pw)
+		fprintf(f, "user %s ", pw->pw_name);
+	else
+		fprintf(f, "user %u ", uid);
+}
+
+static void print_group(FILE *f, gid_t gid)
+{
+	struct group *group = getgrgid(gid);
+
+	if (group)
+		fprintf(f, "group %s ", group->gr_name);
+	else
+		fprintf(f, "group %u ", gid);
+}
+
+static void print_mq(FILE *f, struct rtattr *tb[])
+{
+	if (tb[IFLA_TUN_MULTI_QUEUE]) {
+		__u8 mq = rta_getattr_u8(tb[IFLA_TUN_MULTI_QUEUE]);
+		if (!mq)
+			return;
+		fprintf(f, "multi_queue ");
+	}
+
+	if (tb[IFLA_TUN_NUM_QUEUES]) {
+		__u32 numq = rta_getattr_u32(tb[IFLA_TUN_NUM_QUEUES]);
+		fprintf(f, "numqueues %u ", numq);
+	}
+
+	if (tb[IFLA_TUN_NUM_DISABLED_QUEUES]) {
+		__u32 numq = rta_getattr_u32(tb[IFLA_TUN_NUM_DISABLED_QUEUES]);
+		fprintf(f, "numdisabled %u ", numq);
+	}
+}
+
+static void tun_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
+{
+	if (!tb)
+		return;
+
+	if (tb[IFLA_TUN_TYPE]) {
+		__u8 type = rta_getattr_u8(tb[IFLA_TUN_TYPE]);
+		if (type == IFF_TUN)
+			fprintf(f, "type tun ");
+		else if (type == IFF_TAP)
+			fprintf(f, "type tap ");
+		else
+			fprintf(f, "type UNKNOWN:%hhu ", type);
+	}
+
+	if (tb[IFLA_TUN_PI]) {
+		__u8 pi = rta_getattr_u8(tb[IFLA_TUN_PI]);
+		fprintf(f, "pi %s ", pi ? "on" : "off");
+	}
+
+	if (tb[IFLA_TUN_VNET_HDR]) {
+		__u8 vnet = rta_getattr_u8(tb[IFLA_TUN_VNET_HDR]);
+		fprintf(f, "vnet_hdr %s ", vnet ? "on" : "off");
+	}
+
+	print_mq(f, tb);
+
+	if (tb[IFLA_TUN_PERSIST]) {
+		__u8 persist = rta_getattr_u8(tb[IFLA_TUN_PERSIST]);
+		fprintf(f, "persist %s ", persist ? "on" : "off");
+	}
+
+	if (tb[IFLA_TUN_OWNER])
+		print_owner(f, rta_getattr_u32(tb[IFLA_TUN_OWNER]));
+
+	if (tb[IFLA_TUN_GROUP])
+		print_group(f, rta_getattr_u32(tb[IFLA_TUN_GROUP]));
+}
+
+struct link_util tun_link_util = {
+	.id = "tun",
+	.maxattr = IFLA_TUN_MAX,
+	.print_opt = tun_print_opt,
+};
-- 
2.16.1

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

* Re: [PATCH iproute2-next] ip link: add support to display extended tun attributes
  2018-02-16 10:03 [PATCH iproute2-next] ip link: add support to display extended tun attributes Sabrina Dubroca
@ 2018-02-19 22:19 ` Serhey Popovych
  2018-02-20  4:09   ` David Ahern
  2018-02-20 10:44   ` Sabrina Dubroca
  0 siblings, 2 replies; 4+ messages in thread
From: Serhey Popovych @ 2018-02-19 22:19 UTC (permalink / raw)
  To: Sabrina Dubroca, netdev; +Cc: David Ahern, sbrivio


[-- Attachment #1.1: Type: text/plain, Size: 4436 bytes --]

Sabrina Dubroca wrote:
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
> ---
> 
> I included the uapi changes from the kernel patch. Let me know if I
> need to resend without the header changes.
> 
>  include/uapi/linux/if_link.h | 18 ++++++++++
>  ip/iptuntap.c                | 85 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 103 insertions(+)
> 
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 1726e49fbc6b..e2af6588b50d 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -936,4 +936,22 @@ enum {
>  	IFLA_EVENT_BONDING_OPTIONS,	/* change in bonding options */
>  };
>  
> +/* tun section */
> +
> +enum {
> +	IFLA_TUN_UNSPEC,
> +	IFLA_TUN_OWNER,
> +	IFLA_TUN_GROUP,
> +	IFLA_TUN_TYPE,
> +	IFLA_TUN_PI,
> +	IFLA_TUN_VNET_HDR,
> +	IFLA_TUN_PERSIST,
> +	IFLA_TUN_MULTI_QUEUE,
> +	IFLA_TUN_NUM_QUEUES,
> +	IFLA_TUN_NUM_DISABLED_QUEUES,
> +	__IFLA_TUN_MAX,
> +};
> +
> +#define IFLA_TUN_MAX (__IFLA_TUN_MAX - 1)
> +
>  #endif /* _LINUX_IF_LINK_H */
> diff --git a/ip/iptuntap.c b/ip/iptuntap.c
> index 4628db2832b4..520244c31b89 100644
> --- a/ip/iptuntap.c
> +++ b/ip/iptuntap.c
> @@ -469,3 +469,88 @@ int do_iptuntap(int argc, char **argv)
>  		*argv);
>  	exit(-1);
>  }
> +
> +static void print_owner(FILE *f, uid_t uid)
> +{
> +	struct passwd *pw = getpwuid(uid);
> +
> +	if (pw)
> +		fprintf(f, "user %s ", pw->pw_name);
> +	else
> +		fprintf(f, "user %u ", uid);
> +}
> +
> +static void print_group(FILE *f, gid_t gid)
> +{
> +	struct group *group = getgrgid(gid);
> +
> +	if (group)
> +		fprintf(f, "group %s ", group->gr_name);
> +	else
> +		fprintf(f, "group %u ", gid);
> +}
> +
> +static void print_mq(FILE *f, struct rtattr *tb[])
> +{
> +	if (tb[IFLA_TUN_MULTI_QUEUE]) {
> +		__u8 mq = rta_getattr_u8(tb[IFLA_TUN_MULTI_QUEUE]);
> +		if (!mq)
> +			return;
> +		fprintf(f, "multi_queue ");
> +	}
> +
> +	if (tb[IFLA_TUN_NUM_QUEUES]) {
> +		__u32 numq = rta_getattr_u32(tb[IFLA_TUN_NUM_QUEUES]);
> +		fprintf(f, "numqueues %u ", numq);
> +	}
> +
> +	if (tb[IFLA_TUN_NUM_DISABLED_QUEUES]) {
> +		__u32 numq = rta_getattr_u32(tb[IFLA_TUN_NUM_DISABLED_QUEUES]);
> +		fprintf(f, "numdisabled %u ", numq);
> +	}
> +}
> +
> +static void tun_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
> +{
> +	if (!tb)
> +		return;
> +
> +	if (tb[IFLA_TUN_TYPE]) {
> +		__u8 type = rta_getattr_u8(tb[IFLA_TUN_TYPE]);
> +		if (type == IFF_TUN)
> +			fprintf(f, "type tun ");
> +		else if (type == IFF_TAP)
> +			fprintf(f, "type tap ");
> +		else
> +			fprintf(f, "type UNKNOWN:%hhu ", type);
> +	}
> +
> +	if (tb[IFLA_TUN_PI]) {
> +		__u8 pi = rta_getattr_u8(tb[IFLA_TUN_PI]);
> +		fprintf(f, "pi %s ", pi ? "on" : "off");
> +	}
> +
> +	if (tb[IFLA_TUN_VNET_HDR]) {
> +		__u8 vnet = rta_getattr_u8(tb[IFLA_TUN_VNET_HDR]);
> +		fprintf(f, "vnet_hdr %s ", vnet ? "on" : "off");
> +	}
> +
> +	print_mq(f, tb);
> +
> +	if (tb[IFLA_TUN_PERSIST]) {
> +		__u8 persist = rta_getattr_u8(tb[IFLA_TUN_PERSIST]);
> +		fprintf(f, "persist %s ", persist ? "on" : "off");
> +	}
> +
> +	if (tb[IFLA_TUN_OWNER])
> +		print_owner(f, rta_getattr_u32(tb[IFLA_TUN_OWNER]));
> +
> +	if (tb[IFLA_TUN_GROUP])
> +		print_group(f, rta_getattr_u32(tb[IFLA_TUN_GROUP]));
> +}
> +
> +struct link_util tun_link_util = {
> +	.id = "tun",
> +	.maxattr = IFLA_TUN_MAX,
> +	.print_opt = tun_print_opt,
> +};
> 

I get following checkpatch.pl warnings:

WARNING: Missing a blank line after declarations
#73: FILE: ip/iptuntap.c:497:
+               __u8 mq = rta_getattr_u8(tb[IFLA_TUN_MULTI_QUEUE]);
+               if (!mq)

WARNING: Missing a blank line after declarations
#80: FILE: ip/iptuntap.c:504:
+               __u32 numq = rta_getattr_u32(tb[IFLA_TUN_NUM_QUEUES]);
+               fprintf(f, "numqueues %u ", numq);

WARNING: Missing a blank line after declarations
#85: FILE: ip/iptuntap.c:509:
+               __u32 numq =
rta_getattr_u32(tb[IFLA_TUN_NUM_DISABLED_QUEUES]);
+               fprintf(f, "numdisabled %u ", numq);

maybe they should be fixed before accepting?

I wonder if we can use tun_print_opt() and it's infrastructure
in print_tuntap(). Now they use read_prop() to read same information
from sysfs while netlink provides it.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

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

* Re: [PATCH iproute2-next] ip link: add support to display extended tun attributes
  2018-02-19 22:19 ` Serhey Popovych
@ 2018-02-20  4:09   ` David Ahern
  2018-02-20 10:44   ` Sabrina Dubroca
  1 sibling, 0 replies; 4+ messages in thread
From: David Ahern @ 2018-02-20  4:09 UTC (permalink / raw)
  To: Serhey Popovych, Sabrina Dubroca, netdev; +Cc: sbrivio

On 2/19/18 3:19 PM, Serhey Popovych wrote:
> I get following checkpatch.pl warnings:
> 
> WARNING: Missing a blank line after declarations
> #73: FILE: ip/iptuntap.c:497:
> +               __u8 mq = rta_getattr_u8(tb[IFLA_TUN_MULTI_QUEUE]);
> +               if (!mq)
> 
> WARNING: Missing a blank line after declarations
> #80: FILE: ip/iptuntap.c:504:
> +               __u32 numq = rta_getattr_u32(tb[IFLA_TUN_NUM_QUEUES]);
> +               fprintf(f, "numqueues %u ", numq);
> 
> WARNING: Missing a blank line after declarations
> #85: FILE: ip/iptuntap.c:509:
> +               __u32 numq =
> rta_getattr_u32(tb[IFLA_TUN_NUM_DISABLED_QUEUES]);
> +               fprintf(f, "numdisabled %u ", numq);
> 
> maybe they should be fixed before accepting?

yes. Please fix; headers have been updated to 4.16.0-rc2+, f5c0c6f4299f,
so you can drop those from v2

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

* Re: [PATCH iproute2-next] ip link: add support to display extended tun attributes
  2018-02-19 22:19 ` Serhey Popovych
  2018-02-20  4:09   ` David Ahern
@ 2018-02-20 10:44   ` Sabrina Dubroca
  1 sibling, 0 replies; 4+ messages in thread
From: Sabrina Dubroca @ 2018-02-20 10:44 UTC (permalink / raw)
  To: Serhey Popovych; +Cc: netdev, David Ahern, sbrivio

2018-02-20, 00:19:25 +0200, Serhey Popovych wrote:
> I get following checkpatch.pl warnings:
> 
> WARNING: Missing a blank line after declarations
> #73: FILE: ip/iptuntap.c:497:
> +               __u8 mq = rta_getattr_u8(tb[IFLA_TUN_MULTI_QUEUE]);
> +               if (!mq)
> 
> WARNING: Missing a blank line after declarations
> #80: FILE: ip/iptuntap.c:504:
> +               __u32 numq = rta_getattr_u32(tb[IFLA_TUN_NUM_QUEUES]);
> +               fprintf(f, "numqueues %u ", numq);
> 
> WARNING: Missing a blank line after declarations
> #85: FILE: ip/iptuntap.c:509:
> +               __u32 numq =
> rta_getattr_u32(tb[IFLA_TUN_NUM_DISABLED_QUEUES]);
> +               fprintf(f, "numdisabled %u ", numq);
> 
> maybe they should be fixed before accepting?

IMHO fixing those makes the code slightly uglier, but sure,
I'll send a v2.


> I wonder if we can use tun_print_opt() and it's infrastructure
> in print_tuntap(). Now they use read_prop() to read same information
> from sysfs while netlink provides it.

I wouldn't, for 2 reasons:
 - these netlink attributes aren't exposed by older kernels, so if you
   run "ip tuntap" on an older kernel, you won't get any information
   about tun/tap devices anymore
 - the output format is not the same, I don't want to break existing
   users of ip tuntap that expect a specific format

-- 
Sabrina

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

end of thread, other threads:[~2018-02-20 10:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-16 10:03 [PATCH iproute2-next] ip link: add support to display extended tun attributes Sabrina Dubroca
2018-02-19 22:19 ` Serhey Popovych
2018-02-20  4:09   ` David Ahern
2018-02-20 10:44   ` Sabrina Dubroca

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.