[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130802093625.2c70a330@tlielax.poochiereds.net>
Date: Fri, 2 Aug 2013 09:36:25 -0400
From: Jeff Layton <jlayton@...hat.com>
To: Cong Wang <amwang@...hat.com>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
Trond Myklebust <Trond.Myklebust@...app.com>,
"J. Bruce Fields" <bfields@...ldses.org>, linux-nfs@...r.kernel.org
Subject: Re: [Patch net-next v2 5/8] sunrpc: use generic union inet_addr
On Fri, 2 Aug 2013 15:14:31 +0800
Cong Wang <amwang@...hat.com> wrote:
> From: Cong Wang <amwang@...hat.com>
>
> sunrpc defines some helper functions for sockaddr, actually they
> can re-use the generic functions for union inet_addr too.
Only some of these patches in this series have made it to lists to
which I'm subscribed, so I may be missing some context here...
I'm not sure I really understand the value of "union inet_addr". Why
not just use the conventional method of passing around "struct sockaddr"
pointers, and then casting them to struct sockaddr_in/sockaddr_in6
depending on what the sa_family is set to?
With that you wouldn't need to leave the (now pointless) rpc_* wrappers
in place and could just call your new helpers directly.
>
> Cc: Trond Myklebust <Trond.Myklebust@...app.com>
> Cc: "J. Bruce Fields" <bfields@...ldses.org>
> Cc: linux-nfs@...r.kernel.org
> Signed-off-by: Cong Wang <amwang@...hat.com>
> ---
> include/linux/sunrpc/addr.h | 118 +++----------------------------------------
> include/net/inet_addr.h | 74 +++++++++++++++++++++------
> net/core/utils.c | 25 +++++++++
> 3 files changed, 89 insertions(+), 128 deletions(-)
>
> diff --git a/include/linux/sunrpc/addr.h b/include/linux/sunrpc/addr.h
> index 07d8e53..10d07f6 100644
> --- a/include/linux/sunrpc/addr.h
> +++ b/include/linux/sunrpc/addr.h
> @@ -11,6 +11,7 @@
> #include <linux/in.h>
> #include <linux/in6.h>
> #include <net/ipv6.h>
> +#include <net/inet_addr.h>
>
> size_t rpc_ntop(const struct sockaddr *, char *, const size_t);
> size_t rpc_pton(struct net *, const char *, const size_t,
> @@ -21,135 +22,30 @@ size_t rpc_uaddr2sockaddr(struct net *, const char *, const size_t,
>
> static inline unsigned short rpc_get_port(const struct sockaddr *sap)
> {
> - switch (sap->sa_family) {
> - case AF_INET:
> - return ntohs(((struct sockaddr_in *)sap)->sin_port);
> - case AF_INET6:
> - return ntohs(((struct sockaddr_in6 *)sap)->sin6_port);
> - }
> - return 0;
> + return inet_addr_get_port((const union inet_addr *)sap);
> }
>
> static inline void rpc_set_port(struct sockaddr *sap,
> const unsigned short port)
> {
> - switch (sap->sa_family) {
> - case AF_INET:
> - ((struct sockaddr_in *)sap)->sin_port = htons(port);
> - break;
> - case AF_INET6:
> - ((struct sockaddr_in6 *)sap)->sin6_port = htons(port);
> - break;
> - }
> + inet_addr_set_port((union inet_addr *)sap, port);
> }
>
> #define IPV6_SCOPE_DELIMITER '%'
> #define IPV6_SCOPE_ID_LEN sizeof("%nnnnnnnnnn")
>
> -static inline bool __rpc_cmp_addr4(const struct sockaddr *sap1,
> - const struct sockaddr *sap2)
> -{
> - const struct sockaddr_in *sin1 = (const struct sockaddr_in *)sap1;
> - const struct sockaddr_in *sin2 = (const struct sockaddr_in *)sap2;
> -
> - return sin1->sin_addr.s_addr == sin2->sin_addr.s_addr;
> -}
> -
> -static inline bool __rpc_copy_addr4(struct sockaddr *dst,
> - const struct sockaddr *src)
> -{
> - const struct sockaddr_in *ssin = (struct sockaddr_in *) src;
> - struct sockaddr_in *dsin = (struct sockaddr_in *) dst;
> -
> - dsin->sin_family = ssin->sin_family;
> - dsin->sin_addr.s_addr = ssin->sin_addr.s_addr;
> - return true;
> -}
> -
> -#if IS_ENABLED(CONFIG_IPV6)
> -static inline bool __rpc_cmp_addr6(const struct sockaddr *sap1,
> - const struct sockaddr *sap2)
> -{
> - const struct sockaddr_in6 *sin1 = (const struct sockaddr_in6 *)sap1;
> - const struct sockaddr_in6 *sin2 = (const struct sockaddr_in6 *)sap2;
> -
> - if (!ipv6_addr_equal(&sin1->sin6_addr, &sin2->sin6_addr))
> - return false;
> - else if (ipv6_addr_type(&sin1->sin6_addr) & IPV6_ADDR_LINKLOCAL)
> - return sin1->sin6_scope_id == sin2->sin6_scope_id;
> -
> - return true;
> -}
> -
> -static inline bool __rpc_copy_addr6(struct sockaddr *dst,
> - const struct sockaddr *src)
> -{
> - const struct sockaddr_in6 *ssin6 = (const struct sockaddr_in6 *) src;
> - struct sockaddr_in6 *dsin6 = (struct sockaddr_in6 *) dst;
> -
> - dsin6->sin6_family = ssin6->sin6_family;
> - dsin6->sin6_addr = ssin6->sin6_addr;
> - dsin6->sin6_scope_id = ssin6->sin6_scope_id;
> - return true;
> -}
> -#else /* !(IS_ENABLED(CONFIG_IPV6) */
> -static inline bool __rpc_cmp_addr6(const struct sockaddr *sap1,
> - const struct sockaddr *sap2)
> -{
> - return false;
> -}
> -
> -static inline bool __rpc_copy_addr6(struct sockaddr *dst,
> - const struct sockaddr *src)
> -{
> - return false;
> -}
> -#endif /* !(IS_ENABLED(CONFIG_IPV6) */
> -
> -/**
> - * rpc_cmp_addr - compare the address portion of two sockaddrs.
> - * @sap1: first sockaddr
> - * @sap2: second sockaddr
> - *
> - * Just compares the family and address portion. Ignores port, but
> - * compares the scope if it's a link-local address.
> - *
> - * Returns true if the addrs are equal, false if they aren't.
> - */
> static inline bool rpc_cmp_addr(const struct sockaddr *sap1,
> const struct sockaddr *sap2)
> {
> - if (sap1->sa_family == sap2->sa_family) {
> - switch (sap1->sa_family) {
> - case AF_INET:
> - return __rpc_cmp_addr4(sap1, sap2);
> - case AF_INET6:
> - return __rpc_cmp_addr6(sap1, sap2);
> - }
> - }
> - return false;
> + return inet_addr_equal((const union inet_addr *)sap1,
> + (const union inet_addr *)sap2);
> }
>
> -/**
> - * rpc_copy_addr - copy the address portion of one sockaddr to another
> - * @dst: destination sockaddr
> - * @src: source sockaddr
> - *
> - * Just copies the address portion and family. Ignores port, scope, etc.
> - * Caller is responsible for making certain that dst is large enough to hold
> - * the address in src. Returns true if address family is supported. Returns
> - * false otherwise.
> - */
> static inline bool rpc_copy_addr(struct sockaddr *dst,
> const struct sockaddr *src)
> {
> - switch (src->sa_family) {
> - case AF_INET:
> - return __rpc_copy_addr4(dst, src);
> - case AF_INET6:
> - return __rpc_copy_addr6(dst, src);
> - }
> - return false;
> + return inet_addr_copy((union inet_addr *)dst,
> + (const union inet_addr *)src);
> }
>
> /**
> diff --git a/include/net/inet_addr.h b/include/net/inet_addr.h
> index ee80df4..e846050 100644
> --- a/include/net/inet_addr.h
> +++ b/include/net/inet_addr.h
> @@ -36,17 +36,6 @@ bool in_addr_gen_equal(const struct in_addr_gen *a, const struct in_addr_gen *b)
> }
>
> #if IS_ENABLED(CONFIG_IPV6)
> -static inline
> -bool inet_addr_equal(const union inet_addr *a, const union inet_addr *b)
> -{
> - if (a->sa.sa_family != b->sa.sa_family)
> - return false;
> - if (a->sa.sa_family == AF_INET6)
> - return ipv6_addr_equal(&a->sin6.sin6_addr, &b->sin6.sin6_addr);
> - else
> - return a->sin.sin_addr.s_addr == b->sin.sin_addr.s_addr;
> -}
> -
> static inline bool inet_addr_any(const union inet_addr *ipa)
> {
> if (ipa->sa.sa_family == AF_INET6)
> @@ -65,12 +54,6 @@ static inline bool inet_addr_multicast(const union inet_addr *ipa)
>
> #else /* !CONFIG_IPV6 */
>
> -static inline
> -bool inet_addr_equal(const union inet_addr *a, const union inet_addr *b)
> -{
> - return a->sin.sin_addr.s_addr == b->sin.sin_addr.s_addr;
> -}
> -
> static inline bool inet_addr_any(const union inet_addr *ipa)
> {
> return ipa->sin.sin_addr.s_addr == htonl(INADDR_ANY);
> @@ -82,6 +65,63 @@ static inline bool inet_addr_multicast(const union inet_addr *ipa)
> }
> #endif
>
> +/**
> + * inet_addr_copy - copy the address portion of one inet_addr to another
> + * @dst: destination sockaddr
> + * @src: source sockaddr
> + *
> + * Just copies the address portion and family. Ignores port, scope, etc.
> + * Caller is responsible for making certain that dst is large enough to hold
> + * the address in src. Returns true if address family is supported. Returns
> + * false otherwise.
> + */
> +static inline bool inet_addr_copy(union inet_addr *dst,
> + const union inet_addr *src)
> +{
> + dst->sa.sa_family = src->sa.sa_family;
> +
> + switch (src->sa.sa_family) {
> + case AF_INET:
> + dst->sin.sin_addr.s_addr = src->sin.sin_addr.s_addr;
> + return true;
> +#if IS_ENABLED(CONFIG_IPV6)
> + case AF_INET6:
> + dst->sin6.sin6_addr = src->sin6.sin6_addr;
> + dst->sin6.sin6_scope_id = src->sin6.sin6_scope_id;
> + return true;
> +#endif
> + }
> +
> + return false;
> +}
> +
> +static inline
> +unsigned short inet_addr_get_port(const union inet_addr *sap)
> +{
> + switch (sap->sa.sa_family) {
> + case AF_INET:
> + return ntohs(sap->sin.sin_port);
> + case AF_INET6:
> + return ntohs(sap->sin6.sin6_port);
> + }
> + return 0;
> +}
> +
> +static inline
> +void inet_addr_set_port(union inet_addr *sap,
> + const unsigned short port)
> +{
> + switch (sap->sa.sa_family) {
> + case AF_INET:
> + sap->sin.sin_port = htons(port);
> + break;
> + case AF_INET6:
> + sap->sin6.sin6_port = htons(port);
> + break;
> + }
> +}
> +
> +bool inet_addr_equal(const union inet_addr *a, const union inet_addr *b);
> int simple_inet_pton(const char *str, union inet_addr *addr);
>
> #endif
> diff --git a/net/core/utils.c b/net/core/utils.c
> index 22dd621..837bb18 100644
> --- a/net/core/utils.c
> +++ b/net/core/utils.c
> @@ -374,3 +374,28 @@ int simple_inet_pton(const char *str, union inet_addr *addr)
> return -EINVAL;
> }
> EXPORT_SYMBOL(simple_inet_pton);
> +
> +#if IS_ENABLED(CONFIG_IPV6)
> +bool inet_addr_equal(const union inet_addr *a, const union inet_addr *b)
> +{
> + if (a->sa.sa_family != b->sa.sa_family)
> + return false;
> + else if (a->sa.sa_family == AF_INET6) {
> + if (!ipv6_addr_equal(&a->sin6.sin6_addr, &b->sin6.sin6_addr))
> + return false;
> + else if (__ipv6_addr_needs_scope_id(__ipv6_addr_type(&a->sin6.sin6_addr)))
> + return a->sin6.sin6_scope_id == b->sin6.sin6_scope_id;
> + else
> + return true;
> + } else
> + return a->sin.sin_addr.s_addr == b->sin.sin_addr.s_addr;
> +}
> +#else
> +bool inet_addr_equal(const union inet_addr *a, const union inet_addr *b)
> +{
> + if (a->sa.sa_family == AF_UNSPEC)
> + return a->sa.sa_family == b->sa.sa_family;
> + return a->sin.sin_addr.s_addr == b->sin.sin_addr.s_addr;
> +}
> +#endif
> +EXPORT_SYMBOL(inet_addr_equal);
--
Jeff Layton <jlayton@...hat.com>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists