Netdev List
 help / color / mirror / Atom feed
* Re: [usb regression] Re: [PATCH 2.6.24-rc3] Fix /proc/net breakage
From: Greg KH @ 2007-12-31 19:26 UTC (permalink / raw)
  To: Alan Stern
  Cc: Andreas Mohr, Ingo Molnar, Alexey Dobriyan, Andrew Morton,
	David Woodhouse, Eric W. Biederman, Linus Torvalds,
	Rafael J. Wysocki, Pavel Machek, kernel list, netdev,
	Pavel Emelyanov, Denis V. Lunev, USB list
In-Reply-To: <Pine.LNX.4.44L0.0712311237150.7182-100000@iolanthe.rowland.org>

On Mon, Dec 31, 2007 at 12:49:52PM -0500, Alan Stern wrote:
> On Sun, 30 Dec 2007, Greg KH wrote:
> 
> > > It looks like Greg misused the debugfs API -- which is ironic, because
> > > he wrote debugfs in the first place!  :-)
> > 
> > Oh crap, sorry, I did mess that up :(
> > 
> > > Let me know if this patch fixes the problem.  If it does, I'll submit 
> > > it to Greg with all the proper accoutrements.
> > 
> > This isn't going to work if CONFIG_DEBUGFS is not enabled either :(
> 
> Why not?  The debugging files won't be created, true, but the driver
> should work just fine regardless.  This is exactly the way uhci-hcd
> behaves, and it hasn't caused any problems.

Ok, you are right, it was late and after some wine and I shouldn't have
been responding to email :)

I'll rework this, and send out a patch tonight or tomorrow when I get a
chance (am on the road right now).  We should only care about NULL being
returned here, that's the only "real" error.

And in thinking about it some more, I should go audit all the debugfs
callers to make sure they are doing something sane too, it shouldn't be
this complex at all...

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH] [IPROUTE2] IPT action compatibility with iptables 1.4.0
From: Stephen Hemminger @ 2007-12-31 19:16 UTC (permalink / raw)
  To: hadi; +Cc: Denys Fedoryshchenko, netdev, Pablo Neira Ayuso
In-Reply-To: <1198515305.4427.12.camel@localhost>

On Mon, 24 Dec 2007 11:55:05 -0500
jamal <hadi@cyberus.ca> wrote:

> 
> Stephen,
> 
> Please apply this patch from Denys Fedoryshchenko to make the ipt action
> work with latest iptables.
> 
> cheers,
> jamal
> 

applied and pushed

-- 
Stephen Hemminger <stephen.hemminger@vyatta.com>

^ permalink raw reply

* Re: [PATCH] Force UNIX domain sockets to be built in
From: Michael Buesch @ 2007-12-31 19:05 UTC (permalink / raw)
  To: Torsten Kaiser
  Cc: Alan Cox, Adrian Bunk, Bodo Eggert, Jan Engelhardt, devzero,
	linux-kernel, netdev
In-Reply-To: <64bb37e0712311037w42bae43ayb3c10b2e0310586d@mail.gmail.com>

On Monday 31 December 2007 19:37:43 Torsten Kaiser wrote:
> The base problem is that there already are many options to break
> external modules. (CONFIG_MODULES=n ;) )

Exactly. There already are enough ways to break external modules.
No need to introduce more. ;)

> The question I can't answer in this context is: Do distributions want
> to support external modules?
> Only if yes, your argument is valid. But then they could just disable
> this feature and prevent this kind of bugreports.

That's my point. Nobody will risk bugs to save a few bytes of memory.

-- 
Greetings Michael.

^ permalink raw reply

* Re: [PATCH 0/4] New interface for memory accounting (take 1)
From: Hideo AOKI @ 2007-12-31 19:03 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, herbert, vladislav.yasevich, netdev,
	lksctp-developers, tyasui, mhiramat, satoshi.oshima.fk, billfink,
	andi, johnpol, shemminger, yoshfuji, yumiko.sugita.yf, haoki
In-Reply-To: <47790811.9080004@cosmosbay.com>

Eric Dumazet wrote:

> Hi David
> 
> Could you add the following patch, because it apparently was lost
> during the battle :)
> 
> Thank you
> 
> [PATCH] use SK_MEM_QUANTUM_SHIFT in __sk_mem_reclaim()
> 
> Avoid an expensive divide (as done in commit
> 18030477e70a826b91608aee40a987bbd368fec6 but lost in commit
> 23821d2653111d20e75472c8c5003df1a55309a8)

Hello Eric,

Thank you for catching this. I'm sorry about the lost.

Best regards,
Hideo

-- 
Hitachi Computer Products (America) Inc.

^ permalink raw reply

* Re: [PATCH 2/3] [UDP]: memory accounting in IPv4
From: Eric Dumazet @ 2007-12-31 18:58 UTC (permalink / raw)
  To: Hideo AOKI
  Cc: David Miller, Herbert Xu, netdev, Takahiro Yasui,
	Masami Hiramatsu, Satoshi Oshima, billfink, Andi Kleen,
	Evgeniy Polyakov, Stephen Hemminger, yoshfuji, Yumiko Sugita
In-Reply-To: <47793836.10407@redhat.com>

Hideo AOKI a écrit :
> Eric Dumazet wrote:
>> Hideo AOKI a écrit :
>>> This patch adds UDP memory usage accounting in IPv4. Currently,
>>> receiving buffer accounting is only supported.
>>>
>>> This patch is also introduced memory_allocated variable for UDP protocol.
>> OK, but sockstat_seq_show() should be updated so that this
>> udp_memory_allocated value can be read from /proc/net/sockstat ?
> 
> Yes, and I updated it in the first patch of the patch set.
> 
> I thought dividing patch made easy to review. But I was wrong.
> Sorry about that.

No problem Hideo, I think you did a good job with this UDP memory accounting 
and were very patient.

Thank you




^ permalink raw reply

* Re: [PATCH 1/3] [UDP]: add udp_mem, udp_rmem_min and udp_wmem_min
From: Hideo AOKI @ 2007-12-31 18:58 UTC (permalink / raw)
  To: David Miller
  Cc: herbert, netdev, tyasui, mhiramat, satoshi.oshima.fk, billfink,
	andi, johnpol, shemminger, yoshfuji, yumiko.sugita.yf, haoki
In-Reply-To: <20071231.001925.151533664.davem@davemloft.net>

