svn commit: r228514 - head/sys/net80211

[Bernhard Schmidt]

On Thu, Dec 15, 2011 at 10:35, Adrian Chadd <adrian at freebsd.org> wrote:
> On 15 December 2011 00:02, Bernhard Schmidt <bschmidt at freebsd.org> wrote:
>
>> Why didn't you remove the mac argument? It is assign from wh->i_addr2
>> anyways, seems rather too redundant to me.
>
> Because the semantics for that right now are "check that MAC", so it's
> the callers responsibility to determine which MAC in the header is the
> relevant one to check against.
>
> They're all addr2 though, and I haven't yet thought of a reason it
> could be addr1 or addr3 (or addr4, for that matter); I just decided to
> leave it this way so the semantics of "the caller dictates which MAC
> in the frame is the relevant one to check against" as-is.

And no one else has found a reason to do so in the last 7 years that
code exists :)

> If you think that's me being a bit overly anal about it, then sure,
> please go ahead and turf it. :)
>
> Personally, I'd like to add an enum field (and then remove the MAC) -
> the enum field would indicate to acl_check() _which_ ACL is being
> checked - ie, probe request, association request, and any other frame
> check request. That way it's precisely clear what the ACL check is
> for. But again, that's just me being overly picky. :)

Well, no. The ACL stuff was designed to have one module for each
usage and not one for everything. Following your example you would
have one for assoc frames/probe frames (whatever the desired behavior
is), .. and the already existing one for macs. Well, just this piece isn't
that optimal yet:

/* XXX just one for now */
static  const struct ieee80211_aclator *acl = NULL;

So, my point is, I'd like to keep the functionality of the wlan_acl(4) module
as it is, matching wh->i_addr2 with the list of given macs only. If you (or
someone) else have some different functionality in mind, add a new acl
module which replaces the current one using ieee80211_aclator_register()
and do whatever you want in there.

> So in short: if you're happy removing it, remove it. :)

I agree on passing the frame as an argument to iac_check() and obtain
the mac from there, that definitely is required for more advanced
ACLs. Passing both tough, is imho not required and redundant, so, yes
I think I'm going to remove it.

-- 
Bernhard