public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: devel@linuxdriverproject.org, haiyangz@microsoft.com,
	sthemmin@microsoft.com, netdev@vger.kernel.org
Subject: Re: [PATCH net 3/3] hv_netvsc: Fix the default receive buffer size
Date: Fri, 8 Dec 2017 13:44:59 +0300	[thread overview]
Message-ID: <20171208104459.wtwexebad4acbod3@mwanda> (raw)
In-Reply-To: <20171208001055.24670-4-sthemmin@microsoft.com>

On Thu, Dec 07, 2017 at 04:10:55PM -0800, Stephen Hemminger wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
> 
> The intended size is 16 MB, and the default slot size is 1728.
> So, NETVSC_DEFAULT_RX should be 16*1024*1024 / 1728 = 9709.
> 
> Fixes: 5023a6db73196 ("netvsc: increase default receive buffer size")
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
>  drivers/net/hyperv/netvsc_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index dc70de674ca9..edfcde5d3621 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -50,7 +50,7 @@
>  #define NETVSC_MIN_TX_SECTIONS	10
>  #define NETVSC_DEFAULT_TX	192	/* ~1M */
>  #define NETVSC_MIN_RX_SECTIONS	10	/* ~64K */
> -#define NETVSC_DEFAULT_RX	10485   /* Max ~16M */
> +#define NETVSC_DEFAULT_RX	9709    /* ~16M */

How does this bug look like to the user?  Memory corruption?

It's weird to me reviewing this code that the default sizes are
stored in netvsc_drv.c and the max sizes are stored in hyperv_net.h.
Could we move these to hyperv_net.h?  We could write it like:
#define NETVSC_DEFAULT_RX ((16 * 1024 * 1024) / NETVSC_RECV_SECTION_SIZE)

16MB is sort of a weird default because it's larger than the 15MB
allowed for legacy versions, but it's smaller than the 32MB you'd want
for the current versions.

regards,
dan carpenter

  reply	other threads:[~2017-12-08 10:44 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-08  0:10 [PATCH net 0/3] netvsc fix buffer size issues Stephen Hemminger
2017-12-08  0:10 ` [PATCH net 1/3] hv_netvsc: Correct the max receive buffer size Stephen Hemminger
2017-12-08 10:33   ` Dan Carpenter
2017-12-08 16:03     ` David Miller
2017-12-08 16:11     ` Haiyang Zhang
2017-12-08  0:10 ` [PATCH net 2/3] hv_netvsc: Limit the receive buffer size for legacy hosts Stephen Hemminger
2017-12-08  0:10 ` [PATCH net 3/3] hv_netvsc: Fix the default receive buffer size Stephen Hemminger
2017-12-08 10:44   ` Dan Carpenter [this message]
2017-12-08 16:18     ` Haiyang Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171208104459.wtwexebad4acbod3@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=devel@linuxdriverproject.org \
    --cc=haiyangz@microsoft.com \
    --cc=netdev@vger.kernel.org \
    --cc=stephen@networkplumber.org \
    --cc=sthemmin@microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox