On 02/08/2021 14:36, Vladimir Oltean wrote:
> Nikolay points out that it is incorrect to assume that it is impossible
> to have an fdb entry with fdb->dst == NULL and the BR_FDB_LOCAL bit in
> fdb->flags not set. This is because there are reader-side places that
> test_bit(BR_FDB_LOCAL, &fdb->flags) without the br->hash_lock, and if
> the updating of the FDB entry happens on another CPU, there are no
> memory barriers at writer or reader side which would ensure that the
> reader sees the updates to both fdb->flags and fdb->dst in the same
> order, i.e. the reader will not see an inconsistent FDB entry.
>
> So we must be prepared to deal with FDB entries where fdb->dst and
> fdb->flags are in a potentially inconsistent state, and that means that
> fdb->dst == NULL should remain a condition to pick the net_device that
> we report to switchdev as being the bridge device, which is what the
> code did prior to the blamed patch.
>
> Fixes: 52e4bec15546 ("net: bridge: switchdev: treat local FDBs the same as entries towards the bridge")
> Suggested-by: Nikolay Aleksandrov <nikolay@xxxxxxxxxx>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@xxxxxxx>
> ---
> net/bridge/br_fdb.c | 2 +-
> net/bridge/br_switchdev.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 4ff8c67ac88f..af31cebfda94 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -745,7 +745,7 @@ static int br_fdb_replay_one(struct net_bridge *br, struct notifier_block *nb,
> item.added_by_user = test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags);
> item.offloaded = test_bit(BR_FDB_OFFLOADED, &fdb->flags);
> item.is_local = test_bit(BR_FDB_LOCAL, &fdb->flags);
> - item.info.dev = item.is_local ? br->dev : p->dev;
> + item.info.dev = (!p || item.is_local) ? br->dev : p->dev;
> item.info.ctx = ctx;
>
> err = nb->notifier_call(nb, action, &item);
> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
> index 023de0e958f1..36d75fd4a80c 100644
> --- a/net/bridge/br_switchdev.c
> +++ b/net/bridge/br_switchdev.c
> @@ -134,7 +134,7 @@ br_switchdev_fdb_notify(struct net_bridge *br,
> .is_local = test_bit(BR_FDB_LOCAL, &fdb->flags),
> .offloaded = test_bit(BR_FDB_OFFLOADED, &fdb->flags),
> };
> - struct net_device *dev = info.is_local ? br->dev : dst->dev;
> + struct net_device *dev = (!dst || info.is_local) ? br->dev : dst->dev;
>
> switch (type) {
> case RTM_DELNEIGH:
>
Thanks,
Acked-by: Nikolay Aleksandrov <nikolay@xxxxxxxxxx>
