[PATCH 2/3] staging: cxt1e1: count fragmented packet properly.

[PATCH 2/3] staging: cxt1e1: count fragmented packet properly.

[Daeseok Youn]

OS_mem_token_tlen() is same return value as OS_mem_token_len().
That means packet count is always 1. So OS_mem_token_tlen()
must be total length of packet and OS_mem_token_len() has a
length of fragmented packet. And then it can count total
count of fragmented packets properly.

And OS_mem_token_next() returns NULL, it will be dereferencing
a NULL pointer. So it must return next fragmented packet buffer as
sk_buff.

Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
---
I'm not sure of this patch and not tested.(just only build test).
Please review for this.
Thanks.

 drivers/staging/cxt1e1/musycc.c              |   52 +++++++++++++++-----------
 drivers/staging/cxt1e1/sbecom_inline_linux.h |    4 +-
 2 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/cxt1e1/musycc.c b/drivers/staging/cxt1e1/musycc.c
index 563076c..5fc45a4 100644
--- a/drivers/staging/cxt1e1/musycc.c
+++ b/drivers/staging/cxt1e1/musycc.c
@@ -1579,8 +1579,8 @@ musycc_start_xmit(ci_t *ci, int channum, void *mem_token)
 	mch_t      *ch;
 	struct mdesc *md;
 	void       *m2;
-	int         txd_need_cnt;
-	u_int32_t   len;
+	int         txd_need_cnt = 0;
+	u_int32_t   len, data_len;
 
 	ch = sd_find_chan(ci, channum);
 	if (!ch)
@@ -1611,13 +1611,16 @@ musycc_start_xmit(ci_t *ci, int channum, void *mem_token)
 	/** Determine total amount of data to be sent **/
 	/***********************************************/
 	m2 = mem_token;
