netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/5] Improve locking in the VCAP API
@ 2023-01-17  8:55 Steen Hegelund
  2023-01-17  8:55 ` [PATCH net-next 1/5] net: microchip: sparx5: Add support for rule count by cookie Steen Hegelund
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Steen Hegelund @ 2023-01-17  8:55 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Steen Hegelund, UNGLinuxDriver, Randy Dunlap, Casper Andersson,
	Russell King, Wan Jiabing, Nathan Huckleberry, linux-kernel,
	netdev, linux-arm-kernel, Steen Hegelund, Daniel Machon,
	Horatiu Vultur, Lars Povlsen, Dan Carpenter, Michael Walle

This improves the VCAP cache and the VCAP rule list protection against
access from different sources.

The VCAP Admin lock protects the list of rules for the VCAP instance as
well as the cache used for encoding and decoding rules.

This series provides dedicated functions for accessing rule statistics,
decoding rule content, verifying if a rule exists and getting a rule with
the lock held, as well as ensuring the use of the lock when the list of
rules or the cache is accessed.

Steen Hegelund (5):
  net: microchip: sparx5: Add support for rule count by cookie
  net: microchip: sparx5: Add support to check for existing VCAP rule id
  net: microchip: sparx5: Add VCAP admin locking in debugFS
  net: microchip: sparx5: Improve VCAP admin locking in the VCAP API
  net: microchip: sparx5: Add lock initialization to the KUNIT tests

 .../microchip/sparx5/sparx5_tc_flower.c       |  34 +--
 .../net/ethernet/microchip/vcap/vcap_api.c    | 234 ++++++++++++------
 .../ethernet/microchip/vcap/vcap_api_client.h |   2 +
 .../microchip/vcap/vcap_api_debugfs.c         |  14 +-
 .../microchip/vcap/vcap_api_debugfs_kunit.c   |   1 +
 .../ethernet/microchip/vcap/vcap_api_kunit.c  |   1 +
 .../microchip/vcap/vcap_api_private.h         |   3 +
 7 files changed, 175 insertions(+), 114 deletions(-)

-- 
2.39.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net-next 1/5] net: microchip: sparx5: Add support for rule count by cookie
  2023-01-17  8:55 [PATCH net-next 0/5] Improve locking in the VCAP API Steen Hegelund
@ 2023-01-17  8:55 ` Steen Hegelund
  2023-01-17  8:55 ` [PATCH net-next 2/5] net: microchip: sparx5: Add support to check for existing VCAP rule id Steen Hegelund
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Steen Hegelund @ 2023-01-17  8:55 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Steen Hegelund, UNGLinuxDriver, Randy Dunlap, Casper Andersson,
	Russell King, Wan Jiabing, Nathan Huckleberry, linux-kernel,
	netdev, linux-arm-kernel, Steen Hegelund, Daniel Machon,
	Horatiu Vultur, Lars Povlsen, Dan Carpenter, Michael Walle

This adds support for TC clients to get the packet count for a TC filter
identified by its cookie.

Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
---
 .../microchip/sparx5/sparx5_tc_flower.c       | 34 +---------
 .../net/ethernet/microchip/vcap/vcap_api.c    | 67 ++++++++++++-------
 .../ethernet/microchip/vcap/vcap_api_client.h |  2 +
 3 files changed, 47 insertions(+), 56 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c b/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
index 986e41d3bb28..affaa1656710 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
@@ -35,11 +35,6 @@ struct sparx5_tc_flower_parse_usage {
 	unsigned int used_keys;
 };
 
-struct sparx5_tc_rule_pkt_cnt {
-	u64 cookie;
-	u32 pkts;
-};
-
 /* These protocols have dedicated keysets in IS2 and a TC dissector
  * ETH_P_ARP does not have a TC dissector
  */
@@ -947,44 +942,21 @@ static int sparx5_tc_flower_destroy(struct net_device *ndev,
 	return err;
 }
 
-/* Collect packet counts from all rules with the same cookie */
-static int sparx5_tc_rule_counter_cb(void *arg, struct vcap_rule *rule)
-{
-	struct sparx5_tc_rule_pkt_cnt *rinfo = arg;
-	struct vcap_counter counter;
-	int err = 0;
-
-	if (rule->cookie == rinfo->cookie) {
-		err = vcap_rule_get_counter(rule, &counter);
-		if (err)
-			return err;
-		rinfo->pkts += counter.value;
-		/* Reset the rule counter */
-		counter.value = 0;
-		vcap_rule_set_counter(rule, &counter);
-	}
-	return err;
-}
-
 static int sparx5_tc_flower_stats(struct net_device *ndev,
 				  struct flow_cls_offload *fco,
 				  struct vcap_admin *admin)
 {
 	struct sparx5_port *port = netdev_priv(ndev);
-	struct sparx5_tc_rule_pkt_cnt rinfo = {};
+	struct vcap_counter ctr = {};
 	struct vcap_control *vctrl;
 	ulong lastused = 0;
-	u64 drops = 0;
-	u32 pkts = 0;
 	int err;
 
-	rinfo.cookie = fco->cookie;
 	vctrl = port->sparx5->vcap_ctrl;
-	err = vcap_rule_iter(vctrl, sparx5_tc_rule_counter_cb, &rinfo);
+	err = vcap_get_rule_count_by_cookie(vctrl, &ctr, fco->cookie);
 	if (err)
 		return err;
-	pkts = rinfo.pkts;
-	flow_stats_update(&fco->stats, 0x0, pkts, drops, lastused,
+	flow_stats_update(&fco->stats, 0x0, ctr.value, 0, lastused,
 			  FLOW_ACTION_HW_STATS_IMMEDIATE);
 	return err;
 }
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api.c b/drivers/net/ethernet/microchip/vcap/vcap_api.c
index 0f9e0d735ae3..4e038f96a131 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api.c
@@ -2989,31 +2989,6 @@ void vcap_rule_set_counter_id(struct vcap_rule *rule, u32 counter_id)
 }
 EXPORT_SYMBOL_GPL(vcap_rule_set_counter_id);
 
-/* Provide all rules via a callback interface */
-int vcap_rule_iter(struct vcap_control *vctrl,
-		   int (*callback)(void *, struct vcap_rule *), void *arg)
-{
-	struct vcap_rule_internal *ri;
-	struct vcap_admin *admin;
-	int ret;
-
-	ret = vcap_api_check(vctrl);
-	if (ret)
-		return ret;
-
-	/* Iterate all rules in each VCAP instance */
-	list_for_each_entry(admin, &vctrl->list, list) {
-		list_for_each_entry(ri, &admin->rules, list) {
-			ret = callback(arg, &ri->data);
-			if (ret)
-				return ret;
-		}
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(vcap_rule_iter);
-
 int vcap_rule_set_counter(struct vcap_rule *rule, struct vcap_counter *ctr)
 {
 	struct vcap_rule_internal *ri = to_intrule(rule);
@@ -3105,6 +3080,48 @@ int vcap_rule_get_keysets(struct vcap_rule_internal *ri,
 	return -EINVAL;
 }
 
+/* Collect packet counts from all rules with the same cookie */
+int vcap_get_rule_count_by_cookie(struct vcap_control *vctrl,
+				  struct vcap_counter *ctr, u64 cookie)
+{
+	struct vcap_rule_internal *ri;
+	struct vcap_counter temp = {};
+	struct vcap_admin *admin;
+	int err;
+
+	err = vcap_api_check(vctrl);
+	if (err)
+		return err;
+
+	/* Iterate all rules in each VCAP instance */
+	list_for_each_entry(admin, &vctrl->list, list) {
+		mutex_lock(&admin->lock);
+		list_for_each_entry(ri, &admin->rules, list) {
+			if (ri->data.cookie != cookie)
+				continue;
+
+			err = vcap_read_counter(ri, &temp);
+			if (err)
+				goto unlock;
+			ctr->value += temp.value;
+
+			/* Reset the rule counter */
+			temp.value = 0;
+			temp.sticky = 0;
+			err = vcap_write_counter(ri, &temp);
+			if (err)
+				goto unlock;
+		}
+		mutex_unlock(&admin->lock);
+	}
+	return err;
+
+unlock:
+	mutex_unlock(&admin->lock);
+	return err;
+}
+EXPORT_SYMBOL_GPL(vcap_get_rule_count_by_cookie);
+
 static int vcap_rule_mod_key(struct vcap_rule *rule,
 			     enum vcap_key_field key,
 			     enum vcap_field_type ftype,
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_client.h b/drivers/net/ethernet/microchip/vcap/vcap_api_client.h
index b8980b22352f..2cdcd3b56b30 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_client.h
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_client.h
@@ -202,6 +202,8 @@ int vcap_rule_add_action_u32(struct vcap_rule *rule,
 			     enum vcap_action_field action, u32 value);
 
 /* VCAP rule counter operations */
+int vcap_get_rule_count_by_cookie(struct vcap_control *vctrl,
+				  struct vcap_counter *ctr, u64 cookie);
 int vcap_rule_set_counter(struct vcap_rule *rule, struct vcap_counter *ctr);
 int vcap_rule_get_counter(struct vcap_rule *rule, struct vcap_counter *ctr);
 
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 2/5] net: microchip: sparx5: Add support to check for existing VCAP rule id
  2023-01-17  8:55 [PATCH net-next 0/5] Improve locking in the VCAP API Steen Hegelund
  2023-01-17  8:55 ` [PATCH net-next 1/5] net: microchip: sparx5: Add support for rule count by cookie Steen Hegelund
