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=3<LEARNING,DISCOVER>
                port 6 ifpriority 0 ifcost 0
        re0 flags=3<LEARNING,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 -0000 1.3
+++ ./sbin/ifconfig/brconfig.c 30 Jun 2012 17:47:31 -0000
@@ -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 -0000 1.228
+++ ./sbin/ifconfig/ifconfig.8 30 Jun 2012 17:47:31 -0000
@@ -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 -0000 1.193
+++ ./sys/net/if_bridge.c 30 Jun 2012 17:47:31 -0000
@@ -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] = ea_mask->ether_addr_octet[i]
+   & ea_packet->ether_addr_octet[i];
+ }
+ return (bcmp(&ea_cmp, ea_rules, ETHER_ADDR_LEN));
+}
+
 u_int8_t
 bridge_filterrule(struct brl_head *h, struct ether_header *eh, struct mbuf
*m)
 {
  struct brl_node *n;
- u_int8_t flags;
+ struct ether_addr *ea_rules,*ea_packet,*ea_mask;

  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 (flags == BRL_FLAG_SRCVALID) {
-   if (bcmp(eh->ether_shost, &n->brl_src, ETHER_ADDR_LEN))
+  ea_rules = ea_packet = ea_mask = NULL;
+  if ( n->brl_flags & BRL_FLAG_SRCVALID ) {
+   if (bridge_test_ea( (struct ether_addr *)eh->ether_shost,
+        &n->brl_src, &n->brl_src_mask))
     continue;
-   goto return_action;
   }
-  if (flags == BRL_FLAG_DSTVALID) {
-   if (bcmp(eh->ether_dhost, &n->brl_dst, ETHER_ADDR_LEN))
+  if (n->brl_flags &  BRL_FLAG_DSTVALID) {
+   if (bridge_test_ea( (struct ether_addr *)eh->ether_dhost,
+        &n->brl_dst, &n->brl_dst_mask))
     continue;
-   goto return_action;
   }
+  goto return_action;
  }
  return (BRL_ACTION_PASS);

@@ -2251,6 +2258,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));
+ bcopy(&req->ifbr_src_mask, &n->brl_src_mask, sizeof(struct ether_addr));
+ bcopy(&req->ifbr_dst_mask, &n->brl_dst_mask, sizeof(struct ether_addr));
  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 17:47:31 -0000
@@ -194,7 +194,9 @@
  u_int8_t  ifbr_action;  /* disposition */
  u_int8_t  ifbr_flags;  /* flags */
  struct ether_addr ifbr_src;  /* source mac */
+ struct ether_addr ifbr_src_mask;  /* source mac mask */
  struct ether_addr ifbr_dst;  /* destination mac */
+ struct ether_addr ifbr_dst_mask;  /* destination mac mask */
  char   ifbr_tagname[PF_TAG_NAME_SIZE]; /* pf tagname */
 };
 #define BRL_ACTION_BLOCK 0x01   /* block frame */
@@ -257,7 +259,9 @@
 struct brl_node {
  SIMPLEQ_ENTRY(brl_node) brl_next; /* next rule */
  struct ether_addr brl_src; /* source mac address */
+ struct ether_addr brl_src_mask; /* source mac address mask */
  struct ether_addr brl_dst; /* destination mac address */
+ struct ether_addr brl_dst_mask; /* destination mac address mask */
  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/30 sven falempin <sven.falem...@gmail.com>

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


-- 
---------------------------------------------------------------------------------------------------------------------
() ascii ribbon campaign - against html e-mail
/\

Reply via email to