Re: [PATCH v2] mac80211: Jitter HWMP MPATH reply frames to reduce collision on dense networks.

Hi Johannes,

Thank you for the comments, I'll see if I can apply all of your
suggestions and resubmit.

Alexis

On Thu, Mar 16, 2017 at 3:18 AM, Johannes Berg
[off-list ref] wrote:

Sorry - this is the other half of my reply that I accidentally deleted
before sending...

quoted

+static void flush_tx_skbs(struct ieee80211_sub_if_data *sdata)
+{
+     struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
+     struct mesh_tx_queue *tx_node;
+
+     spin_lock_bh(&ifmsh->mesh_tx_queue_lock);
+
+     /* Note that this check is important because of the two-
stage
+      * way that ieee80211_if_mesh is initialized.
+      */

I think you should fix that rather than work around it. If this is
called with iftype != mesh then this is super problematic anyway, since
ifmsh->tx_queue_len would alias some other variable (there's a union).

quoted

+     if (ifmsh->tx_queue_len > 0) {
+             mhwmp_dbg(sdata, "flushing %d skbs", ifmsh-

quoted

tx_queue_len);

+
+             while (!list_empty(&ifmsh->tx_queue.list)) {
+                     tx_node = list_last_entry(&ifmsh-

quoted

tx_queue.list,

+                                               struct
mesh_tx_queue, list);
+                     kfree_skb(tx_node->skb);
+                     list_del(&tx_node->list);
+                     kfree(tx_node);
+             }
+             ifmsh->tx_queue_len = 0;
+     }
+
+     spin_unlock_bh(&ifmsh->mesh_tx_queue_lock);
+}

All of this also gets *vastly* simpler if it's just skb_queue_purge()
:)

quoted

+     spin_lock_bh(&ifmsh->mesh_tx_queue_lock);
+
+     list_for_each_entry(tx_node, &ifmsh->tx_queue.list, list) {
+             ieee80211_tx_skb(sdata, tx_node->skb);
+     }

I don't think you should hold the lock across _tx_skb(), ISTR problems
with that - particularly with the STA lock, so this might be OK, but it
might also cause lock ordering issues. It's easy to avoid anyway, so
better not to do it.

johannes