David Miller wrote:
> From: Hideo AOKI <haoki@redhat.com>
> Date: Sun, 30 Dec 2007 04:01:46 -0500
> 
>> diff -pruN net-2.6.25-t12t19m-p4/net/ipv4/proc.c net-2.6.25-t12t19m-p5/net/ipv4/proc.c
>> --- net-2.6.25-t12t19m-p4/net/ipv4/proc.c	2007-12-27 10:19:02.000000000 -0500
>> +++ net-2.6.25-t12t19m-p5/net/ipv4/proc.c	2007-12-29 21:09:21.000000000 -0500
>> @@ -56,7 +56,8 @@ static int sockstat_seq_show(struct seq_
>>  		   sock_prot_inuse(&tcp_prot), atomic_read(&tcp_orphan_count),
>>  		   tcp_death_row.tw_count, atomic_read(&tcp_sockets_allocated),
>>  		   atomic_read(&tcp_memory_allocated));
>> -	seq_printf(seq, "UDP: inuse %d\n", sock_prot_inuse(&udp_prot));
>> +	seq_printf(seq, "UDP: inuse %d mem %d\n", sock_prot_inuse(&udp_prot),
>> +		   atomic_read(&udp_memory_allocated));
>>  	seq_printf(seq, "UDPLITE: inuse %d\n", sock_prot_inuse(&udplite_prot));
>>  	seq_printf(seq, "RAW: inuse %d\n", sock_prot_inuse(&raw_prot));
>>  	seq_printf(seq,  "FRAG: inuse %d memory %d\n",
> 
> More careless patch creation.  :-/
> 
> This breaks the build because udp_memory_allocated is not added until
> patch 2.

Hello David,

You are absolutely right.

> Once again I'll combine all three patches into one but I am extremely
> angry about how careless and broken these two patch submissions were.

I really apologize for wasting your time twice.
I do not do this kind mess again.

Many thanks for your support.

Best regards,
Hideo

-- 
Hitachi Computer Products (America) Inc.

^ permalink raw reply

* Re: [PATCH] net: santize headers for iproute2
From: Stephen Hemminger @ 2007-12-31 18:55 UTC (permalink / raw)
  To: David Miller; +Cc: vgusev, netdev
In-Reply-To: <20071229.192212.149111967.davem@davemloft.net>

The Glib 2.7 version of netinet/tcp.h works for building iproute2,
but since iproute needs to build on older versions of glibc;
for now, I'll just put a corrected version of netinet/tcp.h in the iproute2
build tree. 

^ permalink raw reply

* Re: [PATCH 2/4] [CORE]: adding memory accounting points
From: Hideo AOKI @ 2007-12-31 18:52 UTC (permalink / raw)
  To: David Miller
  Cc: herbert, vladislav.yasevich, netdev, lksctp-developers, tyasui,
	mhiramat, satoshi.oshima.fk, billfink, andi, johnpol, shemminger,
	yoshfuji, yumiko.sugita.yf, haoki
In-Reply-To: <20071230.235806.171376126.davem@davemloft.net>

David Miller wrote:

> This patch would not apply, because is contained changes
> present in the first patch, specifically:
<snip>
> And now I see exactly what you did, and it is quite careless.
> 
> You wrote one big patch then tried to split it up by hand.  This
> proves to me that you did not test the patches individually.  Even
> worse, you did not even try to apply each patch nor compile the tree
> each step along the way as a basic sanity check.

Hello David,

You are right. Since I felt the patch was big, I divided
into three for review. And I mistook during the dividing.

> This wastes a lot of my time, as well as the time of other developers
> who might want to try out and test your changes.

I apologize for wasting your time.


> I will fix it up this time, but please do not ever do this again.

I really appreciate the fix. And I understood this.

Best regards,
Hideo

-- 
Hitachi Computer Products (America) Inc.

^ permalink raw reply

* Re: [PATCH 0/4] New interface for memory accounting (take 1)
From: Hideo AOKI @ 2007-12-31 18:46 UTC (permalink / raw)
  To: David Miller
  Cc: herbert, vladislav.yasevich, netdev, lksctp-developers, tyasui,
	mhiramat, satoshi.oshima.fk, billfink, andi, johnpol, shemminger,
	yoshfuji, yumiko.sugita.yf, haoki
In-Reply-To: <20071230.233413.117211235.davem@davemloft.net>

Hello David,

David Miller wrote:
> From: Hideo AOKI <haoki@redhat.com>
> Date: Sun, 30 Dec 2007 03:47:33 -0500
> 
>> The patch set consists of the following 4 patches.
>>
>> [1/4] introducing new memory accounting interface
>> [2/4] adding memory accounting points to consolidate functions
>> [3/4] updating TCP to use new interface
>> [4/4] updating SCTP to use new interface
> 
> I like this work very much and will add this to net-2.6.25
> But I will have to combine it all into one patch.
> 
> You cannot have one patch which breaks the build in any way.  All of
> the kernel must build properly after each patch in your patchset is
> applied.
> 
> Since patch 1 renames all of the sk_stream_*() functions, TCP and SCTP
> stop building.

That's correct.

> We enforce this rule, otherwise when users try to use "git bisect" to
> find out where regressions are added, they will get stuck in places
> like this where the tree will not build due to such careless
> changesets.

Thank you for your explanation. To be honest, I didn't know the rule
exactly. I am so sorry for sending inconvenient patch set.

Best regards,
Hideo

-- 
Hitachi Computer Products (America) Inc.

^ permalink raw reply

* Re: [PATCH 2/3] [UDP]: memory accounting in IPv4
From: Hideo AOKI @ 2007-12-31 18:43 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, Herbert Xu, netdev, Takahiro Yasui,
	Masami Hiramatsu, Satoshi Oshima, billfink, Andi Kleen,
	Evgeniy Polyakov, Stephen Hemminger, yoshfuji, Yumiko Sugita,
	haoki
In-Reply-To: <477764B4.2030200@cosmosbay.com>

Eric Dumazet wrote:
> Hideo AOKI a écrit :
>> This patch adds UDP memory usage accounting in IPv4. Currently,
>> receiving buffer accounting is only supported.
>>
>> This patch is also introduced memory_allocated variable for UDP protocol.
> 
> OK, but sockstat_seq_show() should be updated so that this
> udp_memory_allocated value can be read from /proc/net/sockstat ?

Yes, and I updated it in the first patch of the patch set.

I thought dividing patch made easy to review. But I was wrong.
Sorry about that.

Regards,
Hideo

-- 
Hitachi Computer Products (America) Inc.

^ permalink raw reply

* Re: [PATCH] Force UNIX domain sockets to be built in
From: Torsten Kaiser @ 2007-12-31 18:37 UTC (permalink / raw)
  To: Michael Buesch
  Cc: Alan Cox, Adrian Bunk, Bodo Eggert, Jan Engelhardt, devzero,
	linux-kernel, netdev
In-Reply-To: <200712311818.12825.mb@bu3sch.de>

On Dec 31, 2007 6:18 PM, Michael Buesch <mb@bu3sch.de> wrote:
> On Monday 31 December 2007 17:38:03 Alan Cox wrote:
> > On Mon, 31 Dec 2007 17:17:19 +0100
> > "Torsten Kaiser" <just.for.lkml@googlemail.com> wrote:
> >
> > > a) this could be disabled during development if you want this
> > > b) this would even only affect development if you add new code that
> > > now needs a EXPORT_SYMBOL that was removed on an earlier build. And
> > > right now this would also need to trigger a rerun of depmod. And the
> > > same trigger could redo this garbage collect.
> > >
> > > Or am I missing something obvious?
> >
> > Development is not a phase seperate from use or distribution. A lot of
> > module testers for distributions will not be compiling their own modules
> > but loading in ones to test provided by their vendor - which may of
> > course then need different ksyms

I understand that point.
I just always assumed that kernel tests meant 'please test this patch'
and doing the compile yourself.

But I'm not convinced be the following:
> As an example, the whole purpose wireless-compat package is
> to load latest bleeding edge wireless stuff into a distribution kernel.
> So people are not required to recompile their kernels for using
> drivers that support their hardware.
> And guess what, it is used a _lot_. And lots of bugs are found with it.
> It increases our testing community a lot.

This looks more like a "regular" out-of-tree module for the purpose of
the suggested symbol garbage collector.
And for that case I already a 'don't use it then'-note.

The base problem is that there already are many options to break
external modules. (CONFIG_MODULES=n ;) )
Or in the case of this wireless module: CONFIG_CRYPTO_ARC4=n
(Without 'arc4' ieee80211_wep_init() will fail, that will fail
ieee80211_register_hw() and so no mac80211 driver could be loaded)

> So, all this wouldn't work, if kernel symbols could randomly get
> nuked by some "garbage collector".
>
> In practice, no distribution would use symbol garbage collection, as the
> only benefit from it would be an increased level of bugreports.

The question I can't answer in this context is: Do distributions want
to support external modules?
Only if yes, your argument is valid. But then they could just disable
this feature and prevent this kind of bugreports.

Torsten

^ permalink raw reply

* Re: [PATCH] Force UNIX domain sockets to be built in
From: Adrian Bunk @ 2007-12-31 18:05 UTC (permalink / raw)
  To: Bodo Eggert; +Cc: Jan Engelhardt, devzero, linux-kernel, netdev
In-Reply-To: <alpine.LSU.0.999.0712311553530.4211@be1.lrz>

