netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
To: Cong Wang <amwang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	"David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>,
	Trond Myklebust
	<Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [Patch net-next 4/7] sunrpc: use generic union inet_addr
Date: Tue, 23 Jul 2013 12:40:51 -0400	[thread overview]
Message-ID: <20130723164051.GE12569@fieldses.org> (raw)
In-Reply-To: <1374476713-8838-5-git-send-email-amwang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Mon, Jul 22, 2013 at 03:05:10PM +0800, Cong Wang wrote:
> From: Cong Wang <amwang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> 
> sunrpc defines some helper functions for sockaddr, actually they
> can re-use the generic functions for union inet_addr too.

Happy to see that moved into common code....

> 
> Cc: Trond Myklebust <Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
> Cc: "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
> Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Cong Wang <amwang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  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);
>  }

Is there any reason to keep the rpc_get_port wrapper at all after this?
Or if its still useful to have the convenience of not having to do the
cast, maybe the wrapper should move to common code to?  (Is there some
reason only the rpc code needs this?)

--b.

>  
>  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 3416f65..b3c59d7 100644
> --- a/include/net/inet_addr.h
> +++ b/include/net/inet_addr.h
> @@ -14,17 +14,6 @@ union inet_addr {
>  };
>  
>  #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)
> @@ -43,12 +32,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);
> @@ -60,6 +43,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;
> +	}
> +}
> +
> +extern bool inet_addr_equal(const union inet_addr *a, const union inet_addr *b);
>  extern 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);
> -- 
> 1.7.7.6
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2013-07-23 16:40 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-22  7:05 [Patch net-next 0/7] net: introduce generic type and helpers for IP address Cong Wang
2013-07-22  7:05 ` [Patch net-next 1/7] net: introduce generic union inet_addr Cong Wang
2013-07-22  7:05 ` [Patch net-next 2/7] net: introduce generic simple_inet_pton() Cong Wang
2013-07-22  7:05 ` [Patch net-next 3/7] inetpeer: use generic union inet_addr Cong Wang
2013-07-22 15:18   ` Eric Dumazet
2013-07-23  2:05     ` Cong Wang
2013-07-23  2:26       ` Eric Dumazet
2013-07-23  3:38         ` Cong Wang
2013-07-22  7:05 ` [Patch net-next 4/7] sunrpc: " Cong Wang
     [not found]   ` <1374476713-8838-5-git-send-email-amwang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2013-07-23 16:40     ` J. Bruce Fields [this message]
     [not found]       ` <20130723164051.GE12569-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2013-07-25 12:34         ` Cong Wang
2013-07-25 12:54           ` J. Bruce Fields
2013-07-22  7:05 ` [Patch net-next 5/7] fs: use generic union inet_addr and help functions Cong Wang
2013-07-22  7:05 ` [Patch net-next 6/7] sctp: use generic union inet_addr Cong Wang
2013-07-22  7:05 ` [Patch net-next 7/7] selinux: " Cong Wang
2013-07-22 20:36   ` Paul Moore
2013-07-23  2:07     ` Cong Wang
2013-07-22 20:44 ` [Patch net-next 0/7] net: introduce generic type and helpers for IP address Joe Perches
2013-07-23  2:00   ` Cong Wang
2013-07-23  2:16     ` Joe Perches
2013-07-23  2:26       ` Cong Wang
2013-07-24 21:28 ` David Miller
2013-07-25 12:30   ` Cong Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130723164051.GE12569@fieldses.org \
    --to=bfields-uc3wqj2krung9huczpvpmw@public.gmane.org \
    --cc=Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org \
    --cc=amwang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org \
    --cc=linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).