netdev - Re: [PATCH v2 net-next 3/9] tg3: Reintroduce 5717

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110406173325.GA14870@mcarlson.broadcom.com>
Date:	Wed, 6 Apr 2011 10:33:26 -0700
From:	"Matt Carlson" <mcarlson@...adcom.com>
To:	"Joe Perches" <joe@...ches.com>
cc:	"Matthew Carlson" <mcarlson@...adcom.com>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 net-next 3/9] tg3: Reintroduce 5717_PLUS

On Tue, Apr 05, 2011 at 10:30:53PM -0700, Joe Perches wrote:
> On Tue, 2011-04-05 at 17:22 -0700, Matt Carlson wrote:
> > This patch reintroduces the TG3_FLG3_5717_PLUS to identify 5717 and
> > later devices.
> []
> > @@ -13427,8 +13422,7 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
> []
> > +	if (tp->tg3_flags3 & TG3_FLG3_5717_PLUS)
> >  		tp->tg3_flags3 |= TG3_FLG3_LRG_PROD_RING_CAP;
> 
> This seems redundant.  Maybe consolidate to just
> TG3_FLG3_5717_PLUS and remove LRG_PROD_RING_CAP?
> 
> I don't know if these attributes really are linked.
> 
> Another option may be to use DECLARE_BITMAP
> and set_bit/test_bit so there's no real need
> to use FLAG/FLG2/FLG3, etc.

A while back, I submitted a patch that extended the rx ring sizes for
those devices that were capable.  Dave Miller commented that he didn't
think the additional buffering was benefitial and in fact had a
downside.  (Larger ring sizes will result in more inefficient cache
usage.)

To fix the problem, the driver will need to be able to easily identify
which devices support the larger ring sizes.  This patch represents a
step in that direction.  Follow-on patches will make more use of the
flag, which should justify its existence.

For the record though, I did consider using the TG3_FLG3_5717_PLUS flag.
I'm uncomfortable with using it here because this is more of a server
class device feature.  Should another mobile or desktop device be
created in the future, I'd have to devise this type of flag to identify
what feature I'm really talking about.  I thought it prudent to just
take care of that now.

(Not that this is a compelling argument, but it also is a handy way to
enable and disable the feature while debugging.)

--
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