Commit 74661bee authored by David S. Miller's avatar David S. Miller
Browse files

Merge branch 'switchdev-locking' 



Jiri Pirko says:

====================
switchdev: change locking

This is something which I'm currently struggling with.
Callers of attr_set and obj_add/del often hold not only RTNL, but also
spinlock (bridge). So in that case, the driver implementing the op cannot sleep.

The way rocker is dealing with this now is just to invoke driver operation
and go out, without any checking or reporting of the operation status.

Since it would be nice to at least put a warning in case the operation fails,
it makes sense to do this in delayed work directly in switchdev core
instead of implementing this in separate drivers. And that is what this patchset
is introducing.

So from now on, the locking of switchdev mod ops is consistent. Caller either
holds rtnl mutex or in case it does not, caller sets defer flag, telling
switchdev core to process the op later, in deferred queue.

Function to force to process switchdev deferred ops can be called by op
caller in appropriate location, for example after it releases
spin lock, to force switchdev core to process pending ops.

v1->v2:
- rebased on current net-next head (including Scott's ageing patchset)
v2->v3:
- fixed comment s/of/or/ typo suggested by Nik
v3->v4:
- the actual patchset is sent instead of different branch I send in v3 :/
v4->v5:
- added patch to "const" attr param
- reworked deferred ops infrastructure (mainly patch number 1 and
  internal users (patch 3 and 5)) - resolves the issue pointed out
  by John
====================

Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents adc9048c 771acac2

drivers/net/ethernet/rocker/rocker.c

+7 −8
Original line number Diff line number Diff line
@@ -3672,7 +3672,7 @@ static int rocker_port_fdb_flush(struct rocker_port *rocker_port, 
	    rocker_port->stp_state == BR_STATE_FORWARDING)

		return 0;



	flags |= ROCKER_OP_FLAG_REMOVE;

	flags |= ROCKER_OP_FLAG_NOWAIT | ROCKER_OP_FLAG_REMOVE;



	spin_lock_irqsave(&rocker->fdb_tbl_lock, lock_flags);
@@ -4374,7 +4374,7 @@ static int rocker_port_bridge_ageing_time(struct rocker_port *rocker_port, 
}



static int rocker_port_attr_set(struct net_device *dev,

				struct switchdev_attr *attr,

				const struct switchdev_attr *attr,

				struct switchdev_trans *trans)

{

	struct rocker_port *rocker_port = netdev_priv(dev);
@@ -4382,8 +4382,7 @@ static int rocker_port_attr_set(struct net_device *dev, 


	switch (attr->id) {

	case SWITCHDEV_ATTR_ID_PORT_STP_STATE:

		err = rocker_port_stp_update(rocker_port, trans,

					     ROCKER_OP_FLAG_NOWAIT,

		err = rocker_port_stp_update(rocker_port, trans, 0,

					     attr->u.stp_state);

		break;

	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
@@ -4469,7 +4468,7 @@ static int rocker_port_obj_add(struct net_device *dev, 
		fib4 = SWITCHDEV_OBJ_IPV4_FIB(obj);

		err = rocker_port_fib_ipv4(rocker_port, trans,

					   htonl(fib4->dst), fib4->dst_len,

					   fib4->fi, fib4->tb_id, 0);

					   &fib4->fi, fib4->tb_id, 0);

		break;

	case SWITCHDEV_OBJ_ID_PORT_FDB:

		err = rocker_port_fdb_add(rocker_port, trans,
@@ -4517,7 +4516,7 @@ static int rocker_port_fdb_del(struct rocker_port *rocker_port, 
			       const struct switchdev_obj_port_fdb *fdb)

{

	__be16 vlan_id = rocker_port_vid_to_vlan(rocker_port, fdb->vid, NULL);

	int flags = ROCKER_OP_FLAG_NOWAIT | ROCKER_OP_FLAG_REMOVE;

	int flags = ROCKER_OP_FLAG_REMOVE;



	if (!rocker_port_is_bridged(rocker_port))

		return -EINVAL;
@@ -4541,7 +4540,7 @@ static int rocker_port_obj_del(struct net_device *dev, 
		fib4 = SWITCHDEV_OBJ_IPV4_FIB(obj);

		err = rocker_port_fib_ipv4(rocker_port, NULL,

					   htonl(fib4->dst), fib4->dst_len,

					   fib4->fi, fib4->tb_id,

					   &fib4->fi, fib4->tb_id,

					   ROCKER_OP_FLAG_REMOVE);

		break;

	case SWITCHDEV_OBJ_ID_PORT_FDB:
@@ -4571,7 +4570,7 @@ static int rocker_port_fdb_dump(const struct rocker_port *rocker_port, 
	hash_for_each_safe(rocker->fdb_tbl, bkt, tmp, found, entry) {

		if (found->key.rocker_port != rocker_port)

			continue;

		fdb->addr = found->key.addr;

		ether_addr_copy(fdb->addr, found->key.addr);

		fdb->ndm_state = NUD_REACHABLE;

		fdb->vid = rocker_port_vlan_to_vid(rocker_port,

						   found->key.vlan_id);

include/net/switchdev.h

+13 −7
Original line number Diff line number Diff line
@@ -14,9 +14,11 @@ 
#include <linux/netdevice.h>

#include <linux/notifier.h>

#include <linux/list.h>

#include <net/ip_fib.h>



#define SWITCHDEV_F_NO_RECURSE		BIT(0)

#define SWITCHDEV_F_SKIP_EOPNOTSUPP	BIT(1)

#define SWITCHDEV_F_DEFER		BIT(2)



struct switchdev_trans_item {

	struct list_head list;
@@ -58,8 +60,6 @@ struct switchdev_attr { 
	} u;

};



struct fib_info;



enum switchdev_obj_id {

	SWITCHDEV_OBJ_ID_UNDEFINED,

	SWITCHDEV_OBJ_ID_PORT_VLAN,
@@ -69,6 +69,7 @@ enum switchdev_obj_id { 


struct switchdev_obj {

	enum switchdev_obj_id id;

	u32 flags;

};



/* SWITCHDEV_OBJ_ID_PORT_VLAN */
@@ -87,7 +88,7 @@ struct switchdev_obj_ipv4_fib { 
	struct switchdev_obj obj;

	u32 dst;

	int dst_len;

	struct fib_info *fi;

	struct fib_info fi;

	u8 tos;

	u8 type;

	u32 nlflags;
@@ -100,7 +101,7 @@ struct switchdev_obj_ipv4_fib { 
/* SWITCHDEV_OBJ_ID_PORT_FDB */

struct switchdev_obj_port_fdb {

	struct switchdev_obj obj;

	const unsigned char *addr;

	unsigned char addr[ETH_ALEN];

	u16 vid;

	u16 ndm_state;

};
@@ -132,7 +133,7 @@ struct switchdev_ops { 
	int	(*switchdev_port_attr_get)(struct net_device *dev,

					   struct switchdev_attr *attr);

	int	(*switchdev_port_attr_set)(struct net_device *dev,

					   struct switchdev_attr *attr,

					   const struct switchdev_attr *attr,

					   struct switchdev_trans *trans);

	int	(*switchdev_port_obj_add)(struct net_device *dev,

					  const struct switchdev_obj *obj,
@@ -167,10 +168,11 @@ switchdev_notifier_info_to_dev(const struct switchdev_notifier_info *info) 


#ifdef CONFIG_NET_SWITCHDEV



void switchdev_deferred_process(void);

int switchdev_port_attr_get(struct net_device *dev,

			    struct switchdev_attr *attr);

int switchdev_port_attr_set(struct net_device *dev,

			    struct switchdev_attr *attr);

			    const struct switchdev_attr *attr);

int switchdev_port_obj_add(struct net_device *dev,

			   const struct switchdev_obj *obj);

int switchdev_port_obj_del(struct net_device *dev,
@@ -208,6 +210,10 @@ void switchdev_port_fwd_mark_set(struct net_device *dev, 


#else



static inline void switchdev_deferred_process(void)

{

}



static inline int switchdev_port_attr_get(struct net_device *dev,

					  struct switchdev_attr *attr)

{
@@ -215,7 +221,7 @@ static inline int switchdev_port_attr_get(struct net_device *dev, 
}



static inline int switchdev_port_attr_set(struct net_device *dev,

					  struct switchdev_attr *attr)

					  const struct switchdev_attr *attr)

{

	return -EOPNOTSUPP;

}

net/bridge/br_fdb.c

+5 −2
Original line number Diff line number Diff line
@@ -134,11 +134,14 @@ static void fdb_del_hw_addr(struct net_bridge *br, const unsigned char *addr) 
static void fdb_del_external_learn(struct net_bridge_fdb_entry *f)

{

	struct switchdev_obj_port_fdb fdb = {

		.obj.id = SWITCHDEV_OBJ_ID_PORT_FDB,

		.addr = f->addr.addr,

		.obj = {

			.id = SWITCHDEV_OBJ_ID_PORT_FDB,

			.flags = SWITCHDEV_F_DEFER,

		},

		.vid = f->vlan_id,

	};



	ether_addr_copy(fdb.addr, f->addr.addr);

	switchdev_port_obj_del(f->dst->dev, &fdb.obj);

}

net/bridge/br_if.c

+3 −0
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ 
#include <linux/slab.h>

#include <net/sock.h>

#include <linux/if_vlan.h>

#include <net/switchdev.h>



#include "br_private.h"
@@ -250,6 +251,8 @@ static void del_nbp(struct net_bridge_port *p) 


	nbp_vlan_flush(p);

	br_fdb_delete_by_port(br, p, 0, 1);

	switchdev_deferred_process();



	nbp_update_port_count(br);



	netdev_upper_dev_unlink(dev, br->dev);

net/bridge/br_stp.c

+2 −1
Original line number Diff line number Diff line
@@ -41,13 +41,14 @@ void br_set_state(struct net_bridge_port *p, unsigned int state) 
{

	struct switchdev_attr attr = {

		.id = SWITCHDEV_ATTR_ID_PORT_STP_STATE,

		.flags = SWITCHDEV_F_DEFER,

		.u.stp_state = state,

	};

	int err;



	p->state = state;

	err = switchdev_port_attr_set(p->dev, &attr);

	if (err && err != -EOPNOTSUPP)

	if (err)

		br_warn(p->br, "error setting offload STP state on port %u(%s)\n",

				(unsigned int) p->port_no, p->dev->name);

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE