10:11 a.m.
This patch series adds:
- filtering support for VLAN traffic
- support for a 'mac' chain
- filtering support for STP (spanning tree protocol)
- new filters that enable filtering of multiple IP addresses per interface
- better error reporting if ebtables/ip(6)table commands fail
Regards,
Stefan
10:11 a.m.
New subject: [libvirt] [PATCH V1 1/9] Add support for VLAN filtering
This patch adds support for filtering of VLAN (802.1Q) traffic to the
parser and makes us of the ebtables support for VLAN filtering. This code
now enables the filtering of traffic in chains with prefix 'vlan'.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
docs/schemas/nwfilter.rng | 47 +++++++++++++
src/conf/nwfilter_conf.c | 102 ++++++++++++++++++++++++++++++
src/conf/nwfilter_conf.h | 17 +++++
src/nwfilter/nwfilter_ebiptables_driver.c | 35 ++++++++++
4 files changed, 201 insertions(+)
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",
+ "vlan",
"arp",
"rarp",
"ipv4",
@@ -90,6 +91,7 @@ VIR_ENUM_IMPL(virNWFilterChainSuffix, VI
VIR_ENUM_IMPL(virNWFilterRuleProtocol, VIR_NWFILTER_RULE_PROTOCOL_LAST,
"none",
"mac",
+ "vlan",
"arp",
"rarp",
"ip",
@@ -126,6 +128,7 @@ struct int_map {
static const struct int_map chain_priorities[] = {
INTMAP_ENTRY(NWFILTER_ROOT_FILTER_PRI, "root"),
+ INTMAP_ENTRY(NWFILTER_VLAN_FILTER_PRI, "vlan"),
INTMAP_ENTRY(NWFILTER_IPV4_FILTER_PRI, "ipv4"),
INTMAP_ENTRY(NWFILTER_IPV6_FILTER_PRI, "ipv6"),
INTMAP_ENTRY(NWFILTER_ARP_FILTER_PRI , "arp" ),
@@ -459,6 +462,7 @@ static const struct int_map macProtoMap[
INTMAP_ENTRY(ETHERTYPE_REVARP, "rarp"),
INTMAP_ENTRY(ETHERTYPE_IP , "ipv4"),
INTMAP_ENTRY(ETHERTYPE_IPV6 , "ipv6"),
+ INTMAP_ENTRY(ETHERTYPE_VLAN , "vlan"),
INTMAP_ENTRY_LAST
};
@@ -513,6 +517,75 @@ macProtocolIDFormatter(virBufferPtr buf,
}
+static bool
+checkVlanVlanID(enum attrDatatype datatype, union data *value,
+ virNWFilterRuleDefPtr nwf ATTRIBUTE_UNUSED,
+ nwItemDesc *item ATTRIBUTE_UNUSED)
+{
+ int32_t res;
+
+ res = value->ui;
+ if (res < 0 || res > 4095) {
+ res = -1;
+ }
+
+ if (res != -1) {
+ nwf->p.vlanHdrFilter.dataVlanID.u.u16 = res;
+ nwf->p.vlanHdrFilter.dataVlanID.datatype = datatype;
+ return true;
+ }
+
+ return false;
+}
+
+static bool
+checkVlanProtocolID(enum attrDatatype datatype, union data *value,
+ virNWFilterRuleDefPtr nwf ATTRIBUTE_UNUSED,
+ nwItemDesc *item ATTRIBUTE_UNUSED)
+{
+ int32_t res = -1;
+
+ if (datatype == DATATYPE_STRING) {
+ if (intMapGetByString(macProtoMap, value->c, 1, &res) == 0)
+ res = -1;
+ datatype = DATATYPE_UINT16;
+ } else if (datatype == DATATYPE_UINT16 ||
+ datatype == DATATYPE_UINT16_HEX) {
+ res = value->ui;
+ if (res < 0x3c)
+ res = -1;
+ }
+
+ if (res != -1) {
+ nwf->p.vlanHdrFilter.dataVlanEncap.u.u16 = res;
+ nwf->p.vlanHdrFilter.dataVlanEncap.datatype = datatype;
+ return true;
+ }
+
+ return false;
+}
+
+static bool
+vlanProtocolIDFormatter(virBufferPtr buf,
+ virNWFilterRuleDefPtr nwf,
+ nwItemDesc *item ATTRIBUTE_UNUSED)
+{
+ const char *str = NULL;
+ bool asHex = true;
+
+ if (intMapGetByInt(macProtoMap,
+ nwf->p.vlanHdrFilter.dataVlanEncap.u.u16,
+ &str)) {
+ virBufferAdd(buf, str, -1);
+ } else {
+ if (nwf->p.vlanHdrFilter.dataVlanEncap.datatype == DATATYPE_UINT16)
+ asHex = false;
+ virBufferAsprintf(buf, asHex ? "0x%x" : "%d",
+ nwf->p.vlanHdrFilter.dataVlanEncap.u.u16);
+ }
+ return true;
+}
+
/* generic function to check for a valid (ipv4,ipv6, mac) mask
* A mask is valid of there is a sequence of 1's followed by a sequence
* of 0s or only 1s or only 0s
@@ -951,6 +1024,27 @@ static const virXMLAttr2Struct macAttrib
}
};
+static const virXMLAttr2Struct vlanAttributes[] = {
+ COMMON_MAC_PROPS(ethHdrFilter),
+ {
+ .name = "vlanid",
+ .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX,
+ .dataIdx = offsetof(virNWFilterRuleDef, p.vlanHdrFilter.dataVlanID),
+ .validator = checkVlanVlanID,
+ },
+ {
+ .name = "encap-protocol",
+ .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX | DATATYPE_STRING,
+ .dataIdx = offsetof(virNWFilterRuleDef, p.vlanHdrFilter.dataVlanEncap),
+ .validator = checkVlanProtocolID,
+ .formatter = vlanProtocolIDFormatter,
+ },
+ COMMENT_PROP(vlanHdrFilter),
+ {
+ .name = NULL,
+ }
+};
+
static const virXMLAttr2Struct arpAttributes[] = {
COMMON_MAC_PROPS(arpHdrFilter),
{
@@ -1408,6 +1502,7 @@ static const virAttributes virAttr[] = {
PROTOCOL_ENTRY("arp" , arpAttributes ,
VIR_NWFILTER_RULE_PROTOCOL_ARP),
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("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),
@@ -1737,6 +1832,13 @@ virNWFilterRuleDefFixup(virNWFilterRuleD
rule->p.ethHdrFilter.ethHdr.dataDstMACAddr);
break;
+ case VIR_NWFILTER_RULE_PROTOCOL_VLAN:
+ COPY_NEG_SIGN(rule->p.vlanHdrFilter.ethHdr.dataSrcMACMask,
+ rule->p.vlanHdrFilter.ethHdr.dataSrcMACAddr);
+ COPY_NEG_SIGN(rule->p.vlanHdrFilter.ethHdr.dataDstMACMask,
+ rule->p.vlanHdrFilter.ethHdr.dataDstMACAddr);
+ break;
+
case VIR_NWFILTER_RULE_PROTOCOL_IP:
COPY_NEG_SIGN(rule->p.ipHdrFilter.ipHdr.dataSrcIPMask,
rule->p.ipHdrFilter.ipHdr.dataSrcIPAddr);
Index: libvirt-acl/src/conf/nwfilter_conf.h
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.h
+++ libvirt-acl/src/conf/nwfilter_conf.h
@@ -55,6 +55,9 @@
# ifndef ETHERTYPE_IPV6
# define ETHERTYPE_IPV6 0x86dd
# endif
+# ifndef ETHERTYPE_VLAN
+# define ETHERTYPE_VLAN 0x8100
+# endif
/**
* Chain suffix size is:
@@ -151,6 +154,16 @@ struct _ethHdrFilterDef {
};
+typedef struct _vlanHdrFilterDef vlanHdrFilterDef;
+typedef vlanHdrFilterDef *vlanHdrFilterDefPtr;
+struct _vlanHdrFilterDef {
+ ethHdrDataDef ethHdr;
+ nwItemDesc dataVlanID;
+ nwItemDesc dataVlanEncap;
+ nwItemDesc dataComment;
+};
+
+
typedef struct _arpHdrFilterDef arpHdrFilterDef;
typedef arpHdrFilterDef *arpHdrFilterDefPtr;
struct _arpHdrFilterDef {
@@ -323,6 +336,7 @@ enum virNWFilterChainPolicyType {
enum virNWFilterRuleProtocolType {
VIR_NWFILTER_RULE_PROTOCOL_NONE = 0,
VIR_NWFILTER_RULE_PROTOCOL_MAC,
+ VIR_NWFILTER_RULE_PROTOCOL_VLAN,
VIR_NWFILTER_RULE_PROTOCOL_ARP,
VIR_NWFILTER_RULE_PROTOCOL_RARP,
VIR_NWFILTER_RULE_PROTOCOL_IP,
@@ -364,6 +378,7 @@ enum virNWFilterEbtablesTableType {
# define NWFILTER_MAX_FILTER_PRIORITY MAX_RULE_PRIORITY
# define NWFILTER_ROOT_FILTER_PRI 0
+# define NWFILTER_VLAN_FILTER_PRI -750
# define NWFILTER_IPV4_FILTER_PRI -700
# define NWFILTER_IPV6_FILTER_PRI -600
# define NWFILTER_ARP_FILTER_PRI -500
@@ -401,6 +416,7 @@ struct _virNWFilterRuleDef {
enum virNWFilterRuleProtocolType prtclType;
union {
ethHdrFilterDef ethHdrFilter;
+ vlanHdrFilterDef vlanHdrFilter;
arpHdrFilterDef arpHdrFilter; /* also used for rarp */
ipHdrFilterDef ipHdrFilter;
ipv6HdrFilterDef ipv6HdrFilter;
@@ -440,6 +456,7 @@ struct _virNWFilterEntry {
enum virNWFilterChainSuffixType {
VIR_NWFILTER_CHAINSUFFIX_ROOT = 0,
+ VIR_NWFILTER_CHAINSUFFIX_VLAN,
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
@@ -177,6 +177,7 @@ enum l3_proto_idx {
L3_PROTO_IPV6_IDX,
L3_PROTO_ARP_IDX,
L3_PROTO_RARP_IDX,
+ L2_PROTO_VLAN_IDX,
L3_PROTO_LAST_IDX
};
@@ -187,6 +188,7 @@ static const struct ushort_map l3_protoc
USHORTMAP_ENTRY_IDX(L3_PROTO_IPV6_IDX, ETHERTYPE_IPV6 , "ipv6"),
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(L3_PROTO_LAST_IDX, 0 , NULL),
};
@@ -1980,6 +1982,38 @@ ebtablesCreateRuleInstance(char chainPre
}
break;
+ case VIR_NWFILTER_RULE_PROTOCOL_VLAN:
+
+ virBufferAsprintf(&buf,
+ CMD_DEF_PRE "%s -t %s -%%c %s %%s",
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain);
+
+
+ if (ebtablesHandleEthHdr(&buf,
+ vars,
+ &rule->p.vlanHdrFilter.ethHdr,
+ reverse))
+ goto err_exit;
+
+ virBufferAddLit(&buf,
+ " -p 0x8100");
+
+#define INST_ITEM(STRUCT, ITEM, CLI) \
+ if (HAS_ENTRY_ITEM(&rule->p.STRUCT.ITEM)) { \
+ if (printDataType(vars, \
+ number, sizeof(number), \
+ &rule->p.STRUCT.ITEM)) \
+ goto err_exit; \
+ virBufferAsprintf(&buf, \
+ " " CLI " %s %s", \
+ ENTRY_GET_NEG_SIGN(&rule->p.STRUCT.ITEM), \
+ number); \
+ }
+
+ INST_ITEM(vlanHdrFilter, dataVlanID, "--vlan-id")
+ INST_ITEM(vlanHdrFilter, dataVlanEncap, "--vlan-encap")
+ break;
+
case VIR_NWFILTER_RULE_PROTOCOL_ARP:
case VIR_NWFILTER_RULE_PROTOCOL_RARP:
@@ -2427,6 +2461,7 @@ ebiptablesCreateRuleInstance(virConnectP
switch (rule->prtclType) {
case VIR_NWFILTER_RULE_PROTOCOL_IP:
case VIR_NWFILTER_RULE_PROTOCOL_MAC:
+ case VIR_NWFILTER_RULE_PROTOCOL_VLAN:
case VIR_NWFILTER_RULE_PROTOCOL_ARP:
case VIR_NWFILTER_RULE_PROTOCOL_RARP:
case VIR_NWFILTER_RULE_PROTOCOL_NONE:
Index: libvirt-acl/docs/schemas/nwfilter.rng
===================================================================
--- libvirt-acl.orig/docs/schemas/nwfilter.rng
+++ libvirt-acl/docs/schemas/nwfilter.rng
@@ -30,6 +30,16 @@
</optional>
<optional>
<zeroOrMore>
+ <element name="vlan">
+ <ref name="match-attribute"/>
+ <ref name="common-l2-attributes"/>
+ <ref name="vlan-attributes"/>
+ <ref name="comment-attribute"/>
+ </element>
+ </zeroOrMore>
+ </optional>
+ <optional>
+ <zeroOrMore>
<element name="arp">
<ref name="match-attribute"/>
<ref name="common-l2-attributes"/>
@@ -287,6 +297,9 @@
<choice>
<value>root</value>
<data type="string">
+ <param name="pattern">vlan[a-zA-Z0-9_\.:-]*</param>
+ </data>
+ <data type="string">
<param name="pattern">arp[a-zA-Z0-9_\.:-]*</param>
</data>
<data type="string">
@@ -559,6 +572,21 @@
</interleave>
</define>
+ <define name="vlan-attributes">
+ <interleave>
+ <optional>
+ <attribute name="vlanid">
+ <ref name="vlan-vlanid"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="encap-protocol">
+ <ref name="mac-protocolid"/>
+ </attribute>
+ </optional>
+ </interleave>
+ </define>
+
<define name="arp-attributes">
<interleave>
<optional>
@@ -764,10 +792,29 @@
<value>rarp</value>
<value>ipv4</value>
<value>ipv6</value>
+ <value>vlan</value>
</choice>
</choice>
</define>
+ <define name="vlan-vlanid">
+ <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,3})</param>
+ </data>
+
+ <data type="int">
+ <param name="minInclusive">0</param>
+ <param name="maxInclusive">4095</param>
+ </data>
+ </choice>
+ </define>
+
<define name="uint8range">
<choice>
<!-- variable -->
6:46 p.m.
New subject: [libvirt] [PATCH V1 1/9] Add support for VLAN filtering
On 10/26/2011 09:11 AM, Stefan Berger wrote:
Apologies for delaying so long on reviewing this.
This patch adds support for filtering of VLAN (802.1Q) traffic to
the
parser and makes us of the ebtables support for VLAN filtering. This code
now enables the filtering of traffic in chains with prefix 'vlan'.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
docs/schemas/nwfilter.rng | 47 +++++++++++++
src/conf/nwfilter_conf.c | 102 ++++++++++++++++++++++++++++++
src/conf/nwfilter_conf.h | 17 +++++
src/nwfilter/nwfilter_ebiptables_driver.c | 35 ++++++++++
4 files changed, 201 insertions(+)
This didn't apply cleanly for me. Am I missing the review of another
pre-requisite series? Or is it just something where you need to rebase
and post a v2 for easier review?
+static bool
+checkVlanVlanID(enum attrDatatype datatype, union data *value,
+ virNWFilterRuleDefPtr nwf ATTRIBUTE_UNUSED,
ATTRIBUTE_UNUSED not necessary here, since...
+ nwItemDesc *item ATTRIBUTE_UNUSED)
+{
+ int32_t res;
+
+ res = value->ui;
+ if (res < 0 || res > 4095) {
+ res = -1;
+ }
+
+ if (res != -1) {
+ nwf->p.vlanHdrFilter.dataVlanID.u.u16 = res;
...this uses nwf. Similar comment applies elsewhere in the patch.
Overall, it looks sane, but I didn't compile test it.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:27 a.m.
New subject: [libvirt] [PATCH V1 1/9] Add support for VLAN filtering
On Tue, Nov 15, 2011 at 05:46:21PM -0700, Eric Blake wrote:
On 10/26/2011 09:11 AM, Stefan Berger wrote:
Apologies for delaying so long on reviewing this.
> This patch adds support for filtering of VLAN (802.1Q) traffic to the
> parser and makes us of the ebtables support for VLAN filtering. This code
> now enables the filtering of traffic in chains with prefix 'vlan'.
>
> Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
>
> ---
> docs/schemas/nwfilter.rng | 47 +++++++++++++
> src/conf/nwfilter_conf.c | 102 ++++++++++++++++++++++++++++++
> src/conf/nwfilter_conf.h | 17 +++++
> src/nwfilter/nwfilter_ebiptables_driver.c | 35 ++++++++++
> 4 files changed, 201 insertions(+)
>
This didn't apply cleanly for me. Am I missing the review of another
pre-requisite series? Or is it just something where you need to rebase
and post a v2 for easier review?
It could be some of my netdev API renaming invalidated it ?
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
5:26 a.m.
New subject: [libvirt] [PATCH V1 1/9] Add support for VLAN filtering
On 11/15/2011 07:46 PM, Eric Blake wrote:
On 10/26/2011 09:11 AM, Stefan Berger wrote:
Apologies for delaying so long on reviewing this.
> This patch adds support for filtering of VLAN (802.1Q) traffic to
the
> parser and makes us of the ebtables support for VLAN filtering. This code
> now enables the filtering of traffic in chains with prefix 'vlan'.
>
> Signed-off-by: Stefan Berger<stefanb(a)linux.vnet.ibm.com>
>
> ---
> docs/schemas/nwfilter.rng | 47 +++++++++++++
> src/conf/nwfilter_conf.c | 102 ++++++++++++++++++++++++++++++
> src/conf/nwfilter_conf.h | 17 +++++
> src/nwfilter/nwfilter_ebiptables_driver.c | 35 ++++++++++
> 4 files changed, 201 insertions(+)
>
This didn't apply cleanly for me. Am I missing the review of another
pre-requisite series? Or is it just something where you need to rebase
and post a v2 for easier review?
Yes, sorry for the confusion. This builds on top of the following two
series:
https://www.redhat.com/archives/libvir-list/2011-October/msg01227.html
then came this one (already ACKed):
https://www.redhat.com/archives/libvir-list/2011-October/msg01360.html
>
> +static bool
> +checkVlanVlanID(enum attrDatatype datatype, union data *value,
> + virNWFilterRuleDefPtr nwf ATTRIBUTE_UNUSED,
ATTRIBUTE_UNUSED not necessary here, since...
> + nwItemDesc *item ATTRIBUTE_UNUSED)
> +{
> + int32_t res;
> +
> + res = value->ui;
> + if (res< 0 || res> 4095) {
> + res = -1;
> + }
> +
> + if (res != -1) {
> + nwf->p.vlanHdrFilter.dataVlanID.u.u16 = res;
...this uses nwf. Similar comment applies elsewhere in the patch.
Thanks. Fixing it.
Overall, it looks sane, but I didn't compile test it.
Stefan
10:11 a.m.
New subject: [libvirt] [PATCH V1 2/9] Add test cases for VLAN traffic filtering
This patch adds a few test cases for the XML parsing of VLAN filtering nodes.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
tests/nwfilterxml2xmlin/vlan-test.xml | 45 +++++++++++++++++++++++++++++++++
tests/nwfilterxml2xmlout/vlan-test.xml | 21 +++++++++++++++
tests/nwfilterxml2xmltest.c | 1
3 files changed, 67 insertions(+)
Index: libvirt-acl/tests/nwfilterxml2xmlin/vlan-test.xml
===================================================================
--- /dev/null
+++ libvirt-acl/tests/nwfilterxml2xmlin/vlan-test.xml
@@ -0,0 +1,45 @@
+<filter name='testcase'>
+ <uuid>5c6d49af-b071-6127-b4ec-6f8ed4b55335</uuid>
+ <rule action='continue' direction='inout'>
+ <vlan srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff'
+ dstmacaddr='aa:bb:cc:dd:ee:ff' dstmacmask='ff:ff:ff:ff:ff:ff'
+ vlanid='0x123'
+ />
+ </rule>
+
+ <rule action='return' direction='inout'>
+ <vlan srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff'
+ dstmacaddr='aa:bb:cc:dd:ee:ff' dstmacmask='ff:ff:ff:ff:ff:ff'
+ vlanid='1234'
+ />
+ </rule>
+
+ <rule action='reject' direction='in'>
+ <vlan srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff'
+ dstmacaddr='aa:bb:cc:dd:ee:ff' dstmacmask='ff:ff:ff:ff:ff:ff'
+ vlanid='0x123'
+ />
+ </rule>
+
+ <rule action='accept' direction='in'>
+ <vlan srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff'
+ dstmacaddr='aa:bb:cc:dd:ee:ff' dstmacmask='ff:ff:ff:ff:ff:ff'
+ vlanid='0xffff'
+ />
+ </rule>
+
+ <rule action='drop' direction='out'>
+ <vlan srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff'
+ dstmacaddr='aa:bb:cc:dd:ee:ff' dstmacmask='ff:ff:ff:ff:ff:ff'
+ encap-protocol='arp'
+ />
+ </rule>
+
+ <rule action='accept' direction='out'>
+ <vlan srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff'
+ dstmacaddr='aa:bb:cc:dd:ee:ff' dstmacmask='ff:ff:ff:ff:ff:ff'
+ encap-protocol='0x1234'
+ />
+ </rule>
+
+</filter>
Index: libvirt-acl/tests/nwfilterxml2xmltest.c
===================================================================
--- libvirt-acl.orig/tests/nwfilterxml2xmltest.c
+++ libvirt-acl/tests/nwfilterxml2xmltest.c
@@ -108,6 +108,7 @@ mymain(void)
} while (0)
DO_TEST("mac-test", true);
+ DO_TEST("vlan-test", true);
DO_TEST("arp-test", true);
DO_TEST("rarp-test", true);
DO_TEST("ip-test", true);
Index: libvirt-acl/tests/nwfilterxml2xmlout/vlan-test.xml
===================================================================
--- /dev/null
+++ libvirt-acl/tests/nwfilterxml2xmlout/vlan-test.xml
@@ -0,0 +1,21 @@
+<filter name='testcase' chain='root'>
+ <uuid>5c6d49af-b071-6127-b4ec-6f8ed4b55335</uuid>
+ <rule action='continue' direction='inout'
priority='500'>
+ <vlan srcmacaddr='01:02:03:04:05:06'
srcmacmask='ff:ff:ff:ff:ff:ff' dstmacaddr='aa:bb:cc:dd:ee:ff'
dstmacmask='ff:ff:ff:ff:ff:ff' vlanid='0x123'/>
+ </rule>
+ <rule action='return' direction='inout' priority='500'>
+ <vlan srcmacaddr='01:02:03:04:05:06'
srcmacmask='ff:ff:ff:ff:ff:ff' dstmacaddr='aa:bb:cc:dd:ee:ff'
dstmacmask='ff:ff:ff:ff:ff:ff' vlanid='1234'/>
+ </rule>
+ <rule action='reject' direction='in' priority='500'>
+ <vlan srcmacaddr='01:02:03:04:05:06'
srcmacmask='ff:ff:ff:ff:ff:ff' dstmacaddr='aa:bb:cc:dd:ee:ff'
dstmacmask='ff:ff:ff:ff:ff:ff' vlanid='0x123'/>
+ </rule>
+ <rule action='accept' direction='in' priority='500'>
+ <vlan srcmacaddr='01:02:03:04:05:06'
srcmacmask='ff:ff:ff:ff:ff:ff' dstmacaddr='aa:bb:cc:dd:ee:ff'
dstmacmask='ff:ff:ff:ff:ff:ff'/>
+ </rule>
+ <rule action='drop' direction='out' priority='500'>
+ <vlan srcmacaddr='01:02:03:04:05:06'
srcmacmask='ff:ff:ff:ff:ff:ff' dstmacaddr='aa:bb:cc:dd:ee:ff'
dstmacmask='ff:ff:ff:ff:ff:ff' encap-protocol='arp'/>
+ </rule>
+ <rule action='accept' direction='out' priority='500'>
+ <vlan srcmacaddr='01:02:03:04:05:06'
srcmacmask='ff:ff:ff:ff:ff:ff' dstmacaddr='aa:bb:cc:dd:ee:ff'
dstmacmask='ff:ff:ff:ff:ff:ff' encap-protocol='0x1234'/>
+ </rule>
+</filter>
6:06 p.m.
New subject: [libvirt] [PATCH V1 2/9] Add test cases for VLAN traffic filtering
On 10/26/2011 09:11 AM, Stefan Berger wrote:
This patch adds a few test cases for the XML parsing of VLAN
filtering nodes.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
tests/nwfilterxml2xmlin/vlan-test.xml | 45 +++++++++++++++++++++++++++++++++
tests/nwfilterxml2xmlout/vlan-test.xml | 21 +++++++++++++++
tests/nwfilterxml2xmltest.c | 1
3 files changed, 67 insertions(+)
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:12 a.m.
New subject: [libvirt] [PATCH V1 3/9] Add documentation for VLAN filtering support
Add documentation for the VLAN filtering support. Describe the XML attributes
that are supported.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
docs/formatnwfilter.html.in | 55 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
Index: libvirt-acl/docs/formatnwfilter.html.in
===================================================================
--- libvirt-acl.orig/docs/formatnwfilter.html.in
+++ libvirt-acl/docs/formatnwfilter.html.in
@@ -412,6 +412,61 @@
[...]
</pre>
+ <h5><a name="nwfelemsRulesProtoVLAN">VLAN (802.1Q)</a>
+ <span class="since">(Since 0.9.7)</span>
+ </h5>
+ <p>
+ Protocol ID: <code>vlan</code>
+ <br/>
+ Note: Rules of this type should go eitherinto the <code>root</code> or
+ <code>vlan</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>dstmacaddr</td>
+ <td>MAC_ADDR</td>
+ <td>MAC address of destination</td>
+ </tr>
+ <tr>
+ <td>dstmacmask</td>
+ <td>MAC_MASK</td>
+ <td>Mask applied to MAC address of destination</td>
+ </tr>
+ <tr>
+ <td>vlan-id</td>
+ <td>UINT16 (0x0-0xfff, 0 - 4095)</td>
+ <td>VLAN ID</td>
+ </tr>
+ <tr>
+ <td>encap-protocol</td>
+ <td>UINT16 (0x03c-0xfff), String</td>
+ <td>Encapsulated layer 3 protocol ID</td>
+ </tr>
+ <tr>
+ <td>comment </td>
+ <td>STRING</td>
+ <td>text with max. 256 characters</td>
+ </tr>
+ </table>
+ <p>
+ Valid Strings for <code>encap-protocol</code> are: arp, ipv4, ipv6
+ </p>
+
<h5><a
name="nwfelemsRulesProtoARP">ARP/RARP</a></h5>
<p>
Protocol ID: <code>arp</code> or <code>rarp</code>
6:14 p.m.
New subject: [libvirt] [PATCH V1 3/9] Add documentation for VLAN filtering support
On 10/26/2011 09:12 AM, Stefan Berger wrote:
Add documentation for the VLAN filtering support. Describe the XML
attributes
that are supported.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
Not sure if this overlaps with any of the stuff you pulled in earlier
(it will be Monday before I get my development tree back into a state
where I can apply this to actually test it). But in general, adding
docs is good.
+ <br/>
+ Note: Rules of this type should go eitherinto the <code>root</code> or
s/eitherinto/either into/
ACK with the typo fixed, assuming it still applies fairly easily.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
7:01 a.m.
New subject: [libvirt] [PATCH V1 3/9] Add documentation for VLAN filtering support
On 11/18/2011 07:14 PM, Eric Blake wrote:
On 10/26/2011 09:12 AM, Stefan Berger wrote:
> Add documentation for the VLAN filtering support. Describe the XML attributes
> that are supported.
>
> Signed-off-by: Stefan Berger<stefanb(a)linux.vnet.ibm.com>
Not sure if this overlaps with any of the stuff you pulled in earlier
(it will be Monday before I get my development tree back into a state
where I can apply this to actually test it). But in general, adding
docs is good.
> +<br/>
> + Note: Rules of this type should go eitherinto the<code>root</code>
or
s/eitherinto/either into/
ACK with the typo fixed, assuming it still applies fairly easily.
Was not a problem... I now pushed the 3 VLAN related patches. Thanks.
10:12 a.m.
New subject: [libvirt] [PATCH V1 4/9] Add a mac chain
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(a)us.ibm.com>
Signed-off-by: Stefan Berger <stefanb(a)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>
<value>root</value>
<data type="string">
+ <param name="pattern">mac[a-zA-Z0-9_\.:-]*</param>
+ </data>
+ <data type="string">
<param name="pattern">vlan[a-zA-Z0-9_\.:-]*</param>
</data>
<data type="string">
4:13 p.m.
New subject: [libvirt] [PATCH V1 4/9] Add a mac chain
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 <dlstevens(a)us.ibm.com>
Signed-off-by: Stefan Berger <stefanb(a)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 0xnnnn -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 eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:16 a.m.
New subject: [libvirt] [PATCH V1 4/9] Add a mac chain
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 Stevens<dlstevens(a)us.ibm.com>
> Signed-off-by: Stefan Berger<stefanb(a)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 0xnnnn -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.
10:12 a.m.
New subject: [libvirt] [PATCH V1 5/9] Add support for STP filtering
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(a)linux.vnet.ibm.com>
---
docs/schemas/nwfilter.rng | 154 +++++++++++++++++++++++++
src/conf/nwfilter_conf.c | 178 ++++++++++++++++++++++++++++++
src/conf/nwfilter_conf.h | 41 ++++++
src/libvirt_private.syms | 1
src/nwfilter/nwfilter_ebiptables_driver.c | 90 +++++++++++++++
5 files changed, 461 insertions(+), 3 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 (virSocketParseIpv4Addr(prop,
&item->u.ipaddr) < 0)
@@ -1841,6 +1986,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);
@@ -2979,6 +3149,14 @@ virNWFilterRuleDefDetailsFormat(virBuffe
item->u.u16);
break;
+ case DATATYPE_UINT32_HEX:
+ asHex = true;
+ /* fallthrough */
+ case DATATYPE_UINT32:
+ virBufferAsprintf(buf, asHex ? "0x%x" : "%d",
+ 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
@@ -179,6 +179,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
};
@@ -190,6 +191,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),
};
@@ -290,6 +292,16 @@ _printDataType(virNWFilterVarCombIterPtr
}
break;
+ case DATATYPE_UINT32:
+ case DATATYPE_UINT32_HEX:
+ if (snprintf(buf, bufsize, asHex ? "0x%x" : "%d",
+ 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",
@@ -2012,10 +2024,82 @@ ebtablesCreateRuleInstance(char chainPre
number); \
}
+#define INST_ITEM_2PARMS(STRUCT, ITEM, ITEM_HI, CLI, SEP) \
+ if (HAS_ENTRY_ITEM(&rule->p.STRUCT.ITEM)) { \
+ if (printDataType(vars, \
+ number, sizeof(number), \
+ &rule->p.STRUCT.ITEM)) \
+ goto err_exit; \
+ virBufferAsprintf(&buf, \
+ " " CLI " %s %s", \
+ ENTRY_GET_NEG_SIGN(&rule->p.STRUCT.ITEM), \
+ number); \
+ if (HAS_ENTRY_ITEM(&rule->p.STRUCT.ITEM_HI)) { \
+ if (printDataType(vars, \
+ number, sizeof(number), \
+ &rule->p.STRUCT.ITEM_HI)) \
+ goto err_exit; \
+ virBufferAsprintf(&buf, SEP "%s", number); \
+ } \
+ }
+#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:
@@ -2464,6 +2548,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:
@@ -2907,7 +2992,7 @@ 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, };
+ char protostr[32] = { 0, };
PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
PRINT_CHAIN(chain, chainPrefix, ifname,
@@ -2916,6 +3001,9 @@ ebtablesCreateTmpSubChain(ebiptablesRule
switch (protoidx) {
case L2_PROTO_MAC_IDX:
break;
+ case L2_PROTO_STP_IDX:
+ snprintf(protostr, sizeof(protostr), "-d " NWFILTER_MAC_BGA);
+ break;
default:
snprintf(protostr, sizeof(protostr), "-p 0x%04x ",
l3_protocols[protoidx].attr);
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_\.:-]*</param>
</data>
<data type="string">
+ <param name="pattern">stp[a-zA-Z0-9_\.:-]*</param>
+ </data>
+ <data type="string">
<param name="pattern">vlan[a-zA-Z0-9_\.:-]*</param>
</data>
<data type="string">
@@ -384,7 +397,7 @@
</interleave>
</define>
- <define name="common-l2-attributes">
+ <define name="srcmacandmask-attributes">
<interleave>
<ref name="srcmac-attribute"/>
<optional>
@@ -392,6 +405,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"/>
@@ -590,6 +609,121 @@
</interleave>
</define>
+ <define name="stp-attributes">
+ <interleave>
+ <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>
+ </interleave>
+ </define>
+
<define name="arp-attributes">
<interleave>
<optional>
@@ -854,6 +988,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
@@ -874,6 +874,7 @@ virNWFilterPrintStateMatchFlags;
virNWFilterPrintTCPFlags;
virNWFilterRegisterCallbackDriver;
virNWFilterRuleActionTypeToString;
+virNWFilterRuleDirectionTypeToString;
virNWFilterRuleProtocolTypeToString;
virNWFilterTestUnassignDef;
virNWFilterUnlockFilterUpdates;
4:50 p.m.
New subject: [libvirt] [PATCH V1 5/9] Add support for STP filtering
On 10/26/2011 09:12 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(a)linux.vnet.ibm.com>
---
docs/schemas/nwfilter.rng | 154 +++++++++++++++++++++++++
src/conf/nwfilter_conf.c | 178 ++++++++++++++++++++++++++++++
src/conf/nwfilter_conf.h | 41 ++++++
src/libvirt_private.syms | 1
src/nwfilter/nwfilter_ebiptables_driver.c | 90 +++++++++++++++
5 files changed, 461 insertions(+), 3 deletions(-)
Some conflict resolution, again in the rng, and also in context for
nwfilter_conf.c, but should be trivial.
You already pre-emptively mentioned STP chains in an earlier patch, and
I see the title of 7/9 mentions more about STP documentation, so I'll
assume that between those two, the new .rng additions are properly
documented.
@@ -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 = "forward-delay-hi",
+ .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX,
+ .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataFwdDelayHi),
+ },
+ COMMENT_PROP(stpHdrFilter),
I'm assuming this is an accurate layout mapping the on-the-wire struct
to named fields for reference in XML attributes, although I didn't
actually go hunt down an RFC to verify. Perhaps a comment pointing tot
the STP RFC might prove handy.
@@ -2979,6 +3149,14 @@ virNWFilterRuleDefDetailsFormat(virBuffe
item->u.u16);
break;
+ case DATATYPE_UINT32_HEX:
+ asHex = true;
+ /* fallthrough */
+ case DATATYPE_UINT32:
+ virBufferAsprintf(buf, asHex ? "0x%x" : "%d",
+ item->u.u32);
%u, not %d. Otherwise you introduce a spurious negative sign on values
with the most-significant-bit set.
Also, I'm not entirely sure whether %u and uint32_t always match, or if
there are some 32-bit platforms where uint32_t is long and this would
trigger a type mismatch warning from gcc. On the other hand, this code
only compiles on Linux where we know uint32_t is always int; using
<inttypes.h> for PRIu32 would be more portable, but that's a separate
cleanup.
@@ -290,6 +292,16 @@ _printDataType(virNWFilterVarCombIterPtr
}
break;
+ case DATATYPE_UINT32:
+ case DATATYPE_UINT32_HEX:
+ if (snprintf(buf, bufsize, asHex ? "0x%x" : "%d",
+ item->u.u32) >= bufsize) {
+ virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Buffer too small for uint32 type"));
Again, %u, not %d.
Also, this code tends to be called with a hard-coded allocation of
'number[20];', which is sufficient for uint32_t, but not long enough if
we ever expand to DATATYPE_UINT64. I'm wondering if we should use
"intprops.h" from gnulib, for the INT_BUFSIZE_BOUND() macro, rather than
a hard-coded 20. But at least this snprintf usage checked for error (I
noticed in 4/9 that you used snprintf without error checking).
+ return 1;
Looks like this is code addition to an existing function with positive 1
return convention, so you can defer changing it to -1 until a later
patch that cleans up the entire function (I'm only worried about
completely new functions introduced by this patch).
+ 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);
Looks like you've got some rebasing to do, depending on whether you push
this or your env-var cleanup first.
@@ -2907,7 +2992,7 @@ 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, };
+ char protostr[32] = { 0, };
PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
PRINT_CHAIN(chain, chainPrefix, ifname,
@@ -2916,6 +3001,9 @@ ebtablesCreateTmpSubChain(ebiptablesRule
switch (protoidx) {
case L2_PROTO_MAC_IDX:
break;
+ case L2_PROTO_STP_IDX:
+ snprintf(protostr, sizeof(protostr), "-d " NWFILTER_MAC_BGA);
+ break;
Ah - here's an unchecked snprintf, which is probably worth tweaking for
maintenance safety, especially since you just changed the size of
protostr above.
@@ -590,6 +609,121 @@
</interleave>
</define>
+ <define name="stp-attributes">
+ <interleave>
+ <optional>
+ <attribute name="type">
+ <ref name="uint8range"/>
+ </attribute>
+ </optional>
If I understand RNG correctly, <interleave> isn't required for
attributes (only for sub-elements).
We're starting to get enough comments and merge conflicts as I review
this, that I think it will help if you post a v2 with all of the
remaining patches from both this series and your $EBT patch rolled into
a single commit series showing the final order you plan to push in.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:51 a.m.
New subject: [libvirt] [PATCH V1 5/9] Add support for STP filtering
On 11/21/2011 05:50 PM, Eric Blake wrote:
On 10/26/2011 09:12 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(a)linux.vnet.ibm.com>
>
> ---
> docs/schemas/nwfilter.rng | 154 +++++++++++++++++++++++++
> src/conf/nwfilter_conf.c | 178 ++++++++++++++++++++++++++++++
> src/conf/nwfilter_conf.h | 41 ++++++
> src/libvirt_private.syms | 1
> src/nwfilter/nwfilter_ebiptables_driver.c | 90 +++++++++++++++
> 5 files changed, 461 insertions(+), 3 deletions(-)
Some conflict resolution, again in the rng, and also in context for
nwfilter_conf.c, but should be trivial.
You already pre-emptively mentioned STP chains in an earlier patch, and
I see the title of 7/9 mentions more about STP documentation, so I'll
assume that between those two, the new .rng additions are properly
documented.
> @@ -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 = "forward-delay-hi",
> + .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX,
> + .dataIdx = offsetof(virNWFilterRuleDef, p.stpHdrFilter.dataFwdDelayHi),
> + },
> + COMMENT_PROP(stpHdrFilter),
I'm assuming this is an accurate layout mapping the on-the-wire struct
to named fields for reference in XML attributes, although I didn't
actually go hunt down an RFC to verify. Perhaps a comment pointing tot
the STP RFC might prove handy.
I don't think it's an RFC, but an IEEE standard. I couldn't find a
better page than this one, though:
http://www.javvin.com/protocolSTP.html
> @@ -2979,6 +3149,14 @@ virNWFilterRuleDefDetailsFormat(virBuffe
> item->u.u16);
> break;
>
> + case DATATYPE_UINT32_HEX:
> + asHex = true;
> + /* fallthrough */
> + case DATATYPE_UINT32:
> + virBufferAsprintf(buf, asHex ? "0x%x" :
"%d",
> + item->u.u32);
%u, not %d. Otherwise you introduce a spurious negative sign on values
with the most-significant-bit set.
Fixed.
Also, I'm not entirely sure whether %u and uint32_t always match,
or if
there are some 32-bit platforms where uint32_t is long and this would
trigger a type mismatch warning from gcc. On the other hand, this code
only compiles on Linux where we know uint32_t is always int; using
<inttypes.h> for PRIu32 would be more portable, but that's a separate
cleanup.
> @@ -290,6 +292,16 @@ _printDataType(virNWFilterVarCombIterPtr
> }
> break;
>
> + case DATATYPE_UINT32:
> + case DATATYPE_UINT32_HEX:
> + if (snprintf(buf, bufsize, asHex ? "0x%x" : "%d",
> + item->u.u32)>= bufsize) {
> + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Buffer too small for uint32
type"));
Again, %u, not %d.
Fixed.
Also, this code tends to be called with a hard-coded allocation of
'number[20];', which is sufficient for uint32_t, but not long enough if
we ever expand to DATATYPE_UINT64. I'm wondering if we should use
"intprops.h" from gnulib, for the INT_BUFSIZE_BOUND() macro, rather than
a hard-coded 20. But at least this snprintf usage checked for error (I
noticed in 4/9 that you used snprintf without error checking).
Using that now with all occurrences of number[].
> + return 1;
Looks like this is code addition to an existing function with positive 1
return convention, so you can defer changing it to -1 until a later
patch that cleans up the entire function (I'm only worried about
completely new functions introduced by this patch).
>
> + 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);
Looks like you've got some rebasing to do, depending on whether you push
this or your env-var cleanup first.
That shell var replacement stuff is scheduled for after this patch series...
> @@ -2907,7 +2992,7 @@ 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, };
> + char protostr[32] = { 0, };
>
> PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
> PRINT_CHAIN(chain, chainPrefix, ifname,
> @@ -2916,6 +3001,9 @@ ebtablesCreateTmpSubChain(ebiptablesRule
> switch (protoidx) {
> case L2_PROTO_MAC_IDX:
> break;
> + case L2_PROTO_STP_IDX:
> + snprintf(protostr, sizeof(protostr), "-d " NWFILTER_MAC_BGA);
> + break;
Ah - here's an unchecked snprintf, which is probably worth tweaking for
maintenance safety, especially since you just changed the size of
protostr above.
Fixed.
> @@ -590,6 +609,121 @@
> </interleave>
> </define>
>
> +<define name="stp-attributes">
> +<interleave>
> +<optional>
> +<attribute name="type">
> +<ref name="uint8range"/>
> +</attribute>
> +</optional>
If I understand RNG correctly,<interleave> isn't required for
attributes (only for sub-elements).
Fixed. I'll clean-up the rng at some point
since I did this just about
everywhere...
We're starting to get enough comments and merge conflicts as I
review
this, that I think it will help if you post a v2 with all of the
remaining patches from both this series and your $EBT patch rolled into
a single commit series showing the final order you plan to push in.
10:20 a.m.
New subject: [libvirt] [PATCH V1 5/9] Add support for STP filtering
On 11/22/2011 07:51 AM, Stefan Berger wrote:
On 11/21/2011 05:50 PM, Eric Blake wrote:
> On 10/26/2011 09:12 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'.
>>
> I'm assuming this is an accurate layout mapping the on-the-wire struct
> to named fields for reference in XML attributes, although I didn't
> actually go hunt down an RFC to verify. Perhaps a comment pointing tot
> the STP RFC might prove handy.
>
I don't think it's an RFC, but an IEEE standard. I couldn't find a
better page than this one, though:
http://www.javvin.com/protocolSTP.html
Then how about this comment:
/* STP is documented by IEEE 802.1D; for a synopsis,
* see http://www.javvin.com/protocolSTP.html */
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:12 a.m.
New subject: [libvirt] [PATCH V1 6/9] Add test cases for STP traffic filtering
This patch adds a few test cases for the XML parsing of STP filtering nodes.
Signed-off-by: Stefan Berger <stefanb(a)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>
5:02 p.m.
New subject: [libvirt] [PATCH V1 6/9] Add test cases for STP traffic filtering
On 10/26/2011 09:12 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(a)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(+)
Looks good. ACK.
+ <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'/>
That's a lot of attributes for one element, thankfully we can split it
over multiple lines.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:12 a.m.
New subject: [libvirt] [PATCH V1 7/9] Add documentation for STP filtering support
Add documentation for the STP filtering support. Describe the XML attributes
that are supported.
Signed-off-by: Stefan Berger <stefanb(a)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
@@ -467,6 +467,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.7)</span>
+ </h5>
+ <p>
+ Protocol ID: <code>stp</code>
+ <br/>
+ Note: Rules of this type should go eitherinto 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>
5:08 p.m.
New subject: [libvirt] [PATCH V1 7/9] Add documentation for STP filtering support
On 10/26/2011 09:12 AM, Stefan Berger wrote:
Add documentation for the STP filtering support. Describe the XML
attributes
that are supported.
Signed-off-by: Stefan Berger <stefanb(a)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
@@ -467,6 +467,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.7)</span>
0.9.8, now.
+ </h5>
+ <p>
+ Protocol ID: <code>stp</code>
+ <br/>
+ Note: Rules of this type should go eitherinto the <code>root</code> or
s/eitherinto/either into/
ACK with those nits fixed.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:12 a.m.
New subject: [libvirt] [PATCH V1 8/9] [RFC] New filters to handle multiple IP addresses
With new filters borrowed from David Steven's submission:
A set of new filters to handle multiple IP addresses and MAC addresses
per interface. The alternative would be to replace some of the existing
ones with these here.
Signed-off-by: David L Stevens <dlstevens(a)us.ibm.com>
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
examples/xml/nwfilter/Makefile.am | 6 +++++
examples/xml/nwfilter/clean-traffic-new.xml | 29 ++++++++++++++++++++++++++
examples/xml/nwfilter/no-arp-spoofing-new.xml | 4 +++
examples/xml/nwfilter/no-arpip-spoofing.xml | 12 ++++++++++
examples/xml/nwfilter/no-arpmac-spoofing.xml | 8 +++++++
examples/xml/nwfilter/no-ip-spoofing-new.xml | 5 ++++
examples/xml/nwfilter/no-mac-spoofing-new.xml | 10 ++++++++
7 files changed, 74 insertions(+)
Index: libvirt-acl/examples/xml/nwfilter/Makefile.am
===================================================================
--- libvirt-acl.orig/examples/xml/nwfilter/Makefile.am
+++ libvirt-acl/examples/xml/nwfilter/Makefile.am
@@ -8,11 +8,17 @@ FILTERS = \
allow-incoming-ipv4.xml \
allow-ipv4.xml \
clean-traffic.xml \
+ clean-traffic-new.xml \
no-arp-spoofing.xml \
+ no-arp-spoofing-new.xml \
+ no-arpip-spoofing.xml \
+ no-arpmac-spoofing.xml \
no-ip-multicast.xml \
no-ip-spoofing.xml \
+ no-ip-spoofing-new.xml \
no-mac-broadcast.xml \
no-mac-spoofing.xml \
+ no-mac-spoofing-new.xml \
no-other-l2-traffic.xml \
no-other-rarp-traffic.xml \
qemu-announce-self.xml \
Index: libvirt-acl/examples/xml/nwfilter/no-arpip-spoofing.xml
===================================================================
--- /dev/null
+++ libvirt-acl/examples/xml/nwfilter/no-arpip-spoofing.xml
@@ -0,0 +1,12 @@
+<filter name='no-arpip-spoofing' chain='arpip'
priority='-510'>
+ <!-- no arp spoofing -->
+ <!-- drop if ipaddr does not belong to guest -->
+ <rule action='return' direction='out' priority='400' >
+ <arp match='yes' arpsrcipaddr='$IP' />
+ </rule>
+ <rule action='return' direction='out' priority='410' >
+ <arp match='yes' arpsrcipaddr='0.0.0.0' />
+ </rule>
+ <!-- drop everything else -->
+ <rule action='drop' direction='out' priority='1000' />
+</filter>
Index: libvirt-acl/examples/xml/nwfilter/no-arpmac-spoofing.xml
===================================================================
--- /dev/null
+++ libvirt-acl/examples/xml/nwfilter/no-arpmac-spoofing.xml
@@ -0,0 +1,8 @@
+<filter name='no-arpmac-spoofing' chain='arpmac'
priority='-510'>
+ <rule action='return' direction='out' priority='350' >
+ <arp match='yes' arpsrcmacaddr='$MAC'/>
+ </rule>
+ <!-- drop everything else -->
+ <rule action='drop' direction='out' priority='1000' />
+</filter>
+
Index: libvirt-acl/examples/xml/nwfilter/clean-traffic-new.xml
===================================================================
--- /dev/null
+++ libvirt-acl/examples/xml/nwfilter/clean-traffic-new.xml
@@ -0,0 +1,29 @@
+<filter name='clean-traffic-new' chain='root'>
+ <!-- An example of a traffic filter enforcing clean traffic
+ from a VM by
+ - preventing MAC spoofing -->
+ <filterref filter='no-mac-spoofing-new'/>
+
+ <!-- preventing IP spoofing on outgoing, allow all IPv4 in incoming -->
+ <filterref filter='no-ip-spoofing-new'/>
+
+ <rule direction='out' action='accept' priority='-750'>
+ <mac protocolid='ipv4'/>
+ </rule>
+
+ <filterref filter='allow-incoming-ipv4'/>
+
+ <!-- preventing ARP spoofing/poisoning -->
+ <filterref filter='no-arp-spoofing-new'/>
+
+ <rule direction='out' action='accept' priority='-550'>
+ <mac protocolid='arp'/>
+ </rule>
+
+ <!-- preventing any other traffic than IPv4 and ARP -->
+ <filterref filter='no-other-l2-traffic'/>
+
+ <!-- allow qemu to send a self-announce upon migration end -->
+ <filterref filter='qemu-announce-self'/>
+
+</filter>
Index: libvirt-acl/examples/xml/nwfilter/no-arp-spoofing-new.xml
===================================================================
--- /dev/null
+++ libvirt-acl/examples/xml/nwfilter/no-arp-spoofing-new.xml
@@ -0,0 +1,4 @@
+<filter name='no-arp-spoofing-new'>
+ <filterref filter='no-arpmac-spoofing'/>
+ <filterref filter='no-arpip-spoofing'/>
+</filter>
Index: libvirt-acl/examples/xml/nwfilter/no-mac-spoofing-new.xml
===================================================================
--- /dev/null
+++ libvirt-acl/examples/xml/nwfilter/no-mac-spoofing-new.xml
@@ -0,0 +1,10 @@
+<filter name='no-mac-spoofing-new' chain='mac'
priority='-800'>
+ <!-- return packets with VM's MAC address as source address -->
+ <rule direction='out' action='return'>
+ <mac srcmacaddr='$MAC'/>
+ </rule>
+ <!-- drop everything else -->
+ <rule direction='out' action='drop'>
+ <mac/>
+ </rule>
+</filter>
Index: libvirt-acl/examples/xml/nwfilter/no-ip-spoofing-new.xml
===================================================================
--- /dev/null
+++ libvirt-acl/examples/xml/nwfilter/no-ip-spoofing-new.xml
@@ -0,0 +1,5 @@
+<filter name='no-ip-spoofing-new' chain='ipv4-ip'
priority='-710'>
+ <rule direction='out' action='return'>
+ <ip match='yes' srcipaddr='$IP'/>
+ </rule>
+</filter>
5:01 p.m.
New subject: [libvirt] [PATCH V1 8/9] [RFC] New filters to handle multiple IP addresses
On 10/26/2011 09:12 AM, Stefan Berger wrote:
With new filters borrowed from David Steven's submission:
A set of new filters to handle multiple IP addresses and MAC addresses
per interface. The alternative would be to replace some of the existing
ones with these here.
On IRC, you mentioned that you were dropping this, so I haven't bothered
to review it.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:12 a.m.
New subject: [libvirt] [PATCH V1 9/9] Improve error reporting of failures to apply filtering rules
Display the executed command and failure message if a command failed to
execute.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
src/nwfilter/nwfilter_ebiptables_driver.c | 82 ++++++++++++++++++------------
1 file changed, 50 insertions(+), 32 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
@@ -63,10 +63,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 \
: ""
@@ -2785,12 +2785,16 @@ err_exit:
*/
static int
ebiptablesExecCLI(virBufferPtr buf,
- int *status)
+ int *status, char **errbuf)
{
char *cmds;
char *filename;
int rc;
const char *argv[] = {NULL, NULL};
+ virCommandPtr cmd;
+
+ if (errbuf)
+ VIR_FREE(*errbuf);
if (virBufferError(buf)) {
virReportOOMError();
@@ -2817,7 +2821,13 @@ ebiptablesExecCLI(virBufferPtr buf,
virMutexLock(&execCLIMutex);
- rc = virRun(argv, status);
+ cmd = virCommandNewArgs(argv);
+ if (errbuf)
+ virCommandSetOutputBuffer(cmd, errbuf);
+
+ rc = virCommandRun(cmd, status);
+
+ virCommandFree(cmd);
virMutexUnlock(&execCLIMutex);
@@ -2831,6 +2841,8 @@ ebiptablesExecCLI(virBufferPtr buf,
}
VIR_DEBUG("rc = %d, status = %d", rc, *status);
+ if (errbuf && *errbuf)
+ VIR_DEBUG("error message: %s", *errbuf);
unlink(filename);
@@ -3285,7 +3297,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, &cli_status, NULL) || cli_status != 0)
goto tear_down_tmpebchains;
return 0;
@@ -3401,7 +3413,7 @@ ebtablesApplyDHCPOnlyRules(const char *i
ebtablesRenameTmpRootChain(&buf, 1, ifname);
ebtablesRenameTmpRootChain(&buf, 0, ifname);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, &cli_status, NULL) || cli_status != 0)
goto tear_down_tmpebchains;
VIR_FREE(srcIPParam);
@@ -3474,7 +3486,7 @@ ebtablesApplyDropAllRules(const char *if
ebtablesRenameTmpRootChain(&buf, 1, ifname);
ebtablesRenameTmpRootChain(&buf, 0, ifname);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, &cli_status, NULL) || cli_status != 0)
goto tear_down_tmpebchains;
return 0;
@@ -3517,7 +3529,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;
}
@@ -3677,6 +3689,7 @@ ebiptablesApplyNewRules(virConnectPtr co
bool haveIp6tables = false;
ebiptablesRuleInstPtr ebtChains = NULL;
int nEbtChains = 0;
+ char *errmsg = NULL;
if (!chains_in_set || !chains_out_set) {
virReportOOMError();
@@ -3715,7 +3728,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 */
@@ -3730,7 +3743,7 @@ ebiptablesApplyNewRules(virConnectPtr co
qsort(&ebtChains[0], nEbtChains, sizeof(ebtChains[0]),
ebiptablesRuleOrderSort);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, &cli_status, &errmsg) || cli_status != 0)
goto tear_down_tmpebchains;
/* process ebtables commands; interleave commands from filters with
@@ -3764,7 +3777,7 @@ ebiptablesApplyNewRules(virConnectPtr co
ebtChains[j++].commandTemplate,
'A', -1, 1);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, &cli_status, &errmsg) || cli_status != 0)
goto tear_down_tmpebchains;
if (haveIptables) {
@@ -3773,17 +3786,17 @@ ebiptablesApplyNewRules(virConnectPtr co
iptablesCreateBaseChains(iptables_cmd_path, &buf);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, &cli_status, &errmsg) || cli_status !=
0)
goto tear_down_tmpebchains;
iptablesCreateTmpRootChains(iptables_cmd_path, &buf, ifname);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, &cli_status, &errmsg) || cli_status !=
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, &cli_status, &errmsg) || cli_status !=
0)
goto tear_down_tmpiptchains;
for (i = 0; i < nruleInstances; i++) {
@@ -3794,7 +3807,7 @@ ebiptablesApplyNewRules(virConnectPtr co
'A', -1, 1);
}
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, &cli_status, &errmsg) || cli_status !=
0)
goto tear_down_tmpiptchains;
iptablesCheckBridgeNFCallEnabled(false);
@@ -3806,17 +3819,17 @@ ebiptablesApplyNewRules(virConnectPtr co
iptablesCreateBaseChains(ip6tables_cmd_path, &buf);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, &cli_status, &errmsg) || cli_status !=
0)
goto tear_down_tmpiptchains;
iptablesCreateTmpRootChains(ip6tables_cmd_path, &buf, ifname);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, &cli_status, &errmsg) || cli_status !=
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, &cli_status, &errmsg) || cli_status !=
0)
goto tear_down_tmpip6tchains;
for (i = 0; i < nruleInstances; i++) {
@@ -3826,7 +3839,7 @@ ebiptablesApplyNewRules(virConnectPtr co
'A', -1, 1);
}
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, &cli_status, &errmsg) || cli_status !=
0)
goto tear_down_tmpip6tchains;
iptablesCheckBridgeNFCallEnabled(true);
@@ -3837,7 +3850,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, &cli_status, &errmsg) || cli_status != 0)
goto tear_down_ebsubchains_and_unlink;
virHashFree(chains_in_set);
@@ -3847,6 +3860,8 @@ ebiptablesApplyNewRules(virConnectPtr co
VIR_FREE(ebtChains[i].commandTemplate);
VIR_FREE(ebtChains);
+ VIR_FREE(errmsg);
+
return 0;
tear_down_ebsubchains_and_unlink:
@@ -3874,12 +3889,13 @@ 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"),
+ ifname,
+ errmsg ? errmsg : "");
exit_free_sets:
virHashFree(chains_in_set);
@@ -3889,6 +3905,8 @@ exit_free_sets:
VIR_FREE(ebtChains[i].commandTemplate);
VIR_FREE(ebtChains);
+ VIR_FREE(errmsg);
+
return 1;
}
@@ -3919,7 +3937,7 @@ ebiptablesTearNewRules(virConnectPtr con
ebtablesRemoveTmpRootChain(&buf, 0, ifname);
}
- ebiptablesExecCLI(&buf, &cli_status);
+ ebiptablesExecCLI(&buf, &cli_status, NULL);
return 0;
}
@@ -3938,7 +3956,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) {
@@ -3946,7 +3964,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) {
@@ -3960,7 +3978,7 @@ ebiptablesTearOldRules(virConnectPtr con
ebtablesRenameTmpSubAndRootChains(&buf, ifname);
- ebiptablesExecCLI(&buf, &cli_status);
+ ebiptablesExecCLI(&buf, &cli_status, NULL);
}
return 0;
@@ -3997,7 +4015,7 @@ ebiptablesRemoveRules(virConnectPtr conn
'D', -1,
0);
- if (ebiptablesExecCLI(&buf, &cli_status))
+ if (ebiptablesExecCLI(&buf, &cli_status, NULL))
goto err_exit;
if (cli_status) {
@@ -4048,7 +4066,7 @@ ebiptablesAllTeardown(const char *ifname
ebtablesRemoveRootChain(&buf, 1, ifname);
ebtablesRemoveRootChain(&buf, 0, ifname);
}
- ebiptablesExecCLI(&buf, &cli_status);
+ ebiptablesExecCLI(&buf, &cli_status, NULL);
return 0;
}
@@ -4103,7 +4121,7 @@ ebiptablesDriverInit(bool privileged)
ebtables_cmd_path, EBTABLES_DEFAULT_TABLE,
CMD_STOPONERR(1));
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status)
+ if (ebiptablesExecCLI(&buf, &cli_status, NULL) || cli_status)
VIR_FREE(ebtables_cmd_path);
}
@@ -4116,7 +4134,7 @@ ebiptablesDriverInit(bool privileged)
iptables_cmd_path,
CMD_STOPONERR(1));
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status)
+ if (ebiptablesExecCLI(&buf, &cli_status, NULL) || cli_status)
VIR_FREE(iptables_cmd_path);
}
@@ -4129,7 +4147,7 @@ ebiptablesDriverInit(bool privileged)
ip6tables_cmd_path,
CMD_STOPONERR(1));
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status)
+ if (ebiptablesExecCLI(&buf, &cli_status, NULL) || cli_status)
VIR_FREE(ip6tables_cmd_path);
}
5:42 p.m.
New subject: [libvirt] [PATCH V1 9/9] Improve error reporting of failures to apply filtering rules
On 10/26/2011 09:12 AM, Stefan Berger wrote:
Display the executed command and failure message if a command failed
to
execute.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
src/nwfilter/nwfilter_ebiptables_driver.c | 82 ++++++++++++++++++------------
1 file changed, 50 insertions(+), 32 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
@@ -63,10 +63,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
Okay, after turning the C literal into shell, you have:
eval res=\$\("${cmd} 2>&1 "\)
and after the shell eval, you have:
res=$(command 2>&1 )
That space after '2>&1' could be deleted, but it doesn't hurt to leave
it in.
#define CMD_STOPONERR(X) \
X ? "if [ $? -ne 0 ]; then" \
- " echo \"Failure to execute command '${cmd}'.\";"
\
+ " echo \"Failure to execute command '${cmd}' :
'${res}'.\";" \
Yes, this is a bit more informative.
" exit 1;" \
"fi" CMD_SEPARATOR \
: ""
@@ -2785,12 +2785,16 @@ err_exit:
*/
static int
ebiptablesExecCLI(virBufferPtr buf,
- int *status)
+ int *status, char **errbuf)
{
char *cmds;
char *filename;
int rc;
const char *argv[] = {NULL, NULL};
+ virCommandPtr cmd;
+
+ if (errbuf)
+ VIR_FREE(*errbuf);
This has merge conflicts with existing patches that went in during the
meantime. I'd feel better seeing a v2 to verify proper rebasing.
if (virBufferError(buf)) {
virReportOOMError();
@@ -2817,7 +2821,13 @@ ebiptablesExecCLI(virBufferPtr buf,
virMutexLock(&execCLIMutex);
- rc = virRun(argv, status);
+ cmd = virCommandNewArgs(argv);
+ if (errbuf)
+ virCommandSetOutputBuffer(cmd, errbuf);
It looks a bit odd calling it errbuf, when it is the output collected
from stdout; perhaps calling it outbuf would make more sense.
@@ -3285,7 +3297,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, &cli_status, NULL) || cli_status != 0)
You know, as long as we are cleaning things up, you could pass NULL
instead of &cli_status to enforce that the command has a 0 exit status,
so that you trim lines like this to:
if (ebiptablesExecCLI(&buf, NULL, NULL) < 0)
@@ -3874,12 +3889,13 @@ 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"),
+ ifname,
+ errmsg ? errmsg : "");
That outputs a trailing space if there was no error message. Better
might be:
virNWFilterReportError(VIR_ERR_BUILD_FIREWALL,
_("Some rules could not be created for "
"interface %s%s%s"),
ifname,
errmsg ? ": " : "",
errmsg ? errmsg : "");
Alas, we have an i18n nightmare. errmsg contains English text output by
the shell script, which is not marked for translation by either the
shell script (which itself is tricky - witness the libvirt-guests init
script) nor for translation in the C code that generated the shell
script. Meanwhile, since we didn't call virCommandAddEnvPassCommon() to
set the virCommand to force LC_ALL=C, that means we passed the libvirtd
setting of LC_MESSAGES on through to the child processes, such that sh
and ebtables output might also be translated. For an example, that
means someone using LC_MESSAGES=es_ES.UTF-8 could see:
Algunas reglas no han podido crearse para la interfaz eth0 : Failure to
execute command '/path/to/ebtables -t nat ...' : /bin/sh:
/path/to/ebtables: no se encontró la orden
That's a nasty mix of native/English/native output all in one very long
message. Maybe we should be rethinking the desired output format a bit,
for the sake of generating properly translated messages? Even breaking
things into multiple messages, so that each message is either translated
or English, rather than mixed, might help.
Is it worth trying to fix part of the translation issue, by defining
CMD_STOPONERROR to something that outputs the results of _("Failure to
execute command '%s'"), then taking that translation and properly
escaping it (via virBufferEscapeShell) so that the entire message will
be output literally by shell (so that translators can't cause arbitrary
shell execution by sneaking ';' into their translation), so that the
generated script has a pre-translated message? But that sounds hairy.
It also means that CMD_STOPONERROR would no longer be a string literal,
but a complex function call to properly append stuff into each place you
build up the command sequence into &buf.
Maybe this just serves as yet another reason why using the shell as a
script driver is prone to problems, and that anything we can do
long-term to rewrite this into internal libvirt API that bypasses shell
scripting will give better results, and we just live with the poor
diagnostics in the meantime.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:38 a.m.
New subject: [libvirt] [PATCH V1 9/9] Improve error reporting of failures to apply filtering rules
On 11/21/2011 06:42 PM, Eric Blake wrote:
On 10/26/2011 09:12 AM, Stefan Berger wrote:
> Display the executed command and failure message if a command failed to
> execute.
>
> Signed-off-by: Stefan Berger<stefanb(a)linux.vnet.ibm.com>
>
> ---
> src/nwfilter/nwfilter_ebiptables_driver.c | 82 ++++++++++++++++++------------
> 1 file changed, 50 insertions(+), 32 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
> @@ -63,10 +63,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
Okay, after turning the C literal into shell, you have:
eval res=\$\("${cmd} 2>&1 "\)
and after the shell eval, you have:
res=$(command 2>&1 )
That space after '2>&1' could be deleted, but it doesn't hurt to
leave
it in.
I removed it now.
> #define CMD_STOPONERR(X) \
> X ? "if [ $? -ne 0 ]; then" \
> - " echo \"Failure to execute command
'${cmd}'.\";" \
> + " echo \"Failure to execute command '${cmd}' :
'${res}'.\";" \
Yes, this is a bit more informative.
> " exit 1;" \
> "fi" CMD_SEPARATOR \
> : ""
> @@ -2785,12 +2785,16 @@ err_exit:
> */
> static int
> ebiptablesExecCLI(virBufferPtr buf,
> - int *status)
> + int *status, char **errbuf)
> {
> char *cmds;
> char *filename;
> int rc;
> const char *argv[] = {NULL, NULL};
> + virCommandPtr cmd;
> +
> + if (errbuf)
> + VIR_FREE(*errbuf);
This has merge conflicts with existing patches that went in during the
meantime. I'd feel better seeing a v2 to verify proper rebasing.
ok
>
> if (virBufferError(buf)) {
> virReportOOMError();
> @@ -2817,7 +2821,13 @@ ebiptablesExecCLI(virBufferPtr buf,
>
> virMutexLock(&execCLIMutex);
>
> - rc = virRun(argv, status);
> + cmd = virCommandNewArgs(argv);
> + if (errbuf)
> + virCommandSetOutputBuffer(cmd, errbuf);
It looks a bit odd calling it errbuf, when it is the output collected
from stdout; perhaps calling it outbuf would make more sense.
Fixed.
> @@ -3285,7 +3297,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,&cli_status, NULL) || cli_status != 0)
You know, as long as we are cleaning things up, you could pass NULL
instead of&cli_status to enforce that the command has a 0 exit status,
so that you trim lines like this to:
if (ebiptablesExecCLI(&buf, NULL, NULL)< 0)
I replace the occurrences of
the above pattern with the line you are
showing.
> @@ -3874,12 +3889,13 @@ 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"),
> + ifname,
> + errmsg ? errmsg : "");
That outputs a trailing space if there was no error message. Better
might be:
virNWFilterReportError(VIR_ERR_BUILD_FIREWALL,
_("Some rules could not be created for "
"interface %s%s%s"),
ifname,
errmsg ? ": " : "",
errmsg ? errmsg : "");
Fixed.
Alas, we have an i18n nightmare. errmsg contains English text output
by
the shell script, which is not marked for translation by either the
shell script (which itself is tricky - witness the libvirt-guests init
script) nor for translation in the C code that generated the shell
script. Meanwhile, since we didn't call virCommandAddEnvPassCommon() to
set the virCommand to force LC_ALL=C, that means we passed the libvirtd
setting of LC_MESSAGES on through to the child processes, such that sh
and ebtables output might also be translated. For an example, that
means someone using LC_MESSAGES=es_ES.UTF-8 could see:
Algunas reglas no han podido crearse para la interfaz eth0 : Failure to
execute command '/path/to/ebtables -t nat ...' : /bin/sh:
/path/to/ebtables: no se encontró la orden
That's a nasty mix of native/English/native output all in one very long
message. Maybe we should be rethinking the desired output format a bit,
for the sake of generating properly translated messages? Even breaking
things into multiple messages, so that each message is either translated
or English, rather than mixed, might help.
Is it worth trying to fix part of the translation issue, by defining
CMD_STOPONERROR to something that outputs the results of _("Failure to
execute command '%s'"), then taking that translation and properly
escaping it (via virBufferEscapeShell) so that the entire message will
be output literally by shell (so that translators can't cause arbitrary
shell execution by sneaking ';' into their translation), so that the
generated script has a pre-translated message? But that sounds hairy.
It also means that CMD_STOPONERROR would no longer be a string literal,
but a complex function call to properly append stuff into each place you
build up the command sequence into&buf.
Maybe this just serves as yet another reason why using the shell as a
script driver is prone to problems, and that anything we can do
long-term to rewrite this into internal libvirt API that bypasses shell
scripting will give better results, and we just live with the poor
diagnostics in the meantime.
Will leave it as-is.
4750
days inactive
4777
days old
25 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Stefan Berger