[B.A.T.M.A.N.] [PATCH] batman-adv: pass the WIFI flag from the local to global entry

[B.A.T.M.A.N.] [PATCH] batman-adv: pass the WIFI flag from the local to global entry

[Antonio Quartulli]

in case of client roaming a new global entry is added while a corresponding
local one is still present. In this case the node can safely pass the WIFI flag
from the local to the global entry

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 translation-table.c |   36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/translation-table.c b/translation-table.c
index 37ae4a9..2aacde9 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -500,21 +500,30 @@ batadv_tt_local_set_pending(struct batadv_priv *bat_priv,
 		   tt_local_entry->common.addr, message);
 }
 
+static void __batadv_tt_local_remove(struct batadv_priv *bat_priv,
+				     struct batadv_tt_local_entry *tt_local,
+				     const char *message, bool roaming)
+{
+	uint16_t flags;
+
+	flags = BATADV_TT_CLIENT_DEL;
+	if (roaming)
+		flags |= BATADV_TT_CLIENT_ROAM;
+
+	batadv_tt_local_set_pending(bat_priv, tt_local, flags, message);
+}
+
 void batadv_tt_local_remove(struct batadv_priv *bat_priv, const uint8_t *addr,
 			    const char *message, bool roaming)
 {
 	struct batadv_tt_local_entry *tt_local_entry = NULL;
-	uint16_t flags;
 
 	tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr);
 	if (!tt_local_entry)
 		goto out;
 
-	flags = BATADV_TT_CLIENT_DEL;
-	if (roaming)
-		flags |= BATADV_TT_CLIENT_ROAM;
+	__batadv_tt_local_remove(bat_priv, tt_local_entry, message, roaming);
 
-	batadv_tt_local_set_pending(bat_priv, tt_local_entry, flags, message);
 out:
 	if (tt_local_entry)
 		batadv_tt_local_entry_free_ref(tt_local_entry);
@@ -721,9 +730,11 @@ int batadv_tt_global_add(struct batadv_priv *bat_priv,
 			 uint8_t ttvn)
 {
 	struct batadv_tt_global_entry *tt_global_entry = NULL;
+	struct batadv_tt_local_entry *tt_local_entry = NULL;
 	int ret = 0;
 	int hash_added;
 	struct batadv_tt_common_entry *common;
+	uint16_t wifi_flag;
 
 	tt_global_entry = batadv_tt_global_hash_find(bat_priv, tt_addr);
 
@@ -789,14 +800,23 @@ int batadv_tt_global_add(struct batadv_priv *bat_priv,
 		   tt_global_entry->common.addr, orig_node->orig);
 
 out_remove:
+	tt_local_entry = batadv_tt_local_hash_find(bat_priv, tt_addr);
+	if (!tt_local_entry)
+		goto out;
+
+	wifi_flag = tt_local_entry->common.flags & BATADV_TT_CLIENT_WIFI;
+	tt_global_entry->common.flags |= wifi_flag;
+
 	/* remove address from local hash if present */
-	batadv_tt_local_remove(bat_priv, tt_global_entry->common.addr,
-			       "global tt received",
-			       flags & BATADV_TT_CLIENT_ROAM);
+	__batadv_tt_local_remove(bat_priv, tt_local_entry, "global tt received",
+				 flags & BATADV_TT_CLIENT_ROAM);
+
 	ret = 1;
 out:
 	if (tt_global_entry)
 		batadv_tt_global_entry_free_ref(tt_global_entry);
+	if (tt_local_entry)
+		batadv_tt_local_entry_free_ref(tt_local_entry);
 	return ret;
 }
 
-- 
1.7.9.4

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: pass the WIFI flag from the local to global entry

[Marek Lindner]

On Monday, July 30, 2012 01:15:30 Antonio Quartulli wrote:
> in case of client roaming a new global entry is added while a corresponding
> local one is still present. In this case the node can safely pass the WIFI
> flag from the local to the global entry

And why would we want that ?

Regards,
Marek

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: pass the WIFI flag from the local to global entry

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 796 bytes --]

On Mon, Jul 30, 2012 at 09:59:16AM +0200, Marek Lindner wrote:
> On Monday, July 30, 2012 01:15:30 Antonio Quartulli wrote:
> > in case of client roaming a new global entry is added while a corresponding
> > local one is still present. In this case the node can safely pass the WIFI
> > flag from the local to the global entry
> 
> And why would we want that ?

To let the AP-isolation work. If a client is known to be WIFI and we do not
"pass" this flag on roaming, we end up in a state where this client is not
marked as WIFI anymore (even if it should be).

