+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(a)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(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-cim
--
Chip Vincent
Open Virtualization
IBM Linux Technology Center
cvincent(a)linux.vnet.ibm.com