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
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 */
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
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)
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
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,
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
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,
b.a.t.m.a.n@lists.open-mesh.org