* [B.A.T.M.A.N.] [PATCHv2 1/4] batman-adv: don't switch byte order too often if not needed
2013-10-13 0:50 [B.A.T.M.A.N.] [PATCHv2 0/4] Introduce the SYNCHRONIZED TT flags Antonio Quartulli
@ 2013-10-13 0:50 ` Antonio Quartulli
2013-10-13 9:44 ` Marek Lindner
2013-10-14 11:07 ` Marek Lindner
2013-10-13 0:50 ` [B.A.T.M.A.N.] [PATCHv2 2/4] batman-adv: invoke dev_get_by_index() outside of is_wifi_iface() Antonio Quartulli
` (2 subsequent siblings)
3 siblings, 2 replies; 11+ messages in thread
From: Antonio Quartulli @ 2013-10-13 0:50 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Antonio Quartulli
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);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [B.A.T.M.A.N.] [PATCHv2 1/4] batman-adv: don't switch byte order too often if not needed
2013-10-13 0:50 ` [B.A.T.M.A.N.] [PATCHv2 1/4] batman-adv: don't switch byte order too often if not needed Antonio Quartulli
@ 2013-10-13 9:44 ` Marek Lindner
2013-10-13 9:51 ` Antonio Quartulli
2013-10-14 11:07 ` Marek Lindner
1 sibling, 1 reply; 11+ messages in thread
From: Marek Lindner @ 2013-10-13 9:44 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
[-- Attachment #1: Type: text/plain, Size: 482 bytes --]
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
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2 1/4] batman-adv: don't switch byte order too often if not needed
2013-10-13 9:44 ` Marek Lindner
@ 2013-10-13 9:51 ` Antonio Quartulli
0 siblings, 0 replies; 11+ messages in thread
From: Antonio Quartulli @ 2013-10-13 9:51 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
[-- Attachment #1: Type: text/plain, Size: 903 bytes --]
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,
--
Antonio Quartulli
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2 1/4] batman-adv: don't switch byte order too often if not needed
2013-10-13 0:50 ` [B.A.T.M.A.N.] [PATCHv2 1/4] batman-adv: don't switch byte order too often if not needed Antonio Quartulli
2013-10-13 9:44 ` Marek Lindner
@ 2013-10-14 11:07 ` Marek Lindner
1 sibling, 0 replies; 11+ messages in thread
From: Marek Lindner @ 2013-10-14 11:07 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Antonio Quartulli, Antonio Quartulli
[-- Attachment #1: Type: text/plain, Size: 470 bytes --]
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
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [B.A.T.M.A.N.] [PATCHv2 2/4] batman-adv: invoke dev_get_by_index() outside of is_wifi_iface()
2013-10-13 0:50 [B.A.T.M.A.N.] [PATCHv2 0/4] Introduce the SYNCHRONIZED TT flags Antonio Quartulli
2013-10-13 0:50 ` [B.A.T.M.A.N.] [PATCHv2 1/4] batman-adv: don't switch byte order too often if not needed Antonio Quartulli
@ 2013-10-13 0:50 ` Antonio Quartulli
2013-10-14 13:20 ` Marek Lindner
2013-10-13 0:50 ` [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: improve the TT component to support runtime flag changes Antonio Quartulli
2013-10-13 0:50 ` [B.A.T.M.A.N.] [PATCHv2 4/4] batman-adv: include the synch-flags when compute the global/local table CRC Antonio Quartulli
3 siblings, 1 reply; 11+ messages in thread
From: Antonio Quartulli @ 2013-10-13 0:50 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Antonio Quartulli
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)
--
1.8.3.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: improve the TT component to support runtime flag changes
2013-10-13 0:50 [B.A.T.M.A.N.] [PATCHv2 0/4] Introduce the SYNCHRONIZED TT flags Antonio Quartulli
2013-10-13 0:50 ` [B.A.T.M.A.N.] [PATCHv2 1/4] batman-adv: don't switch byte order too often if not needed Antonio Quartulli
2013-10-13 0:50 ` [B.A.T.M.A.N.] [PATCHv2 2/4] batman-adv: invoke dev_get_by_index() outside of is_wifi_iface() Antonio Quartulli
@ 2013-10-13 0:50 ` Antonio Quartulli
2013-10-14 11:22 ` Marek Lindner
2013-10-13 0:50 ` [B.A.T.M.A.N.] [PATCHv2 4/4] batman-adv: include the synch-flags when compute the global/local table CRC Antonio Quartulli
3 siblings, 1 reply; 11+ messages in thread
From: Antonio Quartulli @ 2013-10-13 0:50 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Antonio Quartulli
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);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* [B.A.T.M.A.N.] [PATCHv2 4/4] batman-adv: include the synch-flags when compute the global/local table CRC
2013-10-13 0:50 [B.A.T.M.A.N.] [PATCHv2 0/4] Introduce the SYNCHRONIZED TT flags Antonio Quartulli
` (2 preceding siblings ...)
2013-10-13 0:50 ` [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: improve the TT component to support runtime flag changes Antonio Quartulli
@ 2013-10-13 0:50 ` Antonio Quartulli
2013-10-14 14:37 ` Marek Lindner
3 siblings, 1 reply; 11+ messages in thread
From: Antonio Quartulli @ 2013-10-13 0:50 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Antonio Quartulli
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;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 11+ messages in thread