DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8F56D2082D
MIME-Version: 1.0
References: <1540808552-28738-1-git-send-email-wright.feng@cypress.com> <1540808552-28738-2-git-send-email-wright.feng@cypress.com>
In-Reply-To: <1540808552-28738-2-git-send-email-wright.feng@cypress.com>
From:   Franky Lin <franky.lin@broadcom.com>
Date:   Mon, 29 Oct 2018 11:50:05 -0700
Message-ID: <CA+8PC_f-EMCWnn1oBorAZjNY2xJXcJbhbP+8fU9wYJXnnZto5w@mail.gmail.com>
Subject: Re: [PATCH 1/3] brcmfmac: calling skb_orphan before sending skb to
 SDIO bus
To:     Wright Feng <Wright.Feng@cypress.com>
Cc:     "open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER" 
        <linux-wireless@vger.kernel.org>,
        Arend Van Spriel <arend.vanspriel@broadcom.com>,
        Hante Meuleman <hante.meuleman@broadcom.com>,
        Kalle Valo <kvalo@codeaurora.org>,
        Chi-Hsien Lin <Chi-Hsien.Lin@cypress.com>,
        brcm80211-dev-list.pdl@broadcom.com
Content-Type: text/plain; charset="UTF-8"
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

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
>