* [PATCH] net: broadcom: bcm4908_enet: update TX stats after actual transmission
@ 2022-10-27 11:24 Rafał Miłecki
2022-10-27 16:12 ` Florian Fainelli
2022-10-27 17:40 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 4+ messages in thread
From: Rafał Miłecki @ 2022-10-27 11:24 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: netdev, bcm-kernel-feedback-list, Rafał Miłecki,
Florian Fainelli
From: Rafał Miłecki <rafal@milecki.pl>
Queueing packets doesn't guarantee their transmission. Update TX stats
after hardware confirms consuming submitted data.
This also fixes a possible race and NULL dereference.
bcm4908_enet_start_xmit() could try to access skb after freeing it in
the bcm4908_enet_poll_tx().
Reported-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
This fixes a potential NULL dereference. It was never seen in real usage
though. I'm not sure if it makes this net-next or net material.
---
drivers/net/ethernet/broadcom/bcm4908_enet.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcm4908_enet.c b/drivers/net/ethernet/broadcom/bcm4908_enet.c
index ca8c86ee44c0..b0aac0bcb060 100644
--- a/drivers/net/ethernet/broadcom/bcm4908_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm4908_enet.c
@@ -571,8 +571,6 @@ static netdev_tx_t bcm4908_enet_start_xmit(struct sk_buff *skb, struct net_devic
if (++ring->write_idx == ring->length - 1)
ring->write_idx = 0;
- enet->netdev->stats.tx_bytes += skb->len;
- enet->netdev->stats.tx_packets++;
return NETDEV_TX_OK;
}
@@ -654,6 +652,7 @@ static int bcm4908_enet_poll_tx(struct napi_struct *napi, int weight)
struct bcm4908_enet_dma_ring_bd *buf_desc;
struct bcm4908_enet_dma_ring_slot *slot;
struct device *dev = enet->dev;
+ unsigned int bytes = 0;
int handled = 0;
while (handled < weight && tx_ring->read_idx != tx_ring->write_idx) {
@@ -664,12 +663,17 @@ static int bcm4908_enet_poll_tx(struct napi_struct *napi, int weight)
dma_unmap_single(dev, slot->dma_addr, slot->len, DMA_TO_DEVICE);
dev_kfree_skb(slot->skb);
- if (++tx_ring->read_idx == tx_ring->length)
- tx_ring->read_idx = 0;
handled++;
+ bytes += slot->len;
+
+ if (++tx_ring->read_idx == tx_ring->length)
+ tx_ring->read_idx = 0;
}
+ enet->netdev->stats.tx_packets += handled;
+ enet->netdev->stats.tx_bytes += bytes;
+
if (handled < weight) {
napi_complete_done(napi, handled);
bcm4908_enet_dma_ring_intrs_on(enet, tx_ring);
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] net: broadcom: bcm4908_enet: update TX stats after actual transmission
2022-10-27 11:24 [PATCH] net: broadcom: bcm4908_enet: update TX stats after actual transmission Rafał Miłecki
@ 2022-10-27 16:12 ` Florian Fainelli
2022-10-27 16:30 ` Rafał Miłecki
2022-10-27 17:40 ` patchwork-bot+netdevbpf
1 sibling, 1 reply; 4+ messages in thread
From: Florian Fainelli @ 2022-10-27 16:12 UTC (permalink / raw)
To: Rafał Miłecki, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: netdev, bcm-kernel-feedback-list, Rafał Miłecki,
Florian Fainelli
On 10/27/2022 4:24 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> Queueing packets doesn't guarantee their transmission. Update TX stats
> after hardware confirms consuming submitted data.
>
> This also fixes a possible race and NULL dereference.
> bcm4908_enet_start_xmit() could try to access skb after freeing it in
> the bcm4908_enet_poll_tx().
>
> Reported-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
To me this is 'net' material since it fixes a potential issue, but let's
see what the netdev maintainers prefer.
--
Florian
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: broadcom: bcm4908_enet: update TX stats after actual transmission
2022-10-27 16:12 ` Florian Fainelli
@ 2022-10-27 16:30 ` Rafał Miłecki
0 siblings, 0 replies; 4+ messages in thread
From: Rafał Miłecki @ 2022-10-27 16:30 UTC (permalink / raw)
To: Florian Fainelli, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: netdev, bcm-kernel-feedback-list, Rafał Miłecki
On 27.10.2022 18:12, Florian Fainelli wrote:
> On 10/27/2022 4:24 AM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> Queueing packets doesn't guarantee their transmission. Update TX stats
>> after hardware confirms consuming submitted data.
>>
>> This also fixes a possible race and NULL dereference.
>> bcm4908_enet_start_xmit() could try to access skb after freeing it in
>> the bcm4908_enet_poll_tx().
>>
>> Reported-by: Florian Fainelli <f.fainelli@gmail.com>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
>
> To me this is 'net' material since it fixes a potential issue, but let's see what the netdev maintainers prefer.
Thanks. In that case it may be also worth adding:
Fixes: 4feffeadbcb2e ("net: broadcom: bcm4908enet: add BCM4908 controller driver")
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: broadcom: bcm4908_enet: update TX stats after actual transmission
2022-10-27 11:24 [PATCH] net: broadcom: bcm4908_enet: update TX stats after actual transmission Rafał Miłecki
2022-10-27 16:12 ` Florian Fainelli
@ 2022-10-27 17:40 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-10-27 17:40 UTC (permalink / raw)
To: =?utf-8?b?UmFmYcWCIE1pxYJlY2tpIDx6YWplYzVAZ21haWwuY29tPg==?=
Cc: davem, edumazet, kuba, pabeni, netdev, bcm-kernel-feedback-list,
rafal, f.fainelli
Hello:
This patch was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 27 Oct 2022 13:24:30 +0200 you wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> Queueing packets doesn't guarantee their transmission. Update TX stats
> after hardware confirms consuming submitted data.
>
> This also fixes a possible race and NULL dereference.
> bcm4908_enet_start_xmit() could try to access skb after freeing it in
> the bcm4908_enet_poll_tx().
>
> [...]
Here is the summary with links:
- net: broadcom: bcm4908_enet: update TX stats after actual transmission
https://git.kernel.org/netdev/net/c/ef3556ee16c6
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-10-27 17:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-27 11:24 [PATCH] net: broadcom: bcm4908_enet: update TX stats after actual transmission Rafał Miłecki
2022-10-27 16:12 ` Florian Fainelli
2022-10-27 16:30 ` Rafał Miłecki
2022-10-27 17:40 ` patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.