pespin submitted this change.

View Change

Approvals: osmith: Looks good to me, but someone else must approve pespin: Looks good to me, approved fixeria: Looks good to me, but someone else must approve Jenkins Builder: Verified
Use osmo_sockaddr everywhere

This allows getting rid of lots of explicit castings, as well as makeing
it easier do improve IPv6 support in the future.

Change-Id: Id87d9cf2ec138091681ab0fe7c7f86d648c0e98e
---
M daemon/cups_client.c
M daemon/daemon_vty.c
M daemon/gtp_endpoint.c
M daemon/gtp_tunnel.c
M daemon/internal.h
M daemon/netdev.c
M daemon/tun_device.c
M daemon/utility.c
8 files changed, 60 insertions(+), 98 deletions(-)

diff --git a/daemon/cups_client.c b/daemon/cups_client.c
index 5b94bff..e2bae46 100644
--- a/daemon/cups_client.c
+++ b/daemon/cups_client.c
@@ -126,7 +126,7 @@
return jret;
}

-static int parse_ep(struct sockaddr_storage *out, json_t *in)
+static int parse_ep(struct osmo_sockaddr *out, json_t *in)
{
json_t *jaddr_type, *jport, *jip;
const char *addr_type, *ip;
@@ -153,14 +153,14 @@
memset(out, 0, sizeof(*out));

if (!strcmp(addr_type, "IPV4")) {
- struct sockaddr_in *sin = (struct sockaddr_in *) out;
+ struct sockaddr_in *sin = &out->u.sin;
if (osmo_hexparse(ip, buf, sizeof(buf)) != 4)
return -EINVAL;
memcpy(&sin->sin_addr, buf, 4);
sin->sin_family = AF_INET;
sin->sin_port = htons(json_integer_value(jport));
} else if (!strcmp(addr_type, "IPV6")) {
- struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *) out;
+ struct sockaddr_in6 *sin6 = &out->u.sin6;
if (osmo_hexparse(ip, buf, sizeof(buf)) != 16)
return -EINVAL;
memcpy(&sin6->sin6_addr, buf, 16);
@@ -172,7 +172,7 @@
return 0;
}

