On 15-10-14 10:40 AM, Jiri Pirko wrote:
> From: Jiri Pirko <j...@mellanox.com>
>
> Caller should know if he can call attr_set directly (when holding RTNL)
> or if he has to defer the att_set processing for later.
>
> This also allows drivers to sleep inside attr_set and report operation
> status back to switchdev core. Switchdev core then warns if status is
> not ok, instead of silent errors happening in drivers.
>
> Benefit from newly introduced switchdev deferred ops infrastructure.
>
A nit but the patch description should note your setting the defer bit
on the bridge set state.
> Signed-off-by: Jiri Pirko <j...@mellanox.com>
> ---
> include/net/switchdev.h | 1 +
> net/bridge/br_stp.c | 3 +-
> net/switchdev/switchdev.c | 108
> ++++++++++++++++++----------------------------
> 3 files changed, 46 insertions(+), 66 deletions(-)
>
> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
> index d1c7f90..f7de6f8 100644
> --- a/include/net/switchdev.h
> +++ b/include/net/switchdev.h
> @@ -17,6 +17,7 @@
>
> #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;
> diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
> index db6d243de..80c34d7 100644
> --- a/net/bridge/br_stp.c
> +++ b/net/bridge/br_stp.c
> @@ -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,
> };
This creates a possible race (with 6/8) I think, please check!
In del_nbp() we call br_stp_disable_port() to set the port state
to BR_STATE_DISABLE and disabling learning events. But with this
patch it can be deferred. Also note the STP agent may be in userspace
which actually seems more likely the case because you likely want to
run some more modern variant of STP than the kernel supports.
So at some point in the future the driver will turn off learning. At
the same time we call br_fdb_delete_by_port which calls a deferred
set of fdb deletes.
I don't see how you guarantee learning is off before you start doing
the deletes here and possibly learning new addresses after the software
side believes the port is down.
So
br_stp_disable_port
br_fdb_delete_by_port
{fdb_del_external_learn}
[hw learns a fdb]
[hw disables learning]
What stops this from happening?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
