* [B.A.T.M.A.N.] [PATCH 1/5] batman-adv: add reference counting for type batadv_tt_orig_list_entry
2012-06-26 20:00 [B.A.T.M.A.N.] [PATCH 0/5] new feature: Speedy Join 2012-06-26 Antonio Quartulli
@ 2012-06-26 20:00 ` Antonio Quartulli
2012-06-26 21:23 ` Sven Eckelmann
` (2 more replies)
2012-06-26 20:00 ` [B.A.T.M.A.N.] [PATCH 2/5] batman-adv: implement batadv_tt_global_entry_get_orig() Antonio Quartulli
` (3 subsequent siblings)
4 siblings, 3 replies; 16+ messages in thread
From: Antonio Quartulli @ 2012-06-26 20:00 UTC (permalink / raw)
To: b.a.t.m.a.n
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
translation-table.c | 19 ++++++++++++++-----
types.h | 1 +
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/translation-table.c b/translation-table.c
index 245cc9a..0f02514 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -152,9 +152,12 @@ static void batadv_tt_orig_list_entry_free_rcu(struct rcu_head *rcu)
static void
batadv_tt_orig_list_entry_free_ref(struct batadv_tt_orig_list_entry *orig_entry)
{
- /* to avoid race conditions, immediately decrease the tt counter */
- atomic_dec(&orig_entry->orig_node->tt_size);
- call_rcu(&orig_entry->rcu, batadv_tt_orig_list_entry_free_rcu);
+ if (atomic_dec_and_test(&orig_entry->refcount)) {
+ /* to avoid race conditions, immediately decrease the tt counter
+ */
+ atomic_dec(&orig_entry->orig_node->tt_size);
+ call_rcu(&orig_entry->rcu, batadv_tt_orig_list_entry_free_rcu);
+ }
}
static void batadv_tt_local_event(struct batadv_priv *bat_priv,
@@ -639,12 +642,17 @@ batadv_tt_global_entry_has_orig(const struct batadv_tt_global_entry *entry,
rcu_read_lock();
head = &entry->orig_list;
hlist_for_each_entry_rcu(tmp_orig_entry, node, head, list) {
- if (tmp_orig_entry->orig_node == orig_node) {
+ if (tmp_orig_entry->orig_node != orig_node)
+ continue;
+ if (!atomic_inc_not_zero(&tmp_orig_entry->refcount))
+ continue;
+
found = true;
+ batadv_tt_orig_list_entry_free_ref(tmp_orig_entry);
break;
- }
}
rcu_read_unlock();
+
return found;
}
@@ -663,6 +671,7 @@ batadv_tt_global_add_orig_entry(struct batadv_tt_global_entry *tt_global_entry,
atomic_inc(&orig_node->tt_size);
orig_entry->orig_node = orig_node;
orig_entry->ttvn = ttvn;
+ atomic_set(&orig_entry->refcount, 0);
spin_lock_bh(&tt_global_entry->list_lock);
hlist_add_head_rcu(&orig_entry->list,
diff --git a/types.h b/types.h
index 64b4317..82b97c3 100644
--- a/types.h
+++ b/types.h
@@ -281,6 +281,7 @@ struct batadv_tt_global_entry {
struct batadv_tt_orig_list_entry {
struct batadv_orig_node *orig_node;
uint8_t ttvn;
+ atomic_t refcount;
struct rcu_head rcu;
struct hlist_node list;
};
--
1.7.9.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [B.A.T.M.A.N.] [PATCH 1/5] batman-adv: add reference counting for type batadv_tt_orig_list_entry
2012-06-26 20:00 ` [B.A.T.M.A.N.] [PATCH 1/5] batman-adv: add reference counting for type batadv_tt_orig_list_entry Antonio Quartulli
@ 2012-06-26 21:23 ` Sven Eckelmann
[not found] ` <3809123.2JX82ntKN2@sven-laptop.home.narfation.org>
[not found] ` <1403237.H233GFHdJC@sven-laptop.home.narfation.org>
2 siblings, 0 replies; 16+ messages in thread
From: Sven Eckelmann @ 2012-06-26 21:23 UTC (permalink / raw)
To: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 1638 bytes --]
On Tuesday 26 June 2012 22:00:18 Antonio Quartulli wrote:
> diff --git a/translation-table.c b/translation-table.c
> index 245cc9a..0f02514 100644
> --- a/translation-table.c
> +++ b/translation-table.c
> @@ -152,9 +152,12 @@ static void batadv_tt_orig_list_entry_free_rcu(struct
> rcu_head *rcu) static void
> batadv_tt_orig_list_entry_free_ref(struct batadv_tt_orig_list_entry
> *orig_entry) {
> - /* to avoid race conditions, immediately decrease the tt counter */
> - atomic_dec(&orig_entry->orig_node->tt_size);
> - call_rcu(&orig_entry->rcu, batadv_tt_orig_list_entry_free_rcu);
> + if (atomic_dec_and_test(&orig_entry->refcount)) {
> + /* to avoid race conditions, immediately decrease the tt counter
> + */
> + atomic_dec(&orig_entry->orig_node->tt_size);
> + call_rcu(&orig_entry->rcu, batadv_tt_orig_list_entry_free_rcu);
> + }
> }
You can just invert the atomic_dec_and_test and use a return to avoid the
weird comment.
[...]
> static void batadv_tt_local_event(struct batadv_priv *bat_priv,
> @@ -639,12 +642,17 @@ batadv_tt_global_entry_has_orig(const struct
> batadv_tt_global_entry *entry, rcu_read_lock();
> head = &entry->orig_list;
> hlist_for_each_entry_rcu(tmp_orig_entry, node, head, list) {
> - if (tmp_orig_entry->orig_node == orig_node) {
> + if (tmp_orig_entry->orig_node != orig_node)
> + continue;
> + if (!atomic_inc_not_zero(&tmp_orig_entry->refcount))
> + continue;
> +
> found = true;
> + batadv_tt_orig_list_entry_free_ref(tmp_orig_entry);
> break;
> - }
> }
> rcu_read_unlock();
Please fix the indentation in this patch and not in the later ones.
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread[parent not found: <3809123.2JX82ntKN2@sven-laptop.home.narfation.org>]
[parent not found: <1403237.H233GFHdJC@sven-laptop.home.narfation.org>]
* [B.A.T.M.A.N.] [PATCH 2/5] batman-adv: implement batadv_tt_global_entry_get_orig()
2012-06-26 20:00 [B.A.T.M.A.N.] [PATCH 0/5] new feature: Speedy Join 2012-06-26 Antonio Quartulli
2012-06-26 20:00 ` [B.A.T.M.A.N.] [PATCH 1/5] batman-adv: add reference counting for type batadv_tt_orig_list_entry Antonio Quartulli
@ 2012-06-26 20:00 ` Antonio Quartulli
2012-06-26 20:00 ` [B.A.T.M.A.N.] [PATCH 3/5] batman-adv: enable roaming packets to carry flags Antonio Quartulli
` (2 subsequent siblings)
4 siblings, 0 replies; 16+ messages in thread
From: Antonio Quartulli @ 2012-06-26 20:00 UTC (permalink / raw)
To: b.a.t.m.a.n
batadv_tt_global_entry_get_orig() searches the originator list associated to a
given tt_global_entry and possibly for the tt_orig_list_entry associated to the
orig_node passed as argument.
batadv_tt_global_entry_has_orig() has been modified in order to use the new
function and avoid code duplication. Now it also returns bool instead of int.
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
translation-table.c | 37 +++++++++++++++++++++++++++----------
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/translation-table.c b/translation-table.c
index 0f02514..2e90a97 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -627,17 +627,17 @@ static void batadv_tt_changes_list_free(struct batadv_priv *bat_priv)
spin_unlock_bh(&bat_priv->tt_changes_list_lock);
}
-/* find out if an orig_node is already in the list of a tt_global_entry.
- * returns 1 if found, 0 otherwise
+/* find out if an orig_node is already in the list of a tt_global_entry and
+ * returns it with an increased refcounter if found, NULL otherwise.
+ *
*/
-static bool
-batadv_tt_global_entry_has_orig(const struct batadv_tt_global_entry *entry,
+static struct batadv_tt_orig_list_entry *
+batadv_tt_global_entry_get_orig(const struct batadv_tt_global_entry *entry,
const struct batadv_orig_node *orig_node)
{
- struct batadv_tt_orig_list_entry *tmp_orig_entry;
+ struct batadv_tt_orig_list_entry *tmp_orig_entry, *orig_entry = NULL;
const struct hlist_head *head;
struct hlist_node *node;
- bool found = false;
rcu_read_lock();
head = &entry->orig_list;
@@ -647,13 +647,30 @@ batadv_tt_global_entry_has_orig(const struct batadv_tt_global_entry *entry,
if (!atomic_inc_not_zero(&tmp_orig_entry->refcount))
continue;
- found = true;
- batadv_tt_orig_list_entry_free_ref(tmp_orig_entry);
- break;
+ orig_entry = tmp_orig_entry;
+ break;
}
rcu_read_unlock();
- return found;
+ return orig_entry;
+}
+
+/* find out if an orig_node is already in the list of a tt_global_entry.
+ * returns true if found, false otherwise
+ */
+static bool
+batadv_tt_global_entry_has_orig(const struct batadv_tt_global_entry *entry,
+ const struct batadv_orig_node *orig_node)
+{
+ bool res = false;
+ struct batadv_tt_orig_list_entry *orig_entry;
+
+ orig_entry = batadv_tt_global_entry_get_orig(entry, orig_node);
+ if (orig_entry) {
+ res = true;
+ batadv_tt_orig_list_entry_free_ref(orig_entry);
+ }
+ return res;
}
static void
--
1.7.9.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [B.A.T.M.A.N.] [PATCH 3/5] batman-adv: enable roaming packets to carry flags
2012-06-26 20:00 [B.A.T.M.A.N.] [PATCH 0/5] new feature: Speedy Join 2012-06-26 Antonio Quartulli
2012-06-26 20:00 ` [B.A.T.M.A.N.] [PATCH 1/5] batman-adv: add reference counting for type batadv_tt_orig_list_entry Antonio Quartulli
2012-06-26 20:00 ` [B.A.T.M.A.N.] [PATCH 2/5] batman-adv: implement batadv_tt_global_entry_get_orig() Antonio Quartulli
@ 2012-06-26 20:00 ` Antonio Quartulli
2012-06-26 21:22 ` Sven Eckelmann
[not found] ` <3265215.erIRmWtS44@sven-laptop.home.narfation.org>
2012-06-26 20:00 ` [B.A.T.M.A.N.] [PATCH 4/5] batman-adv: detect not yet announced clients Antonio Quartulli
2012-06-26 20:00 ` [B.A.T.M.A.N.] [PATCH 5/5] batman-adv: change interface_rx to get orig node Antonio Quartulli
4 siblings, 2 replies; 16+ messages in thread
From: Antonio Quartulli @ 2012-06-26 20:00 UTC (permalink / raw)
To: b.a.t.m.a.n
In case of client-roaming it would be useful to carry TT flags along with
the client entry. Information contained in the flags could be used on the new
mesh node for several reasons (e.g. particular roaming treatment).
This patch modifies the ROAMING_ADV packet according to this idea so that it can
also carry the flags together with the MAC address of the moving client.
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
packet.h | 2 +-
routing.c | 7 ++++---
translation-table.c | 17 ++++++++++-------
3 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/packet.h b/packet.h
index 65d66e4..e0b94a3 100644
--- a/packet.h
+++ b/packet.h
@@ -214,7 +214,7 @@ struct batadv_tt_query_packet {
struct batadv_roam_adv_packet {
struct batadv_header header;
- uint8_t reserved;
+ uint8_t flags;
uint8_t dst[ETH_ALEN];
uint8_t src[ETH_ALEN];
uint8_t client[ETH_ALEN];
diff --git a/routing.c b/routing.c
index bc2b88b..b3da42c 100644
--- a/routing.c
+++ b/routing.c
@@ -710,11 +710,12 @@ int batadv_recv_roam_adv(struct sk_buff *skb, struct batadv_hard_iface *recv_if)
goto out;
batadv_dbg(BATADV_DBG_TT, bat_priv,
- "Received ROAMING_ADV from %pM (client %pM)\n",
- roam_adv_packet->src, roam_adv_packet->client);
+ "Received ROAMING_ADV from %pM (client: %pM flags: 0x%.2x)\n",
+ roam_adv_packet->src, roam_adv_packet->client,
+ roam_adv_packet->flags);
batadv_tt_global_add(bat_priv, orig_node, roam_adv_packet->client,
- BATADV_TT_CLIENT_ROAM,
+ roam_adv_packet->flags | BATADV_TT_CLIENT_ROAM,
atomic_read(&orig_node->last_ttvn) + 1);
/* Roaming phase starts: I have new information but the ttvn has not
diff --git a/translation-table.c b/translation-table.c
index 2e90a97..b265c28 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -29,7 +29,8 @@
#include <linux/crc16.h>
-static void batadv_send_roam_adv(struct batadv_priv *bat_priv, uint8_t *client,
+static void batadv_send_roam_adv(struct batadv_priv *bat_priv,
+ struct batadv_tt_global_entry *tt_global_entry,
struct batadv_orig_node *orig_node);
static void batadv_tt_purge(struct work_struct *work);
static void
@@ -303,8 +304,7 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr,
hlist_for_each_entry_rcu(orig_entry, node, head, list) {
orig_entry->orig_node->tt_poss_change = true;
- batadv_send_roam_adv(bat_priv,
- tt_global_entry->common.addr,
+ batadv_send_roam_adv(bat_priv, tt_global_entry,
orig_entry->orig_node);
}
rcu_read_unlock();
@@ -2051,7 +2051,8 @@ unlock:
return ret;
}
-static void batadv_send_roam_adv(struct batadv_priv *bat_priv, uint8_t *client,
+static void batadv_send_roam_adv(struct batadv_priv *bat_priv,
+ struct batadv_tt_global_entry *tt_global_entry,
struct batadv_orig_node *orig_node)
{
struct batadv_neigh_node *neigh_node = NULL;
@@ -2064,7 +2065,7 @@ static void batadv_send_roam_adv(struct batadv_priv *bat_priv, uint8_t *client,
/* before going on we have to check whether the client has
* already roamed to us too many times
*/
- if (!batadv_tt_check_roam_count(bat_priv, client))
+ if (!batadv_tt_check_roam_count(bat_priv, tt_global_entry->common.addr))
goto out;
skb = dev_alloc_skb(sizeof(*roam_adv_packet) + ETH_HLEN);
@@ -2084,7 +2085,8 @@ static void batadv_send_roam_adv(struct batadv_priv *bat_priv, uint8_t *client,
memcpy(roam_adv_packet->src, primary_if->net_dev->dev_addr, ETH_ALEN);
batadv_hardif_free_ref(primary_if);
memcpy(roam_adv_packet->dst, orig_node->orig, ETH_ALEN);
- memcpy(roam_adv_packet->client, client, ETH_ALEN);
+ memcpy(roam_adv_packet->client, tt_global_entry->common.addr, ETH_ALEN);
+ roam_adv_packet->flags = BATADV_NO_FLAGS;
neigh_node = batadv_orig_node_get_router(orig_node);
if (!neigh_node)
@@ -2092,7 +2094,8 @@ static void batadv_send_roam_adv(struct batadv_priv *bat_priv, uint8_t *client,
batadv_dbg(BATADV_DBG_TT, bat_priv,
"Sending ROAMING_ADV to %pM (client %pM) via %pM\n",
- orig_node->orig, client, neigh_node->addr);
+ orig_node->orig, tt_global_entry->common.addr,
+ neigh_node->addr);
batadv_inc_counter(bat_priv, BATADV_CNT_TT_ROAM_ADV_TX);
--
1.7.9.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [B.A.T.M.A.N.] [PATCH 3/5] batman-adv: enable roaming packets to carry flags
2012-06-26 20:00 ` [B.A.T.M.A.N.] [PATCH 3/5] batman-adv: enable roaming packets to carry flags Antonio Quartulli
@ 2012-06-26 21:22 ` Sven Eckelmann
2012-06-26 21:45 ` Matthias Schiffer
[not found] ` <3265215.erIRmWtS44@sven-laptop.home.narfation.org>
1 sibling, 1 reply; 16+ messages in thread
From: Sven Eckelmann @ 2012-06-26 21:22 UTC (permalink / raw)
To: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 1375 bytes --]
On Tuesday 26 June 2012 22:00:20 Antonio Quartulli wrote:
> In case of client-roaming it would be useful to carry TT flags along with
> the client entry. Information contained in the flags could be used on the
> new mesh node for several reasons (e.g. particular roaming treatment).
>
> This patch modifies the ROAMING_ADV packet according to this idea so that it
> can also carry the flags together with the MAC address of the moving
> client.
>
> Signed-off-by: Antonio Quartulli <ordex@autistici.org>
> ---
> packet.h | 2 +-
> routing.c | 7 ++++---
> translation-table.c | 17 ++++++++++-------
> 3 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/packet.h b/packet.h
> index 65d66e4..e0b94a3 100644
> --- a/packet.h
> +++ b/packet.h
> @@ -214,7 +214,7 @@ struct batadv_tt_query_packet {
>
> struct batadv_roam_adv_packet {
> struct batadv_header header;
> - uint8_t reserved;
> + uint8_t flags;
> uint8_t dst[ETH_ALEN];
> uint8_t src[ETH_ALEN];
> uint8_t client[ETH_ALEN];
I am not 100% sure because I haven't checked the code, but couldn't it be the
case that we send random bits inside reserved at the moment? At least I cannot
remember the part of the code that initialized reserver to any specific value.
That would make the change incompatible with older batman-adv version.
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [B.A.T.M.A.N.] [PATCH 3/5] batman-adv: enable roaming packets to carry flags
2012-06-26 21:22 ` Sven Eckelmann
@ 2012-06-26 21:45 ` Matthias Schiffer
2012-06-27 6:31 ` Antonio Quartulli
0 siblings, 1 reply; 16+ messages in thread
From: Matthias Schiffer @ 2012-06-26 21:45 UTC (permalink / raw)
To: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 1889 bytes --]
On 06/26/2012 11:22 PM, Sven Eckelmann wrote:
> On Tuesday 26 June 2012 22:00:20 Antonio Quartulli wrote:
>> In case of client-roaming it would be useful to carry TT flags along with
>> the client entry. Information contained in the flags could be used on the
>> new mesh node for several reasons (e.g. particular roaming treatment).
>>
>> This patch modifies the ROAMING_ADV packet according to this idea so that it
>> can also carry the flags together with the MAC address of the moving
>> client.
>>
>> Signed-off-by: Antonio Quartulli <ordex@autistici.org>
>> ---
>> packet.h | 2 +-
>> routing.c | 7 ++++---
>> translation-table.c | 17 ++++++++++-------
>> 3 files changed, 15 insertions(+), 11 deletions(-)
>>
>> diff --git a/packet.h b/packet.h
>> index 65d66e4..e0b94a3 100644
>> --- a/packet.h
>> +++ b/packet.h
>> @@ -214,7 +214,7 @@ struct batadv_tt_query_packet {
>>
>> struct batadv_roam_adv_packet {
>> struct batadv_header header;
>> - uint8_t reserved;
>> + uint8_t flags;
>> uint8_t dst[ETH_ALEN];
>> uint8_t src[ETH_ALEN];
>> uint8_t client[ETH_ALEN];
>
> I am not 100% sure because I haven't checked the code, but couldn't it be the
> case that we send random bits inside reserved at the moment? At least I cannot
> remember the part of the code that initialized reserver to any specific value.
> That would make the change incompatible with older batman-adv version.
As stated in an earlier mail, the `reserved' field in the vis packets
isn't initialized either, leading to the same problem, being unable to
ever use this field for anything when you want to stay compatible with
older versions.
IMO this should be fixed in all packets, it's a really bad idea to send
uninitialized memory over the network...
Regards,
Matthias
>
> Kind regards,
> Sven
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [B.A.T.M.A.N.] [PATCH 3/5] batman-adv: enable roaming packets to carry flags
2012-06-26 21:45 ` Matthias Schiffer
@ 2012-06-27 6:31 ` Antonio Quartulli
0 siblings, 0 replies; 16+ messages in thread
From: Antonio Quartulli @ 2012-06-27 6:31 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
[-- Attachment #1: Type: text/plain, Size: 2220 bytes --]
On Tue, Jun 26, 2012 at 11:45:37PM +0200, Matthias Schiffer wrote:
> On 06/26/2012 11:22 PM, Sven Eckelmann wrote:
> > On Tuesday 26 June 2012 22:00:20 Antonio Quartulli wrote:
> >> In case of client-roaming it would be useful to carry TT flags along with
> >> the client entry. Information contained in the flags could be used on the
> >> new mesh node for several reasons (e.g. particular roaming treatment).
> >>
> >> This patch modifies the ROAMING_ADV packet according to this idea so that it
> >> can also carry the flags together with the MAC address of the moving
> >> client.
> >>
> >> Signed-off-by: Antonio Quartulli <ordex@autistici.org>
> >> ---
> >> packet.h | 2 +-
> >> routing.c | 7 ++++---
> >> translation-table.c | 17 ++++++++++-------
> >> 3 files changed, 15 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/packet.h b/packet.h
> >> index 65d66e4..e0b94a3 100644
> >> --- a/packet.h
> >> +++ b/packet.h
> >> @@ -214,7 +214,7 @@ struct batadv_tt_query_packet {
> >>
> >> struct batadv_roam_adv_packet {
> >> struct batadv_header header;
> >> - uint8_t reserved;
> >> + uint8_t flags;
> >> uint8_t dst[ETH_ALEN];
> >> uint8_t src[ETH_ALEN];
> >> uint8_t client[ETH_ALEN];
> >
> > I am not 100% sure because I haven't checked the code, but couldn't it be the
> > case that we send random bits inside reserved at the moment? At least I cannot
> > remember the part of the code that initialized reserver to any specific value.
> > That would make the change incompatible with older batman-adv version.
>
> As stated in an earlier mail, the `reserved' field in the vis packets
> isn't initialized either, leading to the same problem, being unable to
> ever use this field for anything when you want to stay compatible with
> older versions.
>
> IMO this should be fixed in all packets, it's a really bad idea to send
> uninitialized memory over the network...
Yes, I agree. we have to remember this fix the next time we decide to break
compatibility. I don't think we are going to do it now.
Cheers,
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
[parent not found: <3265215.erIRmWtS44@sven-laptop.home.narfation.org>]
* Re: [B.A.T.M.A.N.] [PATCH 3/5] batman-adv: enable roaming packets to carry flags
[not found] ` <3265215.erIRmWtS44@sven-laptop.home.narfation.org>
@ 2012-06-27 6:30 ` Antonio Quartulli
2012-06-27 6:58 ` Antonio Quartulli
0 siblings, 1 reply; 16+ messages in thread
From: Antonio Quartulli @ 2012-06-27 6:30 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 716 bytes --]
On Tue, Jun 26, 2012 at 11:18:29PM +0200, Sven Eckelmann wrote:
> I am not 100% sure because I haven't checked the code, but couldn't it be
> the case that we send random bits inside reserved at the moment? At least
> I cannot remember the part of the code that initialized reserver to any
> specific value. That would make the change incompatible with older
> batman-adv version.
Damn, that's true! It is not initialised anywhere....
What if I append a new field to the roam_adv_packet struct? Old version will
ignore it and there is no size check to drop packets longer than expected.
Cheers,
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [B.A.T.M.A.N.] [PATCH 3/5] batman-adv: enable roaming packets to carry flags
2012-06-27 6:30 ` Antonio Quartulli
@ 2012-06-27 6:58 ` Antonio Quartulli
0 siblings, 0 replies; 16+ messages in thread
From: Antonio Quartulli @ 2012-06-27 6:58 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 1515 bytes --]
On Wed, Jun 27, 2012 at 08:30:22AM +0200, Antonio Quartulli wrote:
> On Tue, Jun 26, 2012 at 11:18:29PM +0200, Sven Eckelmann wrote:
> > I am not 100% sure because I haven't checked the code, but couldn't it be
> > the case that we send random bits inside reserved at the moment? At least
> > I cannot remember the part of the code that initialized reserver to any
> > specific value. That would make the change incompatible with older
> > batman-adv version.
>
> Damn, that's true! It is not initialised anywhere....
> What if I append a new field to the roam_adv_packet struct? Old version will
> ignore it and there is no size check to drop packets longer than expected.
Ok, after discussing with Sven on irc I got to the point that it is simply
better to skip this patch. Appending a new field would make the new code much
more complicated (and ugly) because we have to consider that we could receive
old packets with a smaller size (and they should not be dropped).
I think that the best solution is to remove this patch.
The idea in this code
was to send the new TT_CLIENT_TEMP flag within the roam_adv so that the receiver
node can eventually purge this entry if not claimed anymore. However this is not
needed because a roaming client is already marked as ROAM on the receiver side and
so it will already be purged if not claimed within a fixed amount of time.
Cheers,
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* [B.A.T.M.A.N.] [PATCH 4/5] batman-adv: detect not yet announced clients
2012-06-26 20:00 [B.A.T.M.A.N.] [PATCH 0/5] new feature: Speedy Join 2012-06-26 Antonio Quartulli
` (2 preceding siblings ...)
2012-06-26 20:00 ` [B.A.T.M.A.N.] [PATCH 3/5] batman-adv: enable roaming packets to carry flags Antonio Quartulli
@ 2012-06-26 20:00 ` Antonio Quartulli
2012-06-26 21:50 ` Sven Eckelmann
2012-06-26 20:00 ` [B.A.T.M.A.N.] [PATCH 5/5] batman-adv: change interface_rx to get orig node Antonio Quartulli
4 siblings, 1 reply; 16+ messages in thread
From: Antonio Quartulli @ 2012-06-26 20:00 UTC (permalink / raw)
To: b.a.t.m.a.n
With the current TT mechanism a new client joining the network is not
immediately able to communicate with other hosts because its MAC address has not
been announced yet. This situation holds until the first OGM containing its
joining event will be spread over the mesh network.
This behaviour can be acceptable in networks where the originator interval is a
small value (e.g. 1sec) but if that value is set to an higher time (e.g. 5secs)
the client could suffer from several malfunctions like DHCP client timeouts,
etc.
This patch adds an early detection mechanism that makes nodes in the network
able to recognise "not yet announced clients" by means of the broadcast packets
they emitted on connection (e.g. ARP or DHCP request). The added client will
then be confirmed upon receiving the OGM claiming it or purged if such OGM
is not received within a fixed amount of time.
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
main.h | 2 +
packet.h | 1 +
translation-table.c | 152 ++++++++++++++++++++++++++++++++++++++-------------
translation-table.h | 4 +-
types.h | 1 +
5 files changed, 121 insertions(+), 39 deletions(-)
diff --git a/main.h b/main.h
index 6dca9c4..00c9c34 100644
--- a/main.h
+++ b/main.h
@@ -43,6 +43,8 @@
#define BATADV_PURGE_TIMEOUT 200000 /* 200 seconds */
#define BATADV_TT_LOCAL_TIMEOUT 3600000 /* in miliseconds */
#define BATADV_TT_CLIENT_ROAM_TIMEOUT 600000 /* in miliseconds */
+#define BATADV_TT_CLIENT_TEMP_TIMEOUT_FACT 10
+
/* sliding packet range of received originator messages in sequence numbers
* (should be a multiple of our word size)
*/
diff --git a/packet.h b/packet.h
index e0b94a3..a8c1cd6 100644
--- a/packet.h
+++ b/packet.h
@@ -85,6 +85,7 @@ enum batadv_tt_client_flags {
BATADV_TT_CLIENT_DEL = 1 << 0,
BATADV_TT_CLIENT_ROAM = 1 << 1,
BATADV_TT_CLIENT_WIFI = 1 << 2,
+ BATADV_TT_CLIENT_TEMP = 1 << 3,
BATADV_TT_CLIENT_NOPURGE = 1 << 8,
BATADV_TT_CLIENT_NEW = 1 << 9,
BATADV_TT_CLIENT_PENDING = 1 << 10,
diff --git a/translation-table.c b/translation-table.c
index b265c28..80db248 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -35,6 +35,19 @@ static void batadv_send_roam_adv(struct batadv_priv *bat_priv,
static void batadv_tt_purge(struct work_struct *work);
static void
batadv_tt_global_del_orig_list(struct batadv_tt_global_entry *tt_global_entry);
+static void batadv_tt_global_del(struct batadv_priv *bat_priv,
+ struct batadv_orig_node *orig_node,
+ const unsigned char *addr,
+ const char *message, bool roaming);
+
+/* the temporary client timeout is defined as a multiple of the originator
+ * interval
+ */
+static unsigned long batadv_tt_client_temp_timeout(struct batadv_priv *bat_priv)
+{
+ return BATADV_TT_CLIENT_TEMP_TIMEOUT_FACT *
+ atomic_read(&bat_priv->orig_interval);
+}
/* returns 1 if they are the same mac addr */
static int batadv_compare_tt(const struct hlist_node *node, const void *data2)
@@ -269,6 +282,7 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr,
tt_local_entry->common.flags |= BATADV_TT_CLIENT_WIFI;
atomic_set(&tt_local_entry->common.refcount, 2);
tt_local_entry->last_seen = jiffies;
+ tt_local_entry->common.added_at = tt_local_entry->last_seen;
/* the batman interface mac address should never be purged */
if (batadv_compare_eth(addr, soft_iface->dev_addr))
@@ -308,11 +322,23 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr,
orig_entry->orig_node);
}
rcu_read_unlock();
- /* The global entry has to be marked as ROAMING and
- * has to be kept for consistency purpose
+
+ /* if the global client is marked as TEMP or ROAM we can
+ * directly delete it because it has never been announced yet
+ * and we don't need to keep it for consistency purposes
*/
- tt_global_entry->common.flags |= BATADV_TT_CLIENT_ROAM;
- tt_global_entry->roam_at = jiffies;
+ if ((tt_global_entry->common.flags & BATADV_TT_CLIENT_TEMP) ||
+ (tt_global_entry->common.flags & BATADV_TT_CLIENT_ROAM))
+ batadv_tt_global_del(bat_priv, NULL, addr,
+ "Not yet announced global client roamed to us",
+ true);
+ else {
+ /* The global entry has to be marked as ROAMING and
+ * has to be kept for consistency purpose
+ */
+ tt_global_entry->common.flags |= BATADV_TT_CLIENT_ROAM;
+ tt_global_entry->roam_at = jiffies;
+ }
}
out:
if (tt_local_entry)
@@ -719,7 +745,9 @@ int batadv_tt_global_add(struct batadv_priv *bat_priv,
common->flags = flags;
tt_global_entry->roam_at = 0;
- atomic_set(&common->refcount, 2);
+ atomic_set(&tt_global_entry->common.refcount, 2);
+ tt_global_entry->common.added_at = jiffies;
+
INIT_HLIST_HEAD(&tt_global_entry->orig_list);
spin_lock_init(&tt_global_entry->list_lock);
@@ -739,6 +767,16 @@ int batadv_tt_global_add(struct batadv_priv *bat_priv,
ttvn);
} else {
/* there is already a global entry, use this one. */
+ /* if we are trying to add a temporary node, but we found an
+ * already existent entry, we can exit directly
+ */
+ if (flags & BATADV_TT_CLIENT_TEMP)
+ goto out;
+
+ /* if the client was temporary added before receiving the first
+ * OGM announcing it, we have to clear the TEMP flag
+ */
+ tt_global_entry->common.flags &= ~BATADV_TT_CLIENT_TEMP;
/* If there is the BATADV_TT_CLIENT_ROAM flag set, there is only
* one originator left in the list and we previously received a
@@ -796,11 +834,12 @@ batadv_tt_global_print_entry(struct batadv_tt_global_entry *tt_global_entry,
hlist_for_each_entry_rcu(orig_entry, node, head, list) {
flags = tt_common_entry->flags;
last_ttvn = atomic_read(&orig_entry->orig_node->last_ttvn);
- seq_printf(seq, " * %pM (%3u) via %pM (%3u) [%c%c]\n",
+ seq_printf(seq, " * %pM (%3u) via %pM (%3u) [%c%c%c]\n",
tt_global_entry->common.addr, orig_entry->ttvn,
orig_entry->orig_node->orig, last_ttvn,
(flags & BATADV_TT_CLIENT_ROAM ? 'R' : '.'),
- (flags & BATADV_TT_CLIENT_WIFI ? 'W' : '.'));
+ (flags & BATADV_TT_CLIENT_WIFI ? 'W' : '.'),
+ (flags & BATADV_TT_CLIENT_TEMP ? 'T' : '.'));
}
}
@@ -1055,46 +1094,55 @@ void batadv_tt_global_del_orig(struct batadv_priv *bat_priv,
orig_node->tt_initialised = false;
}
-static void batadv_tt_global_roam_purge_list(struct batadv_priv *bat_priv,
- struct hlist_head *head)
-{
- struct batadv_tt_common_entry *tt_common_entry;
- struct batadv_tt_global_entry *tt_global_entry;
- struct hlist_node *node, *node_tmp;
-
- hlist_for_each_entry_safe(tt_common_entry, node, node_tmp, head,
- hash_entry) {
- tt_global_entry = container_of(tt_common_entry,
- struct batadv_tt_global_entry,
- common);
- if (!(tt_global_entry->common.flags & BATADV_TT_CLIENT_ROAM))
- continue;
- if (!batadv_has_timed_out(tt_global_entry->roam_at,
- BATADV_TT_CLIENT_ROAM_TIMEOUT))
- continue;
-
- batadv_dbg(BATADV_DBG_TT, bat_priv,
- "Deleting global tt entry (%pM): Roaming timeout\n",
- tt_global_entry->common.addr);
-
- hlist_del_rcu(node);
- batadv_tt_global_entry_free_ref(tt_global_entry);
- }
-}
-
-static void batadv_tt_global_roam_purge(struct batadv_priv *bat_priv)
+static void batadv_tt_global_purge(struct batadv_priv *bat_priv)
{
struct batadv_hashtable *hash = bat_priv->tt_global_hash;
struct hlist_head *head;
+ struct hlist_node *node, *node_tmp;
spinlock_t *list_lock; /* protects write access to the hash lists */
uint32_t i;
+ bool purge;
+ char *msg = NULL;
+ struct batadv_tt_common_entry *tt_common_entry;
+ struct batadv_tt_global_entry *tt_global;
+ unsigned long temp_timeout = batadv_tt_client_temp_timeout(bat_priv);
+ unsigned long roam_timeout = BATADV_TT_CLIENT_ROAM_TIMEOUT;
for (i = 0; i < hash->size; i++) {
head = &hash->table[i];
list_lock = &hash->list_locks[i];
spin_lock_bh(list_lock);
- batadv_tt_global_roam_purge_list(bat_priv, head);
+ hlist_for_each_entry_safe(tt_common_entry, node, node_tmp,
+ head, hash_entry) {
+ purge = false;
+ tt_global = container_of(tt_common_entry,
+ struct batadv_tt_global_entry,
+ common);
+ if ((tt_global->common.flags & BATADV_TT_CLIENT_ROAM) &&
+ batadv_has_timed_out(tt_global->roam_at,
+ roam_timeout)) {
+ purge = true;
+ msg = "Roaming timeout\n";
+ }
+
+ if ((tt_global->common.flags & BATADV_TT_CLIENT_TEMP) &&
+ batadv_has_timed_out(tt_global->common.added_at,
+ temp_timeout)) {
+ purge = true;
+ msg = "Temporary client timeout\n";
+ }
+
+ if (!purge)
+ continue;
+
+ batadv_dbg(BATADV_DBG_TT, bat_priv,
+ "Deleting global tt entry (%pM): %s\n",
+ tt_global->common.addr, msg);
+
+ hlist_del_rcu(node);
+ batadv_tt_global_entry_free_ref(tt_global);
+ }
spin_unlock_bh(list_lock);
}
@@ -1235,6 +1283,11 @@ static uint16_t batadv_tt_global_crc(struct batadv_priv *bat_priv,
*/
if (tt_common->flags & BATADV_TT_CLIENT_ROAM)
continue;
+ /* Temporary clients have not been announced yet, so
+ * they have to be skipped while computing the global
+ * crc */
+ if (tt_common->flags & BATADV_TT_CLIENT_TEMP)
+ continue;
/* find out if this global entry is announced by this
* originator
@@ -1388,9 +1441,11 @@ static int batadv_tt_global_valid(const void *entry_ptr,
const struct batadv_tt_global_entry *tt_global_entry;
const struct batadv_orig_node *orig_node = data_ptr;
- if (tt_common_entry->flags & BATADV_TT_CLIENT_ROAM)
+ if (tt_common_entry->flags & BATADV_TT_CLIENT_ROAM ||
+ tt_common_entry->flags & BATADV_TT_CLIENT_TEMP)
return 0;
+
tt_global_entry = container_of(tt_common_entry,
struct batadv_tt_global_entry,
common);
@@ -2087,6 +2142,8 @@ static void batadv_send_roam_adv(struct batadv_priv *bat_priv,
memcpy(roam_adv_packet->dst, orig_node->orig, ETH_ALEN);
memcpy(roam_adv_packet->client, tt_global_entry->common.addr, ETH_ALEN);
roam_adv_packet->flags = BATADV_NO_FLAGS;
+ if (tt_global_entry->common.flags & BATADV_TT_CLIENT_TEMP)
+ roam_adv_packet->flags |= BATADV_TT_CLIENT_TEMP;
neigh_node = batadv_orig_node_get_router(orig_node);
if (!neigh_node)
@@ -2119,7 +2176,7 @@ static void batadv_tt_purge(struct work_struct *work)
bat_priv = container_of(delayed_work, struct batadv_priv, tt_work);
batadv_tt_local_purge(bat_priv);
- batadv_tt_global_roam_purge(bat_priv);
+ batadv_tt_global_purge(bat_priv);
batadv_tt_req_purge(bat_priv);
batadv_tt_roam_purge(bat_priv);
@@ -2393,3 +2450,22 @@ bool batadv_tt_global_client_is_roaming(struct batadv_priv *bat_priv,
out:
return ret;
}
+
+bool batadv_tt_add_temporary_global_entry(struct batadv_priv *bat_priv,
+ struct batadv_orig_node *orig_node,
+ const unsigned char *addr)
+{
+ bool ret = false;
+
+ if (!batadv_tt_global_add(bat_priv, orig_node, addr,
+ BATADV_TT_CLIENT_TEMP,
+ atomic_read(&orig_node->last_ttvn)))
+ goto out;
+
+ batadv_dbg(BATADV_DBG_TT, bat_priv,
+ "Added temporary global client (addr: %pM orig: %pM)\n",
+ addr, orig_node->orig);
+ ret = true;
+out:
+ return ret;
+}
diff --git a/translation-table.h b/translation-table.h
index ffa8735..811fffd 100644
--- a/translation-table.h
+++ b/translation-table.h
@@ -59,6 +59,8 @@ int batadv_tt_append_diff(struct batadv_priv *bat_priv,
int packet_min_len);
bool batadv_tt_global_client_is_roaming(struct batadv_priv *bat_priv,
uint8_t *addr);
-
+bool batadv_tt_add_temporary_global_entry(struct batadv_priv *bat_priv,
+ struct batadv_orig_node *orig_node,
+ const unsigned char *addr);
#endif /* _NET_BATMAN_ADV_TRANSLATION_TABLE_H_ */
diff --git a/types.h b/types.h
index 82b97c3..591bd28 100644
--- a/types.h
+++ b/types.h
@@ -262,6 +262,7 @@ struct batadv_tt_common_entry {
uint8_t addr[ETH_ALEN];
struct hlist_node hash_entry;
uint16_t flags;
+ unsigned long added_at;
atomic_t refcount;
struct rcu_head rcu;
};
--
1.7.9.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [B.A.T.M.A.N.] [PATCH 4/5] batman-adv: detect not yet announced clients
2012-06-26 20:00 ` [B.A.T.M.A.N.] [PATCH 4/5] batman-adv: detect not yet announced clients Antonio Quartulli
@ 2012-06-26 21:50 ` Sven Eckelmann
2012-06-27 6:34 ` Antonio Quartulli
0 siblings, 1 reply; 16+ messages in thread
From: Sven Eckelmann @ 2012-06-26 21:50 UTC (permalink / raw)
To: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 4171 bytes --]
On Tuesday 26 June 2012 22:00:21 Antonio Quartulli wrote:
> With the current TT mechanism a new client joining the network is not
> immediately able to communicate with other hosts because its MAC address has
> not been announced yet. This situation holds until the first OGM containing
> its joining event will be spread over the mesh network.
>
> This behaviour can be acceptable in networks where the originator interval
> is a small value (e.g. 1sec) but if that value is set to an higher time
> (e.g. 5secs) the client could suffer from several malfunctions like DHCP
> client timeouts, etc.
>
> This patch adds an early detection mechanism that makes nodes in the network
> able to recognise "not yet announced clients" by means of the broadcast
> packets they emitted on connection (e.g. ARP or DHCP request). The added
> client will then be confirmed upon receiving the OGM claiming it or purged
> if such OGM is not received within a fixed amount of time.
>
> Signed-off-by: Antonio Quartulli <ordex@autistici.org>
> ---
> main.h | 2 +
> packet.h | 1 +
> translation-table.c | 152
> ++++++++++++++++++++++++++++++++++++++------------- translation-table.h |
> 4 +-
> types.h | 1 +
> 5 files changed, 121 insertions(+), 39 deletions(-)
>
> diff --git a/main.h b/main.h
> index 6dca9c4..00c9c34 100644
> --- a/main.h
> +++ b/main.h
> @@ -43,6 +43,8 @@
> #define BATADV_PURGE_TIMEOUT 200000 /* 200 seconds */
> #define BATADV_TT_LOCAL_TIMEOUT 3600000 /* in miliseconds */
> #define BATADV_TT_CLIENT_ROAM_TIMEOUT 600000 /* in miliseconds */
> +#define BATADV_TT_CLIENT_TEMP_TIMEOUT_FACT 10
At least make a comment what this value says (fact * orig_interval) or
something like that. And you may have to make it UL to really return unsigned
long in batadv_tt_client_temp_timeout.
> -static void batadv_tt_global_roam_purge(struct batadv_priv *bat_priv)
> +static void batadv_tt_global_purge(struct batadv_priv *bat_priv)
> {
> struct batadv_hashtable *hash = bat_priv->tt_global_hash;
> struct hlist_head *head;
> + struct hlist_node *node, *node_tmp;
> spinlock_t *list_lock; /* protects write access to the hash lists */
> uint32_t i;
> + bool purge;
> + char *msg = NULL;
> + struct batadv_tt_common_entry *tt_common_entry;
> + struct batadv_tt_global_entry *tt_global;
> + unsigned long temp_timeout = batadv_tt_client_temp_timeout(bat_priv);
> + unsigned long roam_timeout = BATADV_TT_CLIENT_ROAM_TIMEOUT;
>
> for (i = 0; i < hash->size; i++) {
> head = &hash->table[i];
> list_lock = &hash->list_locks[i];
>
> spin_lock_bh(list_lock);
> - batadv_tt_global_roam_purge_list(bat_priv, head);
> + hlist_for_each_entry_safe(tt_common_entry, node, node_tmp,
> + head, hash_entry) {
> + purge = false;
> + tt_global = container_of(tt_common_entry,
> + struct batadv_tt_global_entry,
> + common);
> + if ((tt_global->common.flags & BATADV_TT_CLIENT_ROAM) &&
> + batadv_has_timed_out(tt_global->roam_at,
> + roam_timeout)) {
> + purge = true;
> + msg = "Roaming timeout\n";
> + }
> +
> + if ((tt_global->common.flags & BATADV_TT_CLIENT_TEMP) &&
> + batadv_has_timed_out(tt_global->common.added_at,
> + temp_timeout)) {
> + purge = true;
> + msg = "Temporary client timeout\n";
> + }
> +
> + if (!purge)
> + continue;
> +
> + batadv_dbg(BATADV_DBG_TT, bat_priv,
> + "Deleting global tt entry (%pM): %s\n",
> + tt_global->common.addr, msg);
> +
> + hlist_del_rcu(node);
> + batadv_tt_global_entry_free_ref(tt_global);
> + }
> spin_unlock_bh(list_lock);
> }
Why writing this whole thing in one function? You don't get a price for the
highest indentation level that banged the hardest against the 80 character
limit.
> - if (tt_common_entry->flags & BATADV_TT_CLIENT_ROAM)
> + if (tt_common_entry->flags & BATADV_TT_CLIENT_ROAM ||
> + tt_common_entry->flags & BATADV_TT_CLIENT_TEMP)
> return 0;
>
> +
> tt_global_entry = container_of(tt_common_entry,
> struct batadv_tt_global_entry,
> common);
And this new line doesn't make sense
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [B.A.T.M.A.N.] [PATCH 4/5] batman-adv: detect not yet announced clients
2012-06-26 21:50 ` Sven Eckelmann
@ 2012-06-27 6:34 ` Antonio Quartulli
0 siblings, 0 replies; 16+ messages in thread
From: Antonio Quartulli @ 2012-06-27 6:34 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 2226 bytes --]
On Tue, Jun 26, 2012 at 11:50:22PM +0200, Sven Eckelmann wrote:
> >
> > for (i = 0; i < hash->size; i++) {
> > head = &hash->table[i];
> > list_lock = &hash->list_locks[i];
> >
> > spin_lock_bh(list_lock);
> > - batadv_tt_global_roam_purge_list(bat_priv, head);
> > + hlist_for_each_entry_safe(tt_common_entry, node, node_tmp,
> > + head, hash_entry) {
> > + purge = false;
> > + tt_global = container_of(tt_common_entry,
> > + struct batadv_tt_global_entry,
> > + common);
> > + if ((tt_global->common.flags & BATADV_TT_CLIENT_ROAM) &&
> > + batadv_has_timed_out(tt_global->roam_at,
> > + roam_timeout)) {
> > + purge = true;
> > + msg = "Roaming timeout\n";
> > + }
> > +
> > + if ((tt_global->common.flags & BATADV_TT_CLIENT_TEMP) &&
> > + batadv_has_timed_out(tt_global->common.added_at,
> > + temp_timeout)) {
> > + purge = true;
> > + msg = "Temporary client timeout\n";
> > + }
> > +
> > + if (!purge)
> > + continue;
> > +
> > + batadv_dbg(BATADV_DBG_TT, bat_priv,
> > + "Deleting global tt entry (%pM): %s\n",
> > + tt_global->common.addr, msg);
> > +
> > + hlist_del_rcu(node);
> > + batadv_tt_global_entry_free_ref(tt_global);
> > + }
> > spin_unlock_bh(list_lock);
> > }
>
> Why writing this whole thing in one function? You don't get a price for the
> highest indentation level that banged the hardest against the 80 character
> limit.
Mh, I like challenges :-) However, I'll add another function like:
has_to_be_purged() or similar in which i will check for the conditions and I
will use it only in the loop.
>
>
> > - if (tt_common_entry->flags & BATADV_TT_CLIENT_ROAM)
> > + if (tt_common_entry->flags & BATADV_TT_CLIENT_ROAM ||
> > + tt_common_entry->flags & BATADV_TT_CLIENT_TEMP)
> > return 0;
> >
> > +
> > tt_global_entry = container_of(tt_common_entry,
> > struct batadv_tt_global_entry,
> > common);
>
> And this new line doesn't make sense
I agree.
Thank you very much Sven.
Cheers,
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* [B.A.T.M.A.N.] [PATCH 5/5] batman-adv: change interface_rx to get orig node
2012-06-26 20:00 [B.A.T.M.A.N.] [PATCH 0/5] new feature: Speedy Join 2012-06-26 Antonio Quartulli
` (3 preceding siblings ...)
2012-06-26 20:00 ` [B.A.T.M.A.N.] [PATCH 4/5] batman-adv: detect not yet announced clients Antonio Quartulli
@ 2012-06-26 20:00 ` Antonio Quartulli
4 siblings, 0 replies; 16+ messages in thread
From: Antonio Quartulli @ 2012-06-26 20:00 UTC (permalink / raw)
To: b.a.t.m.a.n
In order to understand where a broadcast packet is coming from and use
this information to detect not yet announced clients, this patch modifies the
interface_rx() function by passing a new argument: the orig node
corresponding to the node that originated the received packet (if known).
This new argument if not NULL for broadcast packets only (other packets does not
have source field).
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
routing.c | 10 ++++++----
soft-interface.c | 6 +++++-
soft-interface.h | 5 +++--
3 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/routing.c b/routing.c
index b3da42c..30fa89f 100644
--- a/routing.c
+++ b/routing.c
@@ -1026,8 +1026,9 @@ int batadv_recv_unicast_packet(struct sk_buff *skb,
/* packet for me */
if (batadv_is_my_mac(unicast_packet->dest)) {
- batadv_interface_rx(recv_if->soft_iface, skb, recv_if,
- hdr_size);
+ batadv_interface_rx(recv_if->soft_iface, skb, recv_if, hdr_size,
+ NULL);
+
return NET_RX_SUCCESS;
}
@@ -1064,7 +1065,7 @@ int batadv_recv_ucast_frag_packet(struct sk_buff *skb,
return NET_RX_SUCCESS;
batadv_interface_rx(recv_if->soft_iface, new_skb, recv_if,
- sizeof(struct batadv_unicast_packet));
+ sizeof(struct batadv_unicast_packet), NULL);
return NET_RX_SUCCESS;
}
@@ -1151,7 +1152,8 @@ int batadv_recv_bcast_packet(struct sk_buff *skb,
goto out;
/* broadcast for me */
- batadv_interface_rx(recv_if->soft_iface, skb, recv_if, hdr_size);
+ batadv_interface_rx(recv_if->soft_iface, skb, recv_if, hdr_size,
+ orig_node);
ret = NET_RX_SUCCESS;
goto out;
diff --git a/soft-interface.c b/soft-interface.c
index c77473e..8606a5e 100644
--- a/soft-interface.c
+++ b/soft-interface.c
@@ -268,7 +268,7 @@ end:
void batadv_interface_rx(struct net_device *soft_iface,
struct sk_buff *skb, struct batadv_hard_iface *recv_if,
- int hdr_size)
+ int hdr_size, struct batadv_orig_node *orig_node)
{
struct batadv_priv *bat_priv = netdev_priv(soft_iface);
struct ethhdr *ethhdr;
@@ -316,6 +316,10 @@ void batadv_interface_rx(struct net_device *soft_iface,
soft_iface->last_rx = jiffies;
+ if (orig_node)
+ batadv_tt_add_temporary_global_entry(bat_priv, orig_node,
+ ethhdr->h_source);
+
if (batadv_is_ap_isolated(bat_priv, ethhdr->h_source, ethhdr->h_dest))
goto dropped;
diff --git a/soft-interface.h b/soft-interface.h
index 852c683..07a08fe 100644
--- a/soft-interface.h
+++ b/soft-interface.h
@@ -21,8 +21,9 @@
#define _NET_BATMAN_ADV_SOFT_INTERFACE_H_
int batadv_skb_head_push(struct sk_buff *skb, unsigned int len);
-void batadv_interface_rx(struct net_device *soft_iface, struct sk_buff *skb,
- struct batadv_hard_iface *recv_if, int hdr_size);
+void batadv_interface_rx(struct net_device *soft_iface,
+ struct sk_buff *skb, struct batadv_hard_iface *recv_if,
+ int hdr_size, struct batadv_orig_node *orig_node);
struct net_device *batadv_softif_create(const char *name);
void batadv_softif_destroy(struct net_device *soft_iface);
int batadv_softif_is_valid(const struct net_device *net_dev);
--
1.7.9.4
^ permalink raw reply related [flat|nested] 16+ messages in thread