-static int parse_eua(struct sockaddr_storage *out, json_t *jip, json_t *jaddr_type)
+static int parse_eua(struct osmo_sockaddr *out, json_t *jip, json_t *jaddr_type)
{
const char *addr_type, *ip;
uint8_t buf[16];
@@ -186,13 +186,13 @@
memset(out, 0, sizeof(*out));

if (!strcmp(addr_type, "IPV4")) {
- struct sockaddr_in *sin = (struct sockaddr_in *) out;
+ struct sockaddr_in *sin = &out->u.sin;
if (osmo_hexparse(ip, buf, sizeof(buf)) != 4)
return -EINVAL;
memcpy(&sin->sin_addr, buf, 4);
sin->sin_family = AF_INET;
} else if (!strcmp(addr_type, "IPV6")) {
- struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *) out;
+ struct sockaddr_in6 *sin6 = &out->u.sin6;
if (osmo_hexparse(ip, buf, sizeof(buf)) != 16)
return -EINVAL;
memcpy(&sin6->sin6_addr, buf, 16);
@@ -348,7 +348,7 @@

static int cups_client_handle_destroy_tun(struct cups_client *cc, json_t *dtun)
{
- struct sockaddr_storage local_ep_addr;
+ struct osmo_sockaddr local_ep_addr;
json_t *jlocal_gtp_ep, *jrx_teid;
uint32_t rx_teid;
int rc;
diff --git a/daemon/daemon_vty.c b/daemon/daemon_vty.c
index c5cca74..16f23b8 100644
--- a/daemon/daemon_vty.c
+++ b/daemon/daemon_vty.c
@@ -146,7 +146,7 @@
show_ep_hdr(vty);
pthread_rwlock_rdlock(&g_daemon->rwlock);
if (argc > 0) {
- ep = _gtp_endpoint_find(g_daemon, (const struct sockaddr_storage *) ai->ai_addr);
+ ep = _gtp_endpoint_find(g_daemon, (const struct osmo_sockaddr *) ai->ai_addr);
if (!ep) {
pthread_rwlock_unlock(&g_daemon->rwlock);
vty_out(vty, "Cannot find GTP endpoint %s:%s%s", argv[0], argv[1], VTY_NEWLINE);
@@ -183,7 +183,7 @@
return CMD_WARNING;
}

- ep = gtp_endpoint_find_or_create(g_daemon, (struct sockaddr_storage *) ai->ai_addr);
+ ep = gtp_endpoint_find_or_create(g_daemon, (const struct osmo_sockaddr *) ai->ai_addr);
if (!ep) {
vty_out(vty, "Error creating endpoint%s", VTY_NEWLINE);
freeaddrinfo(ai);
@@ -214,7 +214,7 @@
}

pthread_rwlock_wrlock(&g_daemon->rwlock);
- ep = _gtp_endpoint_find(g_daemon, (struct sockaddr_storage *) ai->ai_addr);
+ ep = _gtp_endpoint_find(g_daemon, (const struct osmo_sockaddr *) ai->ai_addr);
if (!ep) {
pthread_rwlock_unlock(&g_daemon->rwlock);
vty_out(vty, "Cannot find to-be-destoryed endpoint%s", VTY_NEWLINE);
@@ -232,11 +232,11 @@
{
char remote_ip[64], remote_port[16], user_addr[64];

- getnameinfo((struct sockaddr *) &t->remote_udp, sizeof(t->remote_udp),
+ getnameinfo(&t->remote_udp.u.sa, sizeof(t->remote_udp.u.sas),
remote_ip, sizeof(remote_ip), remote_port, sizeof(remote_port),
NI_NUMERICHOST|NI_NUMERICSERV);

- getnameinfo((struct sockaddr *) &t->user_addr, sizeof(t->user_addr),
+ getnameinfo(&t->user_addr.u.sa, sizeof(t->user_addr.u.sas),
user_addr, sizeof(user_addr), NULL, 0,
NI_NUMERICHOST|NI_NUMERICSERV);

diff --git a/daemon/gtp_endpoint.c b/daemon/gtp_endpoint.c
index 87d333c..3340aee 100644
--- a/daemon/gtp_endpoint.c
+++ b/daemon/gtp_endpoint.c
@@ -157,7 +157,7 @@
}

static struct gtp_endpoint *
-_gtp_endpoint_create(struct gtp_daemon *d, const struct sockaddr_storage *bind_addr)
+_gtp_endpoint_create(struct gtp_daemon *d, const struct osmo_sockaddr *bind_addr)
{
struct gtp_endpoint *ep = talloc_zero(d, struct gtp_endpoint);
char ipstr[INET6_ADDRSTRLEN];
@@ -167,7 +167,7 @@
if (!ep)
return NULL;

- rc = getnameinfo((struct sockaddr *)bind_addr, sizeof(*bind_addr),
+ rc = getnameinfo(&bind_addr->u.sa, sizeof(bind_addr->u.sas),
ipstr, sizeof(ipstr), portstr, sizeof(portstr), NI_NUMERICHOST|NI_NUMERICSERV);
if (rc != 0)
goto out_free;
@@ -176,12 +176,12 @@
ep->d = d;
ep->use_count = 1;
ep->bind_addr = *bind_addr;
- ep->fd = socket(ep->bind_addr.ss_family, SOCK_DGRAM, IPPROTO_UDP);
+ ep->fd = socket(ep->bind_addr.u.sa.sa_family, SOCK_DGRAM, IPPROTO_UDP);
if (ep->fd < 0) {
LOGEP(ep, LOGL_ERROR, "Cannot create UDP socket: %s\n", strerror(errno));
goto out_free;
}
- rc = bind(ep->fd, (struct sockaddr *) &ep->bind_addr, sizeof(ep->bind_addr));
+ rc = bind(ep->fd, &ep->bind_addr.u.sa, sizeof(ep->bind_addr.u.sas));
if (rc < 0) {
LOGEP(ep, LOGL_ERROR, "Cannot bind UDP socket: %s\n", strerror(errno));
goto out_close;
@@ -205,21 +205,19 @@
}

struct gtp_endpoint *
-_gtp_endpoint_find(struct gtp_daemon *d, const struct sockaddr_storage *bind_addr)
+_gtp_endpoint_find(struct gtp_daemon *d, const struct osmo_sockaddr *bind_addr)
{
struct gtp_endpoint *ep;

llist_for_each_entry(ep, &d->gtp_endpoints, list) {
- if (sockaddr_equals((const struct sockaddr *) &ep->bind_addr,
- (const struct sockaddr *) bind_addr)) {
+ if (osmo_sockaddr_cmp(&ep->bind_addr, bind_addr) == 0)
return ep;
- }
}
return NULL;
}

struct gtp_endpoint *
-gtp_endpoint_find_or_create(struct gtp_daemon *d, const struct sockaddr_storage *bind_addr)
+gtp_endpoint_find_or_create(struct gtp_daemon *d, const struct osmo_sockaddr *bind_addr)
{
struct gtp_endpoint *ep;

@@ -258,7 +256,7 @@
void _gtp_endpoint_deref_destroy(struct gtp_endpoint *ep)
{
struct gtp_daemon *d = ep->d;
- struct sockaddr_storage ss = ep->bind_addr;
+ struct osmo_sockaddr osa = ep->bind_addr;
struct gtp_tunnel *t, *t2;
struct gtp_endpoint *ep2;

@@ -274,7 +272,7 @@
/* _gtp_endpoint_destroy may already have been called via
* _gtp_tunnel_destroy -> gtp_endpoint_release, so we have to
* check if the ep can still be found in the list */
- ep2 = _gtp_endpoint_find(d, &ss);
+ ep2 = _gtp_endpoint_find(d, &osa);
if (ep2 && ep2 == ep)
_gtp_endpoint_destroy(ep2);
}
diff --git a/daemon/gtp_tunnel.c b/daemon/gtp_tunnel.c
index 6c3e81c..a13b163 100644
--- a/daemon/gtp_tunnel.c
+++ b/daemon/gtp_tunnel.c
@@ -87,12 +87,12 @@
/* find tunnel by R(x_teid), T(x_teid) + A(ddr) */
static struct gtp_tunnel *
_gtp_tunnel_find_rta(struct gtp_daemon *d, uint32_t rx_teid, uint32_t tx_teid,
- const struct sockaddr_storage *user_addr)
+ const struct osmo_sockaddr *user_addr)
{
struct gtp_tunnel *t;
llist_for_each_entry(t, &d->gtp_tunnels, list) {
if (t->rx_teid == rx_teid && t->tx_teid == tx_teid &&
- sockaddr_equals((struct sockaddr *) &t->user_addr, (struct sockaddr *)user_addr))
+ osmo_sockaddr_cmp(&t->user_addr, &user_addr) == 0)
return t;
}
return NULL;
@@ -117,14 +117,15 @@

/* UNLOCKED find tunnel by tun + EUA ip (+proto/port) */
struct gtp_tunnel *
-_gtp_tunnel_find_eua(struct tun_device *tun, const struct sockaddr *sa, uint8_t proto)
+_gtp_tunnel_find_eua(struct tun_device *tun, const struct osmo_sockaddr *osa, uint8_t proto)
{
struct gtp_daemon *d = tun->d;
struct gtp_tunnel *t;

llist_for_each_entry(t, &d->gtp_tunnels, list) {
/* TODO: Find best matching filter */
- if (t->tun_dev == tun && sockaddr_equals(sa, (struct sockaddr *) &t->user_addr))
+ if (t->tun_dev == tun &&
+ osmo_sockaddr_cmp(osa, &t->user_addr) == 0)
return t;
}
return NULL;
@@ -149,7 +150,7 @@
talloc_free(t);
}

-bool gtp_tunnel_destroy(struct gtp_daemon *d, const struct sockaddr_storage *bind_addr, uint32_t rx_teid)
+bool gtp_tunnel_destroy(struct gtp_daemon *d, const struct osmo_sockaddr *bind_addr, uint32_t rx_teid)
{
struct gtp_endpoint *ep;
bool rc = false;
diff --git a/daemon/internal.h b/daemon/internal.h
index 19cf16a..d431add 100644
--- a/daemon/internal.h
+++ b/daemon/internal.h
@@ -29,8 +29,6 @@

#define MAX_UDP_PACKET 65535

-bool sockaddr_equals(const struct sockaddr *a, const struct sockaddr *b);
-
struct addrinfo *addrinfo_helper(uint16_t family, uint16_t type, uint8_t proto,
const char *host, uint16_t port, bool passive);
enum {
@@ -44,8 +42,8 @@
* netdev / netlink
***********************************************************************/

-int netdev_add_addr(struct nl_sock *nlsk, int ifindex, const struct sockaddr_storage *ss);
-int netdev_del_addr(struct nl_sock *nlsk, int ifindex, const struct sockaddr_storage *ss);
+int netdev_add_addr(struct nl_sock *nlsk, int ifindex, const struct osmo_sockaddr *osa);
+int netdev_del_addr(struct nl_sock *nlsk, int ifindex, const struct osmo_sockaddr *osa);
int netdev_set_link(struct nl_sock *nlsk, int ifindex, bool up);
int netdev_add_defaultroute(struct nl_sock *nlsk, int ifindex, uint8_t family);

@@ -68,7 +66,7 @@
int fd;

/* local IP:port */
- struct sockaddr_storage bind_addr;
+ struct osmo_sockaddr bind_addr;
char *name;

/* the thread handling Rx from the fd/socket */
@@ -77,10 +75,10 @@


struct gtp_endpoint *
-gtp_endpoint_find_or_create(struct gtp_daemon *d, const struct sockaddr_storage *bind_addr);
+gtp_endpoint_find_or_create(struct gtp_daemon *d, const struct osmo_sockaddr *bind_addr);

struct gtp_endpoint *
-_gtp_endpoint_find(struct gtp_daemon *d, const struct sockaddr_storage *bind_addr);
+_gtp_endpoint_find(struct gtp_daemon *d, const struct osmo_sockaddr *bind_addr);

void _gtp_endpoint_deref_destroy(struct gtp_endpoint *ep);

@@ -212,10 +210,10 @@
uint32_t rx_teid;

/* End user Address (inner IP) */
- struct sockaddr_storage user_addr;
+ struct osmo_sockaddr user_addr;

/* Remote UDP IP/Port*/
- struct sockaddr_storage remote_udp;
+ struct osmo_sockaddr remote_udp;

/* GTP Extension Header */
struct gtp1u_exthdrs exthdr;
@@ -227,7 +225,7 @@
_gtp_tunnel_find_r(struct gtp_daemon *d, uint32_t rx_teid, struct gtp_endpoint *ep);

struct gtp_tunnel *
-_gtp_tunnel_find_eua(struct tun_device *tun, const struct sockaddr *sa, uint8_t proto);
+_gtp_tunnel_find_eua(struct tun_device *tun, const struct osmo_sockaddr *osa, uint8_t proto);

struct gtp_tunnel_params {
/* TEID in receive and transmit direction */
@@ -238,13 +236,13 @@
struct gtp1u_exthdrs exthdr;

/* end user address */
- struct sockaddr_storage user_addr;
+ struct osmo_sockaddr user_addr;

/* remote GTP/UDP IP+Port */
- struct sockaddr_storage remote_udp;
+ struct osmo_sockaddr remote_udp;

/* local GTP/UDP IP+Port (used to lookup/create local EP) */
- struct sockaddr_storage local_udp;
+ struct osmo_sockaddr local_udp;

/* local TUN device name (used to lookup/create local tun) */
const char *tun_name;
@@ -253,7 +251,7 @@
struct gtp_tunnel *gtp_tunnel_alloc(struct gtp_daemon *d, const struct gtp_tunnel_params *cpars);

void _gtp_tunnel_destroy(struct gtp_tunnel *t);
-bool gtp_tunnel_destroy(struct gtp_daemon *d, const struct sockaddr_storage *bind_addr, uint32_t rx_teid);
+bool gtp_tunnel_destroy(struct gtp_daemon *d, const struct osmo_sockaddr *bind_addr, uint32_t rx_teid);


/***********************************************************************
diff --git a/daemon/netdev.c b/daemon/netdev.c
index e619883..165a6bc 100644
--- a/daemon/netdev.c
+++ b/daemon/netdev.c
@@ -24,27 +24,24 @@
#include <netlink/route/nexthop.h>

#include <osmocom/core/utils.h>
+#include <osmocom/core/socket.h>

/***********************************************************************
* netlink helper functions
***********************************************************************/

-static int _netdev_addr(struct nl_sock *nlsk, int ifindex, const struct sockaddr_storage *ss, bool add)
+static int _netdev_addr(struct nl_sock *nlsk, int ifindex, const struct osmo_sockaddr *osa, bool add)
{
- const struct sockaddr_in6 *sin6;
- const struct sockaddr_in *sin;
struct nl_addr *local = NULL;
struct rtnl_addr *addr;
int rc;

- switch (ss->ss_family) {
+ switch (osa->u.sa.sa_family) {
case AF_INET:
- sin = (struct sockaddr_in *) ss;
- local = nl_addr_build(AF_INET, &sin->sin_addr, 4);
+ local = nl_addr_build(AF_INET, &osa->u.sin.sin_addr, 4);
break;
case AF_INET6:
- sin6 = (struct sockaddr_in6 *) ss;
- local = nl_addr_build(AF_INET6, &sin6->sin6_addr, 16);
+ local = nl_addr_build(AF_INET6, &osa->u.sin6.sin6_addr, 16);
break;
}
OSMO_ASSERT(local);
@@ -64,14 +61,14 @@
return rc;
}

-int netdev_add_addr(struct nl_sock *nlsk, int ifindex, const struct sockaddr_storage *ss)
+int netdev_add_addr(struct nl_sock *nlsk, int ifindex, const struct osmo_sockaddr *osa)
{
- return _netdev_addr(nlsk, ifindex, ss, true);
+ return _netdev_addr(nlsk, ifindex, osa, true);
}

-int netdev_del_addr(struct nl_sock *nlsk, int ifindex, const struct sockaddr_storage *ss)
+int netdev_del_addr(struct nl_sock *nlsk, int ifindex, const struct osmo_sockaddr *osa)
{
- return _netdev_addr(nlsk, ifindex, ss, false);
+ return _netdev_addr(nlsk, ifindex, osa, false);
}

int netdev_set_link(struct nl_sock *nlsk, int ifindex, bool up)
diff --git a/daemon/tun_device.c b/daemon/tun_device.c
index 40b6ec3..bc41db9 100644
--- a/daemon/tun_device.c
+++ b/daemon/tun_device.c
@@ -58,8 +58,8 @@

/* extracted information from a packet */
struct pkt_info {
- struct sockaddr_storage saddr;
- struct sockaddr_storage daddr;
+ struct osmo_sockaddr saddr;
+ struct osmo_sockaddr daddr;
uint8_t proto;
};

@@ -71,8 +71,8 @@
memset(out, 0, sizeof(*out));

if (ip4->version == 4) {
- struct sockaddr_in *saddr4 = (struct sockaddr_in *) &out->saddr;
- struct sockaddr_in *daddr4 = (struct sockaddr_in *) &out->daddr;
+ struct sockaddr_in *saddr4 = &out->saddr.u.sin;
+ struct sockaddr_in *daddr4 = &out->daddr.u.sin;

if (in_len < sizeof(*ip4) || in_len < 4*ip4->ihl)
return -1;
@@ -100,8 +100,8 @@
}
} else if (ip4->version == 6) {
const struct ip6_hdr *ip6 = (struct ip6_hdr *) in;
- struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *) &out->saddr;
- struct sockaddr_in6 *daddr6 = (struct sockaddr_in6 *) &out->daddr;
+ struct sockaddr_in6 *saddr6 = &out->saddr.u.sin6;
+ struct sockaddr_in6 *daddr6 = &out->daddr.u.sin6;

if (in_len < sizeof(*ip6))
return -1;
@@ -149,7 +149,7 @@
unsigned int head_len = payload - base_buffer;
unsigned int hdr_len_needed;
unsigned int opt_hdr_len_needed = 0;
- struct sockaddr_storage daddr;
+ struct osmo_sockaddr daddr;
int outfd = t->gtp_ep->fd;
int rc;
uint8_t flags;
@@ -208,7 +208,7 @@

/* 4) write to GTP/UDP socket */
rc = sendto(outfd, gtph, hdr_len_needed + payload_len, 0,
- (struct sockaddr *)&daddr, sizeof(daddr));
+ &daddr.u.sa, sizeof(daddr.u.sas));
return rc;
}

@@ -256,19 +256,19 @@
continue;
}

- if (pinfo.saddr.ss_family == AF_INET6 && pinfo.proto == IPPROTO_ICMPV6) {
+ if (pinfo.saddr.u.sa.sa_family == AF_INET6 && pinfo.proto == IPPROTO_ICMPV6) {
/* 2) TODO: magic voodoo for IPv6 neighbor discovery */
}

/* 3) look-up tunnel based on source IP address (+ filter) */
pthread_rwlock_rdlock(&d->rwlock);
- t = _gtp_tunnel_find_eua(tun, (struct sockaddr *) &pinfo.saddr, pinfo.proto);
+ t = _gtp_tunnel_find_eua(tun, &pinfo.saddr, pinfo.proto);
if (!t) {
char host[128];
char port[8];
pthread_rwlock_unlock(&d->rwlock);
- getnameinfo((const struct sockaddr *)&pinfo.saddr,
- sizeof(pinfo.saddr), host, sizeof(host), port, sizeof(port),
+ getnameinfo(&pinfo.saddr.u.sa,
+ sizeof(pinfo.saddr.u.sas), host, sizeof(host), port, sizeof(port),
NI_NUMERICHOST | NI_NUMERICSERV);
LOGTUN_NC(tun, LOGL_NOTICE, "No tunnel found for source address %s:%s\n", host, port);
continue;
diff --git a/daemon/utility.c b/daemon/utility.c
index 1b36a24..3c3fbea 100644
--- a/daemon/utility.c
+++ b/daemon/utility.c
@@ -17,38 +17,6 @@
* Utility
***********************************************************************/

-bool sockaddr_equals(const struct sockaddr *a, const struct sockaddr *b)
-{
- const struct sockaddr_in *a4, *b4;
- const struct sockaddr_in6 *a6, *b6;
-
- if (a->sa_family != b->sa_family)
- return false;
-
- switch (a->sa_family) {
- case AF_INET:
- a4 = (struct sockaddr_in *) a;
- b4 = (struct sockaddr_in *) b;
- if (a4->sin_port != b4->sin_port)
- return false;
- if (a4->sin_addr.s_addr != b4->sin_addr.s_addr)
- return false;
- break;
- case AF_INET6:
- a6 = (struct sockaddr_in6 *) a;
- b6 = (struct sockaddr_in6 *) b;
- if (a6->sin6_port != b6->sin6_port)
- return false;
- if (memcmp(a6->sin6_addr.s6_addr, b6->sin6_addr.s6_addr, sizeof(b6->sin6_addr.s6_addr)))
- return false;
- break;
- default:
- assert(false);
- }
-
- return true;
-}
-
struct addrinfo *addrinfo_helper(uint16_t family, uint16_t type, uint8_t proto,
const char *host, uint16_t port, bool passive)
{

To view, visit change 42440. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: merged
Gerrit-Project: osmo-uecups
Gerrit-Branch: master
Gerrit-Change-Id: Id87d9cf2ec138091681ab0fe7c7f86d648c0e98e
Gerrit-Change-Number: 42440
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de>
Gerrit-Reviewer: osmith <osmith@sysmocom.de>
Gerrit-Reviewer: pespin <pespin@sysmocom.de>