@ 2023-01-17  8:55 ` Steen Hegelund
  2023-01-17  8:55 ` [PATCH net-next 3/5] net: microchip: sparx5: Add VCAP admin locking in debugFS Steen Hegelund
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Steen Hegelund @ 2023-01-17  8:55 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Steen Hegelund, UNGLinuxDriver, Randy Dunlap, Casper Andersson,
	Russell King, Wan Jiabing, Nathan Huckleberry, linux-kernel,
	netdev, linux-arm-kernel, Steen Hegelund, Daniel Machon,
	Horatiu Vultur, Lars Povlsen, Dan Carpenter, Michael Walle

Add a new function that just checks if the VCAP rule id is already used by
an existing rule.

Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
---
 drivers/net/ethernet/microchip/vcap/vcap_api.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api.c b/drivers/net/ethernet/microchip/vcap/vcap_api.c
index 4e038f96a131..f1dc4fd6bb96 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api.c
@@ -920,6 +920,20 @@ int vcap_set_rule_set_actionset(struct vcap_rule *rule,
 }
 EXPORT_SYMBOL_GPL(vcap_set_rule_set_actionset);
 
+/* Check if a rule with this id exists */
+static bool vcap_rule_exists(struct vcap_control *vctrl, u32 id)
+{
+	struct vcap_rule_internal *ri;
+	struct vcap_admin *admin;
+
+	/* Look for the rule id in all vcaps */
+	list_for_each_entry(admin, &vctrl->list, list)
+		list_for_each_entry(ri, &admin->rules, list)
+			if (ri->data.id == id)
+				return true;
+	return false;
+}
+
 /* Find a rule with a provided rule id */
 static struct vcap_rule_internal *vcap_lookup_rule(struct vcap_control *vctrl,
 						   u32 id)
