pf: add DIOCGETSTATESV2

[kp]

Summary

Add a new version of the DIOCGETSTATES call, which extends the struct to
include the original interface information.

MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")

[mjg]

sys/netpfil/pf/pf_ioctl.c

2888

an upper bound should be checked to make sure this does not request gigabytes of memory

but more importantly this should alloc some amount and export the thing in chunks. can be a separate patch.

[kp]

sys/netpfil/pf/pf_ioctl.c

2888

Good point, although now I wonder if we shouldn't do multiple copyout() calls instead. That way we only allocate memory for one state at a time, and can export multiple gigabytes of data without issue.
(And we should fix the old DIOCGETSTATES call at the same time.)

Even then we might still want the support chunked reads, mostly so userspace doesn't have to allocate all of the memory in one go. Even is libpfctl is likely to do that anyway. (But in a future patch).

[mjg]

sys/netpfil/pf/pf_ioctl.c

2888

libpfctl should mmap a big area and let the kernel fault on it as needed

multiple calls to copyout is what i'm proposing, but doing it on a per-state would not work.

1. copyout on smap-capable CPUs comes with a serializing perf hit
2. you can't do copyout while holding the hashrow lock, so you have to drop it aroudn the call, which adds 2 atomic ops for each state.
3. since the row might have been modified when the lock was dropped, you don't know where to continue from without inserting a sentinel. but it is unclear how the rest of the code would feel about it

that said, bare minimum you need to be able to prepare the entire row for export in one go. simple heuristic to do it would to be create a buffer upfront, say for 64 states and roll with it until it is full AND the row is fully covered. If entries from the next row would not fit, you flush it and continue. if 64 is too small, you realloc with a bigger size.

2888

Note the copyout problems can be worked around, but I don't think it is worth it for this facility.

[kp]

sys/netpfil/pf/pf_ioctl.c

2888

We do already bound the size (up to the number of states * the size of each exported state), so that at least prevents userspace from asking for silly things.

I'm inclined to call that good enough for this commit, and to work up a new patch on top of this series to implement your suggestion, for both the old and the new version of DIOCGETSTATES.

[mjg]

sys/netpfil/pf/pf_ioctl.c

2888

To my reading the allocation is not bounded right now -- userspace, should they want, can pass an arbitrary size.

[kp]

sys/netpfil/pf/pf_ioctl.c

2888

You are correct. I had misread the if (ps->ps_len <= 0) { block.

[kp]

Add ps_req_version
Userspace can provide the version it expects, preventing issues with newer
userspace on an older kernel. I expect that in most cases we will only add
fields, so the kernel may supply newer versions than requested. That will be
determined when we do extend the pf_state_export struct.