Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Limit queue lengths for batman and broadcast packets

[Sven Eckelmann <sven.eckelmann@gmx.de>]

[-- Attachment #1: Type: Text/Plain, Size: 2161 bytes --]

On Monday 05 April 2010 04:46:03 Simon Wunderlich wrote:
> Index: a/batman-adv-kernelland/send.c
> ===================================================================
> --- a/batman-adv-kernelland/send.c	(revision 1616)
> +++ a/batman-adv-kernelland/send.c	(working copy)
> @@ -382,12 +382,21 @@
>  {
>  	struct forw_packet *forw_packet;
> 
> +	if (atomic_dec_and_test(&bcast_queue_left)) {
> +		bat_dbg(DBG_BATMAN, "bcast packet queue full\n");
> +		atomic_inc(&bcast_queue_left);
> +		return;
> +	}
> +

[....]

> --- a/batman-adv-kernelland/aggregation.c	(revision 1616)
> +++ a/batman-adv-kernelland/aggregation.c	(working copy)
> @@ -106,13 +106,27 @@
>  	struct forw_packet *forw_packet_aggr;
>  	unsigned long flags;
> 
> +	/* own packet should always be scheduled */
> +	if (!own_packet) {
> +		if (atomic_dec_and_test(&batman_queue_left)) {
> +			bat_dbg(DBG_BATMAN, "batman packet queue full\n");
> +			atomic_inc(&batman_queue_left);
> +			return;
> +		}
> +	}

It should be possible to have multiple events accessing these functions at the 
same time, or am I wrong?

Just say we have following situation (queue is full; full == 1):

 * bcast comes in and we start add_bcast_packet_to_list  and
   dec_test(left) == 1 -> damn, no room left for us
-> for easier understanding: someone steals our cpu or the processing is
    otherwise interleaved with following
 * another bcast comes in and wants attention (left is now 0):
   dec_test(left) == 0 (because left is now -1... or 0xfff....fff). Lets enqueue
   it and do the rest
 * first bcast continues and and does atomic_inc(left) -> now it is 0
 * now a storm of bcasts comes in and all do atomic_dec_and_test... each one
    will be accepted because left is already zero and needs a looooong time to
    be 0 again (or enough bcast packets were processed from the queue to get
   positive again)

I am not 100% sure if this really can happen, but I thought that it was a hard 
requirement for TCP port passed processor selection for parallel processing of 
incoming packets on multicore/multiprocessor architectures. 

Best regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]