From patchwork Mon Jun 22 12:25:25 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Herbert Xu X-Patchwork-Id: 28993 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@bilbo.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from ozlabs.org (ozlabs.org [203.10.76.45]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mx.ozlabs.org", Issuer "CA Cert Signing Authority" (verified OK)) by bilbo.ozlabs.org (Postfix) with ESMTPS id 5F24FB7167 for ; Mon, 22 Jun 2009 22:25:41 +1000 (EST) Received: by ozlabs.org (Postfix) id 50E35DDDA0; Mon, 22 Jun 2009 22:25:41 +1000 (EST) Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id B7CFBDDD0C for ; Mon, 22 Jun 2009 22:25:40 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752326AbZFVMZb (ORCPT ); Mon, 22 Jun 2009 08:25:31 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752132AbZFVMZa (ORCPT ); Mon, 22 Jun 2009 08:25:30 -0400 Received: from rhun.apana.org.au ([64.62.148.172]:57913 "EHLO arnor.apana.org.au" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751506AbZFVMZ2 (ORCPT ); Mon, 22 Jun 2009 08:25:28 -0400 Received: from gondolin.me.apana.org.au ([192.168.0.6]) by arnor.apana.org.au with esmtp (Exim 4.63 #1 (Debian)) id 1MIiaU-0000Iz-PI; Mon, 22 Jun 2009 22:25:26 +1000 Received: from herbert by gondolin.me.apana.org.au with local (Exim 4.69) (envelope-from ) id 1MIiaT-0006OX-EB; Mon, 22 Jun 2009 20:25:25 +0800 Date: Mon, 22 Jun 2009 20:25:25 +0800 From: Herbert Xu To: Oliver Hartkopp , "David S. Miller" Cc: Linux Netdev List Subject: Re: problem with 'net: Partially allow skb destructors to be used on receive path' Message-ID: <20090622122525.GA24481@gondor.apana.org.au> References: <4A3CE170.8030604@hartkopp.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4A3CE170.8030604@hartkopp.net> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Sat, Jun 20, 2009 at 03:17:36PM +0200, Oliver Hartkopp wrote: > Hello Herbert, > > i got a feedback on the SocketCAN users ML from Michel Marti that the can-raw > socket option CAN_RAW_RECV_OWN_MSGS is out of order since 2.6.30. > > https://lists.berlios.de/pipermail/socketcan-users/2009-June/000959.html OK this patch should fix the problem. net: Move rx skb_orphan call to where needed In order to get the tun driver to account packets, we need to be able to receive packets with destructors set. To be on the safe side, I added an skb_orphan call for all protocols by default since some of them (IP in particular) cannot handle receiving packets destructors properly. Now it seems that at least one protocol (CAN) expects to be able to pass skb->sk through the rx path without getting clobbered. So this patch attempts to fix this properly by moving the skb_orphan call to where it's actually needed. In particular, I've added it to skb_set_owner_[rw] which is what most users of skb->destructor call. This is actually an improvement for tun too since it means that we only give back the amount charged to the socket when the skb is passed to another socket that will also be charged accordingly. Signed-off-by: Herbert Xu Cheers, Tested-by: Oliver Hartkopp diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h index 9f80a76..d16a304 100644 --- a/include/net/sctp/sctp.h +++ b/include/net/sctp/sctp.h @@ -448,6 +448,7 @@ static inline void sctp_skb_set_owner_r(struct sk_buff *skb, struct sock *sk) { struct sctp_ulpevent *event = sctp_skb2event(skb); + skb_orphan(skb); skb->sk = sk; skb->destructor = sctp_sock_rfree; atomic_add(event->rmem_len, &sk->sk_rmem_alloc); diff --git a/include/net/sock.h b/include/net/sock.h index 07133c5..352f06b 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -1252,6 +1252,7 @@ static inline int sk_has_allocations(const struct sock *sk) static inline void skb_set_owner_w(struct sk_buff *skb, struct sock *sk) { + skb_orphan(skb); skb->sk = sk; skb->destructor = sock_wfree; /* @@ -1264,6 +1265,7 @@ static inline void skb_set_owner_w(struct sk_buff *skb, struct sock *sk) static inline void skb_set_owner_r(struct sk_buff *skb, struct sock *sk) { + skb_orphan(skb); skb->sk = sk; skb->destructor = sock_rfree; atomic_add(skb->truesize, &sk->sk_rmem_alloc); diff --git a/net/ax25/ax25_in.c b/net/ax25/ax25_in.c index 5f1d210..de56d39 100644 --- a/net/ax25/ax25_in.c +++ b/net/ax25/ax25_in.c @@ -437,8 +437,7 @@ free: int ax25_kiss_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *ptype, struct net_device *orig_dev) { - skb->sk = NULL; /* Initially we don't know who it's for */ - skb->destructor = NULL; /* Who initializes this, dammit?! */ + skb_orphan(skb); if (!net_eq(dev_net(dev), &init_net)) { kfree_skb(skb); diff --git a/net/core/dev.c b/net/core/dev.c index baf2dc1..60b5728 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -2310,8 +2310,6 @@ ncls: if (!skb) goto out; - skb_orphan(skb); - type = skb->protocol; list_for_each_entry_rcu(ptype, &ptype_base[ntohs(type) & PTYPE_HASH_MASK], list) { diff --git a/net/irda/af_irda.c b/net/irda/af_irda.c index 5922feb..cb762c8 100644 --- a/net/irda/af_irda.c +++ b/net/irda/af_irda.c @@ -913,9 +913,6 @@ static int irda_accept(struct socket *sock, struct socket *newsock, int flags) /* Clean up the original one to keep it in listen state */ irttp_listen(self->tsap); - /* Wow ! What is that ? Jean II */ - skb->sk = NULL; - skb->destructor = NULL; kfree_skb(skb); sk->sk_ack_backlog--; diff --git a/net/irda/ircomm/ircomm_lmp.c b/net/irda/ircomm/ircomm_lmp.c index 67c99d2..7ba9661 100644 --- a/net/irda/ircomm/ircomm_lmp.c +++ b/net/irda/ircomm/ircomm_lmp.c @@ -196,6 +196,7 @@ static int ircomm_lmp_data_request(struct ircomm_cb *self, /* Don't forget to refcount it - see ircomm_tty_do_softint() */ skb_get(skb); + skb_orphan(skb); skb->destructor = ircomm_lmp_flow_control; if ((self->pkt_count++ > 7) && (self->flow_status == FLOW_START)) {