Re: [libvirt] [PATCH V1 4/9] Add a mac chain

2011-11-22 Thread Stefan Berger

On 11/21/2011 05:13 PM, Eric Blake wrote:

On 10/26/2011 09:12 AM, Stefan Berger wrote:

With hunks borrowed from one of David Steven's previous patches, we now
add the capability of having a 'mac' chain which is useful to filter
for multiple valid MAC addresses.

Signed-off-by: David L Stevensdlstev...@us.ibm.com
Signed-off-by: Stefan Bergerstef...@linux.vnet.ibm.com

+char protostr[16] = { 0, };

A bit oversized...



  PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
  PRINT_CHAIN(chain, chainPrefix, ifname,
  (filtername) ? filtername : l3_protocols[protoidx].val);

+switch (protoidx) {
+case L2_PROTO_MAC_IDX:
+break;
+default:
+snprintf(protostr, sizeof(protostr), -p 0x%04x ,

for a max of 11 bytes (including trailing NUL) ever printed into it, but
not the end of the world.  And since you didn't check snprintf results
for failure, if we ever change the size of protostr or the length of
what we print into it, it's a slight maintenance risk we are taking on,
compared to dynamic allocation that always gets the right length.  But I
don't know if it is worth replacing this snprintf with
virAsprintf/VIR_FREE overhead, so I can live with it as is.

I converted it now to virAsprintf / VIR_FREE.

@@ -2918,7 +2930,7 @@ ebtablesCreateTmpSubChain(ebiptablesRule
CMD_DEF(%s -t %s -N %s) CMD_SEPARATOR
CMD_EXEC
%s
-  CMD_DEF(%s -t %s -%%c %s %%s -p 0x%x -j %s)
+  CMD_DEF(%s -t %s -%%c %s %%s %s -j %s)

This results in output with a double space, either:

...%s  -j ...

or

...%s -p 0x  -j ...

Also not the end of the world, but you may want to remove the extra
space before the -j.

Fixed. Will repost as v2.

ACK.  There is some conflict resolution needed in nwfilter.rng, but that
should be trivial.



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH V1 4/9] Add a mac chain

2011-11-21 Thread Eric Blake
On 10/26/2011 09:12 AM, Stefan Berger wrote:
 With hunks borrowed from one of David Steven's previous patches, we now
 add the capability of having a 'mac' chain which is useful to filter
 for multiple valid MAC addresses.
 
 Signed-off-by: David L Stevens dlstev...@us.ibm.com
 Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
 

 +char protostr[16] = { 0, };

A bit oversized...

  
  PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
  PRINT_CHAIN(chain, chainPrefix, ifname,
  (filtername) ? filtername : l3_protocols[protoidx].val);
  
 +switch (protoidx) {
 +case L2_PROTO_MAC_IDX:
 +break;
 +default:
 +snprintf(protostr, sizeof(protostr), -p 0x%04x ,

for a max of 11 bytes (including trailing NUL) ever printed into it, but
not the end of the world.  And since you didn't check snprintf results
for failure, if we ever change the size of protostr or the length of
what we print into it, it's a slight maintenance risk we are taking on,
compared to dynamic allocation that always gets the right length.  But I
don't know if it is worth replacing this snprintf with
virAsprintf/VIR_FREE overhead, so I can live with it as is.

 @@ -2918,7 +2930,7 @@ ebtablesCreateTmpSubChain(ebiptablesRule
CMD_DEF(%s -t %s -N %s) CMD_SEPARATOR
CMD_EXEC
%s
 -  CMD_DEF(%s -t %s -%%c %s %%s -p 0x%x -j %s)
 +  CMD_DEF(%s -t %s -%%c %s %%s %s -j %s)

This results in output with a double space, either:

...%s  -j ...

or

...%s -p 0x  -j ...

Also not the end of the world, but you may want to remove the extra
space before the -j.

ACK.  There is some conflict resolution needed in nwfilter.rng, but that
should be trivial.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH V1 4/9] Add a mac chain

2011-10-26 Thread Stefan Berger
With hunks borrowed from one of David Steven's previous patches, we now
add the capability of having a 'mac' chain which is useful to filter
for multiple valid MAC addresses.

Signed-off-by: David L Stevens dlstev...@us.ibm.com
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com

---
 docs/schemas/nwfilter.rng |3 +++
 src/conf/nwfilter_conf.c  |2 ++
 src/conf/nwfilter_conf.h  |2 ++
 src/nwfilter/nwfilter_ebiptables_driver.c |   16 ++--
 4 files changed, 21 insertions(+), 2 deletions(-)

Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -177,6 +177,7 @@ enum l3_proto_idx {
 L3_PROTO_IPV6_IDX,
 L3_PROTO_ARP_IDX,
 L3_PROTO_RARP_IDX,
+L2_PROTO_MAC_IDX,
 L2_PROTO_VLAN_IDX,
 L3_PROTO_LAST_IDX
 };
@@ -189,6 +190,7 @@ static const struct ushort_map l3_protoc
 USHORTMAP_ENTRY_IDX(L3_PROTO_ARP_IDX , ETHERTYPE_ARP   , arp),
 USHORTMAP_ENTRY_IDX(L3_PROTO_RARP_IDX, ETHERTYPE_REVARP, rarp),
 USHORTMAP_ENTRY_IDX(L2_PROTO_VLAN_IDX, ETHERTYPE_VLAN  , vlan),
+USHORTMAP_ENTRY_IDX(L2_PROTO_MAC_IDX,  0   , mac),
 USHORTMAP_ENTRY_IDX(L3_PROTO_LAST_IDX, 0   , NULL),
 };
 
@@ -2905,11 +2907,21 @@ ebtablesCreateTmpSubChain(ebiptablesRule
 char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH];
 char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP
   : CHAINPREFIX_HOST_OUT_TEMP;
+char protostr[16] = { 0, };
 
 PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
 PRINT_CHAIN(chain, chainPrefix, ifname,
 (filtername) ? filtername : l3_protocols[protoidx].val);
 
+switch (protoidx) {
+case L2_PROTO_MAC_IDX:
+break;
+default:
+snprintf(protostr, sizeof(protostr), -p 0x%04x ,
+ l3_protocols[protoidx].attr);
+break;
+}
+
 virBufferAsprintf(buf,
   CMD_DEF(%s -t %s -F %s) CMD_SEPARATOR
   CMD_EXEC
@@ -2918,7 +2930,7 @@ ebtablesCreateTmpSubChain(ebiptablesRule
   CMD_DEF(%s -t %s -N %s) CMD_SEPARATOR
   CMD_EXEC
   %s
-  CMD_DEF(%s -t %s -%%c %s %%s -p 0x%x -j %s)
+  CMD_DEF(%s -t %s -%%c %s %%s %s -j %s)
   CMD_SEPARATOR
   CMD_EXEC
   %s,
@@ -2930,7 +2942,7 @@ ebtablesCreateTmpSubChain(ebiptablesRule
   CMD_STOPONERR(stopOnError),
 
   ebtables_cmd_path, EBTABLES_DEFAULT_TABLE,
-  rootchain, l3_protocols[protoidx].attr, chain,
+  rootchain, protostr, chain,
 
   CMD_STOPONERR(stopOnError));
 
Index: libvirt-acl/src/conf/nwfilter_conf.c
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -82,6 +82,7 @@ VIR_ENUM_IMPL(virNWFilterEbtablesTable, 
 
 VIR_ENUM_IMPL(virNWFilterChainSuffix, VIR_NWFILTER_CHAINSUFFIX_LAST,
   root,
+  mac,
   vlan,
   arp,
   rarp,
@@ -128,6 +129,7 @@ struct int_map {
 
 static const struct int_map chain_priorities[] = {
 INTMAP_ENTRY(NWFILTER_ROOT_FILTER_PRI, root),
+INTMAP_ENTRY(NWFILTER_MAC_FILTER_PRI,  mac),
 INTMAP_ENTRY(NWFILTER_VLAN_FILTER_PRI, vlan),
 INTMAP_ENTRY(NWFILTER_IPV4_FILTER_PRI, ipv4),
 INTMAP_ENTRY(NWFILTER_IPV6_FILTER_PRI, ipv6),
Index: libvirt-acl/src/conf/nwfilter_conf.h
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.h
+++ libvirt-acl/src/conf/nwfilter_conf.h
@@ -378,6 +378,7 @@ enum virNWFilterEbtablesTableType {
 # define NWFILTER_MAX_FILTER_PRIORITY MAX_RULE_PRIORITY
 
 # define NWFILTER_ROOT_FILTER_PRI 0
+# define NWFILTER_MAC_FILTER_PRI  -800
 # define NWFILTER_VLAN_FILTER_PRI -750
 # define NWFILTER_IPV4_FILTER_PRI -700
 # define NWFILTER_IPV6_FILTER_PRI -600
@@ -456,6 +457,7 @@ struct _virNWFilterEntry {
 
 enum virNWFilterChainSuffixType {
 VIR_NWFILTER_CHAINSUFFIX_ROOT = 0,
+VIR_NWFILTER_CHAINSUFFIX_MAC,
 VIR_NWFILTER_CHAINSUFFIX_VLAN,
 VIR_NWFILTER_CHAINSUFFIX_ARP,
 VIR_NWFILTER_CHAINSUFFIX_RARP,
Index: libvirt-acl/docs/schemas/nwfilter.rng
===
--- libvirt-acl.orig/docs/schemas/nwfilter.rng
+++ libvirt-acl/docs/schemas/nwfilter.rng
@@ -297,6 +297,9 @@
 choice
   valueroot/value
   data type=string
+param name=patternmac[a-zA-Z0-9_\.:-]*/param
+  /data
+  data type=string
 param