@@ -1866,7 +1880,7 @@ static u32 vcap_set_rule_id(struct vcap_rule_internal *ri)
 		return ri->data.id;
 
 	for (u32 next_id = 1; next_id < ~0; ++next_id) {
-		if (!vcap_lookup_rule(ri->vctrl, next_id)) {
+		if (!vcap_rule_exists(ri->vctrl, next_id)) {
 			ri->data.id = next_id;
 			break;
 		}
@@ -2103,7 +2117,7 @@ struct vcap_rule *vcap_alloc_rule(struct vcap_control *vctrl,
 	if (vctrl->vcaps[admin->vtype].rows == 0)
 		return ERR_PTR(-EINVAL);
 	/* Check if a rule with this id already exists */
-	if (vcap_lookup_rule(vctrl, id))
+	if (vcap_rule_exists(vctrl, id))
 		return ERR_PTR(-EEXIST);
 	/* Check if there is room for the rule in the block(s) of the VCAP */
 	maxsize = vctrl->vcaps[admin->vtype].sw_count; /* worst case rule size */
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 3/5] net: microchip: sparx5: Add VCAP admin locking in debugFS
  2023-01-17  8:55 [PATCH net-next 0/5] Improve locking in the VCAP API Steen Hegelund
  2023-01-17  8:55 ` [PATCH net-next 1/5] net: microchip: sparx5: Add support for rule count by cookie Steen Hegelund
  2023-01-17  8:55 ` [PATCH net-next 2/5] net: microchip: sparx5: Add support to check for existing VCAP rule id Steen Hegelund
@ 2023-01-17  8:55 ` Steen Hegelund
  2023-01-17  8:55 ` [PATCH net-next 4/5] net: microchip: sparx5: Improve VCAP admin locking in the VCAP API Steen Hegelund
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Steen Hegelund @ 2023-01-17  8:55 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Steen Hegelund, UNGLinuxDriver, Randy Dunlap, Casper Andersson,
	Russell King, Wan Jiabing, Nathan Huckleberry, linux-kernel,
	netdev, linux-arm-kernel, Steen Hegelund, Daniel Machon,
	Horatiu Vultur, Lars Povlsen, Dan Carpenter, Michael Walle

This ensures that the admin lock is taken before the debugFS functions
starts iterating the VCAP rules.
It also adds a separate function to decode a rule, which expects the lock
to have been taken before it is called.

Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
---
 .../net/ethernet/microchip/vcap/vcap_api.c    | 60 ++++++++++---------
 .../microchip/vcap/vcap_api_debugfs.c         | 14 ++++-
 .../microchip/vcap/vcap_api_private.h         |  3 +
 3 files changed, 47 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api.c b/drivers/net/ethernet/microchip/vcap/vcap_api.c
index f1dc4fd6bb96..198c36627ba1 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api.c
@@ -2170,47 +2170,53 @@ void vcap_free_rule(struct vcap_rule *rule)
 }
 EXPORT_SYMBOL_GPL(vcap_free_rule);
 
-struct vcap_rule *vcap_get_rule(struct vcap_control *vctrl, u32 id)
+/* Decode a rule from the VCAP cache and return a copy */
+struct vcap_rule *vcap_decode_rule(struct vcap_rule_internal *elem)
 {
-	struct vcap_rule_internal *elem;
 	struct vcap_rule_internal *ri;
 	int err;
 
-	ri = NULL;
-
-	err = vcap_api_check(vctrl);
-	if (err)
-		return ERR_PTR(err);
-	elem = vcap_lookup_rule(vctrl, id);
-	if (!elem)
-		return NULL;
-	mutex_lock(&elem->admin->lock);
 	ri = vcap_dup_rule(elem, elem->state == VCAP_RS_DISABLED);
 	if (IS_ERR(ri))
-		goto unlock;
+		return ERR_PTR(PTR_ERR(ri));
 
 	if (ri->state == VCAP_RS_DISABLED)
-		goto unlock;
+		goto out;
 
 	err = vcap_read_rule(ri);
-	if (err) {
-		ri = ERR_PTR(err);
-		goto unlock;
-	}
+	if (err)
+		return ERR_PTR(err);
+
 	err = vcap_decode_keyset(ri);
-	if (err) {
-		ri = ERR_PTR(err);
-		goto unlock;
-	}
+	if (err)
+		return ERR_PTR(err);
+
 	err = vcap_decode_actionset(ri);
-	if (err) {
-		ri = ERR_PTR(err);
-		goto unlock;
-	}
+	if (err)
+		return ERR_PTR(err);
 
-unlock:
+out:
+	return &ri->data;
+}
+
+struct vcap_rule *vcap_get_rule(struct vcap_control *vctrl, u32 id)
+{
+	struct vcap_rule_internal *elem;
+	struct vcap_rule *rule;
+	int err;
+
+	err = vcap_api_check(vctrl);
+	if (err)
+		return ERR_PTR(err);
+
+	elem = vcap_lookup_rule(vctrl, id);
+	if (!elem)
+		return NULL;
+
+	mutex_lock(&elem->admin->lock);
+	rule = vcap_decode_rule(elem);
 	mutex_unlock(&elem->admin->lock);
-	return (struct vcap_rule *)ri;
+	return rule;
 }
 EXPORT_SYMBOL_GPL(vcap_get_rule);
 
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c b/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c
index dc06f6d4f513..d49b1cf7712f 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c
@@ -295,7 +295,7 @@ static int vcap_show_admin(struct vcap_control *vctrl,
 
 	vcap_show_admin_info(vctrl, admin, out);
 	list_for_each_entry(elem, &admin->rules, list) {
-		vrule = vcap_get_rule(vctrl, elem->data.id);
+		vrule = vcap_decode_rule(elem);
 		if (IS_ERR_OR_NULL(vrule)) {
 			ret = PTR_ERR(vrule);
 			break;
@@ -404,8 +404,12 @@ static int vcap_debugfs_show(struct seq_file *m, void *unused)
 		.prf = (void *)seq_printf,
 		.dst = m,
 	};
+	int ret;
 
-	return vcap_show_admin(info->vctrl, info->admin, &out);
+	mutex_lock(&info->admin->lock);
+	ret = vcap_show_admin(info->vctrl, info->admin, &out);
+	mutex_unlock(&info->admin->lock);
+	return ret;
 }
 DEFINE_SHOW_ATTRIBUTE(vcap_debugfs);
 
@@ -417,8 +421,12 @@ static int vcap_raw_debugfs_show(struct seq_file *m, void *unused)
 		.prf = (void *)seq_printf,
 		.dst = m,
 	};
+	int ret;
 
-	return vcap_show_admin_raw(info->vctrl, info->admin, &out);
+	mutex_lock(&info->admin->lock);
+	ret = vcap_show_admin_raw(info->vctrl, info->admin, &out);
+	mutex_unlock(&info->admin->lock);
+	return ret;
 }
 DEFINE_SHOW_ATTRIBUTE(vcap_raw_debugfs);
 
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_private.h b/drivers/net/ethernet/microchip/vcap/vcap_api_private.h
index 86542accffe6..df81d9ff502b 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_private.h
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_private.h
@@ -118,4 +118,7 @@ int vcap_find_keystream_keysets(struct vcap_control *vctrl, enum vcap_type vt,
 /* Get the keysets that matches the rule key type/mask */
 int vcap_rule_get_keysets(struct vcap_rule_internal *ri,
 			  struct vcap_keyset_list *matches);
+/* Decode a rule from the VCAP cache and return a copy */
+struct vcap_rule *vcap_decode_rule(struct vcap_rule_internal *elem);
+
 #endif /* __VCAP_API_PRIVATE__ */
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 4/5] net: microchip: sparx5: Improve VCAP admin locking in the VCAP API
  2023-01-17  8:55 [PATCH net-next 0/5] Improve locking in the VCAP API Steen Hegelund
                   ` (2 preceding siblings ...)
  2023-01-17  8:55 ` [PATCH net-next 3/5] net: microchip: sparx5: Add VCAP admin locking in debugFS Steen Hegelund
