Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 6/7 v2] fs_enet: Be an of_platform device when CONFIG_PPC_CPM_NEW_BINDING is set.
From: Scott Wood @ 2007-08-22 21:16 UTC (permalink / raw)
  To: Vitaly Bordug; +Cc: jgarzik, netdev, linuxppc-dev
In-Reply-To: <20070823010057.74e1355a@localhost.localdomain>

Vitaly Bordug wrote:
> yes, wrong snippet what about
> 
> 
>> #ifdef CONFIG_CPM2 +	r = fs_enet_mdio_bb_init(); +	if (r != 0) +
>> goto out_mdio_bb; +#endif +#ifdef CONFIG_8xx +	r =
>> fs_enet_mdio_fec_init(); +	if (r != 0) +		goto out_mdio_fec; 
>> +#endif
> 
> 
> We had to pray and hope that 8xx would only have fec, and cpm2 has
> some bitbanged stuff. now we can inquire dts and know for sure, at
> least it seems so.

Yeah, that sucks.  We should add kconfig options for each mii type, and
let them have their own init functions.

That only affects the initcalls (and kernel size), though; it still uses 
the phy-handle to decide what mdio controller to actually talk to.

>> How is that different from the old code, where you're hosed without
>>  fep->fpi->bus_id?
>> 
> 
> 
> I wasn't defending old code, and consider "old code is POS, new one
> is just great" game meaningless. I am just stating the problem, that
> we'll have to address later. On 8xx even reference boards may be 
> without phy at all.

OK -- would it suffice to just never call any phylib functions and
always assume the link is up if the phy-handle property is undefined?

> ok, agreed, size is most serious judge here. we'll definitely have to
> revisit pin problem later too (because custom designs sometimes
> switch contradictory devices on-the-fly, disable soc parts for
> alternative function, etc.)

If it's really on-the-fly (and not jumpered/once-per-boot), then board 
code should expose some kind of API to switch the functions, acting like 
a hotplug bus.  Associating it with open/close of the device won't work 
if one of the devices is something like USB which needs to start working 
without any internal open()-like action.

-Scott

^ permalink raw reply

* Re: [PATCH 4/7] fs_enet: mac-fcc: Eliminate __fcc-* macros.
From: Scott Wood @ 2007-08-22 21:17 UTC (permalink / raw)
  To: Vitaly Bordug; +Cc: jgarzik, netdev, linuxppc-dev
In-Reply-To: <20070823010826.6ae3452d@localhost.localdomain>

Vitaly Bordug wrote:
>>-#define W8(_p, _m, _v)	__fcc_out8(&(_p)->_m, (_v))
>>-#define R8(_p, _m)	__fcc_in8(&(_p)->_m)
>>+#define W8(_p, _m, _v)	out_8(&(_p)->_m, (_v))
>>+#define R8(_p, _m)	in_8(&(_p)->_m)
>> #define S8(_p, _m, _v)	W8(_p, _m, R8(_p, _m) | (_v))
>> #define C8(_p, _m, _v)	W8(_p, _m, R8(_p, _m) & ~(_v))
>> 
>>@@ -290,7 +281,7 @@ static void restart(struct net_device *dev)
>> 
>> 	/* clear everything (slow & steady does it) */
>> 	for (i = 0; i < sizeof(*ep); i++)
>>-		__fcc_out8((char *)ep + i, 0);
>>+		out_8((char *)ep + i, 0);
>> 
> 
> Perhaps W8() here, to keep consistency?

W8 expects a struct pointer and member, which we don't have here.

-Scott

^ permalink raw reply

* Re: [RFC] Wild and crazy ideas involving struct sk_buff
From: Thomas Graf @ 2007-08-22 21:20 UTC (permalink / raw)
  To: Paul Moore; +Cc: netdev
In-Reply-To: <200708221631.34234.paul.moore@hp.com>

* Paul Moore <paul.moore@hp.com> 2007-08-22 16:31
> We're currently talking about several different ideas to solve the problem, 
> including leveraging the sk_buff.secmark field, and one of the ideas was to 
> add an additional field to the sk_buff structure.  Knowing how well that idea 
> would go over (lead balloon is probably an understatement at best) I started 
> looking at what I might be able to remove from the sk_buff struct to make 
> room for a new field (the new field would be a u32).  Looking at the sk_buff 
> structure it appears that the sk_buff.dev and sk_buff.iif fields are a bit 
> redundant and removing the sk_buff.dev field could free 32/64 bits depending 
> on the platform.  Is there any reason (performance?) for keeping the 
> sk_buff.dev field around?  Would the community be open to patches which 
> removed it and transition users over to the sk_buff.iif field?  Finally, 
> assuming the sk_buff.dev field was removed, would the community be open to 
> adding a new LSM/SELinux related u32 field to the sk_buff struct?

This reminds of an idea someone brought up a while ago, it involved
having a way to attach additional space to an sk_buff for all the
different marks and other non-essential fields.

I think skb->dev is required because we need to have a reference on the
device while a packet being processing is put on a queue somewhere.

^ permalink raw reply

* Re: [RFC] Wild and crazy ideas involving struct sk_buff
From: Paul Moore @ 2007-08-22 21:23 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, jmorris
In-Reply-To: <20070822.140805.54950393.davem@davemloft.net>

On Wednesday, August 22 2007 5:08:05 pm David Miller wrote:
> From: Paul Moore <paul.moore@hp.com>
> Date: Wed, 22 Aug 2007 16:31:34 -0400
>
> > We're currently talking about several different ideas to solve the
> > problem, including leveraging the sk_buff.secmark field, and one of the
> > ideas was to add an additional field to the sk_buff structure.  Knowing
> > how well that idea would go over (lead balloon is probably an
> > understatement at best) I started looking at what I might be able to
> > remove from the sk_buff struct to make room for a new field (the new
> > field would be a u32).  Looking at the sk_buff structure it appears that
> > the sk_buff.dev and sk_buff.iif fields are a bit redundant and removing
> > the sk_buff.dev field could free 32/64 bits depending on the platform. 
> > Is there any reason (performance?) for keeping the sk_buff.dev field
> > around?  Would the community be open to patches which removed it and
> > transition users over to the sk_buff.iif field?  Finally, assuming the
> > sk_buff.dev field was removed, would the community be open to adding a
> > new LSM/SELinux related u32 field to the sk_buff struct?
>
> It's there for performance, and I bet there might be some semantic
> issues involved.

Okay, thought that was probably the case considering the efforts to shrink the 
sk_buff as much as possible.

> And ironically James Morris still owes me a struct sk_buff removal
> from when I let him put the "secmark" thing in there!
>
> Stop spending money you guys haven't earned yet :-)

Hey, I was just asking how much it cost ... but then again, you know the old 
adage, "if you have to ask, you can't afford it" ;)

Thanks.

-- 
paul moore
linux security @ hp

^ permalink raw reply

* Re: [RFC] Wild and crazy ideas involving struct sk_buff
From: Paul Moore @ 2007-08-22 21:26 UTC (permalink / raw)
  To: Thomas Graf; +Cc: netdev
In-Reply-To: <20070822212005.GR32236@postel.suug.ch>

On Wednesday, August 22 2007 5:20:05 pm Thomas Graf wrote:
> * Paul Moore <paul.moore@hp.com> 2007-08-22 16:31
>
> > We're currently talking about several different ideas to solve the
> > problem, including leveraging the sk_buff.secmark field, and one of the
> > ideas was to add an additional field to the sk_buff structure.  Knowing
> > how well that idea would go over (lead balloon is probably an
> > understatement at best) I started looking at what I might be able to
> > remove from the sk_buff struct to make room for a new field (the new
> > field would be a u32).  Looking at the sk_buff structure it appears that
> > the sk_buff.dev and sk_buff.iif fields are a bit redundant and removing
> > the sk_buff.dev field could free 32/64 bits depending on the platform. 
> > Is there any reason (performance?) for keeping the sk_buff.dev field
> > around?  Would the community be open to patches which removed it and
> > transition users over to the sk_buff.iif field?  Finally, assuming the
> > sk_buff.dev field was removed, would the community be open to adding a
> > new LSM/SELinux related u32 field to the sk_buff struct?
>
> This reminds of an idea someone brought up a while ago, it involved
> having a way to attach additional space to an sk_buff for all the
> different marks and other non-essential fields.

Interesting.  Was it just a thought, or was there some actual 
design/code/patchset to go along with it that described the idea?

-- 
paul moore
linux security @ hp

^ permalink raw reply

* Re: [RFC] Wild and crazy ideas involving struct sk_buff
From: David Miller @ 2007-08-22 21:36 UTC (permalink / raw)
  To: paul.moore; +Cc: tgraf, netdev
In-Reply-To: <200708221726.36783.paul.moore@hp.com>

From: Paul Moore <paul.moore@hp.com>
Date: Wed, 22 Aug 2007 17:26:36 -0400

> Was it just a thought, or was there some actual design/code/patchset
> to go along with it that described the idea?

