The following issues are fixed in the patch below:

- ebtables requires that some of the command line parameters are passed as hex numbers; so have those attributes call a function that prints 16 and 8 bit integers as hex nunbers.

- ip6tables requires '--icmpv6-type' rather than '--icmp-type'

- ebtables complains about protocol identifiers lower than 0x600, so already discard anything lower than 0x600 in the parser

- make the protocol entry types more readable using a #define for its entries

- continue parsing a filtering rule even if a faulty entry is encountered; return an error value at the end and let the caller decide what to do with the rule's object

- fix an error message

src/conf/nwfilter_conf.c

@@ -409,6 +409,8 @@ checkMacProtocolID(enum attrDatatype datatype, void *value,
             res = -1;
     } else if (datatype == DATATYPE_UINT16) {
         res = (uint32_t)*(uint16_t *)value;
+        if (res < 0x600)
+            res = -1;
     }
 
     if (res != -1) {
@@ -766,7 +768,7 @@ static const virXMLAttr2Struct ipv6Attributes[] = {
     },
     {
         .name = "protocol",
-        .datatype = DATATYPE_STRING,
+        .datatype = DATATYPE_STRING | DATATYPE_UINT8,
         .dataIdx = offsetof(virNWFilterRuleDef, p.ipv6HdrFilter.ipHdr.dataProtocolID),
         .validator= checkIPProtocolID,
         .formatter= formatIPProtocolID,
@@ -1048,95 +1050,34 @@ struct _virAttributes {
     enum virNWFilterRuleProtocolType prtclType;
 };
 
+#define PROTOCOL_ENTRY(ID, ATT, PRTCLTYPE) \
+    { .id = ID, .att = ATT, .prtclType = PRTCLTYPE }
+#define PROTOCOL_ENTRY_LAST { .id = NULL }
+
 
 static const virAttributes virAttr[] = {
-    {
-        .id = "arp",
-        .att = arpAttributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_ARP,
-    }, {
-        .id = "mac",
-        .att = macAttributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_MAC,
-    }, {
-        .id = "ip",
-        .att = ipAttributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_IP,
-    }, {
-        .id = "ipv6",
-        .att = ipv6Attributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_IPV6,
-    }, {
-        .id = "tcp",
-        .att = tcpAttributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_TCP,
-    }, {
-        .id = "udp",
-        .att = udpAttributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_UDP,
-    }, {
-        .id = "udplite",
-        .att = udpliteAttributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_UDPLITE,
-    }, {
-        .id = "esp",
-        .att = espAttributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_ESP,
-    }, {
-        .id = "ah",
-        .att = ahAttributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_AH,
-    }, {
-        .id = "sctp",
-        .att = sctpAttributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_SCTP,
-    }, {
-        .id = "icmp",
-        .att = icmpAttributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_ICMP,
-    }, {
-        .id = "all", // = 'any'
-        .att = allAttributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_ALL,
-    }, {
-        .id = "igmp",
-        .att = igmpAttributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_IGMP,
-    }, {
-        .id = "tcp-ipv6",
-        .att = tcpipv6Attributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_TCPoIPV6,
-    }, {
-        .id = "udp-ipv6",
-        .att = udpipv6Attributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_UDPoIPV6,
-    }, {
-        .id = "udplite-ipv6",
-        .att = udpliteipv6Attributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_UDPLITEoIPV6,
-    }, {
-        .id = "esp-ipv6",
-        .att = espipv6Attributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_ESPoIPV6,
-    }, {
-        .id = "ah-ipv6",
-        .att = ahipv6Attributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_AHoIPV6,
-    }, {
-        .id = "sctp-ipv6",
-        .att = sctpipv6Attributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_SCTPoIPV6,
-    }, {
-        .id = "icmpv6",
-        .att = icmpv6Attributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_ICMPV6,
-    }, {
-        .id = "all-ipv6", // = 'any'
-        .att = allipv6Attributes,
-        .prtclType = VIR_NWFILTER_RULE_PROTOCOL_ALLoIPV6,
-    }, {
-        .id = NULL,
-    }
+    PROTOCOL_ENTRY("arp"    , arpAttributes    , VIR_NWFILTER_RULE_PROTOCOL_ARP),
+    PROTOCOL_ENTRY("mac"    , macAttributes    , VIR_NWFILTER_RULE_PROTOCOL_MAC),
+    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),
+    PROTOCOL_ENTRY("udp"    , udpAttributes    , VIR_NWFILTER_RULE_PROTOCOL_UDP),
+    PROTOCOL_ENTRY("udplite", udpliteAttributes, VIR_NWFILTER_RULE_PROTOCOL_UDPLITE),
+    PROTOCOL_ENTRY("esp"    , espAttributes    , VIR_NWFILTER_RULE_PROTOCOL_ESP),
+    PROTOCOL_ENTRY("ah"     , ahAttributes     , VIR_NWFILTER_RULE_PROTOCOL_AH),
+    PROTOCOL_ENTRY("sctp"   , sctpAttributes   , VIR_NWFILTER_RULE_PROTOCOL_SCTP),
+    PROTOCOL_ENTRY("icmp"   , icmpAttributes   , VIR_NWFILTER_RULE_PROTOCOL_ICMP),
+    PROTOCOL_ENTRY("all"    , allAttributes    , VIR_NWFILTER_RULE_PROTOCOL_ALL),
+    PROTOCOL_ENTRY("igmp"   , igmpAttributes   , VIR_NWFILTER_RULE_PROTOCOL_IGMP),
+    PROTOCOL_ENTRY("tcp-ipv6"    , tcpipv6Attributes    , VIR_NWFILTER_RULE_PROTOCOL_TCPoIPV6),
+    PROTOCOL_ENTRY("udp-ipv6"    , udpipv6Attributes    , VIR_NWFILTER_RULE_PROTOCOL_UDPoIPV6),
+    PROTOCOL_ENTRY("udplite-ipv6", udpliteipv6Attributes, VIR_NWFILTER_RULE_PROTOCOL_UDPLITEoIPV6),
+    PROTOCOL_ENTRY("esp-ipv6"    , espipv6Attributes    , VIR_NWFILTER_RULE_PROTOCOL_ESPoIPV6),
+    PROTOCOL_ENTRY("ah-ipv6"     , ahipv6Attributes     , VIR_NWFILTER_RULE_PROTOCOL_AHoIPV6),
+    PROTOCOL_ENTRY("sctp-ipv6"   , sctpipv6Attributes   , VIR_NWFILTER_RULE_PROTOCOL_SCTPoIPV6),
+    PROTOCOL_ENTRY("icmpv6"      , icmpv6Attributes     , VIR_NWFILTER_RULE_PROTOCOL_ICMPV6),
+    PROTOCOL_ENTRY("all-ipv6"    , allipv6Attributes    , VIR_NWFILTER_RULE_PROTOCOL_ALLoIPV6),
+    PROTOCOL_ENTRY_LAST
 };
 
 
@@ -1176,7 +1117,7 @@ virNWFilterRuleDetailsParse(virConnectPtr conn ATTRIBUTE_UNUSED,
                             virNWFilterRuleDefPtr nwf,
                             const virXMLAttr2Struct *att)
 {
-    int rc = 0;
+    int rc = 0, g_rc = 0;
     int idx = 0;
     char *prop;
     int found = 0;
@@ -1194,7 +1135,7 @@ virNWFilterRuleDetailsParse(virConnectPtr conn ATTRIBUTE_UNUSED,
     VIR_FREE(match);
     match = NULL;
 
-    while (att[idx].name != NULL && rc == 0) {
+    while (att[idx].name != NULL) {
         prop = virXMLPropString(node, att[idx].name);
 
         item = (nwItemDesc *)((char *)nwf + att[idx].dataIdx);
@@ -1390,10 +1331,16 @@ virNWFilterRuleDetailsParse(virConnectPtr conn ATTRIBUTE_UNUSED,
             }
             VIR_FREE(prop);
         }
+
+        if (rc) {
+            g_rc = rc;
+            rc = 0;
+        }
+
         idx++;
     }
 
-    return rc;
+    return g_rc;
 }
 
 
@@ -2178,7 +2125,7 @@ virNWFilterPoolObjAssignDef(virConnectPtr conn,
 
     if (virNWFilterDefLoopDetect(conn, pools, def)) {
         virNWFilterReportError(conn, VIR_ERR_INVALID_NWFILTER,
-                              "%s", _("filter would introduce loop"));
+                              "%s", _("filter would introduce a loop"));
         return NULL;
     }
 

src/nwfilter/nwfilter_ebiptables_driver.c

@@ -138,10 +138,11 @@ printVar(virConnectPtr conn,
 
 
 static int
-printDataType(virConnectPtr conn,
-              virNWFilterHashTablePtr vars,
-              char *buf, int bufsize,
-              nwItemDescPtr item)
+_printDataType(virConnectPtr conn,
+               virNWFilterHashTablePtr vars,
+               char *buf, int bufsize,
+               nwItemDescPtr item,
+               bool asHex)
 {
     int done;
     char *data;
@@ -199,8 +200,18 @@ printDataType(virConnectPtr conn,
         virFormatMacAddr(item->u.macaddr.addr, buf);
     break;
 
-    case DATATYPE_UINT16:
+    case DATATYPE_IPV6MASK:
+    case DATATYPE_IPMASK:
         if (snprintf(buf, bufsize, "%d",
+                     item->u.u8) >= bufsize) {
+            virNWFilterReportError(conn, VIR_ERR_INVALID_NWFILTER,
+                                   _("Buffer too small for uint8 type"));
+            return 1;
+        }
+    break;
+
+    case DATATYPE_UINT16:
+        if (snprintf(buf, bufsize, asHex ? "0x%x" : "%d",
                      item->u.u16) >= bufsize) {
             virNWFilterReportError(conn, VIR_ERR_INVALID_NWFILTER, "%s",
                                    _("Buffer too small for uint16 type"));
@@ -208,10 +219,8 @@ printDataType(virConnectPtr conn,
         }
     break;
 
-    case DATATYPE_IPV6MASK:
-    case DATATYPE_IPMASK:
     case DATATYPE_UINT8:
-        if (snprintf(buf, bufsize, "%d",
+        if (snprintf(buf, bufsize, asHex ? "0x%x" : "%d",
                      item->u.u8) >= bufsize) {
             virNWFilterReportError(conn, VIR_ERR_INVALID_NWFILTER, "%s",
                                    _("Buffer too small for uint8 type"));
@@ -230,6 +239,26 @@ printDataType(virConnectPtr conn,
 }
 
 
+static int
+printDataType(virConnectPtr conn,
+              virNWFilterHashTablePtr vars,
+              char *buf, int bufsize,
+              nwItemDescPtr item)
+{
+    return _printDataType(conn, vars, buf, bufsize, item, 0);
+}
+
+
+static int
+printDataTypeAsHex(virConnectPtr conn,
+                   virNWFilterHashTablePtr vars,
+                   char *buf, int bufsize,
+                   nwItemDescPtr item)
+{
+    return _printDataType(conn, vars, buf, bufsize, item, 1);
+}
+
+
 static void
 ebiptablesRuleInstFree(ebiptablesRuleInstPtr inst)
 {
@@ -1270,6 +1299,12 @@ _iptablesCreateRuleInstance(virConnectPtr conn,
             goto err_exit;
 
         if (HAS_ENTRY_ITEM(&rule->p.icmpHdrFilter.dataICMPType)) {
+            const char *parm;
+            if (rule->prtclType == VIR_NWFILTER_RULE_PROTOCOL_ICMP)
+                parm = "--icmp-type";
+            else
+                parm = "--icmpv6-type";
+
             if (printDataType(conn,
                               vars,
                               number, sizeof(number),
@@ -1277,8 +1312,9 @@ _iptablesCreateRuleInstance(virConnectPtr conn,
                 goto err_exit;
 
             virBufferVSprintf(&buf,
-                      " %s --icmp-type %s",
+                      " %s %s %s",
                       ENTRY_GET_NEG_SIGN(&rule->p.icmpHdrFilter.dataICMPType),
+                      parm,
                       number);
 
             if (HAS_ENTRY_ITEM(&rule->p.icmpHdrFilter.dataICMPCode)) {
@@ -1295,6 +1331,30 @@ _iptablesCreateRuleInstance(virConnectPtr conn,
         }
     break;
 
+    case VIR_NWFILTER_RULE_PROTOCOL_IGMP:
+        virBufferVSprintf(&buf,
+                          CMD_DEF_PRE "%s -%%c %s %%s",
+                          iptables_cmd,
+                          chain);
+
+        virBufferAddLit(&buf, " -p igmp");
+
+        if (iptablesHandleSrcMacAddr(conn,
+                                     &buf,
+                                     vars,
+                                     &rule->p.igmpHdrFilter.dataSrcMACAddr,
+                                     directionIn))
+            goto err_exit;
+
+        if (iptablesHandleIpHdr(conn,
+                                &buf,
+                                vars,
+                                &rule->p.igmpHdrFilter.ipHdr,
+                                directionIn))
+            goto err_exit;
+
+    break;
+
     case VIR_NWFILTER_RULE_PROTOCOL_ALL:
     case VIR_NWFILTER_RULE_PROTOCOL_ALLoIPV6:
         virBufferVSprintf(&buf,
@@ -1490,10 +1550,10 @@ ebtablesCreateRuleInstance(virConnectPtr conn,
             goto err_exit;
 
         if (HAS_ENTRY_ITEM(&rule->p.ethHdrFilter.dataProtocolID)) {
-            if (printDataType(conn,
-                              vars,
-                              number, sizeof(number),
-                              &rule->p.ethHdrFilter.dataProtocolID))
+            if (printDataTypeAsHex(conn,
+                                   vars,
+                                   number, sizeof(number),
+                                   &rule->p.ethHdrFilter.dataProtocolID))
                 goto err_exit;
             virBufferVSprintf(&buf,
                           " -p %s %s",
@@ -1541,10 +1601,10 @@ ebtablesCreateRuleInstance(virConnectPtr conn,
         }
 
         if (HAS_ENTRY_ITEM(&rule->p.arpHdrFilter.dataProtocolType)) {
-            if (printDataType(conn,
-                              vars,
-                              number, sizeof(number),
-                              &rule->p.arpHdrFilter.dataProtocolType))
+            if (printDataTypeAsHex(conn,
+                                   vars,
+                                   number, sizeof(number),
+                                   &rule->p.arpHdrFilter.dataProtocolType))
                 goto err_exit;
             virBufferVSprintf(&buf,
                           " --arp-ptype %s %s",