Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] Phonet: hold socket before giving it to sk_deliver_skb()
From: Eric Dumazet @ 2009-10-15 14:52 UTC (permalink / raw)
  To: Rémi Denis-Courmont; +Cc: netdev, Rémi Denis-Courmont
In-Reply-To: <1255611600-18534-1-git-send-email-remi@remlab.net>

Rémi Denis-Courmont a écrit :
> From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> 
> Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> ---
>  net/phonet/socket.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/net/phonet/socket.c b/net/phonet/socket.c
> index 8c84190..0412beb 100644
> --- a/net/phonet/socket.c
> +++ b/net/phonet/socket.c
> @@ -112,8 +112,10 @@ void pn_deliver_sock_broadcast(struct net *net, struct sk_buff *skb)
>  			continue;
>  
>  		clone = skb_clone(skb, GFP_ATOMIC);
> -		if (clone)
> +		if (clone) {
> +			sock_hold(sknode);
>  			sk_receive_skb(sknode, clone, 0);
> +		}
>  	}
>  	spin_unlock(&pnsocks.lock);
>  }

Indeed sk_receive_skb() does a sock_put(sk)

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

^ permalink raw reply

* Re: [RFC][PATCH] pkt_sched: skbedit add support for setting mark
From: jamal @ 2009-10-15 13:09 UTC (permalink / raw)
  To: David Miller; +Cc: denys, alexander.h.duyck, netdev
In-Reply-To: <20091014.150810.264613954.davem@davemloft.net>

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

On Wed, 2009-10-14 at 15:08 -0700, David Miller wrote:

> This patch doesn't apply.

Ok, Ive tested this on a separate machine - it applies and compiles.

cheers,
jamal

