Date: Thu, 28 Mar 2013 15:03:27 +0100 From: Andreas Longwitz <longwitz@incore.de> To: =?ISO-8859-15?Q?Ermal_Lu=E7i?= <eri@freebsd.org>, freebsd-pf@freebsd.org Subject: Re: [patch] Reloading pf rules breaks connections on lo0 Message-ID: <51544DAF.7000203@incore.de> In-Reply-To: <CAPBZQG3YuMyBgChFyjkHs8CTYPioW%2BozBqLwyw56k8jYYS%2B7ww@mail.gmail.com> References: <5134C218.6060701@incore.de> <5149BE75.3040308@incore.de> <CAPBZQG19-ASoz-Cgd2bm9rJyqNw=kqHueKxvzwWgVFb62xJ5dg@mail.gmail.com> <5149E3A8.3020608@incore.de> <CAPBZQG3YuMyBgChFyjkHs8CTYPioW%2BozBqLwyw56k8jYYS%2B7ww@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Ermal Luçi wrote: > > I say intended because so it behaves on the upstream. > By introducing another not needed option you introduce another hack on > top of the already hackish 'set skip' one. > > The correct 'fix' for it to behave correctly is to fetch the interface > list from pf(4) and verify if something needs to be cleared or not. > You can call pfi_get_ifaces and compare it with the defined 'set skip' > rules. > > That is easier than adding a new option. > I agree with your statements completely. The following patch for pfctl.c solves for me the lo0 breaking problem without introducing a new option. The patched pfctl clears the skip flag exactly for those actual skip interfaces not longer included in the new pf.conf anymore. --- pfctl.c.orig 2013-01-14 15:17:48.000000000 +0100 +++ pfctl.c 2013-03-27 22:01:37.000000000 +0100 @@ -67,6 +67,9 @@ int pfctl_enable(int, int); int pfctl_disable(int, int); int pfctl_clear_stats(int, int); +int pfctl_get_skip_ifaces(void); +int pfctl_check_skip_ifaces(char *); +int pfctl_clear_skip_ifaces(struct pfctl *); int pfctl_clear_interface_flags(int, int); int pfctl_clear_rules(int, int, char *); int pfctl_clear_nat(int, int, char *); @@ -101,10 +104,13 @@ struct pf_ruleset *, int, int); int pfctl_load_rule(struct pfctl *, char *, struct pf_rule *, int); const char *pfctl_lookup_option(char *, const char **); +static void radix_perror(void); struct pf_anchor_global pf_anchors; struct pf_anchor pf_main_anchor; +struct pfr_buffer skip_b; + const char *clearopt; char *rulesopt; const char *showopt; @@ -296,6 +302,53 @@ return (0); } +void +radix_perror(void) +{ + extern char *__progname; + fprintf(stderr, "%s: %s.\n", __progname, pfr_strerror(errno)); +} + +int +pfctl_get_skip_ifaces(void) +{ + bzero(&skip_b, sizeof(skip_b)); + skip_b.pfrb_type = PFRB_IFACES; + for (;;) { + pfr_buf_grow(&skip_b, skip_b.pfrb_size); + skip_b.pfrb_size = skip_b.pfrb_msize; + if (pfi_get_ifaces(NULL, skip_b.pfrb_caddr, &skip_b.pfrb_size)) { + radix_perror(); + return (1); + } + if (skip_b.pfrb_size <= skip_b.pfrb_msize) + break; + } + return (0); +} + +int +pfctl_check_skip_ifaces(char *ifname) +{ + struct pfi_kif *p; + + PFRB_FOREACH(p, &skip_b) + if ((p->pfik_flags & PFI_IFLAG_SKIP) && !strcmp(ifname, p->pfik_name)) + p->pfik_flags &= ~PFI_IFLAG_SKIP; + return (0); +} + +int +pfctl_clear_skip_ifaces(struct pfctl *pf) +{ + struct pfi_kif *p; + + PFRB_FOREACH(p, &skip_b) + if (p->pfik_flags & PFI_IFLAG_SKIP) + pfctl_set_interface_flags(pf, p->pfik_name, PFI_IFLAG_SKIP, 0); + return (0); +} + int pfctl_clear_interface_flags(int dev, int opts) { @@ -1437,6 +1490,8 @@ else goto _error; } + if (loadopt & PFCTL_FLAG_OPTION) + pfctl_clear_skip_ifaces(&pf); if ((pf.loadopt & PFCTL_FLAG_FILTER && (pfctl_load_ruleset(&pf, path, rs, PF_RULESET_SCRUB, 0))) || @@ -1861,6 +1916,7 @@ } else { if (ioctl(pf->dev, DIOCSETIFFLAG, &pi)) err(1, "DIOCSETIFFLAG"); + pfctl_check_skip_ifaces(ifname); } } return (0); @@ -2340,7 +2396,7 @@ } if ((rulesopt != NULL) && (loadopt & PFCTL_FLAG_OPTION) && !anchorname[0]) - if (pfctl_clear_interface_flags(dev, opts | PF_OPT_QUIET)) + if (pfctl_get_skip_ifaces()) error = 1; if (rulesopt != NULL && !(opts & (PF_OPT_MERGE|PF_OPT_NOACTION)) && -- Andreas Longwitz
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?51544DAF.7000203>