During this time period the client will be able to talk to the other WIFI clients
even if the AP isolation is enabled

Cheers,


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: pass the WIFI flag from the local to global entry

[Marek Lindner]

On Monday, July 30, 2012 11:23:43 Antonio Quartulli wrote:
>   On Mon, Jul 30, 2012 at 09:59:16AM +0200, Marek Lindner wrote:
> > On Monday, July 30, 2012 01:15:30 Antonio Quartulli wrote:
> > > in case of client roaming a new global entry is added while a
> > > corresponding local one is still present. In this case the node can
> > > safely pass the WIFI flag from the local to the global entry
> >
> > And why would we want that ?
> 
> To let the AP-isolation work. If a client is known to be WIFI and we do not
> "pass" this flag on roaming, we end up in a state where this client is not
> marked as WIFI anymore (even if it should be).
> 
> During this time period the client will be able to talk to the other WIFI
> clients even if the AP isolation is enabled

And why is this explanation not part of the commit message ?

Regards,
Marek

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: pass the WIFI flag from the local to global entry

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 1392 bytes --]

On Mon, Jul 30, 2012 at 11:47:03AM +0200, Marek Lindner wrote:
> On Monday, July 30, 2012 11:23:43 Antonio Quartulli wrote:
> >   On Mon, Jul 30, 2012 at 09:59:16AM +0200, Marek Lindner wrote:
> > > On Monday, July 30, 2012 01:15:30 Antonio Quartulli wrote:
> > > > in case of client roaming a new global entry is added while a
> > > > corresponding local one is still present. In this case the node can
> > > > safely pass the WIFI flag from the local to the global entry
> > >
> > > And why would we want that ?
> > 
> > To let the AP-isolation work. If a client is known to be WIFI and we do not
> > "pass" this flag on roaming, we end up in a state where this client is not
> > marked as WIFI anymore (even if it should be).
> > 
> > During this time period the client will be able to talk to the other WIFI
> > clients even if the AP isolation is enabled
> 
> And why is this explanation not part of the commit message ?

Because it is left as exercise for the reader ;-)
Actually I didn't mention it because the Ap-Isolation part is a "consequence".
What this patch wants to fix is an issue in the client entry state consistency,
regardless of the semantic of such state.

Anyhow, if you think it is worth adding it, I will do that.

Cheers,

> 
> Regards,
> Marek

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: pass the WIFI flag from the local to global entry

[Marek Lindner]

On Monday, July 30, 2012 12:07:04 Antonio Quartulli wrote:
> > And why is this explanation not part of the commit message ?
> 
> Because it is left as exercise for the reader ;-)
> Actually I didn't mention it because the Ap-Isolation part is a
> "consequence". What this patch wants to fix is an issue in the client
> entry state consistency, regardless of the semantic of such state.
> 
> Anyhow, if you think it is worth adding it, I will do that.

Well, we need some explanations *why* we want this change. The way you are 
describing it now it sounds like we want to preserve all relevant flags during 
the transition from local to global client and vice versa ?

Regards,
Marek

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: pass the WIFI flag from the local to global entry

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]

On Mon, Jul 30, 2012 at 12:12:19PM +0200, Marek Lindner wrote:
> On Monday, July 30, 2012 12:07:04 Antonio Quartulli wrote:
> > > And why is this explanation not part of the commit message ?
> > 
> > Because it is left as exercise for the reader ;-)
> > Actually I didn't mention it because the Ap-Isolation part is a
> > "consequence". What this patch wants to fix is an issue in the client
> > entry state consistency, regardless of the semantic of such state.
> > 
> > Anyhow, if you think it is worth adding it, I will do that.
> 
> Well, we need some explanations *why* we want this change. The way you are 
> describing it now it sounds like we want to preserve all relevant flags during 
> the transition from local to global client and vice versa ?

Yes, that is the message I had in mind while writing the commit message. But ok,
I will send v2 and I'll be more verbose in order to let people understand what
this issue could lead to.

Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[B.A.T.M.A.N.] [PATCHv2] batman-adv: pass the WIFI flag from the local to global entry

[Antonio Quartulli]

in case of client roaming a new global entry is added while a corresponding
local one is still present. In this case the node can safely pass the WIFI flag
from the local to the global entry.

This change is required to let the AP-isolation correctly working in case of
roaming: if a generic WIFI client C roams from node A to B, A adds a global
entry for C without adding any WIFI flag. The latter will be set only later,
once A has received C's advertisement from B. In this time period the
AP-Isolation (if enabled) would not correctly work since C is not marked as
WIFI, so allowing it to communicate with other WIFI clients.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---