-	txd_need_cnt = 0;
-	for (len = OS_mem_token_tlen(m2); len > 0;
-	     m2 = (void *) OS_mem_token_next(m2)) {
-		if (!OS_mem_token_len(m2))
-			continue;
-		txd_need_cnt++;
-		len -= OS_mem_token_len(m2);
+	len = OS_mem_token_tlen(m2);
+
+	while (m2 && len > 0) {
+		data_len = OS_mem_token_len(m2);
+		if (data_len) {
+			len -= data_len;
+			txd_need_cnt++;
+		}
+
+		m2 = OS_mem_token_next(m2);
 	}
 
 	if (txd_need_cnt == 0) {
@@ -1658,13 +1661,18 @@ musycc_start_xmit(ci_t *ci, int channum, void *mem_token)
 	/**************************************************/
 	m2 = mem_token;
 	md = ch->txd_usr_add;           /* get current available descriptor */
+	len = OS_mem_token_tlen(m2);
 
-	for (len = OS_mem_token_tlen(m2); len > 0; m2 = OS_mem_token_next(m2)) {
-		int         u = OS_mem_token_len(m2);
+	while (m2 && len > 0) {
+		u_int32_t data_len = OS_mem_token_len(m2);
+		u_int32_t status = 0;
 
-		if (!u)
+		if (!data_len) {
+			m2 = OS_mem_token_next(m2);
 			continue;
-		len -= u;
+		}
+
+		len -= data_len;
 
 		/*
 		 * Enable following chunks, yet wait to enable the FIRST chunk until
@@ -1672,25 +1680,24 @@ musycc_start_xmit(ci_t *ci, int channum, void *mem_token)
 		 */
 		if (md != ch->txd_usr_add)  /* not first chunk */
 			/* transfer ownership from HOST to MUSYCC */
-			u |= MUSYCC_TX_OWNED;
+			status |= MUSYCC_TX_OWNED;
 
 		if (len)                    /* not last chunk */
-			u |= EOBIRQ_ENABLE;
+			status |= EOBIRQ_ENABLE;
 		else if (ch->p.chan_mode == CFG_CH_PROTO_TRANS) {
 			/*
 			 * Per MUSYCC Ref 6.4.9 for Transparent Mode, the host must
 			 * always clear EOMIRQ_ENABLE in every Transmit Buffer Descriptor
 			 * (IE. don't set herein).
 			 */
-			u |= EOBIRQ_ENABLE;
+			status |= EOBIRQ_ENABLE;
 		} else
-			u |= EOMIRQ_ENABLE;     /* EOM, last HDLC chunk */
-
+			status |= EOMIRQ_ENABLE;     /* EOM, last HDLC chunk */
 
 		/* last chunk in hdlc mode */
-		u |= (ch->p.idlecode << IDLE_CODE);
+		status |= (ch->p.idlecode << IDLE_CODE);
 		if (ch->p.pad_fill_count) {
-			u |= (PADFILL_ENABLE | (ch->p.pad_fill_count << EXTRA_FLAGS));
+			status |= (PADFILL_ENABLE | (ch->p.pad_fill_count << EXTRA_FLAGS));
 		}
 		/* Fill in mds on last segment, others set ZERO
 		 * so that entire token is removed ONLY when ALL
@@ -1700,13 +1707,14 @@ musycc_start_xmit(ci_t *ci, int channum, void *mem_token)
 
 		md->data = cpu_to_le32(__pa(OS_mem_token_data(m2)));
 		FLUSH_MEM_WRITE();
-		md->status = cpu_to_le32(u);
+		md->status = cpu_to_le32(status);
 		--ch->txd_free;
 		md = md->snext;
+
+		m2 = OS_mem_token_next(m2);
 	}
 	FLUSH_MEM_WRITE();
 
-
 	/*
 	 * Now transfer ownership of first chunk from HOST to MUSYCC in order to
 	 * fire-off this XMIT.
diff --git a/drivers/staging/cxt1e1/sbecom_inline_linux.h b/drivers/staging/cxt1e1/sbecom_inline_linux.h
index af2bffe..a99073b 100644
--- a/drivers/staging/cxt1e1/sbecom_inline_linux.h
+++ b/drivers/staging/cxt1e1/sbecom_inline_linux.h
@@ -82,14 +82,14 @@ OS_mem_token_data (void *token)
 static inline void *
 OS_mem_token_next (void *token)
 {
-    return NULL;
+    return ((struct sk_buff*)token)->next;
 }
 
 
 static inline int
 OS_mem_token_len (void *token)
 {
-    return ((struct sk_buff *) token)->len;
+    return ((struct sk_buff *) token)->data_len;
 }
 
 
-- 
1.7.1

Re: [PATCH 2/3] staging: cxt1e1: count fragmented packet properly.

[Greg KH]

On Fri, Jun 27, 2014 at 06:56:08PM +0900, Daeseok Youn wrote:
> OS_mem_token_tlen() is same return value as OS_mem_token_len().
> That means packet count is always 1. So OS_mem_token_tlen()
> must be total length of packet and OS_mem_token_len() has a
> length of fragmented packet. And then it can count total
> count of fragmented packets properly.
> 
> And OS_mem_token_next() returns NULL, it will be dereferencing
> a NULL pointer. So it must return next fragmented packet buffer as
> sk_buff.
> 
> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
> ---
> I'm not sure of this patch and not tested.(just only build test).
> Please review for this.
> Thanks.

Please refresh these last two based on the changes in the first one you
should do.

thanks,

greg k-h

Re: [PATCH 2/3] staging: cxt1e1: count fragmented packet properly.

[DaeSeok Youn]

2014-06-30 6:22 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> On Fri, Jun 27, 2014 at 06:56:08PM +0900, Daeseok Youn wrote:
>> OS_mem_token_tlen() is same return value as OS_mem_token_len().
>> That means packet count is always 1. So OS_mem_token_tlen()
>> must be total length of packet and OS_mem_token_len() has a
>> length of fragmented packet. And then it can count total
>> count of fragmented packets properly.
>>
>> And OS_mem_token_next() returns NULL, it will be dereferencing
>> a NULL pointer. So it must return next fragmented packet buffer as
>> sk_buff.
>>
>> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
>> ---
>> I'm not sure of this patch and not tested.(just only build test).
>> Please review for this.
>> Thanks.
>
> Please refresh these last two based on the changes in the first one you
> should do.
OK. I will sent these again after fixing 1/3.

thanks.

Daeseok Youn.
>
> thanks,
>
> greg k-h