* RE: [PATCH 2/3][NET_BATCH] net core use batching
From: Waskiewicz Jr, Peter P @ 2007-10-09 19:07 UTC (permalink / raw)
To: Jeff Garzik
Cc: David Miller, hadi, krkumar2, johnpol, herbert, kaber, shemminger,
jagana, Robert.Olsson, rick.jones2, xma, gaagaan, netdev, rdreier,
mcarlson, mchan, general, tgraf, randy.dunlap, sri
In-Reply-To: <470BD0B7.4070607@garzik.org>
> A misunderstanding, I think.
>
> To my brain, DaveM's item #2 seemed to assume/require the NIC
> hardware to balance fairly across hw TX rings, which seemed
> to preclude the
> 8139cp/tg3 style of strict-prio hardware. That's what I was
> responding to.
>
> As long as there is some modular way to fit 8139cp/tg3 style
> multi-TX into our universe, I'm happy :)
Ah hah. Yes, a misunderstanding on my part. Thanks for the
clarification. Methinks more caffeine is required for today...
-PJ
^ permalink raw reply
* Re: 2.6.23-rc8-mm2 BUG: register_netdevice() issue as (ab)used by ISDN
From: Andreas Mohr @ 2007-10-09 19:06 UTC (permalink / raw)
To: Andreas Mohr, Jeff Garzik, isdn4linux, netdev, akpm
In-Reply-To: <20071008155836.GA19932@pingi.kke.suse.de>
Hi,
On Mon, Oct 08, 2007 at 05:58:36PM +0200, Karsten Keil wrote:
> You could try following patch with 2.6.23-rc8-mm2, it change I4L to use
> alloc_netdev().
I just did the horribly unthinkable:
I rebooted the production internet gateway *remotely*, and:
root@gate:/home/andi# uname -a
Linux gate 2.6.23-rc8-mm2-gate #1 Mon Oct 8 22:14:00 CEST 2007 i586
GNU/Linux
root@gate:/home/andi# dmesg|grep BUG
root@gate:/home/andi# grep BUG /var/log/messages
Oct 7 13:07:40 gate kernel: kernel BUG at net/core/dev.c:3485!
^^^^^^^
This 2.6.23-rc8-mm2-gate was ISDN-patched, obviously.
Oh, and isdnlog *is* indeed running, so this is a first check that
ISDN functionality seems to be up and running...
IOW, thanks!!
Andreas Mohr
--
Want to provide valuable development feedback about your kernel version?
Install http://klive.cpushare.com/ !
^ permalink raw reply
* Re: [PATCH 2/3][NET_BATCH] net core use batching
From: Jeff Garzik @ 2007-10-09 19:04 UTC (permalink / raw)
To: Waskiewicz Jr, Peter P
Cc: David Miller, hadi, krkumar2, johnpol, herbert, kaber, shemminger,
jagana, Robert.Olsson, rick.jones2, xma, gaagaan, netdev, rdreier,
mcarlson, mchan, general, tgraf, randy.dunlap, sri
In-Reply-To: <D5C1322C3E673F459512FB59E0DDC32903C0821A@orsmsx414.amr.corp.intel.com>
Waskiewicz Jr, Peter P wrote:
>> IMO the net driver really should provide a hint as to what it wants.
>>
>> 8139cp and tg3 would probably prefer multiple TX queue
>> behavior to match silicon behavior -- strict prio.
>
> If I understand what you just said, I disagree. If your hardware is
> running strict prio, you don't want to enforce strict prio in the qdisc
> layer; performing two layers of QoS is excessive, and may lead to
> results you don't want. The reason I added the DRR qdisc is for the Si
> that has its own queueing strategy that is not RR. For Si that
> implements RR (like e1000), you can either use the DRR qdisc, or if you
> want to prioritize your flows, use PRIO.
A misunderstanding, I think.
To my brain, DaveM's item #2 seemed to assume/require the NIC hardware
to balance fairly across hw TX rings, which seemed to preclude the
8139cp/tg3 style of strict-prio hardware. That's what I was responding to.
As long as there is some modular way to fit 8139cp/tg3 style multi-TX
into our universe, I'm happy :)
Jeff
^ permalink raw reply
* [ofa-general] RE: [PATCH 2/3][NET_BATCH] net core use batching
From: Waskiewicz Jr, Peter P @ 2007-10-09 18:48 UTC (permalink / raw)
To: Jeff Garzik, David Miller
Cc: johnpol, herbert, gaagaan, Robert.Olsson, netdev, rdreier, hadi,
mcarlson, jagana, general, mchan, tgraf, randy.dunlap, sri,
shemminger, kaber
In-Reply-To: <470AE373.9020207@garzik.org>
> IMO the net driver really should provide a hint as to what it wants.
>
> 8139cp and tg3 would probably prefer multiple TX queue
> behavior to match silicon behavior -- strict prio.
If I understand what you just said, I disagree. If your hardware is
running strict prio, you don't want to enforce strict prio in the qdisc
layer; performing two layers of QoS is excessive, and may lead to
results you don't want. The reason I added the DRR qdisc is for the Si
that has its own queueing strategy that is not RR. For Si that
implements RR (like e1000), you can either use the DRR qdisc, or if you
want to prioritize your flows, use PRIO.
-PJ Waskiewicz
peter.p.waskiewicz.jr@intel.com
^ permalink raw reply
* [PATCH] xen-netfront: rearrange netfront_info structure to separate tx and rx
From: Jeremy Fitzhardinge @ 2007-10-09 18:44 UTC (permalink / raw)
To: Jeff Garzik
Cc: NetDev, Linux Kernel Mailing List, Keir Fraser, Stephen Hemminger,
Christoph Hellwig
Keep tx and rx elements separate on different cachelines to prevent
bouncing.
Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: Stephen Hemminger <shemminger@linux-foundation.org>
Cc: Christoph Hellwig <hch@infradead.org>
---
drivers/net/xen-netfront.c | 37 ++++++++++++++++++-------------------
1 file changed, 18 insertions(+), 19 deletions(-)
===================================================================
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -72,22 +72,12 @@ struct netfront_info {
struct list_head list;
struct net_device *netdev;
+ unsigned int evtchn;
+ struct xenbus_device *xbdev;
+
+ spinlock_t tx_lock;
struct xen_netif_tx_front_ring tx;
- struct xen_netif_rx_front_ring rx;
-
- spinlock_t tx_lock;
- spinlock_t rx_lock;
-
- unsigned int evtchn;
-
- /* Receive-ring batched refills. */
-#define RX_MIN_TARGET 8
-#define RX_DFL_MIN_TARGET 64
-#define RX_MAX_TARGET min_t(int, NET_RX_RING_SIZE, 256)
- unsigned rx_min_target, rx_max_target, rx_target;
- struct sk_buff_head rx_batch;
-
- struct timer_list rx_refill_timer;
+ int tx_ring_ref;
/*
* {tx,rx}_skbs store outstanding skbuffs. Free tx_skb entries
@@ -106,13 +96,22 @@ struct netfront_info {
grant_ref_t grant_tx_ref[NET_TX_RING_SIZE];
unsigned tx_skb_freelist;
+ spinlock_t rx_lock ____cacheline_aligned_in_smp;
+ struct xen_netif_rx_front_ring rx;
+ int rx_ring_ref;
+
+ /* Receive-ring batched refills. */
+#define RX_MIN_TARGET 8
+#define RX_DFL_MIN_TARGET 64
+#define RX_MAX_TARGET min_t(int, NET_RX_RING_SIZE, 256)
+ unsigned rx_min_target, rx_max_target, rx_target;
+ struct sk_buff_head rx_batch;
+
+ struct timer_list rx_refill_timer;
+
struct sk_buff *rx_skbs[NET_RX_RING_SIZE];
grant_ref_t gref_rx_head;
grant_ref_t grant_rx_ref[NET_RX_RING_SIZE];
-
- struct xenbus_device *xbdev;
- int tx_ring_ref;
- int rx_ring_ref;
unsigned long rx_pfn_array[NET_RX_RING_SIZE];
struct multicall_entry rx_mcl[NET_RX_RING_SIZE+1];
^ permalink raw reply
* Re: [ofa-general] Re: [PATCH 2/3][NET_BATCH] net core use batching
From: Andi Kleen @ 2007-10-09 18:30 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Andi Kleen, David Miller, jeff, johnpol, herbert, gaagaan,
Robert.Olsson, netdev, rdreier, peter.p.waskiewicz.jr, hadi,
mcarlson, jagana, general, mchan, tgraf, randy.dunlap, sri, kaber
In-Reply-To: <20071009112225.5f9756e7@freepuppy.rosehill>
> I wonder about the whole idea of queueing in general at such high speeds.
> Given the normal bi-modal distribution of packets, and the predominance
> of 1500 byte MTU; does it make sense to even have any queueing in software
> at all?
Yes that is my point -- it should just pass it through directly
and the driver can then put it into the different per CPU (or per
whatever) queues managed by the hardware.
The only thing the qdisc needs to do is to set some bit that says
"it is ok to put this into difference queues; don't need strict ordering"
Otherwise if the drivers did that unconditionally they might cause
problems with other qdiscs.
This would also require that the driver exports some hint
to the upper layer on how large its internal queues are. A device
with a short queue would still require pfifo_fast. Long queue
devices could just pass through. That again could be a single flag.
-Andi
^ permalink raw reply
* Re: [ofa-general] Re: [PATCH 2/3][NET_BATCH] net core use batching
From: Stephen Hemminger @ 2007-10-09 18:22 UTC (permalink / raw)
To: Andi Kleen
Cc: johnpol, Robert.Olsson, jeff, gaagaan, kaber, netdev, rdreier,
peter.p.waskiewicz.jr, hadi, mcarlson, jagana, general, mchan,
tgraf, randy.dunlap, sri, David Miller, herbert
In-Reply-To: <p73k5pw44ew.fsf@bingen.suse.de>
On 09 Oct 2007 18:51:51 +0200
Andi Kleen <andi@firstfloor.org> wrote:
> David Miller <davem@davemloft.net> writes:
> >
> > 2) Switch the default qdisc away from pfifo_fast to a new DRR fifo
> > with load balancing using the code in #1. I think this is kind
> > of in the territory of what Peter said he is working on.
>
> Hopefully that new qdisc will just use the TX rings of the hardware
> directly. They are typically large enough these days. That might avoid
> some locking in this critical path.
>
> > I know this is controversial, but realistically I doubt users
> > benefit at all from the prioritization that pfifo provides.
>
> I agree. For most interfaces the priority is probably dubious.
> Even for DSL the prioritization will be likely usually done in a router
> these days.
>
> Also for the fast interfaces where we do TSO priority doesn't work
> very well anyways -- with large packets there is not too much
> to prioritize.
>
> > 3) Work on discovering a way to make the locking on transmit as
> > localized to the current thread of execution as possible. Things
> > like RCU and statistic replication, techniques we use widely
> > elsewhere in the stack, begin to come to mind.
>
> If the data is just passed on to the hardware queue, why is any
> locking needed at all? (except for the driver locking of course)
>
> -Andi
I wonder about the whole idea of queueing in general at such high speeds.
Given the normal bi-modal distribution of packets, and the predominance
of 1500 byte MTU; does it make sense to even have any queueing in software
at all?
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* Re: [PATCH][NETNS] Make ifindex generation per-namespace
From: Eric W. Biederman @ 2007-10-09 17:43 UTC (permalink / raw)
To: David Stevens
Cc: Pavel Emelyanov, David Miller, devel, Linux Netdev List,
netdev-owner
In-Reply-To: <OF54650AF1.A1051792-ON8825736F.00588F0B-8825736F.00597D24@us.ibm.com>
David Stevens <dlstevens@us.ibm.com> writes:
> Sorry if this is a dumb question, but what is the model you intend for
> SNMP? Do you want each namespace to be its own virtual machine with
> its own, separate MIB?
Each network namespace appears to user space as a completely separate
network stack. So yes a separate instance of the MIB is appropriate.
Eric
^ permalink raw reply
* Re: [PATCH][NETNS] Make ifindex generation per-namespace
From: Eric W. Biederman @ 2007-10-09 17:41 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: David Miller, Linux Netdev List, devel
In-Reply-To: <470B71CD.3000706@openvz.org>
Pavel Emelyanov <xemul@openvz.org> writes:
> Currently indexes for netdevices come sequentially one by
> one, and the same stays true even for devices that are
> created for namespaces.
>
> Side effects of this are:
> * lo device has not 1 index in a namespace. This may break
> some userspace that relies on it (and AFAIR something
> really broke in OpenVZ VEs without this);
As it happens lo hasn't been registered first for some time
so it hasn't had ifindex of 1 in the normal kernel.
> * after some time namespaces will have devices with indexes
> like 1000000 os similar. This might be confusing for a
> human (tools will not mind).
Only if we wind up creating that many devices.
> So move the (currently "global" and static) ifindex variable
> on the struct net, making the indexes allocation look more
> like on a standalone machine.
>
> Moreover - when we have indexes intersect between namespaces,
> we may catch more BUGs in the future related to "wrong device
> was found for a given index".
Not yet.
I know there are several data structures internal to the kernel that
are indexed by ifindex, and not struct net_device *. There is the
iflink field in struct net_device. We need a way to refer to network
devices in other namespaces in rtnetlink in an unambiguous way. I
don't see any real problems with a global ifindex assignment until
we start migrating applications.
So please hold off on this until the kernel has been audited and
we have removed all of the uses of ifindex that assume ifindex is
global, that we can find.
Right now a namespace local ifindex seems to be just asking for
trouble.
Eric
^ permalink raw reply
* Re: [ofa-general] Re: [PATCH 2/3][NET_BATCH] net core use batching
From: Andi Kleen @ 2007-10-09 16:51 UTC (permalink / raw)
To: David Miller
Cc: jeff, johnpol, herbert, gaagaan, Robert.Olsson, netdev, rdreier,
peter.p.waskiewicz.jr, hadi, mcarlson, jagana, general, mchan,
tgraf, randy.dunlap, sri, shemminger, kaber
In-Reply-To: <20071008.184126.124062865.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
>
> 2) Switch the default qdisc away from pfifo_fast to a new DRR fifo
> with load balancing using the code in #1. I think this is kind
> of in the territory of what Peter said he is working on.
Hopefully that new qdisc will just use the TX rings of the hardware
directly. They are typically large enough these days. That might avoid
some locking in this critical path.
> I know this is controversial, but realistically I doubt users
> benefit at all from the prioritization that pfifo provides.
I agree. For most interfaces the priority is probably dubious.
Even for DSL the prioritization will be likely usually done in a router
these days.
Also for the fast interfaces where we do TSO priority doesn't work
very well anyways -- with large packets there is not too much
to prioritize.
> 3) Work on discovering a way to make the locking on transmit as
> localized to the current thread of execution as possible. Things
> like RCU and statistic replication, techniques we use widely
> elsewhere in the stack, begin to come to mind.
If the data is just passed on to the hardware queue, why is any
locking needed at all? (except for the driver locking of course)
-Andi
^ permalink raw reply
* Re: [PATCH][NETNS] Make ifindex generation per-namespace
From: David Stevens @ 2007-10-09 16:18 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: David Miller, devel, Eric W. Biederman, Linux Netdev List,
netdev-owner
In-Reply-To: <470B71CD.3000706@openvz.org>
Sorry if this is a dumb question, but what is the model you intend for
SNMP? Do you want each namespace to be its own virtual machine with
its own, separate MIB?
Ifindex's have to uniquely identify the interface (virtual or otherwise)
to remote
queriers (not just local applications), so unless you pay the price of
separating
all the SNMP MIBs per namespace too, it seems you'll need some way to
remap these for SNMP queries, right?
+-DLS
^ permalink raw reply
* Re: [PATCH 1/8][BNX2X] resubmit as attachments: add bnx2x to Kconfig and Makefile
From: Eliezer Tamir @ 2007-10-09 16:20 UTC (permalink / raw)
To: David Miller; +Cc: jeff, netdev, mchan
In-Reply-To: <20071008.212931.82537619.davem@davemloft.net>
On Mon, 2007-10-08 at 21:29 -0700, David Miller wrote:
> From: "Eliezer Tamir" <eliezert@broadcom.com>
> Date: Tue, 09 Oct 2007 06:13:23 +0200
>
> > Due to the size of the patch I can not post it to the list.
>
> Understood
>
> > Here is an FTP link.
> > ftp://Net_sys_anon@ftp1.broadcom.com/bnx2x-0.40.10-net-2.6.24-one.patch.txt
> >
> > Or if you prefer, I can post it gzipped.
>
> I took a look at what takes up so much space and it's the firmware and
> this register init stuff.
>
> The firmware is unavoidable, but wrt. the register init tables there
> appears to be a ton of superfluous information in that header file.
First, I must admit that looking over the file again I noticed an error
on my part in generating the file, all the data that initializes FPGA or
EMULATION should not be there. fixed.
(This reduced about half of the lines but only a third in size.)
> It seem extreme overkill and I would guess it exists purely to
> simplify your in-house chip validation. I would really wish you
> wouldn't do this as these tables take up unswappable kernel memory.
>
> No other driver goes about initializing the chip registers using
> tables like this. Many of them are sequences of nothing but zeros or
> the same value repeated over and over! The non-zero values that are
> specific are "magic constants" with no macros to describe up the
> meaning of the bits being set in these registers.
Almost all of the zero-filled tables will be removed.
The rest of the registers do need to be initialized.
I agree that the number of registers that needs to be initialized is
huge, but that is caused by the way the hardware was designed.
The values for the initialization come from several sources:
Some are derived from HW code (the XML files used to derive the verilog
code),
Others (along with much of the machine generated .h files) are generated
at microcode build time, adding a microcode routine will cause the init
values to change, using a new variable can cause an .h file to change.
In the last group which is very small, are registers that are controlled
by the driver.
The values in this file really are machine generated, they really are
not meant to be modified directly by editing the file.
The registers that are under the driver's control are in the main .c
and .h files.
>
> Huge functions like bnx2x_idle_chk() that just validate register
> values with hundreds of lines of C code are not appropriate for
> submission into a Linux kernel network driver. Again, this looks
> like code that assists you with in-house chip validation.
The idle check code is not a manufacturing test, it is meant to help
debug the driver and microcode.
If the driver sends an invalid command to one of the CPUs which then
chokes on it, this will tell you which one of them died and the general
whereabouts of the problem. (ingress CPU X is stuck because output queue
Y is full)
I agree that it is not a clean implementation.
I will remove it and it will be re-written.
> I'm open to putting self tests into the driver, but this adds so much
> bloat it really takes things beyond reasonable.
>
> Please remove this self-test code and compress the register
> initialization information, preferrably into foo_hw_init() C functions
> as is done traditionally in drivers. It will take up less space,
> remove the magic values, and make the hardware easier to understand
> for other developers.
> This should get the driver under the posting limit of 400K, which to
> be honest no driver's should be larger than. Unfortunately that
> firmware file is 350K so you won't have much left to work with :-)
>
> What we could do is post the driver without the firmware, so that
> people can review the actual C-code by just replying to this posting
> and it won't go over the posting size limit.
( Michael has showed me the trick of how to post with evolution, so I
hope that the mangled patch problem is behind us and I think that I can
now post everything without a problem, Hallelujah!)
>
> Thanks!
>
^ permalink raw reply
* [PATCH 6/6][NET-2.6.24] Make the sunrpc use the seq_open_private()
From: Pavel Emelyanov @ 2007-10-09 16:04 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, devel, Neil Brown
In-Reply-To: <470BA351.3070208@openvz.org>
Just switch to the consolidated code.
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Cc: Neil Brown <neilb@cse.unsw.edu.au>
---
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index ebe344f..8e05557 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -1218,23 +1218,15 @@ static const struct seq_operations cache
static int content_open(struct inode *inode, struct file *file)
{
- int res;
struct handle *han;
struct cache_detail *cd = PDE(inode)->data;
- han = kmalloc(sizeof(*han), GFP_KERNEL);
+ han = __seq_open_private(file, &cache_content_op, sizeof(*han));
if (han == NULL)
return -ENOMEM;
han->cd = cd;
-
- res = seq_open(file, &cache_content_op);
- if (res)
- kfree(han);
- else
- ((struct seq_file *)file->private_data)->private = han;
-
- return res;
+ return 0;
}
static const struct file_operations content_file_operations = {
^ permalink raw reply related
* [PATCH 5/6][NET-2.6.24] Make the IRDA use the seq_open_private()
From: Pavel Emelyanov @ 2007-10-09 16:01 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, devel, samuel
In-Reply-To: <470BA351.3070208@openvz.org>
Just switch to the consolidated code
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Cc: Samuel Ortiz <samuel@sortiz.org>
---
diff --git a/net/irda/irlap.c b/net/irda/irlap.c
index 3d76aaf..f3236ac 100644
--- a/net/irda/irlap.c
+++ b/net/irda/irlap.c
@@ -1219,29 +1219,11 @@ static const struct seq_operations irlap
static int irlap_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct irlap_iter_state *s = kzalloc(sizeof(*s), GFP_KERNEL);
+ if (irlap == NULL)
+ return -EINVAL;
- if (!s)
- goto out;
-
- if (irlap == NULL) {
- rc = -EINVAL;
- goto out_kfree;
- }
-
- rc = seq_open(file, &irlap_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &irlap_seq_ops,
+ sizeof(struct irlap_iter_state));
}
const struct file_operations irlap_seq_fops = {
diff --git a/net/irda/irlmp.c b/net/irda/irlmp.c
index 7efa930..7db92ce 100644
--- a/net/irda/irlmp.c
+++ b/net/irda/irlmp.c
@@ -2003,27 +2003,10 @@ static const struct seq_operations irlmp
static int irlmp_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct irlmp_iter_state *s;
-
IRDA_ASSERT(irlmp != NULL, return -EINVAL;);
- s = kmalloc(sizeof(*s), GFP_KERNEL);
- if (!s)
- goto out;
-
- rc = seq_open(file, &irlmp_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &irlmp_seq_ops,
+ sizeof(struct irlmp_iter_state));
}
const struct file_operations irlmp_seq_fops = {
diff --git a/net/irda/irttp.c b/net/irda/irttp.c
index 3d7ab03..1311976 100644
--- a/net/irda/irttp.c
+++ b/net/irda/irttp.c
@@ -1884,25 +1884,8 @@ static const struct seq_operations irttp
static int irttp_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct irttp_iter_state *s;
-
- s = kzalloc(sizeof(*s), GFP_KERNEL);
- if (!s)
- goto out;
-
- rc = seq_open(file, &irttp_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &irttp_seq_ops,
+ sizeof(struct irttp_iter_state));
}
const struct file_operations irttp_seq_fops = {
^ permalink raw reply related
* [PATCH 4/6][NET-2.6.24] Make decnet code use the seq_open_private()
From: Pavel Emelyanov @ 2007-10-09 15:59 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, devel, patrick
In-Reply-To: <470BA351.3070208@openvz.org>
Just switch to the consolidated code.
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Cc: Patrick Caulfield <patrick@tykepenguin.com>
---
diff --git a/net/decnet/dn_neigh.c b/net/decnet/dn_neigh.c
index b66e3be..e851b14 100644
--- a/net/decnet/dn_neigh.c
+++ b/net/decnet/dn_neigh.c
@@ -580,24 +580,8 @@ static const struct seq_operations dn_ne
static int dn_neigh_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct neigh_seq_state *s = kzalloc(sizeof(*s), GFP_KERNEL);
-
- if (!s)
- goto out;
-
- rc = seq_open(file, &dn_neigh_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &dn_neigh_seq_ops,
+ sizeof(struct neigh_seq_state));
}
static const struct file_operations dn_neigh_seq_fops = {
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index b7ebf99..97eee5e 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -1739,23 +1739,8 @@ static const struct seq_operations dn_rt
static int dn_rt_cache_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct dn_rt_cache_iter_state *s;
-
- s = kzalloc(sizeof(*s), GFP_KERNEL);
- if (!s)
- goto out;
- rc = seq_open(file, &dn_rt_cache_seq_ops);
- if (rc)
- goto out_kfree;
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &dn_rt_cache_seq_ops,
+ sizeof(struct dn_rt_cache_iter_state));
}
static const struct file_operations dn_rt_cache_seq_fops = {
^ permalink raw reply related
* Re: [PATCH][E1000E] some cleanups
From: Kok, Auke @ 2007-10-09 16:02 UTC (permalink / raw)
To: hadi; +Cc: netdev
In-Reply-To: <1191936581.4373.192.camel@localhost>
jamal wrote:
> On Mon, 2007-08-10 at 15:40 -0700, Kok, Auke wrote:
>
>> My biggest problem with the patch as you sent it that it's a tonload of changes
>> and no implicit benefit immediately as I can see.
>
> The patch looks scary but is pretty tame when you apply it and stare at
> it.
>
>> I would really have to see the
>> LLTX change as well to make a wellfounded decision whether it is the right thing
>> to go and overhaul this part of the driver or not :)
>>
>> I'm not particularly against the changes per se though.
>>
>> so please, if needed offlist send me those LLTX changes as well.
>
> Dont worry about it - i didnt get any love for a similar patch i did for
> tg3 either ;-> I will hold onto it for now. I could do the un-LLTX
> outside of this - would you like that for both e1000/e1000e ?
if we're going to remove LLTX from e1000 I prefer to do that at a much later time.
Let's focus on e1000e instead - while it is still moving ;)
Auke
^ permalink raw reply
* [PATCH 3/6][NET-2.6.24] Make netfilter code use the seq_open_private
From: Pavel Emelyanov @ 2007-10-09 15:57 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, devel, Patrick McHardy
In-Reply-To: <470BA351.3070208@openvz.org>
Just switch to the consolidated calls.
ipt_recent() has to initialize the private, so use
the __seq_open_private() helper.
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Cc: Patrick McHardy <kaber@trash.net>
---
diff --git a/net/ipv4/netfilter/ipt_recent.c b/net/ipv4/netfilter/ipt_recent.c
index db2a798..11d39fb 100644
--- a/net/ipv4/netfilter/ipt_recent.c
+++ b/net/ipv4/netfilter/ipt_recent.c
@@ -381,25 +381,14 @@ static const struct seq_operations recen
static int recent_seq_open(struct inode *inode, struct file *file)
{
struct proc_dir_entry *pde = PDE(inode);
- struct seq_file *seq;
struct recent_iter_state *st;
- int ret;
- st = kzalloc(sizeof(*st), GFP_KERNEL);
+ st = __seq_open_private(file, &recent_seq_ops, sizeof(*st));
if (st == NULL)
return -ENOMEM;
- ret = seq_open(file, &recent_seq_ops);
- if (ret) {
- kfree(st);
- goto out;
- }
-
st->table = pde->data;
- seq = file->private_data;
- seq->private = st;
-out:
- return ret;
+ return 0;
}
static ssize_t recent_proc_write(struct file *file, const char __user *input,
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
index a5ae2ea..741f3df 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
@@ -174,22 +174,8 @@ static const struct seq_operations ct_se
static int ct_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- struct ct_iter_state *st;
- int ret;
-
- st = kzalloc(sizeof(struct ct_iter_state), GFP_KERNEL);
- if (st == NULL)
- return -ENOMEM;
- ret = seq_open(file, &ct_seq_ops);
- if (ret)
- goto out_free;
- seq = file->private_data;
- seq->private = st;
- return ret;
-out_free:
- kfree(st);
- return ret;
+ return seq_open_private(file, &ct_seq_ops,
+ sizeof(struct ct_iter_state));
}
static const struct file_operations ct_file_ops = {
@@ -291,22 +277,8 @@ static const struct seq_operations exp_s
static int exp_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- struct ct_expect_iter_state *st;
- int ret;
-
- st = kzalloc(sizeof(struct ct_expect_iter_state), GFP_KERNEL);
- if (!st)
- return -ENOMEM;
- ret = seq_open(file, &exp_seq_ops);
- if (ret)
- goto out_free;
- seq = file->private_data;
- seq->private = st;
- return ret;
-out_free:
- kfree(st);
- return ret;
+ return seq_open_private(file, &exp_seq_ops,
+ sizeof(struct ct_expect_iter_state));
}
static const struct file_operations ip_exp_file_ops = {
diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c
index 7a0ae36..175c8d1 100644
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -472,22 +472,8 @@ static const struct seq_operations exp_s
static int exp_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- struct ct_expect_iter_state *st;
- int ret;
-
- st = kzalloc(sizeof(struct ct_expect_iter_state), GFP_KERNEL);
- if (!st)
- return -ENOMEM;
- ret = seq_open(file, &exp_seq_ops);
- if (ret)
- goto out_free;
- seq = file->private_data;
- seq->private = st;
- return ret;
-out_free:
- kfree(st);
- return ret;
+ return seq_open_private(file, &exp_seq_ops,
+ sizeof(struct ct_expect_iter_state));
}
static const struct file_operations exp_file_ops = {
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index 2a19c5f..9efdd37 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -195,22 +195,8 @@ static const struct seq_operations ct_se
static int ct_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- struct ct_iter_state *st;
- int ret;
-
- st = kzalloc(sizeof(struct ct_iter_state), GFP_KERNEL);
- if (st == NULL)
- return -ENOMEM;
- ret = seq_open(file, &ct_seq_ops);
- if (ret)
- goto out_free;
- seq = file->private_data;
- seq->private = st;
- return ret;
-out_free:
- kfree(st);
- return ret;
+ return seq_open_private(file, &ct_seq_ops,
+ sizeof(struct ct_iter_state));
}
static const struct file_operations ct_file_ops = {
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 2135926..2c7bd2e 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -951,22 +951,8 @@ static const struct seq_operations nful_
static int nful_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- struct iter_state *is;
- int ret;
-
- is = kzalloc(sizeof(*is), GFP_KERNEL);
- if (!is)
- return -ENOMEM;
- ret = seq_open(file, &nful_seq_ops);
- if (ret < 0)
- goto out_free;
- seq = file->private_data;
- seq->private = is;
- return ret;
-out_free:
- kfree(is);
- return ret;
+ return seq_open_private(file, &nful_seq_ops,
+ sizeof(struct iter_state));
}
static const struct file_operations nful_file_ops = {
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index 48e095a..49f0480 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -1051,22 +1051,8 @@ static const struct seq_operations nfqnl
static int nfqnl_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- struct iter_state *is;
- int ret;
-
- is = kzalloc(sizeof(*is), GFP_KERNEL);
- if (!is)
- return -ENOMEM;
- ret = seq_open(file, &nfqnl_seq_ops);
- if (ret < 0)
- goto out_free;
- seq = file->private_data;
- seq->private = is;
- return ret;
-out_free:
- kfree(is);
- return ret;
+ return seq_open_private(file, &nfqnl_seq_ops,
+ sizeof(struct iter_state));
}
static const struct file_operations nfqnl_file_ops = {
^ permalink raw reply related
* [PATCH 2/6][NET-2.6.24] Make core networking code use seq_open_private
From: Pavel Emelyanov @ 2007-10-09 15:55 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, devel
In-Reply-To: <470BA351.3070208@openvz.org>
This concerns the ipv4 and ipv6 code mostly, but also the netlink
and unix sockets.
The netlink code is an example of how to use the __seq_open_private()
call - it saves the net namespace on this private.
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
---
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index d824819..36d6798 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -1378,24 +1378,8 @@ static const struct seq_operations arp_s
static int arp_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct neigh_seq_state *s = kzalloc(sizeof(*s), GFP_KERNEL);
-
- if (!s)
- goto out;
-
- rc = seq_open(file, &arp_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &arp_seq_ops,
+ sizeof(struct neigh_seq_state));
}
static const struct file_operations arp_seq_fops = {
diff --git a/net/ipv4/fib_hash.c b/net/ipv4/fib_hash.c
index 9fafbee..527a6e0 100644
--- a/net/ipv4/fib_hash.c
+++ b/net/ipv4/fib_hash.c
@@ -1039,24 +1039,8 @@ static const struct seq_operations fib_s
static int fib_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct fib_iter_state *s = kzalloc(sizeof(*s), GFP_KERNEL);
-
- if (!s)
- goto out;
-
- rc = seq_open(file, &fib_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &fib_seq_ops,
+ sizeof(struct fib_iter_state));
}
static const struct file_operations fib_seq_fops = {
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index be34bd5..81a8285 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -2379,25 +2379,8 @@ static const struct seq_operations fib_t
static int fib_trie_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct fib_trie_iter *s = kmalloc(sizeof(*s), GFP_KERNEL);
-
- if (!s)
- goto out;
-
- rc = seq_open(file, &fib_trie_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
- memset(s, 0, sizeof(*s));
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &fib_trie_seq_ops,
+ sizeof(struct fib_trie_iter));
}
static const struct file_operations fib_trie_fops = {
@@ -2500,25 +2483,8 @@ static const struct seq_operations fib_r
static int fib_route_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct fib_trie_iter *s = kmalloc(sizeof(*s), GFP_KERNEL);
-
- if (!s)
- goto out;
-
- rc = seq_open(file, &fib_route_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
- memset(s, 0, sizeof(*s));
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &fib_route_seq_ops,
+ sizeof(struct fib_trie_iter));
}
static const struct file_operations fib_route_fops = {
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 2b6e59c..7dbc282 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -2410,23 +2410,8 @@ static const struct seq_operations igmp_
static int igmp_mc_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct igmp_mc_iter_state *s = kzalloc(sizeof(*s), GFP_KERNEL);
-
- if (!s)
- goto out;
- rc = seq_open(file, &igmp_mc_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &igmp_mc_seq_ops,
+ sizeof(struct igmp_mc_iter_state));
}
static const struct file_operations igmp_mc_seq_fops = {
@@ -2584,23 +2569,8 @@ static const struct seq_operations igmp_
static int igmp_mcf_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct igmp_mcf_iter_state *s = kzalloc(sizeof(*s), GFP_KERNEL);
-
- if (!s)
- goto out;
- rc = seq_open(file, &igmp_mcf_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &igmp_mcf_seq_ops,
+ sizeof(struct igmp_mcf_iter_state));
}
static const struct file_operations igmp_mcf_seq_fops = {
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index b8b4b49..37bb497 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1714,26 +1714,8 @@ static const struct seq_operations ipmr_
static int ipmr_vif_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct ipmr_vif_iter *s = kmalloc(sizeof(*s), GFP_KERNEL);
-
- if (!s)
- goto out;
-
- rc = seq_open(file, &ipmr_vif_seq_ops);
- if (rc)
- goto out_kfree;
-
- s->ct = 0;
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
-
+ return seq_open_private(file, &ipmr_vif_seq_ops,
+ sizeof(struct ipmr_vif_iter));
}
static const struct file_operations ipmr_vif_fops = {
@@ -1877,25 +1859,8 @@ static const struct seq_operations ipmr_
static int ipmr_mfc_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct ipmr_mfc_iter *s = kmalloc(sizeof(*s), GFP_KERNEL);
-
- if (!s)
- goto out;
-
- rc = seq_open(file, &ipmr_mfc_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
-
+ return seq_open_private(file, &ipmr_mfc_seq_ops,
+ sizeof(struct ipmr_mfc_iter));
}
static const struct file_operations ipmr_mfc_fops = {
diff --git a/net/ipv4/ipvs/ip_vs_ctl.c b/net/ipv4/ipvs/ip_vs_ctl.c
index 61d023d..7345fc2 100644
--- a/net/ipv4/ipvs/ip_vs_ctl.c
+++ b/net/ipv4/ipvs/ip_vs_ctl.c
@@ -1792,24 +1792,8 @@ static const struct seq_operations ip_vs
static int ip_vs_info_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct ip_vs_iter *s = kzalloc(sizeof(*s), GFP_KERNEL);
-
- if (!s)
- goto out;
-
- rc = seq_open(file, &ip_vs_info_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &ip_vs_info_seq_ops,
+ sizeof(struct ip_vs_iter));
}
static const struct file_operations ip_vs_info_fops = {
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 07070c7..3916fac 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -902,24 +902,8 @@ static const struct seq_operations raw_s
static int raw_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct raw_iter_state *s;
-
- s = kzalloc(sizeof(*s), GFP_KERNEL);
- if (!s)
- goto out;
- rc = seq_open(file, &raw_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &raw_seq_ops,
+ sizeof(struct raw_iter_state));
}
static const struct file_operations raw_seq_fops = {
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 307e1f1..21b12de 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -375,23 +375,8 @@ static const struct seq_operations rt_ca
static int rt_cache_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct rt_cache_iter_state *s;
-
- s = kzalloc(sizeof(*s), GFP_KERNEL);
- if (!s)
- goto out;
- rc = seq_open(file, &rt_cache_seq_ops);
- if (rc)
- goto out_kfree;
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &rt_cache_seq_ops,
+ sizeof(struct rt_cache_iter_state));
}
static const struct file_operations rt_cache_seq_fops = {
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6d5c3c2..8b2d760 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2806,24 +2806,8 @@ static const struct seq_operations if6_s
static int if6_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct if6_iter_state *s = kzalloc(sizeof(*s), GFP_KERNEL);
-
- if (!s)
- goto out;
-
- rc = seq_open(file, &if6_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &if6_seq_ops,
+ sizeof(struct if6_iter_state));
}
static const struct file_operations if6_fops = {
diff --git a/net/ipv6/anycast.c b/net/ipv6/anycast.c
index 5810852..f915c4d 100644
--- a/net/ipv6/anycast.c
+++ b/net/ipv6/anycast.c
@@ -549,24 +549,8 @@ static const struct seq_operations ac6_s
static int ac6_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct ac6_iter_state *s = kzalloc(sizeof(*s), GFP_KERNEL);
-
- if (!s)
- goto out;
-
- rc = seq_open(file, &ac6_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &ac6_seq_ops,
+ sizeof(struct ac6_iter_state));
}
static const struct file_operations ac6_seq_fops = {
diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c
index 1791399..217d60f 100644
--- a/net/ipv6/ip6_flowlabel.c
+++ b/net/ipv6/ip6_flowlabel.c
@@ -658,24 +658,8 @@ static const struct seq_operations ip6fl
static int ip6fl_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct ip6fl_iter_state *s = kzalloc(sizeof(*s), GFP_KERNEL);
-
- if (!s)
- goto out;
-
- rc = seq_open(file, &ip6fl_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &ip6fl_seq_ops,
+ sizeof(struct ip6fl_iter_state));
}
static const struct file_operations ip6fl_seq_fops = {
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index cc8d4e2..331d728 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -2426,24 +2426,8 @@ static const struct seq_operations igmp6
static int igmp6_mc_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct igmp6_mc_iter_state *s = kzalloc(sizeof(*s), GFP_KERNEL);
-
- if (!s)
- goto out;
-
- rc = seq_open(file, &igmp6_mc_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &igmp6_mc_seq_ops,
+ sizeof(struct igmp6_mc_iter_state));
}
static const struct file_operations igmp6_mc_seq_fops = {
@@ -2600,24 +2584,8 @@ static const struct seq_operations igmp6
static int igmp6_mcf_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct igmp6_mcf_iter_state *s = kzalloc(sizeof(*s), GFP_KERNEL);
-
- if (!s)
- goto out;
-
- rc = seq_open(file, &igmp6_mcf_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &igmp6_mcf_seq_ops,
+ sizeof(struct igmp6_mcf_iter_state));
}
static const struct file_operations igmp6_mcf_seq_fops = {
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index bdd0974..ca24ef1 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -1289,21 +1289,8 @@ static const struct seq_operations raw6_
static int raw6_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- struct raw6_iter_state *s = kzalloc(sizeof(*s), GFP_KERNEL);
- if (!s)
- goto out;
- rc = seq_open(file, &raw6_seq_ops);
- if (rc)
- goto out_kfree;
- seq = file->private_data;
- seq->private = s;
-out:
- return rc;
-out_kfree:
- kfree(s);
- goto out;
+ return seq_open_private(file, &raw6_seq_ops,
+ sizeof(struct raw6_iter_state));
}
static const struct file_operations raw6_seq_fops = {
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 3ef3282..f934f54 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1845,27 +1845,18 @@ static const struct seq_operations netli
static int netlink_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
struct nl_seq_iter *iter;
- int err;
- iter = kzalloc(sizeof(*iter), GFP_KERNEL);
+ iter = __seq_open_private(file, &netlink_seq_ops, sizeof(*iter));
if (!iter)
return -ENOMEM;
- err = seq_open(file, &netlink_seq_ops);
- if (err) {
- kfree(iter);
- return err;
- }
-
- seq = file->private_data;
- seq->private = iter;
iter->net = get_proc_net(inode);
if (!iter->net) {
seq_release_private(inode, file);
return -ENXIO;
}
+
return 0;
}
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 10e7312..2b57eaf 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2086,25 +2086,7 @@ static const struct seq_operations unix_
static int unix_seq_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
- int rc = -ENOMEM;
- int *iter = kmalloc(sizeof(int), GFP_KERNEL);
-
- if (!iter)
- goto out;
-
- rc = seq_open(file, &unix_seq_ops);
- if (rc)
- goto out_kfree;
-
- seq = file->private_data;
- seq->private = iter;
- *iter = 0;
-out:
- return rc;
-out_kfree:
- kfree(iter);
- goto out;
+ return seq_open_private(file, &unix_seq_ops, sizeof(int));
}
static const struct file_operations unix_seq_fops = {
^ permalink raw reply related
* [PATCH 1/6][NET-2.6.24] Introduce the seq_open_private()
From: Pavel Emelyanov @ 2007-10-09 15:52 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, devel
In-Reply-To: <470BA351.3070208@openvz.org>
This function allocates the zeroed chunk of memory and
call seq_open(). The __seq_open_private() helper returns
the allocated memory to make it possible for the caller
to initialize it.
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
---
diff --git a/fs/seq_file.c b/fs/seq_file.c
index bbb19be..ca71c11 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -429,6 +429,39 @@ int seq_release_private(struct inode *in
}
EXPORT_SYMBOL(seq_release_private);
+void *__seq_open_private(struct file *f, const struct seq_operations *ops,
+ int psize)
+{
+ int rc;
+ void *private;
+ struct seq_file *seq;
+
+ private = kzalloc(psize, GFP_KERNEL);
+ if (private == NULL)
+ goto out;
+
+ rc = seq_open(f, ops);
+ if (rc < 0)
+ goto out_free;
+
+ seq = f->private_data;
+ seq->private = private;
+ return private;
+
+out_free:
+ kfree(private);
+out:
+ return NULL;
+}
+EXPORT_SYMBOL(__seq_open_private);
+
+int seq_open_private(struct file *filp, const struct seq_operations *ops,
+ int psize)
+{
+ return __seq_open_private(filp, ops, psize) ? 0 : -ENOMEM;
+}
+EXPORT_SYMBOL(seq_open_private);
+
int seq_putc(struct seq_file *m, char c)
{
if (m->count < m->size) {
diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
index 83783ab..8bf1e05 100644
--- a/include/linux/seq_file.h
+++ b/include/linux/seq_file.h
@@ -46,6 +46,8 @@ int seq_path(struct seq_file *, struct v
int single_open(struct file *, int (*)(struct seq_file *, void *), void *);
int single_release(struct inode *, struct file *);
+void *__seq_open_private(struct file *, const struct seq_operations *, int);
+int seq_open_private(struct file *, const struct seq_operations *, int);
int seq_release_private(struct inode *, struct file *);
#define SEQ_START_TOKEN ((void *)1)
^ permalink raw reply related
* [PATCH 0/6][NET-2.6.24] Consolidate private allocations in seq files
From: Pavel Emelyanov @ 2007-10-09 15:50 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, devel
Many (very many) seq files in net/ allocate some private data
to use it later (mostly for iteration state). All this code
was obviously get using copy-paste method, so move it into
one place.
Almost all of these places either set this private to 0, or
keep uninitialized. Some places, however, pre-initialize this
area, but there are few of them.
The seq_open_private() call just opens the seq file with
allocated and set to zero area. The __seq_open_private() call
makes the same, but returns the allocated memory to the called
to be initialized later.
I didn't measure how much of the .text section this saves,
but I suspect a lot of :) As far as the code is concerned,
this set saves ~450 lines.
Such thing may be useful for any subsystem, but I found this
mostly in the networking code and fixed only it (for a while).
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
^ permalink raw reply
* Re: [RESEND]: RNDIS host: adjust MTU instead of refusing to talk to devices with MTU < 1.5k
From: Thomas Sailer @ 2007-10-09 15:39 UTC (permalink / raw)
To: netdev; +Cc: David Brownell, greg, linux-usb-devel
In-Reply-To: <20071009151832.9B5FF1ACD0D@adsl-69-226-248-13.dsl.pltn13.pacbell.net>
On Tue, 2007-10-09 at 08:18 -0700, David Brownell wrote:
> > This patch makes the host RNDIS driver talk to RNDIS devices with an MTU
> > less than 1.5k, instead of refusing to talk to such a device.
> > Please apply.
> >
> > Signed-Off-by: Thomas Sailer <t.sailer@alumni.ethz.ch>
>
> Acked-by: David Brownell <dbrownell@users.sourceforge.net>
>
> ... but patches in drivers/net go to netdev, not linux-usb-devel.
> And it'd be nice if the patch comment mentioned some particular
> device that's been observed to act this way.
Ok, next try.
The device in question is a low power radio device that speaks the
bpqether protocol over ethernet. bpqether frames are never longer than
320 bytes, so there is no need to support 1.5kbyte MTU in the device
firmware.
Tom
Signed-Off-by: Thomas Sailer <t.sailer@alumni.ethz.ch>
--- 1/drivers/net/usb/rndis_host.c.orig 2007-09-04 17:51:11.000000000 +0200
+++ 2/drivers/net/usb/rndis_host.c 2007-09-04 17:54:26.000000000 +0200
@@ -512,11 +512,18 @@
}
tmp = le32_to_cpu(u.init_c->max_transfer_size);
if (tmp < dev->hard_mtu) {
- dev_err(&intf->dev,
- "dev can't take %u byte packets (max %u)\n",
- dev->hard_mtu, tmp);
- retval = -EINVAL;
- goto fail_and_release;
+ if (tmp <= net->hard_header_len) {
+ dev_warn(&intf->dev,
+ "dev can't take %u byte packets (max %u)\n",
+ net->hard_header_len+1, tmp);
+ retval = -EINVAL;
+ goto fail_and_release;
+ }
+ dev->hard_mtu = tmp;
+ net->mtu = dev->hard_mtu - net->hard_header_len;
+ dev_warn(&intf->dev,
+ "dev can't take %u byte packets (max %u), adjusting MTU to %u\n",
+ dev->hard_mtu, tmp, net->mtu);
}
/* REVISIT: peripheral "alignment" request is ignored ... */
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
^ permalink raw reply
* Re: [ofa-general] Re: parallel networking
From: Michael Krause @ 2007-10-09 14:59 UTC (permalink / raw)
To: Jeff Garzik, David Miller
Cc: randy.dunlap, Robert.Olsson, herbert, gaagaan, kumarkr, rdreier,
peter.p.waskiewicz.jr, hadi, linux-kernel, kaber, jagana, general,
mchan, tgraf, mingo, johnpol, shemminger, netdev, sri
In-Reply-To: <470ADF15.2090100@garzik.org>
[-- Attachment #1.1: Type: text/plain, Size: 5833 bytes --]
At 06:53 PM 10/8/2007, Jeff Garzik wrote:
>David Miller wrote:
>>From: Jeff Garzik <jeff@garzik.org>
>>Date: Mon, 08 Oct 2007 10:22:28 -0400
>>
>>>In terms of overall parallelization, both for TX as well as RX, my gut
>>>feeling is that we want to move towards an MSI-X, multi-core friendly
>>>model where packets are LIKELY to be sent and received by the same set
>>>of [cpus | cores | packages | nodes] that the [userland] processes
>>>dealing with the data.
>>The problem is that the packet schedulers want global guarantees
>>on packet ordering, not flow centric ones.
>>That is the issue Jamal is concerned about.
>
>Oh, absolutely.
>
>I think, fundamentally, any amount of cross-flow resource management done
>in software is an obstacle to concurrency.
>
>That's not a value judgement, just a statement of fact.
Correct.
>"traffic cops" are intentional bottlenecks we add to the process, to
>enable features like priority flows, filtering, or even simple socket
>fairness guarantees. Each of those bottlenecks serves a valid purpose,
>but at the end of the day, it's still a bottleneck.
>
>So, improving concurrency may require turning off useful features that
>nonetheless hurt concurrency.
Software needs to get out of the main data path - another fact of life.
>>The more I think about it, the more inevitable it seems that we really
>>might need multiple qdiscs, one for each TX queue, to pull this full
>>parallelization off.
>>But the semantics of that don't smell so nice either. If the user
>>attaches a new qdisc to "ethN", does it go to all the TX queues, or
>>what?
>>All of the traffic shaping technology deals with the device as a unary
>>object. It doesn't fit to multi-queue at all.
>
>Well the easy solutions to networking concurrency are
>
>* use virtualization to carve up the machine into chunks
>
>* use multiple net devices
>
>Since new NIC hardware is actively trying to be friendly to
>multi-channel/virt scenarios, either of these is reasonably
>straightforward given the current state of the Linux net stack. Using
>multiple net devices is especially attractive because it works very well
>with the existing packet scheduling.
>
>Both unfortunately impose a burden on the developer and admin, to force
>their apps to distribute flows across multiple [VMs | net devs].
Not the most optimal approach.
>The third alternative is to use a single net device, with SMP-friendly
>packet scheduling. Here you run into the problems you described "device
>as a unary object" etc. with the current infrastructure.
>
>With multiple TX rings, consider that we are pushing the packet scheduling
>from software to hardware... which implies
>* hardware-specific packet scheduling
>* some TC/shaping features not available, because hardware doesn't support it
For a number of years now, we have designed interconnects to support a
reasonable range of arbitration capabilities among hardware resource
sets. With reasonable classification by software to identify a hardware
resource sets (ideally interpretation of the application's view of its
priority combined with policy management software that determines how that
should map among competing application views), one can eliminate most of
the CPU cycles spent into today's implementations. I and others presented
a number of these concepts many years ago during the development which
eventually led to IB and iWARP.
- Each resource set can be assigned to a unique PCIe function or a function
group to enable function / group arbitration to the PCIe link.
- Each resource set can be assigned to a unique PCIe TC and with improved
ordering hints (coming soon) can be used to eliminate false ordering
dependencies.
- Each resource set can be assigned to a unique IB TC / SL or iWARP 802.1p
to signal priority. These can then be used to program respective link
arbitration as well as path selection to enable multi-path load balancing.
- Many IHV have picked up on the arbitration capabilities and extended them
as shown years ago by a number of us to enable resource set arbitration and
a variety of QoS based policies. If software defines a reasonable (i.e.
small) number of management and control knobs, then these can be easily
mapped to most h/w implementations. Some of us are working on how to do
this for virtualized environments and I expect these to be applicable to
all environments in the end.
One other key item to keep in mind is that unless there is contention in
the system, the majority of the QoS mechanisms are meaningless and in a
very large percentage of customer environments, they simply don't scale
with device and interconnect performance. Many applications in fact
remain processor / memory constrained and therefore do not stress the I/O
subsystem or the external interconnects making most of the software
mechanisms rather moot in real customer environments. Simple truth is it
is nearly always cheaper to over-provision the I/O / interconnects than to
use the software approach which while quite applicable in many environments
for the 1 Gbps and below speeds, generally has less meaning / value in the
10 moving to 40 moving to 100 Gbps environments. Does not really matter
whether one believes in protocol off-load or protocol on-load, the
interconnects will be able to handle all commercial workloads and perhaps
all but the most extreme HPC (even there one might contend that any
software intermediary would be discarded in favor of reducing OS / kernel
overhead from the main data path). This isn't to say that software has no
role to play only that role needs to shift from main data path overhead to
one of policy shaping and programming of h/w based arbitration. This will
hold true for both virtualized and non-virtualized environments.
Mike
[-- Attachment #1.2: Type: text/html, Size: 6660 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH][NETNS] Make ifindex generation per-namespace
From: Daniel Lezcano @ 2007-10-09 14:48 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: David Miller, Linux Netdev List, devel, Eric W. Biederman
In-Reply-To: <470B71CD.3000706@openvz.org>
Pavel Emelyanov wrote:
> Currently indexes for netdevices come sequentially one by
> one, and the same stays true even for devices that are
> created for namespaces.
>
> Side effects of this are:
> * lo device has not 1 index in a namespace. This may break
> some userspace that relies on it (and AFAIR something
> really broke in OpenVZ VEs without this);
> * after some time namespaces will have devices with indexes
> like 1000000 os similar. This might be confusing for a
> human (tools will not mind).
>
> So move the (currently "global" and static) ifindex variable
> on the struct net, making the indexes allocation look more
> like on a standalone machine.
>
> Moreover - when we have indexes intersect between namespaces,
> we may catch more BUGs in the future related to "wrong device
> was found for a given index".
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Applied and tested against netns49. Works fine.
Acked-by: Daniel Lezcano <dlezcano@fr.ibm.com>
^ permalink raw reply
* [PATCH 7/7] [IPSEC]: Move state lock into x->type->output
From: Herbert Xu @ 2007-10-09 14:36 UTC (permalink / raw)
To: David S. Miller, netdev, Herbert Xu
In-Reply-To: <20071009143517.GA20486@gondor.apana.org.au>
[IPSEC]: Move state lock into x->type->output
This patch releases the lock on the state before calling x->type->output.
It also adds the lock to the spots where they're currently needed.
Most of those places (all except mip6) are expected to disappear with
async crypto.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/ipv4/ah4.c | 7 ++++++-
net/ipv4/esp4.c | 10 ++++++++--
net/ipv6/ah6.c | 9 ++++++---
net/ipv6/esp6.c | 10 ++++++++--
net/ipv6/mip6.c | 4 ++++
net/xfrm/xfrm_output.c | 8 ++++----
6 files changed, 36 insertions(+), 12 deletions(-)
diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c
index 58af298..3513149 100644
--- a/net/ipv4/ah4.c
+++ b/net/ipv4/ah4.c
@@ -5,6 +5,7 @@
#include <net/ah.h>
#include <linux/crypto.h>
#include <linux/pfkeyv2.h>
+#include <linux/spinlock.h>
#include <net/icmp.h>
#include <net/protocol.h>
#include <asm/scatterlist.h>
@@ -97,10 +98,14 @@ static int ah_output(struct xfrm_state *x, struct sk_buff *skb)
ah->reserved = 0;
ah->spi = x->id.spi;
ah->seq_no = htonl(XFRM_SKB_CB(skb)->seq);
+
+ spin_lock_bh(&x->lock);
err = ah_mac_digest(ahp, skb, ah->auth_data);
+ memcpy(ah->auth_data, ahp->work_icv, ahp->icv_trunc_len);
+ spin_unlock_bh(&x->lock);
+
if (err)
goto error;
- memcpy(ah->auth_data, ahp->work_icv, ahp->icv_trunc_len);
top_iph->tos = iph->tos;
top_iph->ttl = iph->ttl;
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index ffd5653..452910d 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -8,6 +8,7 @@
#include <linux/kernel.h>
#include <linux/pfkeyv2.h>
#include <linux/random.h>
+#include <linux/spinlock.h>
#include <net/icmp.h>
#include <net/protocol.h>
#include <net/udp.h>
@@ -66,6 +67,8 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
top_iph->tot_len = htons(skb->len + alen);
*(skb_tail_pointer(trailer) - 1) = top_iph->protocol;
+ spin_lock_bh(&x->lock);
+
/* this is non-NULL only with UDP Encapsulation */
if (x->encap) {
struct xfrm_encap_tmpl *encap = x->encap;
@@ -111,7 +114,7 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
if (unlikely(nfrags > ESP_NUM_FAST_SG)) {
sg = kmalloc(sizeof(struct scatterlist)*nfrags, GFP_ATOMIC);
if (!sg)
- goto error;
+ goto unlock;
}
skb_to_sgvec(skb, sg, esph->enc_data+esp->conf.ivlen-skb->data, clen);
err = crypto_blkcipher_encrypt(&desc, sg, sg, clen);
@@ -120,7 +123,7 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
} while (0);
if (unlikely(err))
- goto error;
+ goto unlock;
if (esp->conf.ivlen) {
memcpy(esph->enc_data, esp->conf.ivec, esp->conf.ivlen);
@@ -133,6 +136,9 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
memcpy(pskb_put(skb, trailer, alen), esp->auth.work_icv, alen);
}
+unlock:
+ spin_unlock_bh(&x->lock);
+
ip_send_check(top_iph);
error:
diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index ff904a7..c51d775 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -29,6 +29,7 @@
#include <net/ah.h>
#include <linux/crypto.h>
#include <linux/pfkeyv2.h>
+#include <linux/spinlock.h>
#include <linux/string.h>
#include <net/icmp.h>
#include <net/ipv6.h>
@@ -284,12 +285,14 @@ static int ah6_output(struct xfrm_state *x, struct sk_buff *skb)
ah->reserved = 0;
ah->spi = x->id.spi;
ah->seq_no = htonl(XFRM_SKB_CB(skb)->seq);
+
+ spin_lock_bh(&x->lock);
err = ah_mac_digest(ahp, skb, ah->auth_data);
- if (err)
- goto error_free_iph;
memcpy(ah->auth_data, ahp->work_icv, ahp->icv_trunc_len);
+ spin_unlock_bh(&x->lock);
- err = 0;
+ if (err)
+ goto error_free_iph;
memcpy(top_iph, tmp_base, sizeof(tmp_base));
if (tmp_ext) {
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 9fc1940..7355bb0 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -34,6 +34,7 @@
#include <linux/kernel.h>
#include <linux/pfkeyv2.h>
#include <linux/random.h>
+#include <linux/spinlock.h>
#include <net/icmp.h>
#include <net/ipv6.h>
#include <net/protocol.h>
@@ -98,6 +99,8 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
esph->spi = x->id.spi;
esph->seq_no = htonl(XFRM_SKB_CB(skb)->seq);
+ spin_lock_bh(&x->lock);
+
if (esp->conf.ivlen) {
if (unlikely(!esp->conf.ivinitted)) {
get_random_bytes(esp->conf.ivec, esp->conf.ivlen);
@@ -112,7 +115,7 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
if (unlikely(nfrags > ESP_NUM_FAST_SG)) {
sg = kmalloc(sizeof(struct scatterlist)*nfrags, GFP_ATOMIC);
if (!sg)
- goto error;
+ goto unlock;
}
skb_to_sgvec(skb, sg, esph->enc_data+esp->conf.ivlen-skb->data, clen);
err = crypto_blkcipher_encrypt(&desc, sg, sg, clen);
@@ -121,7 +124,7 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
} while (0);
if (unlikely(err))
- goto error;
+ goto unlock;
if (esp->conf.ivlen) {
memcpy(esph->enc_data, esp->conf.ivec, esp->conf.ivlen);
@@ -134,6 +137,9 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
memcpy(pskb_put(skb, trailer, alen), esp->auth.work_icv, alen);
}
+unlock:
+ spin_unlock_bh(&x->lock);
+
error:
return err;
}
diff --git a/net/ipv6/mip6.c b/net/ipv6/mip6.c
index 7261c29..6475bac 100644
--- a/net/ipv6/mip6.c
+++ b/net/ipv6/mip6.c
@@ -172,7 +172,9 @@ static int mip6_destopt_output(struct xfrm_state *x, struct sk_buff *skb)
len = ((char *)hao - (char *)dstopt) + sizeof(*hao);
memcpy(&hao->addr, &iph->saddr, sizeof(hao->addr));
+ spin_lock_bh(&x->lock);
memcpy(&iph->saddr, x->coaddr, sizeof(iph->saddr));
+ spin_unlock_bh(&x->lock);
BUG_TRAP(len == x->props.header_len);
dstopt->hdrlen = (x->props.header_len >> 3) - 1;
@@ -381,7 +383,9 @@ static int mip6_rthdr_output(struct xfrm_state *x, struct sk_buff *skb)
BUG_TRAP(rt2->rt_hdr.hdrlen == 2);
memcpy(&rt2->addr, &iph->daddr, sizeof(rt2->addr));
+ spin_lock_bh(&x->lock);
memcpy(&iph->daddr, x->coaddr, sizeof(iph->daddr));
+ spin_unlock_bh(&x->lock);
return 0;
}
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index 9847bae..0eb3377 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -67,15 +67,15 @@ int xfrm_output(struct sk_buff *skb)
if (err)
goto error;
- err = x->type->output(x, skb);
- if (err)
- goto error;
-
x->curlft.bytes += skb->len;
x->curlft.packets++;
spin_unlock_bh(&x->lock);
+ err = x->type->output(x, skb);
+ if (err)
+ goto error_nolock;
+
if (!(skb->dst = dst_pop(dst))) {
err = -EHOSTUNREACH;
goto error_nolock;
^ permalink raw reply related
* [PATCH 6/7] [IPSEC]: Lock state when copying non-atomic fields to user-space
From: Herbert Xu @ 2007-10-09 14:36 UTC (permalink / raw)
To: David S. Miller, netdev, Herbert Xu
In-Reply-To: <20071009143517.GA20486@gondor.apana.org.au>
[IPSEC]: Lock state when copying non-atomic fields to user-space
This patch adds locking so that when we're copying non-atomic fields such as
life-time or coaddr to user-space we don't get a partial result.
For af_key I've changed every instance of pfkey_xfrm_state2msg apart from
expiration notification to include the keys and life-times. This is in-line
with XFRM behaviour.
The actual cases affected are:
* pfkey_getspi: No change as we don't have any keys to copy.
* key_notify_sa:
+ ADD/UPD: This wouldn't work otherwise.
+ DEL: It can't hurt.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/key/af_key.c | 35 +++++++++++++++++++++++++----------
net/xfrm/xfrm_user.c | 14 ++++++++------
2 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/net/key/af_key.c b/net/key/af_key.c
index 143d46f..7969f8a 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -655,7 +655,8 @@ static inline int pfkey_mode_to_xfrm(int mode)
}
}
-static struct sk_buff * pfkey_xfrm_state2msg(struct xfrm_state *x, int add_keys, int hsc)
+static struct sk_buff *__pfkey_xfrm_state2msg(struct xfrm_state *x,
+ int add_keys, int hsc)
{
struct sk_buff *skb;
struct sadb_msg *hdr;
@@ -1009,6 +1010,24 @@ static struct sk_buff * pfkey_xfrm_state2msg(struct xfrm_state *x, int add_keys,
return skb;
}
+
+static inline struct sk_buff *pfkey_xfrm_state2msg(struct xfrm_state *x)
+{
+ struct sk_buff *skb;
+
+ spin_lock_bh(&x->lock);
+ skb = __pfkey_xfrm_state2msg(x, 1, 3);
+ spin_unlock_bh(&x->lock);
+
+ return skb;
+}
+
+static inline struct sk_buff *pfkey_xfrm_state2msg_expire(struct xfrm_state *x,
+ int hsc)
+{
+ return __pfkey_xfrm_state2msg(x, 0, hsc);
+}
+
static struct xfrm_state * pfkey_msg2xfrm_state(struct sadb_msg *hdr,
void **ext_hdrs)
{
@@ -1322,7 +1341,7 @@ static int pfkey_getspi(struct sock *sk, struct sk_buff *skb, struct sadb_msg *h
}
err = xfrm_alloc_spi(x, min_spi, max_spi);
- resp_skb = err ? ERR_PTR(err) : pfkey_xfrm_state2msg(x, 0, 3);
+ resp_skb = err ? ERR_PTR(err) : pfkey_xfrm_state2msg(x);
if (IS_ERR(resp_skb)) {
xfrm_state_put(x);
@@ -1412,12 +1431,8 @@ static int key_notify_sa(struct xfrm_state *x, struct km_event *c)
{
struct sk_buff *skb;
struct sadb_msg *hdr;
- int hsc = 3;
-
- if (c->event == XFRM_MSG_DELSA)
- hsc = 0;
- skb = pfkey_xfrm_state2msg(x, 0, hsc);
+ skb = pfkey_xfrm_state2msg(x);
if (IS_ERR(skb))
return PTR_ERR(skb);
@@ -1529,7 +1544,7 @@ static int pfkey_get(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr,
if (x == NULL)
return -ESRCH;
- out_skb = pfkey_xfrm_state2msg(x, 1, 3);
+ out_skb = pfkey_xfrm_state2msg(x);
proto = x->id.proto;
xfrm_state_put(x);
if (IS_ERR(out_skb))
@@ -1709,7 +1724,7 @@ static int dump_sa(struct xfrm_state *x, int count, void *ptr)
struct sk_buff *out_skb;
struct sadb_msg *out_hdr;
- out_skb = pfkey_xfrm_state2msg(x, 1, 3);
+ out_skb = pfkey_xfrm_state2msg(x);
if (IS_ERR(out_skb))
return PTR_ERR(out_skb);
@@ -2910,7 +2925,7 @@ static int key_notify_sa_expire(struct xfrm_state *x, struct km_event *c)
else
hsc = 1;
- out_skb = pfkey_xfrm_state2msg(x, 0, hsc);
+ out_skb = pfkey_xfrm_state2msg_expire(x, hsc);
if (IS_ERR(out_skb))
return PTR_ERR(out_skb);
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 2cbbe5e..5238f6a 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -507,8 +507,16 @@ static int copy_to_user_state_extra(struct xfrm_state *x,
struct xfrm_usersa_info *p,
struct sk_buff *skb)
{
+ spin_lock_bh(&x->lock);
copy_to_user_state(x, p);
+ if (x->coaddr)
+ NLA_PUT(skb, XFRMA_COADDR, sizeof(*x->coaddr), x->coaddr);
+
+ if (x->lastused)
+ NLA_PUT_U64(skb, XFRMA_LASTUSED, x->lastused);
+ spin_unlock_bh(&x->lock);
+
if (x->aalg)
NLA_PUT(skb, XFRMA_ALG_AUTH, alg_len(x->aalg), x->aalg);
if (x->ealg)
@@ -522,12 +530,6 @@ static int copy_to_user_state_extra(struct xfrm_state *x,
if (x->security && copy_sec_ctx(x->security, skb) < 0)
goto nla_put_failure;
- if (x->coaddr)
- NLA_PUT(skb, XFRMA_COADDR, sizeof(*x->coaddr), x->coaddr);
-
- if (x->lastused)
- NLA_PUT_U64(skb, XFRMA_LASTUSED, x->lastused);
-
return 0;
nla_put_failure:
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox