Date: Tue, 31 Jul 2007 11:08:49 -0700 From: Julian Elischer <julian@elischer.org> To: "Christian S.J. Peron" <csjp@FreeBSD.org> Cc: freebsd-net@freebsd.org, rwatson@freebsd.org Subject: Re: divert and deadlock issues Message-ID: <46AF7AB1.2070805@elischer.org> In-Reply-To: <20070731162515.GA3684@sub> References: <20070731162515.GA3684@sub>
next in thread | previous in thread | raw e-mail | index | archive | help
Christian S.J. Peron wrote:
> Group,
>
> Robert Watson and I have been discussing some of the consequences around not
> having Giant picked up in the network stack for mpsafenet=0. One of the
> issues that kept coming up was a number of lock ordering issues around divert:
>
> Upon quick inspection I found:
>
> LOR #163 - Locking interactions between IPSEC and divert
> LOR #181 - Locking interactions between PFIL and divert
> LOR #202 - Locking interactions between Multi-cast and divert (??)
> LOR #203 - Locking interactions between IPFW and divert
>
> Most of these exist because the lock ordering between inbound and outbound
> directions are reversed. Also, the notion of inbound and outbound can be
> slightly complicated in some areas. Upon quick inspection of the code,
> it looks like all of these issues can be fixed by simply dropping the
> inp/divert pcb info locks over the call to ip_output().
>
>>From ip_divert.c:
>
> [..]
>
> INP_INFO_WLOCK(&divcbinfo);
> inp = sotoinpcb(so);
> INP_LOCK(inp);
> /*
> * Don't allow both user specified and setsockopt options,
> * and don't allow packet length sizes that will crash
> */
> if (((ip->ip_hl != (sizeof (*ip) >> 2)) && inp->inp_options) ||
> ((u_short)ntohs(ip->ip_len) > m->m_pkthdr.len)) {
> error = EINVAL;
> m_freem(m);
> } else {
> /* Convert fields to host order for ip_output() */
> ip->ip_len = ntohs(ip->ip_len);
> ip->ip_off = ntohs(ip->ip_off);
>
> /* Send packet to output processing */
> ipstat.ips_rawout++; /* XXX */
>
> #ifdef MAC
> mac_create_mbuf_from_inpcb(inp, m);
> #endif
> error = ip_output(m,
> inp->inp_options, NULL,
> ((so->so_options & SO_DONTROUTE) ?
> IP_ROUTETOIF : 0) |
> IP_ALLOWBROADCAST | IP_RAWOUTPUT,
> inp->inp_moptions, NULL);
> }
> INP_UNLOCK(inp);
> INP_INFO_WUNLOCK(&divcbinfo);
> [..]
>
> One idea was to duplicate the socket options mbuf and pass in a NULL pointer
> for the multi-cast options. Keep in mind that these are multicast options
> associated with a divert socket.
>
> So I guess the questions:
>
> (1) Are there any users that are specifying multicast options on divert sockets?
> (2) Are there any users that are specifying socket options in general for
> divert sockets?
who added the code? has it always been there?
>
> Any feedback would be greatly appreciated.
>
> Thanks
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?46AF7AB1.2070805>