v2:
- commit message rearranged

 translation-table.c |   36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/translation-table.c b/translation-table.c
index 37ae4a9..2aacde9 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -500,21 +500,30 @@ batadv_tt_local_set_pending(struct batadv_priv *bat_priv,
 		   tt_local_entry->common.addr, message);
 }
 
+static void __batadv_tt_local_remove(struct batadv_priv *bat_priv,
+				     struct batadv_tt_local_entry *tt_local,
+				     const char *message, bool roaming)
+{
+	uint16_t flags;
+
+	flags = BATADV_TT_CLIENT_DEL;
+	if (roaming)
+		flags |= BATADV_TT_CLIENT_ROAM;
+
+	batadv_tt_local_set_pending(bat_priv, tt_local, flags, message);
+}
+
 void batadv_tt_local_remove(struct batadv_priv *bat_priv, const uint8_t *addr,
 			    const char *message, bool roaming)
 {
 	struct batadv_tt_local_entry *tt_local_entry = NULL;
-	uint16_t flags;
 
 	tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr);
 	if (!tt_local_entry)
 		goto out;
 
-	flags = BATADV_TT_CLIENT_DEL;
-	if (roaming)
-		flags |= BATADV_TT_CLIENT_ROAM;
+	__batadv_tt_local_remove(bat_priv, tt_local_entry, message, roaming);
 
-	batadv_tt_local_set_pending(bat_priv, tt_local_entry, flags, message);
 out:
 	if (tt_local_entry)
 		batadv_tt_local_entry_free_ref(tt_local_entry);
@@ -721,9 +730,11 @@ int batadv_tt_global_add(struct batadv_priv *bat_priv,
 			 uint8_t ttvn)
 {
 	struct batadv_tt_global_entry *tt_global_entry = NULL;
+	struct batadv_tt_local_entry *tt_local_entry = NULL;
 	int ret = 0;
 	int hash_added;
 	struct batadv_tt_common_entry *common;
+	uint16_t wifi_flag;
 
 	tt_global_entry = batadv_tt_global_hash_find(bat_priv, tt_addr);
 
@@ -789,14 +800,23 @@ int batadv_tt_global_add(struct batadv_priv *bat_priv,
 		   tt_global_entry->common.addr, orig_node->orig);
 
 out_remove:
+	tt_local_entry = batadv_tt_local_hash_find(bat_priv, tt_addr);
+	if (!tt_local_entry)
+		goto out;
+
+	wifi_flag = tt_local_entry->common.flags & BATADV_TT_CLIENT_WIFI;
+	tt_global_entry->common.flags |= wifi_flag;
+
 	/* remove address from local hash if present */
-	batadv_tt_local_remove(bat_priv, tt_global_entry->common.addr,
-			       "global tt received",
-			       flags & BATADV_TT_CLIENT_ROAM);
+	__batadv_tt_local_remove(bat_priv, tt_local_entry, "global tt received",
+				 flags & BATADV_TT_CLIENT_ROAM);
+
 	ret = 1;
 out:
 	if (tt_global_entry)
 		batadv_tt_global_entry_free_ref(tt_global_entry);
+	if (tt_local_entry)
+		batadv_tt_local_entry_free_ref(tt_local_entry);
 	return ret;
 }
 
-- 
1.7.9.4

[B.A.T.M.A.N.] [PATCH v3] batman-adv: pass the WIFI flag from the local to global entry

[Antonio Quartulli]

in case of client roaming a new global entry is added while a corresponding
local one is still present. In this case the node can safely pass the WIFI flag
from the local to the global entry.

This change is required to let the AP-isolation correctly working in case of
roaming: if a generic WIFI client C roams from node A to B, A adds a global
entry for C without adding any WIFI flag. The latter will be set only later,
once A has received C's advertisement from B. In this time period the
AP-Isolation (if enabled) would not correctly work since C is not marked as
WIFI, so allowing it to communicate with other WIFI clients.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---

v3:
- rebased on top of latest master + "batman-adv: return proper value in case of
  hash_add failure" there the latter patch must be merged before applying this
  one.


 translation-table.c |   36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/translation-table.c b/translation-table.c
index eb352db..b257067 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -501,21 +501,30 @@ batadv_tt_local_set_pending(struct batadv_priv *bat_priv,
 		   tt_local_entry->common.addr, message);
 }
 
