2012/6/29 Bret Lambert <[email protected]> > Holy crap, you're doing this in a way too fucking complicated manner: > > for (i = 0; i < ETHER_ADDR_LEN; i++) > if (addr[i] != match[i] && match[i] != '*') > return (ENOTAMACTCH); > > Why do people want to cram useless shit where it doesn't want to be? > > A regex parser in the kernel? WTF? Move to a less powerful grade of > drugs, for god's sake. > > Yes, as i said , overkill it is, but the doc would be nice (use regexp)
the 'simpler' it add a new syntax for the ifconfig, so you put a substring with a start you ll have to explain and that s new knowledge As you should now regexp, the complex way use less user brain cells :D SO I do a OFFSET,STRING matching ? like ifconfig bridge0 rule pass in on fxp0 src m1,de:fe > On Fri, Jun 29, 2012 at 10:50 PM, sven falempin <[email protected]> > wrote: > > 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 <[email protected]> > > > >> 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 <[email protected]> > >> > > >> >> 2012/6/29 Henning Brauer <[email protected]> > >> >> > >> >>> * Mike Belopuhov <[email protected]> [2012-06-29 13:46]: > >> >>> > On Fri, Jun 29, 2012 at 1:36 PM, Henning Brauer > >> >>> > <[email protected]> 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, [email protected], [email protected] > >> >>> 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 /\
