Re: [PATCH iproute2-next] ip link: add support to display extended tun attributes

[Serhey Popovych <serhe.popovych@gmail.com>]

[-- 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 --]