[libvirt] [Patch v3 0/5] nwfilter: Support comment attribute in filter rule descriptions
V3: - moved MAX_COMMENT_LENGTH #define into include file (1/5) - renamed IPTABLES_MAX_COMMENT_SIZE to IPTABLES_MAX_COMMENT_LENGTH (2/5) - removal of regex from string patter in schema for comment attribute (3/5) - added libvirt version since when comment attribute is available (4/5) - added test cases (5/5) V2: - work on the iptables instantiation patch (2/5) - work on the parser patch (1/5) - small changes to the test cases (5/5) The following patch series adds support for a comment node to the XML attributes of all protocols. If possible, as for example in case of iptables, the comments are instantiated (iptables ... -m comment --comment ...). The patches do the following: - extend the parser and XML generator to parse and create XML with the comment attribute - instantiate the comment in case of ip(6)tables - extend the nwfilter.rng schema with the comment attribute - add the information to the web docs - add a test case for the XML parser/generator to be run during 'make check' Regards, Stefan
V3: - moved MAX_COMMENT_LENGTH #define into nwfilter_conf.h V2: - introducing a field 'maxstrlen' to control the length of accepted string - removed function validating comment string assuming the XML parser already sinity checked the string The patch below extends the XML parser and generator so that every protocol now can have a comment node. Comments are limited to 256 characters. Signed-off-by: Stefan Berger <stefanb@us.ibm.com> --- src/conf/nwfilter_conf.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/nwfilter_conf.h | 12 ++++++- 2 files changed, 87 insertions(+), 1 deletion(-) Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -246,7 +246,11 @@ virNWFilterRuleDefFree(virNWFilterRuleDe for (i = 0; i < def->nvars; i++) VIR_FREE(def->vars[i]); + for (i = 0; i < def->nstrings; i++) + VIR_FREE(def->strings[i]); + VIR_FREE(def->vars); + VIR_FREE(def->strings); VIR_FREE(def); } @@ -350,6 +354,29 @@ virNWFilterRuleDefAddVar(virNWFilterRule } +static char * +virNWFilterRuleDefAddString(virNWFilterRuleDefPtr nwf, + const char *string, + size_t maxstrlen) +{ + if (VIR_REALLOC_N(nwf->strings, nwf->nstrings+1) < 0) { + virReportOOMError(); + return NULL; + } + + nwf->strings[nwf->nstrings] = strndup(string, maxstrlen); + + if (!nwf->strings[nwf->nstrings]) { + virReportOOMError(); + return NULL; + } + + nwf->nstrings++; + + return nwf->strings[nwf->nstrings-1]; +} + + void virNWFilterPoolObjRemove(virNWFilterPoolObjListPtr pools, virNWFilterPoolObjPtr pool) @@ -399,6 +426,7 @@ struct _virXMLAttr2Struct int dataIdx; // offset of the hasXYZ boolean valueValidator validator; // beyond-standard checkers valueFormatter formatter; // beyond-standard formatter + size_t maxstrlen; }; @@ -655,6 +683,18 @@ dscpValidator(enum attrDatatype datatype } +#define COMMENT_PROP(STRUCT) \ + {\ + .name = "comment",\ + .datatype = DATATYPE_STRINGCOPY,\ + .dataIdx = offsetof(virNWFilterRuleDef, p.STRUCT.dataComment),\ + .maxstrlen = MAX_COMMENT_LENGTH,\ + } + +#define COMMENT_PROP_IPHDR(STRUCT) \ + COMMENT_PROP(STRUCT.ipHdr) + + static const virXMLAttr2Struct macAttributes[] = { COMMON_MAC_PROPS(ethHdrFilter), { @@ -664,6 +704,7 @@ static const virXMLAttr2Struct macAttrib .validator= checkMacProtocolID, .formatter= macProtocolIDFormatter, }, + COMMENT_PROP(ethHdrFilter), { .name = NULL, } @@ -702,6 +743,7 @@ static const virXMLAttr2Struct arpAttrib .datatype = DATATYPE_IPADDR, .dataIdx = offsetof(virNWFilterRuleDef, p.arpHdrFilter.dataARPDstIPAddr), }, + COMMENT_PROP(arpHdrFilter), { .name = NULL, } @@ -762,6 +804,7 @@ static const virXMLAttr2Struct ipAttribu .dataIdx = offsetof(virNWFilterRuleDef, p.ipHdrFilter.ipHdr.dataDSCP), .validator = dscpValidator, }, + COMMENT_PROP_IPHDR(ipHdrFilter), { .name = NULL, } @@ -817,6 +860,7 @@ static const virXMLAttr2Struct ipv6Attri .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX, .dataIdx = offsetof(virNWFilterRuleDef, p.ipv6HdrFilter.portData.dataDstPortEnd), }, + COMMENT_PROP_IPHDR(ipv6HdrFilter), { .name = NULL, } @@ -914,6 +958,7 @@ static const virXMLAttr2Struct tcpAttrib .datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX, .dataIdx = offsetof(virNWFilterRuleDef, p.tcpHdrFilter.dataTCPOption), }, + COMMENT_PROP_IPHDR(tcpHdrFilter), { .name = NULL, } @@ -922,6 +967,7 @@ static const virXMLAttr2Struct tcpAttrib static const virXMLAttr2Struct udpAttributes[] = { COMMON_IP_PROPS(udpHdrFilter, DATATYPE_IPADDR, DATATYPE_IPMASK), COMMON_PORT_PROPS(udpHdrFilter), + COMMENT_PROP_IPHDR(udpHdrFilter), { .name = NULL, } @@ -929,6 +975,7 @@ static const virXMLAttr2Struct udpAttrib static const virXMLAttr2Struct udpliteAttributes[] = { COMMON_IP_PROPS(udpliteHdrFilter, DATATYPE_IPADDR, DATATYPE_IPMASK), + COMMENT_PROP_IPHDR(udpliteHdrFilter), { .name = NULL, } @@ -936,6 +983,7 @@ static const virXMLAttr2Struct udpliteAt static const virXMLAttr2Struct espAttributes[] = { COMMON_IP_PROPS(espHdrFilter, DATATYPE_IPADDR, DATATYPE_IPMASK), + COMMENT_PROP_IPHDR(espHdrFilter), { .name = NULL, } @@ -943,6 +991,7 @@ static const virXMLAttr2Struct espAttrib static const virXMLAttr2Struct ahAttributes[] = { COMMON_IP_PROPS(ahHdrFilter, DATATYPE_IPADDR, DATATYPE_IPMASK), + COMMENT_PROP_IPHDR(ahHdrFilter), { .name = NULL, } @@ -951,6 +1000,7 @@ static const virXMLAttr2Struct ahAttribu static const virXMLAttr2Struct sctpAttributes[] = { COMMON_IP_PROPS(sctpHdrFilter, DATATYPE_IPADDR, DATATYPE_IPMASK), COMMON_PORT_PROPS(sctpHdrFilter), + COMMENT_PROP_IPHDR(sctpHdrFilter), { .name = NULL, } @@ -969,6 +1019,7 @@ static const virXMLAttr2Struct icmpAttri .datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX, .dataIdx = offsetof(virNWFilterRuleDef, p.icmpHdrFilter.dataICMPCode), }, + COMMENT_PROP_IPHDR(icmpHdrFilter), { .name = NULL, } @@ -977,6 +1028,7 @@ static const virXMLAttr2Struct icmpAttri static const virXMLAttr2Struct allAttributes[] = { COMMON_IP_PROPS(allHdrFilter, DATATYPE_IPADDR, DATATYPE_IPMASK), + COMMENT_PROP_IPHDR(allHdrFilter), { .name = NULL, } @@ -985,6 +1037,7 @@ static const virXMLAttr2Struct allAttrib static const virXMLAttr2Struct igmpAttributes[] = { COMMON_IP_PROPS(igmpHdrFilter, DATATYPE_IPADDR, DATATYPE_IPMASK), + COMMENT_PROP_IPHDR(igmpHdrFilter), { .name = NULL, } @@ -999,6 +1052,7 @@ static const virXMLAttr2Struct tcpipv6At .datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX, .dataIdx = offsetof(virNWFilterRuleDef, p.tcpHdrFilter.dataTCPOption), }, + COMMENT_PROP_IPHDR(tcpHdrFilter), { .name = NULL, } @@ -1007,6 +1061,7 @@ static const virXMLAttr2Struct tcpipv6At static const virXMLAttr2Struct udpipv6Attributes[] = { COMMON_IP_PROPS(udpHdrFilter, DATATYPE_IPV6ADDR, DATATYPE_IPV6MASK), COMMON_PORT_PROPS(udpHdrFilter), + COMMENT_PROP_IPHDR(udpHdrFilter), { .name = NULL, } @@ -1015,6 +1070,7 @@ static const virXMLAttr2Struct udpipv6At static const virXMLAttr2Struct udpliteipv6Attributes[] = { COMMON_IP_PROPS(udpliteHdrFilter, DATATYPE_IPV6ADDR, DATATYPE_IPV6MASK), + COMMENT_PROP_IPHDR(udpliteHdrFilter), { .name = NULL, } @@ -1023,6 +1079,7 @@ static const virXMLAttr2Struct udpliteip static const virXMLAttr2Struct espipv6Attributes[] = { COMMON_IP_PROPS(espHdrFilter, DATATYPE_IPV6ADDR, DATATYPE_IPV6MASK), + COMMENT_PROP_IPHDR(espHdrFilter), { .name = NULL, } @@ -1031,6 +1088,7 @@ static const virXMLAttr2Struct espipv6At static const virXMLAttr2Struct ahipv6Attributes[] = { COMMON_IP_PROPS(ahHdrFilter, DATATYPE_IPV6ADDR, DATATYPE_IPV6MASK), + COMMENT_PROP_IPHDR(ahHdrFilter), { .name = NULL, } @@ -1040,6 +1098,7 @@ static const virXMLAttr2Struct ahipv6Att static const virXMLAttr2Struct sctpipv6Attributes[] = { COMMON_IP_PROPS(sctpHdrFilter, DATATYPE_IPV6ADDR, DATATYPE_IPV6MASK), COMMON_PORT_PROPS(sctpHdrFilter), + COMMENT_PROP_IPHDR(sctpHdrFilter), { .name = NULL, } @@ -1058,6 +1117,7 @@ static const virXMLAttr2Struct icmpv6Att .datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX, .dataIdx = offsetof(virNWFilterRuleDef, p.icmpHdrFilter.dataICMPCode), }, + COMMENT_PROP_IPHDR(icmpHdrFilter), { .name = NULL, } @@ -1066,6 +1126,7 @@ static const virXMLAttr2Struct icmpv6Att static const virXMLAttr2Struct allipv6Attributes[] = { COMMON_IP_PROPS(allHdrFilter, DATATYPE_IPV6ADDR, DATATYPE_IPV6MASK), + COMMENT_PROP_IPHDR(allHdrFilter), { .name = NULL, } @@ -1332,6 +1393,17 @@ virNWFilterRuleDetailsParse(xmlNodePtr n found = 1; break; + case DATATYPE_STRINGCOPY: + if (!(item->u.string = + virNWFilterRuleDefAddString(nwf, prop, + att[idx].maxstrlen))) { + rc = -1; + break; + } + data.c = item->u.string; + found = 1; + break; + case DATATYPE_LAST: default: break; @@ -2510,6 +2582,10 @@ virNWFilterRuleDefDetailsFormat(virBuffe (j < 5) ? ":" : ""); break; + case DATATYPE_STRINGCOPY: + virBufferEscapeString(buf, "%s", item->u.string); + break; + case DATATYPE_STRING: default: virBufferVSprintf(buf, Index: libvirt-acl/src/conf/nwfilter_conf.h =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.h +++ libvirt-acl/src/conf/nwfilter_conf.h @@ -73,6 +73,8 @@ enum virNWFilterEntryItemFlags { }; +# define MAX_COMMENT_LENGTH 256 + # define HAS_ENTRY_ITEM(data) \ (((data)->flags) & NWFILTER_ENTRY_ITEM_FLAG_EXISTS) @@ -92,8 +94,9 @@ enum attrDatatype { DATATYPE_STRING = (1 << 8), DATATYPE_IPV6ADDR = (1 << 9), DATATYPE_IPV6MASK = (1 << 10), + DATATYPE_STRINGCOPY = (1 << 11), - DATATYPE_LAST = (1 << 11), + DATATYPE_LAST = (1 << 12), }; @@ -123,6 +126,7 @@ struct _nwItemDesc { uint8_t u8; uint16_t u16; char protocolID[10]; + char *string; } u; }; @@ -142,6 +146,7 @@ typedef ethHdrFilterDef *ethHdrFilterDef struct _ethHdrFilterDef { ethHdrDataDef ethHdr; nwItemDesc dataProtocolID; + nwItemDesc dataComment; }; @@ -156,6 +161,7 @@ struct _arpHdrFilterDef { nwItemDesc dataARPSrcIPAddr; nwItemDesc dataARPDstMACAddr; nwItemDesc dataARPDstIPAddr; + nwItemDesc dataComment; }; @@ -174,6 +180,7 @@ struct _ipHdrDataDef { nwItemDesc dataDstIPTo; nwItemDesc dataDSCP; nwItemDesc dataConnlimitAbove; + nwItemDesc dataComment; }; @@ -376,6 +383,9 @@ struct _virNWFilterRuleDef { int nvars; char **vars; + + int nstrings; + char **strings; };
V3: - renamed IPTABLES_MAX_COMMENT_SIZE to IPTABLES_MAX_COMMENT_LENGTH - building comment string in separate variable -> introducing 'prefix' virBuffer where the comment is written into (if needed) V2: following Eric's comments: - commands in the script are assigned using cmd='...' rather than cmd="..." - invoke commands using eval res=... rather than res=eval ... - rewrote function escaping ` and single quotes (which became more tricky) In this patch I am extending the rule instantiator to create the comment node where supported, which is the case for iptables and ip6tables. Since commands are written in the format cmd='iptables ...-m comment --comment \"\" ' certain characters ('`) in the comment need to be escaped to prevent comments from becoming commands themselves or cause other forms of (bash) substitutions. I have tested this with various input and in my tests the input made it straight into the comment. A test case for TCK will be provided separately that tests this. Signed-off-by: Stefan Berger <stefanb@us.ibm.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 87 ++++++++++++++++++++++++------ src/nwfilter/nwfilter_ebiptables_driver.h | 2 2 files changed, 74 insertions(+), 15 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 @@ -23,6 +23,7 @@ #include <config.h> +#include <string.h> #include <sys/stat.h> #include <fcntl.h> @@ -52,10 +53,10 @@ #define CMD_SEPARATOR "\n" -#define CMD_DEF_PRE "cmd=\"" -#define CMD_DEF_POST "\"" +#define CMD_DEF_PRE "cmd='" +#define CMD_DEF_POST "'" #define CMD_DEF(X) CMD_DEF_PRE X CMD_DEF_POST -#define CMD_EXEC "res=`${cmd}`" CMD_SEPARATOR +#define CMD_EXEC "eval res=\\`\"${cmd}\"\\`" CMD_SEPARATOR #define CMD_STOPONERR(X) \ X ? "if [ $? -ne 0 ]; then" \ " echo \"Failure to execute command '${cmd}'.\";" \ @@ -106,6 +107,7 @@ static const char *m_physdev_out_str = " #define MATCH_PHYSDEV_IN m_physdev_in_str #define MATCH_PHYSDEV_OUT m_physdev_out_str +#define COMMENT_VARNAME "comment" static int ebtablesRemoveBasicRules(const char *ifname); static int ebiptablesDriverInit(void); @@ -292,6 +294,26 @@ printDataTypeAsHex(virNWFilterHashTableP static void +printCommentVar(virBufferPtr dest, const char *buf) +{ + size_t i, len = strlen(buf); + + virBufferAddLit(dest, COMMENT_VARNAME "='"); + + if (len > IPTABLES_MAX_COMMENT_LENGTH) + len = IPTABLES_MAX_COMMENT_LENGTH; + + for (i = 0; i < len; i++) { + if (buf[i] == '\'') + virBufferAddLit(dest, "'\\''"); + else + virBufferAddChar(dest, buf[i]); + } + virBufferAddLit(dest,"'" CMD_SEPARATOR); +} + + +static void ebiptablesRuleInstFree(ebiptablesRuleInstPtr inst) { if (!inst) @@ -846,7 +868,8 @@ iptablesHandleIpHdr(virBufferPtr buf, virNWFilterHashTablePtr vars, ipHdrDataDefPtr ipHdr, int directionIn, - bool *skipRule, bool *skipMatch) + bool *skipRule, bool *skipMatch, + virBufferPtr prefix) { char ipaddr[INET6_ADDRSTRLEN], number[20]; @@ -993,6 +1016,13 @@ iptablesHandleIpHdr(virBufferPtr buf, } } + if (HAS_ENTRY_ITEM(&ipHdr->dataComment)) { + printCommentVar(prefix, ipHdr->dataComment.u.string); + + virBufferAddLit(buf, + " -m comment --comment \"$" COMMENT_VARNAME "\""); + } + return 0; err_exit: @@ -1106,7 +1136,9 @@ _iptablesCreateRuleInstance(int directio { char chain[MAX_CHAINNAME_LENGTH]; char number[20]; + virBuffer prefix = VIR_BUFFER_INITIALIZER; virBuffer buf = VIR_BUFFER_INITIALIZER; + virBufferPtr final = NULL; const char *target; const char *iptables_cmd = (isIPv6) ? ip6tables_cmd_path : iptables_cmd_path; @@ -1148,7 +1180,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.tcpHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; if (iptablesHandlePortData(&buf, @@ -1193,7 +1226,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.udpHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; if (iptablesHandlePortData(&buf, @@ -1225,7 +1259,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.udpliteHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; break; @@ -1252,7 +1287,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.espHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; break; @@ -1279,7 +1315,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.ahHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; break; @@ -1306,7 +1343,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.sctpHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; if (iptablesHandlePortData(&buf, @@ -1341,7 +1379,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.icmpHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; if (HAS_ENTRY_ITEM(&rule->p.icmpHdrFilter.dataICMPType)) { @@ -1400,7 +1439,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.igmpHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; break; @@ -1427,7 +1467,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.allHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; break; @@ -1439,6 +1480,7 @@ _iptablesCreateRuleInstance(int directio if ((srcMacSkipped && bufUsed == virBufferUse(&buf)) || skipRule) { virBufferFreeAndReset(&buf); + virBufferFreeAndReset(&prefix); return 0; } @@ -1458,14 +1500,29 @@ _iptablesCreateRuleInstance(int directio CMD_EXEC, target); - if (virBufferError(&buf)) { + if (virBufferError(&buf) || virBufferError(&prefix)) { virBufferFreeAndReset(&buf); + virBufferFreeAndReset(&prefix); virReportOOMError(); return -1; } + if (virBufferUse(&prefix)) { + virBufferVSprintf(&prefix, "%s", virBufferContentAndReset(&buf)); + + final = &prefix; + + if (virBufferError(&prefix)) { + virBufferFreeAndReset(&prefix); + virReportOOMError(); + return -1; + } + } else + final = &buf; + + return ebiptablesAddRuleInst(res, - virBufferContentAndReset(&buf), + virBufferContentAndReset(final), nwfilter->chainsuffix, '\0', rule->priority, Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.h +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h @@ -45,4 +45,6 @@ extern virNWFilterTechDriver ebiptables_ # define EBIPTABLES_DRIVER_ID "ebiptables" +# define IPTABLES_MAX_COMMENT_LENGTH 256 + #endif
V3: - removed regular expression from comment attribute's string type - removal of 'interleave' from comment-attribute Extend the nwfilter.rng schema to accept comment attributes for all protocol types. Signed-off-by: Stefan Berger <stefanb@us.ibm.com> --- docs/schemas/nwfilter.rng | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) Index: libvirt-acl/docs/schemas/nwfilter.rng =================================================================== --- libvirt-acl.orig/docs/schemas/nwfilter.rng +++ libvirt-acl/docs/schemas/nwfilter.rng @@ -24,6 +24,7 @@ <ref name="match-attribute"/> <ref name="common-l2-attributes"/> <ref name="mac-attributes"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -33,6 +34,7 @@ <ref name="match-attribute"/> <ref name="common-l2-attributes"/> <ref name="arp-attributes"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -42,6 +44,7 @@ <ref name="match-attribute"/> <ref name="common-l2-attributes"/> <ref name="arp-attributes"/> <!-- same as arp --> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -54,6 +57,7 @@ <ref name="common-port-attributes"/> <ref name="ip-attributes"/> <ref name="dscp-attribute"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -65,6 +69,7 @@ <ref name="common-ipv6-attributes-p1"/> <ref name="common-port-attributes"/> <ref name="ip-attributes"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -76,6 +81,7 @@ <ref name="common-port-attributes"/> <ref name="common-ip-attributes-p1"/> <ref name="common-ip-attributes-p2"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -87,6 +93,7 @@ <ref name="common-port-attributes"/> <ref name="common-ip-attributes-p1"/> <ref name="common-ip-attributes-p2"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -98,6 +105,7 @@ <ref name="common-port-attributes"/> <ref name="common-ip-attributes-p1"/> <ref name="common-ip-attributes-p2"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -109,6 +117,7 @@ <ref name="common-ip-attributes-p1"/> <ref name="common-ip-attributes-p2"/> <ref name="icmp-attributes"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -119,6 +128,7 @@ <ref name="srcmac-attribute"/> <ref name="common-ip-attributes-p1"/> <ref name="common-ip-attributes-p2"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -129,6 +139,7 @@ <ref name="srcmac-attribute"/> <ref name="common-ip-attributes-p1"/> <ref name="common-ip-attributes-p2"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -139,6 +150,7 @@ <ref name="srcmac-attribute"/> <ref name="common-ip-attributes-p1"/> <ref name="common-ip-attributes-p2"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -149,6 +161,7 @@ <ref name="srcmac-attribute"/> <ref name="common-ip-attributes-p1"/> <ref name="common-ip-attributes-p2"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -159,6 +172,7 @@ <ref name="srcmac-attribute"/> <ref name="common-ip-attributes-p1"/> <ref name="common-ip-attributes-p2"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -170,6 +184,7 @@ <ref name="common-port-attributes"/> <ref name="common-ipv6-attributes-p1"/> <ref name="common-ipv6-attributes-p2"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -181,6 +196,7 @@ <ref name="common-port-attributes"/> <ref name="common-ipv6-attributes-p1"/> <ref name="common-ipv6-attributes-p2"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -192,6 +208,7 @@ <ref name="common-port-attributes"/> <ref name="common-ipv6-attributes-p1"/> <ref name="common-ipv6-attributes-p2"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -203,6 +220,7 @@ <ref name="common-ipv6-attributes-p1"/> <ref name="common-ipv6-attributes-p2"/> <ref name="icmp-attributes"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -213,6 +231,7 @@ <ref name="srcmac-attribute"/> <ref name="common-ipv6-attributes-p1"/> <ref name="common-ipv6-attributes-p2"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -223,6 +242,7 @@ <ref name="srcmac-attribute"/> <ref name="common-ipv6-attributes-p1"/> <ref name="common-ipv6-attributes-p2"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -233,6 +253,7 @@ <ref name="srcmac-attribute"/> <ref name="common-ipv6-attributes-p1"/> <ref name="common-ipv6-attributes-p2"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -243,6 +264,7 @@ <ref name="srcmac-attribute"/> <ref name="common-ipv6-attributes-p1"/> <ref name="common-ipv6-attributes-p2"/> + <ref name="comment-attribute"/> </element> </zeroOrMore> </optional> @@ -571,6 +593,14 @@ </optional> </define> + <define name="comment-attribute"> + <optional> + <attribute name="comment"> + <ref name="comment-type"/> + </attribute> + </optional> + </define> + <!-- ################ type library ################ --> <define name="UUID"> @@ -826,4 +856,8 @@ <param name="pattern">([Ff][Aa][Ll][Ss][Ee]|0)</param> </data> </define> + + <define name='comment-type'> + <data type="string"/> + </define> </grammar>
V3: - added '(Since 0.8.5)' I am adding a row with information about the newly supported comment attribute to each of the tables describing supported attributes of protocols. Signed-off-by: Stefan Berger <stefanb@us.ibm.com> --- docs/formatnwfilter.html.in | 51 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) Index: libvirt-acl/docs/formatnwfilter.html.in =================================================================== --- libvirt-acl.orig/docs/formatnwfilter.html.in +++ libvirt-acl/docs/formatnwfilter.html.in @@ -388,6 +388,11 @@ <td>UINT16 (0x600-0xffff), STRING</td> <td>Layer 3 protocol ID</td> </tr> + <tr> + <td>comment <span class="since">(Since 0.8.5)</span></td> + <td>STRING</td> + <td>text with max. 256 characters</td> + </tr> </table> <p> Valid Strings for <code>protocolid</code> are: arp, rarp, ipv4, ipv6 @@ -466,6 +471,11 @@ <td>IP_ADDR</td> <td>Destination IP address in ARP/RARP packet</td> </tr> + <tr> + <td>comment <span class="since">(Since 0.8.5)</span></td> + <td>STRING</td> + <td>text with max. 256 characters</td> + </tr> </table> <p> Valid strings for the <code>Opcode</code> field are: @@ -551,6 +561,11 @@ <td>UINT16</td> <td>End of range of valid destination ports; requires <code>protocol</code></td> </tr> + <tr> + <td>comment <span class="since">(Since 0.8.5)</span></td> + <td>STRING</td> + <td>text with max. 256 characters</td> + </tr> </table> <p> Valid strings for <code>protocol</code> are: @@ -636,6 +651,11 @@ <td>UINT16</td> <td>End of range of valid destination ports; requires <code>protocol</code></td> </tr> + <tr> + <td>comment <span class="since">(Since 0.8.5)</span></td> + <td>STRING</td> + <td>text with max. 256 characters</td> + </tr> </table> <p> Valid strings for <code>protocol</code> are: @@ -723,6 +743,11 @@ <td>UINT16</td> <td>End of range of valid destination ports</td> </tr> + <tr> + <td>comment <span class="since">(Since 0.8.5)</span></td> + <td>STRING</td> + <td>text with max. 256 characters</td> + </tr> </table> <p> <br><br> @@ -813,6 +838,11 @@ <td>UINT16</td> <td>ICMP code</td> </tr> + <tr> + <td>comment <span class="since">(Since 0.8.5)</span></td> + <td>STRING</td> + <td>text with max. 256 characters</td> + </tr> </table> <p> <br><br> @@ -892,6 +922,11 @@ <td>IP_ADDR</td> <td>End of range of destination IP address</td> </tr> + <tr> + <td>comment <span class="since">(Since 0.8.5)</span></td> + <td>STRING</td> + <td>text with max. 256 characters</td> + </tr> </table> <p> <br><br> @@ -978,6 +1013,11 @@ <td>UINT16</td> <td>End of range of valid destination ports</td> </tr> + <tr> + <td>comment <span class="since">(Since 0.8.5)</span></td> + <td>STRING</td> + <td>text with max. 256 characters</td> + </tr> </table> <p> <br><br> @@ -1054,6 +1094,11 @@ <td>UINT16</td> <td>ICMPv6 code</td> </tr> + <tr> + <td>comment <span class="since">(Since 0.8.5)</span></td> + <td>STRING</td> + <td>text with max. 256 characters</td> + </tr> </table> <p> <br><br> @@ -1118,7 +1163,11 @@ <td>IPV6_ADDR</td> <td>End of range of destination IP address</td> </tr> - + <tr> + <td>comment <span class="since">(Since 0.8.5)</span></td> + <td>STRING</td> + <td>text with max. 256 characters</td> + </tr> </table> <p> <br><br>
V3: - added some more test cases This patch adds a test case for testing the XML parser's and instantiator's support of the comment attribute. Signed-off-by: Stefan Berger <stefanb@us.ibm.com> --- tests/nwfilterxml2xmlin/comment-test.xml | 71 ++++++++++++++++++++++++++++++ tests/nwfilterxml2xmlout/comment-test.xml | 30 ++++++++++++ tests/nwfilterxml2xmltest.c | 2 3 files changed, 103 insertions(+) Index: libvirt-acl/tests/nwfilterxml2xmltest.c =================================================================== --- libvirt-acl.orig/tests/nwfilterxml2xmltest.c +++ libvirt-acl/tests/nwfilterxml2xmltest.c @@ -124,6 +124,8 @@ mymain(int argc, char **argv) DO_TEST("hex-data-test"); + DO_TEST("comment-test"); + return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); } Index: libvirt-acl/tests/nwfilterxml2xmlin/comment-test.xml =================================================================== --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlin/comment-test.xml @@ -0,0 +1,71 @@ +<filter name='testcase'> + <uuid>01a992d2-f8c8-7c27-f69b-ab0a9d377379</uuid> + + <rule action='accept' direction='in'> + <mac protocolid='0x1234' comment='mac rule'/> + </rule> + + <rule action='accept' direction='out'> + <ip 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' + srcipaddr='10.1.2.3' srcipmask='255.255.255.255' + dstipaddr='10.1.2.3' dstipmask='255.255.255.255' + protocol='udp' + srcportstart='0x123' srcportend='0x234' + dstportstart='0x3456' dstportend='0x4567' + dscp='0x32' comment='ip rule'/> + </rule> + + <rule action='accept' direction='out'> + <ipv6 srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:fe' + dstmacaddr='aa:bb:cc:dd:ee:ff' dstmacmask='ff:ff:ff:ff:ff:80' + srcipaddr='::10.1.2.3' srcipmask='22' + dstipaddr='::10.1.2.3' + dstipmask='ffff:ffff:ffff:ffff:ffff:ffff:ffff:8000' + protocol='tcp' + srcportstart='0x111' srcportend='400' + dstportstart='0x3333' dstportend='65535' comment='ipv6 rule'/> + </rule> + + <rule action='accept' direction='out'> + <arp 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' + hwtype='0x12' + protocoltype='0x56' + opcode='Request' + arpsrcmacaddr='1:2:3:4:5:6' + arpdstmacaddr='a:b:c:d:e:f' + comment='arp rule'/> + </rule> + + <rule action='accept' direction='out'> + <udp srcmacaddr='1:2:3:4:5:6' + dstipaddr='10.1.2.3' dstipmask='255.255.255.255' + dscp='0x22' + srcportstart='0x123' srcportend='400' + dstportstart='0x234' dstportend='0x444' + comment='udp rule'/> + </rule> + + <rule action='accept' direction='in'> + <tcp-ipv6 srcmacaddr='1:2:3:4:5:6' + srcipaddr='a:b:c::' srcipmask='128' + dscp='0x40' + srcportstart='0x20' srcportend='0x21' + dstportstart='0x100' dstportend='0x1111' + comment='tcp/ipv6 rule'/> + </rule> + + <rule action='accept' direction='in'> + <udp-ipv6 comment='`ls`;${COLUMNS};$(ls);"test";&'3 spaces''/> + </rule> + + <rule action='accept' direction='in'> + <sctp-ipv6 comment='comment with lone ', `, ", `, \, $x, and two spaces'/> + </rule> + + <rule action='accept' direction='in'> + <ah-ipv6 comment='tmp=`mktemp`; echo ${RANDOM} > ${tmp} ; cat < ${tmp}; rm -f ${tmp}'/> + </rule> + +</filter> Index: libvirt-acl/tests/nwfilterxml2xmlout/comment-test.xml =================================================================== --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlout/comment-test.xml @@ -0,0 +1,30 @@ +<filter name='testcase' chain='root'> + <uuid>01a992d2-f8c8-7c27-f69b-ab0a9d377379</uuid> + <rule action='accept' direction='in' priority='500'> + <mac protocolid='0x1234' comment='mac rule'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <ip 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' srcipaddr='10.1.2.3' srcipmask='32' dstipaddr='10.1.2.3' dstipmask='32' protocol='udp' srcportstart='0x123' srcportend='0x234' dstportstart='0x3456' dstportend='0x4567' dscp='0x32' comment='ip rule'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <ipv6 srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:fe' dstmacaddr='aa:bb:cc:dd:ee:ff' dstmacmask='ff:ff:ff:ff:ff:80' srcipaddr='::10.1.2.3' srcipmask='22' dstipaddr='::10.1.2.3' dstipmask='113' protocol='tcp' srcportstart='0x111' srcportend='400' dstportstart='0x3333' dstportend='65535' comment='ipv6 rule'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <arp 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' hwtype='0x12' protocoltype='0x56' opcode='Request' arpsrcmacaddr='01:02:03:04:05:06' arpdstmacaddr='0a:0b:0c:0d:0e:0f' comment='arp rule'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <udp srcmacaddr='01:02:03:04:05:06' dstipaddr='10.1.2.3' dstipmask='32' dscp='0x22' srcportstart='0x123' srcportend='400' dstportstart='0x234' dstportend='0x444' comment='udp rule'/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp-ipv6 srcmacaddr='01:02:03:04:05:06' srcipaddr='a:b:c::' srcipmask='128' srcportstart='0x20' srcportend='0x21' dstportstart='0x100' dstportend='0x1111' comment='tcp/ipv6 rule'/> + </rule> + <rule action='accept' direction='in' priority='500'> + <udp-ipv6 comment='`ls`;${COLUMNS};$(ls);"test";&'3 spaces''/> + </rule> + <rule action='accept' direction='in' priority='500'> + <sctp-ipv6 comment='comment with lone ', `, ", `, \, $x, and two spaces'/> + </rule> + <rule action='accept' direction='in' priority='500'> + <ah-ipv6 comment='tmp=`mktemp`; echo ${RANDOM} > ${tmp} ; cat < ${tmp}; rm -f ${tmp}'/> + </rule> +</filter>
On 09/30/2010 09:16 AM, Stefan Berger wrote:
V3: - moved MAX_COMMENT_LENGTH #define into include file (1/5) - renamed IPTABLES_MAX_COMMENT_SIZE to IPTABLES_MAX_COMMENT_LENGTH (2/5) - removal of regex from string patter in schema for comment attribute (3/5) - added libvirt version since when comment attribute is available (4/5) - added test cases (5/5)
V2: - work on the iptables instantiation patch (2/5) - work on the parser patch (1/5) - small changes to the test cases (5/5)
The following patch series adds support for a comment node to the XML attributes of all protocols. If possible, as for example in case of iptables, the comments are instantiated (iptables ... -m comment --comment ...).
The patches do the following: - extend the parser and XML generator to parse and create XML with the comment attribute - instantiate the comment in case of ip(6)tables - extend the nwfilter.rng schema with the comment attribute - add the information to the web docs - add a test case for the XML parser/generator to be run during 'make check'
ACK series - all of these look good to me now! -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 09/30/2010 03:40 PM, Eric Blake wrote:
On 09/30/2010 09:16 AM, Stefan Berger wrote:
V3: - moved MAX_COMMENT_LENGTH #define into include file (1/5) - renamed IPTABLES_MAX_COMMENT_SIZE to IPTABLES_MAX_COMMENT_LENGTH (2/5) - removal of regex from string patter in schema for comment attribute (3/5) - added libvirt version since when comment attribute is available (4/5) - added test cases (5/5)
V2: - work on the iptables instantiation patch (2/5) - work on the parser patch (1/5) - small changes to the test cases (5/5)
The following patch series adds support for a comment node to the XML attributes of all protocols. If possible, as for example in case of iptables, the comments are instantiated (iptables ... -m comment --comment ...).
The patches do the following: - extend the parser and XML generator to parse and create XML with the comment attribute - instantiate the comment in case of ip(6)tables - extend the nwfilter.rng schema with the comment attribute - add the information to the web docs - add a test case for the XML parser/generator to be run during 'make check'
ACK series - all of these look good to me now!
Pushed. Stefan
participants (3)
-
Eric Blake -
Stefan Berger -
Stefan Berger