| Message ID | 20221129043442.14717-1-JunASAKA@zzy040330.moe (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | drivers: rewrite and remove a superfluous parameter. | expand |
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/ 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 <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 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 >>
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.
On December 6, 2022 7:40:43 PM Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: > 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. Okay. Then I probably misunderstood the gist of the discussion you referred to. Now I see the issue got fixed by your patch (1st URL). So sorry for the noise. Regards, Arend
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(-)