[B.A.T.M.A.N.] [PATCH] batman-adv: don't redirect tt_request for me

[B.A.T.M.A.N.] [PATCH] batman-adv: don't redirect tt_request for me

[Antonio Quartulli]

If a tt_request is directed to me, it never has to be redirected

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

diff --git a/translation-table.c b/translation-table.c
index 2a6d7d6..c32c5f6 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -1648,7 +1648,7 @@ out:
 
 }
 
-static bool
+static void
 batadv_send_my_tt_response(struct batadv_priv *bat_priv,
 			   struct batadv_tt_query_packet *tt_request)
 {
@@ -1656,7 +1656,6 @@ batadv_send_my_tt_response(struct batadv_priv *bat_priv,
 	struct batadv_neigh_node *neigh_node = NULL;
 	struct batadv_hard_iface *primary_if = NULL;
 	uint8_t my_ttvn, req_ttvn, ttvn;
-	int ret = false;
 	unsigned char *tt_buff;
 	bool full_table;
 	uint16_t tt_len, tt_tot;
@@ -1751,7 +1750,6 @@ batadv_send_my_tt_response(struct batadv_priv *bat_priv,
 	batadv_inc_counter(bat_priv, BATADV_CNT_TT_RESPONSE_TX);
 
 	batadv_send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr);
-	ret = true;
 	goto out;
 
 unlock:
@@ -1765,8 +1763,6 @@ out:
 		batadv_hardif_free_ref(primary_if);
 	if (!ret)
 		kfree_skb(skb);
-	/* This packet was for me, so it doesn't need to be re-routed */
-	return true;
 }
 
 bool batadv_send_tt_response(struct batadv_priv *bat_priv,
@@ -1777,7 +1773,9 @@ bool batadv_send_tt_response(struct batadv_priv *bat_priv,
 		if (batadv_bla_is_backbone_gw_orig(bat_priv, tt_request->src))
 			return true;
 
-		return batadv_send_my_tt_response(bat_priv, tt_request);
+		batadv_send_my_tt_response(bat_priv, tt_request);
+		/* This packet was for me, so it never needs to be re-routed */
+		return true;
 	} else {
 		return batadv_send_other_tt_response(bat_priv, tt_request);
 	}
-- 
1.7.9.4

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: don't redirect tt_request for me

[Antonio Quartulli]

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

On Thu, Jun 14, 2012 at 11:27:27PM +0200, Antonio Quartulli wrote:
> If a tt_request is directed to me, it never has to be redirected
> 
> Signed-off-by: Antonio Quartulli <ordex@autistici.org>

sorry, drop this patch. I'll send v2

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: don't reroute tt_request for me

[Antonio Quartulli]

If a tt_request is directed to me, it never has to be rerouted

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

diff --git a/translation-table.c b/translation-table.c
index 2a6d7d6..9eb45c8 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -1648,7 +1648,7 @@ out:
 
 }
 
-static bool
+static void
 batadv_send_my_tt_response(struct batadv_priv *bat_priv,
 			   struct batadv_tt_query_packet *tt_request)
 {
@@ -1656,7 +1656,6 @@ batadv_send_my_tt_response(struct batadv_priv *bat_priv,
 	struct batadv_neigh_node *neigh_node = NULL;
 	struct batadv_hard_iface *primary_if = NULL;
 	uint8_t my_ttvn, req_ttvn, ttvn;
-	int ret = false;
 	unsigned char *tt_buff;
 	bool full_table;
 	uint16_t tt_len, tt_tot;
@@ -1751,7 +1750,6 @@ batadv_send_my_tt_response(struct batadv_priv *bat_priv,
 	batadv_inc_counter(bat_priv, BATADV_CNT_TT_RESPONSE_TX);
 
 	batadv_send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr);
-	ret = true;
 	goto out;
 
 unlock:
@@ -1763,10 +1761,6 @@ out:
 		batadv_neigh_node_free_ref(neigh_node);
 	if (primary_if)
 		batadv_hardif_free_ref(primary_if);
-	if (!ret)
-		kfree_skb(skb);
-	/* This packet was for me, so it doesn't need to be re-routed */
-	return true;
 }
 
 bool batadv_send_tt_response(struct batadv_priv *bat_priv,
@@ -1777,7 +1771,9 @@ bool batadv_send_tt_response(struct batadv_priv *bat_priv,
 		if (batadv_bla_is_backbone_gw_orig(bat_priv, tt_request->src))
 			return true;
 
-		return batadv_send_my_tt_response(bat_priv, tt_request);
+		batadv_send_my_tt_response(bat_priv, tt_request);
+		/* This packet was for me, so it never needs to be re-routed */
+		return true;
 	} else {
 		return batadv_send_other_tt_response(bat_priv, tt_request);
 	}
-- 
1.7.9.4

Re: [B.A.T.M.A.N.] [PATCHv2] batman-adv: don't reroute tt_request for me

[Marek Lindner]

On Friday, June 15, 2012 05:38:37 Antonio Quartulli wrote:
> If a tt_request is directed to me, it never has to be rerouted

We will need a little more explanation here about what is going on and what is 
fixed and why we want it to be fixed.

Cheers,
Marek

Re: [B.A.T.M.A.N.] [PATCHv2] batman-adv: don't reroute tt_request for me

[Antonio Quartulli]

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

On Sat, Jun 16, 2012 at 03:50:00AM +0800, Marek Lindner wrote:
> On Friday, June 15, 2012 05:38:37 Antonio Quartulli wrote:
> > If a tt_request is directed to me, it never has to be rerouted
> 
> We will need a little more explanation here about what is going on and what is 
> fixed and why we want it to be fixed.

Sure,
I'll send v2 with a improved message

> 
> Cheers,
> 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.] [PATCHv2] batman-adv: don't reroute tt_request for me

[Antonio Quartulli]

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

On Fri, Jun 15, 2012 at 10:20:35PM +0200, Antonio Quartulli wrote:
> On Sat, Jun 16, 2012 at 03:50:00AM +0800, Marek Lindner wrote:
> > On Friday, June 15, 2012 05:38:37 Antonio Quartulli wrote:
> > > If a tt_request is directed to me, it never has to be rerouted
> > 
> > We will need a little more explanation here about what is going on and what is 
> > fixed and why we want it to be fixed.
> 
> Sure,
> I'll send v2 with a improved message
> 

I clearly meant v3, sorry


> > 
> > Cheers,
> > Marek
> 
> -- 
> Antonio Quartulli
> 
> ..each of us alone is worth nothing..
> Ernesto "Che" Guevara



-- 
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.] [PATCHv3] batman-adv: don't try to re-route tt_request for me

[Antonio Quartulli]

If a TT_Request is directed to a node and it fails in handling it (e.g. it
does not know the source node yet), the node will print a "re-routing" message
and will try to forward the packet. However it will fails because the
destination is itself. To avoid this misbehaviour, the node has to never try to
re-route a packet directed to it.

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

diff --git a/translation-table.c b/translation-table.c
index 2a6d7d6..9eb45c8 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -1648,7 +1648,7 @@ out:
 
 }
 
