netdev - Re: [PATCH v3 0/8] net: ethernet: marvell: Assorted fixes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20140523200127.GA14290@arch.cereza>
Date:	Fri, 23 May 2014 17:01:27 -0300
From:	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>
To:	David Miller <davem@...emloft.net>
Cc:	netdev@...r.kernel.org, thomas.petazzoni@...e-electrons.com,
	sebastian.hesselbarth@...il.com,
	gregory.clement@...e-electrons.com, alior@...vell.com,
	tawfik@...vell.com, andrew@...n.ch
Subject: Re: [PATCH v3 0/8] net: ethernet: marvell: Assorted fixes

On 23 May 03:36 PM, David Miller wrote:
> From: Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>
> Date: Thu, 22 May 2014 20:06:55 -0300
> 
> > New round for this assorted fixes and clean-up series. There is more room for
> > clean-ups, and I'll start preparing more patches once these are accepted.
> > 
> > This series consists of cleanups and minor improvements on mvneta, mv643xx_eth
> > and mvmdio drivers. None of the patches imply any functionality change, except
> > for the patch six "Change the number of default rx queues to one".
> > 
> > This patch reduces the driver's allocated resources and makes the multiqueue
> > path in the poll function not get taken. The previous patchset contains more
> > details:
> > 
> >   http://permalink.gmane.org/gmane.linux.network/315015
> > 
> > As usual, any feedback on this will be well received!
> 
> Series applied to net-next, but some comments:
> 
> 1) Never use web sites to reference informative material, instead put it
>    all explicitly into the commit message in your posting.
> 
>    Web sites and archives come and go, the commit message is forever and
>    readily available to someone studying your changes.
> 

OK, I'll keep this in mind.

> 2) Just get rid of that MVNETA_RX_POLL_WEIGHT and simply use the
>    generic default NAPI_POLL_WEIGHT which has the same value.
> 
>    Any local NAPI weight selection must be done for a good reason and
>    be fully documented with performance metrics et al.
> 

OK.

> 3) I know you inherited this, but the MTU change failure behavior is
>    not so great.  If the system is low on memory it could fail and losing
>    an interface completely during an MTU change is extremely poor behavior.
> 
>    I would suggest holding onto the old allocated TXQ resources, and
>    restoring them if the allocation of the new ones fail.
> 
>    That way the MTU change itself would fail, but the interface would
>    remain up with the original MTU.
> 
>    Please make sure that you properly revert the MTU adjustment in
>    this failure path.
> 

Oh, I see. Thanks for the feedback, I'll take a look at this next week.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html