[PATCH 0 of 4] Various fixes to FilterEntry provider

[1/4] acl_parsing: Share code for icmp and icmp rule types The struct for both ICMP and IGMP rule types have the exact same fields. With this patch, we avoid duplicating code to handle those rules. [2/4] FilterEntry: Should be using srcipaddr instead of srcmacaddr [3/4] FilterEntry: Support for mask in CIDR notation The values for mask fields may have been written using the CIDR notation[1]. For instance, take the libvirt 'no-ip-multicast' builtin filter: <filter name='no-ip-multicast' chain='ipv4'> <uuid>47756f11-6057-1448-2cce-fda40fa23ba4</uuid> <rule action='drop' direction='out' priority='500'> <ip dstipaddr='224.0.0.0' dstipmask='4'/> </rule> </filter> As libvirt-cim expects an address like string, for the mask, in this case the conversion will fail and will output an array with only zero values [0,0,0,0], when it actually should be [240,0,0,0]. [1] http://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing [4/4] FilterEntry: Fix behavior of convert_ip_rule_to_instance The function was always referencing the 'tcp' fileld of the var union in struct acl_rule, while it should take into account the rule type to access the correct fields. This could probably go to acl_parsing, but I thought it would be a less intrusive change to patch only FilterEntry provider. Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> libxkutil/acl_parsing.c | 119 ++++++++++------------------------ libxkutil/acl_parsing.h | 29 +------- src/Virt_FilterEntry.c | 3 +- src/Virt_FilterEntry.c | 2 +- src/Virt_FilterEntry.c | 88 +++++++++++++++++++++---- src/Virt_FilterEntry.c | 162 +++++++++++++++++++++++++++++++++++++---------- 6 files changed, 242 insertions(+), 161 deletions(-)

