* [PATCH net] net: xilinx: axienet: Fix IRQ coalescing packet count overflow
@ 2024-09-03 18:00 Sean Anderson
2024-09-04 16:00 ` Simon Horman
2024-09-04 17:19 ` Pandey, Radhey Shyam
0 siblings, 2 replies; 7+ messages in thread
From: Sean Anderson @ 2024-09-03 18:00 UTC (permalink / raw)
To: Radhey Shyam Pandey, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev
Cc: Michal Simek, linux-arm-kernel, Ariane Keller, linux-kernel,
Daniel Borkmann, Andy Chiu, Sean Anderson
If coalesce_count is greater than 255 it will not fit in the register and
will overflow. Clamp it to 255 for more-predictable results.
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
---
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 9aeb7b9f3ae4..5f27fc1c4375 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -252,7 +252,8 @@ static u32 axienet_usec_to_timer(struct axienet_local *lp, u32 coalesce_usec)
static void axienet_dma_start(struct axienet_local *lp)
{
/* Start updating the Rx channel control register */
- lp->rx_dma_cr = (lp->coalesce_count_rx << XAXIDMA_COALESCE_SHIFT) |
+ lp->rx_dma_cr = (min(lp->coalesce_count_rx, 255) <<
+ XAXIDMA_COALESCE_SHIFT) |
XAXIDMA_IRQ_IOC_MASK | XAXIDMA_IRQ_ERROR_MASK;
/* Only set interrupt delay timer if not generating an interrupt on
* the first RX packet. Otherwise leave at 0 to disable delay interrupt.
@@ -264,7 +265,8 @@ static void axienet_dma_start(struct axienet_local *lp)
axienet_dma_out32(lp, XAXIDMA_RX_CR_OFFSET, lp->rx_dma_cr);
/* Start updating the Tx channel control register */
- lp->tx_dma_cr = (lp->coalesce_count_tx << XAXIDMA_COALESCE_SHIFT) |
+ lp->tx_dma_cr = (min(lp->coalesce_count_tx, 255) <<
+ XAXIDMA_COALESCE_SHIFT) |
XAXIDMA_IRQ_IOC_MASK | XAXIDMA_IRQ_ERROR_MASK;
/* Only set interrupt delay timer if not generating an interrupt on
* the first TX packet. Otherwise leave at 0 to disable delay interrupt.
--
2.35.1.1320.gc452695387.dirty
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: xilinx: axienet: Fix IRQ coalescing packet count overflow
2024-09-03 18:00 [PATCH net] net: xilinx: axienet: Fix IRQ coalescing packet count overflow Sean Anderson
@ 2024-09-04 16:00 ` Simon Horman
2024-09-05 14:34 ` Sean Anderson
2024-09-04 17:19 ` Pandey, Radhey Shyam
1 sibling, 1 reply; 7+ messages in thread
From: Simon Horman @ 2024-09-04 16:00 UTC (permalink / raw)
To: Sean Anderson
Cc: Radhey Shyam Pandey, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, Michal Simek,
linux-arm-kernel, Ariane Keller, linux-kernel, Daniel Borkmann,
Andy Chiu
On Tue, Sep 03, 2024 at 02:00:59PM -0400, Sean Anderson wrote:
> If coalesce_count is greater than 255 it will not fit in the register and
> will overflow. Clamp it to 255 for more-predictable results.
Hi Sean,
Can this occur in practice?
>
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
nit: I think it is usual for the order of these tags to be reversed.
> ---
>
> drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index 9aeb7b9f3ae4..5f27fc1c4375 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -252,7 +252,8 @@ static u32 axienet_usec_to_timer(struct axienet_local *lp, u32 coalesce_usec)
> static void axienet_dma_start(struct axienet_local *lp)
> {
> /* Start updating the Rx channel control register */
> - lp->rx_dma_cr = (lp->coalesce_count_rx << XAXIDMA_COALESCE_SHIFT) |
> + lp->rx_dma_cr = (min(lp->coalesce_count_rx, 255) <<
> + XAXIDMA_COALESCE_SHIFT) |
> XAXIDMA_IRQ_IOC_MASK | XAXIDMA_IRQ_ERROR_MASK;
nit: it would be nice to avoid using a naked 255 here.
Perhaps: #define XAXIDMA_COALESCE_MAX 0xff
> /* Only set interrupt delay timer if not generating an interrupt on
> * the first RX packet. Otherwise leave at 0 to disable delay interrupt.
> @@ -264,7 +265,8 @@ static void axienet_dma_start(struct axienet_local *lp)
> axienet_dma_out32(lp, XAXIDMA_RX_CR_OFFSET, lp->rx_dma_cr);
>
> /* Start updating the Tx channel control register */
> - lp->tx_dma_cr = (lp->coalesce_count_tx << XAXIDMA_COALESCE_SHIFT) |
> + lp->tx_dma_cr = (min(lp->coalesce_count_tx, 255) <<
> + XAXIDMA_COALESCE_SHIFT) |
> XAXIDMA_IRQ_IOC_MASK | XAXIDMA_IRQ_ERROR_MASK;
> /* Only set interrupt delay timer if not generating an interrupt on
> * the first TX packet. Otherwise leave at 0 to disable delay interrupt.
> --
> 2.35.1.1320.gc452695387.dirty
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH net] net: xilinx: axienet: Fix IRQ coalescing packet count overflow
2024-09-03 18:00 [PATCH net] net: xilinx: axienet: Fix IRQ coalescing packet count overflow Sean Anderson
2024-09-04 16:00 ` Simon Horman
@ 2024-09-04 17:19 ` Pandey, Radhey Shyam
2024-09-05 14:55 ` Sean Anderson
1 sibling, 1 reply; 7+ messages in thread
From: Pandey, Radhey Shyam @ 2024-09-04 17:19 UTC (permalink / raw)
To: Sean Anderson, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev@vger.kernel.org
Cc: Simek, Michal, linux-arm-kernel@lists.infradead.org,
Ariane Keller, linux-kernel@vger.kernel.org, Daniel Borkmann,
Andy Chiu
> -----Original Message-----
> From: Sean Anderson <sean.anderson@linux.dev>
> Sent: Tuesday, September 3, 2024 11:31 PM
> To: Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>; David S .
> Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>;
> Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>;
> netdev@vger.kernel.org
> Cc: Simek, Michal <michal.simek@amd.com>; linux-arm-
> kernel@lists.infradead.org; Ariane Keller <ariane.keller@tik.ee.ethz.ch>;
> linux-kernel@vger.kernel.org; Daniel Borkmann <daniel@iogearbox.net>;
> Andy Chiu <andy.chiu@sifive.com>; Sean Anderson
> <sean.anderson@linux.dev>
> Subject: [PATCH net] net: xilinx: axienet: Fix IRQ coalescing packet count
> overflow
>
> If coalesce_count is greater than 255 it will not fit in the register and
> will overflow. Clamp it to 255 for more-predictable results.
>
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet
> driver")
> ---
>
> drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index 9aeb7b9f3ae4..5f27fc1c4375 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -252,7 +252,8 @@ static u32 axienet_usec_to_timer(struct axienet_local
> *lp, u32 coalesce_usec)
> static void axienet_dma_start(struct axienet_local *lp)
> {
> /* Start updating the Rx channel control register */
> - lp->rx_dma_cr = (lp->coalesce_count_rx <<
> XAXIDMA_COALESCE_SHIFT) |
> + lp->rx_dma_cr = (min(lp->coalesce_count_rx, 255) <<
> + XAXIDMA_COALESCE_SHIFT) |
> XAXIDMA_IRQ_IOC_MASK |
Instead of every time clamping coalesce_count_rx on read I think better
to do it place where it set in axienet_ethtools_set_coalesce()? It would
also represent the coalesce count state that is reported by get_coalesce()
and same is being used in programming.
> XAXIDMA_IRQ_ERROR_MASK;
> /* Only set interrupt delay timer if not generating an interrupt on
> * the first RX packet. Otherwise leave at 0 to disable delay interrupt.
> @@ -264,7 +265,8 @@ static void axienet_dma_start(struct axienet_local
> *lp)
> axienet_dma_out32(lp, XAXIDMA_RX_CR_OFFSET, lp->rx_dma_cr);
>
> /* Start updating the Tx channel control register */
> - lp->tx_dma_cr = (lp->coalesce_count_tx <<
> XAXIDMA_COALESCE_SHIFT) |
> + lp->tx_dma_cr = (min(lp->coalesce_count_tx, 255) <<
> + XAXIDMA_COALESCE_SHIFT) |
> XAXIDMA_IRQ_IOC_MASK |
> XAXIDMA_IRQ_ERROR_MASK;
> /* Only set interrupt delay timer if not generating an interrupt on
> * the first TX packet. Otherwise leave at 0 to disable delay interrupt.
> --
> 2.35.1.1320.gc452695387.dirty
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: xilinx: axienet: Fix IRQ coalescing packet count overflow
2024-09-04 16:00 ` Simon Horman
@ 2024-09-05 14:34 ` Sean Anderson
2024-09-06 7:05 ` Simon Horman
0 siblings, 1 reply; 7+ messages in thread
From: Sean Anderson @ 2024-09-05 14:34 UTC (permalink / raw)
To: Simon Horman
Cc: Radhey Shyam Pandey, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, Michal Simek,
linux-arm-kernel, Ariane Keller, linux-kernel, Daniel Borkmann,
Andy Chiu
On 9/4/24 12:00, Simon Horman wrote:
> On Tue, Sep 03, 2024 at 02:00:59PM -0400, Sean Anderson wrote:
>> If coalesce_count is greater than 255 it will not fit in the register and
>> will overflow. Clamp it to 255 for more-predictable results.
>
> Hi Sean,
>
> Can this occur in practice?
Yes. Simply do `ethtool -C ethX rx-frames 300` or something similar and
you will end up with a limit of 44 instead. I ran into this with DIM and
was wondering why the highest-throughput setting (256) was behaving so
poorly...
>>
>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>> Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
>
> nit: I think it is usual for the order of these tags to be reversed.
OK
>> ---
>>
>> drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>> index 9aeb7b9f3ae4..5f27fc1c4375 100644
>> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>> @@ -252,7 +252,8 @@ static u32 axienet_usec_to_timer(struct axienet_local *lp, u32 coalesce_usec)
>> static void axienet_dma_start(struct axienet_local *lp)
>> {
>> /* Start updating the Rx channel control register */
>> - lp->rx_dma_cr = (lp->coalesce_count_rx << XAXIDMA_COALESCE_SHIFT) |
>> + lp->rx_dma_cr = (min(lp->coalesce_count_rx, 255) <<
>> + XAXIDMA_COALESCE_SHIFT) |
>> XAXIDMA_IRQ_IOC_MASK | XAXIDMA_IRQ_ERROR_MASK;
>
> nit: it would be nice to avoid using a naked 255 here.
> Perhaps: #define XAXIDMA_COALESCE_MAX 0xff
OK, but this is the same as the limit used in axienet_usec_to_timer.
--Sean
>> /* Only set interrupt delay timer if not generating an interrupt on
>> * the first RX packet. Otherwise leave at 0 to disable delay interrupt.
>> @@ -264,7 +265,8 @@ static void axienet_dma_start(struct axienet_local *lp)
>> axienet_dma_out32(lp, XAXIDMA_RX_CR_OFFSET, lp->rx_dma_cr);
>>
>> /* Start updating the Tx channel control register */
>> - lp->tx_dma_cr = (lp->coalesce_count_tx << XAXIDMA_COALESCE_SHIFT) |
>> + lp->tx_dma_cr = (min(lp->coalesce_count_tx, 255) <<
>> + XAXIDMA_COALESCE_SHIFT) |
>> XAXIDMA_IRQ_IOC_MASK | XAXIDMA_IRQ_ERROR_MASK;
>> /* Only set interrupt delay timer if not generating an interrupt on
>> * the first TX packet. Otherwise leave at 0 to disable delay interrupt.
>> --
>> 2.35.1.1320.gc452695387.dirty
>>
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: xilinx: axienet: Fix IRQ coalescing packet count overflow
2024-09-04 17:19 ` Pandey, Radhey Shyam
@ 2024-09-05 14:55 ` Sean Anderson
2024-09-09 23:53 ` Sean Anderson
0 siblings, 1 reply; 7+ messages in thread
From: Sean Anderson @ 2024-09-05 14:55 UTC (permalink / raw)
To: Pandey, Radhey Shyam, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev@vger.kernel.org
Cc: Simek, Michal, linux-arm-kernel@lists.infradead.org,
Ariane Keller, linux-kernel@vger.kernel.org, Daniel Borkmann,
Andy Chiu
On 9/4/24 13:19, Pandey, Radhey Shyam wrote:
>> -----Original Message-----
>> From: Sean Anderson <sean.anderson@linux.dev>
>> Sent: Tuesday, September 3, 2024 11:31 PM
>> To: Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>; David S .
>> Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>;
>> Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>;
>> netdev@vger.kernel.org
>> Cc: Simek, Michal <michal.simek@amd.com>; linux-arm-
>> kernel@lists.infradead.org; Ariane Keller <ariane.keller@tik.ee.ethz.ch>;
>> linux-kernel@vger.kernel.org; Daniel Borkmann <daniel@iogearbox.net>;
>> Andy Chiu <andy.chiu@sifive.com>; Sean Anderson
>> <sean.anderson@linux.dev>
>> Subject: [PATCH net] net: xilinx: axienet: Fix IRQ coalescing packet count
>> overflow
>>
>> If coalesce_count is greater than 255 it will not fit in the register and
>> will overflow. Clamp it to 255 for more-predictable results.
>>
>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>> Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet
>> driver")
>> ---
>>
>> drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>> b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>> index 9aeb7b9f3ae4..5f27fc1c4375 100644
>> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>> @@ -252,7 +252,8 @@ static u32 axienet_usec_to_timer(struct axienet_local
>> *lp, u32 coalesce_usec)
>> static void axienet_dma_start(struct axienet_local *lp)
>> {
>> /* Start updating the Rx channel control register */
>> - lp->rx_dma_cr = (lp->coalesce_count_rx <<
>> XAXIDMA_COALESCE_SHIFT) |
>> + lp->rx_dma_cr = (min(lp->coalesce_count_rx, 255) <<
>> + XAXIDMA_COALESCE_SHIFT) |
>> XAXIDMA_IRQ_IOC_MASK |
>
> Instead of every time clamping coalesce_count_rx on read I think better
> to do it place where it set in axienet_ethtools_set_coalesce()? It would
> also represent the coalesce count state that is reported by get_coalesce()
> and same is being used in programming.
The parameter which will be trickier is the timer, which is also clamped
but depends on the (DMA) clock speed. So theoretically it may be
different if the clock gets changed at runtime between when we set
coalesce and when we apply it. But do we even support dynamic rate
changes for that clock?
In either case, I think this will be easier to do as part of [1], since I
am already rearranging the calculation in that patch.
--Sean
[1] https://lore.kernel.org/netdev/20240903192524.4158713-2-sean.anderson@linux.dev/
>> XAXIDMA_IRQ_ERROR_MASK;
>> /* Only set interrupt delay timer if not generating an interrupt on
>> * the first RX packet. Otherwise leave at 0 to disable delay interrupt.
>> @@ -264,7 +265,8 @@ static void axienet_dma_start(struct axienet_local
>> *lp)
>> axienet_dma_out32(lp, XAXIDMA_RX_CR_OFFSET, lp->rx_dma_cr);
>>
>> /* Start updating the Tx channel control register */
>> - lp->tx_dma_cr = (lp->coalesce_count_tx <<
>> XAXIDMA_COALESCE_SHIFT) |
>> + lp->tx_dma_cr = (min(lp->coalesce_count_tx, 255) <<
>> + XAXIDMA_COALESCE_SHIFT) |
>> XAXIDMA_IRQ_IOC_MASK |
>> XAXIDMA_IRQ_ERROR_MASK;
>> /* Only set interrupt delay timer if not generating an interrupt on
>> * the first TX packet. Otherwise leave at 0 to disable delay interrupt.
>> --
>> 2.35.1.1320.gc452695387.dirty
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: xilinx: axienet: Fix IRQ coalescing packet count overflow
2024-09-05 14:34 ` Sean Anderson
@ 2024-09-06 7:05 ` Simon Horman
0 siblings, 0 replies; 7+ messages in thread
From: Simon Horman @ 2024-09-06 7:05 UTC (permalink / raw)
To: Sean Anderson
Cc: Radhey Shyam Pandey, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, Michal Simek,
linux-arm-kernel, Ariane Keller, linux-kernel, Daniel Borkmann,
Andy Chiu
On Thu, Sep 05, 2024 at 10:34:15AM -0400, Sean Anderson wrote:
> On 9/4/24 12:00, Simon Horman wrote:
> > On Tue, Sep 03, 2024 at 02:00:59PM -0400, Sean Anderson wrote:
> >> If coalesce_count is greater than 255 it will not fit in the register and
> >> will overflow. Clamp it to 255 for more-predictable results.
> >
> > Hi Sean,
> >
> > Can this occur in practice?
>
> Yes. Simply do `ethtool -C ethX rx-frames 300` or something similar and
> you will end up with a limit of 44 instead. I ran into this with DIM and
> was wondering why the highest-throughput setting (256) was behaving so
> poorly...
I think it would be nice to include an explanation along those
lines in the patch description.
>
> >>
> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> >> Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
> >
> > nit: I think it is usual for the order of these tags to be reversed.
>
> OK
>
> >> ---
> >>
> >> drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 6 ++++--
> >> 1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> >> index 9aeb7b9f3ae4..5f27fc1c4375 100644
> >> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> >> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> >> @@ -252,7 +252,8 @@ static u32 axienet_usec_to_timer(struct axienet_local *lp, u32 coalesce_usec)
> >> static void axienet_dma_start(struct axienet_local *lp)
> >> {
> >> /* Start updating the Rx channel control register */
> >> - lp->rx_dma_cr = (lp->coalesce_count_rx << XAXIDMA_COALESCE_SHIFT) |
> >> + lp->rx_dma_cr = (min(lp->coalesce_count_rx, 255) <<
> >> + XAXIDMA_COALESCE_SHIFT) |
> >> XAXIDMA_IRQ_IOC_MASK | XAXIDMA_IRQ_ERROR_MASK;
> >
> > nit: it would be nice to avoid using a naked 255 here.
> > Perhaps: #define XAXIDMA_COALESCE_MAX 0xff
>
> OK, but this is the same as the limit used in axienet_usec_to_timer.
Ok, that does muddy the waters a bit.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: xilinx: axienet: Fix IRQ coalescing packet count overflow
2024-09-05 14:55 ` Sean Anderson
@ 2024-09-09 23:53 ` Sean Anderson
0 siblings, 0 replies; 7+ messages in thread
From: Sean Anderson @ 2024-09-09 23:53 UTC (permalink / raw)
To: Pandey, Radhey Shyam, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev@vger.kernel.org
Cc: Simek, Michal, linux-arm-kernel@lists.infradead.org,
Ariane Keller, linux-kernel@vger.kernel.org, Daniel Borkmann,
Andy Chiu
On 9/5/24 10:55, Sean Anderson wrote:
> On 9/4/24 13:19, Pandey, Radhey Shyam wrote:
>>> -----Original Message-----
>>> From: Sean Anderson <sean.anderson@linux.dev>
>>> Sent: Tuesday, September 3, 2024 11:31 PM
>>> To: Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>; David S .
>>> Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>;
>>> Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>;
>>> netdev@vger.kernel.org
>>> Cc: Simek, Michal <michal.simek@amd.com>; linux-arm-
>>> kernel@lists.infradead.org; Ariane Keller <ariane.keller@tik.ee.ethz.ch>;
>>> linux-kernel@vger.kernel.org; Daniel Borkmann <daniel@iogearbox.net>;
>>> Andy Chiu <andy.chiu@sifive.com>; Sean Anderson
>>> <sean.anderson@linux.dev>
>>> Subject: [PATCH net] net: xilinx: axienet: Fix IRQ coalescing packet count
>>> overflow
>>>
>>> If coalesce_count is greater than 255 it will not fit in the register and
>>> will overflow. Clamp it to 255 for more-predictable results.
>>>
>>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>>> Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet
>>> driver")
>>> ---
>>>
>>> drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>>> b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>>> index 9aeb7b9f3ae4..5f27fc1c4375 100644
>>> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>>> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>>> @@ -252,7 +252,8 @@ static u32 axienet_usec_to_timer(struct axienet_local
>>> *lp, u32 coalesce_usec)
>>> static void axienet_dma_start(struct axienet_local *lp)
>>> {
>>> /* Start updating the Rx channel control register */
>>> - lp->rx_dma_cr = (lp->coalesce_count_rx <<
>>> XAXIDMA_COALESCE_SHIFT) |
>>> + lp->rx_dma_cr = (min(lp->coalesce_count_rx, 255) <<
>>> + XAXIDMA_COALESCE_SHIFT) |
>>> XAXIDMA_IRQ_IOC_MASK |
>>
>> Instead of every time clamping coalesce_count_rx on read I think better
>> to do it place where it set in axienet_ethtools_set_coalesce()? It would
>> also represent the coalesce count state that is reported by get_coalesce()
>> and same is being used in programming.
>
> The parameter which will be trickier is the timer, which is also clamped
> but depends on the (DMA) clock speed. So theoretically it may be
> different if the clock gets changed at runtime between when we set
> coalesce and when we apply it. But do we even support dynamic rate
> changes for that clock?
>
> In either case, I think this will be easier to do as part of [1], since I
> am already rearranging the calculation in that patch.
Implemented as https://lore.kernel.org/netdev/20240909235208.1331065-6-sean.anderson@linux.dev/
--Sean
> --Sean
>
> [1] https://lore.kernel.org/netdev/20240903192524.4158713-2-sean.anderson@linux.dev/
>
>>> XAXIDMA_IRQ_ERROR_MASK;
>>> /* Only set interrupt delay timer if not generating an interrupt on
>>> * the first RX packet. Otherwise leave at 0 to disable delay interrupt.
>>> @@ -264,7 +265,8 @@ static void axienet_dma_start(struct axienet_local
>>> *lp)
>>> axienet_dma_out32(lp, XAXIDMA_RX_CR_OFFSET, lp->rx_dma_cr);
>>>
>>> /* Start updating the Tx channel control register */
>>> - lp->tx_dma_cr = (lp->coalesce_count_tx <<
>>> XAXIDMA_COALESCE_SHIFT) |
>>> + lp->tx_dma_cr = (min(lp->coalesce_count_tx, 255) <<
>>> + XAXIDMA_COALESCE_SHIFT) |
>>> XAXIDMA_IRQ_IOC_MASK |
>>> XAXIDMA_IRQ_ERROR_MASK;
>>> /* Only set interrupt delay timer if not generating an interrupt on
>>> * the first TX packet. Otherwise leave at 0 to disable delay interrupt.
>>> --
>>> 2.35.1.1320.gc452695387.dirty
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-09-09 23:53 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-03 18:00 [PATCH net] net: xilinx: axienet: Fix IRQ coalescing packet count overflow Sean Anderson
2024-09-04 16:00 ` Simon Horman
2024-09-05 14:34 ` Sean Anderson
2024-09-06 7:05 ` Simon Horman
2024-09-04 17:19 ` Pandey, Radhey Shyam
2024-09-05 14:55 ` Sean Anderson
2024-09-09 23:53 ` Sean Anderson
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).