Re: [B.A.T.M.A.N.] [PATCH v8 2/2] batman-adv: throughput meter implementation

[Antonio Quartulli <a@unstable.cc>]

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

On Wed, May 11, 2016 at 11:35:30AM +0200, Sven Eckelmann wrote:

[...]

> +/**
> + * enum batadv_tp_meter_reason - reason of a a tp meter test run stop

typ0: double "a"

> + * @BATADV_TP_COMPLETE: sender finished tp run
> + * @BATADV_TP_SIGINT: sender was stopped during run

how about calling this TP_CANCEL ? batman-adv does not know much about the
SIGINT because that is only an interface between the user and batctl.

Another thing: how about adding the prefix "REASON" to all these attributes to
avoid confusion with the others ? What do you think ?

i.e. @BATADV_TP_CANCEL --> BATADV_TP_REASON_CANCEL

> + * @BATADV_TP_DST_UNREACHABLE: receiver could not be reached or didn't answer
> + * @BATADV_TP_RESEND_LIMIT: (unused) sender retry reached limit
> + * @BATADV_TP_ALREADY_ONGOING: test to or from the same node already ongoing
> + * @BATADV_TP_MEMORY_ERROR: test was stopped due to low memory
> + * @BATADV_TP_CANT_SEND: failed to send via outgoing interface
> + * @BATADV_TP_TOO_MANY: too many ongoing sessions
> + */
> +enum batadv_tp_meter_reason {
> +	BATADV_TP_COMPLETE		= 3,
> +	BATADV_TP_SIGINT		= 4,
> +	/* error status >= 128 */
> +	BATADV_TP_DST_UNREACHABLE	= 128,
> +	BATADV_TP_RESEND_LIMIT		= 129,
> +	BATADV_TP_ALREADY_ONGOING	= 130,
> +	BATADV_TP_MEMORY_ERROR		= 131,
> +	BATADV_TP_CANT_SEND		= 132,
> +	BATADV_TP_TOO_MANY		= 133,
> +};
> +

[...]

> --- a/net/batman-adv/bat_iv_ogm.c
> +++ b/net/batman-adv/bat_iv_ogm.c
> @@ -160,8 +160,10 @@ static int batadv_iv_ogm_orig_add_if(struct batadv_orig_node *orig_node,
>  	orig_node->bat_iv.bcast_own = data_ptr;
>  
>  	data_ptr = kmalloc_array(max_if_num, sizeof(u8), GFP_ATOMIC);
> -	if (!data_ptr)
> +	if (!data_ptr) {
> +		kfree(orig_node->bat_iv.bcast_own);
>  		goto unlock;
> +	}
>  

is this chunk coming from another patch ?


[...]

> --- a/net/batman-adv/routing.c
> +++ b/net/batman-adv/routing.c
> @@ -45,6 +45,7 @@
>  #include "packet.h"
>  #include "send.h"
>  #include "soft-interface.h"
> +#include "tp_meter.h"
>  #include "translation-table.h"
>  
>  static int batadv_route_unicast_packet(struct sk_buff *skb,
> @@ -242,7 +243,6 @@ static int batadv_recv_my_icmp_packet(struct batadv_priv *bat_priv,
>  		/* receive the packet */
>  		if (skb_linearize(skb) < 0)
>  			break;
> -

why removing this line ? is it a style adjustment ?

[...]

> +
> +/**
> + * BATADV_TP_PLEN - length of the payload (data after the batadv_unicast header)
> + *  to simulate
> + */
> +#define BATADV_TP_PLEN 1450

shouldn't this depend on BATADV_TP_PACKET_LEN ?


Cheers,

-- 
Antonio Quartulli

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