Bug #164
closedLocal translation table update on a bat0 MAC change
0%
Description
Using batman-adv 2012.3.0
When I change the MAC addresse of bat0, the local translation table is not well updated.
root@openwrt:/# batctl tl Locally retrieved addresses (from bat0) announced via TT (TTVN: 184): * 26:aa:aa:aa:aa:aa [.P...] * xx:xx:xx:xx:xx:xx [.....] * xx:xx:xx:xx:xx:xx [.....] root@openwrt:/# ifconfig bat0 hw ether 36:aa:aa:aa:aa:aa root@openwrt:/# batctl tl Locally retrieved addresses (from bat0) announced via TT (TTVN: 186): * 26:aa:aa:aa:aa:aa [.P...] * 36:aa:aa:aa:aa:aa [.....] * xx:xx:xx:xx:xx:xx [.....] * xx:xx:xx:xx:xx:xx [.....]
--> The addesse 26:aa:aa:aa:aa:aa is not deleted because tagged as NoPurge.
--> The addesse 36:aa:aa:aa:aa:aa is not tagged as NoPurge.
static int batadv_interface_set_mac_addr(struct net_device *dev, void *p) { struct batadv_priv *bat_priv = netdev_priv(dev); struct sockaddr *addr = p; if (!is_valid_ether_addr(addr->sa_data)) return -EADDRNOTAVAIL; /* only modify transtable if it has been initialized before */ if (atomic_read(&bat_priv->mesh_state) == BATADV_MESH_ACTIVE) { batadv_tt_local_remove(bat_priv, dev->dev_addr, "mac address changed", false); batadv_tt_local_add(dev, addr->sa_data, BATADV_NULL_IFINDEX); } memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN); dev->addr_assign_type &= ~NET_ADDR_RANDOM; return 0; }
I think the addesse 36:aa:aa:aa:aa:aa is not tagged as NoPurged because the function batadv_tt_local_add is called before the update of mac addresse memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
I'm right ?
Thanks.
Files
Updated by Antonio Quartulli over 12 years ago
Def D wrote:
I think the addesse 36:aa:aa:aa:aa:aa is not tagged as NoPurged because the function batadv_tt_local_add is called before the update of mac addresse memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
I'm right ?
Exactly. In tt_local_add() the code compares the new address with the bat0 one and if they are equal it adds the NoPurge flag.
We need to copy the address in 'dev->dev_addr' before invoking tt_local_add, but then we have to pay attention to invoke tt_loval_del with the correct argument.
are you going to send a patch to fix this?
Updated by Def D over 12 years ago
- File batman-adv_001_fix_softiface_mac_update.patch batman-adv_001_fix_softiface_mac_update.patch added
- File batman-adv_002_fix_softiface_mac_update_remove_old_tt_entry.patch batman-adv_002_fix_softiface_mac_update_remove_old_tt_entry.patch added
Antonio Quartulli wrote:
We need to copy the address in 'dev->dev_addr' before invoking tt_local_add, but then we have to pay attention to invoke tt_loval_del with the correct argument.
Thank you
Antonio Quartulli wrote:
are you going to send a patch to fix this?
I can try
- batman-adv_001_fix_softiface_mac_update.patch
Update dev->dev_addr before calling tt_local_add into function interface_set_mac_addr
- batman-adv_002_fix_softiface_mac_update_remove_old_tt_entry.patch
To allow removing old MAC addresse: Add argument force in function tt_local_remove to remove flag NoPurge
Tested on my boards.
It is fine ?
Updated by Antonio Quartulli over 12 years ago
I don't think we really need Patch 002. The NOPURGE flag is used only to avoid to purge the entry in case of timeout, therefore in this case the deletion will happen anyway.
For patch 001, instead of adding an else branch, what about storing the old mac in a local variable, passing it to the tt_local_del and so moving the memcpy before the if-loop?
By the way, now I think it would be better to use the git-format-patch and send it over the ml.
Updated by Def D over 12 years ago
- Patch 001
Using a local variable to store old MAC address use a memcpy that can be avoided.
The else branch introduce a duplicate memcpy code to update dev->dev_addr, but it is more optimized. isnt it ?
- Patch 002
Ok, I understand why the patch is not relevant.
Updated by Def D about 12 years ago
- File batman-adv_001_fix_softiface_mac_update_v2.patch batman-adv_001_fix_softiface_mac_update_v2.patch added
Antonio Quartulli wrote:
For patch 001, instead of adding an else branch, what about storing the old mac in a local variable, passing it to the tt_local_del and so moving the memcpy before the if-loop?
I did as you suggested.
Using git-format-patch.
I will (re)send to mailing list.
Updated by Antonio Quartulli about 12 years ago
- Status changed from New to Resolved
Updated by Antonio Quartulli over 11 years ago
- Status changed from Resolved to Closed