libxkutil/acl_parsing.c | 119 ++++++++++++++--------------------------------- libxkutil/acl_parsing.h | 29 +---------- src/Virt_FilterEntry.c | 3 +- 3 files changed, 41 insertions(+), 110 deletions(-) # HG changeset patch # User Eduardo Lima (Etrunko) <eblima@br.ibm.com> # Date 1317840215 10800 # Node ID fa576f11a652c4632afb62c687707effb2e98a80 # Parent 1f93220799a57477a6b1e04a90e8ffe797d85581 acl_parsing: Share code for icmp and icmp rule types The struct for both ICMP and IGMP rule types have the exact same fields. With this patch, we avoid duplicating code to handle those rules. Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> diff --git a/libxkutil/acl_parsing.c b/libxkutil/acl_parsing.c --- a/libxkutil/acl_parsing.c +++ b/libxkutil/acl_parsing.c @@ -97,41 +97,23 @@ free(rule->var.tcp.comment); free(rule->var.tcp.state); break; - case IGMP_RULE: - free(rule->var.igmp.srcmacaddr); - free(rule->var.igmp.srcmacmask); - free(rule->var.igmp.dstmacaddr); - free(rule->var.igmp.dstmacmask); - free(rule->var.igmp.srcipaddr); - free(rule->var.igmp.srcipmask); - free(rule->var.igmp.dstipaddr); - free(rule->var.igmp.dstipmask); - free(rule->var.igmp.srcipfrom); - free(rule->var.igmp.srcipto); - free(rule->var.igmp.dstipfrom); - free(rule->var.igmp.dstipto); - free(rule->var.igmp.type); - free(rule->var.igmp.code); - free(rule->var.igmp.comment); - free(rule->var.igmp.state); - break; - case ICMP_RULE: - free(rule->var.icmp.srcmacaddr); - free(rule->var.icmp.srcmacmask); - free(rule->var.icmp.dstmacaddr); - free(rule->var.icmp.dstmacmask); - free(rule->var.icmp.srcipaddr); - free(rule->var.icmp.srcipmask); - free(rule->var.icmp.dstipaddr); - free(rule->var.icmp.dstipmask); - free(rule->var.icmp.srcipfrom); - free(rule->var.icmp.srcipto); - free(rule->var.icmp.dstipfrom); - free(rule->var.icmp.dstipto); - free(rule->var.icmp.type); - free(rule->var.icmp.code); - free(rule->var.icmp.comment); - free(rule->var.icmp.state); + case ICMP_IGMP_RULE: + free(rule->var.icmp_igmp.srcmacaddr); + free(rule->var.icmp_igmp.srcmacmask); + free(rule->var.icmp_igmp.dstmacaddr); + free(rule->var.icmp_igmp.dstmacmask); + free(rule->var.icmp_igmp.srcipaddr); + free(rule->var.icmp_igmp.srcipmask); + free(rule->var.icmp_igmp.dstipaddr); + free(rule->var.icmp_igmp.dstipmask); + free(rule->var.icmp_igmp.srcipfrom); + free(rule->var.icmp_igmp.srcipto); + free(rule->var.icmp_igmp.dstipfrom); + free(rule->var.icmp_igmp.dstipto); + free(rule->var.icmp_igmp.type); + free(rule->var.icmp_igmp.code); + free(rule->var.icmp_igmp.comment); + free(rule->var.icmp_igmp.state); break; case UNKNOWN_RULE: default: @@ -265,50 +247,25 @@ return 1; } -static int parse_acl_icmp_rule(xmlNode *rnode, struct acl_rule *rule) +static int parse_acl_icmp_igmp_rule(xmlNode *rnode, struct acl_rule *rule) { - CU_DEBUG("ACL icmp rule %s", rnode->name); + CU_DEBUG("ACL %s rule %s", rule->protocol_id, rnode->name); - rule->type = ICMP_RULE; - rule->var.icmp.srcmacaddr = get_attr_value(rnode, "srcmacaddr"); - rule->var.icmp.srcmacmask = get_attr_value(rnode, "srcmacmask"); - rule->var.icmp.dstmacaddr = get_attr_value(rnode, "dstmacaddr"); - rule->var.icmp.dstmacmask = get_attr_value(rnode, "dstmacmask"); - rule->var.icmp.srcipaddr = get_attr_value(rnode, "srcipaddr"); - rule->var.icmp.srcipmask = get_attr_value(rnode, "srcipmask"); - rule->var.icmp.dstipaddr = get_attr_value(rnode, "dstipaddr"); - rule->var.icmp.dstipmask = get_attr_value(rnode, "dstipmask"); - rule->var.icmp.srcipfrom = get_attr_value(rnode, "srcipfrom"); - rule->var.icmp.srcipto = get_attr_value(rnode, "srcipto"); - rule->var.icmp.dstipfrom = get_attr_value(rnode, "dstipfrom"); - rule->var.icmp.dstipto = get_attr_value(rnode, "dstipto"); - rule->var.icmp.comment = get_attr_value(rnode, "comment"); - rule->var.icmp.state = get_attr_value(rnode, "state"); - - return 1; -} - -static int parse_acl_igmp_rule(xmlNode *rnode, struct acl_rule *rule) -{ - CU_DEBUG("ACL igmp rule %s", rnode->name); - - rule->type = IGMP_RULE; - rule->var.igmp.srcmacaddr = get_attr_value(rnode, "srcmacaddr"); - rule->var.igmp.srcmacmask = get_attr_value(rnode, "srcmacmask"); - rule->var.igmp.dstmacaddr = get_attr_value(rnode, "dstmacaddr"); - rule->var.igmp.dstmacmask = get_attr_value(rnode, "dstmacmask"); - rule->var.igmp.srcipaddr = get_attr_value(rnode, "srcipaddr"); - rule->var.igmp.srcipmask = get_attr_value(rnode, "srcipmask"); - rule->var.igmp.dstipaddr = get_attr_value(rnode, "dstipaddr"); - rule->var.igmp.dstipmask = get_attr_value(rnode, "dstipmask"); - rule->var.igmp.srcipfrom = get_attr_value(rnode, "srcipfrom"); - rule->var.igmp.srcipto = get_attr_value(rnode, "srcipto"); - rule->var.igmp.dstipfrom = get_attr_value(rnode, "dstipfrom"); - rule->var.igmp.dstipto = get_attr_value(rnode, "dstipto"); - rule->var.igmp.type = get_attr_value(rnode, "type"); - rule->var.igmp.code = get_attr_value(rnode, "code"); - rule->var.igmp.comment = get_attr_value(rnode, "comment"); - rule->var.igmp.state = get_attr_value(rnode, "state"); + rule->type = ICMP_IGMP_RULE; + rule->var.icmp_igmp.srcmacaddr = get_attr_value(rnode, "srcmacaddr"); + rule->var.icmp_igmp.srcmacmask = get_attr_value(rnode, "srcmacmask"); + rule->var.icmp_igmp.dstmacaddr = get_attr_value(rnode, "dstmacaddr"); + rule->var.icmp_igmp.dstmacmask = get_attr_value(rnode, "dstmacmask"); + rule->var.icmp_igmp.srcipaddr = get_attr_value(rnode, "srcipaddr"); + rule->var.icmp_igmp.srcipmask = get_attr_value(rnode, "srcipmask"); + rule->var.icmp_igmp.dstipaddr = get_attr_value(rnode, "dstipaddr"); + rule->var.icmp_igmp.dstipmask = get_attr_value(rnode, "dstipmask"); + rule->var.icmp_igmp.srcipfrom = get_attr_value(rnode, "srcipfrom"); + rule->var.icmp_igmp.srcipto = get_attr_value(rnode, "srcipto"); + rule->var.icmp_igmp.dstipfrom = get_attr_value(rnode, "dstipfrom"); + rule->var.icmp_igmp.dstipto = get_attr_value(rnode, "dstipto"); + rule->var.icmp_igmp.comment = get_attr_value(rnode, "comment"); + rule->var.icmp_igmp.state = get_attr_value(rnode, "state"); return 1; } @@ -351,10 +308,8 @@ rule->protocol_id = strdup((char *)child->name); parse_acl_tcp_rule(child, rule); } else if (XSTREQ(child->name, "icmp") || - XSTREQ(child->name, "icmpv6")) { - rule->protocol_id = strdup((char *)child->name); - parse_acl_icmp_rule(child, rule); - } else if (XSTREQ(child->name, "igmp") || + XSTREQ(child->name, "icmpv6") || + XSTREQ(child->name, "igmp") || XSTREQ(child->name, "igmp-ipv6") || XSTREQ(child->name, "esp") || XSTREQ(child->name, "esp-ipv6") || @@ -365,7 +320,7 @@ XSTREQ(child->name, "all") || XSTREQ(child->name, "all-ipv6")) { rule->protocol_id = strdup((char *)child->name); - parse_acl_igmp_rule(child, rule); + parse_acl_icmp_igmp_rule(child, rule); } } diff --git a/libxkutil/acl_parsing.h b/libxkutil/acl_parsing.h --- a/libxkutil/acl_parsing.h +++ b/libxkutil/acl_parsing.h @@ -95,7 +95,7 @@ char *state; }; -struct acl_igmp_rule { +struct acl_icmp_igmp_rule { char *srcmacaddr; char *srcmacmask; char *dstmacaddr; @@ -117,27 +117,6 @@ char *state; }; -struct acl_icmp_rule { - char *srcmacaddr; - char *srcmacmask; - char *dstmacaddr; - char *dstmacmask; - - char *srcipaddr; - char *srcipmask; - char *dstipaddr; - char *dstipmask; - - char *srcipfrom; - char *srcipto; - char *dstipfrom; - char *dstipto; - - char *type; - char *code; - char *comment; - char *state; -}; struct acl_rule { char *name; @@ -153,8 +132,7 @@ ARP_RULE, IP_RULE, TCP_RULE, - ICMP_RULE, - IGMP_RULE + ICMP_IGMP_RULE, } type; union { @@ -162,8 +140,7 @@ struct acl_arp_rule arp; struct acl_ip_rule ip; struct acl_tcp_rule tcp; - struct acl_icmp_rule icmp; - struct acl_igmp_rule igmp; + struct acl_icmp_igmp_rule icmp_igmp; } var; }; diff --git a/src/Virt_FilterEntry.c b/src/Virt_FilterEntry.c --- a/src/Virt_FilterEntry.c +++ b/src/Virt_FilterEntry.c @@ -348,8 +348,7 @@ break; case IP_RULE: case TCP_RULE: - case ICMP_RULE: - case IGMP_RULE: + case ICMP_IGMP_RULE: basename = "IPHeadersFilter"; convert_f = convert_ip_rule_to_instance; break;

