Page MenuHomeFreeBSD

ipfilter: Set ipf -T optionlist at boot
ClosedPublic

Authored by cy on Oct 30 2024, 7:36 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Dec 2, 2:50 AM
Unknown Object (File)
Fri, Nov 22, 6:44 AM
Unknown Object (File)
Thu, Nov 21, 11:09 AM
Unknown Object (File)
Thu, Nov 21, 10:25 AM
Unknown Object (File)
Wed, Nov 20, 6:44 AM
Unknown Object (File)
Wed, Nov 20, 6:33 AM
Unknown Object (File)
Wed, Nov 20, 4:20 AM
Unknown Object (File)
Tue, Nov 12, 5:46 PM
Subscribers

Details

Summary

There is no easy way to set ipfilter optionlist variables during boot.
Add plumbing to the rc script to support this.

Original by: G. Paul Ziemba <p-fbsd-bugs@ziemba.us>
PR: 130555
MFC 1 week

Test Plan

Running here

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

cy requested review of this revision.Oct 30 2024, 7:36 PM
jlduran added inline comments.
libexec/rc/rc.d/ipfilter
37

I believe this should now be:

if ${ipfilter_program:-/sbin/ipf} -V | grep -q 'Running: yes'; then

Reference: 854cb10a5882ba2577bafb8eea9a1e383315f53e

cy marked an inline comment as done.Oct 31 2024, 4:55 PM

Fixed, plus another change.

libexec/rc/rc.d/ipfilter
37

You're indeed correct. Thanks for reminding me.

cy marked an inline comment as done.

This update makes the following changes:

  1. It fixes the issue jlduran pinted out.
  2. It reduces the check for ipfilter running by one grep.

I would have just added:

if [ -n "${ifilter_optionlist}" ]; then
	if ${ipfilter_program:-/sbin/ipf} -V | grep -q 'Running: yes'; then
		${ipfilter_program:-/sbin/ipf} -D
	fi
	${ipfilter_program:-/sbin/ipf} -T "${ipfilter_optionlist}"
fi

But the current code does the job well and it's arguably more explicit.

This revision is now accepted and ready to land.Oct 31 2024, 8:35 PM

Sorry, thinking more about it, this is essentially an rc patch.
The start action will always restart if there is an ipfilter_optionlist defined. Two questions:

  1. Can ipfilter rely more on rc.subr?
  2. If i understand 130555 wouldn't this want an extra action?

EDIT: Not suggesting that it should be changed/added in this review.

Sorry, thinking more about it, this is essentially an rc patch.
The start action will always restart if there is an ipfilter_optionlist defined. Two questions:

  1. Can ipfilter rely more on rc.subr?

No. Unless ipfilter options can/should be shared with other rc scripts, this is specific to ipfilter.

  1. If i understand 130555 wouldn't this want an extra action?

EDIT: Not suggesting that it should be changed/added in this review.

The PR just wants ipfilter options to be set at boot. One could do it as in this revision or a new rc script. Best to be done here, as one could change the optionlist and service ipfilter restart.

This revision was automatically updated to reflect the committed changes.