netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Kok, Auke" <auke-jan.h.kok@intel.com>
To: Jeff Garzik <jeff@garzik.org>
Cc: NetDev <netdev@vger.kernel.org>,
	Arjan van de Ven <arjan@linux.intel.com>,
	"Ronciak, John" <john.ronciak@intel.com>,
	Mitch Williams <mitch.a.williams@intel.com>
Subject: Re: ANNOUNCE: igb: Intel 82575 Gigabit Ethernet driver (PCI-Express)
Date: Thu, 19 Jul 2007 16:52:02 -0700	[thread overview]
Message-ID: <469FF922.2090900@intel.com> (raw)
In-Reply-To: <469ED3D7.6040905@garzik.org>

Jeff Garzik wrote:
> Kok, Auke wrote:
>>      http://foo-projects.org/~sofar/igb.patch       [558K]
>>      http://foo-projects.org/~sofar/igb.patch.bz2   [98K]
> 
> Just took a look at this.
> 
> This has the same problem as in the other thread -- huge internal API -- 
> except this time, the problem is emphasized by the fact that the 
> majority of the API hooks only have a single user, making each hook and 
> API entry point demonstrably useless overhead.
> 
> Please remove the useless internal API and resubmit.

Why don't you accept it now and allow us the time to work on this in the coming 
period? The driver works, performs better than all 8257x hardware and uses less 
CPU utilization. That must be good for everyone. Keeping it outside of the linux 
tree is just going to postpone testing the non-internal API parts.

> PLEASE take a look at how bnx2 and tg3 are structured.

They don't use PHYlib. Nobody is perfect...

We can and we should be able to _not_ agree on certain things and live together 
just fine. While I agree that some designs are better, the current internal API 
design has worked really good for us and allowed us to develop this driver and 
ixgbe in much faster rate than ever before. It was most useful in silicon 
validation and extremely flexible for us. At one point, the 82575 silicon was 
even supporter by the e1000 driver, because this internal API made it so easy to 
add new hardware support. For obvious reasons (differences in hw) we decided to 
split this driver off.

It served a good purpose. We can improve it, and I sure want to do that, but I 
think it's better to improve something *upstream*, then going back working at 
things offline without having any certainty that you will accept it after I get 
back, since you might just reject it on another argument, etc. It will take 
quite some effort and convincing to make this happen, and it would help a lot if 
we can work with/in the community on that, instead of off-line.

That will also give the non-internal API parts the better testing.


Auke

  reply	other threads:[~2007-07-19 23:52 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-13 23:29 ANNOUNCE: igb: Intel 82575 Gigabit Ethernet driver (PCI-Express) Kok, Auke
2007-07-19  3:00 ` Jeff Garzik
2007-07-19 23:52   ` Kok, Auke [this message]
2007-07-20  7:33     ` Christoph Hellwig
2007-07-20  8:23       ` Stefan Rompf
2007-07-20 11:00     ` Jeff Garzik

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=469FF922.2090900@intel.com \
    --to=auke-jan.h.kok@intel.com \
    --cc=arjan@linux.intel.com \
    --cc=jeff@garzik.org \
    --cc=john.ronciak@intel.com \
    --cc=mitch.a.williams@intel.com \
    --cc=netdev@vger.kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).