On Wed, May 30, 2012 at 8:20 PM, Eric Dumazet [off-list ref] wrote:

On Wed, 2012-05-30 at 10:36 +0300, Alexandru Copot wrote:

quoted

+struct inet_bind_bucket *
+inet4_find_bind_buckets(struct sock *sk,
+                     unsigned short port,
+                     struct inet_bind_hashbucket **p_bhead,
+                     struct inet_bind_hashbucket **p_portaddr_bhead)
+{
+     struct net *net = sock_net(sk);
+     struct inet_hashinfo *hinfo = sk->sk_prot->h.hashinfo;
+     struct inet_bind_bucket *tb = NULL;
+     struct hlist_node *node;
+
+     struct inet_bind_hashbucket *bhead, *portaddr_bhead, *portaddrany_bhead;
+     bhead = &hinfo->bhash[inet_bhashfn(net, port, hinfo->bhash_size)];
+     portaddr_bhead = inet4_portaddr_hashbucket(hinfo, net,
+                             sk_rcv_saddr(sk), port);
+     portaddrany_bhead = inet4_portaddr_hashbucket(hinfo, net,
+                                             INADDR_ANY, port);
+
+     *p_portaddr_bhead = portaddr_bhead;
+     *p_bhead = bhead;
+
+     /*
+      * prevent dead locks by always taking locks in a fixed order:
+      * - always take the port-only lock first. This is done because in some
+      *   other places this is the lock taken, being folllowed in only some
+      *   cases by the portaddr lock.
+      * - between portaddr and portaddrany always choose the one with the
+      *   lower address. Unlock ordering is not important, as long as the
+      *   locking order is consistent.
+      * - make sure to not take the same lock twice
+      */
+     spin_lock(&bhead->lock);
+     if (portaddr_bhead > portaddrany_bhead) {
+             spin_lock(&portaddrany_bhead->lock);
+             spin_lock(&portaddr_bhead->lock);
+     } else if (portaddr_bhead < portaddrany_bhead) {
+             spin_lock(&portaddr_bhead->lock);
+             spin_lock(&portaddrany_bhead->lock);
+     } else {
+             spin_lock(&portaddr_bhead->lock);
+     }
+
+     if (sk_rcv_saddr(sk) != INADDR_ANY) {
+             struct inet_bind_hashbucket *_head;
+
+             _head = portaddr_bhead;
+             if (bhead->count < portaddr_bhead->count) {
+                     _head = bhead;
+                     inet_bind_bucket_for_each(tb, node, &_head->chain)
+                             if ((net_eq(ib_net(tb), net)) &&
+                                 (tb->port == port) &&
+                                 (tb->ib_addr_ipv4 == sk_rcv_saddr(sk)))
+                                     goto found;
+             } else {
+                     inet_portaddr_bind_bucket_for_each(tb, node, &_head->chain)
+                             if ((net_eq(ib_net(tb), net)) &&
+                                 (tb->port == port) &&
+                                 (tb->ib_addr_ipv4 == sk_rcv_saddr(sk)))
+                                     goto found;
+             }
+             _head = portaddrany_bhead;
+             if (bhead->count < portaddrany_bhead->count) {
+                     _head = bhead;
+                     inet_bind_bucket_for_each(tb, node, &_head->chain)
+                             if ((ib_net(tb) == net) &&
+                                 (tb->port == port) &&
+                                 (tb->ib_addr_ipv4 == INADDR_ANY))
+                                     goto found;
+             } else {
+                     inet_portaddr_bind_bucket_for_each(tb, node, &_head->chain)
+                             if ((ib_net(tb) == net) &&
+                                 (tb->port == port) &&
+                                 (tb->ib_addr_ipv4 == INADDR_ANY))
+                                     goto found;
+             }
+     } else {
+             inet_bind_bucket_for_each(tb, node, &bhead->chain)
+                     if ((ib_net(tb) == net) && (tb->port == port))
+                             goto found;
+     }
+
+     tb = NULL;
+found:
+     if (portaddr_bhead != portaddrany_bhead)
+             spin_unlock(&portaddrany_bhead->lock);
+
+     /* the other locks remain taken, as the caller
+      * may want to change the hash tabels */
+     return tb;
+}
+
+

How this is going to work with IPv6 sockets in the middle of the
chains ?

Now I see it might not work that well. I think I should just skip them
here and only check the IPv4 sockets.

Also, comments are not properly formatted, they should all look like :

       /* the other locks remain taken, as the caller
        * may want to change the hash tables
        */

And finally, make sure LOCKDEP is happy with your locking code.

I will check that too.