On Mon, Dec 31, 2007 at 04:19:23PM +0100, Bodo Eggert wrote:
> On Mon, 31 Dec 2007, Adrian Bunk wrote:
> > On Mon, Dec 31, 2007 at 02:26:42PM +0100, Bodo Eggert wrote:
> > > On Mon, 31 Dec 2007, Adrian Bunk wrote:
> > > > On Mon, Dec 31, 2007 at 01:09:43PM +0100, Bodo Eggert wrote:
> 
> > > > > As suggested by Adrian Bunk, UNIX domain sockets should always be built in 
> > > > > on normal systems. This is especially true since udev needs these sockets
> > > > > and fails to run if UNIX=m.
> > > > > 
> > > > > Signed-Off-By: Bodo Eggert <7eggert@gmx.de>
> > > > > 
> > > > > ---
> > > > > Last minute change: I decided against making it a bool because embedded 
> > > > > folks might depend on a small kernel image. Edited in the patch below.
> > > > >...
> > > > 
> > > > Is this just a purely theoretical thought or is this a reasonable use 
> > > > case people actually use in practice?
> > >  
> > > For now, it's a theoretical thought, but having an embedded device, I can 
> > > see the reason for $EVERYTHING=m there.
> > 
> > The only advantage I see is that the kernel image you have to flash 
> > can be made smaller - with the disadvantage that the running kernel
> > is bigger by more than 10%.
> > 
> > If you don't believe me, try it yourself:
> > Build all drivers statically into your kernel, and then compare the 
> > vmlinux sizes with CONFIG_MODULES=n and CONFIG_MODULES=y.
> 
> If you'd aim for a small kernel image, you would build anything as a module 
> that is not requred for booting.
>...

Small kernel image to flash (e.g. due to a size limit there), if you 
would aim for a small running kernel you wouldn't use modules.

And I'm not so sure about the image size without seeing actual numbers, 
e.g. for your device.

Especially comparing your all-modular approach with a CONFIG_MODULES=n 
kernel using Denys' section garbage collection patches in both cases 
would be interesting.

I'm not claiming you are wrong, all I say is that I'm not convinced 
without actual numbers that you are right.

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

* Re: [patch 7/7] netxen: fix byte-swapping in tx and rx
From: Dhananjay Phadke @ 2007-12-31 18:08 UTC (permalink / raw)
  To: netdev; +Cc: Al Viro, jeff
In-Reply-To: <20071226235334.GF27894@ZenIV.linux.org.uk>

Here's the reworked patch.

This cleans up some unnecessary byte-swapping while setting up tx and
interpreting rx desc. The 64 bit rx status data should be converted
to host endian format only once and the macros just need to extract
bitfields.

This saves a spate of interrupts on pseries blades caused by buggy 
(non) processing rx status ring.

Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>