It was a thought mentioned at the first two netconfs, but it
went nowhere because the more we discussed the implementation
the more horrific it began to sound :-)

^ permalink raw reply

* Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Adrian Bunk @ 2007-08-22 21:38 UTC (permalink / raw)
  To: Valdis.Kletnieks
  Cc: paulmck, Segher Boessenkool, Paul Mackerras, Russell King,
	Christoph Lameter, heiko.carstens, horms, linux-kernel, ak,
	netdev, cfriesen, akpm, rpjday, Nick Piggin, linux-arch,
	jesper.juhl, satyam, zlynx, schwidefsky, Chris Snook, Herbert Xu,
	davem, Linus Torvalds, wensong, wjiang
In-Reply-To: <28709.1187736676@turing-police.cc.vt.edu>

On Tue, Aug 21, 2007 at 06:51:16PM -0400, Valdis.Kletnieks@vt.edu wrote:
> On Tue, 21 Aug 2007 09:16:43 PDT, "Paul E. McKenney" said:
> 
> > I agree that instant gratification is hard to come by when synching
> > up compiler and kernel versions.  Nonetheless, it should be possible
> > to create APIs that are are conditioned on the compiler version.
> 
> We've tried that, sort of.  See the mess surrounding the whole
> extern/static/inline/__whatever boondogle, which seems to have
> changed semantics in every single gcc release since 2.95 or so.
>...

There is exactly one semantics change in gcc in this area, and that is 
the change of the "extern inline" semantics in gcc 4.3 to the
C99 semantics.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply

* [PATCH][VIA-RHINE]: Fix CONFIG_VIA_RHINE_NAPI usage
From: Arnaldo Carvalho de Melo @ 2007-08-22 21:46 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev

Hi David,

	napi is broken here, with this patch at least I can isolate this
breakage and boot the machine with a non-napi via-rhine driver.

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/drivers/net/via-rhine.c b/drivers/net/via-rhine.c
index 8f4cf82..4cecd06 100644
--- a/drivers/net/via-rhine.c
+++ b/drivers/net/via-rhine.c
@@ -390,7 +390,9 @@ struct rhine_private {
 	struct pci_dev *pdev;
 	long pioaddr;
 	struct net_device *dev;
+#ifdef CONFIG_VIA_RHINE_NAPI
 	struct napi_struct napi;
+#endif
 	struct net_device_stats stats;
 	spinlock_t lock;
 
@@ -1059,9 +1061,9 @@ static void init_registers(struct net_device *dev)
 	iowrite32(rp->tx_ring_dma, ioaddr + TxRingPtr);
 
 	rhine_set_rx_mode(dev);
-
+#ifdef CONFIG_VIA_RHINE_NAPI
 	napi_enable(&rp->napi);
-
+#endif
 	/* Enable interrupts by setting the interrupt mask. */
 	iowrite16(IntrRxDone | IntrRxErr | IntrRxEmpty| IntrRxOverflow |
 	       IntrRxDropped | IntrRxNoBuf | IntrTxAborted |
@@ -1836,8 +1838,9 @@ static int rhine_close(struct net_device *dev)
 	spin_lock_irq(&rp->lock);
 
 	netif_stop_queue(dev);
+#ifdef CONFIG_VIA_RHINE_NAPI
 	napi_disable(&rp->napi);
-
+#endif
 	if (debug > 1)
 		printk(KERN_DEBUG "%s: Shutting down ethercard, "
 		       "status was %4.4x.\n",

^ permalink raw reply related

* eHEA driver issues from net-2.6.24
From: Andrew Theurer @ 2007-08-22 21:55 UTC (permalink / raw)
  To: David S. Miller, Jan-Bernd Themann; +Cc: netdev

In testing the new NAPI improvements on ehea, I get the following:

kernel BUG at include/linux/netdevice.h:318!
cpu 0x1: Vector: 700 (Program Check) at [c00000000f613ac0]
    pc: d000000000091054: .ehea_poll+0x1e8/0x1334 [ehea]
    lr: c0000000003fe394: .net_rx_action+0x1b8/0x254
    sp: c00000000f613d40
   msr: 8000000000029032
  current = 0xc0000005aff9e8b0
  paca    = 0xc0000000006e1300
    pid   = 0, comm = swapper
kernel BUG at include/linux/netdevice.h:318!
enter ? for help
[c00000000f613e40] c0000000003fe394 .net_rx_action+0x1b8/0x254
[c00000000f613ef0] c000000000057b70 .__do_softirq+0xa8/0x164
[c00000000f613f90] c000000000024438 .call_do_softirq+0x14/0x24
[c000000b8ffbf9f0] c00000000000bd30 .do_softirq+0x68/0xac
[c000000b8ffbfa80] c000000000057cc4 .irq_exit+0x54/0x6c
[c000000b8ffbfb00] c00000000000c358 .do_IRQ+0x170/0x1ac
[c000000b8ffbfb90] c000000000004780 hardware_interrupt_entry+0x18/0x98
--- Exception: 501 (Hardware Interrupt) at c000000000010bdc 
.cpu_idle+0x114/0x1e0
[c000000b8ffbfe80] c000000000010bd0 .cpu_idle+0x108/0x1e0 (unreliable)
[c000000b8ffbff00] c000000000026db0 .start_secondary+0x160/0x184
[c000000b8ffbff90] c000000000008364 .start_secondary_prolog+0xc/0x10


I'm a little confused if the port_napi_enable() is being called when the 
device is initialized, but then again, this is all new to me (should it 
be called in ehea_open?).  I see it called on some reset routines, but 
not on the first initialization.


Also, on this code, in ehea_sense_port_attr()

/* Number of default QPs */
        if (use_mcs)
                port->num_def_qps = cb0->num_default_qps;
        else
                port->num_def_qps = 1;
               

When using napi, since we have multi-queue napi support now, wouldn't we 
want to use all the default qps instead of 1?

Thanks,

-Andrew




 

^ permalink raw reply

* Re: [PATCH][VIA-RHINE]: Fix CONFIG_VIA_RHINE_NAPI usage
From: David Miller @ 2007-08-22 21:58 UTC (permalink / raw)
  To: acme; +Cc: netdev
In-Reply-To: <20070822214659.GC23220@ghostprotocols.net>

From: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Date: Wed, 22 Aug 2007 18:46:59 -0300

> Hi David,
> 
> 	napi is broken here, with this patch at least I can isolate this
> breakage and boot the machine with a non-napi via-rhine driver.
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Thanks for finding this problem.

The core issue seems to be that via-rhine.c tries to perform
a napi_enable() when there has not been a previous napi_disable().

So lets make sure that all paths that lead to an init_registers()
call makes a preceeding napi_disable().

Please give this patch a try:

diff --git a/drivers/net/via-rhine.c b/drivers/net/via-rhine.c
index 8f4cf82..0e6aefe 100644
--- a/drivers/net/via-rhine.c
+++ b/drivers/net/via-rhine.c
@@ -1167,6 +1167,7 @@ static int rhine_open(struct net_device *dev)
 		free_irq(rp->pdev->irq, dev);
 		return rc;
 	}
+	napi_disable(&rp->napi);
 	alloc_rbufs(dev);
 	alloc_tbufs(dev);
 	rhine_chip_reset(dev);
@@ -1195,6 +1196,8 @@ static void rhine_tx_timeout(struct net_device *dev)
 	/* protect against concurrent rx interrupts */
 	disable_irq(rp->pdev->irq);
 
+	napi_disable(&rp->napi);
+
 	spin_lock(&rp->lock);
 
 	/* clear all descriptors */
@@ -1935,6 +1938,7 @@ static int rhine_suspend(struct pci_dev *pdev, pm_message_t state)
 	if (!netif_running(dev))
 		return 0;
 
+	napi_disable(&rp->napi);
 	netif_device_detach(dev);
 	pci_save_state(pdev);
 

^ permalink raw reply related

* Re: eHEA driver issues from net-2.6.24
From: David Miller @ 2007-08-22 22:03 UTC (permalink / raw)
  To: habanero; +Cc: ossthema, netdev
In-Reply-To: <46CCB0B7.3030302@us.ibm.com>

From: Andrew Theurer <habanero@us.ibm.com>
Date: Wed, 22 Aug 2007 16:55:03 -0500

Thanks for finally getting to test this, I thought nobody
would test this until it got merged into 2.6.24 :-/

> kernel BUG at include/linux/netdevice.h:318!
> enter ? for help
> [c00000000f613e40] c0000000003fe394 .net_rx_action+0x1b8/0x254
> [c00000000f613ef0] c000000000057b70 .__do_softirq+0xa8/0x164
> [c00000000f613f90] c000000000024438 .call_do_softirq+0x14/0x24
> [c000000b8ffbf9f0] c00000000000bd30 .do_softirq+0x68/0xac
> [c000000b8ffbfa80] c000000000057cc4 .irq_exit+0x54/0x6c
> [c000000b8ffbfb00] c00000000000c358 .do_IRQ+0x170/0x1ac
> [c000000b8ffbfb90] c000000000004780 hardware_interrupt_entry+0x18/0x98
> --- Exception: 501 (Hardware Interrupt) at c000000000010bdc 
> .cpu_idle+0x114/0x1e0
> [c000000b8ffbfe80] c000000000010bd0 .cpu_idle+0x108/0x1e0 (unreliable)
> [c000000b8ffbff00] c000000000026db0 .start_secondary+0x160/0x184
> [c000000b8ffbff90] c000000000008364 .start_secondary_prolog+0xc/0x10
>
> I'm a little confused if the port_napi_enable() is being called when the 
> device is initialized, but then again, this is all new to me (should it 
> be called in ehea_open?).  I see it called on some reset routines, but 
> not on the first initialization.

This is similar to the problem that Arnaldo hit a few minutes
ago in the VIA Rhine driver.

You can't only make a napi_enable() call when there has been
a previous napi_disable().

One way to fix this would be to forcefully napi_disable() on
all the per-port NAPI structs at the beginning of ehea_open(),
which should set things up to satisfy the pre-condition of the
napi_enable() calls.

You'll need to audit the entire driver to make sure this invariant
is held properly.

> Also, on this code, in ehea_sense_port_attr()
> 
> /* Number of default QPs */
>         if (use_mcs)
>                 port->num_def_qps = cb0->num_default_qps;
>         else
>                 port->num_def_qps = 1;
>                
> 
> When using napi, since we have multi-queue napi support now, wouldn't we 
> want to use all the default qps instead of 1?

I don't know how this hardware works, you tell me :-)

^ permalink raw reply

* Re: [PATCH 8/9] define global BIT macro
From: Ralf Baechle @ 2007-08-22 22:03 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Andrew Morton, linux-kernel, source, dougthompson,
	bluesmoke-devel, dtor, linux-input, netdev, James.Bottomley,
	linux-scsi, gtolstolytkin, vitalywool, dsaxena, linux-mips,
	mchehab, video4linux-list, jbenc, flamingice, linux-wireless
In-Reply-To: <428714662539710215@wsc.cz>

On Sat, Aug 18, 2007 at 11:44:12AM +0200, Jiri Slaby wrote:

> define global BIT macro
> 
> move all local BIT defines to the new globally define macro.
> 
> Signed-off-by: Jiri Slaby <jirislaby@gmail.com>

Acked-by: Ralf Baechle <ralf@linux-mips.org>

for the MACE ethernet and MIPS bits.

  Ralf

^ permalink raw reply

* Re: [RFC] Wild and crazy ideas involving struct sk_buff
From: Arnaldo Carvalho de Melo @ 2007-08-22 22:06 UTC (permalink / raw)
  To: David Miller; +Cc: paul.moore, tgraf, netdev
In-Reply-To: <20070822.143627.62367286.davem@davemloft.net>

Em Wed, Aug 22, 2007 at 02:36:27PM -0700, David Miller escreveu:
> From: Paul Moore <paul.moore@hp.com>
> Date: Wed, 22 Aug 2007 17:26:36 -0400
> 
> > Was it just a thought, or was there some actual design/code/patchset
> > to go along with it that described the idea?
> 
> It was a thought mentioned at the first two netconfs, but it
> went nowhere because the more we discussed the implementation
> the more horrific it began to sound :-)

Which makes it more interesting to make it work :-)

