Well if that is the problem it would be easy to correct
I think the following examples *prove* that the macmode and maclist for the virtual interfaces are overwriting the macmode and maclist for the associated physical interfaces.
First, if I unset the nvram parameter wl0_vifs (which is the virtual interface associated with eth1 = wl0), by executing:
nvram unset wl0_vifs
then running 'wlconf eth1 up' no longer zeroes out the 'macmode' and 'maclist' entries as confirmed by running 'wl -i eth1 macmode' and 'wl -i eth1 mac' (note I am using a version of wl that I copied from an older Kong firmware that includes the 'macmode' and 'mac' functions)
This presumably works because wlconf then no longer adds the virtual interface wl0.1 associated with the physical interfaces eth1=wl0 to the list (bclist) of interfaces to configure (see excerpted source code below).
Second, alternatively, if I manually create and set new 'macmode' and 'maclist' nvram parameters for wl0.1 (they are not normally set), by executing
nvram set wl0.1_macmode=$(nvram get wl0_macmode)
nvram set wl0.1_maclist="$(nvram get wl0_maclist)"
then running 'wlconf -i eth1' no longer zeros out the 'macmode' and 'maclist' for eth1=wl0 as confirmed by running 'wl -i eth1 macmode' and 'wl -i eth1 mac'
Again this suggests that wlconf first sets the 'macmode' and 'maclist' for eth1 = wl0 based on wl0_macmode and wl0_maclist and then goes back and overwrites it with the 'macmode' and 'maclist' entries for wl0.1 based on wl0.1_macmode and wl0.1_maclist which are normally unset.
The following code associates each virtual interfaces with the underlying physical interface.
Code:
/* additional virtual BSS Configs from wlX_vifs */
if (nvram_default_match(strcat_r(prefix, "mode", tmp),"ap","ap") || nvram_match(strcat_r(prefix, "mode", tmp),"apsta") || nvram_match(strcat_r(prefix, "mode", tmp),"apstawet"))
{
foreach(var, nvram_safe_get(strcat_r(prefix, "vifs", tmp)), next) {
if (bclist->count == WL_MAXBSSCFG) {
WLCONF_DBG("wlconf(%s): exceeded max number of BSS Configs (%d)"
"in nvram %s\n"
"while configuring interface \"%s\"\n",
ifname, WL_MAXBSSCFG, strcat_r(prefix, "vifs", tmp), var);
continue;
}
bsscfg = &bclist->bsscfgs[bclist->count];
if (get_ifname_unit(var, NULL, &bsscfg->idx) != 0) {
WLCONF_DBG("wlconfg(%s): unable to parse unit.subunit in interface "
"name \"%s\"\n",
ifname, var);
continue;
}
strncpy(bsscfg->ifname, var, PREFIX_LEN-1);
snprintf(bsscfg->prefix, PREFIX_LEN, "%s_", bsscfg->ifname);
bclist->count++;
}
These interfaces are then added to the structure 'bclist'
The following code then sets the macmode and maclist for each interface in bclist, presumably causing each virtual interfaces to overwrite the macmode and maclist for the underlying physical interfaces that were previously set.
Code:
for (i = 0; i < bclist->count; i++) {
struct {int bsscfg_idx; int enable;} setbuf;
char vifname[VIFNAME_LEN];
char *name_ptr = name;
/* NAS runs if we have an AKM or radius authentication */
nas_will_run = wlconf_akm_options(bclist->bsscfgs[i].prefix) ||
nvram_default_match(strcat_r(bclist->bsscfgs[i].prefix, "auth_mode", tmp),
"radius","disabled");
/* Set the MAC list */
maclist = (struct maclist *)buf;
maclist->count = 0;
if (!nvram_match(strcat_r(bsscfg->prefix, "macmode", tmp), "disabled")) {
ea = maclist->ea;
foreach(var, nvram_safe_get(strcat_r(bsscfg->prefix, "maclist", tmp)),
next) {
if (((char *)((&ea[1])->octet)) > ((char *)(&buf[sizeof(buf)])))
break;
if (ether_atoe(var, ea->octet)) {
maclist->count++;
ea++;
}
}
}
if (setbuf.bsscfg_idx == 0) {
name_ptr = name;
} else { /* Non-primary BSS; changes name syntax */
char tmp[VIFNAME_LEN];
int len;
So bottom line, the code needs to be adjusted so that the virtual interfaces don't overwrite the macmode and maclist for the associated physical interfaces.
Now the underlying problem may be that that WL_IOCTL doesn't differentiate between virtual and physical interfaces...
The above suggests that running the following simple hack at startup will solve the problem until wlconf is properly fixed.
Code:
nvram set wl0.1_macmode=$(nvram get wl0_macmode)
nvram set wl0.1_maclist="$(nvram get wl0_maclist)"
wlconf eth1 up
nvram set wl1.1_macmode=$(nvram get wl1_macmode)
nvram set wl1.1_maclist="$(nvram get wl1_maclist)"
wlconf eth2 up
Note that this is simpler than the hack I created two years ago (see https://forum.dd-wrt.com/phpBB2/viewtopic.php?t=329902) when I first encountered the problem and it doesn't require 'wl' to have the 'maclist' and 'mac' functions.
Alternatively, you could just run the above once followed by 'nvram commit' but just remember to do this after every change to the MAC filter list.
Last edited by puterboy2 on Thu Aug 12, 2021 1:38; edited 1 time in total
I just realized that my hack may be problematic in that it enforces the same MAC Filtering on the virtual interface as well which may not be what you want for a guest network...
So, we really need to fix wlconf so that it treats the physical interface (e.g., wl0) and the virtual interface (e.g., eth1) separately.
Again the manual 'wl' command proves that the physical and virtual interfaces can be set separately.
Posted: Mon Aug 16, 2021 20:56 Post subject: BUG FOUND AND SOLVED (I think) WITH PATCH ATTACHED
EGC added some debugging code to wlconf.c that helped me confirm the error which then motivated me to read through the wlconf.c code more thoroughly and find the bug. (THANKS EGC!!!!!)
The crux of the problem is that in the FOR loop that iterates through the physical and virtual interfaces corresponding to each physical interface, the macmode and maclist are all set to the value of the nvram macmode and maclist variables for the *last* interface.
So, if you have, for example, eth1 (wl0) and wl0.1, then both eth1 (wl0) and wl0.1 have their macmode and maclist set respectively according to the values of wl0.1_macmode and wl0.1_maclist respectively.
Of course, if there are no virtual interfaces, then everything works since there is only the physical interface (i.e. bclist->count = 1).
The problem is that in wlconf.c, the pointer 'bsscfg' (and in particular 'bsscfg->prefix') is set to '&bclist->bsscfgs[i]' in the prior FOR loop labeled "Security settings for each BSS Configuration" and exits the FOR loop set to "&bclist->bsscfgs[bclist->count -1]" (corresponding to the last interface).
This same pointer is used again in the following FOR loop labeled "Finally enable BSS Configs or Join BSS" where the macmode and maclist are set for each interface but it is never reassigned so it ends up always using the nvram values for the macmode and maclist of the last interface.
What you need to do is add the following to the assignments at the head of the FOR loop where the maclist and macmode are set, (say following the line "char *name_ptr = name")
Code:
bsscfg = &bclist->bsscfgs[i]
See attached patch...
See below for the full context of the code pulled from wlconf.c:
Code:
/* Security settings for each BSS Configuration */
for (i = 0; i < bclist->count; i++) {
bsscfg = &bclist->bsscfgs[i];
wlconf_security_options(name, bsscfg->prefix, bsscfg->idx, wet);
}
#define VIFNAME_LEN 16
/*
* Finally enable BSS Configs or Join BSS
*
* AP: Enable BSS Config to bring AP up only when nas will not run
* STA: Join the BSS regardless.
*/
for (i = 0; i < bclist->count; i++) {
struct {int bsscfg_idx; int enable;} setbuf;
char vifname[VIFNAME_LEN];
char *name_ptr = name;
/* NAS runs if we have an AKM or radius authentication */
nas_will_run = wlconf_akm_options(bclist->bsscfgs[i].prefix) ||
nvram_default_match(strcat_r(bclist->bsscfgs[i].prefix, "auth_mode", tmp),
"radius","disabled");
/* Set the MAC list */
maclist = (struct maclist *)buf;
maclist->count = 0;
if (!nvram_match(strcat_r(bsscfg->prefix, "macmode", tmp), "disabled")) {
ea = maclist->ea;
foreach(var, nvram_safe_get(strcat_r(bsscfg->prefix, "maclist", tmp)),
next) {
if (((char *)((&ea[1])->octet)) > ((char *)(&buf[sizeof(buf)])))
break;
if (ether_atoe(var, ea->octet)) {
maclist->count++;
ea++;
}
}
}
if (setbuf.bsscfg_idx == 0) {
name_ptr = name;
} else { /* Non-primary BSS; changes name syntax */
char tmp[VIFNAME_LEN];
int len;
as it seemed you weren't even sure it was related to this bug let alone suggesting that links 1 and 3 contained a fix. Given that the post was unclear (at least to me), I ended up just debugging the code myself -- though now of course I understand what you were referring to and would have saved time had I just done some diffs as you suggested.
But either way, it's pretty clear that your links #1 & #3 as well as my patch are identical and fix the problem -- though will be great to verify in practice.
Any reason why this bug that was fixed as far back as 2009 in the tomato code was never fixed in dd-wrt?
Regardless, the *cool* thing is that this suggests that if you use the CLI, wlconf (for the Broadcom chipset) can support distinct macmodes and maclists for each physical/virtual interface.
This could also be used (presumably) to get around the limitation of 64 MAC addresses per interfaces as one could create additional virtual interfaces - potentially all using the same SSID and wifi password as the original physical interface but having a different group of up to 64 MAC addresses. Then you could bridge the interfaces as necessary so that they can communicate with each other...
One would have to test this in practice though to see if it actually works
Joined: 08 May 2018 Posts: 14242 Location: Texas, USA
Posted: Tue Aug 17, 2021 3:13 Post subject:
I've emailed BrainSlayer, so to implement the 2009 or 2015 version is up to him. Of course, there was an obvious reason why I posted that information; but at the time, I wasn't going to examine the files closely, or participate further. _________________ "Life is but a fleeting moment, a vapor that vanishes quickly; All is vanity"
Contribute To DD-WRT Pogo - A minimal level of ability is expected and needed... DD-WRT Releases 2023 (PolitePol)
DD-WRT Releases 2023 (RSS Everything)
----------------------
Linux User #377467 counter.li.org / linuxcounter.net
Joined: 06 Jun 2006 Posts: 7492 Location: Dresden, Germany
Posted: Tue Aug 17, 2021 8:48 Post subject:
puterboy2 wrote:
egc wrote:
Well if that is the problem it would be easy to correct
I think the following examples *prove* that the macmode and maclist for the virtual interfaces are overwriting the macmode and maclist for the associated physical interfaces.
First, if I unset the nvram parameter wl0_vifs (which is the virtual interface associated with eth1 = wl0), by executing:
nvram unset wl0_vifs
then running 'wlconf eth1 up' no longer zeroes out the 'macmode' and 'maclist' entries as confirmed by running 'wl -i eth1 macmode' and 'wl -i eth1 mac' (note I am using a version of wl that I copied from an older Kong firmware that includes the 'macmode' and 'mac' functions)
This presumably works because wlconf then no longer adds the virtual interface wl0.1 associated with the physical interfaces eth1=wl0 to the list (bclist) of interfaces to configure (see excerpted source code below).
Second, alternatively, if I manually create and set new 'macmode' and 'maclist' nvram parameters for wl0.1 (they are not normally set), by executing
nvram set wl0.1_macmode=$(nvram get wl0_macmode)
nvram set wl0.1_maclist="$(nvram get wl0_maclist)"
then running 'wlconf -i eth1' no longer zeros out the 'macmode' and 'maclist' for eth1=wl0 as confirmed by running 'wl -i eth1 macmode' and 'wl -i eth1 mac'
Again this suggests that wlconf first sets the 'macmode' and 'maclist' for eth1 = wl0 based on wl0_macmode and wl0_maclist and then goes back and overwrites it with the 'macmode' and 'maclist' entries for wl0.1 based on wl0.1_macmode and wl0.1_maclist which are normally unset.
The following code associates each virtual interfaces with the underlying physical interface.
Code:
/* additional virtual BSS Configs from wlX_vifs */
if (nvram_default_match(strcat_r(prefix, "mode", tmp),"ap","ap") || nvram_match(strcat_r(prefix, "mode", tmp),"apsta") || nvram_match(strcat_r(prefix, "mode", tmp),"apstawet"))
{
foreach(var, nvram_safe_get(strcat_r(prefix, "vifs", tmp)), next) {
if (bclist->count == WL_MAXBSSCFG) {
WLCONF_DBG("wlconf(%s): exceeded max number of BSS Configs (%d)"
"in nvram %s\n"
"while configuring interface \"%s\"\n",
ifname, WL_MAXBSSCFG, strcat_r(prefix, "vifs", tmp), var);
continue;
}
bsscfg = &bclist->bsscfgs[bclist->count];
if (get_ifname_unit(var, NULL, &bsscfg->idx) != 0) {
WLCONF_DBG("wlconfg(%s): unable to parse unit.subunit in interface "
"name \"%s\"\n",
ifname, var);
continue;
}
strncpy(bsscfg->ifname, var, PREFIX_LEN-1);
snprintf(bsscfg->prefix, PREFIX_LEN, "%s_", bsscfg->ifname);
bclist->count++;
}
These interfaces are then added to the structure 'bclist'
The following code then sets the macmode and maclist for each interface in bclist, presumably causing each virtual interfaces to overwrite the macmode and maclist for the underlying physical interfaces that were previously set.
Code:
for (i = 0; i < bclist->count; i++) {
struct {int bsscfg_idx; int enable;} setbuf;
char vifname[VIFNAME_LEN];
char *name_ptr = name;
/* NAS runs if we have an AKM or radius authentication */
nas_will_run = wlconf_akm_options(bclist->bsscfgs[i].prefix) ||
nvram_default_match(strcat_r(bclist->bsscfgs[i].prefix, "auth_mode", tmp),
"radius","disabled");
/* Set the MAC list */
maclist = (struct maclist *)buf;
maclist->count = 0;
if (!nvram_match(strcat_r(bsscfg->prefix, "macmode", tmp), "disabled")) {
ea = maclist->ea;
foreach(var, nvram_safe_get(strcat_r(bsscfg->prefix, "maclist", tmp)),
next) {
if (((char *)((&ea[1])->octet)) > ((char *)(&buf[sizeof(buf)])))
break;
if (ether_atoe(var, ea->octet)) {
maclist->count++;
ea++;
}
}
}
if (setbuf.bsscfg_idx == 0) {
name_ptr = name;
} else { /* Non-primary BSS; changes name syntax */
char tmp[VIFNAME_LEN];
int len;
So bottom line, the code needs to be adjusted so that the virtual interfaces don't overwrite the macmode and maclist for the associated physical interfaces.
Now the underlying problem may be that that WL_IOCTL doesn't differentiate between virtual and physical interfaces...
no. read carefully thecode. name_ptr contains not the physical interface name if bsscfg idx is > 0. so its sets it individually for each bss instead of just the physical interface _________________ "So you tried to use the computer and it started smoking? Sounds like a Mac to me.." - Louis Rossmann https://www.youtube.com/watch?v=eL_5YDRWqGE&t=60s
Joined: 06 Jun 2006 Posts: 7492 Location: Dresden, Germany
Posted: Tue Aug 17, 2021 8:55 Post subject:
i have to correct myself. your overlong explaination is basicly just a description for a small typo. i will review the code more. i found also other similar problems in it _________________ "So you tried to use the computer and it started smoking? Sounds like a Mac to me.." - Louis Rossmann https://www.youtube.com/watch?v=eL_5YDRWqGE&t=60s
@