pull request: batman-adv 2011-11-26

  Hi David,

the following 10 patches constitute the first batch I'd like to get the pulled
into net-next-2.6/3.3. They're mostly uncritical fixes around the recently
introduced tt code, some code refactoring, the kstrto update and the range
check fix reported by Thomas Jarosch. 

Thanks,
Marek

The following changes since commit 1ea6b8f48918282bdca0b32a34095504ee65bab5:

  Linux 3.2-rc1 (2011-11-07 16:16:02 -0800)

are available in the git repository at:
  git://git.open-mesh.org/linux-merge.git for_david

Antonio Quartulli (5):
      batman-adv: tt_global_del_orig() has to print the correct message
      batman-adv: use orig_hash_find() instead of get_orig_node() in TT code
      batman-adv: fixed hash functions type to uint32_t instead of int
      batman-adv: linearise the tt_response skb only if needed
      batman-adv: check for tt_reponse packet real length

Marek Lindner (1):
      batman-adv: refactoring gateway handling code

Simon Wunderlich (2):
      batman-adv: directly write tt entries without buffering
      batman-adv: Fix range check for expected packets

Sven Eckelmann (2):
      batman-adv: update internal version number
      batman-adv: Replace obsolete strict_strto<foo> with kstrto<foo>

 net/batman-adv/bat_sysfs.c         |    4 +-
 net/batman-adv/bitarray.c          |    2 +-
 net/batman-adv/gateway_client.c    |  153 ++++++++++++++++++++++--------------
 net/batman-adv/gateway_client.h    |    5 +-
 net/batman-adv/gateway_common.c    |    4 +-
 net/batman-adv/hash.c              |    4 +-
 net/batman-adv/hash.h              |   13 ++--
 net/batman-adv/main.h              |    2 +-
 net/batman-adv/originator.c        |   13 ++-
 net/batman-adv/originator.h        |    2 +-
 net/batman-adv/routing.c           |   22 ++++--
 net/batman-adv/soft-interface.c    |   43 +++++++---
 net/batman-adv/translation-table.c |  100 ++++++-----------------
 net/batman-adv/vis.c               |   17 +++--
 14 files changed, 202 insertions(+), 182 deletions(-)
  

From: Marek Lindner <lindner_marek@yahoo.de>
Date: Sat, 26 Nov 2011 22:26:42 +0800

> the following 10 patches constitute the first batch I'd like to get the pulled
> into net-next-2.6/3.3. They're mostly uncritical fixes around the recently
> introduced tt code, some code refactoring, the kstrto update and the range
> check fix reported by Thomas Jarosch. 

Pulled, thanks.

Some things to look into:

+			if (unlikely(skb_headlen(skb) <
+					sizeof(struct tt_query_packet) +
+					tt_len))

This isn't formatted correctly, all the leading edges should line
up to the openning parenthesis of the unlikely:

+			if (unlikely(skb_headlen(skb) <
+				     sizeof(struct tt_query_packet) +
+				     tt_len))

Next, there is a lot of linearization done by the stack, but really the
thing to do is to make sure that the part you want to look at is
linear.

You do this using pskb_may_pull() right before you want to look at some
headers. It makes sure that, for the length given, that many bytes are
linear at the head of the skb.

Thanks.
  

Hello David,

On Sat, Nov 26, 2011 at 02:41:22 -0500, David Miller wrote:
[CUT]
> Some things to look into:
> 
> +			if (unlikely(skb_headlen(skb) <
> +					sizeof(struct tt_query_packet) +
> +					tt_len))
> 
> This isn't formatted correctly, all the leading edges should line
> up to the openning parenthesis of the unlikely:
> 
> +			if (unlikely(skb_headlen(skb) <
> +				     sizeof(struct tt_query_packet) +
> +				     tt_len))
> 

Thank you for reporting this issue. We have already prepared a patch which is
going to be sent within the next batch.

> Next, there is a lot of linearization done by the stack, but really the
> thing to do is to make sure that the part you want to look at is
> linear.
> 
> You do this using pskb_may_pull() right before you want to look at some
> headers. It makes sure that, for the length given, that many bytes are
> linear at the head of the skb.
> 

For this issue, we had some problem to understand it.

First of all I think you are referring to patch 08/10, in which I moved a
skb_linearise() operation.

To be sure it is really needed, I backtracked the code flow and noted down any
eventual psbk_may_pull() (or any other linearisation operation). The result is:

