> Hi,
>
> On Wed, Mar 13, 2019 at 12:13:52PM +0800, yhchuang@xxxxxxxxxxx wrote:
> > diff --git a/drivers/net/wireless/realtek/rtw88/pci.c
> b/drivers/net/wireless/realtek/rtw88/pci.c
> > new file mode 100644
> > index 0000000..cf3bffb
> > --- /dev/null
> > +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> > @@ -0,0 +1,1211 @@
> ...
> > +static u8 ac_to_hwq[] = {
> > + [0] = RTW_TX_QUEUE_VO,
> > + [1] = RTW_TX_QUEUE_VI,
> > + [2] = RTW_TX_QUEUE_BE,
> > + [3] = RTW_TX_QUEUE_BK,
> > +};
>
> I don't want to dig up and repeat the details of my comments, but I
> think you missed my earlier suggestion here: you should use the
> ieee80211_ac_numbers enum instead of bare 0-3 indexing.
Fixed, thanks. Will be in the next patches.
>
> > +static u8 rtw_hw_queue_mapping(struct sk_buff *skb)
> > +{
> > + struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> > + __le16 fc = hdr->frame_control;
> > + u8 q_mapping = skb_get_queue_mapping(skb);
> > + u8 queue;
> > +
> > + if (unlikely(ieee80211_is_beacon(fc)))
> > + queue = RTW_TX_QUEUE_BCN;
> > + else if (unlikely(ieee80211_is_mgmt(fc) || ieee80211_is_ctl(fc)))
> > + queue = RTW_TX_QUEUE_MGMT;
> > + else
> > + queue = ac_to_hwq[q_mapping];
>
> It also seems wise to be defensive about the values of 'q_mapping', so
> we don't accidentally overstep the array if for some reason the
> implementation details of mac80211 change some day. e.g.:
>
> else if (WARN_ON_ONCE(q_mapping >= ARRAY_SIZE(ac_to_hwq)))
> // Pick a default.
Yeah, should check if q_mapping does not fit into the array.
Thanks.
>
> Brian
>
> > +
> > + return queue;
> > +}
> > +
>
Yan-Hsuan