Anyway, there is space there to be used, in 32 and 64 bits we have two
bytes after queue_mapping (this kernel doesn't have CONFIG_NET_SCHED nor
CONFIG_NET_CLS_ACT enabled, if the first was we wouldn't have the hole,
but if both, as its common in distro kernels, were we would have the 2
bytes hole, as we can see on the pahole output for the 64bits kernel,
that uses the kernel config used in the Red Hat kernels.

Bonus points would go to continue the skb offset fight and reduce the
size of mac_header to two bytes, and then moving it to just after
queue_mapping. Do that and you'll get some credits :-P

- Arnaldo

[acme@mica net-2.6.24]$ pahole -C sk_buff ../32bits/net/core/skbuff.o
struct sk_buff {
	struct sk_buff *           next;                 /*     0     4 */
	struct sk_buff *           prev;                 /*     4     4 */
	struct sock *              sk;                   /*     8     4 */
	ktime_t                    tstamp;               /*    12     8 */
	struct net_device *        dev;                  /*    20     4 */
	struct dst_entry *         dst;                  /*    24     4 */
	struct sec_path *          sp;                   /*    28     4 */
	char                       cb[48];               /*    32    48 */
	/* --- cacheline 1 boundary (64 bytes) was 16 bytes ago --- */
	unsigned int               len;                  /*    80     4 */
	unsigned int               data_len;             /*    84     4 */
	__u16                      mac_len;              /*    88     2 */
	__u16                      hdr_len;              /*    90     2 */
	union {
		__wsum             csum;                 /*           4 */
		struct {
			__u16      csum_start;           /*    92     2 */
			__u16      csum_offset;          /*    94     2 */
		}                                        /*           4 */
	};                                               /*    92     4 */
	__u32                      priority;             /*    96     4 */
	__u8                       local_df:1;           /*   100     1 */
	__u8                       cloned:1;             /*   100     1 */
	__u8                       ip_summed:2;          /*   100     1 */
	__u8                       nohdr:1;              /*   100     1 */
	__u8                       nfctinfo:3;           /*   100     1 */
	__u8                       pkt_type:3;           /*   101     1 */
	__u8                       fclone:2;             /*   101     1 */
	__u8                       ipvs_property:1;      /*   101     1 */
	__u8                       nf_trace:1;           /*   101     1 */

	/* XXX 1 bit hole, try to pack */

	__be16                     protocol;             /*   102     2 */
	void            (*destructor)(struct sk_buff *); /*   104     4 */
	int                        iif;                  /*   108     4 */
	__u16                      queue_mapping;        /*   112     2 */

	/* XXX 2 bytes hole, try to pack */

	__u32                      mark;                 /*   116     4 */
	sk_buff_data_t             transport_header;     /*   120     4 */
	sk_buff_data_t             network_header;       /*   124     4 */
	/* --- cacheline 2 boundary (128 bytes) --- */
	sk_buff_data_t             mac_header;           /*   128     4 */
	sk_buff_data_t             tail;                 /*   132     4 */
	sk_buff_data_t             end;                  /*   136     4 */
	unsigned char *            head;                 /*   140     4 */
	unsigned char *            data;                 /*   144     4 */
	unsigned int               truesize;             /*   148     4 */
	atomic_t                   users;                /*   152     4 */

	/* size: 156, cachelines: 3 */
	/* sum members: 154, holes: 1, sum holes: 2 */
	/* bit holes: 1, sum bit holes: 1 bits */
	/* last cacheline: 28 bytes */
};

[acme@mica net-2.6.24]$ pahole -C sk_buff ../64bits/net/core/skbuff.o
struct sk_buff {
	struct sk_buff *           next;                 /*     0     8 */
	struct sk_buff *           prev;                 /*     8     8 */
	struct sock *              sk;                   /*    16     8 */
	ktime_t                    tstamp;               /*    24     8 */
	struct net_device *        dev;                  /*    32     8 */
	struct dst_entry *         dst;                  /*    40     8 */
	struct sec_path *          sp;                   /*    48     8 */
	char                       cb[48];               /*    56    48 */
	/* --- cacheline 1 boundary (64 bytes) was 40 bytes ago --- */
	unsigned int               len;                  /*   104     4 */
	unsigned int               data_len;             /*   108     4 */
	__u16                      mac_len;              /*   112     2 */
	__u16                      hdr_len;              /*   114     2 */
	union {
		__wsum             csum;                 /*           4 */
		struct {
			__u16      csum_start;           /*   116     2 */
			__u16      csum_offset;          /*   118     2 */
		}                                        /*           4 */
	};                                               /*   116     4 */
	__u32                      priority;             /*   120     4 */
	__u8                       local_df:1;           /*   124     1 */
	__u8                       cloned:1;             /*   124     1 */
	__u8                       ip_summed:2;          /*   124     1 */
	__u8                       nohdr:1;              /*   124     1 */
	__u8                       nfctinfo:3;           /*   124     1 */
	__u8                       pkt_type:3;           /*   125     1 */
	__u8                       fclone:2;             /*   125     1 */
	__u8                       ipvs_property:1;      /*   125     1 */
	__u8                       nf_trace:1;           /*   125     1 */

	/* XXX 1 bit hole, try to pack */

	__be16                     protocol;             /*   126     2 */
	/* --- cacheline 2 boundary (128 bytes) --- */
	void            (*destructor)(struct sk_buff *); /*   128     8 */
	struct nf_conntrack *      nfct;                 /*   136     8 */
	struct sk_buff *           nfct_reasm;           /*   144     8 */
	struct nf_bridge_info *    nf_bridge;            /*   152     8 */
	int                        iif;                  /*   160     4 */
	__u16                      queue_mapping;        /*   164     2 */
	__u16                      tc_index;             /*   166     2 */
	__u16                      tc_verd;              /*   168     2 */

	/* XXX 2 bytes hole, try to pack */

	dma_cookie_t               dma_cookie;           /*   172     4 */
	__u32                      secmark;              /*   176     4 */
	__u32                      mark;                 /*   180     4 */
	sk_buff_data_t             transport_header;     /*   184     4 */
	sk_buff_data_t             network_header;       /*   188     4 */
	/* --- cacheline 3 boundary (192 bytes) --- */
	sk_buff_data_t             mac_header;           /*   192     4 */
	sk_buff_data_t             tail;                 /*   196     4 */
	sk_buff_data_t             end;                  /*   200     4 */

	/* XXX 4 bytes hole, try to pack */

	unsigned char *            head;                 /*   208     8 */
	unsigned char *            data;                 /*   216     8 */
	unsigned int               truesize;             /*   224     4 */
	atomic_t                   users;                /*   228     4 */

	/* size: 232, cachelines: 4 */
	/* sum members: 226, holes: 2, sum holes: 6 */
	/* bit holes: 1, sum bit holes: 1 bits */
	/* last cacheline: 40 bytes */
};

^ permalink raw reply

* Re: [RFC] Wild and crazy ideas involving struct sk_buff
From: James Morris @ 2007-08-22 22:09 UTC (permalink / raw)
  To: David Miller; +Cc: paul.moore, tgraf, netdev
In-Reply-To: <20070822.143627.62367286.davem@davemloft.net>

On Wed, 22 Aug 2007, David Miller wrote:

> From: Paul Moore <paul.moore@hp.com>
> Date: Wed, 22 Aug 2007 17:26:36 -0400
> 
> > Was it just a thought, or was there some actual design/code/patchset
> > to go along with it that described the idea?
> 
> It was a thought mentioned at the first two netconfs, but it
> went nowhere because the more we discussed the implementation
> the more horrific it began to sound :-)

