[libvirt] [PATCH V2 0/5] NWFilter: Filter more protocols and other extensions

This patch series adds: - support for a 'mac' chain - filtering support for STP (spanning tree protocol) - better error reporting if ebtables/ip(6)table commands fail v2: - shortened series since VLAN patches have been pushed - addressed Eric Blake's comments Regards, Stefan

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 <dlstevens@us.ibm.com> Signed-off-by: Stefan Berger <stefanb@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 | 22 ++++++++++++++++++++-- 4 files changed, 27 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 @@ -188,6 +188,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 }; @@ -205,6 +206,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), }; @@ -2806,11 +2808,25 @@ ebtablesCreateTmpSubChain(ebiptablesRule char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH]; char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP : CHAINPREFIX_HOST_OUT_TEMP; + char *protostr = NULL; 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: + virAsprintf(&protostr, "-p 0x%04x", l3_protocols[protoidx].attr); + break; + } + + if (!protostr) { + virReportOOMError(); + return -1; + } + virBufferAsprintf(&buf, CMD_DEF("%s -t %s -F %s") CMD_SEPARATOR CMD_EXEC @@ -2819,7 +2835,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", @@ -2831,10 +2847,12 @@ ebtablesCreateTmpSubChain(ebiptablesRule CMD_STOPONERR(stopOnError), ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, - rootchain, l3_protocols[protoidx].attr, chain, + rootchain, protostr, chain, CMD_STOPONERR(stopOnError)); + VIR_FREE(protostr); + if (virBufferError(&buf) || VIR_EXPAND_N(tmp, count, 1) < 0) { virReportOOMError(); 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> <value>root</value> <data type="string"> + <param name="pattern">mac[a-zA-Z0-9_\.:-]{0,9}</param> + </data> + <data type="string"> <param name="pattern">vlan[a-zA-Z0-9_\.:-]{0,8}</param> </data> <data type="string">

On 11/22/2011 08:51 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 <dlstevens@us.ibm.com> Signed-off-by: Stefan Berger <stefanb@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 | 22 ++++++++++++++++++++-- 4 files changed, 27 insertions(+), 2 deletions(-)
Almost - in addressing my v1 comments, you introduced some other problems.
@@ -2806,11 +2808,25 @@ ebtablesCreateTmpSubChain(ebiptablesRule char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH]; char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP : CHAINPREFIX_HOST_OUT_TEMP; + char *protostr = NULL;
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: + virAsprintf(&protostr, "-p 0x%04x", l3_protocols[protoidx].attr); + break; + } + + if (!protostr) { + virReportOOMError();
Oops. This gives a spurious OOM failure if protoidx is L2_PROTO_MAC_IDX.
@@ -2819,7 +2835,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") ^^
While you fixed the double space problem for a non-empty protostr, you still have the double space for L2_PROTO_MAC_IDX. To completely avoid a double space, as well as the spurious OOM, you'd need: case L2_PROTO_MAC_IDX: protostr = strdup(""); break; default: virAsprintf(&protostr, "-p 0x%04x ", l3_protocols[protoidx].attr); break; ... CMD_DEF("%s -t %s -%%c %s %%s %s-j %s") ACK with those tweaks. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 11/22/2011 01:47 PM, Eric Blake wrote:
On 11/22/2011 08:51 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<dlstevens@us.ibm.com> Signed-off-by: Stefan Berger<stefanb@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 | 22 ++++++++++++++++++++-- 4 files changed, 27 insertions(+), 2 deletions(-) Almost - in addressing my v1 comments, you introduced some other problems.
@@ -2806,11 +2808,25 @@ ebtablesCreateTmpSubChain(ebiptablesRule char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH]; char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP : CHAINPREFIX_HOST_OUT_TEMP; + char *protostr = NULL;
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: + virAsprintf(&protostr, "-p 0x%04x", l3_protocols[protoidx].attr); + break; + } + + if (!protostr) { + virReportOOMError(); Oops. This gives a spurious OOM failure if protoidx is L2_PROTO_MAC_IDX.
@@ -2819,7 +2835,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") ^^
While you fixed the double space problem for a non-empty protostr, you still have the double space for L2_PROTO_MAC_IDX. To completely avoid a double space, as well as the spurious OOM, you'd need:
case L2_PROTO_MAC_IDX: protostr = strdup(""); break; My further testing also pointed that out to me in the meantime... :-/ default: virAsprintf(&protostr, "-p 0x%04x ", l3_protocols[protoidx].attr); I removed the trailing whitespace above... break; ... CMD_DEF("%s -t %s -%%c %s %%s %s-j %s")
ACK with those tweaks.
Stefan

This patch adds support for filtering of STP (spanning tree protocol) traffic to the parser and makes us of the ebtables support for STP filtering. This code now enables the filtering of traffic in chains with prefix 'stp'. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- v2: - addressing Eric Blake's comments - replacing occurrences of number[20] with number[INT_BUFFER_BOUND(uint32)]; this also works for IP masks which are expressed as "%d". --- docs/schemas/nwfilter.rng | 152 +++++++++++++++++++++++++ src/conf/nwfilter_conf.c | 178 ++++++++++++++++++++++++++++++ src/conf/nwfilter_conf.h | 41 ++++++ src/libvirt_private.syms | 1 src/nwfilter/nwfilter_ebiptables_driver.c | 100 ++++++++++++++++ 5 files changed, 465 insertions(+), 7 deletions(-) Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -84,6 +84,7 @@ VIR_ENUM_IMPL(virNWFilterChainSuffix, VI "root", "mac", "vlan", + "stp", "arp", "rarp", "ipv4", @@ -93,6 +94,7 @@ VIR_ENUM_IMPL(virNWFilterRuleProtocol, V "none", "mac", "vlan", + "stp", "arp", "rarp", "ip", @@ -1047,6 +1049,136 @@ static const virXMLAttr2Struct vlanAttri } }; +static const virXMLAttr2Struct stpAttributes[] = { + /* spanning tree uses a special destination MAC address */ + { + .name = SRCMACADDR, + .datatype = DATATYPE_MACADDR, + .dataIdx = offsetof(virNWFilterRuleDef, + p.stpHdrFilter.ethHdr.dataSrcMACAddr), + }, + { + .name = SRCMACMASK, + .datatype = DATATYPE_MACMASK, + .dataIdx = offsetof(virNWFilterRuleDef, + p.stpHdrFilter.ethHdr.dataSrcMACMask), + }, + { + .name = "type", + .datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataType), + }, + { + .name = "flags", + .datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataFlags), + }, + { + .name = "root-priority", + .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataRootPri), + }, + { + .name = "root-priority-hi", + .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataRootPriHi), + }, + { + .name = "root-address", + .datatype = DATATYPE_MACADDR, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataRootAddr), + }, + { + .name = "root-address-mask", + .datatype = DATATYPE_MACMASK, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataRootAddrMask), + }, + { + .name = "root-cost", + .datatype = DATATYPE_UINT32 | DATATYPE_UINT32_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataRootCost), + }, + { + .name = "root-cost-hi", + .datatype = DATATYPE_UINT32 | DATATYPE_UINT32_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataRootCostHi), + }, + { + .name = "sender-priority", + .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataSndrPrio), + }, + { + .name = "sender-priority-hi", + .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataSndrPrioHi), + }, + { + .name = "sender-address", + .datatype = DATATYPE_MACADDR, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataSndrAddr), + }, + { + .name = "sender-address-mask", + .datatype = DATATYPE_MACMASK, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataSndrAddrMask), + }, + { + .name = "port", + .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataPort), + }, + { + .name = "port-hi", + .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataPortHi), + }, + { + .name = "age", + .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataAge), + }, + { + .name = "age-hi", + .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataAgeHi), + }, + { + .name = "max-age", + .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataMaxAge), + }, + { + .name = "max-age-hi", + .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataMaxAgeHi), + }, + { + .name = "hello-time", + .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataHelloTime), + }, + { + .name = "hello-time-hi", + .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataHelloTimeHi), + }, + { + .name = "forward-delay", + .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataFwdDelay), + }, + { + .name = "forward-delay-hi", + .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX, + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataFwdDelayHi), + }, + COMMENT_PROP(stpHdrFilter), + { + .name = NULL, + } +}; + static const virXMLAttr2Struct arpAttributes[] = { COMMON_MAC_PROPS(arpHdrFilter), { @@ -1505,6 +1637,7 @@ static const virAttributes virAttr[] = { PROTOCOL_ENTRY("rarp" , arpAttributes , VIR_NWFILTER_RULE_PROTOCOL_RARP), PROTOCOL_ENTRY("mac" , macAttributes , VIR_NWFILTER_RULE_PROTOCOL_MAC), PROTOCOL_ENTRY("vlan" , vlanAttributes , VIR_NWFILTER_RULE_PROTOCOL_VLAN), + PROTOCOL_ENTRY("stp" , stpAttributes , VIR_NWFILTER_RULE_PROTOCOL_STP), PROTOCOL_ENTRY("ip" , ipAttributes , VIR_NWFILTER_RULE_PROTOCOL_IP), PROTOCOL_ENTRY("ipv6" , ipv6Attributes , VIR_NWFILTER_RULE_PROTOCOL_IPV6), PROTOCOL_ENTRY("tcp" , tcpAttributes , VIR_NWFILTER_RULE_PROTOCOL_TCP), @@ -1628,6 +1761,18 @@ virNWFilterRuleDetailsParse(xmlNodePtr n rc = -1; break; + case DATATYPE_UINT32_HEX: + base = 16; + /* fallthrough */ + case DATATYPE_UINT32: + if (virStrToLong_ui(prop, NULL, base, &uint_val) >= 0) { + item->u.u32 = uint_val; + found = 1; + data.ui = uint_val; + } else + rc = -1; + break; + case DATATYPE_IPADDR: if (virSocketAddrParseIPv4(&item->u.ipaddr, prop) < 0) rc = -1; @@ -1839,6 +1984,31 @@ virNWFilterRuleDefFixup(virNWFilterRuleD rule->p.vlanHdrFilter.ethHdr.dataDstMACAddr); break; + case VIR_NWFILTER_RULE_PROTOCOL_STP: + COPY_NEG_SIGN(rule->p.stpHdrFilter.ethHdr.dataSrcMACMask, + rule->p.stpHdrFilter.ethHdr.dataSrcMACAddr); + COPY_NEG_SIGN(rule->p.stpHdrFilter.dataRootPriHi, + rule->p.stpHdrFilter.dataRootPri); + COPY_NEG_SIGN(rule->p.stpHdrFilter.dataRootAddrMask, + rule->p.stpHdrFilter.dataRootAddr); + COPY_NEG_SIGN(rule->p.stpHdrFilter.dataRootCostHi, + rule->p.stpHdrFilter.dataRootCost); + COPY_NEG_SIGN(rule->p.stpHdrFilter.dataSndrPrioHi, + rule->p.stpHdrFilter.dataSndrPrio); + COPY_NEG_SIGN(rule->p.stpHdrFilter.dataSndrAddrMask, + rule->p.stpHdrFilter.dataSndrAddr); + COPY_NEG_SIGN(rule->p.stpHdrFilter.dataPortHi, + rule->p.stpHdrFilter.dataPort); + COPY_NEG_SIGN(rule->p.stpHdrFilter.dataAgeHi, + rule->p.stpHdrFilter.dataAge); + COPY_NEG_SIGN(rule->p.stpHdrFilter.dataMaxAgeHi, + rule->p.stpHdrFilter.dataMaxAge); + COPY_NEG_SIGN(rule->p.stpHdrFilter.dataHelloTimeHi, + rule->p.stpHdrFilter.dataHelloTime); + COPY_NEG_SIGN(rule->p.stpHdrFilter.dataFwdDelayHi, + rule->p.stpHdrFilter.dataFwdDelay); + break; + case VIR_NWFILTER_RULE_PROTOCOL_IP: COPY_NEG_SIGN(rule->p.ipHdrFilter.ipHdr.dataSrcIPMask, rule->p.ipHdrFilter.ipHdr.dataSrcIPAddr); @@ -2921,6 +3091,14 @@ virNWFilterRuleDefDetailsFormat(virBuffe item->u.u16); break; + case DATATYPE_UINT32_HEX: + asHex = true; + /* fallthrough */ + case DATATYPE_UINT32: + virBufferAsprintf(buf, asHex ? "0x%x" : "%u", + item->u.u32); + break; + case DATATYPE_IPADDR: case DATATYPE_IPV6ADDR: virNWIPAddressFormat(buf, Index: libvirt-acl/src/conf/nwfilter_conf.h =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.h +++ libvirt-acl/src/conf/nwfilter_conf.h @@ -101,10 +101,14 @@ enum attrDatatype { DATATYPE_IPV6MASK = (1 << 10), DATATYPE_STRINGCOPY = (1 << 11), DATATYPE_BOOLEAN = (1 << 12), + DATATYPE_UINT32 = (1 << 13), + DATATYPE_UINT32_HEX = (1 << 14), - DATATYPE_LAST = (1 << 13), + DATATYPE_LAST = (1 << 15), }; +# define NWFILTER_MAC_BGA "01:80:c2:00:00:00" + typedef struct _nwMACAddress nwMACAddress; typedef nwMACAddress *nwMACAddressPtr; @@ -125,6 +129,7 @@ struct _nwItemDesc { bool boolean; uint8_t u8; uint16_t u16; + uint32_t u32; char protocolID[10]; char *string; struct { @@ -164,6 +169,36 @@ struct _vlanHdrFilterDef { }; +typedef struct _stpHdrFilterDef stpHdrFilterDef; +typedef stpHdrFilterDef *stpHdrFilterDefPtr; +struct _stpHdrFilterDef { + ethHdrDataDef ethHdr; + nwItemDesc dataType; + nwItemDesc dataFlags; + nwItemDesc dataRootPri; + nwItemDesc dataRootPriHi; + nwItemDesc dataRootAddr; + nwItemDesc dataRootAddrMask; + nwItemDesc dataRootCost; + nwItemDesc dataRootCostHi; + nwItemDesc dataSndrPrio; + nwItemDesc dataSndrPrioHi; + nwItemDesc dataSndrAddr; + nwItemDesc dataSndrAddrMask; + nwItemDesc dataPort; + nwItemDesc dataPortHi; + nwItemDesc dataAge; + nwItemDesc dataAgeHi; + nwItemDesc dataMaxAge; + nwItemDesc dataMaxAgeHi; + nwItemDesc dataHelloTime; + nwItemDesc dataHelloTimeHi; + nwItemDesc dataFwdDelay; + nwItemDesc dataFwdDelayHi; + nwItemDesc dataComment; +}; + + typedef struct _arpHdrFilterDef arpHdrFilterDef; typedef arpHdrFilterDef *arpHdrFilterDefPtr; struct _arpHdrFilterDef { @@ -337,6 +372,7 @@ enum virNWFilterRuleProtocolType { VIR_NWFILTER_RULE_PROTOCOL_NONE = 0, VIR_NWFILTER_RULE_PROTOCOL_MAC, VIR_NWFILTER_RULE_PROTOCOL_VLAN, + VIR_NWFILTER_RULE_PROTOCOL_STP, VIR_NWFILTER_RULE_PROTOCOL_ARP, VIR_NWFILTER_RULE_PROTOCOL_RARP, VIR_NWFILTER_RULE_PROTOCOL_IP, @@ -378,6 +414,7 @@ enum virNWFilterEbtablesTableType { # define NWFILTER_MAX_FILTER_PRIORITY MAX_RULE_PRIORITY # define NWFILTER_ROOT_FILTER_PRI 0 +# define NWFILTER_STP_FILTER_PRI -810 # define NWFILTER_MAC_FILTER_PRI -800 # define NWFILTER_VLAN_FILTER_PRI -750 # define NWFILTER_IPV4_FILTER_PRI -700 @@ -418,6 +455,7 @@ struct _virNWFilterRuleDef { union { ethHdrFilterDef ethHdrFilter; vlanHdrFilterDef vlanHdrFilter; + stpHdrFilterDef stpHdrFilter; arpHdrFilterDef arpHdrFilter; /* also used for rarp */ ipHdrFilterDef ipHdrFilter; ipv6HdrFilterDef ipv6HdrFilter; @@ -459,6 +497,7 @@ enum virNWFilterChainSuffixType { VIR_NWFILTER_CHAINSUFFIX_ROOT = 0, VIR_NWFILTER_CHAINSUFFIX_MAC, VIR_NWFILTER_CHAINSUFFIX_VLAN, + VIR_NWFILTER_CHAINSUFFIX_STP, VIR_NWFILTER_CHAINSUFFIX_ARP, VIR_NWFILTER_CHAINSUFFIX_RARP, VIR_NWFILTER_CHAINSUFFIX_IPv4, 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 @@ -41,6 +41,7 @@ #include "virfile.h" #include "command.h" #include "configmake.h" +#include "intprops.h" #define VIR_FROM_THIS VIR_FROM_NWFILTER @@ -190,6 +191,7 @@ enum l3_proto_idx { L3_PROTO_RARP_IDX, L2_PROTO_MAC_IDX, L2_PROTO_VLAN_IDX, + L2_PROTO_STP_IDX, L3_PROTO_LAST_IDX }; @@ -206,6 +208,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_STP_IDX, 0 , "stp"), USHORTMAP_ENTRY_IDX(L2_PROTO_MAC_IDX, 0 , "mac"), USHORTMAP_ENTRY_IDX(L3_PROTO_LAST_IDX, 0 , NULL), }; @@ -306,6 +309,16 @@ _printDataType(virNWFilterVarCombIterPtr } break; + case DATATYPE_UINT32: + case DATATYPE_UINT32_HEX: + if (snprintf(buf, bufsize, asHex ? "0x%x" : "%u", + item->u.u32) >= bufsize) { + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Buffer too small for uint32 type")); + return 1; + } + break; + case DATATYPE_UINT16: case DATATYPE_UINT16_HEX: if (snprintf(buf, bufsize, asHex ? "0x%x" : "%d", @@ -937,7 +950,7 @@ iptablesHandleIpHdr(virBufferPtr buf, virBufferPtr prefix) { char ipaddr[INET6_ADDRSTRLEN], - number[20]; + number[INT_BUFSIZE_BOUND(uint32_t)]; const char *src = "--source"; const char *dst = "--destination"; const char *srcrange = "--src-range"; @@ -1218,7 +1231,7 @@ _iptablesCreateRuleInstance(int directio bool maySkipICMP) { char chain[MAX_CHAINNAME_LENGTH]; - char number[20]; + char number[INT_BUFSIZE_BOUND(uint32_t)]; virBuffer prefix = VIR_BUFFER_INITIALIZER; virBuffer buf = VIR_BUFFER_INITIALIZER; virBuffer afterStateMatch = VIR_BUFFER_INITIALIZER; @@ -1953,7 +1966,8 @@ ebtablesCreateRuleInstance(char chainPre char macaddr[VIR_MAC_STRING_BUFLEN], ipaddr[INET_ADDRSTRLEN], ipv6addr[INET6_ADDRSTRLEN], - number[20]; + number[INT_BUFSIZE_BOUND(uint32_t)], + field[MAX(VIR_MAC_STRING_BUFLEN, INET6_ADDRSTRLEN)]; char chain[MAX_CHAINNAME_LENGTH]; virBuffer buf = VIR_BUFFER_INITIALIZER; const char *target; @@ -2019,19 +2033,91 @@ ebtablesCreateRuleInstance(char chainPre #define INST_ITEM(STRUCT, ITEM, CLI) \ if (HAS_ENTRY_ITEM(&rule->p.STRUCT.ITEM)) { \ if (printDataType(vars, \ - number, sizeof(number), \ + field, sizeof(field), \ &rule->p.STRUCT.ITEM)) \ goto err_exit; \ virBufferAsprintf(&buf, \ " " CLI " %s %s", \ ENTRY_GET_NEG_SIGN(&rule->p.STRUCT.ITEM), \ - number); \ + field); \ } +#define INST_ITEM_2PARMS(STRUCT, ITEM, ITEM_HI, CLI, SEP) \ + if (HAS_ENTRY_ITEM(&rule->p.STRUCT.ITEM)) { \ + if (printDataType(vars, \ + field, sizeof(field), \ + &rule->p.STRUCT.ITEM)) \ + goto err_exit; \ + virBufferAsprintf(&buf, \ + " " CLI " %s %s", \ + ENTRY_GET_NEG_SIGN(&rule->p.STRUCT.ITEM), \ + field); \ + if (HAS_ENTRY_ITEM(&rule->p.STRUCT.ITEM_HI)) { \ + if (printDataType(vars, \ + field, sizeof(field), \ + &rule->p.STRUCT.ITEM_HI)) \ + goto err_exit; \ + virBufferAsprintf(&buf, SEP "%s", field); \ + } \ + } +#define INST_ITEM_RANGE(S, I, I_HI, C) \ + INST_ITEM_2PARMS(S, I, I_HI, C, ":") +#define INST_ITEM_MASK(S, I, MASK, C) \ + INST_ITEM_2PARMS(S, I, MASK, C, "/") + INST_ITEM(vlanHdrFilter, dataVlanID, "--vlan-id") INST_ITEM(vlanHdrFilter, dataVlanEncap, "--vlan-encap") break; + case VIR_NWFILTER_RULE_PROTOCOL_STP: + + /* cannot handle inout direction with srcmask set in reverse dir. + since this clashes with -d below... */ + if (reverse && + HAS_ENTRY_ITEM(&rule->p.stpHdrFilter.ethHdr.dataSrcMACAddr)) { + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, + _("STP filtering in %s direction with " + "source MAC address set is not supported"), + virNWFilterRuleDirectionTypeToString( + VIR_NWFILTER_RULE_DIRECTION_INOUT)); + return -1; + } + + virBufferAsprintf(&buf, + CMD_DEF_PRE "%s -t %s -%%c %s %%s", + ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain); + + + if (ebtablesHandleEthHdr(&buf, + vars, + &rule->p.stpHdrFilter.ethHdr, + reverse)) + goto err_exit; + + virBufferAddLit(&buf, " -d " NWFILTER_MAC_BGA); + + INST_ITEM(stpHdrFilter, dataType, "--stp-type") + INST_ITEM(stpHdrFilter, dataFlags, "--stp-flags") + INST_ITEM_RANGE(stpHdrFilter, dataRootPri, dataRootPriHi, + "--stp-root-pri"); + INST_ITEM_MASK( stpHdrFilter, dataRootAddr, dataRootAddrMask, + "--stp-root-addr"); + INST_ITEM_RANGE(stpHdrFilter, dataRootCost, dataRootCostHi, + "--stp-root-cost"); + INST_ITEM_RANGE(stpHdrFilter, dataSndrPrio, dataSndrPrioHi, + "--stp-sender-prio"); + INST_ITEM_MASK( stpHdrFilter, dataSndrAddr, dataSndrAddrMask, + "--stp-sender-addr"); + INST_ITEM_RANGE(stpHdrFilter, dataPort, dataPortHi, "--stp-port"); + INST_ITEM_RANGE(stpHdrFilter, dataAge, dataAgeHi, "--stp-msg-age"); + INST_ITEM_RANGE(stpHdrFilter, dataMaxAge, dataMaxAgeHi, + "--stp-max-age"); + INST_ITEM_RANGE(stpHdrFilter, dataHelloTime, dataHelloTimeHi, + "--stp-hello-time"); + INST_ITEM_RANGE(stpHdrFilter, dataFwdDelay, dataFwdDelayHi, + "--stp-forward-delay"); + break; + case VIR_NWFILTER_RULE_PROTOCOL_ARP: case VIR_NWFILTER_RULE_PROTOCOL_RARP: @@ -2480,6 +2566,7 @@ ebiptablesCreateRuleInstance(virConnectP case VIR_NWFILTER_RULE_PROTOCOL_IP: case VIR_NWFILTER_RULE_PROTOCOL_MAC: case VIR_NWFILTER_RULE_PROTOCOL_VLAN: + case VIR_NWFILTER_RULE_PROTOCOL_STP: case VIR_NWFILTER_RULE_PROTOCOL_ARP: case VIR_NWFILTER_RULE_PROTOCOL_RARP: case VIR_NWFILTER_RULE_PROTOCOL_NONE: @@ -2817,6 +2904,9 @@ ebtablesCreateTmpSubChain(ebiptablesRule switch (protoidx) { case L2_PROTO_MAC_IDX: break; + case L2_PROTO_STP_IDX: + virAsprintf(&protostr, "-d " NWFILTER_MAC_BGA); + break; default: virAsprintf(&protostr, "-p 0x%04x", l3_protocols[protoidx].attr); break; Index: libvirt-acl/docs/schemas/nwfilter.rng =================================================================== --- libvirt-acl.orig/docs/schemas/nwfilter.rng +++ libvirt-acl/docs/schemas/nwfilter.rng @@ -40,6 +40,16 @@ </optional> <optional> <zeroOrMore> + <element name="stp"> + <ref name="match-attribute"/> + <ref name="srcmacandmask-attributes"/> + <ref name="stp-attributes"/> + <ref name="comment-attribute"/> + </element> + </zeroOrMore> + </optional> + <optional> + <zeroOrMore> <element name="arp"> <ref name="match-attribute"/> <ref name="common-l2-attributes"/> @@ -300,6 +310,9 @@ <param name="pattern">mac[a-zA-Z0-9_\.:-]{0,9}</param> </data> <data type="string"> + <param name="pattern">stp[a-zA-Z0-9_\.:-]{0,9}</param> + </data> + <data type="string"> <param name="pattern">vlan[a-zA-Z0-9_\.:-]{0,8}</param> </data> <data type="string"> @@ -382,7 +395,7 @@ </interleave> </define> - <define name="common-l2-attributes"> + <define name="srcmacandmask-attributes"> <interleave> <ref name="srcmac-attribute"/> <optional> @@ -390,6 +403,12 @@ <ref name="addrMAC"/> </attribute> </optional> + </interleave> + </define> + + <define name="common-l2-attributes"> + <interleave> + <ref name="srcmacandmask-attributes"/> <optional> <attribute name="dstmacaddr"> <ref name="addrMAC"/> @@ -588,6 +607,119 @@ </interleave> </define> + <define name="stp-attributes"> + <optional> + <attribute name="type"> + <ref name="uint8range"/> + </attribute> + </optional> + <optional> + <attribute name="flags"> + <ref name="uint8range"/> + </attribute> + </optional> + <optional> + <attribute name="root-priority"> + <ref name="uint16range"/> + </attribute> + </optional> + <optional> + <attribute name="root-priority-hi"> + <ref name="uint16range"/> + </attribute> + </optional> + <optional> + <attribute name="root-address"> + <ref name="addrMAC"/> + </attribute> + </optional> + <optional> + <attribute name="root-address-mask"> + <ref name="addrMAC"/> + </attribute> + </optional> + <optional> + <attribute name="root-cost"> + <ref name="uint32range"/> + </attribute> + </optional> + <optional> + <attribute name="root-cost-hi"> + <ref name="uint32range"/> + </attribute> + </optional> + <optional> + <attribute name="sender-priority"> + <ref name="uint16range"/> + </attribute> + </optional> + <optional> + <attribute name="sender-priority-hi"> + <ref name="uint16range"/> + </attribute> + </optional> + <optional> + <attribute name="sender-address"> + <ref name="addrMAC"/> + </attribute> + </optional> + <optional> + <attribute name="sender-address-mask"> + <ref name="addrMAC"/> + </attribute> + </optional> + <optional> + <attribute name="port"> + <ref name="uint16range"/> + </attribute> + </optional> + <optional> + <attribute name="port-hi"> + <ref name="uint16range"/> + </attribute> + </optional> + <optional> + <attribute name="age"> + <ref name="uint16range"/> + </attribute> + </optional> + <optional> + <attribute name="age-hi"> + <ref name="uint16range"/> + </attribute> + </optional> + <optional> + <attribute name="max-age"> + <ref name="uint16range"/> + </attribute> + </optional> + <optional> + <attribute name="max-age-hi"> + <ref name="uint16range"/> + </attribute> + </optional> + <optional> + <attribute name="hello-time"> + <ref name="uint16range"/> + </attribute> + </optional> + <optional> + <attribute name="hello-time-hi"> + <ref name="uint16range"/> + </attribute> + </optional> + <optional> + <attribute name="forward-delay"> + <ref name="uint16range"/> + </attribute> + </optional> + <optional> + <attribute name="forward-delay-hi"> + <ref name="uint16range"/> + </attribute> + </optional> + </define> + <define name="arp-attributes"> <interleave> <optional> @@ -852,6 +984,24 @@ </choice> </define> + <define name="uint32range"> + <choice> + <!-- variable --> + <data type="string"> + <param name="pattern">$[a-zA-Z0-9_]+</param> + </data> + + <data type="string"> + <param name="pattern">0x[0-9a-fA-F]{1,8}</param> + </data> + + <data type="int"> + <param name="minInclusive">0</param> + <param name="maxInclusive">4294967295</param> + </data> + </choice> + </define> + <define name="boolean"> <choice> <value>yes</value> Index: libvirt-acl/src/libvirt_private.syms =================================================================== --- libvirt-acl.orig/src/libvirt_private.syms +++ libvirt-acl/src/libvirt_private.syms @@ -829,6 +829,7 @@ virNWFilterPrintStateMatchFlags; virNWFilterPrintTCPFlags; virNWFilterRegisterCallbackDriver; virNWFilterRuleActionTypeToString; +virNWFilterRuleDirectionTypeToString; virNWFilterRuleProtocolTypeToString; virNWFilterTestUnassignDef; virNWFilterUnlockFilterUpdates;

On 11/22/2011 08:51 AM, Stefan Berger wrote:
This patch adds support for filtering of STP (spanning tree protocol) traffic to the parser and makes us of the ebtables support for STP filtering. This code now enables the filtering of traffic in chains with prefix 'stp'.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
v2: - addressing Eric Blake's comments - replacing occurrences of number[20] with number[INT_BUFFER_BOUND(uint32)]; this also works for IP masks which are expressed as "%d".
The maximum %d is one byte longer than %u, thanks to a negative sign; so you've got a potential off-by-one issue (although a negative IP mask doesn't make sense, so you may be safe for now).
+static const virXMLAttr2Struct stpAttributes[] = { + /* spanning tree uses a special destination MAC address */ + {
I'd feel a bit better if this comment appears near here: /* STP is documented by IEEE 802.1D; for a synopsis, * see http://www.javvin.com/protocolSTP.html */
@@ -937,7 +950,7 @@ iptablesHandleIpHdr(virBufferPtr buf, virBufferPtr prefix) { char ipaddr[INET6_ADDRSTRLEN], - number[20]; + number[INT_BUFSIZE_BOUND(uint32_t)];
Maybe it's best to rely on util.h giving us MAX, and doing: char number[MAX(INT_BUFSIZE_BOUND(uint32_t), INT_BUFSIZE_BOUND(int))]; so that we can use both %u for uint32_t, and %d for int, without worrying about any other weird type promotions going on. ACK with those nits addressed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

This patch adds a few test cases for the XML parsing of STP filtering nodes. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- tests/nwfilterxml2xmlin/stp-test.xml | 26 ++++++++++++++++++++++++++ tests/nwfilterxml2xmlout/stp-test.xml | 12 ++++++++++++ tests/nwfilterxml2xmltest.c | 1 + 3 files changed, 39 insertions(+) Index: libvirt-acl/tests/nwfilterxml2xmlin/stp-test.xml =================================================================== --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlin/stp-test.xml @@ -0,0 +1,26 @@ +<filter name='testcase' chain='stp-xyz'> + <uuid>5c6d49af-b071-6127-b4ec-6f8ed4b55335</uuid> + <rule action='continue' direction='in'> + <stp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff' + type='0x12' flags='0x44'/> + </rule> + + <rule action='return' direction='out'> + <stp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff' + root-priority='0x1234' root-priority-hi='0x2345' + root-address="6:5:4:3:2:1" root-address-mask='ff:ff:ff:ff:ff:ff' + root-cost='0x11223344' root-cost-hi='0x22334455' /> + </rule> + + <rule action='reject' direction='in'> + <stp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff' + sender-priority='0x1234' + sender-address="6:5:4:3:2:1" + port='123' port-hi='234' + age='5544' age-hi='5555' + max-age='7777' max-age-hi='8888' + hello-time='12345' hello-time-hi='12346' + forward-delay='54321' forward-delay-hi='65432'/> + </rule> + +</filter> Index: libvirt-acl/tests/nwfilterxml2xmltest.c =================================================================== --- libvirt-acl.orig/tests/nwfilterxml2xmltest.c +++ libvirt-acl/tests/nwfilterxml2xmltest.c @@ -109,6 +109,7 @@ mymain(void) DO_TEST("mac-test", true); DO_TEST("vlan-test", true); + DO_TEST("stp-test", false); DO_TEST("arp-test", true); DO_TEST("rarp-test", true); DO_TEST("ip-test", true); Index: libvirt-acl/tests/nwfilterxml2xmlout/stp-test.xml =================================================================== --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlout/stp-test.xml @@ -0,0 +1,12 @@ +<filter name='testcase' chain='stp-xyz'> + <uuid>5c6d49af-b071-6127-b4ec-6f8ed4b55335</uuid> + <rule action='continue' direction='in' priority='500'> + <stp srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff' type='0x12' flags='0x44'/> + </rule> + <rule action='return' direction='out' priority='500'> + <stp srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff' root-priority='0x1234' root-priority-hi='0x2345' root-address='06:05:04:03:02:01' root-address-mask='ff:ff:ff:ff:ff:ff' root-cost='0x11223344' root-cost-hi='0x22334455'/> + </rule> + <rule action='reject' direction='in' priority='500'> + <stp srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff' sender-priority='0x1234' sender-address='06:05:04:03:02:01' port='123' port-hi='234' age='5544' age-hi='5555' max-age='7777' max-age-hi='8888' hello-time='12345' hello-time-hi='12346' forward-delay='54321' forward-delay-hi='65432'/> + </rule> +</filter>

On 11/22/2011 08:51 AM, Stefan Berger wrote:
This patch adds a few test cases for the XML parsing of STP filtering nodes.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
--- tests/nwfilterxml2xmlin/stp-test.xml | 26 ++++++++++++++++++++++++++ tests/nwfilterxml2xmlout/stp-test.xml | 12 ++++++++++++ tests/nwfilterxml2xmltest.c | 1 + 3 files changed, 39 insertions(+)
My ACK from v1 still applies. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Add documentation for the STP filtering support. Describe the XML attributes that are supported. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- docs/formatnwfilter.html.in | 142 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 142 insertions(+) Index: libvirt-acl/docs/formatnwfilter.html.in =================================================================== --- libvirt-acl.orig/docs/formatnwfilter.html.in +++ libvirt-acl/docs/formatnwfilter.html.in @@ -603,6 +603,148 @@ Valid Strings for <code>encap-protocol</code> are: arp, ipv4, ipv6 </p> + <h5><a name="nwfelemsRulesProtoSTP">STP (Spanning Tree Protocol)</a> + <span class="since">(Since 0.9.8)</span> + </h5> + <p> + Protocol ID: <code>stp</code> + <br/> + Note: Rules of this type should go either into the <code>root</code> or + <code>stp</code> chain. + </p> + <table class="top_table"> + <tr> + <th> Attribute </th> + <th> Datatype </th> + <th> Semantics </th> + </tr> + <tr> + <td>srcmacaddr</td> + <td>MAC_ADDR</td> + <td>MAC address of sender</td> + </tr> + <tr> + <td>srcmacmask</td> + <td>MAC_MASK</td> + <td>Mask applied to MAC address of sender</td> + </tr> + <tr> + <td>type</td> + <td>UINT8</td> + <td>Bridge Protcol Data Unit (BPDU) type</td> + </tr> + <tr> + <td>flags</td> + <td>UINT8</td> + <td>BPDU flag</td> + </tr> + <tr> + <td>root-priority</td> + <td>UINT16</td> + <td>Root priority (range start)</td> + </tr> + <tr> + <td>root-priority-hi</td> + <td>UINT16</td> + <td>Root priority range end</td> + </tr> + <tr> + <td>root-address</td> + <td>MAC_ADDRESS</td> + <td>Root MAC address</td> + </tr> + <tr> + <td>root-address-mask</td> + <td>MAC_MASK</td> + <td>Root MAC address mask</td> + </tr> + <tr> + <td>root-cost</td> + <td>UINT32</td> + <td>Root path cost (range start)</td> + </tr> + <tr> + <td>root-cost-hi</td> + <td>UINT32</td> + <td>Root path cost range end</td> + </tr> + <tr> + <td>sender-priority</td> + <td>UINT16</td> + <td>Sender priority (range start)</td> + </tr> + <tr> + <td>sender-priority-hi</td> + <td>UINT16</td> + <td>Sender priority range end</td> + </tr> + <tr> + <td>sender-address</td> + <td>MAC_ADDRESS</td> + <td>BPDU sender MAC address</td> + </tr> + <tr> + <td>sender-address-mask</td> + <td>MAC_MASK</td> + <td>BPDU sender MAC address mask</td> + </tr> + <tr> + <td>port</td> + <td>UINT16</td> + <td>Port identifier (range start)</td> + </tr> + <tr> + <td>port_hi</td> + <td>UINT16</td> + <td>Port identifier range end</td> + </tr> + <tr> + <td>msg-age</td> + <td>UINT16</td> + <td>Message age timer (range start)</td> + </tr> + <tr> + <td>msg-age-hi</td> + <td>UINT16</td> + <td>Message age timer range end</td> + </tr> + <tr> + <td>max-age</td> + <td>UINT16</td> + <td>Maximum age timer (range start)</td> + </tr> + <tr> + <td>max-age-hi</td> + <td>UINT16</td> + <td>Maximum age timer range end</td> + </tr> + <tr> + <td>hello-time</td> + <td>UINT16</td> + <td>Hello time timer (range start)</td> + </tr> + <tr> + <td>hello-time-hi</td> + <td>UINT16</td> + <td>Hello time timer range end</td> + </tr> + <tr> + <td>forward-delay</td> + <td>UINT16</td> + <td>Forward delay (range start)</td> + </tr> + <tr> + <td>forward-delay-hi</td> + <td>UINT16</td> + <td>Forward delay range end</td> + </tr> + <tr> + <td>comment</td> + <td>STRING</td> + <td>text with max. 256 characters</td> + </tr> + </table> + <h5><a name="nwfelemsRulesProtoARP">ARP/RARP</a></h5> <p> Protocol ID: <code>arp</code> or <code>rarp</code>

On 11/22/2011 08:51 AM, Stefan Berger wrote:
Add documentation for the STP filtering support. Describe the XML attributes that are supported.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
--- docs/formatnwfilter.html.in | 142 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 142 insertions(+)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Display the executed command and failure message if a command failed to execute. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- v2: - addressing Eric Blake's comments --- src/nwfilter/nwfilter_ebiptables_driver.c | 86 +++++++++++++++++------------- 1 file changed, 50 insertions(+), 36 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 @@ -65,10 +65,10 @@ #define CMD_DEF_PRE "cmd='" #define CMD_DEF_POST "'" #define CMD_DEF(X) CMD_DEF_PRE X CMD_DEF_POST -#define CMD_EXEC "eval res=\\$\\(\"${cmd}\"\\)" CMD_SEPARATOR +#define CMD_EXEC "eval res=\\$\\(\"${cmd} 2>&1\"\\)" CMD_SEPARATOR #define CMD_STOPONERR(X) \ X ? "if [ $? -ne 0 ]; then" \ - " echo \"Failure to execute command '${cmd}'.\";" \ + " echo \"Failure to execute command '${cmd}' : '${res}'.\";" \ " exit 1;" \ "fi" CMD_SEPARATOR \ : "" @@ -2707,6 +2707,10 @@ ebiptablesDisplayRuleInstance(virConnect * execute. * @status: Pointer to an integer for returning the WEXITSTATUS of the * commands executed via the script the was run. + * @outbuf: Optional pointer to a string that will hold the buffer with + * output of the executed command. The actual buffer holding + * the message will be newly allocated by this function and + * any passed in buffer freed first. * * Returns 0 in case of success, < 0 in case of an error. The returned * value is NOT the result of running the commands inside the shell @@ -2718,17 +2722,24 @@ ebiptablesDisplayRuleInstance(virConnect */ static int ebiptablesExecCLI(virBufferPtr buf, - int *status) + int *status, char **outbuf) { int rc = -1; virCommandPtr cmd; - *status = 0; + if (status) + *status = 0; + if (!virBufferError(buf) && !virBufferUse(buf)) return 0; + if (outbuf) + VIR_FREE(*outbuf); + cmd = virCommandNewArgList("/bin/sh", "-c", NULL); virCommandAddArgBuffer(cmd, buf); + if (outbuf) + virCommandSetOutputBuffer(cmd, outbuf); virMutexLock(&execCLIMutex); @@ -3138,7 +3149,6 @@ ebtablesApplyBasicRules(const char *ifna const unsigned char *macaddr) { virBuffer buf = VIR_BUFFER_INITIALIZER; - int cli_status; char chain[MAX_CHAINNAME_LENGTH]; char chainPrefix = CHAINPREFIX_HOST_IN_TEMP; char macaddr_str[VIR_MAC_STRING_BUFLEN]; @@ -3193,7 +3203,7 @@ ebtablesApplyBasicRules(const char *ifna ebtablesLinkTmpRootChain(&buf, 1, ifname, 1); ebtablesRenameTmpRootChain(&buf, 1, ifname); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, NULL, NULL) < 0) goto tear_down_tmpebchains; return 0; @@ -3229,7 +3239,6 @@ ebtablesApplyDHCPOnlyRules(const char *i const char *dhcpserver) { virBuffer buf = VIR_BUFFER_INITIALIZER; - int cli_status; char chain_in [MAX_CHAINNAME_LENGTH], chain_out[MAX_CHAINNAME_LENGTH]; char macaddr_str[VIR_MAC_STRING_BUFLEN]; @@ -3309,7 +3318,7 @@ ebtablesApplyDHCPOnlyRules(const char *i ebtablesRenameTmpRootChain(&buf, 1, ifname); ebtablesRenameTmpRootChain(&buf, 0, ifname); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, NULL, NULL) < 0) goto tear_down_tmpebchains; VIR_FREE(srcIPParam); @@ -3342,7 +3351,6 @@ static int ebtablesApplyDropAllRules(const char *ifname) { virBuffer buf = VIR_BUFFER_INITIALIZER; - int cli_status; char chain_in [MAX_CHAINNAME_LENGTH], chain_out[MAX_CHAINNAME_LENGTH]; @@ -3382,7 +3390,7 @@ ebtablesApplyDropAllRules(const char *if ebtablesRenameTmpRootChain(&buf, 1, ifname); ebtablesRenameTmpRootChain(&buf, 0, ifname); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, NULL, NULL) < 0) goto tear_down_tmpebchains; return 0; @@ -3425,7 +3433,7 @@ static int ebtablesCleanAll(const char * ebtablesRemoveTmpRootChain(&buf, 1, ifname); ebtablesRemoveTmpRootChain(&buf, 0, ifname); - ebiptablesExecCLI(&buf, &cli_status); + ebiptablesExecCLI(&buf, &cli_status, NULL); return 0; } @@ -3582,6 +3590,7 @@ ebiptablesApplyNewRules(virConnectPtr co bool haveIp6tables = false; ebiptablesRuleInstPtr ebtChains = NULL; int nEbtChains = 0; + char *errmsg = NULL; if (!chains_in_set || !chains_out_set) { virReportOOMError(); @@ -3620,7 +3629,7 @@ ebiptablesApplyNewRules(virConnectPtr co ebtablesRemoveTmpSubChains(&buf, ifname); ebtablesRemoveTmpRootChain(&buf, 1, ifname); ebtablesRemoveTmpRootChain(&buf, 0, ifname); - ebiptablesExecCLI(&buf, &cli_status); + ebiptablesExecCLI(&buf, &cli_status, NULL); } /* create needed chains */ @@ -3635,7 +3644,7 @@ ebiptablesApplyNewRules(virConnectPtr co qsort(&ebtChains[0], nEbtChains, sizeof(ebtChains[0]), ebiptablesRuleOrderSort); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0) goto tear_down_tmpebchains; /* process ebtables commands; interleave commands from filters with @@ -3669,7 +3678,7 @@ ebiptablesApplyNewRules(virConnectPtr co ebtChains[j++].commandTemplate, 'A', -1, 1); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0) goto tear_down_tmpebchains; if (haveIptables) { @@ -3678,17 +3687,17 @@ ebiptablesApplyNewRules(virConnectPtr co iptablesCreateBaseChains(iptables_cmd_path, &buf); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0) goto tear_down_tmpebchains; iptablesCreateTmpRootChains(iptables_cmd_path, &buf, ifname); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0) goto tear_down_tmpiptchains; iptablesLinkTmpRootChains(iptables_cmd_path, &buf, ifname); iptablesSetupVirtInPost(iptables_cmd_path, &buf, ifname); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0) goto tear_down_tmpiptchains; for (i = 0; i < nruleInstances; i++) { @@ -3699,7 +3708,7 @@ ebiptablesApplyNewRules(virConnectPtr co 'A', -1, 1); } - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0) goto tear_down_tmpiptchains; iptablesCheckBridgeNFCallEnabled(false); @@ -3711,17 +3720,17 @@ ebiptablesApplyNewRules(virConnectPtr co iptablesCreateBaseChains(ip6tables_cmd_path, &buf); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0) goto tear_down_tmpiptchains; iptablesCreateTmpRootChains(ip6tables_cmd_path, &buf, ifname); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0) goto tear_down_tmpip6tchains; iptablesLinkTmpRootChains(ip6tables_cmd_path, &buf, ifname); iptablesSetupVirtInPost(ip6tables_cmd_path, &buf, ifname); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0) goto tear_down_tmpip6tchains; for (i = 0; i < nruleInstances; i++) { @@ -3731,7 +3740,7 @@ ebiptablesApplyNewRules(virConnectPtr co 'A', -1, 1); } - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0) goto tear_down_tmpip6tchains; iptablesCheckBridgeNFCallEnabled(true); @@ -3742,7 +3751,7 @@ ebiptablesApplyNewRules(virConnectPtr co if (virHashSize(chains_out_set) != 0) ebtablesLinkTmpRootChain(&buf, 0, ifname, 1); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0) goto tear_down_ebsubchains_and_unlink; virHashFree(chains_in_set); @@ -3752,6 +3761,8 @@ ebiptablesApplyNewRules(virConnectPtr co VIR_FREE(ebtChains[i].commandTemplate); VIR_FREE(ebtChains); + VIR_FREE(errmsg); + return 0; tear_down_ebsubchains_and_unlink: @@ -3779,12 +3790,14 @@ tear_down_tmpebchains: ebtablesRemoveTmpRootChain(&buf, 0, ifname); } - ebiptablesExecCLI(&buf, &cli_status); + ebiptablesExecCLI(&buf, &cli_status, NULL); virNWFilterReportError(VIR_ERR_BUILD_FIREWALL, _("Some rules could not be created for " - "interface %s."), - ifname); + "interface %s%s%s"), + ifname, + errmsg ? ": " : "", + errmsg ? errmsg : ""); exit_free_sets: virHashFree(chains_in_set); @@ -3794,6 +3807,8 @@ exit_free_sets: VIR_FREE(ebtChains[i].commandTemplate); VIR_FREE(ebtChains); + VIR_FREE(errmsg); + return 1; } @@ -3824,7 +3839,7 @@ ebiptablesTearNewRules(virConnectPtr con ebtablesRemoveTmpRootChain(&buf, 0, ifname); } - ebiptablesExecCLI(&buf, &cli_status); + ebiptablesExecCLI(&buf, &cli_status, NULL); return 0; } @@ -3843,7 +3858,7 @@ ebiptablesTearOldRules(virConnectPtr con iptablesRemoveRootChains(iptables_cmd_path, &buf, ifname); iptablesRenameTmpRootChains(iptables_cmd_path, &buf, ifname); - ebiptablesExecCLI(&buf, &cli_status); + ebiptablesExecCLI(&buf, &cli_status, NULL); } if (ip6tables_cmd_path) { @@ -3851,7 +3866,7 @@ ebiptablesTearOldRules(virConnectPtr con iptablesRemoveRootChains(ip6tables_cmd_path, &buf, ifname); iptablesRenameTmpRootChains(ip6tables_cmd_path, &buf, ifname); - ebiptablesExecCLI(&buf, &cli_status); + ebiptablesExecCLI(&buf, &cli_status, NULL); } if (ebtables_cmd_path) { @@ -3865,7 +3880,7 @@ ebiptablesTearOldRules(virConnectPtr con ebtablesRenameTmpSubAndRootChains(&buf, ifname); - ebiptablesExecCLI(&buf, &cli_status); + ebiptablesExecCLI(&buf, &cli_status, NULL); } return 0; @@ -3902,7 +3917,7 @@ ebiptablesRemoveRules(virConnectPtr conn 'D', -1, 0); - if (ebiptablesExecCLI(&buf, &cli_status)) + if (ebiptablesExecCLI(&buf, &cli_status, NULL)) goto err_exit; if (cli_status) { @@ -3953,7 +3968,7 @@ ebiptablesAllTeardown(const char *ifname ebtablesRemoveRootChain(&buf, 1, ifname); ebtablesRemoveRootChain(&buf, 0, ifname); } - ebiptablesExecCLI(&buf, &cli_status); + ebiptablesExecCLI(&buf, &cli_status, NULL); return 0; } @@ -3987,7 +4002,6 @@ static int ebiptablesDriverInit(bool privileged) { virBuffer buf = VIR_BUFFER_INITIALIZER; - int cli_status; if (!privileged) return 0; @@ -4008,7 +4022,7 @@ ebiptablesDriverInit(bool privileged) ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, CMD_STOPONERR(1)); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status) + if (ebiptablesExecCLI(&buf, NULL, NULL) < 0) VIR_FREE(ebtables_cmd_path); } @@ -4021,7 +4035,7 @@ ebiptablesDriverInit(bool privileged) iptables_cmd_path, CMD_STOPONERR(1)); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status) + if (ebiptablesExecCLI(&buf, NULL, NULL) < 0) VIR_FREE(iptables_cmd_path); } @@ -4034,7 +4048,7 @@ ebiptablesDriverInit(bool privileged) ip6tables_cmd_path, CMD_STOPONERR(1)); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status) + if (ebiptablesExecCLI(&buf, NULL, NULL) < 0) VIR_FREE(ip6tables_cmd_path); }

On 11/22/2011 08:51 AM, Stefan Berger wrote:
Display the executed command and failure message if a command failed to execute.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
v2: - addressing Eric Blake's comments
Regarding my v1 complaint about i18n issues, poorly translated error messages are superior to no messages at all, so I'm okay giving this one ACK, and leaving i18n improvements for another day. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 11/22/2011 02:58 PM, Eric Blake wrote:
On 11/22/2011 08:51 AM, Stefan Berger wrote:
Display the executed command and failure message if a command failed to execute.
Signed-off-by: Stefan Berger<stefanb@linux.vnet.ibm.com>
---
v2: - addressing Eric Blake's comments Regarding my v1 complaint about i18n issues, poorly translated error messages are superior to no messages at all, so I'm okay giving this one ACK, and leaving i18n improvements for another day.
I pushed them with the white spaces in (1/5) addressed as you suggested. Thanks Stefan
participants (2)
-
Eric Blake
-
Stefan Berger