| Message ID | 1540808552-28738-2-git-send-email-wright.feng@cypress.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | brcmfmac: throughput enhancement for SDIO and flow control mode | expand |
On Mon, Oct 29, 2018 at 3:27 AM Wright Feng <Wright.Feng@cypress.com> wrote: > > Linux 3.6 introduces TSQ which has a per socket threshold for TCP Tx > packets to reduce latency. In fcmode 1 and fcmode 2, host driver enqueues > skb in hanger and TCP doesn't push new skb until host frees the skb when > receiving fwstatus event. So using skb_orphan before sending skb to bus > will make the skb removing the ownership of socket. With this patch, we > are able to get better throughput in fcmode 1 and fcmode 2. > > Tested 43455 TCP throughput in 20 MHz bandwidth with / without this patch. > fcmode 0: 59.5 / 59.6 (mbps) > fcmode 1: 59.3 / 23.4 (mbps) > fcmode 2: 59.6 / 21.5 (mbps) > > Signed-off-by: Wright Feng <wright.feng@cypress.com> > --- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c > index b2e1ab5..519b25d 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c > @@ -2298,6 +2298,7 @@ static uint brcmf_sdio_sendfromq(struct brcmf_sdio *bus, uint maxframes) > &prec_out); > if (pkt == NULL) > break; > + skb_orphan(pkt); TSQ allows device driver to tweak for a deeper queue now. [1]. We should use that instead of orphaning the packet before handing over to firmware which would remove the bufferbloat protection by TSQ. Thanks, -Franky [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3a9b76fd0db9f > __skb_queue_tail(&pktq, pkt); > } > spin_unlock_bh(&bus->txq_lock); > -- > 1.9.1 >
On 2018/10/30 上午 02:50, Franky Lin wrote: > On Mon, Oct 29, 2018 at 3:27 AM Wright Feng <Wright.Feng@cypress.com> wrote: >> >> Linux 3.6 introduces TSQ which has a per socket threshold for TCP Tx >> packets to reduce latency. In fcmode 1 and fcmode 2, host driver enqueues >> skb in hanger and TCP doesn't push new skb until host frees the skb when >> receiving fwstatus event. So using skb_orphan before sending skb to bus >> will make the skb removing the ownership of socket. With this patch, we >> are able to get better throughput in fcmode 1 and fcmode 2. >> >> Tested 43455 TCP throughput in 20 MHz bandwidth with / without this patch. >> fcmode 0: 59.5 / 59.6 (mbps) >> fcmode 1: 59.3 / 23.4 (mbps) >> fcmode 2: 59.6 / 21.5 (mbps) >> >> Signed-off-by: Wright Feng <wright.feng@cypress.com> >> --- >> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c >> index b2e1ab5..519b25d 100644 >> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c >> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c >> @@ -2298,6 +2298,7 @@ static uint brcmf_sdio_sendfromq(struct brcmf_sdio *bus, uint maxframes) >> &prec_out); >> if (pkt == NULL) >> break; >> + skb_orphan(pkt); > > TSQ allows device driver to tweak for a deeper queue now. [1]. We > should use that instead of orphaning the packet before handing over to > firmware which would remove the bufferbloat protection by TSQ. > > Thanks, > -Franky > Since the aggregation problem has been fixed by setting sk_pacing_shift, I will submit v2 without this one. Thanks for the information. -Wright > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3a9b76fd0db9f > >> __skb_queue_tail(&pktq, pkt); >> } >> spin_unlock_bh(&bus->txq_lock); >> -- >> 1.9.1 >>
On Thu, Nov 1, 2018 at 8:08 PM Wright Feng <Wright.Feng@cypress.com> wrote: > > > > On 2018/10/30 上午 02:50, Franky Lin wrote: > > On Mon, Oct 29, 2018 at 3:27 AM Wright Feng <Wright.Feng@cypress.com> wrote: > >> > >> Linux 3.6 introduces TSQ which has a per socket threshold for TCP Tx > >> packets to reduce latency. In fcmode 1 and fcmode 2, host driver enqueues > >> skb in hanger and TCP doesn't push new skb until host frees the skb when > >> receiving fwstatus event. So using skb_orphan before sending skb to bus > >> will make the skb removing the ownership of socket. With this patch, we > >> are able to get better throughput in fcmode 1 and fcmode 2. > >> > >> Tested 43455 TCP throughput in 20 MHz bandwidth with / without this patch. > >> fcmode 0: 59.5 / 59.6 (mbps) > >> fcmode 1: 59.3 / 23.4 (mbps) > >> fcmode 2: 59.6 / 21.5 (mbps) > >> > >> Signed-off-by: Wright Feng <wright.feng@cypress.com> > >> --- > >> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c > >> index b2e1ab5..519b25d 100644 > >> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c > >> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c > >> @@ -2298,6 +2298,7 @@ static uint brcmf_sdio_sendfromq(struct brcmf_sdio *bus, uint maxframes) > >> &prec_out); > >> if (pkt == NULL) > >> break; > >> + skb_orphan(pkt); > > > > TSQ allows device driver to tweak for a deeper queue now. [1]. We > > should use that instead of orphaning the packet before handing over to > > firmware which would remove the bufferbloat protection by TSQ. > > > > Thanks, > > -Franky > > > Since the aggregation problem has been fixed by setting sk_pacing_shift, > I will submit v2 without this one. > Thanks for the information. No the problem is not fixed. The patch just provides an interface sk_pacing_shift_update so the device driver can tweak the scale to allow more packets being queued. You need to call sk_pacing_shift_update somewhere in brcmfmac. Thanks, -Franky > > -Wright > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3a9b76fd0db9f > > > >> __skb_queue_tail(&pktq, pkt); > >> } > >> spin_unlock_bh(&bus->txq_lock); > >> -- > >> 1.9.1 > >>
Franky Lin 於 11/3/2018 3:51 AM 寫道: > On Thu, Nov 1, 2018 at 8:08 PM Wright Feng <Wright.Feng@cypress.com> wrote: >> >> >> >> On 2018/10/30 上午 02:50, Franky Lin wrote: >>> On Mon, Oct 29, 2018 at 3:27 AM Wright Feng <Wright.Feng@cypress.com> wrote: >>>> >>>> Linux 3.6 introduces TSQ which has a per socket threshold for TCP Tx >>>> packets to reduce latency. In fcmode 1 and fcmode 2, host driver enqueues >>>> skb in hanger and TCP doesn't push new skb until host frees the skb when >>>> receiving fwstatus event. So using skb_orphan before sending skb to bus >>>> will make the skb removing the ownership of socket. With this patch, we >>>> are able to get better throughput in fcmode 1 and fcmode 2. >>>> >>>> Tested 43455 TCP throughput in 20 MHz bandwidth with / without this patch. >>>> fcmode 0: 59.5 / 59.6 (mbps) >>>> fcmode 1: 59.3 / 23.4 (mbps) >>>> fcmode 2: 59.6 / 21.5 (mbps) >>>> >>>> Signed-off-by: Wright Feng <wright.feng@cypress.com> >>>> --- >>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c >>>> index b2e1ab5..519b25d 100644 >>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c >>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c >>>> @@ -2298,6 +2298,7 @@ static uint brcmf_sdio_sendfromq(struct brcmf_sdio *bus, uint maxframes) >>>> &prec_out); >>>> if (pkt == NULL) >>>> break; >>>> + skb_orphan(pkt); >>> >>> TSQ allows device driver to tweak for a deeper queue now. [1]. We >>> should use that instead of orphaning the packet before handing over to >>> firmware which would remove the bufferbloat protection by TSQ. >>> >>> Thanks, >>> -Franky >>> >> Since the aggregation problem has been fixed by setting sk_pacing_shift, >> I will submit v2 without this one. >> Thanks for the information. > > No the problem is not fixed. The patch just provides an interface > sk_pacing_shift_update so the device driver can tweak the scale to > allow more packets being queued. You need to call > sk_pacing_shift_update somewhere in brcmfmac. > > Thanks, > -Franky We are still implementing the function of setting sk_pacing_shift in brcmfmac, so the patch will be submitted in another patch series once we have done the fully tests. -Wright >> >> -Wright >>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3a9b76fd0db9f >>> >>>> __skb_queue_tail(&pktq, pkt); >>>> } >>>> spin_unlock_bh(&bus->txq_lock); >>>> -- >>>> 1.9.1 >>>>
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index b2e1ab5..519b25d 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -2298,6 +2298,7 @@ static uint brcmf_sdio_sendfromq(struct brcmf_sdio *bus, uint maxframes) &prec_out); if (pkt == NULL) break; + skb_orphan(pkt); __skb_queue_tail(&pktq, pkt); } spin_unlock_bh(&bus->txq_lock);
Linux 3.6 introduces TSQ which has a per socket threshold for TCP Tx packets to reduce latency. In fcmode 1 and fcmode 2, host driver enqueues skb in hanger and TCP doesn't push new skb until host frees the skb when receiving fwstatus event. So using skb_orphan before sending skb to bus will make the skb removing the ownership of socket. With this patch, we are able to get better throughput in fcmode 1 and fcmode 2. Tested 43455 TCP throughput in 20 MHz bandwidth with / without this patch. fcmode 0: 59.5 / 59.6 (mbps) fcmode 1: 59.3 / 23.4 (mbps) fcmode 2: 59.6 / 21.5 (mbps) Signed-off-by: Wright Feng <wright.feng@cypress.com> --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 1 + 1 file changed, 1 insertion(+)