Patchwork [net-next,10/16] Don't lookup the socket if there's a socket attached to the skb

login
register
mail settings
Submitter KOVACS Krisztian
Date Oct. 1, 2008, 2:24 p.m.
Message ID <20081001142431.4893.69772.stgit@este>
Download mbox | patch
Permalink /patch/2267/
State Changes Requested
Headers show

Comments

KOVACS Krisztian - Oct. 1, 2008, 2:24 p.m.
Use the socket cached in the TPROXY target if it's present.

Signed-off-by: KOVACS Krisztian <hidden@sch.bme.hu>
---

 net/ipv4/tcp_ipv4.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
KOVACS Krisztian - Oct. 1, 2008, 3:38 p.m.
Hi,

On Wed, 2008-10-01 at 07:50 -0700, David Miller wrote:
> From: KOVACS Krisztian <hidden@sch.bme.hu>
> Date: Wed, 01 Oct 2008 16:24:31 +0200
> 
> > Use the socket cached in the TPROXY target if it's present.
> > 
> > Signed-off-by: KOVACS Krisztian <hidden@sch.bme.hu>
> 
> Ok, this starts to get into controversial territory.
> :-)
> 
> At the very least I think:
> 
> 1) We should do this unconditionally, and even put
>    a "unlikely" there in the test.
> 
> 2) Actually, the whole operation belongs in a generic
>    net/sock.h helper function, and this includes the
>    leading if() test.

The problem is that if you include the if() test then you have to
include the lookup call as well and that's different for TCP/UDP.

Of course we could create a generic helper that then calls the
appropriate lookup function but that's also an unnecessary extra branch,
isn't it?
David Miller - Oct. 1, 2008, 3:51 p.m.
From: KOVACS Krisztian <hidden@sch.bme.hu>
Date: Wed, 01 Oct 2008 17:38:20 +0200

> The problem is that if you include the if() test then you have to
> include the lookup call as well and that's different for TCP/UDP.

No, I only mean to make a helper for this construct:

	if (unlikely(skb->sk)) {
		...
	}

so, something like:

static inline struct sock *sock_skb_steal(struct sk_buff *skb)
{
	if (unlikely(skb->sk)) {
		struct sock *sk = skb->sk;

		skb->destructor = NULL;
		skb->sk = NULL;
		return sk;
	}
	return NULL;
}

and then also get rid of the ifdefs at the place where
these calls are made (TCP and UDP).
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 1ac4d05..0029db9 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1577,8 +1577,17 @@  int tcp_v4_rcv(struct sk_buff *skb)
 	TCP_SKB_CB(skb)->flags	 = iph->tos;
 	TCP_SKB_CB(skb)->sacked	 = 0;
 
+#if defined(CONFIG_NETFILTER_TPROXY) || defined(CONFIG_NETFILTER_TPROXY_MODULE)
+	if (unlikely(skb->sk)) {
+		/* steal reference */
+		sk = skb->sk;
+		skb->destructor = NULL;
+		skb->sk = NULL;
+	} else
+#endif
 	sk = __inet_lookup(net, &tcp_hashinfo, iph->saddr,
 			th->source, iph->daddr, th->dest, inet_iif(skb));
+
 	if (!sk)
 		goto no_tcp_socket;