| ef0edfbe | 31-Oct-2024 |
Wei Fang <wei.fang@nxp.com> |
net: enetc: allocate vf_state during PF probes
[ Upstream commit e15c5506dd39885cd047f811a64240e2e8ab401b ]
In the previous implementation, vf_state is allocated memory only when VF
is enabled. How
net: enetc: allocate vf_state during PF probes
[ Upstream commit e15c5506dd39885cd047f811a64240e2e8ab401b ]
In the previous implementation, vf_state is allocated memory only when VF
is enabled. However, net_device_ops::ndo_set_vf_mac() may be called before
VF is enabled to configure the MAC address of VF. If this is the case,
enetc_pf_set_vf_mac() will access vf_state, resulting in access to a null
pointer. The simplified error log is as follows.
root@ls1028ardb:~# ip link set eno0 vf 1 mac 00:0c:e7:66:77:89
[ 173.543315] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004
[ 173.637254] pc : enetc_pf_set_vf_mac+0x3c/0x80 Message from sy
[ 173.641973] lr : do_setlink+0x4a8/0xec8
[ 173.732292] Call trace:
[ 173.734740] enetc_pf_set_vf_mac+0x3c/0x80
[ 173.738847] __rtnl_newlink+0x530/0x89c
[ 173.742692] rtnl_newlink+0x50/0x7c
[ 173.746189] rtnetlink_rcv_msg+0x128/0x390
[ 173.750298] netlink_rcv_skb+0x60/0x130
[ 173.754145] rtnetlink_rcv+0x18/0x24
[ 173.757731] netlink_unicast+0x318/0x380
[ 173.761665] netlink_sendmsg+0x17c/0x3c8
Fixes: d4fd0404c1c9 ("enetc: Introduce basic PF and VF ENETC ethernet drivers")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Link: https://patch.msgid.link/20241031060247.1290941-2-wei.fang@nxp.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
show more ...
|
| 760a7c96 | 10-Oct-2024 |
Wei Fang <wei.fang@nxp.com> |
net: enetc: disable NAPI after all rings are disabled
commit 6b58fadd44aafbbd6af5f0b965063e1fd2063992 upstream.
When running "xdp-bench tx eno0" to test the XDP_TX feature of ENETC
on LS1028A, it w
net: enetc: disable NAPI after all rings are disabled
commit 6b58fadd44aafbbd6af5f0b965063e1fd2063992 upstream.
When running "xdp-bench tx eno0" to test the XDP_TX feature of ENETC
on LS1028A, it was found that if the command was re-run multiple times,
Rx could not receive the frames, and the result of xdp-bench showed
that the rx rate was 0.
root@ls1028ardb:~# ./xdp-bench tx eno0
Hairpinning (XDP_TX) packets on eno0 (ifindex 3; driver fsl_enetc)
Summary 2046 rx/s 0 err,drop/s
Summary 0 rx/s 0 err,drop/s
Summary 0 rx/s 0 err,drop/s
Summary 0 rx/s 0 err,drop/s
By observing the Rx PIR and CIR registers, CIR is always 0x7FF and
PIR is always 0x7FE, which means that the Rx ring is full and can no
longer accommodate other Rx frames. Therefore, the problem is caused
by the Rx BD ring not being cleaned up.
Further analysis of the code revealed that the Rx BD ring will only
be cleaned if the "cleaned_cnt > xdp_tx_in_flight" condition is met.
Therefore, some debug logs were added to the driver and the current
values of cleaned_cnt and xdp_tx_in_flight were printed when the Rx
BD ring was full. The logs are as follows.
[ 178.762419] [XDP TX] >> cleaned_cnt:1728, xdp_tx_in_flight:2140
[ 178.771387] [XDP TX] >> cleaned_cnt:1941, xdp_tx_in_flight:2110
[ 178.776058] [XDP TX] >> cleaned_cnt:1792, xdp_tx_in_flight:2110
From the results, the max value of xdp_tx_in_flight has reached 2140.
However, the size of the Rx BD ring is only 2048. So xdp_tx_in_flight
did not drop to 0 after enetc_stop() is called and the driver does not
clear it. The root cause is that NAPI is disabled too aggressively,
without having waited for the pending XDP_TX frames to be transmitted,
and their buffers recycled, so that xdp_tx_in_flight cannot naturally
drop to 0. Later, enetc_free_tx_ring() does free those stale, unsent
XDP_TX packets, but it is not coded up to also reset xdp_tx_in_flight,
hence the manifestation of the bug.
One option would be to cover this extra condition in enetc_free_tx_ring(),
but now that the ENETC_TX_DOWN exists, we have created a window at
the beginning of enetc_stop() where NAPI can still be scheduled, but
any concurrent enqueue will be blocked. Therefore, enetc_wait_bdrs()
and enetc_disable_tx_bdrs() can be called with NAPI still scheduled,
and it is guaranteed that this will not wait indefinitely, but instead
give us an indication that the pending TX frames have orderly dropped
to zero. Only then should we call napi_disable().
This way, enetc_free_tx_ring() becomes entirely redundant and can be
dropped as part of subsequent cleanup.
The change also refactors enetc_start() so that it looks like the
mirror opposite procedure of enetc_stop().
Fixes: ff58fda09096 ("net: enetc: prioritize ability to go down over packet processing")
Cc: stable@vger.kernel.org
Signed-off-by: Wei Fang <wei.fang@nxp.com>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Link: https://patch.msgid.link/20241010092056.298128-5-wei.fang@nxp.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
show more ...
|
| a419f478 | 10-Oct-2024 |
Wei Fang <wei.fang@nxp.com> |
net: enetc: disable Tx BD rings after they are empty
commit 0a93f2ca4be6c4616d371f18a3fabad2df7f8d55 upstream.
The Tx BD rings are disabled first in enetc_stop() and the driver
waits for them to be
net: enetc: disable Tx BD rings after they are empty
commit 0a93f2ca4be6c4616d371f18a3fabad2df7f8d55 upstream.
The Tx BD rings are disabled first in enetc_stop() and the driver
waits for them to become empty. This operation is not safe while
the ring is actively transmitting frames, and will cause the ring
to not be empty and hardware exception. As described in the NETC
block guide, software should only disable an active Tx ring after
all pending ring entries have been consumed (i.e. when PI = CI).
Disabling a transmit ring that is actively processing BDs risks
a HW-SW race hazard whereby a hardware resource becomes assigned
to work on one or more ring entries only to have those entries be
removed due to the ring becoming disabled.
When testing XDP_REDIRECT feautre, although all frames were blocked
from being put into Tx rings during ring reconfiguration, the similar
warning log was still encountered:
fsl_enetc 0000:00:00.2 eno2: timeout for tx ring #6 clear
fsl_enetc 0000:00:00.2 eno2: timeout for tx ring #7 clear
The reason is that when there are still unsent frames in the Tx ring,
disabling the Tx ring causes the remaining frames to be unable to be
sent out. And the Tx ring cannot be restored, which means that even
if the xdp program is uninstalled, the Tx frames cannot be sent out
anymore. Therefore, correct the operation order in enect_start() and
enect_stop().
Fixes: ff58fda09096 ("net: enetc: prioritize ability to go down over packet processing")
Cc: stable@vger.kernel.org
Signed-off-by: Wei Fang <wei.fang@nxp.com>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Link: https://patch.msgid.link/20241010092056.298128-4-wei.fang@nxp.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
show more ...
|
| 37184349 | 10-Oct-2024 |
Wei Fang <wei.fang@nxp.com> |
net: enetc: block concurrent XDP transmissions during ring reconfiguration
commit c728a95ccf2a8ba544facfc30a4418d4c68c39f0 upstream.
When testing the XDP_REDIRECT function on the LS1028A platform,
net: enetc: block concurrent XDP transmissions during ring reconfiguration
commit c728a95ccf2a8ba544facfc30a4418d4c68c39f0 upstream.
When testing the XDP_REDIRECT function on the LS1028A platform, we
found a very reproducible issue that the Tx frames can no longer be
sent out even if XDP_REDIRECT is turned off. Specifically, if there
is a lot of traffic on Rx direction, when XDP_REDIRECT is turned on,
the console may display some warnings like "timeout for tx ring #6
clear", and all redirected frames will be dropped, the detailed log
is as follows.
root@ls1028ardb:~# ./xdp-bench redirect eno0 eno2
Redirecting from eno0 (ifindex 3; driver fsl_enetc) to eno2 (ifindex 4; driver fsl_enetc)
[203.849809] fsl_enetc 0000:00:00.2 eno2: timeout for tx ring #5 clear
[204.006051] fsl_enetc 0000:00:00.2 eno2: timeout for tx ring #6 clear
[204.161944] fsl_enetc 0000:00:00.2 eno2: timeout for tx ring #7 clear
eno0->eno2 1420505 rx/s 1420590 err,drop/s 0 xmit/s
xmit eno0->eno2 0 xmit/s 1420590 drop/s 0 drv_err/s 15.71 bulk-avg
eno0->eno2 1420484 rx/s 1420485 err,drop/s 0 xmit/s
xmit eno0->eno2 0 xmit/s 1420485 drop/s 0 drv_err/s 15.71 bulk-avg
By analyzing the XDP_REDIRECT implementation of enetc driver, the
driver will reconfigure Tx and Rx BD rings when a bpf program is
installed or uninstalled, but there is no mechanisms to block the
redirected frames when enetc driver reconfigures rings. Similarly,
XDP_TX verdicts on received frames can also lead to frames being
enqueued in the Tx rings. Because XDP ignores the state set by the
netif_tx_wake_queue() API, so introduce the ENETC_TX_DOWN flag to
suppress transmission of XDP frames.
Fixes: c33bfaf91c4c ("net: enetc: set up XDP program under enetc_reconfigure()")
Cc: stable@vger.kernel.org
Signed-off-by: Wei Fang <wei.fang@nxp.com>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Link: https://patch.msgid.link/20241010092056.298128-3-wei.fang@nxp.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
show more ...
|
| bfce089d | 03-Aug-2023 |
Vladimir Oltean <vladimir.oltean@nxp.com> |
net: enetc: remove of_device_is_available() handling
Since commit 6fffbc7ae137 ("PCI: Honor firmware's device disabled
status"), this is redundant and does nothing, because enetc_pf_probe()
no longe
net: enetc: remove of_device_is_available() handling
Since commit 6fffbc7ae137 ("PCI: Honor firmware's device disabled
status"), this is redundant and does nothing, because enetc_pf_probe()
no longer even gets called.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
show more ...
|
| f1e668d2 | 09-Jun-2023 |
Vladimir Oltean <vladimir.oltean@nxp.com> |
net: enetc: reset taprio stats when taprio is deleted
Currently, the window_drop stats persist even if an incorrect Qdisc was
removed from the interface and a new one is installed. This is because
t
net: enetc: reset taprio stats when taprio is deleted
Currently, the window_drop stats persist even if an incorrect Qdisc was
removed from the interface and a new one is installed. This is because
the enetc driver keeps the state, and that is persistent across multiple
Qdiscs.
To resolve the issue, clear all win_drop counters from all TX queues
when the currently active Qdisc is removed. These counters are zero
by default. The counters visible in ethtool -S are also affected,
but I don't care very much about preserving those enough to keep them
monotonically incrementing.
Fixes: 4802fca8d1af ("net: enetc: report statistics counters for taprio")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
show more ...
|
| fdebd850 | 02-Jun-2023 |
Wei Fang <wei.fang@nxp.com> |
net: enetc: correct rx_bytes statistics of XDP
The rx_bytes statistics of XDP are always zero, because rx_byte_cnt
is not updated after it is initialized to 0. So fix it.
Fixes: d1b15102dd16 ("net:
net: enetc: correct rx_bytes statistics of XDP
The rx_bytes statistics of XDP are always zero, because rx_byte_cnt
is not updated after it is initialized to 0. So fix it.
Fixes: d1b15102dd16 ("net: enetc: add support for XDP_DROP and XDP_PASS")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
show more ...
|