[-- Attachment #2: skbe-mark --]
[-- Type: text/plain, Size: 3617 bytes --]

commit 6da17c574694ad4c02268dd64e85792051946aab
Author: Jamal Hadi Salim <hadi@cyberus.ca>
Date:   Wed Oct 14 08:16:23 2009 -0400

    [PATCH] pkt_sched: skbedit add support for setting mark
    
    This adds support for setting the skb mark.
    
    Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>

diff --git a/include/linux/tc_act/tc_skbedit.h b/include/linux/tc_act/tc_skbedit.h
index a14e461..7a2e910 100644
--- a/include/linux/tc_act/tc_skbedit.h
+++ b/include/linux/tc_act/tc_skbedit.h
@@ -26,6 +26,7 @@
 
 #define SKBEDIT_F_PRIORITY		0x1
 #define SKBEDIT_F_QUEUE_MAPPING		0x2
+#define SKBEDIT_F_MARK			0x4
 
 struct tc_skbedit {
 	tc_gen;
@@ -37,6 +38,7 @@ enum {
 	TCA_SKBEDIT_PARMS,
 	TCA_SKBEDIT_PRIORITY,
 	TCA_SKBEDIT_QUEUE_MAPPING,
+	TCA_SKBEDIT_MARK,
 	__TCA_SKBEDIT_MAX
 };
 #define TCA_SKBEDIT_MAX (__TCA_SKBEDIT_MAX - 1)
diff --git a/include/net/tc_act/tc_skbedit.h b/include/net/tc_act/tc_skbedit.h
index 6abb3ed..e103fe0 100644
--- a/include/net/tc_act/tc_skbedit.h
+++ b/include/net/tc_act/tc_skbedit.h
@@ -26,7 +26,9 @@ struct tcf_skbedit {
 	struct tcf_common	common;
 	u32			flags;
 	u32     		priority;
+	u32     		mark;
 	u16			queue_mapping;
+	/* XXX: 16-bit pad here? */
 };
 #define to_skbedit(pc) \
 	container_of(pc, struct tcf_skbedit, common)
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 4ab916b..e9607fe 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -54,6 +54,8 @@ static int tcf_skbedit(struct sk_buff *skb, struct tc_action *a,
 	if (d->flags & SKBEDIT_F_QUEUE_MAPPING &&
 	    skb->dev->real_num_tx_queues > d->queue_mapping)
 		skb_set_queue_mapping(skb, d->queue_mapping);
+	if (d->flags & SKBEDIT_F_MARK)
+		skb->mark = d->mark;
 
 	spin_unlock(&d->tcf_lock);
 	return d->tcf_action;
@@ -63,6 +65,7 @@ static const struct nla_policy skbedit_policy[TCA_SKBEDIT_MAX + 1] = {
 	[TCA_SKBEDIT_PARMS]		= { .len = sizeof(struct tc_skbedit) },
 	[TCA_SKBEDIT_PRIORITY]		= { .len = sizeof(u32) },
 	[TCA_SKBEDIT_QUEUE_MAPPING]	= { .len = sizeof(u16) },
+	[TCA_SKBEDIT_MARK]		= { .len = sizeof(u32) },
 };
 
 static int tcf_skbedit_init(struct nlattr *nla, struct nlattr *est,
@@ -72,7 +75,7 @@ static int tcf_skbedit_init(struct nlattr *nla, struct nlattr *est,
 	struct tc_skbedit *parm;
 	struct tcf_skbedit *d;
 	struct tcf_common *pc;
-	u32 flags = 0, *priority = NULL;
+	u32 flags = 0, *priority = NULL, *mark = NULL;
 	u16 *queue_mapping = NULL;
 	int ret = 0, err;
 
@@ -95,6 +98,12 @@ static int tcf_skbedit_init(struct nlattr *nla, struct nlattr *est,
 		flags |= SKBEDIT_F_QUEUE_MAPPING;
 		queue_mapping = nla_data(tb[TCA_SKBEDIT_QUEUE_MAPPING]);
 	}
+
+	if (tb[TCA_SKBEDIT_MARK] != NULL) {
+		flags |= SKBEDIT_F_MARK;
+		mark = nla_data(tb[TCA_SKBEDIT_MARK]);
+	}
+
 	if (!flags)
 		return -EINVAL;
 
@@ -124,6 +133,9 @@ static int tcf_skbedit_init(struct nlattr *nla, struct nlattr *est,
 		d->priority = *priority;
 	if (flags & SKBEDIT_F_QUEUE_MAPPING)
 		d->queue_mapping = *queue_mapping;
+	if (flags & SKBEDIT_F_MARK)
+		d->mark = *mark;
+
 	d->tcf_action = parm->action;
 
 	spin_unlock_bh(&d->tcf_lock);
@@ -161,6 +173,9 @@ static inline int tcf_skbedit_dump(struct sk_buff *skb, struct tc_action *a,
 	if (d->flags & SKBEDIT_F_QUEUE_MAPPING)
 		NLA_PUT(skb, TCA_SKBEDIT_QUEUE_MAPPING,
 			sizeof(d->queue_mapping), &d->queue_mapping);
+	if (d->flags & SKBEDIT_F_MARK)
+		NLA_PUT(skb, TCA_SKBEDIT_MARK, sizeof(d->mark),
+			&d->mark);
 	t.install = jiffies_to_clock_t(jiffies - d->tcf_tm.install);
 	t.lastuse = jiffies_to_clock_t(jiffies - d->tcf_tm.lastuse);
 	t.expires = jiffies_to_clock_t(d->tcf_tm.expires);

^ permalink raw reply related

* [PATCH] Phonet: hold socket before giving it to sk_deliver_skb()
From: Rémi Denis-Courmont @ 2009-10-15 13:00 UTC (permalink / raw)
  To: netdev; +Cc: Rémi Denis-Courmont
In-Reply-To: <3a7a7527184221041233e9ade71e4bf1@chewa.net>

From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 net/phonet/socket.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/net/phonet/socket.c b/net/phonet/socket.c
index 8c84190..0412beb 100644
--- a/net/phonet/socket.c
+++ b/net/phonet/socket.c
@@ -112,8 +112,10 @@ void pn_deliver_sock_broadcast(struct net *net, struct sk_buff *skb)
 			continue;
 
 		clone = skb_clone(skb, GFP_ATOMIC);
-		if (clone)
+		if (clone) {
+			sock_hold(sknode);
 			sk_receive_skb(sknode, clone, 0);
+		}
 	}
 	spin_unlock(&pnsocks.lock);
 }
-- 
1.6.0.4


^ permalink raw reply related

* Re: [PATCH net-next 0/5] Phonet: basic routing support
From: Rémi Denis-Courmont @ 2009-10-15 12:58 UTC (permalink / raw)
  To: netdev
In-Reply-To: <20091014.150843.59514584.davem@davemloft.net>


On Wed, 14 Oct 2009 15:08:43 -0700 (PDT), David Miller
<davem@davemloft.net> wrote:
> From: Rémi Denis-Courmont <remi@remlab.net>
> Date: Wed, 14 Oct 2009 12:47:34 +0200
> 
>>
>> This patchset provides rudimentary support for routing Phonet packets.
>> Configuration is done with the common rtnetlink infrastructure.
>>
>> This is useful when there is more than one Phonet interface in the same
>> namespace,
>> e.g. a serial bus to a cellular modem and a USB gadget function to a PC.
> 
> Looks good, applied to net-next-2.6, thanks.

Hmmrr or not. I took an old buggy patchset. Trivial fix follows. Sorry.

-- 
Rémi Denis-Courmont


^ permalink raw reply

* Re: [RFC] [PATCH 5/5] net: Encapsulate inner code of __sk_dst_reset
From: Krishna Kumar2 @ 2009-10-15 12:53 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: davem, herbert, netdev
In-Reply-To: <4AD6F11A.90908@gmail.com>

Hi Eric,

Eric Dumazet <eric.dumazet@gmail.com> wrote on 10/15/2009 03:23:30 PM:

I am responding in one post for your convenience.

> Hmm, why not cache ops->ndo_select_queue(dev, skb) choice too ?
>
>       if (ops->ndo_select_queue)
>          queue_index = ops->ndo_select_queue(dev, skb);
>       else {
>          queue_index = 0;
>          if (dev->real_num_tx_queues > 1)
>             queue_index = skb_tx_hash(dev, skb);
>       }
>       if (sk && sk->sk_dst_cache)
>          sk_record_tx_queue(sk, queue_index);
>
> Or should ndo_select_queue() method take care of calling
sk_record_tx_queue() itself ?

I initially cached for ndo_select_queue too, but felt it should not do
that and leave to the driver for each skb. But your idea of the driver
caching is good - I think ixgbe_select_queue & mlx4_en_select_queue
(but not iwm_select_queue) can internally cache it if they are calling
skb_tx_hash.

> > diff -ruNp org/net/ipv6/inet6_connection_sock.c
new/net/ipv6/inet6_connection_sock.c
> > --- org/net/ipv6/inet6_connection_sock.c   2009-10-14
18:00:17.000000000 +0530
> > +++ new/net/ipv6/inet6_connection_sock.c   2009-10-14
18:00:30.000000000 +0530
> > @@ -168,9 +168,7 @@ struct dst_entry *__inet6_csk_dst_check(
> >     if (dst) {
> >        struct rt6_info *rt = (struct rt6_info *)dst;
> >        if (rt->rt6i_flow_cache_genid != atomic_read(&flow_cache_genid))
{
> > -         sk_record_tx_queue(sk, -1);
> > -         sk->sk_dst_cache = NULL;
> > -         dst_release(dst);
> > +         ___sk_dst_reset(sk, dst);
> >           dst = NULL;
> >        }
> >     }
>
> Encapsulation seems un-necessary to me, since only use cases are
> ___sk_dst_reset(sk, sk->sk_dst_cache)
>
>
> static inline void __sk_dst_reset(struct sock *sk)
> {
>    struct dst_entry *old_dst = sk->sk_dst_cache;
>
>    sk_record_tx_queue(sk, -1);
>      sk->sk_dst_cache = NULL;
>     dst_release(old_dst);
> }

That's right. For the IPv6 case, I can simply call __sk_dst_reset(sk).
I will make this change and remove [patch 5/5].

> Hmm, two remarks :
>
> 1) It adds a 32bits hole on 64bit arches
> 2) sk_tx_queue_mapping is only read in tx path, but sits close to
> skc_refcnt, which is now only read/written in rx path (by socket lookups)
>
> But since sock_common is small, 56 bytes on x86_64,(under a cache line),
> there is nothing we can do at this moment.
>
> My plan is to move skc_refcnt at the end of sock_common and I'll need to
add
> new generic fields into sock_common to make offsetof(skc_refcnt) = 64.
>
> Next to sock_common, will be placed fields used in rx path.

So that will fix this problem too as tx mapping will then be part of
read-cache line, I guess.

> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Thanks for your feedback and approval. I will wait for any more comments
(on the ndo_select_queue also), and resubmit v2 tomorrow.

Thanks,

- KK


^ permalink raw reply

* Re: TCP_DEFER_ACCEPT is missing counter update
From: Willy Tarreau @ 2009-10-15 12:41 UTC (permalink / raw)
  To: Julian Anastasov; +Cc: David Miller, netdev, eric.dumazet
In-Reply-To: <Pine.LNX.4.58.0910151120340.2879@u.domain.uli>

Hello Julian,

On Thu, Oct 15, 2009 at 11:47:51AM +0300, Julian Anastasov wrote:
(...)
> 	If one changes TCP_DEFER_ACCEPT to create socket it
> will save wakeups but not resources. I'm wondering if the
> behavior should be changed at all. For me the options are two:
> 
> a) you want to save resources: use TCP_DEFER_ACCEPT. To help
> proxies use large values for TCP_SYNCNT and TCP_DEFER_ACCEPT.
> 
> b) you can live with wakeups and many sockets: do not use
> TCP_DEFER_ACCEPT. Suitable for servers using short timeouts
> for first request.

and c) you want to avoid wakeups as much as possible and you'd like
to drop just one empty ACK packet, so that as soon as you accept a
an HTTP connection, you can read the request without polling at all.

Right now I'm able to process a complete HTTP request without
registering the any FD in epoll *at all* for most requests if the first
two ACKs are close enough and the server responds quickly. This saves a
substantial amount of CPU cycles. Epoll is fast, but calling epoll_ctl()
100000 times a second still has a measurable cost. Doing an accept() on
an empty connection implies this cost. Waiting for data always saves this
cost, but causes the undesirable side effects that have been reported.
Waiting for data just a few milliseconds is enough to save this cost
99.99% of the time, just as skipping the first empty packet.

Since you're saying that updating the value is wrong when it's used as
a flag, would a patch to implement a specific option for this usage be
accepted ?  Either by passing a negative value to TCP_DEFER_ACCEPT, or
by using another flag ?

Thanks,
Willy


^ permalink raw reply

* Re: [Patch] (updated) netfilter: remove deprecated CONFIG_NF_CT_ACCT
From: Krzysztof Olędzki @ 2009-10-15 12:04 UTC (permalink / raw)
  To: Amerigo Wang; +Cc: linux-kernel, netdev, akpm, netfilter-devel
In-Reply-To: <20091015080211.4561.78538.sendpatchset@localhost.localdomain>

[resend, previously I incorrectly copied netfilter-devel-owner]

Adding CC to netfilter-devel@vger.kernel.org, where netfilter
related issues are discussed.

On Thu, 15 Oct 2009, Amerigo Wang wrote:

>
> CONFIG_NF_CT_ACCT is scheduled to be removed in 2.6.29.
>
> Cc: Krzysztof Piotr Oledzki <ole@ans.pl>
> Signed-off-by: WANG Cong <amwang@redhat.com>

This is not so easy - it was decided that we don't want to remove it 
just that because of connbytes. The final conclusion was to keep it 
disabled by default (if not enabled by kernel/modules/sysctl option) but 
enabling it automatically for the current NS if nefilter rules contains 
"-m connbytes".

Sorry, I should have gotten into this and finished it earlier, my bad. :(

Best regards,

  				Krzysztof Olędzki

^ permalink raw reply

* Re: [net-next-2.6 PATCH 1/4] TCPCT part 1: initial SYN exchange with SYNACK data
From: William Allen Simpson @ 2009-10-15 11:45 UTC (permalink / raw)
  To: netdev
In-Reply-To: <20091014.231540.01186298.davem@davemloft.net>

David Miller wrote:
> This callback is generic and isn't designed to take opaque
> data.  All the other arguments to this callback are strongly
> typed, and this is on purpose.
> 
> You'll need to find another way to implement this.
> 
This was the most controversial change, so I made it the first separate
patch to get strong review.

#1 You recently shot down adding a GFP_ATOMIC kref (Adam's original code),
after having approved it a year ago.

#2 The entire struct could be added to all struct request_sock, but you
already rejected adding fewer bytes to the much larger tcp_sock.  And that
isn't the best strategy, as request_sock otherwise would not have a cookie
and is intended to be small.

#3 It's not possible to wrap and extend request_sock, as that is already
done by IPv6 and others, causing a conflict.  That was suggested by
another maintainer, and I drafted some code last week, but wasn't able to
figure out a non-conflicting code path.

#4 Passing a pointer parameter is the only option left that I've discovered.
Now, you've rejected that as well....

So, lead me to "another way"?

^ permalink raw reply

* Re: NOHZ: local_softirq_pending 08
From: Jarek Poplawski @ 2009-10-15 11:40 UTC (permalink / raw)
  To: Tilman Schmidt
  Cc: David Miller, johannes, hidave.darkstar, linux-kernel, tglx,
	linux-wireless, linux-ppp, netdev, paulus
In-Reply-To: <4AD31213.6020006@imap.cc>

On 12-10-2009 13:25, Tilman Schmidt wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On Mon, 12 Oct 2009 03:32:46 -0700 (PDT), David Miller wrote:
>> The PPP receive paths in ppp_generic.c do a local_bh_disable()/
>> local_bh_enable() around packet receiving (via ppp_recv_lock()/
>> ppp_recv_unlock() in ppp_do_recv).
>>
>> So at least that part is perfectly fine.
>>
>> ppp_input(), as called from ppp_sync_process(), also disables BH's
>> around ppp_do_recv() calls (via read_lock_bh()/read_unlock_bh()).
>>
>> So that's fine too.
>>
>> Do you have a bug report or are you just scanning around looking
>> for trouble? :-)
> 
> I have encountered the message in the subject during a test of
> the Gigaset CAPI driver, and would like to determine whether
> it's a bug in the driver, a bug somewhere else, or no bug at
> all. The test scenario was PPP over ISDN with pppd+capiplugin.
> In an alternative scenario, also PPP over ISDN but with
> smpppd+capidrv, the message did not occur.
> 
> Johannes' answer pointed me to the netif_rx() function.
> The Gigaset driver itself doesn't call that function at all.
> In the scenario where I saw the message, it was the SYNC_PPP
> line discipline that did. But from your explanation I gather
> that the cause cannot lie there.
> 
> So now I'm looking for other possible causes of that message.

Anyway, I agree with Michael Buesch there is no reason to waste time
for tracking all netif_rx vs netif_rx_ni uses, and it seems we could
avoid it by using the "proper" version of raise_softirq_irqoff() in
__napi_schedule(). Could anybody try if I'm not wrong?

Thanks,
Jarek P.
---

 net/core/dev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 28b0b9e..7fc4009 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2728,7 +2728,7 @@ void __napi_schedule(struct napi_struct *n)
 
 	local_irq_save(flags);
 	list_add_tail(&n->poll_list, &__get_cpu_var(softnet_data).poll_list);
-	__raise_softirq_irqoff(NET_RX_SOFTIRQ);
+	raise_softirq_irqoff(NET_RX_SOFTIRQ);
 	local_irq_restore(flags);
 }
 EXPORT_SYMBOL(__napi_schedule);

