diff mbox

udp: Introduce special NULL pointers for hlist termination

Message ID 490B183E.3010707@cosmosbay.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Eric Dumazet Oct. 31, 2008, 2:37 p.m. UTC 
Corey Minyard a écrit :
  
> 
> It is annoying that it doesn't help the performance for multicast.  
> However, I think the current patch will solve the DOS issue for 
> multicast, since it switches to a normal spinlock and has a per-list lock.

About multicast, it should be possible to do something about it, if it happens
to be an issue.

That is, do a lockless lookup and accumulate matching sockets ptr in a table

(incrementing their refcount if not zero, checking key, adding in a local stack).

If lookup must be restarted, forget all accumulated sockets (sock_put(ptrs))
   goto begin;

Then, send the (cloned) packet to all accumulated sockets, and
 sock_put() them to release the refcount.



Well, looking at current implementation, I found that udp_v4_mcast_next()
doesnt take into account the 'struct net *net', so we have a bug here...

udp_v6_mcast_next() is buggy too (or at least its caller is)

David, please find a patch against net-2.6

Thanks

[PATCH] udp: multicast packets need to check namespace

Current UDP multicast delivery is not namespace aware.


Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
---
 net/ipv4/udp.c |   14 ++++++++------
 net/ipv6/udp.c |    8 ++++----
 2 files changed, 12 insertions(+), 10 deletions(-)

Comments

Pavel Emelyanov Oct. 31, 2008, 2:55 p.m. UTC | #1 
> Well, looking at current implementation, I found that udp_v4_mcast_next()
> doesnt take into account the 'struct net *net', so we have a bug here...
> 
> udp_v6_mcast_next() is buggy too (or at least its caller is)
> 
> David, please find a patch against net-2.6
> 
> Thanks
> 
> [PATCH] udp: multicast packets need to check namespace
> 
> Current UDP multicast delivery is not namespace aware.
> 
> 
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

Acked-by: Pavel Emelyanov <xemul@openvz.org>

Thanks!

> ---
>  net/ipv4/udp.c |   14 ++++++++------
>  net/ipv6/udp.c |    8 ++++----
>  2 files changed, 12 insertions(+), 10 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
David Miller Nov. 2, 2008, 4:22 a.m. UTC | #2 
From: Pavel Emelyanov <xemul@openvz.org>
Date: Fri, 31 Oct 2008 17:55:50 +0300

> > Well, looking at current implementation, I found that udp_v4_mcast_next()
> > doesnt take into account the 'struct net *net', so we have a bug here...
> > 
> > udp_v6_mcast_next() is buggy too (or at least its caller is)
> > 
> > David, please find a patch against net-2.6
> > 
> > Thanks
> > 
> > [PATCH] udp: multicast packets need to check namespace
> > 
> > Current UDP multicast delivery is not namespace aware.
> > 
> > 
> > Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
> 
> Acked-by: Pavel Emelyanov <xemul@openvz.org>

Applied, thanks everyone.
--
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
diff mbox

Patch

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 2095abc..76e3cc5 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -284,7 +284,7 @@  struct sock *udp4_lib_lookup(struct net *net, __be32 saddr, __be16 sport,
 }
 EXPORT_SYMBOL_GPL(udp4_lib_lookup);
 
-static inline struct sock *udp_v4_mcast_next(struct sock *sk,
+static inline struct sock *udp_v4_mcast_next(struct net *net, struct sock *sk,
 					     __be16 loc_port, __be32 loc_addr,
 					     __be16 rmt_port, __be32 rmt_addr,
 					     int dif)
@@ -295,8 +295,9 @@  static inline struct sock *udp_v4_mcast_next(struct sock *sk,
 
 	sk_for_each_from(s, node) {
 		struct inet_sock *inet = inet_sk(s);
-
-		if (s->sk_hash != hnum					||
+	
+		if (!net_eq(sock_net(s), net)				||
+		    s->sk_hash != hnum					||
 		    (inet->daddr && inet->daddr != rmt_addr)		||
 		    (inet->dport != rmt_port && inet->dport)		||
 		    (inet->rcv_saddr && inet->rcv_saddr != loc_addr)	||
@@ -1079,15 +1080,16 @@  static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
 	read_lock(&udp_hash_lock);
 	sk = sk_head(&udptable[udp_hashfn(net, ntohs(uh->dest))]);
 	dif = skb->dev->ifindex;
-	sk = udp_v4_mcast_next(sk, uh->dest, daddr, uh->source, saddr, dif);
+	sk = udp_v4_mcast_next(net, sk, uh->dest, daddr, uh->source, saddr, dif);
 	if (sk) {
 		struct sock *sknext = NULL;
 
 		do {
 			struct sk_buff *skb1 = skb;
 
-			sknext = udp_v4_mcast_next(sk_next(sk), uh->dest, daddr,
-						   uh->source, saddr, dif);
+			sknext = udp_v4_mcast_next(net, sk_next(sk), uh->dest,
+						   daddr, uh->source, saddr,
+						   dif);
 			if (sknext)
 				skb1 = skb_clone(skb, GFP_ATOMIC);
 
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index e51da8c..71e259e 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -328,7 +328,7 @@  drop:
 	return -1;
 }
 
-static struct sock *udp_v6_mcast_next(struct sock *sk,
+static struct sock *udp_v6_mcast_next(struct net *net, struct sock *sk,
 				      __be16 loc_port, struct in6_addr *loc_addr,
 				      __be16 rmt_port, struct in6_addr *rmt_addr,
 				      int dif)
@@ -340,7 +340,7 @@  static struct sock *udp_v6_mcast_next(struct sock *sk,
 	sk_for_each_from(s, node) {
 		struct inet_sock *inet = inet_sk(s);
 
-		if (sock_net(s) != sock_net(sk))
+		if (!net_eq(sock_net(s), net))
 			continue;
 
 		if (s->sk_hash == num && s->sk_family == PF_INET6) {
@@ -383,14 +383,14 @@  static int __udp6_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
 	read_lock(&udp_hash_lock);
 	sk = sk_head(&udptable[udp_hashfn(net, ntohs(uh->dest))]);
 	dif = inet6_iif(skb);
-	sk = udp_v6_mcast_next(sk, uh->dest, daddr, uh->source, saddr, dif);
+	sk = udp_v6_mcast_next(net, sk, uh->dest, daddr, uh->source, saddr, dif);
 	if (!sk) {
 		kfree_skb(skb);
 		goto out;
 	}
 
 	sk2 = sk;
-	while ((sk2 = udp_v6_mcast_next(sk_next(sk2), uh->dest, daddr,
+	while ((sk2 = udp_v6_mcast_next(net, sk_next(sk2), uh->dest, daddr,
 					uh->source, saddr, dif))) {
 		struct sk_buff *buff = skb_clone(skb, GFP_ATOMIC);
 		if (buff) {