Date: Mon, 24 Jun 2013 20:30:01 GMT From: Mikolaj Golub <trociny@FreeBSD.org> To: freebsd-net@FreeBSD.org Subject: Re: kern/179901: [netinet] [patch] Multicast SO_REUSEADDR handled incorrectly Message-ID: <201306242030.r5OKU1Eq013389@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/179901; it has been noted by GNATS.
From: Mikolaj Golub <trociny@FreeBSD.org>
To: bug-followup@FreeBSD.org, freebsd@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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201306242030.r5OKU1Eq013389>