[libvirt] [PATCH] nwfilter: Some fixes to XML parser

Stefan Berger stefanb at us.ibm.com
Wed Mar 31 20:45:04 UTC 2010


While writing a couple of test cases for the nwfilter's XML parser I
found some cases where the output ended up not looking as expected. So
the following changes are in the patch below:

- if the protocol ID in the MAC header is an integer, just write it into
the datastructure without trying to find a corresponding string for it
and if none is found failing
- when writing the protocol ID as string, simply write it as integer if
no corresponding string can be found
- same changes for arpOpcode parsing and printing
- same changes for protocol ID in an IP packet
- DSCP value needs to be written into the data structure
- IP protocol version number is redundant at this level, so remove it
- parse the protocol ID found inside an IP packet not only as string but
also as uint8
- arrange the display of the src and destination masks to be shown after
the src and destination ip address respectively in the XML
- the existing libvirt IP address parser accepts for example '25' as an
IP address. I want this to be parsed as a CIDR type netmask. So try to
parse it as an integer first (CIDR netmask) and if that doesn't work as
a dotted IP address style netmask.
- instantiation of rules with MAC masks didn't work because they weren't
printed into a buffer, yet.

Signed-off-by: Stefan Berger <stefanb at us.ibm.com>

---
 src/conf/nwfilter_conf.c                  |  128 ++++++++++++++----------------
 src/nwfilter/nwfilter_ebiptables_driver.c |    1 
 2 files changed, 61 insertions(+), 68 deletions(-)

Index: libvirt-acl/src/conf/nwfilter_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -403,15 +403,12 @@ checkMacProtocolID(enum attrDatatype dat
                    virNWFilterRuleDefPtr nwf ATTRIBUTE_UNUSED)
 {
     int32_t res = -1;
-    const char *str;
 
     if (datatype == DATATYPE_STRING) {
         if (intMapGetByString(macProtoMap, (char *)value, 1, &res) == 0)
             res = -1;
     } else if (datatype == DATATYPE_UINT16) {
-        if (intMapGetByInt(macProtoMap,
-                           (int32_t)*(uint16_t *)value, &str) == 0)
-            res = -1;
+        res = (uint32_t)*(uint16_t *)value;
     }
 
     if (res != -1) {
@@ -433,9 +430,10 @@ macProtocolIDFormatter(virBufferPtr buf,
                        nwf->p.ethHdrFilter.dataProtocolID.u.u16,
                        &str)) {
         virBufferVSprintf(buf, "%s", str);
-        return 1;
+    } else {
+        virBufferVSprintf(buf, "%d", nwf->p.ethHdrFilter.dataProtocolID.u.u16);
     }
-    return 0;
+    return 1;
 }
 
 
@@ -500,15 +498,12 @@ arpOpcodeValidator(enum attrDatatype dat
                    virNWFilterRuleDefPtr nwf)
 {
     int32_t res = -1;
-    const char *str;
 
     if (datatype == DATATYPE_STRING) {
         if (intMapGetByString(arpOpcodeMap, (char *)value, 1, &res) == 0)
             res = -1;
     } else if (datatype == DATATYPE_UINT16) {
-        if (intMapGetByInt(arpOpcodeMap,
-                           (uint32_t)*(uint16_t *)value, &str) == 0)
-            res = -1;
+        res = (uint32_t)*(uint16_t *)value;
     }
 
     if (res != -1) {
@@ -530,9 +525,10 @@ arpOpcodeFormatter(virBufferPtr buf,
                        nwf->p.arpHdrFilter.dataOpcode.u.u16,
                        &str)) {
         virBufferVSprintf(buf, "%s", str);
-        return 1;
+    } else {
+        virBufferVSprintf(buf, "%d", nwf->p.arpHdrFilter.dataOpcode.u.u16);
     }
-    return 0;
+    return 1;
 }
 
 
