Re: [PATCH] Assign next hop address to pending mesh frames once the path is resolved.

[Johannes Berg <johannes@sipsolutions.net>]

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

On Tue, 2009-07-07 at 22:53 -0700, Javier Cardona wrote:
> Regression.  Frames transmitted when a mesh path was wating to be resolved were
> being transmitted with an invalid Receiver Address.

> -	rcu_assign_pointer(mpath->next_hop, sta);
> +	struct sk_buff *skb, *skb_first = NULL;
> +	struct ieee80211_hdr *hdr;
> +
> +	rcu_read_lock();
> +	mpath->next_hop = sta;
> +
> +	while ((skb = skb_dequeue(&mpath->frame_queue)) != skb_first) {
> +		if (!skb_first)
> +			skb_first = skb;
> +		hdr = (struct ieee80211_hdr *) skb->data;
> +		memcpy(hdr->addr1, sta->sta.addr, ETH_ALEN);
> +		skb_queue_tail(&mpath->frame_queue, skb);
> +	}
> +	if (skb_first)
> +		skb_queue_tail(&mpath->frame_queue, skb_first);
> +
> +	rcu_read_unlock();

Since skb queues have a locks, why use rcu too?

Also I think you should probably use a different pattern -- this looks
prone to breakage, maybe something like

	sk_buff_head tmpq;
	unsigned long flags;

	__skb_queue_head_init(&tmpq);

	spin_lock_irqsave(&frame_queue->lock);

	while (skb = __skb_dequeue(&frame_queue)) {
		hdr = (struct ieee80211_hdr *) skb->data;
		memcpy(hdr->addr1, sta->sta.addr, ETH_ALEN);
		__skb_queue_tail(&tmpq, skb);
	}

	skb_queue_splice(&tmpq, frame_queue);
	spin_unlock_irqrestore(&frame_queue->lock);

johannes

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