| Message ID | 20210630182748.3481-1-m.chetan.kumar@linux.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: wwan: iosm: fixes | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | warning | 3 maintainers not CCed: m.chetan.kumar@intel.com ryazanov.s.a@gmail.com loic.poulain@linaro.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 17 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On 6/30/21 8:27 PM, M Chetan Kumar wrote: > Update tx stats on successful packet consume, drop. > > Signed-off-by: M Chetan Kumar <m.chetan.kumar@linux.intel.com> > --- > drivers/net/wwan/iosm/iosm_ipc_wwan.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.c b/drivers/net/wwan/iosm/iosm_ipc_wwan.c > index 84e37c4b0f74..561944a33725 100644 > --- a/drivers/net/wwan/iosm/iosm_ipc_wwan.c > +++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.c > @@ -123,6 +123,8 @@ static int ipc_wwan_link_transmit(struct sk_buff *skb, > > /* Return code of zero is success */ > if (ret == 0) { > + netdev->stats.tx_packets++; > + netdev->stats.tx_bytes += skb->len; What makes you think skb has not been consumed already ? It seems clear it has been given, this thread can not expect skb has not been mangled/freed. skb->len might now contain garbage, or even crash the kernel under appropriate debug features. > ret = NETDEV_TX_OK; > } else if (ret == -EBUSY) { > ret = NETDEV_TX_BUSY; > @@ -140,7 +142,8 @@ static int ipc_wwan_link_transmit(struct sk_buff *skb, > ret); > > dev_kfree_skb_any(skb); > - return ret; > + netdev->stats.tx_dropped++; > + return NETDEV_TX_OK; > } > > /* Ops structure for wwan net link */ >
On 7/1/2021 2:06 AM, Eric Dumazet wrote: > > > On 6/30/21 8:27 PM, M Chetan Kumar wrote: >> Update tx stats on successful packet consume, drop. >> >> Signed-off-by: M Chetan Kumar <m.chetan.kumar@linux.intel.com> >> --- >> drivers/net/wwan/iosm/iosm_ipc_wwan.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.c b/drivers/net/wwan/iosm/iosm_ipc_wwan.c >> index 84e37c4b0f74..561944a33725 100644 >> --- a/drivers/net/wwan/iosm/iosm_ipc_wwan.c >> +++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.c >> @@ -123,6 +123,8 @@ static int ipc_wwan_link_transmit(struct sk_buff *skb, >> >> /* Return code of zero is success */ >> if (ret == 0) { >> + netdev->stats.tx_packets++; >> + netdev->stats.tx_bytes += skb->len; > > What makes you think skb has not been consumed already ? > It seems clear it has been given, this thread can not expect skb has not been mangled/freed. > skb->len might now contain garbage, or even crash the kernel under appropriate debug features. Ya. there could be a possibility skb might have been dequeued from ul_list and UL task is already processing it. We can't rule out. Will backup skb->len to local var and use it for tx_bytes update. Regards, Chetan
diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.c b/drivers/net/wwan/iosm/iosm_ipc_wwan.c index 84e37c4b0f74..561944a33725 100644 --- a/drivers/net/wwan/iosm/iosm_ipc_wwan.c +++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.c @@ -123,6 +123,8 @@ static int ipc_wwan_link_transmit(struct sk_buff *skb, /* Return code of zero is success */ if (ret == 0) { + netdev->stats.tx_packets++; + netdev->stats.tx_bytes += skb->len; ret = NETDEV_TX_OK; } else if (ret == -EBUSY) { ret = NETDEV_TX_BUSY; @@ -140,7 +142,8 @@ static int ipc_wwan_link_transmit(struct sk_buff *skb, ret); dev_kfree_skb_any(skb); - return ret; + netdev->stats.tx_dropped++; + return NETDEV_TX_OK; } /* Ops structure for wwan net link */
Update tx stats on successful packet consume, drop. Signed-off-by: M Chetan Kumar <m.chetan.kumar@linux.intel.com> --- drivers/net/wwan/iosm/iosm_ipc_wwan.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)