Page MenuHomeFreeBSD

pf: Introduce nvlist variant of DIOCADDRULE
ClosedPublic

Authored by kp on Apr 2 2021, 6:54 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 10, 10:49 AM
Unknown Object (File)
Sun, Mar 10, 10:49 AM
Unknown Object (File)
Sun, Mar 10, 10:49 AM
Unknown Object (File)
Sun, Mar 10, 10:46 AM
Unknown Object (File)
Sun, Mar 10, 10:46 AM
Unknown Object (File)
Sun, Mar 10, 10:46 AM
Unknown Object (File)
Sun, Mar 10, 10:46 AM
Unknown Object (File)
Sun, Mar 10, 10:29 AM

Details

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")

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj added inline comments.
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?

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

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.

This revision is now accepted and ready to land.Apr 3 2021, 3:37 PM
  • Require the number of array elements to be exactly the maximum number
  • Explicitly check the nvlist error state
This revision now requires review to proceed.Apr 5 2021, 11:36 AM
glebius added inline comments.
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

  • Use 'unit*' rather than 'u_int*'.
  • Fix potential rule leak
donner added inline comments.
sys/netpfil/pf/pf.h
193

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

kp marked an inline comment as done.Apr 7 2021, 7:01 PM
kp added inline comments.
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 marked an inline comment as done.

Rebase (remove rt_listid)

This revision was not accepted when it landed; it landed in state Needs Review.Apr 10 2021, 9:17 AM
This revision was automatically updated to reflect the committed changes.