public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH iproute2 0/6] utils: Get rid of inet_get_addr()
@ 2018-01-18 18:13 Serhey Popovych
  2018-01-18 18:13 ` [PATCH iproute2 1/6] utils: Always specify family for address in get_addr_1() Serhey Popovych
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Serhey Popovych @ 2018-01-18 18:13 UTC (permalink / raw)
  To: netdev

It looks confusing to have multiple independent
routines to get internet address from it's string
representation: get_addr() and inet_get_addr().

Most complicated users of inet_get_addr() is
iplink_geneve.c and iplink_vxlan.c because they
required to handle both AF_INET and AF_INET6
for their local/remote endpoints.

On the other hand get_addr() does not provide
additional information like address type: need
to address this. to get rid of current and
possible future code duplications. Note that
this functionality is first step to make proto
independent handling of local/remote endpoints
in ip/tunnel code (there will be additional
series based on this one).

Also fix get_addr_1() and get_prefix() to make
sure it always provide correct ->family and
->bitlen.

As always comments, suggestions and criticism
are welcome.

Thanks,
Serhii

Serhey Popovych (6):
  utils: Always specify family for address in get_addr_1()
  utils: Always specify family and ->bytelen in get_prefix_1()
  utils: Fast inet address classification after get_addr()
  iplink_geneve: Get rid of inet_get_addr()
  iplink_vxlan: Get rid of inet_get_addr()
  ip: Get rid of inet_get_addr()

 include/utils.h       |   36 ++++++++++++++++-
 ip/iplink_geneve.c    |   23 +++++------
 ip/iplink_vxlan.c     |   72 ++++++++++++---------------------
 ip/iproute_lwtunnel.c |    5 ++-
 ip/ipseg6.c           |    8 ++--
 lib/utils.c           |  108 +++++++++++++++++++++++++++++--------------------
 6 files changed, 142 insertions(+), 110 deletions(-)

-- 
1.7.10.4

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

* [PATCH iproute2 1/6] utils: Always specify family for address in get_addr_1()
  2018-01-18 18:13 [PATCH iproute2 0/6] utils: Get rid of inet_get_addr() Serhey Popovych
@ 2018-01-18 18:13 ` Serhey Popovych
  2018-01-18 18:13 ` [PATCH iproute2 2/6] utils: Always specify family and ->bytelen in get_prefix_1() Serhey Popovych
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Serhey Popovych @ 2018-01-18 18:13 UTC (permalink / raw)
  To: netdev

Set ->family correctly when string representing address
is "default", "all" or "any": get_addr_1() might be called
with AF_UNSPEC (e.g. get_addr() -> get_addr_1()).

Extend support for zero address to all address families,
not only AF_INET and AF_INET6 when one explicitly given
as @family: use af_byte_len() to correctly set address length.

Still assume AF_INET when @family is AF_UNSPEC.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 lib/utils.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/utils.c b/lib/utils.c
index 9fa5220..a023e74 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -543,8 +543,8 @@ int get_addr_1(inet_prefix *addr, const char *name, int family)
 	    strcmp(name, "any") == 0) {
 		if ((family == AF_DECnet) || (family == AF_MPLS))
 			return -1;
-		addr->family = family;
-		addr->bytelen = (family == AF_INET6 ? 16 : 4);
+		addr->family = (family != AF_UNSPEC) ? family : AF_INET;
+		addr->bytelen = af_byte_len(addr->family);
 		addr->bitlen = -1;
 		return 0;
 	}
-- 
1.7.10.4

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

* [PATCH iproute2 2/6] utils: Always specify family and ->bytelen in get_prefix_1()
  2018-01-18 18:13 [PATCH iproute2 0/6] utils: Get rid of inet_get_addr() Serhey Popovych
  2018-01-18 18:13 ` [PATCH iproute2 1/6] utils: Always specify family for address in get_addr_1() Serhey Popovych
@ 2018-01-18 18:13 ` Serhey Popovych
  2018-01-18 18:13 ` [PATCH iproute2 3/6] utils: Fast inet address classification after get_addr() Serhey Popovych
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Serhey Popovych @ 2018-01-18 18:13 UTC (permalink / raw)
  To: netdev

Handle default/all/any special case in get_addr_1() to setup
->family and ->bytelen correctly.

