Date: Tue, 7 Feb 2023 17:22:43 GMT From: Gleb Smirnoff <glebius@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 220d89212943 - main - inpcb: immediately return matching pcb on lookup Message-ID: <202302071722.317HMhS0089906@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by glebius: URL: https://cgit.FreeBSD.org/src/commit/?id=220d892129439e4596b8ef054f56b6e0ede03538 commit 220d892129439e4596b8ef054f56b6e0ede03538 Author: Gleb Smirnoff <glebius@FreeBSD.org> AuthorDate: 2023-02-07 17:21:52 +0000 Commit: Gleb Smirnoff <glebius@FreeBSD.org> CommitDate: 2023-02-07 17:21:52 +0000 inpcb: immediately return matching pcb on lookup This saves a lot of CPU cycles if you got large connection table. The code removed originates from 413628a7e3d, a very large changeset. Discussed that with Bjoern, Jamie we can't recover why would we ever have identical 4-tuples in the hash, even in the presence of jails. Bjoern did a test that confirms that it is impossible to allocate an identical connection from a jail to a host. Code review also confirms that system shouldn't allow for such connections to exist. With a lack of proper test suite we decided to take a risk and go forward with removing that code. Reviewed by: gallatin, bz, markj Differential Revision: https://reviews.freebsd.org/D38015 --- sys/netinet/in_pcb.c | 18 +++--------------- sys/netinet6/in6_pcb.c | 18 +++--------------- 2 files changed, 6 insertions(+), 30 deletions(-) diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c index 2a775ff5fc31..728178f1011c 100644 --- a/sys/netinet/in_pcb.c +++ b/sys/netinet/in_pcb.c @@ -2225,7 +2225,7 @@ in_pcblookup_hash_locked(struct inpcbinfo *pcbinfo, struct in_addr faddr, struct ifnet *ifp, uint8_t numa_domain) { struct inpcbhead *head; - struct inpcb *inp, *tmpinp; + struct inpcb *inp; u_short fport = fport_arg, lport = lport_arg; KASSERT((lookupflags & ~(INPLOOKUP_WILDCARD)) == 0, @@ -2239,7 +2239,6 @@ in_pcblookup_hash_locked(struct inpcbinfo *pcbinfo, struct in_addr faddr, /* * First look for an exact match. */ - tmpinp = NULL; head = &pcbinfo->ipi_hashbase[INP_PCBHASH(&faddr, lport, fport, pcbinfo->ipi_hashmask)]; CK_LIST_FOREACH(inp, head, inp_hash) { @@ -2251,20 +2250,9 @@ in_pcblookup_hash_locked(struct inpcbinfo *pcbinfo, struct in_addr faddr, if (inp->inp_faddr.s_addr == faddr.s_addr && inp->inp_laddr.s_addr == laddr.s_addr && inp->inp_fport == fport && - inp->inp_lport == lport) { - /* - * XXX We should be able to directly return - * the inp here, without any checks. - * Well unless both bound with SO_REUSEPORT? - */ - if (prison_flag(inp->inp_cred, PR_IP4)) - return (inp); - if (tmpinp == NULL) - tmpinp = inp; - } + inp->inp_lport == lport) + return (inp); } - if (tmpinp != NULL) - return (tmpinp); /* * Then look for a wildcard match, if requested. diff --git a/sys/netinet6/in6_pcb.c b/sys/netinet6/in6_pcb.c index 1b57fce7ec47..80540f2695e7 100644 --- a/sys/netinet6/in6_pcb.c +++ b/sys/netinet6/in6_pcb.c @@ -966,7 +966,7 @@ in6_pcblookup_hash_locked(struct inpcbinfo *pcbinfo, struct in6_addr *faddr, int lookupflags, struct ifnet *ifp, uint8_t numa_domain) { struct inpcbhead *head; - struct inpcb *inp, *tmpinp; + struct inpcb *inp; u_short fport = fport_arg, lport = lport_arg; KASSERT((lookupflags & ~(INPLOOKUP_WILDCARD)) == 0, @@ -981,7 +981,6 @@ in6_pcblookup_hash_locked(struct inpcbinfo *pcbinfo, struct in6_addr *faddr, /* * First look for an exact match. */ - tmpinp = NULL; head = &pcbinfo->ipi_hashbase[INP6_PCBHASH(faddr, lport, fport, pcbinfo->ipi_hashmask)]; CK_LIST_FOREACH(inp, head, inp_hash) { @@ -991,20 +990,9 @@ in6_pcblookup_hash_locked(struct inpcbinfo *pcbinfo, struct in6_addr *faddr, if (IN6_ARE_ADDR_EQUAL(&inp->in6p_faddr, faddr) && IN6_ARE_ADDR_EQUAL(&inp->in6p_laddr, laddr) && inp->inp_fport == fport && - inp->inp_lport == lport) { - /* - * XXX We should be able to directly return - * the inp here, without any checks. - * Well unless both bound with SO_REUSEPORT? - */ - if (prison_flag(inp->inp_cred, PR_IP6)) - return (inp); - if (tmpinp == NULL) - tmpinp = inp; - } + inp->inp_lport == lport) + return (inp); } - if (tmpinp != NULL) - return (tmpinp); /* * Then look for a wildcard match, if requested.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202302071722.317HMhS0089906>