The following reply was made to PR kern/179901; it has been noted by GNATS.
From: Mikolaj Golub <troc...@freebsd.org> To: bug-follo...@freebsd.org, free...@grem.de Cc: Subject: Re: kern/179901: [netinet] [patch] Multicast SO_REUSEADDR handled incorrectly Date: Mon, 24 Jun 2013 23:29:42 +0300 --9amGYk9869ThD9tj Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Michael, Thank you for your analysis and the patch. I have the following notes to your patch though: 1) INET6 needs fixing too. 2) It looks like after introducing INP_REUSEADDR there is no need in handling the IN_MULTICAST case in ip_ctloutput(). 3) Actually you don't have to use IN_MULTICAST() in in_pcbbind_setup(): the information is already encoded in reuseport variable. 4) The patch not only fixes the regression introduced by r227207, but also changes the historical behavior before r227207. Although the change might be correct it is better to separate these issues. Feeling guilty for the regression introduced in r227207 I am eager to fix it ASAP, before 9.2 release. But I don't have strong opinion about changing the historical behavior. So, could you please look at the attached patch, which is based on your idea of INP_REUSEADDR flag? Now the code more resembles the code before r227207 in looks and I am a little more confident that there is no regression. I would appreciate any testing. Note, it is against CURRENT; STABLE will require patching in_pcb.h manually due to newly introduced INP_FREED flag. -- Mikolaj Golub --9amGYk9869ThD9tj Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="pr179901.1.patch" Index: sys/netinet/in_pcb.c =================================================================== --- sys/netinet/in_pcb.c (revision 252162) +++ sys/netinet/in_pcb.c (working copy) @@ -467,6 +467,23 @@ in_pcb_lport(struct inpcb *inp, struct in_addr *la return (0); } + +/* + * Return cached socket options. + */ +int +inp_so_options(const struct inpcb *inp) +{ + int so_options; + + so_options = 0; + + if ((inp->inp_flags2 & INP_REUSEPORT) != 0) + so_options |= SO_REUSEPORT; + if ((inp->inp_flags2 & INP_REUSEADDR) != 0) + so_options |= SO_REUSEADDR; + return (so_options); +} #endif /* INET || INET6 */ #ifdef INET @@ -595,8 +612,8 @@ in_pcbbind_setup(struct inpcb *inp, struct sockadd if (tw == NULL || (reuseport & tw->tw_so_options) == 0) return (EADDRINUSE); - } else if (t && (reuseport == 0 || - (t->inp_flags2 & INP_REUSEPORT) == 0)) { + } else if (t && + (reuseport & inp_so_options(t)) == 0) { #ifdef INET6 if (ntohl(sin->sin_addr.s_addr) != INADDR_ANY || Index: sys/netinet/in_pcb.h =================================================================== --- sys/netinet/in_pcb.h (revision 252162) +++ sys/netinet/in_pcb.h (working copy) @@ -442,6 +442,7 @@ struct tcpcb * inp_inpcbtotcpcb(struct inpcb *inp); void inp_4tuple_get(struct inpcb *inp, uint32_t *laddr, uint16_t *lp, uint32_t *faddr, uint16_t *fp); +int inp_so_options(const struct inpcb *inp); #endif /* _KERNEL */ @@ -543,6 +544,7 @@ void inp_4tuple_get(struct inpcb *inp, uint32_t * #define INP_PCBGROUPWILD 0x00000004 /* in pcbgroup wildcard list */ #define INP_REUSEPORT 0x00000008 /* SO_REUSEPORT option is set */ #define INP_FREED 0x00000010 /* inp itself is not valid */ +#define INP_REUSEADDR 0x00000020 /* SO_REUSEADDR option is set */ /* * Flags passed to in_pcblookup*() functions. Index: sys/netinet/ip_output.c =================================================================== --- sys/netinet/ip_output.c (revision 252162) +++ sys/netinet/ip_output.c (working copy) @@ -900,13 +900,10 @@ ip_ctloutput(struct socket *so, struct sockopt *so switch (sopt->sopt_name) { case SO_REUSEADDR: INP_WLOCK(inp); - if (IN_MULTICAST(ntohl(inp->inp_laddr.s_addr))) { - if ((so->so_options & - (SO_REUSEADDR | SO_REUSEPORT)) != 0) - inp->inp_flags2 |= INP_REUSEPORT; - else - inp->inp_flags2 &= ~INP_REUSEPORT; - } + if ((so->so_options & SO_REUSEADDR) != 0) + inp->inp_flags2 |= INP_REUSEADDR; + else + inp->inp_flags2 &= ~INP_REUSEADDR; INP_WUNLOCK(inp); error = 0; break; Index: sys/netinet6/in6_pcb.c =================================================================== --- sys/netinet6/in6_pcb.c (revision 252162) +++ sys/netinet6/in6_pcb.c (working copy) @@ -265,8 +265,8 @@ in6_pcbbind(register struct inpcb *inp, struct soc INP_IPV6PROTO) == (t->inp_vflag & INP_IPV6PROTO)))) return (EADDRINUSE); - } else if (t && (reuseport == 0 || - (t->inp_flags2 & INP_REUSEPORT) == 0) && + } else if (t && + (reuseport & inp_so_options(t)) == 0 && (ntohl(t->inp_laddr.s_addr) != INADDR_ANY || (t->inp_vflag & INP_IPV6PROTO) != 0)) return (EADDRINUSE); Index: sys/netinet6/ip6_output.c =================================================================== --- sys/netinet6/ip6_output.c (revision 252162) +++ sys/netinet6/ip6_output.c (working copy) @@ -1477,13 +1477,10 @@ ip6_ctloutput(struct socket *so, struct sockopt *s switch (sopt->sopt_name) { case SO_REUSEADDR: INP_WLOCK(in6p); - if (IN_MULTICAST(ntohl(in6p->inp_laddr.s_addr))) { - if ((so->so_options & - (SO_REUSEADDR | SO_REUSEPORT)) != 0) - in6p->inp_flags2 |= INP_REUSEPORT; - else - in6p->inp_flags2 &= ~INP_REUSEPORT; - } + if ((so->so_options & SO_REUSEADDR) != 0) + in6p->inp_flags2 |= INP_REUSEADDR; + else + in6p->inp_flags2 &= ~INP_REUSEADDR; INP_WUNLOCK(in6p); error = 0; break; --9amGYk9869ThD9tj-- _______________________________________________ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"