ANNOUNCE: igb: Intel 82575 Gigabit Ethernet driver (PCI-Express)

ANNOUNCE: igb: Intel 82575 Gigabit Ethernet driver (PCI-Express)

[Kok, Auke]

All,

We are pleased to announce a new Gigabit Ethernet product and its driver to the
linux community. This product is the Intel(R) 82575 Gigabit Ethernet adapter
family. Physical adapters will be available to the public soon. These adapters
come in 2- and 4-port versions (copper PHY) currently. Other variants will be
available later.

The 82575 chipset supports significantly different features that warrant a new
driver. The descriptor format is (just like the ixgbe driver) different. The
device can use multiple MSI-X vectors and multiple queues for both send and
receive. This allows us to optimize some of the driver code specifically as well
compared to the e1000-supported devices.

This driver was forked from e1000 several months ago and extensively reworked
and cleaned up since. The driver was also tested on several platforms in our
validation labs.

Allthough some of the codebase is currently shared with the e1000 driver (this
igb driver has a copy of that code where needed), we realize that many of the
changes that we are discussing for e1000 (the pci-express adapters that e1000
supports particularly) will also apply to this driver. However, since this is a
completely new driver that is relatively free of all old NIC support, we feel
that it is currently the right time to post this driver.

Unfortunately, the patch to insert this driver is too large to send to netdev. I
have therefore posted the patch on http:

      http://foo-projects.org/~sofar/igb.patch       [558K]
      http://foo-projects.org/~sofar/igb.patch.bz2   [98K]

And also put up the patch in a git tree for those who want to pull:

      git-pull git://lost.foo-projects.org/~ahkok/git/linux-2.6 igb

Please review and provide comments!

Current issues:

There are several discussions ongoing about multiqueue and NAPI that also apply
to this driver, so several TODO items already exist (remove fake netdevs for
instance). As we get to these issues with the new e1000e and ixgbe driver, we
will try to address them in this igb driver as well, but feel free to raise them
early and now as well.


Thanks,

Auke Kok

Re: ANNOUNCE: igb: Intel 82575 Gigabit Ethernet driver (PCI-Express)

[Jeff Garzik]

Kok, Auke wrote:
> 
> 
> All,
> 
> We are pleased to announce a new Gigabit Ethernet product and its driver 
> to the
> linux community. This product is the Intel(R) 82575 Gigabit Ethernet 
> adapter
> family. Physical adapters will be available to the public soon. These 
> adapters
> come in 2- and 4-port versions (copper PHY) currently. Other variants 
> will be
> available later.
> 
> The 82575 chipset supports significantly different features that warrant 
> a new
> driver. The descriptor format is (just like the ixgbe driver) different. 
> The
> device can use multiple MSI-X vectors and multiple queues for both send and
> receive. This allows us to optimize some of the driver code specifically 
> as well
> compared to the e1000-supported devices.
> 
> This driver was forked from e1000 several months ago and extensively 
> reworked
> and cleaned up since. The driver was also tested on several platforms in 
> our
> validation labs.
> 
> Allthough some of the codebase is currently shared with the e1000 driver 
> (this
> igb driver has a copy of that code where needed), we realize that many 
> of the
> changes that we are discussing for e1000 (the pci-express adapters that 
> e1000
> supports particularly) will also apply to this driver. However, since 
> this is a
> completely new driver that is relatively free of all old NIC support, we 
> feel
> that it is currently the right time to post this driver.
> 
> Unfortunately, the patch to insert this driver is too large to send to 
> netdev. I
> have therefore posted the patch on http:
> 
>      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.

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

	Jeff

Re: ANNOUNCE: igb: Intel 82575 Gigabit Ethernet driver (PCI-Express)

[Kok, Auke]

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

Re: ANNOUNCE: igb: Intel 82575 Gigabit Ethernet driver (PCI-Express)

[Christoph Hellwig]

On Thu, Jul 19, 2007 at 04:52:02PM -0700, Kok, Auke wrote:
> 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.

Well, these useless abstractions are really annoying and nothing we allow
into other drivers either.  So I'd be miuch happier if you stripped them
out quickly.  It's not like it's a whole lot of work.

Re: ANNOUNCE: igb: Intel 82575 Gigabit Ethernet driver (PCI-Express)

[Stefan Rompf]

Am Freitag, 20. Juli 2007 09:33 schrieben Sie:

> Well, these useless abstractions are really annoying and nothing we allow
> into other drivers either.

However, they make it easier for multiple developers to work as a team. This 
is useful for development pace and makes maintainance less dependant on one 
person.

Stefan

Re: ANNOUNCE: igb: Intel 82575 Gigabit Ethernet driver (PCI-Express)

[Jeff Garzik]

Kok, Auke wrote:
> 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.


I do not know how to say it more clearly than it has been said for weeks:

	do-anything internal API is a merge stopper.

_Especially_ in igb's case, more than others, because it is obviously a 
one-user-per-hook no-op API.  Pure overhead.  100% real pork products.

Since you praise it in the following paragraphs, that demonstrates 
previous little incentive to remove it, if I merge this driver as-is.


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

This is the same argument we get from every vendor who wants to keep 
their lovely cross-OS modularization/etc. API.

igb has been submitted in OBVIOUSLY BLOATED state.  I could care less 
how useful it was in silicon validation, if this is the result:  an 
implement-any-MAC-or-PHY-in-the-world internal API where every hook... 
has precisely ONE user, thus obviating the need for the hook and API at 
all.  Simple mathematical reduction.


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

Let us review the e1000new major merge objections (posted July 9, msgid 
<469280F5.8000909@garzik.org>):

	1) Transition plan (irrelevant to this igb thread)
	2) Internal do-anything API
	3) Lack of Intel attention to on-going driver maintenance and
	   cleanups, rather than strict focus bolting on new features
	   or fixes.

Intel is just wasting everybody's time by coming back with a new driver 
that will spark the same objection (#2) as with the previous driver. 
You then ask the Linux community to TRUST you that you will clean up a 
driver, running counter to experience associated with objection #3.

It needs to look like a lean and mean Linux driver, not a driver with 
CLEAR bloat left over from silicon validation.  Your customers -- my 
users -- deserve much better.

Here is the key:  I don't expect perfection (much to Christoph's chagrin 
:)) in a driver submission.  I want to see three key things in a new driver:

	1) it's clean

	2) it is a good foundation for the future

	3) stable, reliable, active maintainership process exists

Arjan says #3 is in place, so we will take that on faith.  It's mostly 
clean (#1), but it's mainly problem is that it's overengineered.   This 
igb driver violates one of Linux's key principles:  "do what you must, 
and no more."  That internal API, demonstrably a no-op in igb, is not a 
good foundation for the future (#2).


With regards to "certainty"...  you should know that there are never any 
promises.  I have to look at each incarnation with fresh eyes.  ASSUMING 
the driver is moving in the right direction (i.e. author accepts 
feedback), each re-review should be more rapid.  But it is key to 
understand that often initial comments and initial revisions are 
required simply to make the driver reviewable.  This is the natural 
process that every driver goes through.

But if you want certainty, I would be more than happy to take igb as-is, 
merge it into netdev-2.6.git, clean it up myself to illustrate the 
proper direction, and then push it upstream.  That would get a modified 
igb upstream sooner rather than later.  Would that be preferred?  I 
could knock that out this weekend.  Or I am happy to mentor.  Whatever 
it takes.

Whoever does it, this needs a lot of simple algabraic/logical reductions.

It needs to look like, to be, a Linux driver first and foremost.  Linux 
users should not suffer needless bloat because of the happenstance of 
the internal silicon validation process.

	Jeff