[PATCH] VIOC: New Network Device Driver

[PATCH] VIOC: New Network Device Driver

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



Misha Tomushev

Re: [PATCH] VIOC: New Network Device Driver

[Jan-Benedict Glaw]

[-- Attachment #1: Type: text/plain, Size: 591 bytes --]

On Thu, 2006-09-14 17:15:21 -0700, Misha Tomushev <misha@fabric7.com> 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      jbglaw@lug-owl.de              +49-172-7608481
 Signature of:                    Don't believe in miracles: Rely on them!
 the second  :

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] VIOC: New Network Device Driver

[Arnd Bergmann]

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

Re: [PATCH] VIOC: New Network Device Driver

[Francois Romieu]

Arnd Bergmann <arnd@arndb.de> :
[...]
> 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

RE: [PATCH] VIOC: New Network Device Driver

[Misha Tomushev]

-----Original Message-----
From: Arnd Bergmann [mailto:arnd@arndb.de] 
Sent: Sunday, September 10, 2006 2:39 PM
To: Misha Tomushev
Cc: jgarzik@pobox.com; netdev@vger.kernel.org;
linux-kernel@vger.kernel.org
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? 

Re: [PATCH] VIOC: New Network Device Driver

[Arnd Bergmann]

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