=> in batman_skb_recv()
	- pskb_may_pull(2)
  => in recv_tt_query()
  	  - pskb_may_pull(sizeof(header))
	  - skb_linearise()

Actually it seems we haven't any useless linearisation.
Would you mind explain us where you actually found the problem, please?

It might also be that I misunderstood your advice.

Thank you.


Best Regards,
  

From: Antonio Quartulli <ordex@autistici.org>
Date: Fri, 2 Dec 2011 18:12:16 +0100

> First of all I think you are referring to patch 08/10, in which I moved a
> skb_linearise() operation.
> 
> To be sure it is really needed, I backtracked the code flow and noted down any
> eventual psbk_may_pull() (or any other linearisation operation). The result is:
> 
> => in batman_skb_recv()
> 	- pskb_may_pull(2)
>   => in recv_tt_query()
>   	  - pskb_may_pull(sizeof(header))
> 	  - skb_linearise()
> 
> Actually it seems we haven't any useless linearisation.
> Would you mind explain us where you actually found the problem, please?
> 
> It might also be that I misunderstood your advice.

You only need to call pskb_may_pull() on the parts of the packet you want to
access directly to parse headers etc.

If you use that interface properly, you never need to linearize, ever.
  

On Fri, Dec 02, 2011 at 12:57:25 -0500, David Miller wrote:
> From: Antonio Quartulli <ordex@autistici.org>
> Date: Fri, 2 Dec 2011 18:12:16 +0100
> 
> > First of all I think you are referring to patch 08/10, in which I moved a
> > skb_linearise() operation.
> > 
> > To be sure it is really needed, I backtracked the code flow and noted down any
> > eventual psbk_may_pull() (or any other linearisation operation). The result is:
> > 
> > => in batman_skb_recv()
> > 	- pskb_may_pull(2)
> >   => in recv_tt_query()
> >   	  - pskb_may_pull(sizeof(header))
> > 	  - skb_linearise()
> > 
> > Actually it seems we haven't any useless linearisation.
> > Would you mind explain us where you actually found the problem, please?
> > 
> > It might also be that I misunderstood your advice.
> 
> You only need to call pskb_may_pull() on the parts of the packet you want to
> access directly to parse headers etc.
> 
> If you use that interface properly, you never need to linearize, ever.

Sorry for being too generic: we actually invoke skb_linearise() because
we want to access the whole skb.

We first call pskb_may_pull() to pull the header only and then, under certain
conditions, we linearise the whole skb to access it all. Should I use
pskb_may_pull() even in this case?

Sorry for stealing you so much time on this issue, but I would like to fully
understand it in order to avoid any further mistake.


Thank you again.

Best Regards,
  

From: Antonio Quartulli <ordex@autistici.org>
Date: Sat, 3 Dec 2011 02:55:29 +0100

> We first call pskb_may_pull() to pull the header only and then, under certain
> conditions, we linearise the whole skb to access it all. Should I use
> pskb_may_pull() even in this case?

Why would you need to access the whole thing?

There are only two types of possible accesses:

1) Header parsing --> use pskb_may_pull() as needed.

2) Copying the data to some other location, such as a user
   buffer.  Use skb_copy_datagram_iovec or similar which handle
   fragmented SKBs just fine.

You should handle fragmented SKBs as much as possible, because
linearization is expensive and often amounts to a memory allocation
plus a copy if you linearize the whole thing.
  

On Fri, Dec 02, 2011 at 08:59:55PM -0500, David Miller wrote:
> From: Antonio Quartulli <ordex@autistici.org>
> Date: Sat, 3 Dec 2011 02:55:29 +0100
> 
> > We first call pskb_may_pull() to pull the header only and then, under certain
> > conditions, we linearise the whole skb to access it all. Should I use
> > pskb_may_pull() even in this case?
> 
> Why would you need to access the whole thing?

It contains only information for the routing/pathfinding to client nodes (nodes
which do not directly participate in this mesh).

> There are only two types of possible accesses:
> 
> 1) Header parsing --> use pskb_may_pull() as needed.
> 
> 2) Copying the data to some other location, such as a user
>    buffer.  Use skb_copy_datagram_iovec or similar which handle
>    fragmented SKBs just fine.

I think a "copy" using skb_header_pointer/skb_copy_bits would be the right
thing to do. batman-adv needs the data for kernel data structures and don't
send it to a userspace program (at least not as primary functionality). I don't
have benchmarks which compare both solutions (with a heavily fragmented skb)
when accessing small, packed structures in a skb.

Thanks,
	Sven