Bug 250994 - [pf] set skip on $group lost after pf reload
Summary: [pf] set skip on $group lost after pf reload
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: 12.2-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: Kristof Provost
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-11-09 21:15 UTC by pumpy
Modified: 2021-01-14 10:53 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description pumpy 2020-11-09 21:15:15 UTC
affected 12.1 too.

enable pf, add a couple tap interfaces, add `set skip on tap` to pf options config, reboot.

it's working (pfctl -vsI shows "(skip)" after tap group and interfaces) but after running `service pf reload` the (skip) status is lost.
Comment 1 Kristof Provost freebsd_committer 2020-11-09 21:18:55 UTC
That's almost certainly fixed by r366667 / r366647  (r367057 on stable/12), but those didn't come in time for 12.2.

Can you test either CURRENT or a latest stable/12 build?
Comment 2 pumpy 2020-11-13 19:57:47 UTC
(In reply to Kristof Provost from comment #1)

ya ill test 12.2 but it'll take a few days to fit it in. ty!
Comment 3 pumpy 2020-12-06 04:16:00 UTC
(In reply to Kristof Provost from comment #1)

on 12.2-stable r368289 still doesn't work. changed pf.conf set skip on tap0/1 to set skip on tap, reboot, pfctl -vsI shows tap tap0 tap1 (skip), service pf reload, only tap shows (skip). all 3 should still show (skip)
Comment 4 Kristof Provost freebsd_committer 2020-12-07 20:35:18 UTC
(In reply to pumpy from comment #3)
Confirmed, I see the same on CURRENT.

I currently suspect the set skip optimisation code in pfctl, but I'm still debugging.
Comment 5 commit-hook freebsd_committer 2021-01-11 22:32:33 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=0c156a3c32cd0d9168570da5686ddc96abcbbc5a

commit 0c156a3c32cd0d9168570da5686ddc96abcbbc5a
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2021-01-11 13:09:08 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-01-11 21:30:44 +0000

    pfctl: Another set skip <group> fix

    When retrieving the list of group members we cannot simply use
    ifa_lookup(), because it expects the interface to have an IP (v4 or v6)
    address. This means that interfaces with no address are not found.
    This presents as interfacing being alternately marked as skip and not
    whenever the rules are re-loaded.

    Happily we only need to fix ifa_grouplookup(). Teach it to also accept
    AF_LINK (i.e. interface) node_hosts.

    PR:             250994
    MFC after:      3 days

 sbin/pfctl/pfctl_parser.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)
Comment 6 commit-hook freebsd_committer 2021-01-14 10:52:27 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=f9b0587dc2f98800f9f72944fd66a695200c554b

commit f9b0587dc2f98800f9f72944fd66a695200c554b
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2021-01-11 13:09:08 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-01-14 08:39:57 +0000

    pfctl: Another set skip <group> fix

    When retrieving the list of group members we cannot simply use
    ifa_lookup(), because it expects the interface to have an IP (v4 or v6)
    address. This means that interfaces with no address are not found.
    This presents as interfacing being alternately marked as skip and not
    whenever the rules are re-loaded.

    Happily we only need to fix ifa_grouplookup(). Teach it to also accept
    AF_LINK (i.e. interface) node_hosts.

    PR:             250994
    MFC after:      3 days

    (cherry picked from commit 0c156a3c32cd0d9168570da5686ddc96abcbbc5a)

 sbin/pfctl/pfctl_parser.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)