diff mbox series

[iproute2-next,v3] tipc: support interface name when activating UDP bearer

Message ID 20181018050323.5082-1-hoang.h.le@dektech.com.au
State Changes Requested, archived
Delegated to: David Ahern
Headers show
Series [iproute2-next,v3] tipc: support interface name when activating UDP bearer | expand

Commit Message

Hoang Huu Le Oct. 18, 2018, 5:03 a.m. UTC 
Support for indicating interface name has an ip address in parallel
with specifying ip address when activating UDP bearer.
This liberates the user from keeping track of the current ip address
for each device.

Old command syntax:
$tipc bearer enable media udp name NAME localip IP

New command syntax:
$tipc bearer enable media udp name NAME [localip IP|dev DEVICE]

v2:
    - Removed initial value for fd
    - Fixed the returning value for cmd_bearer_validate_and_get_addr
      to make its consistent with using: zero or non-zero
v3:
    - Switch to use helper 'get_ifname' to retrieve interface name

Acked-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Hoang Le <hoang.h.le@dektech.com.au>
---
 tipc/bearer.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 57 insertions(+), 5 deletions(-)

Comments

Stephen Hemminger Oct. 18, 2018, 8:02 p.m. UTC | #1 
On Thu, 18 Oct 2018 12:03:23 +0700
Hoang Le <hoang.h.le@dektech.com.au> wrote:

>  
> +static int cmd_bearer_validate_and_get_addr(const char *name, char *straddr)
> +{
> +	struct ifreq ifc = {};
> +	struct sockaddr_in *ip4addr;
> +	struct sockaddr_in6 *ip6addr;
> +	int fd;
> +
> +	if (!name || !straddr || get_ifname(ifc.ifr_name, name))
> +		return 0;
> +
> +	fd = socket(PF_INET, SOCK_DGRAM, 0);
> +	if (fd <= 0) {
> +		fprintf(stderr, "Failed to create socket\n");
> +		return 0;
> +	}
> +
> +	if (ioctl(fd, SIOCGIFADDR, &ifc) < 0) {
> +		fprintf(stderr, "ioctl failed: %s\n", strerror(errno));
> +		close(fd);
> +		return 0;
> +	}
> +
> +	ip4addr = (struct sockaddr_in *)&ifc.ifr_addr;
> +	if (inet_ntop(AF_INET, &ip4addr->sin_addr, straddr,
> +		      INET_ADDRSTRLEN) == NULL)	{
> +		ip6addr = (struct sockaddr_in6 *)&ifc.ifr_addr;
> +		if (inet_ntop(AF_INET6, &ip6addr->sin6_addr, straddr,
> +			      INET6_ADDRSTRLEN) == NULL) {
> +			fprintf(stderr, "UDP local address error\n");
> +			close(fd);
> +			return 0;
> +		}
> +	}
> +	close(fd);
> +	return 1;
> +}

The concept of this is good, but not sure if it is implemented in
the best way.


Using legacy SIOCGIFADDR has many limitations. It doesn't handle multiple
addresses per interface for example. Why not use netlink like rest of iproute2?
Also trying twice for IPv4 than IPv6 seems
unnecessary. Perhaps use getnameinfo or look at address famliy.


Remember Linux is weak host model so you might not get what you expect.
diff mbox series

Patch

diff --git a/tipc/bearer.c b/tipc/bearer.c
index 05dc84aa8ded..ea971516d3f4 100644
--- a/tipc/bearer.c
+++ b/tipc/bearer.c
@@ -19,10 +19,13 @@ 
 #include <linux/tipc_netlink.h>
 #include <linux/tipc.h>
 #include <linux/genetlink.h>
+#include <linux/if.h>
+#include <sys/ioctl.h>
 
 #include <libmnl/libmnl.h>
 #include <sys/socket.h>
 
+#include "utils.h"
 #include "cmdl.h"
 #include "msg.h"
 #include "bearer.h"
@@ -68,7 +71,7 @@  static void cmd_bearer_enable_l2_help(struct cmdl *cmdl, char *media)
 static void cmd_bearer_enable_udp_help(struct cmdl *cmdl, char *media)
 {
 	fprintf(stderr,
-		"Usage: %s bearer enable [OPTIONS] media %s name NAME localip IP [UDP OPTIONS]\n\n",
+		"Usage: %s bearer enable [OPTIONS] media %s name NAME [localip IP|device DEVICE] [UDP OPTIONS]\n\n",
 		cmdl->argv[0], media);
 	fprintf(stderr,
 		"OPTIONS\n"
@@ -121,6 +124,43 @@  static int generate_multicast(short af, char *buf, int bufsize)
 	return 0;
 }
 
+static int cmd_bearer_validate_and_get_addr(const char *name, char *straddr)
+{
+	struct ifreq ifc = {};
+	struct sockaddr_in *ip4addr;
+	struct sockaddr_in6 *ip6addr;
+	int fd;
+
+	if (!name || !straddr || get_ifname(ifc.ifr_name, name))
+		return 0;
+
+	fd = socket(PF_INET, SOCK_DGRAM, 0);
+	if (fd <= 0) {
+		fprintf(stderr, "Failed to create socket\n");
+		return 0;
+	}
+
+	if (ioctl(fd, SIOCGIFADDR, &ifc) < 0) {
+		fprintf(stderr, "ioctl failed: %s\n", strerror(errno));
+		close(fd);
+		return 0;
+	}
+
+	ip4addr = (struct sockaddr_in *)&ifc.ifr_addr;
+	if (inet_ntop(AF_INET, &ip4addr->sin_addr, straddr,
+		      INET_ADDRSTRLEN) == NULL)	{
+		ip6addr = (struct sockaddr_in6 *)&ifc.ifr_addr;
+		if (inet_ntop(AF_INET6, &ip6addr->sin6_addr, straddr,
+			      INET6_ADDRSTRLEN) == NULL) {
+			fprintf(stderr, "UDP local address error\n");
+			close(fd);
+			return 0;
+		}
+	}
+	close(fd);
+	return 1;
+}
+
 static int nl_add_udp_enable_opts(struct nlmsghdr *nlh, struct opt *opts,
 				  struct cmdl *cmdl)
 {
@@ -138,13 +178,25 @@  static int nl_add_udp_enable_opts(struct nlmsghdr *nlh, struct opt *opts,
 		.ai_family = AF_UNSPEC,
 		.ai_socktype = SOCK_DGRAM
 	};
+	char addr[INET6_ADDRSTRLEN] = {0};
 
-	if (!(opt = get_opt(opts, "localip"))) {
-		fprintf(stderr, "error, udp bearer localip missing\n");
-		cmd_bearer_enable_udp_help(cmdl, "udp");
+	opt = get_opt(opts, "device");
+	if (opt && !cmd_bearer_validate_and_get_addr(opt->val, addr)) {
+		fprintf(stderr, "error, no device name available\n");
 		return -EINVAL;
 	}
-	locip = opt->val;
+
+	if (strlen(addr) > 0) {
+		locip = addr;
+	} else {
+		opt = get_opt(opts, "localip");
+		if (!opt) {
+			fprintf(stderr, "error, udp bearer localip/device missing\n");
+			cmd_bearer_enable_udp_help(cmdl, "udp");
+			return -EINVAL;
+		}
+		locip = opt->val;
+	}
 
 	if ((opt = get_opt(opts, "remoteip")))
 		remip = opt->val;