Don't forget Rusty's skb reservation patches from 1999...


-- 
James Morris
<jmorris@namei.org>

^ permalink raw reply

* Re: [PATCH][VIA-RHINE]: Fix CONFIG_VIA_RHINE_NAPI usage
From: Arnaldo Carvalho de Melo @ 2007-08-22 22:19 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20070822.145855.32177470.davem@davemloft.net>

Em Wed, Aug 22, 2007 at 02:58:55PM -0700, David Miller escreveu:
> From: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Date: Wed, 22 Aug 2007 18:46:59 -0300
> 
> > Hi David,
> > 
> > 	napi is broken here, with this patch at least I can isolate this
> > breakage and boot the machine with a non-napi via-rhine driver.
> > 
> 
> Thanks for finding this problem.
> 
> The core issue seems to be that via-rhine.c tries to perform
> a napi_enable() when there has not been a previous napi_disable().
> 
> So lets make sure that all paths that lead to an init_registers()
> call makes a preceeding napi_disable().
> 
> Please give this patch a try:

Tried with and without CONFIG_VIA_RHINE_NAPI, did stress testing with
netperf only with NAPI enabled. Works great


Signed-off-by: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
 
> diff --git a/drivers/net/via-rhine.c b/drivers/net/via-rhine.c
> index 8f4cf82..0e6aefe 100644
> --- a/drivers/net/via-rhine.c
> +++ b/drivers/net/via-rhine.c
> @@ -1167,6 +1167,7 @@ static int rhine_open(struct net_device *dev)
>  		free_irq(rp->pdev->irq, dev);
>  		return rc;
>  	}
> +	napi_disable(&rp->napi);
>  	alloc_rbufs(dev);
>  	alloc_tbufs(dev);
>  	rhine_chip_reset(dev);
> @@ -1195,6 +1196,8 @@ static void rhine_tx_timeout(struct net_device *dev)
>  	/* protect against concurrent rx interrupts */
>  	disable_irq(rp->pdev->irq);
>  
> +	napi_disable(&rp->napi);
> +
>  	spin_lock(&rp->lock);
>  
>  	/* clear all descriptors */
> @@ -1935,6 +1938,7 @@ static int rhine_suspend(struct pci_dev *pdev, pm_message_t state)
>  	if (!netif_running(dev))
>  		return 0;
>  
> +	napi_disable(&rp->napi);
>  	netif_device_detach(dev);
>  	pci_save_state(pdev);
>  
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: eHEA driver issues from net-2.6.24
From: Andrew Theurer @ 2007-08-22 22:20 UTC (permalink / raw)
  To: David Miller; +Cc: ossthema, netdev
In-Reply-To: <20070822.150302.44981242.davem@davemloft.net>

David Miller wrote:
> From: Andrew Theurer <habanero@us.ibm.com>
> Date: Wed, 22 Aug 2007 16:55:03 -0500
>
> Thanks for finally getting to test this, I thought nobody
> would test this until it got merged into 2.6.24 :-/
>
>   
>> kernel BUG at include/linux/netdevice.h:318!
>> enter ? for help
>> [c00000000f613e40] c0000000003fe394 .net_rx_action+0x1b8/0x254
>> [c00000000f613ef0] c000000000057b70 .__do_softirq+0xa8/0x164
>> [c00000000f613f90] c000000000024438 .call_do_softirq+0x14/0x24
>> [c000000b8ffbf9f0] c00000000000bd30 .do_softirq+0x68/0xac
>> [c000000b8ffbfa80] c000000000057cc4 .irq_exit+0x54/0x6c
>> [c000000b8ffbfb00] c00000000000c358 .do_IRQ+0x170/0x1ac
>> [c000000b8ffbfb90] c000000000004780 hardware_interrupt_entry+0x18/0x98
>> --- Exception: 501 (Hardware Interrupt) at c000000000010bdc 
>> .cpu_idle+0x114/0x1e0
>> [c000000b8ffbfe80] c000000000010bd0 .cpu_idle+0x108/0x1e0 (unreliable)
>> [c000000b8ffbff00] c000000000026db0 .start_secondary+0x160/0x184
>> [c000000b8ffbff90] c000000000008364 .start_secondary_prolog+0xc/0x10
>>
>> I'm a little confused if the port_napi_enable() is being called when the 
>> device is initialized, but then again, this is all new to me (should it 
>> be called in ehea_open?).  I see it called on some reset routines, but 
>> not on the first initialization.
>>     
>
> This is similar to the problem that Arnaldo hit a few minutes
> ago in the VIA Rhine driver.
>
> You can't only make a napi_enable() call when there has been
> a previous napi_disable().
>
> One way to fix this would be to forcefully napi_disable() on
> all the per-port NAPI structs at the beginning of ehea_open(),
> which should set things up to satisfy the pre-condition of the
> napi_enable() calls.
>   
OK, Ill try this. 
> You'll need to audit the entire driver to make sure this invariant
> is held properly.
>
>   
>> Also, on this code, in ehea_sense_port_attr()
>>
>> /* Number of default QPs */
>>         if (use_mcs)
>>                 port->num_def_qps = cb0->num_default_qps;
>>         else
>>                 port->num_def_qps = 1;
>>                
>>
>> When using napi, since we have multi-queue napi support now, wouldn't we 
>> want to use all the default qps instead of 1?
>>     
>
> I don't know how this hardware works, you tell me :-)
>   
Heh, I don't know it well, either. Maybe Jan Bernd can chime in.

Thanks for your help,

-Andrew


^ permalink raw reply

* Re: [RFC] Wild and crazy ideas involving struct sk_buff
From: Paul Moore @ 2007-08-22 22:20 UTC (permalink / raw)
  To: James Morris; +Cc: David Miller, tgraf, netdev
In-Reply-To: <Xine.LNX.4.64.0708221506380.20059@us.intercode.com.au>

On Wednesday, August 22 2007 6:09:36 pm James Morris wrote:
> On Wed, 22 Aug 2007, David Miller wrote:
> > From: Paul Moore <paul.moore@hp.com>
> > Date: Wed, 22 Aug 2007 17:26:36 -0400
> >
> > > Was it just a thought, or was there some actual design/code/patchset
> > > to go along with it that described the idea?
> >
> > It was a thought mentioned at the first two netconfs, but it
> > went nowhere because the more we discussed the implementation
> > the more horrific it began to sound :-)
>
> Don't forget Rusty's skb reservation patches from 1999...

I'm guessing those would probably need to be forward ported just a teensy 
weensy bit ;)

-- 
paul moore
linux security @ hp

^ permalink raw reply

* Re: [RFC] Wild and crazy ideas involving struct sk_buff
From: James Morris @ 2007-08-22 22:31 UTC (permalink / raw)
  To: Paul Moore; +Cc: David Miller, tgraf, netdev
In-Reply-To: <200708221820.56516.paul.moore@hp.com>

On Wed, 22 Aug 2007, Paul Moore wrote:

> > Don't forget Rusty's skb reservation patches from 1999...
> 
> I'm guessing those would probably need to be forward ported just a teensy 
> weensy bit ;)

The point being that the idea has been around since last century and has 
never gone anywhere :-)


