2006-09-10 10:40:37

by Misha Tomushev

[permalink] [raw]
Subject: [PATCH] VIOC: New Network Device Driver

VIOC Device Driver provides a standard device interface to the internal
fabric interconnected network used on servers designed and built by
Fabric 7 Systems.

The patch can be found at ftp.fabric7.com/VIOC.



Misha Tomushev


2006-09-10 10:50:27

by Jan-Benedict Glaw

[permalink] [raw]
Subject: Re: [PATCH] VIOC: New Network Device Driver

On Thu, 2006-09-14 17:15:21 -0700, Misha Tomushev <[email protected]> wrote:
> VIOC Device Driver provides a standard device interface to the internal
> fabric interconnected network used on servers designed and built by
> Fabric 7 Systems.
>
> The patch can be found at ftp.fabric7.com/VIOC.

To get the driver into upstream kernel sources, you'd post it in
reviewable pieces to the list.

Thanks, JBG

--
Jan-Benedict Glaw [email protected] +49-172-7608481
Signature of: Don't believe in miracles: Rely on them!
the second :


Attachments:
(No filename) (575.00 B)
signature.asc (189.00 B)
Digital signature
Download all attachments

2006-09-10 21:40:05

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] VIOC: New Network Device Driver

Am Friday 15 September 2006 02:15 schrieb Misha Tomushev:
> VIOC Device Driver provides a standard device interface to the internal
> fabric interconnected network used on servers designed and built by
> Fabric 7 Systems.
>
> The patch can be found at ftp.fabric7.com/VIOC.

We recently had a discussion about tx descriptor cleanup in general.
It would probably be more efficient to call vnic_clean_txq from the
vioc_rx_poll() function. To do that, your tx interrupt handler
should disable the tx interrupt line and call netif_rx_schedule,
like you do for the receive interrupts.

A few comments on coding style:

- Lots of macros like your GET_VNIC_TX_BUFADDR_LO: they seem overly
complicated. Maybe replace the users with something simpler, e.g.
instead of 'if (GET_VNIC_RXC_FLAGGED(rxcd) != VNIC_RXC_FLAGGED_HW_W)',
do 'if (vnic_rxc_word3(rxcd) & VNIC_RXC_FLAGGED_HW_W)'.
- whitespace: please follow the style in Documentation/CodingStyle,
use tabs for indentation instead of spaces, run everything through
'lindent' or 'indent -kr -i8' once to get spaces in the right places.
- unnecessary typecasts: try to avoid casts in the C source, in particular
from or to 'void *', that is done by C automatically. When you do a
macro like GETRELADDR(), make it return the right type so you don't need
a cast.
- macros: whereever possible, use an inline function instead
- printk: use dev_info/dev_dbg/... instead of plain printk, when you have
a pointer to a device.
- extern declarations: belong into header files, not C files. This will
guarantee that the definition matches the declaration.
- static forward declarations: get rid of them by moving the static
functions into the right order. This also makes reading easier, since you
know static functions are only called from below.
- vmalloc: try to avoid. use it only when allocating more than a few pages.

Arnd <><

2006-09-10 22:24:53

by Francois Romieu

[permalink] [raw]
Subject: Re: [PATCH] VIOC: New Network Device Driver

Arnd Bergmann <[email protected]> :
[...]
> A few comments on coding style:

Add:
- use netdev_priv()
- use DMA_{32/64}_BIT_MASK in place of private #define
- turn some define into enum ?

--
Ueimor

2006-09-11 17:58:25

by Misha Tomushev

[permalink] [raw]
Subject: RE: [PATCH] VIOC: New Network Device Driver



-----Original Message-----
From: Arnd Bergmann [mailto:[email protected]]
Sent: Sunday, September 10, 2006 2:39 PM
To: Misha Tomushev
Cc: [email protected]; [email protected];
[email protected]
Subject: Re: [PATCH] VIOC: New Network Device Driver

Am Friday 15 September 2006 02:15 schrieb Misha Tomushev:
> VIOC Device Driver provides a standard device interface to the
internal
> fabric interconnected network used on servers designed and built by
> Fabric 7 Systems.
>
> The patch can be found at ftp.fabric7.com/VIOC.

We recently had a discussion about tx descriptor cleanup in general.
It would probably be more efficient to call vnic_clean_txq from the
vioc_rx_poll() function. To do that, your tx interrupt handler
should disable the tx interrupt line and call netif_rx_schedule,
like you do for the receive interrupts.

The descriptor clean-up does not contribute anything to the performance
of the driver, it just replenishes the memory pools. It almost does not
need interrupts. Why would we want to add more cycles to the receive
logic, when driver is doing useful work for something that can run
almost at any time?

2006-09-11 19:16:01

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] VIOC: New Network Device Driver

On Monday 11 September 2006 19:58, Misha Tomushev wrote:
> The descriptor clean-up does not contribute anything to the performance
> of the driver, it just replenishes the memory pools. It almost does not
> need interrupts. Why would we want to add more cycles to the receive
> logic, when driver is doing useful work for something that can run
> almost at any time?

It can run at almost any time, just not in the interrupt context,
where it needs to schedule the tx softirq first.

Also, the number of tx interrupts you get is a tradeoff between
causing overhead of calling the interrupt handler and stalling
sockets that are waiting for space to become free after transmission.
By using ->poll for it, you can avoid tx interrupts completely
most of the time and free skbs immediately when data comes in.

Arnd <><