On Mon, Mar 23, 2009 at 9:28 AM, Johannes Berg<johannes@xxxxxxxxxxxxxxxx> wrote:> Fragmentation currently uses an allocated array to store the> fragment skbs, and then keeps track of which have been sent> and which are still pending etc. This is rather complicated;> make it simpler by just chaining the fragments into skb->next> and removing from that list when sent. Also simplifies all> code that needs to touch fragments, since it now only needs> to walk the skb->next list.>> This is a prerequisite for fixing the stored packet code,> which I need to do for proper aggregation packet storing.>> Signed-off-by: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
<-- snip -->
> @@ -1099,45 +1088,36 @@ static int ieee80211_tx_prepare(struct i> return 0;> }>> -static int __ieee80211_tx(struct ieee80211_local *local, struct sk_buff *skb,> +static int __ieee80211_tx(struct ieee80211_local *local,> struct ieee80211_tx_data *tx)> {> + struct sk_buff *skb = tx->skb, *next;> struct ieee80211_tx_info *info;> - int ret, i;> + int ret;> + bool fragm = false;>> - if (skb) {> + local->mdev->trans_start = jiffies;> +> + while (skb) {> if (ieee80211_queue_stopped(&local->hw,> skb_get_queue_mapping(skb)))> return IEEE80211_TX_PENDING;>> - ret = local->ops->tx(local_to_hw(local), skb);> - if (ret)> - return IEEE80211_TX_AGAIN;> - local->mdev->trans_start = jiffies;> - ieee80211_led_tx(local, 1);> - }> - if (tx->extra_frag) {> - for (i = 0; i < tx->num_extra_frag; i++) {> - if (!tx->extra_frag[i])> - continue;> - info = IEEE80211_SKB_CB(tx->extra_frag[i]);> + if (fragm) {> + info = IEEE80211_SKB_CB(skb);> info->flags &= ~(IEEE80211_TX_CTL_CLEAR_PS_FILT |> IEEE80211_TX_CTL_FIRST_FRAGMENT);> - if (ieee80211_queue_stopped(&local->hw,> - skb_get_queue_mapping(tx->extra_frag[i])))> - return IEEE80211_TX_FRAG_AGAIN;> -> - ret = local->ops->tx(local_to_hw(local),> - tx->extra_frag[i]);> - if (ret)> - return IEEE80211_TX_FRAG_AGAIN;> - local->mdev->trans_start = jiffies;> - ieee80211_led_tx(local, 1);> - tx->extra_frag[i] = NULL;> }> - kfree(tx->extra_frag);> - tx->extra_frag = NULL;> +> + next = skb->next;> + ret = local->ops->tx(local_to_hw(local), skb);> + if (ret != NETDEV_TX_OK)> + return IEEE80211_TX_AGAIN;> + tx->skb = skb = next;> + ieee80211_led_tx(local, 1);> + fragm = true;> }> +> return IEEE80211_TX_OK;> }>
I see only one possible regression with this patch and that is that itseems you forgot to update the mdev->trans_start in the new loop oneach fragment.
As far as I can tell this would only introduce a regression with ifsomeone is using a netdev watchdog on the mdev, if we don't care aboutthat and I'm not missing any other case where this could be affectedwe might as well remove that first update as at least for mac80211 itscompletely untouched.
Other than that:
Reviewed-by: Luis R. Rodriguez <lrodriguez@xxxxxxxxxxx>
Luis��.n��������+%������w��{.n�����{���zW����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
