10:46 a.m.
[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(a)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(-)
10:46 a.m.
New subject: [PATCH 1 of 4] acl_parsing: Share code for icmp and icmp rule types
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;
11:35 a.m.
New subject: [PATCH 1 of 4] acl_parsing: Share code for icmp and icmp rule types
+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
10:46 a.m.
New subject: [PATCH 2 of 4] FilterEntry: Should be using srcipaddr instead of srcmacaddr
src/Virt_FilterEntry.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
# HG changeset patch
# User Eduardo Lima (Etrunko) <eblima(a)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(a)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);
11:35 a.m.
New subject: [PATCH 2 of 4] FilterEntry: Should be using srcipaddr instead of srcmacaddr
+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(a)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(a)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(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
10:46 a.m.
New subject: [PATCH 3 of 4] FilterEntry: Support for mask in CIDR notation
src/Virt_FilterEntry.c | 88 ++++++++++++++++++++++++++++++++++++++++++-------
1 files changed, 74 insertions(+), 14 deletions(-)
# HG changeset patch
# User Eduardo Lima (Etrunko) <eblima(a)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(a)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)) {
11:35 a.m.
New subject: [PATCH 3 of 4] FilterEntry: Support for mask in CIDR notation
+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(a)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(a)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(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
10:46 a.m.
New subject: [PATCH 4 of 4] FilterEntry: Fix behavior of convert_ip_rule_to_instance
src/Virt_FilterEntry.c | 162 ++++++++++++++++++++++++++++++++++++++----------
1 files changed, 126 insertions(+), 36 deletions(-)
# HG changeset patch
# User Eduardo Lima (Etrunko) <eblima(a)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(a)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(
11:38 a.m.
New subject: [PATCH 4 of 4] FilterEntry: Fix behavior of convert_ip_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(a)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(a)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(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
11:40 a.m.
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(a)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(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
4821
days inactive
4826
days old
9 comments
2 participants
participants (2)
-
Chip Vincent -
Eduardo Lima (Etrunko)