-- 
James Morris
<jmorris@namei.org>

^ permalink raw reply

* Re: [RFT] sky2: yukon-ec-u phy power problems
From: Kevin E @ 2007-08-22 22:37 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Willy Tarreau, linux-kernel, netdev
In-Reply-To: <20070821100713.2867dce3@freepuppy.rosehill.hemminger.net>


--- Stephen Hemminger
<shemminger@linux-foundation.org> wrote:

> The sky2 driver clears some bits in the PHY control
> register, that cause
> the PHY interface to get changed.  Some of these
> deal with voltage and power
> savings as well. This may explain some of the
> failures on Gigabyte DS-3 motherboard.
> 
> The clue to possible problem was looking at why
> loading/unloading sky2 would
> break vendor sk98lin driver (ie what registers
> changed).
> 
> 
> --- a/drivers/net/sky2.c	2007-08-21
> 09:53:41.000000000 -0700
> +++ b/drivers/net/sky2.c	2007-08-21
> 09:53:43.000000000 -0700
> @@ -696,8 +696,8 @@ static void sky2_mac_init(struct
> sky2_hw
>  	int i;
>  	const u8 *addr = hw->dev[port]->dev_addr;
>  
> -	sky2_write32(hw, SK_REG(port, GPHY_CTRL),
> GPC_RST_SET);
> -	sky2_write32(hw, SK_REG(port, GPHY_CTRL),
> GPC_RST_CLR);
> +	sky2_write8(hw, SK_REG(port, GPHY_CTRL),
> GPC_RST_SET);
> +	sky2_write8(hw, SK_REG(port, GPHY_CTRL),
> GPC_RST_CLR);
>  
>  	sky2_write8(hw, SK_REG(port, GMAC_CTRL),
> GMC_RST_CLR);
>  

This appears to have fixed the problem!  I made the
change to the kernel on the broken MB and the link
stayed up.  So I went ahead and made the change to the
kernel on the working one too.   Right now I'm using
netcat to saturate the link and make sure it stays up.
 So far it's looking good, if I have any problems I'll
let you know.

Thanks,
Kevin



       
____________________________________________________________________________________
Building a website is a piece of cake. Yahoo! Small Business gives you all the tools to get online.
http://smallbusiness.yahoo.com/webhosting 

^ permalink raw reply

* [RFC] remove obsolete shaper code
From: Stephen Hemminger @ 2007-08-22 23:58 UTC (permalink / raw)
  To: David S. Miller, Alan Cox; +Cc: netdev

This driver is obsolete, and fixing it for adding upcoming changes
to make hard_header_ops
is problematic because it changes the underlying device handles.

The driver has been superseded by iproute2/traffic control for
a long time. Let's kill it.

Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

--- a/drivers/net/Kconfig	2007-08-22 13:15:51.000000000 -0700
+++ b/drivers/net/Kconfig	2007-08-22 16:45:11.000000000 -0700
@@ -2970,23 +2970,6 @@ config NET_FC
 	  adaptor below. You also should have said Y to "SCSI support" and
 	  "SCSI generic support".
 
-config SHAPER
-	tristate "Traffic Shaper (OBSOLETE)"
-	depends on EXPERIMENTAL
-	---help---
-	  The traffic shaper is a virtual network device that allows you to
-	  limit the rate of outgoing data flow over some other network device.
-	  The traffic that you want to slow down can then be routed through
-	  these virtual devices. See
-	  <file:Documentation/networking/shaper.txt> for more information.
-
-	  An alternative to this traffic shaper are traffic schedulers which
-	  you'll get if you say Y to "QoS and/or fair queuing" in
-	  "Networking options".
-
-	  To compile this driver as a module, choose M here: the module
-	  will be called shaper.  If unsure, say N.
-
 config NETCONSOLE
 	tristate "Network console logging support (EXPERIMENTAL)"
 	depends on EXPERIMENTAL
--- a/drivers/net/shaper.c	2007-08-22 16:32:18.000000000 -0700
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,650 +0,0 @@
-/*
- *			Simple traffic shaper for Linux NET3.
- *
- *	(c) Copyright 1996 Alan Cox <alan@redhat.com>, All Rights Reserved.
- *				http://www.redhat.com
- *
- *	This program is free software; you can redistribute it and/or
- *	modify it under the terms of the GNU General Public License
- *	as published by the Free Software Foundation; either version
- *	2 of the License, or (at your option) any later version.
- *
- *	Neither Alan Cox nor CymruNet Ltd. admit liability nor provide
- *	warranty for any of this software. This material is provided
- *	"AS-IS" and at no charge.
- *
- *
- *	Algorithm:
- *
- *	Queue Frame:
- *		Compute time length of frame at regulated speed
- *		Add frame to queue at appropriate point
- *		Adjust time length computation for followup frames
- *		Any frame that falls outside of its boundaries is freed
- *
- *	We work to the following constants
- *
- *		SHAPER_QLEN	Maximum queued frames
- *		SHAPER_LATENCY	Bounding latency on a frame. Leaving this latency
- *				window drops the frame. This stops us queueing
- *				frames for a long time and confusing a remote
- *				host.
- *		SHAPER_MAXSLIP	Maximum time a priority frame may jump forward.
- *				That bounds the penalty we will inflict on low
- *				priority traffic.
- *		SHAPER_BURST	Time range we call "now" in order to reduce
- *				system load. The more we make this the burstier
- *				the behaviour, the better local performance you
- *				get through packet clustering on routers and the
- *				worse the remote end gets to judge rtts.
- *
- *	This is designed to handle lower speed links ( < 200K/second or so). We
- *	run off a 100-150Hz base clock typically. This gives us a resolution at
- *	200Kbit/second of about 2Kbit or 256 bytes. Above that our timer
- *	resolution may start to cause much more burstiness in the traffic. We
- *	could avoid a lot of that by calling kick_shaper() at the end of the
- *	tied device transmissions. If you run above about 100K second you
- *	may need to tune the supposed speed rate for the right values.
- *
- *	BUGS:
- *		Downing the interface under the shaper before the shaper
- *		will render your machine defunct. Don't for now shape over
- *		PPP or SLIP therefore!
- *		This will be fixed in BETA4
- *
- * Update History :
- *
- *              bh_atomic() SMP races fixes and rewritten the locking code to
- *              be SMP safe and irq-mask friendly.
- *              NOTE: we can't use start_bh_atomic() in kick_shaper()
- *              because it's going to be recalled from an irq handler,
- *              and synchronize_bh() is a nono if called from irq context.
- *						1999  Andrea Arcangeli
- *
- *              Device statistics (tx_pakets, tx_bytes,
- *              tx_drops: queue_over_time and collisions: max_queue_exceded)
- *                               1999/06/18 Jordi Murgo <savage@apostols.org>
- *
- *		Use skb->cb for private data.
- *				 2000/03 Andi Kleen
- */
-
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/fcntl.h>
-#include <linux/mm.h>
-#include <linux/slab.h>
-#include <linux/string.h>
-#include <linux/errno.h>
-#include <linux/netdevice.h>
-#include <linux/etherdevice.h>
-#include <linux/skbuff.h>
-#include <linux/if_arp.h>
-#include <linux/init.h>
-#include <linux/if_shaper.h>
-#include <linux/jiffies.h>
-
-#include <net/dst.h>
-#include <net/arp.h>
-
-struct shaper_cb {
-	unsigned long	shapeclock;		/* Time it should go out */
-	unsigned long	shapestamp;		/* Stamp for shaper    */
-	__u32		shapelatency;		/* Latency on frame */
-	__u32		shapelen;		/* Frame length in clocks */
-	__u16		shapepend;		/* Pending */
-};
-#define SHAPERCB(skb) ((struct shaper_cb *) ((skb)->cb))
-
-static int sh_debug;		/* Debug flag */
-
-#define SHAPER_BANNER	"CymruNet Traffic Shaper BETA 0.04 for Linux 2.1\n"
-
-static void shaper_kick(struct shaper *sh);
-
-/*
- *	Compute clocks on a buffer
- */
-
-static int shaper_clocks(struct shaper *shaper, struct sk_buff *skb)
-{
- 	int t=skb->len/shaper->bytespertick;
- 	return t;
-}
-
-/*
- *	Set the speed of a shaper. We compute this in bytes per tick since
- *	thats how the machine wants to run. Quoted input is in bits per second
- *	as is traditional (note not BAUD). We assume 8 bit bytes.
- */
-
-static void shaper_setspeed(struct shaper *shaper, int bitspersec)
-{
-	shaper->bitspersec=bitspersec;
-	shaper->bytespertick=(bitspersec/HZ)/8;
-	if(!shaper->bytespertick)
-		shaper->bytespertick++;
-}
-
-/*
- *	Throw a frame at a shaper.
- */
-
-
-static int shaper_start_xmit(struct sk_buff *skb, struct net_device *dev)
-{
-	struct shaper *shaper = dev->priv;
- 	struct sk_buff *ptr;
-
-	spin_lock(&shaper->lock);
- 	ptr=shaper->sendq.prev;
-
- 	/*
- 	 *	Set up our packet details
- 	 */
-
- 	SHAPERCB(skb)->shapelatency=0;
- 	SHAPERCB(skb)->shapeclock=shaper->recovery;
- 	if(time_before(SHAPERCB(skb)->shapeclock, jiffies))
- 		SHAPERCB(skb)->shapeclock=jiffies;
- 	skb->priority=0;	/* short term bug fix */
- 	SHAPERCB(skb)->shapestamp=jiffies;
-
- 	/*
- 	 *	Time slots for this packet.
- 	 */
-
- 	SHAPERCB(skb)->shapelen= shaper_clocks(shaper,skb);
-
-	{
-		struct sk_buff *tmp;
-		/*
-		 *	Up our shape clock by the time pending on the queue
-		 *	(Should keep this in the shaper as a variable..)
-		 */
-		for(tmp=skb_peek(&shaper->sendq); tmp!=NULL &&
-			tmp!=(struct sk_buff *)&shaper->sendq; tmp=tmp->next)
-			SHAPERCB(skb)->shapeclock+=SHAPERCB(tmp)->shapelen;
-		/*
-		 *	Queue over time. Spill packet.
-		 */
-		if(time_after(SHAPERCB(skb)->shapeclock,jiffies + SHAPER_LATENCY)) {
-			dev_kfree_skb(skb);
-			shaper->stats.tx_dropped++;
-		} else
-			skb_queue_tail(&shaper->sendq, skb);
-	}
-
-	if(sh_debug)
- 		printk("Frame queued.\n");
- 	if(skb_queue_len(&shaper->sendq)>SHAPER_QLEN)
- 	{
- 		ptr=skb_dequeue(&shaper->sendq);
-                dev_kfree_skb(ptr);
-                shaper->stats.collisions++;
- 	}
-	shaper_kick(shaper);
-	spin_unlock(&shaper->lock);
- 	return 0;
-}
-
-/*
- *	Transmit from a shaper
- */
-
-static void shaper_queue_xmit(struct shaper *shaper, struct sk_buff *skb)
-{
-	struct sk_buff *newskb=skb_clone(skb, GFP_ATOMIC);
-	if(sh_debug)
-		printk("Kick frame on %p\n",newskb);
-	if(newskb)
-	{
-		newskb->dev=shaper->dev;
-		newskb->priority=2;
-		if(sh_debug)
-			printk("Kick new frame to %s, %d\n",
-				shaper->dev->name,newskb->priority);
-		dev_queue_xmit(newskb);
-
-                shaper->stats.tx_bytes += skb->len;
-		shaper->stats.tx_packets++;
-
-                if(sh_debug)
-			printk("Kicked new frame out.\n");
-		dev_kfree_skb(skb);
-	}
-}
-
-/*
- *	Timer handler for shaping clock
- */
-
-static void shaper_timer(unsigned long data)
-{
-	struct shaper *shaper = (struct shaper *)data;
-
-	spin_lock(&shaper->lock);
-	shaper_kick(shaper);
-	spin_unlock(&shaper->lock);
-}
-
-/*
- *	Kick a shaper queue and try and do something sensible with the
- *	queue.
- */
-
-static void shaper_kick(struct shaper *shaper)
-{
-	struct sk_buff *skb;
-
-	/*
-	 *	Walk the list (may be empty)
-	 */
-
-	while((skb=skb_peek(&shaper->sendq))!=NULL)
-	{
-		/*
-		 *	Each packet due to go out by now (within an error
-		 *	of SHAPER_BURST) gets kicked onto the link
-		 */
-
-		if(sh_debug)
-			printk("Clock = %ld, jiffies = %ld\n", SHAPERCB(skb)->shapeclock, jiffies);
-		if(time_before_eq(SHAPERCB(skb)->shapeclock, jiffies + SHAPER_BURST))
-		{
-			/*
-			 *	Pull the frame and get interrupts back on.
-			 */
-
-			skb_unlink(skb, &shaper->sendq);
-			if (shaper->recovery <
-			    SHAPERCB(skb)->shapeclock + SHAPERCB(skb)->shapelen)
-				shaper->recovery = SHAPERCB(skb)->shapeclock + SHAPERCB(skb)->shapelen;
-			/*
-			 *	Pass on to the physical target device via
-			 *	our low level packet thrower.
-			 */
-
-			SHAPERCB(skb)->shapepend=0;
-			shaper_queue_xmit(shaper, skb);	/* Fire */
-		}
-		else
-			break;
-	}
-
-	/*
-	 *	Next kick.
-	 */
-
-	if(skb!=NULL)
-		mod_timer(&shaper->timer, SHAPERCB(skb)->shapeclock);
-}
-
-
-/*
- *	Bring the interface up. We just disallow this until a
- *	bind.
- */
-
-static int shaper_open(struct net_device *dev)
-{
-	struct shaper *shaper=dev->priv;
-
-	/*
-	 *	Can't open until attached.
-	 *	Also can't open until speed is set, or we'll get
-	 *	a division by zero.
-	 */
-
-	if(shaper->dev==NULL)
-		return -ENODEV;
-	if(shaper->bitspersec==0)
-		return -EINVAL;
-	return 0;
-}
-
-/*
- *	Closing a shaper flushes the queues.
- */
-
-static int shaper_close(struct net_device *dev)
-{
-	struct shaper *shaper=dev->priv;
-	struct sk_buff *skb;
-
-	while ((skb = skb_dequeue(&shaper->sendq)) != NULL)
-		dev_kfree_skb(skb);
-
-	spin_lock_bh(&shaper->lock);
-	shaper_kick(shaper);
-	spin_unlock_bh(&shaper->lock);
-
-	del_timer_sync(&shaper->timer);
-	return 0;
-}
-
-/*
- *	Revectored calls. We alter the parameters and call the functions
- *	for our attached device. This enables us to bandwidth allocate after
- *	ARP and other resolutions and not before.
- */
-
-static struct net_device_stats *shaper_get_stats(struct net_device *dev)
-{
-     	struct shaper *sh=dev->priv;
-	return &sh->stats;
-}
-
-static int shaper_header(struct sk_buff *skb, struct net_device *dev,
-	unsigned short type, void *daddr, void *saddr, unsigned len)
-{
-	struct shaper *sh=dev->priv;
-	int v;
-	if(sh_debug)
-		printk("Shaper header\n");
-	skb->dev=sh->dev;
-	v=sh->hard_header(skb,sh->dev,type,daddr,saddr,len);
-	skb->dev=dev;
-	return v;
-}
-
-static int shaper_rebuild_header(struct sk_buff *skb)
-{
-	struct shaper *sh=skb->dev->priv;
-	struct net_device *dev=skb->dev;
-	int v;
-	if(sh_debug)
-		printk("Shaper rebuild header\n");
-	skb->dev=sh->dev;
-	v=sh->rebuild_header(skb);
-	skb->dev=dev;
-	return v;
-}
-
-#if 0
-static int shaper_cache(struct neighbour *neigh, struct hh_cache *hh)
-{
-	struct shaper *sh=neigh->dev->priv;
-	struct net_device *tmp;
-	int ret;
-	if(sh_debug)
-		printk("Shaper header cache bind\n");
-	tmp=neigh->dev;
-	neigh->dev=sh->dev;
-	ret=sh->hard_header_cache(neigh,hh);
-	neigh->dev=tmp;
-	return ret;
-}
-
-static void shaper_cache_update(struct hh_cache *hh, struct net_device *dev,
-	unsigned char *haddr)
-{
-	struct shaper *sh=dev->priv;
-	if(sh_debug)
-		printk("Shaper cache update\n");
-	sh->header_cache_update(hh, sh->dev, haddr);
-}
-#endif
-
-#ifdef CONFIG_INET
-
-static int shaper_neigh_setup(struct neighbour *n)
-{
-#ifdef CONFIG_INET
-	if (n->nud_state == NUD_NONE) {
-		n->ops = &arp_broken_ops;
-		n->output = n->ops->output;
-	}
-#endif
-	return 0;
-}
-
-static int shaper_neigh_setup_dev(struct net_device *dev, struct neigh_parms *p)
-{
-#ifdef CONFIG_INET
-	if (p->tbl->family == AF_INET) {
-		p->neigh_setup = shaper_neigh_setup;
-		p->ucast_probes = 0;
-		p->mcast_probes = 0;
-	}
-#endif
-	return 0;
-}
-
-#else /* !(CONFIG_INET) */
-
-static int shaper_neigh_setup_dev(struct net_device *dev, struct neigh_parms *p)
-{
-	return 0;
-}
-
-#endif
-
-static int shaper_attach(struct net_device *shdev, struct shaper *sh, struct net_device *dev)
-{
-	sh->dev = dev;
-	sh->hard_start_xmit=dev->hard_start_xmit;
-	sh->get_stats=dev->get_stats;
-	if(dev->hard_header)
-	{
-		sh->hard_header=dev->hard_header;
-		shdev->hard_header = shaper_header;
-	}
-	else
-		shdev->hard_header = NULL;
-
-	if(dev->rebuild_header)
-	{
-		sh->rebuild_header	= dev->rebuild_header;
-		shdev->rebuild_header	= shaper_rebuild_header;
-	}
-	else
-		shdev->rebuild_header	= NULL;
-
-#if 0
-	if(dev->hard_header_cache)
-	{
-		sh->hard_header_cache	= dev->hard_header_cache;
-		shdev->hard_header_cache= shaper_cache;
-	}
-	else
-	{
-		shdev->hard_header_cache= NULL;
-	}
-
-	if(dev->header_cache_update)
-	{
-		sh->header_cache_update	= dev->header_cache_update;
-		shdev->header_cache_update = shaper_cache_update;
-	}
-	else
-		shdev->header_cache_update= NULL;
-#else
-	shdev->header_cache_update = NULL;
-	shdev->hard_header_cache = NULL;
-#endif
-	shdev->neigh_setup = shaper_neigh_setup_dev;
-
-	shdev->hard_header_len=dev->hard_header_len;
-	shdev->type=dev->type;
-	shdev->addr_len=dev->addr_len;
-	shdev->mtu=dev->mtu;
-	sh->bitspersec=0;
-	return 0;
-}
-
-static int shaper_ioctl(struct net_device *dev,  struct ifreq *ifr, int cmd)
-{
-	struct shaperconf *ss= (struct shaperconf *)&ifr->ifr_ifru;
-	struct shaper *sh=dev->priv;
-
-	if(ss->ss_cmd == SHAPER_SET_DEV || ss->ss_cmd == SHAPER_SET_SPEED)
-	{
-		if(!capable(CAP_NET_ADMIN))
-			return -EPERM;
-	}
-
-	switch(ss->ss_cmd)
-	{
-		case SHAPER_SET_DEV:
-		{
-			struct net_device *them=__dev_get_by_name(ss->ss_name);
-			if(them==NULL)
-				return -ENODEV;
-			if(sh->dev)
-				return -EBUSY;
-			return shaper_attach(dev,dev->priv, them);
-		}
-		case SHAPER_GET_DEV:
-			if(sh->dev==NULL)
-				return -ENODEV;
-			strcpy(ss->ss_name, sh->dev->name);
-			return 0;
-		case SHAPER_SET_SPEED:
-			shaper_setspeed(sh,ss->ss_speed);
-			return 0;
-		case SHAPER_GET_SPEED:
-			ss->ss_speed=sh->bitspersec;
-			return 0;
-		default:
-			return -EINVAL;
-	}
-}
-
-static void shaper_init_priv(struct net_device *dev)
-{
-	struct shaper *sh = dev->priv;
-
-	skb_queue_head_init(&sh->sendq);
-	init_timer(&sh->timer);
-	sh->timer.function=shaper_timer;
-	sh->timer.data=(unsigned long)sh;
-	spin_lock_init(&sh->lock);
-}
-
-/*
- *	Add a shaper device to the system
- */
-
-static void __init shaper_setup(struct net_device *dev)
-{
-	/*
-	 *	Set up the shaper.
-	 */
-
-	SET_MODULE_OWNER(dev);
-
-	shaper_init_priv(dev);
-
-	dev->open		= shaper_open;
-	dev->stop		= shaper_close;
-	dev->hard_start_xmit 	= shaper_start_xmit;
-	dev->get_stats 		= shaper_get_stats;
-	dev->set_multicast_list = NULL;
-
-	/*
-	 *	Intialise the packet queues
-	 */
-
-	/*
-	 *	Handlers for when we attach to a device.
-	 */
-
-	dev->hard_header 	= shaper_header;
-	dev->rebuild_header 	= shaper_rebuild_header;
-#if 0
-	dev->hard_header_cache	= shaper_cache;
-	dev->header_cache_update= shaper_cache_update;
-#endif
-	dev->neigh_setup	= shaper_neigh_setup_dev;
-	dev->do_ioctl		= shaper_ioctl;
-	dev->hard_header_len	= 0;
-	dev->type		= ARPHRD_ETHER;	/* initially */
-	dev->set_mac_address	= NULL;
-	dev->mtu		= 1500;
-	dev->addr_len		= 0;
-	dev->tx_queue_len	= 10;
-	dev->flags		= 0;
-}
-
-static int shapers = 1;
-#ifdef MODULE
-
-module_param(shapers, int, 0);
-MODULE_PARM_DESC(shapers, "Traffic shaper: maximum number of shapers");
-
-#else /* MODULE */
-
-static int __init set_num_shapers(char *str)
-{
-	shapers = simple_strtol(str, NULL, 0);
-	return 1;
-}
-
-__setup("shapers=", set_num_shapers);
-
-#endif /* MODULE */
-
-static struct net_device **devs;
-
-static unsigned int shapers_registered = 0;
-
-static int __init shaper_init(void)
-{
-	int i;
-	size_t alloc_size;
-	struct net_device *dev;
-	char name[IFNAMSIZ];
-
-	if (shapers < 1)
-		return -ENODEV;
-
-	alloc_size = sizeof(*dev) * shapers;
-	devs = kzalloc(alloc_size, GFP_KERNEL);
-	if (!devs)
-		return -ENOMEM;
-
-	for (i = 0; i < shapers; i++) {
-
-		snprintf(name, IFNAMSIZ, "shaper%d", i);
-		dev = alloc_netdev(sizeof(struct shaper), name,
-				   shaper_setup);
-		if (!dev)
-			break;
-
-		if (register_netdev(dev)) {
-			free_netdev(dev);
-			break;
-		}
-
-		devs[i] = dev;
-		shapers_registered++;
-	}
-
-	if (!shapers_registered) {
-		kfree(devs);
-		devs = NULL;
-	}
-
-	return (shapers_registered ? 0 : -ENODEV);
-}
-
-static void __exit shaper_exit (void)
-{
-	int i;
-
-	for (i = 0; i < shapers_registered; i++) {
-		if (devs[i]) {
-			unregister_netdev(devs[i]);
-			free_netdev(devs[i]);
-		}
-	}
-
-	kfree(devs);
-	devs = NULL;
-}
-
-module_init(shaper_init);
-module_exit(shaper_exit);
-MODULE_LICENSE("GPL");
-
--- a/include/linux/if_shaper.h	2007-08-17 14:18:47.000000000 -0700
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,63 +0,0 @@
-#ifndef __LINUX_SHAPER_H
-#define __LINUX_SHAPER_H
-
-#ifdef __KERNEL__
-
-#define SHAPER_QLEN	10
-/*
- *	This is a bit speed dependent (read it shouldn't be a constant!)
- *
- *	5 is about right for 28.8 upwards. Below that double for every
- *	halving of speed or so. - ie about 20 for 9600 baud.
- */
-#define SHAPER_LATENCY	(5*HZ)
-#define SHAPER_MAXSLIP	2
-#define SHAPER_BURST	(HZ/50)		/* Good for >128K then */
-
-struct shaper
-{
-	struct sk_buff_head sendq;
-	__u32 bytespertick;
-	__u32 bitspersec;
-	__u32 shapelatency;
-	__u32 shapeclock;
-	unsigned long recovery;	/* Time we can next clock a packet out on
-				   an empty queue */
-	spinlock_t lock;
-        struct net_device_stats stats;
-	struct net_device *dev;
-	int  (*hard_start_xmit) (struct sk_buff *skb,
-		struct net_device *dev);
-	int  (*hard_header) (struct sk_buff *skb,
-		struct net_device *dev,
-		unsigned short type,
-		void *daddr,
-		void *saddr,
-		unsigned len);
-	int  (*rebuild_header)(struct sk_buff *skb);
-	int (*hard_header_cache)(struct neighbour *neigh, struct hh_cache *hh);
-	void (*header_cache_update)(struct hh_cache *hh, struct net_device *dev, unsigned char *  haddr);
-	struct net_device_stats* (*get_stats)(struct net_device *dev);
-	struct timer_list timer;
-};
-
-#endif
-
-#define SHAPER_SET_DEV		0x0001
-#define SHAPER_SET_SPEED	0x0002
-#define SHAPER_GET_DEV		0x0003
-#define SHAPER_GET_SPEED	0x0004
-
-struct shaperconf
-{
-	__u16	ss_cmd;
-	union
-	{
-		char 	ssu_name[14];
-		__u32	ssu_speed;
-	} ss_u;
-#define ss_speed ss_u.ssu_speed
-#define ss_name ss_u.ssu_name
-};
-
-#endif
--- a/Documentation/networking/shaper.txt	2007-08-17 14:18:24.000000000 -0700
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,48 +0,0 @@
-Traffic Shaper For Linux
-
-This is the current BETA release of the traffic shaper for Linux. It works
-within the following limits:
-
-o	Minimum shaping speed is currently about 9600 baud (it can only
-shape down to 1 byte per clock tick)
-
-o	Maximum is about 256K, it will go above this but get a bit blocky.
-
-o	If you ifconfig the master device that a shaper is attached to down
-then your machine will follow.
-
-o	The shaper must be a module.
-
-
-Setup:
-
-	A shaper device is configured using the shapeconfig program.
-Typically you will do something like this
-
-shapecfg attach shaper0 eth1
-shapecfg speed shaper0 64000
-ifconfig shaper0 myhost netmask 255.255.255.240 broadcast 1.2.3.4.255 up
-route add -net some.network netmask a.b.c.d dev shaper0
-
-The shaper should have the same IP address as the device it is attached to
-for normal use.
-
-Gotchas:
-
-	The shaper shapes transmitted traffic. It's rather impossible to
-shape received traffic except at the end (or a router) transmitting it.
-
-	Gated/routed/rwhod/mrouted all see the shaper as an additional device
-and will treat it as such unless patched. Note that for mrouted you can run
-mrouted tunnels via a traffic shaper to control bandwidth usage.
-
-	The shaper is device/route based. This makes it very easy to use
-with any setup BUT less flexible. You may need to use iproute2 to set up
-multiple route tables to get the flexibility.
-
-	There is no "borrowing" or "sharing" scheme. This is a simple
-traffic limiter. We implement Van Jacobson and Sally Floyd's CBQ
-architecture into Linux 2.2. This is the preferred solution. Shaper is
-for simple or back compatible setups.
-
-Alan

^ permalink raw reply

* Re: [RFC] remove obsolete shaper code
From: David Miller @ 2007-08-23  0:01 UTC (permalink / raw)
  To: shemminger; +Cc: alan, netdev
In-Reply-To: <20070822165854.7a65f099@freepuppy.rosehill.hemminger.net>

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Wed, 22 Aug 2007 16:58:54 -0700

> This driver is obsolete, and fixing it for adding upcoming changes
> to make hard_header_ops
> is problematic because it changes the underlying device handles.
> 
> The driver has been superseded by iproute2/traffic control for
> a long time. Let's kill it.
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

Please do this with a proper feature-removal-schedule.txt entry and
timeframe.

^ permalink raw reply

* [PATCH] IOAT: ioatdma needs to to play nice in a multi-dma-client world
From: Shannon Nelson @ 2007-08-23  0:12 UTC (permalink / raw)
  To: netdev, davem, akpm, linux-kernel; +Cc: shannon.nelson, dan.j.williams, jgarzik

Now that the DMA engine has a multi-client interface, fix the ioatdma
driver to play along.  At the same time, remove a couple of unnecessary
reads and writes.

Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
---

 drivers/dma/ioatdma.c |   18 ++++--------------
 1 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/dma/ioatdma.c b/drivers/dma/ioatdma.c
index 2d1f178..41b18c5 100644
--- a/drivers/dma/ioatdma.c
+++ b/drivers/dma/ioatdma.c
@@ -191,17 +191,12 @@ static int ioat_dma_alloc_chan_resources(struct dma_chan *chan)
 	int i;
 	LIST_HEAD(tmp_list);
 
-	/*
-	 * In-use bit automatically set by reading chanctrl
-	 * If 0, we got it, if 1, someone else did
-	 */
-	chanctrl = readw(ioat_chan->reg_base + IOAT_CHANCTRL_OFFSET);
-	if (chanctrl & IOAT_CHANCTRL_CHANNEL_IN_USE)
-		return -EBUSY;
+	/* have we already been set up? */
+	if (!list_empty(&ioat_chan->free_desc))
+		return INITIAL_IOAT_DESC_COUNT;
 
         /* Setup register to interrupt and write completion status on error */
-	chanctrl = IOAT_CHANCTRL_CHANNEL_IN_USE |
-		IOAT_CHANCTRL_ERR_INT_EN |
+	chanctrl = IOAT_CHANCTRL_ERR_INT_EN |
 		IOAT_CHANCTRL_ANY_ERR_ABORT_EN |
 		IOAT_CHANCTRL_ERR_COMPLETION_EN;
         writew(chanctrl, ioat_chan->reg_base + IOAT_CHANCTRL_OFFSET);
@@ -282,11 +277,6 @@ static void ioat_dma_free_chan_resources(struct dma_chan *chan)
 			in_use_descs - 1);
 
 	ioat_chan->last_completion = ioat_chan->completion_addr = 0;