-static bool
+static void
 batadv_send_my_tt_response(struct batadv_priv *bat_priv,
 			   struct batadv_tt_query_packet *tt_request)
 {
@@ -1656,7 +1656,6 @@ batadv_send_my_tt_response(struct batadv_priv *bat_priv,
 	struct batadv_neigh_node *neigh_node = NULL;
 	struct batadv_hard_iface *primary_if = NULL;
 	uint8_t my_ttvn, req_ttvn, ttvn;
-	int ret = false;
 	unsigned char *tt_buff;
 	bool full_table;
 	uint16_t tt_len, tt_tot;
@@ -1751,7 +1750,6 @@ batadv_send_my_tt_response(struct batadv_priv *bat_priv,
 	batadv_inc_counter(bat_priv, BATADV_CNT_TT_RESPONSE_TX);
 
 	batadv_send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr);
-	ret = true;
 	goto out;
 
 unlock:
@@ -1763,10 +1761,6 @@ out:
 		batadv_neigh_node_free_ref(neigh_node);
 	if (primary_if)
 		batadv_hardif_free_ref(primary_if);
-	if (!ret)
-		kfree_skb(skb);
-	/* This packet was for me, so it doesn't need to be re-routed */
-	return true;
 }
 
 bool batadv_send_tt_response(struct batadv_priv *bat_priv,
@@ -1777,7 +1771,9 @@ bool batadv_send_tt_response(struct batadv_priv *bat_priv,
 		if (batadv_bla_is_backbone_gw_orig(bat_priv, tt_request->src))
 			return true;
 
-		return batadv_send_my_tt_response(bat_priv, tt_request);
+		batadv_send_my_tt_response(bat_priv, tt_request);
+		/* This packet was for me, so it never needs to be re-routed */
+		return true;
 	} else {
 		return batadv_send_other_tt_response(bat_priv, tt_request);
 	}
-- 
1.7.9.4

Re: [B.A.T.M.A.N.] [PATCHv3] batman-adv: don't try to re-route tt_request for me

[Marek Lindner]

On Saturday, June 16, 2012 14:27:41 Antonio Quartulli wrote:
> -static bool
> +static void
>  batadv_send_my_tt_response(struct batadv_priv *bat_priv,
>                            struct batadv_tt_query_packet *tt_request)
>  {
> @@ -1656,7 +1656,6 @@ batadv_send_my_tt_response(struct batadv_priv
> *bat_priv, struct batadv_neigh_node *neigh_node = NULL;
>         struct batadv_hard_iface *primary_if = NULL;
>         uint8_t my_ttvn, req_ttvn, ttvn;
> -       int ret = false;
>         unsigned char *tt_buff;
>         bool full_table;
>         uint16_t tt_len, tt_tot;
> @@ -1751,7 +1750,6 @@ batadv_send_my_tt_response(struct batadv_priv
> *bat_priv, batadv_inc_counter(bat_priv, BATADV_CNT_TT_RESPONSE_TX);
>  
>         batadv_send_skb_packet(skb, neigh_node->if_incoming,
> neigh_node->addr); -       ret = true;
>         goto out;
>  
>  unlock:
> @@ -1763,10 +1761,6 @@ out:
>                 batadv_neigh_node_free_ref(neigh_node);
>         if (primary_if)
>                 batadv_hardif_free_ref(primary_if);
> -       if (!ret)
> -               kfree_skb(skb);
> -       /* This packet was for me, so it doesn't need to be re-routed */
> -       return true;
>  }

Unless I am mistake you are adding a beautiful memory leak here.

Regards,
Marek

Re: [B.A.T.M.A.N.] [PATCHv3] batman-adv: don't try to re-route tt_request for me

[Antonio Quartulli]

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

On Sun, Jun 17, 2012 at 03:00:14AM +0800, Marek Lindner wrote:
> On Saturday, June 16, 2012 14:27:41 Antonio Quartulli wrote:
> > -static bool
> > +static void
> >  batadv_send_my_tt_response(struct batadv_priv *bat_priv,
> >                            struct batadv_tt_query_packet *tt_request)
> >  {
> > @@ -1656,7 +1656,6 @@ batadv_send_my_tt_response(struct batadv_priv
> > *bat_priv, struct batadv_neigh_node *neigh_node = NULL;
> >         struct batadv_hard_iface *primary_if = NULL;
> >         uint8_t my_ttvn, req_ttvn, ttvn;
> > -       int ret = false;
> >         unsigned char *tt_buff;
> >         bool full_table;
> >         uint16_t tt_len, tt_tot;
> > @@ -1751,7 +1750,6 @@ batadv_send_my_tt_response(struct batadv_priv
> > *bat_priv, batadv_inc_counter(bat_priv, BATADV_CNT_TT_RESPONSE_TX);
> >  
> >         batadv_send_skb_packet(skb, neigh_node->if_incoming,
> > neigh_node->addr); -       ret = true;
> >         goto out;
> >  
> >  unlock:
> > @@ -1763,10 +1761,6 @@ out:
> >                 batadv_neigh_node_free_ref(neigh_node);
> >         if (primary_if)
> >                 batadv_hardif_free_ref(primary_if);
> > -       if (!ret)
> > -               kfree_skb(skb);
> > -       /* This packet was for me, so it doesn't need to be re-routed */
> > -       return true;
> >  }
> 
> Unless I am mistake you are adding a beautiful memory leak here.

You are right. I misinterpreted this skb.
I'll send v4 soon!

Thank you

> 
> 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.] [PATCHv3] batman-adv: don't try to re-route tt_request for me

[Antonio Quartulli]

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

On Sun, Jun 17, 2012 at 12:11:18AM +0200, Antonio Quartulli wrote:
> On Sun, Jun 17, 2012 at 03:00:14AM +0800, Marek Lindner wrote:
> > On Saturday, June 16, 2012 14:27:41 Antonio Quartulli wrote:
> You are right. I misinterpreted this skb.
> I'll send v4 soon!
> 
> Thank you
> 

After having read this patch more and more I can conclude that we do not really
need it. send_my_tt_response() will always return true and so the problem I
mentioned cannot be triggered at all.
I don't know if we really need a bool function if it does always return true.
But this is a different issue.

Cheers,


-- 
Antonio Quartulli

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

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