Hello list,
as some of you may know, each TT client store in a node local/global table is also accompanied by a set of flags some of which are of global interest (every node in the network needs to know if this flag is set or not).
However it may happen that a flag may be unset (or set) after that a TT client has already be announced. With the current implementation there is no way for the nodes to detect this event and to react accordingly.
With this patchset we introduce all those means needed to synchronize a selected subset of TT flags among the network. Flags belonging to this set are the "Synchronized TT flags".
Right now only the WIFI flag belongs to this set, but another on is already planned to be added soonish.
Cheers,
[This patchset is based on top of origin/simon/network-wide-multiif]
Antonio Quartulli (4): batman-adv: don't switch byte order too often if not needed batman-adv: invoke dev_get_by_index() outside of is_wifi_iface() batman-adv: improve the TT component to support runtime flag changes batman-adv: include the synch-flags when compute the global/local table CRC
hard-interface.c | 31 +++---------------------------- hard-interface.h | 1 - packet.h | 17 +++++++++++++++-- routing.c | 8 +++++--- translation-table.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 5 files changed, 73 insertions(+), 36 deletions(-)
From: Antonio Quartulli antonio@open-mesh.com
If possible, operations like ntohs/ntohl should not be performed too often. Use a variable to locally store the converted value and then use it.
Signed-off-by: Antonio Quartulli antonio@open-mesh.com --- routing.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/routing.c b/routing.c index 666f02d..4336361 100644 --- a/routing.c +++ b/routing.c @@ -947,6 +947,7 @@ int batadv_recv_bcast_packet(struct sk_buff *skb, int hdr_size = sizeof(*bcast_packet); int ret = NET_RX_DROP; int32_t seq_diff; + uint32_t seqno;
/* drop packet if it has not necessary minimum size */ if (unlikely(!pskb_may_pull(skb, hdr_size))) @@ -982,12 +983,13 @@ int batadv_recv_bcast_packet(struct sk_buff *skb,
spin_lock_bh(&orig_node->bcast_seqno_lock);
+ seqno = ntohl(bcast_packet->seqno); /* check whether the packet is a duplicate */ if (batadv_test_bit(orig_node->bcast_bits, orig_node->last_bcast_seqno, - ntohl(bcast_packet->seqno))) + seqno)) goto spin_unlock;
- seq_diff = ntohl(bcast_packet->seqno) - orig_node->last_bcast_seqno; + seq_diff = seqno - orig_node->last_bcast_seqno;
/* check whether the packet is old and the host just restarted. */ if (batadv_window_protected(bat_priv, seq_diff, @@ -998,7 +1000,7 @@ int batadv_recv_bcast_packet(struct sk_buff *skb, * if required. */ if (batadv_bit_get_packet(bat_priv, orig_node->bcast_bits, seq_diff, 1)) - orig_node->last_bcast_seqno = ntohl(bcast_packet->seqno); + orig_node->last_bcast_seqno = seqno;
spin_unlock_bh(&orig_node->bcast_seqno_lock);
On Sunday 13 October 2013 02:50:17 Antonio Quartulli wrote:
From: Antonio Quartulli antonio@open-mesh.com
If possible, operations like ntohs/ntohl should not be performed too often. Use a variable to locally store the converted value and then use it.
Signed-off-by: Antonio Quartulli antonio@open-mesh.com
routing.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
I don't quite see why this patch is part of this series.
Cheers, Marek
On Sun, Oct 13, 2013 at 05:44:27PM +0800, Marek Lindner wrote:
On Sunday 13 October 2013 02:50:17 Antonio Quartulli wrote:
From: Antonio Quartulli antonio@open-mesh.com
If possible, operations like ntohs/ntohl should not be performed too often. Use a variable to locally store the converted value and then use it.
Signed-off-by: Antonio Quartulli antonio@open-mesh.com
routing.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
I don't quite see why this patch is part of this series.
because at the very beginning I wrongly did this change within patch 2/4 and Simon correctly suggested me to separate it....
So since then it has been part of this series, but there is no technical reason. It is just yet another patch which is not really related to the tt-synch thing.
Cheers,
On Sunday 13 October 2013 02:50:17 Antonio Quartulli wrote:
From: Antonio Quartulli antonio@open-mesh.com
If possible, operations like ntohs/ntohl should not be performed too often. Use a variable to locally store the converted value and then use it.
Signed-off-by: Antonio Quartulli antonio@open-mesh.com
routing.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
Applied in revision 60087ff (master branch).
Thanks, Marek
From: Antonio Quartulli antonio@open-mesh.com
Upcoming changes need to perform other checks on the incoming net_device struct.
To avoid performing dev_get_by_index() for each and every check, it is better to move it outside of is_wifi_iface() and search the netdev object once only.
Signed-off-by: Antonio Quartulli antonio@open-mesh.com --- hard-interface.c | 31 +++---------------------------- hard-interface.h | 1 - translation-table.c | 8 +++++++- 3 files changed, 10 insertions(+), 30 deletions(-)
diff --git a/hard-interface.c b/hard-interface.c index a8f4caa..57c2a19 100644 --- a/hard-interface.c +++ b/hard-interface.c @@ -127,6 +127,9 @@ static int batadv_is_valid_iface(const struct net_device *net_dev) */ bool batadv_is_wifi_netdev(struct net_device *net_device) { + if (!net_device) + return false; + #ifdef CONFIG_WIRELESS_EXT /* pre-cfg80211 drivers have to implement WEXT, so it is possible to * check for wireless_handlers != NULL @@ -142,34 +145,6 @@ bool batadv_is_wifi_netdev(struct net_device *net_device) return false; }
-/** - * batadv_is_wifi_iface - check if the given interface represented by ifindex - * is a wifi interface - * @ifindex: interface index to check - * - * Returns true if the interface represented by ifindex is a 802.11 wireless - * device, false otherwise. - */ -bool batadv_is_wifi_iface(int ifindex) -{ - struct net_device *net_device = NULL; - bool ret = false; - - if (ifindex == BATADV_NULL_IFINDEX) - goto out; - - net_device = dev_get_by_index(&init_net, ifindex); - if (!net_device) - goto out; - - ret = batadv_is_wifi_netdev(net_device); - -out: - if (net_device) - dev_put(net_device); - return ret; -} - static struct batadv_hard_iface * batadv_hardif_get_active(const struct net_device *soft_iface) { diff --git a/hard-interface.h b/hard-interface.h index 243b36c..331c87f 100644 --- a/hard-interface.h +++ b/hard-interface.h @@ -51,7 +51,6 @@ void batadv_hardif_remove_interfaces(void); int batadv_hardif_min_mtu(struct net_device *soft_iface); void batadv_update_min_mtu(struct net_device *soft_iface); void batadv_hardif_free_rcu(struct rcu_head *rcu); -bool batadv_is_wifi_iface(int ifindex); bool batadv_is_wifi_netdev(struct net_device *net_device);
/** diff --git a/translation-table.c b/translation-table.c index 8c0d244..a0f31cf 100644 --- a/translation-table.c +++ b/translation-table.c @@ -477,11 +477,15 @@ bool batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr, struct batadv_priv *bat_priv = netdev_priv(soft_iface); struct batadv_tt_local_entry *tt_local; struct batadv_tt_global_entry *tt_global; + struct net_device *in_dev = NULL; struct hlist_head *head; struct batadv_tt_orig_list_entry *orig_entry; int hash_added, table_size, packet_size_max; bool ret = false, roamed_back = false;
+ if (ifindex != BATADV_NULL_IFINDEX) + in_dev = dev_get_by_index(&init_net, ifindex); + tt_local = batadv_tt_local_hash_find(bat_priv, addr, vid); tt_global = batadv_tt_global_hash_find(bat_priv, addr, vid);
@@ -542,7 +546,7 @@ bool batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr, */ tt_local->common.flags = BATADV_TT_CLIENT_NEW; tt_local->common.vid = vid; - if (batadv_is_wifi_iface(ifindex)) + if (batadv_is_wifi_netdev(in_dev)) tt_local->common.flags |= BATADV_TT_CLIENT_WIFI; atomic_set(&tt_local->common.refcount, 2); tt_local->last_seen = jiffies; @@ -595,6 +599,8 @@ check_roaming: ret = true;
out: + if (in_dev) + dev_put(in_dev); if (tt_local) batadv_tt_local_entry_free_ref(tt_local); if (tt_global)
On Sunday 13 October 2013 02:50:18 Antonio Quartulli wrote:
From: Antonio Quartulli antonio@open-mesh.com
Upcoming changes need to perform other checks on the incoming net_device struct.
To avoid performing dev_get_by_index() for each and every check, it is better to move it outside of is_wifi_iface() and search the netdev object once only.
Signed-off-by: Antonio Quartulli antonio@open-mesh.com
hard-interface.c | 31 +++---------------------------- hard-interface.h | 1 - translation-table.c | 8 +++++++- 3 files changed, 10 insertions(+), 30 deletions(-)
Applied in revision f7391e9.
Thanks, Marek
From: Antonio Quartulli antonio@open-mesh.com
Some flags (i.e. the WIFI flag) may change after that the related client has already been announced. However it is useful to informa the rest of the network about this change.
Add a runtime-flag-switch detection mechanism and re-announce the related TT entry to advertise the new flag value.
This mechanism can be easily exploited by future flags that may need the same treatment.
Signed-off-by: Antonio Quartulli antonio@open-mesh.com --- packet.h | 6 ++++++ translation-table.c | 25 ++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/packet.h b/packet.h index f4bbb05..bf4c91f 100644 --- a/packet.h +++ b/packet.h @@ -123,6 +123,12 @@ enum batadv_tt_client_flags { };
/** + * BATADV_TT_REMOTE_MASK - bitmask selecting the flags that are sent over the + * wire only + */ +#define BATADV_TT_REMOTE_MASK 0x00FF + +/** * batadv_vlan_flags - flags for the four MSB of any vlan ID field * @BATADV_VLAN_HAS_TAG: whether the field contains a valid vlan tag or not */ diff --git a/translation-table.c b/translation-table.c index a0f31cf..4212e9a 100644 --- a/translation-table.c +++ b/translation-table.c @@ -358,6 +358,13 @@ static void batadv_tt_local_event(struct batadv_priv *bat_priv, goto del; if (del_op_requested && !del_op_entry) goto del; + + /* this is a second add in the same originator interval. It + * means that flags have been changed: update them! + */ + if (!del_op_requested && !del_op_entry) + entry->change.flags = flags; + continue; del: list_del(&entry->list); @@ -482,6 +489,7 @@ bool batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr, struct batadv_tt_orig_list_entry *orig_entry; int hash_added, table_size, packet_size_max; bool ret = false, roamed_back = false; + uint8_t remote_flags;
if (ifindex != BATADV_NULL_IFINDEX) in_dev = dev_get_by_index(&init_net, ifindex); @@ -596,8 +604,23 @@ check_roaming: } }
- ret = true; + /* store the current remote flags before altering them. This helps + * understanding is flags are changing or not + */ + remote_flags = tt_local->common.flags & BATADV_TT_REMOTE_MASK; + + if (batadv_is_wifi_netdev(in_dev)) + tt_local->common.flags |= BATADV_TT_CLIENT_WIFI; + else + tt_local->common.flags &= ~BATADV_TT_CLIENT_WIFI;
+ /* if any "dynamic" flag has been modified, resend an ADD event for this + * entry so that all the nodes can get the new flags + */ + if (remote_flags ^ (tt_local->common.flags & BATADV_TT_REMOTE_MASK)) + batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS); + + ret = true; out: if (in_dev) dev_put(in_dev);
On Sunday 13 October 2013 02:50:19 Antonio Quartulli wrote:
From: Antonio Quartulli antonio@open-mesh.com
Some flags (i.e. the WIFI flag) may change after that the related client has already been announced. However it is useful to informa the rest of the network about this change.
Add a runtime-flag-switch detection mechanism and re-announce the related TT entry to advertise the new flag value.
This mechanism can be easily exploited by future flags that may need the same treatment.
Signed-off-by: Antonio Quartulli antonio@open-mesh.com
packet.h | 6 ++++++ translation-table.c | 25 ++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-)
Applied in revision 5d07c1f.
Thanks, Marek
From: Antonio Quartulli antonio@open-mesh.com
Flags covered by TT_SYNCH_MASK are kept in sync among the nodes in the network and therefore they have to be considered while computing the global/local table CRC.
In this way a generic originator is able to understand if its table contains the correct flags or not.
Bits from 4 to 7 in the TT flags fields are now reserved for "synchronized" flags only.
This allows future developers to add more flags of this type without breaking compatibility.
It's important to note that not all the remote TT flags are synchronised. This comes from the fact that some flags are used to inject an information once only.
Signed-off-by: Antonio Quartulli antonio@open-mesh.com --- packet.h | 11 +++++++++-- translation-table.c | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/packet.h b/packet.h index bf4c91f..4683241 100644 --- a/packet.h +++ b/packet.h @@ -110,12 +110,13 @@ enum batadv_tt_data_flags {
/* BATADV_TT_CLIENT flags. * Flags from BIT(0) to BIT(7) are sent on the wire, while flags from BIT(8) to - * BIT(15) are used for local computation only + * BIT(15) are used for local computation only. + * Flags from BIT(4) to BIT(7) are kept in synch with the rest of the network. */ enum batadv_tt_client_flags { BATADV_TT_CLIENT_DEL = BIT(0), BATADV_TT_CLIENT_ROAM = BIT(1), - BATADV_TT_CLIENT_WIFI = BIT(2), + BATADV_TT_CLIENT_WIFI = BIT(4), BATADV_TT_CLIENT_NOPURGE = BIT(8), BATADV_TT_CLIENT_NEW = BIT(9), BATADV_TT_CLIENT_PENDING = BIT(10), @@ -129,6 +130,12 @@ enum batadv_tt_client_flags { #define BATADV_TT_REMOTE_MASK 0x00FF
/** + * BATADV_TT_SYNCH_MASK - bitmask of the flags that need to be kept in sync + * among the nodes. These flags are used to compute the global/local CRC + */ +#define BATADV_TT_SYNCH_MASK 0x00F0 + +/** * batadv_vlan_flags - flags for the four MSB of any vlan ID field * @BATADV_VLAN_HAS_TAG: whether the field contains a valid vlan tag or not */ diff --git a/translation-table.c b/translation-table.c index 4212e9a..538b86f 100644 --- a/translation-table.c +++ b/translation-table.c @@ -1960,6 +1960,7 @@ static uint32_t batadv_tt_global_crc(struct batadv_priv *bat_priv, struct batadv_tt_global_entry *tt_global; struct hlist_head *head; uint32_t i, crc_tmp, crc = 0; + uint8_t flags;
for (i = 0; i < hash->size; i++) { head = &hash->table[i]; @@ -1998,6 +1999,13 @@ static uint32_t batadv_tt_global_crc(struct batadv_priv *bat_priv,
crc_tmp = crc32c(0, &tt_common->vid, sizeof(tt_common->vid)); + + /* compute the CRC on flags that have to be kept in sync + * among nodes + */ + flags = tt_common->flags & BATADV_TT_SYNCH_MASK; + crc_tmp = crc32c(crc_tmp, &flags, sizeof(flags)); + crc ^= crc32c(crc_tmp, tt_common->addr, ETH_ALEN); } rcu_read_unlock(); @@ -2023,6 +2031,7 @@ static uint32_t batadv_tt_local_crc(struct batadv_priv *bat_priv, struct batadv_tt_common_entry *tt_common; struct hlist_head *head; uint32_t i, crc_tmp, crc = 0; + uint8_t flags;
for (i = 0; i < hash->size; i++) { head = &hash->table[i]; @@ -2043,6 +2052,13 @@ static uint32_t batadv_tt_local_crc(struct batadv_priv *bat_priv,
crc_tmp = crc32c(0, &tt_common->vid, sizeof(tt_common->vid)); + + /* compute the CRC on flags that have to be kept in sync + * among nodes + */ + flags = tt_common->flags & BATADV_TT_SYNCH_MASK; + crc_tmp = crc32c(crc_tmp, &flags, sizeof(flags)); + crc ^= crc32c(crc_tmp, tt_common->addr, ETH_ALEN); } rcu_read_unlock(); @@ -3525,6 +3541,9 @@ int batadv_tt_init(struct batadv_priv *bat_priv) { int ret;
+ /* synchronized flags must be remote */ + BUILD_BUG_ON(!(BATADV_TT_SYNCH_MASK & BATADV_TT_REMOTE_MASK)); + ret = batadv_tt_local_init(bat_priv); if (ret < 0) return ret;
On Sunday 13 October 2013 02:50:20 Antonio Quartulli wrote:
From: Antonio Quartulli antonio@open-mesh.com
Flags covered by TT_SYNCH_MASK are kept in sync among the nodes in the network and therefore they have to be considered while computing the global/local table CRC.
In this way a generic originator is able to understand if its table contains the correct flags or not.
Bits from 4 to 7 in the TT flags fields are now reserved for "synchronized" flags only.
This allows future developers to add more flags of this type without breaking compatibility.
It's important to note that not all the remote TT flags are synchronised. This comes from the fact that some flags are used to inject an information once only.
Signed-off-by: Antonio Quartulli antonio@open-mesh.com
packet.h | 11 +++++++++-- translation-table.c | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-)
Applied in revision 05d4de2.
Thanks, Marek
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
Marek Lindner