[Libvirt-cim] [PATCH] ACL: Add KVM_FilterEntry class
Chip Vincent
cvincent at linux.vnet.ibm.com
Wed Sep 28 14:54:42 UTC 2011
I'm still in the process of testing, but the changes appear reasonable.
Only one nit (see below).
On 09/26/2011 05:13 PM, Eduardo Lima (Etrunko) wrote:
> schema/FilterEntry.mof | 30 ++++
> schema/FilterEntry.registration | 1 +
> src/Virt_EntriesInFilterList.c | 1 +
> src/Virt_FilterEntry.c | 241 ++++++++++++++-------------------------
> 4 files changed, 120 insertions(+), 153 deletions(-)
>
>
> # HG changeset patch
> # User Eduardo Lima (Etrunko)<eblima at br.ibm.com>
> # Date 1315334426 10800
> # Node ID 9a59a56e226f3f800ff7214b81ab5f2fe6362fcc
> # Parent db809376d763493849c2a19f587969eaec619b75
> ACL: Add KVM_FilterEntry class
>
> This is the generic rule meant to match simple rules such the one in
> 'allow-arp' filter. For instance:
>
> <filter name='allow-arp' chain='arp'>
> <uuid>15fea95d-4300-19ea-2685-5f1b14c6759b</uuid>
> <rule action='accept' direction='inout' priority='500'/>
> </filter>
>
> This patch also refactors the provider code to share the conversion of
> common properties between the various types of filter entries.
>
> Signed-off-by: Eduardo Lima (Etrunko)<eblima at br.ibm.com>
>
> diff --git a/schema/FilterEntry.mof b/schema/FilterEntry.mof
> --- a/schema/FilterEntry.mof
> +++ b/schema/FilterEntry.mof
> @@ -74,3 +74,33 @@
> MaxValue(1000)]
> uint16 Priority = 500;
> };
> +
> +[Provider("cmpi::Virt_FilterEntry")]
> +class KVM_FilterEntry : CIM_FilterEntry
> +{
> + [Description("This defines whether the Filter is used for input, "
> + "output, or both input and output filtering. All values are "
> + "used with respect to the interface for which the Filter "
> + "applies. \"Not Applicable\" (0) is used when there is no "
> + "direction applicable to the Filter. \"Input\" (1) is "
> + "used when the Filter applies to packets that are inbound "
> + "on the related interface. \"Output\" (2) is used when the "
> + "Filter applies to packets that are outbound on the "
> + "related interface. \"Both\" (3) is used to indicate that "
> + "the direction is immaterial, e.g., to filter on a source "
> + "subnet regardless of whether the flow is inbound or "
> + "outbound."),
> + ValueMap { "0", "1", "2", "3", "4" },
> + Values { "Not Applicable", "Input, Output", "Both", "Mirrored" }]
> + uint16 Direction;
> +
> + [Description("The priority of the rule controls the order in which "
> + "the rule will be, instantiated relative to other rules. "
> + "Rules with lower value will be instantiated and therefore "
> + "evaluated before rules with higher value. Valid values are "
> + "in the range of 0 to 1000. If this attribute is not "
> + "provided, the value 500 will automatically be assigned."),
> + MinValue(0),
> + MaxValue(1000)]
> + uint16 Priority = 500;
> +};
> diff --git a/schema/FilterEntry.registration b/schema/FilterEntry.registration
> --- a/schema/FilterEntry.registration
> +++ b/schema/FilterEntry.registration
> @@ -2,3 +2,4 @@
> # Classname Namespace ProviderName ProviderModule ProviderTypes
> KVM_Hdr8021Filter root/virt Virt_FilterEntry Virt_FilterEntry instance
> KVM_IPHeadersFilter root/virt Virt_FilterEntry Virt_FilterEntry instance
> +KVM_FilterEntry root/virt Virt_FilterEntry Virt_FilterEntry instance
> diff --git a/src/Virt_EntriesInFilterList.c b/src/Virt_EntriesInFilterList.c
> --- a/src/Virt_EntriesInFilterList.c
> +++ b/src/Virt_EntriesInFilterList.c
> @@ -160,6 +160,7 @@
> };
>
> static char *part_component[] = {
> + "KVM_FilterEntry",
> "KVM_Hdr8021Filter",
> "KVM_IPHeadersFilter",
> NULL
> diff --git a/src/Virt_FilterEntry.c b/src/Virt_FilterEntry.c
> --- a/src/Virt_FilterEntry.c
> +++ b/src/Virt_FilterEntry.c
> @@ -36,23 +36,6 @@
>
> const static CMPIBroker *_BROKER;
>
> -static bool is_mac_rule(int type)
> -{
> - if (type == MAC_RULE || type == ARP_RULE)
> - return 1;
> -
> - return 0;
> -}
> -
> -static bool is_ip_rule(int type)
> -{
> - if (type == IP_RULE || type == TCP_RULE || type == ICMP_RULE ||
> - type == IGMP_RULE)
> - return 1;
> -
> - return 0;
> -}
> -
> static int octets_from_mac(const char * s, unsigned int *buffer,
> unsigned int size)
> {
> @@ -172,59 +155,15 @@
> return action;
> }
>
> -static CMPIInstance *convert_mac_rule_to_instance(
> +static void convert_mac_rule_to_instance(
> struct acl_rule *rule,
> - const CMPIBroker *broker,
> - const CMPIContext *context,
> - const CMPIObjectPath *reference,
> - CMPIStatus *s)
> + CMPIInstance *inst,
> + const CMPIBroker *broker)
> {
> - CMPIInstance *inst = NULL;
> - const char *sys_name = NULL;
> - const char *sys_ccname = NULL;
> - int action, direction, priority = 0;
> unsigned int bytes[48];
> unsigned int size = 0;
> CMPIArray *array = NULL;
>
> - inst = get_typed_instance(broker,
> - CLASSNAME(reference),
> - "Hdr8021Filter",
> - NAMESPACE(reference));
> - if (inst == NULL) {
> - cu_statusf(broker, s,
> - CMPI_RC_ERR_FAILED,
> - "Unable to get 8021 filter instance");
> -
> - goto out;
> - }
> -
> - *s = get_host_system_properties(&sys_name,
> -&sys_ccname,
> - reference,
> - broker,
> - context);
> -
> - if (s->rc != CMPI_RC_OK) {
> - cu_statusf(broker, s,
> - CMPI_RC_ERR_FAILED,
> - "Unable to get host attributes");
> - goto out;
> - }
> -
> - CMSetProperty(inst, "SystemName", sys_name, CMPI_chars);
> - CMSetProperty(inst, "SystemCreationClassName", sys_ccname, CMPI_chars);
> - CMSetProperty(inst, "Name", (CMPIValue *)rule->name, CMPI_chars);
> -
> - action = convert_action(rule->action);
> - CMSetProperty(inst, "Action", (CMPIValue *)&action, CMPI_uint16);
> -
> - direction = convert_direction(rule->direction);
> - CMSetProperty(inst, "Direction", (CMPIValue *)&direction, CMPI_uint16);
> -
> - priority = convert_priority(rule->priority);
> - CMSetProperty(inst, "Priority", (CMPIValue *)&priority, CMPI_uint16);
> -
> memset(bytes, 0, sizeof(bytes));
> size = octets_from_mac(rule->var.mac.srcmacaddr,
> bytes, sizeof(bytes));
> @@ -260,64 +199,18 @@
> if (array != NULL)
> CMSetProperty(inst, "HdrDestMACMask8021", (CMPIValue *)
> (CMPIValue *)&array, CMPI_uint8A);
> -
> - out:
> - return inst;
> }
>
> -static CMPIInstance *convert_ip_rule_to_instance(
> +static void convert_ip_rule_to_instance(
> struct acl_rule *rule,
> - const CMPIBroker *broker,
> - const CMPIContext *context,
> - const CMPIObjectPath *reference,
> - CMPIStatus *s)
> + CMPIInstance *inst,
> + const CMPIBroker *broker)
> {
> - CMPIInstance *inst = NULL;
> - const char *sys_name = NULL;
> - const char *sys_ccname = NULL;
> - int action, direction, priority = 0;
> unsigned int bytes[48];
> unsigned int size = 0;
> unsigned int n = 0;
> CMPIArray *array = NULL;
>
> - inst = get_typed_instance(broker,
> - CLASSNAME(reference),
> - "IPHeadersFilter",
> - NAMESPACE(reference));
> - if (inst == NULL) {
> - cu_statusf(broker, s,
> - CMPI_RC_ERR_FAILED,
> - "Unable to get ip headers filter instance");
> - goto out;
> - }
> -
> - *s = get_host_system_properties(&sys_name,
> -&sys_ccname,
> - reference,
> - broker,
> - context);
> -
> - if (s->rc != CMPI_RC_OK) {
> - cu_statusf(broker, s,
> - CMPI_RC_ERR_FAILED,
> - "Unable to get host attributes");
> - goto out;
> - }
> -
> - CMSetProperty(inst, "SystemName", sys_name, CMPI_chars);
> - CMSetProperty(inst, "SystemCreationClassName", sys_ccname, CMPI_chars);
> - CMSetProperty(inst, "Name", (CMPIValue *)rule->name, CMPI_chars);
> -
> - action = convert_action(rule->action);
> - CMSetProperty(inst, "Action", (CMPIValue *)&action, CMPI_uint16);
> -
> - direction = convert_direction(rule->direction);
> - CMSetProperty(inst, "Direction", (CMPIValue *)&direction, CMPI_uint16);
> -
> - priority = convert_priority(rule->priority);
> - CMSetProperty(inst, "Priority", (CMPIValue *)&priority, CMPI_uint16);
> -
> if (strstr(rule->protocol_id, "v6"))
> n = 6;
> else
> @@ -427,8 +320,6 @@
> }
> }
>
> - out:
> - return inst;
> }
>
> static CMPIInstance *convert_rule_to_instance(
> @@ -439,25 +330,77 @@
> CMPIStatus *s)
> {
> CMPIInstance *instance = NULL;
> + const char *sys_name = NULL;
> + const char *sys_ccname = NULL;
> + const char *basename = NULL;
> + int action, direction, priority = 0;
> +
> + void (*convert_func)(struct acl_rule*, CMPIInstance*, const CMPIBroker*);
>
> if (rule == NULL)
> return NULL;
>
> - if(is_mac_rule(rule->type)) {
> - instance = convert_mac_rule_to_instance(rule,
> - broker,
> - context,
> - reference,
> - s);
> - }
> - else if(is_ip_rule(rule->type)) {
> - instance = convert_ip_rule_to_instance(rule,
> - broker,
> - context,
> - reference,
> - s);
> + switch (rule->type) {
> + case MAC_RULE:
> + case ARP_RULE:
> + basename = "Hdr8021Filter";
> + convert_func = convert_mac_rule_to_instance;
> + break;
> + case IP_RULE:
> + case TCP_RULE:
> + case ICMP_RULE:
> + case IGMP_RULE:
> + basename = "IPHeadersFilter";
> + convert_func = convert_ip_rule_to_instance;
> + break;
> + default:
> + basename = "FilterEntry";
> + convert_func = NULL;
> + break;
> }
>
> + instance = get_typed_instance(broker,
> + CLASSNAME(reference),
> + basename,
> + NAMESPACE(reference));
> +
> + if (instance == NULL) {
> + cu_statusf(broker, s,
> + CMPI_RC_ERR_FAILED,
> + "Unable to get filter entry instance");
> + goto out;
> + }
> +
> + *s = get_host_system_properties(&sys_name,
> +&sys_ccname,
> + reference,
> + broker,
> + context);
> +
> + if (s->rc != CMPI_RC_OK) {
> + cu_statusf(broker, s,
> + CMPI_RC_ERR_FAILED,
> + "Unable to get host attributes");
> + goto out;
> + }
> +
> + CMSetProperty(instance, "SystemName", sys_name, CMPI_chars);
> + CMSetProperty(instance, "SystemCreationClassName", sys_ccname, CMPI_chars);
> + CMSetProperty(instance, "Name", (CMPIValue *)rule->name, CMPI_chars);
> +
> + action = convert_action(rule->action);
> + CMSetProperty(instance, "Action", (CMPIValue *)&action, CMPI_uint16);
> +
> + direction = convert_direction(rule->direction);
> + CMSetProperty(instance, "Direction", (CMPIValue *)&direction, CMPI_uint16);
> +
> + priority = convert_priority(rule->priority);
> + CMSetProperty(instance, "Priority", (CMPIValue *)&priority, CMPI_uint16);
> +
> + if (convert_func)
> + convert_func(rule, instance, broker);
> +
> + out:
> return instance;
> }
There are some lines above that are longer than 80.
>
> @@ -475,10 +418,19 @@
>
> CU_DEBUG("Reference = %s", REF2STR(reference));
>
> - if (STREQC(CLASSNAME(reference), "KVM_Hdr8021Filter"))
> + if (STREQC(CLASSNAME(reference), "KVM_Hdr8021Filter")) {
> class_type = MAC;
> - else if (STREQC(CLASSNAME(reference), "KVM_IPHeadersFilter"))
> + } else if (STREQC(CLASSNAME(reference), "KVM_IPHeadersFilter")) {
> class_type = IP;
> + } else if (STREQC(CLASSNAME(reference), "KVM_FilterEntry")) {
> + class_type = NONE;
> + } else {
> + cu_statusf(broker,&s,
> + CMPI_RC_ERR_FAILED,
> + "Unrecognized class type");
> + goto out;
> + }
> +
>
> conn = connect_by_classname(_BROKER, CLASSNAME(reference),&s);
> if (conn == NULL)
> @@ -490,29 +442,12 @@
> for (j = 0; j< filters[i].rule_ct; j++) {
> CMPIInstance *instance = NULL;
>
> - if (((class_type == NONE) ||
> - (class_type == MAC))&&
> - is_mac_rule(filters[i].rules[j]->type)) {
> - instance = convert_mac_rule_to_instance(
> - filters[i].rules[j],
> - broker,
> - context,
> - reference,
> -&s);
> - }
> - else if (((class_type == NONE) ||
> - (class_type == IP))&&
> - is_ip_rule(filters[i].rules[j]->type)) {
> - instance = convert_ip_rule_to_instance(
> - filters[i].rules[j],
> - broker,
> - context,
> - reference,
> -&s);
> - }
> - else
> - CU_DEBUG("Unrecognized rule type %u",
> - filters[i].rules[j]->type);
> + instance = convert_rule_to_instance(
> + filters[i].rules[j],
> + broker,
> + context,
> + reference,
> +&s);
>
> if (instance != NULL)
> inst_list_add(list, instance);
>
> _______________________________________________
> 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