2022-09-28 17:53:45

by Nam Cao

[permalink] [raw]
Subject: [RFC PATCH v2 0/4] staging: vt6655: Implement allocation failure handling

This driver does not handle allocation failure when receiving data very
well. This patchset implements better handling in the case of allocation
failure.

Also do some necessary clean-up to implement this.

v2:
- squash 3 commits that were doing a single thing
- add new commit which removes a redundant assignment
- take device_init_rx_desc() out of unnecessary else condition.
- remove return statement at the end of void function
- add a missing rd = rd->next statement in device_rx_srv(): because
we already drop the current buffer, we should move on to the next
buffer in the ring where new data will be written to.

Nam Cao (4):
staging: vt6655: remove redundant if condition
staging: vt6655: change vnt_receive_frame return type to void
staging: vt6655: remove redundant assignment
staging: vt6655: implement allocation failure handling

drivers/staging/vt6655/device_main.c | 40 +++++++++++++++++-----------
drivers/staging/vt6655/dpc.c | 8 +++---
drivers/staging/vt6655/dpc.h | 2 +-
3 files changed, 28 insertions(+), 22 deletions(-)

--
2.25.1


2022-09-28 17:54:50

by Nam Cao

[permalink] [raw]
Subject: [RFC PATCH v2 1/4] staging: vt6655: remove redundant if condition

The function vnt_receive_frame always returns true, so checking its
return value is redundant. Remove it.

Signed-off-by: Nam Cao <[email protected]>
---
drivers/staging/vt6655/device_main.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
index 56c3cf3ba53d..21d10d9fde47 100644
--- a/drivers/staging/vt6655/device_main.c
+++ b/drivers/staging/vt6655/device_main.c
@@ -832,12 +832,12 @@ static int device_rx_srv(struct vnt_private *priv, unsigned int idx)
if (!rd->rd_info->skb)
break;

- if (vnt_receive_frame(priv, rd)) {
- if (!device_alloc_rx_buf(priv, rd)) {
- dev_err(&priv->pcid->dev,
- "can not allocate rx buf\n");
- break;
- }
+ vnt_receive_frame(priv, rd);
+
+ if (!device_alloc_rx_buf(priv, rd)) {
+ dev_err(&priv->pcid->dev,
+ "can not allocate rx buf\n");
+ break;
}
rd->rd0.owner = OWNED_BY_NIC;
}
--
2.25.1

2022-09-28 17:56:37

by Nam Cao

[permalink] [raw]
Subject: [RFC PATCH v2 2/4] staging: vt6655: change vnt_receive_frame return type to void

The function vnt_receive_frame's returned value is not used anywhere.
Furthermore, it always return true. Change its return type to void.

Signed-off-by: Nam Cao <[email protected]>
---
drivers/staging/vt6655/dpc.c | 8 +++-----
drivers/staging/vt6655/dpc.h | 2 +-
2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/vt6655/dpc.c b/drivers/staging/vt6655/dpc.c
index c6ed3537f439..3bf60039fa9a 100644
--- a/drivers/staging/vt6655/dpc.c
+++ b/drivers/staging/vt6655/dpc.c
@@ -115,7 +115,7 @@ static bool vnt_rx_data(struct vnt_private *priv, struct sk_buff *skb,
return true;
}

-bool vnt_receive_frame(struct vnt_private *priv, struct vnt_rx_desc *curr_rd)
+void vnt_receive_frame(struct vnt_private *priv, struct vnt_rx_desc *curr_rd)
{
struct vnt_rd_info *rd_info = curr_rd->rd_info;
struct sk_buff *skb;
@@ -133,13 +133,11 @@ bool vnt_receive_frame(struct vnt_private *priv, struct vnt_rx_desc *curr_rd)
/* Frame Size error drop this packet.*/
dev_dbg(&priv->pcid->dev, "Wrong frame size %d\n", frame_size);
dev_kfree_skb_irq(skb);
- return true;
+ return;
}

if (vnt_rx_data(priv, skb, frame_size))
- return true;
+ return;

dev_kfree_skb_irq(skb);
-
- return true;
}
diff --git a/drivers/staging/vt6655/dpc.h b/drivers/staging/vt6655/dpc.h
index 40364c0ab7f6..f67c1ef23171 100644
--- a/drivers/staging/vt6655/dpc.h
+++ b/drivers/staging/vt6655/dpc.h
@@ -16,6 +16,6 @@

#include "device.h"

-bool vnt_receive_frame(struct vnt_private *priv, struct vnt_rx_desc *curr_rd);
+void vnt_receive_frame(struct vnt_private *priv, struct vnt_rx_desc *curr_rd);

#endif /* __RXTX_H__ */
--
2.25.1

2022-09-28 18:18:03

by Nam Cao

[permalink] [raw]
Subject: [RFC PATCH v2 3/4] staging: vt6655: remove redundant assignment

In function device_rx_srv, the assignment rd->rd0.owner = OWNED_BY_NIC
is redundant because this is already done in device_alloc_rx_buf() which
is called right above. Remove it.

Signed-off-by: Nam Cao <[email protected]>
---
drivers/staging/vt6655/device_main.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
index 21d10d9fde47..c8cae6df7f51 100644
--- a/drivers/staging/vt6655/device_main.c
+++ b/drivers/staging/vt6655/device_main.c
@@ -839,7 +839,6 @@ static int device_rx_srv(struct vnt_private *priv, unsigned int idx)
"can not allocate rx buf\n");
break;
}
- rd->rd0.owner = OWNED_BY_NIC;
}

priv->pCurrRD[idx] = rd;
--
2.25.1

2022-09-29 20:56:17

by Philipp Hortmann

[permalink] [raw]
Subject: Re: [RFC PATCH v2 0/4] staging: vt6655: Implement allocation failure handling

On 9/28/22 19:21, Nam Cao wrote:
> This driver does not handle allocation failure when receiving data very
> well. This patchset implements better handling in the case of allocation
> failure.
>
> Also do some necessary clean-up to implement this.
>
> v2:
> - squash 3 commits that were doing a single thing
> - add new commit which removes a redundant assignment
> - take device_init_rx_desc() out of unnecessary else condition.
> - remove return statement at the end of void function
> - add a missing rd = rd->next statement in device_rx_srv(): because
> we already drop the current buffer, we should move on to the next
> buffer in the ring where new data will be written to.
>
> Nam Cao (4):
> staging: vt6655: remove redundant if condition
> staging: vt6655: change vnt_receive_frame return type to void
> staging: vt6655: remove redundant assignment
> staging: vt6655: implement allocation failure handling
>
> drivers/staging/vt6655/device_main.c | 40 +++++++++++++++++-----------
> drivers/staging/vt6655/dpc.c | 8 +++---
> drivers/staging/vt6655/dpc.h | 2 +-
> 3 files changed, 28 insertions(+), 22 deletions(-)
>

Tested-by: Philipp Hortmann <[email protected]>