Skip to content

Commit b16ed2b

Browse files
davem330davem330
committed
Merge branch 'rework-inet_csk_get_port'
Josef Bacik says: ==================== Rework inet_csk_get_port V3->V4: -Removed the random include of addrconf.h that is no longer needed. V2->V3: -Dropped the fastsock from the tb and instead just carry the saddrs, family, and ipv6 only flag. -Reworked the helper functions to deal with this change so I could still use them when checking the fast path. -Killed tb->num_owners as per Eric's request. -Attached a reproducer to the bottom of this email. V1->V2: -Added a new patch 'inet: collapse ipv4/v6 rcv_saddr_equal functions into one' at Hannes' suggestion. -Dropped ->bind_conflict and just use the new helper. -Fixed a compile bug from the original ->bind_conflict patch. The original description of the series follows: At some point recently the guys working on our load balancer added the ability to use SO_REUSEPORT. When they restarted their app with this option enabled they immediately hit a softlockup on what appeared to be the inet_bind_bucket->lock. Eventually what all of our debugging and discussion led us to was the fact that the application comes up without SO_REUSEPORT, shuts down which creates around 100k twsk's, and then comes up and tries to open a bunch of sockets using SO_REUSEPORT, which meant traversing the inet_bind_bucket owners list under the lock. Since this lock is needed for dealing with the twsk's and basically anything else related to connections we would softlockup, and sometimes not ever recover. To solve this problem I did what you see in Path 5/5. Once we have a SO_REUSEPORT socket on the tb->owners list we know that the socket has no conflicts with any of the other sockets on that list. So we can add a copy of the sock_common (really all we need is the recv_saddr but it seemed ugly to copy just the ipv6, ipv4, and flag to indicate if we were ipv6 only in there so I've copied the whole common) in order to check subsequent SO_REUSEPORT sockets. If they match the previous one then we can skip the expensive inet_csk_bind_conflict check. This is what eliminated the soft lockup that we were seeing. Patches 1-4 are cleanups and re-workings. For instance when we specify port == 0 we need to find an open port, but we would do two passes through inet_csk_bind_conflict every time we found a possible port. We would also keep track of the smallest_port value in order to try and use it if we found no port our first run through. This however made no sense as it would have had to fail the first pass through inet_csk_bind_conflict, so would not actually pass the second pass through either. Finally I split the function into two functions in order to make it easier to read and to distinguish between the two behaviors. I have tested this on one of our load balancing boxes during peak traffic and it hasn't fallen over. But this is not my area, so obviously feel free to point out where I'm being stupid and I'll get it fixed up and retested. Thanks, ==================== Signed-off-by: David S. Miller <davem@davemloft.net>
2 parents ab70e58 + 637bc8b commit b16ed2b

15 files changed

+230
-247
lines changed

‎include/net/addrconf.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,7 @@ int __ipv6_get_lladdr(struct inet6_dev *idev, struct in6_addr *addr,
8888
u32 banned_flags);
8989
int ipv6_get_lladdr(struct net_device *dev, struct in6_addr *addr,
9090
u32 banned_flags);
91-
int ipv4_rcv_saddr_equal(const struct sock *sk, const struct sock *sk2,
92-
bool match_wildcard);
93-
int ipv6_rcv_saddr_equal(const struct sock *sk, const struct sock *sk2,
91+
int inet_rcv_saddr_equal(const struct sock *sk, const struct sock *sk2,
9492
bool match_wildcard);
9593
void addrconf_join_solict(struct net_device *dev, const struct in6_addr *addr);
9694
void addrconf_leave_solict(struct inet6_dev *idev, const struct in6_addr *addr);

‎include/net/inet6_connection_sock.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,11 @@
1515

1616
#include <linux/types.h>
1717

18-
struct inet_bind_bucket;
1918
struct request_sock;
2019
struct sk_buff;
2120
struct sock;
2221
struct sockaddr;
2322

24-
int inet6_csk_bind_conflict(const struct sock *sk,
25-
const struct inet_bind_bucket *tb, bool relax,
26-
bool soreuseport_ok);
27-
2823
struct dst_entry *inet6_csk_route_req(const struct sock *sk, struct flowi6 *fl6,
2924
const struct request_sock *req, u8 proto);
3025

‎include/net/inet_connection_sock.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,6 @@ struct inet_connection_sock_af_ops {
6262
char __user *optval, int __user *optlen);
6363
#endif
6464
void (*addr2sockaddr)(struct sock *sk, struct sockaddr *);
65-
int (*bind_conflict)(const struct sock *sk,
66-
const struct inet_bind_bucket *tb,
67-
bool relax, bool soreuseport_ok);
6865
void (*mtu_reduced)(struct sock *sk);
6966
};
7067

@@ -263,9 +260,6 @@ inet_csk_rto_backoff(const struct inet_connection_sock *icsk,
263260

264261
struct sock *inet_csk_accept(struct sock *sk, int flags, int *err);
265262

266-
int inet_csk_bind_conflict(const struct sock *sk,
267-
const struct inet_bind_bucket *tb, bool relax,
268-
bool soreuseport_ok);
269263
int inet_csk_get_port(struct sock *sk, unsigned short snum);
270264

271265
struct dst_entry *inet_csk_route_req(const struct sock *sk, struct flowi4 *fl4,

‎include/net/inet_hashtables.h

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,21 @@ struct inet_ehash_bucket {
7474
* users logged onto your box, isn't it nice to know that new data
7575
* ports are created in O(1) time? I thought so. ;-) -DaveM
7676
*/
77+
#define FASTREUSEPORT_ANY 1
78+
#define FASTREUSEPORT_STRICT 2
79+
7780
struct inet_bind_bucket {
7881
possible_net_t ib_net;
7982
unsigned short port;
8083
signed char fastreuse;
8184
signed char fastreuseport;
8285
kuid_t fastuid;
83-
int num_owners;
86+
#if IS_ENABLED(CONFIG_IPV6)
87+
struct in6_addr fast_v6_rcv_saddr;
88+
#endif
89+
__be32 fast_rcv_saddr;
90+
unsigned short fast_sk_family;
91+
bool fast_ipv6_only;
8492
struct hlist_node node;
8593
struct hlist_head owners;
8694
};
@@ -203,10 +211,7 @@ void inet_hashinfo_init(struct inet_hashinfo *h);
203211

204212
bool inet_ehash_insert(struct sock *sk, struct sock *osk);
205213
bool inet_ehash_nolisten(struct sock *sk, struct sock *osk);
206-
int __inet_hash(struct sock *sk, struct sock *osk,
207-
int (*saddr_same)(const struct sock *sk1,
208-
const struct sock *sk2,
209-
bool match_wildcard));
214+
int __inet_hash(struct sock *sk, struct sock *osk);
210215
int inet_hash(struct sock *sk);
211216
void inet_unhash(struct sock *sk);
212217

‎include/net/udp.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,6 @@ static inline void udp_lib_close(struct sock *sk, long timeout)
204204
}
205205

206206
int udp_lib_get_port(struct sock *sk, unsigned short snum,
207-
int (*)(const struct sock *, const struct sock *, bool),
208207
unsigned int hash2_nulladdr);
209208

210209
u32 udp_flow_hashrnd(void);

‎net/dccp/ipv4.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -904,7 +904,6 @@ static const struct inet_connection_sock_af_ops dccp_ipv4_af_ops = {
904904
.getsockopt = ip_getsockopt,
905905
.addr2sockaddr = inet_csk_addr2sockaddr,
906906
.sockaddr_len = sizeof(struct sockaddr_in),
907-
.bind_conflict = inet_csk_bind_conflict,
908907
#ifdef CONFIG_COMPAT
909908
.compat_setsockopt = compat_ip_setsockopt,
910909
.compat_getsockopt = compat_ip_getsockopt,

‎net/dccp/ipv6.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -937,7 +937,6 @@ static const struct inet_connection_sock_af_ops dccp_ipv6_af_ops = {
937937
.getsockopt = ipv6_getsockopt,
938938
.addr2sockaddr = inet6_csk_addr2sockaddr,
939939
.sockaddr_len = sizeof(struct sockaddr_in6),
940-
.bind_conflict = inet6_csk_bind_conflict,
941940
#ifdef CONFIG_COMPAT
942941
.compat_setsockopt = compat_ipv6_setsockopt,
943942
.compat_getsockopt = compat_ipv6_getsockopt,
@@ -958,7 +957,6 @@ static const struct inet_connection_sock_af_ops dccp_ipv6_mapped = {
958957
.getsockopt = ipv6_getsockopt,
959958
.addr2sockaddr = inet6_csk_addr2sockaddr,
960959
.sockaddr_len = sizeof(struct sockaddr_in6),
961-
.bind_conflict = inet6_csk_bind_conflict,
962960
#ifdef CONFIG_COMPAT
963961
.compat_setsockopt = compat_ipv6_setsockopt,
964962
.compat_getsockopt = compat_ipv6_getsockopt,

0 commit comments

Comments
 (0)