netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] caif: Bugfix add check NULL pointer before calling functions.
@ 2011-05-22 21:18 Sjur Brændeland
  2011-05-22 21:18 ` [PATCH 2/5] caif: Fixes freeze on Link layer removal Sjur Brændeland
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Sjur Brændeland @ 2011-05-22 21:18 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Sjur Brændeland

Add check on layer->dn != NULL before calling functions in
layer below.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 net/caif/cfctrl.c |   35 ++++++++++++++++++++++++++---------
 1 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/net/caif/cfctrl.c b/net/caif/cfctrl.c
index 0c00a60..f8ac313 100644
--- a/net/caif/cfctrl.c
+++ b/net/caif/cfctrl.c
@@ -178,20 +178,23 @@ static void init_info(struct caif_payload_info *info, struct cfctrl *cfctrl)
 void cfctrl_enum_req(struct cflayer *layer, u8 physlinkid)
 {
 	struct cfctrl *cfctrl = container_obj(layer);
-	int ret;
 	struct cfpkt *pkt = cfpkt_create(CFPKT_CTRL_PKT_LEN);
+	struct cflayer *dn = cfctrl->serv.layer.dn;
 	if (!pkt) {
 		pr_warn("Out of memory\n");
 		return;
 	}
+	if (!dn) {
+		pr_debug("not able to send enum request\n");
+		return;
+	}
 	caif_assert(offsetof(struct cfctrl, serv.layer) == 0);
 	init_info(cfpkt_info(pkt), cfctrl);
 	cfpkt_info(pkt)->dev_info->id = physlinkid;
 	cfctrl->serv.dev_info.id = physlinkid;
 	cfpkt_addbdy(pkt, CFCTRL_CMD_ENUM);
 	cfpkt_addbdy(pkt, physlinkid);
-	ret =
-	    cfctrl->serv.layer.dn->transmit(cfctrl->serv.layer.dn, pkt);
+	dn->transmit(dn, pkt);
 }
 
 int cfctrl_linkup_request(struct cflayer *layer,
@@ -206,6 +209,12 @@ int cfctrl_linkup_request(struct cflayer *layer,
 	int ret;
 	char utility_name[16];
 	struct cfpkt *pkt;
+	struct cflayer *dn = cfctrl->serv.layer.dn;
+
+	if (!dn) {
+		pr_debug("not able to send linkup request\n");
+		return -ENODEV;
+	}
 
 	if (cfctrl_cancel_req(layer, user_layer) > 0) {
 		/* Slight Paranoia, check if already connecting */
@@ -282,7 +291,7 @@ int cfctrl_linkup_request(struct cflayer *layer,
 	 */
 	cfpkt_info(pkt)->dev_info->id = param->phyid;
 	ret =
-	    cfctrl->serv.layer.dn->transmit(cfctrl->serv.layer.dn, pkt);
+	    dn->transmit(dn, pkt);
 	if (ret < 0) {
 		int count;
 
@@ -301,15 +310,23 @@ int cfctrl_linkdown_req(struct cflayer *layer, u8 channelid,
 	int ret;
 	struct cfctrl *cfctrl = container_obj(layer);
 	struct cfpkt *pkt = cfpkt_create(CFPKT_CTRL_PKT_LEN);
+	struct cflayer *dn = cfctrl->serv.layer.dn;
+
 	if (!pkt) {
 		pr_warn("Out of memory\n");
 		return -ENOMEM;
 	}
+
+	if (!dn) {
+		pr_debug("not able to send link-down request\n");
+		return -ENODEV;
+	}
+
 	cfpkt_addbdy(pkt, CFCTRL_CMD_LINK_DESTROY);
 	cfpkt_addbdy(pkt, channelid);
 	init_info(cfpkt_info(pkt), cfctrl);
 	ret =
-	    cfctrl->serv.layer.dn->transmit(cfctrl->serv.layer.dn, pkt);
+	    dn->transmit(dn, pkt);
 #ifndef CAIF_NO_LOOP
 	cfctrl->loop_linkused[channelid] = 0;
 #endif
@@ -477,7 +494,7 @@ static int cfctrl_recv(struct cflayer *layer, struct cfpkt *pkt)
 				cfpkt_extr_head(pkt, &param, len);
 				break;
 			default:
-				pr_warn("Request setup - invalid link type (%d)\n",
+				pr_warn("Request setup, invalid type (%d)\n",
 					serv);
 				goto error;
 			}
@@ -489,7 +506,8 @@ static int cfctrl_recv(struct cflayer *layer, struct cfpkt *pkt)
 
 			if (CFCTRL_ERR_BIT == (CFCTRL_ERR_BIT & cmdrsp) ||
 				cfpkt_erroneous(pkt)) {
-				pr_err("Invalid O/E bit or parse error on CAIF control channel\n");
+				pr_err("Invalid O/E bit or parse error "
+						"on CAIF control channel\n");
 				cfctrl->res.reject_rsp(cfctrl->serv.layer.up,
 						       0,
 						       req ? req->client_layer
@@ -550,9 +568,8 @@ static void cfctrl_ctrlcmd(struct cflayer *layr, enum caif_ctrlcmd ctrl,
 	case _CAIF_CTRLCMD_PHYIF_FLOW_OFF_IND:
 	case CAIF_CTRLCMD_FLOW_OFF_IND:
 		spin_lock_bh(&this->info_list_lock);
-		if (!list_empty(&this->list)) {
+		if (!list_empty(&this->list))
 			pr_debug("Received flow off in control layer\n");
-		}
 		spin_unlock_bh(&this->info_list_lock);
 		break;
 	case _CAIF_CTRLCMD_PHYIF_DOWN_IND: {
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/5] caif: Fixes freeze on Link layer removal.
  2011-05-22 21:18 [PATCH 1/5] caif: Bugfix add check NULL pointer before calling functions Sjur Brændeland
@ 2011-05-22 21:18 ` Sjur Brændeland
  2011-05-23  0:13   ` David Miller
  2011-05-22 21:18 ` [PATCH 3/5] caif: Fix freezes when running CAIF loopback device Sjur Brændeland
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Sjur Brændeland @ 2011-05-22 21:18 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Sjur Brændeland

