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
/\