@ 2023-01-17  8:55 ` Steen Hegelund
  2023-01-17  8:55 ` [PATCH net-next 5/5] net: microchip: sparx5: Add lock initialization to the KUNIT tests Steen Hegelund
  2023-01-18 14:40 ` [PATCH net-next 0/5] Improve locking in the VCAP API patchwork-bot+netdevbpf
  5 siblings, 0 replies; 7+ messages in thread
From: Steen Hegelund @ 2023-01-17  8:55 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Steen Hegelund, UNGLinuxDriver, Randy Dunlap, Casper Andersson,
	Russell King, Wan Jiabing, Nathan Huckleberry, linux-kernel,
	netdev, linux-arm-kernel, Steen Hegelund, Daniel Machon,
	Horatiu Vultur, Lars Povlsen, Dan Carpenter, Michael Walle

This improves the VCAP cache and the VCAP rule list protection against
access from different sources.

Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
---
 .../net/ethernet/microchip/vcap/vcap_api.c    | 97 +++++++++++++------
 1 file changed, 67 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api.c b/drivers/net/ethernet/microchip/vcap/vcap_api.c
index 198c36627ba1..71f787a78295 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api.c
@@ -934,18 +934,21 @@ static bool vcap_rule_exists(struct vcap_control *vctrl, u32 id)
 	return false;
 }
 
-/* Find a rule with a provided rule id */
-static struct vcap_rule_internal *vcap_lookup_rule(struct vcap_control *vctrl,
-						   u32 id)
+/* Find a rule with a provided rule id return a locked vcap */
+static struct vcap_rule_internal *
+vcap_get_locked_rule(struct vcap_control *vctrl, u32 id)
 {
 	struct vcap_rule_internal *ri;
 	struct vcap_admin *admin;
 
 	/* Look for the rule id in all vcaps */
-	list_for_each_entry(admin, &vctrl->list, list)
+	list_for_each_entry(admin, &vctrl->list, list) {
+		mutex_lock(&admin->lock);
 		list_for_each_entry(ri, &admin->rules, list)
 			if (ri->data.id == id)
 				return ri;
+		mutex_unlock(&admin->lock);
+	}
 	return NULL;
 }
 
@@ -954,12 +957,21 @@ int vcap_lookup_rule_by_cookie(struct vcap_control *vctrl, u64 cookie)
 {
 	struct vcap_rule_internal *ri;
 	struct vcap_admin *admin;
+	int id = 0;
 
 	/* Look for the rule id in all vcaps */
-	list_for_each_entry(admin, &vctrl->list, list)
-		list_for_each_entry(ri, &admin->rules, list)
-			if (ri->data.cookie == cookie)
-				return ri->data.id;
+	list_for_each_entry(admin, &vctrl->list, list) {
+		mutex_lock(&admin->lock);
+		list_for_each_entry(ri, &admin->rules, list) {
+			if (ri->data.cookie == cookie) {
+				id = ri->data.id;
+				break;
+			}
+		}
+		mutex_unlock(&admin->lock);
+		if (id)
+			return id;
+	}
 	return -ENOENT;
 }
 EXPORT_SYMBOL_GPL(vcap_lookup_rule_by_cookie);
@@ -2116,17 +2128,28 @@ struct vcap_rule *vcap_alloc_rule(struct vcap_control *vctrl,
 	/* Sanity check that this VCAP is supported on this platform */
 	if (vctrl->vcaps[admin->vtype].rows == 0)
 		return ERR_PTR(-EINVAL);
+
+	mutex_lock(&admin->lock);
 	/* Check if a rule with this id already exists */
-	if (vcap_rule_exists(vctrl, id))
-		return ERR_PTR(-EEXIST);
+	if (vcap_rule_exists(vctrl, id)) {
+		err = -EINVAL;
+		goto out_unlock;
+	}
+
 	/* Check if there is room for the rule in the block(s) of the VCAP */
 	maxsize = vctrl->vcaps[admin->vtype].sw_count; /* worst case rule size */
-	if (vcap_rule_space(admin, maxsize))
-		return ERR_PTR(-ENOSPC);
+	if (vcap_rule_space(admin, maxsize)) {
+		err = -ENOSPC;
+		goto out_unlock;
+	}
+
 	/* Create a container for the rule and return it */
 	ri = kzalloc(sizeof(*ri), GFP_KERNEL);
-	if (!ri)
-		return ERR_PTR(-ENOMEM);
+	if (!ri) {
+		err = -ENOMEM;
+		goto out_unlock;
+	}
+
 	ri->data.vcap_chain_id = vcap_chain_id;
 	ri->data.user = user;
 	ri->data.priority = priority;
@@ -2139,13 +2162,21 @@ struct vcap_rule *vcap_alloc_rule(struct vcap_control *vctrl,
 	ri->ndev = ndev;
 	ri->admin = admin; /* refer to the vcap instance */
 	ri->vctrl = vctrl; /* refer to the client */
-	if (vcap_set_rule_id(ri) == 0)
+
+	if (vcap_set_rule_id(ri) == 0) {
+		err = -EINVAL;
 		goto out_free;
+	}
+
+	mutex_unlock(&admin->lock);
 	return (struct vcap_rule *)ri;
 
 out_free:
 	kfree(ri);
-	return ERR_PTR(-EINVAL);
+out_unlock:
+	mutex_unlock(&admin->lock);
+	return ERR_PTR(err);
+
 }
 EXPORT_SYMBOL_GPL(vcap_alloc_rule);
 
@@ -2209,11 +2240,10 @@ struct vcap_rule *vcap_get_rule(struct vcap_control *vctrl, u32 id)
 	if (err)
 		return ERR_PTR(err);
 
-	elem = vcap_lookup_rule(vctrl, id);
+	elem = vcap_get_locked_rule(vctrl, id);
 	if (!elem)
 		return NULL;
 
-	mutex_lock(&elem->admin->lock);
 	rule = vcap_decode_rule(elem);
 	mutex_unlock(&elem->admin->lock);
 	return rule;
@@ -2231,11 +2261,9 @@ int vcap_mod_rule(struct vcap_rule *rule)
 	if (err)
 		return err;
 
-	if (!vcap_lookup_rule(ri->vctrl, ri->data.id))
+	if (!vcap_get_locked_rule(ri->vctrl, ri->data.id))
 		return -ENOENT;
 
-	mutex_lock(&ri->admin->lock);
-
 	vcap_rule_set_state(ri);
 	if (ri->state == VCAP_RS_DISABLED)
 		goto out;
@@ -2252,8 +2280,6 @@ int vcap_mod_rule(struct vcap_rule *rule)
 
 	memset(&ctr, 0, sizeof(ctr));
 	err =  vcap_write_counter(ri, &ctr);
-	if (err)
-		goto out;
 
 out:
 	mutex_unlock(&ri->admin->lock);
@@ -2320,20 +2346,19 @@ int vcap_del_rule(struct vcap_control *vctrl, struct net_device *ndev, u32 id)
 	if (err)
 		return err;
 	/* Look for the rule id in all vcaps */
-	ri = vcap_lookup_rule(vctrl, id);
+	ri = vcap_get_locked_rule(vctrl, id);
 	if (!ri)
-		return -EINVAL;
+		return -ENOENT;
+
 	admin = ri->admin;
 
 	if (ri->addr > admin->last_used_addr)
 		gap = vcap_fill_rule_gap(ri);
 
 	/* Delete the rule from the list of rules and the cache */
-	mutex_lock(&admin->lock);
 	list_del(&ri->list);
 	vctrl->ops->init(ndev, admin, admin->last_used_addr, ri->size + gap);
 	vcap_free_rule(&ri->data);
-	mutex_unlock(&admin->lock);
 
 	/* Update the last used address, set to default when no rules */
 	if (list_empty(&admin->rules)) {
@@ -2343,7 +2368,9 @@ int vcap_del_rule(struct vcap_control *vctrl, struct net_device *ndev, u32 id)
 				       list);
 		admin->last_used_addr = elem->addr;
 	}
-	return 0;
+
+	mutex_unlock(&admin->lock);
+	return err;
 }
 EXPORT_SYMBOL_GPL(vcap_del_rule);
 
@@ -3021,7 +3048,12 @@ int vcap_rule_set_counter(struct vcap_rule *rule, struct vcap_counter *ctr)
 		pr_err("%s:%d: counter is missing\n", __func__, __LINE__);
 		return -EINVAL;
 	}
-	return vcap_write_counter(ri, ctr);
+
+	mutex_lock(&ri->admin->lock);
+	err = vcap_write_counter(ri, ctr);
+	mutex_unlock(&ri->admin->lock);
+
+	return err;
 }
 EXPORT_SYMBOL_GPL(vcap_rule_set_counter);
 
@@ -3037,7 +3069,12 @@ int vcap_rule_get_counter(struct vcap_rule *rule, struct vcap_counter *ctr)
 		pr_err("%s:%d: counter is missing\n", __func__, __LINE__);
 		return -EINVAL;
 	}
-	return vcap_read_counter(ri, ctr);
+
+	mutex_lock(&ri->admin->lock);
+	err = vcap_read_counter(ri, ctr);
+	mutex_unlock(&ri->admin->lock);
+
+	return err;
 }
 EXPORT_SYMBOL_GPL(vcap_rule_get_counter);
 
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 5/5] net: microchip: sparx5: Add lock initialization to the KUNIT tests
  2023-01-17  8:55 [PATCH net-next 0/5] Improve locking in the VCAP API Steen Hegelund
                   ` (3 preceding siblings ...)
  2023-01-17  8:55 ` [PATCH net-next 4/5] net: microchip: sparx5: Improve VCAP admin locking in the VCAP API Steen Hegelund
@ 2023-01-17  8:55 ` Steen Hegelund
  2023-01-18 14:40 ` [PATCH net-next 0/5] Improve locking in the VCAP API patchwork-bot+netdevbpf
  5 siblings, 0 replies; 7+ messages in thread
From: Steen Hegelund @ 2023-01-17  8:55 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Steen Hegelund, UNGLinuxDriver, Randy Dunlap, Casper Andersson,
	Russell King, Wan Jiabing, Nathan Huckleberry, linux-kernel,
	netdev, linux-arm-kernel, Steen Hegelund, Daniel Machon,
	Horatiu Vultur, Lars Povlsen, Dan Carpenter, Michael Walle

Ensure that the KUNIT tests lock instance is initialized before the test is
executed.

Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
---
 drivers/net/ethernet/microchip/vcap/vcap_api_debugfs_kunit.c | 1 +
 drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c         | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs_kunit.c b/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs_kunit.c
index 9211cb453a3d..cbf7e0f110b8 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs_kunit.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs_kunit.c
@@ -246,6 +246,7 @@ static void vcap_test_api_init(struct vcap_admin *admin)
 	INIT_LIST_HEAD(&admin->list);
 	INIT_LIST_HEAD(&admin->rules);
 	INIT_LIST_HEAD(&admin->enabled);
+	mutex_init(&admin->lock);
 	list_add_tail(&admin->list, &test_vctrl.list);
 	memset(test_updateaddr, 0, sizeof(test_updateaddr));
 	test_updateaddridx = 0;
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
index 22690c669028..82981176218c 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
@@ -236,6 +236,7 @@ static void vcap_test_api_init(struct vcap_admin *admin)
 	INIT_LIST_HEAD(&admin->list);
 	INIT_LIST_HEAD(&admin->rules);
 	INIT_LIST_HEAD(&admin->enabled);
+	mutex_init(&admin->lock);
 	list_add_tail(&admin->list, &test_vctrl.list);
 	memset(test_updateaddr, 0, sizeof(test_updateaddr));
 	test_updateaddridx = 0;
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next 0/5] Improve locking in the VCAP API
  2023-01-17  8:55 [PATCH net-next 0/5] Improve locking in the VCAP API Steen Hegelund
                   ` (4 preceding siblings ...)
  2023-01-17  8:55 ` [PATCH net-next 5/5] net: microchip: sparx5: Add lock initialization to the KUNIT tests Steen Hegelund
@ 2023-01-18 14:40 ` patchwork-bot+netdevbpf
  5 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-01-18 14:40 UTC (permalink / raw)
  To: Steen Hegelund
  Cc: davem, edumazet, kuba, pabeni, UNGLinuxDriver, rdunlap,
	casper.casan, rmk+kernel, wanjiabing, nhuck, linux-kernel,
	netdev, linux-arm-kernel, Steen.Hegelund, daniel.machon,
	horatiu.vultur, lars.povlsen, error27, michael

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Tue, 17 Jan 2023 09:55:39 +0100 you wrote:
> This improves the VCAP cache and the VCAP rule list protection against
> access from different sources.
> 
> The VCAP Admin lock protects the list of rules for the VCAP instance as
> well as the cache used for encoding and decoding rules.
> 
> This series provides dedicated functions for accessing rule statistics,
> decoding rule content, verifying if a rule exists and getting a rule with
> the lock held, as well as ensuring the use of the lock when the list of
> rules or the cache is accessed.
> 
> [...]

Here is the summary with links:
  - [net-next,1/5] net: microchip: sparx5: Add support for rule count by cookie
    https://git.kernel.org/netdev/net-next/c/27d293cceee5
  - [net-next,2/5] net: microchip: sparx5: Add support to check for existing VCAP rule id
    https://git.kernel.org/netdev/net-next/c/975d86acaec7
  - [net-next,3/5] net: microchip: sparx5: Add VCAP admin locking in debugFS
    https://git.kernel.org/netdev/net-next/c/9579e2c271b4
  - [net-next,4/5] net: microchip: sparx5: Improve VCAP admin locking in the VCAP API
    https://git.kernel.org/netdev/net-next/c/1972b6d927ac
  - [net-next,5/5] net: microchip: sparx5: Add lock initialization to the KUNIT tests
    https://git.kernel.org/netdev/net-next/c/595655e08174

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-01-18 14:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-17  8:55 [PATCH net-next 0/5] Improve locking in the VCAP API Steen Hegelund
2023-01-17  8:55 ` [PATCH net-next 1/5] net: microchip: sparx5: Add support for rule count by cookie Steen Hegelund
2023-01-17  8:55 ` [PATCH net-next 2/5] net: microchip: sparx5: Add support to check for existing VCAP rule id Steen Hegelund
2023-01-17  8:55 ` [PATCH net-next 3/5] net: microchip: sparx5: Add VCAP admin locking in debugFS Steen Hegelund
2023-01-17  8:55 ` [PATCH net-next 4/5] net: microchip: sparx5: Improve VCAP admin locking in the VCAP API Steen Hegelund
2023-01-17  8:55 ` [PATCH net-next 5/5] net: microchip: sparx5: Add lock initialization to the KUNIT tests Steen Hegelund
2023-01-18 14:40 ` [PATCH net-next 0/5] Improve locking in the VCAP API patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).