Date: Thu, 15 Dec 2011 11:15:59 +0100 From: Bernhard Schmidt <bschmidt@freebsd.org> To: Adrian Chadd <adrian@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r228514 - head/sys/net80211 Message-ID: <CAAgh0_Y6y4mWorVKUHmRBZ8iHWng8P4piVgZTBi4GWHgy3PFjw@mail.gmail.com> In-Reply-To: <CAJ-Vmo=Wtk8fzt7-OFPAFPV7bHgOJ5uyZ5CW42Jc%2Bwb1z3CPMA@mail.gmail.com> References: <201112150052.pBF0qUA5022051@svn.freebsd.org> <CAAgh0_avwsQ7uzEnwxKtDnnGjck7tJ5AkjqhBYrz2NuRivbbAg@mail.gmail.com> <CAJ-Vmo=Wtk8fzt7-OFPAFPV7bHgOJ5uyZ5CW42Jc%2Bwb1z3CPMA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Dec 15, 2011 at 10:35, Adrian Chadd <adrian@freebsd.org> wrote: > On 15 December 2011 00:02, Bernhard Schmidt <bschmidt@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAAgh0_Y6y4mWorVKUHmRBZ8iHWng8P4piVgZTBi4GWHgy3PFjw>