Search Linux Wireless

RE: [PATCH v7] Add new mac80211 driver mwlwifi.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> Emmanuel Grumbach [mailto:egrumbach@xxxxxxxxx] wrote:
> 
> Hi,
> 
> I was interested in the checking the A-MSDU implementation only, so I
> reviewed only that.
> 
> I am not sure I really followed your implementation though.
> In iwlwifi I chose the LSO mechanism to implement A-MSDU. I understand that
> for an AP it makes less sense since you don't really have locally generated
> traffic and LSO won't work well with packets forwarded by the IP layer.
> So you seem to have added a timeout mechanism inside the driver that
> accumulates the packet for a certain period of time and send them all when
> the max number of packets has been reached?
> But I didn't see any timer there, so that if you start an A-MSDU and then
> suddenly stop receiving packets for transmission, the packet already in the
> buffer will stay stale?
>

The flush mechanism is hooked on queue empty interrupt.

> A few more comments below.
> 
> On Thu, Nov 12, 2015 at 5:51 AM, David Lin <dlin@xxxxxxxxxxx> wrote:
> > This patch provides the mwlwifi driver for Marvell 8863, 8864 and 8897
> > chipsets.
> > This driver was developed as part of the openwrt.org project to
> > support Linksys WRT1900AC and is maintained on
> https://github.com/kaloz/mwlwifi.
> >
> > The mwlwifi driver differs from existing mwifiex driver:
> > o mwlwifi is a "softmac driver" using the kernel? mac802.11 subsystem
> > to provide full AP/Wireless Bridge functionality (routers).
> > o mwifiex is a "fullmac driver" which provides a comprehensive set of
> > client functions (laptops/embedded devices) o only mwlwifi supports
> > Marvell AP chip 886X series
> >
> 
> [snip]
> 
> > +
> > +static int mwl_mac80211_ampdu_action(struct ieee80211_hw *hw,
> > +                                    struct ieee80211_vif *vif,
> > +                                    enum
> ieee80211_ampdu_mlme_action action,
> > +                                    struct ieee80211_sta *sta,
> > +                                    u16 tid, u16 *ssn, u8 buf_size,
> > +bool amsdu) {
> > +       int rc = 0;
> > +       struct mwl_priv *priv = hw->priv;
> > +       struct mwl_ampdu_stream *stream;
> > +       u8 *addr = sta->addr, idx;
> > +       struct mwl_sta *sta_info;
> > +
> > +       sta_info = mwl_dev_get_sta(sta);
> > +
> > +       spin_lock_bh(&priv->stream_lock);
> > +
> > +       stream = mwl_fwcmd_lookup_stream(hw, addr, tid);
> > +
> > +       switch (action) {
> > +       case IEEE80211_AMPDU_RX_START:
> > +       case IEEE80211_AMPDU_RX_STOP:
> > +               break;
> > +       case IEEE80211_AMPDU_TX_START:
> > +               if (!sta_info->is_ampdu_allowed) {
> > +                       wiphy_warn(hw->wiphy, "ampdu not
> allowed\n");
> > +                       rc = -EPERM;
> > +                       break;
> > +               }
> > +
> > +               if (!stream) {
> > +                       stream = mwl_fwcmd_add_stream(hw, sta,
> tid);
> > +                       if (!stream) {
> > +                               wiphy_warn(hw->wiphy, "no stream
> found\n");
> > +                               rc = -EPERM;
> > +                               break;
> > +                       }
> > +               }
> > +
> > +               spin_unlock_bh(&priv->stream_lock);
> > +               rc = mwl_fwcmd_check_ba(hw, stream, vif);
> > +               spin_lock_bh(&priv->stream_lock);
> > +               if (rc) {
> > +                       mwl_fwcmd_remove_stream(hw, stream);
> > +                       wiphy_err(hw->wiphy,
> > +                                 "ampdu start error code: %d\n",
> rc);
> > +                       rc = -EPERM;
> > +                       break;
> > +               }
> > +               stream->state = AMPDU_STREAM_IN_PROGRESS;
> > +               *ssn = 0;
> > +               ieee80211_start_tx_ba_cb_irqsafe(vif, addr, tid);
> 
> This is why you have the race you mention below. What you should really be
> doing is to wait until the all the packets for that RA / TID are sent from all the
> HW Tx queues (only one realistically), and only then call the _cb. This will
> allow you to send the ADDBA req on the VO queue as it should be according to
> spec.
> 
> [snip]
> 
> > +
> > +void mwl_rx_recv(unsigned long data)
> > +{
> 
> [snip]
> 
> > +
> > +                       if
> (unlikely(ieee80211_is_action(wh->frame_control) &&
> > +                                    mgmt->u.action.category ==
> > +                                    WLAN_CATEGORY_BACK &&
> > +
> mgmt->u.action.u.addba_resp.action_code ==
> > +                                    WLAN_ACTION_ADDBA_RESP))
> {
> > +                               capab =
> mgmt->u.action.u.addba_resp.capab;
> > +                               if (le16_to_cpu(capab) & 1)
> > +
> mwl_rx_enable_sta_amsdu(priv,
> > + mgmt->sa);
> 
> err... no. mac80211 knows how to do that today. Johannes already reported a
> similar issue somewhere else in your code.

Yes. The patch is ready on 2015/9.

> 
> > +                       }
> > +               }
> > +
> > +               if (ieee80211_is_data_qos(wh->frame_control) &&
> > +                   ieee80211_has_a4(wh->frame_control)) {
> > +                       u8 *qc = ieee80211_get_qos_ctl(wh);
> > +
> > +                       if (*qc &
> IEEE80211_QOS_CTL_A_MSDU_PRESENT)
> > +                               if
> (mwl_rx_process_mesh_amsdu(priv, prx_skb,
> > +
> > + &status))
> 
> This sounds to be something you want to teach mac80211 about.
> 
> [snip]
> 
> > +
> > +#define SYSADPT_AMSDU_FW_MAX_SIZE      3300
> > +
> > +#define SYSADPT_AMSDU_4K_MAX_SIZE
> SYSADPT_AMSDU_FW_MAX_SIZE
> > +
> > +#define SYSADPT_AMSDU_8K_MAX_SIZE
> SYSADPT_AMSDU_FW_MAX_SIZE
> 
> In case the FW will grow later?
> 
> > +
> > +#define SYSADPT_AMSDU_ALLOW_SIZE       1600
> > +
> > +#define SYSADPT_AMSDU_FLUSH_TIME       500
> > +
> > +#define SYSADPT_AMSDU_PACKET_THRESHOLD 10
> > +
> > +#define SYSADPT_MAX_TID                8
> > +
> > +#endif /* _mwl_sysadpt_h_ */
> > diff --git a/drivers/net/wireless/marvell/mwlwifi/tx.c
> > b/drivers/net/wireless/marvell/mwlwifi/tx.c
> > new file mode 100644
> > index 0000000..68a994d
> > --- /dev/null
> > +++ b/drivers/net/wireless/marvell/mwlwifi/tx.c
> > @@ -0,0 +1,1251 @@
> > +/*
> > + * Copyright (C) 2006-2015, Marvell International Ltd.
> > + *
> > + * This software file (the "File") is distributed by Marvell
> > +International
> > + * Ltd. under the terms of the GNU General Public License Version 2,
> > +June 1991
> > + * (the "License").  You may use, redistribute and/or modify this
> > +File in
> > + * accordance with the terms and conditions of the License, a copy of
> > +which
> > + * is available by writing to the Free Software Foundation, Inc.
> > + *
> > + * THE FILE IS DISTRIBUTED AS-IS, WITHOUT WARRANTY OF ANY KIND, AND
> > +THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY OR FITNESS FOR A
> PARTICULAR
> > +PURPOSE
> > + * ARE EXPRESSLY DISCLAIMED.  The License provides additional details
> > +about
> > + * this warranty disclaimer.
> > + */
> > +
> > +/* Description:  This file implements transmit related functions. */
> > +
> > +#include <linux/etherdevice.h>
> > +#include <linux/skbuff.h>
> > +
> > +#include "sysadpt.h"
> > +#include "dev.h"
> > +#include "fwcmd.h"
> > +#include "tx.h"
> > +
> > +#define MAX_NUM_TX_RING_BYTES  (SYSADPT_MAX_NUM_TX_DESC * \
> > +                               sizeof(struct mwl_tx_desc))
> > +
> > +#define MAX_NUM_TX_HNDL_BYTES   (SYSADPT_MAX_NUM_TX_DESC *
> \
> > +                               sizeof(struct mwl_tx_hndl))
> > +
> > +#define EAGLE_TXD_XMITCTRL_USE_MC_RATE     0x8     /* Use
> multicast data rate */
> > +
> > +#define MWL_QOS_ACK_POLICY_MASK                   0x0060
> > +#define MWL_QOS_ACK_POLICY_NORMAL          0x0000
> > +#define MWL_QOS_ACK_POLICY_BLOCKACK        0x0060
> > +
> > +#define EXT_IV                             0x20
> > +#define INCREASE_IV(iv16, iv32) \
> > +{ \
> > +       (iv16)++; \
> > +       if ((iv16) == 0) \
> > +               (iv32)++; \
> > +}
> > +
> > +/* Transmit rate information constants */
> > +#define TX_RATE_FORMAT_LEGACY         0
> > +#define TX_RATE_FORMAT_11N            1
> > +#define TX_RATE_FORMAT_11AC           2
> > +
> > +#define TX_RATE_BANDWIDTH_20          0
> > +#define TX_RATE_BANDWIDTH_40          1
> > +#define TX_RATE_BANDWIDTH_80          2
> > +
> > +#define TX_RATE_INFO_STD_GI           0
> > +#define TX_RATE_INFO_SHORT_GI         1
> > +
> > +enum {
> > +       IEEE_TYPE_MANAGEMENT = 0,
> > +       IEEE_TYPE_CONTROL,
> > +       IEEE_TYPE_DATA
> > +};
> > +
> > +struct ccmp_hdr {
> > +       __le16 iv16;
> > +       u8 rsvd;
> > +       u8 key_id;
> > +       __le32 iv32;
> > +} __packed;
> > +
> > +static int mwl_tx_ring_alloc(struct mwl_priv *priv) {
> > +       struct mwl_desc_data *desc;
> > +       int num;
> > +       u8 *mem;
> > +
> > +       desc = &priv->desc_data[0];
> > +
> > +       mem = dma_alloc_coherent(&priv->pdev->dev,
> > +                                MAX_NUM_TX_RING_BYTES *
> > +                                SYSADPT_NUM_OF_DESC_DATA,
> > +                                &desc->pphys_tx_ring,
> > +                                GFP_KERNEL);
> > +
> > +       if (!mem) {
> > +               wiphy_err(priv->hw->wiphy, "cannot alloc mem\n");
> > +               return -ENOMEM;
> > +       }
> > +
> > +       for (num = 0; num < SYSADPT_NUM_OF_DESC_DATA; num++) {
> > +               desc = &priv->desc_data[num];
> > +
> > +               desc->ptx_ring = (struct mwl_tx_desc *)
> > +                       (mem + num * MAX_NUM_TX_RING_BYTES);
> > +
> > +               desc->pphys_tx_ring = (dma_addr_t)
> > +                       ((u32)priv->desc_data[0].pphys_tx_ring +
> > +                       num * MAX_NUM_TX_RING_BYTES);
> > +
> > +               memset(desc->ptx_ring, 0x00,
> > +                      MAX_NUM_TX_RING_BYTES);
> > +       }
> > +
> > +       mem = kmalloc(MAX_NUM_TX_HNDL_BYTES *
> SYSADPT_NUM_OF_DESC_DATA,
> > +                     GFP_KERNEL);
> > +
> > +       if (!mem) {
> > +               wiphy_err(priv->hw->wiphy, "cannot alloc mem\n");
> > +               dma_free_coherent(&priv->pdev->dev,
> > +                                 MAX_NUM_TX_RING_BYTES *
> > +                                 SYSADPT_NUM_OF_DESC_DATA,
> > +                                 priv->desc_data[0].ptx_ring,
> > +                                 priv->desc_data[0].pphys_tx_ring);
> > +               return -ENOMEM;
> > +       }
> > +
> > +       for (num = 0; num < SYSADPT_NUM_OF_DESC_DATA; num++) {
> > +               desc = &priv->desc_data[num];
> > +
> > +               desc->tx_hndl = (struct mwl_tx_hndl *)
> > +                       (mem + num * MAX_NUM_TX_HNDL_BYTES);
> > +
> > +               memset(desc->tx_hndl, 0x00,
> > +                      MAX_NUM_TX_HNDL_BYTES);
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static int mwl_tx_ring_init(struct mwl_priv *priv) {
> > +       int num, i;
> > +       struct mwl_desc_data *desc;
> > +
> > +       for (num = 0; num < SYSADPT_NUM_OF_DESC_DATA; num++) {
> > +               skb_queue_head_init(&priv->txq[num]);
> > +               priv->fw_desc_cnt[num] = 0;
> > +
> > +               desc = &priv->desc_data[num];
> > +
> > +               if (desc->ptx_ring) {
> > +                       for (i = 0; i < SYSADPT_MAX_NUM_TX_DESC;
> i++) {
> > +                               desc->ptx_ring[i].status =
> > +
> cpu_to_le32(EAGLE_TXD_STATUS_IDLE);
> > +                               desc->ptx_ring[i].pphys_next =
> > +
> cpu_to_le32((u32)desc->pphys_tx_ring +
> > +                                       ((i + 1) * sizeof(struct
> mwl_tx_desc)));
> > +                               desc->tx_hndl[i].pdesc =
> > +                                       &desc->ptx_ring[i];
> > +                               if (i < SYSADPT_MAX_NUM_TX_DESC
> - 1)
> > +                                       desc->tx_hndl[i].pnext =
> > +                                               &desc->tx_hndl[i
> + 1];
> > +                       }
> > +                       desc->ptx_ring[SYSADPT_MAX_NUM_TX_DESC
> - 1].pphys_next =
> > +
> cpu_to_le32((u32)desc->pphys_tx_ring);
> > +                       desc->tx_hndl[SYSADPT_MAX_NUM_TX_DESC -
> 1].pnext =
> > +                               &desc->tx_hndl[0];
> > +
> > +                       desc->pstale_tx_hndl = &desc->tx_hndl[0];
> > +                       desc->pnext_tx_hndl  = &desc->tx_hndl[0];
> > +               } else {
> > +                       wiphy_err(priv->hw->wiphy, "no valid TX
> mem\n");
> > +                       return -ENOMEM;
> > +               }
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static void mwl_tx_ring_cleanup(struct mwl_priv *priv) {
> > +       int cleaned_tx_desc = 0;
> > +       int num, i;
> > +       struct mwl_desc_data *desc;
> > +
> > +       for (num = 0; num < SYSADPT_NUM_OF_DESC_DATA; num++) {
> > +               skb_queue_purge(&priv->txq[num]);
> > +               priv->fw_desc_cnt[num] = 0;
> > +
> > +               desc = &priv->desc_data[num];
> > +
> > +               if (desc->ptx_ring) {
> > +                       for (i = 0; i < SYSADPT_MAX_NUM_TX_DESC;
> i++) {
> > +                               if (!desc->tx_hndl[i].psk_buff)
> > +                                       continue;
> > +
> > +                               wiphy_info(priv->hw->wiphy,
> > +                                          "unmapped and free'd
> %i 0x%p 0x%x\n",
> > +                                          i,
> > +
> desc->tx_hndl[i].psk_buff->data,
> > +                                          le32_to_cpu(
> > +
> desc->ptx_ring[i].pkt_ptr));
> > +                               pci_unmap_single(priv->pdev,
> > +                                                le32_to_cpu(
> > +
> desc->ptx_ring[i].pkt_ptr),
> > +
> desc->tx_hndl[i].psk_buff->len,
> > +
> PCI_DMA_TODEVICE);
> > +
> dev_kfree_skb_any(desc->tx_hndl[i].psk_buff);
> > +                               desc->ptx_ring[i].status =
> > +
> cpu_to_le32(EAGLE_TXD_STATUS_IDLE);
> > +                               desc->ptx_ring[i].pkt_ptr = 0;
> > +                               desc->ptx_ring[i].pkt_len = 0;
> > +                               desc->tx_hndl[i].psk_buff = NULL;
> > +                               cleaned_tx_desc++;
> > +                       }
> > +               }
> > +       }
> > +
> > +       wiphy_info(priv->hw->wiphy, "cleaned %i TX descr\n",
> > +cleaned_tx_desc); }
> > +
> > +static void mwl_tx_ring_free(struct mwl_priv *priv) {
> > +       int num;
> > +
> > +       if (priv->desc_data[0].ptx_ring) {
> > +               dma_free_coherent(&priv->pdev->dev,
> > +                                 MAX_NUM_TX_RING_BYTES *
> > +                                 SYSADPT_NUM_OF_DESC_DATA,
> > +                                 priv->desc_data[0].ptx_ring,
> > +                                 priv->desc_data[0].pphys_tx_ring);
> > +       }
> > +
> > +       for (num = 0; num < SYSADPT_NUM_OF_DESC_DATA; num++) {
> > +               if (priv->desc_data[num].ptx_ring)
> > +                       priv->desc_data[num].ptx_ring = NULL;
> > +               priv->desc_data[num].pstale_tx_hndl = NULL;
> > +               priv->desc_data[num].pnext_tx_hndl = NULL;
> > +       }
> > +
> > +       kfree(priv->desc_data[0].tx_hndl);
> > +}
> > +
> > +static inline void mwl_tx_add_dma_header(struct mwl_priv *priv,
> > +                                        struct sk_buff *skb,
> > +                                        int head_pad,
> > +                                        int tail_pad) {
> > +       struct ieee80211_hdr *wh;
> > +       int hdrlen;
> > +       int reqd_hdrlen;
> > +       struct mwl_dma_data *tr;
> > +
> > +       /* Add a firmware DMA header; the firmware requires that we
> > +        * present a 2-byte payload length followed by a 4-address
> > +        * header (without QoS field), followed (optionally) by any
> > +        * WEP/ExtIV header (but only filled in for CCMP).
> > +        */
> > +       wh = (struct ieee80211_hdr *)skb->data;
> > +
> > +       hdrlen = ieee80211_hdrlen(wh->frame_control);
> > +
> > +       reqd_hdrlen = sizeof(*tr) + head_pad;
> > +
> > +       if (hdrlen != reqd_hdrlen)
> > +               skb_push(skb, reqd_hdrlen - hdrlen);
> > +
> > +       if (ieee80211_is_data_qos(wh->frame_control))
> > +               hdrlen -= IEEE80211_QOS_CTL_LEN;
> > +
> > +       tr = (struct mwl_dma_data *)skb->data;
> > +
> > +       if (wh != &tr->wh)
> > +               memmove(&tr->wh, wh, hdrlen);
> > +
> > +       if (hdrlen != sizeof(tr->wh))
> > +               memset(((void *)&tr->wh) + hdrlen, 0, sizeof(tr->wh) -
> > + hdrlen);
> > +
> > +       /* Firmware length is the length of the fully formed "802.11
> > +        * payload".  That is, everything except for the 802.11 header.
> > +        * This includes all crypto material including the MIC.
> > +        */
> > +       tr->fwlen = cpu_to_le16(skb->len - sizeof(*tr) + tail_pad); }
> > +
> > +static inline void mwl_tx_encapsulate_frame(struct mwl_priv *priv,
> > +                                           struct sk_buff *skb,
> > +                                           struct
> ieee80211_key_conf *k_conf,
> > +                                           bool *ccmp) {
> > +       int head_pad = 0;
> > +       int data_pad = 0;
> > +
> > +       /* Make sure the packet header is in the DMA header format
> (4-address
> > +        * without QoS), and add head & tail padding when HW crypto is
> enabled.
> > +        *
> > +        * We have the following trailer padding requirements:
> > +        * - WEP: 4 trailer bytes (ICV)
> > +        * - TKIP: 12 trailer bytes (8 MIC + 4 ICV)
> > +        * - CCMP: 8 trailer bytes (MIC)
> > +        */
> > +
> > +       if (k_conf) {
> > +               head_pad = k_conf->iv_len;
> > +
> > +               switch (k_conf->cipher) {
> > +               case WLAN_CIPHER_SUITE_WEP40:
> > +               case WLAN_CIPHER_SUITE_WEP104:
> > +                       data_pad = 4;
> > +                       break;
> > +               case WLAN_CIPHER_SUITE_TKIP:
> > +                       data_pad = 12;
> > +                       break;
> > +               case WLAN_CIPHER_SUITE_CCMP:
> > +                       data_pad = 8;
> > +                       *ccmp = true;
> > +                       break;
> > +               }
> > +       }
> > +
> > +       mwl_tx_add_dma_header(priv, skb, head_pad, data_pad); }
> > +
> > +static inline void mwl_tx_insert_ccmp_hdr(u8 *pccmp_hdr,
> > +                                         u8 key_id, u16 iv16, u32
> > +iv32) {
> > +       struct ccmp_hdr *ccmp_h = (struct ccmp_hdr *)pccmp_hdr;
> > +
> > +       ccmp_h->iv16 = cpu_to_le16(iv16);
> > +       ccmp_h->rsvd = 0;
> > +       ccmp_h->key_id = EXT_IV | (key_id << 6);
> > +       ccmp_h->iv32 = cpu_to_le32(iv32); }
> > +
> > +static inline int mwl_tx_tid_queue_mapping(u8 tid) {
> > +       switch (tid) {
> > +       case 0:
> > +       case 3:
> > +               return IEEE80211_AC_BE;
> > +       case 1:
> > +       case 2:
> > +               return IEEE80211_AC_BK;
> > +       case 4:
> > +       case 5:
> > +               return IEEE80211_AC_VI;
> > +       case 6:
> > +       case 7:
> > +               return IEEE80211_AC_VO;
> > +       default:
> > +               break;
> > +       }
> > +
> > +       return -1;
> > +}
> > +
> > +static inline void mwl_tx_count_packet(struct ieee80211_sta *sta, u8
> > +tid) {
> > +       struct mwl_sta *sta_info;
> > +       struct mwl_tx_info *tx_stats;
> > +
> > +       if (WARN_ON(tid >= SYSADPT_MAX_TID))
> > +               return;
> > +
> > +       sta_info = mwl_dev_get_sta(sta);
> > +
> > +       tx_stats = &sta_info->tx_stats[tid];
> > +
> > +       if (tx_stats->start_time == 0)
> > +               tx_stats->start_time = jiffies;
> > +
> > +       /* reset the packet count after each second elapses.  If the
> number of
> > +        * packets ever exceeds the ampdu_min_traffic threshold, we will
> allow
> > +        * an ampdu stream to be started.
> > +        */
> > +       if (jiffies - tx_stats->start_time > HZ) {
> > +               tx_stats->pkts = 0;
> > +               tx_stats->start_time = 0;
> > +       } else {
> > +               tx_stats->pkts++;
> > +       }
> > +}
> > +
> > +static inline bool mwl_tx_available(struct mwl_priv *priv, int
> > +desc_num) {
> > +       struct mwl_tx_hndl *tx_hndl;
> > +
> > +       tx_hndl = priv->desc_data[desc_num].pnext_tx_hndl;
> > +
> > +       if (!tx_hndl->pdesc)
> > +               return false;
> > +
> > +       if (tx_hndl->pdesc->status != EAGLE_TXD_STATUS_IDLE) {
> > +               /* Interrupt F/W anyway */
> > +               if (tx_hndl->pdesc->status &
> > +                   cpu_to_le32(EAGLE_TXD_STATUS_FW_OWNED))
> > +                       writel(MACREG_H2ARIC_BIT_PPA_READY,
> > +                              priv->iobase1 +
> > +
> MACREG_REG_H2A_INTERRUPT_EVENTS);
> > +               return false;
> > +       }
> > +
> > +       return true;
> > +}
> > +
> > +static inline void mwl_tx_skb(struct mwl_priv *priv, int desc_num,
> > +                             struct sk_buff *tx_skb) {
> > +       struct ieee80211_tx_info *tx_info;
> > +       struct mwl_tx_ctrl *tx_ctrl;
> > +       struct mwl_tx_hndl *tx_hndl;
> > +       struct mwl_tx_desc *tx_desc;
> > +       struct ieee80211_sta *sta;
> > +       struct ieee80211_vif *vif;
> > +       struct mwl_vif *mwl_vif;
> > +       struct ieee80211_key_conf *k_conf;
> > +       bool ccmp = false;
> > +       struct mwl_dma_data *dma_data;
> > +       struct ieee80211_hdr *wh;
> > +       dma_addr_t dma;
> > +
> > +       if (WARN_ON(!tx_skb))
> > +               return;
> > +
> > +       tx_info = IEEE80211_SKB_CB(tx_skb);
> > +       tx_ctrl = (struct mwl_tx_ctrl *)&tx_info->status;
> > +       sta = (struct ieee80211_sta *)tx_ctrl->sta;
> > +       vif = (struct ieee80211_vif *)tx_ctrl->vif;
> > +       mwl_vif = mwl_dev_get_vif(vif);
> > +       k_conf = (struct ieee80211_key_conf *)tx_ctrl->k_conf;
> > +
> > +       mwl_tx_encapsulate_frame(priv, tx_skb, k_conf, &ccmp);
> > +
> > +       dma_data = (struct mwl_dma_data *)tx_skb->data;
> > +       wh = &dma_data->wh;
> > +
> > +       if (ieee80211_is_data(wh->frame_control)) {
> > +               if (is_multicast_ether_addr(wh->addr1)) {
> > +                       if (ccmp) {
> > +
> mwl_tx_insert_ccmp_hdr(dma_data->data,
> > +
> mwl_vif->keyidx,
> > +
> mwl_vif->iv16,
> > +
> mwl_vif->iv32);
> > +                               INCREASE_IV(mwl_vif->iv16,
> mwl_vif->iv32);
> > +                       }
> > +               } else {
> > +                       if (ccmp) {
> > +                               if (vif->type ==
> NL80211_IFTYPE_STATION) {
> > +
> mwl_tx_insert_ccmp_hdr(dma_data->data,
> > +
> mwl_vif->keyidx,
> > +
> mwl_vif->iv16,
> > +
> mwl_vif->iv32);
> > +
> INCREASE_IV(mwl_vif->iv16,
> > +
> mwl_vif->iv32);
> > +                               } else {
> > +                                       struct mwl_sta *sta_info;
> > +
> > +                                       sta_info =
> > + mwl_dev_get_sta(sta);
> > +
> > +
> mwl_tx_insert_ccmp_hdr(dma_data->data,
> > +
> 0,
> > +
> sta_info->iv16,
> > +
> sta_info->iv32);
> > +
> INCREASE_IV(sta_info->iv16,
> > +
> sta_info->iv32);
> > +                               }
> > +                       }
> > +               }
> > +       }
> > +
> > +       tx_hndl = priv->desc_data[desc_num].pnext_tx_hndl;
> > +       tx_hndl->psk_buff = tx_skb;
> > +       tx_desc = tx_hndl->pdesc;
> > +       tx_desc->tx_priority = tx_ctrl->tx_priority;
> > +       tx_desc->qos_ctrl = cpu_to_le16(tx_ctrl->qos_ctrl);
> > +       tx_desc->pkt_len = cpu_to_le16(tx_skb->len);
> > +       tx_desc->packet_info = 0;
> > +       tx_desc->data_rate = 0;
> > +       tx_desc->type = tx_ctrl->type;
> > +       tx_desc->xmit_control = tx_ctrl->xmit_control;
> > +       tx_desc->sap_pkt_info = 0;
> > +       dma = pci_map_single(priv->pdev, tx_skb->data,
> > +                            tx_skb->len, PCI_DMA_TODEVICE);
> > +       if (pci_dma_mapping_error(priv->pdev, dma)) {
> > +               dev_kfree_skb_any(tx_skb);
> > +               wiphy_err(priv->hw->wiphy,
> > +                         "failed to map pci memory!\n");
> > +               return;
> > +       }
> > +       tx_desc->pkt_ptr = cpu_to_le32(dma);
> > +       tx_desc->status = cpu_to_le32(EAGLE_TXD_STATUS_FW_OWNED);
> > +       /* make sure all the memory transactions done by cpu were
> completed */
> > +       wmb();  /*Data Memory Barrier*/
> > +       writel(MACREG_H2ARIC_BIT_PPA_READY,
> > +              priv->iobase1 +
> MACREG_REG_H2A_INTERRUPT_EVENTS);
> > +       priv->desc_data[desc_num].pnext_tx_hndl = tx_hndl->pnext;
> > +       priv->fw_desc_cnt[desc_num]++; }
> > +
> > +static inline struct sk_buff *mwl_tx_do_amsdu(struct mwl_priv *priv,
> > +                                             int desc_num,
> > +                                             struct sk_buff
> *tx_skb,
> > +                                             struct
> ieee80211_tx_info
> > +*tx_info) {
> > +       struct ieee80211_sta *sta;
> > +       struct mwl_sta *sta_info;
> > +       struct mwl_tx_ctrl *tx_ctrl = (struct mwl_tx_ctrl
> *)&tx_info->status;
> > +       struct ieee80211_tx_info *amsdu_info;
> > +       struct sk_buff_head *amsdu_pkts;
> > +       struct mwl_amsdu_frag *amsdu;
> > +       int amsdu_allow_size;
> > +       struct ieee80211_hdr *wh;
> > +       int wh_len;
> > +       u16 len;
> > +       u8 *data;
> > +
> > +       sta = (struct ieee80211_sta *)tx_ctrl->sta;
> > +       sta_info = mwl_dev_get_sta(sta);
> > +
> > +       if (!sta_info->is_amsdu_allowed)
> > +               return tx_skb;
> > +
> > +       wh = (struct ieee80211_hdr *)tx_skb->data;
> > +       if (sta_info->is_mesh_node &&
> is_multicast_ether_addr(wh->addr3))
> > +               return tx_skb;
> > +
> > +       if (sta_info->amsdu_ctrl.cap == MWL_AMSDU_SIZE_4K)
> > +               amsdu_allow_size = SYSADPT_AMSDU_4K_MAX_SIZE;
> > +       else if (sta_info->amsdu_ctrl.cap == MWL_AMSDU_SIZE_8K)
> > +               amsdu_allow_size = SYSADPT_AMSDU_8K_MAX_SIZE;
> > +       else
> > +               return tx_skb;
> > +
> > +       spin_lock_bh(&sta_info->amsdu_lock);
> > +       amsdu = &sta_info->amsdu_ctrl.frag[desc_num];
> > +
> > +       if (tx_skb->len > SYSADPT_AMSDU_ALLOW_SIZE) {
> > +               if (amsdu->num) {
> > +                       mwl_tx_skb(priv, desc_num, amsdu->skb);
> > +                       amsdu->num = 0;
> > +                       amsdu->cur_pos = NULL;
> > +
> > +                       if (!mwl_tx_available(priv, desc_num)) {
> > +
> skb_queue_head(&priv->txq[desc_num], tx_skb);
> > +
> spin_unlock_bh(&sta_info->amsdu_lock);
> > +                               return NULL;
> > +                       }
> > +               }
> > +               spin_unlock_bh(&sta_info->amsdu_lock);
> > +               return tx_skb;
> > +       }
> > +
> > +       /* potential amsdu size, should add amsdu header 14 bytes +
> > +        * maximum padding 3.
> > +        */
> > +       wh_len = ieee80211_hdrlen(wh->frame_control);
> > +       len = tx_skb->len - wh_len + 17;
> > +
> > +       if (amsdu->num) {
> > +               if ((amsdu->skb->len + len) > amsdu_allow_size) {
> > +                       mwl_tx_skb(priv, desc_num, amsdu->skb);
> > +                       amsdu->num = 0;
> > +                       amsdu->cur_pos = NULL;
> > +               }
> > +       }
> > +
> > +       amsdu->jiffies = jiffies;
> > +       len = tx_skb->len - wh_len;
> > +
> > +       if (amsdu->num == 0) {
> > +               struct sk_buff *newskb;
> > +
> > +               amsdu_pkts = (struct sk_buff_head *)
> > +                       kmalloc(sizeof(*amsdu_pkts), GFP_KERNEL);
> > +               if (!amsdu_pkts) {
> > +                       spin_unlock_bh(&sta_info->amsdu_lock);
> > +                       return tx_skb;
> > +               }
> > +               newskb = dev_alloc_skb(amsdu_allow_size +
> > +
> SYSADPT_MIN_BYTES_HEADROOM);
> > +               if (!newskb) {
> > +                       spin_unlock_bh(&sta_info->amsdu_lock);
> > +                       kfree(amsdu_pkts);
> > +                       return tx_skb;
> > +               }
> > +
> > +               data = newskb->data;
> > +               memcpy(data, tx_skb->data, wh_len);
> > +               if (sta_info->is_mesh_node) {
> > +                       ether_addr_copy(data + wh_len, wh->addr3);
> > +                       ether_addr_copy(data + wh_len + ETH_ALEN,
> wh->addr4);
> > +               } else {
> > +                       ether_addr_copy(data + wh_len,
> > +                                       ieee80211_get_DA(wh));
> > +                       ether_addr_copy(data + wh_len + ETH_ALEN,
> > +                                       ieee80211_get_SA(wh));
> > +               }
> > +               *(u8 *)(data + wh_len + ETH_HLEN - 1) = len & 0xff;
> > +               *(u8 *)(data + wh_len + ETH_HLEN - 2) = (len >> 8) &
> > + 0xff;
> 
> These bit oeprations are not needed. The pointer must be WORD aligned,
> right?
> This is why you have padding after all, unless you have some weird DMA
> games?
> You seem to be assuming this alignment anyway because you use
> ether_addr_copy above.
> 
> > +
> > +       /* Queue ADDBA request in the respective data queue.  While
> setting up
> > +        * the ampdu stream, mac80211 queues further packets for that
> > +        * particular ra/tid pair.  However, packets piled up in the
> hardware
> > +        * for that ra/tid pair will still go out. ADDBA request and the
> > +        * related data packets going out from different queues
> asynchronously
> > +        * will cause a shift in the receiver window which might result in
> > +        * ampdu packets getting dropped at the receiver after the stream
> has
> > +        * been setup.
> > +        */
> 
> Here is the race you are facing and that race can be avoided by the proper
> usage of the _cb
> 
> > +       if (mgmtframe) {
> > +               u16 capab;
> > +
> > +               if (unlikely(ieee80211_is_action(wh->frame_control) &&
> > +                            mgmt->u.action.category ==
> WLAN_CATEGORY_BACK &&
> > +
> mgmt->u.action.u.addba_req.action_code ==
> > +                            WLAN_ACTION_ADDBA_REQ)) {
> > +                       capab =
> le16_to_cpu(mgmt->u.action.u.addba_req.capab);
> > +                       tid = (capab &
> IEEE80211_ADDBA_PARAM_TID_MASK) >> 2;
> > +                       index = mwl_tx_tid_queue_mapping(tid);
> > +                       capab |= 1;
> > +                       mgmt->u.action.u.addba_req.capab =
> cpu_to_le16(capab);
> > +               }
> > +
> > +               if (unlikely(ieee80211_is_action(wh->frame_control) &&
> > +                            mgmt->u.action.category ==
> WLAN_CATEGORY_BACK &&
> > +
> mgmt->u.action.u.addba_resp.action_code ==
> > +                            WLAN_ACTION_ADDBA_RESP)) {
> > +                       capab =
> le16_to_cpu(mgmt->u.action.u.addba_resp.capab);
> > +                       capab |= 1;
> > +                       mgmt->u.action.u.addba_resp.capab =
> cpu_to_le16(capab);
> > +               }
> > +       }
> > +
> > +       index = SYSADPT_TX_WMM_QUEUES - index - 1;
> > +       txpriority = index;
> > +
> > +       if (sta && sta->ht_cap.ht_supported && !eapol_frame &&
> > +           ieee80211_is_data_qos(wh->frame_control)) {
> > +               tid = qos & 0xf;
> > +               mwl_tx_count_packet(sta, tid);
> > +
> > +               spin_lock_bh(&priv->stream_lock);
> > +               stream = mwl_fwcmd_lookup_stream(hw, sta->addr, tid);
> > +
> > +               if (stream) {
> > +                       if (stream->state == AMPDU_STREAM_ACTIVE)
> {
> > +                               if (WARN_ON(!(qos &
> > +
> MWL_QOS_ACK_POLICY_BLOCKACK))) {
> > +
> spin_unlock_bh(&priv->stream_lock);
> > +                                       dev_kfree_skb_any(skb);
> > +                                       return;
> > +                               }
> > +
> > +                               txpriority =
> > +
> (SYSADPT_TX_WMM_QUEUES + stream->idx) %
> > +
> SYSADPT_TOTAL_HW_QUEUES;
> > +                       } else if (stream->state ==
> AMPDU_STREAM_NEW) {
> > +                               /* We get here if the driver sends us
> packets
> > +                                * after we've initiated a stream, but
> before
> > +                                * our ampdu_action routine has
> been called
> > +                                * with
> IEEE80211_AMPDU_TX_START to get the SSN
> > +                                * for the ADDBA request.  So this
> packet can
> > +                                * go out with no risk of sequence
> number
> > +                                * mismatch.  No special handling is
> required.
> > +                                */
> > +                       } else {
> > +                               /* Drop packets that would go out
> after the
> > +                                * ADDBA request was sent but
> before the ADDBA
> > +                                * response is received.  If we don't
> do this,
> > +                                * the recipient would probably
> receive it
> > +                                * after the ADDBA request with SSN
> 0.  This
> > +                                * will cause the recipient's BA
> receive window
> > +                                * to shift, which would cause the
> subsequent
> > +                                * packets in the BA stream to be
> discarded.
> > +                                * mac80211 queues our packets for
> us in this
> > +                                * case, so this is really just a safety
> check.
> > +                                */
> 
> Well... I think you can really avoid that.

AMPDU related code will be checked based on your suggestions later.
��.n��������+%������w��{.n�����{���zW����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f




[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux