Re: [PATCH net-next v4 05/12] net: ethernet: oa_tc6: implement error interrupts unmasking

Ramón Nordin Rodriguez
2024-04-27T19:52:19Z

> +static int oa_tc6_unmask_macphy_error_interrupts(struct oa_tc6 *tc6)
> +{
> + u32 regval;
> + int ret;
> +
> + ret = oa_tc6_read_register(tc6, OA_TC6_REG_INT_MASK0, &regval);
> + if (ret)
> + return ret;
> +
> + regval &= ~(INT_MASK0_TX_PROTOCOL_ERR_MASK |
> + INT_MASK0_RX_BUFFER_OVERFLOW_ERR_MASK |
> + INT_MASK0_LOSS_OF_FRAME_ERR_MASK |
> + INT_MASK0_HEADER_ERR_MASK);
> +
> + return oa_tc6_write_register(tc6, OA_TC6_REG_INT_MASK0, regval);
> +}
> +

This togheter with patch 11 works poorly for me. I get alot of kernel
output, dropped packets and lower performance.
Below is an example for a run when I curl a 10MB blob

time curl 20.0.0.55:8000/rdump -o dump -w '{%speed_download}'
% Total % Received % Xferd Average Speed Time Time Time Current
Dload U[ 387.944737] net_ratelimit: 38 callbacks suppressed
pload Total Spent Left Sp[ 387.944755] eth0: Receive buffer overflow error
eed
0 0 0 0 0 0 0 0 --:--:-- --:-[ 387.961424] eth0: Receive buffer overflow error
0 10.0M 0 2896 0 0 13031 0 0:13:24 --:--:-- 0:13:24 12986[ 388.204257] eth0: Receive buffer overflow error
[ 388.209848] eth0: Receive buffer overflow error
[ 388.240705] eth0: Receive buffer overflow error
[ 388.246205] eth0: Receive buffer overflow error
[ 388.360265] eth0: Receive buffer overflow error
[ 388.365755] eth0: Receive buffer overflow error
[ 388.371328] eth0: Receive buffer overflow error
[ 388.396937] eth0: Receive buffer overflow error
32 10.0M 32 3362k 0 0 826k 0 0:00:12 0:00:04 0:00:08 826k[ 392.950696] net_ratelimit: 84 callbacks suppressed
[ 392.950711] eth0: Receive buffer overflow error
41 10.0M 41 4259k 0 0 840k 0 0:00:12 0:00:05 0:00:07 878k[ 393.009785] eth0: Receive buffer overflow error
[ 393.016651] eth0: Receive buffer overflow error
[ 393.121278] eth0: Receive buffer overflow error
[ 393.126876] eth0: Receive buffer overflow error
[ 393.204983] eth0: Receive buffer overflow error
[ 393.210590] eth0: Receive buffer overflow error
[ 393.248977] eth0: Receive buffer overflow error
[ 393.254575] eth0: Receive buffer overflow error
[ 393.352949] eth0: Receive buffer overflow error
77 10.0M 77 7903k 0 0 870k 0 0:00:11 0:00:09 0:00:02 906k[ 397.956674] net_ratelimit: 88 callbacks suppressed
[ 397.956691] eth0: Receive buffer overflow error
[ 397.967182] eth0: Receive buffer overflow error
86 10.0M 86 8837k 0 0 877k 0 0:00:11 0:00:10 0:00:01 915k[ 398.048630] eth0: Receive buffer overflow error
[ 398.054171] eth0: Receive buffer overflow error
[ 398.092858] eth0: Receive buffer overflow error
[ 398.113975] eth0: Receive buffer overflow error
[ 398.177558] eth0: Receive buffer overflow error
[ 398.209782] eth0: Receive buffer overflow error
[ 398.272621] eth0: Receive buffer overflow error
[ 398.278306] eth0: Receive buffer overflow error
100 10.0M 100 10.0M 0 0 877k 0 0:00:11 0:00:11 --:--:-- 921k
{%speed_download}
real 0m11.681s
user 0m0.117s
sys 0m0.226s

I tried this patch

diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
index 9f17f3712137..bd7bd3ef6897 100644
--- a/drivers/net/ethernet/oa_tc6.c
+++ b/drivers/net/ethernet/oa_tc6.c
@@ -615,21 +615,9 @@ static int oa_tc6_sw_reset_macphy(struct oa_tc6 *tc6)
return oa_tc6_write_register(tc6, OA_TC6_REG_STATUS0, regval);
}

-static int oa_tc6_unmask_macphy_error_interrupts(struct oa_tc6 *tc6)
+static int oa_tc6_disable_imask0_interrupts(struct oa_tc6 *tc6)
{
- u32 regval;
- int ret;
-
- ret = oa_tc6_read_register(tc6, OA_TC6_REG_INT_MASK0, &regval);
- if (ret)
- return ret;
-
- regval &= ~(INT_MASK0_TX_PROTOCOL_ERR_MASK |
- INT_MASK0_RX_BUFFER_OVERFLOW_ERR_MASK |
- INT_MASK0_LOSS_OF_FRAME_ERR_MASK |
- INT_MASK0_HEADER_ERR_MASK);
-
- return oa_tc6_write_register(tc6, OA_TC6_REG_INT_MASK0, regval);
+ return oa_tc6_write_register(tc6, OA_TC6_REG_INT_MASK0, (u32)-1);
}

static int oa_tc6_enable_data_transfer(struct oa_tc6 *tc6)
@@ -1234,7 +1222,7 @@ struct oa_tc6 *oa_tc6_init(struct spi_device *spi, struct net_device *netdev)
return NULL;
}

- ret = oa_tc6_unmask_macphy_error_interrupts(tc6);
+ ret = oa_tc6_disable_imask0_interrupts(tc6);
if (ret) {
dev_err(&tc6->spi->dev,
"MAC-PHY error interrupts unmask failed: %d\n", ret);

Which results in no log spam, ~5-10% higher throughput and no dropped
packets when I look at /sys/class/net/eth0/statistics/rx_dropped

Wheras when I did an equivalent run with a 100MB file I got 1918
rx_dropped with interrupts enabled.

When I dig through wireshark I can see some retransmissions when
interrupts are disabled, but the network stack seems to handle that with
grace.

Maybe the decision on wheter to enable interrupts or not should be on
the mac driver level, not network framework level?
If this really is preferrable for some reason, could a module option be
added so that it does not require running a fork to disable interrupts?

There is also a interrupt mask 1 register that is not touched in this
patch series, and since things started working better for me with just
touching reg0 I didn't bother tweaking anything int reg1.

BR
Ramón