| Message ID | 20190502153757.29038-3-jlayton@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] libceph: make ceph_pr_addr take an struct ceph_entity_addr pointer | expand |
On Thu, 2019-05-02 at 11:37 -0400, Jeff Layton wrote: > GCC9 is throwing a lot of warnings about unaligned access. This patch > fixes them up by changing most of the helper functions to take a > struct ceph_entity_addr instead. > > The exception is when setting the port in a sockaddr_storage. For that > I ended up making a completely separate function to handle setting the > port inside a struct ceph_entity_addr. > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > net/ceph/messenger.c | 55 ++++++++++++++++++++++++++++---------------- > 1 file changed, 35 insertions(+), 20 deletions(-) > > diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c > index 38a4db87c756..2d4ae261011c 100644 > --- a/net/ceph/messenger.c > +++ b/net/ceph/messenger.c > @@ -449,7 +449,7 @@ static void set_sock_callbacks(struct socket *sock, > */ > static int ceph_tcp_connect(struct ceph_connection *con) > { > - struct sockaddr_storage *paddr = &con->peer_addr.in_addr; > + struct sockaddr_storage addr = con->peer_addr.in_addr; > struct socket *sock; > unsigned int noio_flag; > int ret; > @@ -458,7 +458,7 @@ static int ceph_tcp_connect(struct ceph_connection *con) > > /* sock_create_kern() allocates with GFP_KERNEL */ > noio_flag = memalloc_noio_save(); > - ret = sock_create_kern(read_pnet(&con->msgr->net), paddr->ss_family, > + ret = sock_create_kern(read_pnet(&con->msgr->net), addr.ss_family, > SOCK_STREAM, IPPROTO_TCP, &sock); > memalloc_noio_restore(noio_flag); > if (ret) > @@ -474,7 +474,7 @@ static int ceph_tcp_connect(struct ceph_connection *con) > dout("connect %s\n", ceph_pr_addr(&con->peer_addr)); > > con_sock_state_connecting(con); > - ret = sock->ops->connect(sock, (struct sockaddr *)paddr, sizeof(*paddr), > + ret = sock->ops->connect(sock, (struct sockaddr *)&addr, sizeof(addr), > O_NONBLOCK); > if (ret == -EINPROGRESS) { > dout("connect %s EINPROGRESS sk_state = %u\n", > @@ -1794,12 +1794,13 @@ static int verify_hello(struct ceph_connection *con) > return 0; > } > > -static bool addr_is_blank(struct sockaddr_storage *ss) > +static bool addr_is_blank(struct ceph_entity_addr *ea) > { > - struct in_addr *addr = &((struct sockaddr_in *)ss)->sin_addr; > - struct in6_addr *addr6 = &((struct sockaddr_in6 *)ss)->sin6_addr; > + struct sockaddr_storage ss = ea->in_addr; > + struct in_addr *addr = &((struct sockaddr_in *)&ss)->sin_addr; > + struct in6_addr *addr6 = &((struct sockaddr_in6 *)&ss)->sin6_addr; > > - switch (ss->ss_family) { > + switch (ss.ss_family) { > case AF_INET: > return addr->s_addr == htonl(INADDR_ANY); > case AF_INET6: > @@ -1809,18 +1810,20 @@ static bool addr_is_blank(struct sockaddr_storage *ss) > } > } > > -static int addr_port(struct sockaddr_storage *ss) > +static int addr_port(struct ceph_entity_addr *ea) > { > - switch (ss->ss_family) { > + struct sockaddr_storage ss = ea->in_addr; > + > + switch (ss.ss_family) { > case AF_INET: > - return ntohs(((struct sockaddr_in *)ss)->sin_port); > + return ntohs(((struct sockaddr_in *)&ss)->sin_port); > case AF_INET6: > - return ntohs(((struct sockaddr_in6 *)ss)->sin6_port); > + return ntohs(((struct sockaddr_in6 *)&ss)->sin6_port); > } > return 0; > } > > -static void addr_set_port(struct sockaddr_storage *ss, int p) > +static void sockaddr_set_port(struct sockaddr_storage *ss, int p) > { > switch (ss->ss_family) { > case AF_INET: > @@ -1832,6 +1835,18 @@ static void addr_set_port(struct sockaddr_storage *ss, int p) > } > } > > +static void addr_set_port(struct ceph_entity_addr *addr, int p) > +{ > + switch (get_unaligned(&addr->in_addr.ss_family)) { > + case AF_INET: > + put_unaligned(htons(p), &((struct sockaddr_in *)&addr->in_addr)->sin_port); > + break; > + case AF_INET6: > + put_unaligned(htons(p), &((struct sockaddr_in6 *)&addr->in_addr)->sin6_port); > + break; > + } > +} > + > /* > * Unlike other *_pton function semantics, zero indicates success. > */ > @@ -1941,7 +1956,7 @@ int ceph_parse_ips(const char *c, const char *end, > dout("parse_ips on '%.*s'\n", (int)(end-c), c); > for (i = 0; i < max_count; i++) { > const char *ipend; > - struct sockaddr_storage *ss = &addr[i].in_addr; > + struct sockaddr_storage ss = addr[i].in_addr; > int port; > char delim = ','; > > @@ -1950,7 +1965,7 @@ int ceph_parse_ips(const char *c, const char *end, > p++; > } > > - ret = ceph_parse_server_name(p, end - p, ss, delim, &ipend); > + ret = ceph_parse_server_name(p, end - p, &ss, delim, &ipend); > if (ret) > goto bad; > ret = -EINVAL; > @@ -1981,9 +1996,9 @@ int ceph_parse_ips(const char *c, const char *end, > port = CEPH_MON_PORT; > } > > - addr_set_port(ss, port); > + sockaddr_set_port(&ss, port); preemptive self-NAK on this patch, as this is not right. I'll have to fix this part a different way. > > - dout("parse_ips got %s\n", ceph_pr_sockaddr(ss)); > + dout("parse_ips got %s\n", ceph_pr_sockaddr(&ss)); > > if (p == end) > break; > @@ -2022,7 +2037,7 @@ static int process_banner(struct ceph_connection *con) > */ > if (memcmp(&con->peer_addr, &con->actual_peer_addr, > sizeof(con->peer_addr)) != 0 && > - !(addr_is_blank(&con->actual_peer_addr.in_addr) && > + !(addr_is_blank(&con->peer_addr_for_me) && > con->actual_peer_addr.nonce == con->peer_addr.nonce)) { > pr_warn("wrong peer, want %s/%d, got %s/%d\n", > ceph_pr_addr(&con->peer_addr), > @@ -2036,13 +2051,13 @@ static int process_banner(struct ceph_connection *con) > /* > * did we learn our address? > */ > - if (addr_is_blank(&con->msgr->inst.addr.in_addr)) { > - int port = addr_port(&con->msgr->inst.addr.in_addr); > + if (addr_is_blank(&con->msgr->inst.addr)) { > + int port = addr_port(&con->msgr->inst.addr); > > memcpy(&con->msgr->inst.addr.in_addr, > &con->peer_addr_for_me.in_addr, > sizeof(con->peer_addr_for_me.in_addr)); > - addr_set_port(&con->msgr->inst.addr.in_addr, port); > + addr_set_port(&con->msgr->inst.addr, port); > encode_my_addr(con->msgr); > dout("process_banner learned my addr is %s\n", > ceph_pr_addr(&con->msgr->inst.addr));
diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c index 38a4db87c756..2d4ae261011c 100644 --- a/net/ceph/messenger.c +++ b/net/ceph/messenger.c @@ -449,7 +449,7 @@ static void set_sock_callbacks(struct socket *sock, */ static int ceph_tcp_connect(struct ceph_connection *con) { - struct sockaddr_storage *paddr = &con->peer_addr.in_addr; + struct sockaddr_storage addr = con->peer_addr.in_addr; struct socket *sock; unsigned int noio_flag; int ret; @@ -458,7 +458,7 @@ static int ceph_tcp_connect(struct ceph_connection *con) /* sock_create_kern() allocates with GFP_KERNEL */ noio_flag = memalloc_noio_save(); - ret = sock_create_kern(read_pnet(&con->msgr->net), paddr->ss_family, + ret = sock_create_kern(read_pnet(&con->msgr->net), addr.ss_family, SOCK_STREAM, IPPROTO_TCP, &sock); memalloc_noio_restore(noio_flag); if (ret) @@ -474,7 +474,7 @@ static int ceph_tcp_connect(struct ceph_connection *con) dout("connect %s\n", ceph_pr_addr(&con->peer_addr)); con_sock_state_connecting(con); - ret = sock->ops->connect(sock, (struct sockaddr *)paddr, sizeof(*paddr), + ret = sock->ops->connect(sock, (struct sockaddr *)&addr, sizeof(addr), O_NONBLOCK); if (ret == -EINPROGRESS) { dout("connect %s EINPROGRESS sk_state = %u\n", @@ -1794,12 +1794,13 @@ static int verify_hello(struct ceph_connection *con) return 0; } -static bool addr_is_blank(struct sockaddr_storage *ss) +static bool addr_is_blank(struct ceph_entity_addr *ea) { - struct in_addr *addr = &((struct sockaddr_in *)ss)->sin_addr; - struct in6_addr *addr6 = &((struct sockaddr_in6 *)ss)->sin6_addr; + struct sockaddr_storage ss = ea->in_addr; + struct in_addr *addr = &((struct sockaddr_in *)&ss)->sin_addr; + struct in6_addr *addr6 = &((struct sockaddr_in6 *)&ss)->sin6_addr; - switch (ss->ss_family) { + switch (ss.ss_family) { case AF_INET: return addr->s_addr == htonl(INADDR_ANY); case AF_INET6: @@ -1809,18 +1810,20 @@ static bool addr_is_blank(struct sockaddr_storage *ss) } } -static int addr_port(struct sockaddr_storage *ss) +static int addr_port(struct ceph_entity_addr *ea) { - switch (ss->ss_family) { + struct sockaddr_storage ss = ea->in_addr; + + switch (ss.ss_family) { case AF_INET: - return ntohs(((struct sockaddr_in *)ss)->sin_port); + return ntohs(((struct sockaddr_in *)&ss)->sin_port); case AF_INET6: - return ntohs(((struct sockaddr_in6 *)ss)->sin6_port); + return ntohs(((struct sockaddr_in6 *)&ss)->sin6_port); } return 0; } -static void addr_set_port(struct sockaddr_storage *ss, int p) +static void sockaddr_set_port(struct sockaddr_storage *ss, int p) { switch (ss->ss_family) { case AF_INET: @@ -1832,6 +1835,18 @@ static void addr_set_port(struct sockaddr_storage *ss, int p) } } +static void addr_set_port(struct ceph_entity_addr *addr, int p) +{ + switch (get_unaligned(&addr->in_addr.ss_family)) { + case AF_INET: + put_unaligned(htons(p), &((struct sockaddr_in *)&addr->in_addr)->sin_port); + break; + case AF_INET6: + put_unaligned(htons(p), &((struct sockaddr_in6 *)&addr->in_addr)->sin6_port); + break; + } +} + /* * Unlike other *_pton function semantics, zero indicates success. */ @@ -1941,7 +1956,7 @@ int ceph_parse_ips(const char *c, const char *end, dout("parse_ips on '%.*s'\n", (int)(end-c), c); for (i = 0; i < max_count; i++) { const char *ipend; - struct sockaddr_storage *ss = &addr[i].in_addr; + struct sockaddr_storage ss = addr[i].in_addr; int port; char delim = ','; @@ -1950,7 +1965,7 @@ int ceph_parse_ips(const char *c, const char *end, p++; } - ret = ceph_parse_server_name(p, end - p, ss, delim, &ipend); + ret = ceph_parse_server_name(p, end - p, &ss, delim, &ipend); if (ret) goto bad; ret = -EINVAL; @@ -1981,9 +1996,9 @@ int ceph_parse_ips(const char *c, const char *end, port = CEPH_MON_PORT; } - addr_set_port(ss, port); + sockaddr_set_port(&ss, port); - dout("parse_ips got %s\n", ceph_pr_sockaddr(ss)); + dout("parse_ips got %s\n", ceph_pr_sockaddr(&ss)); if (p == end) break; @@ -2022,7 +2037,7 @@ static int process_banner(struct ceph_connection *con) */ if (memcmp(&con->peer_addr, &con->actual_peer_addr, sizeof(con->peer_addr)) != 0 && - !(addr_is_blank(&con->actual_peer_addr.in_addr) && + !(addr_is_blank(&con->peer_addr_for_me) && con->actual_peer_addr.nonce == con->peer_addr.nonce)) { pr_warn("wrong peer, want %s/%d, got %s/%d\n", ceph_pr_addr(&con->peer_addr), @@ -2036,13 +2051,13 @@ static int process_banner(struct ceph_connection *con) /* * did we learn our address? */ - if (addr_is_blank(&con->msgr->inst.addr.in_addr)) { - int port = addr_port(&con->msgr->inst.addr.in_addr); + if (addr_is_blank(&con->msgr->inst.addr)) { + int port = addr_port(&con->msgr->inst.addr); memcpy(&con->msgr->inst.addr.in_addr, &con->peer_addr_for_me.in_addr, sizeof(con->peer_addr_for_me.in_addr)); - addr_set_port(&con->msgr->inst.addr.in_addr, port); + addr_set_port(&con->msgr->inst.addr, port); encode_my_addr(con->msgr); dout("process_banner learned my addr is %s\n", ceph_pr_addr(&con->msgr->inst.addr));
GCC9 is throwing a lot of warnings about unaligned access. This patch fixes them up by changing most of the helper functions to take a struct ceph_entity_addr instead. The exception is when setting the port in a sockaddr_storage. For that I ended up making a completely separate function to handle setting the port inside a struct ceph_entity_addr. Signed-off-by: Jeff Layton <jlayton@kernel.org> --- net/ceph/messenger.c | 55 ++++++++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 20 deletions(-)