From: Oliver Hartkopp <oliver@hartkopp.net>
To: David Miller <davem@davemloft.net>
Cc: Urs Thuermann <urs.thuermann@volkswagen.de>,
Linux Netdev List <netdev@vger.kernel.org>
Subject: [RESEND] can: omit unneeded skb_clone() calls
Date: Tue, 06 Jan 2009 11:18:54 +0100 [thread overview]
Message-ID: <4963300E.6010605@hartkopp.net> (raw)
[-- Attachment #1: Type: text/plain, Size: 964 bytes --]
The AF_CAN core delivered always cloned sk_buffs to the AF_CAN
protocols, although this was _only_ needed by the can-raw protocol.
With this (additionally documented) change, the AF_CAN core calls the
callback functions of the registered AF_CAN protocols with the original
(uncloned) sk_buff pointer and let's the can-raw protocol do the
skb_clone() itself which omits all unneeded skb_clone() calls for other
AF_CAN protocols.
Signed-off-by: Oliver Hartkopp <oliver@hartkopp.net>
Signed-off-by: Urs Thuermann <urs.thuermann@volkswagen.de>
---
Hello Dave,
this is a simple (and tested) improvement to omit superfluous skb_clone()
calls in the CAN core.
Please check if it's ok for the current merge window as it is not really a fix.
If it's not ok, i'll resubmit it when net-next-2.6 re-opens.
The resend was due to a removal of these parentheses:
- if ((!ro->recv_own_msgs) && (skb->sk == sk))
+ if (!ro->recv_own_msgs && skb->sk == sk)
Thanks,
Oliver
[-- Attachment #2: omit_skbclone_resend.patch --]
[-- Type: text/x-patch, Size: 3451 bytes --]
diff --git a/include/linux/can/core.h b/include/linux/can/core.h
index f50785a..25085cb 100644
--- a/include/linux/can/core.h
+++ b/include/linux/can/core.h
@@ -19,7 +19,7 @@
#include <linux/skbuff.h>
#include <linux/netdevice.h>
-#define CAN_VERSION "20081130"
+#define CAN_VERSION "20090105"
/* increment this number each time you change some user-space interface */
#define CAN_ABI_VERSION "8"
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 3dadb33..fa417ca 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -414,6 +414,12 @@ static struct hlist_head *find_rcv_list(canid_t *can_id, canid_t *mask,
* The filter can be inverted (CAN_INV_FILTER bit set in can_id) or it can
* filter for error frames (CAN_ERR_FLAG bit set in mask).
*
+ * The provided pointer to the sk_buff is guaranteed to be valid as long as
+ * the callback function is running. The callback function must *not* free
+ * the given sk_buff while processing it's task. When the given sk_buff is
+ * needed after the end of the callback function it must be cloned inside
+ * the callback function with skb_clone().
+ *
* Return:
* 0 on success
* -ENOMEM on missing cache mem to create subscription entry
@@ -569,13 +575,8 @@ EXPORT_SYMBOL(can_rx_unregister);
static inline void deliver(struct sk_buff *skb, struct receiver *r)
{
- struct sk_buff *clone = skb_clone(skb, GFP_ATOMIC);
-
- if (clone) {
- clone->sk = skb->sk;
- r->func(clone, r->data);
- r->matches++;
- }
+ r->func(skb, r->data);
+ r->matches++;
}
static int can_rcv_filter(struct dev_rcv_lists *d, struct sk_buff *skb)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 6248ae2..1649c8a 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -633,7 +633,7 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
hrtimer_cancel(&op->timer);
if (op->can_id != rxframe->can_id)
- goto rx_freeskb;
+ return;
/* save rx timestamp */
op->rx_stamp = skb->tstamp;
@@ -645,19 +645,19 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
if (op->flags & RX_RTR_FRAME) {
/* send reply for RTR-request (placed in op->frames[0]) */
bcm_can_tx(op);
- goto rx_freeskb;
+ return;
}
if (op->flags & RX_FILTER_ID) {
/* the easiest case */
bcm_rx_update_and_send(op, &op->last_frames[0], rxframe);
- goto rx_freeskb_starttimer;
+ goto rx_starttimer;
}
if (op->nframes == 1) {
/* simple compare with index 0 */
bcm_rx_cmp_to_index(op, 0, rxframe);
- goto rx_freeskb_starttimer;
+ goto rx_starttimer;
}
if (op->nframes > 1) {
@@ -678,10 +678,8 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
}
}
-rx_freeskb_starttimer:
+rx_starttimer:
bcm_rx_starttimer(op);
-rx_freeskb:
- kfree_skb(skb);
}
/*
diff --git a/net/can/raw.c b/net/can/raw.c
index 27aab63..0703cba 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -99,13 +99,14 @@ static void raw_rcv(struct sk_buff *skb, void *data)
struct raw_sock *ro = raw_sk(sk);
struct sockaddr_can *addr;
- if (!ro->recv_own_msgs) {
- /* check the received tx sock reference */
- if (skb->sk == sk) {
- kfree_skb(skb);
- return;
- }
- }
+ /* check the received tx sock reference */
+ if (!ro->recv_own_msgs && skb->sk == sk)
+ return;
+
+ /* clone the given skb to be able to enqueue it into the rcv queue */
+ skb = skb_clone(skb, GFP_ATOMIC);
+ if (!skb)
+ return;
/*
* Put the datagram to the queue so that raw_recvmsg() can
next reply other threads:[~2009-01-06 10:18 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-01-06 10:18 Oliver Hartkopp [this message]
2009-01-06 19:08 ` [RESEND] can: omit unneeded skb_clone() calls David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4963300E.6010605@hartkopp.net \
--to=oliver@hartkopp.net \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
--cc=urs.thuermann@volkswagen.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).