^ permalink raw reply related

* Re: [Patch] (updated) netfilter: remove deprecated CONFIG_NF_CT_ACCT
From: Krzysztof Oledzki @ 2009-10-15 11:18 UTC (permalink / raw)
  To: Amerigo Wang; +Cc: linux-kernel, netdev, akpm, netfilter-devel-owner
In-Reply-To: <20091015080211.4561.78538.sendpatchset@localhost.localdomain>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 711 bytes --]


Adding CC to netfilter-devel-owner@vger.kernel.org, where netfilter 
related issues are discussed.

On Thu, 15 Oct 2009, Amerigo Wang wrote:

>
> CONFIG_NF_CT_ACCT is scheduled to be removed in 2.6.29.
>
> Cc: Krzysztof Piotr Oledzki <ole@ans.pl>
> Signed-off-by: WANG Cong <amwang@redhat.com>

This is not so easy - it was decided that we don't want to remove it just 
that because of connbytes. The final conclusion was to keep it disabled by 
default (if not enabled by kernel/modules/sysctl option) but enabling it 
automatically for the current NS if nefilter rules contains "-m 
connbytes".

Sorry, I should have gotten into this and finished it earlier, my bad. :(

Best regards,

 				Krzysztof Olędzki

^ permalink raw reply

* Short Notice (Winning Alert)
From: Camelot Group End Of Year Bonaza @ 2009-10-15 10:41 UTC (permalink / raw)


You have Won 891,934.00 GBP Send Your names,Address,Tel,Age
To Mr Fred Martin For Claims


^ permalink raw reply

* Re: [RFC] [PATCH 1/5] net: Introduce sk_tx_queue_mapping
From: Eric Dumazet @ 2009-10-15 10:32 UTC (permalink / raw)
  To: Krishna Kumar; +Cc: davem, netdev, herbert
In-Reply-To: <20091015055632.30145.52459.sendpatchset@localhost.localdomain>

Krishna Kumar a écrit :
> From: Krishna Kumar <krkumar2@in.ibm.com>
> 
> Introduce sk_tx_queue_mapping; and functions that set, test and get
> this value. Reset sk_tx_queue_mapping to -1 whenever the dst cache
> is set/reset, and in socket alloc & free (free probably doesn't need
> it).
> 
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
> ---
>  include/net/sock.h |   21 +++++++++++++++++++++
>  net/core/sock.c    |    7 ++++++-
>  2 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff -ruNp org/include/net/sock.h new/include/net/sock.h
> --- org/include/net/sock.h	2009-10-14 10:36:52.000000000 +0530
> +++ new/include/net/sock.h	2009-10-14 17:59:44.000000000 +0530
> @@ -107,6 +107,7 @@ struct net;
>   *	@skc_node: main hash linkage for various protocol lookup tables
>   *	@skc_nulls_node: main hash linkage for UDP/UDP-Lite protocol
>   *	@skc_refcnt: reference count
> + *	@skc_tx_queue_mapping: tx queue number for this connection
>   *	@skc_hash: hash value used with various protocol lookup tables
>   *	@skc_family: network address family
>   *	@skc_state: Connection state
> @@ -128,6 +129,7 @@ struct sock_common {
>  		struct hlist_nulls_node skc_nulls_node;
>  	};
>  	atomic_t		skc_refcnt;
> +	int			skc_tx_queue_mapping;
>  

Hmm, two remarks :

1) It adds a 32bits hole on 64bit arches
2) sk_tx_queue_mapping is only read in tx path, but sits close to
skc_refcnt, which is now only read/written in rx path (by socket lookups)

