pf: Introduce nvlist variant of DIOCADDRULE

[kp]

Summary

This will make future extensions of the API much easier.
The intent is to remove support for DIOCADDRULE in FreeBSD 14.

MFC after: 4 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")

[markj]

sys/netpfil/pf/pf_ioctl.c

96

Why not include it via a system path?

2345

malloc(M_WAITOK) won't fail.

2352

Do nvl and nvlpacked get freed anywhere?

[kp]

• Free nvl/nvlpacked
• Don't check for failure on M_WAITOK

[markj]

Seems ok to me for what it's worth. My comments are mostly nits.

sys/netpfil/pf/pf_ioctl.c

1630

They're equivalent, but it looks like the last parameter should be sizeof(*paddr).

1728

I think there's nothing checking that the supplied array has exactly two elements. Should the checking be more strict, so that an array of length 1 is rejected? There are a few examples of this.

sys/netpfil/pf/pf_nv.c

37

Same thing here with respect to the include path.

[kp]

• Require the number of array elements to be exactly the maximum number
• Explicitly check the nvlist error state

[glebius]

sys/netpfil/pf/pf_ioctl.c

1697

Better use uint8_t in the new code.

1802

Don't we leak rule here?

2025

Better use uint32_t in new code. More of this applies to new files pf_nv.h and pf_nv.c

[kp]

• Use 'unit*' rather than 'u_int*'.
• Fix potential rule leak

[donner]

sys/netpfil/pf/pf.h

193

Component "size" is unused. I do not see any use case for it. Can it be dropped?

[kp]

sys/netpfil/pf/pf.h

193

It's not used here, but it is required for get operations (e.g. D29559). For those userspace must supply a larger buffer than required for just the request, so that the kernel can put the reply in the data buffer.

In that case len will be the length of the request data, but we will have provided size bytes of space for the kernel to put the response into.

[kp]

Rebase (remove rt_listid)