compilable diff, ( i reboot new kernel and test userland l8r ) ifconfig bridge0 rule pass in on fxp0 src *:de:ff ifconfig bridge0 rule pass in on fxp0 src de:ff:*
rantings time ~~~~~~~~~~~~~~~~~ Bret aint't no fool when it comes to the kernel do not make a mess ~~~~~~~~~~~~~~~~~ did it from today snapshot with current cvs OpenBSD currentBSD.whatever.sub 5.2 GENERIC#251 i386 qemu/kvm $ dmesg \M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M -r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r \M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M -]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-] \M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M ^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^? \^?\M-r\M-]\M^?\^?\M-r\M-]\M [...] r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?\M-r\M-]\M^?\^?OpenB SD 5.2-beta (RAMDISK_CD) #180: Thu Jun 28 01:45:40 MDT 2012 t...@i386.openbsd.org:/usr/src/sys/arch/i386/compile/RAMDISK_CD cpu0: QEMU Virtual CPU version 0.13.0 ("GenuineIntel" 686-class) 2.40 GHz [...] syncing disks... done rebooting... OpenBSD 5.2-beta (GENERIC) #251: Thu Jun 28 01:30:25 MDT 2012 t...@i386.openbsd.org:/usr/src/sys/arch/i386/compile/GENERIC [...] ~~~~~~~~~~~~~~~~~ Index: sbin/ifconfig/brconfig.c =================================================================== RCS file: /cvs/src/sbin/ifconfig/brconfig.c,v retrieving revision 1.3 diff -u -r1.3 brconfig.c --- sbin/ifconfig/brconfig.c 14 Dec 2009 19:22:20 -0000 1.3 +++ sbin/ifconfig/brconfig.c 30 Jun 2012 00:02:57 -0000 @@ -802,6 +802,9 @@ int argc = targc; struct ifbrlreq rule; struct ether_addr *ea, *dea; + int m_flag = 0, m_size = 0; + int8_t* m_b; + char* mac; if (argc == 0) { warnx("invalid rule\n"); @@ -852,11 +855,15 @@ goto bad_rule; rule.ifbr_flags |= BRL_FLAG_DSTVALID; dea = &rule.ifbr_dst; + m_flag = BRL_FLAG_DST_M; + m_b = &rule.ifbr_dst_mb; } else if (strcmp(argv[0], "src") == 0) { if (rule.ifbr_flags & BRL_FLAG_SRCVALID) goto bad_rule; rule.ifbr_flags |= BRL_FLAG_SRCVALID; dea = &rule.ifbr_src; + m_flag = BRL_FLAG_SRC_M; + m_b = &rule.ifbr_src_mb; } else if (strcmp(argv[0], "tag") == 0) { if (argc < 2) { warnx("missing tag name\n"); @@ -880,7 +887,40 @@ if (argc == 0) goto bad_rule; if (dea != NULL) { - ea = ether_aton(argv[0]); + m_size = strnlen(argv[0], ETHER_ADDR_LEN+1 ); + if ( m_size > ETHER_ADDR_LEN || m_size < 3 ) { + warnx("mac address expression too long or too small %s", argv[0]); + return (1); + } + if ( ( argv[0][0] == '*' && argv[0][1] == ':' ) || + ( argv[0][m_size-1] == '*' && argv[0][m_size-2] == ':' ) + ) { + int n = 0; + char* mac = malloc( (ETHER_ADDR_LEN+1)*sizeof(char) ); + char* p; + if ( mac == NULL ) { + warnx("not enough memory"); + return (1); + } + for ( p = argv[0]; *p != '\0'; ++p) { + if ( *p == ':' ) n++; + } + if ( argv[0][0] == '*' ) { + for (; n > 0; --n) strlcat( mac, "0:", ETHER_ADDR_LEN); + strlcat( mac, &(argv[0][2]), ETHER_ADDR_LEN); + *m_b = -n; + } + if ( argv[0][m_size-1] == '*' ) { + strlcat( mac, argv[0], ETHER_ADDR_LEN); + for (; n > 0; --n) strlcat( mac, ":0", ETHER_ADDR_LEN); + *m_b = n; + } + rule.ifbr_flags |= m_flag; + ea = ether_aton(mac); + free(mac); + } else { + ea = ether_aton(argv[0]); + } if (ea == NULL) { warnx("invalid address: %s", argv[0]); return (1); Index: sys/net/if_bridge.c =================================================================== RCS file: /cvs/src/sys/net/if_bridge.c,v retrieving revision 1.193 diff -u -r1.193 if_bridge.c --- sys/net/if_bridge.c 4 Jul 2011 06:54:49 -0000 1.193 +++ sys/net/if_bridge.c 30 Jun 2012 00:02:57 -0000 @@ -2208,29 +2208,37 @@ bridge_filterrule(struct brl_head *h, struct ether_header *eh, struct mbuf *m) { struct brl_node *n; - u_int8_t flags; SIMPLEQ_FOREACH(n, h, brl_next) { - flags = n->brl_flags & (BRL_FLAG_SRCVALID|BRL_FLAG_DSTVALID); - if (flags == 0) - goto return_action; - if (flags == (BRL_FLAG_SRCVALID|BRL_FLAG_DSTVALID)) { - if (bcmp(eh->ether_shost, &n->brl_src, ETHER_ADDR_LEN)) - continue; - if (bcmp(eh->ether_dhost, &n->brl_dst, ETHER_ADDR_LEN)) - continue; - goto return_action; + if ( n->brl_flags & BRL_FLAG_SRCVALID ) { + if ( n->brl_flags & BRL_FLAG_SRC_M ) { + if ( n->brl_src_size < 0 ) { + if (bcmp(eh->ether_shost + n->brl_src_size, + &n->brl_src, n->brl_src_size)) + continue; + } else { + if (bcmp(eh->ether_shost, &n->brl_src, n->brl_src_size)) + continue; + } + } else + if (bcmp(eh->ether_shost, &n->brl_src, ETHER_ADDR_LEN)) + continue; } - if (flags == BRL_FLAG_SRCVALID) { - if (bcmp(eh->ether_shost, &n->brl_src, ETHER_ADDR_LEN)) - continue; - goto return_action; - } - if (flags == BRL_FLAG_DSTVALID) { - if (bcmp(eh->ether_dhost, &n->brl_dst, ETHER_ADDR_LEN)) - continue; - goto return_action; + if (n->brl_flags & BRL_FLAG_DSTVALID) { + if ( n->brl_flags & BRL_FLAG_DST_M ) { + if ( n->brl_dst_size < 0 ) { + if (bcmp(eh->ether_shost + n->brl_dst_size, + &n->brl_dst, n->brl_dst_size)) + continue; + } else { + if (bcmp(eh->ether_shost, &n->brl_dst, n->brl_dst_size)) + continue; + } + } else + if (bcmp(eh->ether_dhost, &n->brl_dst, ETHER_ADDR_LEN)) + continue; } + goto return_action; } return (BRL_ACTION_PASS); @@ -2251,6 +2259,8 @@ return (ENOMEM); bcopy(&req->ifbr_src, &n->brl_src, sizeof(struct ether_addr)); bcopy(&req->ifbr_dst, &n->brl_dst, sizeof(struct ether_addr)); + n->brl_src_size = req->ifbr_src_mb; + n->brl_dst_size = req->ifbr_dst_mb; n->brl_action = req->ifbr_action; n->brl_flags = req->ifbr_flags; #if NPF > 0 Index: sys/net/if_bridge.h =================================================================== RCS file: /cvs/src/sys/net/if_bridge.h,v retrieving revision 1.34 diff -u -r1.34 if_bridge.h --- sys/net/if_bridge.h 20 Nov 2010 14:23:09 -0000 1.34 +++ sys/net/if_bridge.h 30 Jun 2012 00:02:57 -0000 @@ -194,7 +194,9 @@ u_int8_t ifbr_action; /* disposition */ u_int8_t ifbr_flags; /* flags */ struct ether_addr ifbr_src; /* source mac */ + int8_t ifbr_src_mb; /* source mac meaning bytes (minus is from the end) */ struct ether_addr ifbr_dst; /* destination mac */ + int8_t ifbr_dst_mb; /* destination mac meaning bytes */ char ifbr_tagname[PF_TAG_NAME_SIZE]; /* pf tagname */ }; #define BRL_ACTION_BLOCK 0x01 /* block frame */ @@ -203,6 +205,8 @@ #define BRL_FLAG_OUT 0x04 /* output rule */ #define BRL_FLAG_SRCVALID 0x02 /* src valid */ #define BRL_FLAG_DSTVALID 0x01 /* dst valid */ +#define BRL_FLAG_SRC_M 0x10 /* src is a pattern */ +#define BRL_FLAG_DST_M 0x20 /* dst is a pattern */ struct ifbrlconf { char ifbrl_name[IFNAMSIZ]; /* bridge ifs name */ @@ -257,7 +261,9 @@ struct brl_node { SIMPLEQ_ENTRY(brl_node) brl_next; /* next rule */ struct ether_addr brl_src; /* source mac address */ + int8_t brl_src_size; /* source mac address meaning bytes*/ struct ether_addr brl_dst; /* destination mac address */ + int8_t brl_dst_size; /* destination mac address meaning bytes */ u_int16_t brl_tag; /* pf tag ID */ u_int8_t brl_action; /* what to do with match */ u_int8_t brl_flags; /* comparision flags */ 2012/6/29 sven falempin <sven.falem...@gmail.com> > Feel free to commit those code refactoring :) > > So, > I have problem compiling my diff -for re mac bridge taging- for testing. > > ../../../../net/if_bridge.h:40:19: error: regex.h: No such file or > directory > > but /usr/src/include/regex.h looks quite accessible .. > > # find /usr/src -type f -name regex.h > /usr/src/gnu/gcc/fixincludes/tests/base/regex.h > /usr/src/gnu/usr.bin/cvs/lib/regex.h > /usr/src/gnu/usr.bin/gcc/gcc/fixinc/tests/base/regex.h > /usr/src/include/regex.h > > (those 4 same include file name are scary lol, bug galore ahead !) > > Anyway i dont like my diff because struct brl_node does become a non POD > type, with a regfree > i am tempted to use the C power : bad code > like > char thatsnocharbutp[LARGEPLACE] > then bcopy the reg into it. (because data are data) > I m quite sure you wont like it ? > > Other possibility is to regcomp just after the regexec (even uglyer IMHO) > or not using regexp at all and just allow stupider matching, > like ignoring a part of mac address "" > Which would be much Faster > ifconfig bridge0 rule pass in on fxp0 src 1,1,de > to do > ifconfig bridge0 rule pass in on fxp0 src *:de:*:*:*:* > and > src 2,1,de,3,1,ff > or > src 2,2,de:ff > to do > ifconfig bridge0 rule pass in on fxp0 src *:de:ff:*:*:* > > because regexp is overkill > > here is the header mods for current code i m trying to test > > Index: sys/net/if_bridge.h > =================================================================== > RCS file: /cvs/src/sys/net/if_bridge.h,v > retrieving revision 1.34 > diff -u -r1.34 if_bridge.h > --- sys/net/if_bridge.h 20 Nov 2010 14:23:09 -0000 1.34 > +++ sys/net/if_bridge.h 29 Jun 2012 20:18:43 -0000 > @@ -36,6 +36,8 @@ > #define _NET_IF_BRIDGE_H_ > > #include <net/pfvar.h> > +#include <sys/types.h> > +#include <regex.h> > > /* > * Bridge control request: add/delete member interfaces. > @@ -185,6 +187,7 @@ > struct timeval ifbop_last_tc_time; > }; > > +#define BRL_RE_MAX 64 /* maximum length > of regular expression string for mac address*/ > /* > * Bridge mac rules > */ > @@ -194,7 +197,9 @@ > u_int8_t ifbr_action; /* disposition */ > u_int8_t ifbr_flags; /* flags */ > struct ether_addr ifbr_src; /* source mac */ > + char ifbr_src_re[BRL_RE_MAX];/* source mac > regular expression */ > struct ether_addr ifbr_dst; /* destination mac > */ > + char ifbr_dst_re[BRL_RE_MAX];/* destination mac > regular expression */ > char ifbr_tagname[PF_TAG_NAME_SIZE]; /* pf > tagname */ > }; > #define BRL_ACTION_BLOCK 0x01 /* block > frame */ > @@ -203,6 +208,8 @@ > #define BRL_FLAG_OUT 0x04 /* output > rule */ > #define BRL_FLAG_SRCVALID 0x02 /* src > valid */ > #define BRL_FLAG_DSTVALID 0x01 /* dst > valid */ > +#define BRL_FLAG_SRC_RE 0x10 /* src is > regex */ > +#define BRL_FLAG_DST_RE 0x20 /* dst is > regex */ > > struct ifbrlconf { > char ifbrl_name[IFNAMSIZ]; /* bridge ifs name */ > @@ -257,7 +264,9 @@ > struct brl_node { > SIMPLEQ_ENTRY(brl_node) brl_next; /* next rule */ > struct ether_addr brl_src; /* source mac address */ > + struct regex_t brl_src_preg; /* source mac > address regular expression */ > struct ether_addr brl_dst; /* destination mac address > */ > + struct regex_t brl_dst_preg; /* destination mac > address regular expression */ > u_int16_t brl_tag; /* pf tag ID */ > u_int8_t brl_action; /* what to do with match */ > u_int8_t brl_flags; /* comparision flags */ > > > 2012/6/29 Ted Unangst <t...@tedunangst.com> > >> On Fri, Jun 29, 2012 at 15:08, sven falempin wrote: >> > Code Rewriting (nothing new) and asking >> > >> > I seriously wonder if 'that' is good in sys/net/if_bridge.c >> > if (flags == 0) >> > goto return_action; >> > >> > Because if i m not wrong it could be rewritten this way (diff) >> >> That does look clearer to me. >> > Index: sys/net/if_bridge.c >> > =================================================================== >> > RCS file: /cvs/src/sys/net/if_bridge.c,v >> > retrieving revision 1.193 >> > diff -u -r1.193 if_bridge.c >> > --- sys/net/if_bridge.c 4 Jul 2011 06:54:49 -0000 1.193 >> > +++ sys/net/if_bridge.c 29 Jun 2012 19:05:19 -0000 >> > @@ -2208,29 +2208,17 @@ >> > bridge_filterrule(struct brl_head *h, struct ether_header *eh, struct >> mbuf >> > *m) >> > { >> > struct brl_node *n; >> > - u_int8_t flags; >> > >> > SIMPLEQ_FOREACH(n, h, brl_next) { >> > - flags = n->brl_flags & >> > (BRL_FLAG_SRCVALID|BRL_FLAG_DSTVALID); >> > - if (flags == 0) >> > - goto return_action; >> > - if (flags == (BRL_FLAG_SRCVALID|BRL_FLAG_DSTVALID)) { >> > + if ( n->brl_flags & BRL_FLAG_SRCVALID ) { >> > if (bcmp(eh->ether_shost, &n->brl_src, >> > ETHER_ADDR_LEN)) >> > - continue; >> > - if (bcmp(eh->ether_dhost, &n->brl_dst, >> > ETHER_ADDR_LEN)) >> > - continue; >> > - goto return_action; >> > + continue; >> > } >> > - if (flags == BRL_FLAG_SRCVALID) { >> > - if (bcmp(eh->ether_shost, &n->brl_src, >> > ETHER_ADDR_LEN)) >> > + if (n->brl_flags & BRL_FLAG_DSTVALID) { >> > + if (bcmp(eh->ether_dhost, &n->brl_dst, >> > ETHER_ADDR_LEN)) >> > continue; >> > - goto return_action; >> > - } >> > - if (flags == BRL_FLAG_DSTVALID) { >> > - if (bcmp(eh->ether_dhost, &n->brl_dst, >> > ETHER_ADDR_LEN)) >> > - continue; >> > - goto return_action; >> > } >> > + goto return_action; >> > } >> > return (BRL_ACTION_PASS); >> > 2012/6/29 sven falempin <sven.falem...@gmail.com> >> > >> >> 2012/6/29 Henning Brauer <lists-openbsdt...@bsws.de> >> >> >> >>> * Mike Belopuhov <m...@crypt.org.ru> [2012-06-29 13:46]: >> >>> > On Fri, Jun 29, 2012 at 1:36 PM, Henning Brauer >> >>> > <lists-openbsdt...@bsws.de> wrote: >> >>> > > now it's very unclear what your actual problem is - the struct is >> >>> > > called ifbreq and used in a number of places, most notably of >> course >> >>> > > the ioctls. >> >>> > he's trying to add patterns to the mac address matching code >> >>> >> >>> got that much ;) >> >>> >> >>> > and pretends to be done with the homework (: >> >>> >> >>> well... let's see wether we'll get a diff. >> >>> >> >>> >> >> is there a theory that '&' is more time consuming than ' ==' >> >> because the flag use is weard >> >> >> >> Must .. compile ... all .. kernel .... :( (Am i right ?) >> >> >> >> >> >>> -- >> >>> Henning Brauer, h...@bsws.de, henn...@openbsd.org >> >>> BS Web Services, http://bsws.de, Full-Service ISP >> >>> Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to >> Fully >> >>> Managed >> >>> Henning Brauer Consulting, http://henningbrauer.com/ >> >>> >> >>> >> >> >> >> >> >> -- >> >> () ascii ribbon campaign - against html e-mail >> >> /\ >> >> >> >> >> > >> > >> > > > > -- > () ascii ribbon campaign - against html e-mail > /\ > > -- () ascii ribbon campaign - against html e-mail /\