summaryrefslogtreecommitdiff
path: root/drivers/net/ethernet/freescale
AgeCommit message (Collapse)AuthorFilesLines
2023-09-19net: enetc: use EXPORT_SYMBOL_GPL for enetc_phc_indexChristoph Hellwig1-1/+1
commit 569820befb16ffc755ab7af71f4f08cc5f68f0fe upstream. enetc_phc_index is only used via symbol_get, which was only ever intended for very internal symbols like this one. Use EXPORT_SYMBOL_GPL for it so that symbol_get can enforce only being used on EXPORT_SYMBOL_GPL symbols. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-06-21net: enetc: correct the indexes of highest and 2nd highest TCsWei Fang1-2/+2
[ Upstream commit 21225873be1472b7c59ed3650396af0e40578112 ] For ENETC hardware, the TCs are numbered from 0 to N-1, where N is the number of TCs. Numerically higher TC has higher priority. It's obvious that the highest priority TC index should be N-1 and the 2nd highest priority TC index should be N-2. However, the previous logic uses netdev_get_prio_tc_map() to get the indexes of highest priority and 2nd highest priority TCs, it does not make sense and is incorrect to give a "tc" argument to netdev_get_prio_tc_map(). So the driver may get the wrong indexes of the two highest priotiry TCs which would lead to failed to set the CBS for the two highest priotiry TCs. e.g. $ tc qdisc add dev eno0 parent root handle 100: mqprio num_tc 6 \ map 0 0 1 1 2 3 4 5 queues 1@0 1@1 1@2 1@3 2@4 2@6 hw 1 $ tc qdisc replace dev eno0 parent 100:6 cbs idleslope 100000 \ sendslope -900000 hicredit 12 locredit -113 offload 1 $ Error: Specified device failed to setup cbs hardware offload. ^^^^^ In this example, the previous logic deems the indexes of the two highest priotiry TCs should be 3 and 2. Actually, the indexes are 5 and 4, because the number of TCs is 6. So it would be failed to configure the CBS for the two highest priority TCs. Fixes: c431047c4efe ("enetc: add support Credit Based Shaper(CBS) for hardware offload") Signed-off-by: Wei Fang <wei.fang@nxp.com> Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2023-05-30net: fec: Better handle pm_runtime_get() failing in .remove()Uwe Kleine-König1-4/+11
[ Upstream commit f816b9829b19394d318e01953aa3b2721bca040d ] In the (unlikely) event that pm_runtime_get() (disguised as pm_runtime_resume_and_get()) fails, the remove callback returned an error early. The problem with this is that the driver core ignores the error value and continues removing the device. This results in a resource leak. Worse the devm allocated resources are freed and so if a callback of the driver is called later the register mapping is already gone which probably results in a crash. Fixes: a31eda65ba21 ("net: fec: fix clock count mis-match") Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Link: https://lore.kernel.org/r/20230510200020.1534610-1-u.kleine-koenig@pengutronix.de Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
2023-05-17net: enetc: check the index of the SFI rather than the handleWei Fang1-1/+1
[ Upstream commit 299efdc2380aac588557f4d0b2ce7bee05bd0cf2 ] We should check whether the current SFI (Stream Filter Instance) table is full before creating a new SFI entry. However, the previous logic checks the handle by mistake and might lead to unpredictable behavior. Fixes: 888ae5a3952b ("net: enetc: add tc flower psfp offload driver") Signed-off-by: Wei Fang <wei.fang@nxp.com> Reviewed-by: Leon Romanovsky <leonro@nvidia.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2023-01-14net: add a helper to avoid issues with HW TX timestamping and SO_TXTIMEVladimir Oltean1-6/+2
[ Upstream commit 847cbfc014adafeac401e19e349b0fd524f201c3 ] As explained in commit 29d98f54a4fe ("net: enetc: allow hardware timestamping on TX queues with tc-etf enabled"), hardware TX timestamping requires an skb with skb->tstamp = 0. When a packet is sent with SO_TXTIME, the skb->skb_mstamp_ns corrupts the value of skb->tstamp, so the drivers need to explicitly reset skb->tstamp to zero after consuming the TX time. Create a helper named skb_txtime_consumed() which does just that. All drivers which offload TC_SETUP_QDISC_ETF should implement it, and it would make it easier to assess during review whether they do the right thing in order to be compatible with hardware timestamping or not. Suggested-by: Vinicius Costa Gomes <vinicius.gomes@intel.com> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net> Stable-dep-of: db0b124f02ba ("igc: Enhance Qbv scheduling by using first flag bit") Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-12-02net: enetc: preserve TX ring priority across reconfigurationVladimir Oltean3-11/+19
[ Upstream commit 290b5fe096e7dd0aad730d1af4f7f2d9fea43e11 ] In the blamed commit, a rudimentary reallocation procedure for RX buffer descriptors was implemented, for the situation when their format changes between normal (no PTP) and extended (PTP). enetc_hwtstamp_set() calls enetc_close() and enetc_open() in a sequence, and this sequence loses information which was previously configured in the TX BDR Mode Register, specifically via the enetc_set_bdr_prio() call. The TX ring priority is configured by tc-mqprio and tc-taprio, and affects important things for TSN such as the TX time of packets. The issue manifests itself most visibly by the fact that isochron --txtime reports premature packet transmissions when PTP is first enabled on an enetc interface. Save the TX ring priority in a new field in struct enetc_bdr (occupies a 2 byte hole on arm64) in order to make this survive a ring reconfiguration. Fixes: 434cebabd3a2 ("enetc: Add dynamic allocation of extended Rx BD rings") Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Reviewed-by: Alexander Lobakin <alexandr.lobakin@intel.com> Link: https://lore.kernel.org/r/20221122130936.1704151-1-vladimir.oltean@nxp.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-12-02net: enetc: cache accesses to &priv->si->hwVladimir Oltean3-48/+49
[ Upstream commit 715bf2610f1d1adf3d4f9b7b3dd729984ec4270a ] The &priv->si->hw construct dereferences 2 pointers and makes lines longer than they need to be, in turn making the code harder to read. Replace &priv->si->hw accesses with a "hw" variable when there are 2 or more accesses within a function that dereference this. This includes loops, since &priv->si->hw is a loop invariant. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org> Stable-dep-of: 290b5fe096e7 ("net: enetc: preserve TX ring priority across reconfiguration") Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-12-02net: enetc: manage ENETC_F_QBV in priv->active_offloads only when enabledVladimir Oltean2-4/+8
[ Upstream commit 32bf8e1f6fb9f6dc334b2b98dffc2e5dcd51e513 ] Future work in this driver would like to look at priv->active_offloads & ENETC_F_QBV to determine whether a tc-taprio qdisc offload was installed, but this does not produce the intended effect. All the other flags in priv->active_offloads are managed dynamically, except ENETC_F_QBV which is set statically based on the probed SI capability. This change makes priv->active_offloads & ENETC_F_QBV really track the presence of a tc-taprio schedule on the port. Some existing users, like the enetc_sched_speed_set() call from phylink_mac_link_up(), are best kept using the old logic: the tc-taprio offload does not re-trigger another link mode resolve, so the scheduler needs to be functional from the get go, as long as Qbv is supported at all on the port. So to preserve functionality there, look at the static station interface capability from pf->si->hw_features instead. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Reviewed-by: Claudiu Manoil <claudiu.manoil@nxp.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org> Stable-dep-of: 290b5fe096e7 ("net: enetc: preserve TX ring priority across reconfiguration") Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-11-16net: fman: Unregister ethernet device on removalSean Anderson1-0/+9
[ Upstream commit b7cbc6740bd6ad5d43345a2504f7e4beff0d709f ] When the mac device gets removed, it leaves behind the ethernet device. This will result in a segfault next time the ethernet device accesses mac_dev. Remove the ethernet device when we get removed to prevent this. This is not completely reversible, since some resources aren't cleaned up properly, but that can be addressed later. Fixes: 3933961682a3 ("fsl/fman: Add FMan MAC driver") Signed-off-by: Sean Anderson <sean.anderson@seco.com> Link: https://lore.kernel.org/r/20221103182831.2248833-1-sean.anderson@seco.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-11-10net: fec: fix improper use of NETDEV_TX_BUSYZhang Changzhong1-2/+2
[ Upstream commit 06a4df5863f73af193a4ff7abf7cb04058584f06 ] The ndo_start_xmit() method must not free skb when returning NETDEV_TX_BUSY, since caller is going to requeue freed skb. Fix it by returning NETDEV_TX_OK in case of dma_map_single() fails. Fixes: 79f339125ea3 ("net: fec: Add software TSO support") Signed-off-by: Zhang Changzhong <zhangchangzhong@huawei.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-11-03net: enetc: survive memory pressure without crashingVladimir Oltean1-0/+5
[ Upstream commit 84ce1ca3fe9e1249bf21176ff162200f1c4e5ed1 ] Under memory pressure, enetc_refill_rx_ring() may fail, and when called during the enetc_open() -> enetc_setup_rxbdr() procedure, this is not checked for. An extreme case of memory pressure will result in exactly zero buffers being allocated for the RX ring, and in such a case it is expected that hardware drops all RX packets due to lack of buffers. This does not happen, because the reset-default value of the consumer and produces index is 0, and this makes the ENETC think that all buffers have been initialized and that it owns them (when in reality none were). The hardware guide explains this best: | Configure the receive ring producer index register RBaPIR with a value | of 0. The producer index is initially configured by software but owned | by hardware after the ring has been enabled. Hardware increments the | index when a frame is received which may consume one or more BDs. | Hardware is not allowed to increment the producer index to match the | consumer index since it is used to indicate an empty condition. The ring | can hold at most RBLENR[LENGTH]-1 received BDs. | | Configure the receive ring consumer index register RBaCIR. The | consumer index is owned by software and updated during operation of the | of the BD ring by software, to indicate that any receive data occupied | in the BD has been processed and it has been prepared for new data. | - If consumer index and producer index are initialized to the same | value, it indicates that all BDs in the ring have been prepared and | hardware owns all of the entries. | - If consumer index is initialized to producer index plus N, it would | indicate N BDs have been prepared. Note that hardware cannot start if | only a single buffer is prepared due to the restrictions described in | (2). | - Software may write consumer index to match producer index anytime | while the ring is operational to indicate all received BDs prior have | been processed and new BDs prepared for hardware. Normally, the value of rx_ring->rcir (consumer index) is brought in sync with the rx_ring->next_to_use software index, but this only happens if page allocation ever succeeded. When PI==CI==0, the hardware appears to receive frames and write them to DMA address 0x0 (?!), then set the READY bit in the BD. The enetc_clean_rx_ring() function (and its XDP derivative) is naturally not prepared to handle such a condition. It will attempt to process those frames using the rx_swbd structure associated with index i of the RX ring, but that structure is not fully initialized (enetc_new_page() does all of that). So what happens next is undefined behavior. To operate using no buffer, we must initialize the CI to PI + 1, which will block the hardware from advancing the CI any further, and drop everything. The issue was seen while adding support for zero-copy AF_XDP sockets, where buffer memory comes from user space, which can even decide to supply no buffers at all (example: "xdpsock --txonly"). However, the bug is present also with the network stack code, even though it would take a very determined person to trigger a page allocation failure at the perfect time (a series of ifup/ifdown under memory pressure should eventually reproduce it given enough retries). Fixes: d4fd0404c1c9 ("enetc: Introduce basic PF and VF ENETC ethernet drivers") Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Reviewed-by: Claudiu Manoil <claudiu.manoil@nxp.com> Link: https://lore.kernel.org/r/20221027182925.3256653-1-vladimir.oltean@nxp.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-11-03net: fec: limit register access on i.MX6ULJuergen Borleis1-2/+44
[ Upstream commit 0a8b43b12dd78daa77a7dc007b92770d262a2714 ] Using 'ethtool -d […]' on an i.MX6UL leads to a kernel crash: Unhandled fault: external abort on non-linefetch (0x1008) at […] due to this SoC has less registers in its FEC implementation compared to other i.MX6 variants. Thus, a run-time decision is required to avoid access to non-existing registers. Fixes: a51d3ab50702 ("net: fec: use a more proper compatible string for i.MX6UL type device") Signed-off-by: Juergen Borleis <jbe@pengutronix.de> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Link: https://lore.kernel.org/r/20221024080552.21004-1-jbe@pengutronix.de Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-10-26net: fs_enet: Fix wrong check in do_pd_setupZheng Yongjun1-1/+1
[ Upstream commit ec3f06b542a960806a81345042e4eee3f8c5dec4 ] Should check of_iomap return value 'fep->fec.fecp' instead of 'fep->fcc.fccp' Fixes: 976de6a8c304 ("fs_enet: Be an of_platform device when CONFIG_PPC_CPM_NEW_BINDING is set.") Signed-off-by: Zheng Yongjun <zhengyongjun3@huawei.com> Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-09-28net: enetc: move enetc_set_psfp() out of the common enetc_set_features()Vladimir Oltean5-35/+44
[ Upstream commit fed38e64d9b99d65a36c0dbadc3d3f8ddd9ea030 ] The VF netdev driver shouldn't respond to changes in the NETIF_F_HW_TC flag; only PFs should. Moreover, TSN-specific code should go to enetc_qos.c, which should not be included in the VF driver. Fixes: 79e499829f3f ("net: enetc: add hw tc hw offload features for PSPF capability") Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Link: https://lore.kernel.org/r/20220916133209.3351399-1-vladimir.oltean@nxp.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-08-25fec: Fix timer capture timing in `fec_ptp_enable_pps()`Csókás Bence1-5/+1
commit 61d5e2a251fb20c2c5e998c3f1d52ed6d5360319 upstream. Code reimplements functionality already in `fec_ptp_read()`, but misses check for FEC_QUIRK_BUG_CAPTURE. Replace with function call. Fixes: 28b5f058cf1d ("net: fec: ptp: fix convergence issue to support LinuxPTP stack") Signed-off-by: Csókás Bence <csokas.bence@prolan.hu> Link: https://lore.kernel.org/r/20220811101348.13755-1-csokas.bence@prolan.hu Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2022-08-25dpaa2-eth: trace the allocated address instead of page structChen Lin1-2/+2
commit e34f49348f8b7a53205b6f77707a3a6a40cf420b upstream. We should trace the allocated address instead of page struct. Fixes: 27c874867c4e ("dpaa2-eth: Use a single page per Rx buffer") Signed-off-by: Chen Lin <chen.lin5@zte.com.cn> Reviewed-by: Ioana Ciornei <ioana.ciornei@nxp.com> Link: https://lore.kernel.org/r/20220811151651.3327-1-chen45464546@163.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2022-05-09net: fec: add missing of_node_put() in fec_enet_init_stop_mode()Yang Yingliang1-1/+1
[ Upstream commit d2b52ec056d5bddb055c8f21d7489a23548d0838 ] Put device node in error path in fec_enet_init_stop_mode(). Fixes: 8a448bf832af ("net: ethernet: fec: move GPR register offset and bit into DT") Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> Link: https://lore.kernel.org/r/20220426125231.375688-1-yangyingliang@huawei.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-04-27dpaa_eth: Fix missing of_node_put in dpaa_get_ts_info()Lv Ruyi1-2/+6
[ Upstream commit 1a7eb80d170c28be2928433702256fe2a0bd1e0f ] Both of of_get_parent() and of_parse_phandle() return node pointer with refcount incremented, use of_node_put() on it to decrease refcount when done. Reported-by: Zeal Robot <zealci@zte.com.cn> Signed-off-by: Lv Ruyi <lv.ruyi@zte.com.cn> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-04-13dpaa2-ptp: Fix refcount leak in dpaa2_ptp_probeMiaoqian Lin1-1/+3
[ Upstream commit 2b04bd4f03bba021959ca339314f6739710f0954 ] This node pointer is returned by of_find_compatible_node() with refcount incremented. Calling of_node_put() to aovid the refcount leak. Fixes: d346c9e86d86 ("dpaa2-ptp: reuse ptp_qoriq driver") Signed-off-by: Miaoqian Lin <linmq006@gmail.com> Link: https://lore.kernel.org/r/20220404125336.13427-1-linmq006@gmail.com Signed-off-by: Paolo Abeni <pabeni@redhat.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-04-08net: enetc: report software timestamping via SO_TIMESTAMPINGVladimir Oltean1-1/+4
[ Upstream commit feb13dcb1818b775fbd9191f797be67cd605f03e ] Let user space properly determine that the enetc driver provides software timestamps. Fixes: 4caefbce06d1 ("enetc: add software timestamping") Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Reviewed-by: Claudiu Manoil <claudiu.manoil@nxp.com> Link: https://lore.kernel.org/r/20220324161210.4122281-1-vladimir.oltean@nxp.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-03-16gianfar: ethtool: Fix refcount leak in gfar_get_ts_infoMiaoqian Lin1-0/+1
[ Upstream commit 2ac5b58e645c66932438bb021cb5b52097ce70b0 ] The of_find_compatible_node() function returns a node pointer with refcount incremented, We should use of_node_put() on it when done Add the missing of_node_put() to release the refcount. Fixes: 7349a74ea75c ("net: ethernet: gianfar_ethtool: get phc index through drvdata") Signed-off-by: Miaoqian Lin <linmq006@gmail.com> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com> Reviewed-by: Claudiu Manoil <claudiu.manoil@nxp.com> Link: https://lore.kernel.org/r/20220310015313.14938-1-linmq006@gmail.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-02-23dpaa2-eth: Initialize mutex used in one step timestamping pathRadu Bulie1-1/+1
commit 07dd44852be89386ab12210df90a2d78779f3bff upstream. 1588 Single Step Timestamping code path uses a mutex to enforce atomicity for two events: - update of ptp single step register - transmit ptp event packet Before this patch the mutex was not initialized. This caused unexpected crashes in the Tx function. Fixes: c55211892f463 ("dpaa2-eth: support PTP Sync packet one-step timestamping") Signed-off-by: Radu Bulie <radu-andrei.bulie@nxp.com> Reviewed-by: Ioana Ciornei <ioana.ciornei@nxp.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2022-02-16dpaa2-eth: unregister the netdev before disconnecting from the PHYRobert-Ionut Alexa1-2/+2
[ Upstream commit 9ccc6e0c8959a019bb40f6b18704b142c04b19a8 ] The netdev should be unregistered before we are disconnecting from the MAC/PHY so that the dev_close callback is called and the PHY and the phylink workqueues are actually stopped before we are disconnecting and destroying the phylink instance. Fixes: 719479230893 ("dpaa2-eth: add MAC/PHY support through phylink") Signed-off-by: Robert-Ionut Alexa <robert-ionut.alexa@nxp.com> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-01-27net/fsl: xgmac_mdio: Fix incorrect iounmap when removing moduleTobias Waldekranz1-1/+2
commit 3f7c239c7844d2044ed399399d97a5f1c6008e1b upstream. As reported by sparse: In the remove path, the driver would attempt to unmap its own priv pointer - instead of the io memory that it mapped in probe. Fixes: 9f35a7342cff ("net/fsl: introduce Freescale 10G MDIO driver") Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2022-01-27net/fsl: xgmac_mdio: Add workaround for erratum A-009885Tobias Waldekranz1-6/+19
commit 6198c722019774d38018457a8bfb9ba3ed8c931e upstream. Once an MDIO read transaction is initiated, we must read back the data register within 16 MDC cycles after the transaction completes. Outside of this window, reads may return corrupt data. Therefore, disable local interrupts in the critical section, to maximize the probability that we can satisfy this requirement. Fixes: d55ad2967d89 ("powerpc/mpc85xx: Create dts components for the FSL QorIQ DPAA FMan") Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2022-01-27fsl/fman: Check for null pointer after calling devm_ioremapJiasheng Jiang1-5/+16
[ Upstream commit d5a73ec96cc57cf67e51b12820fc2354e7ca46f8 ] As the possible failure of the allocation, the devm_ioremap() may return NULL pointer. Take tgec_initialization() as an example. If allocation fails, the params->base_addr will be NULL pointer and will be assigned to tgec->regs in tgec_config(). Then it will cause the dereference of NULL pointer in set_mac_address(), which is called by tgec_init(). Therefore, it should be better to add the sanity check after the calling of the devm_ioremap(). Fixes: 3933961682a3 ("fsl/fman: Add FMan MAC driver") Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-01-05fsl/fman: Fix missing put_device() call in fman_port_probeMiaoqian Lin1-5/+7
[ Upstream commit bf2b09fedc17248b315f80fb249087b7d28a69a6 ] The reference taken by 'of_find_device_by_node()' must be released when not needed anymore. Add the corresponding 'put_device()' in the and error handling paths. Fixes: 18a6c85fcc78 ("fsl/fman: Add FMan Port Support") Signed-off-by: Miaoqian Lin <linmq006@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-12-14net: fec: only clear interrupt of handling queue in fec_enet_rx_queue()Joakim Zhang2-1/+4
commit b5bd95d17102b6719e3531d627875b9690371383 upstream. Background: We have a customer is running a Profinet stack on the 8MM which receives and responds PNIO packets every 4ms and PNIO-CM packets every 40ms. However, from time to time the received PNIO-CM package is "stock" and is only handled when receiving a new PNIO-CM or DCERPC-Ping packet (tcpdump shows the PNIO-CM and the DCERPC-Ping packet at the same time but the PNIO-CM HW timestamp is from the expected 40 ms and not the 2s delay of the DCERPC-Ping). After debugging, we noticed PNIO, PNIO-CM and DCERPC-Ping packets would be handled by different RX queues. The root cause should be driver ack all queues' interrupt when handle a specific queue in fec_enet_rx_queue(). The blamed patch is introduced to receive as much packets as possible once to avoid interrupt flooding. But it's unreasonable to clear other queues'interrupt when handling one queue, this patch tries to fix it. Fixes: ed63f1dcd578 (net: fec: clear receive interrupts before processing a packet) Cc: Russell King <rmk+kernel@arm.linux.org.uk> Reported-by: Nicolas Diaz <nicolas.diaz@nxp.com> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> Link: https://lore.kernel.org/r/20211206135457.15946-1-qiangqing.zhang@nxp.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-12-08dpaa2-eth: destroy workqueue at the end of remove functionDongliang Mu1-0/+2
commit f4a8adbfe4841491b60c14fe610571e1422359f9 upstream. The commit c55211892f46 ("dpaa2-eth: support PTP Sync packet one-step timestamping") forgets to destroy workqueue at the end of remove function. Fix this by adding destroy_workqueue before fsl_mc_portal_free and free_netdev. Fixes: c55211892f46 ("dpaa2-eth: support PTP Sync packet one-step timestamping") Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-11-26net: dpaa2-eth: fix use-after-free in dpaa2_eth_removePavel Skripkin1-2/+2
[ Upstream commit 9b5a333272a48c2f8b30add7a874e46e8b26129c ] Access to netdev after free_netdev() will cause use-after-free bug. Move debug log before free_netdev() call to avoid it. Fixes: 7472dd9f6499 ("staging: fsl-dpaa2/eth: Move print message") Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-11-18net: enetc: unmap DMA in enetc_send_cmd()Tim Gardner1-7/+11
[ Upstream commit cd4bc63de774eee95e9bac26a565cd80e0fca421 ] Coverity complains of a possible dereference of a null return value. 5. returned_null: kzalloc returns NULL. [show details] 6. var_assigned: Assigning: si_data = NULL return value from kzalloc. 488 si_data = kzalloc(data_size, __GFP_DMA | GFP_KERNEL); 489 cbd.length = cpu_to_le16(data_size); 490 491 dma = dma_map_single(&priv->si->pdev->dev, si_data, 492 data_size, DMA_FROM_DEVICE); While this kzalloc() is unlikely to fail, I did notice that the function returned without unmapping si_data. Fix this by refactoring the error paths and checking for kzalloc() failure. Fixes: 888ae5a3952ba ("net: enetc: add tc flower psfp offload driver") Cc: Claudiu Manoil <claudiu.manoil@nxp.com> Cc: "David S. Miller" <davem@davemloft.net> Cc: Jakub Kicinski <kuba@kernel.org> Cc: netdev@vger.kernel.org Cc: linux-kernel@vger.kernel.org (open list) Signed-off-by: Tim Gardner <tim.gardner@canonical.com> Acked-by: Claudiu Manoil <claudiu.manoil@nxp.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-10-27net: enetc: fix ethtool counter name for PM0_TERRVladimir Oltean1-1/+1
[ Upstream commit fb8dc5fc8cbdfd62ecd16493848aee2f42ed84d9 ] There are two counters named "MAC tx frames", one of them is actually incorrect. The correct name for that counter should be "MAC tx error frames", which is symmetric to the existing "MAC rx error frames". Fixes: 16eb4c85c964 ("enetc: Add ethtool statistics") Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Reviewed-by: <Claudiu Manoil <claudiu.manoil@nxp.com> Link: https://lore.kernel.org/r/20211020165206.1069889-1-vladimir.oltean@nxp.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-10-06net: enetc: fix the incorrect clearing of IF_MODE bitsVladimir Oltean1-2/+1
[ Upstream commit 325fd36ae76a6d089983b2d2eccb41237d35b221 ] The enetc phylink .mac_config handler intends to clear the IFMODE field (bits 1:0) of the PM0_IF_MODE register, but incorrectly clears all the other fields instead. For normal operation, the bug was inconsequential, due to the fact that we write the PM0_IF_MODE register in two stages, first in phylink .mac_config (which incorrectly cleared out a bunch of stuff), then we update the speed and duplex to the correct values in phylink .mac_link_up. Judging by the code (not tested), it looks like maybe loopback mode was broken, since this is one of the settings in PM0_IF_MODE which is incorrectly cleared. Fixes: c76a97218dcb ("net: enetc: force the RGMII speed and duplex instead of operating in inband mode") Reported-by: Pavel Machek (CIP) <pavel@denx.de> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-09-30enetc: Fix uninitialized struct dim_sample field usageClaudiu Manoil1-1/+1
[ Upstream commit 9f7afa05c9522b086327929ae622facab0f0f72b ] The only struct dim_sample member that does not get initialized by dim_update_sample() is comp_ctr. (There is special API to initialize comp_ctr: dim_update_sample_with_comps(), and it is currently used only for RDMA.) comp_ctr is used to compute curr_stats->cmps and curr_stats->cpe_ratio (see dim_calc_stats()) which in turn are consumed by the rdma_dim_*() API. Therefore, functionally, the net_dim*() API consumers are not affected. Nevertheless, fix the computation of statistics based on an uninitialized variable, even if the mentioned statistics are not used at the moment. Fixes: ae0e6a5d1627 ("enetc: Add adaptive interrupt coalescing") Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-09-30enetc: Fix illegal access when reading affinity_hintClaudiu Manoil1-4/+1
[ Upstream commit 7237a494decfa17d0b9d0076e6cee3235719de90 ] irq_set_affinity_hit() stores a reference to the cpumask_t parameter in the irq descriptor, and that reference can be accessed later from irq_affinity_hint_proc_show(). Since the cpu_mask parameter passed to irq_set_affinity_hit() has only temporary storage (it's on the stack memory), later accesses to it are illegal. Thus reads from the corresponding procfs affinity_hint file can result in paging request oops. The issue is fixed by the get_cpu_mask() helper, which provides a permanent storage for the cpumask_t parameter. Fixes: d4fd0404c1c9 ("enetc: Introduce basic PF and VF ENETC ethernet drivers") Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-08-12net: fec: fix use-after-free in fec_drv_removePavel Skripkin1-1/+1
[ Upstream commit 44712965bf12ae1758cec4de53816ed4b914ca1a ] Smatch says: drivers/net/ethernet/freescale/fec_main.c:3994 fec_drv_remove() error: Using fep after free_{netdev,candev}(ndev); drivers/net/ethernet/freescale/fec_main.c:3995 fec_drv_remove() error: Using fep after free_{netdev,candev}(ndev); Since fep pointer is netdev private data, accessing it after free_netdev() call can cause use-after-free bug. Fix it by moving free_netdev() call at the end of the function Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Fixes: a31eda65ba21 ("net: fec: fix clock count mis-match") Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> Reviewed-by: Joakim Zhang <qiangqing.zhang@nxp.com> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-07-19net: fec: add ndo_select_queue to fix TX bandwidth fluctuationsFugang Duan1-0/+32
[ Upstream commit 52c4a1a85f4b346c39c896c0168f4a843b3385ff ] As we know that AVB is enabled by default, and the ENET IP design is queue 0 for best effort, queue 1&2 for AVB Class A&B. Bandwidth of each queue 1&2 set in driver is 50%, TX bandwidth fluctuated when selecting tx queues randomly with FEC_QUIRK_HAS_AVB quirk available. This patch adds ndo_select_queue callback to select queues for transmitting to fix this issue. It will always return queue 0 if this is not a vlan packet, and return queue 1 or 2 based on priority of vlan packet. You may complain that in fact we only use single queue for trasmitting if we are not targeted to VLAN. Yes, but seems we have no choice, since AVB is enabled when the driver probed, we can't switch this feature dynamicly. After compare multiple queues to single queue, TX throughput almost no improvement. One way we can implemet is to configure the driver to multiple queues with Round-robin scheme by default. Then add ndo_setup_tc callback to enable/disable AVB feature for users. Unfortunately, ENET AVB IP seems not follow the standard 802.1Qav spec. We only can program DMAnCFG[IDLE_SLOPE] field to calculate bandwidth fraction. And idle slope is restricted to certain valus (a total of 19). It's far away from CBS QDisc implemented in Linux TC framework. If you strongly suggest to do this, I think we only can support limited numbers of bandwidth and reject others, but it's really urgly and wried. With this patch, VLAN tagged packets route to queue 0/1/2 based on vlan priority; VLAN untagged packets route to queue 0. Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de> Reported-by: Frieder Schrempf <frieder.schrempf@kontron.de> Signed-off-by: Fugang Duan <fugang.duan@nxp.com> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-06-23net: fec_ptp: add clock rate zero checkFugang Duan1-0/+4
commit cb3cefe3f3f8af27c6076ef7d1f00350f502055d upstream. Add clock rate zero check to fix coverity issue of "divide by 0". Fixes: commit 85bd1798b24a ("net: fec: fix spin_lock dead lock") Signed-off-by: Fugang Duan <fugang.duan@nxp.com> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-06-23net: fec_ptp: fix issue caused by refactor the fec_devtypeJoakim Zhang1-3/+1
[ Upstream commit d23765646e71b43ed2b809930411ba5c0aadee7b ] Commit da722186f654 ("net: fec: set GPR bit on suspend by DT configuration.") refactor the fec_devtype, need adjust ptp driver accordingly. Fixes: da722186f654 ("net: fec: set GPR bit on suspend by DT configuration.") Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-06-03net: fec: fix the potential memory leak in fec_enet_init()Fugang Duan1-2/+9
[ Upstream commit 619fee9eb13b5d29e4267cb394645608088c28a8 ] If the memory allocated for cbd_base is failed, it should free the memory allocated for the queues, otherwise it causes memory leak. And if the memory allocated for the queues is failed, it can return error directly. Fixes: 59d0f7465644 ("net: fec: init multi queue date structure") Signed-off-by: Fugang Duan <fugang.duan@nxp.com> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-05-14net: enetc: fix link error againArnd Bergmann1-3/+1
[ Upstream commit 74c97ea3b61e4ce149444f904ee8d4fc7073505b ] A link time bug that I had fixed before has come back now that another sub-module was added to the enetc driver: ERROR: modpost: "enetc_ierb_register_pf" [drivers/net/ethernet/freescale/enetc/fsl-enetc.ko] undefined! The problem is that the enetc Makefile is not actually used for the ierb module if that is the only built-in driver in there and everything else is a loadable module. Fix it by always entering the directory this time, regardless of which symbols are configured. This should reliably fix the problem and prevent it from coming back another time. Fixes: 112463ddbe82 ("net: dsa: felix: fix link error") Fixes: e7d48e5fbf30 ("net: enetc: add a mini driver for the Integrated Endpoint Register Block") Signed-off-by: Arnd Bergmann <arnd@arndb.de> Acked-by: Vladimir Oltean <vladimir.oltean@nxp.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-04-14gianfar: Handle error code at MAC address changeClaudiu Manoil1-1/+5
[ Upstream commit bff5b62585123823842833ab20b1c0a7fa437f8c ] Handle return error code of eth_mac_addr(); Fixes: 3d23a05c75c7 ("gianfar: Enable changing mac addr when if up") Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-03-30net: enetc: set MAC RX FIFO to recommended valueAlex Marginean2-0/+8
[ Upstream commit 1b2395dfff5bb40228a187f21f577cd90673d344 ] On LS1028A, the MAC RX FIFO defaults to the value 2, which is too high and may lead to RX lock-up under traffic at a rate higher than 6 Gbps. Set it to 1 instead, as recommended by the hardware design team and by later versions of the ENETC block guide. Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com> Reviewed-by: Claudiu Manoil <claudiu.manoil@nxp.com> Reviewed-by: Jason Liu <jason.hui.liu@nxp.com> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-03-30gianfar: fix jumbo packets+napi+rx overrun crashMichael Braun1-0/+15
[ Upstream commit d8861bab48b6c1fc3cdbcab8ff9d1eaea43afe7f ] When using jumbo packets and overrunning rx queue with napi enabled, the following sequence is observed in gfar_add_rx_frag: | lstatus | | skb | t | lstatus, size, flags | first | len, data_len, *ptr | ---+--------------------------------------+-------+-----------------------+ 13 | 18002348, 9032, INTERRUPT LAST | 0 | 9600, 8000, f554c12e | 12 | 10000640, 1600, INTERRUPT | 0 | 8000, 6400, f554c12e | 11 | 10000640, 1600, INTERRUPT | 0 | 6400, 4800, f554c12e | 10 | 10000640, 1600, INTERRUPT | 0 | 4800, 3200, f554c12e | 09 | 10000640, 1600, INTERRUPT | 0 | 3200, 1600, f554c12e | 08 | 14000640, 1600, INTERRUPT FIRST | 0 | 1600, 0, f554c12e | 07 | 14000640, 1600, INTERRUPT FIRST | 1 | 0, 0, f554c12e | 06 | 1c000080, 128, INTERRUPT LAST FIRST | 1 | 0, 0, abf3bd6e | 05 | 18002348, 9032, INTERRUPT LAST | 0 | 8000, 6400, c5a57780 | 04 | 10000640, 1600, INTERRUPT | 0 | 6400, 4800, c5a57780 | 03 | 10000640, 1600, INTERRUPT | 0 | 4800, 3200, c5a57780 | 02 | 10000640, 1600, INTERRUPT | 0 | 3200, 1600, c5a57780 | 01 | 10000640, 1600, INTERRUPT | 0 | 1600, 0, c5a57780 | 00 | 14000640, 1600, INTERRUPT FIRST | 1 | 0, 0, c5a57780 | So at t=7 a new packets is started but not finished, probably due to rx overrun - but rx overrun is not indicated in the flags. Instead a new packets starts at t=8. This results in skb->len to exceed size for the LAST fragment at t=13 and thus a negative fragment size added to the skb. This then crashes: kernel BUG at include/linux/skbuff.h:2277! Oops: Exception in kernel mode, sig: 5 [#1] ... NIP [c04689f4] skb_pull+0x2c/0x48 LR [c03f62ac] gfar_clean_rx_ring+0x2e4/0x844 Call Trace: [ec4bfd38] [c06a84c4] _raw_spin_unlock_irqrestore+0x60/0x7c (unreliable) [ec4bfda8] [c03f6a44] gfar_poll_rx_sq+0x48/0xe4 [ec4bfdc8] [c048d504] __napi_poll+0x54/0x26c [ec4bfdf8] [c048d908] net_rx_action+0x138/0x2c0 [ec4bfe68] [c06a8f34] __do_softirq+0x3a4/0x4fc [ec4bfed8] [c0040150] run_ksoftirqd+0x58/0x70 [ec4bfee8] [c0066ecc] smpboot_thread_fn+0x184/0x1cc [ec4bff08] [c0062718] kthread+0x140/0x144 [ec4bff38] [c0012350] ret_from_kernel_thread+0x14/0x1c This patch fixes this by checking for computed LAST fragment size, so a negative sized fragment is never added. In order to prevent the newer rx frame from getting corrupted, the FIRST flag is checked to discard the incomplete older frame. Signed-off-by: Michael Braun <michael-dev@fami-braun.de> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-03-30net: fec: ptp: avoid register access when ipg clock is disabledHeiko Thiery1-0/+7
[ Upstream commit 6a4d7234ae9a3bb31181f348ade9bbdb55aeb5c5 ] When accessing the timecounter register on an i.MX8MQ the kernel hangs. This is only the case when the interface is down. This can be reproduced by reading with 'phc_ctrl eth0 get'. Like described in the change in 91c0d987a9788dcc5fe26baafd73bf9242b68900 the igp clock is disabled when the interface is down and leads to a system hang. So we check if the ptp clock status before reading the timecounter register. Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com> Acked-by: Richard Cochran <richardcochran@gmail.com> Link: https://lore.kernel.org/r/20210225211514.9115-1-heiko.thiery@gmail.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-03-17net: enetc: initialize RFS/RSS memories for unused ports tooVladimir Oltean3-9/+36
[ Upstream commit 3222b5b613db558e9a494bbf53f3c984d90f71ea ] Michael reports that since linux-next-20210211, the AER messages for ECC errors have started reappearing, and this time they can be reliably reproduced with the first ping on one of his LS1028A boards. $ ping 1[ 33.258069] pcieport 0000:00:1f.0: AER: Multiple Corrected error received: 0000:00:00.0 72.16.0.1 PING [ 33.267050] pcieport 0000:00:1f.0: AER: can't find device of ID0000 172.16.0.1 (172.16.0.1): 56 data bytes 64 bytes from 172.16.0.1: seq=0 ttl=64 time=17.124 ms 64 bytes from 172.16.0.1: seq=1 ttl=64 time=0.273 ms $ devmem 0x1f8010e10 32 0xC0000006 It isn't clear why this is necessary, but it seems that for the errors to go away, we must clear the entire RFS and RSS memory, not just for the ports in use. Sadly the code is structured in such a way that we can't have unified logic for the used and unused ports. For the minimal initialization of an unused port, we need just to enable and ioremap the PF memory space, and a control buffer descriptor ring. Unused ports must then free the CBDR because the driver will exit, but used ports can not pick up from where that code path left, since the CBDR API does not reinitialize a ring when setting it up, so its producer and consumer indices are out of sync between the software and hardware state. So a separate enetc_init_unused_port function was created, and it gets called right after the PF memory space is enabled. Fixes: 07bf34a50e32 ("net: enetc: initialize the RFS and RSS memories") Reported-by: Michael Walle <michael@walle.cc> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Tested-by: Michael Walle <michael@walle.cc> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-03-17enetc: Fix unused var build warning for CONFIG_OFArnd Bergmann1-11/+10
[ Upstream commit 4560b2a3ecdd5d587c4c6eea4339899f173a559a ] When CONFIG_OF is disabled, there is a harmless warning about an unused variable: enetc_pf.c: In function 'enetc_phylink_create': enetc_pf.c:981:17: error: unused variable 'dev' [-Werror=unused-variable] Slightly rearrange the code to pass around the of_node as a function argument, which avoids the problem without hurting readability. Fixes: 71b77a7a27a3 ("enetc: Migrate to PHYLINK and PCS_LYNX") Signed-off-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com> Link: https://lore.kernel.org/r/20201204120800.17193-1-claudiu.manoil@nxp.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
2021-03-17net: enetc: allow hardware timestamping on TX queues with tc-etf enabledVladimir Oltean1-0/+6
commit 29d98f54a4fe1b6a9089bec8715a1b89ff9ad59c upstream. The txtime is passed to the driver in skb->skb_mstamp_ns, which is actually in a union with skb->tstamp (the place where software timestamps are kept). Since commit b50a5c70ffa4 ("net: allow simultaneous SW and HW transmit timestamping"), __sock_recv_timestamp has some logic for making sure that the two calls to skb_tstamp_tx: skb_tx_timestamp(skb) # Software timestamp in the driver -> skb_tstamp_tx(skb, NULL) and skb_tstamp_tx(skb, &shhwtstamps) # Hardware timestamp in the driver will both do the right thing and in a race-free manner, meaning that skb_tx_timestamp will deliver a cmsg with the software timestamp only, and skb_tstamp_tx with a non-NULL hwtstamps argument will deliver a cmsg with the hardware timestamp only. Why are races even possible? Well, because although the software timestamp skb->tstamp is private per skb, the hardware timestamp skb_hwtstamps(skb) lives in skb_shinfo(skb), an area which is shared between skbs and their clones. And skb_tstamp_tx works by cloning the packets when timestamping them, therefore attempting to perform hardware timestamping on an skb's clone will also change the hardware timestamp of the original skb. And the original skb might have been yet again cloned for software timestamping, at an earlier stage. So the logic in __sock_recv_timestamp can't be as simple as saying "does this skb have a hardware timestamp? if yes I'll send the hardware timestamp to the socket, otherwise I'll send the software timestamp", precisely because the hardware timestamp is shared. Instead, it's quite the other way around: __sock_recv_timestamp says "does this skb have a software timestamp? if yes, I'll send the software timestamp, otherwise the hardware one". This works because the software timestamp is not shared with clones. But that means we have a problem when we attempt hardware timestamping with skbs that don't have the skb->tstamp == 0. __sock_recv_timestamp will say "oh, yeah, this must be some sort of odd clone" and will not deliver the hardware timestamp to the socket. And this is exactly what is happening when we have txtime enabled on the socket: as mentioned, that is put in a union with skb->tstamp, so it is quite easy to mistake it. Do what other drivers do (intel igb/igc) and write zero to skb->tstamp before taking the hardware timestamp. It's of no use to us now (we're already on the TX confirmation path). Fixes: 0d08c9ec7d6e ("enetc: add support time specific departure base on the qos etf") Cc: Vinicius Costa Gomes <vinicius.gomes@intel.com> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-03-17net: enetc: keep RX ring consumer index in sync with hardwareVladimir Oltean1-0/+2
commit 3a5d12c9be6f30080600c8bacaf310194e37d029 upstream. The RX rings have a producer index owned by hardware, where newly received frame buffers are placed, and a consumer index owned by software, where newly allocated buffers are placed, in expectation of hardware being able to place frame data in them. Hardware increments the producer index when a frame is received, however it is not allowed to increment the producer index to match the consumer index (RBCIR) since the ring can hold at most RBLENR[LENGTH]-1 received BDs. Whenever the producer index matches the value of the consumer index, the ring has no unprocessed received frames and all BDs in the ring have been initialized/prepared by software, i.e. hardware owns all BDs in the ring. The code uses the next_to_clean variable to keep track of the producer index, and the next_to_use variable to keep track of the consumer index. The RX rings are seeded from enetc_refill_rx_ring, which is called from two places: 1. initially the ring is seeded until full with enetc_bd_unused(rx_ring), i.e. with 511 buffers. This will make next_to_clean=0 and next_to_use=511: .ndo_open -> enetc_open -> enetc_setup_bdrs -> enetc_setup_rxbdr -> enetc_refill_rx_ring 2. then during the data path processing, it is refilled with 16 buffers at a time: enetc_msix -> napi_schedule -> enetc_poll -> enetc_clean_rx_ring -> enetc_refill_rx_ring There is just one problem: the initial seeding done during .ndo_open updates just the producer index (ENETC_RBPIR) with 0, and the software next_to_clean and next_to_use variables. Notably, it will not update the consumer index to make the hardware aware of the newly added buffers. Wait, what? So how does it work? Well, the reset values of the producer index and of the consumer index of a ring are both zero. As per the description in the second paragraph, it means that the ring is full of buffers waiting for hardware to put frames in them, which by coincidence is almost true, because we have in fact seeded 511 buffers into the ring. But will the hardware attempt to access the 512th entry of the ring, which has an invalid BD in it? Well, no, because in order to do that, it would have to first populate the first 511 entries, and the NAPI enetc_poll will kick in by then. Eventually, after 16 processed slots have become available in the RX ring, enetc_clean_rx_ring will call enetc_refill_rx_ring and then will [ finally ] update the consumer index with the new software next_to_use variable. From now on, the next_to_clean and next_to_use variables are in sync with the producer and consumer ring indices. So the day is saved, right? Well, not quite. Freeing the memory allocated for the rings is done in: enetc_close -> enetc_clear_bdrs -> enetc_clear_rxbdr -> this just disables the ring -> enetc_free_rxtx_rings -> enetc_free_rx_ring -> sets next_to_clean and next_to_use to 0 but again, nothing is committed to the hardware producer and consumer indices (yay!). The assumption is that the ring is disabled, so the indices don't matter anyway, and it's the responsibility of the "open" code path to set those up. .. Except that the "open" code path does not set those up properly. While initially, things almost work, during subsequent enetc_close -> enetc_open sequences, we have problems. To be precise, the enetc_open that is subsequent to enetc_close will again refill the ring with 511 entries, but it will leave the consumer index untouched. Untouched means, of course, equal to the value it had before disabling the ring and draining the old buffers in enetc_close. But as mentioned, enetc_setup_rxbdr will at least update the producer index though, through this line of code: enetc_rxbdr_wr(hw, idx, ENETC_RBPIR, 0); so at this stage we'll have: next_to_clean=0 (in hardware 0) next_to_use=511 (in hardware we'll have the refill index prior to enetc_close) Again, the next_to_clean and producer index are in sync and set to correct values, so the driver manages to limp on. Eventually, 16 ring entries will be consumed by enetc_poll, and the savior enetc_clean_rx_ring will come and call enetc_refill_rx_ring, and then update the hardware consumer ring based upon the new next_to_use. So.. it works? Well, by coincidence, it almost does, but there's a circumstance where enetc_clean_rx_ring won't be there to save us. If the previous value of the consumer index was 15, there's a problem, because the NAPI poll sequence will only issue a refill when 16 or more buffers have been consumed. It's easiest to illustrate this with an example: ip link set eno0 up ip addr add 192.168.100.1/24 dev eno0 ping 192.168.100.1 -c 20 # ping this port from another board ip link set eno0 down ip link set eno0 up ping 192.168.100.1 -c 20 # ping it again from the same other board One by one: 1. ip link set eno0 up -> calls enetc_setup_rxbdr: -> calls enetc_refill_rx_ring(511 buffers) -> next_to_clean=0 (in hw 0) -> next_to_use=511 (in hw 0) 2. ping 192.168.100.1 -c 20 # ping this port from another board enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=1 next_to_clean 0 (in hw 1) next_to_use 511 (in hw 0) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=2 next_to_clean 1 (in hw 2) next_to_use 511 (in hw 0) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=3 next_to_clean 2 (in hw 3) next_to_use 511 (in hw 0) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=4 next_to_clean 3 (in hw 4) next_to_use 511 (in hw 0) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=5 next_to_clean 4 (in hw 5) next_to_use 511 (in hw 0) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=6 next_to_clean 5 (in hw 6) next_to_use 511 (in hw 0) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=7 next_to_clean 6 (in hw 7) next_to_use 511 (in hw 0) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=8 next_to_clean 7 (in hw 8) next_to_use 511 (in hw 0) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=9 next_to_clean 8 (in hw 9) next_to_use 511 (in hw 0) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=10 next_to_clean 9 (in hw 10) next_to_use 511 (in hw 0) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=11 next_to_clean 10 (in hw 11) next_to_use 511 (in hw 0) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=12 next_to_clean 11 (in hw 12) next_to_use 511 (in hw 0) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=13 next_to_clean 12 (in hw 13) next_to_use 511 (in hw 0) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=14 next_to_clean 13 (in hw 14) next_to_use 511 (in hw 0) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=15 next_to_clean 14 (in hw 15) next_to_use 511 (in hw 0) enetc_clean_rx_ring: enetc_refill_rx_ring(16) increments next_to_use by 16 (mod 512) and writes it to hw enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=0 next_to_clean 15 (in hw 16) next_to_use 15 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=1 next_to_clean 16 (in hw 17) next_to_use 15 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=2 next_to_clean 17 (in hw 18) next_to_use 15 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=3 next_to_clean 18 (in hw 19) next_to_use 15 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=4 next_to_clean 19 (in hw 20) next_to_use 15 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=5 next_to_clean 20 (in hw 21) next_to_use 15 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=6 next_to_clean 21 (in hw 22) next_to_use 15 (in hw 15) 20 packets transmitted, 20 packets received, 0% packet loss 3. ip link set eno0 down enetc_free_rx_ring: next_to_clean 0 (in hw 22), next_to_use 0 (in hw 15) 4. ip link set eno0 up -> calls enetc_setup_rxbdr: -> calls enetc_refill_rx_ring(511 buffers) -> next_to_clean=0 (in hw 0) -> next_to_use=511 (in hw 15) 5. ping 192.168.100.1 -c 20 # ping it again from the same other board enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=1 next_to_clean 0 (in hw 1) next_to_use 511 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=2 next_to_clean 1 (in hw 2) next_to_use 511 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=3 next_to_clean 2 (in hw 3) next_to_use 511 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=4 next_to_clean 3 (in hw 4) next_to_use 511 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=5 next_to_clean 4 (in hw 5) next_to_use 511 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=6 next_to_clean 5 (in hw 6) next_to_use 511 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=7 next_to_clean 6 (in hw 7) next_to_use 511 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=8 next_to_clean 7 (in hw 8) next_to_use 511 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=9 next_to_clean 8 (in hw 9) next_to_use 511 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=10 next_to_clean 9 (in hw 10) next_to_use 511 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=11 next_to_clean 10 (in hw 11) next_to_use 511 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=12 next_to_clean 11 (in hw 12) next_to_use 511 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=13 next_to_clean 12 (in hw 13) next_to_use 511 (in hw 15) enetc_clean_rx_ring: rx_frm_cnt=1 cleaned_cnt=14 next_to_clean 13 (in hw 14) next_to_use 511 (in hw 15) 20 packets transmitted, 12 packets received, 40% packet loss And there it dies. No enetc_refill_rx_ring (because cleaned_cnt must be equal to 15 for that to happen), no nothing. The hardware enters the condition where the producer (14) + 1 is equal to the consumer (15) index, which makes it believe it has no more free buffers to put packets in, so it starts discarding them: ip netns exec ns0 ethtool -S eno0 | grep -v ': 0' NIC statistics: Rx ring 0 discarded frames: 8 Summarized, if the interface receives between 16 and 32 (mod 512) frames and then there is a link flap, then the port will eventually die with no way to recover. If it receives less than 16 (mod 512) frames, then the initial NAPI poll [ before the link flap ] will not update the consumer index in hardware (it will remain zero) which will be ok when the buffers are later reinitialized. If more than 32 (mod 512) frames are received, the initial NAPI poll has the chance to refill the ring twice, updating the consumer index to at least 32. So after the link flap, the consumer index is still wrong, but the post-flap NAPI poll gets a chance to refill the ring once (because it passes through cleaned_cnt=15) and makes the consumer index be again back in sync with next_to_use. The solution to this problem is actually simple, we just need to write next_to_use into the hardware consumer index at enetc_open time, which always brings it back in sync after an initial buffer seeding process. The simpler thing would be to put the write to the consumer index into enetc_refill_rx_ring directly, but there are issues with the MDIO locking: in the NAPI poll code we have the enetc_lock_mdio() taken from top-level and we use the unlocked enetc_wr_reg_hot, whereas in enetc_open, the enetc_lock_mdio() is not taken at the top level, but instead by each individual enetc_wr_reg, so we are forced to put an additional enetc_wr_reg in enetc_setup_rxbdr. Better organization of the code is left as a refactoring exercise. Fixes: d4fd0404c1c9 ("enetc: Introduce basic PF and VF ENETC ethernet drivers") Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-03-17net: enetc: remove bogus write to SIRXIDR from enetc_setup_rxbdrVladimir Oltean1-1/+0
commit 96a5223b918c8b79270fc0fec235a7ebad459098 upstream. The Station Interface Receive Interrupt Detect Register (SIRXIDR) contains a 16-bit wide mask of 'interrupt detected' events for each ring associated with a port. Bit i is write-1-to-clean for RX ring i. I have no explanation whatsoever how this line of code came to be inserted in the blamed commit. I checked the downstream versions of that patch and none of them have it. The somewhat comical aspect of it is that we're writing a binary number to the SIRXIDR register, which is derived from enetc_bd_unused(rx_ring). Since the RX rings have 512 buffer descriptors, we end up writing 511 to this register, which is 0x1ff, so we are effectively clearing the 'interrupt detected' event for rings 0-8. This register is not what is used for interrupt handling though - it only provides a summary for the entire SI. The hardware provides one separate Interrupt Detect Register per RX ring, which auto-clears upon read. So there doesn't seem to be any adverse effect caused by this bogus write. There is, however, one reason why this should be handled as a bugfix: next_to_clean _should_ be committed to hardware, just not to that register, and this was obscuring the fact that it wasn't. This is fixed in the next patch, and removing the bogus line now allows the fix patch to be backported beyond that point. Fixes: fd5736bf9f23 ("enetc: Workaround for MDIO register access issue") Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
generated by cgit v1.2.3 (git 2.39.0) at 2025-09-27 05:02:37 +0300