But since sock_common is small, 56 bytes on x86_64,(under a cache line),
there is nothing we can do at this moment.

My plan is to move skc_refcnt at the end of sock_common and I'll need to add
new generic fields into sock_common to make offsetof(skc_refcnt) = 64.

Next to sock_common, will be placed fields used in rx path.

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

^ permalink raw reply

* Re: [PATCH net-next-2.6] net: sk_drops consolidation
From: Eric Dumazet @ 2009-10-15 10:20 UTC (permalink / raw)
  To: Rémi Denis-Courmont; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <70b57eea9586336e9a32b8e04afb9569@chewa.net>

Rémi Denis-Courmont a écrit :
> 
> For what it's worth, Phonet parts look fine to me.
> 
> 
> 
> Acked-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> 
> 
> 
Thanks for reviewing Rémi


^ permalink raw reply

* [PATCH net-next-2.6] net: sk_drops consolidation part 2
From: Eric Dumazet @ 2009-10-15 10:12 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20091014.204021.97690983.davem@davemloft.net>

- skb_kill_datagram() can increment sk->sk_drops itself, not callers.

- UDP on IPV4 & IPV6 dropped frames (because of bad checksum or policy checks) increment sk_drops

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/core/datagram.c |    1 +
 net/ipv4/udp.c      |    2 ++
 net/ipv6/raw.c      |    1 -
 net/ipv6/udp.c      |    4 +++-
 4 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/core/datagram.c b/net/core/datagram.c
index 1c6cf3a..4d57f5e 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -262,6 +262,7 @@ int skb_kill_datagram(struct sock *sk, struct sk_buff *skb, unsigned int flags)
 	}
 
 	kfree_skb(skb);
+	atomic_inc(&sk->sk_drops);
 	sk_mem_reclaim_partial(sk);
 
 	return err;
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 45a8a7e..696e8c1 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -866,6 +866,7 @@ static unsigned int first_packet_length(struct sock *sk)
 		udp_lib_checksum_complete(skb)) {
 		UDP_INC_STATS_BH(sock_net(sk), UDP_MIB_INERRORS,
 				 IS_UDPLITE(sk));
+		atomic_inc(&sk->sk_drops);
 		__skb_unlink(skb, rcvq);
 		__skb_queue_tail(&list_kill, skb);
 	}
@@ -1185,6 +1186,7 @@ int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 
 drop:
 	UDP_INC_STATS_BH(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
+	atomic_inc(&sk->sk_drops);
 	kfree_skb(skb);
 	return -1;
 }
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index fd737ef..2f3f030 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -517,7 +517,6 @@ csum_copy_err:
 	   as some normal condition.
 	 */
 	err = (flags&MSG_DONTWAIT) ? -EAGAIN : -EHOSTUNREACH;
-	atomic_inc(&sk->sk_drops);
 	goto out;
 }
 
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index b86425b..00c2da4 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -390,11 +390,13 @@ int udpv6_queue_rcv_skb(struct sock * sk, struct sk_buff *skb)
 		if (rc == -ENOMEM)
 			UDP6_INC_STATS_BH(sock_net(sk),
 					UDP_MIB_RCVBUFERRORS, is_udplite);
-		goto drop;
+		goto drop_no_sk_drops_inc;
 	}
 
 	return 0;
 drop:
