* Re: [PATCH v3 net 2/5] tcp: prevent bogus FRTO undos with non-SACK flows
From: Yuchung Cheng @ 2018-03-28 22:26 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: netdev, Neal Cardwell, Eric Dumazet, Sergei Shtylyov
In-Reply-To: <1520936711-16784-3-git-send-email-ilpo.jarvinen@helsinki.fi>
On Tue, Mar 13, 2018 at 3:25 AM, Ilpo Järvinen
<ilpo.jarvinen@helsinki.fi> wrote:
>
> If SACK is not enabled and the first cumulative ACK after the RTO
> retransmission covers more than the retransmitted skb, a spurious
> FRTO undo will trigger (assuming FRTO is enabled for that RTO).
> The reason is that any non-retransmitted segment acknowledged will
> set FLAG_ORIG_SACK_ACKED in tcp_clean_rtx_queue even if there is
> no indication that it would have been delivered for real (the
> scoreboard is not kept with TCPCB_SACKED_ACKED bits in the non-SACK
> case so the check for that bit won't help like it does with SACK).
> Having FLAG_ORIG_SACK_ACKED set results in the spurious FRTO undo
> in tcp_process_loss.
>
> We need to use more strict condition for non-SACK case and check
> that none of the cumulatively ACKed segments were retransmitted
> to prove that progress is due to original transmissions. Only then
> keep FLAG_ORIG_SACK_ACKED set, allowing FRTO undo to proceed in
> non-SACK case.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> ---
> net/ipv4/tcp_input.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 4a26c09..c60745c 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -3166,6 +3166,15 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> pkts_acked = rexmit_acked + newdata_acked;
>
> tcp_remove_reno_sacks(sk, pkts_acked);
> +
> + /* If any of the cumulatively ACKed segments was
> + * retransmitted, non-SACK case cannot confirm that
> + * progress was due to original transmission due to
> + * lack of TCPCB_SACKED_ACKED bits even if some of
> + * the packets may have been never retransmitted.
> + */
> + if (flag & FLAG_RETRANS_DATA_ACKED)
> + flag &= ~FLAG_ORIG_SACK_ACKED;
How about keeping your excellent comment but move the fix to F-RTO
code directly so it's more clear? this way the flag remains clear that
indicates some never-retransmitted data are acked/sacked.
// pseudo code for illustration
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 8d480542aa07..f7f3357de618 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2629,8 +2629,15 @@ static void tcp_process_loss(struct sock *sk,
int flag, bool is_dupack,
if (tp->frto) { /* F-RTO RFC5682 sec 3.1 (sack enhanced version). */
/* Step 3.b. A timeout is spurious if not all data are
* lost, i.e., never-retransmitted data are (s)acked.
+ *
+ * If any of the cumulatively ACKed segments was
+ * retransmitted, non-SACK case cannot confirm that
+ * progress was due to original transmission due to
+ * lack of TCPCB_SACKED_ACKED bits even if some of
+ * the packets may have been never retransmitted.
*/
if ((flag & FLAG_ORIG_SACK_ACKED) &&
+ (tcp_is_sack(tp) || !FLAG_RETRANS_DATA_ACKED) &&
tcp_try_undo_loss(sk, true))
return;
> } else {
> int delta;
>
> --
> 2.7.4
>
^ permalink raw reply related
* [PATCH net-next v2 3/3] net: bcmgenet: Fix coalescing settings handling
From: Florian Fainelli @ 2018-03-28 22:15 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, davem, jaedon.shin, pgynther, opendmb,
Michael Chan, gospo, talgi, saeedm
In-Reply-To: <20180328221538.29290-1-f.fainelli@gmail.com>
There were a number of issues with setting the RX coalescing parameters:
- we would not be preserving values that would have been configured
across close/open calls, instead we would always reset to no timeout
and 1 interrupt per packet, this would also prevent DIM from setting its
default usec/pkts values
- when adaptive RX would be turned on, we woud not be fetching the
default parameters, we would stay with no timeout/1 packet per interrupt
until the estimator kicks in and changes that
- finally disabling adaptive RX coalescing while providing parameters
would not be honored, and we would stay with whatever DIM had previously
determined instead of the user requested parameters
Fixes: 9f4ca05827a2 ("net: bcmgenet: Add support for adaptive RX coalescing")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 92 +++++++++++++++++---------
drivers/net/ethernet/broadcom/genet/bcmgenet.h | 4 +-
2 files changed, 61 insertions(+), 35 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 7db8edc643ec..f8af472f324f 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -625,26 +625,46 @@ static int bcmgenet_get_coalesce(struct net_device *dev,
return 0;
}
-static void bcmgenet_set_rx_coalesce(struct bcmgenet_rx_ring *ring)
+static void bcmgenet_set_rx_coalesce(struct bcmgenet_rx_ring *ring,
+ u32 usecs, u32 pkts)
{
struct bcmgenet_priv *priv = ring->priv;
unsigned int i = ring->index;
u32 reg;
- bcmgenet_rdma_ring_writel(priv, i, ring->dim.coal_pkts,
- DMA_MBUF_DONE_THRESH);
+ bcmgenet_rdma_ring_writel(priv, i, pkts, DMA_MBUF_DONE_THRESH);
reg = bcmgenet_rdma_readl(priv, DMA_RING0_TIMEOUT + i);
reg &= ~DMA_TIMEOUT_MASK;
- reg |= DIV_ROUND_UP(ring->dim.coal_usecs * 1000, 8192);
+ reg |= DIV_ROUND_UP(usecs * 1000, 8192);
bcmgenet_rdma_writel(priv, reg, DMA_RING0_TIMEOUT + i);
}
+static void bcmgenet_set_ring_rx_coalesce(struct bcmgenet_rx_ring *ring,
+ struct ethtool_coalesce *ec)
+{
+ struct net_dim_cq_moder moder;
+ u32 usecs, pkts;
+
+ ring->rx_coalesce_usecs = ec->rx_coalesce_usecs;
+ ring->rx_max_coalesced_frames = ec->rx_max_coalesced_frames;
+ usecs = ring->rx_coalesce_usecs;
+ pkts = ring->rx_max_coalesced_frames;
+
+ if (ec->use_adaptive_rx_coalesce && !ring->dim.use_dim) {
+ moder = net_dim_get_def_profile(ring->dim.dim.mode);
+ usecs = moder.usec;
+ pkts = moder.pkts;
+ }
+
+ ring->dim.use_dim = ec->use_adaptive_rx_coalesce;
+ bcmgenet_set_rx_coalesce(ring, usecs, pkts);
+}
+
static int bcmgenet_set_coalesce(struct net_device *dev,
struct ethtool_coalesce *ec)
{
struct bcmgenet_priv *priv = netdev_priv(dev);
- struct bcmgenet_rx_ring *ring;
unsigned int i;
/* Base system clock is 125Mhz, DMA timeout is this reference clock
@@ -680,27 +700,9 @@ static int bcmgenet_set_coalesce(struct net_device *dev,
ec->tx_max_coalesced_frames,
DMA_MBUF_DONE_THRESH);
- for (i = 0; i < priv->hw_params->rx_queues; i++) {
- ring = &priv->rx_rings[i];
- ring->dim.coal_usecs = ec->rx_coalesce_usecs;
- ring->dim.coal_pkts = ec->rx_max_coalesced_frames;
- if (!ec->use_adaptive_rx_coalesce && ring->dim.use_dim) {
- ring->dim.coal_pkts = 1;
- ring->dim.coal_usecs = 0;
- }
- ring->dim.use_dim = ec->use_adaptive_rx_coalesce;
- bcmgenet_set_rx_coalesce(ring);
- }
-
- ring = &priv->rx_rings[DESC_INDEX];
- ring->dim.coal_usecs = ec->rx_coalesce_usecs;
- ring->dim.coal_pkts = ec->rx_max_coalesced_frames;
- if (!ec->use_adaptive_rx_coalesce && ring->dim.use_dim) {
- ring->dim.coal_pkts = 1;
- ring->dim.coal_usecs = 0;
- }
- ring->dim.use_dim = ec->use_adaptive_rx_coalesce;
- bcmgenet_set_rx_coalesce(ring);
+ for (i = 0; i < priv->hw_params->rx_queues; i++)
+ bcmgenet_set_ring_rx_coalesce(&priv->rx_rings[i], ec);
+ bcmgenet_set_ring_rx_coalesce(&priv->rx_rings[DESC_INDEX], ec);
return 0;
}
@@ -1924,10 +1926,7 @@ static void bcmgenet_dim_work(struct work_struct *work)
struct net_dim_cq_moder cur_profile =
net_dim_get_profile(dim->mode, dim->profile_ix);
- ring->dim.coal_usecs = cur_profile.usec;
- ring->dim.coal_pkts = cur_profile.pkts;
-
- bcmgenet_set_rx_coalesce(ring);
+ bcmgenet_set_rx_coalesce(ring, cur_profile.usec, cur_profile.pkts);
dim->state = NET_DIM_START_MEASURE;
}
@@ -2079,9 +2078,11 @@ static void init_umac(struct bcmgenet_priv *priv)
dev_dbg(kdev, "done init umac\n");
}
-static void bcmgenet_init_dim(struct bcmgenet_net_dim *dim,
+static void bcmgenet_init_dim(struct bcmgenet_rx_ring *ring,
void (*cb)(struct work_struct *work))
{
+ struct bcmgenet_net_dim *dim = &ring->dim;
+
INIT_WORK(&dim->dim.work, cb);
dim->dim.mode = NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
dim->event_ctr = 0;
@@ -2089,6 +2090,25 @@ static void bcmgenet_init_dim(struct bcmgenet_net_dim *dim,
dim->bytes = 0;
}
+static void bcmgenet_init_rx_coalesce(struct bcmgenet_rx_ring *ring)
+{
+ struct bcmgenet_net_dim *dim = &ring->dim;
+ struct net_dim_cq_moder moder;
+ u32 usecs, pkts;
+
+ usecs = ring->rx_coalesce_usecs;
+ pkts = ring->rx_max_coalesced_frames;
+
+ /* If DIM was enabled, re-apply default parameters */
+ if (dim->use_dim) {
+ moder = net_dim_get_def_profile(dim->dim.mode);
+ usecs = moder.usec;
+ pkts = moder.pkts;
+ }
+
+ bcmgenet_set_rx_coalesce(ring, usecs, pkts);
+}
+
/* Initialize a Tx ring along with corresponding hardware registers */
static void bcmgenet_init_tx_ring(struct bcmgenet_priv *priv,
unsigned int index, unsigned int size,
@@ -2178,7 +2198,8 @@ static int bcmgenet_init_rx_ring(struct bcmgenet_priv *priv,
if (ret)
return ret;
- bcmgenet_init_dim(&ring->dim, bcmgenet_dim_work);
+ bcmgenet_init_dim(ring, bcmgenet_dim_work);
+ bcmgenet_init_rx_coalesce(ring);
/* Initialize Rx NAPI */
netif_napi_add(priv->dev, &ring->napi, bcmgenet_rx_poll,
@@ -2186,7 +2207,6 @@ static int bcmgenet_init_rx_ring(struct bcmgenet_priv *priv,
bcmgenet_rdma_ring_writel(priv, index, 0, RDMA_PROD_INDEX);
bcmgenet_rdma_ring_writel(priv, index, 0, RDMA_CONS_INDEX);
- bcmgenet_rdma_ring_writel(priv, index, 1, DMA_MBUF_DONE_THRESH);
bcmgenet_rdma_ring_writel(priv, index,
((size << DMA_RING_SIZE_SHIFT) |
RX_BUF_LENGTH), DMA_RING_BUF_SIZE);
@@ -3424,6 +3444,7 @@ static int bcmgenet_probe(struct platform_device *pdev)
struct net_device *dev;
const void *macaddr;
struct resource *r;
+ unsigned int i;
int err = -EIO;
const char *phy_mode_str;
@@ -3552,6 +3573,11 @@ static int bcmgenet_probe(struct platform_device *pdev)
netif_set_real_num_tx_queues(priv->dev, priv->hw_params->tx_queues + 1);
netif_set_real_num_rx_queues(priv->dev, priv->hw_params->rx_queues + 1);
+ /* Set default coalescing parameters */
+ for (i = 0; i < priv->hw_params->rx_queues; i++)
+ priv->rx_rings[i].rx_max_coalesced_frames = 1;
+ priv->rx_rings[DESC_INDEX].rx_max_coalesced_frames = 1;
+
/* libphy will determine the link state */
netif_carrier_off(dev);
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 22c41e0430fb..b773bc07edf7 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -578,8 +578,6 @@ struct bcmgenet_net_dim {
u16 event_ctr;
unsigned long packets;
unsigned long bytes;
- u32 coal_usecs;
- u32 coal_pkts;
struct net_dim dim;
};
@@ -598,6 +596,8 @@ struct bcmgenet_rx_ring {
unsigned int end_ptr; /* Rx ring end CB ptr */
unsigned int old_discards;
struct bcmgenet_net_dim dim;
+ u32 rx_max_coalesced_frames;
+ u32 rx_coalesce_usecs;
void (*int_enable)(struct bcmgenet_rx_ring *);
void (*int_disable)(struct bcmgenet_rx_ring *);
struct bcmgenet_priv *priv;
--
2.14.1
^ permalink raw reply related
* [PATCH net-next v2 2/3] net: systemport: Fix coalescing settings handling
From: Florian Fainelli @ 2018-03-28 22:15 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, davem, jaedon.shin, pgynther, opendmb,
Michael Chan, gospo, talgi, saeedm
In-Reply-To: <20180328221538.29290-1-f.fainelli@gmail.com>
There were a number of issues with setting the RX coalescing parameters:
- we would not be preserving values that would have been configured
across close/open calls, instead we would always reset to no timeout
and 1 interrupt per packet, this would also prevent DIM from setting its
default usec/pkts values
- when adaptive RX would be turned on, we woud not be fetching the
default parameters, we would stay with no timeout/1 packet per
interrupt until the estimator kicks in and changes that
- finally disabling adaptive RX coalescing while providing parameters
would not be honored, and we would stay with whatever DIM had
previously determined instead of the user requested parameters
Fixes: b6e0e875421e ("net: systemport: Implement adaptive interrupt coalescing")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 62 +++++++++++++++++++++---------
drivers/net/ethernet/broadcom/bcmsysport.h | 4 +-
2 files changed, 46 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 1e52bb7d822e..4a75b1de22e0 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -574,16 +574,16 @@ static int bcm_sysport_set_wol(struct net_device *dev,
return 0;
}
-static void bcm_sysport_set_rx_coalesce(struct bcm_sysport_priv *priv)
+static void bcm_sysport_set_rx_coalesce(struct bcm_sysport_priv *priv,
+ u32 usecs, u32 pkts)
{
u32 reg;
reg = rdma_readl(priv, RDMA_MBDONE_INTR);
reg &= ~(RDMA_INTR_THRESH_MASK |
RDMA_TIMEOUT_MASK << RDMA_TIMEOUT_SHIFT);
- reg |= priv->dim.coal_pkts;
- reg |= DIV_ROUND_UP(priv->dim.coal_usecs * 1000, 8192) <<
- RDMA_TIMEOUT_SHIFT;
+ reg |= pkts;
+ reg |= DIV_ROUND_UP(usecs * 1000, 8192) << RDMA_TIMEOUT_SHIFT;
rdma_writel(priv, reg, RDMA_MBDONE_INTR);
}
@@ -626,6 +626,8 @@ static int bcm_sysport_set_coalesce(struct net_device *dev,
struct ethtool_coalesce *ec)
{
struct bcm_sysport_priv *priv = netdev_priv(dev);
+ struct net_dim_cq_moder moder;
+ u32 usecs, pkts;
unsigned int i;
/* Base system clock is 125Mhz, DMA timeout is this reference clock
@@ -646,15 +648,21 @@ static int bcm_sysport_set_coalesce(struct net_device *dev,
for (i = 0; i < dev->num_tx_queues; i++)
bcm_sysport_set_tx_coalesce(&priv->tx_rings[i], ec);
- priv->dim.coal_usecs = ec->rx_coalesce_usecs;
- priv->dim.coal_pkts = ec->rx_max_coalesced_frames;
+ priv->rx_coalesce_usecs = ec->rx_coalesce_usecs;
+ priv->rx_max_coalesced_frames = ec->rx_max_coalesced_frames;
+ usecs = priv->rx_coalesce_usecs;
+ pkts = priv->rx_max_coalesced_frames;
- if (!ec->use_adaptive_rx_coalesce && priv->dim.use_dim) {
- priv->dim.coal_pkts = 1;
- priv->dim.coal_usecs = 0;
+ if (ec->use_adaptive_rx_coalesce && !priv->dim.use_dim) {
+ moder = net_dim_get_def_profile(priv->dim.dim.mode);
+ usecs = moder.usec;
+ pkts = moder.pkts;
}
+
priv->dim.use_dim = ec->use_adaptive_rx_coalesce;
- bcm_sysport_set_rx_coalesce(priv);
+
+ /* Apply desired coalescing parameters */
+ bcm_sysport_set_rx_coalesce(priv, usecs, pkts);
return 0;
}
@@ -1058,10 +1066,7 @@ static void bcm_sysport_dim_work(struct work_struct *work)
struct net_dim_cq_moder cur_profile =
net_dim_get_profile(dim->mode, dim->profile_ix);
- priv->dim.coal_usecs = cur_profile.usec;
- priv->dim.coal_pkts = cur_profile.pkts;
-
- bcm_sysport_set_rx_coalesce(priv);
+ bcm_sysport_set_rx_coalesce(priv, cur_profile.usec, cur_profile.pkts);
dim->state = NET_DIM_START_MEASURE;
}
@@ -1408,9 +1413,11 @@ static void bcm_sysport_adj_link(struct net_device *dev)
phy_print_status(phydev);
}
-static void bcm_sysport_init_dim(struct bcm_sysport_net_dim *dim,
+static void bcm_sysport_init_dim(struct bcm_sysport_priv *priv,
void (*cb)(struct work_struct *work))
{
+ struct bcm_sysport_net_dim *dim = &priv->dim;
+
INIT_WORK(&dim->dim.work, cb);
dim->dim.mode = NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
dim->event_ctr = 0;
@@ -1418,6 +1425,25 @@ static void bcm_sysport_init_dim(struct bcm_sysport_net_dim *dim,
dim->bytes = 0;
}
+static void bcm_sysport_init_rx_coalesce(struct bcm_sysport_priv *priv)
+{
+ struct bcm_sysport_net_dim *dim = &priv->dim;
+ struct net_dim_cq_moder moder;
+ u32 usecs, pkts;
+
+ usecs = priv->rx_coalesce_usecs;
+ pkts = priv->rx_max_coalesced_frames;
+
+ /* If DIM was enabled, re-apply default parameters */
+ if (dim->use_dim) {
+ moder = net_dim_get_def_profile(dim->dim.mode);
+ usecs = moder.usec;
+ pkts = moder.pkts;
+ }
+
+ bcm_sysport_set_rx_coalesce(priv, usecs, pkts);
+}
+
static int bcm_sysport_init_tx_ring(struct bcm_sysport_priv *priv,
unsigned int index)
{
@@ -1658,8 +1684,6 @@ static int bcm_sysport_init_rx_ring(struct bcm_sysport_priv *priv)
rdma_writel(priv, 0, RDMA_END_ADDR_HI);
rdma_writel(priv, priv->num_rx_desc_words - 1, RDMA_END_ADDR_LO);
- rdma_writel(priv, 1, RDMA_MBDONE_INTR);
-
netif_dbg(priv, hw, priv->netdev,
"RDMA cfg, num_rx_bds=%d, rx_bds=%p\n",
priv->num_rx_bds, priv->rx_bds);
@@ -1827,7 +1851,8 @@ static void bcm_sysport_netif_start(struct net_device *dev)
struct bcm_sysport_priv *priv = netdev_priv(dev);
/* Enable NAPI */
- bcm_sysport_init_dim(&priv->dim, bcm_sysport_dim_work);
+ bcm_sysport_init_dim(priv, bcm_sysport_dim_work);
+ bcm_sysport_init_rx_coalesce(priv);
napi_enable(&priv->napi);
/* Enable RX interrupt and TX ring full interrupt */
@@ -2333,6 +2358,7 @@ static int bcm_sysport_probe(struct platform_device *pdev)
/* libphy will adjust the link state accordingly */
netif_carrier_off(dev);
+ priv->rx_max_coalesced_frames = 1;
u64_stats_init(&priv->syncp);
priv->dsa_notifier.notifier_call = bcm_sysport_dsa_notifier;
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
index 57e18ef8f206..d6e5d0cbf3a3 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.h
+++ b/drivers/net/ethernet/broadcom/bcmsysport.h
@@ -701,8 +701,6 @@ struct bcm_sysport_net_dim {
u16 event_ctr;
unsigned long packets;
unsigned long bytes;
- u32 coal_usecs;
- u32 coal_pkts;
struct net_dim dim;
};
@@ -755,6 +753,8 @@ struct bcm_sysport_priv {
unsigned int rx_c_index;
struct bcm_sysport_net_dim dim;
+ u32 rx_max_coalesced_frames;
+ u32 rx_coalesce_usecs;
/* PHY device */
struct device_node *phy_dn;
--
2.14.1
^ permalink raw reply related
* [PATCH net-next v2 1/3] net: systemport: Remove adaptive TX coalescing
From: Florian Fainelli @ 2018-03-28 22:15 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, davem, jaedon.shin, pgynther, opendmb,
Michael Chan, gospo, talgi, saeedm
In-Reply-To: <20180328221538.29290-1-f.fainelli@gmail.com>
Adaptive TX coalescing is not currently giving us any advantages and
ends up making the CPU spin more frequently until TX completion. Deny
and disable adaptive TX coalescing for now and rely on static
configuration, we can always add it back later.
Reviewed-by: Tal Gilboa <talgi@mellanox.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 61 ++++--------------------------
drivers/net/ethernet/broadcom/bcmsysport.h | 1 -
2 files changed, 8 insertions(+), 54 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 4e26f606a7f2..1e52bb7d822e 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -15,7 +15,6 @@
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/netdevice.h>
-#include <linux/net_dim.h>
#include <linux/etherdevice.h>
#include <linux/platform_device.h>
#include <linux/of.h>
@@ -588,7 +587,8 @@ static void bcm_sysport_set_rx_coalesce(struct bcm_sysport_priv *priv)
rdma_writel(priv, reg, RDMA_MBDONE_INTR);
}
-static void bcm_sysport_set_tx_coalesce(struct bcm_sysport_tx_ring *ring)
+static void bcm_sysport_set_tx_coalesce(struct bcm_sysport_tx_ring *ring,
+ struct ethtool_coalesce *ec)
{
struct bcm_sysport_priv *priv = ring->priv;
u32 reg;
@@ -596,8 +596,8 @@ static void bcm_sysport_set_tx_coalesce(struct bcm_sysport_tx_ring *ring)
reg = tdma_readl(priv, TDMA_DESC_RING_INTR_CONTROL(ring->index));
reg &= ~(RING_INTR_THRESH_MASK |
RING_TIMEOUT_MASK << RING_TIMEOUT_SHIFT);
- reg |= ring->dim.coal_pkts;
- reg |= DIV_ROUND_UP(ring->dim.coal_usecs * 1000, 8192) <<
+ reg |= ec->tx_max_coalesced_frames;
+ reg |= DIV_ROUND_UP(ec->tx_coalesce_usecs * 1000, 8192) <<
RING_TIMEOUT_SHIFT;
tdma_writel(priv, reg, TDMA_DESC_RING_INTR_CONTROL(ring->index));
}
@@ -606,18 +606,12 @@ static int bcm_sysport_get_coalesce(struct net_device *dev,
struct ethtool_coalesce *ec)
{
struct bcm_sysport_priv *priv = netdev_priv(dev);
- struct bcm_sysport_tx_ring *ring;
- unsigned int i;
u32 reg;
reg = tdma_readl(priv, TDMA_DESC_RING_INTR_CONTROL(0));
ec->tx_coalesce_usecs = (reg >> RING_TIMEOUT_SHIFT) * 8192 / 1000;
ec->tx_max_coalesced_frames = reg & RING_INTR_THRESH_MASK;
- for (i = 0; i < dev->num_tx_queues; i++) {
- ring = &priv->tx_rings[i];
- ec->use_adaptive_tx_coalesce |= ring->dim.use_dim;
- }
reg = rdma_readl(priv, RDMA_MBDONE_INTR);
@@ -632,7 +626,6 @@ static int bcm_sysport_set_coalesce(struct net_device *dev,
struct ethtool_coalesce *ec)
{
struct bcm_sysport_priv *priv = netdev_priv(dev);
- struct bcm_sysport_tx_ring *ring;
unsigned int i;
/* Base system clock is 125Mhz, DMA timeout is this reference clock
@@ -646,20 +639,12 @@ static int bcm_sysport_set_coalesce(struct net_device *dev,
return -EINVAL;
if ((ec->tx_coalesce_usecs == 0 && ec->tx_max_coalesced_frames == 0) ||
- (ec->rx_coalesce_usecs == 0 && ec->rx_max_coalesced_frames == 0))
+ (ec->rx_coalesce_usecs == 0 && ec->rx_max_coalesced_frames == 0) ||
+ ec->use_adaptive_tx_coalesce)
return -EINVAL;
- for (i = 0; i < dev->num_tx_queues; i++) {
- ring = &priv->tx_rings[i];
- ring->dim.coal_pkts = ec->tx_max_coalesced_frames;
- ring->dim.coal_usecs = ec->tx_coalesce_usecs;
- if (!ec->use_adaptive_tx_coalesce && ring->dim.use_dim) {
- ring->dim.coal_pkts = 1;
- ring->dim.coal_usecs = 0;
- }
- ring->dim.use_dim = ec->use_adaptive_tx_coalesce;
- bcm_sysport_set_tx_coalesce(ring);
- }
+ for (i = 0; i < dev->num_tx_queues; i++)
+ bcm_sysport_set_tx_coalesce(&priv->tx_rings[i], ec);
priv->dim.coal_usecs = ec->rx_coalesce_usecs;
priv->dim.coal_pkts = ec->rx_max_coalesced_frames;
@@ -940,8 +925,6 @@ static unsigned int __bcm_sysport_tx_reclaim(struct bcm_sysport_priv *priv,
ring->packets += pkts_compl;
ring->bytes += bytes_compl;
u64_stats_update_end(&priv->syncp);
- ring->dim.packets = pkts_compl;
- ring->dim.bytes = bytes_compl;
ring->c_index = c_index;
@@ -987,7 +970,6 @@ static int bcm_sysport_tx_poll(struct napi_struct *napi, int budget)
{
struct bcm_sysport_tx_ring *ring =
container_of(napi, struct bcm_sysport_tx_ring, napi);
- struct net_dim_sample dim_sample;
unsigned int work_done = 0;
work_done = bcm_sysport_tx_reclaim(ring->priv, ring);
@@ -1004,12 +986,6 @@ static int bcm_sysport_tx_poll(struct napi_struct *napi, int budget)
return 0;
}
- if (ring->dim.use_dim) {
- net_dim_sample(ring->dim.event_ctr, ring->dim.packets,
- ring->dim.bytes, &dim_sample);
- net_dim(&ring->dim.dim, dim_sample);
- }
-
return budget;
}
@@ -1089,23 +1065,6 @@ static void bcm_sysport_dim_work(struct work_struct *work)
dim->state = NET_DIM_START_MEASURE;
}
-static void bcm_sysport_dim_tx_work(struct work_struct *work)
-{
- struct net_dim *dim = container_of(work, struct net_dim, work);
- struct bcm_sysport_net_dim *ndim =
- container_of(dim, struct bcm_sysport_net_dim, dim);
- struct bcm_sysport_tx_ring *ring =
- container_of(ndim, struct bcm_sysport_tx_ring, dim);
- struct net_dim_cq_moder cur_profile =
- net_dim_get_profile(dim->mode, dim->profile_ix);
-
- ring->dim.coal_usecs = cur_profile.usec;
- ring->dim.coal_pkts = cur_profile.pkts;
-
- bcm_sysport_set_tx_coalesce(ring);
- dim->state = NET_DIM_START_MEASURE;
-}
-
/* RX and misc interrupt routine */
static irqreturn_t bcm_sysport_rx_isr(int irq, void *dev_id)
{
@@ -1152,7 +1111,6 @@ static irqreturn_t bcm_sysport_rx_isr(int irq, void *dev_id)
continue;
txr = &priv->tx_rings[ring];
- txr->dim.event_ctr++;
if (likely(napi_schedule_prep(&txr->napi))) {
intrl2_0_mask_set(priv, ring_bit);
@@ -1185,7 +1143,6 @@ static irqreturn_t bcm_sysport_tx_isr(int irq, void *dev_id)
continue;
txr = &priv->tx_rings[ring];
- txr->dim.event_ctr++;
if (likely(napi_schedule_prep(&txr->napi))) {
intrl2_1_mask_set(priv, BIT(ring));
@@ -1551,7 +1508,6 @@ static int bcm_sysport_init_tx_ring(struct bcm_sysport_priv *priv,
reg |= (1 << index);
tdma_writel(priv, reg, TDMA_TIER1_ARB_0_QUEUE_EN);
- bcm_sysport_init_dim(&ring->dim, bcm_sysport_dim_tx_work);
napi_enable(&ring->napi);
netif_dbg(priv, hw, priv->netdev,
@@ -1582,7 +1538,6 @@ static void bcm_sysport_fini_tx_ring(struct bcm_sysport_priv *priv,
return;
napi_disable(&ring->napi);
- cancel_work_sync(&ring->dim.dim.work);
netif_napi_del(&ring->napi);
bcm_sysport_tx_clean(priv, ring);
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
index e1c97d4a82b4..57e18ef8f206 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.h
+++ b/drivers/net/ethernet/broadcom/bcmsysport.h
@@ -723,7 +723,6 @@ struct bcm_sysport_tx_ring {
struct bcm_sysport_priv *priv; /* private context backpointer */
unsigned long packets; /* packets statistics */
unsigned long bytes; /* bytes statistics */
- struct bcm_sysport_net_dim dim; /* Net DIM context */
unsigned int switch_queue; /* switch port queue number */
unsigned int switch_port; /* switch port queue number */
bool inspect; /* inspect switch port and queue */
--
2.14.1
^ permalink raw reply related
* [PATCH net-next v2 0/3] net: Broadcom drivers coalescing fixes
From: Florian Fainelli @ 2018-03-28 22:15 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, davem, jaedon.shin, pgynther, opendmb,
Michael Chan, gospo, talgi, saeedm
Hi all,
Following Tal's review of the adaptive RX/TX coalescing feature added to the
SYSTEMPORT and GENET driver a number of things showed up:
- adaptive TX coalescing is not actually a good idea with the current way
the estimator will program the ring, this results in a higher CPU load, NAPI
on TX already does a reasonably good job at maintaining the interrupt count low
- both SYSTEMPORT and GENET would suffer from the same issues while configuring
coalescing parameters where the values would just not be applied correctly
based on user settings, so we fix that too
Tal, thanks again for your feedback, I would appreciate if you could review that
the new behavior appears to be implemented correctly.
Thanks!
Changes in v2:
- added Tal's reviewed-by to the first patch
- split DIM initialization from coalescing parameters initialization
- avoid duplicating the same code in bcmgenet_set_coalesce() when configuring RX rings
- fixed the condition where default DIM parameters would be applied when
adaptive RX coalescing would be enabled, do this only if it was disabled before
Florian Fainelli (3):
net: systemport: Remove adaptive TX coalescing
net: systemport: Fix coalescing settings handling
net: bcmgenet: Fix coalescing settings handling
drivers/net/ethernet/broadcom/bcmsysport.c | 123 +++++++++++--------------
drivers/net/ethernet/broadcom/bcmsysport.h | 5 +-
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 92 +++++++++++-------
drivers/net/ethernet/broadcom/genet/bcmgenet.h | 4 +-
4 files changed, 115 insertions(+), 109 deletions(-)
--
2.14.1
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Nicholas Piggin @ 2018-03-28 22:09 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: David Miller, paulmck, arnd, linux-rdma, linuxppc-dev, linus971,
will.deacon, alexander.duyck, okaya, jgg, David.Laight, oohall,
netdev, alexander.h.duyck, torvalds
In-Reply-To: <1522272692.21446.42.camel@kernel.crashing.org>
On Thu, 29 Mar 2018 08:31:32 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Thu, 2018-03-29 at 02:23 +1000, Nicholas Piggin wrote:
> > On Wed, 28 Mar 2018 11:55:09 -0400 (EDT)
> > David Miller <davem@davemloft.net> wrote:
> >
> > > From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Date: Thu, 29 Mar 2018 02:13:16 +1100
> > >
> > > > Let's fix all archs, it's way easier than fixing all drivers. Half of
> > > > the archs are unused or dead anyway.
> > >
> > > Agreed.
> >
> > While we're making decrees here, can we do something about mmiowb?
> > The semantics are basically indecipherable.
>
> I was going to tackle that next :-)
>
> > This is a variation on the mandatory write barrier that causes writes to weakly
> > ordered I/O regions to be partially ordered. Its effects may go beyond the
> > CPU->Hardware interface and actually affect the hardware at some level.
> >
> > How can a driver writer possibly get that right?
> >
> > IIRC it was added for some big ia64 system that was really expensive
> > to implement the proper wmb() semantics on. So wmb() semantics were
> > quietly downgraded, then the subsequently broken drivers they cared
> > about were fixed by adding the stronger mmiowb().
> >
> > What should have happened was wmb and writel remained correct, sane, and
> > expensive, and they add an mmio_wmb() to order MMIO stores made by the
> > writel_relaxed accessors, then use that to speed up the few drivers they
> > care about.
> >
> > Now that ia64 doesn't matter too much, can we deprecate mmiowb and just
> > make wmb ordering talk about stores to the device, not to some
> > intermediate stage of the interconnect where it can be subsequently
> > reordered wrt the device? Drivers can be converted back to using wmb
> > or writel gradually.
>
> I was under the impression that mmiowb was specifically about ordering
> writel's with a subsequent spin_unlock, without it, MMIOs from
> different CPUs (within the same lock) would still arrive OO.
Yes more or less, and I think that until mmiowb was introduced, wmb
or writel was sufficient for this.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH] net: phy: marvell10g: add thermal hwmon device
From: Andrew Lunn @ 2018-03-28 22:09 UTC (permalink / raw)
To: Russell King, Guenter Roeck; +Cc: Florian Fainelli, netdev
In-Reply-To: <E1f18Bu-0005CL-O4@rmk-PC.armlinux.org.uk>
Hi Russell
Guenter Roeck, the HWMON maintainer should also be involved in
reviewing this patch.
Andrew
On Wed, Mar 28, 2018 at 11:20:26AM +0100, Russell King wrote:
> Add a thermal monitoring device for the Marvell 88x3310, which updates
> once a second. We also need to hook into the suspend/resume mechanism
> to ensure that the thermal monitoring is reconfigured when we resume.
>
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> drivers/net/phy/marvell10g.c | 184 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 182 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> index 8a0bd98fdec7..db9d66781da6 100644
> --- a/drivers/net/phy/marvell10g.c
> +++ b/drivers/net/phy/marvell10g.c
> @@ -21,8 +21,10 @@
> * If both the fiber and copper ports are connected, the first to gain
> * link takes priority and the other port is completely locked out.
> */
> -#include <linux/phy.h>
> +#include <linux/ctype.h>
> +#include <linux/hwmon.h>
> #include <linux/marvell_phy.h>
> +#include <linux/phy.h>
>
> enum {
> MV_PCS_BASE_T = 0x0000,
> @@ -40,6 +42,19 @@ enum {
> */
> MV_AN_CTRL1000 = 0x8000, /* 1000base-T control register */
> MV_AN_STAT1000 = 0x8001, /* 1000base-T status register */
> +
> + /* Vendor2 MMD registers */
> + MV_V2_TEMP_CTRL = 0xf08a,
> + MV_V2_TEMP_CTRL_MASK = 0xc000,
> + MV_V2_TEMP_CTRL_SAMPLE = 0x0000,
> + MV_V2_TEMP_CTRL_DISABLE = 0xc000,
> + MV_V2_TEMP = 0xf08c,
> + MV_V2_TEMP_UNKNOWN = 0x9600, /* unknown function */
> +};
> +
> +struct mv3310_priv {
> + struct device *hwmon_dev;
> + char *hwmon_name;
> };
>
> static int mv3310_modify(struct phy_device *phydev, int devad, u16 reg,
> @@ -60,17 +75,180 @@ static int mv3310_modify(struct phy_device *phydev, int devad, u16 reg,
> return ret < 0 ? ret : 1;
> }
>
> +#ifdef CONFIG_HWMON
> +static umode_t mv3310_hwmon_is_visible(const void *data,
> + enum hwmon_sensor_types type,
> + u32 attr, int channel)
> +{
> + if (type == hwmon_chip && attr == hwmon_chip_update_interval)
> + return 0444;
> + if (type == hwmon_temp && attr == hwmon_temp_input)
> + return 0444;
> + return 0;
> +}
> +
> +static int mv3310_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *value)
> +{
> + struct phy_device *phydev = dev_get_drvdata(dev);
> + int temp;
> +
> + if (type == hwmon_chip && attr == hwmon_chip_update_interval) {
> + *value = MSEC_PER_SEC;
> + return 0;
> + }
> +
> + if (type == hwmon_temp && attr == hwmon_temp_input) {
> + temp = phy_read_mmd(phydev, MDIO_MMD_VEND2, MV_V2_TEMP);
> + if (temp < 0)
> + return temp;
> +
> + *value = ((temp & 0xff) - 75) * 1000;
> +
> + return 0;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static const struct hwmon_ops mv3310_hwmon_ops = {
> + .is_visible = mv3310_hwmon_is_visible,
> + .read = mv3310_hwmon_read,
> +};
> +
> +static u32 mv3310_hwmon_chip_config[] = {
> + HWMON_C_REGISTER_TZ | HWMON_C_UPDATE_INTERVAL,
> + 0,
> +};
> +
> +static const struct hwmon_channel_info mv3310_hwmon_chip = {
> + .type = hwmon_chip,
> + .config = mv3310_hwmon_chip_config,
> +};
> +
> +static u32 mv3310_hwmon_temp_config[] = {
> + HWMON_T_INPUT,
> + 0,
> +};
> +
> +static const struct hwmon_channel_info mv3310_hwmon_temp = {
> + .type = hwmon_temp,
> + .config = mv3310_hwmon_temp_config,
> +};
> +
> +static const struct hwmon_channel_info *mv3310_hwmon_info[] = {
> + &mv3310_hwmon_chip,
> + &mv3310_hwmon_temp,
> + NULL,
> +};
> +
> +static const struct hwmon_chip_info mv3310_hwmon_chip_info = {
> + .ops = &mv3310_hwmon_ops,
> + .info = mv3310_hwmon_info,
> +};
> +
> +static int mv3310_hwmon_config(struct phy_device *phydev, bool enable)
> +{
> + u16 val;
> + int ret;
> +
> + ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_V2_TEMP,
> + MV_V2_TEMP_UNKNOWN);
> + if (ret < 0)
> + return ret;
> +
> + val = enable ? MV_V2_TEMP_CTRL_SAMPLE : MV_V2_TEMP_CTRL_DISABLE;
> + ret = mv3310_modify(phydev, MDIO_MMD_VEND2, MV_V2_TEMP_CTRL,
> + MV_V2_TEMP_CTRL_MASK, val);
> +
> + return ret < 0 ? ret : 0;
> +}
> +
> +static void mv3310_hwmon_disable(void *data)
> +{
> + struct phy_device *phydev = data;
> +
> + mv3310_hwmon_config(phydev, false);
> +}
> +
> +static int mv3310_hwmon_probe(struct phy_device *phydev)
> +{
> + struct device *dev = &phydev->mdio.dev;
> + struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
> + int i, j, ret;
> +
> + priv->hwmon_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL);
> + if (!priv->hwmon_name)
> + return -ENODEV;
> +
> + for (i = j = 0; priv->hwmon_name[i]; i++) {
> + if (isalnum(priv->hwmon_name[i])) {
> + if (i != j)
> + priv->hwmon_name[j] = priv->hwmon_name[i];
> + j++;
> + }
> + }
> + priv->hwmon_name[j] = '\0';
> +
> + ret = mv3310_hwmon_config(phydev, true);
> + if (ret)
> + return ret;
> +
> + ret = devm_add_action_or_reset(dev, mv3310_hwmon_disable, phydev);
> + if (ret)
> + return ret;
> +
> + priv->hwmon_dev = devm_hwmon_device_register_with_info(dev,
> + priv->hwmon_name, phydev,
> + &mv3310_hwmon_chip_info, NULL);
> +
> + return PTR_ERR_OR_ZERO(priv->hwmon_dev);
> +}
> +#else
> +static inline int mv3310_hwmon_config(struct phy_device *phydev, bool enable)
> +{
> + return 0;
> +}
> +
> +static int mv3310_hwmon_probe(struct phy_device *phydev)
> +{
> + return 0;
> +}
> +#endif
> +
> static int mv3310_probe(struct phy_device *phydev)
> {
> + struct mv3310_priv *priv;
> u32 mmd_mask = MDIO_DEVS_PMAPMD | MDIO_DEVS_AN;
> + int ret;
>
> if (!phydev->is_c45 ||
> (phydev->c45_ids.devices_in_package & mmd_mask) != mmd_mask)
> return -ENODEV;
>
> + priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + dev_set_drvdata(&phydev->mdio.dev, priv);
> +
> + ret = mv3310_hwmon_probe(phydev);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int mv3310_suspend(struct phy_device *phydev)
> +{
> return 0;
> }
>
> +static int mv3310_resume(struct phy_device *phydev)
> +{
> + return mv3310_hwmon_config(phydev, true);
> +}
> +
> /*
> * Resetting the MV88X3310 causes it to become non-responsive. Avoid
> * setting the reset bit(s).
> @@ -376,9 +554,11 @@ static struct phy_driver mv3310_drivers[] = {
> SUPPORTED_FIBRE |
> SUPPORTED_10000baseT_Full |
> SUPPORTED_Backplane,
> - .probe = mv3310_probe,
> .soft_reset = mv3310_soft_reset,
> .config_init = mv3310_config_init,
> + .probe = mv3310_probe,
> + .suspend = mv3310_suspend,
> + .resume = mv3310_resume,
> .config_aneg = mv3310_config_aneg,
> .aneg_done = mv3310_aneg_done,
> .read_status = mv3310_read_status,
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH] ath10k: avoid possible string overflow
From: Arnd Bergmann @ 2018-03-28 22:06 UTC (permalink / raw)
To: Kalle Valo
Cc: Arnd Bergmann, Manikanta Pubbisetty, Anilkumar Kolli, Carl Huang,
Gustavo A. R. Silva, Johannes Berg, Maharaja Kennadyrajan, ath10k,
linux-wireless, netdev, linux-kernel
The way that 'strncat' is used here raised a warning in gcc-8:
drivers/net/wireless/ath/ath10k/wmi.c: In function 'ath10k_wmi_tpc_stats_final_disp_tables':
drivers/net/wireless/ath/ath10k/wmi.c:4649:4: error: 'strncat' output truncated before terminating nul copying as many bytes from a string as its length [-Werror=stringop-truncation]
Effectively, this is simply a strcat() but the use of strncat() suggests
some form of overflow check. Regardless of whether this might actually
overflow, using strlcat() instead of strncat() avoids the warning and
makes the code more robust.
Fixes: bc64d05220f3 ("ath10k: debugfs support to get final TPC stats for 10.4 variants")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/wireless/ath/ath10k/wmi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 9649bb752bbd..42522ed115f3 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -4309,7 +4309,7 @@ static void ath10k_tpc_config_disp_tables(struct ath10k *ar,
rate_code[i],
type);
snprintf(buff, sizeof(buff), "%8d ", tpc[j]);
- strncat(tpc_value, buff, strlen(buff));
+ strlcat(tpc_value, buff, sizeof(tpc_value));
}
tpc_stats->tpc_table[type].pream_idx[i] = pream_idx;
tpc_stats->tpc_table[type].rate_code[i] = rate_code[i];
@@ -4646,7 +4646,7 @@ ath10k_wmi_tpc_stats_final_disp_tables(struct ath10k *ar,
rate_code[i],
type, pream_idx);
snprintf(buff, sizeof(buff), "%8d ", tpc[j]);
- strncat(tpc_value, buff, strlen(buff));
+ strlcat(tpc_value, buff, sizeof(tpc_value));
}
tpc_stats->tpc_table_final[type].pream_idx[i] = pream_idx;
tpc_stats->tpc_table_final[type].rate_code[i] = rate_code[i];
--
2.9.0
^ permalink raw reply related
* Re: [PATCH net-next 2/2] sfp/phylink: move module EEPROM ethtool access into netdev core ethtool
From: Andrew Lunn @ 2018-03-28 22:02 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, Russell King, Thomas Petazzoni, David S. Miller,
open list, Antoine Tenart, Yan Markman, Stefan Chulski,
Maxime Chevallier, Miquel Raynal, Marcin Wojtas
In-Reply-To: <20180328190339.31433-3-f.fainelli@gmail.com>
On Wed, Mar 28, 2018 at 12:03:39PM -0700, Florian Fainelli wrote:
> From: Russell King <rmk+kernel@armlinux.org.uk>
>
> Provide a pointer to the SFP bus in struct net_device, so that the
> ethtool module EEPROM methods can access the SFP directly, rather
> than needing every user to provide a hook for it.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> drivers/net/ethernet/marvell/mvneta.c | 18 ------------------
> drivers/net/phy/phylink.c | 28 ----------------------------
> drivers/net/phy/sfp-bus.c | 6 ++----
> include/linux/netdevice.h | 3 +++
> include/linux/phylink.h | 3 ---
> net/core/ethtool.c | 7 +++++++
> 6 files changed, 12 insertions(+), 53 deletions(-)
The diffstats look good, DSA and all other uses of phylink will make a
similar saving in code. The new member in the netdev structure is well
away from members which are commented as being cache line sensitive.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [Resend Patch 1/3] Vmbus: Add function to report available ring buffer to write in total ring size percentage
From: Martin K. Petersen @ 2018-03-28 22:00 UTC (permalink / raw)
To: Long Li
Cc: Stephen Hemminger, linux-scsi, Martin K . Petersen, netdev,
Haiyang Zhang, James E . J . Bottomley, linux-kernel, devel
In-Reply-To: <20180328004840.22787-1-longli@linuxonhyperv.com>
Long,
> Netvsc has a function to calculate how much ring buffer in percentage is
> available to write. This function is also useful for storvsc and other
> vmbus devices.
What is the submission strategy for this series? Do you expect it to go
through net or scsi? If the latter, I'll need an ack from davem.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [RFC PATCH v2 00/14] Introducing AF_XDP support
From: Eric Leblond @ 2018-03-28 21:18 UTC (permalink / raw)
To: Björn Töpel, magnus.karlsson, alexander.h.duyck,
alexander.duyck, john.fastabend, ast, brouer,
willemdebruijn.kernel, daniel, netdev
Cc: Björn Töpel, michael.lundkvist, jesse.brandeburg,
anjali.singhai, qi.z.zhang, ravineet.singh
In-Reply-To: <20180327165919.17933-1-bjorn.topel@gmail.com>
Hello,
On Tue, 2018-03-27 at 18:59 +0200, Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
>
>
optimized for high performance packet processing and, in upcoming
> patch sets, zero-copy semantics. In this v2 version, we have removed
> all zero-copy related code in order to make it smaller, simpler and
> hopefully more review friendly. This RFC only supports copy-mode for
> the generic XDP path (XDP_SKB) for both RX and TX and copy-mode for
> RX
>
...
>
> How is then packets distributed between these two XSK? We have
> introduced a new BPF map called XSKMAP (or BPF_MAP_TYPE_XSKMAP in
> full). The user-space application can place an XSK at an arbitrary
> place in this map. The XDP program can then redirect a packet to a
> specific index in this map and at this point XDP validates that the
> XSK in that map was indeed bound to that device and queue number. If
> not, the packet is dropped. If the map is empty at that index, the
> packet is also dropped. This also means that it is currently
> mandatory
> to have an XDP program loaded (and one XSK in the XSKMAP) to be able
> to get any traffic to user space through the XSK.
If I get it correctly, this feature will have to be used to bound
multiple sockets to a single queue and the eBPF filter will be
responsible of the load balancing. Am I correct ?
> AF_XDP can operate in two different modes: XDP_SKB and XDP_DRV. If
> the
> driver does not have support for XDP, or XDP_SKB is explicitly chosen
...
Thanks a lot for this work, I'm gonna try to implement this in
Suricata.
Best regards,
^ permalink raw reply
* Re: [PATCH net-next 2/3] net: systemport: Fix coalescing settings handling
From: Florian Fainelli @ 2018-03-28 21:53 UTC (permalink / raw)
To: Tal Gilboa, netdev
Cc: davem, jaedon.shin, pgynther, opendmb, Michael Chan, gospo,
saeedm
In-Reply-To: <61c5d8c2-5bc8-9f61-97aa-fb42eceb753f@mellanox.com>
On 03/28/2018 02:23 PM, Tal Gilboa wrote:
> On 3/27/2018 10:47 PM, Florian Fainelli wrote:
>> There were a number of issues with setting the RX coalescing parameters:
>>
>> - we would not be preserving values that would have been configured
>> across close/open calls, instead we would always reset to no timeout
>> and 1 interrupt per packet, this would also prevent DIM from
>> setting its
>> default usec/pkts values
>>
>> - when adaptive RX would be turned on, we woud not be fetching the
>> default parameters, we would stay with no timeout/1 packet per
>> interrupt until the estimator kicks in and changes that
>>
>> - finally disabling adaptive RX coalescing while providing parameters
>> would not be honored, and we would stay with whatever DIM had
>> previously determined instead of the user requested parameters
>>
>> Fixes: b6e0e875421e ("net: systemport: Implement adaptive interrupt
>> coalescing")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> drivers/net/ethernet/broadcom/bcmsysport.c | 57
>> ++++++++++++++++++++----------
>> drivers/net/ethernet/broadcom/bcmsysport.h | 4 +--
>> 2 files changed, 41 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c
>> b/drivers/net/ethernet/broadcom/bcmsysport.c
>> index 1e52bb7d822e..43ad6300c351 100644
>> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
>> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
>> @@ -574,16 +574,16 @@ static int bcm_sysport_set_wol(struct net_device
>> *dev,
>> return 0;
>> }
>> -static void bcm_sysport_set_rx_coalesce(struct bcm_sysport_priv *priv)
>> +static void bcm_sysport_set_rx_coalesce(struct bcm_sysport_priv *priv,
>> + u32 usecs, u32 pkts)
>> {
>> u32 reg;
>> reg = rdma_readl(priv, RDMA_MBDONE_INTR);
>> reg &= ~(RDMA_INTR_THRESH_MASK |
>> RDMA_TIMEOUT_MASK << RDMA_TIMEOUT_SHIFT);
>> - reg |= priv->dim.coal_pkts;
>> - reg |= DIV_ROUND_UP(priv->dim.coal_usecs * 1000, 8192) <<
>> - RDMA_TIMEOUT_SHIFT;
>> + reg |= pkts;
>> + reg |= DIV_ROUND_UP(usecs * 1000, 8192) << RDMA_TIMEOUT_SHIFT;
>> rdma_writel(priv, reg, RDMA_MBDONE_INTR);
>> }
>> @@ -626,6 +626,8 @@ static int bcm_sysport_set_coalesce(struct
>> net_device *dev,
>> struct ethtool_coalesce *ec)
>> {
>> struct bcm_sysport_priv *priv = netdev_priv(dev);
>> + struct net_dim_cq_moder moder;
>> + u32 usecs, pkts;
>> unsigned int i;
>> /* Base system clock is 125Mhz, DMA timeout is this reference
>> clock
>> @@ -646,15 +648,22 @@ static int bcm_sysport_set_coalesce(struct
>> net_device *dev,
>> for (i = 0; i < dev->num_tx_queues; i++)
>> bcm_sysport_set_tx_coalesce(&priv->tx_rings[i], ec);
>> - priv->dim.coal_usecs = ec->rx_coalesce_usecs;
>> - priv->dim.coal_pkts = ec->rx_max_coalesced_frames;
>> + priv->rx_coalesce_usecs = ec->rx_coalesce_usecs;
>> + priv->rx_max_coalesced_frames = ec->rx_max_coalesced_frames;
>> + usecs = priv->rx_coalesce_usecs;
>> + pkts = priv->rx_max_coalesced_frames;
>> - if (!ec->use_adaptive_rx_coalesce && priv->dim.use_dim) {
>> - priv->dim.coal_pkts = 1;
>> - priv->dim.coal_usecs = 0;
>> + /* If DIM is enabled, immediately obtain default parameters */
>> + if (ec->use_adaptive_rx_coalesce) {
>> + moder = net_dim_get_def_profile(priv->dim.dim.mode);
>> + usecs = moder.usec;
>> + pkts = moder.pkts;
>> }
>
> What if DIM is already in use? We don't want to set default values if
> DIM is already working.
Indeed, good catch, thank you!
>
>> +
>> priv->dim.use_dim = ec->use_adaptive_rx_coalesce;
>> - bcm_sysport_set_rx_coalesce(priv);
>> +
>> + /* Apply desired coalescing parameters */
>> + bcm_sysport_set_rx_coalesce(priv, usecs, pkts);
>> return 0;
>> }
>> @@ -1058,10 +1067,7 @@ static void bcm_sysport_dim_work(struct
>> work_struct *work)
>> struct net_dim_cq_moder cur_profile =
>> net_dim_get_profile(dim->mode, dim->profile_ix);
>> - priv->dim.coal_usecs = cur_profile.usec;
>> - priv->dim.coal_pkts = cur_profile.pkts;
>> -
>> - bcm_sysport_set_rx_coalesce(priv);
>> + bcm_sysport_set_rx_coalesce(priv, cur_profile.usec,
>> cur_profile.pkts);
>> dim->state = NET_DIM_START_MEASURE;
>> }
>> @@ -1408,14 +1414,30 @@ static void bcm_sysport_adj_link(struct
>> net_device *dev)
>> phy_print_status(phydev);
>> }
>> -static void bcm_sysport_init_dim(struct bcm_sysport_net_dim *dim,
>> +static void bcm_sysport_init_dim(struct bcm_sysport_priv *priv,
>> void (*cb)(struct work_struct *work))
>> {
>> + struct bcm_sysport_net_dim *dim = &priv->dim;
>> + struct net_dim_cq_moder moder;
>> + u32 usecs, pkts;
>> +
>> INIT_WORK(&dim->dim.work, cb);
>> dim->dim.mode = NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
>> dim->event_ctr = 0;
>> dim->packets = 0;
>> dim->bytes = 0;
>
> I would expect only dim related code in a function called init_dim(). I
> think the code below should be elsewhere. Or maybe change the function
> name to init_rx_coalesce().
Sure, I can definitively do split the initialization, that makes sense.
--
Florian
^ permalink raw reply
* Re: [PATCH] gfs2: Stop using rhashtable_walk_peek
From: NeilBrown @ 2018-03-28 21:53 UTC (permalink / raw)
To: Andreas Gruenbacher, cluster-devel
Cc: netdev, linux-kernel, Thomas Graf, Herbert Xu, Tom Herbert
In-Reply-To: <20180328160009.29633-1-agruenba@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2913 bytes --]
On Wed, Mar 28 2018, Andreas Gruenbacher wrote:
> Function rhashtable_walk_peek is problematic because there is no
> guarantee that the glock previously returned still exists; when that key
> is deleted, rhashtable_walk_peek can end up returning a different key,
> which would cause an inconsistent glock dump. So instead of using
> rhashtable_walk_peek, keep track of the current glock in the seq file
> iterator functions.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
> fs/gfs2/glock.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
> index 82fb5583445c..f1fc353875d3 100644
> --- a/fs/gfs2/glock.c
> +++ b/fs/gfs2/glock.c
> @@ -55,6 +55,7 @@ struct gfs2_glock_iter {
> struct gfs2_sbd *sdp; /* incore superblock */
> struct rhashtable_iter hti; /* rhashtable iterator */
> struct gfs2_glock *gl; /* current glock struct */
> + bool gl_held;
> loff_t last_pos; /* last position */
> };
>
> @@ -1923,9 +1924,11 @@ void gfs2_glock_exit(void)
>
> static void gfs2_glock_iter_next(struct gfs2_glock_iter *gi, loff_t n)
> {
> - if (n == 0)
> - gi->gl = rhashtable_walk_peek(&gi->hti);
> - else {
> + if (n != 0 || !gi->gl) {
> + if (gi->gl_held) {
> + gfs2_glock_queue_put(gi->gl);
> + gi->gl_held = false;
> + }
> gi->gl = rhashtable_walk_next(&gi->hti);
> n--;
> }
Thank for this patch!
The above looks a bit fragile to me.
gfs2_glock_iter_next() (And hence gfs2_glock_seq_start()) will sometimes
exit with gl_held true, and sometimes with it false.
gfs2_glock_seq_stop() assumes that it is false.
Normally gfs2_glock_seq_next() will normally be called between these
two and will clear gl_held, but I don't think there is a hard guarantee
of that.
Maybe we should always 'put' gi->gl in iter_next if gl_held??
Thanks,
NeilBrown
> @@ -1988,7 +1991,10 @@ static void gfs2_glock_seq_stop(struct seq_file *seq, void *iter_ptr)
> {
> struct gfs2_glock_iter *gi = seq->private;
>
> - gi->gl = NULL;
> + if (gi->gl) {
> + lockref_get(&gi->gl->gl_lockref);
> + gi->gl_held = true;
> + }
> rhashtable_walk_stop(&gi->hti);
> }
>
> @@ -2061,6 +2067,7 @@ static int __gfs2_glocks_open(struct inode *inode, struct file *file,
> */
> gi->last_pos = -1;
> gi->gl = NULL;
> + gi->gl_held = false;
> rhashtable_walk_enter(&gl_hash_table, &gi->hti);
> }
> return ret;
> @@ -2076,7 +2083,8 @@ static int gfs2_glocks_release(struct inode *inode, struct file *file)
> struct seq_file *seq = file->private_data;
> struct gfs2_glock_iter *gi = seq->private;
>
> - gi->gl = NULL;
> + if (gi->gl_held)
> + gfs2_glock_put(gi->gl);
> rhashtable_walk_exit(&gi->hti);
> return seq_release_private(inode, file);
> }
> --
> 2.14.3
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* [PATCH v2 net-next 0/2] Add ATU/VTU statistics
From: Andrew Lunn @ 2018-03-28 21:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn
Previous patches have added basic support for Address Translation Unit
and VLAN translation Unit violation interrupts. Add statistics
counters for when these occur, which can be accessed using
ethtool. Downgrade one of the particularly spammy warnings from VTU
violations to debug only, now that we have a counter for it.
Andrew Lunn (2):
net: dsa: mv88e6xxx: Keep ATU/VTU violation statistics
net: dsa: mv88e6xxx: Make VTU miss violations less spammy
drivers/net/dsa/mv88e6xxx/chip.c | 50 ++++++++++++++++++++++++++++-----
drivers/net/dsa/mv88e6xxx/chip.h | 13 ++++++---
drivers/net/dsa/mv88e6xxx/global1_atu.c | 12 +++++---
drivers/net/dsa/mv88e6xxx/global1_vtu.c | 8 +++---
drivers/net/dsa/mv88e6xxx/serdes.c | 15 ++++++----
drivers/net/dsa/mv88e6xxx/serdes.h | 8 +++---
6 files changed, 77 insertions(+), 29 deletions(-)
--
2.16.2
^ permalink raw reply
* [PATCH v2 net-next 2/2] net: dsa: mv88e6xxx: Make VTU miss violations less spammy
From: Andrew Lunn @ 2018-03-28 21:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn
In-Reply-To: <1522273829-23542-1-git-send-email-andrew@lunn.ch>
VTU miss violations can happen under normal conditions. Don't spam the
kernel log, downgrade the output to debug level only. The statistics
counter will indicate it is happening, if anybody not debugging is
interested.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
v2: Rather than delete, use dev_dbg_ratelimited()
---
drivers/net/dsa/mv88e6xxx/global1_vtu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mv88e6xxx/global1_vtu.c b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
index 2cbaf946e7ed..058326924f3e 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_vtu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
@@ -548,10 +548,11 @@ static irqreturn_t mv88e6xxx_g1_vtu_prob_irq_thread_fn(int irq, void *dev_id)
}
if (val & MV88E6XXX_G1_VTU_OP_MISS_VIOLATION) {
- dev_err_ratelimited(chip->dev, "VTU miss violation for vid %d, source port %d\n",
+ dev_dbg_ratelimited(chip->dev, "VTU miss violation for vid %d, source port %d\n",
entry.vid, spid);
chip->ports[spid].vtu_miss_violation++;
}
+
mutex_unlock(&chip->reg_lock);
return IRQ_HANDLED;
--
2.16.2
^ permalink raw reply related
* [PATCH v2 net-next 1/2] net: dsa: mv88e6xxx: Keep ATU/VTU violation statistics
From: Andrew Lunn @ 2018-03-28 21:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn
In-Reply-To: <1522273829-23542-1-git-send-email-andrew@lunn.ch>
Count the numbers of various ATU and VTU violation statistics and
return them as part of the ethtool -S statistics.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
v2: int -> unsigned int
---
drivers/net/dsa/mv88e6xxx/chip.c | 50 ++++++++++++++++++++++++++++-----
drivers/net/dsa/mv88e6xxx/chip.h | 13 ++++++---
drivers/net/dsa/mv88e6xxx/global1_atu.c | 12 +++++---
drivers/net/dsa/mv88e6xxx/global1_vtu.c | 8 ++++--
drivers/net/dsa/mv88e6xxx/serdes.c | 15 ++++++----
drivers/net/dsa/mv88e6xxx/serdes.h | 8 +++---
6 files changed, 78 insertions(+), 28 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 9a5d786b4885..3d2091099f7f 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -723,6 +723,24 @@ static int mv88e6320_stats_get_strings(struct mv88e6xxx_chip *chip,
STATS_TYPE_BANK0 | STATS_TYPE_BANK1);
}
+static const uint8_t *mv88e6xxx_atu_vtu_stats_strings[] = {
+ "atu_member_violation",
+ "atu_miss_violation",
+ "atu_full_violation",
+ "vtu_member_violation",
+ "vtu_miss_violation",
+};
+
+static void mv88e6xxx_atu_vtu_get_strings(uint8_t *data)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings); i++)
+ strlcpy(data + i * ETH_GSTRING_LEN,
+ mv88e6xxx_atu_vtu_stats_strings[i],
+ ETH_GSTRING_LEN);
+}
+
static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port,
uint8_t *data)
{
@@ -736,9 +754,12 @@ static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port,
if (chip->info->ops->serdes_get_strings) {
data += count * ETH_GSTRING_LEN;
- chip->info->ops->serdes_get_strings(chip, port, data);
+ count = chip->info->ops->serdes_get_strings(chip, port, data);
}
+ data += count * ETH_GSTRING_LEN;
+ mv88e6xxx_atu_vtu_get_strings(data);
+
mutex_unlock(&chip->reg_lock);
}
@@ -783,10 +804,13 @@ static int mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port)
if (chip->info->ops->serdes_get_sset_count)
serdes_count = chip->info->ops->serdes_get_sset_count(chip,
port);
- if (serdes_count < 0)
+ if (serdes_count < 0) {
count = serdes_count;
- else
- count += serdes_count;
+ goto out;
+ }
+ count += serdes_count;
+ count += ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings);
+
out:
mutex_unlock(&chip->reg_lock);
@@ -841,6 +865,16 @@ static int mv88e6390_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
0);
}
+static void mv88e6xxx_atu_vtu_get_stats(struct mv88e6xxx_chip *chip, int port,
+ uint64_t *data)
+{
+ *data++ = chip->ports[port].atu_member_violation;
+ *data++ = chip->ports[port].atu_miss_violation;
+ *data++ = chip->ports[port].atu_full_violation;
+ *data++ = chip->ports[port].vtu_member_violation;
+ *data++ = chip->ports[port].vtu_miss_violation;
+}
+
static void mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port,
uint64_t *data)
{
@@ -849,12 +883,14 @@ static void mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port,
if (chip->info->ops->stats_get_stats)
count = chip->info->ops->stats_get_stats(chip, port, data);
+ mutex_lock(&chip->reg_lock);
if (chip->info->ops->serdes_get_stats) {
data += count;
- mutex_lock(&chip->reg_lock);
- chip->info->ops->serdes_get_stats(chip, port, data);
- mutex_unlock(&chip->reg_lock);
+ count = chip->info->ops->serdes_get_stats(chip, port, data);
}
+ data += count;
+ mv88e6xxx_atu_vtu_get_stats(chip, port, data);
+ mutex_unlock(&chip->reg_lock);
}
static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index bad211014e91..80490f66bc06 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -194,6 +194,11 @@ struct mv88e6xxx_port_hwtstamp {
struct mv88e6xxx_port {
u64 serdes_stats[2];
+ u64 atu_member_violation;
+ u64 atu_miss_violation;
+ u64 atu_full_violation;
+ u64 vtu_member_violation;
+ u64 vtu_miss_violation;
};
struct mv88e6xxx_chip {
@@ -409,10 +414,10 @@ struct mv88e6xxx_ops {
/* Statistics from the SERDES interface */
int (*serdes_get_sset_count)(struct mv88e6xxx_chip *chip, int port);
- void (*serdes_get_strings)(struct mv88e6xxx_chip *chip, int port,
- uint8_t *data);
- void (*serdes_get_stats)(struct mv88e6xxx_chip *chip, int port,
- uint64_t *data);
+ int (*serdes_get_strings)(struct mv88e6xxx_chip *chip, int port,
+ uint8_t *data);
+ int (*serdes_get_stats)(struct mv88e6xxx_chip *chip, int port,
+ uint64_t *data);
/* VLAN Translation Unit operations */
int (*vtu_getnext)(struct mv88e6xxx_chip *chip,
diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
index 20d941f4273b..307410898fc9 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_atu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c
@@ -336,8 +336,6 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
if (err)
goto out;
- mutex_unlock(&chip->reg_lock);
-
if (val & MV88E6XXX_G1_ATU_OP_AGE_OUT_VIOLATION) {
dev_err_ratelimited(chip->dev,
"ATU age out violation for %pM\n",
@@ -348,17 +346,23 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
dev_err_ratelimited(chip->dev,
"ATU member violation for %pM portvec %x\n",
entry.mac, entry.portvec);
+ chip->ports[entry.portvec].atu_member_violation++;
}
- if (val & MV88E6XXX_G1_ATU_OP_MEMBER_VIOLATION)
+ if (val & MV88E6XXX_G1_ATU_OP_MEMBER_VIOLATION) {
dev_err_ratelimited(chip->dev,
"ATU miss violation for %pM portvec %x\n",
entry.mac, entry.portvec);
+ chip->ports[entry.portvec].atu_miss_violation++;
+ }
- if (val & MV88E6XXX_G1_ATU_OP_FULL_VIOLATION)
+ if (val & MV88E6XXX_G1_ATU_OP_FULL_VIOLATION) {
dev_err_ratelimited(chip->dev,
"ATU full violation for %pM portvec %x\n",
entry.mac, entry.portvec);
+ chip->ports[entry.portvec].atu_full_violation++;
+ }
+ mutex_unlock(&chip->reg_lock);
return IRQ_HANDLED;
diff --git a/drivers/net/dsa/mv88e6xxx/global1_vtu.c b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
index 7997961647de..2cbaf946e7ed 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_vtu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
@@ -539,18 +539,20 @@ static irqreturn_t mv88e6xxx_g1_vtu_prob_irq_thread_fn(int irq, void *dev_id)
if (err)
goto out;
- mutex_unlock(&chip->reg_lock);
-
spid = val & MV88E6XXX_G1_VTU_OP_SPID_MASK;
if (val & MV88E6XXX_G1_VTU_OP_MEMBER_VIOLATION) {
dev_err_ratelimited(chip->dev, "VTU member violation for vid %d, source port %d\n",
entry.vid, spid);
+ chip->ports[spid].vtu_member_violation++;
}
- if (val & MV88E6XXX_G1_VTU_OP_MISS_VIOLATION)
+ if (val & MV88E6XXX_G1_VTU_OP_MISS_VIOLATION) {
dev_err_ratelimited(chip->dev, "VTU miss violation for vid %d, source port %d\n",
entry.vid, spid);
+ chip->ports[spid].vtu_miss_violation++;
+ }
+ mutex_unlock(&chip->reg_lock);
return IRQ_HANDLED;
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index b6166424216a..fb058fd35c0d 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -106,20 +106,21 @@ int mv88e6352_serdes_get_sset_count(struct mv88e6xxx_chip *chip, int port)
return 0;
}
-void mv88e6352_serdes_get_strings(struct mv88e6xxx_chip *chip,
- int port, uint8_t *data)
+int mv88e6352_serdes_get_strings(struct mv88e6xxx_chip *chip,
+ int port, uint8_t *data)
{
struct mv88e6352_serdes_hw_stat *stat;
int i;
if (!mv88e6352_port_has_serdes(chip, port))
- return;
+ return 0;
for (i = 0; i < ARRAY_SIZE(mv88e6352_serdes_hw_stats); i++) {
stat = &mv88e6352_serdes_hw_stats[i];
memcpy(data + i * ETH_GSTRING_LEN, stat->string,
ETH_GSTRING_LEN);
}
+ return ARRAY_SIZE(mv88e6352_serdes_hw_stats);
}
static uint64_t mv88e6352_serdes_get_stat(struct mv88e6xxx_chip *chip,
@@ -149,8 +150,8 @@ static uint64_t mv88e6352_serdes_get_stat(struct mv88e6xxx_chip *chip,
return val;
}
-void mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
- uint64_t *data)
+int mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
+ uint64_t *data)
{
struct mv88e6xxx_port *mv88e6xxx_port = &chip->ports[port];
struct mv88e6352_serdes_hw_stat *stat;
@@ -158,7 +159,7 @@ void mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
int i;
if (!mv88e6352_port_has_serdes(chip, port))
- return;
+ return 0;
BUILD_BUG_ON(ARRAY_SIZE(mv88e6352_serdes_hw_stats) >
ARRAY_SIZE(mv88e6xxx_port->serdes_stats));
@@ -169,6 +170,8 @@ void mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
mv88e6xxx_port->serdes_stats[i] += value;
data[i] = mv88e6xxx_port->serdes_stats[i];
}
+
+ return ARRAY_SIZE(mv88e6352_serdes_hw_stats);
}
/* Set the power on/off for 10GBASE-R and 10GBASE-X4/X2 */
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.h b/drivers/net/dsa/mv88e6xxx/serdes.h
index 641baa75f910..1897c01c6e19 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.h
+++ b/drivers/net/dsa/mv88e6xxx/serdes.h
@@ -45,8 +45,8 @@
int mv88e6352_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on);
int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on);
int mv88e6352_serdes_get_sset_count(struct mv88e6xxx_chip *chip, int port);
-void mv88e6352_serdes_get_strings(struct mv88e6xxx_chip *chip,
- int port, uint8_t *data);
-void mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
- uint64_t *data);
+int mv88e6352_serdes_get_strings(struct mv88e6xxx_chip *chip,
+ int port, uint8_t *data);
+int mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
+ uint64_t *data);
#endif
--
2.16.2
^ permalink raw reply related
* Re: [PATCH 07/30] aio: add delayed cancel support
From: Al Viro @ 2018-03-28 21:34 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180328163526.GV30522@ZenIV.linux.org.uk>
On Wed, Mar 28, 2018 at 05:35:26PM +0100, Al Viro wrote:
> On Wed, Mar 28, 2018 at 09:29:03AM +0200, Christoph Hellwig wrote:
> > static void aio_fsync_work(struct work_struct *work)
> > {
> > struct fsync_iocb *req = container_of(work, struct fsync_iocb, work);
> > + struct aio_kiocb *iocb = container_of(req, struct aio_kiocb, fsync);
> > + struct file *file = req->file;
> > int ret;
> >
> > ret = vfs_fsync(req->file, req->datasync);
> > - fput(req->file);
> > - aio_complete(container_of(req, struct aio_kiocb, fsync), ret, 0);
> > + if (aio_complete(iocb, ret, 0, 0))
> > + fput(file);
>
> IDGI.
> 1) can aio_complete() ever return false here?
> 2) do we ever have aio_kiocb that would not have an associated
> struct file * that needs to be dropped on successful aio_complete()? AFAICS,
> rw, fsync and poll variants all have one, and I'm not sure what kind of
> async IO *could* be done without an opened file.
OK, hell with that. I've tried to play with turning kiocb into a struct with
anon union in it, with poll and fsync parts folded into that sucker and ki_filp
lifted into common part. Possible, but it's hairy as hell and can be done
afterwards.
However, doing that digging has turned up something really nasty. Look:
in io_cancel(2) you have
spin_lock_irq(&ctx->ctx_lock);
kiocb = lookup_kiocb(ctx, iocb, key);
if (kiocb) {
if (kiocb->flags & AIO_IOCB_DELAYED_CANCEL) {
kiocb->flags |= AIO_IOCB_CANCELLED;
} else {
ret = kiocb_cancel(kiocb);
kiocb = NULL;
}
}
spin_unlock_irq(&ctx->ctx_lock);
Now, suppose two threads call io_cancel() on the same aio_poll in progress.
Both hit that code and *both* find the same kiocb. Sure, the first one
will shortly do
if (kiocb)
ret = kiocb_cancel(kiocb);
which will remove it from the list. Too late, though - you've already dropped
->ctx_lock, letting the second one find it. Result: two aio_poll_cancel() in
parallel, with resulting double-free and double-fput().
You really need to remove it from the ->active_reqs before dropping the lock.
free_ioctx_users() does it correctly, io_cancel(2) fucks it up.
I'd add something like
struct aio_kiocb *kiocb_cancel_locked(struct aio_kiocb *kiocb)
{
if (!kiocb)
return ERR_PTR(-EINVAL);
if (kiocb->flags & AIO_IOCB_DELAYED_CANCEL) {
list_del(&kiocb->ki_list);
kiocb->flags |= AIO_IOCB_CANCELLED;
return kiocb;
} else {
return ERR_PTR(kiocb_cancel(kiocb));
}
}
with
spin_lock_irq(&ctx->ctx_lock);
while (!list_empty(&ctx->active_reqs)) {
req = list_first_entry(&ctx->active_reqs,
struct aio_kiocb, ki_list);
req = kiocb_cancel_locked(req);
if (!IS_ERR_OR_NULL(req))
list_add_tail(&req->ki_list, &list);
}
spin_unlock_irq(&ctx->ctx_lock);
in free_ioctx_users() and
spin_lock_irq(&ctx->ctx_lock);
kiocb = kiocb_cancel_locked(lookup_kiocb(ctx, iocb, key));
spin_unlock_irq(&ctx->ctx_lock);
ret = IS_ERR_OR_NULL(kiocb) ? PTR_ERR(kiocb) : kiocb_cancel(kiocb);
in io_cancel(2)...
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 4/6] rhashtable: allow a walk of the hash table without missing objects.
From: NeilBrown @ 2018-03-28 21:34 UTC (permalink / raw)
To: Herbert Xu; +Cc: Thomas Graf, netdev, linux-kernel
In-Reply-To: <20180328073023.GB17306@gondor.apana.org.au>
[-- Attachment #1: Type: text/plain, Size: 1561 bytes --]
On Wed, Mar 28 2018, Herbert Xu wrote:
> On Wed, Mar 28, 2018 at 06:17:57PM +1100, NeilBrown wrote:
>>
>> Sounds like over-kill to me.
>> It might be reasonable to have a CONFIG_DEBUG_RHASHTABLE which enables
>> extra to code to catch misuse, but I don't see the justification for
>> always performing these checks.
>> The DEBUG code could just scan the chain (usually quite short) to see if
>> the given element is present. Of course it might have already been
>> rehashed to the next table, so you would to allow for that possibility -
>> probably check tbl->rehash.
>
> No this is not meant to debug users incorrectly using the cursor.
> This is a replacement of your continue interface by automatically
> validating the cursor.
>
> In fact we can make it even more reliable. We can insert the walker
> right into the bucket chain, that way the walking will always be
> consistent.
>
> The only problem is that we need be able to differentiate between
> a walker, a normal object, and the end of the list. I think it
> should be doable.
Yes, I think that could work. The code to stop over a walker object
during an unlocked search wouldn't be straight forward and would need
careful analysis.
However about storing the hash chains in order by object address?
Then rhashtable_walk_start() can easily find it's place regardless of
whether the old object was still present or not, using <= on the
address.
"Insert" would need to record an insert location and insert there rather
than at the head of the chain.
I might try coding that.
Thanks,
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Benjamin Herrenschmidt @ 2018-03-28 21:31 UTC (permalink / raw)
To: Nicholas Piggin, David Miller
Cc: paulmck, arnd, linux-rdma, linuxppc-dev, linus971, will.deacon,
alexander.duyck, okaya, jgg, David.Laight, oohall, netdev,
alexander.h.duyck, torvalds
In-Reply-To: <20180329022324.037c3f39@roar.ozlabs.ibm.com>
On Thu, 2018-03-29 at 02:23 +1000, Nicholas Piggin wrote:
> On Wed, 28 Mar 2018 11:55:09 -0400 (EDT)
> David Miller <davem@davemloft.net> wrote:
>
> > From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Date: Thu, 29 Mar 2018 02:13:16 +1100
> >
> > > Let's fix all archs, it's way easier than fixing all drivers. Half of
> > > the archs are unused or dead anyway.
> >
> > Agreed.
>
> While we're making decrees here, can we do something about mmiowb?
> The semantics are basically indecipherable.
I was going to tackle that next :-)
> This is a variation on the mandatory write barrier that causes writes to weakly
> ordered I/O regions to be partially ordered. Its effects may go beyond the
> CPU->Hardware interface and actually affect the hardware at some level.
>
> How can a driver writer possibly get that right?
>
> IIRC it was added for some big ia64 system that was really expensive
> to implement the proper wmb() semantics on. So wmb() semantics were
> quietly downgraded, then the subsequently broken drivers they cared
> about were fixed by adding the stronger mmiowb().
>
> What should have happened was wmb and writel remained correct, sane, and
> expensive, and they add an mmio_wmb() to order MMIO stores made by the
> writel_relaxed accessors, then use that to speed up the few drivers they
> care about.
>
> Now that ia64 doesn't matter too much, can we deprecate mmiowb and just
> make wmb ordering talk about stores to the device, not to some
> intermediate stage of the interconnect where it can be subsequently
> reordered wrt the device? Drivers can be converted back to using wmb
> or writel gradually.
I was under the impression that mmiowb was specifically about ordering
writel's with a subsequent spin_unlock, without it, MMIOs from
different CPUs (within the same lock) would still arrive OO.
If that's indeed the case, I would suggest ia64 switches to a similar
per-cpu flag trick powerpc uses.
Cheers,
Ben.
> Thanks,
> Nick
^ permalink raw reply
* Re: [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
From: Rustad, Mark D @ 2018-03-28 21:31 UTC (permalink / raw)
To: Alexander Duyck
Cc: bhelgaas@google.com, Duyck, Alexander H,
linux-pci@vger.kernel.org, virtio-dev@lists.oasis-open.org,
kvm@vger.kernel.org, netdev@vger.kernel.org, Daly, Dan,
linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org,
Busch, Keith, netanel@amazon.com, ddutile@redhat.com,
mheyne@amazon.de, Wang, Liang-min, dwmw2@infradead.org,
hch@lst.de, "dwmw@amaz
In-Reply-To: <20180315184132.3102.90947.stgit@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 1765 bytes --]
On Mar 15, 2018, at 11:42 AM, Alexander Duyck <alexander.duyck@gmail.com> wrote:
> From: Alexander Duyck <alexander.h.duyck@intel.com>
>
> Hardware-realized virtio_pci devices can implement SR-IOV, so this
> patch enables its use. The device in question is an upcoming Intel
> NIC that implements both a virtio_net PF and virtio_net VFs. These
> are hardware realizations of what has been up to now been a software
> interface.
>
> The device in question has the following 4-part PCI IDs:
>
> PF: vendor: 1af4 device: 1041 subvendor: 8086 subdevice: 15fe
> VF: vendor: 1af4 device: 1041 subvendor: 8086 subdevice: 05fe
>
> The patch currently needs no check for device ID, because the callback
> will never be made for devices that do not assert the capability or
> when run on a platform incapable of SR-IOV.
>
> One reason for this patch is because the hardware requires the
> vendor ID of a VF to be the same as the vendor ID of the PF that
> created it. So it seemed logical to simply have a fully-functioning
> virtio_net PF create the VFs. This patch makes that possible.
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
>
> v4: Dropped call to pci_disable_sriov in virtio_pci_remove function
> v5: Replaced call to pci_sriov_configure_unmanaged with
> pci_sriov_configure_simple
> v6: Dropped "#ifdef" checks for IOV wrapping sriov_configure definition
> v7: No code change, added Reviewed-by
>
> drivers/virtio/virtio_pci_common.c | 1 +
> 1 file changed, 1 insertion(+)
Tested with the identified device.
Tested-by: Mark Rustad <mark.d.rustad@intel.com>
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply
* Re: [pci PATCH v7 1/5] pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources
From: Rustad, Mark D @ 2018-03-28 21:30 UTC (permalink / raw)
To: Alexander Duyck
Cc: bhelgaas@google.com, Duyck, Alexander H,
linux-pci@vger.kernel.org, virtio-dev@lists.oasis-open.org,
kvm@vger.kernel.org, Netdev, Daly, Dan, LKML,
linux-nvme@lists.infradead.org, Busch, Keith, netanel@amazon.com,
ddutile@redhat.com, mheyne@amazon.de, Wang, Liang-min,
dwmw2@infradead.org, hch@lst.de, dwmw@amazon.co.uk
In-Reply-To: <20180315184055.3102.2435.stgit@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 1040 bytes --]
On Mar 15, 2018, at 11:41 AM, Alexander Duyck <alexander.duyck@gmail.com> wrote:
> From: Alexander Duyck <alexander.h.duyck@intel.com>
>
> This patch adds a common configuration function called
> pci_sriov_configure_simple that will allow for managing VFs on devices
> where the PF is not capable of managing VF resources.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
>
> v5: New patch replacing pci_sriov_configure_unmanaged with
> pci_sriov_configure_simple
> Dropped bits related to autoprobe changes
> v6: Defined pci_sriov_configure_simple as NULL if IOV is disabled
> v7: Updated pci_sriov_configure_simple to drop need for err value
> Fixed comment explaining why pci_sriov_configure_simple is NULL
>
> drivers/pci/iov.c | 31 +++++++++++++++++++++++++++++++
> include/linux/pci.h | 3 +++
> 2 files changed, 34 insertions(+)
Tested with the device identified in patch #2.
Tested-by: Mark Rustad <mark.d.rustad@intel.com>
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply
* Re: [PATCH 5/6] rhashtable: support guaranteed successful insertion.
From: NeilBrown @ 2018-03-28 21:26 UTC (permalink / raw)
To: Herbert Xu; +Cc: Thomas Graf, netdev, linux-kernel
In-Reply-To: <20180328072727.GA17306@gondor.apana.org.au>
[-- Attachment #1: Type: text/plain, Size: 3887 bytes --]
On Wed, Mar 28 2018, Herbert Xu wrote:
> On Wed, Mar 28, 2018 at 06:04:40PM +1100, NeilBrown wrote:
>>
>> I disagree. My patch 6 only makes it common instead of exceedingly
>> rare. If any table in the list other than the first has a chain with 16
>> elements, then trying to insert an element with a hash which matches
>> that chain will fail with -EBUSY. This is theoretically possible
>> already, though astronomically unlikely. So that case will never be
>> tested for.
>
> No that's not true. If the table is correctly sized then the
> probability of having a chain with 16 elements is extremely low.
I say "astronomically unlikely", you say "probability .. is extremely
low". I think we are in agreement here.
The point remains that if an error *can* be returned then I have to
write code to handle it and test that code. I'd rather not.
>
> Even if it does happen we won't fail because we will perform
> an immediate rehash. We only fail if it happens right away
> after the rehash (that is, at least another 16 elements have
> been inserted and you're trying to insert a 17th element, all
> while the new hash table has not been completely populated),
> which means that somebody has figured out our hash secret and
> failing in that case makes sense.
>
>> It is hard to know if it is necessary. And making the new table larger
>> will make the error less likely, but still won't make it impossible. So
>> callers will have to handle it - just like they currently have to handle
>> -ENOMEM even though it is highly unlikely (and not strictly necessary).
>
> Callers should not handle an ENOMEM error by retrying. Nor should
> they retry an EBUSY return value.
I never suggested retrying, but I would have to handle it somehow. I'd
rather not.
>
>> Are these errors ever actually useful? I thought I had convinced myself
>> before that they were (to throttle attacks on the hash function), but
>> they happen even less often than I thought.
>
> The EBUSY error indicates that the hash table has essentially
> degenereated into a linked list because somebody has worked out
> our hash secret.
While I have no doubt that there are hashtables where someone could try
to attack the hash, I am quite sure there are others where is such an
attack is meaningless - any code which could generate the required range of
keys, could do far worse things more easily.
>
>> Maybe. Reading a percpu counter isn't cheap. Reading it whenever a hash
>> chain reaches 16 is reasonable, but I think we would want to read it a
>> lot more often than that. So probably store the last-sampled time (with
>> no locking) and only sample the counter if last-sampled is more than
>> jiffies - 10*HZ (???)
>
> We could also take the spinlock table approach and have a counter
> per bucket spinlock. This should be sufficient as you'll contend
> on the bucket spinlock table anyway.
Yes, storing a sharded count in the spinlock table does seem like an
appropriate granularity. However that leads me to ask: why do we have
the spinlock table? Why not bit spinlocks in the hashchain head like
include/linux/list_bl uses?
>
> This also allows us to estimate the total table size and not have
> to always do a last-ditch growth when it's too late.
I don't understand how it can ever be "too late", though I appreciate
that in some cases "sooner" is better than "later"
If we give up on the single atomic_t counter, then we must accept that
the number of elements could exceed any given value. The only promise
we can provide is that it wont exceed N% of the table size for more than
T seconds.
Thanks,
NeilBrown
>
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 2/3] net: systemport: Fix coalescing settings handling
From: Tal Gilboa @ 2018-03-28 21:23 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: davem, jaedon.shin, pgynther, opendmb, Michael Chan, gospo,
saeedm
In-Reply-To: <20180327194707.31857-3-f.fainelli@gmail.com>
On 3/27/2018 10:47 PM, Florian Fainelli wrote:
> There were a number of issues with setting the RX coalescing parameters:
>
> - we would not be preserving values that would have been configured
> across close/open calls, instead we would always reset to no timeout
> and 1 interrupt per packet, this would also prevent DIM from setting its
> default usec/pkts values
>
> - when adaptive RX would be turned on, we woud not be fetching the
> default parameters, we would stay with no timeout/1 packet per
> interrupt until the estimator kicks in and changes that
>
> - finally disabling adaptive RX coalescing while providing parameters
> would not be honored, and we would stay with whatever DIM had
> previously determined instead of the user requested parameters
>
> Fixes: b6e0e875421e ("net: systemport: Implement adaptive interrupt coalescing")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/ethernet/broadcom/bcmsysport.c | 57 ++++++++++++++++++++----------
> drivers/net/ethernet/broadcom/bcmsysport.h | 4 +--
> 2 files changed, 41 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
> index 1e52bb7d822e..43ad6300c351 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
> @@ -574,16 +574,16 @@ static int bcm_sysport_set_wol(struct net_device *dev,
> return 0;
> }
>
> -static void bcm_sysport_set_rx_coalesce(struct bcm_sysport_priv *priv)
> +static void bcm_sysport_set_rx_coalesce(struct bcm_sysport_priv *priv,
> + u32 usecs, u32 pkts)
> {
> u32 reg;
>
> reg = rdma_readl(priv, RDMA_MBDONE_INTR);
> reg &= ~(RDMA_INTR_THRESH_MASK |
> RDMA_TIMEOUT_MASK << RDMA_TIMEOUT_SHIFT);
> - reg |= priv->dim.coal_pkts;
> - reg |= DIV_ROUND_UP(priv->dim.coal_usecs * 1000, 8192) <<
> - RDMA_TIMEOUT_SHIFT;
> + reg |= pkts;
> + reg |= DIV_ROUND_UP(usecs * 1000, 8192) << RDMA_TIMEOUT_SHIFT;
> rdma_writel(priv, reg, RDMA_MBDONE_INTR);
> }
>
> @@ -626,6 +626,8 @@ static int bcm_sysport_set_coalesce(struct net_device *dev,
> struct ethtool_coalesce *ec)
> {
> struct bcm_sysport_priv *priv = netdev_priv(dev);
> + struct net_dim_cq_moder moder;
> + u32 usecs, pkts;
> unsigned int i;
>
> /* Base system clock is 125Mhz, DMA timeout is this reference clock
> @@ -646,15 +648,22 @@ static int bcm_sysport_set_coalesce(struct net_device *dev,
> for (i = 0; i < dev->num_tx_queues; i++)
> bcm_sysport_set_tx_coalesce(&priv->tx_rings[i], ec);
>
> - priv->dim.coal_usecs = ec->rx_coalesce_usecs;
> - priv->dim.coal_pkts = ec->rx_max_coalesced_frames;
> + priv->rx_coalesce_usecs = ec->rx_coalesce_usecs;
> + priv->rx_max_coalesced_frames = ec->rx_max_coalesced_frames;
> + usecs = priv->rx_coalesce_usecs;
> + pkts = priv->rx_max_coalesced_frames;
>
> - if (!ec->use_adaptive_rx_coalesce && priv->dim.use_dim) {
> - priv->dim.coal_pkts = 1;
> - priv->dim.coal_usecs = 0;
> + /* If DIM is enabled, immediately obtain default parameters */
> + if (ec->use_adaptive_rx_coalesce) {
> + moder = net_dim_get_def_profile(priv->dim.dim.mode);
> + usecs = moder.usec;
> + pkts = moder.pkts;
> }
What if DIM is already in use? We don't want to set default values if
DIM is already working.
> +
> priv->dim.use_dim = ec->use_adaptive_rx_coalesce;
> - bcm_sysport_set_rx_coalesce(priv);
> +
> + /* Apply desired coalescing parameters */
> + bcm_sysport_set_rx_coalesce(priv, usecs, pkts);
>
> return 0;
> }
> @@ -1058,10 +1067,7 @@ static void bcm_sysport_dim_work(struct work_struct *work)
> struct net_dim_cq_moder cur_profile =
> net_dim_get_profile(dim->mode, dim->profile_ix);
>
> - priv->dim.coal_usecs = cur_profile.usec;
> - priv->dim.coal_pkts = cur_profile.pkts;
> -
> - bcm_sysport_set_rx_coalesce(priv);
> + bcm_sysport_set_rx_coalesce(priv, cur_profile.usec, cur_profile.pkts);
> dim->state = NET_DIM_START_MEASURE;
> }
>
> @@ -1408,14 +1414,30 @@ static void bcm_sysport_adj_link(struct net_device *dev)
> phy_print_status(phydev);
> }
>
> -static void bcm_sysport_init_dim(struct bcm_sysport_net_dim *dim,
> +static void bcm_sysport_init_dim(struct bcm_sysport_priv *priv,
> void (*cb)(struct work_struct *work))
> {
> + struct bcm_sysport_net_dim *dim = &priv->dim;
> + struct net_dim_cq_moder moder;
> + u32 usecs, pkts;
> +
> INIT_WORK(&dim->dim.work, cb);
> dim->dim.mode = NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
> dim->event_ctr = 0;
> dim->packets = 0;
> dim->bytes = 0;
I would expect only dim related code in a function called init_dim(). I
think the code below should be elsewhere. Or maybe change the function
name to init_rx_coalesce().
> +
> + usecs = priv->rx_coalesce_usecs;
> + pkts = priv->rx_max_coalesced_frames;
> +
> + /* If DIM was enabled, re-apply default parameters */
> + if (dim->use_dim) {
> + moder = net_dim_get_def_profile(dim->dim.mode);
> + usecs = moder.usec;
> + pkts = moder.pkts;
> + }
> +
> + bcm_sysport_set_rx_coalesce(priv, usecs, pkts);
> }
>
> static int bcm_sysport_init_tx_ring(struct bcm_sysport_priv *priv,
> @@ -1658,8 +1680,6 @@ static int bcm_sysport_init_rx_ring(struct bcm_sysport_priv *priv)
> rdma_writel(priv, 0, RDMA_END_ADDR_HI);
> rdma_writel(priv, priv->num_rx_desc_words - 1, RDMA_END_ADDR_LO);
>
> - rdma_writel(priv, 1, RDMA_MBDONE_INTR);
> -
> netif_dbg(priv, hw, priv->netdev,
> "RDMA cfg, num_rx_bds=%d, rx_bds=%p\n",
> priv->num_rx_bds, priv->rx_bds);
> @@ -1827,7 +1847,7 @@ static void bcm_sysport_netif_start(struct net_device *dev)
> struct bcm_sysport_priv *priv = netdev_priv(dev);
>
> /* Enable NAPI */
> - bcm_sysport_init_dim(&priv->dim, bcm_sysport_dim_work);
> + bcm_sysport_init_dim(priv, bcm_sysport_dim_work);
> napi_enable(&priv->napi);
>
> /* Enable RX interrupt and TX ring full interrupt */
> @@ -2333,6 +2353,7 @@ static int bcm_sysport_probe(struct platform_device *pdev)
> /* libphy will adjust the link state accordingly */
> netif_carrier_off(dev);
>
> + priv->rx_max_coalesced_frames = 1;
> u64_stats_init(&priv->syncp);
>
> priv->dsa_notifier.notifier_call = bcm_sysport_dsa_notifier;
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
> index 57e18ef8f206..d6e5d0cbf3a3 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.h
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.h
> @@ -701,8 +701,6 @@ struct bcm_sysport_net_dim {
> u16 event_ctr;
> unsigned long packets;
> unsigned long bytes;
> - u32 coal_usecs;
> - u32 coal_pkts;
> struct net_dim dim;
> };
>
> @@ -755,6 +753,8 @@ struct bcm_sysport_priv {
> unsigned int rx_c_index;
>
> struct bcm_sysport_net_dim dim;
> + u32 rx_max_coalesced_frames;
> + u32 rx_coalesce_usecs;
>
> /* PHY device */
> struct device_node *phy_dn;
>
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: bcmgenet: Fix coalescing settings handling
From: Tal Gilboa @ 2018-03-28 21:23 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: davem, jaedon.shin, pgynther, opendmb, Michael Chan, gospo,
saeedm
In-Reply-To: <20180327194707.31857-4-f.fainelli@gmail.com>
On 3/27/2018 10:47 PM, Florian Fainelli wrote:
> There were a number of issues with setting the RX coalescing parameters:
>
> - we would not be preserving values that would have been configured
> across close/open calls, instead we would always reset to no timeout
> and 1 interrupt per packet, this would also prevent DIM from setting its
> default usec/pkts values
>
> - when adaptive RX would be turned on, we woud not be fetching the
> default parameters, we would stay with no timeout/1 packet per interrupt
> until the estimator kicks in and changes that
>
> - finally disabling adaptive RX coalescing while providing parameters
> would not be honored, and we would stay with whatever DIM had previously
> determined instead of the user requested parameters
>
> Fixes: 9f4ca05827a2 ("net: bcmgenet: Add support for adaptive RX coalescing")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 78 ++++++++++++++++++--------
> drivers/net/ethernet/broadcom/genet/bcmgenet.h | 4 +-
> 2 files changed, 57 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 7db8edc643ec..76409debb796 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -625,18 +625,18 @@ static int bcmgenet_get_coalesce(struct net_device *dev,
> return 0;
> }
>
> -static void bcmgenet_set_rx_coalesce(struct bcmgenet_rx_ring *ring)
> +static void bcmgenet_set_rx_coalesce(struct bcmgenet_rx_ring *ring,
> + u32 usecs, u32 pkts)
> {
> struct bcmgenet_priv *priv = ring->priv;
> unsigned int i = ring->index;
> u32 reg;
>
> - bcmgenet_rdma_ring_writel(priv, i, ring->dim.coal_pkts,
> - DMA_MBUF_DONE_THRESH);
> + bcmgenet_rdma_ring_writel(priv, i, pkts, DMA_MBUF_DONE_THRESH);
>
> reg = bcmgenet_rdma_readl(priv, DMA_RING0_TIMEOUT + i);
> reg &= ~DMA_TIMEOUT_MASK;
> - reg |= DIV_ROUND_UP(ring->dim.coal_usecs * 1000, 8192);
> + reg |= DIV_ROUND_UP(usecs * 1000, 8192);
> bcmgenet_rdma_writel(priv, reg, DMA_RING0_TIMEOUT + i);
> }
>
> @@ -645,6 +645,8 @@ static int bcmgenet_set_coalesce(struct net_device *dev,
> {
> struct bcmgenet_priv *priv = netdev_priv(dev);
> struct bcmgenet_rx_ring *ring;
> + struct net_dim_cq_moder moder;
> + u32 usecs, pkts;
> unsigned int i;
>
> /* Base system clock is 125Mhz, DMA timeout is this reference clock
> @@ -682,25 +684,37 @@ static int bcmgenet_set_coalesce(struct net_device *dev,
>
> for (i = 0; i < priv->hw_params->rx_queues; i++) {
> ring = &priv->rx_rings[i];
> - ring->dim.coal_usecs = ec->rx_coalesce_usecs;
> - ring->dim.coal_pkts = ec->rx_max_coalesced_frames;
> - if (!ec->use_adaptive_rx_coalesce && ring->dim.use_dim) {
> - ring->dim.coal_pkts = 1;
> - ring->dim.coal_usecs = 0;
> +
> + ring->rx_coalesce_usecs = ec->rx_coalesce_usecs;
> + ring->rx_max_coalesced_frames = ec->rx_max_coalesced_frames;
> + usecs = ring->rx_coalesce_usecs;
> + pkts = ring->rx_max_coalesced_frames;
> +
> + if (ec->use_adaptive_rx_coalesce) {
> + moder = net_dim_get_def_profile(ring->dim.dim.mode);
> + usecs = moder.usec;
> + pkts = moder.pkts;
> }
What if DIM is already in use? We don't want to set default values if
DIM is already working.
> +
> ring->dim.use_dim = ec->use_adaptive_rx_coalesce;
> - bcmgenet_set_rx_coalesce(ring);
> + bcmgenet_set_rx_coalesce(ring, usecs, pkts);
> }
>
> ring = &priv->rx_rings[DESC_INDEX];
> - ring->dim.coal_usecs = ec->rx_coalesce_usecs;
> - ring->dim.coal_pkts = ec->rx_max_coalesced_frames;
> - if (!ec->use_adaptive_rx_coalesce && ring->dim.use_dim) {
> - ring->dim.coal_pkts = 1;
> - ring->dim.coal_usecs = 0;
> +
> + ring->rx_coalesce_usecs = ec->rx_coalesce_usecs;
> + ring->rx_max_coalesced_frames = ec->rx_max_coalesced_frames;
> + usecs = ring->rx_coalesce_usecs;
> + pkts = ring->rx_max_coalesced_frames;
> +
> + if (ec->use_adaptive_rx_coalesce) {
> + moder = net_dim_get_def_profile(ring->dim.dim.mode);
> + usecs = moder.usec;
> + pkts = moder.pkts;
> }
Same as above.
> +
> ring->dim.use_dim = ec->use_adaptive_rx_coalesce;
> - bcmgenet_set_rx_coalesce(ring);
> + bcmgenet_set_rx_coalesce(ring, usecs, pkts);
>
> return 0;
> }
> @@ -1924,10 +1938,7 @@ static void bcmgenet_dim_work(struct work_struct *work)
> struct net_dim_cq_moder cur_profile =
> net_dim_get_profile(dim->mode, dim->profile_ix);
>
> - ring->dim.coal_usecs = cur_profile.usec;
> - ring->dim.coal_pkts = cur_profile.pkts;
> -
> - bcmgenet_set_rx_coalesce(ring);
> + bcmgenet_set_rx_coalesce(ring, cur_profile.usec, cur_profile.pkts);
> dim->state = NET_DIM_START_MEASURE;
> }
>
> @@ -2079,14 +2090,30 @@ static void init_umac(struct bcmgenet_priv *priv)
> dev_dbg(kdev, "done init umac\n");
> }
>
> -static void bcmgenet_init_dim(struct bcmgenet_net_dim *dim,
> +static void bcmgenet_init_dim(struct bcmgenet_rx_ring *ring,
> void (*cb)(struct work_struct *work))
> {
> + struct bcmgenet_net_dim *dim = &ring->dim;
> + struct net_dim_cq_moder moder;
> + u32 usecs, pkts;
> +
> INIT_WORK(&dim->dim.work, cb);
> dim->dim.mode = NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
> dim->event_ctr = 0;
> dim->packets = 0;
> dim->bytes = 0;
I would expect only dim related code in a function called init_dim(). I
think the code below should be elsewhere. Or maybe change the function
name to init_rx_coalesce().
> +
> + usecs = ring->rx_coalesce_usecs;
> + pkts = ring->rx_max_coalesced_frames;
> +
> + /* If DIM was enabled, re-apply default parameters */
> + if (dim->use_dim) {
> + moder = net_dim_get_def_profile(dim->dim.mode);
> + usecs = moder.usec;
> + pkts = moder.pkts;
> + }
> +
> + bcmgenet_set_rx_coalesce(ring, usecs, pkts);
> }
>
> /* Initialize a Tx ring along with corresponding hardware registers */
> @@ -2178,7 +2205,7 @@ static int bcmgenet_init_rx_ring(struct bcmgenet_priv *priv,
> if (ret)
> return ret;
>
> - bcmgenet_init_dim(&ring->dim, bcmgenet_dim_work);
> + bcmgenet_init_dim(ring, bcmgenet_dim_work);
>
> /* Initialize Rx NAPI */
> netif_napi_add(priv->dev, &ring->napi, bcmgenet_rx_poll,
> @@ -2186,7 +2213,6 @@ static int bcmgenet_init_rx_ring(struct bcmgenet_priv *priv,
>
> bcmgenet_rdma_ring_writel(priv, index, 0, RDMA_PROD_INDEX);
> bcmgenet_rdma_ring_writel(priv, index, 0, RDMA_CONS_INDEX);
> - bcmgenet_rdma_ring_writel(priv, index, 1, DMA_MBUF_DONE_THRESH);
> bcmgenet_rdma_ring_writel(priv, index,
> ((size << DMA_RING_SIZE_SHIFT) |
> RX_BUF_LENGTH), DMA_RING_BUF_SIZE);
> @@ -3424,6 +3450,7 @@ static int bcmgenet_probe(struct platform_device *pdev)
> struct net_device *dev;
> const void *macaddr;
> struct resource *r;
> + unsigned int i;
> int err = -EIO;
> const char *phy_mode_str;
>
> @@ -3552,6 +3579,11 @@ static int bcmgenet_probe(struct platform_device *pdev)
> netif_set_real_num_tx_queues(priv->dev, priv->hw_params->tx_queues + 1);
> netif_set_real_num_rx_queues(priv->dev, priv->hw_params->rx_queues + 1);
>
> + /* Set default coalescing parameters */
> + for (i = 0; i < priv->hw_params->rx_queues; i++)
> + priv->rx_rings[i].rx_max_coalesced_frames = 1;
> + priv->rx_rings[DESC_INDEX].rx_max_coalesced_frames = 1;
> +
> /* libphy will determine the link state */
> netif_carrier_off(dev);
>
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> index 22c41e0430fb..b773bc07edf7 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> @@ -578,8 +578,6 @@ struct bcmgenet_net_dim {
> u16 event_ctr;
> unsigned long packets;
> unsigned long bytes;
> - u32 coal_usecs;
> - u32 coal_pkts;
> struct net_dim dim;
> };
>
> @@ -598,6 +596,8 @@ struct bcmgenet_rx_ring {
> unsigned int end_ptr; /* Rx ring end CB ptr */
> unsigned int old_discards;
> struct bcmgenet_net_dim dim;
> + u32 rx_max_coalesced_frames;
> + u32 rx_coalesce_usecs;
> void (*int_enable)(struct bcmgenet_rx_ring *);
> void (*int_disable)(struct bcmgenet_rx_ring *);
> struct bcmgenet_priv *priv;
>
^ permalink raw reply
* Re: [PATCH] sunrpc: remove incorrect HMAC request initialization
From: J . Bruce Fields @ 2018-03-28 21:12 UTC (permalink / raw)
To: Eric Biggers
Cc: Trond Myklebust, Anna Schumaker, Jeff Layton, linux-nfs, netdev,
linux-crypto, Herbert Xu, Michael Young, stable
In-Reply-To: <20180328175722.193355-1-ebiggers@google.com>
Applying, thanks!--b.
On Wed, Mar 28, 2018 at 10:57:22AM -0700, Eric Biggers wrote:
> make_checksum_hmac_md5() is allocating an HMAC transform and doing
> crypto API calls in the following order:
>
> crypto_ahash_init()
> crypto_ahash_setkey()
> crypto_ahash_digest()
>
> This is wrong because it makes no sense to init() the request before a
> key has been set, given that the initial state depends on the key. And
> digest() is short for init() + update() + final(), so in this case
> there's no need to explicitly call init() at all.
>
> Before commit 9fa68f620041 ("crypto: hash - prevent using keyed hashes
> without setting key") the extra init() had no real effect, at least for
> the software HMAC implementation. (There are also hardware drivers that
> implement HMAC-MD5, and it's not immediately obvious how gracefully they
> handle init() before setkey().) But now the crypto API detects this
> incorrect initialization and returns -ENOKEY. This is breaking NFS
> mounts in some cases.
>
> Fix it by removing the incorrect call to crypto_ahash_init().
>
> Reported-by: Michael Young <m.a.young@durham.ac.uk>
> Fixes: 9fa68f620041 ("crypto: hash - prevent using keyed hashes without setting key")
> Fixes: fffdaef2eb4a ("gss_krb5: Add support for rc4-hmac encryption")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> net/sunrpc/auth_gss/gss_krb5_crypto.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/net/sunrpc/auth_gss/gss_krb5_crypto.c b/net/sunrpc/auth_gss/gss_krb5_crypto.c
> index 12649c9fedab..8654494b4d0a 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_crypto.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_crypto.c
> @@ -237,9 +237,6 @@ make_checksum_hmac_md5(struct krb5_ctx *kctx, char *header, int hdrlen,
>
> ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP, NULL, NULL);
>
> - err = crypto_ahash_init(req);
> - if (err)
> - goto out;
> err = crypto_ahash_setkey(hmac_md5, cksumkey, kctx->gk5e->keylength);
> if (err)
> goto out;
> --
> 2.17.0.rc1.321.gba9d0f2565-goog
^ 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