@@ -560,16 +556,12 @@ static bool checkIPProtocolID(enum attrD
                               virNWFilterRuleDefPtr nwf)
 {
     int32_t res = -1;
-    const char *str;
 
     if (datatype == DATATYPE_STRING) {
         if (intMapGetByString(ipProtoMap, (char *)value, 1, &res) == 0)
             res = -1;
     } else if (datatype == DATATYPE_UINT8) {
-        // may just accept what user provides and not test...
-        if (intMapGetByInt(ipProtoMap,
-                           (uint32_t)*(uint16_t *)value, &str) == 0)
-            res = -1;
+        res = (uint32_t)*(uint16_t *)value;
     }
 
     if (res != -1) {
@@ -591,19 +583,24 @@ formatIPProtocolID(virBufferPtr buf,
                        nwf->p.ipHdrFilter.ipHdr.dataProtocolID.u.u8,
                        &str)) {
         virBufferVSprintf(buf, "%s", str);
-        return 1;
+    } else {
+        virBufferVSprintf(buf, "%d",
+                          nwf->p.ipHdrFilter.ipHdr.dataProtocolID.u.u8);
     }
-    return 0;
+    return 1;
 }
 
 
 static bool
 dscpValidator(enum attrDatatype datatype ATTRIBUTE_UNUSED, void *val,
-              virNWFilterRuleDefPtr nwf ATTRIBUTE_UNUSED)
+              virNWFilterRuleDefPtr nwf)
 {
     uint8_t dscp = *(uint16_t *)val;
     if (dscp > 63)
         return 0;
+
+    nwf->p.ipHdrFilter.ipHdr.dataDSCP.u.u8 = dscp;
+
     return 1;
 }
 
