On Thu, Jul 2, 2015 at 1:18 AM, Alex Gartrell <alexgartr...@gmail.com> wrote: > On Wednesday, July 1, 2015, Eric Dumazet <eduma...@google.com> wrote: >> >> On Wed, Jul 1, 2015 at 11:14 PM, David Miller <da...@davemloft.net> wrote: >> > From: Alex Gartrell <agartr...@fb.com> >> > Date: Wed, 1 Jul 2015 13:13:09 -0700 >> > >> >> If we early-demux bind a TCP_TIMEWAIT socket to an skb and then orphan >> >> it >> >> (as we need to do in the ipvs forwarding case), sock_wfree and >> >> sock_rfree >> >> are going to reach into the inet_timewait_sock and mess with fields >> >> that >> >> don't exist. >> >> >> >> Signed-off-by: Alex Gartrell <agartr...@fb.com> >> > >> > If we're forwarding, we should not find a local socket, period. >> >> A socket cannot change state to TCP_TIMEWAIT. >> >> A new object is allocated and old one is removed from ehash, then >> freed (rcu rules being applied) >> >> Also sock_wfree() has nothing to do with early demux. It is for output >> path skbs only. > > > Alright I kind of cheated and didn't include full context here. The problem > is that within ipvs we are getting packets that have been early demuxed and > associated with time wait sockets which we then wish to forward immediately > (ip_vs_xmit.c). Under normal circumstances it would never be associated > with any sk at all, but it is because of early demux, so we want to drop the > relationship by calling skb_orphan. This invokes the destructor which lands > us there. > > So that is how we reach this illegal "treating a twsk like an sk" state. > > If there is a better way to drop the association than skb_orphan I will use > it.
I think you are mistaken Alex. socket early demux cannot possibly set skb->destructor to sock_rfree() If skb->destructor is set by early demux, it correctly points to sock_edemux() And this one correctly handles all socket variants. /* All sockets share common refcount, but have different destructors */ void sock_gen_put(struct sock *sk) { if (!atomic_dec_and_test(&sk->sk_refcnt)) return; if (sk->sk_state == TCP_TIME_WAIT) inet_twsk_free(inet_twsk(sk)); else if (sk->sk_state == TCP_NEW_SYN_RECV) reqsk_free(inet_reqsk(sk)); else sk_free(sk); } EXPORT_SYMBOL_GPL(sock_gen_put); void sock_edemux(struct sk_buff *skb) { sock_gen_put(skb->sk); } EXPORT_SYMBOL(sock_edemux); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html