Index: netdev-2.6/drivers/net/netxen/netxen_nic.h
===================================================================
--- netdev-2.6.orig/drivers/net/netxen/netxen_nic.h
+++ netdev-2.6/drivers/net/netxen/netxen_nic.h
@@ -309,23 +309,26 @@ struct netxen_ring_ctx {
 	((cmd_desc)->port_ctxid |= ((var) & 0xF0))
 
 #define netxen_set_cmd_desc_flags(cmd_desc, val)	\
-	((cmd_desc)->flags_opcode &= ~cpu_to_le16(0x7f), \
-	(cmd_desc)->flags_opcode |= cpu_to_le16((val) & 0x7f))
+	(cmd_desc)->flags_opcode = ((cmd_desc)->flags_opcode & \
+		~cpu_to_le16(0x7f)) | cpu_to_le16((val) & 0x7f)
 #define netxen_set_cmd_desc_opcode(cmd_desc, val)	\
-	((cmd_desc)->flags_opcode &= ~cpu_to_le16(0x3f<<7), \
-	(cmd_desc)->flags_opcode |= cpu_to_le16(((val & 0x3f)<<7)))
+	(cmd_desc)->flags_opcode = ((cmd_desc)->flags_opcode & \
+		~cpu_to_le16((u16)0x3f << 7)) | cpu_to_le16(((val) & 0x3f) << 7)
 
 #define netxen_set_cmd_desc_num_of_buff(cmd_desc, val)	\
-	((cmd_desc)->num_of_buffers_total_length &= ~cpu_to_le32(0xff), \
-	(cmd_desc)->num_of_buffers_total_length |= cpu_to_le32((val) & 0xff))
+	(cmd_desc)->num_of_buffers_total_length = \
+		((cmd_desc)->num_of_buffers_total_length & \
+		~cpu_to_le32(0xff)) | cpu_to_le32((val) & 0xff)
 #define netxen_set_cmd_desc_totallength(cmd_desc, val)	\
-	((cmd_desc)->num_of_buffers_total_length &= ~cpu_to_le32(0xffffff00), \
-	(cmd_desc)->num_of_buffers_total_length |= cpu_to_le32(val << 8))
+	(cmd_desc)->num_of_buffers_total_length = \
+		((cmd_desc)->num_of_buffers_total_length & \
+		~cpu_to_le32((u32)0xffffff << 8)) | \
+		cpu_to_le32(((val) & 0xffffff) << 8)
 
 #define netxen_get_cmd_desc_opcode(cmd_desc)	\
-	((le16_to_cpu((cmd_desc)->flags_opcode) >> 7) & 0x003F)
+	((le16_to_cpu((cmd_desc)->flags_opcode) >> 7) & 0x003f)
 #define netxen_get_cmd_desc_totallength(cmd_desc)	\
-	(le32_to_cpu((cmd_desc)->num_of_buffers_total_length) >> 8)
+	((le32_to_cpu((cmd_desc)->num_of_buffers_total_length) >> 8) & 0xffffff)
 
 struct cmd_desc_type0 {
 	u8 tcp_hdr_offset;	/* For LSO only */
@@ -412,29 +415,29 @@ struct rcv_desc {
 #define netxen_get_sts_desc_lro_last_frag(status_desc)	\
 	(((status_desc)->lro & 0x80) >> 7)
 
-#define netxen_get_sts_port(status_desc)	\
-	(le64_to_cpu((status_desc)->status_desc_data) & 0x0F)
-#define netxen_get_sts_status(status_desc)	\
-	((le64_to_cpu((status_desc)->status_desc_data) >> 4) & 0x0F)
-#define netxen_get_sts_type(status_desc)	\
-	((le64_to_cpu((status_desc)->status_desc_data) >> 8) & 0x0F)
-#define netxen_get_sts_totallength(status_desc)	\
-	((le64_to_cpu((status_desc)->status_desc_data) >> 12) & 0xFFFF)
-#define netxen_get_sts_refhandle(status_desc)	\
-	((le64_to_cpu((status_desc)->status_desc_data) >> 28) & 0xFFFF)
-#define netxen_get_sts_prot(status_desc)	\
-	((le64_to_cpu((status_desc)->status_desc_data) >> 44) & 0x0F)
+#define netxen_get_sts_port(sts_data)	\
+	((sts_data) & 0x0F)
+#define netxen_get_sts_status(sts_data)	\
+	(((sts_data) >> 4) & 0x0F)
+#define netxen_get_sts_type(sts_data)	\
+	(((sts_data) >> 8) & 0x0F)
+#define netxen_get_sts_totallength(sts_data)	\
+	(((sts_data) >> 12) & 0xFFFF)
+#define netxen_get_sts_refhandle(sts_data)	\
+	(((sts_data) >> 28) & 0xFFFF)
+#define netxen_get_sts_prot(sts_data)	\
+	(((sts_data) >> 44) & 0x0F)
+#define netxen_get_sts_opcode(sts_data)	\
+	(((sts_data) >> 58) & 0x03F)
+
 #define netxen_get_sts_owner(status_desc)	\
 	((le64_to_cpu((status_desc)->status_desc_data) >> 56) & 0x03)
-#define netxen_get_sts_opcode(status_desc)	\
-	((le64_to_cpu((status_desc)->status_desc_data) >> 58) & 0x03F)
-
-#define netxen_clear_sts_owner(status_desc)	\
-	((status_desc)->status_desc_data &=	\
-	~cpu_to_le64(((unsigned long long)3) << 56 ))
-#define netxen_set_sts_owner(status_desc, val)	\
-	((status_desc)->status_desc_data |=	\
-	cpu_to_le64(((unsigned long long)((val) & 0x3)) << 56 ))
+#define netxen_set_sts_owner(status_desc, val)	{ \
+	(status_desc)->status_desc_data = \
+		((status_desc)->status_desc_data & \
+		~cpu_to_le64(0x3ULL << 56)) | \
+		cpu_to_le64((u64)((val) & 0x3) << 56); \
+}
 
 struct status_desc {
 	/* Bit pattern: 0-3 port, 4-7 status, 8-11 type, 12-27 total_length
Index: netdev-2.6/drivers/net/netxen/netxen_nic_init.c
===================================================================
--- netdev-2.6.orig/drivers/net/netxen/netxen_nic_init.c
+++ netdev-2.6/drivers/net/netxen/netxen_nic_init.c
@@ -1053,16 +1053,17 @@ static void netxen_process_rcv(struct ne
 {
 	struct pci_dev *pdev = adapter->pdev;
 	struct net_device *netdev = adapter->netdev;
-	int index = netxen_get_sts_refhandle(desc);
+	u64 sts_data = le64_to_cpu(desc->status_desc_data);
+	int index = netxen_get_sts_refhandle(sts_data);
 	struct netxen_recv_context *recv_ctx = &(adapter->recv_ctx[ctxid]);
 	struct netxen_rx_buffer *buffer;
 	struct sk_buff *skb;
-	u32 length = netxen_get_sts_totallength(desc);
+	u32 length = netxen_get_sts_totallength(sts_data);
 	u32 desc_ctx;
 	struct netxen_rcv_desc_ctx *rcv_desc;
 	int ret;
 
-	desc_ctx = netxen_get_sts_type(desc);
+	desc_ctx = netxen_get_sts_type(sts_data);
 	if (unlikely(desc_ctx >= NUM_RCV_DESC_RINGS)) {
 		printk("%s: %s Bad Rcv descriptor ring\n",
 		       netxen_nic_driver_name, netdev->name);
@@ -1102,7 +1103,7 @@ static void netxen_process_rcv(struct ne
 	skb = (struct sk_buff *)buffer->skb;
 
 	if (likely(adapter->rx_csum &&
-				netxen_get_sts_status(desc) == STATUS_CKSUM_OK)) {
+				netxen_get_sts_status(sts_data) == STATUS_CKSUM_OK)) {
 		adapter->stats.csummed++;
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 	} else
@@ -1191,7 +1192,6 @@ u32 netxen_process_rcv_ring(struct netxe
 			break;
 		}
 		netxen_process_rcv(adapter, ctxid, desc);
-		netxen_clear_sts_owner(desc);
 		netxen_set_sts_owner(desc, STATUS_OWNER_PHANTOM);
 		consumer = (consumer + 1) & (adapter->max_rx_desc_count - 1);
 		count++;
Index: netdev-2.6/drivers/net/netxen/netxen_nic_main.c
===================================================================
--- netdev-2.6.orig/drivers/net/netxen/netxen_nic_main.c
+++ netdev-2.6/drivers/net/netxen/netxen_nic_main.c
@@ -1144,16 +1144,8 @@ static int netxen_nic_xmit_frame(struct 
 		}
 	}
 
-	i = netxen_get_cmd_desc_totallength(&hw->cmd_desc_head[saved_producer]);
-
-	hw->cmd_desc_head[saved_producer].flags_opcode =
-		cpu_to_le16(hw->cmd_desc_head[saved_producer].flags_opcode);
-	hw->cmd_desc_head[saved_producer].num_of_buffers_total_length =
-	  cpu_to_le32(hw->cmd_desc_head[saved_producer].
-			  num_of_buffers_total_length);
-
 	spin_lock_bh(&adapter->tx_lock);
-	adapter->stats.txbytes += i;
+	adapter->stats.txbytes += skb->len;
 
 	/* Code to update the adapter considering how many producer threads
 	   are currently working */

^ permalink raw reply

* Re: [PATCH] ip[6]_tables.c: remove some inlines
From: Denys Vlasenko @ 2007-12-31 17:55 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Harald Welte, netdev, linux-kernel,
	Netfilter Development Mailinglist
In-Reply-To: <47792032.3020609@trash.net>

On Monday 31 December 2007 17:00, Patrick McHardy wrote:
> Denys Vlasenko wrote:
> > ip[6]_tables.c seem to abuse inline.
> >
> > This patch removes inlines except those which are used
> > by packet matching code and thus are performance-critical.
> >   
> 
> Some people also consider the ruleset replacement path performance
> critical, but overall I agree with your patch. I'm travelling currently
> though so it will take a few days until I'll apply it.
> 
> Do you have some numbers that show the actual difference these
> changes make?

Before:

$ size */*/*/ip*tables*.o
   text    data     bss     dec     hex filename
   6402     500      16    6918    1b06 net/ipv4/netfilter/ip_tables.o
   7130     500      16    7646    1dde net/ipv6/netfilter/ip6_tables.o

After:

$ size */*/*/ip*tables*.o
   text    data     bss     dec     hex filename
   6307     500      16    6823    1aa7 net/ipv4/netfilter/ip_tables.o
   7010     500      16    7526    1d66 net/ipv6/netfilter/ip6_tables.o

--
vda

^ permalink raw reply

* Re: [PATCH] Re: Nested VLAN causes recursive locking error
From: Jarek Poplawski @ 2007-12-31 17:54 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: Chuck Ebbert, Netdev
In-Reply-To: <20071231174555.GA3097@ami.dom.local>

On Mon, Dec 31, 2007 at 06:45:55PM +0100, Jarek Poplawski wrote:
> Patric, [...]

OOPS... Sorry Patrick!

Jarek P.

^ permalink raw reply

* Re: [PATCH] Force UNIX domain sockets to be built in
From: Adrian Bunk @ 2007-12-31 17:51 UTC (permalink / raw)
  To: Bodo Eggert; +Cc: Jan Engelhardt, devzero, linux-kernel, netdev
In-Reply-To: <alpine.LSU.0.999.0712311553530.4211@be1.lrz>

On Mon, Dec 31, 2007 at 04:19:23PM +0100, Bodo Eggert wrote:
> On Mon, 31 Dec 2007, Adrian Bunk wrote:
> > On Mon, Dec 31, 2007 at 02:26:42PM +0100, Bodo Eggert wrote:
> > > On Mon, 31 Dec 2007, Adrian Bunk wrote:
> > > > On Mon, Dec 31, 2007 at 01:09:43PM +0100, Bodo Eggert wrote:
> 
> > > > > As suggested by Adrian Bunk, UNIX domain sockets should always be built in 
> > > > > on normal systems. This is especially true since udev needs these sockets
> > > > > and fails to run if UNIX=m.
> > > > > 
> > > > > Signed-Off-By: Bodo Eggert <7eggert@gmx.de>
> > > > > 
> > > > > ---
> > > > > Last minute change: I decided against making it a bool because embedded 
> > > > > folks might depend on a small kernel image. Edited in the patch below.
> > > > >...
> > > > 
> > > > Is this just a purely theoretical thought or is this a reasonable use 
> > > > case people actually use in practice?
> > >  
> > > For now, it's a theoretical thought, but having an embedded device, I can 
> > > see the reason for $EVERYTHING=m there.
> > 
> > The only advantage I see is that the kernel image you have to flash 
> > can be made smaller - with the disadvantage that the running kernel
> > is bigger by more than 10%.
> > 
> > If you don't believe me, try it yourself:
> > Build all drivers statically into your kernel, and then compare the 
> > vmlinux sizes with CONFIG_MODULES=n and CONFIG_MODULES=y.
> 
> If you'd aim for a small kernel image, you would build anything as a module 
> that is not requred for booting.
> 
> > > > After all, changing it to a bool will allow us to make the kernel image 
> > > > for nearly everyone smaller by a few hundred bytes...
> > > 
> > > I can't see why optionally building it as a module would force us to make 
> > > the kernel bigger. It may be a little more ugly to support =m, but thats it,
> > > isn't it?
> > 
> > On architectures like x86 where __exit code is freed at runtime 
> > af_unix_exit() makes your kernel image (but not the running kernel) 
> > bigger.
> > 
> > With CONFIG_MODULES=y the 13 EXPORT_SYMBOL's that only exist for the 
> > theoretical possibility of CONIG_UNIX=m waste a few hundred bytes 
> > of memory.
> 
> #define m='m'
> #if CONIG_UNIX=='m'
> #define EXPORT_SYMBOL_AF_UNIX EXPORT_SYMBOL
> #else
> #define EXPORT_SYMBOL_AF_UNIX()
> #endif
> #undef m
> 
> You could also use "#if defined(C_U) && (C_U == m)".

Look at the number of exports in the kernel and the number of lines 
changed per months, and it becomes obvious that somwthing like that
would not be maintainable.

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

* Re: [usb regression] Re: [PATCH 2.6.24-rc3] Fix /proc/net breakage
From: Alan Stern @ 2007-12-31 17:49 UTC (permalink / raw)
  To: Greg KH
  Cc: Andreas Mohr, Ingo Molnar, Alexey Dobriyan, Andrew Morton,
	David Woodhouse, Eric W. Biederman, Linus Torvalds,
	Rafael J. Wysocki, Pavel Machek, kernel list, netdev,
	Pavel Emelyanov, Denis V. Lunev, USB list
In-Reply-To: <20071231052500.GB4187@kroah.com>

On Sun, 30 Dec 2007, Greg KH wrote:

> > It looks like Greg misused the debugfs API -- which is ironic, because
> > he wrote debugfs in the first place!  :-)
> 
> Oh crap, sorry, I did mess that up :(
> 
> > Let me know if this patch fixes the problem.  If it does, I'll submit 
> > it to Greg with all the proper accoutrements.
> 
> This isn't going to work if CONFIG_DEBUGFS is not enabled either :(

Why not?  The debugging files won't be created, true, but the driver
should work just fine regardless.  This is exactly the way uhci-hcd
behaves, and it hasn't caused any problems.

> > Index: 2.6.24-rc6-mm1/drivers/usb/host/ohci-hcd.c
> > ===================================================================
> > --- 2.6.24-rc6-mm1.orig/drivers/usb/host/ohci-hcd.c
> > +++ 2.6.24-rc6-mm1/drivers/usb/host/ohci-hcd.c
> > @@ -1067,14 +1067,8 @@ static int __init ohci_hcd_mod_init(void
> >  
> >  #ifdef DEBUG
> >  	ohci_debug_root = debugfs_create_dir("ohci", NULL);
> > -	if (!ohci_debug_root || IS_ERR(ohci_debug_root)) {
> > -		if (!ohci_debug_root)
> > -			retval = -ENOENT;
> > -		else
> > -			retval = PTR_ERR(ohci_debug_root);
> > -
> > -		goto error_debug;
> > -	}
> > +	if (!ohci_debug_root)
> > +		return -ENOENT;
> 
> It needs to check for ERR_PTR(-ENODEV) which is the return value if
> debugfs is not enabled, and if so, just ignore things.

No.  That is the mistake you made before.  If debugfs isn't enabled 
then the driver should just ignore things, yes -- and in particular it 
should ignore the fact that the return code is ERR_PTR(-ENODEV).  No 
extra checking is required.

> If NULL is returned, or anything else, it's a real error.

If NULL is returned, it's a real error, agreed.  But if anything else
is returned then the call was successful as far as the driver is 
concerned.

> So, try something like:
> 	if (!ohci_debug_root) {
> 		retval = -ENOENT;
> 		goto error_debug;
> 	}
> 	if (IS_ERR(ohci_debug_root) && PTR_ERR(ohci_debug_root) != -ENODEV) {
> 		retval = PTR_ERR(ohci_debug_root);
> 		goto error_debug;
> 	}
> 
> and let me know of that works for you.

Greg, it sounds like you have forgotten the rationale you originally 
used for specifying the return codes in debugfs!  The idea was to 
return non-NULL if CONFIG_DEBUGFS was disabled, so that drivers could 
pretend the calls had succeeded and not need to worry about matters 
beyond their control.

Only a NULL return indicates a genuine error.  The kerneldoc for 
debugfs_create_dir says this very plainly.

> Although the combination of CONFIG_USB_DEBUG and not CONFIG_DEBUGFS is
> strange, we could just enable CONFIG_DEBUGFS is USB_DEBUG is enabled and
> then simplify this logic a bunch at the same time.

Most distributions enable CONFIG_DEBUGFS by default, don't they?  So 
this problem only comes up when people make up their own configs.  
Having USB_DEBUG enabled and DEBUGFS disabled seems like a perfectly 
reasonable combination to me.  I wouldn't change any aspect of the 
configuration logic.

Alan Stern


^ permalink raw reply

* Re: [IPSEC]: Move all calls to xfrm_audit_state_icvfail to xfrm_input
From: Paul Moore @ 2007-12-31 17:48 UTC (permalink / raw)
  To: Herbert Xu; +Cc: David S. Miller, netdev
In-Reply-To: <20071231042355.GA9215@gondor.apana.org.au>

On Sunday 30 December 2007 11:23:55 pm Herbert Xu wrote:
> Hi Dave:
>
> While refreshing the async IPsec patches I noticed some fresh code
> duplication.
>
> [IPSEC]: Move all calls to xfrm_audit_state_icvfail to xfrm_input
>
> Let's nip the code duplication in the bud :)

Thanks, not sure why I didn't see that in the first place :)

> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c
> index ec8de0a..d76803a 100644
> --- a/net/ipv4/ah4.c
> +++ b/net/ipv4/ah4.c
> @@ -179,10 +179,8 @@ static int ah_input(struct xfrm_state *x, struct
> sk_buff *skb) err = ah_mac_digest(ahp, skb, ah->auth_data);
>  		if (err)
>  			goto unlock;
> -		if (memcmp(ahp->work_icv, auth_data, ahp->icv_trunc_len)) {
> -			xfrm_audit_state_icvfail(x, skb, IPPROTO_AH);
> +		if (memcmp(ahp->work_icv, auth_data, ahp->icv_trunc_len))
>  			err = -EBADMSG;
> -		}
>  	}
>  unlock:
>  	spin_unlock(&x->lock);
> diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
> index b334c76..28ea5c7 100644
> --- a/net/ipv4/esp4.c
> +++ b/net/ipv4/esp4.c
> @@ -191,7 +191,6 @@ static int esp_input(struct xfrm_state *x, struct
> sk_buff *skb) BUG();
>
>  		if (unlikely(memcmp(esp->auth.work_icv, sum, alen))) {
> -			xfrm_audit_state_icvfail(x, skb, IPPROTO_ESP);
>  			err = -EBADMSG;
>  			goto unlock;
>  		}
> diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
> index 2d32772..fb0d07a 100644
> --- a/net/ipv6/ah6.c
> +++ b/net/ipv6/ah6.c
> @@ -380,10 +380,8 @@ static int ah6_input(struct xfrm_state *x, struct
> sk_buff *skb) err = ah_mac_digest(ahp, skb, ah->auth_data);
>  		if (err)
>  			goto unlock;
> -		if (memcmp(ahp->work_icv, auth_data, ahp->icv_trunc_len)) {
> -			xfrm_audit_state_icvfail(x, skb, IPPROTO_AH);
> +		if (memcmp(ahp->work_icv, auth_data, ahp->icv_trunc_len))
>  			err = -EBADMSG;
> -		}
>  	}
>  unlock:
>  	spin_unlock(&x->lock);
> diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
> index e10f10b..5bd5292 100644
> --- a/net/ipv6/esp6.c
> +++ b/net/ipv6/esp6.c
> @@ -186,7 +186,6 @@ static int esp6_input(struct xfrm_state *x, struct
> sk_buff *skb) BUG();
>
>  		if (unlikely(memcmp(esp->auth.work_icv, sum, alen))) {
> -			xfrm_audit_state_icvfail(x, skb, IPPROTO_ESP);
>  			ret = -EBADMSG;
>  			goto unlock;
>  		}
> diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
> index 1b250f3..039e701 100644
> --- a/net/xfrm/xfrm_input.c
> +++ b/net/xfrm/xfrm_input.c
> @@ -186,8 +186,11 @@ int xfrm_input(struct sk_buff *skb, int nexthdr,
> __be32 spi, int encap_type) resume:
>  		spin_lock(&x->lock);
>  		if (nexthdr <= 0) {
> -			if (nexthdr == -EBADMSG)
> +			if (nexthdr == -EBADMSG) {
> +				xfrm_audit_state_icvfail(x, skb,
> +							 x->type->proto);
>  				x->stats.integrity_failed++;
> +			}
>  			XFRM_INC_STATS(LINUX_MIB_XFRMINSTATEPROTOERROR);
>  			goto drop_unlock;
>  		}
>
> Thanks,



-- 
paul moore
linux security @ hp

^ permalink raw reply

* Re: [PATCH] Re: Nested VLAN causes recursive locking error
From: Jarek Poplawski @ 2007-12-31 17:45 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: Chuck Ebbert, Netdev
In-Reply-To: <477904F1.6090403@trash.net>

On Mon, Dec 31, 2007 at 04:04:17PM +0100, Patrick McHardy wrote:
> Jarek Poplawski wrote:
>> diff -Nurp linux-2.6.24-rc5-/net/8021q/vlan.c linux-2.6.24-rc5+/net/8021q/vlan.c
>> --- linux-2.6.24-rc5-/net/8021q/vlan.c	2007-12-17 13:29:19.000000000 +0100
>> +++ linux-2.6.24-rc5+/net/8021q/vlan.c	2007-12-20 14:21:02.000000000 +0100
>> @@ -307,12 +307,15 @@ int unregister_vlan_device(struct net_de
>>  	return ret;
>>  }
>>  +#ifdef CONFIG_LOCKDEP
>>  /*
>>   * vlan network devices have devices nesting below it, and are a special
>>   * "super class" of normal network devices; split their locks off into a
>>   * separate class since they always nest.
>>   */
>>  static struct lock_class_key vlan_netdev_xmit_lock_key;
>> +static int subclass; /* vlan nesting vlan */
>> +#endif
>>   static const struct header_ops vlan_header_ops = {
>>  	.create	 = vlan_dev_hard_header,
>> @@ -349,7 +352,14 @@ static int vlan_dev_init(struct net_devi
>>  		dev->hard_start_xmit = vlan_dev_hard_start_xmit;
>>  	}
>>  -	lockdep_set_class(&dev->_xmit_lock, &vlan_netdev_xmit_lock_key);
>> +#ifdef CONFIG_LOCKDEP
>> +	if ((real_dev->priv_flags & IFF_802_1Q_VLAN) &&
>> +	    subclass < MAX_LOCKDEP_SUBCLASSES - 1)
>> +		subclass++;
>> +
>
> That will increment the subclass globally, but it should actually just
> use real_dev->subclass + 1. Otherwise we'll permenently fail after
> registering 8 nested devices and unregistering them again.
>

Very good point! (As usual...) Actually, this shouldn't fail but work
with this one, last subclass only, so similarly like now, without this
patch.

Patric, currently I'm not sure this patch is very necessary... Since
I don't use vlans, I don't know how much it's real or theoretical
only problem. Probably the easiest solution would be limiting this
to two subclasses - any nested vlan gets second. Otherwise, it seems
there is some place needed to store these subclasses or use some
unofficial checks on lockdep's structures?

So, if you think this is useful and have better idea how this should
be done, I would be very glad if you re-do this your way (plus there
should be no problem with testing). Otherwise, give me some hint...

Thanks & Happy New Year!
Jarek P.

^ permalink raw reply

* Re: [PATCH] Force UNIX domain sockets to be built in
From: Patrick Mau @ 2007-12-31 17:43 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: Bodo Eggert, Adrian Bunk, devzero, linux-kernel, netdev
In-Reply-To: <Pine.LNX.4.64.0712311628330.28123@fbirervta.pbzchgretzou.qr>

On Mon, Dec 31, 2007 at 04:34:55PM +0100, Jan Engelhardt wrote:
> >
> >If you'd aim for a small kernel image, you would build anything as a module 
> >that is not requred for booting.
> >
> Yes, there is a tradeoff for both.
> 
> Example:
> 16:30 ichi:../net/802 > l fc.o fc.ko 
> -rw-r--r-- 1 jengelh users 7961 Dec 27 15:19 fc.ko
> -rw-r--r-- 1 jengelh users 2453 Dec 28 23:58 fc.o
> (from a recent not-so-complete patch turning CONFIG_FC etc. into =m)
> 
> If fc was modular, it might save the 2453 bytes off the core kernel image,
> but adds ~5508 bytes to disk.
> So one has to pick =y or =m depending on whatever suits his/her situation.

May I ask something that might be obvious for most of the
development community:

Modules have to be loaded in seperate pages, right ?

Does that mean that each module wastes partially used
pages of memory at runtime ?

I've always tried to build as much into the kernel image as
possible, because all of my systems have only 512M memory.

Thanks,
Patrick


^ permalink raw reply

* Re: [PATCH] Force UNIX domain sockets to be built in
From: Michael Buesch @ 2007-12-31 17:18 UTC (permalink / raw)
  To: Alan Cox
  Cc: Torsten Kaiser, Adrian Bunk, Bodo Eggert, Jan Engelhardt, devzero,
	linux-kernel, netdev
In-Reply-To: <20071231163803.33acb647@the-village.bc.nu>

On Monday 31 December 2007 17:38:03 Alan Cox wrote:
> On Mon, 31 Dec 2007 17:17:19 +0100
> "Torsten Kaiser" <just.for.lkml@googlemail.com> wrote:
> 
> > a) this could be disabled during development if you want this
> > b) this would even only affect development if you add new code that
> > now needs a EXPORT_SYMBOL that was removed on an earlier build. And
> > right now this would also need to trigger a rerun of depmod. And the
> > same trigger could redo this garbage collect.
> > 
> > Or am I missing something obvious?
> 
> Development is not a phase seperate from use or distribution. A lot of
> module testers for distributions will not be compiling their own modules
> but loading in ones to test provided by their vendor - which may of
> course then need different ksyms

As an example, the whole purpose wireless-compat package is
to load latest bleeding edge wireless stuff into a distribution kernel.
So people are not required to recompile their kernels for using
drivers that support their hardware.
And guess what, it is used a _lot_. And lots of bugs are found with it.
It increases our testing community a lot.

So, all this wouldn't work, if kernel symbols could randomly get
nuked by some "garbage collector".

In practice, no distribution would use symbol garbage collection, as the
only benefit from it would be an increased level of bugreports.

-- 
Greetings Michael.

^ permalink raw reply

* Re: [PATCH] ip[6]_tables.c: remove some inlines
From: Patrick McHardy @ 2007-12-31 17:00 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Harald Welte, netdev, linux-kernel,
	Netfilter Development Mailinglist
In-Reply-To: <200712311643.41946.vda.linux@googlemail.com>

Denys Vlasenko wrote:
> ip[6]_tables.c seem to abuse inline.
>
> This patch removes inlines except those which are used
> by packet matching code and thus are performance-critical.
>   

Some people also consider the ruleset replacement path performance
critical, but overall I agree with your patch. I'm travelling currently
though so it will take a few days until I'll apply it.

Do you have some numbers that show the actual difference these
changes make?



^ permalink raw reply

* Re: [PATCH] Force UNIX domain sockets to be built in
From: Alan Cox @ 2007-12-31 16:38 UTC (permalink / raw)
  To: Torsten Kaiser
  Cc: Michael Buesch, Adrian Bunk, Bodo Eggert, Jan Engelhardt, devzero,
	linux-kernel, netdev
In-Reply-To: <64bb37e0712310817g621d818dyebbf2228063d4d5f@mail.gmail.com>

On Mon, 31 Dec 2007 17:17:19 +0100
"Torsten Kaiser" <just.for.lkml@googlemail.com> wrote:

> a) this could be disabled during development if you want this
> b) this would even only affect development if you add new code that
> now needs a EXPORT_SYMBOL that was removed on an earlier build. And
> right now this would also need to trigger a rerun of depmod. And the
> same trigger could redo this garbage collect.
> 
> Or am I missing something obvious?