+1. I doubt this will change anytime, so good optimization. On 10/06/2011 11:46 AM, Eduardo Lima (Etrunko) wrote:
libxkutil/acl_parsing.c | 119 ++++++++++++++--------------------------------- libxkutil/acl_parsing.h | 29 +---------- src/Virt_FilterEntry.c | 3 +- 3 files changed, 41 insertions(+), 110 deletions(-)
# HG changeset patch # User Eduardo Lima (Etrunko)<eblima@br.ibm.com> # Date 1317840215 10800 # Node ID fa576f11a652c4632afb62c687707effb2e98a80 # Parent 1f93220799a57477a6b1e04a90e8ffe797d85581 acl_parsing: Share code for icmp and icmp rule types
The struct for both ICMP and IGMP rule types have the exact same fields. With this patch, we avoid duplicating code to handle those rules.
Signed-off-by: Eduardo Lima (Etrunko)<eblima@br.ibm.com>
diff --git a/libxkutil/acl_parsing.c b/libxkutil/acl_parsing.c --- a/libxkutil/acl_parsing.c +++ b/libxkutil/acl_parsing.c @@ -97,41 +97,23 @@ free(rule->var.tcp.comment); free(rule->var.tcp.state); break; - case IGMP_RULE: - free(rule->var.igmp.srcmacaddr); - free(rule->var.igmp.srcmacmask); - free(rule->var.igmp.dstmacaddr); - free(rule->var.igmp.dstmacmask); - free(rule->var.igmp.srcipaddr); - free(rule->var.igmp.srcipmask); - free(rule->var.igmp.dstipaddr); - free(rule->var.igmp.dstipmask); - free(rule->var.igmp.srcipfrom); - free(rule->var.igmp.srcipto); - free(rule->var.igmp.dstipfrom); - free(rule->var.igmp.dstipto); - free(rule->var.igmp.type); - free(rule->var.igmp.code); - free(rule->var.igmp.comment); - free(rule->var.igmp.state); - break; - case ICMP_RULE: - free(rule->var.icmp.srcmacaddr); - free(rule->var.icmp.srcmacmask); - free(rule->var.icmp.dstmacaddr); - free(rule->var.icmp.dstmacmask); - free(rule->var.icmp.srcipaddr); - free(rule->var.icmp.srcipmask); - free(rule->var.icmp.dstipaddr); - free(rule->var.icmp.dstipmask); - free(rule->var.icmp.srcipfrom); - free(rule->var.icmp.srcipto); - free(rule->var.icmp.dstipfrom); - free(rule->var.icmp.dstipto); - free(rule->var.icmp.type); - free(rule->var.icmp.code); - free(rule->var.icmp.comment); - free(rule->var.icmp.state); + case ICMP_IGMP_RULE: + free(rule->var.icmp_igmp.srcmacaddr); + free(rule->var.icmp_igmp.srcmacmask); + free(rule->var.icmp_igmp.dstmacaddr); + free(rule->var.icmp_igmp.dstmacmask); + free(rule->var.icmp_igmp.srcipaddr); + free(rule->var.icmp_igmp.srcipmask); + free(rule->var.icmp_igmp.dstipaddr); + free(rule->var.icmp_igmp.dstipmask); + free(rule->var.icmp_igmp.srcipfrom); + free(rule->var.icmp_igmp.srcipto); + free(rule->var.icmp_igmp.dstipfrom); + free(rule->var.icmp_igmp.dstipto); + free(rule->var.icmp_igmp.type); + free(rule->var.icmp_igmp.code); + free(rule->var.icmp_igmp.comment); + free(rule->var.icmp_igmp.state); break; case UNKNOWN_RULE: default: @@ -265,50 +247,25 @@ return 1; }
-static int parse_acl_icmp_rule(xmlNode *rnode, struct acl_rule *rule) +static int parse_acl_icmp_igmp_rule(xmlNode *rnode, struct acl_rule *rule) { - CU_DEBUG("ACL icmp rule %s", rnode->name); + CU_DEBUG("ACL %s rule %s", rule->protocol_id, rnode->name);
- rule->type = ICMP_RULE; - rule->var.icmp.srcmacaddr = get_attr_value(rnode, "srcmacaddr"); - rule->var.icmp.srcmacmask = get_attr_value(rnode, "srcmacmask"); - rule->var.icmp.dstmacaddr = get_attr_value(rnode, "dstmacaddr"); - rule->var.icmp.dstmacmask = get_attr_value(rnode, "dstmacmask"); - rule->var.icmp.srcipaddr = get_attr_value(rnode, "srcipaddr"); - rule->var.icmp.srcipmask = get_attr_value(rnode, "srcipmask"); - rule->var.icmp.dstipaddr = get_attr_value(rnode, "dstipaddr"); - rule->var.icmp.dstipmask = get_attr_value(rnode, "dstipmask"); - rule->var.icmp.srcipfrom = get_attr_value(rnode, "srcipfrom"); - rule->var.icmp.srcipto = get_attr_value(rnode, "srcipto"); - rule->var.icmp.dstipfrom = get_attr_value(rnode, "dstipfrom"); - rule->var.icmp.dstipto = get_attr_value(rnode, "dstipto"); - rule->var.icmp.comment = get_attr_value(rnode, "comment"); - rule->var.icmp.state = get_attr_value(rnode, "state"); - - return 1; -} - -static int parse_acl_igmp_rule(xmlNode *rnode, struct acl_rule *rule) -{ - CU_DEBUG("ACL igmp rule %s", rnode->name); - - rule->type = IGMP_RULE; - rule->var.igmp.srcmacaddr = get_attr_value(rnode, "srcmacaddr"); - rule->var.igmp.srcmacmask = get_attr_value(rnode, "srcmacmask"); - rule->var.igmp.dstmacaddr = get_attr_value(rnode, "dstmacaddr"); - rule->var.igmp.dstmacmask = get_attr_value(rnode, "dstmacmask"); - rule->var.igmp.srcipaddr = get_attr_value(rnode, "srcipaddr"); - rule->var.igmp.srcipmask = get_attr_value(rnode, "srcipmask"); - rule->var.igmp.dstipaddr = get_attr_value(rnode, "dstipaddr"); - rule->var.igmp.dstipmask = get_attr_value(rnode, "dstipmask"); - rule->var.igmp.srcipfrom = get_attr_value(rnode, "srcipfrom"); - rule->var.igmp.srcipto = get_attr_value(rnode, "srcipto"); - rule->var.igmp.dstipfrom = get_attr_value(rnode, "dstipfrom"); - rule->var.igmp.dstipto = get_attr_value(rnode, "dstipto"); - rule->var.igmp.type = get_attr_value(rnode, "type"); - rule->var.igmp.code = get_attr_value(rnode, "code"); - rule->var.igmp.comment = get_attr_value(rnode, "comment"); - rule->var.igmp.state = get_attr_value(rnode, "state"); + rule->type = ICMP_IGMP_RULE; + rule->var.icmp_igmp.srcmacaddr = get_attr_value(rnode, "srcmacaddr"); + rule->var.icmp_igmp.srcmacmask = get_attr_value(rnode, "srcmacmask"); + rule->var.icmp_igmp.dstmacaddr = get_attr_value(rnode, "dstmacaddr"); + rule->var.icmp_igmp.dstmacmask = get_attr_value(rnode, "dstmacmask"); + rule->var.icmp_igmp.srcipaddr = get_attr_value(rnode, "srcipaddr"); + rule->var.icmp_igmp.srcipmask = get_attr_value(rnode, "srcipmask"); + rule->var.icmp_igmp.dstipaddr = get_attr_value(rnode, "dstipaddr"); + rule->var.icmp_igmp.dstipmask = get_attr_value(rnode, "dstipmask"); + rule->var.icmp_igmp.srcipfrom = get_attr_value(rnode, "srcipfrom"); + rule->var.icmp_igmp.srcipto = get_attr_value(rnode, "srcipto"); + rule->var.icmp_igmp.dstipfrom = get_attr_value(rnode, "dstipfrom"); + rule->var.icmp_igmp.dstipto = get_attr_value(rnode, "dstipto"); + rule->var.icmp_igmp.comment = get_attr_value(rnode, "comment"); + rule->var.icmp_igmp.state = get_attr_value(rnode, "state");
return 1; } @@ -351,10 +308,8 @@ rule->protocol_id = strdup((char *)child->name); parse_acl_tcp_rule(child, rule); } else if (XSTREQ(child->name, "icmp") || - XSTREQ(child->name, "icmpv6")) { - rule->protocol_id = strdup((char *)child->name); - parse_acl_icmp_rule(child, rule); - } else if (XSTREQ(child->name, "igmp") || + XSTREQ(child->name, "icmpv6") || + XSTREQ(child->name, "igmp") || XSTREQ(child->name, "igmp-ipv6") || XSTREQ(child->name, "esp") || XSTREQ(child->name, "esp-ipv6") || @@ -365,7 +320,7 @@ XSTREQ(child->name, "all") || XSTREQ(child->name, "all-ipv6")) { rule->protocol_id = strdup((char *)child->name); - parse_acl_igmp_rule(child, rule); + parse_acl_icmp_igmp_rule(child, rule); } }
diff --git a/libxkutil/acl_parsing.h b/libxkutil/acl_parsing.h --- a/libxkutil/acl_parsing.h +++ b/libxkutil/acl_parsing.h @@ -95,7 +95,7 @@ char *state; };
-struct acl_igmp_rule { +struct acl_icmp_igmp_rule { char *srcmacaddr; char *srcmacmask; char *dstmacaddr; @@ -117,27 +117,6 @@ char *state; };
-struct acl_icmp_rule { - char *srcmacaddr; - char *srcmacmask; - char *dstmacaddr; - char *dstmacmask; - - char *srcipaddr; - char *srcipmask; - char *dstipaddr; - char *dstipmask; - - char *srcipfrom; - char *srcipto; - char *dstipfrom; - char *dstipto; - - char *type; - char *code; - char *comment; - char *state; -};
struct acl_rule { char *name; @@ -153,8 +132,7 @@ ARP_RULE, IP_RULE, TCP_RULE, - ICMP_RULE, - IGMP_RULE + ICMP_IGMP_RULE, } type;
union { @@ -162,8 +140,7 @@ struct acl_arp_rule arp; struct acl_ip_rule ip; struct acl_tcp_rule tcp; - struct acl_icmp_rule icmp; - struct acl_igmp_rule igmp; + struct acl_icmp_igmp_rule icmp_igmp; } var; };
diff --git a/src/Virt_FilterEntry.c b/src/Virt_FilterEntry.c --- a/src/Virt_FilterEntry.c +++ b/src/Virt_FilterEntry.c @@ -348,8 +348,7 @@ break; case IP_RULE: case TCP_RULE: - case ICMP_RULE: - case IGMP_RULE: + case ICMP_IGMP_RULE: basename = "IPHeadersFilter"; convert_f = convert_ip_rule_to_instance; break;
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com

