* [B.A.T.M.A.N.] [PATCH] batman-adv: generalise tt_local_reset_flags()
@ 2011-10-30 19:09 Antonio Quartulli
2011-10-31 0:42 ` Simon Wunderlich
2011-11-07 15:47 ` [B.A.T.M.A.N.] [PATCHv2] " Antonio Quartulli
0 siblings, 2 replies; 6+ messages in thread
From: Antonio Quartulli @ 2011-10-30 19:09 UTC (permalink / raw)
To: b.a.t.m.a.n
The tt_local_reset_flags() is actually used for one use case only. It is not
generalised enough to be used indifferent situations. This patch make it general
enough in order to let other code use it whenever a flag flip is requested over
the whole hash table (passed as parameter).
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
This patch depends on:
batman-adv: create a common substructure for tt_global/local_entry
translation-table.c | 31 ++++++++++++++++++++-----------
1 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/translation-table.c b/translation-table.c
index 76134bc..5b60aba 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -1695,19 +1695,20 @@ void tt_free(struct bat_priv *bat_priv)
kfree(bat_priv->tt_buff);
}
-/* This function will reset the specified flags from all the entries in
- * the given hash table and will increment num_local_tt for each involved
- * entry */
-static void tt_local_reset_flags(struct bat_priv *bat_priv, uint16_t flags)
+/* This function will flip to new_value (if not already) the specified flags for
+ * all the entries in the given hash table and returns the number of modified
+ * entries */
+static uint16_t tt_flip_flags(struct hashtable_t *hash, uint16_t flags,
+ uint8_t new_value)
{
uint32_t i;
- struct hashtable_t *hash = bat_priv->tt_local_hash;
+ uint16_t changed_num = 0;
struct hlist_head *head;
struct hlist_node *node;
struct tt_common_entry *tt_common_entry;
if (!hash)
- return;
+ goto out;
for (i = 0; i < hash->size; i++) {
head = &hash->table[i];
@@ -1715,14 +1716,18 @@ static void tt_local_reset_flags(struct bat_priv *bat_priv, uint16_t flags)
rcu_read_lock();
hlist_for_each_entry_rcu(tt_common_entry, node,
head, hash_entry) {
- if (!(tt_common_entry->flags & flags))
+ if ((tt_common_entry->flags & flags) != new_value)
continue;
- tt_common_entry->flags &= ~flags;
- atomic_inc(&bat_priv->num_local_tt);
+ /* depending on 'new_value', enable or disable the flags
+ * pointed by 'flags' */
+ tt_common_entry->flags &=
+ (~flags | (new_value ? flags : NO_FLAGS));
+ changed_num++;
}
rcu_read_unlock();
}
-
+out:
+ return changed_num;
}
/* Purge out all the tt local entries marked with TT_CLIENT_PENDING */
@@ -1766,7 +1771,11 @@ static void tt_local_purge_pending_clients(struct bat_priv *bat_priv)
void tt_commit_changes(struct bat_priv *bat_priv)
{
- tt_local_reset_flags(bat_priv, TT_CLIENT_NEW);
+ uint16_t changed_num = tt_flip_flags(bat_priv->tt_local_hash,
+ TT_CLIENT_NEW, 0);
+ /* all the reset entries have now to be effectively counted as local
+ * entries */
+ atomic_add(changed_num, &bat_priv->num_local_tt);
tt_local_purge_pending_clients(bat_priv);
/* Increment the TTVN only once per OGM interval */
--
1.7.3.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: generalise tt_local_reset_flags()
2011-10-30 19:09 [B.A.T.M.A.N.] [PATCH] batman-adv: generalise tt_local_reset_flags() Antonio Quartulli
@ 2011-10-31 0:42 ` Simon Wunderlich
2011-11-01 13:43 ` Antonio Quartulli
2011-11-07 15:47 ` [B.A.T.M.A.N.] [PATCHv2] " Antonio Quartulli
1 sibling, 1 reply; 6+ messages in thread
From: Simon Wunderlich @ 2011-10-31 0:42 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
[-- Attachment #1: Type: text/plain, Size: 3548 bytes --]
Hey Antonio,
I don't quite understand what this patch is good for - it generalises the function,
but does not use the function at another point. So what it is good for (next to
add more complexity to batman ;] ).
Furthermore it counts the changes and adds it to num_local_tt, is there a bug fix
hidden in this patch somewhere? :)
Cheers,
Simon
On Sun, Oct 30, 2011 at 08:09:01PM +0100, Antonio Quartulli wrote:
> The tt_local_reset_flags() is actually used for one use case only. It is not
> generalised enough to be used indifferent situations. This patch make it general
> enough in order to let other code use it whenever a flag flip is requested over
> the whole hash table (passed as parameter).
>
> Signed-off-by: Antonio Quartulli <ordex@autistici.org>
> ---
>
> This patch depends on:
>
> batman-adv: create a common substructure for tt_global/local_entry
>
>
> translation-table.c | 31 ++++++++++++++++++++-----------
> 1 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/translation-table.c b/translation-table.c
> index 76134bc..5b60aba 100644
> --- a/translation-table.c
> +++ b/translation-table.c
> @@ -1695,19 +1695,20 @@ void tt_free(struct bat_priv *bat_priv)
> kfree(bat_priv->tt_buff);
> }
>
> -/* This function will reset the specified flags from all the entries in
> - * the given hash table and will increment num_local_tt for each involved
> - * entry */
> -static void tt_local_reset_flags(struct bat_priv *bat_priv, uint16_t flags)
> +/* This function will flip to new_value (if not already) the specified flags for
> + * all the entries in the given hash table and returns the number of modified
> + * entries */
> +static uint16_t tt_flip_flags(struct hashtable_t *hash, uint16_t flags,
> + uint8_t new_value)
> {
> uint32_t i;
> - struct hashtable_t *hash = bat_priv->tt_local_hash;
> + uint16_t changed_num = 0;
> struct hlist_head *head;
> struct hlist_node *node;
> struct tt_common_entry *tt_common_entry;
>
> if (!hash)
> - return;
> + goto out;
>
> for (i = 0; i < hash->size; i++) {
> head = &hash->table[i];
> @@ -1715,14 +1716,18 @@ static void tt_local_reset_flags(struct bat_priv *bat_priv, uint16_t flags)
> rcu_read_lock();
> hlist_for_each_entry_rcu(tt_common_entry, node,
> head, hash_entry) {
> - if (!(tt_common_entry->flags & flags))
> + if ((tt_common_entry->flags & flags) != new_value)
> continue;
> - tt_common_entry->flags &= ~flags;
> - atomic_inc(&bat_priv->num_local_tt);
> + /* depending on 'new_value', enable or disable the flags
> + * pointed by 'flags' */
> + tt_common_entry->flags &=
> + (~flags | (new_value ? flags : NO_FLAGS));
> + changed_num++;
> }
> rcu_read_unlock();
> }
> -
> +out:
> + return changed_num;
> }
>
> /* Purge out all the tt local entries marked with TT_CLIENT_PENDING */
> @@ -1766,7 +1771,11 @@ static void tt_local_purge_pending_clients(struct bat_priv *bat_priv)
>
> void tt_commit_changes(struct bat_priv *bat_priv)
> {
> - tt_local_reset_flags(bat_priv, TT_CLIENT_NEW);
> + uint16_t changed_num = tt_flip_flags(bat_priv->tt_local_hash,
> + TT_CLIENT_NEW, 0);
> + /* all the reset entries have now to be effectively counted as local
> + * entries */
> + atomic_add(changed_num, &bat_priv->num_local_tt);
> tt_local_purge_pending_clients(bat_priv);
>
> /* Increment the TTVN only once per OGM interval */
> --
> 1.7.3.4
>
>
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: generalise tt_local_reset_flags()
2011-10-31 0:42 ` Simon Wunderlich
@ 2011-11-01 13:43 ` Antonio Quartulli
2011-11-04 15:00 ` Simon Wunderlich
0 siblings, 1 reply; 6+ messages in thread
From: Antonio Quartulli @ 2011-11-01 13:43 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
On Mon, Oct 31, 2011 at 01:42:01AM +0100, Simon Wunderlich wrote:
> Hey Antonio,
>
> I don't quite understand what this patch is good for - it generalises the function,
> but does not use the function at another point. So what it is good for (next to
> add more complexity to batman ;] ).
>
> Furthermore it counts the changes and adds it to num_local_tt, is there a bug fix
> hidden in this patch somewhere? :)
No hidden bug-fix here :)
This patch aims to generalise the tt_local_reset_flags() function only. As I
discussed with Sven on IRC, this function could be quite confusing, because its
name suggests that it can be used either for other purposes, but this is not
true: it can only be used as it is now in the code.
Therefore this patch wants to generalise it enough in order to be eventually
used elsewhere.
The increment of tt_local_num is actually done inside the function and it is
useful only for the use case proposed in the current code. To generalise the
function I had to move the increment outside (somebody else could want to flip a
certain flag without affecting the local counter :-) ).
IMHO this patch is not strictly needed, but it would clean the code up (Until
it doesn't add any complexity :P)
Cheers,
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: generalise tt_local_reset_flags()
2011-11-01 13:43 ` Antonio Quartulli
@ 2011-11-04 15:00 ` Simon Wunderlich
0 siblings, 0 replies; 6+ messages in thread
From: Simon Wunderlich @ 2011-11-04 15:00 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
[-- Attachment #1: Type: text/plain, Size: 1942 bytes --]
Hey there,
On Tue, Nov 01, 2011 at 02:43:49PM +0100, Antonio Quartulli wrote:
> On Mon, Oct 31, 2011 at 01:42:01AM +0100, Simon Wunderlich wrote:
> > Hey Antonio,
> >
> > I don't quite understand what this patch is good for - it generalises the function,
> > but does not use the function at another point. So what it is good for (next to
> > add more complexity to batman ;] ).
> >
> > Furthermore it counts the changes and adds it to num_local_tt, is there a bug fix
> > hidden in this patch somewhere? :)
>
> No hidden bug-fix here :)
> This patch aims to generalise the tt_local_reset_flags() function only. As I
> discussed with Sven on IRC, this function could be quite confusing, because its
> name suggests that it can be used either for other purposes, but this is not
> true: it can only be used as it is now in the code.
>
> Therefore this patch wants to generalise it enough in order to be eventually
> used elsewhere.
OK
>
> The increment of tt_local_num is actually done inside the function and it is
> useful only for the use case proposed in the current code. To generalise the
> function I had to move the increment outside (somebody else could want to flip a
> certain flag without affecting the local counter :-) ).
Ah okay, I missed that it was already done within the function previously - sorry,
my fault. No hidden bug fix :D
>
> IMHO this patch is not strictly needed, but it would clean the code up (Until
> it doesn't add any complexity :P)
I think thats okay. I reviewed it a second time and it does what its supposed to
do as far as i can tell.
The only thing I found a little bit confusing was the variable name "new_value",
as it turns out it is only a switch (0 - switch all flags from the flags variable
off, 1 - switch these flags on). But maybe thats just me. ;)
Otherwise,
Acked-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Cheers,
Simon
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [B.A.T.M.A.N.] [PATCHv2] batman-adv: generalise tt_local_reset_flags()
2011-10-30 19:09 [B.A.T.M.A.N.] [PATCH] batman-adv: generalise tt_local_reset_flags() Antonio Quartulli
2011-10-31 0:42 ` Simon Wunderlich
@ 2011-11-07 15:47 ` Antonio Quartulli
2011-11-08 17:02 ` Marek Lindner
1 sibling, 1 reply; 6+ messages in thread
From: Antonio Quartulli @ 2011-11-07 15:47 UTC (permalink / raw)
To: b.a.t.m.a.n
The tt_local_reset_flags() is actually used for one use case only. It is not
generalised enough to be used indifferent situations. This patch make it general
enough in order to let other code use it whenever a flag set is requested over
the whole hash table (passed as parameter). The function is now called
tt_set_flags()
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
- argument new_value substituted with enable
- boolean logic more readable inside tt_set_flags()
translation-table.c | 36 ++++++++++++++++++++++++------------
1 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/translation-table.c b/translation-table.c
index 76134bc..379529d 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -1695,19 +1695,20 @@ void tt_free(struct bat_priv *bat_priv)
kfree(bat_priv->tt_buff);
}
-/* This function will reset the specified flags from all the entries in
- * the given hash table and will increment num_local_tt for each involved
- * entry */
-static void tt_local_reset_flags(struct bat_priv *bat_priv, uint16_t flags)
+/* This function will flip to new_value (if not already) the specified flags for
+ * all the entries in the given hash table and returns the number of modified
+ * entries */
+static uint16_t tt_set_flags(struct hashtable_t *hash, uint16_t flags,
+ bool enable)
{
uint32_t i;
- struct hashtable_t *hash = bat_priv->tt_local_hash;
+ uint16_t changed_num = 0;
struct hlist_head *head;
struct hlist_node *node;
struct tt_common_entry *tt_common_entry;
if (!hash)
- return;
+ goto out;
for (i = 0; i < hash->size; i++) {
head = &hash->table[i];
@@ -1715,14 +1716,21 @@ static void tt_local_reset_flags(struct bat_priv *bat_priv, uint16_t flags)
rcu_read_lock();
hlist_for_each_entry_rcu(tt_common_entry, node,
head, hash_entry) {
- if (!(tt_common_entry->flags & flags))
- continue;
- tt_common_entry->flags &= ~flags;
- atomic_inc(&bat_priv->num_local_tt);
+ if (enable) {
+ if ((tt_common_entry->flags & flags) == flags)
+ continue;
+ tt_common_entry->flags |= flags;
+ } else {
+ if (!(tt_common_entry->flags & flags))
+ continue;
+ tt_common_entry->flags &= ~flags;
+ }
+ changed_num++;
}
rcu_read_unlock();
}
-
+out:
+ return changed_num;
}
/* Purge out all the tt local entries marked with TT_CLIENT_PENDING */
@@ -1766,7 +1774,11 @@ static void tt_local_purge_pending_clients(struct bat_priv *bat_priv)
void tt_commit_changes(struct bat_priv *bat_priv)
{
- tt_local_reset_flags(bat_priv, TT_CLIENT_NEW);
+ uint16_t changed_num = tt_set_flags(bat_priv->tt_local_hash,
+ TT_CLIENT_NEW, false);
+ /* all the reset entries have now to be effectively counted as local
+ * entries */
+ atomic_add(changed_num, &bat_priv->num_local_tt);
tt_local_purge_pending_clients(bat_priv);
/* Increment the TTVN only once per OGM interval */
--
1.7.3.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [B.A.T.M.A.N.] [PATCHv2] batman-adv: generalise tt_local_reset_flags()
2011-11-07 15:47 ` [B.A.T.M.A.N.] [PATCHv2] " Antonio Quartulli
@ 2011-11-08 17:02 ` Marek Lindner
0 siblings, 0 replies; 6+ messages in thread
From: Marek Lindner @ 2011-11-08 17:02 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
On Monday, November 07, 2011 23:47:01 Antonio Quartulli wrote:
> The tt_local_reset_flags() is actually used for one use case only. It is
> not generalised enough to be used indifferent situations. This patch make
> it general enough in order to let other code use it whenever a flag set is
> requested over the whole hash table (passed as parameter). The function is
> now called tt_set_flags()
Applied in reivsion 3e0b5d1.
Thanks,
Marek
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-11-08 17:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-30 19:09 [B.A.T.M.A.N.] [PATCH] batman-adv: generalise tt_local_reset_flags() Antonio Quartulli
2011-10-31 0:42 ` Simon Wunderlich
2011-11-01 13:43 ` Antonio Quartulli
2011-11-04 15:00 ` Simon Wunderlich
2011-11-07 15:47 ` [B.A.T.M.A.N.] [PATCHv2] " Antonio Quartulli
2011-11-08 17:02 ` Marek Lindner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox