[rt2x00-users] [PATCH 06/12] rt2x00: Fix rt2800pci use of WCID.
Ivo van Doorn
ivdoorn at gmail.com
Sun Aug 16 12:27:55 UTC 2009
On Saturday 15 August 2009, Benoit PAPILLAULT wrote:
> Fixed the crypto bug introduced ealier when using WCID incorrectly. We now use
> WCID as before (ie with the crypto key index). The process done in
> rt2800pci_txdone() has been changed to only process the last descriptor of the
> specified queue. We use WCID/ACK/PID fields for checking the TX_STA_FIFO
> content (it's not perfect thought).
>
> Signed-off-by: Benoit PAPILLAULT <benoit.papillault at free.fr>
> ---
> drivers/net/wireless/rt2x00/rt2800pci.c | 105 ++++++++++++++++++-------------
> 1 files changed, 60 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
> index 8626a71..05f1bce 100644
> --- a/drivers/net/wireless/rt2x00/rt2800pci.c
> +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
> @@ -2114,8 +2114,10 @@ static void rt2800pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
> __le32 *txd = skbdesc->desc;
> __le32 *txwi = (__le32 *)(skb->data - rt2x00dev->hw->extra_tx_headroom);
> u32 word;
> - int wcid = skbdesc->entry->entry_idx + 1;
> - int pid = skbdesc->entry->queue->qid + 1;
> + int tx_wcid = test_bit(ENTRY_TXD_ENCRYPT, &txdesc->flags) ?
> + txdesc->key_idx : 0xff;
> + int tx_ack = test_bit(ENTRY_TXD_ACK, &txdesc->flags);
> + int tx_pid = skbdesc->entry->queue->qid + 1;
>
> /*
> * Initialize TX Info descriptor
> @@ -2141,21 +2143,28 @@ static void rt2800pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
> rt2x00_desc_write(txwi, 0, word);
>
> word = 0;
> - rt2x00_set_field32(&word, TXWI_W1_ACK,
> - test_bit(ENTRY_TXD_ACK, &txdesc->flags));
> + rt2x00_set_field32(&word, TXWI_W1_ACK, tx_ack);
> rt2x00_set_field32(&word, TXWI_W1_NSEQ,
> test_bit(ENTRY_TXD_GENERATE_SEQ, &txdesc->flags));
> rt2x00_set_field32(&word, TXWI_W1_BW_WIN_SIZE, txdesc->ba_size);
> - rt2x00_set_field32(&word, TXWI_W1_WIRELESS_CLI_ID, wcid);
> + rt2x00_set_field32(&word, TXWI_W1_WIRELESS_CLI_ID, tx_wcid);
> rt2x00_set_field32(&word, TXWI_W1_MPDU_TOTAL_BYTE_COUNT,
> skb->len - txdesc->l2pad);
Apparently already fixed because of my previous comments. ;)
> @@ -2404,13 +2413,12 @@ static void rt2800pci_txdone(struct rt2x00_dev *rt2x00dev)
> {
> struct data_queue *queue;
> struct queue_entry *entry;
> - struct queue_entry *entry_done;
> - struct queue_entry_priv_pci *entry_priv;
> + __le32 *txwi;
> struct txdone_entry_desc txdesc;
> u32 word;
> u32 reg;
> int i;
> - int wcid, pid;
> + int wcid, ack, pid, tx_wcid, tx_ack, tx_pid;
>
> /*
> * To avoid an endlees loop, we only read the TX_STA_FIFO register up
> @@ -2424,65 +2432,72 @@ static void rt2800pci_txdone(struct rt2x00_dev *rt2x00dev)
> if (!rt2x00_get_field32(reg, TX_STA_FIFO_VALID))
> break;
>
> - pid = rt2x00_get_field32(reg, TX_STA_FIFO_PID_TYPE);
> - wcid = rt2x00_get_field32(reg, TX_STA_FIFO_WCID);
> + wcid = rt2x00_get_field32(reg, TX_STA_FIFO_WCID);
> + ack = rt2x00_get_field32(reg, TX_STA_FIFO_TX_ACK_REQUIRED);
> + pid = rt2x00_get_field32(reg, TX_STA_FIFO_PID_TYPE);
>
> - printk("%s: TX_STA_FIFO=%08x VALID=1 WCID=%d PID=%d\n",
> - __func__, reg, wcid, pid);
> + printk("%s: TX_STA_FIFO=%08x WCID=%d ACK=%d PID=%d\n",
> + __func__, reg, wcid, ack, pid);
>
> /*
> * Skip this entry when it contains an invalid
> * queue identication number.
> */
> - if (pid < 1)
> + if (pid < 1) {
> + printk("%s: invalid PID=%d\n", __func__, pid);
> continue;
> + }
>
> queue = rt2x00queue_get_queue(rt2x00dev, pid - 1);
> - if (unlikely(!queue))
> + if (unlikely(!queue)) {
> + printk("%s: invalid PID=%d\n", __func__, pid);
> continue;
> -
> - printk("%s: queue=%p\n", __func__, queue);
> + }
>
> /*
> - * Skip this entry when it contains an invalid
> - * index number. We must have :
> - * 0 <= wcid - 1 < queue->limit
> + * Inside each queue, we process each entry in a chronological
> + * order
> */
> - if (unlikely(wcid < 1))
> - continue;
> - if (unlikely(wcid > queue->limit))
> + if (queue->length == 0) {
> + printk("%s: invalid queue %p empty\n", __func__, queue);
> continue;
> + }
> + entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
> +
> + printk("%s: queue=%p entry=%p entry->skb=%p skb->data=%p\n",
> + __func__,
> + queue,
> + entry, entry->skb, entry->skb->data);
> +
> + /* Check if we got a match : HOW ? */
> + txwi = (__le32 *)(entry->skb->data -
> + rt2x00dev->hw->extra_tx_headroom);
> +
> + rt2x00_desc_read(txwi, 1, &word);
> + tx_wcid = rt2x00_get_field32(word, TXWI_W1_WIRELESS_CLI_ID);
> + tx_ack = rt2x00_get_field32(word, TXWI_W1_ACK);
> + tx_pid = rt2x00_get_field32(word, TXWI_W1_PACKETID);
>
> - entry = &queue->entries[wcid-1];
> - entry_priv = entry->priv_data;
> - rt2x00_desc_read((__le32 *)entry->skb->data, 0, &word);
> -
> - entry_done = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
> - while (entry != entry_done) {
> - /*
> - * Catch up.
> - * Just report any entries we missed as failed.
> - */
> - WARNING(rt2x00dev,
> - "TX status report missed for entry %d\n",
> - entry_done->entry_idx);
> -
> - txdesc.flags = 0;
> - __set_bit(TXDONE_UNKNOWN, &txdesc.flags);
> - txdesc.retry = 0;
> -
> - rt2x00lib_txdone(entry_done, &txdesc);
> - entry_done = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
> + printk("%s: TXWI WCID=%d ACK=%d PID=%d\n",
> + __func__, tx_wcid, tx_ack, tx_pid);
> +
> + if ((wcid != tx_wcid) || (ack != tx_ack) || (pid != tx_pid)) {
> + printk("%s: invalid WCID/ACK/PID\n",
> + __func__);
> }
>
> /*
> * Obtain the status about this packet.
> */
> +
> +
> txdesc.flags = 0;
> if (rt2x00_get_field32(reg, TX_STA_FIFO_TX_SUCCESS))
> __set_bit(TXDONE_SUCCESS, &txdesc.flags);
> else
> __set_bit(TXDONE_FAILURE, &txdesc.flags);
> +
> + rt2x00_desc_read(txwi, 0, &word);
> txdesc.retry = rt2x00_get_field32(word, TXWI_W0_MCS);
>
> rt2x00lib_txdone(entry, &txdesc);
With this part of the patch is a bit hard to see what you are trying to accomplish
because of all the debug stuff. Apparently you are assuming that there are
no missing TX reports possible, but are you sure that is a valid assumption?
Ivo
More information about the users
mailing list