On Mon, 02 Nov 2020 09:52:19 +0100 Eelco Chaudron wrote:

On 30 Oct 2020, at 22:28, Jakub Kicinski wrote:

quoted

quoted

@@ -1695,6 +1695,9 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, 

struct genl_info *info)
 	if (err)
 		goto err_destroy_ports;

+	/* So far only local changes have been made, now need the lock. */
+	ovs_lock();  

Should we move the lock below assignments to param?

Looks a little strange to protect stack variables with a global lock.  

You are right, I should have moved it down after the assignment. I will 
send out a v2.

quoted

Let's update the name of the label.  

Guess now it is, unlock and destroy meters, so what label are you 
looking for?

err_unlock_and_destroy_meters: which looks a bit long, or just 
err_unlock:

I feel like I saw some names like err_unlock_and_destroy_meters in OvS
code, but can't find them in this file right now.

I'd personally go for kist err_unlock, or maybe err_unlock_ovs as is
used in other functions in this file.

But as long as it starts with err_unlock it's fine by me :)