Development is not a phase seperate from use or distribution. A lot of
module testers for distributions will not be compiling their own modules
but loading in ones to test provided by their vendor - which may of
course then need different ksyms

Alan

^ permalink raw reply

* [PATCH] ip[6]_tables.c: remove some inlines
From: Denys Vlasenko @ 2007-12-31 16:43 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Harald Welte, netdev, linux-kernel,
	Netfilter Development Mailinglist
In-Reply-To: <4775169C.8040302@trash.net>

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

On Friday 28 December 2007 15:30, Patrick McHardy wrote:
> >> This clashes with my pending patches, which I'll push upstream
> >> today. I also spent some time resyncing ip_tables and ip6_tables
> >> so a diff of both (with some sed'ing) shows only the actual
> >> differences, so please update ip6_tables as well.
> >
> > I would be happy to rediff the patch.
> >
> > Which tree should I sync against in order to not collide
> > with your changes?
> 
> The net-2.6.25.git tree contains all my changes.

ip[6]_tables.c seem to abuse inline.

This patch removes inlines except those which are used
by packet matching code and thus are performance-critical.

Please take this patch into netfilter queue.

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
--
vda

[-- Attachment #2: net-2.6.25_iptable_deinlined.patch --]
[-- Type: text/x-diff, Size: 11649 bytes --]

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index f5b66ec..982b7f9 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -75,6 +75,7 @@ do {								\
    Hence the start of any table is given by get_table() below.  */
 
 /* Returns whether matches rule or not. */
+/* Performance critical - called for every packet */
 static inline bool
 ip_packet_match(const struct iphdr *ip,
 		const char *indev,
@@ -153,7 +154,7 @@ ip_packet_match(const struct iphdr *ip,
 	return true;
 }
 
-static inline bool
+static bool
 ip_checkentry(const struct ipt_ip *ip)
 {
 	if (ip->flags & ~IPT_F_MASK) {
@@ -183,8 +184,9 @@ ipt_error(struct sk_buff *skb,
 	return NF_DROP;
 }
 
-static inline
-bool do_match(struct ipt_entry_match *m,
+/* Performance critical - called for every packet */
+static inline bool
+do_match(struct ipt_entry_match *m,
 	      const struct sk_buff *skb,
 	      const struct net_device *in,
 	      const struct net_device *out,
@@ -199,6 +201,7 @@ bool do_match(struct ipt_entry_match *m,
 		return false;
 }
 
+/* Performance critical */
 static inline struct ipt_entry *
 get_entry(void *base, unsigned int offset)
 {
@@ -206,6 +209,7 @@ get_entry(void *base, unsigned int offset)
 }
 
 /* All zeroes == unconditional rule. */
+/* Mildly perf critical (only if packet tracing is on) */
 static inline int
 unconditional(const struct ipt_ip *ip)
 {
@@ -221,7 +225,7 @@ unconditional(const struct ipt_ip *ip)
 
 #if defined(CONFIG_NETFILTER_XT_TARGET_TRACE) || \
     defined(CONFIG_NETFILTER_XT_TARGET_TRACE_MODULE)
-static const char *hooknames[] = {
+static const char *const hooknames[] = {
 	[NF_INET_PRE_ROUTING]		= "PREROUTING",
 	[NF_INET_LOCAL_IN]		= "INPUT",
 	[NF_INET_FORWARD]		= "FORWARD",
@@ -235,7 +239,7 @@ enum nf_ip_trace_comments {
 	NF_IP_TRACE_COMMENT_POLICY,
 };
 
-static const char *comments[] = {
+static const char *const comments[] = {
 	[NF_IP_TRACE_COMMENT_RULE]	= "rule",
 	[NF_IP_TRACE_COMMENT_RETURN]	= "return",
 	[NF_IP_TRACE_COMMENT_POLICY]	= "policy",
@@ -251,6 +255,7 @@ static struct nf_loginfo trace_loginfo = {
 	},
 };
 
+/* Mildly perf critical (only if packet tracing is on) */
 static inline int
 get_chainname_rulenum(struct ipt_entry *s, struct ipt_entry *e,
 		      char *hookname, char **chainname,
@@ -567,7 +572,7 @@ mark_source_chains(struct xt_table_info *newinfo,
 	return 1;
 }
 
-static inline int
+static int
 cleanup_match(struct ipt_entry_match *m, unsigned int *i)
 {
 	if (i && (*i)-- == 0)
@@ -579,7 +584,7 @@ cleanup_match(struct ipt_entry_match *m, unsigned int *i)
 	return 0;
 }
 
-static inline int
+static int
 check_entry(struct ipt_entry *e, const char *name)
 {
 	struct ipt_entry_target *t;
@@ -600,7 +605,8 @@ check_entry(struct ipt_entry *e, const char *name)
 	return 0;
 }
 
-static inline int check_match(struct ipt_entry_match *m, const char *name,
+static int
+check_match(struct ipt_entry_match *m, const char *name,
 			      const struct ipt_ip *ip,
 			      unsigned int hookmask, unsigned int *i)
 {
@@ -623,7 +629,7 @@ static inline int check_match(struct ipt_entry_match *m, const char *name,
 	return ret;
 }
 
-static inline int
+static int
 find_check_match(struct ipt_entry_match *m,
 		 const char *name,
 		 const struct ipt_ip *ip,
@@ -652,7 +658,7 @@ err:
 	return ret;
 }
 
-static inline int check_target(struct ipt_entry *e, const char *name)
+static int check_target(struct ipt_entry *e, const char *name)
 {
 	struct ipt_entry_target *t;
 	struct xt_target *target;
@@ -673,7 +679,7 @@ static inline int check_target(struct ipt_entry *e, const char *name)
 	return ret;
 }
 
-static inline int
+static int
 find_check_entry(struct ipt_entry *e, const char *name, unsigned int size,
 		 unsigned int *i)
 {
@@ -717,7 +723,7 @@ find_check_entry(struct ipt_entry *e, const char *name, unsigned int size,
 	return ret;
 }
 
-static inline int
+static int
 check_entry_size_and_hooks(struct ipt_entry *e,
 			   struct xt_table_info *newinfo,
 			   unsigned char *base,
@@ -760,7 +766,7 @@ check_entry_size_and_hooks(struct ipt_entry *e,
 	return 0;
 }
 
-static inline int
+static int
 cleanup_entry(struct ipt_entry *e, unsigned int *i)
 {
 	struct ipt_entry_target *t;
@@ -916,7 +922,7 @@ get_counters(const struct xt_table_info *t,
 	}
 }
 
-static inline struct xt_counters * alloc_counters(struct xt_table *table)
+static struct xt_counters * alloc_counters(struct xt_table *table)
 {
 	unsigned int countersize;
 	struct xt_counters *counters;
@@ -1304,7 +1310,7 @@ do_replace(void __user *user, unsigned int len)
 
 /* We're lazy, and add to the first CPU; overflow works its fey magic
  * and everything is OK. */
-static inline int
+static int
 add_counter_to_entry(struct ipt_entry *e,
 		     const struct xt_counters addme[],
 		     unsigned int *i)
@@ -1465,7 +1471,7 @@ out:
 	return ret;
 }
 
-static inline int
+static int
 compat_find_calc_match(struct ipt_entry_match *m,
 		       const char *name,
 		       const struct ipt_ip *ip,
@@ -1489,7 +1495,7 @@ compat_find_calc_match(struct ipt_entry_match *m,
 	return 0;
 }
 
-static inline int
+static int
 compat_release_match(struct ipt_entry_match *m, unsigned int *i)
 {
 	if (i && (*i)-- == 0)
@@ -1499,7 +1505,7 @@ compat_release_match(struct ipt_entry_match *m, unsigned int *i)
 	return 0;
 }
 
-static inline int
+static int
 compat_release_entry(struct compat_ipt_entry *e, unsigned int *i)
 {
 	struct ipt_entry_target *t;
@@ -1514,7 +1520,7 @@ compat_release_entry(struct compat_ipt_entry *e, unsigned int *i)
 	return 0;
 }
 
-static inline int
+static int
 check_compat_entry_size_and_hooks(struct compat_ipt_entry *e,
 				  struct xt_table_info *newinfo,
 				  unsigned int *size,
@@ -1637,7 +1643,8 @@ compat_copy_entry_from_user(struct compat_ipt_entry *e, void **dstptr,
 	return ret;
 }
 
-static inline int compat_check_entry(struct ipt_entry *e, const char *name,
+static int
+compat_check_entry(struct ipt_entry *e, const char *name,
 				     unsigned int *i)
 {
 	int j, ret;
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 4ed16d2..dd7860f 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -90,6 +90,7 @@ ip6t_ext_hdr(u8 nexthdr)
 }
 
 /* Returns whether matches rule or not. */
+/* Performance critical - called for every packet */
 static inline bool
 ip6_packet_match(const struct sk_buff *skb,
 		 const char *indev,
@@ -182,7 +183,7 @@ ip6_packet_match(const struct sk_buff *skb,
 }
 
 /* should be ip6 safe */
-static inline bool
+static bool
 ip6_checkentry(const struct ip6t_ip6 *ipv6)
 {
 	if (ipv6->flags & ~IP6T_F_MASK) {
@@ -212,8 +213,9 @@ ip6t_error(struct sk_buff *skb,
 	return NF_DROP;
 }
 
-static inline
-bool do_match(struct ip6t_entry_match *m,
+/* Performance critical - called for every packet */
+static inline bool
+do_match(struct ip6t_entry_match *m,
 	      const struct sk_buff *skb,
 	      const struct net_device *in,
 	      const struct net_device *out,
@@ -236,6 +238,7 @@ get_entry(void *base, unsigned int offset)
 }
 
 /* All zeroes == unconditional rule. */
+/* Mildly perf critical (only if packet tracing is on) */
 static inline int
 unconditional(const struct ip6t_ip6 *ipv6)
 {
@@ -251,7 +254,7 @@ unconditional(const struct ip6t_ip6 *ipv6)
 #if defined(CONFIG_NETFILTER_XT_TARGET_TRACE) || \
     defined(CONFIG_NETFILTER_XT_TARGET_TRACE_MODULE)
 /* This cries for unification! */
-static const char *hooknames[] = {
+static const char *const hooknames[] = {
 	[NF_INET_PRE_ROUTING]		= "PREROUTING",
 	[NF_INET_LOCAL_IN]		= "INPUT",
 	[NF_INET_FORWARD]		= "FORWARD",
@@ -265,7 +268,7 @@ enum nf_ip_trace_comments {
 	NF_IP6_TRACE_COMMENT_POLICY,
 };
 
-static const char *comments[] = {
+static const char *const comments[] = {
 	[NF_IP6_TRACE_COMMENT_RULE]	= "rule",
 	[NF_IP6_TRACE_COMMENT_RETURN]	= "return",
 	[NF_IP6_TRACE_COMMENT_POLICY]	= "policy",
@@ -281,6 +284,7 @@ static struct nf_loginfo trace_loginfo = {
 	},
 };
 
+/* Mildly perf critical (only if packet tracing is on) */
 static inline int
 get_chainname_rulenum(struct ip6t_entry *s, struct ip6t_entry *e,
 		      char *hookname, char **chainname,
@@ -595,7 +599,7 @@ mark_source_chains(struct xt_table_info *newinfo,
 	return 1;
 }
 
-static inline int
+static int
 cleanup_match(struct ip6t_entry_match *m, unsigned int *i)
 {
 	if (i && (*i)-- == 0)
@@ -607,7 +611,7 @@ cleanup_match(struct ip6t_entry_match *m, unsigned int *i)
 	return 0;
 }
 
-static inline int
+static int
 check_entry(struct ip6t_entry *e, const char *name)
 {
 	struct ip6t_entry_target *t;
@@ -628,7 +632,7 @@ check_entry(struct ip6t_entry *e, const char *name)
 	return 0;
 }
 
-static inline int check_match(struct ip6t_entry_match *m, const char *name,
+static int check_match(struct ip6t_entry_match *m, const char *name,
 			      const struct ip6t_ip6 *ipv6,
 			      unsigned int hookmask, unsigned int *i)
 {
@@ -651,7 +655,7 @@ static inline int check_match(struct ip6t_entry_match *m, const char *name,
 	return ret;
 }
 
-static inline int
+static int
 find_check_match(struct ip6t_entry_match *m,
 		 const char *name,
 		 const struct ip6t_ip6 *ipv6,
@@ -680,7 +684,7 @@ err:
 	return ret;
 }
 
-static inline int check_target(struct ip6t_entry *e, const char *name)
+static int check_target(struct ip6t_entry *e, const char *name)
 {
 	struct ip6t_entry_target *t;
 	struct xt_target *target;
@@ -701,7 +705,7 @@ static inline int check_target(struct ip6t_entry *e, const char *name)
 	return ret;
 }
 
-static inline int
+static int
 find_check_entry(struct ip6t_entry *e, const char *name, unsigned int size,
 		 unsigned int *i)
 {
@@ -745,7 +749,7 @@ find_check_entry(struct ip6t_entry *e, const char *name, unsigned int size,
 	return ret;
 }
 
-static inline int
+static int
 check_entry_size_and_hooks(struct ip6t_entry *e,
 			   struct xt_table_info *newinfo,
 			   unsigned char *base,
@@ -788,7 +792,7 @@ check_entry_size_and_hooks(struct ip6t_entry *e,
 	return 0;
 }
 
-static inline int
+static int
 cleanup_entry(struct ip6t_entry *e, unsigned int *i)
 {
 	struct ip6t_entry_target *t;
@@ -944,7 +948,7 @@ get_counters(const struct xt_table_info *t,
 	}
 }
 
-static inline struct xt_counters *alloc_counters(struct xt_table *table)
+static struct xt_counters *alloc_counters(struct xt_table *table)
 {
 	unsigned int countersize;
 	struct xt_counters *counters;
@@ -1494,7 +1498,7 @@ out:
 	return ret;
 }
 
-static inline int
+static int
 compat_find_calc_match(struct ip6t_entry_match *m,
 		       const char *name,
 		       const struct ip6t_ip6 *ipv6,
@@ -1518,7 +1522,7 @@ compat_find_calc_match(struct ip6t_entry_match *m,
 	return 0;
 }
 
-static inline int
+static int
 compat_release_match(struct ip6t_entry_match *m, unsigned int *i)
 {
 	if (i && (*i)-- == 0)
@@ -1528,7 +1532,7 @@ compat_release_match(struct ip6t_entry_match *m, unsigned int *i)
 	return 0;
 }
 
-static inline int
+static int
 compat_release_entry(struct compat_ip6t_entry *e, unsigned int *i)
 {
 	struct ip6t_entry_target *t;
@@ -1543,7 +1547,7 @@ compat_release_entry(struct compat_ip6t_entry *e, unsigned int *i)
 	return 0;
 }
 
-static inline int
+static int
 check_compat_entry_size_and_hooks(struct compat_ip6t_entry *e,
 				  struct xt_table_info *newinfo,
 				  unsigned int *size,
@@ -1666,7 +1670,7 @@ compat_copy_entry_from_user(struct compat_ip6t_entry *e, void **dstptr,
 	return ret;
 }
 
-static inline int compat_check_entry(struct ip6t_entry *e, const char *name,
+static int compat_check_entry(struct ip6t_entry *e, const char *name,
 				     unsigned int *i)
 {
 	int j, ret;

^ permalink raw reply related

* Re: Bad escriptions in Kconfig
From: Douglas Gilbert @ 2007-12-31 16:34 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: Bodo Eggert, linux-kernel, linux-scsi, netdev
In-Reply-To: <20071231154731.GY11638@parisc-linux.org>

Matthew Wilcox wrote:
> On Mon, Dec 31, 2007 at 10:16:43AM -0500, Douglas Gilbert wrote:
>> Bodo Eggert wrote:
>>> ---
>>> SCSI target support (SCSI_TGT) [N/m/y/?] (NEW) ?
>>>
>>> If you want to use SCSI target mode drivers enable this option.
>>> If you choose M, the module will be called scsi_tgt.
>>> ---
>>>
>>> What TF is a SCSI target mode, what is a target mode driver?
>> Heard of google :-)
>>
>> For explanations of SCSI (and other storage) terminology
>> reference could be made to SAM-3 or SAM-4 drafts (because
>> the real standards cost money) at www.t10.org .
>>
>> Perhaps many other subsections in the kernel could have
>> similar references.
> 
> I think that's an appalling idea.  Someone's trying to configure their
> kernel, not research hundreds of new ideas on the internet.  Here's a
> better description:
> 
> 	help
> 	  The SCSI target code allows your computer to appear as a SCSI
> 	  device.  This is useful in a SAN or NAS environment where you
> 	  want other computers to be able to treat this computer as a disc.
> 
> 	  To compile this driver as a module, choose M here: the module
> 	  will be called scsi_tgt.

Appalling or not, it is more accurate to define a SCSI target
properly than equate it to a direct access logical unit (i.e.
a disk). IMO the designers of that driver and associated
applications made exactly the same mistake which they are now
fixing. That driver is also transport specific with support
for iSCSI and iSER. Try writing a SPI, FC or SAS target driver
for scsi_tgt (I failed with SAS).

It could also be noted that if the reader doesn't understand
the description they can most likely say 'N'.

Doug Gilbert


^ 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