I noticed there is a superfluous "*hdr" parameter in rtl8xxxu module
when I am trying to fix some bugs for the rtl8192eu wifi dongle. This
parameter can be removed and then gained from the skb object to make the
function more beautiful.
Signed-off-by: JunASAKA <[email protected]>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index ac641a56efb0..4c3d97e8e51f 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4767,9 +4767,10 @@ static u32 rtl8xxxu_80211_to_rtl_queue(u32 queue)
return rtlqueue;
}
-static u32 rtl8xxxu_queue_select(struct ieee80211_hdr *hdr, struct sk_buff *skb)
+static u32 rtl8xxxu_queue_select(struct sk_buff *skb)
{
u32 queue;
+ struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
if (ieee80211_is_mgmt(hdr->frame_control))
queue = TXDESC_QUEUE_MGNT;
@@ -5118,7 +5119,7 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
if (control && control->sta)
sta = control->sta;
- queue = rtl8xxxu_queue_select(hdr, skb);
+ queue = rtl8xxxu_queue_select(skb);
tx_desc = skb_push(skb, tx_desc_size);
--
2.38.1
On 29/11/2022 06:34, JunASAKA wrote:
> I noticed there is a superfluous "*hdr" parameter in rtl8xxxu module
> when I am trying to fix some bugs for the rtl8192eu wifi dongle. This
> parameter can be removed and then gained from the skb object to make the
> function more beautiful.
>
> Signed-off-by: JunASAKA <[email protected]>
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index ac641a56efb0..4c3d97e8e51f 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4767,9 +4767,10 @@ static u32 rtl8xxxu_80211_to_rtl_queue(u32 queue)
> return rtlqueue;
> }
>
> -static u32 rtl8xxxu_queue_select(struct ieee80211_hdr *hdr, struct sk_buff *skb)
> +static u32 rtl8xxxu_queue_select(struct sk_buff *skb)
> {
> u32 queue;
> + struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
>
> if (ieee80211_is_mgmt(hdr->frame_control))
> queue = TXDESC_QUEUE_MGNT;
> @@ -5118,7 +5119,7 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> if (control && control->sta)
> sta = control->sta;
>
> - queue = rtl8xxxu_queue_select(hdr, skb);
> + queue = rtl8xxxu_queue_select(skb);
>
> tx_desc = skb_push(skb, tx_desc_size);
>
See the recent discussion about this here:
https://lore.kernel.org/linux-wireless/[email protected]/
https://lore.kernel.org/linux-wireless/[email protected]/
Any luck with the bugs?
Hi Bitterblue Smith,
Thanks for your reply. I've seen the discussion.
As for the bugs of the module, my Tenda U1 wifi module which is using the rtl8192eu chip running into problems with the rtl8xxxu module, more information can be found here: https://bugzilla.kernel.org/show_bug.cgi?id=216746. I want to solve this problem but I haven't got enough experience upon it. I'll appreciate it if you could do me a favour on it. Thanks again.
Jun ASAKA.
Hi Bitterblue Smith,
I have seen the patch you've mentioned. Actually, when I was trying to address the rtl8192eu problem, I saw that patch and
considered it would tackle my problem, but it turns out that it doesn't work for me. And I found this rtl8xxxu_queue_select()
function which has a *hdr parameter that can be gained from skb since skb is indeed neccessary for this function to work.
What do you think of these? And please take a look of my problem above on your convenience, thanks a lot.
Thanks and Regards,
Jun ASAKA.
On 29/11/2022 16:32, JunASAKA wrote:
> Hi Bitterblue Smith,
>
> Thanks for your reply. I've seen the discussion.
> As for the bugs of the module, my Tenda U1 wifi module which is using the rtl8192eu chip running into problems with the rtl8xxxu module, more information can be found here: https://bugzilla.kernel.org/show_bug.cgi?id=216746. I want to solve this problem but I haven't got enough experience upon it. I'll appreciate it if you could do me a favour on it. Thanks again.
>
> Jun ASAKA.
My only idea is to compare all the code with the working driver.
I'm still busy with other things, though.
I think you're right and I am comparing those sorces. But the realtek official driver
is far different from the one in rtl8xxxu module. I think it's difficult for me to do it, but
I am trying my best.
Jun ASAKA.
Hello,
On Tue, Nov 29, 2022 at 11:22:12PM +0800, JunASAKA wrote:
> I think you're right and I am comparing those sorces. But the realtek official driver
> is far different from the one in rtl8xxxu module. I think it's difficult for me to do it, but
> I am trying my best.
Would you please preserve the original subject from the thread
of messages you are responding to instead of replacing it with a
useless "reply to Bitterblue Smith" which nobody knows what it's
about and which doesn't bring anything to the discussion since it's
written in your message who you're responding to ? It's the third
such message you send to the list, at first I tagged them as spam
until I saw some responses. Most readers will likely do the same,
and even by doing this the best you're doing is to train anti-spam
systems to learn these as valid messages (which they do not look
like).
Thanks in advance,
Willy
I see. Sorry for that.
Jun ASAKA.
On November 29, 2022 3:06:37 PM Bitterblue Smith <[email protected]>
wrote:
> On 29/11/2022 06:34, JunASAKA wrote:
>> I noticed there is a superfluous "*hdr" parameter in rtl8xxxu module
>> when I am trying to fix some bugs for the rtl8192eu wifi dongle. This
>> parameter can be removed and then gained from the skb object to make the
>> function more beautiful.
>>
>> Signed-off-by: JunASAKA <[email protected]>
>> ---
>> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index ac641a56efb0..4c3d97e8e51f 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -4767,9 +4767,10 @@ static u32 rtl8xxxu_80211_to_rtl_queue(u32 queue)
>> return rtlqueue;
>> }
>>
>> -static u32 rtl8xxxu_queue_select(struct ieee80211_hdr *hdr, struct sk_buff
>> *skb)
>> +static u32 rtl8xxxu_queue_select(struct sk_buff *skb)
>> {
>> u32 queue;
>> + struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
>>
>> if (ieee80211_is_mgmt(hdr->frame_control))
>> queue = TXDESC_QUEUE_MGNT;
>> @@ -5118,7 +5119,7 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>> if (control && control->sta)
>> sta = control->sta;
>>
>> - queue = rtl8xxxu_queue_select(hdr, skb);
>> + queue = rtl8xxxu_queue_select(skb);
>>
>> tx_desc = skb_push(skb, tx_desc_size);
>
> See the recent discussion about this here:
> https://lore.kernel.org/linux-wireless/[email protected]/
> https://lore.kernel.org/linux-wireless/[email protected]/
Not sure why I looked but I did. You may want to look at rtl8xxxu_tx()
which is the .tx callback that mac80211 uses and the first statement in
there is also assuming skb->data points to the 802.11 header.
Regards,
Arend
>
On December 6, 2022 6:59:36 PM Arend Van Spriel
<[email protected]> wrote:
> On November 29, 2022 3:06:37 PM Bitterblue Smith <[email protected]>
> wrote:
>
>> On 29/11/2022 06:34, JunASAKA wrote:
>>> I noticed there is a superfluous "*hdr" parameter in rtl8xxxu module
>>> when I am trying to fix some bugs for the rtl8192eu wifi dongle. This
>>> parameter can be removed and then gained from the skb object to make the
>>> function more beautiful.
>>>
>>> Signed-off-by: JunASAKA <[email protected]>
>>> ---
>>> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> index ac641a56efb0..4c3d97e8e51f 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> @@ -4767,9 +4767,10 @@ static u32 rtl8xxxu_80211_to_rtl_queue(u32 queue)
>>> return rtlqueue;
>>> }
>>>
>>> -static u32 rtl8xxxu_queue_select(struct ieee80211_hdr *hdr, struct sk_buff
>>> *skb)
>>> +static u32 rtl8xxxu_queue_select(struct sk_buff *skb)
>>> {
>>> u32 queue;
>>> + struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
>>>
>>> if (ieee80211_is_mgmt(hdr->frame_control))
>>> queue = TXDESC_QUEUE_MGNT;
>>> @@ -5118,7 +5119,7 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>> if (control && control->sta)
>>> sta = control->sta;
>>>
>>> - queue = rtl8xxxu_queue_select(hdr, skb);
>>> + queue = rtl8xxxu_queue_select(skb);
>>>
>>> tx_desc = skb_push(skb, tx_desc_size);
>>
>> See the recent discussion about this here:
>> https://lore.kernel.org/linux-wireless/[email protected]/
>> https://lore.kernel.org/linux-wireless/[email protected]/
>
> Not sure why I looked but I did. You may want to look at rtl8xxxu_tx()
> which is the .tx callback that mac80211 uses and the first statement in
> there is also assuming skb->data points to the 802.11 header.
Here the documentation of the .tx callback:
@tx: Handler that 802.11 module calls for each transmitted frame.
* skb contains the buffer *starting from the IEEE 802.11 header*.
* The low-level driver should send the frame out based on
* configuration in the TX control data. This handler should,
* preferably, never fail and stop queues appropriately.
* Must be atomic.
I don't see any pushes or pulls before the queue select so that would mean
mac80211 is not complying to the described behavior.
Regards,
Arend
>
> Regards,
> Arend
>>
On 06/12/2022 20:19, Arend Van Spriel wrote:
> On December 6, 2022 6:59:36 PM Arend Van Spriel <[email protected]> wrote:
>
>> On November 29, 2022 3:06:37 PM Bitterblue Smith <[email protected]>
>> wrote:
>>
>>> On 29/11/2022 06:34, JunASAKA wrote:
>>>> I noticed there is a superfluous "*hdr" parameter in rtl8xxxu module
>>>> when I am trying to fix some bugs for the rtl8192eu wifi dongle. This
>>>> parameter can be removed and then gained from the skb object to make the
>>>> function more beautiful.
>>>>
>>>> Signed-off-by: JunASAKA <[email protected]>
>>>> ---
>>>> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 +++--
>>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>>> index ac641a56efb0..4c3d97e8e51f 100644
>>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>>> @@ -4767,9 +4767,10 @@ static u32 rtl8xxxu_80211_to_rtl_queue(u32 queue)
>>>> return rtlqueue;
>>>> }
>>>>
>>>> -static u32 rtl8xxxu_queue_select(struct ieee80211_hdr *hdr, struct sk_buff
>>>> *skb)
>>>> +static u32 rtl8xxxu_queue_select(struct sk_buff *skb)
>>>> {
>>>> u32 queue;
>>>> + struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
>>>>
>>>> if (ieee80211_is_mgmt(hdr->frame_control))
>>>> queue = TXDESC_QUEUE_MGNT;
>>>> @@ -5118,7 +5119,7 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>>> if (control && control->sta)
>>>> sta = control->sta;
>>>>
>>>> - queue = rtl8xxxu_queue_select(hdr, skb);
>>>> + queue = rtl8xxxu_queue_select(skb);
>>>>
>>>> tx_desc = skb_push(skb, tx_desc_size);
>>>
>>> See the recent discussion about this here:
>>> https://lore.kernel.org/linux-wireless/[email protected]/
>>> https://lore.kernel.org/linux-wireless/[email protected]/
>>
>> Not sure why I looked but I did. You may want to look at rtl8xxxu_tx()
>> which is the .tx callback that mac80211 uses and the first statement in
>> there is also assuming skb->data points to the 802.11 header.
>
> Here the documentation of the .tx callback:
>
> @tx: Handler that 802.11 module calls for each transmitted frame.
> * skb contains the buffer *starting from the IEEE 802.11 header*.
> * The low-level driver should send the frame out based on
> * configuration in the TX control data. This handler should,
> * preferably, never fail and stop queues appropriately.
> * Must be atomic.
>
> I don't see any pushes or pulls before the queue select so that would mean mac80211 is not complying to the described behavior.
>
> Regards,
> Arend
>
>>
>> Regards,
>> Arend
>>>
>
>
>
mac80211 is behaving as described in the documentation, as far as I know.
Technically, rtl8xxxu_queue_select's hdr parameter is not needed.