-
-	/* Tell hw the chan is free */
-	chanctrl = readw(ioat_chan->reg_base + IOAT_CHANCTRL_OFFSET);
-	chanctrl &= ~IOAT_CHANCTRL_CHANNEL_IN_USE;
-	writew(chanctrl, ioat_chan->reg_base + IOAT_CHANCTRL_OFFSET);
 }
 
 static struct dma_async_tx_descriptor *

^ permalink raw reply related

* Re: [PATCH] [02/10] pasemi_mac: Stop using the pci config space accessors for register read/writes
From: Stephen Rothwell @ 2007-08-23  0:31 UTC (permalink / raw)
  To: Olof Johansson; +Cc: jgarzik, netdev, linuxppc-dev
In-Reply-To: <20070822141248.GC16830@lixom.net>

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

Hi Olof,

On Wed, 22 Aug 2007 09:12:48 -0500 Olof Johansson <olof@lixom.net> wrote:
>
> -static unsigned int read_iob_reg(struct pasemi_mac *mac, unsigned int reg)
> +static inline unsigned int read_iob_reg(struct pasemi_mac *mac, unsigned int reg)
          ^^^^^^
For static functions in C files, we tend not to bother marking them
inline any more as the compiler does a pretty good job theses days.

>  {
>  	unsigned int val;
>  
> -	pci_read_config_dword(mac->iob_pdev, reg, &val);
> +	val = in_le32(mac->iob_regs+reg);
> +
>  	return val;

Why not just "return in_le32(mac->iob_regs+reg);" ?
And similarly below?

> +static inline void __iomem * __devinit map_onedev(struct pci_dev *p, int index)
          ^^^^^^                ^^^^^^^^^
Mixing inline and __*init is just plain silly :-)

