* rtlwifi handling of sequence numbers with aggregation
@ 2017-08-25 17:51 Jes Sorensen
2017-08-26 2:15 ` Larry Finger
0 siblings, 1 reply; 4+ messages in thread
From: Jes Sorensen @ 2017-08-25 17:51 UTC (permalink / raw)
To: linux-wireless; +Cc: Larry Finger
Hi,
Looking at some bits in rtlwifi I came across a discrepancy between the
PCI and USB code.
Consider the following code:
In rtl_pci_tx():
if (ieee80211_is_data_qos(fc)) {
tid = rtl_get_tid(skb);
if (sta) {
sta_entry = (struct rtl_sta_info *)sta->drv_priv;
seq_number = (le16_to_cpu(hdr->seq_ctrl) &
IEEE80211_SCTL_SEQ) >> 4;
seq_number += 1;
if (!ieee80211_has_morefrags(hdr->frame_control))
sta_entry->tids[tid].seq_number = seq_number;
}
In _rtl_usb_tx_preprocess():
if (ieee80211_is_data_qos(fc)) {
qc = ieee80211_get_qos_ctl(hdr);
tid = qc[0] & IEEE80211_QOS_CTL_TID_MASK;
seq_number = (le16_to_cpu(hdr->seq_ctrl) &
IEEE80211_SCTL_SEQ) >> 4;
seq_number += 1;
seq_number <<= 4;
}
[snip]
if (!ieee80211_has_morefrags(hdr->frame_control)) {
if (qc)
mac->tids[tid].seq_number = seq_number;
}
The seq_number is picked up from ieee80211_ops->ampdu_action()
which calls into rtl_tx_agg_start():
tid_data = &sta_entry->tids[tid];
RT_TRACE(rtlpriv, COMP_SEND, DBG_DMESG,
"on ra = %pM tid = %d seq:%d\n", sta->addr, tid,
tid_data->seq_number);
*ssn = tid_data->seq_number;
My question here is why does the USB code shift seq_number << 4 while
the PCI code doesn't? I assume one of these is wrong, but which one?
Jes
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: rtlwifi handling of sequence numbers with aggregation
2017-08-25 17:51 rtlwifi handling of sequence numbers with aggregation Jes Sorensen
@ 2017-08-26 2:15 ` Larry Finger
2017-08-29 14:18 ` Larry Finger
0 siblings, 1 reply; 4+ messages in thread
From: Larry Finger @ 2017-08-26 2:15 UTC (permalink / raw)
To: Jes Sorensen, linux-wireless
On 08/25/2017 12:51 PM, Jes Sorensen wrote:
> Hi,
>
> Looking at some bits in rtlwifi I came across a discrepancy between the
> PCI and USB code.
>
> Consider the following code:
>
> In rtl_pci_tx():
> if (ieee80211_is_data_qos(fc)) {
> tid = rtl_get_tid(skb);
> if (sta) {
> sta_entry = (struct rtl_sta_info *)sta->drv_priv;
> seq_number = (le16_to_cpu(hdr->seq_ctrl) &
> IEEE80211_SCTL_SEQ) >> 4;
> seq_number += 1;
>
> if (!ieee80211_has_morefrags(hdr->frame_control))
> sta_entry->tids[tid].seq_number = seq_number;
> }
>
> In _rtl_usb_tx_preprocess():
> if (ieee80211_is_data_qos(fc)) {
> qc = ieee80211_get_qos_ctl(hdr);
> tid = qc[0] & IEEE80211_QOS_CTL_TID_MASK;
> seq_number = (le16_to_cpu(hdr->seq_ctrl) &
> IEEE80211_SCTL_SEQ) >> 4;
> seq_number += 1;
> seq_number <<= 4;
> }
> [snip]
> if (!ieee80211_has_morefrags(hdr->frame_control)) {
> if (qc)
> mac->tids[tid].seq_number = seq_number;
> }
>
> The seq_number is picked up from ieee80211_ops->ampdu_action()
> which calls into rtl_tx_agg_start():
> tid_data = &sta_entry->tids[tid];
>
> RT_TRACE(rtlpriv, COMP_SEND, DBG_DMESG,
> "on ra = %pM tid = %d seq:%d\n", sta->addr, tid,
> tid_data->seq_number);
>
> *ssn = tid_data->seq_number;
>
> My question here is why does the USB code shift seq_number << 4 while
> the PCI code doesn't? I assume one of these is wrong, but which one?
Based on my prejudices, I would suspect the USB driver before that of the PCI
version. I have BCc'd my contact at Realtek to see what he has to say on the issue.
Larry
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: rtlwifi handling of sequence numbers with aggregation
2017-08-26 2:15 ` Larry Finger
@ 2017-08-29 14:18 ` Larry Finger
2017-08-29 17:32 ` Jes Sorensen
0 siblings, 1 reply; 4+ messages in thread
From: Larry Finger @ 2017-08-29 14:18 UTC (permalink / raw)
To: Jes Sorensen; +Cc: linux-wireless
On 08/25/2017 09:15 PM, Larry Finger wrote:
> On 08/25/2017 12:51 PM, Jes Sorensen wrote:
>> Hi,
>>
>> Looking at some bits in rtlwifi I came across a discrepancy between the
>> PCI and USB code.
>>
>> Consider the following code:
>>
>> In rtl_pci_tx():
>> if (ieee80211_is_data_qos(fc)) {
>> tid = rtl_get_tid(skb);
>> if (sta) {
>> sta_entry = (struct rtl_sta_info *)sta->drv_priv;
>> seq_number = (le16_to_cpu(hdr->seq_ctrl) &
>> IEEE80211_SCTL_SEQ) >> 4;
>> seq_number += 1;
>>
>> if (!ieee80211_has_morefrags(hdr->frame_control))
>> sta_entry->tids[tid].seq_number = seq_number;
>> }
>>
>> In _rtl_usb_tx_preprocess():
>> if (ieee80211_is_data_qos(fc)) {
>> qc = ieee80211_get_qos_ctl(hdr);
>> tid = qc[0] & IEEE80211_QOS_CTL_TID_MASK;
>> seq_number = (le16_to_cpu(hdr->seq_ctrl) &
>> IEEE80211_SCTL_SEQ) >> 4;
>> seq_number += 1;
>> seq_number <<= 4;
>> }
>> [snip]
>> if (!ieee80211_has_morefrags(hdr->frame_control)) {
>> if (qc)
>> mac->tids[tid].seq_number = seq_number;
>> }
>>
>> The seq_number is picked up from ieee80211_ops->ampdu_action()
>> which calls into rtl_tx_agg_start():
>> tid_data = &sta_entry->tids[tid];
>>
>> RT_TRACE(rtlpriv, COMP_SEND, DBG_DMESG,
>> "on ra = %pM tid = %d seq:%d\n", sta->addr, tid,
>> tid_data->seq_number);
>>
>> *ssn = tid_data->seq_number;
>>
>> My question here is why does the USB code shift seq_number << 4 while
>> the PCI code doesn't? I assume one of these is wrong, but which one?
>
> Based on my prejudices, I would suspect the USB driver before that of the PCI
> version. I have BCc'd my contact at Realtek to see what he has to say on the issue.
My contact at Realtek replies that the PCI code is correct. He further states
that "Since mac80211 uses ieee80211_tx_next_seq() to store next sequence number
in sta->tid_seq[tid] and fill the sequence number in AMPDU parameters, I think
driver doesn't need to maintain the seq_number anymore. So, let's remove 'u16
seq_number' from 'struct rtl_tid_data', please refer to attachment." I have not
had a chance to test his patch, but I trust his analysis.
Larry
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: rtlwifi handling of sequence numbers with aggregation
2017-08-29 14:18 ` Larry Finger
@ 2017-08-29 17:32 ` Jes Sorensen
0 siblings, 0 replies; 4+ messages in thread
From: Jes Sorensen @ 2017-08-29 17:32 UTC (permalink / raw)
To: Larry Finger; +Cc: linux-wireless
On 08/29/2017 10:18 AM, Larry Finger wrote:
> On 08/25/2017 09:15 PM, Larry Finger wrote:
>> Based on my prejudices, I would suspect the USB driver before that of
>> the PCI version. I have BCc'd my contact at Realtek to see what he has
>> to say on the issue.
>
> My contact at Realtek replies that the PCI code is correct. He further
> states that "Since mac80211 uses ieee80211_tx_next_seq() to store next
> sequence number in sta->tid_seq[tid] and fill the sequence number in
> AMPDU parameters, I think driver doesn't need to maintain the seq_number
> anymore. So, let's remove 'u16 seq_number' from 'struct rtl_tid_data',
> please refer to attachment." I have not had a chance to test his patch,
> but I trust his analysis.
I don't see any attachment, but I'm happy with a correct solution if
you'll work with them on getting that pushed in.
Cheers,
Jes
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-08-29 17:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-25 17:51 rtlwifi handling of sequence numbers with aggregation Jes Sorensen
2017-08-26 2:15 ` Larry Finger
2017-08-29 14:18 ` Larry Finger
2017-08-29 17:32 ` Jes Sorensen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.