[B.A.T.M.A.N.] [PATCH 0/2] Fixes reported by Al Viro

[B.A.T.M.A.N.] [PATCH 0/2] Fixes reported by Al Viro

[Antonio Quartulli]

The following two patches fix the two issues found by Al Viro and reported in
his email.

Patch 1) simply convert the tt_crc field in the bat_priv structure from atomic_t
to uint16_t. Actually there is no reason to declare it as atomic_t.

Patch 2) fixes a little bug we have when sending a tt_request message: now we
don't convert the tt_crc that we are sending within the request to network
order. OTOH we convert this field from NO to HO on the receiver side.

Therefore, nodes on the path of the request, which have HO different from NO,
will fail to reply to the request and will forward it towards the final
destination.


Cheers,
	Antonio

[B.A.T.M.A.N.] [PATCH 1/2] batman-adv: convert bat_priv->tt_crc from atomic_t to uint16_t

[Antonio Quartulli]

In the code we neever need to atomically check and set the bat_priv->tt_crc
field value. It is simply set and read once in different pieces of the code.
Therefore this field can be safely be converted from atomic_t to uint16_t.

Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 bat_iv_ogm.c |    3 +--
 send.c       |    2 +-
 types.h      |    2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
index 994369d..ed743a4 100644
--- a/bat_iv_ogm.c
+++ b/bat_iv_ogm.c
@@ -576,8 +576,7 @@ static void bat_iv_ogm_schedule(struct hard_iface *hard_iface,
 			htonl((uint32_t)atomic_read(&hard_iface->seqno));
 
 	batman_ogm_packet->ttvn = atomic_read(&bat_priv->ttvn);
-	batman_ogm_packet->tt_crc = htons((uint16_t)
-						atomic_read(&bat_priv->tt_crc));
+	batman_ogm_packet->tt_crc = htons(bat_priv->tt_crc);
 	if (tt_num_changes >= 0)
 		batman_ogm_packet->tt_num_changes = tt_num_changes;
 
diff --git a/send.c b/send.c
index 815cc9c..cebc14a 100644
--- a/send.c
+++ b/send.c
@@ -112,7 +112,7 @@ static int prepare_packet_buffer(struct bat_priv *bat_priv,
 
 	realloc_packet_buffer(hard_iface, new_len);
 
-	atomic_set(&bat_priv->tt_crc, tt_local_crc(bat_priv));
+	bat_priv->tt_crc = tt_local_crc(bat_priv);
 
 	/* reset the sending counter */
 	atomic_set(&bat_priv->tt_ogm_append_cnt, TT_OGM_APPEND_MAX);
diff --git a/types.h b/types.h
index 15f538a..995e910 100644
--- a/types.h
+++ b/types.h
@@ -224,7 +224,7 @@ struct bat_priv {
 	spinlock_t vis_list_lock; /* protects vis_info::recv_list */
 	atomic_t num_local_tt;
 	/* Checksum of the local table, recomputed before sending a new OGM */
-	atomic_t tt_crc;
+	uint16_t tt_crc;
 	unsigned char *tt_buff;
 	int16_t tt_buff_len;
 	spinlock_t tt_buff_lock; /* protects tt_buff */
-- 
1.7.9.4

Re: [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: convert bat_priv->tt_crc from atomic_t to uint16_t

[Marek Lindner]

On Saturday, April 14, 2012 13:15:26 Antonio Quartulli wrote:
> In the code we neever need to atomically check and set the bat_priv->tt_crc
> field value. It is simply set and read once in different pieces of the
> code. Therefore this field can be safely be converted from atomic_t to
> uint16_t.

Applied in revision 7d256a1.

Thanks,
Marek

[B.A.T.M.A.N.] [PATCH 2/2] batman-adv: convert the tt_crc to network order

[Antonio Quartulli]

Before sending out a TT_Request packet we must convert the tt_crc field value
to network order (since it is 16bits long).

Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 translation-table.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/translation-table.c b/translation-table.c
index 72dfbe1..88e4c8e 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -1341,7 +1341,7 @@ static int send_tt_request(struct bat_priv *bat_priv,
 	memcpy(tt_request->dst, dst_orig_node->orig, ETH_ALEN);
 	tt_request->header.ttl = TTL;
 	tt_request->ttvn = ttvn;
-	tt_request->tt_data = tt_crc;
+	tt_request->tt_data = htons(tt_crc);
 	tt_request->flags = TT_REQUEST;
 
 	if (full_table)
-- 
1.7.9.4

Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: convert the tt_crc to network order

[Marek Lindner]

On Saturday, April 14, 2012 13:15:27 Antonio Quartulli wrote:
> Before sending out a TT_Request packet we must convert the tt_crc field
> value to network order (since it is 16bits long).

Applied in revision ffcc504.

Thanks,
Marek

Re: [B.A.T.M.A.N.] [PATCH 0/2] Fixes reported by Al Viro

[Antonio Quartulli]

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

On Sat, Apr 14, 2012 at 01:15:25PM +0200, Antonio Quartulli wrote:
> The following two patches fix the two issues found by Al Viro and reported in
> his email.
> 
> Patch 1) simply convert the tt_crc field in the bat_priv structure from atomic_t
> to uint16_t. Actually there is no reason to declare it as atomic_t.
> 
> Patch 2) fixes a little bug we have when sending a tt_request message: now we
> don't convert the tt_crc that we are sending within the request to network
> order. OTOH we convert this field from NO to HO on the receiver side.
> 
> Therefore, nodes on the path of the request, which have HO different from NO,
> will fail to reply to the request and will forward it towards the final
> destination.

I think David'd want to see this fix in the next pull request or so...should we
commit these patches to next directly?

Cheers,

-- 
Antonio Quartulli

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

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

Re: [B.A.T.M.A.N.] [PATCH 0/2] Fixes reported by Al Viro

[Marek Lindner]

On Saturday, April 14, 2012 13:29:15 Antonio Quartulli wrote:
>   On Sat, Apr 14, 2012 at 01:15:25PM +0200, Antonio Quartulli wrote:
> > The following two patches fix the two issues found by Al Viro and
> > reported in his email.
> >
> > 
> >
> > Patch 1) simply convert the tt_crc field in the bat_priv structure from
> > atomic_t to uint16_t. Actually there is no reason to declare it as
> > atomic_t.
> >
> > 
> >
> > Patch 2) fixes a little bug we have when sending a tt_request message:
> > now we don't convert the tt_crc that we are sending within the request
> > to network order. OTOH we convert this field from NO to HO on the
> > receiver side.
> >
> > 
> >
> > Therefore, nodes on the path of the request, which have HO different from
> > NO, will fail to reply to the request and will forward it towards the
> > final destination.
> 
> I think David'd want to see this fix in the next pull request or
> so...should we commit these patches to next directly?

Probably. The second patch could even go to stable, right ?

Cheers,
Marek

Re: [B.A.T.M.A.N.] [PATCH 0/2] Fixes reported by Al Viro

[Antonio Quartulli]

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

On Sat, Apr 14, 2012 at 01:34:15PM +0200, Marek Lindner wrote:
> On Saturday, April 14, 2012 13:29:15 Antonio Quartulli wrote:
> >   On Sat, Apr 14, 2012 at 01:15:25PM +0200, Antonio Quartulli wrote:
> > > The following two patches fix the two issues found by Al Viro and
> > > reported in his email.
> > >
> > > 
> > >
> > > Patch 1) simply convert the tt_crc field in the bat_priv structure from
> > > atomic_t to uint16_t. Actually there is no reason to declare it as
> > > atomic_t.
> > >
> > > 
> > >
> > > Patch 2) fixes a little bug we have when sending a tt_request message:
> > > now we don't convert the tt_crc that we are sending within the request
> > > to network order. OTOH we convert this field from NO to HO on the
> > > receiver side.
> > >
> > > 
> > >
> > > Therefore, nodes on the path of the request, which have HO different from
> > > NO, will fail to reply to the request and will forward it towards the
> > > final destination.
> > 
> > I think David'd want to see this fix in the next pull request or
> > so...should we commit these patches to next directly?
> 
> Probably. The second patch could even go to stable, right ?

I'd say so


Cheers,

-- 
Antonio Quartulli

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

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