[Libvirt-cim] [PATCH 1 of 4] acl_parsing: Share code for icmp and icmp rule types
Chip Vincent
cvincent at linux.vnet.ibm.com
Tue Oct 11 16:35:26 UTC 2011
+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 at 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 at 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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim
--
Chip Vincent
Open Virtualization
IBM Linux Technology Center
cvincent at linux.vnet.ibm.com
More information about the Libvirt-cim
mailing list