src/Virt_FilterEntry.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) # HG changeset patch # User Eduardo Lima (Etrunko) <eblima@br.ibm.com> # Date 1317912440 10800 # Node ID a323895be993b3807cc41348ba3b74c76fb42596 # Parent fa576f11a652c4632afb62c687707effb2e98a80 FilterEntry: Should be using srcipaddr instead of srcmacaddr Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> diff --git a/src/Virt_FilterEntry.c b/src/Virt_FilterEntry.c --- a/src/Virt_FilterEntry.c +++ b/src/Virt_FilterEntry.c @@ -238,7 +238,7 @@ (CMPIValue *)&array, CMPI_uint8A); } else { memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.srcmacaddr, + size = octets_from_ip(rule->var.tcp.srcipaddr, bytes, sizeof(bytes)); array = octets_to_cmpi(broker, bytes, size);

+1. Good catch. On 10/06/2011 11:46 AM, Eduardo Lima (Etrunko) wrote:
src/Virt_FilterEntry.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
# HG changeset patch # User Eduardo Lima (Etrunko)<eblima@br.ibm.com> # Date 1317912440 10800 # Node ID a323895be993b3807cc41348ba3b74c76fb42596 # Parent fa576f11a652c4632afb62c687707effb2e98a80 FilterEntry: Should be using srcipaddr instead of srcmacaddr
Signed-off-by: Eduardo Lima (Etrunko)<eblima@br.ibm.com>
diff --git a/src/Virt_FilterEntry.c b/src/Virt_FilterEntry.c --- a/src/Virt_FilterEntry.c +++ b/src/Virt_FilterEntry.c @@ -238,7 +238,7 @@ (CMPIValue *)&array, CMPI_uint8A); } else { memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.srcmacaddr, + size = octets_from_ip(rule->var.tcp.srcipaddr, bytes, sizeof(bytes));
array = octets_to_cmpi(broker, bytes, size);
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com

src/Virt_FilterEntry.c | 88 ++++++++++++++++++++++++++++++++++++++++++------- 1 files changed, 74 insertions(+), 14 deletions(-) # HG changeset patch # User Eduardo Lima (Etrunko) <eblima@br.ibm.com> # Date 1317914740 10800 # Node ID 1a08d8186f3064dfb0c38ecb5846ffc1e7d5de4d # Parent a323895be993b3807cc41348ba3b74c76fb42596 FilterEntry: Support for mask in CIDR notation The values for mask fields may have been written using the CIDR notation[1]. For instance, take the libvirt 'no-ip-multicast' builtin filter: <filter name='no-ip-multicast' chain='ipv4'> <uuid>47756f11-6057-1448-2cce-fda40fa23ba4</uuid> <rule action='drop' direction='out' priority='500'> <ip dstipaddr='224.0.0.0' dstipmask='4'/> </rule> </filter> As libvirt-cim expects an address like string, for the mask, in this case the conversion will fail and will output an array with only zero values [0,0,0,0], when it actually should be [240,0,0,0]. [1] http://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> diff --git a/src/Virt_FilterEntry.c b/src/Virt_FilterEntry.c --- a/src/Virt_FilterEntry.c +++ b/src/Virt_FilterEntry.c @@ -115,6 +115,44 @@ return array; } +static char *cidr_to_str(const char *cidr) +{ + char *ret = NULL; + int val; + unsigned int o1, o2, o3, o4; + + if (cidr == NULL || strlen(cidr) == 0) + return NULL; + + CU_DEBUG("Enter %s(%s)", __FUNCTION__, cidr); + + /* String value to integer */ + val = atoi(cidr); + if (val < 0 || val > 32) + return NULL; + + if (val == 0) + return strdup("0.0.0.0"); + else if (val == 32) + return strdup("255.255.255.255"); + + /* CIDR to bits */ + val = (0xffffffff >> (32 - val)) << (32 - val); + + /* bits to octets */ + o1 = (val & 0xff000000) >> 24; + o2 = (val & 0x00ff0000) >> 16; + o3 = (val & 0x0000ff00) >> 8; + o4 = val & 0x000000ff; + + /* octets to address string */ + ret = calloc(1, sizeof(*ret) * 16); + snprintf(ret, 16, "%u.%u.%u.%u", o1, o2, o3, o4); + + CU_DEBUG("%s: returning '%s'", __FUNCTION__, ret); + return ret; +} + static int convert_direction(const char *s) { enum {NOT_APPLICABLE, INPUT, OUTPUT, BOTH} direction = NOT_APPLICABLE; @@ -246,14 +284,25 @@ CMSetProperty(inst, "HdrSrcAddress", (CMPIValue *)&array, CMPI_uint8A); - memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.srcipmask, - bytes, sizeof(bytes)); + /* CIDR notation? */ + if (rule->var.tcp.srcipmask) { + char *netmask = strdup(rule->var.tcp.srcipmask); + if (strstr(netmask, ".") == NULL) { + char *tmp = cidr_to_str(netmask); + free(netmask); + netmask = tmp; + } - array = octets_to_cmpi(broker, bytes, size); - if (array != NULL) - CMSetProperty(inst, "HdrSrcMask", - (CMPIValue *)&array, CMPI_uint8A); + memset(bytes, 0, sizeof(bytes)); + size = octets_from_ip(netmask, bytes, sizeof(bytes)); + + array = octets_to_cmpi(broker, bytes, size); + if (array != NULL) + CMSetProperty(inst, "HdrSrcMask", + (CMPIValue *)&array, CMPI_uint8A); + + free(netmask); + } } if (rule->var.tcp.dstipfrom && rule->var.tcp.dstipto) { @@ -284,14 +333,25 @@ CMSetProperty(inst, "HdrDestAddress", (CMPIValue *)&array, CMPI_uint8A); - memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.dstipmask, - bytes, sizeof(bytes)); + /* CIDR notation? */ + if (rule->var.tcp.dstipmask) { + char *netmask = strdup(rule->var.tcp.dstipmask); + if (strstr(netmask, ".") == NULL) { + char *tmp = cidr_to_str(netmask); + free(netmask); + netmask = tmp; + } - array = octets_to_cmpi(broker, bytes, size); - if (array != NULL) - CMSetProperty(inst, "HdrDestMask", - (CMPIValue *)&array, CMPI_uint8A); + memset(bytes, 0, sizeof(bytes)); + size = octets_from_ip(netmask, bytes, sizeof(bytes)); + + array = octets_to_cmpi(broker, bytes, size); + if (array != NULL) + CMSetProperty(inst, "HdrDestMask", + (CMPIValue *)&array, CMPI_uint8A); + + free(netmask); + } } if ((rule->type == IP_RULE) || (rule->type == TCP_RULE)) {

+1. On 10/06/2011 11:46 AM, Eduardo Lima (Etrunko) wrote:
src/Virt_FilterEntry.c | 88 ++++++++++++++++++++++++++++++++++++++++++------- 1 files changed, 74 insertions(+), 14 deletions(-)
# HG changeset patch # User Eduardo Lima (Etrunko)<eblima@br.ibm.com> # Date 1317914740 10800 # Node ID 1a08d8186f3064dfb0c38ecb5846ffc1e7d5de4d # Parent a323895be993b3807cc41348ba3b74c76fb42596 FilterEntry: Support for mask in CIDR notation
The values for mask fields may have been written using the CIDR notation[1]. For instance, take the libvirt 'no-ip-multicast' builtin filter:
<filter name='no-ip-multicast' chain='ipv4'> <uuid>47756f11-6057-1448-2cce-fda40fa23ba4</uuid> <rule action='drop' direction='out' priority='500'> <ip dstipaddr='224.0.0.0' dstipmask='4'/> </rule> </filter>
As libvirt-cim expects an address like string, for the mask, in this case the conversion will fail and will output an array with only zero values [0,0,0,0], when it actually should be [240,0,0,0].
[1] http://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing
Signed-off-by: Eduardo Lima (Etrunko)<eblima@br.ibm.com>
diff --git a/src/Virt_FilterEntry.c b/src/Virt_FilterEntry.c --- a/src/Virt_FilterEntry.c +++ b/src/Virt_FilterEntry.c @@ -115,6 +115,44 @@ return array; }
+static char *cidr_to_str(const char *cidr) +{ + char *ret = NULL; + int val; + unsigned int o1, o2, o3, o4; + + if (cidr == NULL || strlen(cidr) == 0) + return NULL; + + CU_DEBUG("Enter %s(%s)", __FUNCTION__, cidr); + + /* String value to integer */ + val = atoi(cidr); + if (val< 0 || val> 32) + return NULL; + + if (val == 0) + return strdup("0.0.0.0"); + else if (val == 32) + return strdup("255.255.255.255"); + + /* CIDR to bits */ + val = (0xffffffff>> (32 - val))<< (32 - val); + + /* bits to octets */ + o1 = (val& 0xff000000)>> 24; + o2 = (val& 0x00ff0000)>> 16; + o3 = (val& 0x0000ff00)>> 8; + o4 = val& 0x000000ff; + + /* octets to address string */ + ret = calloc(1, sizeof(*ret) * 16); + snprintf(ret, 16, "%u.%u.%u.%u", o1, o2, o3, o4); + + CU_DEBUG("%s: returning '%s'", __FUNCTION__, ret); + return ret; +} + static int convert_direction(const char *s) { enum {NOT_APPLICABLE, INPUT, OUTPUT, BOTH} direction = NOT_APPLICABLE; @@ -246,14 +284,25 @@ CMSetProperty(inst, "HdrSrcAddress", (CMPIValue *)&array, CMPI_uint8A);
- memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.srcipmask, - bytes, sizeof(bytes)); + /* CIDR notation? */ + if (rule->var.tcp.srcipmask) { + char *netmask = strdup(rule->var.tcp.srcipmask); + if (strstr(netmask, ".") == NULL) { + char *tmp = cidr_to_str(netmask); + free(netmask); + netmask = tmp; + }
- array = octets_to_cmpi(broker, bytes, size); - if (array != NULL) - CMSetProperty(inst, "HdrSrcMask", - (CMPIValue *)&array, CMPI_uint8A); + memset(bytes, 0, sizeof(bytes)); + size = octets_from_ip(netmask, bytes, sizeof(bytes)); + + array = octets_to_cmpi(broker, bytes, size); + if (array != NULL) + CMSetProperty(inst, "HdrSrcMask", + (CMPIValue *)&array, CMPI_uint8A); + + free(netmask); + } }
if (rule->var.tcp.dstipfrom&& rule->var.tcp.dstipto) { @@ -284,14 +333,25 @@ CMSetProperty(inst, "HdrDestAddress", (CMPIValue *)&array, CMPI_uint8A);
- memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.dstipmask, - bytes, sizeof(bytes)); + /* CIDR notation? */ + if (rule->var.tcp.dstipmask) { + char *netmask = strdup(rule->var.tcp.dstipmask); + if (strstr(netmask, ".") == NULL) { + char *tmp = cidr_to_str(netmask); + free(netmask); + netmask = tmp; + }
- array = octets_to_cmpi(broker, bytes, size); - if (array != NULL) - CMSetProperty(inst, "HdrDestMask", - (CMPIValue *)&array, CMPI_uint8A); + memset(bytes, 0, sizeof(bytes)); + size = octets_from_ip(netmask, bytes, sizeof(bytes)); + + array = octets_to_cmpi(broker, bytes, size); + if (array != NULL) + CMSetProperty(inst, "HdrDestMask", + (CMPIValue *)&array, CMPI_uint8A); + + free(netmask); + } }
if ((rule->type == IP_RULE) || (rule->type == TCP_RULE)) {
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com

src/Virt_FilterEntry.c | 162 ++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 126 insertions(+), 36 deletions(-) # HG changeset patch # User Eduardo Lima (Etrunko) <eblima@br.ibm.com> # Date 1317915956 10800 # Node ID e6c6f9ccb51303f67fce26e74de9ab746d10a04b # Parent 1a08d8186f3064dfb0c38ecb5846ffc1e7d5de4d FilterEntry: Fix behavior of convert_ip_rule_to_instance The function was always referencing the 'tcp' fileld of the var union in struct acl_rule, while it should take into account the rule type to access the correct fields. This could probably go to acl_parsing, but I thought it would be a less intrusive change to patch only FilterEntry provider. Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> diff --git a/src/Virt_FilterEntry.c b/src/Virt_FilterEntry.c --- a/src/Virt_FilterEntry.c +++ b/src/Virt_FilterEntry.c @@ -35,6 +35,27 @@ #include "Virt_HostSystem.h" const static CMPIBroker *_BROKER; +struct rule_data_t { + const char *srcmacaddr; + const char *srcmacmask; + const char *dstmacaddr; + const char *dstmacmask; + + const char *srcipaddr; + const char *srcipmask; + const char *dstipaddr; + const char *dstipmask; + + const char *srcipfrom; + const char *srcipto; + const char *dstipfrom; + const char *dstipto; + + const char *srcportstart; + const char *srcportend; + const char *dstportstart; + const char *dstportend; +}; static int octets_from_mac(const char * s, unsigned int *buffer, unsigned int size) @@ -239,6 +260,75 @@ (CMPIValue *)&array, CMPI_uint8A); } +static void fill_rule_data(struct acl_rule *rule, + struct rule_data_t *data) +{ + if (rule == NULL || data == NULL) + return; + + memset(data, 0, sizeof(*data)); + + switch (rule->type) { + case IP_RULE: + data->srcmacaddr = rule->var.ip.srcmacaddr; + data->srcmacmask = rule->var.ip.srcmacmask; + data->dstmacaddr = rule->var.ip.srcmacaddr; + data->dstmacmask = rule->var.ip.dstmacmask; + + data->srcipaddr = rule->var.ip.srcipaddr; + data->srcipmask = rule->var.ip.srcipmask; + data->dstipaddr = rule->var.ip.dstipaddr; + data->dstipmask = rule->var.ip.dstipmask; + + data->srcportstart = rule->var.ip.srcportstart; + data->srcportend = rule->var.ip.srcportend; + data->dstportstart = rule->var.ip.dstportstart; + data->dstportend = rule->var.ip.dstportend; + break; + + case TCP_RULE: + data->srcmacaddr = rule->var.tcp.srcmacaddr; + + data->srcipaddr = rule->var.tcp.srcipaddr; + data->srcipmask = rule->var.tcp.srcipmask; + data->dstipaddr = rule->var.tcp.dstipaddr; + data->dstipmask = rule->var.tcp.dstipmask; + + data->srcipfrom = rule->var.tcp.srcipfrom; + data->srcipto = rule->var.tcp.srcipto; + data->dstipfrom = rule->var.tcp.dstipfrom; + data->dstipto = rule->var.tcp.dstipto; + + data->srcportstart = rule->var.tcp.srcportstart; + data->srcportend = rule->var.tcp.srcportend; + data->dstportstart = rule->var.tcp.dstportstart; + data->dstportend = rule->var.tcp.dstportend; + break; + + case ICMP_IGMP_RULE: + data->srcmacaddr = rule->var.icmp_igmp.srcmacaddr; + data->srcmacmask = rule->var.icmp_igmp.srcmacmask; + data->dstmacaddr = rule->var.icmp_igmp.srcmacaddr; + data->dstmacmask = rule->var.icmp_igmp.dstmacmask; + + data->srcipaddr = rule->var.icmp_igmp.srcipaddr; + data->srcipmask = rule->var.icmp_igmp.srcipmask; + data->dstipaddr = rule->var.icmp_igmp.dstipaddr; + data->dstipmask = rule->var.icmp_igmp.dstipmask; + + data->srcipfrom = rule->var.icmp_igmp.srcipfrom; + data->srcipto = rule->var.icmp_igmp.srcipto; + data->dstipfrom = rule->var.icmp_igmp.dstipfrom; + data->dstipto = rule->var.icmp_igmp.dstipto; + break; + + default: + CU_DEBUG("%s(): unhandled rule type '%d'", + __FUNCTION__, rule->type); + break; + } +} + static void convert_ip_rule_to_instance( struct acl_rule *rule, CMPIInstance *inst, @@ -248,6 +338,7 @@ unsigned int size = 0; unsigned int n = 0; CMPIArray *array = NULL; + struct rule_data_t rule_data; if (strstr(rule->protocol_id, "v6")) n = 6; @@ -256,9 +347,11 @@ CMSetProperty(inst, "HdrIPVersion",(CMPIValue *)&n, CMPI_uint8); - if (rule->var.tcp.srcipfrom && rule->var.tcp.srcipto) { + fill_rule_data(rule, &rule_data); + + if (rule_data.srcipfrom && rule_data.srcipto) { memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.srcipfrom, + size = octets_from_ip(rule_data.srcipfrom, bytes, sizeof(bytes)); array = octets_to_cmpi(broker, bytes, size); @@ -267,7 +360,7 @@ (CMPIValue *)&array, CMPI_uint8A); memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.srcipto, + size = octets_from_ip(rule_data.srcipto, bytes, sizeof(bytes)); array = octets_to_cmpi(broker, bytes, size); @@ -276,7 +369,7 @@ (CMPIValue *)&array, CMPI_uint8A); } else { memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.srcipaddr, + size = octets_from_ip(rule_data.srcipaddr, bytes, sizeof(bytes)); array = octets_to_cmpi(broker, bytes, size); @@ -285,8 +378,8 @@ (CMPIValue *)&array, CMPI_uint8A); /* CIDR notation? */ - if (rule->var.tcp.srcipmask) { - char *netmask = strdup(rule->var.tcp.srcipmask); + if (rule_data.srcipmask) { + char *netmask = strdup(rule_data.srcipmask); if (strstr(netmask, ".") == NULL) { char *tmp = cidr_to_str(netmask); free(netmask); @@ -305,9 +398,9 @@ } } - if (rule->var.tcp.dstipfrom && rule->var.tcp.dstipto) { + if (rule_data.dstipfrom && rule_data.dstipto) { memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.dstipfrom, + size = octets_from_ip(rule_data.dstipfrom, bytes, sizeof(bytes)); array = octets_to_cmpi(broker, bytes, size); @@ -316,7 +409,7 @@ (CMPIValue *)&array, CMPI_uint8A); memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.dstipto, + size = octets_from_ip(rule_data.dstipto, bytes, sizeof(bytes)); array = octets_to_cmpi(broker, bytes, size); @@ -325,7 +418,7 @@ (CMPIValue *)&array, CMPI_uint8A); } else { memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.dstipaddr, + size = octets_from_ip(rule_data.dstipaddr, bytes, sizeof(bytes)); array = octets_to_cmpi(broker, bytes, size); @@ -334,8 +427,8 @@ (CMPIValue *)&array, CMPI_uint8A); /* CIDR notation? */ - if (rule->var.tcp.dstipmask) { - char *netmask = strdup(rule->var.tcp.dstipmask); + if (rule_data.dstipmask) { + char *netmask = strdup(rule_data.dstipmask); if (strstr(netmask, ".") == NULL) { char *tmp = cidr_to_str(netmask); free(netmask); @@ -354,32 +447,29 @@ } } - if ((rule->type == IP_RULE) || (rule->type == TCP_RULE)) { - if (rule->var.tcp.srcportstart) { - n = atoi(rule->var.tcp.srcportstart); - CMSetProperty(inst, "HdrSrcPortStart", - (CMPIValue *)&n, CMPI_uint16); - } - - if (rule->var.tcp.srcportend) { - n = atoi(rule->var.tcp.srcportend); - CMSetProperty(inst, "HdrSrcPortEnd", - (CMPIValue *)&n, CMPI_uint16); - } - - if (rule->var.tcp.dstportstart) { - n = atoi(rule->var.tcp.dstportstart); - CMSetProperty(inst, "HdrDestPortStart", - (CMPIValue *)&n, CMPI_uint16); - } - - if (rule->var.tcp.dstportend) { - n = atoi(rule->var.tcp.dstportend); - CMSetProperty(inst, "HdrDestPortEnd", - (CMPIValue *)&n, CMPI_uint16); - } + if (rule_data.srcportstart) { + n = atoi(rule_data.srcportstart); + CMSetProperty(inst, "HdrSrcPortStart", + (CMPIValue *)&n, CMPI_uint16); } + if (rule_data.srcportend) { + n = atoi(rule_data.srcportend); + CMSetProperty(inst, "HdrSrcPortEnd", + (CMPIValue *)&n, CMPI_uint16); + } + + if (rule_data.dstportstart) { + n = atoi(rule_data.dstportstart); + CMSetProperty(inst, "HdrDestPortStart", + (CMPIValue *)&n, CMPI_uint16); + } + + if (rule_data.dstportend) { + n = atoi(rule_data.dstportend); + CMSetProperty(inst, "HdrDestPortEnd", + (CMPIValue *)&n, CMPI_uint16); + } } static CMPIInstance *convert_rule_to_instance(

+1. I like the new struct in the provider since that's the layer that "flattens" the data--that is, converts the various rules types to one of the three instance types. On 10/06/2011 11:46 AM, Eduardo Lima (Etrunko) wrote:
src/Virt_FilterEntry.c | 162 ++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 126 insertions(+), 36 deletions(-)
# HG changeset patch # User Eduardo Lima (Etrunko)<eblima@br.ibm.com> # Date 1317915956 10800 # Node ID e6c6f9ccb51303f67fce26e74de9ab746d10a04b # Parent 1a08d8186f3064dfb0c38ecb5846ffc1e7d5de4d FilterEntry: Fix behavior of convert_ip_rule_to_instance
The function was always referencing the 'tcp' fileld of the var union in struct acl_rule, while it should take into account the rule type to access the correct fields. This could probably go to acl_parsing, but I thought it would be a less intrusive change to patch only FilterEntry provider.
Signed-off-by: Eduardo Lima (Etrunko)<eblima@br.ibm.com>
diff --git a/src/Virt_FilterEntry.c b/src/Virt_FilterEntry.c --- a/src/Virt_FilterEntry.c +++ b/src/Virt_FilterEntry.c @@ -35,6 +35,27 @@ #include "Virt_HostSystem.h"
const static CMPIBroker *_BROKER; +struct rule_data_t { + const char *srcmacaddr; + const char *srcmacmask; + const char *dstmacaddr; + const char *dstmacmask; + + const char *srcipaddr; + const char *srcipmask; + const char *dstipaddr; + const char *dstipmask; + + const char *srcipfrom; + const char *srcipto; + const char *dstipfrom; + const char *dstipto; + + const char *srcportstart; + const char *srcportend; + const char *dstportstart; + const char *dstportend; +};
static int octets_from_mac(const char * s, unsigned int *buffer, unsigned int size) @@ -239,6 +260,75 @@ (CMPIValue *)&array, CMPI_uint8A); }
+static void fill_rule_data(struct acl_rule *rule, + struct rule_data_t *data) +{ + if (rule == NULL || data == NULL) + return; + + memset(data, 0, sizeof(*data)); + + switch (rule->type) { + case IP_RULE: + data->srcmacaddr = rule->var.ip.srcmacaddr; + data->srcmacmask = rule->var.ip.srcmacmask; + data->dstmacaddr = rule->var.ip.srcmacaddr; + data->dstmacmask = rule->var.ip.dstmacmask; + + data->srcipaddr = rule->var.ip.srcipaddr; + data->srcipmask = rule->var.ip.srcipmask; + data->dstipaddr = rule->var.ip.dstipaddr; + data->dstipmask = rule->var.ip.dstipmask; + + data->srcportstart = rule->var.ip.srcportstart; + data->srcportend = rule->var.ip.srcportend; + data->dstportstart = rule->var.ip.dstportstart; + data->dstportend = rule->var.ip.dstportend; + break; + + case TCP_RULE: + data->srcmacaddr = rule->var.tcp.srcmacaddr; + + data->srcipaddr = rule->var.tcp.srcipaddr; + data->srcipmask = rule->var.tcp.srcipmask; + data->dstipaddr = rule->var.tcp.dstipaddr; + data->dstipmask = rule->var.tcp.dstipmask; + + data->srcipfrom = rule->var.tcp.srcipfrom; + data->srcipto = rule->var.tcp.srcipto; + data->dstipfrom = rule->var.tcp.dstipfrom; + data->dstipto = rule->var.tcp.dstipto; + + data->srcportstart = rule->var.tcp.srcportstart; + data->srcportend = rule->var.tcp.srcportend; + data->dstportstart = rule->var.tcp.dstportstart; + data->dstportend = rule->var.tcp.dstportend; + break; + + case ICMP_IGMP_RULE: + data->srcmacaddr = rule->var.icmp_igmp.srcmacaddr; + data->srcmacmask = rule->var.icmp_igmp.srcmacmask; + data->dstmacaddr = rule->var.icmp_igmp.srcmacaddr; + data->dstmacmask = rule->var.icmp_igmp.dstmacmask; + + data->srcipaddr = rule->var.icmp_igmp.srcipaddr; + data->srcipmask = rule->var.icmp_igmp.srcipmask; + data->dstipaddr = rule->var.icmp_igmp.dstipaddr; + data->dstipmask = rule->var.icmp_igmp.dstipmask; + + data->srcipfrom = rule->var.icmp_igmp.srcipfrom; + data->srcipto = rule->var.icmp_igmp.srcipto; + data->dstipfrom = rule->var.icmp_igmp.dstipfrom; + data->dstipto = rule->var.icmp_igmp.dstipto; + break; + + default: + CU_DEBUG("%s(): unhandled rule type '%d'", + __FUNCTION__, rule->type); + break; + } +} + static void convert_ip_rule_to_instance( struct acl_rule *rule, CMPIInstance *inst, @@ -248,6 +338,7 @@ unsigned int size = 0; unsigned int n = 0; CMPIArray *array = NULL; + struct rule_data_t rule_data;
if (strstr(rule->protocol_id, "v6")) n = 6; @@ -256,9 +347,11 @@
CMSetProperty(inst, "HdrIPVersion",(CMPIValue *)&n, CMPI_uint8);
- if (rule->var.tcp.srcipfrom&& rule->var.tcp.srcipto) { + fill_rule_data(rule,&rule_data); + + if (rule_data.srcipfrom&& rule_data.srcipto) { memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.srcipfrom, + size = octets_from_ip(rule_data.srcipfrom, bytes, sizeof(bytes));
array = octets_to_cmpi(broker, bytes, size); @@ -267,7 +360,7 @@ (CMPIValue *)&array, CMPI_uint8A);
memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.srcipto, + size = octets_from_ip(rule_data.srcipto, bytes, sizeof(bytes));
array = octets_to_cmpi(broker, bytes, size); @@ -276,7 +369,7 @@ (CMPIValue *)&array, CMPI_uint8A); } else { memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.srcipaddr, + size = octets_from_ip(rule_data.srcipaddr, bytes, sizeof(bytes));
array = octets_to_cmpi(broker, bytes, size); @@ -285,8 +378,8 @@ (CMPIValue *)&array, CMPI_uint8A);
/* CIDR notation? */ - if (rule->var.tcp.srcipmask) { - char *netmask = strdup(rule->var.tcp.srcipmask); + if (rule_data.srcipmask) { + char *netmask = strdup(rule_data.srcipmask); if (strstr(netmask, ".") == NULL) { char *tmp = cidr_to_str(netmask); free(netmask); @@ -305,9 +398,9 @@ } }
- if (rule->var.tcp.dstipfrom&& rule->var.tcp.dstipto) { + if (rule_data.dstipfrom&& rule_data.dstipto) { memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.dstipfrom, + size = octets_from_ip(rule_data.dstipfrom, bytes, sizeof(bytes));
array = octets_to_cmpi(broker, bytes, size); @@ -316,7 +409,7 @@ (CMPIValue *)&array, CMPI_uint8A);
memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.dstipto, + size = octets_from_ip(rule_data.dstipto, bytes, sizeof(bytes));
array = octets_to_cmpi(broker, bytes, size); @@ -325,7 +418,7 @@ (CMPIValue *)&array, CMPI_uint8A); } else { memset(bytes, 0, sizeof(bytes)); - size = octets_from_ip(rule->var.tcp.dstipaddr, + size = octets_from_ip(rule_data.dstipaddr, bytes, sizeof(bytes));
array = octets_to_cmpi(broker, bytes, size); @@ -334,8 +427,8 @@ (CMPIValue *)&array, CMPI_uint8A);
/* CIDR notation? */ - if (rule->var.tcp.dstipmask) { - char *netmask = strdup(rule->var.tcp.dstipmask); + if (rule_data.dstipmask) { + char *netmask = strdup(rule_data.dstipmask); if (strstr(netmask, ".") == NULL) { char *tmp = cidr_to_str(netmask); free(netmask); @@ -354,32 +447,29 @@ } }
- if ((rule->type == IP_RULE) || (rule->type == TCP_RULE)) { - if (rule->var.tcp.srcportstart) { - n = atoi(rule->var.tcp.srcportstart); - CMSetProperty(inst, "HdrSrcPortStart", - (CMPIValue *)&n, CMPI_uint16); - } - - if (rule->var.tcp.srcportend) { - n = atoi(rule->var.tcp.srcportend); - CMSetProperty(inst, "HdrSrcPortEnd", - (CMPIValue *)&n, CMPI_uint16); - } - - if (rule->var.tcp.dstportstart) { - n = atoi(rule->var.tcp.dstportstart); - CMSetProperty(inst, "HdrDestPortStart", - (CMPIValue *)&n, CMPI_uint16); - } - - if (rule->var.tcp.dstportend) { - n = atoi(rule->var.tcp.dstportend); - CMSetProperty(inst, "HdrDestPortEnd", - (CMPIValue *)&n, CMPI_uint16); - } + if (rule_data.srcportstart) { + n = atoi(rule_data.srcportstart); + CMSetProperty(inst, "HdrSrcPortStart", + (CMPIValue *)&n, CMPI_uint16); }
+ if (rule_data.srcportend) { + n = atoi(rule_data.srcportend); + CMSetProperty(inst, "HdrSrcPortEnd", + (CMPIValue *)&n, CMPI_uint16); + } + + if (rule_data.dstportstart) { + n = atoi(rule_data.dstportstart); + CMSetProperty(inst, "HdrDestPortStart", + (CMPIValue *)&n, CMPI_uint16); + } + + if (rule_data.dstportend) { + n = atoi(rule_data.dstportend); + CMSetProperty(inst, "HdrDestPortEnd", + (CMPIValue *)&n, CMPI_uint16); + } }
static CMPIInstance *convert_rule_to_instance(
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com

Thanks. Pushed series. On 10/06/2011 11:46 AM, Eduardo Lima (Etrunko) wrote:
[1/4] acl_parsing: Share code for icmp and icmp rule types
The struct for both ICMP and IGMP rule types have the exact same fields. With this patch, we avoid duplicating code to handle those rules.
[2/4] FilterEntry: Should be using srcipaddr instead of srcmacaddr
[3/4] FilterEntry: Support for mask in CIDR notation
The values for mask fields may have been written using the CIDR notation[1]. For instance, take the libvirt 'no-ip-multicast' builtin filter:
<filter name='no-ip-multicast' chain='ipv4'> <uuid>47756f11-6057-1448-2cce-fda40fa23ba4</uuid> <rule action='drop' direction='out' priority='500'> <ip dstipaddr='224.0.0.0' dstipmask='4'/> </rule> </filter>
As libvirt-cim expects an address like string, for the mask, in this case the conversion will fail and will output an array with only zero values [0,0,0,0], when it actually should be [240,0,0,0].
[1] http://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing
[4/4] FilterEntry: Fix behavior of convert_ip_rule_to_instance
The function was always referencing the 'tcp' fileld of the var union in struct acl_rule, while it should take into account the rule type to access the correct fields. This could probably go to acl_parsing, but I thought it would be a less intrusive change to patch only FilterEntry provider.
Signed-off-by: Eduardo Lima (Etrunko)<eblima@br.ibm.com>
libxkutil/acl_parsing.c | 119 ++++++++++------------------------ libxkutil/acl_parsing.h | 29 +------- src/Virt_FilterEntry.c | 3 +- src/Virt_FilterEntry.c | 2 +- src/Virt_FilterEntry.c | 88 +++++++++++++++++++++---- src/Virt_FilterEntry.c | 162 +++++++++++++++++++++++++++++++++++++---------- 6 files changed, 242 insertions(+), 161 deletions(-)
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com
participants (2)
-
Chip Vincent
-
Eduardo Lima (Etrunko)