Re: [B.A.T.M.A.N.] [PATCH 07/19] batman-adv: Prepare and send own multicast tracker packets

[Simon Wunderlich <simon.wunderlich@s2003.tu-chemnitz.de>]

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

Hello,

found one more thing which will break packet sending, please see inline:

On Sat, Jan 22, 2011 at 02:21:30AM +0100, Linus Lüssing wrote:
> [...]
> +/**
> + * Sends (splitted parts of) a multicast tracker packet on the according
> + * interfaces.
> + *
> + * @tracker_packet:	A compact multicast tracker packet with all groups and
> + *			destinations attached.
> + */
> +void route_mcast_tracker_packet(
> +			struct mcast_tracker_packet *tracker_packet,
> +			int tracker_packet_len, struct bat_priv *bat_priv)
> +{
> +	struct dest_entries_list next_hops, *tmp;
> +	struct mcast_tracker_packet *next_hop_tracker_packets,
> +				    *next_hop_tracker_packet;
> +	struct dest_entries_list *next_hop;
> +	struct sk_buff *skb;
> +	int num_next_hops, i;
> +	int *tracker_packet_lengths;
> +
> +	rcu_read_lock();
> +	num_next_hops = tracker_next_hops(tracker_packet, &next_hops,
> +					  bat_priv);
> +	if (!num_next_hops)
> +		goto out;
> +	next_hop_tracker_packets = kmalloc(tracker_packet_len * num_next_hops,
> +					   GFP_ATOMIC);
> +	if (!next_hop_tracker_packets)
> +		goto free;
> +
> +	tracker_packet_lengths = kmalloc(num_next_hops * sizeof(int),
> +					  GFP_ATOMIC);
> +	if (!tracker_packet_lengths)
> +		goto free2;
> +
> +	i = 0;
> +	list_for_each_entry_safe(next_hop, tmp, &next_hops.list, list) {
> +		next_hop_tracker_packet = (struct mcast_tracker_packet *)
> +					  ((char *)next_hop_tracker_packets +
> +					   i * tracker_packet_len);
> +		memcpy(next_hop_tracker_packet, tracker_packet,
> +		       tracker_packet_len);
> +		zero_tracker_packet(next_hop_tracker_packet, next_hop->dest,
> +				    bat_priv);
> +		tracker_packet_lengths[i] = shrink_tracker_packet(
> +				next_hop_tracker_packet, tracker_packet_len);
> +		i++;
> +	}
> +
> +	i = 0;
> +	/* Add ethernet header, send 'em! */
> +	list_for_each_entry_safe(next_hop, tmp, &next_hops.list, list) {
> +		if (tracker_packet_lengths[i] ==
> +		    sizeof(struct mcast_tracker_packet))
> +			goto skip_send;
> +
> +		skb = build_tracker_packet_skb(&next_hop_tracker_packets[i],

Don't use next_hop_tracker_packets[i]! This will give you a wrong
pointer, because the base is "sizeof(struct mcast_tracker_packet)" 
and not "tracker_packet_len". Instead, please use a 
"next_hop_tracker_packet" construct like in the loop above.

(BTW, i think you could merge the two loops and skip the array 
"tracker_packet_lengths" for keeping the lengths, i think ...)

> +					       tracker_packet_lengths[i],
> +					       next_hop->dest);
> +		if (skb)
> +			send_skb_packet(skb, next_hop->batman_if,
> +					next_hop->dest);
> +skip_send:
> +		list_del(&next_hop->list);
> +		kfree(next_hop);
> +		i++;
> +	}
> +

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