LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Re: [PATCH] VIOC: New Network Device Driver
  2006-09-15  0:15 [PATCH] VIOC: New Network Device Driver Misha Tomushev
@ 2006-09-10 10:50 ` Jan-Benedict Glaw
  2006-09-10 21:39 ` Arnd Bergmann
  1 sibling, 0 replies; 6+ messages in thread
From: Jan-Benedict Glaw @ 2006-09-10 10:50 UTC (permalink / raw)
  To: Misha Tomushev; +Cc: jgarzik, netdev, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] VIOC: New Network Device Driver
  2006-09-15  0:15 [PATCH] VIOC: New Network Device Driver Misha Tomushev
  2006-09-10 10:50 ` Jan-Benedict Glaw
@ 2006-09-10 21:39 ` Arnd Bergmann
  2006-09-10 22:21   ` Francois Romieu
  2006-09-11 17:58   ` Misha Tomushev
  1 sibling, 2 replies; 6+ messages in thread
From: Arnd Bergmann @ 2006-09-10 21:39 UTC (permalink / raw)
  To: Misha Tomushev; +Cc: jgarzik, netdev, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] VIOC: New Network Device Driver
  2006-09-10 21:39 ` Arnd Bergmann
@ 2006-09-10 22:21   ` Francois Romieu
  2006-09-11 17:58   ` Misha Tomushev
  1 sibling, 0 replies; 6+ messages in thread
From: Francois Romieu @ 2006-09-10 22:21 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Misha Tomushev, jgarzik, netdev, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] VIOC: New Network Device Driver
  2006-09-10 21:39 ` Arnd Bergmann
  2006-09-10 22:21   ` Francois Romieu
@ 2006-09-11 17:58   ` Misha Tomushev
  2006-09-11 19:15     ` Arnd Bergmann
  1 sibling, 1 reply; 6+ messages in thread
From: Misha Tomushev @ 2006-09-11 17:58 UTC (permalink / raw)
  To: 'Arnd Bergmann'; +Cc: jgarzik, netdev, linux-kernel



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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] VIOC: New Network Device Driver
  2006-09-11 17:58   ` Misha Tomushev
@ 2006-09-11 19:15     ` Arnd Bergmann
  0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2006-09-11 19:15 UTC (permalink / raw)
  To: Misha Tomushev; +Cc: jgarzik, netdev, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] VIOC: New Network Device Driver
@ 2006-09-15  0:15 Misha Tomushev
  2006-09-10 10:50 ` Jan-Benedict Glaw
  2006-09-10 21:39 ` Arnd Bergmann
  0 siblings, 2 replies; 6+ messages in thread
From: Misha Tomushev @ 2006-09-15  0:15 UTC (permalink / raw)
  To: jgarzik; +Cc: netdev, linux-kernel

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2006-09-11 19:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-09-15  0:15 [PATCH] VIOC: New Network Device Driver Misha Tomushev
2006-09-10 10:50 ` Jan-Benedict Glaw
2006-09-10 21:39 ` Arnd Bergmann
2006-09-10 22:21   ` Francois Romieu
2006-09-11 17:58   ` Misha Tomushev
2006-09-11 19:15     ` Arnd Bergmann

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