Turning the BATADV_TT_CLIENT_ROAM handling in batadv_tt_local_add() into an atomic single claim changed the behaviour of the roaming check in three ways.
* the cleanup of the global entry was moved out of the block which is only entered when the roaming process was not handled yet. * the flag is now published before tt_global->roam_at is stamped, with the whole batadv_send_roam_adv() loop in between. Revert this handling but avoid sending the initial roaming-away announcements when another context took responsibility for it. To keep it more readable, extract it in a new function. Reported-by: Sashiko <[email protected]> Fixes: 8b4cee3c9355 ("batman-adv: tt: use atomic flag modifications") Signed-off-by: Sven Eckelmann <[email protected]> --- net/batman-adv/translation-table.c | 94 +++++++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 36 deletions(-) diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index 7907f1c6..b2e15dd1 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -667,6 +667,63 @@ static void batadv_tt_global_free(struct batadv_priv *bat_priv, batadv_tt_global_entry_put(tt_global); } +/** + * batadv_tt_local_add_roam() - handle roamed clients during batadv_tt_local_add() + * @bat_priv: the bat priv with all the mesh interface information + * @tt_global: the global TT entry + * @roamed_back: whether @tt_global roamed back + */ +static void batadv_tt_local_add_roam(struct batadv_priv *bat_priv, + struct batadv_tt_global_entry *tt_global, + bool roamed_back) +{ + struct batadv_tt_orig_list_entry *orig_entry; + struct hlist_head *head; + u16 old_flags; + + if (!tt_global) + return; + + /* Check whether it is a roaming, but don't do anything if the roaming + * process has already been handled + */ + if (atomic_read(&tt_global->common.flags) & BATADV_TT_CLIENT_ROAM) + return; + + if (!roamed_back) { + /* The global entry has to be marked as ROAMING and has to be + * kept for consistency purpose. + * + * batadv_tt_global_to_purge() evaluates roam_at as soon as it + * observes BATADV_TT_CLIENT_ROAM, so the timeout has to be + * stamped before the flag is published. Otherwise the entry can + * be deleted right away as "Roaming timeout". + */ + tt_global->roam_at = jiffies; + + /* The flag is claimed via an atomic test-and-set to make sure + * that only a single context announces the roaming + */ + old_flags = atomic_fetch_or(BATADV_TT_CLIENT_ROAM, + &tt_global->common.flags); + if (old_flags & BATADV_TT_CLIENT_ROAM) + return; + } + + /* These node are probably going to update their tt table */ + head = &tt_global->orig_list; + rcu_read_lock(); + hlist_for_each_entry_rcu(orig_entry, head, list) { + batadv_send_roam_adv(bat_priv, tt_global->common.addr, + tt_global->common.vid, + orig_entry->orig_node); + } + rcu_read_unlock(); + + if (roamed_back) + batadv_tt_global_free(bat_priv, tt_global, "Roaming canceled"); +} + /** * batadv_tt_local_add() - add a new client to the local table or update an * existing client @@ -685,14 +742,12 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr, { struct batadv_priv *bat_priv = netdev_priv(mesh_iface); struct batadv_tt_global_entry *tt_global = NULL; - struct batadv_tt_orig_list_entry *orig_entry; struct batadv_tt_local_entry *tt_local; struct net *net = dev_net(mesh_iface); struct net_device *in_dev = NULL; struct batadv_meshif_vlan *vlan; bool roamed_back = false; bool iif_is_wifi = false; - struct hlist_head *head; int packet_size_max; u8 new_remote_flags; bool ret = false; @@ -701,7 +756,6 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr, int table_size; u32 match_mark; u8 clear_flags; - u16 old_flags; u8 set_flags; u16 flags; @@ -821,39 +875,7 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr, batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS); check_roaming: - /* Check whether it is a roaming, but don't do anything if the roaming - * process has already been handled. The ROAM flag is claimed via an - * atomic test-and-set to make sure that only a single context handles - * the advertisement for a client - */ - old_flags = BATADV_TT_CLIENT_ROAM; - if (tt_global && - !(atomic_read(&tt_global->common.flags) & BATADV_TT_CLIENT_ROAM)) - old_flags = atomic_fetch_or(BATADV_TT_CLIENT_ROAM, - &tt_global->common.flags); - - if (tt_global && !(old_flags & BATADV_TT_CLIENT_ROAM)) { - /* These node are probably going to update their tt table */ - head = &tt_global->orig_list; - rcu_read_lock(); - hlist_for_each_entry_rcu(orig_entry, head, list) { - batadv_send_roam_adv(bat_priv, tt_global->common.addr, - tt_global->common.vid, - orig_entry->orig_node); - } - rcu_read_unlock(); - - if (!roamed_back) { - /* The global entry was marked as ROAMING and - * has to be kept for consistency purpose - */ - tt_global->roam_at = jiffies; - } - } - - /* clean up independent of the roaming advertisement handler */ - if (tt_global && roamed_back) - batadv_tt_global_free(bat_priv, tt_global, "Roaming canceled"); + batadv_tt_local_add_roam(bat_priv, tt_global, roamed_back); /* store the current remote flags before altering them. This helps * understanding is flags are changing or not -- 2.47.3
