zebra: pbr hash cleanup WIP

    In the zebra PBR rule database, the hash was previously calculated
    over many fields, including most of the filter and action values.

    But a given rule should be identifiable by just:

	1. the identity of its originating daemon (ZAPI socket FD); and
	2. the 'unique' field in struct pbr_rule

    However, pbrd assigns a 'unique' field value per map (which may
    be assigned to multiple interfaces, yielding multiple corresponding
    rules). We could possibly modify pbrd to generate a 'unique'
    field value per rule instance, but the simpler solution for the
    moment is to also hash on the interface name.

    This commit changes the PBR hash function to use only the
    following parameters:

	1. the identity of its originating daemon (ZAPI socket FD); and
	2. the 'unique' field in struct pbr_rule
	3. the VRF ID
	4. the interface name

    The goal of the above change is to get a better handle on what
    parameters uniquely identify a rule, to (eventually) allow the
    assignment or parallel generation of a globally-unique identifier
    for each rule that can be used with a dataplane.

    This commit also removes the inefficient pbr_rule_lookup_unique()
    code that iterated linearly over the entire rule database.

Signed-off-by: G. Paul Ziemba <[email protected]>

lib/pbr.h

@@ -134,7 +134,7 @@ struct pbr_rule {
 
 	uint32_t seq;
 	uint32_t priority;
-	uint32_t unique;
+	uint32_t unique; /* scope: originating daemon + interface */
 	struct pbr_filter filter;
 	struct pbr_action action;
 

zebra/zebra_pbr.c

@@ -156,118 +156,43 @@ void zebra_pbr_rules_free(void *arg)
 
 uint32_t zebra_pbr_rules_hash_key(const void *arg)
 {
-	const struct zebra_pbr_rule *rule;
+	const struct zebra_pbr_rule *rule = arg;
 	uint32_t key;
 
-	rule = arg;
-	key = jhash_3words(rule->rule.seq, rule->rule.priority,
-			   rule->rule.action.table,
-			   prefix_hash_key(&rule->rule.filter.src_ip));
-
-	key = jhash_3words(rule->rule.filter.fwmark, rule->vrf_id,
-			   rule->rule.filter.ip_proto, key);
+	key = jhash_2words(rule->rule.unique, rule->sock, rule->vrf_id);
 
 	key = jhash(rule->ifname, strlen(rule->ifname), key);
 
-	key = jhash_3words(rule->rule.filter.pcp, rule->rule.filter.vlan_id,
-			   rule->rule.filter.vlan_flags, key);
-
-	key = jhash_3words(rule->rule.filter.src_port,
-			   rule->rule.filter.dst_port,
-			   prefix_hash_key(&rule->rule.filter.dst_ip), key);
-
-	key = jhash_2words(rule->rule.unique, rule->sock, key);
-
 	return key;
 }
 
+/*
+ * In zebra's PBR rule database, a rule is uniquely identified by
+ * two fields:
+ *
+ *   socket number (i.e., originating daemon)
+ *   interface name
+ *   unique field (originating daemon must ensure unique for itself+interface)
+ */
 bool zebra_pbr_rules_hash_equal(const void *arg1, const void *arg2)
 {
 	const struct zebra_pbr_rule *r1, *r2;
 
 	r1 = (const struct zebra_pbr_rule *)arg1;
 	r2 = (const struct zebra_pbr_rule *)arg2;
 
-	if (r1->rule.seq != r2->rule.seq)
-		return false;
-
-	if (r1->rule.priority != r2->rule.priority)
-		return false;
-
 	if (r1->sock != r2->sock)
 		return false;
 
-	if (r1->rule.unique != r2->rule.unique)
-		return false;
-
-	if (r1->rule.action.table != r2->rule.action.table)
-		return false;
-
-	if (r1->rule.filter.src_port != r2->rule.filter.src_port)
-		return false;
-
-	if (r1->rule.filter.dst_port != r2->rule.filter.dst_port)
-		return false;
-
-	if (r1->rule.filter.fwmark != r2->rule.filter.fwmark)
-		return false;
-
-	if (r1->rule.filter.ip_proto != r2->rule.filter.ip_proto)
-		return false;
-
-	if (!prefix_same(&r1->rule.filter.src_ip, &r2->rule.filter.src_ip))
-		return false;
-
-	if (!prefix_same(&r1->rule.filter.dst_ip, &r2->rule.filter.dst_ip))
-		return false;
-
 	if (strcmp(r1->rule.ifname, r2->rule.ifname) != 0)
 		return false;
 
-	if (r1->vrf_id != r2->vrf_id)
+	if (r1->rule.unique != r2->rule.unique)
 		return false;
 
 	return true;
 }
 
-struct pbr_rule_unique_lookup {
-	struct zebra_pbr_rule *rule;
-	int sock;
-	uint32_t unique;
-	char ifname[IFNAMSIZ + 1];
-	vrf_id_t vrf_id;
-};
-
-static int pbr_rule_lookup_unique_walker(struct hash_bucket *b, void *data)
-{
-	struct pbr_rule_unique_lookup *pul = data;
-	struct zebra_pbr_rule *rule = b->data;
-
-	if (pul->sock == rule->sock && pul->unique == rule->rule.unique &&
-	    strmatch(pul->ifname, rule->rule.ifname) &&
-	    pul->vrf_id == rule->vrf_id) {
-		pul->rule = rule;
-		return HASHWALK_ABORT;
-	}
-
-	return HASHWALK_CONTINUE;
-}
-
-static struct zebra_pbr_rule *
-pbr_rule_lookup_unique(struct zebra_pbr_rule *zrule)
-{
-	struct pbr_rule_unique_lookup pul;
-
-	pul.unique = zrule->rule.unique;
-	strlcpy(pul.ifname, zrule->rule.ifname, IFNAMSIZ);
-	pul.rule = NULL;
-	pul.vrf_id = zrule->vrf_id;
-	pul.sock = zrule->sock;
-	hash_walk(zrouter.rules_hash, &pbr_rule_lookup_unique_walker, &pul);
-
-	return pul.rule;
-}
-
 void zebra_pbr_ipset_free(void *arg)
 {
 	struct zebra_pbr_ipset *ipset;
@@ -702,11 +627,8 @@ void zebra_pbr_add_rule(struct zebra_pbr_rule *rule)
 	struct zebra_pbr_rule *old;
 	struct zebra_pbr_rule *new;
 
-	/**
-	 * Check if we already have it (this checks via a unique ID, walking
-	 * over the hash table, not via a hash operation).
-	 */
-	found = pbr_rule_lookup_unique(rule);
+	/* Check if we already have it */
+	found = hash_lookup(zrouter.rules_hash, rule);
 
 	/* If found, this is an update */
 	if (found) {