> +fallback:
> +	/* This is hardcoded and ugly, but we have some firmware versions
> +	 * who don't provide the register space in the device tree. Luckily
           ^^^
"that"

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

^ permalink raw reply

* Re: [PATCH] improved xfrm_audit_log() patch
From: Joy Latten @ 2007-08-23  1:29 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-audit, sgrubb
In-Reply-To: <20070822.125157.43008100.davem@davemloft.net>

On Wed, 2007-08-22 at 12:51 -0700, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Tue, 21 Aug 2007 00:24:05 -0700 (PDT)
> 
> > Looks good, applied to net-2.6.24, thanks Joy.
> 
> Something is still buggered up in this patch, you can't add this local
> "audit_info" variable unconditionally to these functions, and
> alternatively you also can't add a bunch of ifdefs to xfrm_user.c to
> cover it up either.
> 
I wonder if I am subconsciously trying to break a record or 
something! My apologies as time is valuable. 

I mean to get this right. My rationale for using audit_info was to
reduce amount of arguments to xfrm_audit_log(). However, I now like
it better when I just called xfrm_audit_log(NETLINK_CB(skb).loginuid,
NETLINK_CB(skb).sid, ...). User determines where/how to get loginuid and
secid and nothing happens when AUDIT not configured. But would make
xfrm_audit_log() have 7 arguments instead of 6.

My alternative is to remove xfrm_get_auditinfo() out of the 
#ifdef CONFIG_AUDITSYSCALL and always fill in audit_info
regardless if AUDIT is configured or not. Less calls to
xfrm_audit_log() but perhaps unnecessary info when AUDIT 
not configured.

Would first solution be acceptable?

Joy   



^ permalink raw reply

* Re: [PATCH 0/9 Rev3] Implement batching skb API and support in IPoIB
From: Krishna Kumar2 @ 2007-08-23  2:43 UTC (permalink / raw)
  To: David Miller
  Cc: gaagaan, general, hadi, herbert, jagana, jeff, johnpol, kaber,
	kumarkr, mcarlson, mchan, netdev, peter.p.waskiewicz.jr, rdreier,
	rick.jones2, Robert.Olsson, shemminger, sri, tgraf, xma
In-Reply-To: <20070822.021440.123973929.davem@davemloft.net>

David Miller <davem@davemloft.net> wrote on 08/22/2007 02:44:40 PM:

> From: Krishna Kumar2 <krkumar2@in.ibm.com>
> Date: Wed, 22 Aug 2007 12:33:04 +0530
>
> > Does turning off batching solve that problem? What I mean by that is:
> > batching can be disabled if a TSO device is worse for some cases.
>
> This new batching stuff isn't going to be enabled or disabled
> on a per-device basis just to get "parity" with how things are
> now.
>
> It should be enabled by default, and give at least as good
> performance as what can be obtained right now.

That was how it was in earlier revisions. In revision4 I coded it so
that it is enabled only if explicitly set by the user. I can revert
that change.

> Otherwise it's a clear regression.

Definitely. For drivers that support it, it should not reduce performance.

Thanks,

- KK


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox