On 05/18/11 23:31, Stuart Henderson wrote:
> "set skip" in PF has a slightly unexpected behaviour; rather
> than skipping by interface group, it matches on the non-numeric
> part of an interface name.
I think the prefix match test is a common behaviour so I think you
should keep that. Example granti (note that there is no "sis" group).
$ ifconfig sis | egrep '^sis|^ group'
sis0: flags=28843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST,NOINET6> mtu 1500
groups: a_group
sis1: flags=8842<BROADCAST,RUNNING,SIMPLEX,MULTICAST> mtu 1500
groups: another_group
sis2: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> mtu 1500
groups: yet_another
However it seems not entirely consistant since if I create a "sis"
group, it will only list the interfaces within that group.
Hmmm, looking further, it seems ordinary rules only match on the
interface name or group as well (in pfi_kif_match()), so maybe
you're just plain right after all. :-)
Note that the default ruleset does include a 'set skip on lo' but
that's fine since lo* interfaces are by default added to the "lo"
group. If people get bitten by this change, they could either add
an interface-name-matching group to each interface or we do that
automatically, as is done for vlan's, lo's etc.
So in the end this looks right, so ok halex@, but bear in mind I'm
not by far authoritative in this part of the tree.
/Alexander
> for example:
>
> ifconfig carp5 group foo -group carp
> set skip on carp
>
> -> carp5 is still skipped.
>
> no manpage change included as "set skip" is already documented
> as taking an ifspec, which is later defined as accepting an
> interface-group. but it is a change of behaviour, so it will
> need mentioning in current.html.
>
> any comments, suggestions, ok's?
>
> Index: pf_if.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pf_if.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 pf_if.c
> --- pf_if.c 28 Jun 2010 23:21:41 -0000 1.61
> +++ pf_if.c 18 May 2011 21:22:35 -0000
> @@ -714,7 +714,8 @@ pfi_get_ifaces(const char *name, struct
> int
> pfi_skip_if(const char *filter, struct pfi_kif *p)
> {
> - int n;
> + struct ifg_list *i;
> + int n;
>
> if (filter == NULL || !*filter)
> return (0);
> @@ -724,10 +725,12 @@ pfi_skip_if(const char *filter, struct p
> if (n < 1 || n >= IFNAMSIZ)
> return (1); /* sanity check */
> if (filter[n-1] >= '0' && filter[n-1] <= '9')
> - return (1); /* only do exact match in that case */
> - if (strncmp(p->pfik_name, filter, n))
> - return (1); /* prefix doesn't match */
> - return (p->pfik_name[n] < '0' || p->pfik_name[n] > '9');
> + return (1); /* group names may not end in a digit */
> + if (p->pfik_ifp != NULL)
> + TAILQ_FOREACH(i, &p->pfik_ifp->if_groups, ifgl_next)
> + if (!strncmp(i->ifgl_group->ifg_group, filter, n))
> + return (0); /* iface is in group "filter" */
> + return (1);
> }
>
> int