+	atomic_inc(&sk->sk_drops);
+drop_no_sk_drops_inc:
 	UDP6_INC_STATS_BH(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
 	kfree_skb(skb);
 	return -1;

^ permalink raw reply related

* Re: [RFC] [PATCH 5/5] net: Encapsulate inner code of __sk_dst_reset
From: Eric Dumazet @ 2009-10-15  9:53 UTC (permalink / raw)
  To: Krishna Kumar; +Cc: davem, netdev, herbert
In-Reply-To: <20091015055758.30145.77495.sendpatchset@localhost.localdomain>

Krishna Kumar a écrit :
> From: Krishna Kumar <krkumar2@in.ibm.com>
> 
> Reuse code by adding ___sk_dst_reset() which is called by
> __sk_dst_reset() and __sk_dst_check(). This new API is also
> called from IPv6 to hide the internals of __sk_dst_reset
> and the setting of sk_tx_queue_mapping.
> 
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
> ---
>  include/net/sock.h               |   13 +++++++++----
>  net/core/sock.c                  |    4 +---
>  net/ipv6/inet6_connection_sock.c |    4 +---
>  3 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff -ruNp org/include/net/sock.h new/include/net/sock.h
> --- org/include/net/sock.h	2009-10-14 18:00:17.000000000 +0530
> +++ new/include/net/sock.h	2009-10-14 18:00:30.000000000 +0530
> @@ -1186,17 +1186,22 @@ sk_dst_set(struct sock *sk, struct dst_e
>  }
>  
>  static inline void
> -__sk_dst_reset(struct sock *sk)
> +___sk_dst_reset(struct sock *sk, struct dst_entry *old_dst)
>  {
> -	struct dst_entry *old_dst;
> -
>  	sk_record_tx_queue(sk, -1);
> -	old_dst = sk->sk_dst_cache;
>  	sk->sk_dst_cache = NULL;
>  	dst_release(old_dst);
>  }
>  
>  static inline void
> +__sk_dst_reset(struct sock *sk)
> +{
> +	struct dst_entry *old_dst = sk->sk_dst_cache;
> +
> +	___sk_dst_reset(sk, old_dst);
> +}
> +
> +static inline void
>  sk_dst_reset(struct sock *sk)
>  {
>  	write_lock(&sk->sk_dst_lock);
> diff -ruNp org/net/core/sock.c new/net/core/sock.c
> --- org/net/core/sock.c	2009-10-14 18:00:22.000000000 +0530
> +++ new/net/core/sock.c	2009-10-14 18:00:30.000000000 +0530
> @@ -364,9 +364,7 @@ struct dst_entry *__sk_dst_check(struct 
>  	struct dst_entry *dst = sk->sk_dst_cache;
>  
>  	if (dst && dst->obsolete && dst->ops->check(dst, cookie) == NULL) {
> -		sk_record_tx_queue(sk, -1);
> -		sk->sk_dst_cache = NULL;
> -		dst_release(dst);
> +		___sk_dst_reset(sk, dst);
>  		return NULL;
>  	}
>  
> diff -ruNp org/net/ipv6/inet6_connection_sock.c new/net/ipv6/inet6_connection_sock.c
> --- org/net/ipv6/inet6_connection_sock.c	2009-10-14 18:00:17.000000000 +0530
> +++ new/net/ipv6/inet6_connection_sock.c	2009-10-14 18:00:30.000000000 +0530
> @@ -168,9 +168,7 @@ struct dst_entry *__inet6_csk_dst_check(
>  	if (dst) {
>  		struct rt6_info *rt = (struct rt6_info *)dst;
>  		if (rt->rt6i_flow_cache_genid != atomic_read(&flow_cache_genid)) {
> -			sk_record_tx_queue(sk, -1);
> -			sk->sk_dst_cache = NULL;
> -			dst_release(dst);
> +			___sk_dst_reset(sk, dst);
>  			dst = NULL;
>  		}
>  	}
> 
> 

Encapsulation seems un-necessary to me, since only use cases are
___sk_dst_reset(sk, sk->sk_dst_cache)


static inline void __sk_dst_reset(struct sock *sk)
{
	struct dst_entry *old_dst = sk->sk_dst_cache;

	sk_record_tx_queue(sk, -1);
  	sk->sk_dst_cache = NULL;
 	dst_release(old_dst);
}


^ permalink raw reply

* Re: [RFC] [PATCH 2/5] net: Use sk_tx_queue_mapping for connected sockets
From: Eric Dumazet @ 2009-10-15  9:41 UTC (permalink / raw)
  To: Krishna Kumar; +Cc: davem, netdev, herbert
In-Reply-To: <20091015055702.30145.41799.sendpatchset@localhost.localdomain>

Krishna Kumar a écrit :
> From: Krishna Kumar <krkumar2@in.ibm.com>
> 
> For connected sockets, the first run of dev_pick_tx saves the
> calculated txq in sk_tx_queue_mapping. This is not saved if
> either skb rx was recorded, or if the device has a queue select
> handler. Next iterations of dev_pick_tx uses the cached value of
> sk_tx_queue_mapping.
> 
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
> ---
>  net/core/dev.c |   24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff -ruNp org/net/core/dev.c new/net/core/dev.c
> --- org/net/core/dev.c	2009-10-14 17:59:40.000000000 +0530
> +++ new/net/core/dev.c	2009-10-14 18:00:04.000000000 +0530
> @@ -1791,13 +1791,25 @@ EXPORT_SYMBOL(skb_tx_hash);
>  static struct netdev_queue *dev_pick_tx(struct net_device *dev,
>  					struct sk_buff *skb)
>  {
> -	const struct net_device_ops *ops = dev->netdev_ops;
> -	u16 queue_index = 0;
> +	u16 queue_index;
> +	struct sock *sk = skb->sk;
> +
> +	if (sk_tx_queue_recorded(sk)) {
> +		queue_index = sk_get_tx_queue(sk);
> +	} else {
> +		const struct net_device_ops *ops = dev->netdev_ops;
>  
> -	if (ops->ndo_select_queue)
> -		queue_index = ops->ndo_select_queue(dev, skb);
> -	else if (dev->real_num_tx_queues > 1)
> -		queue_index = skb_tx_hash(dev, skb);
> +		if (ops->ndo_select_queue) {
> +			queue_index = ops->ndo_select_queue(dev, skb);
> +		} else {
> +			queue_index = 0;
> +			if (dev->real_num_tx_queues > 1)
> +				queue_index = skb_tx_hash(dev, skb);
> +
> +			if (sk && sk->sk_dst_cache)
> +				sk_record_tx_queue(sk, queue_index);
> +		}
> +	}
>  
>  	skb_set_queue_mapping(skb, queue_index);
>  	return netdev_get_tx_queue(dev, queue_index);
> 
> 

Hmm, why not cache ops->ndo_select_queue(dev, skb) choice too ?

		if (ops->ndo_select_queue)
			queue_index = ops->ndo_select_queue(dev, skb);
		else {
			queue_index = 0;
			if (dev->real_num_tx_queues > 1)
				queue_index = skb_tx_hash(dev, skb);
		}
		if (sk && sk->sk_dst_cache)
			sk_record_tx_queue(sk, queue_index);

Or should ndo_select_queue() method take care of calling sk_record_tx_queue() itself ?

^ permalink raw reply

* [PATCH] can: provide library functions for skb allocation
From: Wolfgang Grandegger @ 2009-10-15  9:22 UTC (permalink / raw)
  To: Linux Netdev List; +Cc: SocketCAN Core Mailing List, Marc Kleine-Budde

This patch makes the private functions alloc_can_skb() and
alloc_can_err_skb() of the at91_can driver public and adapts all
drivers  to use these. While making the patch I realized, that the
skb's are *not* setup consistently. It's now done as shown below:

  skb->protocol = __constant_htons(ETH_P_CAN);
  skb->pkt_type = PACKET_BROADCAST;
  skb->ip_summed = CHECKSUM_UNNECESSARY;
  *cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame));
  memset(*cf, 0, sizeof(struct can_frame));

The frame is zeroed out to avoid uninitialized data to be passed
to user space. Some drivers or library code used "htons(ETH_P_CAN)"
or did not set "pkt_type" or "ip_summed" or did not zero the fame.

Signed-off-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
---
 drivers/net/can/at91_can.c        |   32 ---------------------------
 drivers/net/can/dev.c             |   44 +++++++++++++++++++++++++++++++-------
 drivers/net/can/sja1000/sja1000.c |   12 +---------
 drivers/net/can/ti_hecc.c         |   17 +++-----------
 drivers/net/can/usb/ems_usb.c     |   16 +------------
 5 files changed, 44 insertions(+), 77 deletions(-)

Index: net-next-2.6/drivers/net/can/dev.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/dev.c
+++ net-next-2.6/drivers/net/can/dev.c
@@ -291,7 +291,7 @@ void can_put_echo_skb(struct sk_buff *sk
 		skb->sk = srcsk;
 
 		/* make settings for echo to reduce code in irq context */
-		skb->protocol = htons(ETH_P_CAN);
+		skb->protocol = __constant_htons(ETH_P_CAN);
 		skb->pkt_type = PACKET_BROADCAST;
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 		skb->dev = dev;
@@ -366,17 +366,12 @@ void can_restart(unsigned long data)
 	can_flush_echo_skb(dev);
 
 	/* send restart message upstream */
-	skb = dev_alloc_skb(sizeof(struct can_frame));
+	skb = alloc_can_err_skb(dev, &cf);
 	if (skb == NULL) {
 		err = -ENOMEM;
 		goto restart;
 	}
-	skb->dev = dev;
-	skb->protocol = htons(ETH_P_CAN);
-	cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame));
-	memset(cf, 0, sizeof(struct can_frame));
-	cf->can_id = CAN_ERR_FLAG | CAN_ERR_RESTARTED;
-	cf->can_dlc = CAN_ERR_DLC;
+	cf->can_id |= CAN_ERR_RESTARTED;
 
 	netif_rx(skb);
 
@@ -449,6 +444,39 @@ static void can_setup(struct net_device 
 	dev->features = NETIF_F_NO_CSUM;
 }
 
+struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf)
+{
+	struct sk_buff *skb;
+
+	skb = netdev_alloc_skb(dev, sizeof(struct can_frame));
+	if (unlikely(!skb))
+		return NULL;
+
+	skb->protocol = __constant_htons(ETH_P_CAN);
+	skb->pkt_type = PACKET_BROADCAST;
+	skb->ip_summed = CHECKSUM_UNNECESSARY;
+	*cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame));
+	memset(*cf, 0, sizeof(struct can_frame));
+
+	return skb;
+}
+EXPORT_SYMBOL_GPL(alloc_can_skb);
+
+struct sk_buff *alloc_can_err_skb(struct net_device *dev, struct can_frame **cf)
+{
+	struct sk_buff *skb;
+
+	skb = alloc_can_skb(dev, cf);
+	if (unlikely(!skb))
+		return NULL;
+
+	(*cf)->can_id = CAN_ERR_FLAG;
+	(*cf)->can_dlc = CAN_ERR_DLC;
+
+	return skb;
+}
+EXPORT_SYMBOL_GPL(alloc_can_err_skb);
+
 /*
  * Allocate and setup space for the CAN network device
  */
