Re: [B.A.T.M.A.N.] [PATCH 5/6] batman-adv: Save overheard and tx packets for decoding.

[Antonio Quartulli <ordex@autistici.org>]

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

Hi Martin,

On Fri, Nov 30, 2012 at 04:08:34PM +0100, Martin Hundebøll wrote:
> @@ -47,8 +48,9 @@ static void batadv_nc_start_timer(struct batadv_priv *bat_priv)
>  int batadv_nc_init(struct batadv_priv *bat_priv)
>  {
>  	bat_priv->nc.timestamp_fwd_flush = jiffies;
> +	bat_priv->nc.timestamp_sniffed_purge = jiffies;
>  
> -	if (bat_priv->nc.coding_hash)
> +	if (bat_priv->nc.coding_hash || bat_priv->nc.decoding_hash)
>  		return 0;
>  
>  	bat_priv->nc.coding_hash = batadv_hash_new(128);
> @@ -58,6 +60,13 @@ int batadv_nc_init(struct batadv_priv *bat_priv)
>  	batadv_hash_set_lock_class(bat_priv->nc.coding_hash,
>  				   &batadv_nc_coding_hash_lock_class_key);
>  
> +	bat_priv->nc.decoding_hash = batadv_hash_new(128);
> +	if (!bat_priv->nc.decoding_hash)
> +		goto err;

the label err brings the flow to a lonely "return -ENOMEM". However if you it
the case above it means that you already allocated "bat_priv->nc.coding_hash"
successfully. May you need another label so that you can free "coding_hash"
and avoid memory leak?

[...]

>  
>  /**
> + * batadv_nc_sniffed_purge - Checks whether the given sniffed (overheard) nc
> + *  packet has hit its buffering timeout. If so, the packet is no longer kept
> + *  and the entry deleted from the queue. Has to be called with the appropriate
> + *  locks.

Same as the previous patches..Move the core of the description below please.

> + * @bat_priv: the bat priv with all the soft interface information
> + * @nc_path: the nc path the packet belongs to
> + * @nc_packet: the nc packet to be checked
> + *
> + * Returns false as soon as the entry in the fifo queue has not been timed out
> + * yet and true otherwise.
> + */
> +static bool batadv_nc_sniffed_purge(struct batadv_priv *bat_priv,
> +				    struct batadv_nc_path *nc_path,
> +				    struct batadv_nc_packet *nc_packet)
> +{
> +	unsigned long timeout = bat_priv->nc.max_buffer_time;
> +	bool res = false;
> +
> +	/* Packets are added to tail, so the remaining packets did not time
> +	 * out and we can stop processing the current queue
> +	 */

This is very nice :)

> +	if (atomic_read(&bat_priv->mesh_state) == BATADV_MESH_ACTIVE &&
> +	    !batadv_has_timed_out(nc_packet->timestamp, timeout))
> +		goto out;
> +
> +	/* purge nc packet */
> +	list_del(&nc_packet->list);
> +	batadv_nc_packet_free(nc_packet);
> +
> +	res = true;
> +
> +out:
> +	return res;
> +}
> +

[...]

> diff --git a/routing.c b/routing.c
> index d7bd4d1..59e1894 100644
> --- a/routing.c
> +++ b/routing.c
> @@ -1053,8 +1053,14 @@ int batadv_recv_unicast_packet(struct sk_buff *skb,
>  	if (is4addr)
>  		hdr_size = sizeof(*unicast_4addr_packet);
>  
> -	if (batadv_check_unicast_packet(skb, hdr_size) < 0)
> +	if (batadv_check_unicast_packet(skb, hdr_size) < 0) {
> +		/* Even though the packet is to be dropped, we might save
> +		 * it to use for decoding a later received coded packet
> +		 */

Even if the packet is not well shaped? In the end this packet should not be
forwarded by anybody so maybe we should prevent this kind of packet to be
enqueued for coding on the sender side?


> +		batadv_nc_skb_store_sniffed_unicast(bat_priv, skb);
> +
>  		return NET_RX_DROP;
> +	}
>  


Cheers,

-- 
Antonio Quartulli

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

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