+static void __batadv_tt_local_remove(struct batadv_priv *bat_priv,
+				     struct batadv_tt_local_entry *tt_local,
+				     const char *message, bool roaming)
+{
+	uint16_t flags;
+
+	flags = BATADV_TT_CLIENT_DEL;
+	if (roaming)
+		flags |= BATADV_TT_CLIENT_ROAM;
+
+	batadv_tt_local_set_pending(bat_priv, tt_local, flags, message);
+}
+
 void batadv_tt_local_remove(struct batadv_priv *bat_priv, const uint8_t *addr,
 			    const char *message, bool roaming)
 {
 	struct batadv_tt_local_entry *tt_local_entry = NULL;
-	uint16_t flags;
 
 	tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr);
 	if (!tt_local_entry)
 		goto out;
 
-	flags = BATADV_TT_CLIENT_DEL;
-	if (roaming)
-		flags |= BATADV_TT_CLIENT_ROAM;
+	__batadv_tt_local_remove(bat_priv, tt_local_entry, message, roaming);
 
-	batadv_tt_local_set_pending(bat_priv, tt_local_entry, flags, message);
 out:
 	if (tt_local_entry)
 		batadv_tt_local_entry_free_ref(tt_local_entry);
@@ -722,9 +731,11 @@ int batadv_tt_global_add(struct batadv_priv *bat_priv,
 			 uint8_t ttvn)
 {
 	struct batadv_tt_global_entry *tt_global_entry = NULL;
+	struct batadv_tt_local_entry *tt_local_entry = NULL;
 	int ret = 0;
 	int hash_added;
 	struct batadv_tt_common_entry *common;
+	uint16_t wifi_flag;
 
 	tt_global_entry = batadv_tt_global_hash_find(bat_priv, tt_addr);
 
@@ -791,13 +802,22 @@ int batadv_tt_global_add(struct batadv_priv *bat_priv,
 	ret = 1;
 
 out_remove:
+	tt_local_entry = batadv_tt_local_hash_find(bat_priv, tt_addr);
+	if (!tt_local_entry)
+		goto out;
+
+	wifi_flag = tt_local_entry->common.flags & BATADV_TT_CLIENT_WIFI;
+	tt_global_entry->common.flags |= wifi_flag;
+
 	/* remove address from local hash if present */
-	batadv_tt_local_remove(bat_priv, tt_global_entry->common.addr,
-			       "global tt received",
-			       flags & BATADV_TT_CLIENT_ROAM);
+	__batadv_tt_local_remove(bat_priv, tt_local_entry, "global tt received",
+				 flags & BATADV_TT_CLIENT_ROAM);
+
 out:
 	if (tt_global_entry)
 		batadv_tt_global_entry_free_ref(tt_global_entry);
+	if (tt_local_entry)
+		batadv_tt_local_entry_free_ref(tt_local_entry);
 	return ret;
 }
 
-- 
1.7.9.4

Re: [B.A.T.M.A.N.] [PATCH v3] batman-adv: pass the WIFI flag from the local to global entry

[Marek Lindner]

On Saturday, August 11, 2012 00:43:19 Antonio Quartulli wrote:
> +static void __batadv_tt_local_remove(struct batadv_priv *bat_priv,
> +                                    struct batadv_tt_local_entry
> *tt_local, +                                    const char *message, bool
> roaming) +{
> +       uint16_t flags;
> +
> +       flags = BATADV_TT_CLIENT_DEL;
> +       if (roaming)
> +               flags |= BATADV_TT_CLIENT_ROAM;
> +
> +       batadv_tt_local_set_pending(bat_priv, tt_local, flags, message);
> +}

How about adding some kernel documentation ?  :)

Cheers,
Marek

Re: [B.A.T.M.A.N.] [PATCH v3] batman-adv: pass the WIFI flag from the local to global entry

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 879 bytes --]

On Wed, Aug 22, 2012 at 02:03:50 +0200, Marek Lindner wrote:
> On Saturday, August 11, 2012 00:43:19 Antonio Quartulli wrote:
> > +static void __batadv_tt_local_remove(struct batadv_priv *bat_priv,
> > +                                    struct batadv_tt_local_entry
> > *tt_local, +                                    const char *message, bool
> > roaming) +{
> > +       uint16_t flags;
> > +
> > +       flags = BATADV_TT_CLIENT_DEL;
> > +       if (roaming)
> > +               flags |= BATADV_TT_CLIENT_ROAM;
> > +
> > +       batadv_tt_local_set_pending(bat_priv, tt_local, flags, message);
> > +}
> 
> How about adding some kernel documentation ?  :)

I agree :) We discussed kernel documentation after I sent this patch. Will add
and send version 4.

Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]