Re: Bridge rules
* sven falempin sven.falem...@gmail.com [2012-06-30 02:06]:
- 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]);
+ }
NO WAY.
doing this with string fiddling (in the kernel!) is beyond inacceptible.
--
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/
Re: Bridge rules
On 2012/06/29 20:05, sven falempin wrote: ifconfig bridge0 rule pass in on fxp0 src de:ff:* wouldn't it be simpler to just allow a mask value to be set, then you don't need to mess with extra flag variables, just mask the MAC address with this value before comparison. ifconfig bridge0 rule pass in on fxp0 src *:de:ff what use-case do you have for this? matching on the vendor part sort-of makes sense, but I'm at a loss to see anywhere you might want to match the *end* of a MAC address and ignore the start...
Re: Bridge rules
Stuart, The flag is there to not change old behavior. Of course matching the beggining of mac make sense the rest is just strange behavior. But a mac address could be spoof, so it may be used. Its just a - and an if else. thx. I do not understand the other complain. especilly when it s userland code (the string stuff was done inside ifconfig) Maybe 'you' meant: ifconfig bridge0 rule pass in on fxp0 src de:ff:de:ff:de:ff mask 00:ff:00:ff:00:ff and then do | before bcmp which would be nicer and i can remove the flag by putting a default mask agree Henning ? 2012/6/30 Stuart Henderson s...@spacehopper.org On 2012/06/29 20:05, sven falempin wrote: ifconfig bridge0 rule pass in on fxp0 src de:ff:* wouldn't it be simpler to just allow a mask value to be set, then you don't need to mess with extra flag variables, just mask the MAC address with this value before comparison. ifconfig bridge0 rule pass in on fxp0 src *:de:ff what use-case do you have for this? matching on the vendor part sort-of makes sense, but I'm at a loss to see anywhere you might want to match the *end* of a MAC address and ignore the start... -- - () ascii ribbon campaign - against html e-mail /\
Re: Bridge rules
* sven falempin sven.falem...@gmail.com [2012-06-30 15:49]: I do not understand the other complain. especilly when it s userland code (the string stuff was done inside ifconfig) using string matching for this is the wrong approach to begin with. mac addresses are just numbers, after all. so a msked compare it is. -- 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/
Re: Bridge rules
On 2012/06/30 09:47, sven falempin wrote: Stuart, The flag is there to not change old behavior. Since masking with all 0's is pointless, you can use that to identify the standard behaviour, checking against 0 is a fast way to determine if the mask should be applied at all (this means a mask of all 1's would have the same filtering effect as no mask, but would be slightly slower).
Re: Bridge rules
should be more likely an expected diff 2012/6/30 Stuart Henderson s...@spacehopper.org On 2012/06/30 09:47, sven falempin wrote: Stuart, The flag is there to not change old behavior. Since masking with all 0's is pointless, you can use that to identify the standard behaviour, checking against 0 is a fast way to determine if the mask should be applied at all (this means a mask of all 1's would have the same filtering effect as no mask, but would be slightly slower). -- - () ascii ribbon campaign - against html e-mail /\ [demime 1.01d removed an attachment of type application/octet-stream which had a name of mac-filter.diff]
Re: Bridge rules
beyond the missing in bzero in brconfig.c
i certainly broke something
bridge0: flags=0
groups: bridge
priority 32768 hellotime 2 fwddelay 15 maxage 20 holdcnt 6 proto
rstp
vether0 flags=3LEARNING,DISCOVER
port 6 ifpriority 0 ifcost 0
re0 flags=3LEARNING,DISCOVER
port 1 ifpriority 0 ifcost 0
# ifconfig bridge0 192.168.2.2
ifconfig: SIOCAIFADDR: Inappropriate ioctl for device
# ifconfig rule pass in on bridge0 src fe:e1:00:00:00:00 mask ff:ff:0:0:0:0
tag LOL
ifconfig: SIOCGVH: Device not configured
Probably i m gonna learn something !
Constructive advice welcome.
- - - - - - - - - - -
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 - 1.3
+++ ./sbin/ifconfig/brconfig.c 30 Jun 2012 17:47:31 -
@@ -793,7 +793,8 @@
* Parse a rule definition and send it upwards.
*
* Syntax:
- * {block|pass} {in|out|in/out} on {ifs} [src {mac}] [dst {mac}]
+ * {block|pass} {in|out|in/out} on {ifs}
+ * [src {mac} [mask {mac}]] [dst {mac} [mask {mac}]] [tag {tagname}]
*/
int
bridge_rule(int targc, char **targv, int ln)
@@ -801,7 +802,8 @@
char **argv = targv;
int argc = targc;
struct ifbrlreq rule;
- struct ether_addr *ea, *dea;
+ struct ether_addr *ea, *dea = NULL;
+ u_int8_t pflag = 0;
if (argc == 0) {
warnx(invalid rule\n);
@@ -810,6 +812,8 @@
rule.ifbr_tagname[0] = 0;
rule.ifbr_flags = 0;
rule.ifbr_action = 0;
+ bzero(rule.ifbr_src_mask, sizeof(rule.ifbr_src_mask));
+ bzero(rule.ifbr_dst_mask, sizeof(rule.ifbr_src_mask));
strlcpy(rule.ifbr_name, name, sizeof(rule.ifbr_name));
if (strcmp(argv[0], block) == 0)
@@ -847,21 +851,35 @@
argc--; argv++;
while (argc) {
+ if (argc 2) {
+ warnx(missing %s value\n,argv[0]);
+ goto bad_rule;
+ }
if (strcmp(argv[0], dst) == 0) {
if (rule.ifbr_flags BRL_FLAG_DSTVALID)
goto bad_rule;
- rule.ifbr_flags |= BRL_FLAG_DSTVALID;
+ pflag = BRL_FLAG_DSTVALID;
dea = rule.ifbr_dst;
} else if (strcmp(argv[0], src) == 0) {
if (rule.ifbr_flags BRL_FLAG_SRCVALID)
goto bad_rule;
- rule.ifbr_flags |= BRL_FLAG_SRCVALID;
+ pflag = BRL_FLAG_SRCVALID;
dea = rule.ifbr_src;
- } else if (strcmp(argv[0], tag) == 0) {
- if (argc 2) {
-warnx(missing tag name\n);
+ } else if (strcmp(argv[0], mask) == 0) {
+ if (dea == NULL) {
+warnx(no src or dst mac address before mask\n);
+goto bad_rule;
+ }
+ if ( pflag == 0 ) {
+warnx(multiple mask definition\n);
goto bad_rule;
}
+ pflag = 0;
+ if (rule.ifbr_flags BRL_FLAG_SRCVALID)
+dea = rule.ifbr_src_mask;
+ else
+dea = rule.ifbr_dst_mask;
+ } else if (strcmp(argv[0], tag) == 0) {
if (rule.ifbr_tagname[0]) {
warnx(tag already defined\n);
goto bad_rule;
@@ -876,10 +894,8 @@
goto bad_rule;
argc--; argv++;
-
- if (argc == 0)
- goto bad_rule;
if (dea != NULL) {
+ rule.ifbr_flags |= pflag;
ea = ether_aton(argv[0]);
if (ea == NULL) {
warnx(invalid address: %s, argv[0]);
Index: ./sbin/ifconfig/ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.228
diff -u -r1.228 ifconfig.8
--- ./sbin/ifconfig/ifconfig.8 31 May 2012 17:50:59 - 1.228
+++ ./sbin/ifconfig/ifconfig.8 30 Jun 2012 17:47:31 -
@@ -688,6 +688,7 @@
.Cm on Ar interface
.Op Cm src Ar address
.Op Cm dst Ar address
+.Op Cm mask Ar address
.Op Cm tag Ar tagname
.Xc
Add a filtering rule to an interface.
@@ -703,6 +704,7 @@
and, if given, the tag of the rule.
If no source or destination address is specified, the
rule will match all frames (good for creating a catchall policy).
+mask is only valid after a given src or dst.
.It Cm rulefile Ar filename
Load a set of rules from the file
.Ar filename .
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 - 1.193
+++ ./sys/net/if_bridge.c 30 Jun 2012 17:47:31 -
@@ -145,6 +145,8 @@
struct ifbrlreq *, int out);
int bridge_flushrule(struct bridge_iflist *);
int bridge_brlconf(struct bridge_softc *, struct ifbrlconf *);
+int bridge_test_ea(struct ether_addr *, struct ether_addr *,
+struct ether_addr *);
u_int8_t bridge_filterrule(struct brl_head *, struct ether_header *,
struct mbuf *);
struct mbuf *bridge_ip(struct bridge_softc *, int, struct ifnet *,
@@ -2204,33 +2206,38 @@
return (1);
}
+//inline
+int
+bridge_test_ea(struct ether_addr *ea_packet, struct ether_addr *ea_rules,
+struct ether_addr *ea_mask) {
+ int i;
+ struct ether_addr ea_cmp;
+ for (i = 0; i ETHER_ADDR_LEN; ++i) {
+ ea_cmp.ether_addr_octet[i]
Re: Bridge rules
* sven falempin sven.falem...@gmail.com [2012-06-28 23:53]:
Doc :
ifconfig bridge0 rule pass in on fxp0 src 0:de:ad:be:ef:0 tag USER1
Want to do something like
ifconfig bridge0 rule pass in on fxp0 src 0:de:ad:*:*:* tag
OPENBSDAWESOME
or
ifconfig bridge0 rule pass in on fxp0 src /\\A00:de:ad:/ tag
OPENBSDAWESOME
Read Code :
found, in if_bridge.c (following SIOCBRDGARL from ifconfig)
bcopy(req-ifbr_src, n-brl_src, sizeof(struct ether_addr));
bcopy(req-ifbr_dst, n-brl_dst, sizeof(struct ether_addr));
Search for usage of the (documented) field
find /usr/src/ -type f | xargs grep ifbr_src
/usr/src/share/man/man4/bridge.4: struct ether_addr ifbr_src; /* source mac
*/
/usr/src/sbin/ifconfig/brconfig.c: printf( src %s,
ether_ntoa(r-ifbr_src));
/usr/src/sbin/ifconfig/brconfig.c: dea = rule.ifbr_src;
Now I'm lost
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.
sys/net/if_bridge.h:43:struct ifbreq {
sys/net/if_bridge.h:110:struct ifbreq *ifbicu_req;
sys/sys/sockio.h:96:#define SIOCBRDGADD _IOW('i', 60, struct ifbreq)
/* add bridge ifs */
sys/sys/sockio.h:97:#define SIOCBRDGGSIFS _IOWR('i', 60, struct ifbreq)
/* get span ifs */
sys/sys/sockio.h:98:#define SIOCBRDGDEL _IOW('i', 61, struct ifbreq)
/* del bridge ifs */
sys/sys/sockio.h:99:#define SIOCBRDGGIFFLGS _IOWR('i', 62, struct ifbreq)
/* get brdg if flags */
sys/sys/sockio.h:100:#defineSIOCBRDGSIFFLGS _IOW('i', 63, struct ifbreq)
/* set brdg if flags */
sys/sys/sockio.h:103:#defineSIOCBRDGADDS _IOW('i', 65, struct ifbreq)
/* add span port */
sys/sys/sockio.h:104:#defineSIOCBRDGIFS _IOWR('i', 66, struct ifbreq)
/* get member ifs */
sys/sys/sockio.h:105:#defineSIOCBRDGDELS _IOW('i', 66, struct ifbreq)
/* del span port */
sys/sys/sockio.h:111:#defineSIOCBRDGFLUSH_IOW('i', 72, struct ifbreq)
/* flush addr cache */
sys/sys/sockio.h:124:#defineSIOCBRDGSIFPRIO _IOW('i', 84, struct ifbreq)
/* set if priority */
sys/sys/sockio.h:125:#defineSIOCBRDGSIFCOST _IOW('i', 85, struct ifbreq)
/* set if cost */
sys/net/bridgestp.c:2126: struct ifbreq *ifbr = (struct ifbreq *)data;
sys/net/if_bridge.c:291:struct ifbreq *req = (struct ifbreq *)data;
sys/net/if_bridge.c:789:struct ifbreq *breq = NULL;
sys/net/if_bridge.c:802:if ((breq = (struct ifbreq *)
the actual filtering is in sys/net/if_bridge.c, bridge_filterrule().
--
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/
Re: Bridge rules
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 and pretends to be done with the homework (: * sven falempin sven.falem...@gmail.com [2012-06-28 23:53]: Want to do something like ifconfig bridge0 rule pass in on fxp0 src 0:de:ad:*:*:* tag ZOMG or ifconfig bridge0 rule pass in on fxp0 src /\\A00:de:ad:/ tag RERULZ Now I'm lost of course you are (;
Re: Bridge rules
* 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. -- 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/
Re: Bridge rules
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 /\
Re: Bridge rules
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)
--
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 - 1.193
+++ sys/net/if_bridge.c 29 Jun 2012 19:05:19 -
@@ -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
/\
Re: Bridge rules
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 - 1.193
+++ sys/net/if_bridge.c 29 Jun 2012 19:05:19 -
@@ -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
/\
Re: Bridge rules
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 - 1.34
+++ sys/net/if_bridge.h 29 Jun 2012 20:18:43 -
@@ -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_tifbr_action;/* disposition */
u_int8_tifbr_flags; /* flags */
struct ether_addr ifbr_src; /* source mac */
+ charifbr_src_re[BRL_RE_MAX];/* source mac
regular expression */
struct ether_addr ifbr_dst; /* destination mac
*/
+ charifbr_dst_re[BRL_RE_MAX];/* destination mac
regular expression */
charifbr_tagname[PF_TAG_NAME_SIZE]; /* pf
tagname */
};
#defineBRL_ACTION_BLOCK0x01/* block
frame */
@@ -203,6 +208,8 @@
#defineBRL_FLAG_OUT0x04/* output
rule */
#defineBRL_FLAG_SRCVALID 0x02/* src
valid */
#defineBRL_FLAG_DSTVALID 0x01/* dst
valid */
+#defineBRL_FLAG_SRC_RE 0x10/* src is
regex */
+#defineBRL_FLAG_DST_RE 0x20/* dst is
regex */
struct ifbrlconf {
charifbrl_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_tbrl_action; /* what to do with match */
u_int8_tbrl_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 - 1.193
+++ sys/net/if_bridge.c 29 Jun 2012 19:05:19 -
@@ -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;
-
Re: Bridge rules
2012/6/29 Bret Lambert bret.lamb...@gmail.com
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 sven.falem...@gmail.com
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 - 1.34
+++ sys/net/if_bridge.h 29 Jun 2012 20:18:43 -
@@ -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_tifbr_action;/* disposition */
u_int8_tifbr_flags; /* flags */
struct ether_addr ifbr_src; /* source mac */
+ charifbr_src_re[BRL_RE_MAX];/* source mac
regular expression */
struct ether_addr ifbr_dst; /* destination mac
*/
+ charifbr_dst_re[BRL_RE_MAX];/* destination
mac
regular expression */
charifbr_tagname[PF_TAG_NAME_SIZE]; /* pf
tagname */
};
#defineBRL_ACTION_BLOCK0x01/* block
frame */
@@ -203,6 +208,8 @@
#defineBRL_FLAG_OUT0x04/* output
rule */
#defineBRL_FLAG_SRCVALID 0x02/* src
valid */
#defineBRL_FLAG_DSTVALID 0x01/* dst
valid */
+#defineBRL_FLAG_SRC_RE 0x10/* src is
regex */
+#defineBRL_FLAG_DST_RE 0x20/* dst is
regex */
struct ifbrlconf {
charifbrl_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_tbrl_action; /* what to do with match
*/
u_int8_tbrl_flags; /* comparision flags */
2012/6/29 Ted Unangst t...@tedunangst.com
On Fri, Jun 29, 2012 at 15:08, sven falempin
Re: Bridge rules
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.c14 Dec 2009 19:22:20 - 1.3
+++ sbin/ifconfig/brconfig.c30 Jun 2012 00:02:57 -
@@ -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]);
