Hello,
On Tue, Aug 22, 2023 at 12:21:59AM +0200, Alexander Bluhm wrote:
> On Thu, Aug 17, 2023 at 12:02:35PM +1000, David Gwynne wrote:
> > there are links between the pcb/socket layer and pf as an optimisation,
> > and links on mbufs between both sides of a forwarded connection.
> > these links let pf skip an rb tree lookup for outgoing packets.
> >
> > right now these links are between pf_state_key structs, which are the
> > things that contain the actual addresses used by the connection, but you
> > then have to iterate over a list in pf_state_keys to get to the pf_state
> > structures.
> >
> > i dont understand why we dont just link the actual pf_state structs.
> > my best guess is there wasnt enough machinery (ie, refcnts and
> > mtxes) on a pf_state struct to make it safe, so the compromise was
> > the pf_state keys. it still got to avoid the tree lookup.
>
> This linkage is much older than MP in pf. There was no refcount
> and mutex when added by henning@.
>
> > i wanted this to make it easier to look up information on pf states from
> > the socket layer, but sashan@ said i should send it out. i do think it
> > makes things a bit easier to understand.
> >
> > the most worrying bit is the change to pf_state_find().
> >
> > thoughts? ok?
>
> It took me years to get the logic correct for all corner cases.
> When using pf-divert with UDP strange things start to happen. Most
> things are covered by regress/sys/net/pf_divert . You have to setup
> two machines to run it.
I'm running the diff on my home router which is doing simple IPv4 NAT,
nothing fancy. I have not noticed any problem so far. Uptime is ~3 days.
I have not used pf_divert tests.
>
> I don't know why it was written that way, but I know it works for
> me. What do you want to fix?
>
>
I've seen the diff off-list as a part of another change. I did ask
David to post this part (the diff) to tech@ separately because I like it.
In my opinion it makes things bit more straightforward. Things
just look more pretty with this diff in. Currently we have
something like this:
{ mbuf, pcb } <-> state key <-> { state, state ... }
with this diff we get to:
{ mbuf, pcb } <-> state <-> state key
Basically when we do process packet we are interested in state
not state key itself. The state key is just a mean to reach the state,
so why not to reach/keep state directly. I like the simplicity
introduced by proposed diff.
I support this change. I also prefer to reach wide consent on this
before the diff will arrive to tree.
thanks and
regards
sashan
