From f159a33754215eac82b26912bce5592294f9a989 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Fri, 21 Jun 2013 03:34:31 +0000 Subject: shrink mog_packaddr and improve portability We cannot assume sa_family_t is the first element of "struct sockaddr_in" or "struct sockaddr_in6". FreeBSD has a "sa_len" member as the first element while Linux does not. So only keep the parts of the "struct sockaddr*" we need and use inet_ntop instead of getnameinfo. This also gives us a little more space to add additional fields to "struct mog_http" in the future without increasing memory (or CPU cache) use. --- Makefile.am | 1 - accept_loop.c | 2 +- cmogstored.h | 11 ++++++----- http.c | 25 +++++++++++++------------ http_put.c | 12 +++--------- inherit.c | 12 ++++++++---- mgmt.c | 13 +++++++------ nameinfo.c | 43 ++++++++++++++++++++++--------------------- packaddr.c | 21 --------------------- packaddr.h | 41 +++++++++++++++++++++++------------------ 10 files changed, 83 insertions(+), 98 deletions(-) delete mode 100644 packaddr.c diff --git a/Makefile.am b/Makefile.am index ee8a442..a0c0989 100644 --- a/Makefile.am +++ b/Makefile.am @@ -70,7 +70,6 @@ mog_src += nameinfo.c mog_src += nostd/setproctitle.h mog_src += notify.c mog_src += notify.h -mog_src += packaddr.c mog_src += packaddr.h mog_src += path_parser.h mog_src += pidfile.c diff --git a/accept_loop.c b/accept_loop.c index dd9c929..874e6d7 100644 --- a/accept_loop.c +++ b/accept_loop.c @@ -98,7 +98,7 @@ void *mog_accept_loop(void *arg) client_fd = mog_accept_fn(accept_fd, sa, &salen); if (client_fd >= 0) - ac->post_accept_fn(client_fd, ac->svc, sa, salen); + ac->post_accept_fn(client_fd, ac->svc, &msa, salen); else accept_error_check(ac); } diff --git a/cmogstored.h b/cmogstored.h index 7b1f164..580de8d 100644 --- a/cmogstored.h +++ b/cmogstored.h @@ -235,7 +235,7 @@ struct mog_queue { /* accept.c */ typedef void (*mog_post_accept_fn)(int fd, struct mog_svc *, - struct sockaddr *, socklen_t); + union mog_sockaddr *, socklen_t); struct mog_accept { struct mog_svc *svc; mog_post_accept_fn post_accept_fn; @@ -414,7 +414,7 @@ void mog_thrpool_set_size(struct mog_thrpool *, size_t size); /* mgmt.c */ void mog_mgmt_writev(struct mog_mgmt *, struct iovec *, int iovcnt); void mog_mgmt_post_accept(int fd, struct mog_svc *, - struct sockaddr *, socklen_t); + union mog_sockaddr *, socklen_t); enum mog_next mog_mgmt_queue_step(struct mog_fd *) MOG_CHECK; void mog_mgmt_quit_step(struct mog_fd *); @@ -469,9 +469,9 @@ enum mog_next mog_http_get_in_progress(struct mog_fd *); /* http.c */ void mog_http_post_accept(int fd, struct mog_svc *, - struct sockaddr *, socklen_t); + union mog_sockaddr *, socklen_t); void mog_httpget_post_accept(int fd, struct mog_svc *, - struct sockaddr *, socklen_t); + union mog_sockaddr *, socklen_t); enum mog_next mog_http_queue_step(struct mog_fd *) MOG_CHECK; void mog_http_quit_step(struct mog_fd *); char *mog_http_path(struct mog_http *, char *buf); @@ -582,6 +582,7 @@ pid_t mog_upgrade_spawn(void); /* exit.c */ _Noreturn void cmogstored_exit(void); +verify(sizeof(in_port_t) <= sizeof(uint16_t)); /* * We only deal with ipv4 and ipv6 addresses (and no human-friendly * hostnames/service names), so we can use smaller constants than the @@ -594,4 +595,4 @@ struct mog_ni { }; /* nameinfo.c */ -int mog_nameinfo(struct sockaddr *, socklen_t, struct mog_ni *); +void mog_nameinfo(struct mog_packaddr *, struct mog_ni *); diff --git a/http.c b/http.c index 3f20f60..74ca982 100644 --- a/http.c +++ b/http.c @@ -279,46 +279,47 @@ void mog_http_quit_step(struct mog_fd *mfd) /* stringify the address for tracers */ static MOG_NOINLINE void -trace_http_accepted(struct mog_fd *mfd, struct sockaddr *sa, socklen_t salen) +trace_http_accepted(struct mog_fd *mfd) { #ifdef HAVE_SYSTEMTAP + struct mog_packaddr *mpa = &mfd->as.http.mpa; struct mog_ni ni; - int rc = mog_nameinfo(sa, salen, &ni); - const char *host = rc == 0 ? ni.ni_host : gai_strerror(rc); - TRACE(CMOGSTORED_HTTP_ACCEPTED(mfd->fd, host, ni.ni_serv)); + mog_nameinfo(mpa, &ni); + TRACE(CMOGSTORED_HTTP_ACCEPTED(mfd->fd, ni.ni_host, ni.ni_serv)); #endif /* !HAVE_SYSTEMTAP */ } static void http_post_accept_common(struct mog_fd *mfd, struct mog_svc *svc, - struct sockaddr *sa, socklen_t salen) + union mog_sockaddr *msa, socklen_t salen) { struct mog_http *http = &mfd->as.http; + mog_http_init(http, svc); + mog_packaddr_init(&http->mpa, msa, salen); + if (TRACE_ENABLED(CMOGSTORED_HTTP_ACCEPTED)) - trace_http_accepted(mfd, sa, salen); + trace_http_accepted(mfd); - mog_http_init(http, svc); - mog_packaddr_init(&http->mpa, sa, salen); mog_idleq_add(svc->queue, mfd, MOG_QEV_RD); } /* called immediately after accept(), this initializes the mfd (once) */ void mog_http_post_accept(int fd, struct mog_svc *svc, - struct sockaddr *sa, socklen_t salen) + union mog_sockaddr *msa, socklen_t salen) { struct mog_fd *mfd = mog_fd_init(fd, MOG_FD_TYPE_HTTP); - http_post_accept_common(mfd, svc, sa, salen); + http_post_accept_common(mfd, svc, msa, salen); } /* called immediately after accept(), this initializes the mfd (once) */ void mog_httpget_post_accept(int fd, struct mog_svc *svc, - struct sockaddr *sa, socklen_t salen) + union mog_sockaddr *msa, socklen_t salen) { struct mog_fd *mfd = mog_fd_init(fd, MOG_FD_TYPE_HTTPGET); - http_post_accept_common(mfd, svc, sa, salen); + http_post_accept_common(mfd, svc, msa, salen); } /* diff --git a/http_put.c b/http_put.c index 9974c93..3ff7166 100644 --- a/http_put.c +++ b/http_put.c @@ -444,18 +444,12 @@ MOG_NOINLINE static void read_err_dbg(struct mog_fd *mfd, ssize_t r) { int save_errno = errno; struct mog_ni ni; - struct sockaddr *sa; - const char *addr; const char *path = "(unknown)"; long long bytes = -1; const char *errfmt; - socklen_t len; unsigned last = last_data_recv(mfd->fd); - int rc; - mog_packaddr_read(&mfd->as.http.mpa, &sa, &len); - rc = mog_nameinfo(sa, len, &ni); - addr = rc == 0 ? ni.ni_host : gai_strerror(rc); + mog_nameinfo(&mfd->as.http.mpa, &ni); if (mfd->as.http.forward) { path = mfd->as.http.forward->as.file.path; @@ -466,11 +460,11 @@ MOG_NOINLINE static void read_err_dbg(struct mog_fd *mfd, ssize_t r) errfmt = (r == 0) ? PFX"premature EOF" : PFX"%m"; #undef PFX errno = save_errno; - syslog(LOG_ERR, errfmt, path, addr, ni.ni_serv, bytes); + syslog(LOG_ERR, errfmt, path, ni.ni_host, ni.ni_serv, bytes); if (last != (unsigned)-1) syslog(LOG_ERR, "last_data_recv=%ums from %s%s for PUT %s", - last, addr, ni.ni_serv, path); + last, ni.ni_host, ni.ni_serv, path); } static enum mog_next identity_put_in_progress(struct mog_fd *mfd) diff --git a/inherit.c b/inherit.c index 75d3050..d2f67f0 100644 --- a/inherit.c +++ b/inherit.c @@ -37,16 +37,20 @@ static void register_listen_fd(int fd) struct listener tmp; struct listener *ins; struct mog_ni ni; - int rc; + struct mog_packaddr mpa; struct sockaddr *sa = mog_sockaddr_sa(&tmp.msa); tmp.len = (socklen_t)sizeof(tmp.msa); if (getsockname(fd, sa, &tmp.len) != 0) die_errno("getsockname(fd=%d) failed", fd); - rc = mog_nameinfo(sa, tmp.len, &ni); - if (rc != 0) - die("getnameinfo failed: %s (fd=%d)", gai_strerror(rc), fd); + if (sa->sa_family != AF_INET && sa->sa_family != AF_INET6) + die("invalid address family=%d (not AF_INET/AF_INET6)", + (int)sa->sa_family); + + mog_packaddr_init(&mpa, &tmp.msa, tmp.len); + + mog_nameinfo(&mpa, &ni); syslog(LOG_INFO, "inherited %s%s on fd=%d", ni.ni_host, ni.ni_serv, fd); diff --git a/mgmt.c b/mgmt.c index 503398d..9909d27 100644 --- a/mgmt.c +++ b/mgmt.c @@ -275,26 +275,27 @@ void mog_mgmt_quit_step(struct mog_fd *mfd) /* stringify the address for tracers */ static MOG_NOINLINE void -trace_mgmt_accepted(struct mog_fd *mfd, struct sockaddr *sa, socklen_t salen) +trace_mgmt_accepted( + struct mog_fd *mfd, union mog_sockaddr *msa, socklen_t salen) { #ifdef HAVE_SYSTEMTAP + struct mog_packaddr mpa; struct mog_ni ni; - int rc = mog_nameinfo(sa, salen, &ni); - const char *host = rc == 0 ? ni.ni_host : gai_strerror(rc); - TRACE(CMOGSTORED_MGMT_ACCEPTED(mfd->fd, host, ni.ni_serv)); + mog_nameinfo(&mpa, &ni); + TRACE(CMOGSTORED_MGMT_ACCEPTED(mfd->fd, ni.ni_host, ni.ni_serv)); #endif /* !HAVE_SYSTEMTAP */ } /* called immediately after accept(), this initializes the mfd (once) */ void mog_mgmt_post_accept(int fd, struct mog_svc *svc, - struct sockaddr *sa, socklen_t salen) + union mog_sockaddr *msa, socklen_t salen) { struct mog_fd *mfd = mog_fd_init(fd, MOG_FD_TYPE_MGMT); struct mog_mgmt *mgmt = &mfd->as.mgmt; if (TRACE_ENABLED(CMOGSTORED_MGMT_ACCEPTED)) - trace_mgmt_accepted(mfd, sa, salen); + trace_mgmt_accepted(mfd, msa, salen); mog_mgmt_init(mgmt, svc); mog_idleq_add(svc->queue, mfd, MOG_QEV_RD); diff --git a/nameinfo.c b/nameinfo.c index 5d9122f..f5af802 100644 --- a/nameinfo.c +++ b/nameinfo.c @@ -3,45 +3,46 @@ * License: GPLv3 or later (see COPYING for details) */ #include "cmogstored.h" +#include /* - * small wrapper around getnameinfo(3), this only handles numeric types + * small replacement for getnameinfo(3), this only handles numeric types * for IPv4 and IPv6 and uses the compact mog_ni structure to reduce * stack usage in error reporting. - * - * returns the return value of getnameinfo(3) */ -int mog_nameinfo(struct sockaddr *sa, socklen_t len, struct mog_ni *ni) +void mog_nameinfo(struct mog_packaddr *mpa, struct mog_ni *ni) { char *hostptr = ni->ni_host; size_t hostlen = sizeof(ni->ni_host) - (sizeof("[]") - 1); - char *servptr = ni->ni_serv + 1; /* offset for ':' */ size_t servlen = sizeof(ni->ni_serv) - 1; /* offset for ':' */ - - int flags = NI_NUMERICHOST | NI_NUMERICSERV; int rc; + const void *src; + const char *ret; - if (sa->sa_family == AF_INET6) { + if (mpa->sa_family == AF_INET6) { hostptr[0] = '['; /* leading '[' */ + src = mpa->as.in6_ptr; hostptr++; + } else { + assert(mpa->sa_family == AF_INET && "bad family"); + src = &mpa->as.in_addr; } - rc = getnameinfo(sa, len, hostptr, hostlen, servptr, servlen, flags); + ret = inet_ntop(mpa->sa_family, src, hostptr, (socklen_t)hostlen); /* terminate serv string on error */ - if (rc != 0) { - ni->ni_serv[0] = 0; - } else { - ni->ni_serv[0] = ':'; - - /* add trailing ']' */ - if (sa->sa_family == AF_INET6) { - hostlen = strlen(hostptr); - hostptr[hostlen] = ']'; - hostptr[hostlen + 1] = 0; - } + assert(ret == hostptr && "inet_ntop"); + ni->ni_serv[0] = ':'; + + /* add trailing ']' */ + if (mpa->sa_family == AF_INET6) { + hostlen = strlen(hostptr); + hostptr[hostlen] = ']'; + hostptr[hostlen + 1] = 0; } - return rc; + /* port number */ + rc = snprintf(servptr, servlen, "%u", (unsigned)ntohs(mpa->port)); + assert(rc > 0 && rc < (int)servlen && "we suck at snprintf"); } diff --git a/packaddr.c b/packaddr.c deleted file mode 100644 index 75450d2..0000000 --- a/packaddr.c +++ /dev/null @@ -1,21 +0,0 @@ -/* - * Copyright (C) 2012-2013, Eric Wong - * License: GPLv3 or later (see COPYING for details) - */ -#include "cmogstored.h" - -void mog_packaddr_read(struct mog_packaddr *mpa, - struct sockaddr **sa, socklen_t *salen) -{ - switch (mpa->as.sa_family) { - case AF_INET: - *salen = (socklen_t)sizeof(struct sockaddr_in); - *sa = (struct sockaddr *)&mpa->as.in4; - return; - case AF_INET6: - *salen = (socklen_t)sizeof(struct sockaddr_in6); - *sa = mog_sockaddr_sa(mpa->as.msa.ptr); - return; - } - assert(0 && "unrecognized sa_family" && mpa->as.sa_family); -} diff --git a/packaddr.h b/packaddr.h index 4c9d880..fdfb9cb 100644 --- a/packaddr.h +++ b/packaddr.h @@ -3,9 +3,11 @@ * License: GPLv3 or later (see COPYING for details) */ -/* avoid sockaddr_storage since that bigger than we need */ +/* + * avoid sockaddr_storage since that bigger than we need + * This is meant to be cast to "struct sockaddr" via mog_sockaddr_sa + */ union mog_sockaddr { - sa_family_t sa_family; struct sockaddr_in in; struct sockaddr_in6 in6; uint8_t bytes[sizeof(struct sockaddr_in6)]; @@ -17,33 +19,36 @@ static inline struct sockaddr *mog_sockaddr_sa(union mog_sockaddr *msa) return (struct sockaddr *)msa; } +/* this is the relevant part we may store in "struct mog_fd" */ struct mog_packaddr { + sa_family_t sa_family; + in_port_t port; union { - sa_family_t sa_family; - struct sockaddr_in in4; - struct { - sa_family_t sa_family; /* padding */ - union mog_sockaddr *ptr; - } msa; + struct in6_addr *in6_ptr; + struct in_addr in_addr; } as; } __attribute__((packed)); static inline void mog_packaddr_free(struct mog_packaddr *mpa) { - if (mpa->as.sa_family != AF_INET) - free(mpa->as.msa.ptr); + if (mpa->sa_family != AF_INET) + free(mpa->as.in6_ptr); } static inline void mog_packaddr_init(struct mog_packaddr *mpa, - struct sockaddr *sa, socklen_t salen) + union mog_sockaddr *msa, socklen_t salen) { - if (sa->sa_family == AF_INET) { - memcpy(&mpa->as.in4, sa, (size_t)salen); + mpa->sa_family = msa->in.sin_family; + + if (mpa->sa_family == AF_INET) { + mpa->port = msa->in.sin_port; + memcpy(&mpa->as.in_addr, &msa->in.sin_addr, + sizeof(struct in_addr)); } else { - mpa->as.sa_family = sa->sa_family; - mpa->as.msa.ptr = xmalloc(sizeof(union mog_sockaddr)); - memcpy(mpa->as.msa.ptr, sa, (size_t)salen); + assert(mpa->sa_family == AF_INET6 && "invalid sa_family"); + mpa->port = msa->in6.sin6_port; + mpa->as.in6_ptr = xmalloc(sizeof(struct in6_addr)); + memcpy(mpa->as.in6_ptr, &msa->in6.sin6_addr, + sizeof(struct in6_addr)); } } - -void mog_packaddr_read(struct mog_packaddr *, struct sockaddr **, socklen_t *); -- cgit v1.2.3-24-ge0c7