Please make a helper function which handles this copying.

Why assert this?

Can we please have some helper function or macro which operates on the inpcbinfo_ctx and handles both the unlock and clearing of the pointer?

There is only one place like this. Are you sure there will be more value from such function than obfuscation?

There is no strong requirement that it needs to be on stack. But if it is not, then the KPI is definitely used not the way it was designed. If you really insist, I can remove this assertion.

Yes, I can do that. Any opinion on the better name for struct inpcbinfo_ctx and its most common name ipictx? The helper macro you suggest definitely would have name derived from that.

Can we use MTX_NEW here?

Is this comment still valid? (there is also another one below)

Why? This flag is needed to hint the mutex(9) that it should not panic if mtx_init() sees something instead of zeroed memory. This part of inpcb is always zeroed.

Yep, this comment is traveling through the code for years. Originates from Robert Watson and very early days of the network stack getting rid of Giant. As I understand it the concern was that we are checking inp_vflag locklessly. AFAIK, the inpcb can't mutate from IPv6 one to not IPv6 one. I agree all these comments need to be cleansed up with proper commit message. They are also in in6_pcb.c and in raw_ip.c.

I know we already have to many assert/mpass here.
But why not using INPBUCKET_LOCK_ASSERT here?
As far as I learned from our style of coding. We use MPASS and lock assert mostly for documentation since it's inside INVARIANT.

Why? This flag is needed to hint the mutex(9) that it should not panic if mtx_init() sees something instead of zeroed memory.

Oh, manual says:

MTX_NEW Do not check for double-init

I thought it would prevent redundant check.

This part of inpcb is always zeroed.

Exactly.

Not important at all, but to me as a non-native speaker, the comma placement in 'returns with locked context, fills lportp' made me initially read it as filling lportp being conditional on the lock.

Same comment is on other functions below too.

Why u_short to u_int? why not uint16_t?
It should not be possible to overflow that in context of ports.

Maybe in_pcblookup_ctx? Or even just struct in_pcblookup.

I'd prefer to either omit this assertion, or add a comment explaining why the correctness of this code depends on the ctx being stack-allocated. (I don't see why.)

This is how it is going to look like :)

INPBUCKET_LOCK_ASSERT(&inp->inp_pcbinfo->ipi_lbgrouphashbase[INP_PCBPORTHASH(inp->inp_lport, inp->inp_pcbinfo->ipi_lbgrouphashmask)]);

Or introduce couple new variables under #ifdef INVARIANTS for the check.

In this particular case I decided it is easier to leave comment and check all callers. I'm not strong on that. If you and Mark want - I can add this bulky assertion.

Cause new code uses atomic(9) on them.

The term lookup seems to be occupied already by the fast path - we lookup an inpcb in ip_input() for a packet arrived. Here we have context of an operation that edits: adds, moves or removes an inpcb. It also is associated with the pcbinfo database, not with a single inpcb.

Note that structure name hints for a variable name and prefixes for the inline functions defined below.

Wow, no thanks! :)

Make sense, so why not uint32_t? :D
atomic should be fine (as defined in its manual) with uint32_t AFAICU.
I just want to learn honestly!

I'm also not expert around atomics, learning too. My rationale was that atomic_int is guaranteed to be present on any arch, while fixed-length atomics may be not. At the same time there is implicit knowledge that int is at least 32-bit long, a de facto standard. Thus int is going to be enough and is known to compile everywhere.

Do we really need to? IMHO, just using int is fine.

Sure, the XXX REVIEWERS not going to be pushed. I was afraid that current name is not nice and was hoping a better name could be found, but looks like the current name has no objections, so will leave it as is.

No, I just mean that if you want to keep using 16-bit fields, there is a path forward. I think int is okay.