* [B.A.T.M.A.N.] [PATCH] batman-adv: properly handle Network Ordered variables
@ 2013-04-27 15:59 Antonio Quartulli
2013-04-27 16:46 ` Marek Lindner
0 siblings, 1 reply; 3+ messages in thread
From: Antonio Quartulli @ 2013-04-27 15:59 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Antonio Quartulli
32bit long variables stored in structures sent over the wire
must report the big endian notation and must be correctly
handled during read and write operations by means of htonl
and ntohl.
Introduced by: 0853ec7fafe0a195754454832993c6b35e22b842
("batman-adv: tvlv - gateway download/upload bandwidth container")
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
gateway_client.c | 24 +++++++++++++-----------
gateway_common.c | 4 ++--
packet.h | 4 ++--
3 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/gateway_client.c b/gateway_client.c
index f00db73..973f02d 100644
--- a/gateway_client.c
+++ b/gateway_client.c
@@ -344,8 +344,10 @@ static void batadv_gw_node_add(struct batadv_priv *bat_priv,
batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
"Found new gateway %pM -> gw bandwidth: %u.%u/%u.%u MBit\n",
orig_node->orig,
- gateway->bandwidth_down / 10, gateway->bandwidth_down % 10,
- gateway->bandwidth_up / 10, gateway->bandwidth_up % 10);
+ ntohl(gateway->bandwidth_down) / 10,
+ ntohl(gateway->bandwidth_down) % 10,
+ ntohl(gateway->bandwidth_up) / 10,
+ ntohl(gateway->bandwidth_up) % 10);
}
/**
@@ -399,8 +401,8 @@ void batadv_gw_node_update(struct batadv_priv *bat_priv,
goto out;
}
- if ((gw_node->bandwidth_down == gateway->bandwidth_down) &&
- (gw_node->bandwidth_up == gateway->bandwidth_up))
+ if ((gw_node->bandwidth_down == ntohl(gateway->bandwidth_down)) &&
+ (gw_node->bandwidth_up == ntohl(gateway->bandwidth_up)))
goto out;
batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
@@ -410,16 +412,16 @@ void batadv_gw_node_update(struct batadv_priv *bat_priv,
gw_node->bandwidth_down % 10,
gw_node->bandwidth_up / 10,
gw_node->bandwidth_up % 10,
- gateway->bandwidth_down / 10,
- gateway->bandwidth_down % 10,
- gateway->bandwidth_up / 10,
- gateway->bandwidth_up % 10);
+ ntohl(gateway->bandwidth_down) / 10,
+ ntohl(gateway->bandwidth_down) % 10,
+ ntohl(gateway->bandwidth_up) / 10,
+ ntohl(gateway->bandwidth_up) % 10);
- gw_node->bandwidth_down = gateway->bandwidth_down;
- gw_node->bandwidth_up = gateway->bandwidth_up;
+ gw_node->bandwidth_down = ntohl(gateway->bandwidth_down);
+ gw_node->bandwidth_up = ntohl(gateway->bandwidth_up);
gw_node->deleted = 0;
- if (gateway->bandwidth_down == 0) {
+ if (ntohl(gateway->bandwidth_down) == 0) {
gw_node->deleted = jiffies;
batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
"Gateway %pM removed from gateway list\n",
diff --git a/gateway_common.c b/gateway_common.c
index 9904710..07fd877 100644
--- a/gateway_common.c
+++ b/gateway_common.c
@@ -202,8 +202,8 @@ static void batadv_gw_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
gateway.bandwidth_up = 0;
} else {
gateway_ptr = tvlv_value;
- gateway.bandwidth_down = ntohl(gateway_ptr->bandwidth_down);
- gateway.bandwidth_up = ntohl(gateway_ptr->bandwidth_up);
+ gateway.bandwidth_down = gateway_ptr->bandwidth_down;
+ gateway.bandwidth_up = gateway_ptr->bandwidth_up;
if ((gateway.bandwidth_down == 0) ||
(gateway.bandwidth_up == 0)) {
gateway.bandwidth_down = 0;
diff --git a/packet.h b/packet.h
index 58fe4eb..559c738 100644
--- a/packet.h
+++ b/packet.h
@@ -372,8 +372,8 @@ struct batadv_tvlv_long {
* @bandwidth_up: advertised uplink upload bandwidth
*/
struct batadv_tvlv_gateway_data {
- uint32_t bandwidth_down;
- uint32_t bandwidth_up;
+ __be32 bandwidth_down;
+ __be32 bandwidth_up;
};
/**
--
1.8.1.5
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: properly handle Network Ordered variables
2013-04-27 15:59 [B.A.T.M.A.N.] [PATCH] batman-adv: properly handle Network Ordered variables Antonio Quartulli
@ 2013-04-27 16:46 ` Marek Lindner
2013-04-27 16:59 ` Antonio Quartulli
0 siblings, 1 reply; 3+ messages in thread
From: Marek Lindner @ 2013-04-27 16:46 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
On Saturday, April 27, 2013 23:59:53 Antonio Quartulli wrote:
> --- a/gateway_common.c
> +++ b/gateway_common.c
> @@ -202,8 +202,8 @@ static void batadv_gw_tvlv_ogm_handler_v1(struct
> batadv_priv *bat_priv, gateway.bandwidth_up = 0;
> } else {
> gateway_ptr = tvlv_value;
> - gateway.bandwidth_down =
> ntohl(gateway_ptr->bandwidth_down); - gateway.bandwidth_up =
> ntohl(gateway_ptr->bandwidth_up); + gateway.bandwidth_down =
> gateway_ptr->bandwidth_down; + gateway.bandwidth_up =
> gateway_ptr->bandwidth_up;
> if ((gateway.bandwidth_down == 0) ||
> (gateway.bandwidth_up == 0)) {
> gateway.bandwidth_down = 0;
There is no bug. The original code converted the incoming big endian value to
host endian (whatever that is) and works with that value internally. Now that
you reversed the logic you have to adjust *all* occurences. Especially, when
the tvlv bandwidth value is copied into a gw_node struct. You can use sparse
to help you find the sections in the code by changing gw_node to be32 as well.
However, I am not convinced that converting the values every single time we
access the variable is the optimal approach.
Cheers,
Marek
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: properly handle Network Ordered variables
2013-04-27 16:46 ` Marek Lindner
@ 2013-04-27 16:59 ` Antonio Quartulli
0 siblings, 0 replies; 3+ messages in thread
From: Antonio Quartulli @ 2013-04-27 16:59 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
[-- Attachment #1: Type: text/plain, Size: 2186 bytes --]
On Sun, Apr 28, 2013 at 12:46:11AM +0800, Marek Lindner wrote:
> On Saturday, April 27, 2013 23:59:53 Antonio Quartulli wrote:
> > --- a/gateway_common.c
> > +++ b/gateway_common.c
> > @@ -202,8 +202,8 @@ static void batadv_gw_tvlv_ogm_handler_v1(struct
> > batadv_priv *bat_priv, gateway.bandwidth_up = 0;
> > } else {
> > gateway_ptr = tvlv_value;
> > - gateway.bandwidth_down =
> > ntohl(gateway_ptr->bandwidth_down); - gateway.bandwidth_up =
> > ntohl(gateway_ptr->bandwidth_up); + gateway.bandwidth_down =
> > gateway_ptr->bandwidth_down; + gateway.bandwidth_up =
> > gateway_ptr->bandwidth_up;
> > if ((gateway.bandwidth_down == 0) ||
> > (gateway.bandwidth_up == 0)) {
> > gateway.bandwidth_down = 0;
>
> There is no bug.
I did not intend to fix a bug, I just wanted to use the __be32 notation for
stuff sent over the wire.
Well, I have to admit I thought you forgot some htonl/ntohl (this is why the
notation is useful, to spot missing conversions..) but this was not the case :)
> The original code converted the incoming big endian value to
> host endian (whatever that is) and works with that value internally. Now that
> you reversed the logic you have to adjust *all* occurences. Especially, when
> the tvlv bandwidth value is copied into a gw_node struct. You can use sparse
> to help you find the sections in the code by changing gw_node to be32 as well.
>
So far everything seems to be converted the right way: sparse does not complain,
and here is the code where the GW component does the assignment yu talked about:
420 gw_node->bandwidth_down = ntohl(gateway->bandwidth_down);
421 gw_node->bandwidth_up = ntohl(gateway->bandwidth_up);
> However, I am not convinced that converting the values every single time we
> access the variable is the optimal approach.
>
True, not really beautiful but this will avoid future bugs thanks to the __be
notation.
Cheers,
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-04-27 16:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-27 15:59 [B.A.T.M.A.N.] [PATCH] batman-adv: properly handle Network Ordered variables Antonio Quartulli
2013-04-27 16:46 ` Marek Lindner
2013-04-27 16:59 ` Antonio Quartulli
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.