| Age | Commit message (Collapse) | Author |
|---|
|
The flag has been introduced in the commit 750011e239a5 ("net: stmmac: Add
support for HW-accelerated VLAN stripping") in order to logically split up
the HW-offloaded VLAN tag stripping and the stripping implemented in the
driver. As the commit log says it's supposed to be in sync with the
NETIF_F_HW_VLAN_CTAG_RX feature-flag, otherwise the software-based VLAN
tag stripping will be performed.
First of all the denoted semantics is incorrect, since the MAC and
driver-based VLAN tag stripping are mutually exclusive features and are
supposed to be enabled/disabled by the
NETIF_F_HW_VLAN_CTAG_RX/NETIF_F_HW_VLAN_STAG_RX feature flags. Secondly
the flags can be directly retrieved from the net_device::features field
anytime in the driver. So introducing another mirror-flag for that was
unnecessary. Finally the MAC-based VLAN tag stripping is always persistent
in the DW *MAC IP-cores supporting VLANs (at least DW Ether QoS and DW
XGMAC). Thus as long as the HW-offloaded VLAN tag stripping is implemented
in the respective VLAN driver it can be utilized instead of the
software-based tag stripping code.
So to speak let's drop the mac_device_info::hw_vlan_en flag and just
switch on/off the VLAN tags stripping feature based on the respective
flags set in the net_device::features field. Please note that since the
VLAN-based features are available on the kernel with the CONFIG_VLAN_8021Q
config enabled it is pointless to parse VLAN Ethernet-header if the config
is disable. In that case the entire stmmac_get_rx_vlan() body can be
compiled off thus speeding up the Rx fast-path of the driver.
Note as a nice side effect of this change the VLAN tag stripping feature
is now capable to be switched on/off by the system administrator
irrespective whether it's HW-offloaded or not.
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Indeed there has been no need in adding the
stmmac_vlan_ops::add_hw_vlan_rx_fltr() and
stmmac_vlan_ops::del_hw_vlan_rx_fltr()
execution dependent from the mac_device_info::num_vlan field value. If the
VLAN perfect filters feature is supported by the driver (DW QoS Ether and
DW XGMAC) then mac_device_info::num_vlan will be always initialized at
least with one. That's because the modern IP-cores (DW QoS Ether and DW
XGMAC) always support at least a Basic VLAN perfect filter engine. If the
driver doesn't support the VLAN perfect filters functionality for the
particular IP-core then the denoted callbacks just won't be available and
their execution attempt will cause the -EINVAL error returned. Thus let's
just correctly parse the return value of the methods call and drop the
mac_device_info::num_vlan conditional statement.
This change is a short preparation before adding a comprehensive
VLAN-engine support to the driver.
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
Note if the dma_features::nrvf_num was accessible in the VLAN-related
callbacks the mac_device_info::num_vlan field could have been completely
dropped as redundant. Do that when it is.
|
|
The commit 041cc86b3653 ("net: stmmac: Enable TSO on VLANs") stated that
the TSO feature was malfunction on DW QoS Eth and DW XGMAC for the VLAN
tagged frames. In particular the commit log claims that the very first TCP
segment is VLAN tagged, but the proceeding ones aren't. This hasn't been
confirmed at least on a DW XGMAC 2.11a device. All the transferred
TCP/IPv4 segments are VLAN tagged.
Moreover the blamed commit stated that it enabled TSO for VLANs while it
actually disabled it. Instead the driver now just inserts the VLAN
TPID/TCI fields to the payload. It's also strange to see that the
HW-accelerated VLAN IDs were actually specified for the TSO SKBs since
before that commit the NETIF_F_TSO flag had been cleared for the VLAN
interfaces. All of that makes me thinking that if there was a problem it
has been purely investigated. Revert the change until the actually broken
hardware is met.
Fixes: 041cc86b3653 ("net: stmmac: Enable TSO on VLANs")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Originally the VLAN Insertion Offload feature was configured to always
insert S-VLAN type due to the MAC_VLAN_Incl.CSVL flag being always set. So
as long as there was at least one 802.1ad virtual interface created any
other virtual VLAN-interface would have generated S-TAG'ed frames, which
was caused by having both MAC_VLAN_Incl.CSVL and MAC_VLAN_Tag.ESVL flags
set. The former flag wouldn't work without the later one - that's why at
least one 802.1ad virtual interface needed to be created to meet the
problem. That has been fixed in the commit c657f86106c8 ("net: stmmac:
vlan: Disable 802.1AD tag insertion offload") just by removing the VLAN
S-TAG Insertion support from the driver with justification that the
controller doesn't support inserting both VLAN C-TAG and S-TAG at a time.
That is only one VLAN tag type can be enabled at a time.
That's true but it doesn't mean the feature should have been just deleted.
The driver can be enabled to support only one of these tags type insertion
at a time. That can be easily achieved by having the networking core
net_device_ops::{ndo_fix_features(),ndo_set_features()} callbacks
utilized. So let's use them to make sure that only one of VLAN tags type
insertion is enabled at a time - NETIF_F_HW_VLAN_CTAG_TX or
NETIF_F_HW_VLAN_STAG_TX. The MAC_VLAN_Incl.CSVL flag will be enabled only
if the NETIF_F_HW_VLAN_STAG_TX feature is requested.
Please note if there is no 802.1ad virtual interface (basically no S-TAG
filter enabled) it won't be possible to generate S-TAG'ed frames no mater
what due to the VLAN_TAG.ESVL flag cleared. This will be fixed in another
commit sometime later.
Fixes: c657f86106c8 ("net: stmmac: vlan: Disable 802.1AD tag insertion offload")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
The commit bd7ad51253a7 ("net: stmmac: Fix VLAN HW state restore") fixed
the problem with the VLAN filters not being restored if the device was
closed or after resume. That could have been done in a clearer and more
coherent way just by calling the stmmac_restore_hw_vlan_rx_fltr() method
in the stmmac_set_filter() method. The later in its turn is called in the
net_device_ops::ndo_set_rx_mode() callback of the driver, which in its
turn is called on device open and resumes. The only
problematic part is to store the HASH/perfect filters context. But it can
be done by adding two more mac_device_info fields aside with the Extended
VLAN perfect filter settings. A similar approach with cacheing and using
ndo_set_rx_mode() to restore VLAN-filters is implemented in some another
network drivers.
Let's do that then. As a nice side-effect the stmmac_vlan_restore() will
be no longer needed. Also the stmmac_ops::set_filter() methods now look
more coherent setting up the all MAC and VLAN filters.
Note the vlan_write_filter() method must be altered to call the
atomic-version of the CSR polling function, since it's caller -
vlan_restore_hw_vlan_rx_fltr() is now called from the BH-disabled context
(see dev_set_rx_mode()).
bd7ad51253a7 ("net: stmmac: Fix VLAN HW state restore")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
It was found out that the problem with the VLAN filters setting up is in
the missing PHY RXC clock when EEE is enabled and the interface being
down. As a result any attempt to create a virtual VLAN interface causes
the error like:
# ip link add link end1 name end1.5 type vlan id 5
15c40000.ethernet end1: Timeout accessing MAC_VLAN_Tag_Filter
RTNETLINK answers: Device or resource busy
# ip link set end1 down
renesas-gbeth 15c40000.ethernet end1: Timeout accessing MAC_VLAN_Tag_Filter
renesas-gbeth 15c40000.ethernet end1: failed to kill vid 0081/0
This was fixed in the commit c171e679ee66 ("net: stmmac: Disable EEE RX
clock stop when VLAN is enabled") and commit 2cd70e3968f5 ("net: stmmac:
Defer VLAN HW configuration when interface is down"). So the problem is no
longer exist. But the later commit has turned to be too aggressive in
fixing it by adding the netif_running() check on each VLAN CSRs access. In
the meantime writing to the normal MAC VLAN CSRs won't cause any problem,
the timeout happens only in the MAC_VLAN_Tag_Filter indirect access. See
the log messages above.
So let's drop the redundant netif_running() checks and add a single one to
the vlan_write_filter() method. The method will return -EAGAIN error in
case if the access could be performed right now so the callee would do
that again later when the interface is brought up.
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
The commit c657f86106c8 ("net: stmmac: vlan: Disable 802.1AD tag insertion
offload") has dropped the double VLAN erroneous insertion for Tx frames.
The Rx VLAN-frames filtering code also suffers from that feature being
enabled but with much less harm. The feature is enabled if VLAN S-TAG
filtering is requested, but it never actually activated since the ERIVLT
and EIVLS flags aren't touched by the code.
Presumably the EDVLP flag setting up used to be working in conjunction
with the VLAN-insertion feature, since it enables the double VLAN
processing for both Rx and _Tx_ paths. So without it no inner VLAN would
be inserted if requested. But that feature has been incorrectly enabled
for the S-VLAN frames and dropped in the framework of the commit
c657f86106c8 ("net: stmmac: vlan: Disable 802.1AD tag insertion offload")
thus fixing a completely broken 802.1ad tags support added in the initial
commit 30d932279dc2 ("net: stmmac: Add support for VLAN Insertion
Offload").
So let's drop the code currently handling the EDVLP flag setting up. Also
fix the naming around the just dropped code to describing the actual
feature the entities are utilized for - enable S-VLAN filtering. After
this change there will be almost no the double VLAN feature trace left in
the driver. The only leftovers are stmmac_desc_ops::set_vlan_tag()
callbacks but the respective part doesn't effect the controller state, so
be it for now.
Note though based on [1, 2, 3] Double VLAN processing feature must be
available and enabled so the *GMACs could properly perform the L2/L3
checksum verification of the frames with one-level inner VLAN tagged.
Let's unconditionally enable it thus speeding up the incoming Double VLAN
tagged frames handling.
[1] DesignWare Cores XLGMAC - Enterprise Ethernet MAC, Revision 2.00a,
September 2017, p.181.
[1] DesignWare Cores XGMAC - 10G Ethernet MAC Databook, Revision 3.20a,
September 2022, p.253.
[2] DesignWare® Cores Ethernet Quality-of-Service, Revision 5.20a,
April 2020, p.368.
Fixes: 3cd1cfcba26e ("net: stmmac: Implement VLAN Hash Filtering in XGMAC")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Originally that functionality was added in the commit c7ab0b8088d7 ("net:
stmmac: Fallback to VLAN Perfect filtering if HASH is not available") in
order to initialize at least some filter if no HASH-based VLAN filter
available since at least one slot of the VLAN perfect filter always
available. But earlier another commit 3cd1cfcba26e ("net: stmmac:
Implement VLAN Hash Filtering in XGMAC") had made sure that no
HW-offloaded VLAN frames filter utilized if no HASH-based VLAN filter
provided in a controller. Thus that functionality has never been actually
used in practice. Moreover the later commit ed64639bc1e0 ("net: stmmac:
Add support for VLAN Rx filtering") implied using that perfect-filter
register if no Extended VLAN filter feature is available thus partly
duplicating what has already been implemented in the driver.
So to speak let's drop the unused code especially seeing the respective
device feature is utilized in a framework of the perfect-filter
initialization procedure already.
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
The dwxgmac2_update_vlan_hash() method now completely match to
vlan_update_hash(). In the meantime the rest of the callbacks utilized in
dwmac_vlan_ops and dwxgmac210_vlan_ops are the same. Thus drop the
dwxgmac210_vlan_ops descriptor and use now common dwmac_vlan_ops
descriptor instead. Rename it to dwmac4_vlan_ops so not to be accidentally
utilized for the older IP-cores which have a bit different VLAN engine and
isn't compatible with current VLAN implementation in the driver.
Note the current VLAN module can be fully utilized for DW XLGMAC IP-cores
too. But leave it be as is for the engineers with the compatible hardware
at hand. Don't bother with that right now.
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
The MAC_HW_Feature3.DVLAN flag has been available since the DW XGMAC
IP-core v3.00a, but the Double VLAN Processing feature can be enable since
the v2.00a version (or later than v1.20a). Thus it's incorrect to
determine the DVLAN feature availability based on the flag state on the
early versions of the IP-core. Since we don't know since what particular
IP-core version the Double VLAN Processing feature is available let's
provide the auto-detection procedure for any of them if DVLAN flag has
been met cleared.
The auto-detection procedure is created by checking the
MAC_VLAN_Tag.EIVLRXS (Enable Inner VLAN Tag in Rx Status) flag
writability. The IP-cores databooks define the flag as 0x0 after reset and
available only if the DWCXG_DOUBLE_VLAN_EN parameter is enabled. Otherwise
it's reserved, RO and zero.
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
The amount of the Extended Rx Perfect Filter slots is reported via the
FEATURE3.NRVF field on the DW IP-core supporting that features: DW QoS
Ether and DW XGMAC/XLGMAC/etc. The field encrypts the size of the filter
as follows:
- 000: No Extended Rx VLAN Filters
- 001: 4 Extended Rx VLAN Filters
- 010: 8 Extended Rx VLAN Filters
- 011: 16 Extended Rx VLAN Filters
- 100: 24 Extended Rx VLAN Filters
- 101: 32 Extended Rx VLAN Filters
- 110-111: Reserved
In the meantime the dma_cap DebugFS node incorrectly decrypts the field as
(nrvf << 1) and only for the DW XGMAC IP-core. Let's fix that by
appropriately parsing the FEATURE3.NRVF field on the both IP-cores
supporting the Extended Rx Perfect Filter feature.
Fixes: ed64639bc1e0 ("net: stmmac: Add support for VLAN Rx filtering")
Fixes: 669a55560e4b ("net: stmmac: Check more MAC HW features for XGMAC Core 3.20")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
In fact the always strip mode enabled in the vlan_set_hw_mode() method
means stripping both VLAN tag types C and S ones. Thus both of these
NET-device features must be enabled/disabled simultaneously if requested
by ethtool.
Fix that by adding the respective conditionals to the ndo_fix_feature()
method and making sure that both of these features are synchronously
switchable.
Fixes: 750011e239a5 ("net: stmmac: Add support for HW-accelerated VLAN stripping")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Commit 8eb301bd7b0f ("net: stmmac: enable HW-accelerated VLAN stripping
for gmac4 only") attempted to fix the problem of the HW-accelerated VLAN
tag stripping feature being enabled for all MAC cores even despite of
having it supported by the DW GMAC4 driver only. But the solution only
works up until the ndo_set_features() method is called. When it's done the
mac_device_info::hw_vlan_en field will be set to true thus disabling the
software-based VLAN tag stripping implementation and relying on the MAC to
do that. Of course that won't be done on the DW GMAC- nor DW XLGMAC-based
devices since the driver currently doesn't support the feature on these
IP-cores.
Let's completely fix the denoted problem just by changing the HW-offloaded
VLAN tag detection flag only if the NETIF_F_HW_VLAN_CTAG_RX flag is set in
ndt_device::hw_features. The later condition is only met for the DW GMAC4
and DW XGMAC IP-cores currently.
Fixes: 8eb301bd7b0f ("net: stmmac: enable HW-accelerated VLAN stripping for gmac4 only")
Fixes: 750011e239a5 ("net: stmmac: Add support for HW-accelerated VLAN stripping")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
The stmmac_vlan_update() method falls back to the basic perfect filter
case if no VLAN hash filter detected. In that case the method completely
overrides the VLAN_TAG register clearing the settings like EVLRXS or EVLS,
which are required for the VLAN interface to work correctly. For instance
EVLRXS activates VLAN Tag in Rx status getting the Rx DMA-descriptors.
So let's fix that in a way it has been done for DW XGMAC VLAN in commit
907a076881f1 ("net: stmmac: xgmac: fix incorrect XGMAC_VLAN_TAG register
writting") - by using the already read value of the VLAN_TAG register.
Fixes: c7ab0b8088d7 ("net: stmmac: Fallback to VLAN Perfect filtering if HASH is not available")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
If no Extended VLAN filter Filtering supported, then a basic perfect
VLAN-filter will be available. That filter regards VID=0 as a special value
which if specified in the filter register basically means to accept any
VLAN frame irrespective to the hash-based VLAN-filter settings. Here is
the MAC_VLAN_Tag.VID field description actual for both DW QoS Eth and DW
XGMAC IP-cores:
> If this field ([11:0] if ETV is set) is all zeros, the MAC does not
> check the 15th and 16th bytes for VLAN tag comparison and
> declares all packets with Type field value of 0x8100 or 0x88a8
> as VLAN packets.
This is also documented by the table "VLAN Match Status" in the IP-cores
databooks:
VID | VLAN Perfect Filter | VTHM | VLAN Hash Filter | Final VLAN |
| Match Result | Bit | Match Result | Match Status |
----+---------------------+------+------------------+--------------+
0 | Pass | 0 | Any | Pass |
0 | Pass | 1 | Any | Pass |
So in order to have the VLAN-filtering actually working for hardware with
no Extended VLAN filter support by default let's always initialize the
MAC_VLAN_Tag.VID with 0xffff's if no real VID specified. Thus no real VLAN
frames would be permitted except the packets with the reserved VID, which
is better than permitting all VIDs and making VLAN hash filter basically
useless.
Fixes: 3cd1cfcba26e ("net: stmmac: Implement VLAN Hash Filtering in XGMAC")
Fixes: ed64639bc1e0 ("net: stmmac: Add support for VLAN Rx filtering")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
The blamed commit added the Extended VLAN Tag Perfect filtering support to
the DW Ether QoS part of the driver. Then the implementation has been
reused for DW XGMACs in commit 534df0c1724b ("net: stmmac: dwxgmac2: Add
support for HW-accelerated VLAN stripping").
Originally it was intended for the VLAN TAG perfect filters to be used
orthogonally with the VLAN Tag Hash filtering. So one functionality
would work irrespective to another one being available. Well, with some
flaws but it has worked. The problem comes when you get to have a device
with no VLAN Tag Hash filter but with the Extended VLAN Tag filter
support. In such case by default the S-TAG filter won't be activated
causing C-TAG frames filtering instead, because no ERSVLM or DOVLTC flag
set in the MAC_VLAN_Tag_Filter register. These flags are responsible for
VLAN S-TAG tag activating or disabling VLAN tag type checking. Anyway in
case if the VLAN Tag Hash filtering is supported it will be _responsible_
for permitting both C- and S-Tags of the same VID, which at least isn't
secure. But the VLAN perfect filter S-TAG entries will be misconfigured.
Let's fix the problem denoted above by setting up the ERSVLM flag in case
if the 802.1ad VLAN-protocol requested. The flag will be preserved in the
private-data VIDs cache so to be properly restored after system resume and
to differentiate the tags with the same VIDs but different types.
Fixes: ed64639bc1e0 ("net: stmmac: Add support for VLAN Rx filtering")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
The commit 8c5f48d99de2 ("net: stmmac: dwxgmac2: Also pass control frames
while in promisc mode") made sure that the Ethernet flow-control packets
get to be passed to the system memory when IFF_PROMISC by setting the
MAC_Packet_Filter.PCF flag. But it forgot to have the bit cleared after
the promiscuous mode was disabled. Let's fix that.
Fixes: 8c5f48d99de2 ("net: stmmac: dwxgmac2: Also pass control frames while in promisc mode")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Currently the promiscuous mode doesn't work for VLAN frames. So if there
is a VLAN interface created on top of an DW XGMAC native interface only
VLAN-frames with the specified VIDs will be received despite of having the
promiscuous mode activated. That's because the stmmac_ops::set_filter()
callback doesn't disable the VLAN-filters when the IFF_PROMISC mode is
detected.
Let's fix the problem the way it has already been done for DW Ether QoS
GMAC in commit Fixes: a7602e7332b9 ("net: stmmac: don't reject VLANs when
IFF_PROMISC is set"). The only thing that needs to be done is moving the
PACKET_FILTER.VTFE flag toggling to the denoted set_filter() callback.
Thus if the IFF_PROMISC flag is set the PACKET_FILTER.VTFE flag will be
cleared and VLAN-filter will be de-activated. Otherwise the flag will be
left set permitting VLAN-filters to work (if any was configured).
Fixes: 3cd1cfcba26e ("net: stmmac: Implement VLAN Hash Filtering in XGMAC")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Tx/Rx COE and TSO features apply additional constraint to DMA-mode and
MTU. First of all DMA Store-and-forward mode implies that the frames
length mustn't exceed the Tx/Rx FIFO depth. Secondly all DW GMAC, DW QoS
Ether and DW XGMAC/XLGMAC demands to apply additional constraint on the Tx
frames size if Tx COE is active [1, 2, 3]:
"You must make sure that the Transmit FIFO is deep enough to store a
complete frame before that frame is transferred to the MAC transmitter.
The reason being that when space is not available to accept the programmed
burst length of data, then the MTL Tx FIFO starts reading to avoid
dead-lock. When reading starts, the COE fails and consequently all
succeeding frames may get corrupted because of improper recovery.
Therefore, you must enable the checksum insertion only in the frames that
are less than the following number of bytes in size (even in the
store-and-forward mode): TXFIFO_SIZE - ((PBL + N)*(DATAWIDTH/8))"
Thirdly similar but less strict constraint exist if TSO feature is
enabled on DW QoS Ether or DW XGMAC/XLGMAC [2, 3]:
"The header length plus the MSS size (which is equal to the size of each
TCP segment) must not exceed 16383 bytes, otherwise, the MAC transmitter
truncates the packet after 16383 bytes causing a CRC error. The header
length plus MSS size plus programmed PBL value in register must be lesser
than the programmed Tx Queue size."
It was surprising to realize that almost none of these constraints are
taken into account in the driver, except MTU being less than Tx FIFO size.
In the meantime exceeding any of them will cause frames full corruption.
That's what happens in case if Jumbo/Giant frames activated.
Let's fix the denoted problem by adding the comprehensive procedure
to support the net_device::max_mtu field in the correct state. For that
the driver now performs the max MTU constraint calculation on:
1. device probe stage so to start device using with a correct constraing.
2. on each change of the active Tx/Rx queues since it causes Tx/Rx FIFO
size change on the modern DW network controllers.
3. on each Tx COE feature activation/de-activation due to the
feature-specify frame size requirement.
Note on each action causing net_device::max_mtu change the driver now has
to make sure the current MTU doesn't exceed the new constraint otherwise
the action won't be accepted.
As a nice consequence of this change the net_device_ops::ndo_change_mtu()
callback won't need to have FIFO-specific MTU sanity checks. The
respective constraints are now reflected in net_device::max_mtu.
[1] DesignWare Cores Ethernet MAC Universal Databook, Revision 3.73a,
October 2013, p. 185.
[2] DesignWare Cores Ethernet Quality-of-Service Databook, Revision 5.20a,
April 2020, p. 364/380.
[3] DesignWare Cores XGMAC - 10G Ethernet MAC Databook, Revision 3.20a,
October 2022, p. 249/257.
Fixes: eaf4fac47807 ("net: stmmac: Do not accept invalid MTU values")
Fixes: d6ddfacd95c7 ("net: stmmac: Add DMA related callbacks for XGMAC2")
Fixes: f748be531d70 ("stmmac: support new GMAC4")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
In accordance with [1,2,3] the PBL (Programmable Burst Length) values are
only valid if they belong to the set [1, 2, 4, 8, 16, 32]. Specifying any
different value results to Undefined Behaviour. Moreover the maximum value
is also limited. The maximum burst length must not exceed the half of
the Tx/Rx Queue Depth (Queue Depth = Queue or FIFO size / Data width).
Without this requirement being fulfilled the communications will be
fully broken.
Since the system interface data bus width is now available in the driver
private data let's use it to calculate the effective MTL FIFO depth and
make sure that the DMA PBL specified by the platform is correct at least
for a single activated queue in order to avoid hardware UB-related errors.
The best place to do that is to introduce a new stmmac_dma_verify()
method, which would be called on the driver probing stage. It will ensure
that the PBL is valid at least for the default device setup. Besides the
method will contain the rest of the DMA settings verification and
adjustment so to be as coherent as possible.
[1] DesignWare Cores Ethernet MAC Universal Databook, Revision 3.73a,
October 2013, p. 380, Table 6-5.
[2] DesignWare Cores Ethernet Quality-of-Service Databook, Revision 5.20a,
April 2020, p. 1185, Table 17-303 and p. 1190, Table 17-304.
[3] DesignWare Cores XGMAC - 10G Ethernet MAC Databook, Revision 2.11a,
September 2015, p. 471, Table 7-17 and p. 474, Table 7-18.
Fixes: d6ddfacd95c7 ("net: stmmac: Add DMA related callbacks for XGMAC2")
Fixes: f748be531d70 ("stmmac: support new GMAC4")
Fixes: 47dd7a540b8a ("net: add support for STMicroelectronics Ethernet controllers.")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
As a preparation to the PBL and MTU constraint implementation let's
introduce a dedicated method which would just calculate the Tx/Rx FIFO
sizes based on the currently active queues settings. Thus the driver would
have the duplicated code of the FIFO depth calculation moved to one
coherent method and be prepare for the announced fixes.
Note there is no point in checking whether there is a FIFO size passed via
the platform configs storage each time the Tx/Rx FIFO size values are
required, since neither the FIFO size platform setting nor the FIFO size
DMA-capability get changed after the corresponding devices are probed and
added to the system. Thus the total Tx/Rx FIFO size can be set just once
on the device probe (HW-initialization) procedure: if non-zero platform
FIFO size is detected then the FIFO size DMA-capability will be ignored.
Also note this change fixes the Loongson GMAC/GNET device support which
the only DW GMAC v3.70a device currently declared to support the
multi-channel feature. The commit ad72f783de06 ("net: stmmac: Add
multi-channel support") has forgotten to fix stmmac_change_mtu() so the
method wouldn't divide the MTL FIFO memory between the queues/channels.
The multi-channels DW GMACs have the per-channel non-configurable MTL FIFO
memory. So the stmmac_change_mtu() has been wrong in unconditionally
dividing the total MTL FIFO memory and using the resultant as the upper
limit of MTU.
Fixes: ad72f783de06 ("net: stmmac: Add multi-channel support")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Currently the driver initialize the maximum MTU value as follows:
1. DW XGMAC/XLGMAC: 16KiB
2. DW GMAC with Enhanced DMA-descriptors and DW QoS Ether: 9000 bytes
3. DW MAX100 and DW GMAC with Normal DMA-descriptors: PAGE_SIZE.
This is wrong in the aspects 2. and 3. Here is why. First of all DW QoS
Ether have always supported Tx and Rx up to 16KiB frames size in the same
way as DW XGMAC-like devices. The latest Jumbo-related commit has enabled
that in the driver too. Secondly no matter what type of the DMA-descriptor
selected DMA GMACs have always supported up to 9K frames at least in the
cross-descriptor manner. Thirdly the legacy DW MAC100 controllers have
never been able to xmit/recv frames greater than 1500 bytes. There is no
any Jumbo-like setup in the MAC config registers. It has been even more
wrong to use SKB_MAX_HEAD() macro to calculate max MTU constraint since
it's basically converted to PAGE_SIZE which is platform-specific, can be
even greater than 16KiB and doesn't determine the actual MTU constraint
applicable for the particular DW network controller. Finally the MTU
verification procedure isn't complete in the net_device:ndo_change_mtu()
method. The upper-limit is specific to the DW XGMACs only (and actually
redundant). Moreover the Tx FIFO size-based constraint is also incorrect -
it isn't enough to set up the upper MTU-limit based on the Tx-FIFO size
(but it will be fixed later).
Let's fix all of the inconsistencies above by activating the 16KiB Jumbos
for DW QoS Ethernet, setting up 9000 max MTU for any DW GMAC, getting
back the normal max MTU for the legacy controllers and dropping the manual
max MTU verification in the net_device:ndo_change_mtu() method.
Fixes: eaf4fac47807 ("net: stmmac: Do not accept invalid MTU values")
Fixes: f748be531d70 ("stmmac: support new GMAC4")
Fixes: 45db81e1590c ("stmmac: limit max_mtu in case of 4KiB and use __netdev_alloc_skb (V2)")
Fixes: 48febf7e6476 ("stmmac: allow mtu bigger than 1500 in case of normal desc (V4)")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
DW *MAC IP-cores can be synthesized with various system interface data bus
widths. For instance DW GMACs and DW QoS Ether can work with buses of 32,
64 and 128 bits width, while DW xGMACs can be created with system bus of
64 and 128 bits width. The bus width value affects several very important
parameters like DMA transfers granularity and performance, DMA maximum
burst length, Rx DMA buffers alignment. So in order to be able to tune and
verify these parameters let's add the data bus width device property
support. It can be passed either by the platform code or via the DT-node
property. For some reason HW capability register doesn't contain that
parameter in its fields.
Note if the data bus width isn't specified then the driver will use the
default value of 16 bytes. That will apply the strongest constraint on the
PBL values and Rx DMA buffers alignment (the later one is actually
currently hard-coded anyway). In order to get a better HW performance it's
preferable to specify the actual value for a particular version of the
controller.
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
DW *MAC IP-cores can be synthesized with various system interface data bus
widths. The bus width normally depends on the SoC system bus capability and
is selected by the hardware engineers in a way to deliver the best
interface throughput.
In accordance with the IP-cores databooks the system bus width is fixed to
a single value for all system bus transfers. For instance if it's set to
16 bytes (data bus width of 128 bits) then each DMA bus transfer size will
be a multiple of 16 bytes and can't be smaller than that value (that's
where the Rx DMA buffer alignment restriction comes from). In addition to
that aside with the system interface performance the bus width determines
the DW MAC Programmable Burst Length upper limit and Rx descriptors buffer
size alignment.
Since the system bus width is selected during DW *MAC IP-core synthesize
procedure, is specific to each controller and can't be detected on runtime
let's add a new property "snps,data-width" to the DW MAC DT-bindings file.
The property needs to be placed in the root DW MAC DT-nodes because no
matter which system bus interface type has been selected during the
IP-core synthesize (AXI, AHB, MTL), the parameter mainly determines the
internal MTL core data bus width. The external system/application bus data
width is then fixed with that value.
Note the system bus width can be of 32, 64 and 128-bits for DW GMAC and DW
QoS Ether, while DW xGMAC can have only 64 and 128 bits wide system data
bus.
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Similarly to what has been relevant for DW XGMAC the driver currently
always enable the Jumbo frames up to 16K bytes to receive and transmit for
DW QoS Ether too (DW GMAC4). It's done by permanently setting the
MAC_CONFIG.JD and MAC_CONFIG.JE flags. Basically it means the driver
completely ignores the MTU settings permitting up to 16K outgoing frames
and no greater than 9K incoming frames.
It isn't right to disregard the MTU setting especially such asymmetrically
since the MTU value might be an important parameter of an Ethernet network
segment. Moreover based on what is said in [1] the most preferred way to
support MTU would be to drop the oversized frames instead of passing them
up to the networking core subsystem. It's also important to note that even
though [1] says that the upper layer protocols must not pass a socket
buffer (skb) to a device to transmit with more data than the MTU in fact
it does at least in case of the pktgen module.
The driver currently doesn't fulfill these requirements and
passes/transfers the incoming/outcoming frames further ignoring the MTU
value. Let's fix that by activating the Giant frame setting and by setting
the internal watchdog up to drop the incoming frames which size is greater
then MTU and truncate the outgoing 2048/16383-octets oversized frames.
Thus the behaviour will match to what is already implemented for DW
GMACs/XGMACs and follow the networking subsystem recommendations. This
shall significantly speed up the oversized frames handling.
Note the driver currently just drops the frames with over Giant frame size
so the describes recv truncation seems unnecessary. But it will be
utilized in the NETIF_F_RXALL feature implementation.
Also note since this change the driver will support DW QoS Ether
controller to send/receive frames up to MTU=16K. The net_device::max_mtu
parameter will be accordingly fixed a bit later.
[1] Documentation/networking/netdevices.rst
Fixes: 41f2a3e6367e ("net: stmmac: dwmac4: Enable RX Jumbo frame support")
Fixes: 477286b53f55 ("stmmac: add GMAC4 core support")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
The driver currently always enables the Jumbo frames up to 16K bytes to
receive and transmit. It's done by setting the MAC_TX_CONFIG.JD flag and
initializing the MAC_RX_CONFIG.GPSL field with the 16K frame size.
Basically it means the driver completely ignores the MTU settings despite
of what was said in the commit 8a488c3f97cd ("net: stmmac: xgmac: Enable
RX Jumbo frame support").
It isn't right to disregard the MTU setting since the MTU value might be
an important parameter of an Ethernet network segment. Moreover based on
what is said in [1] the most preferred way to support MTU would be to drop
the oversized frames instead of passing them up to the networking core
subsystem. It's also important to note that even though [1] says that the
upper layer protocols must not pass a socket buffer (skb) to a device to
transmit with more data than the MTU in fact it does at least in case of
the pktgen module.
So currently the driver doesn't fulfill these requirements and
passes/transfers the incoming/outcoming frames further ignoring the MTU
value. Let's fix that by activating the Giant frame setting and by setting
the internal watchdog up to drop the incoming frames which size is greater
then MTU and truncate the outgoing oversized frames with size threshold
2048/16383. Thus the behaviour will get to be closer to what is already
implemented for DW GMACs (except a more accurate Giant frame setting and
one truncation threshold less of the Tx frames) and follow the networking
subsystem recommendations. This shall significantly speed up the oversized
frames handling.
Note the driver currently just drops the frames with over Giant frame size
so the describes recv truncation seems unnecessary. But it will be
utilized in the NETIF_F_RXALL feature implementation.
[1] Documentation/networking/netdevices.rst
Fixes: 8a488c3f97cd ("net: stmmac: xgmac: Enable RX Jumbo frame support")
Fixes: 2142754f8b9c ("net: stmmac: Add MAC related callbacks for XGMAC2")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
The driver setups the frame size limits by using the MAC_CONTROL.JD and
MAC_CONTROL.JE flags. The former one is responsible for the transmit
Jabber engine disabling and the later one enables the Jumbo frames support
for both Rx and Tx path. But if the MAC_CONTROL.JD is set MAC_CONTROL.JE
won't affect the Tx path and only works for the Rx traffic. Thus the
driver currently permits to transmit frames up to 16K bytes irrespective
to the MTU value, to receive frames of up to 1518(1522), 2000, 9018(9022)
bytes with no packet dropped due to the giant frame status, to get
truncated frames over 2048 and 10240 bytes. Moreover giant frames dropping
isn't working by default for the DW GMAC controllers of v3.70a and newer
when there is no Giant frame status reported via the Rx DMA-descriptors
(if Advanced Timestamp or Rx COE v1/v2 features is selected during the
IP-core synthesizing).
It isn't right to consider the MTU setting that relaxed and to have the
device working so much differently depending on the controller version.
Moreover based on what is said in [1] the most preferred solution would be
to drop the oversized frames instead of passing them up to the networking
core subsystem. Also note even though [1] says that the upper layer
protocols must not pass a socket buffer (skb) to a device to transmit with
more data than the MTU in fact it does at least in case of the pktgen
module.
In case of DW GMAC the driver can work way better to comply with what the
networking subsystem requires. First of all the MAC_CONTROL.JD can be
cleared thus enabling the MAC internal watchdog to truncate the oversized
frames transmission on 2048 or 10240 outgoing octets. Secondly starting
from v3.70a there is a watchdog timeout register which can be used to
fine-tune the MAC to truncate the recv frames of the size greater than
specified. Finally since the v3.70a IP-core there has been the
DMA_CONTROL.DGF flag activating the Giant frames dropping.
All the features above can be used to implement a more clever Jumbo frames
support so the network interface behaviour would be unified across various
DW GMAC IP-core versions (and looking a bit ahead across modern DW QoS
Ether/XGMAC/etc). So DW GMAC v3.70a and newer controllers will have
implemented the next MTU semantics in the driver:
1. MTU <= 1500: truncate xmit frames greater than 2048, drop recv
frames greater than 1518(1522).
2. MTU <= 1978: truncate xmit frames greater than 2048, drop recv
frames greater than 1996(2000).
3. 1978 < MTU <= 9000: truncate xmit frames greater than 10240 and recv
frames greater than ALIGN_UP(MTU+22, 1024), drop recv frames greater than
9018(9022).
4. 9000 < MTU <= 16383: truncate xmit and recv frames greater than 16K
bytes, drop recv frames greater than 9018(9022).
This shall not only extend the Jumbo-frames support, but also shall speed
up the oversized frames handling since the truncated packets will likely
to fit into a single Rx DMA-descriptor, which then will be just dropped as
erroneous and re-initialized.
In case of the DW GMAC older than v3.70a version the semantics will be as
follows:
1. MTU <= 1500: truncate xmit and recv frames greater than 2048,
drop recv frames greater than 1518(1522).
2. 1500 < MTU <= 9000: truncate xmit and recv frames greater than 10240,
drop recv frames greater than 9018(9022).
3. 9000 < MTU <= 16383: truncate xmit and recv frames greater than 16K,
drop recv frames greater than 9018(9022).
It's almost the same as before except that the oversized transmitted
frames will be truncated from now.
Note the driver currently just drops the frames with over Giant frame size
so the describes recv truncation seems unnecessary. But it will be
utilized in the NETIF_F_RXALL feature implementation.
[1] Documentation/networking/netdevices.rst
Fixes: 2618abb73c89 ("stmmac: Fix kernel crashes for jumbo frames")
Fixes: 84c9f8c41df9 ("net: stmmac: Add ip version to dts bindings")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Currently the MTU change procedure doesn't work properly on the DW GMAC
devices. It does change the Rx buffer size respectively though but as long
as there is greater than 2K/9K frames get to be received they will be
discarded by the MAC. The problem is that the stmmac_ops::core_init()
method relies on the net_device::mtu field value which on the MTU-change
procedure contains not new but old MTU. So the GMAC_CONTROL.JE and
GMAC_CONTROL.2K flags responsible for the Rx frames length constraints
won't be properly initialized.
Let's fix that by making sure that the net_device::mtu field contains a
new MTU before the interface restart procedure is executed in the
stmmac_change_mtu() method.
Note even though the commit 347007968744 ("net: ethernet: stmicro: stmmac:
permit MTU change with interface up") changed the core part of the STMMAC
driver the problem is specific to the DW GMAC only. The rest of
stmmac_ops::core_init() implementations have the Giant frames always
enabled. So the MTU change procedure just doesn't cause the frames length
constraints enabling/disabling.
Fixes: 347007968744 ("net: ethernet: stmicro: stmmac: permit MTU change with interface up")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
The recent commits fixing the Rx DMA-descriptors initialization for the DW
MAC100/GMAC switched the code to being agnostic from the power-of-2 Rx
buffers size. Instead they fully follow the HW databooks requirements the
buffers address and size to be properly aligned. The same has been
applicable for the DW QoS Ether, DW XGMAC/XLGMAC devices support from the
very initial commit adding them to the driver.
So let's drop the pre-defined set of the Rx buffers sizes and just use the
one MTU-based Rx buffer size instead. Not sure why hasn't this been done
from the very initial driver commit. Sigh...
Note this shall improve the driver performance as well since more buffers
will be able to fit the orderly allocated pages by the Page Pool means.
Thus less more data page-locality, less TLB misses, better CPU performance
in handling the incoming traffic.
Fixes: 47dd7a540b8a ("net: add support for STMicroelectronics Ethernet controllers.")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
Not really a fix, but well...
|
|
As soon as the Jumbo frames with MTU over 2K/4K (for the Normal
descriptors) and over 8K (for the chained Enhanced descriptors) enabled
the frames exceeding these limits won't be delivered to the networking
core and will be just discarded. This happens due to the driver not
supporting the cross-descriptors data reception (see the
stmmac_desc_ops::rx_status() callbacks handling the non-last segment
descriptors).
DW GMAC layer of the STMMAC driver currently supports the two types of the
Rx DMA-descriptors which besides the different layout differ by the
maximum buffers size:
Normal Rx descriptor: 2K (chain), 2x2K (ring)
Enhanced Rx descriptor: 8K (chain), 2x8K (ring)
Since the cross-descriptor data reception is basically disabled the ringed
Enhanced Rx-descriptors will only work for the 9K jumbo frames retrieval.
The rest of the modes will cause the over the buffer-size frames dropping.
This is definitely wrong seeing the driver unconditionally setups max 9K
MTU constraint for any DW GMAC devices. Moreover the older non-GMAC
IP-cores are supposed to work with the SKB_MAX_HEAD() MTUs which implies
up to Page-sized frames reception. In that case depending on the
system-wide page size the Jumbo frames reception won't properly work
either.
Let's fix the problem described above. In fact the driver code has already
been prepared for that by the preceding fixes and the only what left to be
done is to just return the "rx_not_ls" status from the DW GMAC-specific
stmmac_desc_ops::rx_status() callback. The STMMAC core driver will handle
the rest as is.
Fixes: 286a83721720 ("stmmac: add CHAINED descriptor mode support (V4)")
Fixes: 47dd7a540b8a ("net: add support for STMicroelectronics Ethernet controllers.")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
Note the patch used to fix the stmmac_rx_buf1_len(). It must take into
account that it's possible to have two buffers initialized in the Rx
DMA-descriptors. So the very first buffer of the last segment could be not
containing the tail of the Rx frame. It's specific for the DW GMAC4/XGMAC
IP-cores supporting the split-headers and failing to split the retrieved
frame for some reason.
|
|
Currently the DMA-descriptor release method is implemented for the Tx
descriptors and specific for the related code semantics. The Rx
DMA-descriptors are being prepared by means of the
stmmac_desc_ops::set_rx_owner() methods, which besides to setting the
DMA-owner flag also initialize the Interrupt-on-Completion flag and the
buffer1-valid flag. This makes the functions body noncoherent and thus
the callee code - harder to read/comprehend. Instead let's introduce the
stmmac_desc_ops::release_rx_desc() methods for all currently supported DW
MAC IP-cores, which would implement the semantics required for the
Rx DMA-descriptors being released for DMA.
Note this change also moves the DMA-Wr memory barrier to the new
release_rx_desc() and makes sure it's executed only ones on the
initial descriptor release. This shall speed-up the procedure a bit.
Besides the commit happens to fix a problem specific for the DW MAC
IP-cores with the fully written back Rx DMA-descriptors (DW GMAC >= v4.x
and DW XGMAC >= v2.x). Since the DW MAC DMA-engine of these IP-cores
overwrites all the descriptors fields it also clears/sets the
Interrupt-on-Completion flag depending on the RDES3.CTX bit state. So the
release procedure must correctly restore the IOC flag in accordance with
the disable_rx_ic argument value (Rx WDT status) including the possible
clearance.
Fixes: 753a71090f33 ("stmmac: add descriptors function for GMAC 4.xx")
Fixes: 874dfb65a484 ("net: stmmac: Add descriptor related callbacks for XGMAC2")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Currently the Rx DMA-descriptors mode setup is partly broken. First of all
the ring mode works for the Enhanced descriptors only. As long as the
Normal descriptors support is detected enabling the ring descriptors mode
will cause malfunctioned over 2K frames reception. This happens due to the
ring-mode init procedure originally being developed for the 8KB buffers
specific for the Enhanced descriptors. But since the commit 286a83721720
("stmmac: add CHAINED descriptor mode support (V4)") it has been converted
to be available for both Normal and Enhanced descriptor types without
taking into account that the former descriptors type has the buffer of 2KB
size.
Secondly the commit e3ad57c96715 ("stmmac: review RX/TX ring management")
has broken the Rx-descriptors chain and extended ring pointers
re-initialization in case of IEEE 1833-2002 Timestamp-ing. The
Rx-descriptor refill procedures used to rely on the
stmmac_rx_queue::dirty_rx field state to re-init the pointers, but since
the blamed commit the field state has no longer being advanced in sync
with the descriptors cleanups. It's set only after all the pending Rx
DMA-descriptors have been cleaned up. So the intermediate descriptors will
have the chain broken, meanwhile the first dirty one will point to the
first non-dirty. This is definitely wrong.
Let's fix the problems denoted above at once since both of them concern
the same part of driver code. The solution is simple - just move the chain
and ring mode initialization procedure to the dedicated
stmmac_desc_ops::prepare_rx_desc() methods and make sure they are called
at the moments where the Rx DMA-descriptors initialization and
re-initialization is required.
Note as a side-effect of the this change another problem is fixed. The
commit bba2556efad6 ("net: stmmac: Enable RX via AF_XDP zero-copy") has
missed the chain and ring mode pointers initialization on zero-copy
Rx-buffers allocation. Now by calling the stmmac_prepare_rx_desc() method
it has that done.
Moreover as a result of the denoted conversion there is no longer need in
the stmmac_mode_ops-related infrastructure. So it has been fully dropped.
Fixes: 286a83721720 ("stmmac: add CHAINED descriptor mode support (V4)")
Fixes: e3ad57c96715 ("stmmac: review RX/TX ring management")
Fixes: bba2556efad6 ("net: stmmac: Enable RX via AF_XDP zero-copy")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Currently the Rx-buffer is selected based on the interface MTU and is
limited to one of the next sizes in the stmmac_set_bfsize() method:
BUF_SIZE_16KiB 16368 (0x3FF0)
BUF_SIZE_8KiB 8188 (0x1FFC)
BUF_SIZE_4KiB 4096 (0x1000)
BUF_SIZE_2KiB 2048 (0x800)
DEFAULT_BUFSIZE 1536 (0x600)
In the meantime the MTU can be no greater than 16368, 9000 or PAGE_SIZE.
There are several problems in that.
First of all the 8K and 16K sizes are too big for the DW GMAC with Normal
descriptor and with Chained Enhanced descriptors. Here is the max size of
the Rx buffers depending on the DW MAC version:
DW GMAC Normal Rx descriptor: 2K (chain), 2x2K (ring)
DW GMAC Enhanced Rx descriptor: 8K (chain), 2x8K (ring)
DW QoS Ethernet: 16K (ring)
DW XGMAC/XLGMAC: 16k (ring)
This has been like that for the Normal descriptors since the initial
driver commit and for the Chained Enhanced Descriptors since the
b2f3a481c4cd ("net: stmmac: Enable 16KB buffer size"). Moreover the later
commit made the stmmac_desc_ops::set_16kib_bfsize() callbacks useless
since stmmac_set_bfsize() may setup the 16KiB buffer size anyway.
Second is more series problem. The buffer size is determined purely based
on MTU with no Ethernet header taken into account. That will definitely
cause problems should MTU be specified close to the threshold values listed
above. There will be no room for the entire MTU-size frames and they will
be just discarded.
Thirdly the buffer size alignment is kind of random. It's 16 bytes for the
BUF_SIZE_16KiB buffer size, 4 bytes - for the BUF_SIZE_8KiB buffer size,
and 4K/2K/1536 bytes - for the rest of the cases. This is also problematic
since based on the databooks the "Rx buffer size must be a multiple of 4,
8 or 16 depending on the bus widths, otherwise the transfer might result
into undefined behavior." As you can see currently the requirement isn't
fully fulfilled since the bus-width isn't taken into account in the driver
at all, and the BUF_SIZE_16KiB size only align to the widest data-bus
width. Moreover the BUF_SIZE_4KiB and BUF_SIZE_2KiB buf sizes exceed the
Normal Descriptor buf size constraint and being converted to just 1 byte
aligned sizes, which definitely violate the requirement denoted earlier.
So let's fix all the problems above at once since each of them concern the
same part of the code in the driver and fixing one would require fixing
another. For that to be properly done the set_16kib_bfsize() method
must be replaced with stmmac_desc_ops::get_rx_len() since the buffer size
depends on the descriptor format and the descriptors linkage mode. Then
to be on a safe side the strictest alignment must be applied during the
buffer size calculation which must also take into account the Ethernet
header size.
Note as a side-effect of this change the MTU alignment will be no longer
needed, which basically means to revert a part of the commit eaf4fac47807
("net: stmmac: Do not accept invalid MTU values"). Not that it has been
proper implemented there anyway since the alignment requirement is
applicable to the Rx buffer size only (as noted above in the log message).
Fixes: b2f3a481c4cd ("net: stmmac: Enable 16KB buffer size")
Fixes: 47dd7a540b8a ("net: add support for STMicroelectronics Ethernet controllers.")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
As soon as the Jumbo frames with MTU over 8K enabled it's possible to have
a garbage or truncated frame transmitted to a recipient. The problem is
multi-dimensional and mainly connected with over-complicated and weakly
tested DW GMAC Jumbos implementation.
First if all let's clarify the DW GMAC descriptors capabilities crucial
for the denoted problems. DW MAC and GMAC support two types of the
DMA-descriptors Normal and Enhanced - the former descriptor can have up to
two 2K buffers attached, meanwhile the later one can be initialized with
up to two 8K buffers. The DMA-descriptors can be allocated either as a
continuous array of descriptors (ring mode) or as a set of individually
linked descriptors (chain mode). All of that affects the size of the
buffers support by each descriptor since in the chain mode the descriptor
field responsible for the buffer2 pointer is utilized for linking the
adjacent descriptors. Thus here is the maximum buffers size
Normal Tx descriptor: 2K (chain), 2x2K (ring)
Enhanced Tx descriptor: 8K (chain), 2x8K (ring)
The main problem happens when an SKB with a Jumboed fragment is being
transmitted. The stmmac_xmit() method allocates only one descriptor in for
each segment that case. So if it's a Chained Enhanced descriptor then over
8K fragment just won't fit in there and will be simply truncated. If it's
a Ringed Enhanced Descriptor then the buffers length will be properly
initialized by the enh_desc_prepare_tx_desc() method, but the descriptor
won't have the buffer2 pointer field initialized and a garbage from the
DMA address 0x0 will be transmitted.
The same problem happens for the Normal descriptors except that it happens
for over 2K fragments. Moreover SKBs with Jumboed head won't be properly
handled for the Ringed Normal Descriptors mode either. See the buffer2
offset utilized in the stmmac_jumbo_frm() method for the buffer2 pointer.
It's always 4K meanwhile the buffer size in this case is 2K. Thus some
data will be missed in the sent out frame.
So to speak the Jumbos support is almost completely broken for DW
MAC/GMAC. The only working case is when an SKB with Jumboed head is
submitted and the Enhanced Tx descriptor is utilized.
Let's fix the denoted problems. But instead of fixing each part of the
affected code and making it even more complicated let's do that in a more
elegant way. The jumbos support can be significantly simplified by
implementing a allocation-based pattern already developed for the TSO
feature. The only difference is that a simple
stmmac_desc_ops::prepare_tx_desc() method will be utilized for the
Tx DMA-descriptors initialization. Thus a lot of redundant code in the
chain_mode.c/ring_mode.c modules will be replaced with a single
stmmac_tx_allocator() method utilized for the DMA-descriptors allocation
for all SKB head and fragments.
As a nice side-effect of this change the stmmac_xmit() and
stmmac_tso_xmit() methods now look very much similar which makes the code
much more readable and maintainable.
Fixes: 286a83721720 ("stmmac: add CHAINED descriptor mode support (V4)")
Fixes: 47dd7a540b8a ("net: add support for STMicroelectronics Ethernet controllers.")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
Note after this change there is no longer need in the GMAC max MTU 9K
constraint for Tx. It can be up to 16K from now. The only thing left to
fix is the Rx path. Alas it will be harder to implement.
|
|
It's possible to have the DMA-descriptors linked with each other on the
DW GMAC IP-cores. It's called the chain mode, when a DMA-descriptor refers
to the next one by a pointer. But in case of the IEEE 1588-2002 Timestamp
support the chain will be broken by the DMA-engine writing the outbound
timestamp to the last descriptor of the transferred frame. So the
descriptors chain must be restored afterwards in that case.
The restoration has been implemented by means of the
stmmac_mode_ops::clean_desc3() function called in the framework of the
stmmac_tx_clean() method responsible for the Tx DMA-descriptors
preparation for the next transfers. But the procedure has been broken in
e3ad57c96715 ("stmmac: review RX/TX ring management"). The clean_desc3()
method relies on the stmmac_tx_queue::dirty_rx field state to
re-initialize the respective descriptor and find the next one. But the
dirty Tx-pointer now is advanced only after all the Tx-descriptors are
cleaned up and moved right to the current Tx-pointer. So the intermediate
descriptors will be left with the chain broken, meanwhile the first dirty
one will point to the first non-dirty. This is definitely wrong.
Let's fix that problem. But instead of either getting back the Tx
DMA-descriptors cleanup loop semantics or fixing the clean_desc3() method
semantics it's better to optimize the chain mode initialization and
restoration procedures. It can be done by moving the chain-mode
initialization to the stmmac_desc_ops::release_tx_desc() method. So after
doing that the Tx DMA-descriptors chaining will be localized in a single
method and the problem described above will be fixed since the next
descriptor pointer will be passed noew to the release_tx_desc() function.
Note the chain-mode flag has been initialized in that function even before
this change.
Fixes: e3ad57c96715 ("stmmac: review RX/TX ring management")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Currently the FCS field stripping is done purely by software. That is the
driver just reduces the frame length by the FCS size if NETIF_F_RXFCS
feature isn't enabled. In the meantime the Pad/FCS stripping can be done
by the hardware itself thus speeding up the incoming traffic processing a
bit.
The Auto Pad/FCS stripping can be enabled by means of the ACS and CST
flags in the MAC-control register of all the DW MAC IP-cores. The former
one enables the Pad/FCS stripping from the Ethernet 802.3 frames
(Length/Type field is less than 1536). The later flag switches on just the
CRC stripping for the Ethernet II type frames (Length/Type field is
greater or equal to 1536).
Ideally both flags could have been enabled to implement the denoted
feature and be done with it. But the problems are that first the IP-cores
prior DW GMAC v3.50a version don't support CST flag and second the ACS
flag causes all pads and FCS truncation on transferring frames from RPE to
RFC module up to the _length specified in the Ethertype-field_. If first
problem can be quite easy handled in the driver, the second one can't be
because it corrupts the DSA frames which have tags placed at the
Ethertype-field and looking like a very short Ethernet 802.3 frame for the
RPE module of DW MACs. That's why all of the previous commits trying to
fix the problem couldn't do that properly. The best solution would be to
just switch on CST and never enable the ACS flag on.
As a final and the most appropriate solution let's add the HW-accelerated
FCS stripping support for the Ethernet II frames only. It will be enabled
unless NETIF_F_RXFCS feature is request for DW GMAC v3.50a and higher, DW
QoS Ether and DW XGMAC/XLGMAC IP-cores.
Fixes: 929d43421ee5 ("net: stmmac: Disable automatic FCS/Pad stripping")
Fixes: 8cad443eacf6 ("net: stmmac: Fix reception of Broadcom switches tags")
Fixes: 3eeb29972b11 ("stmmac: fix automatic PAD/FCS stripping")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
The HW-accelerated FCS/Pad stripping was removed in the commit
929d43421ee5 ("net: stmmac: Disable automatic FCS/Pad stripping"). Since
then the FCS/Pad cutting of has been done by the driver itselft and for
all incoming traffic. Due to that it will be quite easy to add the
NETIF_F_RXFCS feature support - just convert the FCS stripping off code to
being conditionally called. While at it move it out of the
stmmac_rx()/stmmac_rx_zc() methods to the buffers length calculation
functions to reduce the amount of the conditionals and to simplify the Rx
methods.
Also make sure the FCS trimming is applied to the very last Rx DMA-buffer.
That is in case if Split Packet Header feature enabled and no header
splitting happened both buffers can be utilized for the data reception.
Thus the second buffer length must be trimmed of FCS field on that
occasion.
Of course the NETIF_F_RXALL feature will be disabled by default and will
be enabled upon the user request.
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
All except the ancient DW MAC100 controllers support permitting the
erroneous frames to be delivered to the system memory instead of
discarding them. The only that needs to be done is to fix the Rx
DMA-descriptor status getters to return the error status and to enable the
controller' DMA to stop dropping such the frames. It's done by setting up
the flags DMA_CONTROL.DT (disable dropping the TCP/UDP/IP frames with
incorrect csum), DMA_CONTROL.FEF (forward frames with CRC error,
collision error, GMII_ER, giant frame, watchdog timeouted, or cut off due
to the FIFO overflow), DMA_CONTROL.FUF (forward undesized good frames).
Of course the NETIF_F_RXALL feature will be disabled by default and will
be enabled upon the user request by the setting up the denoted flags in
the net_device::ndo_set_features() callback.
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
The DMA_CONTROL.DGF (drop giant frames) flag setup will be added later.
Until then the Giant frames won't dropped in anyway unless they exceed
the critical size causing the WDT-based truncation.
|
|
Currently the IP checksum insertion on Tx is implied to be performed by HW
for any IPv4/IPv6 network protocol types. It's definitely incorrect since
based on [1,2,3] Tx COE supports the TCP/UDP/ICMP payloads only. Moreover
the IPv6 extension headers are limited to be a single Hop-by-Hop and
multiple Destination headers. From that perspective the commit
c2945c435c99 ("net: stmmac: Prevent DSA tags from breaking COE") wasn't
fully correct in providing just IP-type constraint.
Let's fix the problem by providing a comprehensive method which would make
sure that the passed frame can be checksummed by the controller Tx
Checksum Offload Engine.
[1] DesignWare Cores Ethernet MAC Universal Databook, Revision 3.73a,
October 2013, p. 187.
[2] DesignWare Cores Ethernet Quality-of-Service Databook, Revision 5.20a,
April 2020, pp. 366.
[3] DesignWare Cores XGMAC - 10G Ethernet MAC Databook, Revision 3.20a,
September 2022, p.251.
Fixes: c2945c435c99 ("net: stmmac: Prevent DSA tags from breaking COE")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Currently the IP checksum verification status is incorrectly set to
CHECKSUM_UNNECESSARY for all IPv4/IPv6 packets as per the
stmmac_has_ip_ethertype() method implementation. It's wrong since the Rx
COE embedded into the DW *MAC IP-cores supports the TCP/UDP/ICMP protocols
only [1,2,3,4]. From that perspective the commit c2945c435c99 ("net:
stmmac: Prevent DSA tags from breaking COE") was close to solving the
denoted problem, but didn't provide a comprehensive solution.
Since all the low-level driver sub-modules now support reporting the CSUM
verification status from Rx DMA-descriptor let's use it to correctly set
the IP-sum status in the Rx-frames SKB.
[1] DesignWare Cores Ethernet MAC Universal Databook, Revision 3.50a,
February 2009, p. 114.
[2] DesignWare Cores Ethernet MAC Universal Databook, Revision 3.73a,
October 2013, p. 189.
[3] DesignWare Cores Ethernet Quality-of-Service Databook, Revision 5.20a,
April 2020, pp. 368.
[4] DesignWare Cores XGMAC - 10G Ethernet MAC Databook, Revision 3.20a,
September 2022, p.253.
[n] ...
Fixes: c2945c435c99 ("net: stmmac: Prevent DSA tags from breaking COE")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Currently the stmmac_desc_ops::rx_status() method always returns "Good
frame" status for the IP-packets with erroneous checksum. Here is what DW
QoS Eth databook says about the Rx COE flags in the Rx DMA-descriptor:
"RDES0.IPCE IP Payload Error ...
Bit 15 (ES) of RDES3 is not set when this bit is set." [1]
Thus the "Discard frame" status won't be returned from the
stmmac_desc_ops::rx_status() method for the frames failed to pass the
control-sum check. Instead such packets will be passed to the upper
software layers with the "checksum unnecessary" status, which makes the
upper layer being unaware of the packets inconsistency. This is definitely
a bug.
Moreover the "Good frame" is returned for the unverified frames too
(unsupported by the Rx Checksum Offload engine). This isn't that much
problematic as the bug but generally speaking this is also incorrect
because it doesn't give an opportunity for the upper software layer to
detect the IP csum status for sure. Instead the STMMAC core driver has to
implement an heuristic method like stmmac_has_ip_ethertype() which is not
only incomplete but also causes a false-positive CSUM verification status
passed up to the networking core.
Let's fix all of the problems above in the DW QoS Eth (GMAC4+) as a
preparation before a complete fix of the commit c2945c435c99 ("net:
stmmac: Prevent DSA tags from breaking COE") is provided.
Fixes: 753a71090f33 ("stmmac: add descriptors function for GMAC 4.xx")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Currently the stmmac_desc_ops::rx_status() method always returns "Good
frame" status for any non-erroneous frame including the one for which the
IP-checksums weren't verified. Generally speaking this isn't right because
it doesn't give an opportunity for the upper software layer to detect the
IP csum status for sure. Instead the STMMAC core driver has to implement
an heuristic method like stmmac_has_ip_ethertype() which is not only
incomplete but also causes a false-positive CSUM verification status
passed up to the networking core.
Let's fix that in the DW XGMAC module as a preparation before a complete
fix of the commit c2945c435c99 ("net: stmmac: Prevent DSA tags from
breaking COE") is provided.
Fixes: 874dfb65a484 ("net: stmmac: Add descriptor related callbacks for XGMAC2")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Indeed. The commit c24602ef8664 ("stmmac: support extend descriptors")
added the Extended Enhanced Descriptor support to the STMMAC driver. The
problem is that it was done in a nominal way, so the extended status
actually reflecting for instance the Rx COE type 2 state has been
merely utilized to collect for the incoming traffic statistics.
The problem was that even though the driver claimed to support the most
modern DW GMAC IP-cores (up to v3.73a) with the Extended Enhanced DMA
descriptor in fact it didn't giving an expression that the driver works
well in the Rx COE v2 status handling. Due to that the important status
flags (like csum_none, etc) will be missed in the STMMAC core driver.
Let's fix the inconsistency by taking into account the Rx COE v2 status
in the Extended Enhanced Descriptor when it's relevant: DW GMAC IP-core
higher than v3.50a and there is the Advanced Time Stamp or Rx COE v2
features.
Fixes: c24602ef8664 ("stmmac: support extend descriptors")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Rx Checksum Offload Engine has been available in the DW GMAC IP-core since
v3.30a release. Back then the IP-core already provided two types of the
DMA-descriptors to indicate the results of the IP header and payload
checksumming: normal and enhanced. The problem was that both of these
descriptor types didn't have free flags for that. So the engineers
invented the RDES0.{0,5,7} bits permutation (the bits originally
indicating the least important statuses) so if the Rx COE type 2 was
available their semantics was changed to indicate the results of the
engine work. The bits state parsing was developed in the
enh_desc_coe_rdes0() method.
The situation changed in the DW GMAC v3.50a IP-core. Since that release
the controller has supported the _extended_ version of the enhanced
descriptor. It meant that if Rx COE type 2 (so called Full Checksum
Offload Engine) or Advanced Timestamp features were enabled in the
IP-core, the descriptor can have been optionally extended with four more
dwords containing the features status. Due to that the RDES0.{0,5,7} bits
permutation was no required and was dropped in the v3.50a IP-core release.
There wouldn't have been a problem in the situation described above if the
STMMAC driver was left supporting the STM GMAC onlu. But since the commit
84c9f8c41df9 ("net: stmmac: Add ip version to dts bindings") the driver
was declared to support the generic DW GMAC IP-cores which was wrong
without properly implementing the specifics of the newer GMAC revisions.
Let's fix that by parsing the permuted RDES0.{0,5,7} bits on the DW GMACs
earlier than v3.50a. The Rx COE type 2 status parsing of the extended
enhanced descriptor will be fixed later.
The reasonable question is how come there has been no problem officially
spotted so far? Well, most likely that's because the RDES0.{0,5,7} bits
semantics in the extended enhanced descriptor didn't cause any traffic
loses when parsed in the enh_desc_coe_rdes0() method. For instance the
most frequently seen statuses 0x5/0x4 just means Ethernet-II frame
with/without the extended status available in the RDES4-RDES7 dwords.
[1] DesignWare Cores Ethernet MAC Universal Databook, Revision 3.50a,
February 2009, p. 113.
[2] DesignWare Cores Ethernet MAC Universal Databook, Revision 3.73a,
October 2013, p. 188.
[3] Link: https://lore.kernel.org/netdev/20240111-prevent_dsa_tags-v5-1-63e795a4d129@bootlin.com
Fixes: 84c9f8c41df9 ("net: stmmac: Add ip version to dts bindings")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
If NETIF_F_RXCSUM feature is user-disabled and the network interface is
cycled down/up, then the feature will be completely off and couldn't be
re-enabled until the driver or system is reloaded. This happens due to the
way the runtime-check has been implemented in the stmmac_hw_setup()
method.
See if the stmmac_rx_ipc() method couldn't enable the Checksum Offload
engine (which happens for sure if the feature was disabled by the user),
then driver assumes that no Rx COE detected and clears out all the flags
(mac_device_info::rx_csum and plat_stmmacenet_data::rx_coe) indicating the
engine availability. As such the user won't be able to enable feature
anymore since the ndo_set_feature() callback will constantly call disable
Rx COE (call stmmac_rx_ipc() with mac_device_info::rx_csum cleared)
despite of the NETIF_F_RXCSUM flag state.
Let's fix that by re-developing the RXCSUM feature support a bit. First
let's runtime check the RxCOE availability only once during the net-device
registration. It can be done because the run-time check on each
device-open has turned to be pointless since the blamed commit if the
feature isn't declared to be available. Second let's drop the
mac_device_info::rx_csum and always use the NETIF_F_RXCSUM flag state to
determined the Rx COE feature state since the driver won't assume anymore
that the feature might be somehow failed to be enabled.
Fixes: d2afb5bdffde ("stmmac: fix the rx csum feature")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
Note the problem could be fixed in a less invasive way but the suggested
solution makes the RXCSUM feature implementation looking cleaner and more
maintainable.
|
|
Here is what the DW GMAC databooks say about the Rx COE type 1 abilities:
"The application can enable IPv4 header checksum _checking_ and TCP/UDP
checksum _offload_ by setting the GMAC Configuration register’s IPC bit.
This module calculates the 16-bit ones’ complement of the Ethernet frame’s
payload data’s (DATA field) ones’ complement sum. ..." [1,2,3]
Basically it means that the engine _verifies_ the IPv4 header checksum
only and just calculates the 16-bit one's complement sum of the IPv4
datagrams for TCP/UDP protocols. The calculated value is just appended to
the frame pushed up to the system memory, which the driver just _ignores_.
The problem is that despite of all that the NETIF_F_RXCSUM feature is set
for Rx COE type 1 and the packets Rx procedure reports that the all the
IPv4 payloads csum is verified. It's definitely wrong.
In order to fix that let's just drop the entire Rx COE type 1 support
since it has never been properly handled in the driver anyway and most
importantly has been declared as legacy since DW GMAC v3.30a release [2,3]
claiming that the Type 2 engine is generally preferable. Regarding the
IPv4 header checking which is claimed to be performed by the type 1
engine, it's performed by the network core subsystem anyway.
Note originally, prior the commit 38912bdbde5f ("stmmac: sanitize the rx
coe and add the type-1 csum (v2)"), the driver had had the Rx COE type 2
support only [4]. So the implemented solution means to basically revert
the blamed commit provided never actually properly working change.
[1] DesignWare Cores Ethernet MAC Universal Databook, Revision 3.20a,
August 2003, p. 85.
[2] DesignWare Cores Ethernet MAC Universal Databook, Revision 3.50a,
February 2009, p. 114.
[3] DesignWare Cores Ethernet MAC Universal Databook, Revision 3.73a,
October 2013, p. 188.
[4] Link: https://lore.kernel.org/netdev/4F7A9D7E.8080007@st.com/
Fixes: 38912bdbde5f ("stmmac: sanitize the rx coe and add the type-1 csum (v2)")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Since recent commits the Split Payload-Header feature is no longer
available if no relevant dependencies are provided, like Rx CoE feature.
As such let's drop the no longer relevant code and redundant field from
the device private date:
1. stmmac_priv::sph_cap field has been redundant in the first place, since
it just reflects the SPH feature availability and could be replaced with a
one more conditional statement in the XDP sub-module.
2. (priv->hw.rx_csum > 0) check is no longer required since Rx CoE won't
be switchable if the SPH feature is activated.
3. SPH-enable code can be freely dropped from the ndo_set_features()
callback since the SPH-feature enable/disable is performed in the device
open/close cycle in the stmmac_hw_setup() method.
All of the above shall greatly simplify the SPH feature related code.
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
The driver permits toggling the NETIF_F_RXCSUM feature flag by the system
user requests. In the meantime the SPH feature fully depends on the Rx
Checksum Offload Engine to "determine the packet type" and respectively
split header and payload. If the packet header is detached then the
payload will be delivered by means of the secondary buffer of the current
and subsequent descriptors. If the header couldn't be split, then all the
buffers will be utilized for the packets reception. [1] The later case isn't
implemented in the driver. So the Jumbo-frames received by means of
greater than one descriptor won't work.
Let's fix that in simple way just by preventing the Rx COE feature
disabling in case if the Split Packet Header feature is enabled.
[1] DesignWare Cores XGMAC - 10G Ethernet MAC Databook, Revision 3.20a,
September 2022, p.1662.
Fixes: 67afd6d1cfdf ("net: stmmac: Add Split Header support and enable it in XGMAC cores")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
Currently the Receive Side Scaling feature is always enabled if detected.
This isn't that suitable especially seeing it's mutually exclusive with
the L3/L4 filters. So let's make the driver to be more flexible by
implementing the code switching the RSS feature on/off based on the
NETIF_F_RXHASH feature flag state.
It's not that much problematic. First the stmmac_ops::rss_configure()
callback must be fixed to accept the feature enable/disable flag. Second
the callback must be called in the net_device_ops::ndo_set_features()
method with the feature state flag passed to enable/disable feature per
the system user request. (Note calling rss_configure() from the
ndo_set_features() method has been pointless before this change since
RSS feature has never been switchable.) Thirdly the NETIF_F_RXHASH flag
must be set in the net_device::hw_features field so to be available for
the user to change. Finally the selftest code must be aligned with the
main changes provided described above.
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
|
The driver permits toggling the NETIF_F_RXCSUM feature flag by the system
user requests. In the meantime the RSS feature fully depends on the Rx
Checksum Offload Engine to "parse the received packets and extract two or
four L3/L4 tuples from them" [1]. So RSS won't work without Rx COE being
enabled. Let's fix that by implementing the respective dependencies in the
ndo_fix_features()/ndo_set_features() network device callbacks.
[1] DesignWare Cores XGMAC - 10G Ethernet MAC Databook, Revision 3.20a,
September 2022, p.263.
Fixes: 76067459c686 ("net: stmmac: Implement RSS and enable it in XGMAC core")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
|