Make get_addr_1() return ->bitlen == -2 instead of -1 to
distinguish default/all/any special case from the rest:
it is safe because all callers check ->bitlen < 0, not
explicit value -1.

Reduce intendation by one level and get rid of goto/label
to make code more readable.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 lib/utils.c |   64 +++++++++++++++++++++++++++++------------------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/lib/utils.c b/lib/utils.c
index a023e74..48c4bcb 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -545,7 +545,7 @@ int get_addr_1(inet_prefix *addr, const char *name, int family)
 			return -1;
 		addr->family = (family != AF_UNSPEC) ? family : AF_INET;
 		addr->bytelen = af_byte_len(addr->family);
-		addr->bitlen = -1;
+		addr->bitlen = -2;
 		return 0;
 	}
 
@@ -644,46 +644,46 @@ int af_byte_len(int af)
 
 int get_prefix_1(inet_prefix *dst, char *arg, int family)
 {
-	int err;
-	unsigned int plen;
 	char *slash;
-
-	memset(dst, 0, sizeof(*dst));
-
-	if (strcmp(arg, "default") == 0 ||
-	    strcmp(arg, "any") == 0 ||
-	    strcmp(arg, "all") == 0) {
-		if ((family == AF_DECnet) || (family == AF_MPLS))
-			return -1;
-		dst->family = family;
-		dst->bytelen = 0;
-		dst->bitlen = 0;
-		dst->flags |= PREFIXLEN_SPECIFIED;
-		return 0;
-	}
+	int err, bitlen, flags;
 
 	slash = strchr(arg, '/');
 	if (slash)
 		*slash = 0;
 
 	err = get_addr_1(dst, arg, family);
-	if (err == 0) {
-		dst->bitlen = af_bit_len(dst->family);
-
-		if (slash) {
-			if (get_netmask(&plen, slash+1, 0)
-			    || plen > dst->bitlen) {
-				err = -1;
-				goto done;
-			}
-			dst->flags |= PREFIXLEN_SPECIFIED;
-			dst->bitlen = plen;
-		}
-	}
-done:
+
 	if (slash)
 		*slash = '/';
-	return err;
+
+	if (err)
+		return err;
+
+	bitlen = af_bit_len(dst->family);
+
+	flags = PREFIXLEN_SPECIFIED;
+	if (slash) {
+		unsigned int plen;
+
+		if (dst->bitlen == -2)
+			return -1;
+		if (get_netmask(&plen, slash + 1, 0))
+			return -1;
+		if (plen > bitlen)
+			return -1;
+
+		bitlen = plen;
+	} else {
+		if (dst->bitlen == -2)
+			bitlen = 0;
+		else
+			flags = 0;
+	}
+
+	dst->flags |= flags;
+	dst->bitlen = bitlen;
+
+	return 0;
 }
 
 static const char *family_name_verbose(int family)
-- 
1.7.10.4

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

* [PATCH iproute2 3/6] utils: Fast inet address classification after get_addr()
  2018-01-18 18:13 [PATCH iproute2 0/6] utils: Get rid of inet_get_addr() Serhey Popovych
  2018-01-18 18:13 ` [PATCH iproute2 1/6] utils: Always specify family for address in get_addr_1() Serhey Popovych
  2018-01-18 18:13 ` [PATCH iproute2 2/6] utils: Always specify family and ->bytelen in get_prefix_1() Serhey Popovych
@ 2018-01-18 18:13 ` Serhey Popovych
  2018-01-18 18:13 ` [PATCH iproute2 4/6] iplink_geneve: Get rid of inet_get_addr() Serhey Popovych
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Serhey Popovych @ 2018-01-18 18:13 UTC (permalink / raw)
  To: netdev

It looks very useful to receive additional information
from get_addr_1() and get_addr() about address to simplify
caller and get rid of code duplications.

For now following information can be returned:

  1) address is unspecified (zero)
  2) address is multicast
  3) address is internet: family is either AF_INET or
     AF_INET6.

More information can be added in the future.

