Re: refactor pcb lookup
Looks good by me.
> On 29 Aug 2022, at 14:15, Alexander Bluhm wrote:
>
> Anyone?
>
> On Sat, Aug 20, 2022 at 03:24:28PM +0200, Alexander Bluhm wrote:
>> Hi,
>>
>> Can we rename the the function in_pcbhashlookup() to in_pcblookup()?
>> Then we have in_pcblookup() and in_pcblookup_listen() as public PCB
>> interface. Using a hash table is only an implementation detail.
>>
>> For internal use I would like to introduce in_pcbhash_insert() and
>> in_pcbhash_lookup() to avoid code duplication.
>>
>> Routing domain is unsigned, change the type to u_int.
>>
>> If the diff is too large for review, I can split these parts.
>>
>> ok?
>>
>> bluhm
>>
>> Index: net/pf.c
>> ===
>> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
>> retrieving revision 1.1137
>> diff -u -p -r1.1137 pf.c
>> --- net/pf.c 8 Aug 2022 12:06:30 - 1.1137
>> +++ net/pf.c 19 Aug 2022 16:22:47 -
>> @@ -3348,7 +3348,7 @@ pf_socket_lookup(struct pf_pdesc *pd)
>> * Fails when rtable is changed while evaluating the ruleset
>> * The socket looked up will not match the one hit in the end.
>> */
>> -inp = in_pcbhashlookup(tb, saddr->v4, sport, daddr->v4, dport,
>> +inp = in_pcblookup(tb, saddr->v4, sport, daddr->v4, dport,
>> pd->rdomain);
>> if (inp == NULL) {
>> inp = in_pcblookup_listen(tb, daddr->v4, dport,
>> @@ -3359,7 +3359,7 @@ pf_socket_lookup(struct pf_pdesc *pd)
>> break;
>> #ifdef INET6
>> case AF_INET6:
>> -inp = in6_pcbhashlookup(tb, >v6, sport, >v6,
>> +inp = in6_pcblookup(tb, >v6, sport, >v6,
>> dport, pd->rdomain);
>> if (inp == NULL) {
>> inp = in6_pcblookup_listen(tb, >v6, dport,
>> Index: netinet/in_pcb.c
>> ===
>> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v
>> retrieving revision 1.270
>> diff -u -p -r1.270 in_pcb.c
>> --- netinet/in_pcb.c 8 Aug 2022 12:06:30 - 1.270
>> +++ netinet/in_pcb.c 19 Aug 2022 20:41:23 -
>> @@ -120,14 +120,16 @@ struct baddynamicports baddynamicports;
>> struct baddynamicports rootonlyports;
>> struct pool inpcb_pool;
>>
>> -voidin_pcbrehash_locked(struct inpcb *);
>> +voidin_pcbhash_insert(struct inpcb *);
>> +struct inpcb *in_pcbhash_lookup(struct inpcbtable *, u_int,
>> +const struct in_addr *, u_short, const struct in_addr *, u_short);
>> int in_pcbresize(struct inpcbtable *, int);
>>
>> #define INPCBHASH_LOADFACTOR(_x)(((_x) * 3) / 4)
>>
>> -struct inpcbhead *in_pcbhash(struct inpcbtable *, int,
>> +struct inpcbhead *in_pcbhash(struct inpcbtable *, u_int,
>> const struct in_addr *, u_short, const struct in_addr *, u_short);
>> -struct inpcbhead *in_pcblhash(struct inpcbtable *, int, u_short);
>> +struct inpcbhead *in_pcblhash(struct inpcbtable *, u_int, u_short);
>>
>> /*
>> * in_pcb is used for inet and inet6. in6_pcb only contains special
>> @@ -141,12 +143,12 @@ in_init(void)
>> }
>>
>> struct inpcbhead *
>> -in_pcbhash(struct inpcbtable *table, int rdom,
>> +in_pcbhash(struct inpcbtable *table, u_int rdomain,
>> const struct in_addr *faddr, u_short fport,
>> const struct in_addr *laddr, u_short lport)
>> {
>> SIPHASH_CTX ctx;
>> -u_int32_t nrdom = htonl(rdom);
>> +u_int32_t nrdom = htonl(rdomain);
>>
>> SipHash24_Init(, >inpt_key);
>> SipHash24_Update(, , sizeof(nrdom));
>> @@ -159,10 +161,10 @@ in_pcbhash(struct inpcbtable *table, int
>> }
>>
>> struct inpcbhead *
>> -in_pcblhash(struct inpcbtable *table, int rdom, u_short lport)
>> +in_pcblhash(struct inpcbtable *table, u_int rdomain, u_short lport)
>> {
>> SIPHASH_CTX ctx;
>> -u_int32_t nrdom = htonl(rdom);
>> +u_int32_t nrdom = htonl(rdomain);
>>
>> SipHash24_Init(, >inpt_lkey);
>> SipHash24_Update(, , sizeof(nrdom));
>> @@ -226,9 +228,6 @@ int
>> in_pcballoc(struct socket *so, struct inpcbtable *table)
>> {
>> struct inpcb *inp;
>> -struct inpcbhead *head;
>> -
>> -NET_ASSERT_LOCKED();
>>
>> inp = pool_get(_pool, PR_NOWAIT|PR_ZERO);
>> if (inp == NULL)
>> @@ -257,19 +256,7 @@ in_pcballoc(struct socket *so, struct in
>> if (table->inpt_count++ > INPCBHASH_LOADFACTOR(table->inpt_size))
>> (void)in_pcbresize(table, table->inpt_size * 2);
>> TAILQ_INSERT_HEAD(>inpt_queue, inp, inp_queue);
>> -head = in_pcblhash(table, inp->inp_rtableid, inp->inp_lport);
>> -LIST_INSERT_HEAD(head, inp, inp_lhash);
>> -#ifdef INET6
>> -if (sotopf(so) == PF_INET6)
>> -head = in6_pcbhash(table, rtable_l2(inp->inp_rtableid),
>> ->inp_faddr6, inp->inp_fport,
>> ->inp_laddr6, inp->inp_lport);
>> -else
>> -#endif /* INET6 */
>> -head = in_pcbhash(table,
Re: refactor pcb lookup
Anyone?
On Sat, Aug 20, 2022 at 03:24:28PM +0200, Alexander Bluhm wrote:
> Hi,
>
> Can we rename the the function in_pcbhashlookup() to in_pcblookup()?
> Then we have in_pcblookup() and in_pcblookup_listen() as public PCB
> interface. Using a hash table is only an implementation detail.
>
> For internal use I would like to introduce in_pcbhash_insert() and
> in_pcbhash_lookup() to avoid code duplication.
>
> Routing domain is unsigned, change the type to u_int.
>
> If the diff is too large for review, I can split these parts.
>
> ok?
>
> bluhm
>
> Index: net/pf.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
> retrieving revision 1.1137
> diff -u -p -r1.1137 pf.c
> --- net/pf.c 8 Aug 2022 12:06:30 - 1.1137
> +++ net/pf.c 19 Aug 2022 16:22:47 -
> @@ -3348,7 +3348,7 @@ pf_socket_lookup(struct pf_pdesc *pd)
>* Fails when rtable is changed while evaluating the ruleset
>* The socket looked up will not match the one hit in the end.
>*/
> - inp = in_pcbhashlookup(tb, saddr->v4, sport, daddr->v4, dport,
> + inp = in_pcblookup(tb, saddr->v4, sport, daddr->v4, dport,
> pd->rdomain);
> if (inp == NULL) {
> inp = in_pcblookup_listen(tb, daddr->v4, dport,
> @@ -3359,7 +3359,7 @@ pf_socket_lookup(struct pf_pdesc *pd)
> break;
> #ifdef INET6
> case AF_INET6:
> - inp = in6_pcbhashlookup(tb, >v6, sport, >v6,
> + inp = in6_pcblookup(tb, >v6, sport, >v6,
> dport, pd->rdomain);
> if (inp == NULL) {
> inp = in6_pcblookup_listen(tb, >v6, dport,
> Index: netinet/in_pcb.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v
> retrieving revision 1.270
> diff -u -p -r1.270 in_pcb.c
> --- netinet/in_pcb.c 8 Aug 2022 12:06:30 - 1.270
> +++ netinet/in_pcb.c 19 Aug 2022 20:41:23 -
> @@ -120,14 +120,16 @@ struct baddynamicports baddynamicports;
> struct baddynamicports rootonlyports;
> struct pool inpcb_pool;
>
> -void in_pcbrehash_locked(struct inpcb *);
> +void in_pcbhash_insert(struct inpcb *);
> +struct inpcb *in_pcbhash_lookup(struct inpcbtable *, u_int,
> +const struct in_addr *, u_short, const struct in_addr *, u_short);
> int in_pcbresize(struct inpcbtable *, int);
>
> #define INPCBHASH_LOADFACTOR(_x)(((_x) * 3) / 4)
>
> -struct inpcbhead *in_pcbhash(struct inpcbtable *, int,
> +struct inpcbhead *in_pcbhash(struct inpcbtable *, u_int,
> const struct in_addr *, u_short, const struct in_addr *, u_short);
> -struct inpcbhead *in_pcblhash(struct inpcbtable *, int, u_short);
> +struct inpcbhead *in_pcblhash(struct inpcbtable *, u_int, u_short);
>
> /*
> * in_pcb is used for inet and inet6. in6_pcb only contains special
> @@ -141,12 +143,12 @@ in_init(void)
> }
>
> struct inpcbhead *
> -in_pcbhash(struct inpcbtable *table, int rdom,
> +in_pcbhash(struct inpcbtable *table, u_int rdomain,
> const struct in_addr *faddr, u_short fport,
> const struct in_addr *laddr, u_short lport)
> {
> SIPHASH_CTX ctx;
> - u_int32_t nrdom = htonl(rdom);
> + u_int32_t nrdom = htonl(rdomain);
>
> SipHash24_Init(, >inpt_key);
> SipHash24_Update(, , sizeof(nrdom));
> @@ -159,10 +161,10 @@ in_pcbhash(struct inpcbtable *table, int
> }
>
> struct inpcbhead *
> -in_pcblhash(struct inpcbtable *table, int rdom, u_short lport)
> +in_pcblhash(struct inpcbtable *table, u_int rdomain, u_short lport)
> {
> SIPHASH_CTX ctx;
> - u_int32_t nrdom = htonl(rdom);
> + u_int32_t nrdom = htonl(rdomain);
>
> SipHash24_Init(, >inpt_lkey);
> SipHash24_Update(, , sizeof(nrdom));
> @@ -226,9 +228,6 @@ int
> in_pcballoc(struct socket *so, struct inpcbtable *table)
> {
> struct inpcb *inp;
> - struct inpcbhead *head;
> -
> - NET_ASSERT_LOCKED();
>
> inp = pool_get(_pool, PR_NOWAIT|PR_ZERO);
> if (inp == NULL)
> @@ -257,19 +256,7 @@ in_pcballoc(struct socket *so, struct in
> if (table->inpt_count++ > INPCBHASH_LOADFACTOR(table->inpt_size))
> (void)in_pcbresize(table, table->inpt_size * 2);
> TAILQ_INSERT_HEAD(>inpt_queue, inp, inp_queue);
> - head = in_pcblhash(table, inp->inp_rtableid, inp->inp_lport);
> - LIST_INSERT_HEAD(head, inp, inp_lhash);
> -#ifdef INET6
> - if (sotopf(so) == PF_INET6)
> - head = in6_pcbhash(table, rtable_l2(inp->inp_rtableid),
> - >inp_faddr6, inp->inp_fport,
> - >inp_laddr6, inp->inp_lport);
> - else
> -#endif /* INET6 */
> - head = in_pcbhash(table, rtable_l2(inp->inp_rtableid),
> - >inp_faddr, inp->inp_fport,
> - >inp_laddr, inp->inp_lport);
> - LIST_INSERT_HEAD(head,
refactor pcb lookup
Hi,
Can we rename the the function in_pcbhashlookup() to in_pcblookup()?
Then we have in_pcblookup() and in_pcblookup_listen() as public PCB
interface. Using a hash table is only an implementation detail.
For internal use I would like to introduce in_pcbhash_insert() and
in_pcbhash_lookup() to avoid code duplication.
Routing domain is unsigned, change the type to u_int.
If the diff is too large for review, I can split these parts.
ok?
bluhm
Index: net/pf.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1137
diff -u -p -r1.1137 pf.c
--- net/pf.c8 Aug 2022 12:06:30 - 1.1137
+++ net/pf.c19 Aug 2022 16:22:47 -
@@ -3348,7 +3348,7 @@ pf_socket_lookup(struct pf_pdesc *pd)
* Fails when rtable is changed while evaluating the ruleset
* The socket looked up will not match the one hit in the end.
*/
- inp = in_pcbhashlookup(tb, saddr->v4, sport, daddr->v4, dport,
+ inp = in_pcblookup(tb, saddr->v4, sport, daddr->v4, dport,
pd->rdomain);
if (inp == NULL) {
inp = in_pcblookup_listen(tb, daddr->v4, dport,
@@ -3359,7 +3359,7 @@ pf_socket_lookup(struct pf_pdesc *pd)
break;
#ifdef INET6
case AF_INET6:
- inp = in6_pcbhashlookup(tb, >v6, sport, >v6,
+ inp = in6_pcblookup(tb, >v6, sport, >v6,
dport, pd->rdomain);
if (inp == NULL) {
inp = in6_pcblookup_listen(tb, >v6, dport,
Index: netinet/in_pcb.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v
retrieving revision 1.270
diff -u -p -r1.270 in_pcb.c
--- netinet/in_pcb.c8 Aug 2022 12:06:30 - 1.270
+++ netinet/in_pcb.c19 Aug 2022 20:41:23 -
@@ -120,14 +120,16 @@ struct baddynamicports baddynamicports;
struct baddynamicports rootonlyports;
struct pool inpcb_pool;
-void in_pcbrehash_locked(struct inpcb *);
+void in_pcbhash_insert(struct inpcb *);
+struct inpcb *in_pcbhash_lookup(struct inpcbtable *, u_int,
+const struct in_addr *, u_short, const struct in_addr *, u_short);
intin_pcbresize(struct inpcbtable *, int);
#defineINPCBHASH_LOADFACTOR(_x)(((_x) * 3) / 4)
-struct inpcbhead *in_pcbhash(struct inpcbtable *, int,
+struct inpcbhead *in_pcbhash(struct inpcbtable *, u_int,
const struct in_addr *, u_short, const struct in_addr *, u_short);
-struct inpcbhead *in_pcblhash(struct inpcbtable *, int, u_short);
+struct inpcbhead *in_pcblhash(struct inpcbtable *, u_int, u_short);
/*
* in_pcb is used for inet and inet6. in6_pcb only contains special
@@ -141,12 +143,12 @@ in_init(void)
}
struct inpcbhead *
-in_pcbhash(struct inpcbtable *table, int rdom,
+in_pcbhash(struct inpcbtable *table, u_int rdomain,
const struct in_addr *faddr, u_short fport,
const struct in_addr *laddr, u_short lport)
{
SIPHASH_CTX ctx;
- u_int32_t nrdom = htonl(rdom);
+ u_int32_t nrdom = htonl(rdomain);
SipHash24_Init(, >inpt_key);
SipHash24_Update(, , sizeof(nrdom));
@@ -159,10 +161,10 @@ in_pcbhash(struct inpcbtable *table, int
}
struct inpcbhead *
-in_pcblhash(struct inpcbtable *table, int rdom, u_short lport)
+in_pcblhash(struct inpcbtable *table, u_int rdomain, u_short lport)
{
SIPHASH_CTX ctx;
- u_int32_t nrdom = htonl(rdom);
+ u_int32_t nrdom = htonl(rdomain);
SipHash24_Init(, >inpt_lkey);
SipHash24_Update(, , sizeof(nrdom));
@@ -226,9 +228,6 @@ int
in_pcballoc(struct socket *so, struct inpcbtable *table)
{
struct inpcb *inp;
- struct inpcbhead *head;
-
- NET_ASSERT_LOCKED();
inp = pool_get(_pool, PR_NOWAIT|PR_ZERO);
if (inp == NULL)
@@ -257,19 +256,7 @@ in_pcballoc(struct socket *so, struct in
if (table->inpt_count++ > INPCBHASH_LOADFACTOR(table->inpt_size))
(void)in_pcbresize(table, table->inpt_size * 2);
TAILQ_INSERT_HEAD(>inpt_queue, inp, inp_queue);
- head = in_pcblhash(table, inp->inp_rtableid, inp->inp_lport);
- LIST_INSERT_HEAD(head, inp, inp_lhash);
-#ifdef INET6
- if (sotopf(so) == PF_INET6)
- head = in6_pcbhash(table, rtable_l2(inp->inp_rtableid),
- >inp_faddr6, inp->inp_fport,
- >inp_laddr6, inp->inp_lport);
- else
-#endif /* INET6 */
- head = in_pcbhash(table, rtable_l2(inp->inp_rtableid),
- >inp_faddr, inp->inp_fport,
- >inp_laddr, inp->inp_lport);
- LIST_INSERT_HEAD(head, inp, inp_hash);
+ in_pcbhash_insert(inp);
mtx_leave(>inpt_mtx);
so->so_pcb = inp;
@@ -511,7 +498,7 @@ in_pcbconnect(struct inpcb *inp, struct
if (error)
return