CAIF Socket layer - caif_socket.c:
- Plug mem-leak at reconnect.
- Always call disconnect to cleanup CAIF stack.
- Disconnect will always report success.

CAIF configuration layer - cfcnfg.c
- Disconnect must dismantle the caif stack correctly
- Protect against faulty removals (check on id zero)

CAIF mux layer - cfmuxl.c
- When inserting new service layer in the MUX remove
  any old entries with the same ID.
- When removing CAIF Link layer, remove the associated
  service layers before notifying service layers.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 net/caif/caif_socket.c |   13 ++++-------
 net/caif/cfcnfg.c      |   44 ++++++++++++++++++------------------------
 net/caif/cfmuxl.c      |   49 +++++++++++++++++++++++++++++++++++++----------
 3 files changed, 62 insertions(+), 44 deletions(-)

diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index b840395..a986280 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -19,7 +19,7 @@
 #include <linux/uaccess.h>
 #include <linux/debugfs.h>
 #include <linux/caif/caif_socket.h>
-#include <asm/atomic.h>
+#include <linux/atomic.h>
 #include <net/sock.h>
 #include <net/tcp_states.h>
 #include <net/caif/caif_layer.h>
@@ -816,6 +816,7 @@ static int caif_connect(struct socket *sock, struct sockaddr *uaddr,
 		if (sk->sk_shutdown & SHUTDOWN_MASK) {
 			/* Allow re-connect after SHUTDOWN_IND */
 			caif_disconnect_client(sock_net(sk), &cf_sk->layer);
+			caif_free_client(&cf_sk->layer);
 			break;
 		}
 		/* No reconnect on a seqpacket socket */
@@ -926,7 +927,6 @@ static int caif_release(struct socket *sock)
 {
 	struct sock *sk = sock->sk;
 	struct caifsock *cf_sk = container_of(sk, struct caifsock, sk);
-	int res = 0;
 
 	if (!sk)
 		return 0;
@@ -953,10 +953,7 @@ static int caif_release(struct socket *sock)
 	sk->sk_state = CAIF_DISCONNECTED;
 	sk->sk_shutdown = SHUTDOWN_MASK;
 
-	if (cf_sk->sk.sk_socket->state == SS_CONNECTED ||
-		cf_sk->sk.sk_socket->state == SS_CONNECTING)
-		res = caif_disconnect_client(sock_net(sk), &cf_sk->layer);
-
+	caif_disconnect_client(sock_net(sk), &cf_sk->layer);
 	cf_sk->sk.sk_socket->state = SS_DISCONNECTING;
 	wake_up_interruptible_poll(sk_sleep(sk), POLLERR|POLLHUP);
 
@@ -964,7 +961,7 @@ static int caif_release(struct socket *sock)
 	sk_stream_kill_queues(&cf_sk->sk);
 	release_sock(sk);
 	sock_put(sk);
-	return res;
+	return 0;
 }
 
 /* Copied from af_unix.c:unix_poll(), added CAIF tx_flow handling */
@@ -1120,7 +1117,7 @@ static int caif_create(struct net *net, struct socket *sock, int protocol,
 	set_rx_flow_on(cf_sk);
 
 	/* Set default options on configuration */
-	cf_sk->sk.sk_priority= CAIF_PRIO_NORMAL;
+	cf_sk->sk.sk_priority = CAIF_PRIO_NORMAL;
 	cf_sk->conn_req.link_selector = CAIF_LINK_LOW_LATENCY;
 	cf_sk->conn_req.protocol = protocol;
 	/* Increase the number of sockets created. */
diff --git a/net/caif/cfcnfg.c b/net/caif/cfcnfg.c
index 351c2ca..52fe33b 100644
--- a/net/caif/cfcnfg.c
+++ b/net/caif/cfcnfg.c
@@ -182,39 +182,26 @@ static int cfcnfg_get_id_from_ifi(struct cfcnfg *cnfg, int ifi)
 
 int caif_disconnect_client(struct net *net, struct cflayer *adap_layer)
 {
-	u8 channel_id = 0;
-	int ret = 0;
-	struct cflayer *servl = NULL;
+	u8 channel_id;
 	struct cfcnfg *cfg = get_cfcnfg(net);
 
 	caif_assert(adap_layer != NULL);
-
-	channel_id = adap_layer->id;
-	if (adap_layer->dn == NULL || channel_id == 0) {
-		pr_err("adap_layer->dn == NULL or adap_layer->id is 0\n");
-		ret = -ENOTCONN;
-		goto end;
-	}
-
-	servl = cfmuxl_remove_uplayer(cfg->mux, channel_id);
-	if (servl == NULL) {
-		pr_err("PROTOCOL ERROR - "
-				"Error removing service_layer Channel_Id(%d)",
-				channel_id);
-		ret = -EINVAL;
-		goto end;
-	}
-
-	ret = cfctrl_linkdown_req(cfg->ctrl, channel_id, adap_layer);
-
-end:
 	cfctrl_cancel_req(cfg->ctrl, adap_layer);
+	channel_id = adap_layer->id;
+	if (channel_id != 0) {
+		struct cflayer *servl;
+		servl = cfmuxl_remove_uplayer(cfg->mux, channel_id);
+		if (servl != NULL)
+			layer_set_up(servl, NULL);
+	} else
+		pr_debug("nothing to disconnect\n");
+	cfctrl_linkdown_req(cfg->ctrl, channel_id, adap_layer);
 
 	/* Do RCU sync before initiating cleanup */
 	synchronize_rcu();
 	if (adap_layer->ctrlcmd != NULL)
 		adap_layer->ctrlcmd(adap_layer, CAIF_CTRLCMD_DEINIT_RSP, 0);
-	return ret;
+	return 0;
 
 }
 EXPORT_SYMBOL(caif_disconnect_client);
@@ -400,6 +387,14 @@ cfcnfg_linkup_rsp(struct cflayer *layer, u8 channel_id, enum cfctrl_srv serv,
 	struct cfcnfg_phyinfo *phyinfo;
 	struct net_device *netdev;
 
+	if (channel_id == 0) {
+		pr_warn("received channel_id zero\n");
+		if (adapt_layer != NULL && adapt_layer->ctrlcmd != NULL)
+			adapt_layer->ctrlcmd(adapt_layer,
+						CAIF_CTRLCMD_INIT_FAIL_RSP, 0);
+		return;
+	}
+
 	rcu_read_lock();
 
 	if (adapt_layer == NULL) {
@@ -523,7 +518,6 @@ got_phyid:
 	phyinfo->use_stx = stx;
 	phyinfo->use_fcs = fcs;
 
-	phy_layer->type = phy_type;
 	frml = cffrml_create(phyid, fcs);
 
 	if (!frml) {
diff --git a/net/caif/cfmuxl.c b/net/caif/cfmuxl.c
index 2a56df7..3a66b8c 100644
--- a/net/caif/cfmuxl.c
+++ b/net/caif/cfmuxl.c
@@ -62,16 +62,6 @@ struct cflayer *cfmuxl_create(void)
 	return &this->layer;
 }
 
-int cfmuxl_set_uplayer(struct cflayer *layr, struct cflayer *up, u8 linkid)
-{
-	struct cfmuxl *muxl = container_obj(layr);
-
-	spin_lock_bh(&muxl->receive_lock);
-	list_add_rcu(&up->node, &muxl->srvl_list);
-	spin_unlock_bh(&muxl->receive_lock);
-	return 0;
-}
-
 int cfmuxl_set_dnlayer(struct cflayer *layr, struct cflayer *dn, u8 phyid)
 {
 	struct cfmuxl *muxl = (struct cfmuxl *) layr;
@@ -93,6 +83,24 @@ static struct cflayer *get_from_id(struct list_head *list, u16 id)
 	return NULL;
 }
 
+int cfmuxl_set_uplayer(struct cflayer *layr, struct cflayer *up, u8 linkid)
+{
+	struct cfmuxl *muxl = container_obj(layr);
+	struct cflayer *old;
+
+	spin_lock_bh(&muxl->receive_lock);
+
+	/* Two entries with same id is wrong, so remove old layer from mux */
+	old = get_from_id(&muxl->srvl_list, linkid);
+	if (old != NULL)
+		list_del_rcu(&old->node);
+
+	list_add_rcu(&up->node, &muxl->srvl_list);
+	spin_unlock_bh(&muxl->receive_lock);
+
+	return 0;
+}
+
 struct cflayer *cfmuxl_remove_dnlayer(struct cflayer *layr, u8 phyid)
 {
 	struct cfmuxl *muxl = container_obj(layr);
@@ -146,6 +154,11 @@ struct cflayer *cfmuxl_remove_uplayer(struct cflayer *layr, u8 id)
 	struct cfmuxl *muxl = container_obj(layr);
 	int idx = id % UP_CACHE_SIZE;
 
+	if (id == 0) {
+		pr_warn("Trying to remove control layer\n");
+		return NULL;
+	}
+
 	spin_lock_bh(&muxl->receive_lock);
 	up = get_from_id(&muxl->srvl_list, id);
 	if (up == NULL)
@@ -235,12 +248,26 @@ static void cfmuxl_ctrlcmd(struct cflayer *layr, enum caif_ctrlcmd ctrl,
 {
 	struct cfmuxl *muxl = container_obj(layr);
 	struct cflayer *layer;
+	int idx;
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(layer, &muxl->srvl_list, node) {
-		if (cfsrvl_phyid_match(layer, phyid) && layer->ctrlcmd)
+
+		if (cfsrvl_phyid_match(layer, phyid) && layer->ctrlcmd) {
+
+			if ((ctrl == _CAIF_CTRLCMD_PHYIF_FLOW_OFF_IND ||
+				ctrl == CAIF_CTRLCMD_REMOTE_SHUTDOWN_IND) &&
+					layer->id != 0) {
+
+				idx = layer->id % UP_CACHE_SIZE;
+				spin_lock_bh(&muxl->receive_lock);
+				rcu_assign_pointer(muxl->up_cache[idx], NULL);
+				list_del_rcu(&layer->node);
+				spin_unlock_bh(&muxl->receive_lock);
+			}
 			/* NOTE: ctrlcmd is not allowed to block */
 			layer->ctrlcmd(layer, ctrl, phyid);
+		}
 	}
 	rcu_read_unlock();
 }
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/5] caif: Fix freezes when running CAIF loopback device
  2011-05-22 21:18 [PATCH 1/5] caif: Bugfix add check NULL pointer before calling functions Sjur Brændeland
  2011-05-22 21:18 ` [PATCH 2/5] caif: Fixes freeze on Link layer removal Sjur Brændeland
@ 2011-05-22 21:18 ` Sjur Brændeland
  2011-05-23  0:13   ` David Miller
  2011-05-22 21:18 ` [PATCH 4/5] caif: Update documentation of CAIF transmit and receive functions Sjur Brændeland
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Sjur Brændeland @ 2011-05-22 21:18 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Sjur Brændeland

Fix spinlock bugs when running out of link-ids in loopback tests and
avoid allocating link-id when error is set in link-setup-response.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 net/caif/cfctrl.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/caif/cfctrl.c b/net/caif/cfctrl.c
index f8ac313..e22671b 100644
--- a/net/caif/cfctrl.c
+++ b/net/caif/cfctrl.c
@@ -368,7 +368,8 @@ static int cfctrl_recv(struct cflayer *layer, struct cfpkt *pkt)
 	cfpkt_extr_head(pkt, &cmdrsp, 1);
 	cmd = cmdrsp & CFCTRL_CMD_MASK;
 	if (cmd != CFCTRL_CMD_LINK_ERR
-	    && CFCTRL_RSP_BIT != (CFCTRL_RSP_BIT & cmdrsp)) {
+	    && CFCTRL_RSP_BIT != (CFCTRL_RSP_BIT & cmdrsp)
+		&& CFCTRL_ERR_BIT != (CFCTRL_ERR_BIT & cmdrsp)) {
 		if (handle_loop(cfctrl, cmd, pkt) != 0)
 			cmdrsp |= CFCTRL_ERR_BIT;
 	}
@@ -604,16 +605,16 @@ static int handle_loop(struct cfctrl *ctrl, int cmd, struct cfpkt *pkt)
 	case CFCTRL_CMD_LINK_SETUP:
 		spin_lock_bh(&ctrl->loop_linkid_lock);
 		if (!dec) {
-			for (linkid = last_linkid + 1; linkid < 255; linkid++)
+			for (linkid = last_linkid + 1; linkid < 254; linkid++)
 				if (!ctrl->loop_linkused[linkid])
 					goto found;
 		}
 		dec = 1;
-		for (linkid = last_linkid - 1; linkid > 0; linkid--)
+		for (linkid = last_linkid - 1; linkid > 1; linkid--)
 			if (!ctrl->loop_linkused[linkid])
 				goto found;
 		spin_unlock_bh(&ctrl->loop_linkid_lock);
-
+		return -1;
 found:
 		if (linkid < 10)
 			dec = 0;
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/5] caif: Update documentation of CAIF transmit and receive functions.
  2011-05-22 21:18 [PATCH 1/5] caif: Bugfix add check NULL pointer before calling functions Sjur Brændeland
  2011-05-22 21:18 ` [PATCH 2/5] caif: Fixes freeze on Link layer removal Sjur Brændeland
  2011-05-22 21:18 ` [PATCH 3/5] caif: Fix freezes when running CAIF loopback device Sjur Brændeland
@ 2011-05-22 21:18 ` Sjur Brændeland
  2011-05-23  0:13   ` David Miller
  2011-05-22 21:18 ` [PATCH 5/5] caif: Plug memory leak for checksum error Sjur Brændeland
  2011-05-23  0:13 ` [PATCH 1/5] caif: Bugfix add check NULL pointer before calling functions David Miller
  4 siblings, 1 reply; 10+ messages in thread
From: Sjur Brændeland @ 2011-05-22 21:18 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Sjur Brændeland

Trivial patch updating documentation in header files only.
Error handling of CAIF transmit errors was changed by commit:
      caif: Don't resend if dev_queue_xmit fails.
This patch updates the documentation accordingly.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
Hi Dave.

[David Miller]
>net/caif/cfctrl.c:cfctrl_enum_req() does not check the return value of
>the ->transmit() method.
>
>The documentation for this method states that one error, ownership
>of the packet is transferred back to the caller, which means that we
>have a leak here.
>
>Please fix this bug.

In this case the documentation is buggy, Error handling was of
transmit errors was changed the April 11th by commit 
"caif: Don't resend if dev_queue_xmit fails", but documentation was not
updated.

I'm sorry about this slip-up.

Regards,
Sjur

 include/net/caif/caif_layer.h |   36 +++++++++++++++++++++---------------
 1 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/include/net/caif/caif_layer.h b/include/net/caif/caif_layer.h
index c8b07a9..35bc788 100644
--- a/include/net/caif/caif_layer.h
+++ b/include/net/caif/caif_layer.h
@@ -15,7 +15,6 @@ struct cfpktq;
 struct caif_payload_info;
 struct caif_packet_funcs;
 
-
 #define CAIF_LAYER_NAME_SZ 16
 
 /**
@@ -33,7 +32,6 @@ do {								\
 	}							\
 } while (0)
 
-
 /**
  * enum caif_ctrlcmd - CAIF Stack Control Signaling sent in layer.ctrlcmd().
  *
@@ -141,7 +139,7 @@ enum caif_direction {
  *    - All layers must use this structure. If embedding it, then place this
  *	structure first in the layer specific structure.
  *
- *    - Each layer should not depend on any others layer private data.
+ *    - Each layer should not depend on any others layer's private data.
  *
  *    - In order to send data upwards do
  *	layer->up->receive(layer->up, packet);
@@ -155,16 +153,23 @@ struct cflayer {
 	struct list_head node;
 
 	/*
-	 *  receive() - Receive Function.
+	 *  receive() - Receive Function (non-blocking).
 	 *  Contract: Each layer must implement a receive function passing the
 	 *  CAIF packets upwards in the stack.
 	 *	Packet handling rules:
-	 *	      - The CAIF packet (cfpkt) cannot be accessed after
-	 *		     passing it to the next layer using up->receive().
+	 *	      - The CAIF packet (cfpkt) ownership is passed to the
+	 *		called receive function. This means that the the
+	 *		packet cannot be accessed after passing it to the
+	 *		above layer using up->receive().
+	 *
 	 *	      - If parsing of the packet fails, the packet must be
-	 *		     destroyed and -1 returned from the function.
+	 *		destroyed and negative error code returned
+	 *		from the function.
+	 *		EXCEPTION: If the framing layer (cffrml) returns
+	 *			-EILSEQ, the packet is not freed.
+	 *
 	 *	      - If parsing succeeds (and above layers return OK) then
-	 *		      the function must return a value > 0.
+	 *		      the function must return a value >= 0.
 	 *
 	 *  Returns result < 0 indicates an error, 0 or positive value
 	 *	     indicates success.
@@ -176,7 +181,7 @@ struct cflayer {
 	int (*receive)(struct cflayer *layr, struct cfpkt *cfpkt);
 
 	/*
-	 *  transmit() - Transmit Function.
+	 *  transmit() - Transmit Function (non-blocking).
 	 *  Contract: Each layer must implement a transmit function passing the
 	 *	CAIF packet downwards in the stack.
 	 *	Packet handling rules:
@@ -185,15 +190,16 @@ struct cflayer {
 	 *		cannot be accessed after passing it to the below
 	 *		layer using dn->transmit().
 	 *
-	 *	      - If transmit fails, however, the ownership is returned
-	 *		to thecaller. The caller of "dn->transmit()" must
-	 *		destroy or resend packet.
+	 *	      - Upon error the packet ownership is still passed on,
+	 *		so the packet shall be freed where error is detected.
+	 *		Callers of the transmit function shall not free packets,
+	 *		but errors shall be returned.
 	 *
 	 *	      - Return value less than zero means error, zero or
 	 *		greater than zero means OK.
 	 *
-	 *	 result < 0 indicates an error, 0 or positive value
-	 *	 indicate success.
+	 *  Returns result < 0 indicates an error, 0 or positive value
+	 *		indicates success.
 	 *
 	 *  @layr:	Pointer to the current layer the receive function
 	 *		isimplemented for (this pointer).
@@ -202,7 +208,7 @@ struct cflayer {
 	int (*transmit) (struct cflayer *layr, struct cfpkt *cfpkt);
 
 	/*
-	 *  cttrlcmd() - Control Function upwards in CAIF Stack.
+	 *  cttrlcmd() - Control Function upwards in CAIF Stack  (non-blocking).
 	 *  Used for signaling responses (CAIF_CTRLCMD_*_RSP)
 	 *  and asynchronous events from the modem  (CAIF_CTRLCMD_*_IND)
 	 *
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/5] caif: Plug memory leak for checksum error
  2011-05-22 21:18 [PATCH 1/5] caif: Bugfix add check NULL pointer before calling functions Sjur Brændeland
                   ` (2 preceding siblings ...)
  2011-05-22 21:18 ` [PATCH 4/5] caif: Update documentation of CAIF transmit and receive functions Sjur Brændeland
@ 2011-05-22 21:18 ` Sjur Brændeland
  2011-05-23  0:14   ` David Miller
  2011-05-23  0:13 ` [PATCH 1/5] caif: Bugfix add check NULL pointer before calling functions David Miller
  4 siblings, 1 reply; 10+ messages in thread
From: Sjur Brændeland @ 2011-05-22 21:18 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Sjur Brændeland

In case of checksum error, the framing layer returns -EILSEQ, but
does not free the packet. Plug this hole by freeing the packet if
-EILSEQ is returned.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 net/caif/caif_dev.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/net/caif/caif_dev.c b/net/caif/caif_dev.c
index 366ca0f..682c0fe 100644
--- a/net/caif/caif_dev.c
+++ b/net/caif/caif_dev.c
@@ -142,6 +142,7 @@ static int receive(struct sk_buff *skb, struct net_device *dev,
 {
 	struct cfpkt *pkt;
 	struct caif_device_entry *caifd;
+	int err;
 
 	pkt = cfpkt_fromnative(CAIF_DIR_IN, skb);
 
@@ -159,7 +160,11 @@ static int receive(struct sk_buff *skb, struct net_device *dev,
 	caifd_hold(caifd);
 	rcu_read_unlock();
 
-	caifd->layer.up->receive(caifd->layer.up, pkt);
+	err = caifd->layer.up->receive(caifd->layer.up, pkt);
+
+	/* For -EILSEQ the packet is not freed so so it now */
+	if (err == -EILSEQ)
+		cfpkt_destroy(pkt);
 
 	/* Release reference to stack upwards */
 	caifd_put(caifd);
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/5] caif: Bugfix add check NULL pointer before calling functions.
  2011-05-22 21:18 [PATCH 1/5] caif: Bugfix add check NULL pointer before calling functions Sjur Brændeland
                   ` (3 preceding siblings ...)
  2011-05-22 21:18 ` [PATCH 5/5] caif: Plug memory leak for checksum error Sjur Brændeland
@ 2011-05-23  0:13 ` David Miller
  4 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2011-05-23  0:13 UTC (permalink / raw)
  To: sjur.brandeland; +Cc: netdev

From: Sjur Brændeland <sjur.brandeland@stericsson.com>
Date: Sun, 22 May 2011 23:18:50 +0200

> Add check on layer->dn != NULL before calling functions in
> layer below.
> 
> Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

Applied.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/5] caif: Fixes freeze on Link layer removal.
  2011-05-22 21:18 ` [PATCH 2/5] caif: Fixes freeze on Link layer removal Sjur Brændeland
@ 2011-05-23  0:13   ` David Miller
  0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2011-05-23  0:13 UTC (permalink / raw)
  To: sjur.brandeland; +Cc: netdev

From: Sjur Brændeland <sjur.brandeland@stericsson.com>
Date: Sun, 22 May 2011 23:18:51 +0200

> CAIF Socket layer - caif_socket.c:
> - Plug mem-leak at reconnect.
> - Always call disconnect to cleanup CAIF stack.
> - Disconnect will always report success.
> 
> CAIF configuration layer - cfcnfg.c
> - Disconnect must dismantle the caif stack correctly
> - Protect against faulty removals (check on id zero)
> 
> CAIF mux layer - cfmuxl.c
> - When inserting new service layer in the MUX remove
>   any old entries with the same ID.
> - When removing CAIF Link layer, remove the associated
>   service layers before notifying service layers.
> 
> Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

Applied.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/5] caif: Fix freezes when running CAIF loopback device
  2011-05-22 21:18 ` [PATCH 3/5] caif: Fix freezes when running CAIF loopback device Sjur Brændeland
@ 2011-05-23  0:13   ` David Miller
  0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2011-05-23  0:13 UTC (permalink / raw)
  To: sjur.brandeland; +Cc: netdev

From: Sjur Brændeland <sjur.brandeland@stericsson.com>
Date: Sun, 22 May 2011 23:18:52 +0200

> Fix spinlock bugs when running out of link-ids in loopback tests and
> avoid allocating link-id when error is set in link-setup-response.
> 
> Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

Applied.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/5] caif: Update documentation of CAIF transmit and receive functions.
  2011-05-22 21:18 ` [PATCH 4/5] caif: Update documentation of CAIF transmit and receive functions Sjur Brændeland
@ 2011-05-23  0:13   ` David Miller
  0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2011-05-23  0:13 UTC (permalink / raw)
  To: sjur.brandeland; +Cc: netdev

From: Sjur Brændeland <sjur.brandeland@stericsson.com>
Date: Sun, 22 May 2011 23:18:53 +0200

> Trivial patch updating documentation in header files only.
> Error handling of CAIF transmit errors was changed by commit:
>       caif: Don't resend if dev_queue_xmit fails.
> This patch updates the documentation accordingly.
> 
> Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

Applied.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 5/5] caif: Plug memory leak for checksum error
  2011-05-22 21:18 ` [PATCH 5/5] caif: Plug memory leak for checksum error Sjur Brændeland
@ 2011-05-23  0:14   ` David Miller
  0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2011-05-23  0:14 UTC (permalink / raw)
  To: sjur.brandeland; +Cc: netdev

From: Sjur Brændeland <sjur.brandeland@stericsson.com>
Date: Sun, 22 May 2011 23:18:54 +0200

> In case of checksum error, the framing layer returns -EILSEQ, but
> does not free the packet. Plug this hole by freeing the packet if
> -EILSEQ is returned.
> 
> Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

Appied.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-05-23  0:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-22 21:18 [PATCH 1/5] caif: Bugfix add check NULL pointer before calling functions Sjur Brændeland
2011-05-22 21:18 ` [PATCH 2/5] caif: Fixes freeze on Link layer removal Sjur Brændeland
2011-05-23  0:13   ` David Miller
2011-05-22 21:18 ` [PATCH 3/5] caif: Fix freezes when running CAIF loopback device Sjur Brændeland
2011-05-23  0:13   ` David Miller
2011-05-22 21:18 ` [PATCH 4/5] caif: Update documentation of CAIF transmit and receive functions Sjur Brændeland
2011-05-23  0:13   ` David Miller
2011-05-22 21:18 ` [PATCH 5/5] caif: Plug memory leak for checksum error Sjur Brændeland
2011-05-23  0:14   ` David Miller
2011-05-23  0:13 ` [PATCH 1/5] caif: Bugfix add check NULL pointer before calling functions David Miller

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).