Introduce inline helpers to make code using this new
address classification interface more self explaining:

  bool is_addrtype_inet(inet_prefix *addr)
    true if @addr is inet address

  bool is_addrtype_inet_unspec(inet_prefix *addr)
    true if @addr is unspecified inet address

  bool is_addrtype_inet_multi(inet_prefix *addr)
    true if @addr is multicast inet address

  bool is_addrtype_inet_not_unspec(inet_prefix *addr)
    true if @addr is not unspecified inet address
    false if @addr is not inet or unspecified inet

  bool is_addrtype_inet_not_multi(inet_prefix *addr)
    true if @addr is not multicast inet address
    false if @addr is not inet or multicast inet

Last two are useful for case when we need inet address
that is not unspecified or multicast.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 include/utils.h |   35 ++++++++++++++++++++++++++++++++++-
 lib/utils.c     |   32 +++++++++++++++++++++++++++++++-
 2 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/include/utils.h b/include/utils.h
index d3895d5..6f072f6 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -54,7 +54,40 @@ typedef struct
 	__u32 data[64];
 } inet_prefix;
 
-#define PREFIXLEN_SPECIFIED 1
+enum {
+	PREFIXLEN_SPECIFIED	= (1 << 0),
+	ADDRTYPE_INET		= (1 << 1),
+	ADDRTYPE_UNSPEC		= (1 << 2),
+	ADDRTYPE_MULTI		= (1 << 3),
+
+	ADDRTYPE_INET_UNSPEC	= ADDRTYPE_INET | ADDRTYPE_UNSPEC,
+	ADDRTYPE_INET_MULTI	= ADDRTYPE_INET | ADDRTYPE_MULTI
+};
+
+static inline bool is_addrtype_inet(const inet_prefix *p)
+{
+	return p->flags & ADDRTYPE_INET;
+}
+
+static inline bool is_addrtype_inet_unspec(const inet_prefix *p)
+{
+	return (p->flags & ADDRTYPE_INET_UNSPEC) == ADDRTYPE_INET_UNSPEC;
+}
+
+static inline bool is_addrtype_inet_multi(const inet_prefix *p)
+{
+	return (p->flags & ADDRTYPE_INET_MULTI) == ADDRTYPE_INET_MULTI;
+}
+
+static inline bool is_addrtype_inet_not_unspec(const inet_prefix *p)
+{
+	return (p->flags & ADDRTYPE_INET_UNSPEC) == ADDRTYPE_INET;
+}
+
+static inline bool is_addrtype_inet_not_multi(const inet_prefix *p)
+{
+	return (p->flags & ADDRTYPE_INET_MULTI) == ADDRTYPE_INET;
+}
 
 #define DN_MAXADDL 20
 #ifndef AF_DECnet
diff --git a/lib/utils.c b/lib/utils.c
index 48c4bcb..e66c1ff 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -534,7 +534,7 @@ int get_addr64(__u64 *ap, const char *cp)
 	return 1;
 }
 
-int get_addr_1(inet_prefix *addr, const char *name, int family)
+static int __get_addr_1(inet_prefix *addr, const char *name, int family)
 {
 	memset(addr, 0, sizeof(*addr));
 
@@ -619,6 +619,36 @@ int get_addr_1(inet_prefix *addr, const char *name, int family)
 	return 0;
 }
 
