* [PATCH net-next v2 2/3] net: stmmac: Fix descriptors address being in > 32 bits address space
From: Jose Abreu @ 2019-07-04 15:04 UTC (permalink / raw)
To: linux-kernel, netdev, linux-stm32, linux-arm-kernel
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue, Maxime Coquelin, Maxime Ripard, Chen-Yu Tsai
In-Reply-To: <cover.1562252534.git.joabreu@synopsys.com>
Commit a993db88d17d ("net: stmmac: Enable support for > 32 Bits
addressing in XGMAC"), introduced support for > 32 bits addressing in
XGMAC but the conversion of descriptors to dma_addr_t was left out.
As some devices assing coherent memory in regions > 32 bits we need to
set lower and upper value of descriptors address when initializing DMA
channels.
Luckly, this was working for me because I was assigning CMA to < 4GB
address space for performance reasons.
Fixes: a993db88d17d ("net: stmmac: Enable support for > 32 Bits addressing in XGMAC")
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Chen-Yu Tsai <wens@csie.org>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 8 ++++----
drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c | 8 ++++----
drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c | 8 ++++----
drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 8 ++++----
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 ++
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 10 ++++++----
drivers/net/ethernet/stmicro/stmmac/hwif.h | 4 ++--
7 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 6d5cba4075eb..2856f3fe5266 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -289,18 +289,18 @@ static void sun8i_dwmac_dma_init(void __iomem *ioaddr,
static void sun8i_dwmac_dma_init_rx(void __iomem *ioaddr,
struct stmmac_dma_cfg *dma_cfg,
- u32 dma_rx_phy, u32 chan)
+ dma_addr_t dma_rx_phy, u32 chan)
{
/* Write RX descriptors address */
- writel(dma_rx_phy, ioaddr + EMAC_RX_DESC_LIST);
+ writel(lower_32_bits(dma_rx_phy), ioaddr + EMAC_RX_DESC_LIST);
}
static void sun8i_dwmac_dma_init_tx(void __iomem *ioaddr,
struct stmmac_dma_cfg *dma_cfg,
- u32 dma_tx_phy, u32 chan)
+ dma_addr_t dma_tx_phy, u32 chan)
{
/* Write TX descriptors address */
- writel(dma_tx_phy, ioaddr + EMAC_TX_DESC_LIST);
+ writel(lower_32_bits(dma_tx_phy), ioaddr + EMAC_TX_DESC_LIST);
}
/* sun8i_dwmac_dump_regs() - Dump EMAC address space
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
index 1fdedf77678f..2bac49b49f73 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
@@ -112,18 +112,18 @@ static void dwmac1000_dma_init(void __iomem *ioaddr,
static void dwmac1000_dma_init_rx(void __iomem *ioaddr,
struct stmmac_dma_cfg *dma_cfg,
- u32 dma_rx_phy, u32 chan)
+ dma_addr_t dma_rx_phy, u32 chan)
{
/* RX descriptor base address list must be written into DMA CSR3 */
- writel(dma_rx_phy, ioaddr + DMA_RCV_BASE_ADDR);
+ writel(lower_32_bits(dma_rx_phy), ioaddr + DMA_RCV_BASE_ADDR);
}
static void dwmac1000_dma_init_tx(void __iomem *ioaddr,
struct stmmac_dma_cfg *dma_cfg,
- u32 dma_tx_phy, u32 chan)
+ dma_addr_t dma_tx_phy, u32 chan)
{
/* TX descriptor base address list must be written into DMA CSR4 */
- writel(dma_tx_phy, ioaddr + DMA_TX_BASE_ADDR);
+ writel(lower_32_bits(dma_tx_phy), ioaddr + DMA_TX_BASE_ADDR);
}
static u32 dwmac1000_configure_fc(u32 csr6, int rxfifosz)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c
index c980cc7360a4..8f0d9bc7cab5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c
@@ -31,18 +31,18 @@ static void dwmac100_dma_init(void __iomem *ioaddr,
static void dwmac100_dma_init_rx(void __iomem *ioaddr,
struct stmmac_dma_cfg *dma_cfg,
- u32 dma_rx_phy, u32 chan)
+ dma_addr_t dma_rx_phy, u32 chan)
{
/* RX descriptor base addr lists must be written into DMA CSR3 */
- writel(dma_rx_phy, ioaddr + DMA_RCV_BASE_ADDR);
+ writel(lower_32_bits(dma_rx_phy), ioaddr + DMA_RCV_BASE_ADDR);
}
static void dwmac100_dma_init_tx(void __iomem *ioaddr,
struct stmmac_dma_cfg *dma_cfg,
- u32 dma_tx_phy, u32 chan)
+ dma_addr_t dma_tx_phy, u32 chan)
{
/* TX descriptor base addr lists must be written into DMA CSR4 */
- writel(dma_tx_phy, ioaddr + DMA_TX_BASE_ADDR);
+ writel(lower_32_bits(dma_tx_phy), ioaddr + DMA_TX_BASE_ADDR);
}
/* Store and Forward capability is not used at all.
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
index 0f208e13da9f..6cbcdaea55f6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
@@ -70,7 +70,7 @@ static void dwmac4_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
static void dwmac4_dma_init_rx_chan(void __iomem *ioaddr,
struct stmmac_dma_cfg *dma_cfg,
- u32 dma_rx_phy, u32 chan)
+ dma_addr_t dma_rx_phy, u32 chan)
{
u32 value;
u32 rxpbl = dma_cfg->rxpbl ?: dma_cfg->pbl;
@@ -79,12 +79,12 @@ static void dwmac4_dma_init_rx_chan(void __iomem *ioaddr,
value = value | (rxpbl << DMA_BUS_MODE_RPBL_SHIFT);
writel(value, ioaddr + DMA_CHAN_RX_CONTROL(chan));
- writel(dma_rx_phy, ioaddr + DMA_CHAN_RX_BASE_ADDR(chan));
+ writel(lower_32_bits(dma_rx_phy), ioaddr + DMA_CHAN_RX_BASE_ADDR(chan));
}
static void dwmac4_dma_init_tx_chan(void __iomem *ioaddr,
struct stmmac_dma_cfg *dma_cfg,
- u32 dma_tx_phy, u32 chan)
+ dma_addr_t dma_tx_phy, u32 chan)
{
u32 value;
u32 txpbl = dma_cfg->txpbl ?: dma_cfg->pbl;
@@ -97,7 +97,7 @@ static void dwmac4_dma_init_tx_chan(void __iomem *ioaddr,
writel(value, ioaddr + DMA_CHAN_TX_CONTROL(chan));
- writel(dma_tx_phy, ioaddr + DMA_CHAN_TX_BASE_ADDR(chan));
+ writel(lower_32_bits(dma_tx_phy), ioaddr + DMA_CHAN_TX_BASE_ADDR(chan));
}
static void dwmac4_dma_init_channel(void __iomem *ioaddr,
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index 9a9792527530..7f86dffb264d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -199,7 +199,9 @@
#define XGMAC_RxPBL GENMASK(21, 16)
#define XGMAC_RxPBL_SHIFT 16
#define XGMAC_RXST BIT(0)
+#define XGMAC_DMA_CH_TxDESC_HADDR(x) (0x00003110 + (0x80 * (x)))
#define XGMAC_DMA_CH_TxDESC_LADDR(x) (0x00003114 + (0x80 * (x)))
+#define XGMAC_DMA_CH_RxDESC_HADDR(x) (0x00003118 + (0x80 * (x)))
#define XGMAC_DMA_CH_RxDESC_LADDR(x) (0x0000311c + (0x80 * (x)))
#define XGMAC_DMA_CH_TxDESC_TAIL_LPTR(x) (0x00003124 + (0x80 * (x)))
#define XGMAC_DMA_CH_RxDESC_TAIL_LPTR(x) (0x0000312c + (0x80 * (x)))
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 229c58758cbd..a4f236e3593e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -44,7 +44,7 @@ static void dwxgmac2_dma_init_chan(void __iomem *ioaddr,
static void dwxgmac2_dma_init_rx_chan(void __iomem *ioaddr,
struct stmmac_dma_cfg *dma_cfg,
- u32 dma_rx_phy, u32 chan)
+ dma_addr_t phy, u32 chan)
{
u32 rxpbl = dma_cfg->rxpbl ?: dma_cfg->pbl;
u32 value;
@@ -54,12 +54,13 @@ static void dwxgmac2_dma_init_rx_chan(void __iomem *ioaddr,
value |= (rxpbl << XGMAC_RxPBL_SHIFT) & XGMAC_RxPBL;
writel(value, ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
- writel(dma_rx_phy, ioaddr + XGMAC_DMA_CH_RxDESC_LADDR(chan));
+ writel(upper_32_bits(phy), ioaddr + XGMAC_DMA_CH_RxDESC_HADDR(chan));
+ writel(lower_32_bits(phy), ioaddr + XGMAC_DMA_CH_RxDESC_LADDR(chan));
}
static void dwxgmac2_dma_init_tx_chan(void __iomem *ioaddr,
struct stmmac_dma_cfg *dma_cfg,
- u32 dma_tx_phy, u32 chan)
+ dma_addr_t phy, u32 chan)
{
u32 txpbl = dma_cfg->txpbl ?: dma_cfg->pbl;
u32 value;
@@ -70,7 +71,8 @@ static void dwxgmac2_dma_init_tx_chan(void __iomem *ioaddr,
value |= XGMAC_OSP;
writel(value, ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
- writel(dma_tx_phy, ioaddr + XGMAC_DMA_CH_TxDESC_LADDR(chan));
+ writel(upper_32_bits(phy), ioaddr + XGMAC_DMA_CH_TxDESC_HADDR(chan));
+ writel(lower_32_bits(phy), ioaddr + XGMAC_DMA_CH_TxDESC_LADDR(chan));
}
static void dwxgmac2_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 2acfbc70e3c8..278c0dbec9d9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -150,10 +150,10 @@ struct stmmac_dma_ops {
struct stmmac_dma_cfg *dma_cfg, u32 chan);
void (*init_rx_chan)(void __iomem *ioaddr,
struct stmmac_dma_cfg *dma_cfg,
- u32 dma_rx_phy, u32 chan);
+ dma_addr_t phy, u32 chan);
void (*init_tx_chan)(void __iomem *ioaddr,
struct stmmac_dma_cfg *dma_cfg,
- u32 dma_tx_phy, u32 chan);
+ dma_addr_t phy, u32 chan);
/* Configure the AXI Bus Mode Register */
void (*axi)(void __iomem *ioaddr, struct stmmac_axi *axi);
/* Dump DMA registers */
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 1/3] net: stmmac: Implement RX Coalesce Frames setting
From: Jose Abreu @ 2019-07-04 15:04 UTC (permalink / raw)
To: linux-kernel, netdev, linux-stm32, linux-arm-kernel
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue, Jakub Kicinski
In-Reply-To: <cover.1562252534.git.joabreu@synopsys.com>
Add support for coalescing RX path by specifying number of frames which
don't need to have interrupt on completion bit set.
This is only available when RX Watchdog is enabled.
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/stmicro/stmmac/common.h | 1 +
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 ++
drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 7 +++++--
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 18 ++++++++++++------
4 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 2403a65167b2..dfd47fdfa447 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -252,6 +252,7 @@ struct stmmac_safety_stats {
#define STMMAC_MAX_COAL_TX_TICK 100000
#define STMMAC_TX_MAX_FRAMES 256
#define STMMAC_TX_FRAMES 1
+#define STMMAC_RX_FRAMES 25
/* Packets types */
enum packets_types {
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 123898235cb0..513f4e2df5f6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -55,6 +55,7 @@ struct stmmac_tx_queue {
};
struct stmmac_rx_queue {
+ u32 rx_count_frames;
u32 queue_index;
struct stmmac_priv *priv_data;
struct dma_extended_desc *dma_erx;
@@ -110,6 +111,7 @@ struct stmmac_priv {
/* Frequently used values are kept adjacent for cache effect */
u32 tx_coal_frames;
u32 tx_coal_timer;
+ u32 rx_coal_frames;
int tx_coalesce;
int hwts_tx_en;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index cfd93eefb50e..6efb66820d4c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -701,8 +701,10 @@ static int stmmac_get_coalesce(struct net_device *dev,
ec->tx_coalesce_usecs = priv->tx_coal_timer;
ec->tx_max_coalesced_frames = priv->tx_coal_frames;
- if (priv->use_riwt)
+ if (priv->use_riwt) {
+ ec->rx_max_coalesced_frames = priv->rx_coal_frames;
ec->rx_coalesce_usecs = stmmac_riwt2usec(priv->rx_riwt, priv);
+ }
return 0;
}
@@ -715,7 +717,7 @@ static int stmmac_set_coalesce(struct net_device *dev,
unsigned int rx_riwt;
/* Check not supported parameters */
- if ((ec->rx_max_coalesced_frames) || (ec->rx_coalesce_usecs_irq) ||
+ if ((ec->rx_coalesce_usecs_irq) ||
(ec->rx_max_coalesced_frames_irq) || (ec->tx_coalesce_usecs_irq) ||
(ec->use_adaptive_rx_coalesce) || (ec->use_adaptive_tx_coalesce) ||
(ec->pkt_rate_low) || (ec->rx_coalesce_usecs_low) ||
@@ -749,6 +751,7 @@ static int stmmac_set_coalesce(struct net_device *dev,
/* Only copy relevant parameters, ignore all others. */
priv->tx_coal_frames = ec->tx_max_coalesced_frames;
priv->tx_coal_timer = ec->tx_coalesce_usecs;
+ priv->rx_coal_frames = ec->rx_max_coalesced_frames;
priv->rx_riwt = rx_riwt;
stmmac_rx_watchdog(priv, priv->ioaddr, priv->rx_riwt, rx_cnt);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 3425d4dda03d..c8fe85ef9a7e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2268,20 +2268,21 @@ static void stmmac_tx_timer(struct timer_list *t)
}
/**
- * stmmac_init_tx_coalesce - init tx mitigation options.
+ * stmmac_init_coalesce - init mitigation options.
* @priv: driver private structure
* Description:
- * This inits the transmit coalesce parameters: i.e. timer rate,
+ * This inits the coalesce parameters: i.e. timer rate,
* timer handler and default threshold used for enabling the
* interrupt on completion bit.
*/
-static void stmmac_init_tx_coalesce(struct stmmac_priv *priv)
+static void stmmac_init_coalesce(struct stmmac_priv *priv)
{
u32 tx_channel_count = priv->plat->tx_queues_to_use;
u32 chan;
priv->tx_coal_frames = STMMAC_TX_FRAMES;
priv->tx_coal_timer = STMMAC_COAL_TX_TIMER;
+ priv->rx_coal_frames = STMMAC_RX_FRAMES;
for (chan = 0; chan < tx_channel_count; chan++) {
struct stmmac_tx_queue *tx_q = &priv->tx_queue[chan];
@@ -2651,7 +2652,7 @@ static int stmmac_open(struct net_device *dev)
goto init_error;
}
- stmmac_init_tx_coalesce(priv);
+ stmmac_init_coalesce(priv);
phylink_start(priv->phylink);
@@ -3298,6 +3299,7 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
while (dirty-- > 0) {
struct dma_desc *p;
+ bool use_rx_wd;
if (priv->extend_desc)
p = (struct dma_desc *)(rx_q->dma_erx + entry);
@@ -3340,7 +3342,11 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
}
dma_wmb();
- stmmac_set_rx_owner(priv, p, priv->use_riwt);
+ rx_q->rx_count_frames++;
+ rx_q->rx_count_frames %= priv->rx_coal_frames;
+ use_rx_wd = priv->use_riwt && rx_q->rx_count_frames;
+
+ stmmac_set_rx_owner(priv, p, use_rx_wd);
dma_wmb();
@@ -4623,7 +4629,7 @@ int stmmac_resume(struct device *dev)
stmmac_clear_descriptors(priv);
stmmac_hw_setup(ndev, false);
- stmmac_init_tx_coalesce(priv);
+ stmmac_init_coalesce(priv);
stmmac_set_rx_mode(ndev);
stmmac_enable_all_queues(priv);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 0/3] net: stmmac: Some improvements and a fix
From: Jose Abreu @ 2019-07-04 15:04 UTC (permalink / raw)
To: linux-kernel, netdev, linux-stm32, linux-arm-kernel
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
Some performace improvements (01/03 and 03/03) and a fix (02/03), all for -next.
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Jose Abreu (3):
net: stmmac: Implement RX Coalesce Frames setting
net: stmmac: Fix descriptors address being in > 32 bits address space
net: stmmac: Introducing support for Page Pool
drivers/net/ethernet/stmicro/stmmac/Kconfig | 1 +
drivers/net/ethernet/stmicro/stmmac/common.h | 1 +
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 8 +-
.../net/ethernet/stmicro/stmmac/dwmac1000_dma.c | 8 +-
drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c | 8 +-
drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 8 +-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 +
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 10 +-
drivers/net/ethernet/stmicro/stmmac/hwif.h | 4 +-
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 12 +-
.../net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 7 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 215 +++++++--------------
12 files changed, 112 insertions(+), 172 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH net-next v2 3/3] net: stmmac: Introducing support for Page Pool
From: Jose Abreu @ 2019-07-04 15:04 UTC (permalink / raw)
To: linux-kernel, netdev, linux-stm32, linux-arm-kernel
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue, Ilias Apalodimas, Jesper Dangaard Brouer,
Arnd Bergmann
In-Reply-To: <cover.1562252534.git.joabreu@synopsys.com>
Mapping and unmapping DMA region is an high bottleneck in stmmac driver,
specially in the RX path.
This commit introduces support for Page Pool API and uses it in all RX
queues. With this change, we get more stable troughput and some increase
of banwidth with iperf:
- MAC1000 - 950 Mbps
- XGMAC: 9.22 Gbps
Changes from v1:
- Use page_pool_get_dma_addr() (Jesper)
- Add a comment (Jesper)
- Add page_pool_free() call (Jesper)
- Reintroduce sync_single_for_device (Arnd / Ilias)
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/stmicro/stmmac/Kconfig | 1 +
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 10 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 201 +++++++---------------
3 files changed, 68 insertions(+), 144 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 943189dcccb1..2325b40dff6e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -3,6 +3,7 @@ config STMMAC_ETH
tristate "STMicroelectronics Multi-Gigabit Ethernet driver"
depends on HAS_IOMEM && HAS_DMA
select MII
+ select PAGE_POOL
select PHYLINK
select CRC32
imply PTP_1588_CLOCK
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 513f4e2df5f6..5cd966c154f3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -20,6 +20,7 @@
#include <linux/ptp_clock_kernel.h>
#include <linux/net_tstamp.h>
#include <linux/reset.h>
+#include <net/page_pool.h>
struct stmmac_resources {
void __iomem *addr;
@@ -54,14 +55,19 @@ struct stmmac_tx_queue {
u32 mss;
};
+struct stmmac_rx_buffer {
+ struct page *page;
+ dma_addr_t addr;
+};
+
struct stmmac_rx_queue {
u32 rx_count_frames;
u32 queue_index;
+ struct page_pool *page_pool;
+ struct stmmac_rx_buffer *buf_pool;
struct stmmac_priv *priv_data;
struct dma_extended_desc *dma_erx;
struct dma_desc *dma_rx ____cacheline_aligned_in_smp;
- struct sk_buff **rx_skbuff;
- dma_addr_t *rx_skbuff_dma;
unsigned int cur_rx;
unsigned int dirty_rx;
u32 rx_zeroc_thresh;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index c8fe85ef9a7e..ec7ae81b65cc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1197,26 +1197,14 @@ static int stmmac_init_rx_buffers(struct stmmac_priv *priv, struct dma_desc *p,
int i, gfp_t flags, u32 queue)
{
struct stmmac_rx_queue *rx_q = &priv->rx_queue[queue];
- struct sk_buff *skb;
+ struct stmmac_rx_buffer *buf = &rx_q->buf_pool[i];
- skb = __netdev_alloc_skb_ip_align(priv->dev, priv->dma_buf_sz, flags);
- if (!skb) {
- netdev_err(priv->dev,
- "%s: Rx init fails; skb is NULL\n", __func__);
+ buf->page = page_pool_dev_alloc_pages(rx_q->page_pool);
+ if (!buf->page)
return -ENOMEM;
- }
- rx_q->rx_skbuff[i] = skb;
- rx_q->rx_skbuff_dma[i] = dma_map_single(priv->device, skb->data,
- priv->dma_buf_sz,
- DMA_FROM_DEVICE);
- if (dma_mapping_error(priv->device, rx_q->rx_skbuff_dma[i])) {
- netdev_err(priv->dev, "%s: DMA mapping error\n", __func__);
- dev_kfree_skb_any(skb);
- return -EINVAL;
- }
-
- stmmac_set_desc_addr(priv, p, rx_q->rx_skbuff_dma[i]);
+ buf->addr = page_pool_get_dma_addr(buf->page);
+ stmmac_set_desc_addr(priv, p, buf->addr);
if (priv->dma_buf_sz == BUF_SIZE_16KiB)
stmmac_init_desc3(priv, p);
@@ -1232,13 +1220,11 @@ static int stmmac_init_rx_buffers(struct stmmac_priv *priv, struct dma_desc *p,
static void stmmac_free_rx_buffer(struct stmmac_priv *priv, u32 queue, int i)
{
struct stmmac_rx_queue *rx_q = &priv->rx_queue[queue];
+ struct stmmac_rx_buffer *buf = &rx_q->buf_pool[i];
- if (rx_q->rx_skbuff[i]) {
- dma_unmap_single(priv->device, rx_q->rx_skbuff_dma[i],
- priv->dma_buf_sz, DMA_FROM_DEVICE);
- dev_kfree_skb_any(rx_q->rx_skbuff[i]);
- }
- rx_q->rx_skbuff[i] = NULL;
+ if (buf->page)
+ page_pool_put_page(rx_q->page_pool, buf->page, false);
+ buf->page = NULL;
}
/**
@@ -1321,10 +1307,6 @@ static int init_dma_rx_desc_rings(struct net_device *dev, gfp_t flags)
queue);
if (ret)
goto err_init_rx_buffers;
-
- netif_dbg(priv, probe, priv->dev, "[%p]\t[%p]\t[%x]\n",
- rx_q->rx_skbuff[i], rx_q->rx_skbuff[i]->data,
- (unsigned int)rx_q->rx_skbuff_dma[i]);
}
rx_q->cur_rx = 0;
@@ -1498,8 +1480,9 @@ static void free_dma_rx_desc_resources(struct stmmac_priv *priv)
sizeof(struct dma_extended_desc),
rx_q->dma_erx, rx_q->dma_rx_phy);
- kfree(rx_q->rx_skbuff_dma);
- kfree(rx_q->rx_skbuff);
+ kfree(rx_q->buf_pool);
+ if (rx_q->page_pool && page_pool_request_shutdown(rx_q->page_pool))
+ page_pool_free(rx_q->page_pool);
}
}
@@ -1551,20 +1534,29 @@ static int alloc_dma_rx_desc_resources(struct stmmac_priv *priv)
/* RX queues buffers and DMA */
for (queue = 0; queue < rx_count; queue++) {
struct stmmac_rx_queue *rx_q = &priv->rx_queue[queue];
+ struct page_pool_params pp_params = { 0 };
rx_q->queue_index = queue;
rx_q->priv_data = priv;
- rx_q->rx_skbuff_dma = kmalloc_array(DMA_RX_SIZE,
- sizeof(dma_addr_t),
- GFP_KERNEL);
- if (!rx_q->rx_skbuff_dma)
+ pp_params.flags = PP_FLAG_DMA_MAP;
+ pp_params.pool_size = DMA_RX_SIZE;
+ pp_params.order = DIV_ROUND_UP(priv->dma_buf_sz, PAGE_SIZE);
+ pp_params.nid = dev_to_node(priv->device);
+ pp_params.dev = priv->device;
+ pp_params.dma_dir = DMA_FROM_DEVICE;
+
+ rx_q->page_pool = page_pool_create(&pp_params);
+ if (IS_ERR(rx_q->page_pool)) {
+ ret = PTR_ERR(rx_q->page_pool);
+ rx_q->page_pool = NULL;
goto err_dma;
+ }
- rx_q->rx_skbuff = kmalloc_array(DMA_RX_SIZE,
- sizeof(struct sk_buff *),
- GFP_KERNEL);
- if (!rx_q->rx_skbuff)
+ rx_q->buf_pool = kmalloc_array(DMA_RX_SIZE,
+ sizeof(*rx_q->buf_pool),
+ GFP_KERNEL);
+ if (!rx_q->buf_pool)
goto err_dma;
if (priv->extend_desc) {
@@ -3295,9 +3287,8 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
int dirty = stmmac_rx_dirty(priv, queue);
unsigned int entry = rx_q->dirty_rx;
- int bfsize = priv->dma_buf_sz;
-
while (dirty-- > 0) {
+ struct stmmac_rx_buffer *buf = &rx_q->buf_pool[entry];
struct dma_desc *p;
bool use_rx_wd;
@@ -3306,49 +3297,22 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
else
p = rx_q->dma_rx + entry;
- if (likely(!rx_q->rx_skbuff[entry])) {
- struct sk_buff *skb;
-
- skb = netdev_alloc_skb_ip_align(priv->dev, bfsize);
- if (unlikely(!skb)) {
- /* so for a while no zero-copy! */
- rx_q->rx_zeroc_thresh = STMMAC_RX_THRESH;
- if (unlikely(net_ratelimit()))
- dev_err(priv->device,
- "fail to alloc skb entry %d\n",
- entry);
- break;
- }
-
- rx_q->rx_skbuff[entry] = skb;
- rx_q->rx_skbuff_dma[entry] =
- dma_map_single(priv->device, skb->data, bfsize,
- DMA_FROM_DEVICE);
- if (dma_mapping_error(priv->device,
- rx_q->rx_skbuff_dma[entry])) {
- netdev_err(priv->dev, "Rx DMA map failed\n");
- dev_kfree_skb(skb);
+ if (!buf->page) {
+ buf->page = page_pool_dev_alloc_pages(rx_q->page_pool);
+ if (!buf->page)
break;
- }
-
- stmmac_set_desc_addr(priv, p, rx_q->rx_skbuff_dma[entry]);
- stmmac_refill_desc3(priv, rx_q, p);
-
- if (rx_q->rx_zeroc_thresh > 0)
- rx_q->rx_zeroc_thresh--;
-
- netif_dbg(priv, rx_status, priv->dev,
- "refill entry #%d\n", entry);
}
- dma_wmb();
+
+ buf->addr = page_pool_get_dma_addr(buf->page);
+ stmmac_set_desc_addr(priv, p, buf->addr);
+ stmmac_refill_desc3(priv, rx_q, p);
rx_q->rx_count_frames++;
rx_q->rx_count_frames %= priv->rx_coal_frames;
use_rx_wd = priv->use_riwt && rx_q->rx_count_frames;
- stmmac_set_rx_owner(priv, p, use_rx_wd);
-
dma_wmb();
+ stmmac_set_rx_owner(priv, p, use_rx_wd);
entry = STMMAC_GET_ENTRY(entry, DMA_RX_SIZE);
}
@@ -3373,9 +3337,6 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
unsigned int next_entry = rx_q->cur_rx;
int coe = priv->hw->rx_csum;
unsigned int count = 0;
- bool xmac;
-
- xmac = priv->plat->has_gmac4 || priv->plat->has_xgmac;
if (netif_msg_rx_status(priv)) {
void *rx_head;
@@ -3389,11 +3350,12 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
stmmac_display_ring(priv, rx_head, DMA_RX_SIZE, true);
}
while (count < limit) {
+ struct stmmac_rx_buffer *buf;
+ struct dma_desc *np, *p;
int entry, status;
- struct dma_desc *p;
- struct dma_desc *np;
entry = next_entry;
+ buf = &rx_q->buf_pool[entry];
if (priv->extend_desc)
p = (struct dma_desc *)(rx_q->dma_erx + entry);
@@ -3423,20 +3385,9 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
stmmac_rx_extended_status(priv, &priv->dev->stats,
&priv->xstats, rx_q->dma_erx + entry);
if (unlikely(status == discard_frame)) {
+ page_pool_recycle_direct(rx_q->page_pool, buf->page);
priv->dev->stats.rx_errors++;
- if (priv->hwts_rx_en && !priv->extend_desc) {
- /* DESC2 & DESC3 will be overwritten by device
- * with timestamp value, hence reinitialize
- * them in stmmac_rx_refill() function so that
- * device can reuse it.
- */
- dev_kfree_skb_any(rx_q->rx_skbuff[entry]);
- rx_q->rx_skbuff[entry] = NULL;
- dma_unmap_single(priv->device,
- rx_q->rx_skbuff_dma[entry],
- priv->dma_buf_sz,
- DMA_FROM_DEVICE);
- }
+ buf->page = NULL;
} else {
struct sk_buff *skb;
int frame_len;
@@ -3476,58 +3427,20 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
frame_len, status);
}
- /* The zero-copy is always used for all the sizes
- * in case of GMAC4 because it needs
- * to refill the used descriptors, always.
- */
- if (unlikely(!xmac &&
- ((frame_len < priv->rx_copybreak) ||
- stmmac_rx_threshold_count(rx_q)))) {
- skb = netdev_alloc_skb_ip_align(priv->dev,
- frame_len);
- if (unlikely(!skb)) {
- if (net_ratelimit())
- dev_warn(priv->device,
- "packet dropped\n");
- priv->dev->stats.rx_dropped++;
- continue;
- }
-
- dma_sync_single_for_cpu(priv->device,
- rx_q->rx_skbuff_dma
- [entry], frame_len,
- DMA_FROM_DEVICE);
- skb_copy_to_linear_data(skb,
- rx_q->
- rx_skbuff[entry]->data,
- frame_len);
-
- skb_put(skb, frame_len);
- dma_sync_single_for_device(priv->device,
- rx_q->rx_skbuff_dma
- [entry], frame_len,
- DMA_FROM_DEVICE);
- } else {
- skb = rx_q->rx_skbuff[entry];
- if (unlikely(!skb)) {
- if (net_ratelimit())
- netdev_err(priv->dev,
- "%s: Inconsistent Rx chain\n",
- priv->dev->name);
- priv->dev->stats.rx_dropped++;
- continue;
- }
- prefetch(skb->data - NET_IP_ALIGN);
- rx_q->rx_skbuff[entry] = NULL;
- rx_q->rx_zeroc_thresh++;
-
- skb_put(skb, frame_len);
- dma_unmap_single(priv->device,
- rx_q->rx_skbuff_dma[entry],
- priv->dma_buf_sz,
- DMA_FROM_DEVICE);
+ skb = netdev_alloc_skb_ip_align(priv->dev, frame_len);
+ if (unlikely(!skb)) {
+ priv->dev->stats.rx_dropped++;
+ continue;
}
+ dma_sync_single_for_cpu(priv->device, buf->addr,
+ frame_len, DMA_FROM_DEVICE);
+ skb_copy_to_linear_data(skb, page_address(buf->page),
+ frame_len);
+ skb_put(skb, frame_len);
+ dma_sync_single_for_device(priv->device, buf->addr,
+ frame_len, DMA_FROM_DEVICE);
+
if (netif_msg_pktdata(priv)) {
netdev_dbg(priv->dev, "frame received (%dbytes)",
frame_len);
@@ -3547,6 +3460,10 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
napi_gro_receive(&ch->rx_napi, skb);
+ /* Data payload copied into SKB, page ready for recycle */
+ page_pool_recycle_direct(rx_q->page_pool, buf->page);
+ buf->page = NULL;
+
priv->dev->stats.rx_packets++;
priv->dev->stats.rx_bytes += frame_len;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Jesper Dangaard Brouer @ 2019-07-04 15:09 UTC (permalink / raw)
To: Jose Abreu
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Joao Pinto,
David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Maxime Coquelin, Maxime Ripard, Chen-Yu Tsai, brouer
In-Reply-To: <BYAPR12MB326902688C3F40BB3DA6EEEBD3FA0@BYAPR12MB3269.namprd12.prod.outlook.com>
On Thu, 4 Jul 2019 14:45:59 +0000
Jose Abreu <Jose.Abreu@synopsys.com> wrote:
> From: Jesper Dangaard Brouer <brouer@redhat.com>
>
> > The page_pool_request_shutdown() API return indication if there are any
> > in-flight frames/pages, to know when it is safe to call
> > page_pool_free(), which you are also missing a call to.
> >
> > This page_pool_request_shutdown() is only intended to be called from
> > xdp_rxq_info_unreg() code, that handles and schedule a work queue if it
> > need to wait for in-flight frames/pages.
>
> So you mean I can't call it or I should implement the same deferred work
> ?
>
> Notice that in stmmac case there will be no in-flight frames/pages
> because we free them all before calling this ...
You can just use page_pool_free() (p.s I'm working on reintroducing
page_pool_destroy wrapper). As you say, you will not have in-flight
frames/pages in this driver use-case.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: bonded active-backup ethernet-wifi drops packets
From: Brian J. Murrell @ 2019-07-04 15:12 UTC (permalink / raw)
To: netdev
In-Reply-To: <0292e9eefb12f1b1e493f5af8ab78fa00744ed20.camel@interlinx.bc.ca>
[-- Attachment #1: Type: text/plain, Size: 3232 bytes --]
On Tue, 2019-06-18 at 14:57 -0400, Brian J. Murrell wrote:
> Hi.
>
> I have an active-backup bonded connection on a 5.1.6 kernel where the
> slaves are an Ethernet interface and a wifi interface. The goal is
> to
> have network transparent (i.e. same and IP address on both
> interfaces)
> interface which takes advantage of high-speed and low-latency when it
> can be physically plugged into the wired network but have portability
> when unplugged through WiFi.
>
> It all works, mostly. :-/
>
> I find that even when the primary interface, being the Ethernet
> interface is plugged in and active, the bonded interface will drop
> packets periodically.
>
> If I down the bonded interface and plumb the Ethernet interface
> directly, not as a slave of the bonded interface, no such packet
> dropping occurs.
>
> My measure of packet dropping, is by observing the output of "sudo
> ping
> -f <ip_address>. In less than a few minutes even, on the bonded
> interface, even with the Ethernet interface as the active slave, I
> will
> have a long string of dots indicating pings that were never
> replied. On the unbonded Ethernet interface, no dots, even when
> measured over many days.
>
> My bonding config:
>
> $ cat /proc/net/bonding/bond0
> Ethernet Channel Bonding Driver: v3.7.1 (April 27, 2011)
>
> Bonding Mode: fault-tolerance (active-backup)
> Primary Slave: enp0s31f6 (primary_reselect always)
> Currently Active Slave: enp0s31f6
> MII Status: up
> MII Polling Interval (ms): 100
> Up Delay (ms): 0
> Down Delay (ms): 0
>
> Slave Interface: enp0s31f6
> MII Status: up
> Speed: 1000 Mbps
> Duplex: full
> Link Failure Count: 0
> Permanent HW addr: 0c:54:15:4a:b2:0d
> Slave queue ID: 0
>
> Slave Interface: wlp2s0
> MII Status: up
> Speed: Unknown
> Duplex: Unknown
> Link Failure Count: 1
> Permanent HW addr: 0c:54:15:4a:b2:0d
> Slave queue ID: 0
>
> Current interface config/stats:
>
> $ ifconfig bond0
> bond0: flags=5187<UP,BROADCAST,RUNNING,MASTER,MULTICAST> mtu 1500
> inet 10.75.22.245 netmask 255.255.255.0 broadcast
> 10.75.22.255
> inet6 fe80::ee66:b8c9:d55:a28f prefixlen 64 scopeid
> 0x20<link>
> inet6 2001:123:ab:123:d36d:5e5d:acc8:e9bc prefixlen
> 64 scopeid 0x0<global>
> ether 0c:54:15:4a:b2:0d txqueuelen 1000 (Ethernet)
> RX packets 1596206 bytes 165221404 (157.5 MiB)
> RX errors 0 dropped 0 overruns 0 frame 0
> TX packets 1590552 bytes 162689350 (155.1 MiB)
> TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
>
> Devices:
> 00:1f.6 Ethernet controller: Intel Corporation Ethernet Connection
> (2) I219-LM (rev 31)
> 02:00.0 Network controller: Intel Corporation Wireless 8265 / 8275
> (rev 78)
>
> Happy to provide any other useful information.
>
> Any ideas why the dropping, only when using the bonded interface?
Wondering if I have the wrong list with this question. Is there a list
where this question would be more on-topic or focused?
Perhaps I didn't provide enough information? I am happy to provide
whatever is needed. I just don't know what more is needed at this
point.
Cheers,
b.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* RE: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Jose Abreu @ 2019-07-04 15:18 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Jose Abreu
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Joao Pinto,
David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Maxime Coquelin, Maxime Ripard, Chen-Yu Tsai
In-Reply-To: <20190704170920.1e81ed6e@carbon>
From: Jesper Dangaard Brouer <brouer@redhat.com>
> You can just use page_pool_free() (p.s I'm working on reintroducing
> page_pool_destroy wrapper). As you say, you will not have in-flight
> frames/pages in this driver use-case.
Well, if I remove the request_shutdown() it will trigger the "API usage
violation" WARN ...
I think this is due to alloc cache only be freed in request_shutdown(),
or I'm having some leak :D
^ permalink raw reply
* Re: [net-next, PATCH, v2] net: netsec: Sync dma for device on buffer allocation
From: Arnd Bergmann @ 2019-07-04 15:22 UTC (permalink / raw)
To: Ilias Apalodimas; +Cc: Networking, Jassi Brar, Ard Biesheuvel
In-Reply-To: <1562251569-16506-1-git-send-email-ilias.apalodimas@linaro.org>
On Thu, Jul 4, 2019 at 4:46 PM Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
> diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
> index 5544a722543f..ada7626bf3a2 100644
> --- a/drivers/net/ethernet/socionext/netsec.c
>
> + dma_start = page_pool_get_dma_addr(page);
> /* We allocate the same buffer length for XDP and non-XDP cases.
> * page_pool API will map the whole page, skip what's needed for
> * network payloads and/or XDP
> */
> - *dma_handle = page_pool_get_dma_addr(page) + NETSEC_RXBUF_HEADROOM;
> + *dma_handle = dma_start + NETSEC_RXBUF_HEADROOM;
> /* Make sure the incoming payload fits in the page for XDP and non-XDP
> * cases and reserve enough space for headroom + skb_shared_info
> */
> *desc_len = PAGE_SIZE - NETSEC_RX_BUF_NON_DATA;
> + dma_dir = page_pool_get_dma_dir(dring->page_pool);
> + dma_sync_single_for_device(priv->dev, dma_start, PAGE_SIZE, dma_dir);
Should this maybe become part of the page_pool_*() interfaces?
Basically in order to map a page from the pool, any driver would have
to go through these exact steps, so you could make it a combined function
call
dma_addr_t page_pool_sync_for_device(dring->page_pool, page);
Or even fold the page_pool_dev_alloc_pages() into it as well and
make that return both the virtual and dma addresses.
Arnd
^ permalink raw reply
* RE: [PATCH v1 net-next] net: stmmac: enable clause 45 mdio support
From: Jose Abreu @ 2019-07-04 15:27 UTC (permalink / raw)
To: Andrew Lunn, Voon, Weifeng
Cc: David S. Miller, Maxime Coquelin, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Giuseppe Cavallaro,
Florian Fainelli, Alexandre Torgue, biao huang, Ong, Boon Leong,
Kweh, Hock Leong
In-Reply-To: <20190704135420.GD13859@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
> Yes, that is all clear. The stmmac_mdio_c45_setup() does part of this
> setup. There is also a write to mii_address which i snipped out when
> replying. But why do you need to write to the data registers during a
> read? C22 does not need this write. Are there some bits in the top of
> the data register which are relevant to C45?
Yes. The register is 32bits. 16 lower bits are for data and remaining
for C45 register address.
^ permalink raw reply
* RE: [PATCH v1 net-next] net: stmmac: enable clause 45 mdio support
From: Voon, Weifeng @ 2019-07-04 15:29 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S. Miller, Maxime Coquelin, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Jose Abreu, Giuseppe Cavallaro,
Florian Fainelli, Alexandre Torgue, biao huang, Ong, Boon Leong,
Kweh, Hock Leong
In-Reply-To: <20190704135420.GD13859@lunn.ch>
> > > > > > @@ -155,22 +171,26 @@ static int stmmac_mdio_read(struct
> > > > > > mii_bus *bus,
> > > > > int phyaddr, int phyreg)
> > > > > > struct stmmac_priv *priv = netdev_priv(ndev);
> > > > > > unsigned int mii_address = priv->hw->mii.addr;
> > > > > > unsigned int mii_data = priv->hw->mii.data;
> > > > > > - u32 v;
> > > > > > - int data;
> > > > > > u32 value = MII_BUSY;
> > > > > > + int data = 0;
> > > > > > + u32 v;
> > > > > >
> > > > > > value |= (phyaddr << priv->hw->mii.addr_shift)
> > > > > > & priv->hw->mii.addr_mask;
> > > > > > value |= (phyreg << priv->hw->mii.reg_shift) & priv->hw-
> > > > > >mii.reg_mask;
> > > > > > value |= (priv->clk_csr << priv->hw->mii.clk_csr_shift)
> > > > > > & priv->hw->mii.clk_csr_mask;
> > > > > > - if (priv->plat->has_gmac4)
> > > > > > + if (priv->plat->has_gmac4) {
> > > > > > value |= MII_GMAC4_READ;
> > > > > > + if (phyreg & MII_ADDR_C45)
> > > > > > + stmmac_mdio_c45_setup(priv, phyreg, &value,
> &data);
> > > > > > + }
> > > > > >
> > > > > > if (readl_poll_timeout(priv->ioaddr + mii_address,
> v, !(v &
> > > > > MII_BUSY),
> > > > > > 100, 10000))
> > > > > > return -EBUSY;
> > > > > >
> > > > > > + writel(data, priv->ioaddr + mii_data);
> > > > >
> > > > > That looks odd. Could you explain why it is needed.
> > > > >
> > > > > Thanks
> > > > > Andrew
> > > >
> > > > Hi Andrew,
> > > > This mdio c45 support needed to access DWC xPCS which is a
> > > > Clause-45
> > >
> > > I mean it looks odd doing a write to the data register in the middle
> > > of stmmac_mdio_read().
> >
> > MAC is using an indirect access to access mdio devices. In order to
> > read, the driver needs to write into both mii_data and mii_address to
> > select c45, read/write command, phy address, address to read, and etc.
>
> Yes, that is all clear. The stmmac_mdio_c45_setup() does part of this
> setup. There is also a write to mii_address which i snipped out when
> replying. But why do you need to write to the data registers during a
> read? C22 does not need this write. Are there some bits in the top of
> the data register which are relevant to C45?
>
Yes, the top 16 bit of the data register only valid when C45 is enable.
It contains the Register address which MDIO c45 frame intended for.
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Jesper Dangaard Brouer @ 2019-07-04 15:33 UTC (permalink / raw)
To: Jose Abreu
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Joao Pinto,
David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Maxime Coquelin, Maxime Ripard, Chen-Yu Tsai, brouer
In-Reply-To: <BYAPR12MB32692AA2F18A530D56383739D3FA0@BYAPR12MB3269.namprd12.prod.outlook.com>
On Thu, 4 Jul 2019 15:18:19 +0000
Jose Abreu <Jose.Abreu@synopsys.com> wrote:
> From: Jesper Dangaard Brouer <brouer@redhat.com>
>
> > You can just use page_pool_free() (p.s I'm working on reintroducing
> > page_pool_destroy wrapper). As you say, you will not have in-flight
> > frames/pages in this driver use-case.
>
> Well, if I remove the request_shutdown() it will trigger the "API usage
> violation" WARN ...
>
> I think this is due to alloc cache only be freed in request_shutdown(),
> or I'm having some leak :D
Sorry, for not being clear. You of-cause first have to call
page_pool_request_shutdown() and then call page_pool_free().
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH 6/7] nfp: Use spinlock_t instead of struct spinlock
From: Sebastian Andrzej Siewior @ 2019-07-04 15:38 UTC (permalink / raw)
To: linux-kernel
Cc: tglx, Peter Zijlstra, Sebastian Andrzej Siewior, Jakub Kicinski,
David S. Miller, oss-drivers, netdev
In-Reply-To: <20190704153803.12739-1-bigeasy@linutronix.de>
For spinlocks the type spinlock_t should be used instead of "struct
spinlock".
Use spinlock_t for spinlock's definition.
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: oss-drivers@netronome.com
Cc: netdev@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
drivers/net/ethernet/netronome/nfp/nfp_net.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net.h b/drivers/net/ethernet/netronome/nfp/nfp_net.h
index df9aff2684ed0..4690363fc5421 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net.h
@@ -392,7 +392,7 @@ struct nfp_net_r_vector {
struct {
struct tasklet_struct tasklet;
struct sk_buff_head queue;
- struct spinlock lock;
+ spinlock_t lock;
};
};
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next v2 3/3] net: stmmac: Introducing support for Page Pool
From: Jesper Dangaard Brouer @ 2019-07-04 15:37 UTC (permalink / raw)
To: Jose Abreu
Cc: linux-kernel, netdev, linux-stm32, linux-arm-kernel, Joao Pinto,
David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Ilias Apalodimas, Arnd Bergmann, brouer
In-Reply-To: <fd2b12e6fc99f6064b0c04e1baae24328d16289f.1562252534.git.joabreu@synopsys.com>
On Thu, 4 Jul 2019 17:04:14 +0200
Jose Abreu <Jose.Abreu@synopsys.com> wrote:
> @@ -1498,8 +1480,9 @@ static void free_dma_rx_desc_resources(struct stmmac_priv *priv)
> sizeof(struct dma_extended_desc),
> rx_q->dma_erx, rx_q->dma_rx_phy);
>
> - kfree(rx_q->rx_skbuff_dma);
> - kfree(rx_q->rx_skbuff);
> + kfree(rx_q->buf_pool);
> + if (rx_q->page_pool && page_pool_request_shutdown(rx_q->page_pool))
> + page_pool_free(rx_q->page_pool);
> }
> }
This code is okay, but I would likely write it as:
if (rx_q->page_pool) {
page_pool_request_shutdown(rx_q->page_pool));
page_pool_free(rx_q->page_pool);
}
Because (as you noticed) page_pool_free() have some API misuse checks,
that will get triggered, and thus provide a warning of you forget to
update this when driver evolves.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* RE: [PATCH net-next v2 3/3] net: stmmac: Introducing support for Page Pool
From: Jose Abreu @ 2019-07-04 15:40 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Jose Abreu
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Joao Pinto,
David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Ilias Apalodimas, Arnd Bergmann
In-Reply-To: <20190704173758.6d985aa3@carbon>
From: Jesper Dangaard Brouer <brouer@redhat.com>
> This code is okay, but I would likely write it as:
>
> if (rx_q->page_pool) {
> page_pool_request_shutdown(rx_q->page_pool));
> page_pool_free(rx_q->page_pool);
> }
>
> Because (as you noticed) page_pool_free() have some API misuse checks,
> that will get triggered, and thus provide a warning of you forget to
> update this when driver evolves.
Yeah, makes sense. I will update and resend. Thanks for the review!
^ permalink raw reply
* Re: [PATCH v1 net-next] net: stmmac: enable clause 45 mdio support
From: Andrew Lunn @ 2019-07-04 15:52 UTC (permalink / raw)
To: Voon, Weifeng
Cc: David S. Miller, Maxime Coquelin, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Jose Abreu, Giuseppe Cavallaro,
Florian Fainelli, Alexandre Torgue, biao huang, Ong, Boon Leong,
Kweh, Hock Leong
In-Reply-To: <D6759987A7968C4889FDA6FA91D5CBC8147388E0@PGSMSX103.gar.corp.intel.com>
> Yes, the top 16 bit of the data register only valid when C45 is enable.
> It contains the Register address which MDIO c45 frame intended for.
I think there is too much passing variables around by reference than
by value, to make this code easy to understand.
Maybe a better structure would be
static int stmmac_mdion_c45_read(struct stmmac_priv *priv, int phyaddr, int phyreg)
{
unsigned int reg_shift = priv->hw->mii.reg_shift;
unsigned int reg_mask = priv->hw->mii.reg_mask;
u32 mii_addr_val, mii_data_val;
mii_addr_val = MII_GMAC4_C45E |
((phyreg >> MII_DEVADDR_C45_SHIFT) << reg_shift) & reg_mask;
mii_data_val = (phyreg & MII_REGADDR_C45_MASK) << MII_GMAC4_REG_ADDR_SHIFT;
writel(mii_data_val, priv->ioaddr + priv->hw->mii_data);
writel(mii_addr_val, priv->ioaddr + priv->hw->mii_addrress);
return (int)readl(priv->ioaddr + mii_data) & MII_DATA_MASK;
}
static int stmmac_mdio_read(struct mii_bus *bus, int phyaddr, int phyreg)
{
...
if (readl_poll_timeout(priv->ioaddr + mii_address, v, !(v & MII_BUSY),
100, 10000))
return -EBUSY;
if (priv->plat->has_gmac4 && phyreg & MII_ADDR_C45)
return stmmac_mdio_c45_read(priv, phyaddr, phyreg);
Andrew
^ permalink raw reply
* Re: i.mx6ul with DSA in multi chip addressing mode - no MDIO access
From: Andrew Lunn @ 2019-07-04 15:53 UTC (permalink / raw)
To: Benjamin Beckmeyer; +Cc: netdev
In-Reply-To: <00b365da-9c7a-a78a-c10a-f031748e0af7@eks-engel.de>
> &mdio0 {
> interrupt-parent = <&gpio1>;
> interrupts = <3 IRQ_TYPE_LEVEL_HIGH>;
>
> switch0: switch0@2 {
> compatible = "marvell,mv88e6190";
> reg = <2>;
> pinctrl-0 = <&pinctrl_gpios>;
> reset-gpios = <&gpio4 16 GPIO_ACTIVE_LOW>;
> dsa,member = <0 0>;
This is wrong. The interrupt is a switch property, not an MDIO bus
property. So it belongs inside the switch node.
Andrew
^ permalink raw reply
* [PATCH net] ipv4: Fix NULL pointer dereference in ipv4_neigh_lookup()
From: Ido Schimmel @ 2019-07-04 16:26 UTC (permalink / raw)
To: netdev; +Cc: davem, dsahern, jiri, shalomt, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Both ip_neigh_gw4() and ip_neigh_gw6() can return either a valid pointer
or an error pointer, but the code currently checks that the pointer is
not NULL.
Fix this by checking that the pointer is not an error pointer, as this
can result in a NULL pointer dereference [1]. Specifically, I believe
that what happened is that ip_neigh_gw4() returned '-EINVAL'
(0xffffffffffffffea) to which the offset of 'refcnt' (0x70) was added,
which resulted in the address 0x000000000000005a.
[1]
BUG: KASAN: null-ptr-deref in refcount_inc_not_zero_checked+0x6e/0x180
Read of size 4 at addr 000000000000005a by task swapper/2/0
CPU: 2 PID: 0 Comm: swapper/2 Not tainted 5.2.0-rc6-custom-reg-179657-gaa32d89 #396
Hardware name: Mellanox Technologies Ltd. MSN2010/SA002610, BIOS 5.6.5 08/24/2017
Call Trace:
<IRQ>
dump_stack+0x73/0xbb
__kasan_report+0x188/0x1ea
kasan_report+0xe/0x20
refcount_inc_not_zero_checked+0x6e/0x180
ipv4_neigh_lookup+0x365/0x12c0
__neigh_update+0x1467/0x22f0
arp_process.constprop.6+0x82e/0x1f00
__netif_receive_skb_one_core+0xee/0x170
process_backlog+0xe3/0x640
net_rx_action+0x755/0xd90
__do_softirq+0x29b/0xae7
irq_exit+0x177/0x1c0
smp_apic_timer_interrupt+0x164/0x5e0
apic_timer_interrupt+0xf/0x20
</IRQ>
Fixes: 5c9f7c1dfc2e ("ipv4: Add helpers for neigh lookup for nexthop")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reported-by: Shalom Toledo <shalomt@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
net/ipv4/route.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 8ea0735a6754..b2b35b38724d 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -447,7 +447,7 @@ static struct neighbour *ipv4_neigh_lookup(const struct dst_entry *dst,
n = ip_neigh_gw4(dev, pkey);
}
- if (n && !refcount_inc_not_zero(&n->refcnt))
+ if (!IS_ERR(n) && !refcount_inc_not_zero(&n->refcnt))
n = NULL;
rcu_read_unlock_bh();
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2] rtl8xxxu: Fix wifi low signal strength issue of RTL8723BU
From: Jes Sorensen @ 2019-07-04 16:43 UTC (permalink / raw)
To: Chris Chiu, kvalo, davem; +Cc: linux-wireless, netdev, linux-kernel, linux
In-Reply-To: <20190704105528.74028-1-chiu@endlessm.com>
On 7/4/19 6:55 AM, Chris Chiu wrote:
> The WiFi tx power of RTL8723BU is extremely low after booting. So
> the WiFi scan gives very limited AP list and it always fails to
> connect to the selected AP. This module only supports 1x1 antenna
> and the antenna is switched to bluetooth due to some incorrect
> register settings.
>
> Compare with the vendor driver https://github.com/lwfinger/rtl8723bu,
> we realized that the 8723bu's enable_rf() does the same thing as
> rtw_btcoex_HAL_Initialize() in vendor driver. And it by default
> sets the antenna path to BTC_ANT_PATH_BT which we verified it's
> the cause of the wifi weak tx power. The vendor driver will set
> the antenna path to BTC_ANT_PATH_PTA in the consequent btcoexist
> mechanism, by the function halbtc8723b1ant_PsTdma.
>
> This commit hand over the antenna control to PTA(Packet Traffic
> Arbitration), which compares the weight of bluetooth/wifi traffic
> then determine whether to continue current wifi traffic or not.
> After PTA take control, The wifi signal will be back to normal and
> the bluetooth scan can also work at the same time. However, the
> btcoexist still needs to be handled under different circumstances.
> If there's a BT connection established, the wifi still fails to
> connect until BT disconnected.
>
> Signed-off-by: Chris Chiu <chiu@endlessm.com>
> ---
>
>
> Note:
> v2:
> - Replace BIT(11) with the descriptive definition
> - Meaningful comment for the REG_S0S1_PATH_SWITCH setting
>
>
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c | 11 ++++++++---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 3 ++-
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> index 3adb1d3d47ac..ceffe05bd65b 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> @@ -1525,7 +1525,7 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv *priv)
> /*
> * WLAN action by PTA
> */
> - rtl8xxxu_write8(priv, REG_WLAN_ACT_CONTROL_8723B, 0x04);
> + rtl8xxxu_write8(priv, REG_WLAN_ACT_CONTROL_8723B, 0x0c);
>
> /*
> * BT select S0/S1 controlled by WiFi
> @@ -1568,9 +1568,14 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv *priv)
> rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.ant_sel_rsv));
>
> /*
> - * 0x280, 0x00, 0x200, 0x80 - not clear
> + * Different settings per different antenna position.
> + * Antenna Position: | Normal Inverse
> + * --------------------------------------------------
> + * Antenna switch to BT: | 0x280, 0x00
> + * Antenna switch to WiFi: | 0x0, 0x280
> + * Antenna switch to PTA: | 0x200, 0x80
> */
> - rtl8xxxu_write32(priv, REG_S0S1_PATH_SWITCH, 0x00);
> + rtl8xxxu_write32(priv, REG_S0S1_PATH_SWITCH, 0x80);
Per the documentation, shouldn't this be set to 0x200 then rather than 0x80?
We may need to put in place some code to detect whether we have normal
or inverse configuration of the dongle otherwise?
I really appreciate you're digging into this!
Cheers,
Jes
^ permalink raw reply
* [PATCH net-next V2] net: core: page_pool: add user refcnt and reintroduce page_pool_destroy
From: Jesper Dangaard Brouer @ 2019-07-04 16:47 UTC (permalink / raw)
To: netdev, Ilias Apalodimas, ivan.khoronzhuk, Jesper Dangaard Brouer
Cc: grygorii.strashko, jakub.kicinski, daniel, john.fastabend, ast,
linux-kernel, linux-omap
Jesper recently removed page_pool_destroy() (from driver invocation) and
moved shutdown and free of page_pool into xdp_rxq_info_unreg(), in-order to
handle in-flight packets/pages. This created an asymmetry in drivers
create/destroy pairs.
This patch reintroduce page_pool_destroy and add page_pool user refcnt.
This serves the purpose to simplify drivers error handling as driver now
drivers always calls page_pool_destroy() and don't need to track if
xdp_rxq_info_reg_mem_model() was unsuccessful.
This could be used for a special cases where a single RX-queue (with a single
page_pool) provides packets for two net_device'es, and thus needs to register
the same page_pool twice with two xdp_rxq_info structures. This use-case is
explicitly denied in this V2 patch, as it needs more discussion upstream.
This patch is primarily to ease API usage for drivers. The recently merged
netsec driver, actually have a bug in this area, which is solved by this API
change.
This patch is a modified version of Ivan Khoronzhu's original patch.
Link: https://lore.kernel.org/netdev/20190625175948.24771-2-ivan.khoronzhuk@linaro.org/
Fixes: 5c67bf0ec4d0 ("net: netsec: Use page_pool API")
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
I hope we can agree on this first step. Afterwards we can discuss, which is the
best approach for Ivan's two netdev's with one RX-queue API change.
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 6 ++---
drivers/net/ethernet/socionext/netsec.c | 8 ++----
include/net/page_pool.h | 26 +++++++++++++++++++++
net/core/page_pool.c | 8 ++++++
net/core/xdp.c | 9 +++++++
5 files changed, 48 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 1085040675ae..ce1c7a449eae 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -545,10 +545,8 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
}
err = xdp_rxq_info_reg_mem_model(&rq->xdp_rxq,
MEM_TYPE_PAGE_POOL, rq->page_pool);
- if (err) {
- page_pool_free(rq->page_pool);
+ if (err)
goto err_free;
- }
for (i = 0; i < wq_sz; i++) {
if (rq->wq_type == MLX5_WQ_TYPE_LINKED_LIST_STRIDING_RQ) {
@@ -613,6 +611,7 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
if (rq->xdp_prog)
bpf_prog_put(rq->xdp_prog);
xdp_rxq_info_unreg(&rq->xdp_rxq);
+ page_pool_destroy(rq->page_pool);
mlx5_wq_destroy(&rq->wq_ctrl);
return err;
@@ -643,6 +642,7 @@ static void mlx5e_free_rq(struct mlx5e_rq *rq)
}
xdp_rxq_info_unreg(&rq->xdp_rxq);
+ page_pool_destroy(rq->page_pool);
mlx5_wq_destroy(&rq->wq_ctrl);
}
diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index 5544a722543f..43ab0ce90704 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -1210,15 +1210,11 @@ static void netsec_uninit_pkt_dring(struct netsec_priv *priv, int id)
}
}
- /* Rx is currently using page_pool
- * since the pool is created during netsec_setup_rx_dring(), we need to
- * free the pool manually if the registration failed
- */
+ /* Rx is currently using page_pool */
if (id == NETSEC_RING_RX) {
if (xdp_rxq_info_is_reg(&dring->xdp_rxq))
xdp_rxq_info_unreg(&dring->xdp_rxq);
- else
- page_pool_free(dring->page_pool);
+ page_pool_destroy(dring->page_pool);
}
memset(dring->desc, 0, sizeof(struct netsec_desc) * DESC_NUM);
diff --git a/include/net/page_pool.h b/include/net/page_pool.h
index ee9c871d2043..0a0984d8f5d5 100644
--- a/include/net/page_pool.h
+++ b/include/net/page_pool.h
@@ -101,6 +101,12 @@ struct page_pool {
struct ptr_ring ring;
atomic_t pages_state_release_cnt;
+
+ /* A page_pool is strictly tied to a single RX-queue being
+ * protected by NAPI, due to above pp_alloc_cache. This
+ * refcnt serves purpose is to simplify drivers error handling.
+ */
+ refcount_t user_cnt;
};
struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp);
@@ -134,6 +140,15 @@ static inline void page_pool_free(struct page_pool *pool)
#endif
}
+/* Drivers use this instead of page_pool_free */
+static inline void page_pool_destroy(struct page_pool *pool)
+{
+ if (!pool)
+ return;
+
+ page_pool_free(pool);
+}
+
/* Never call this directly, use helpers below */
void __page_pool_put_page(struct page_pool *pool,
struct page *page, bool allow_direct);
@@ -201,4 +216,15 @@ static inline bool is_page_pool_compiled_in(void)
#endif
}
+static inline unsigned int page_pool_get(struct page_pool *pool)
+{
+ refcount_inc(&pool->user_cnt);
+ return refcount_read(&pool->user_cnt);
+}
+
+static inline bool page_pool_put(struct page_pool *pool)
+{
+ return refcount_dec_and_test(&pool->user_cnt);
+}
+
#endif /* _NET_PAGE_POOL_H */
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index b366f59885c1..3272dc7a8c81 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -49,6 +49,9 @@ static int page_pool_init(struct page_pool *pool,
atomic_set(&pool->pages_state_release_cnt, 0);
+ /* Driver calling page_pool_create() also call page_pool_destroy() */
+ refcount_set(&pool->user_cnt, 1);
+
if (pool->p.flags & PP_FLAG_DMA_MAP)
get_device(pool->p.dev);
@@ -70,6 +73,7 @@ struct page_pool *page_pool_create(const struct page_pool_params *params)
kfree(pool);
return ERR_PTR(err);
}
+
return pool;
}
EXPORT_SYMBOL(page_pool_create);
@@ -356,6 +360,10 @@ static void __warn_in_flight(struct page_pool *pool)
void __page_pool_free(struct page_pool *pool)
{
+ /* Only last user actually free/release resources */
+ if (!page_pool_put(pool))
+ return;
+
WARN(pool->alloc.count, "API usage violation");
WARN(!ptr_ring_empty(&pool->ring), "ptr_ring is not empty");
diff --git a/net/core/xdp.c b/net/core/xdp.c
index b29d7b513a18..57c2317d9ce5 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -370,6 +370,15 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
goto err;
}
+ if (type == MEM_TYPE_PAGE_POOL) {
+ if (page_pool_get(xdp_alloc->page_pool) > 2) {
+ WARN(1, "API misuse, argue to enable use-case");
+ page_pool_put(xdp_alloc->page_pool);
+ errno = -EINVAL;
+ goto err;
+ }
+ }
+
mutex_unlock(&mem_id_lock);
trace_mem_connect(xdp_alloc, xdp_rxq);
^ permalink raw reply related
* Re: [PATCH mlx5-next 4/5] net/mlx5: Introduce TLS TX offload hardware bits and structures
From: Saeed Mahameed @ 2019-07-04 17:06 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Saeed Mahameed, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, Eran Ben Elisha, Tariq Toukan
In-Reply-To: <20190703092735.GZ4727@mtr-leonro.mtl.com>
On Wed, Jul 3, 2019 at 5:27 AM <leon@kernel.org> wrote:
>
> On Wed, Jul 03, 2019 at 07:39:32AM +0000, Saeed Mahameed wrote:
> > From: Eran Ben Elisha <eranbe@mellanox.com>
> >
> > Add TLS offload related IFC structs, layouts and enumerations.
> >
> > Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
> > Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> > ---
> > include/linux/mlx5/device.h | 14 +++++
> > include/linux/mlx5/mlx5_ifc.h | 104 ++++++++++++++++++++++++++++++++--
> > 2 files changed, 114 insertions(+), 4 deletions(-)
>
> <...>
>
> > @@ -2725,7 +2739,8 @@ struct mlx5_ifc_traffic_counter_bits {
> >
> > struct mlx5_ifc_tisc_bits {
> > u8 strict_lag_tx_port_affinity[0x1];
> > - u8 reserved_at_1[0x3];
> > + u8 tls_en[0x1];
> > + u8 reserved_at_1[0x2];
>
> It should be reserved_at_2.
>
it should be at_1.
> Thanks
^ permalink raw reply
* Re: [PATCH mlx5-next 0/5] Mellanox, mlx5 low level updates 2019-07-02
From: Saeed Mahameed @ 2019-07-04 17:10 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org
In-Reply-To: <20190703073909.14965-1-saeedm@mellanox.com>
On Wed, 2019-07-03 at 07:39 +0000, Saeed Mahameed wrote:
> Hi All,
>
> This series includes some low level updates to mlx5 driver, required
> for
> shared mlx5-next branch.
>
> Tariq extends the WQE control fields names.
> Eran adds the required HW definitions and structures for upcoming TLS
> support.
> Parav improves and refactors the E-Switch "function changed" handler.
>
> In case of no objections these patches will be applied to mlx5-next
> and
> will be sent later as pull request to both rdma-next and net-next
> trees.
>
> Thanks,
> Saeed.
Applied to mlx5-next.
^ permalink raw reply
* Re: [PATCH v6 net-next 1/5] xdp: allow same allocator usage
From: Ivan Khoronzhuk @ 2019-07-04 17:11 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: grygorii.strashko, davem, ast, linux-kernel, linux-omap,
xdp-newbies, ilias.apalodimas, netdev, daniel, jakub.kicinski,
john.fastabend
In-Reply-To: <20190704144144.5edd18eb@carbon>
On Thu, Jul 04, 2019 at 02:41:44PM +0200, Jesper Dangaard Brouer wrote:
>On Thu, 4 Jul 2019 13:22:40 +0300
>Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>
>> On Wed, Jul 03, 2019 at 07:40:13PM +0200, Jesper Dangaard Brouer wrote:
>> >On Wed, 3 Jul 2019 13:18:59 +0300
>> >Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>> >
>> >> First of all, it is an absolute requirement that each RX-queue have
>> >> their own page_pool object/allocator. And this change is intendant
>> >> to handle special case, where a single RX-queue can receive packets
>> >> from two different net_devices.
>> >>
>> >> In order to protect against using same allocator for 2 different rx
>> >> queues, add queue_index to xdp_mem_allocator to catch the obvious
>> >> mistake where queue_index mismatch, as proposed by Jesper Dangaard
>> >> Brouer.
>> >>
>> >> Adding this on xdp allocator level allows drivers with such dependency
>> >> change the allocators w/o modifications.
>> >>
>> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> >> ---
>> >> include/net/xdp_priv.h | 2 ++
>> >> net/core/xdp.c | 55 ++++++++++++++++++++++++++++++++++++++++++
>> >> 2 files changed, 57 insertions(+)
>> >>
>> >> diff --git a/include/net/xdp_priv.h b/include/net/xdp_priv.h
>> >> index 6a8cba6ea79a..9858a4057842 100644
>> >> --- a/include/net/xdp_priv.h
>> >> +++ b/include/net/xdp_priv.h
>> >> @@ -18,6 +18,8 @@ struct xdp_mem_allocator {
>> >> struct rcu_head rcu;
>> >> struct delayed_work defer_wq;
>> >> unsigned long defer_warn;
>> >> + unsigned long refcnt;
>> >> + u32 queue_index;
>> >> };
>> >
>> >I don't like this approach, because I think we need to extend struct
>> >xdp_mem_allocator with a net_device pointer, for doing dev_hold(), to
>> >correctly handle lifetime issues. (As I tried to explain previously).
>> >This will be much harder after this change, which is why I proposed the
>> >other patch.
>> My concern comes not from zero also.
>> It's partly continuation of not answered questions from here:
>> https://lwn.net/ml/netdev/20190625122822.GC6485@khorivan/
>>
>> "For me it's important to know only if it means that alloc.count is
>> freed at first call of __mem_id_disconnect() while shutdown.
>> The workqueue for the rest is connected only with ring cache protected
>> by ring lock and not supposed that alloc.count can be changed while
>> workqueue tries to shutdonwn the pool."
>
>Yes. The alloc.count is only freed on first call. I considered
>changing the shutdown API, to have two shutdown calls, where the call
>used from the work-queue will not have the loop emptying alloc.count,
>but instead have a WARN_ON(alloc.count), as it MUST be empty (once is
>code running from work-queue).
>
>> So patch you propose to leave works only because of luck, because fast
>> cache is cleared before workqueue is scheduled and no races between two
>> workqueues for fast cache later. I'm not really against this patch, but
>> I have to try smth better.
>
>It is not "luck". It does the correct thing as we never enter the
>while loop in __page_pool_request_shutdown() from a work-queue, but it
>is not obvious from the code. The not-so-nice thing is that two
>work-queue shutdowns will be racing with each-other, in the multi
>netdev use-case, but access to the ptr_ring is safe/locked.
So, having this, and being prudent to generic code changes, lets roll back
to idea from v.4:
https://lkml.org/lkml/2019/6/25/996
but use changes from following patch, reintroducing page destroy:
https://www.spinics.net/lists/netdev/msg583145.html
with appropriate small modifications for cpsw.
In case of some issue connected with it (not supposed), or two/more
allocators used by cpsw, or one more driver having such multi ndev
capabilities (supposed), would be nice to use this link as reference
and it can be base for similar modifications.
Unless Jesper disagrees with this ofc.
I will send v7 soon after verification is completed.
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH 1/1] tools/dtrace: initial implementation of DTrace
From: Brendan Gregg @ 2019-07-04 17:13 UTC (permalink / raw)
To: Kris Van Hees
Cc: netdev, bpf, dtrace-devel, LKML, Steven Rostedt, Masami Hiramatsu,
Arnaldo Carvalho de Melo, Alexei Starovoitov, Daniel Borkmann,
Peter Zijlstra, Chris Mason
In-Reply-To: <201907040314.x643EUoA017906@aserv0122.oracle.com>
On Wed, Jul 3, 2019 at 8:17 PM Kris Van Hees <kris.van.hees@oracle.com> wrote:
>
> This initial implementation of a tiny subset of DTrace functionality
> provides the following options:
>
> dtrace [-lvV] [-b bufsz] -s script
> -b set trace buffer size
> -l list probes (only works with '-s script' for now)
> -s enable or list probes for the specified BPF program
> -V report DTrace API version
>
> The patch comprises quite a bit of code due to DTrace requiring a few
> crucial components, even in its most basic form.
This patchset has moved from adding tests (which actually belong in
selftests, not tools/dtrace), to the start of adding a giant tracer to
the kernel code base.
First, in some ways you're doing this for me -- I've been the number
one user of DTrace for 15 years -- and thanks, but no thanks. I don't
need this anymore. I needed this 6 years ago, and I would have helped
you build it, but in the meantime Linux has built something better,
built from the ground up for BPF: bpftrace.
Second, you argued that DTrace was needed because of speculative
tracing (a feature I never used), as you had customers who wanted it.
Those customers aren't going to be happy with this initial tiny
implementation of DTrace -- this is really the start of adding a large
and complex tracer to the kernel.
We've all been working under the assumption that these user-space
tracers did not belong in the kernel, and so far that's been working
fine for us. Is it now open season for tracers in the kernel? Let's
have:
tools/bpftrace
tools/ply
tools/systemtap
tools/LTTng
tools/sysdig
tools/ktap
etc
Yes, that's ridiculous. If there's only going to be one, let's have
the best one, bpftrace. We'll offer a version that is GPL, C, and has
no dependencies.
But it would be news to us all that we're allowed to have even one.
There are more things that don't make sense about this, but I'll stop
here for now.
Brendan
^ permalink raw reply
* Re: [PATCH net-next V2] net: core: page_pool: add user refcnt and reintroduce page_pool_destroy
From: Ivan Khoronzhuk @ 2019-07-04 17:13 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, Ilias Apalodimas, grygorii.strashko, jakub.kicinski,
daniel, john.fastabend, ast, linux-kernel, linux-omap
In-Reply-To: <156225871578.1603.6630229522953924907.stgit@firesoul>
On Thu, Jul 04, 2019 at 06:47:07PM +0200, Jesper Dangaard Brouer wrote:
Have trouble with inet today...I will pick up it as my changes depend on it.
And send probably in couple hours after verification.
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH mlx5-next 4/5] net/mlx5: Introduce TLS TX offload hardware bits and structures
From: Leon Romanovsky @ 2019-07-04 17:15 UTC (permalink / raw)
To: Saeed Mahameed
Cc: Saeed Mahameed, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, Eran Ben Elisha, Tariq Toukan
In-Reply-To: <CALzJLG-em1w+Lgf2UutbG2Lzq8bx3zUqoLGx26H2_EXOuuk+jg@mail.gmail.com>
On Thu, Jul 04, 2019 at 01:06:58PM -0400, Saeed Mahameed wrote:
> On Wed, Jul 3, 2019 at 5:27 AM <leon@kernel.org> wrote:
> >
> > On Wed, Jul 03, 2019 at 07:39:32AM +0000, Saeed Mahameed wrote:
> > > From: Eran Ben Elisha <eranbe@mellanox.com>
> > >
> > > Add TLS offload related IFC structs, layouts and enumerations.
> > >
> > > Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
> > > Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> > > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> > > ---
> > > include/linux/mlx5/device.h | 14 +++++
> > > include/linux/mlx5/mlx5_ifc.h | 104 ++++++++++++++++++++++++++++++++--
> > > 2 files changed, 114 insertions(+), 4 deletions(-)
> >
> > <...>
> >
> > > @@ -2725,7 +2739,8 @@ struct mlx5_ifc_traffic_counter_bits {
> > >
> > > struct mlx5_ifc_tisc_bits {
> > > u8 strict_lag_tx_port_affinity[0x1];
> > > - u8 reserved_at_1[0x3];
> > > + u8 tls_en[0x1];
> > > + u8 reserved_at_1[0x2];
> >
> > It should be reserved_at_2.
> >
>
> it should be at_1.
Why? See mlx5_ifc_flow_table_prop_layout_bits, mlx5_ifc_roce_cap_bits, e.t.c.
Thanks
>
> > Thanks
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox