Re: Setting sin/sin6 port to 0 before ifa_ifwithaddr()

[David Young <dyoung%pobox.com@localhost>]

On Tue, Apr 14, 2009 at 12:43:48PM +0300, Elad Efrat wrote:
> Hey
>
> The netinet and netinet6 both have code like the following:
>
>     254  sin6->sin6_port = 0;         /* yech... */
>     255  if ((in6p->in6p_flags & IN6P_FAITH) == 0 &&
>     256      (ia = ifa_ifwithaddr((struct sockaddr *)sin6)) == 0)
>     257          return (EADDRNOTAVAIL);
>
> because at some point in the past ifa_ifwithaddr() used to do a
> simple byte compare, including the port.
>
> Perhaps we can get rid of it? Please see attached diff.
>
> Thanks,
>
> -e.

> Index: netinet/in_pcb.c
> ===================================================================
> RCS file: /usr/cvs/src/sys/netinet/in_pcb.c,v
> retrieving revision 1.130
> diff -u -p -r1.130 in_pcb.c
> --- netinet/in_pcb.c  18 Mar 2009 16:00:22 -0000      1.130
> +++ netinet/in_pcb.c  13 Apr 2009 15:44:39 -0000
> @@ -262,7 +262,6 @@ in_pcbbind(void *v, struct mbuf *nam, st
>               if (so->so_options & SO_REUSEADDR)
>                       reuseport = SO_REUSEADDR|SO_REUSEPORT;
>       } else if (!in_nullhost(sin->sin_addr)) {
> -             sin->sin_port = 0;              /* yech... */
>               INADDR_TO_IA(sin->sin_addr, ia);
>               /* check for broadcast addresses */
>               if (ia == NULL)
> Index: netinet6/in6_pcb.c
> ===================================================================
> RCS file: /usr/cvs/src/sys/netinet6/in6_pcb.c,v
> retrieving revision 1.101
> diff -u -p -r1.101 in6_pcb.c
> --- netinet6/in6_pcb.c        18 Mar 2009 17:06:52 -0000      1.101
> +++ netinet6/in6_pcb.c        13 Apr 2009 15:41:42 -0000
> @@ -251,7 +251,6 @@ in6_pcbbind(void *v, struct mbuf *nam, s
>               } else if (!IN6_IS_ADDR_UNSPECIFIED(&sin6->sin6_addr)) {
>                       struct ifaddr *ia = NULL;
>  
> -                     sin6->sin6_port = 0;            /* yech... */
>                       if ((in6p->in6p_flags & IN6P_FAITH) == 0 &&
>                           (ia = ifa_ifwithaddr((struct sockaddr *)sin6)) == 0)
>                               return (EADDRNOTAVAIL);

The part above looks ok.

> Index: netinet/in_proto.c
> ===================================================================
> RCS file: /usr/cvs/src/sys/netinet/in_proto.c,v
> retrieving revision 1.97
> diff -u -p -r1.97 in_proto.c
> --- netinet/in_proto.c        28 Feb 2009 18:31:12 -0000      1.97
> +++ netinet/in_proto.c        13 Apr 2009 16:02:32 -0000
> @@ -464,6 +464,7 @@ struct domain inetdomain = {
>       .dom_mowner = MOWNER_INIT("",""),
>       .dom_sa_cmpofs = offsetof(struct sockaddr_in, sin_addr),
>       .dom_sa_cmplen = sizeof(struct in_addr),
> +     .dom_sockaddr_cmp = sockaddr_in_cmp,
>       .dom_sa_any = (const struct sockaddr *)&in_any,
>       .dom_sockaddr_const_addr = sockaddr_in_const_addr,
>       .dom_sockaddr_addr = sockaddr_in_addr,

> Index: netinet6/in6_proto.c
> ===================================================================
> RCS file: /usr/cvs/src/sys/netinet6/in6_proto.c,v
> retrieving revision 1.84
> diff -u -p -r1.84 in6_proto.c
> --- netinet6/in6_proto.c      23 Mar 2009 18:43:20 -0000      1.84
> +++ netinet6/in6_proto.c      13 Apr 2009 16:02:21 -0000
> @@ -416,6 +416,7 @@ struct domain inet6domain = {
>       .dom_mowner = MOWNER_INIT("",""),
>       .dom_sa_cmpofs = offsetof(struct sockaddr_in6, sin6_addr),
>       .dom_sa_cmplen = sizeof(struct in6_addr),
> +     .dom_sockaddr_cmp = sockaddr_in6_cmp,
>       .dom_sa_any = (const struct sockaddr *)&in6_any,
>       .dom_rtcache = LIST_HEAD_INITIALIZER(inet6domain.dom_rtcache)
>  };

I don't think that that is necessary for your purposes.  Also,
calling the comparison routine through a function pointer may have an
undesirable performance impact on some architectures (ARM?).

Dave

-- 
David Young             OJC Technologies
dyoung%ojctech.com@localhost      Urbana, IL * (217) 278-3933