Index: net-next-2.6/drivers/net/can/at91_can.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/at91_can.c
+++ net-next-2.6/drivers/net/can/at91_can.c
@@ -221,38 +221,6 @@ static inline void set_mb_mode(const str
 	set_mb_mode_prio(priv, mb, mode, 0);
 }
 
-static struct sk_buff *alloc_can_skb(struct net_device *dev,
-		struct can_frame **cf)
-{
-	struct sk_buff *skb;
-
-	skb = netdev_alloc_skb(dev, sizeof(struct can_frame));
-	if (unlikely(!skb))
-		return NULL;
-
-	skb->protocol = htons(ETH_P_CAN);
-	skb->ip_summed = CHECKSUM_UNNECESSARY;
-	*cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame));
-
-	return skb;
-}
-
-static struct sk_buff *alloc_can_err_skb(struct net_device *dev,
-		struct can_frame **cf)
-{
-	struct sk_buff *skb;
-
-	skb = alloc_can_skb(dev, cf);
-	if (unlikely(!skb))
-		return NULL;
-
-	memset(*cf, 0, sizeof(struct can_frame));
-	(*cf)->can_id = CAN_ERR_FLAG;
-	(*cf)->can_dlc = CAN_ERR_DLC;
-
-	return skb;
-}
-
 /*
  * Swtich transceiver on or off
  */
Index: net-next-2.6/drivers/net/can/sja1000/sja1000.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/sja1000/sja1000.c
+++ net-next-2.6/drivers/net/can/sja1000/sja1000.c
@@ -296,11 +296,9 @@ static void sja1000_rx(struct net_device
 	uint8_t dlc;
 	int i;
 
-	skb = dev_alloc_skb(sizeof(struct can_frame));
+	skb = alloc_can_skb(dev, &cf);
 	if (skb == NULL)
 		return;
-	skb->dev = dev;
-	skb->protocol = htons(ETH_P_CAN);
 
 	fi = priv->read_reg(priv, REG_FI);
 	dlc = fi & 0x0F;
@@ -351,15 +349,9 @@ static int sja1000_err(struct net_device
 	enum can_state state = priv->can.state;
 	uint8_t ecc, alc;
 
-	skb = dev_alloc_skb(sizeof(struct can_frame));
+	skb = alloc_can_err_skb(dev, &cf);
 	if (skb == NULL)
 		return -ENOMEM;
-	skb->dev = dev;
-	skb->protocol = htons(ETH_P_CAN);
-	cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame));
-	memset(cf, 0, sizeof(struct can_frame));
-	cf->can_id = CAN_ERR_FLAG;
-	cf->can_dlc = CAN_ERR_DLC;
 
 	if (isrc & IRQ_DOI) {
 		/* data overrun interrupt */
Index: net-next-2.6/drivers/net/can/usb/ems_usb.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/usb/ems_usb.c
+++ net-next-2.6/drivers/net/can/usb/ems_usb.c
@@ -311,14 +311,10 @@ static void ems_usb_rx_can_msg(struct em
 	int i;
 	struct net_device_stats *stats = &dev->netdev->stats;
 
-	skb = netdev_alloc_skb(dev->netdev, sizeof(struct can_frame));
+	skb = alloc_can_skb(dev->netdev, &cf);
 	if (skb == NULL)
 		return;
 
-	skb->protocol = htons(ETH_P_CAN);
-
-	cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame));
-
 	cf->can_id = msg->msg.can_msg.id;
 	cf->can_dlc = min_t(u8, msg->msg.can_msg.length, 8);
 
@@ -346,18 +342,10 @@ static void ems_usb_rx_err(struct ems_us
 	struct sk_buff *skb;
 	struct net_device_stats *stats = &dev->netdev->stats;
 
-	skb = netdev_alloc_skb(dev->netdev, sizeof(struct can_frame));
+	skb = alloc_can_err_skb(dev->netdev, &cf);
 	if (skb == NULL)
 		return;
 
-	skb->protocol = htons(ETH_P_CAN);
-
-	cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame));
-	memset(cf, 0, sizeof(struct can_frame));
-
-	cf->can_id = CAN_ERR_FLAG;
-	cf->can_dlc = CAN_ERR_DLC;
-
 	if (msg->type == CPC_MSG_TYPE_CAN_STATE) {
 		u8 state = msg->msg.can_state;
 
Index: net-next-2.6/drivers/net/can/ti_hecc.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/ti_hecc.c
+++ net-next-2.6/drivers/net/can/ti_hecc.c
@@ -535,18 +535,15 @@ static int ti_hecc_rx_pkt(struct ti_hecc
 	u32 data, mbx_mask;
 	unsigned long flags;
 
-	skb = netdev_alloc_skb(priv->ndev, sizeof(struct can_frame));
+	skb = alloc_can_skb(priv->ndev, &cf);
 	if (!skb) {
 		if (printk_ratelimit())
 			dev_err(priv->ndev->dev.parent,
-				"ti_hecc_rx_pkt: netdev_alloc_skb() failed\n");
+				"ti_hecc_rx_pkt: alloc_can_skb() failed\n");
 		return -ENOMEM;
 	}
-	skb->protocol = __constant_htons(ETH_P_CAN);
-	skb->ip_summed = CHECKSUM_UNNECESSARY;
 
 	mbx_mask = BIT(mbxno);
-	cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame));
 	data = hecc_read_mbx(priv, mbxno, HECC_CANMID);
 	if (data & HECC_CANMID_IDE)
 		cf->can_id = (data & CAN_EFF_MASK) | CAN_EFF_FLAG;
@@ -656,19 +653,13 @@ static int ti_hecc_error(struct net_devi
 	struct sk_buff *skb;
 
 	/* propogate the error condition to the can stack */
-	skb = netdev_alloc_skb(ndev, sizeof(struct can_frame));
+	skb = alloc_can_err_skb(ndev, &cf);
 	if (!skb) {
 		if (printk_ratelimit())
 			dev_err(priv->ndev->dev.parent,
-				"ti_hecc_error: netdev_alloc_skb() failed\n");
+				"ti_hecc_error: alloc_can_err_skb() failed\n");
 		return -ENOMEM;
 	}