@@ -685,11 +682,6 @@ static const virXMLAttr2Struct arpAttrib
 static const virXMLAttr2Struct ipAttributes[] = {
     COMMON_MAC_PROPS(ipHdrFilter),
     {
-        .name = "version",
-        .datatype = DATATYPE_UINT8,
-        .dataIdx = offsetof(virNWFilterRuleDef, p.ipHdrFilter.ipHdr.dataIPVersion),
-    },
-    {
         .name = SRCIPADDR,
         .datatype = DATATYPE_IPADDR,
         .dataIdx = offsetof(virNWFilterRuleDef, p.ipHdrFilter.ipHdr.dataSrcIPAddr),
@@ -711,7 +703,7 @@ static const virXMLAttr2Struct ipAttribu
     },
     {
         .name = "protocol",
-        .datatype = DATATYPE_STRING,
+        .datatype = DATATYPE_STRING | DATATYPE_UINT8,
         .dataIdx = offsetof(virNWFilterRuleDef, p.ipHdrFilter.ipHdr.dataProtocolID),
         .validator= checkIPProtocolID,
         .formatter= formatIPProtocolID,
@@ -756,16 +748,16 @@ static const virXMLAttr2Struct ipv6Attri
         .dataIdx = offsetof(virNWFilterRuleDef, p.ipv6HdrFilter.ipHdr.dataSrcIPAddr),
     },
     {
-        .name = DSTIPADDR,
-        .datatype = DATATYPE_IPV6ADDR,
-        .dataIdx = offsetof(virNWFilterRuleDef, p.ipv6HdrFilter.ipHdr.dataDstIPAddr),
-    },
-    {
         .name = SRCIPMASK,
         .datatype = DATATYPE_IPV6MASK,
         .dataIdx = offsetof(virNWFilterRuleDef, p.ipv6HdrFilter.ipHdr.dataSrcIPMask),
     },
     {
+        .name = DSTIPADDR,
+        .datatype = DATATYPE_IPV6ADDR,
+        .dataIdx = offsetof(virNWFilterRuleDef, p.ipv6HdrFilter.ipHdr.dataDstIPAddr),
+    },
+    {
         .name = DSTIPMASK,
         .datatype = DATATYPE_IPV6MASK,
         .dataIdx = offsetof(virNWFilterRuleDef, p.ipv6HdrFilter.ipHdr.dataDstIPMask),
@@ -818,16 +810,16 @@ static const virXMLAttr2Struct ipv6Attri
         .dataIdx = offsetof(virNWFilterRuleDef, p.STRUCT.ipHdr.dataSrcIPAddr),\
     },\
     {\
-        .name = DSTIPADDR,\
-        .datatype = ADDRTYPE,\
-        .dataIdx = offsetof(virNWFilterRuleDef, p.STRUCT.ipHdr.dataDstIPAddr),\
-    },\
-    {\
         .name = SRCIPMASK,\
         .datatype = MASKTYPE,\
         .dataIdx = offsetof(virNWFilterRuleDef, p.STRUCT.ipHdr.dataSrcIPMask),\
     },\
     {\
+        .name = DSTIPADDR,\
+        .datatype = ADDRTYPE,\
+        .dataIdx = offsetof(virNWFilterRuleDef, p.STRUCT.ipHdr.dataDstIPAddr),\
+    },\
+    {\
         .name = DSTIPMASK,\
         .datatype = MASKTYPE,\
         .dataIdx = offsetof(virNWFilterRuleDef, p.STRUCT.ipHdr.dataDstIPMask),\
@@ -1276,26 +1268,26 @@ virNWFilterRuleDetailsParse(virConnectPt
 
                         case DATATYPE_IPMASK:
                             storage_ptr = &item->u.u8;
-                            if (!virNWIPv4AddressParser(prop, &ipaddr)) {
-                                if (sscanf(prop, "%d", &int_val) == 1) {
-                                    if (int_val >= 0 && int_val <= 32) {
-                                        if (!validator)
-                                            *(uint8_t *)storage_ptr =
-                                                   (uint8_t)int_val;
-                                        found = 1;
-                                        data_ptr = &int_val;
-                                    } else
-                                        rc = -1;
+                            if (virStrToLong_i(prop, NULL, 10, &int_val) == 0) {
+                                if (int_val >= 0 && int_val <= 32) {
+                                    if (!validator)
+                                        *(uint8_t *)storage_ptr =
+                                               (uint8_t)int_val;
+                                    found = 1;
+                                    data_ptr = &int_val;
                                 } else
                                     rc = -1;
                             } else {
-                                int_val = virSocketGetNumNetmaskBits(
+                                if (virNWIPv4AddressParser(prop, &ipaddr)) {
+                                    int_val = virSocketGetNumNetmaskBits(
                                                      &ipaddr.addr);
-                                if (int_val >= 0)
-                                    *(uint8_t *)storage_ptr = int_val;
-                                else
+                                    if (int_val >= 0)
+                                        *(uint8_t *)storage_ptr = int_val;
+                                    else
+                                        rc = -1;
+                                    found = 1;
+                                } else
                                     rc = -1;
-                                found = 1;
                             }
                         break;
 
@@ -1330,26 +1322,26 @@ virNWFilterRuleDetailsParse(virConnectPt
 
                         case DATATYPE_IPV6MASK:
                             storage_ptr = &item->u.u8;
-                            if (!virNWIPv6AddressParser(prop, &ipaddr)) {
-                                if (sscanf(prop, "%d", &int_val) == 1) {
-                                    if (int_val >= 0 && int_val <= 128) {
-                                        if (!validator)
-                                            *(uint8_t *)storage_ptr =
-                                                   (uint8_t)int_val;
-                                        found = 1;
-                                        data_ptr = &int_val;
-                                    } else
-                                        rc = -1;
+                            if (virStrToLong_i(prop, NULL, 10, &int_val) == 0) {
+                                if (int_val >= 0 && int_val <= 128) {
+                                    if (!validator)
+                                        *(uint8_t *)storage_ptr =
+                                               (uint8_t)int_val;
+                                    found = 1;
+                                    data_ptr = &int_val;
                                 } else
                                     rc = -1;
                             } else {
-                                int_val = virSocketGetNumNetmaskBits(
-                                                     &ipaddr.addr);
-                                if (int_val >= 0)
-                                    *(uint8_t *)storage_ptr = int_val;
-                                else
-                                    rc = -1;
-                                found = 1;
+                                if (virNWIPv6AddressParser(prop, &ipaddr)) {
+                                    int_val = virSocketGetNumNetmaskBits(
+                                                  &ipaddr.addr);
+                                    if (int_val >= 0)
+                                        *(uint8_t *)storage_ptr = int_val;
+                                    else
+                                        rc = -1;
+                                    found = 1;
+                                } else
+                                   rc = -1;
                             }
                         break;
 
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
@@ -189,6 +189,7 @@ printDataType(virConnectPtr conn,
     break;
 
     case DATATYPE_MACADDR:
+    case DATATYPE_MACMASK:
         if (bufsize < VIR_MAC_STRING_BUFLEN) {
             virNWFilterReportError(conn, VIR_ERR_INVALID_NWFILTER,
                                    _("Buffer too small for MAC address"));




More information about the libvir-list mailing list