Received-SPF: pass (google.com: domain of linux-kernel+bounces-161153-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Sat, 27 Apr 2024 21:52:15 +0200
From: =?iso-8859-1?Q?Ram=F3n?= Nordin Rodriguez <ramon.nordin.rodriguez@ferroamp.se>
To: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com>
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, horms@kernel.org, saeedm@nvidia.com,
	anthony.l.nguyen@intel.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, andrew@lunn.ch, corbet@lwn.net,
	linux-doc@vger.kernel.org, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, horatiu.vultur@microchip.com,
	ruanjinjie@huawei.com, steen.hegelund@microchip.com,
	vladimir.oltean@nxp.com, UNGLinuxDriver@microchip.com,
	Thorsten.Kummermehr@microchip.com, Pier.Beruto@onsemi.com,
	Selvamani.Rajagopal@onsemi.com, Nicolas.Ferre@microchip.com,
	benjamin.bigler@bernformulastudent.ch
Subject: Re: [PATCH net-next v4 05/12] net: ethernet: oa_tc6: implement error
 interrupts unmasking
Message-ID: <Zi1Xbz7ARLm3HkqW@builder>
References: <20240418125648.372526-1-Parthiban.Veerasooran@microchip.com>
 <20240418125648.372526-6-Parthiban.Veerasooran@microchip.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20240418125648.372526-6-Parthiban.Veerasooran@microchip.com>

> +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