-	skb->protocol = __constant_htons(ETH_P_CAN);
-	skb->ip_summed = CHECKSUM_UNNECESSARY;
-	cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame));
-	memset(cf, 0, sizeof(struct can_frame));
-	cf->can_id = CAN_ERR_FLAG;
-	cf->can_dlc = CAN_ERR_DLC;
 
 	if (int_status & HECC_CANGIF_WLIF) { /* warning level int */
 		if ((int_status & HECC_CANGIF_BOIF) == 0) {

^ permalink raw reply

* [patch 5/5] [PATCH] ctcm rollback in case of errors
From: Ursula Braun @ 2009-10-15  8:54 UTC (permalink / raw)
  To: davem, netdev, linux-s390
  Cc: schwidefsky, heiko.carstens, Einar Lueck, Ursula Braun
In-Reply-To: <20091015085454.124154000@linux.vnet.ibm.com>

[-- Attachment #1: 612-ctcm-error-rollback.diff --]
[-- Type: text/plain, Size: 3211 bytes --]

From: Einar Lueck <elelueck@de.ibm.com>

Group device now cleanly reacts to failures during channel start and
implements a clean rollback.

Signed-off-by: Einar Lueck <elelueck@de.ibm.com>
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
---

 drivers/s390/net/ctcm_main.c |   59 ++++++++++++++++++++++++++++++-------------
 1 file changed, 42 insertions(+), 17 deletions(-)

diff -urpN linux-2.6/drivers/s390/net/ctcm_main.c linux-2.6-patched/drivers/s390/net/ctcm_main.c
--- linux-2.6/drivers/s390/net/ctcm_main.c	2009-10-15 10:19:32.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/ctcm_main.c	2009-10-15 10:19:52.000000000 +0200
@@ -1530,11 +1530,16 @@ static int ctcm_new_device(struct ccwgro
 	struct net_device *dev;
 	struct ccw_device *cdev0;
 	struct ccw_device *cdev1;
+	struct channel *readc;
+	struct channel *writec;
 	int ret;
+	int result;
 
 	priv = dev_get_drvdata(&cgdev->dev);
-	if (!priv)
-		return -ENODEV;
+	if (!priv) {
+		result = -ENODEV;
+		goto out_err_result;
+	}
 
 	cdev0 = cgdev->cdev[0];
 	cdev1 = cgdev->cdev[1];
@@ -1545,31 +1550,40 @@ static int ctcm_new_device(struct ccwgro
 	snprintf(write_id, CTCM_ID_SIZE, "ch-%s", dev_name(&cdev1->dev));
 
 	ret = add_channel(cdev0, type, priv);
-	if (ret)
-		return ret;
+	if (ret) {
+		result = ret;
+		goto out_err_result;
+	}
 	ret = add_channel(cdev1, type, priv);
-	if (ret)
-		return ret;
+	if (ret) {
+		result = ret;
+		goto out_remove_channel1;
+	}
 
 	ret = ccw_device_set_online(cdev0);
 	if (ret != 0) {
-		/* may be ok to fail now - can be done later */
 		CTCM_DBF_TEXT_(TRACE, CTC_DBF_NOTICE,
 			"%s(%s) set_online rc=%d",
 				CTCM_FUNTAIL, read_id, ret);
+		result = -EIO;
+		goto out_remove_channel2;
 	}
 
 	ret = ccw_device_set_online(cdev1);
 	if (ret != 0) {
-		/* may be ok to fail now - can be done later */
 		CTCM_DBF_TEXT_(TRACE, CTC_DBF_NOTICE,
 			"%s(%s) set_online rc=%d",
 				CTCM_FUNTAIL, write_id, ret);
+
+		result = -EIO;
+		goto out_ccw1;
 	}
 
 	dev = ctcm_init_netdevice(priv);
-	if (dev == NULL)
-			goto out;
+	if (dev == NULL) {
+		result = -ENODEV;
+		goto out_ccw2;
+	}
 
 	for (direction = READ; direction <= WRITE; direction++) {
 		priv->channel[direction] =
@@ -1587,12 +1601,14 @@ static int ctcm_new_device(struct ccwgro
 	/* sysfs magic */
 	SET_NETDEV_DEV(dev, &cgdev->dev);
 
-	if (register_netdev(dev))
-			goto out_dev;
+	if (register_netdev(dev)) {
+		result = -ENODEV;
+		goto out_dev;
+	}
 
 	if (ctcm_add_attributes(&cgdev->dev)) {
-		unregister_netdev(dev);
-			goto out_dev;
+		result = -ENODEV;
+		goto out_unregister;
 	}
 
 	strlcpy(priv->fsm->name, dev->name, sizeof(priv->fsm->name));
@@ -1608,13 +1624,22 @@ static int ctcm_new_device(struct ccwgro
 			priv->channel[WRITE]->id, priv->protocol);
 
 	return 0;
+out_unregister:
+	unregister_netdev(dev);
 out_dev:
 	ctcm_free_netdevice(dev);
-out:
+out_ccw2:
 	ccw_device_set_offline(cgdev->cdev[1]);
+out_ccw1:
 	ccw_device_set_offline(cgdev->cdev[0]);
-
-	return -ENODEV;
+out_remove_channel2:
+	readc = channel_get(type, read_id, READ);
+	channel_remove(readc);
+out_remove_channel1:
+	writec = channel_get(type, write_id, WRITE);
+	channel_remove(writec);
+out_err_result:
+	return result;
 }
 
 /**


^ permalink raw reply

* [patch 3/5] [PATCH] lcs: ODEBUG: object is on stack, but not annotated.
From: Ursula Braun @ 2009-10-15  8:54 UTC (permalink / raw)
  To: davem, netdev, linux-s390
  Cc: schwidefsky, heiko.carstens, Klaus-Dieter Wacker, Ursula Braun
In-Reply-To: <20091015085454.124154000@linux.vnet.ibm.com>

[-- Attachment #1: 610-lcs-odebug.diff --]
[-- Type: text/plain, Size: 993 bytes --]

From: Klaus-Dieter Wacker <kdwacker@de.ibm.com>

Timer_list structure in lcs_send_lancmd() is allocated on stack. 
Initialization with init_timer() leads to above ODEBUG message.
Instead use init_timer_on_stack() which prevents above msg.

Signed-off-by: Klaus-Dieter Wacker <kdwacker@de.ibm.com>
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
---

 drivers/s390/net/lcs.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -urpN linux-2.6/drivers/s390/net/lcs.c linux-2.6-patched/drivers/s390/net/lcs.c
--- linux-2.6/drivers/s390/net/lcs.c	2009-10-15 10:19:32.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/lcs.c	2009-10-15 10:19:51.000000000 +0200
@@ -889,7 +889,7 @@ lcs_send_lancmd(struct lcs_card *card, s
 	rc = lcs_ready_buffer(&card->write, buffer);
 	if (rc)
 		return rc;
-	init_timer(&timer);
+	init_timer_on_stack(&timer);
 	timer.function = lcs_lancmd_timeout;
 	timer.data = (unsigned long) reply;
 	timer.expires = jiffies + HZ*card->lancmd_timeout;


^ permalink raw reply

* [patch 2/5] [PATCH] af_iucv: remove duplicate sock_set_flag
From: Ursula Braun @ 2009-10-15  8:54 UTC (permalink / raw)
  To: davem, netdev, linux-s390
  Cc: schwidefsky, heiko.carstens, Hendrik Brueckner, Ursula Braun
In-Reply-To: <20091015085454.124154000@linux.vnet.ibm.com>

[-- Attachment #1: 602-af_iucv-fallout.diff --]
[-- Type: text/plain, Size: 735 bytes --]

From: Ursula Braun <ursula.braun@de.ibm.com>

Remove duplicate sock_set_flag(sk, SOCK_ZAPPED) in iucv_sock_close,
which has been overlooked in September-commit
7514bab04e567c9408fe0facbde4277f09d5eb74.

Cc: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>

---

 net/iucv/af_iucv.c |    1 -
 1 file changed, 1 deletion(-)

Index: linux-next-uschi/net/iucv/af_iucv.c
===================================================================
--- linux-next-uschi.orig/net/iucv/af_iucv.c
+++ linux-next-uschi/net/iucv/af_iucv.c
@@ -428,7 +428,6 @@ static void iucv_sock_close(struct sock 
 		break;
 
 	default:
-		sock_set_flag(sk, SOCK_ZAPPED);
 		/* nothing to do here */
 		break;
 	}


^ permalink raw reply

* [patch 4/5] [PATCH] lcs: Recognize return codes of ccw_device_set_online().
From: Ursula Braun @ 2009-10-15  8:54 UTC (permalink / raw)
  To: davem, netdev, linux-s390
  Cc: schwidefsky, heiko.carstens, Klaus-Dieter Wacker, Ursula Braun
In-Reply-To: <20091015085454.124154000@linux.vnet.ibm.com>

[-- Attachment #1: 611-lcs-set_online-rc.diff --]
[-- Type: text/plain, Size: 1385 bytes --]

From: Klaus-Dieter Wacker <kdwacker@de.ibm.com>

The creation of a new lcs device requires a call to the function
ccw_device_set_online() for the read and the write channel. Failure
of either call should terminate the lcs device creation immediately
with return code -ENODEV.

Signed-off-by: Klaus-Dieter Wacker <kdwacker@de.ibm.com>
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
---

 drivers/s390/net/lcs.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff -urpN linux-2.6/drivers/s390/net/lcs.c linux-2.6-patched/drivers/s390/net/lcs.c
--- linux-2.6/drivers/s390/net/lcs.c	2009-10-15 10:19:52.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/lcs.c	2009-10-15 10:19:52.000000000 +0200
@@ -2130,8 +2130,12 @@ lcs_new_device(struct ccwgroup_device *c
 	card->write.ccwdev = ccwgdev->cdev[1];
 
 	recover_state = card->state;
-	ccw_device_set_online(card->read.ccwdev);
-	ccw_device_set_online(card->write.ccwdev);
+	rc = ccw_device_set_online(card->read.ccwdev);
+	if (rc)
+		goto out_err;
+	rc = ccw_device_set_online(card->write.ccwdev);
+	if (rc)
+		goto out_werr;
 
 	LCS_DBF_TEXT(3, setup, "lcsnewdv");
 
@@ -2210,8 +2214,10 @@ netdev_out:
 	return 0;
 out:
 
-	ccw_device_set_offline(card->read.ccwdev);
 	ccw_device_set_offline(card->write.ccwdev);
+out_werr:
+	ccw_device_set_offline(card->read.ccwdev);
+out_err:
 	return -ENODEV;
 }
 


^ permalink raw reply

* [patch 1/5] [PATCH] af_iucv: use sk functions to modify sk->sk_ack_backlog
From: Ursula Braun @ 2009-10-15  8:54 UTC (permalink / raw)
  To: davem, netdev, linux-s390
  Cc: schwidefsky, heiko.carstens, Hendrik Brueckner, Ursula Braun
In-Reply-To: <20091015085454.124154000@linux.vnet.ibm.com>

[-- Attachment #1: 601-af_iucv-use-sk-fns.diff --]
[-- Type: text/plain, Size: 1187 bytes --]

From: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>

Instead of modifying sk->sk_ack_backlog directly, use respective
socket functions.

Signed-off-by: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
---

 net/iucv/af_iucv.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-next-uschi/net/iucv/af_iucv.c
===================================================================
--- linux-next-uschi.orig/net/iucv/af_iucv.c
+++ linux-next-uschi/net/iucv/af_iucv.c
@@ -536,7 +536,7 @@ void iucv_accept_enqueue(struct sock *pa
 	list_add_tail(&iucv_sk(sk)->accept_q, &par->accept_q);
 	spin_unlock_irqrestore(&par->accept_q_lock, flags);
 	iucv_sk(sk)->parent = parent;
-	parent->sk_ack_backlog++;
+	sk_acceptq_added(parent);
 }
 
 void iucv_accept_unlink(struct sock *sk)
@@ -547,7 +547,7 @@ void iucv_accept_unlink(struct sock *sk)
 	spin_lock_irqsave(&par->accept_q_lock, flags);
 	list_del_init(&iucv_sk(sk)->accept_q);
 	spin_unlock_irqrestore(&par->accept_q_lock, flags);
-	iucv_sk(sk)->parent->sk_ack_backlog--;
+	sk_acceptq_removed(iucv_sk(sk)->parent);
 	iucv_sk(sk)->parent = NULL;
 	sock_put(sk);
 }


^ permalink raw reply

* [patch 0/5] s390: networking patches for linux-next
From: Ursula Braun @ 2009-10-15  8:54 UTC (permalink / raw)
  To: davem, netdev, linux-s390; +Cc: schwidefsky, heiko.carstens

Dave,

here are small improvements for net/iucv/ and drivers/s390/net/ .
They apply to linux-next.

Summary:

Hendrik Brueckner (1)
af_iucv: use sk functions to modify sk->sk_ack_backlog

Ursula Braun (1)
af_iucv: remove duplicate sock_set_flag

Klaus-Dieter Wacker (2)
lcs: ODEBUG: object is on stack, but not annotated.
lcs: Recognize return codes of ccw_device_set_online().

Einar Lueck (1)
ctcm: rollback in case of errors

Thanks,
        Ursula

^ permalink raw reply

* Re: Enable syn cookies by default
From: Olaf van der Spek @ 2009-10-15  8:59 UTC (permalink / raw)
  To: netdev
In-Reply-To: <b2cc26e40910100601q7aed04acjcc9973ef06e6458f@mail.gmail.com>

On Sat, Oct 10, 2009 at 3:01 PM, Olaf van der Spek <olafvdspek@gmail.com> wrote:
> Hi,
>
> I'm forwarding Debian feature request #520668.
>
> Could syn cookies be enabled by default?
>
> AFAIK syn cookies only get send when the half-open TCP connection
> queue is full. So stuff like window scaling should work fine in normal
> situations.
>
> Speaking of which:
> When the half-open TCP connection queue is full and syn cookies are
> enabled, you get a message like "kernel: possible SYN flooding on port
> 2710. Sending cookies."
> However when syn cookies are disabled, you don't get any message (in
> kern.log), although connections to your server are timing out.
> Could such a message be added?
> Maybe with a suggestion to increase the size of that queue or to
> enable syn cookies.
>
> Greetings,
>
> Olaf
>
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=520668
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=520667
> https://bugs.launchpad.net/ubuntu/+bug/57091
>

Somebody?

^ permalink raw reply

* Re: [iproute2 PATCH] Add 'ip tuntap' support.
From: David Woodhouse @ 2009-10-15  8:56 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20090919125222.5f4718f1@s6510>

On Sat, 2009-09-19 at 12:52 -0700, Stephen Hemminger wrote:
> 
> I added it, but:
>   * cleaned up whitespace
>   * use if_tun.h from sanitized 2.6.30
>   * ifdef IFF_TUN_EXCL flag, so can build with older code
>    (currently iproute is releasing for 2.6.30) 

Where did you add it? I still don't see it in the repo at
git://git.kernel.org/pub/scm/linux/kernel/git/shemminger/iproute2.git

-- 
dwmw2


^ 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