| Message ID | 20221129043442.14717-1-JunASAKA@zzy040330.moe |
|---|---|
| State | New |
| Headers | show |
| Series | drivers: rewrite and remove a superfluous parameter. | expand |
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.
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.
I see. Sorry for that. Jun ASAKA.
On November 29, 2022 3:06:37 PM Bitterblue Smith <rtl8821cerfe2@gmail.com> 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 <JunASAKA@zzy040330.moe> >> --- >> 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/acd30174-4541-7343-e49a-badd199f4151@gmail.com/ > https://lore.kernel.org/linux-wireless/2af44c28-1c12-46b9-85b9-011560bf7f7e@gmail.com/ 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 06/12/2022 20:19, Arend Van Spriel wrote: > On December 6, 2022 6:59:36 PM Arend Van Spriel <arend.vanspriel@broadcom.com> wrote: > >> On November 29, 2022 3:06:37 PM Bitterblue Smith <rtl8821cerfe2@gmail.com> >> 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 <JunASAKA@zzy040330.moe> >>>> --- >>>> 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/acd30174-4541-7343-e49a-badd199f4151@gmail.com/ >>> https://lore.kernel.org/linux-wireless/2af44c28-1c12-46b9-85b9-011560bf7f7e@gmail.com/ >> >> 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.
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);
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 <JunASAKA@zzy040330.moe> --- drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)