+int get_addr_1(inet_prefix *addr, const char *name, int family)
+{
+	int ret;
+
+	ret = __get_addr_1(addr, name, family);
+	if (ret)
+		return ret;
+
+	switch (addr->family) {
+	case AF_INET:
+		if (!addr->data[0])
+			addr->flags |= ADDRTYPE_INET_UNSPEC;
+		else if (IN_MULTICAST(ntohl(addr->data[0])))
+			addr->flags |= ADDRTYPE_INET_MULTI;
+		else
+			addr->flags |= ADDRTYPE_INET;
+		break;
+	case AF_INET6:
+		if (IN6_IS_ADDR_UNSPECIFIED(addr->data))
+			addr->flags |= ADDRTYPE_INET_UNSPEC;
+		else if (IN6_IS_ADDR_MULTICAST(addr->data))
+			addr->flags |= ADDRTYPE_INET_MULTI;
+		else
+			addr->flags |= ADDRTYPE_INET;
+		break;
+	}
+
+	return 0;
+}
+
 int af_bit_len(int af)
 {
 	switch (af) {
-- 
1.7.10.4

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

* [PATCH iproute2 4/6] iplink_geneve: Get rid of inet_get_addr()
  2018-01-18 18:13 [PATCH iproute2 0/6] utils: Get rid of inet_get_addr() Serhey Popovych
                   ` (2 preceding siblings ...)
  2018-01-18 18:13 ` [PATCH iproute2 3/6] utils: Fast inet address classification after get_addr() Serhey Popovych
@ 2018-01-18 18:13 ` Serhey Popovych
  2018-01-18 18:13 ` [PATCH iproute2 5/6] iplink_vxlan: " Serhey Popovych
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Serhey Popovych @ 2018-01-18 18:13 UTC (permalink / raw)
  To: netdev

Now we have additional information about address
class from get_addr() we can use it in place of
inet_get_addr().

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/iplink_geneve.c |   23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/ip/iplink_geneve.c b/ip/iplink_geneve.c
index f0f1d1c..2b97e7a 100644
--- a/ip/iplink_geneve.c
+++ b/ip/iplink_geneve.c
@@ -57,9 +57,8 @@ static void check_duparg(__u64 *attrs, int type, const char *key,
 static int geneve_parse_opt(struct link_util *lu, int argc, char **argv,
 			  struct nlmsghdr *n)
 {
+	inet_prefix daddr;
 	__u32 vni = 0;
-	__u32 daddr = 0;
-	struct in6_addr daddr6 = IN6ADDR_ANY_INIT;
 	__u32 label = 0;
 	__u8 ttl = 0;
 	__u8 tos = 0;
@@ -72,6 +71,8 @@ static int geneve_parse_opt(struct link_util *lu, int argc, char **argv,
 	bool set_op = (n->nlmsg_type == RTM_NEWLINK &&
 		       !(n->nlmsg_flags & NLM_F_CREATE));
 
+	daddr.flags = 0;
+
 	while (argc > 0) {
 		if (!matches(*argv, "id") ||
 		    !matches(*argv, "vni")) {
@@ -84,11 +85,8 @@ static int geneve_parse_opt(struct link_util *lu, int argc, char **argv,
 			NEXT_ARG();
 			check_duparg(&attrs, IFLA_GENEVE_REMOTE, "remote",
 				     *argv);
-			if (!inet_get_addr(*argv, &daddr, &daddr6)) {
-				fprintf(stderr, "Invalid address \"%s\"\n", *argv);
-				return -1;
-			}
-			if (IN6_IS_ADDR_MULTICAST(&daddr6) || IN_MULTICAST(ntohl(daddr)))
+			get_addr(&daddr, *argv, AF_UNSPEC);
+			if (!is_addrtype_inet_not_multi(&daddr))
 				invarg("invalid remote address", *argv);
 		} else if (!matches(*argv, "ttl") ||
 			   !matches(*argv, "hoplimit")) {
@@ -191,18 +189,17 @@ static int geneve_parse_opt(struct link_util *lu, int argc, char **argv,
 		 * ID (VNI) to identify the geneve device, and we do not need
 		 * the remote IP.
 		 */
-		if (!set_op && !daddr && IN6_IS_ADDR_UNSPECIFIED(&daddr6)) {
+		if (!set_op && !is_addrtype_inet(&daddr)) {
 			fprintf(stderr, "geneve: remote link partner not specified\n");
 			return -1;
 		}
 	}
 
 	addattr32(n, 1024, IFLA_GENEVE_ID, vni);
-	if (daddr)
-		addattr_l(n, 1024, IFLA_GENEVE_REMOTE, &daddr, 4);
-	if (!IN6_IS_ADDR_UNSPECIFIED(&daddr6)) {
-		addattr_l(n, 1024, IFLA_GENEVE_REMOTE6, &daddr6,
-			  sizeof(struct in6_addr));
+	if (is_addrtype_inet(&daddr)) {
+		int type = (daddr.family == AF_INET) ? IFLA_GENEVE_REMOTE :
+						       IFLA_GENEVE_REMOTE6;
+		addattr_l(n, sizeof(1024), type, daddr.data, daddr.bytelen);
 	}
 	if (!set_op || GENEVE_ATTRSET(attrs, IFLA_GENEVE_LABEL))
 		addattr32(n, 1024, IFLA_GENEVE_LABEL, label);
-- 
1.7.10.4

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

* [PATCH iproute2 5/6] iplink_vxlan: Get rid of inet_get_addr()
  2018-01-18 18:13 [PATCH iproute2 0/6] utils: Get rid of inet_get_addr() Serhey Popovych
                   ` (3 preceding siblings ...)
  2018-01-18 18:13 ` [PATCH iproute2 4/6] iplink_geneve: Get rid of inet_get_addr() Serhey Popovych
@ 2018-01-18 18:13 ` Serhey Popovych
  2018-01-18 18:13 ` [PATCH iproute2 6/6] ip: " Serhey Popovych
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Serhey Popovych @ 2018-01-18 18:13 UTC (permalink / raw)
  To: netdev

Now we have additional information about address
class from get_addr() we can use it in place of
inet_get_addr().

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/iplink_vxlan.c |   72 +++++++++++++++++++----------------------------------
 1 file changed, 26 insertions(+), 46 deletions(-)

diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index 661eaa7..29a8ed2 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -74,11 +74,9 @@ static void check_duparg(__u64 *attrs, int type, const char *key,
 static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
 			  struct nlmsghdr *n)
 {
+	inet_prefix saddr;
+	inet_prefix daddr;
 	__u32 vni = 0;
-	__u32 gaddr = 0;
-	__u32 daddr = 0;
-	struct in6_addr gaddr6 = IN6ADDR_ANY_INIT;
-	struct in6_addr daddr6 = IN6ADDR_ANY_INIT;
 	__u8 learning = 1;
 	__u16 dstport = 0;
 	__u8 metadata = 0;
@@ -86,6 +84,9 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
 	bool set_op = (n->nlmsg_type == RTM_NEWLINK &&
 		       !(n->nlmsg_flags & NLM_F_CREATE));
 
+	saddr.family = daddr.family = AF_UNSPEC;
+	saddr.flags = daddr.flags = 0;
+
 	while (argc > 0) {
 		if (!matches(*argv, "id") ||
 		    !matches(*argv, "vni")) {
@@ -98,54 +99,33 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
 			    vni >= 1u << 24)
 				invarg("invalid id", *argv);
 		} else if (!matches(*argv, "group")) {
-			if (daddr || !IN6_IS_ADDR_UNSPECIFIED(&daddr6)) {
+			if (is_addrtype_inet_not_multi(&daddr)) {
 				fprintf(stderr, "vxlan: both group and remote");
 				fprintf(stderr, " cannot be specified\n");
 				return -1;
 			}
 			NEXT_ARG();
 			check_duparg(&attrs, IFLA_VXLAN_GROUP, "group", *argv);
-			if (!inet_get_addr(*argv, &gaddr, &gaddr6)) {
-				fprintf(stderr, "Invalid address \"%s\"\n", *argv);
-				return -1;
-			}
-			if (!IN6_IS_ADDR_MULTICAST(&gaddr6) && !IN_MULTICAST(ntohl(gaddr)))
+			get_addr(&daddr, *argv, saddr.family);
+			if (!is_addrtype_inet_multi(&daddr))
 				invarg("invalid group address", *argv);
 		} else if (!matches(*argv, "remote")) {
-			if (gaddr || !IN6_IS_ADDR_UNSPECIFIED(&gaddr6)) {
+			if (is_addrtype_inet_multi(&daddr)) {
 				fprintf(stderr, "vxlan: both group and remote");
 				fprintf(stderr, " cannot be specified\n");
 				return -1;
 			}
 			NEXT_ARG();
 			check_duparg(&attrs, IFLA_VXLAN_GROUP, "remote", *argv);
-			if (!inet_get_addr(*argv, &daddr, &daddr6)) {
-				fprintf(stderr, "Invalid address \"%s\"\n", *argv);
-				return -1;
-			}
-			if (IN6_IS_ADDR_MULTICAST(&daddr6) || IN_MULTICAST(ntohl(daddr)))
+			get_addr(&daddr, *argv, saddr.family);
+			if (!is_addrtype_inet_not_multi(&daddr))
 				invarg("invalid remote address", *argv);
 		} else if (!matches(*argv, "local")) {
-			__u32 saddr = 0;
-			struct in6_addr saddr6 = IN6ADDR_ANY_INIT;
-
 			NEXT_ARG();
 			check_duparg(&attrs, IFLA_VXLAN_LOCAL, "local", *argv);
-			if (strcmp(*argv, "any")) {
-				if (!inet_get_addr(*argv, &saddr, &saddr6)) {
-					fprintf(stderr, "Invalid address \"%s\"\n", *argv);
-					return -1;
-				}
-			}
-
-			if (IN_MULTICAST(ntohl(saddr)) || IN6_IS_ADDR_MULTICAST(&saddr6))
+			get_addr(&saddr, *argv, daddr.family);
+			if (!is_addrtype_inet_not_multi(&saddr))
 				invarg("invalid local address", *argv);
-
-			if (saddr)
-				addattr_l(n, 1024, IFLA_VXLAN_LOCAL, &saddr, 4);
-			else if (!IN6_IS_ADDR_UNSPECIFIED(&saddr6))
-				addattr_l(n, 1024, IFLA_VXLAN_LOCAL6, &saddr6,
-					  sizeof(struct in6_addr));
 		} else if (!matches(*argv, "dev")) {
 			unsigned int link;
 
@@ -350,7 +330,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
 		return -1;
 	}
 
-	if ((gaddr || !IN6_IS_ADDR_UNSPECIFIED(&gaddr6)) &&
+	if (is_addrtype_inet_multi(&daddr) &&
 	    !VXLAN_ATTRSET(attrs, IFLA_VXLAN_LINK)) {
 		fprintf(stderr, "vxlan: 'group' requires 'dev' to be specified\n");
 		return -1;
@@ -369,18 +349,18 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
 
 	if (VXLAN_ATTRSET(attrs, IFLA_VXLAN_ID))
 		addattr32(n, 1024, IFLA_VXLAN_ID, vni);
-	if (gaddr)
-		addattr_l(n, 1024, IFLA_VXLAN_GROUP, &gaddr, 4);
-	else if (daddr)
-		addattr_l(n, 1024, IFLA_VXLAN_GROUP, &daddr, 4);
-	else if (!IN6_IS_ADDR_UNSPECIFIED(&gaddr6))
-		addattr_l(n, 1024, IFLA_VXLAN_GROUP6, &gaddr6, sizeof(struct in6_addr));
-	else if (!IN6_IS_ADDR_UNSPECIFIED(&daddr6))
-		addattr_l(n, 1024, IFLA_VXLAN_GROUP6, &daddr6, sizeof(struct in6_addr));
-	else if (preferred_family == AF_INET)
-		addattr_l(n, 1024, IFLA_VXLAN_GROUP, &daddr, 4);
-	else if (preferred_family == AF_INET6)
-		addattr_l(n, 1024, IFLA_VXLAN_GROUP6, &daddr6, sizeof(struct in6_addr));
+
+	if (is_addrtype_inet(&saddr)) {
+		int type = (saddr.family == AF_INET) ? IFLA_VXLAN_LOCAL
+						     : IFLA_VXLAN_LOCAL6;
+		addattr_l(n, 1024, type, saddr.data, saddr.bytelen);
+	}
+
+	if (is_addrtype_inet(&daddr)) {
+		int type = (daddr.family == AF_INET) ? IFLA_VXLAN_GROUP
+						     : IFLA_VXLAN_GROUP6;
+		addattr_l(n, 1024, type, daddr.data, daddr.bytelen);
+	}
 
 	if (!set_op || VXLAN_ATTRSET(attrs, IFLA_VXLAN_LEARNING))
 		addattr8(n, 1024, IFLA_VXLAN_LEARNING, learning);
-- 
1.7.10.4

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

* [PATCH iproute2 6/6] ip: Get rid of inet_get_addr()
  2018-01-18 18:13 [PATCH iproute2 0/6] utils: Get rid of inet_get_addr() Serhey Popovych
                   ` (4 preceding siblings ...)
  2018-01-18 18:13 ` [PATCH iproute2 5/6] iplink_vxlan: " Serhey Popovych
@ 2018-01-18 18:13 ` Serhey Popovych
  2018-01-18 18:35 ` [PATCH iproute2 0/6] utils: " Stephen Hemminger
  2018-01-21 18:13 ` David Ahern
  7 siblings, 0 replies; 9+ messages in thread
From: Serhey Popovych @ 2018-01-18 18:13 UTC (permalink / raw)
  To: netdev

Both geneve and vxlan modules are converted to
use get_addr() we can replace inet_get_addr()
in less problematic places and finally get
rid of inet_get_addr().

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 include/utils.h       |    1 -
 ip/iproute_lwtunnel.c |    5 ++++-
 ip/ipseg6.c           |    8 +++-----
 lib/utils.c           |    8 --------
 4 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/include/utils.h b/include/utils.h
index 6f072f6..f562547 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -269,7 +269,6 @@ void print_nlmsg_timestamp(FILE *fp, const struct nlmsghdr *n);
 extern int cmdlineno;
 ssize_t getcmdline(char **line, size_t *len, FILE *in);
 int makeargs(char *line, char *argv[], int maxargs);
-int inet_get_addr(const char *src, __u32 *dst, struct in6_addr *dst6);
 
 struct iplink_req {
 	struct nlmsghdr		n;
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index 740da7c..2ccc783 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -434,7 +434,10 @@ static struct ipv6_sr_hdr *parse_srh(char *segbuf, int hmac, bool encap)
 
 	i = srh->first_segment;
 	for (s = strtok(segbuf, ","); s; s = strtok(NULL, ",")) {
-		inet_get_addr(s, NULL, &srh->segments[i]);
+		inet_prefix addr;
+
+		get_addr(&addr, s, AF_INET6);
+		memcpy(&srh->segments[i], addr.data, sizeof(struct in6_addr));
 		i--;
 	}
 
diff --git a/ip/ipseg6.c b/ip/ipseg6.c
index 461a3c1..e3ab31a 100644
--- a/ip/ipseg6.c
+++ b/ip/ipseg6.c
@@ -49,7 +49,7 @@ static int genl_family = -1;
 
 static struct {
 	unsigned int cmd;
-	struct in6_addr addr;
+	inet_prefix addr;
 	__u32 keyid;
 	const char *pass;
 	__u8 alg_id;
@@ -152,7 +152,7 @@ static int seg6_do_cmd(void)
 		break;
 	}
 	case SEG6_CMD_SET_TUNSRC:
-		addattr_l(&req.n, sizeof(req), SEG6_ATTR_DST, &opts.addr,
+		addattr_l(&req.n, sizeof(req), SEG6_ATTR_DST, opts.addr.data,
 			  sizeof(struct in6_addr));
 		break;
 	case SEG6_CMD_DUMPHMAC:
@@ -226,9 +226,7 @@ int do_seg6(int argc, char **argv)
 		} else if (matches(*argv, "set") == 0) {
 			NEXT_ARG();
 			opts.cmd = SEG6_CMD_SET_TUNSRC;
-			if (!inet_get_addr(*argv, NULL, &opts.addr))
-				invarg("tunsrc ADDRESS value is invalid",
-				       *argv);
+			get_addr(&opts.addr, *argv, AF_INET6);
 		} else {
 			invarg("unknown", *argv);
 		}
diff --git a/lib/utils.c b/lib/utils.c
index e66c1ff..e20b60e 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1280,14 +1280,6 @@ int makeargs(char *line, char *argv[], int maxargs)
 	return argc;
 }
 
-int inet_get_addr(const char *src, __u32 *dst, struct in6_addr *dst6)
-{
-	if (strchr(src, ':'))
-		return inet_pton(AF_INET6, src, dst6);
-	else
-		return inet_pton(AF_INET, src, dst);
-}
-
 void print_nlmsg_timestamp(FILE *fp, const struct nlmsghdr *n)
 {
 	char *tstr;
-- 
1.7.10.4

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

* Re: [PATCH iproute2 0/6] utils: Get rid of inet_get_addr()
  2018-01-18 18:13 [PATCH iproute2 0/6] utils: Get rid of inet_get_addr() Serhey Popovych
                   ` (5 preceding siblings ...)
  2018-01-18 18:13 ` [PATCH iproute2 6/6] ip: " Serhey Popovych
@ 2018-01-18 18:35 ` Stephen Hemminger
  2018-01-21 18:13 ` David Ahern
  7 siblings, 0 replies; 9+ messages in thread
From: Stephen Hemminger @ 2018-01-18 18:35 UTC (permalink / raw)
  To: Serhey Popovych; +Cc: netdev

On Thu, 18 Jan 2018 20:13:41 +0200
Serhey Popovych <serhe.popovych@gmail.com> wrote:

> It looks confusing to have multiple independent
> routines to get internet address from it's string
> representation: get_addr() and inet_get_addr().
> 
> Most complicated users of inet_get_addr() is
> iplink_geneve.c and iplink_vxlan.c because they
> required to handle both AF_INET and AF_INET6
> for their local/remote endpoints.
> 
> On the other hand get_addr() does not provide
> additional information like address type: need
> to address this. to get rid of current and
> possible future code duplications. Note that
> this functionality is first step to make proto
> independent handling of local/remote endpoints
> in ip/tunnel code (there will be additional
> series based on this one).
> 
> Also fix get_addr_1() and get_prefix() to make
> sure it always provide correct ->family and
> ->bitlen.  
> 
> As always comments, suggestions and criticism
> are welcome.
> 
> Thanks,
> Serhii
> 
> Serhey Popovych (6):
>   utils: Always specify family for address in get_addr_1()
>   utils: Always specify family and ->bytelen in get_prefix_1()
>   utils: Fast inet address classification after get_addr()
>   iplink_geneve: Get rid of inet_get_addr()
>   iplink_vxlan: Get rid of inet_get_addr()
>   ip: Get rid of inet_get_addr()
> 
>  include/utils.h       |   36 ++++++++++++++++-
>  ip/iplink_geneve.c    |   23 +++++------
>  ip/iplink_vxlan.c     |   72 ++++++++++++---------------------
>  ip/iproute_lwtunnel.c |    5 ++-
>  ip/ipseg6.c           |    8 ++--
>  lib/utils.c           |  108 +++++++++++++++++++++++++++++--------------------
>  6 files changed, 142 insertions(+), 110 deletions(-)
> 

Let's consider this for net-next.
Not for current release.

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

* Re: [PATCH iproute2 0/6] utils: Get rid of inet_get_addr()
  2018-01-18 18:13 [PATCH iproute2 0/6] utils: Get rid of inet_get_addr() Serhey Popovych
                   ` (6 preceding siblings ...)
  2018-01-18 18:35 ` [PATCH iproute2 0/6] utils: " Stephen Hemminger
@ 2018-01-21 18:13 ` David Ahern
  7 siblings, 0 replies; 9+ messages in thread
From: David Ahern @ 2018-01-21 18:13 UTC (permalink / raw)
  To: Serhey Popovych, netdev

On 1/18/18 11:13 AM, Serhey Popovych wrote:
> It looks confusing to have multiple independent
> routines to get internet address from it's string
> representation: get_addr() and inet_get_addr().
> 
> Most complicated users of inet_get_addr() is
> iplink_geneve.c and iplink_vxlan.c because they
> required to handle both AF_INET and AF_INET6
> for their local/remote endpoints.
> 
> On the other hand get_addr() does not provide
> additional information like address type: need
> to address this. to get rid of current and
> possible future code duplications. Note that
> this functionality is first step to make proto
> independent handling of local/remote endpoints
> in ip/tunnel code (there will be additional
> series based on this one).
> 
> Also fix get_addr_1() and get_prefix() to make
> sure it always provide correct ->family and
> ->bitlen.
> 
> As always comments, suggestions and criticism
> are welcome.

Series applied to iproute2-next. Thanks,

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

end of thread, other threads:[~2018-01-21 18:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-18 18:13 [PATCH iproute2 0/6] utils: Get rid of inet_get_addr() Serhey Popovych
2018-01-18 18:13 ` [PATCH iproute2 1/6] utils: Always specify family for address in get_addr_1() Serhey Popovych
2018-01-18 18:13 ` [PATCH iproute2 2/6] utils: Always specify family and ->bytelen in get_prefix_1() Serhey Popovych
2018-01-18 18:13 ` [PATCH iproute2 3/6] utils: Fast inet address classification after get_addr() Serhey Popovych
2018-01-18 18:13 ` [PATCH iproute2 4/6] iplink_geneve: Get rid of inet_get_addr() Serhey Popovych
2018-01-18 18:13 ` [PATCH iproute2 5/6] iplink_vxlan: " Serhey Popovych
2018-01-18 18:13 ` [PATCH iproute2 6/6] ip: " Serhey Popovych
2018-01-18 18:35 ` [PATCH iproute2 0/6] utils: " Stephen Hemminger
2018-01-21 18:13 ` David Ahern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox