* [PATCH net-next] skbuff: only inherit relevant tx_flags
From: Willem de Bruijn @ 2017-06-08 15:35 UTC (permalink / raw)
To: netdev; +Cc: davem, soheil, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
When inheriting tx_flags from one skbuff to another, always apply a
mask to avoid overwriting unrelated other bits in the field.
The two SKBTX_SHARED_FRAG cases clears all other bits. In practice,
tx_flags are zero at this point now. But this is fragile. Timestamp
flags are set, for instance, if in tcp_gso_segment, after this clear
in skb_segment.
The SKBTX_ANY_TSTAMP mask in __skb_tstamp_tx ensures that new
skbs do not accidentally inherit flags such as SKBTX_SHARED_FRAG.
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/core/skbuff.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 82cfc9c7a090..e508c1eae67f 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2646,7 +2646,8 @@ void skb_split(struct sk_buff *skb, struct sk_buff *skb1, const u32 len)
{
int pos = skb_headlen(skb);
- skb_shinfo(skb1)->tx_flags = skb_shinfo(skb)->tx_flags & SKBTX_SHARED_FRAG;
+ skb_shinfo(skb1)->tx_flags |= skb_shinfo(skb)->tx_flags &
+ SKBTX_SHARED_FRAG;
if (len < pos) /* Split line is inside header. */
skb_split_inside_header(skb, skb1, len, pos);
else /* Second chunk has no header, nothing to copy. */
@@ -3261,8 +3262,8 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
skb_copy_from_linear_data_offset(head_skb, offset,
skb_put(nskb, hsize), hsize);
- skb_shinfo(nskb)->tx_flags = skb_shinfo(head_skb)->tx_flags &
- SKBTX_SHARED_FRAG;
+ skb_shinfo(nskb)->tx_flags |= skb_shinfo(head_skb)->tx_flags &
+ SKBTX_SHARED_FRAG;
while (pos < offset + len) {
if (i >= nfrags) {
@@ -3948,7 +3949,8 @@ void __skb_tstamp_tx(struct sk_buff *orig_skb,
return;
if (tsonly) {
- skb_shinfo(skb)->tx_flags = skb_shinfo(orig_skb)->tx_flags;
+ skb_shinfo(skb)->tx_flags |= skb_shinfo(orig_skb)->tx_flags &
+ SKBTX_ANY_TSTAMP;
skb_shinfo(skb)->tskey = skb_shinfo(orig_skb)->tskey;
}
--
2.13.0.506.g27d5fe0cd-goog
^ permalink raw reply related
* Re: [PATCH] mISDN: remove unnecessary variable assignments
From: David Miller @ 2017-06-08 15:33 UTC (permalink / raw)
To: garsilva; +Cc: isdn, netdev, linux-kernel
In-Reply-To: <20170607201501.GA7245@embeddedgus>
From: "Gustavo A. R. Silva" <garsilva@embeddedor.com>
Date: Wed, 7 Jun 2017 15:15:01 -0500
> Remove unnecessary variable assignments.
>
> Addresses-Coverity-ID: 1226917
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
Applied to net-next, thanks.
^ permalink raw reply
* [PATCH 2/5] net: mvpp2: use {get,put}_cpu() instead of smp_processor_id()
From: Thomas Petazzoni @ 2017-06-08 15:27 UTC (permalink / raw)
To: David S. Miller, netdev
Cc: Marcin Wojtas, Stefan Chulski, Nadav Haklai, Hanna Hawa,
Yehuda Yitschak, Antoine Tenart, Gregory Clement, Markus Elfring,
Marc Zyngier, Thomas Petazzoni
In-Reply-To: <1496935647-29670-1-git-send-email-thomas.petazzoni@free-electrons.com>
smp_processor_id() should not be used in migration-enabled contexts. We
originally thought it was OK in the specific situation of this driver,
but it was wrong, and calling smp_processor_id() in a migration-enabled
context prints a big fat warning when CONFIG_DEBUG_PREEMPT=y.
Therefore, this commit replaces the smp_processor_id() in
migration-enabled contexts by the appropriate get_cpu/put_cpu sections.
Reported-by: Marc Zyngier <marc.zyngier@arm.com>
Fixes: a786841df72e ("net: mvpp2: handle register mapping and access for PPv2.2")
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/net/ethernet/marvell/mvpp2.c | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index cff45e3..fb6e9dc 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -3715,7 +3715,7 @@ static void mvpp2_bm_bufs_get_addrs(struct device *dev, struct mvpp2 *priv,
dma_addr_t *dma_addr,
phys_addr_t *phys_addr)
{
- int cpu = smp_processor_id();
+ int cpu = get_cpu();
*dma_addr = mvpp2_percpu_read(priv, cpu,
MVPP2_BM_PHY_ALLOC_REG(bm_pool->id));
@@ -3736,6 +3736,8 @@ static void mvpp2_bm_bufs_get_addrs(struct device *dev, struct mvpp2 *priv,
if (sizeof(phys_addr_t) == 8)
*phys_addr |= (u64)phys_addr_highbits << 32;
}
+
+ put_cpu();
}
/* Free all buffers from the pool */
@@ -3921,7 +3923,7 @@ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool,
dma_addr_t buf_dma_addr,
phys_addr_t buf_phys_addr)
{
- int cpu = smp_processor_id();
+ int cpu = get_cpu();
if (port->priv->hw_version == MVPP22) {
u32 val = 0;
@@ -3948,6 +3950,8 @@ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool,
MVPP2_BM_VIRT_RLS_REG, buf_phys_addr);
mvpp2_percpu_write(port->priv, cpu,
MVPP2_BM_PHY_RLS_REG(pool), buf_dma_addr);
+
+ put_cpu();
}
/* Refill BM pool */
@@ -4730,7 +4734,7 @@ static void mvpp2_txp_max_tx_size_set(struct mvpp2_port *port)
static void mvpp2_rx_pkts_coal_set(struct mvpp2_port *port,
struct mvpp2_rx_queue *rxq)
{
- int cpu = smp_processor_id();
+ int cpu = get_cpu();
if (rxq->pkts_coal > MVPP2_OCCUPIED_THRESH_MASK)
rxq->pkts_coal = MVPP2_OCCUPIED_THRESH_MASK;
@@ -4738,6 +4742,8 @@ static void mvpp2_rx_pkts_coal_set(struct mvpp2_port *port,
mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_NUM_REG, rxq->id);
mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_THRESH_REG,
rxq->pkts_coal);
+
+ put_cpu();
}
static u32 mvpp2_usec_to_cycles(u32 usec, unsigned long clk_hz)
@@ -4918,7 +4924,7 @@ static int mvpp2_rxq_init(struct mvpp2_port *port,
mvpp2_write(port->priv, MVPP2_RXQ_STATUS_REG(rxq->id), 0);
/* Set Rx descriptors queue starting address - indirect access */
- cpu = smp_processor_id();
+ cpu = get_cpu();
mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_NUM_REG, rxq->id);
if (port->priv->hw_version == MVPP21)
rxq_dma = rxq->descs_dma;
@@ -4927,6 +4933,7 @@ static int mvpp2_rxq_init(struct mvpp2_port *port,
mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_DESC_ADDR_REG, rxq_dma);
mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_DESC_SIZE_REG, rxq->size);
mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_INDEX_REG, 0);
+ put_cpu();
/* Set Offset */
mvpp2_rxq_offset_set(port, rxq->id, NET_SKB_PAD);
@@ -4989,10 +4996,11 @@ static void mvpp2_rxq_deinit(struct mvpp2_port *port,
* free descriptor number
*/
mvpp2_write(port->priv, MVPP2_RXQ_STATUS_REG(rxq->id), 0);
- cpu = smp_processor_id();
+ cpu = get_cpu();
mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_NUM_REG, rxq->id);
mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_DESC_ADDR_REG, 0);
mvpp2_percpu_write(port->priv, cpu, MVPP2_RXQ_DESC_SIZE_REG, 0);
+ put_cpu();
}
/* Create and initialize a Tx queue */
@@ -5015,7 +5023,7 @@ static int mvpp2_txq_init(struct mvpp2_port *port,
txq->last_desc = txq->size - 1;
/* Set Tx descriptors queue starting address - indirect access */
- cpu = smp_processor_id();
+ cpu = get_cpu();
mvpp2_percpu_write(port->priv, cpu, MVPP2_TXQ_NUM_REG, txq->id);
mvpp2_percpu_write(port->priv, cpu, MVPP2_TXQ_DESC_ADDR_REG,
txq->descs_dma);
@@ -5040,6 +5048,7 @@ static int mvpp2_txq_init(struct mvpp2_port *port,
mvpp2_percpu_write(port->priv, cpu, MVPP2_TXQ_PREF_BUF_REG,
MVPP2_PREF_BUF_PTR(desc) | MVPP2_PREF_BUF_SIZE_16 |
MVPP2_PREF_BUF_THRESH(desc_per_txq / 2));
+ put_cpu();
/* WRR / EJP configuration - indirect access */
tx_port_num = mvpp2_egress_port(port);
@@ -5110,10 +5119,11 @@ static void mvpp2_txq_deinit(struct mvpp2_port *port,
mvpp2_write(port->priv, MVPP2_TXQ_SCHED_TOKEN_CNTR_REG(txq->id), 0);
/* Set Tx descriptors queue starting address and size */
- cpu = smp_processor_id();
+ cpu = get_cpu();
mvpp2_percpu_write(port->priv, cpu, MVPP2_TXQ_NUM_REG, txq->id);
mvpp2_percpu_write(port->priv, cpu, MVPP2_TXQ_DESC_ADDR_REG, 0);
mvpp2_percpu_write(port->priv, cpu, MVPP2_TXQ_DESC_SIZE_REG, 0);
+ put_cpu();
}
/* Cleanup Tx ports */
@@ -5123,7 +5133,7 @@ static void mvpp2_txq_clean(struct mvpp2_port *port, struct mvpp2_tx_queue *txq)
int delay, pending, cpu;
u32 val;
- cpu = smp_processor_id();
+ cpu = get_cpu();
mvpp2_percpu_write(port->priv, cpu, MVPP2_TXQ_NUM_REG, txq->id);
val = mvpp2_percpu_read(port->priv, cpu, MVPP2_TXQ_PREF_BUF_REG);
val |= MVPP2_TXQ_DRAIN_EN_MASK;
@@ -5150,6 +5160,7 @@ static void mvpp2_txq_clean(struct mvpp2_port *port, struct mvpp2_tx_queue *txq)
val &= ~MVPP2_TXQ_DRAIN_EN_MASK;
mvpp2_percpu_write(port->priv, cpu, MVPP2_TXQ_PREF_BUF_REG, val);
+ put_cpu();
for_each_present_cpu(cpu) {
txq_pcpu = per_cpu_ptr(txq->pcpu, cpu);
--
2.7.4
^ permalink raw reply related
* [PATCH 3/5] net: mvpp2: add comments about smp_processor_id() usage
From: Thomas Petazzoni @ 2017-06-08 15:27 UTC (permalink / raw)
To: David S. Miller, netdev
Cc: Marcin Wojtas, Stefan Chulski, Nadav Haklai, Hanna Hawa,
Yehuda Yitschak, Antoine Tenart, Gregory Clement, Markus Elfring,
Marc Zyngier, Thomas Petazzoni
In-Reply-To: <1496935647-29670-1-git-send-email-thomas.petazzoni@free-electrons.com>
A previous commit modified a number of smp_processor_id() used in
migration-enabled contexts into get_cpu/put_cpu sections. However, a few
smp_processor_id() calls remain in the driver, and this commit adds
comments explaining why they can be kept.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/net/ethernet/marvell/mvpp2.c | 33 +++++++++++++++++++++++++++++----
1 file changed, 29 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index fb6e9dc..037f6bd 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -4152,7 +4152,10 @@ static inline void mvpp2_interrupts_disable(struct mvpp2_port *port)
MVPP2_ISR_DISABLE_INTERRUPT(cpu_mask));
}
-/* Mask the current CPU's Rx/Tx interrupts */
+/* Mask the current CPU's Rx/Tx interrupts
+ * Called by on_each_cpu(), guaranteed to run with migration disabled,
+ * using smp_processor_id() is OK.
+ */
static void mvpp2_interrupts_mask(void *arg)
{
struct mvpp2_port *port = arg;
@@ -4161,7 +4164,10 @@ static void mvpp2_interrupts_mask(void *arg)
MVPP2_ISR_RX_TX_MASK_REG(port->id), 0);
}
-/* Unmask the current CPU's Rx/Tx interrupts */
+/* Unmask the current CPU's Rx/Tx interrupts.
+ * Called by on_each_cpu(), guaranteed to run with migration disabled,
+ * using smp_processor_id() is OK.
+ */
static void mvpp2_interrupts_unmask(void *arg)
{
struct mvpp2_port *port = arg;
@@ -4519,7 +4525,11 @@ mvpp2_txq_next_desc_get(struct mvpp2_tx_queue *txq)
return txq->descs + tx_desc;
}
-/* Update HW with number of aggregated Tx descriptors to be sent */
+/* Update HW with number of aggregated Tx descriptors to be sent
+ *
+ * Called only from mvpp2_tx(), so migration is disabled, using
+ * smp_processor_id() is OK.
+ */
static void mvpp2_aggr_txq_pend_desc_add(struct mvpp2_port *port, int pending)
{
/* aggregated access - relevant TXQ number is written in TX desc */
@@ -4530,6 +4540,9 @@ static void mvpp2_aggr_txq_pend_desc_add(struct mvpp2_port *port, int pending)
/* Check if there are enough free descriptors in aggregated txq.
* If not, update the number of occupied descriptors and repeat the check.
+ *
+ * Called only from mvpp2_tx(), so migration is disabled, using
+ * smp_processor_id() is OK.
*/
static int mvpp2_aggr_desc_num_check(struct mvpp2 *priv,
struct mvpp2_tx_queue *aggr_txq, int num)
@@ -4548,7 +4561,12 @@ static int mvpp2_aggr_desc_num_check(struct mvpp2 *priv,
return 0;
}
-/* Reserved Tx descriptors allocation request */
+/* Reserved Tx descriptors allocation request
+ *
+ * Called only from mvpp2_txq_reserved_desc_num_proc(), itself called
+ * only by mvpp2_tx(), so migration is disabled, using
+ * smp_processor_id() is OK.
+ */
static int mvpp2_txq_alloc_reserved_desc(struct mvpp2 *priv,
struct mvpp2_tx_queue *txq, int num)
{
@@ -4652,6 +4670,10 @@ static u32 mvpp2_txq_desc_csum(int l3_offs, int l3_proto,
/* Get number of sent descriptors and decrement counter.
* The number of sent descriptors is returned.
* Per-CPU access
+ *
+ * Called only from mvpp2_txq_done(), called from mvpp2_tx()
+ * (migration disabled) and from the TX completion tasklet (migration
+ * disabled) so using smp_processor_id() is OK.
*/
static inline int mvpp2_txq_sent_desc_proc(struct mvpp2_port *port,
struct mvpp2_tx_queue *txq)
@@ -4666,6 +4688,9 @@ static inline int mvpp2_txq_sent_desc_proc(struct mvpp2_port *port,
MVPP2_TRANSMITTED_COUNT_OFFSET;
}
+/* Called through on_each_cpu(), so runs on all CPUs, with migration
+ * disabled, therefore using smp_processor_id() is OK.
+ */
static void mvpp2_txq_sent_counter_clear(void *arg)
{
struct mvpp2_port *port = arg;
--
2.7.4
^ permalink raw reply related
* [PATCH 5/5] net: mvpp2: remove mvpp2_pool_refill()
From: Thomas Petazzoni @ 2017-06-08 15:27 UTC (permalink / raw)
To: David S. Miller, netdev
Cc: Marcin Wojtas, Stefan Chulski, Nadav Haklai, Hanna Hawa,
Yehuda Yitschak, Antoine Tenart, Gregory Clement, Markus Elfring,
Marc Zyngier, Thomas Petazzoni
In-Reply-To: <1496935647-29670-1-git-send-email-thomas.petazzoni@free-electrons.com>
When all a function does is calling another function with the exact same
arguments, in the exact same order, you know it's time to remove said
function. Which is exactly what this commit does.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/net/ethernet/marvell/mvpp2.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 4ca1639..747e3a4 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -3943,14 +3943,6 @@ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool,
put_cpu();
}
-/* Refill BM pool */
-static void mvpp2_pool_refill(struct mvpp2_port *port, int pool,
- dma_addr_t dma_addr,
- phys_addr_t phys_addr)
-{
- mvpp2_bm_pool_put(port, pool, dma_addr, phys_addr);
-}
-
/* Allocate buffers for the pool */
static int mvpp2_bm_bufs_add(struct mvpp2_port *port,
struct mvpp2_bm_pool *bm_pool, int buf_num)
@@ -4980,7 +4972,7 @@ static void mvpp2_rxq_drop_pkts(struct mvpp2_port *port,
pool = (status & MVPP2_RXD_BM_POOL_ID_MASK) >>
MVPP2_RXD_BM_POOL_ID_OFFS;
- mvpp2_pool_refill(port, pool,
+ mvpp2_bm_pool_put(port, pool,
mvpp2_rxdesc_dma_addr_get(port, rx_desc),
mvpp2_rxdesc_cookie_get(port, rx_desc));
}
@@ -5434,7 +5426,7 @@ static int mvpp2_rx_refill(struct mvpp2_port *port,
if (!buf)
return -ENOMEM;
- mvpp2_pool_refill(port, pool, dma_addr, phys_addr);
+ mvpp2_bm_pool_put(port, pool, dma_addr, phys_addr);
return 0;
}
@@ -5518,7 +5510,7 @@ static int mvpp2_rx(struct mvpp2_port *port, int rx_todo,
dev->stats.rx_errors++;
mvpp2_rx_error(port, rx_desc);
/* Return the buffer to the pool */
- mvpp2_pool_refill(port, pool, dma_addr, phys_addr);
+ mvpp2_bm_pool_put(port, pool, dma_addr, phys_addr);
continue;
}
--
2.7.4
^ permalink raw reply related
* [PATCH 4/5] net: mvpp2: remove unused mvpp2_bm_cookie_pool_set() function
From: Thomas Petazzoni @ 2017-06-08 15:27 UTC (permalink / raw)
To: David S. Miller, netdev
Cc: Marcin Wojtas, Stefan Chulski, Nadav Haklai, Hanna Hawa,
Yehuda Yitschak, Antoine Tenart, Gregory Clement, Markus Elfring,
Marc Zyngier, Thomas Petazzoni
In-Reply-To: <1496935647-29670-1-git-send-email-thomas.petazzoni@free-electrons.com>
This function is not used in the driver, remove it.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/net/ethernet/marvell/mvpp2.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 037f6bd..4ca1639 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -3907,17 +3907,6 @@ static void *mvpp2_buf_alloc(struct mvpp2_port *port,
return data;
}
-/* Set pool number in a BM cookie */
-static inline u32 mvpp2_bm_cookie_pool_set(u32 cookie, int pool)
-{
- u32 bm;
-
- bm = cookie & ~(0xFF << MVPP2_BM_COOKIE_POOL_OFFS);
- bm |= ((pool & 0xFF) << MVPP2_BM_COOKIE_POOL_OFFS);
-
- return bm;
-}
-
/* Release buffer to BM */
static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool,
dma_addr_t buf_dma_addr,
--
2.7.4
^ permalink raw reply related
* [PATCH 1/5] net: mvpp2: remove mvpp2_bm_cookie_{build,pool_get}
From: Thomas Petazzoni @ 2017-06-08 15:27 UTC (permalink / raw)
To: David S. Miller, netdev
Cc: Marcin Wojtas, Stefan Chulski, Nadav Haklai, Hanna Hawa,
Yehuda Yitschak, Antoine Tenart, Gregory Clement, Markus Elfring,
Marc Zyngier, Thomas Petazzoni
In-Reply-To: <1496935647-29670-1-git-send-email-thomas.petazzoni@free-electrons.com>
This commit removes the useless remove
mvpp2_bm_cookie_{build,pool_get} functions. All what
mvpp2_bm_cookie_build() was doing is compute a 32-bit value by
concatenating the pool number and the CPU number... only to get the pool
number re-extracted by mvpp2_bm_cookie_pool_get() later on.
Instead, just get the pool number directly from RX descriptor status,
and pass it to mvpp2_pool_refill() and mvpp2_rx_refill().
This has the added benefit of dropping a smp_processor_id() call in a
migration-enabled context, which is wrong, and is the original
motivation for making this change.
Fixes: 3f518509dedc9 ("ethernet: Add new driver for Marvell Armada 375 network unit")
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/net/ethernet/marvell/mvpp2.c | 51 ++++++++++--------------------------
1 file changed, 14 insertions(+), 37 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 9b875d7..cff45e3 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -666,10 +666,6 @@ enum mvpp2_prs_l3_cast {
#define MVPP2_BM_SWF_LONG_POOL(port) ((port > 2) ? 2 : port)
#define MVPP2_BM_SWF_SHORT_POOL 3
-/* BM cookie (32 bits) definition */
-#define MVPP2_BM_COOKIE_POOL_OFFS 8
-#define MVPP2_BM_COOKIE_CPU_OFFS 24
-
/* BM short pool packet size
* These value assure that for SWF the total number
* of bytes allocated for each buffer will be 512
@@ -3920,12 +3916,6 @@ static inline u32 mvpp2_bm_cookie_pool_set(u32 cookie, int pool)
return bm;
}
-/* Get pool number from a BM cookie */
-static inline int mvpp2_bm_cookie_pool_get(unsigned long cookie)
-{
- return (cookie >> MVPP2_BM_COOKIE_POOL_OFFS) & 0xFF;
-}
-
/* Release buffer to BM */
static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool,
dma_addr_t buf_dma_addr,
@@ -3961,12 +3951,10 @@ static inline void mvpp2_bm_pool_put(struct mvpp2_port *port, int pool,
}
/* Refill BM pool */
-static void mvpp2_pool_refill(struct mvpp2_port *port, u32 bm,
+static void mvpp2_pool_refill(struct mvpp2_port *port, int pool,
dma_addr_t dma_addr,
phys_addr_t phys_addr)
{
- int pool = mvpp2_bm_cookie_pool_get(bm);
-
mvpp2_bm_pool_put(port, pool, dma_addr, phys_addr);
}
@@ -4515,21 +4503,6 @@ static void mvpp2_rxq_offset_set(struct mvpp2_port *port,
mvpp2_write(port->priv, MVPP2_RXQ_CONFIG_REG(prxq), val);
}
-/* Obtain BM cookie information from descriptor */
-static u32 mvpp2_bm_cookie_build(struct mvpp2_port *port,
- struct mvpp2_rx_desc *rx_desc)
-{
- int cpu = smp_processor_id();
- int pool;
-
- pool = (mvpp2_rxdesc_status_get(port, rx_desc) &
- MVPP2_RXD_BM_POOL_ID_MASK) >>
- MVPP2_RXD_BM_POOL_ID_OFFS;
-
- return ((pool & 0xFF) << MVPP2_BM_COOKIE_POOL_OFFS) |
- ((cpu & 0xFF) << MVPP2_BM_COOKIE_CPU_OFFS);
-}
-
/* Tx descriptors helper methods */
/* Get pointer to next Tx descriptor to be processed (send) by HW */
@@ -4980,9 +4953,13 @@ static void mvpp2_rxq_drop_pkts(struct mvpp2_port *port,
for (i = 0; i < rx_received; i++) {
struct mvpp2_rx_desc *rx_desc = mvpp2_rxq_next_desc_get(rxq);
- u32 bm = mvpp2_bm_cookie_build(port, rx_desc);
+ u32 status = mvpp2_rxdesc_status_get(port, rx_desc);
+ int pool;
+
+ pool = (status & MVPP2_RXD_BM_POOL_ID_MASK) >>
+ MVPP2_RXD_BM_POOL_ID_OFFS;
- mvpp2_pool_refill(port, bm,
+ mvpp2_pool_refill(port, pool,
mvpp2_rxdesc_dma_addr_get(port, rx_desc),
mvpp2_rxdesc_cookie_get(port, rx_desc));
}
@@ -5420,7 +5397,7 @@ static void mvpp2_rx_csum(struct mvpp2_port *port, u32 status,
/* Reuse skb if possible, or allocate a new skb and add it to BM pool */
static int mvpp2_rx_refill(struct mvpp2_port *port,
- struct mvpp2_bm_pool *bm_pool, u32 bm)
+ struct mvpp2_bm_pool *bm_pool, int pool)
{
dma_addr_t dma_addr;
phys_addr_t phys_addr;
@@ -5432,7 +5409,7 @@ static int mvpp2_rx_refill(struct mvpp2_port *port,
if (!buf)
return -ENOMEM;
- mvpp2_pool_refill(port, bm, dma_addr, phys_addr);
+ mvpp2_pool_refill(port, pool, dma_addr, phys_addr);
return 0;
}
@@ -5490,7 +5467,7 @@ static int mvpp2_rx(struct mvpp2_port *port, int rx_todo,
unsigned int frag_size;
dma_addr_t dma_addr;
phys_addr_t phys_addr;
- u32 bm, rx_status;
+ u32 rx_status;
int pool, rx_bytes, err;
void *data;
@@ -5502,8 +5479,8 @@ static int mvpp2_rx(struct mvpp2_port *port, int rx_todo,
phys_addr = mvpp2_rxdesc_cookie_get(port, rx_desc);
data = (void *)phys_to_virt(phys_addr);
- bm = mvpp2_bm_cookie_build(port, rx_desc);
- pool = mvpp2_bm_cookie_pool_get(bm);
+ pool = (rx_status & MVPP2_RXD_BM_POOL_ID_MASK) >>
+ MVPP2_RXD_BM_POOL_ID_OFFS;
bm_pool = &port->priv->bm_pools[pool];
/* In case of an error, release the requested buffer pointer
@@ -5516,7 +5493,7 @@ static int mvpp2_rx(struct mvpp2_port *port, int rx_todo,
dev->stats.rx_errors++;
mvpp2_rx_error(port, rx_desc);
/* Return the buffer to the pool */
- mvpp2_pool_refill(port, bm, dma_addr, phys_addr);
+ mvpp2_pool_refill(port, pool, dma_addr, phys_addr);
continue;
}
@@ -5531,7 +5508,7 @@ static int mvpp2_rx(struct mvpp2_port *port, int rx_todo,
goto err_drop_frame;
}
- err = mvpp2_rx_refill(port, bm_pool, bm);
+ err = mvpp2_rx_refill(port, bm_pool, pool);
if (err) {
netdev_err(port->dev, "failed to refill BM pools\n");
goto err_drop_frame;
--
2.7.4
^ permalink raw reply related
* [PATCH 0/5] net: mvpp2: fixes and cleanups
From: Thomas Petazzoni @ 2017-06-08 15:27 UTC (permalink / raw)
To: David S. Miller, netdev
Cc: Marcin Wojtas, Stefan Chulski, Nadav Haklai, Hanna Hawa,
Yehuda Yitschak, Antoine Tenart, Gregory Clement, Markus Elfring,
Marc Zyngier, Thomas Petazzoni
Hello,
Here is a small set of fixes/improvements for the mvpp2 driver.
The first two patches are fixes: they fix bogus usage of
smp_processor_id() in a migration-enabled context. Indeed currently
the driver outputs some fat warnings in CONFIG_DEBUG_PREEMPT-enabled
kernels. Therefore, some fixes should be pushed to stable.
The last three patches are cleanups and not needed for stable, but
they stack on top of the fixes.
Thanks,
Thomas
Thomas Petazzoni (5):
net: mvpp2: remove mvpp2_bm_cookie_{build,pool_get}
net: mvpp2: use {get,put}_cpu() instead of smp_processor_id()
net: mvpp2: add comments about smp_processor_id() usage
net: mvpp2: remove unused mvpp2_bm_cookie_pool_set() function
net: mvpp2: remove mvpp2_pool_refill()
drivers/net/ethernet/marvell/mvpp2.c | 126 +++++++++++++++++------------------
1 file changed, 60 insertions(+), 66 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [RFC PATCH net-next 5/5] selftests/bpf: change test_verifier expectations
From: Edward Cree via iovisor-dev @ 2017-06-08 15:27 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, netdev-u79uwXL29TY76Z2rM5mHXA, iovisor-dev,
LKML, davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20170608024334.jgo63s2fnoi5wmia@ast-mbp>
On 08/06/17 03:43, Alexei Starovoitov wrote:
> On Wed, Jun 07, 2017 at 04:00:02PM +0100, Edward Cree wrote:
>> Some of the verifier's error messages have changed, and some constructs
>> that previously couldn't be verified are now accepted.
>>
>> Signed-off-by: Edward Cree <ecree-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>
>> ---
>> tools/testing/selftests/bpf/test_verifier.c | 226 ++++++++++++++--------------
>> 1 file changed, 116 insertions(+), 110 deletions(-)
> imo this rewrite needs more than one additional test.
> Like i counted at least 2 new verifier features (like negative and ptr & 0x40)
> All the new logic needs to be covered by tests.
Yes, I will write some new tests to cover the new features. I just wanted
to get some comments on the patch first, in case I was barking up entirely
the wrong tree.
-Ed
^ permalink raw reply
* Re: [PATCH v2 net-next] tcp: add TCPMemoryPressuresChrono counter
From: David Miller @ 2017-06-08 15:26 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1496867352.736.45.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 07 Jun 2017 13:29:12 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> DRAM supply shortage and poor memory pressure tracking in TCP
> stack makes any change in SO_SNDBUF/SO_RCVBUF (or equivalent autotuning
> limits) and tcp_mem[] quite hazardous.
>
> TCPMemoryPressures SNMP counter is an indication of tcp_mem sysctl
> limits being hit, but only tracking number of transitions.
>
> If TCP stack behavior under stress was perfect :
> 1) It would maintain memory usage close to the limit.
> 2) Memory pressure state would be entered for short times.
>
> We certainly prefer 100 events lasting 10ms compared to one event
> lasting 200 seconds.
>
> This patch adds a new SNMP counter tracking cumulative duration of
> memory pressure events, given in ms units.
>
> $ cat /proc/sys/net/ipv4/tcp_mem
> 3088 4117 6176
> $ grep TCP /proc/net/sockstat
> TCP: inuse 180 orphan 0 tw 2 alloc 234 mem 4140
> $ nstat -n ; sleep 10 ; nstat |grep Pressure
> TcpExtTCPMemoryPressures 1700
> TcpExtTCPMemoryPressuresChrono 5209
>
> v2: Used EXPORT_SYMBOL_GPL() instead of EXPORT_SYMBOL() as David
> instructed.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [RFC PATCH net-next 3/5] bpf/verifier: feed pointer-to-unknown-scalar casts into scalar ALU path
From: Edward Cree via iovisor-dev @ 2017-06-08 15:25 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, netdev-u79uwXL29TY76Z2rM5mHXA, iovisor-dev,
LKML, davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20170608023540.5ecmmobhl2rtgrg5@ast-mbp>
On 08/06/17 03:35, Alexei Starovoitov wrote:
> such large back and forth move doesn't help reviewing.
> may be just merge it into previous patch?
> Or keep that function in the right place in patch 2 already?
I think 'diff' got a bit confused, and maybe with different options I could
have got it to produce something more readable. But I think I will just
merge this into patch 2; it's only separate because it started out as an
experiment.
-Ed
^ permalink raw reply
* Re: [RFC PATCH net-next 4/5] bpf/verifier: track signed and unsigned min/max values
From: Edward Cree via iovisor-dev @ 2017-06-08 15:23 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, netdev-u79uwXL29TY76Z2rM5mHXA, iovisor-dev,
LKML, davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20170608024036.xsgbqghn6kqrk2cr@ast-mbp>
On 08/06/17 03:40, Alexei Starovoitov wrote:
> On Wed, Jun 07, 2017 at 03:59:25PM +0100, Edward Cree wrote:
>> Allows us to, sometimes, combine information from a signed check of one
>> bound and an unsigned check of the other.
>> We now track the full range of possible values, rather than restricting
>> ourselves to [0, 1<<30) and considering anything beyond that as
>> unknown. While this is probably not necessary, it makes the code more
>> straightforward and symmetrical between signed and unsigned bounds.
>>
>> Signed-off-by: Edward Cree <ecree-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>
>> ---
>> include/linux/bpf_verifier.h | 22 +-
>> kernel/bpf/verifier.c | 661 +++++++++++++++++++++++++------------------
>> 2 files changed, 395 insertions(+), 288 deletions(-)
>>
>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> index e341469..10a5944 100644
>> --- a/include/linux/bpf_verifier.h
>> +++ b/include/linux/bpf_verifier.h
>> @@ -11,11 +11,15 @@
>> #include <linux/filter.h> /* for MAX_BPF_STACK */
>> #include <linux/tnum.h>
>>
>> - /* Just some arbitrary values so we can safely do math without overflowing and
>> - * are obviously wrong for any sort of memory access.
>> - */
>> -#define BPF_REGISTER_MAX_RANGE (1024 * 1024 * 1024)
>> -#define BPF_REGISTER_MIN_RANGE -1
>> +/* Maximum variable offset umax_value permitted when resolving memory accesses.
>> + * In practice this is far bigger than any realistic pointer offset; this limit
>> + * ensures that umax_value + (int)off + (int)size cannot overflow a u64.
>> + */
>> +#define BPF_MAX_VAR_OFF (1ULL << 31)
>> +/* Maximum variable size permitted for ARG_CONST_SIZE[_OR_ZERO]. This ensures
>> + * that converting umax_value to int cannot overflow.
>> + */
>> +#define BPF_MAX_VAR_SIZ INT_MAX
>>
>> struct bpf_reg_state {
>> enum bpf_reg_type type;
>> @@ -38,7 +42,7 @@ struct bpf_reg_state {
>> * PTR_TO_MAP_VALUE_OR_NULL, we have to NULL-check it _first_.
>> */
>> u32 id;
>> - /* These three fields must be last. See states_equal() */
>> + /* These five fields must be last. See states_equal() */
>> /* For scalar types (SCALAR_VALUE), this represents our knowledge of
>> * the actual value.
>> * For pointer types, this represents the variable part of the offset
>> @@ -51,8 +55,10 @@ struct bpf_reg_state {
>> * These refer to the same value as align, not necessarily the actual
>> * contents of the register.
>> */
>> - s64 min_value; /* minimum possible (s64)value */
>> - u64 max_value; /* maximum possible (u64)value */
>> + s64 smin_value; /* minimum possible (s64)value */
>> + s64 smax_value; /* maximum possible (s64)value */
>> + u64 umin_value; /* minimum possible (u64)value */
>> + u64 umax_value; /* maximum possible (u64)value */
> have uneasy feeling about this one.
> It's 16 extra bytes to be stored in every reg_state and memcmp later
> while we didn't have cases where people wanted negative values
> in ptr+var cases. Why bother than?
It was the only way I could see to both pass my new test (correctly reject
an uninformative combination of JGT and JSGT), and still pass one of the
other tests where we have to accept an informative combination of JGT and
JSGT. This isn't so much about supporting negative numbers as it is about
deducing the right bounds from signed checks, or a mixture of signed and
unsigned checks on the same value.
For instance, if you check a register is s< 5, you know nothing yet about
its unsigned maximum (it could be -1). But if you then check it's u< 10,
or even if you check it's s>= 0, you've now learned its sign bit so you
can conclude from the previous check that it's u< 5. But to conclude
that, you have to have stored the bound from the previous check.
I'm not too worried about the extra 16 bytes, because this is a control-
plane operation, and I'd be surprised if its performance really turned out
to be a problem. But if there's a better way to handle these checks, I'm
all ears.
>> unknown. While this is probably not necessary, it makes the code more
>> straightforward and symmetrical between signed and unsigned bounds.
> it's hard for me to see the 'straightforward' part yet.
Well, the new reg_set_min_max[_inv]() are simpler, as they just update the
relevant bound then call __reg_deduce_bounds() to propagate that knowledge
into the others, rather than having confusing (and, as we've seen, buggy)
logic in each case about "if we did this kind of check we've learned that
thing in this branch".
Also, all the care to check "did we exceed BPF_REGISTER_MAX_RANGE?" goes
away, as does special handling of negatives to turn them into
BPF_REGISTER_MIN_RANGE (again, this has bugs in the current code). Instead
we just have to check "does our operation on the bounds overflow?", and if
so, mark our bounds as unknown.
I think a lot of the arithmetic ops become a more mechanical "does this
overflow? No? Then let's compute new bounds". But then, that's partly
because the semantics of the old min_value and max_value weren't documented
anywhere (do they refer to the signed or the unsigned value in the
register?) and so it's unclear to me why some of the code does what it does.
-Ed
^ permalink raw reply
* Re: net/can: trying to register non-static key in can_rx_register
From: Andrey Konovalov @ 2017-06-08 15:21 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Oliver Hartkopp, David S. Miller, linux-can, netdev, LKML,
Dmitry Vyukov, Kostya Serebryany, syzkaller
In-Reply-To: <97710f24-092e-ade4-fa14-3bc812bbf42f@pengutronix.de>
On Thu, Jun 8, 2017 at 5:20 PM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 06/08/2017 05:16 PM, Andrey Konovalov wrote:
>> Hi,
>>
>> I've got the following error report while fuzzing the kernel with syzkaller.
>>
>> On commit b29794ec95c6856b316c2295904208bf11ffddd9 (4.12-rc4+).
>>
>> INFO: trying to register non-static key.
>> the code is fine but needs lockdep annotation.
>> turning off the locking correctness validator.
>
> I've posted a patch for this problem earlier today:
> https://marc.info/?l=linux-can&m=149691617012069&w=2
>
> I'll send a pull request to David soon.
Great, thanks!
>
> regards,
> Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Industrial Linux Solutions | Phone: +49-231-2826-924 |
> Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
>
^ permalink raw reply
* Re: net/can: trying to register non-static key in can_rx_register
From: Marc Kleine-Budde @ 2017-06-08 15:20 UTC (permalink / raw)
To: Andrey Konovalov, Oliver Hartkopp, David S. Miller, linux-can,
netdev, LKML
Cc: Dmitry Vyukov, Kostya Serebryany, syzkaller
In-Reply-To: <CAAeHK+ywJzK4Hd=O_+j+Dw7pb=s-MgwVLJywrLAQdkwfybdu1Q@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 806 bytes --]
On 06/08/2017 05:16 PM, Andrey Konovalov wrote:
> Hi,
>
> I've got the following error report while fuzzing the kernel with syzkaller.
>
> On commit b29794ec95c6856b316c2295904208bf11ffddd9 (4.12-rc4+).
>
> INFO: trying to register non-static key.
> the code is fine but needs lockdep annotation.
> turning off the locking correctness validator.
I've posted a patch for this problem earlier today:
https://marc.info/?l=linux-can&m=149691617012069&w=2
I'll send a pull request to David soon.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* net/can: trying to register non-static key in can_rx_register
From: Andrey Konovalov @ 2017-06-08 15:16 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde, David S. Miller, linux-can,
netdev, LKML
Cc: Dmitry Vyukov, Kostya Serebryany, syzkaller
Hi,
I've got the following error report while fuzzing the kernel with syzkaller.
On commit b29794ec95c6856b316c2295904208bf11ffddd9 (4.12-rc4+).
INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.
CPU: 2 PID: 17404 Comm: syz-executor1 Not tainted 4.12.0-rc4+ #15
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:16 [inline]
dump_stack+0x292/0x395 lib/dump_stack.c:52
register_lock_class+0x717/0x1aa0 kernel/locking/lockdep.c:755
__lock_acquire+0x269/0x3690 kernel/locking/lockdep.c:3255
? 0xffffffffa0000000
lock_acquire+0x22d/0x560 kernel/locking/lockdep.c:3855
__raw_spin_lock include/linux/spinlock_api_smp.h:142 [inline]
_raw_spin_lock+0x2f/0x40 kernel/locking/spinlock.c:151
spin_lock include/linux/spinlock.h:299 [inline]
can_rx_register+0x4f3/0x700 net/can/af_can.c:483
raw_enable_filters+0xd7/0x1e0 net/can/raw.c:192
raw_enable_allfilters+0x8a/0x1e0 net/can/raw.c:258
raw_bind+0x244/0x830 net/can/raw.c:435
SYSC_bind+0x20e/0x4c0 net/socket.c:1422
SyS_bind+0x29/0x30 net/socket.c:1408
entry_SYSCALL_64_fastpath+0x1f/0xbe
RIP: 0033:0x446389
RSP: 002b:00007fa1efda2c08 EFLAGS: 00000292 ORIG_RAX: 0000000000000031
RAX: ffffffffffffffda RBX: 0000000000000220 RCX: 0000000000446389
RDX: 0000000000000060 RSI: 0000000020001fa0 RDI: 0000000000000017
RBP: 00000000ffffffff R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000292 R12: 0000000000000017
R13: 0000000000003770 R14: 00000000006e4830 R15: 0000000000c13000
^ permalink raw reply
* Re: [PATCH net] net: ipv6: Release route when device is unregistering
From: David Miller @ 2017-06-08 15:13 UTC (permalink / raw)
To: dsahern; +Cc: netdev, roopa
In-Reply-To: <20170607182623.20734-1-dsahern@gmail.com>
From: David Ahern <dsahern@gmail.com>
Date: Wed, 7 Jun 2017 12:26:23 -0600
> Roopa reported attempts to delete a bond device that is referenced in a
> multipath route is hanging:
>
> $ ifdown bond2 # ifupdown2 command that deletes virtual devices
> unregister_netdevice: waiting for bond2 to become free. Usage count = 2
>
> Steps to reproduce:
> echo 1 > /proc/sys/net/ipv6/conf/all/ignore_routes_with_linkdown
> ip link add dev bond12 type bond
> ip link add dev bond13 type bond
> ip addr add 2001:db8:2::0/64 dev bond12
> ip addr add 2001:db8:3::0/64 dev bond13
> ip route add 2001:db8:33::0/64 nexthop via 2001:db8:2::2 nexthop via 2001:db8:3::2
> ip link del dev bond12
> ip link del dev bond13
>
> The root cause is the recent change to keep routes on a linkdown. Update
> the check to detect when the device is unregistering and release the
> route for that case.
>
> Fixes: a1a22c12060e4 ("net: ipv6: Keep nexthop of multipath route on admin down")
> Reported-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> Signed-off-by: David Ahern <dsahern@gmail.com>
Applied and queued up for -stable, thanks David.
^ permalink raw reply
* Re: [PATCH net] net: Zero ifla_vf_info in rtnl_fill_vfinfo()
From: David Miller @ 2017-06-08 14:58 UTC (permalink / raw)
To: Yuval.Mintz; +Cc: netdev, mschmidt
In-Reply-To: <20170607180033.19747-1-Yuval.Mintz@cavium.com>
From: Yuval Mintz <Yuval.Mintz@cavium.com>
Date: Wed, 7 Jun 2017 21:00:33 +0300
> Some of the structure's fields are not initialized by the
> rtnetlink. If driver doesn't set those in ndo_get_vf_config(),
> they'd leak memory to user.
>
> Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
> CC: Michal Schmidt <mschmidt@redhat.com>
Good catch, applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] mwifiex: Replace semaphore async_sem with mutex
From: Arnd Bergmann @ 2017-06-08 14:58 UTC (permalink / raw)
To: Binoy Jayan
Cc: Linux Kernel Mailing List, Rajendra, Mark Brown, Amitkumar Karwar,
Nishant Sarmukadam, Ganapathi Bhat, Xinming Hu, Kalle Valo,
linux-wireless, Networking
In-Reply-To: <1496916183-4097-1-git-send-email-binoy.jayan@linaro.org>
On Thu, Jun 8, 2017 at 12:03 PM, Binoy Jayan <binoy.jayan@linaro.org> wrote:
> The semaphore 'async_sem' is used as a simple mutex, so
> it should be written as one. Semaphores are going away in the future.
>
> Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
> ---
>
> This patch is part of a bigger effort to eliminate unwanted
> semaphores from the linux kernel.
Looks good,
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply
* Re: [PATCH net-next 0/4] tcp: Namespaceify 3 sysctls
From: David Miller @ 2017-06-08 14:53 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet
In-Reply-To: <20170607173439.17092-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Wed, 7 Jun 2017 10:34:35 -0700
> Move tcp_sack, tcp_window_scaling and tcp_timestamps
> sysctls to network namespaces.
>
> Eric Dumazet (4):
> tcp: add a struct net parameter to tcp_parse_options()
> tcp: Namespaceify sysctl_tcp_sack
> tcp: Namespaceify sysctl_tcp_window_scaling
> tcp: Namespaceify sysctl_tcp_timestamps
Series applied, thanks Eric.
^ permalink raw reply
* Re: [RFC PATCH net-next 2/5] bpf/verifier: rework value tracking
From: Edward Cree via iovisor-dev @ 2017-06-08 14:53 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, netdev-u79uwXL29TY76Z2rM5mHXA, iovisor-dev,
LKML, davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20170608023239.lsqijtfcg5fadpai@ast-mbp>
On 08/06/17 03:32, Alexei Starovoitov wrote:
> On Wed, Jun 07, 2017 at 03:58:31PM +0100, Edward Cree wrote:
>> +/* Arithmetic and logical ops */
>> +/* Shift a tnum left (by a fixed shift) */
>> +struct tnum tn_sl(struct tnum a, u8 shift);
>> +/* Shift a tnum right (by a fixed shift) */
>> +struct tnum tn_sr(struct tnum a, u8 shift);
> I think in few month we will forget what these abbreviations mean.
> Can you change it to tnum_rshift, tnum_lshift, tnum_add ?
Sure, will do.
>> +/* half-multiply add: acc += (unknown * mask * value) */
>> +static struct tnum hma(struct tnum acc, u64 value, u64 mask)
> hma? is it a standard abbreviation?
No, just a weird operation that appears in my multiply algorithm. Since
it's static I didn't worry too much about naming it well.
(The abbreviation was inspired by floating point 'fma', fused multiply-add.)
>> -static void init_reg_state(struct bpf_reg_state *regs)
>> +static void mark_reg_known_zero(struct bpf_reg_state *regs, u32 regno)
>> {
>> - int i;
>> -
>> - for (i = 0; i < MAX_BPF_REG; i++)
>> - mark_reg_not_init(regs, i);
>> -
>> - /* frame pointer */
>> - regs[BPF_REG_FP].type = FRAME_PTR;
>> -
>> - /* 1st arg to a function */
>> - regs[BPF_REG_1].type = PTR_TO_CTX;
>> + BUG_ON(regno >= MAX_BPF_REG);
>> + __mark_reg_known_zero(regs + regno);
> I know we have BUG_ONs in the code and it was never hit,
> but since you're rewriting it please change it to WARN_ON and
> set all regs into NOT_INIT in such case.
> This way if we really have a bug, it hopefully won't crash.
Sure, will do.
>> -/* check read/write into an adjusted map element */
>> -static int check_map_access_adj(struct bpf_verifier_env *env, u32 regno,
>> +/* check read/write into a map element with possible variable offset */
>> +static int check_map_access(struct bpf_verifier_env *env, u32 regno,
>> int off, int size)
>> {
>> struct bpf_verifier_state *state = &env->cur_state;
>> struct bpf_reg_state *reg = &state->regs[regno];
>> int err;
>>
>> - /* We adjusted the register to this map value, so we
>> - * need to change off and size to min_value and max_value
>> - * respectively to make sure our theoretical access will be
>> - * safe.
>> + /* We may have adjusted the register to this map value, so we
>> + * need to try adding each of min_value and max_value to off
>> + * to make sure our theoretical access will be safe.
>> */
>> if (log_level)
>> print_verifier_state(state);
>> - env->varlen_map_value_access = true;
>> + /* If the offset is variable, we will need to be stricter in state
>> + * pruning from now on.
>> + */
>> + if (reg->align.mask)
>> + env->varlen_map_value_access = true;
> i think this align.mask access was used in few places.
> May be worth to do static inline helper with clear name?
Sure, seems reasonable.
>> + char tn_buf[48];
>> +
>> + tn_strn(tn_buf, sizeof(tn_buf), reg->align);
>> + verbose("variable ctx access align=%s off=%d size=%d",
>> + tn_buf, off, size);
>> + return -EACCES;
>> + }
>> + off += reg->align.value;
> I think 'align' is an odd name for this field.
> May be rename off/align fields into
> s32 fixed_off;
> struct tnum var_off;
Yeah, it got that name for 'historical' reasons i.e. this patch series
started out as just a rewrite of the alignment tracking, then grew...
I'll do the rename in the next version.
>>
>> - } else if (reg->type == FRAME_PTR || reg->type == PTR_TO_STACK) {
>> + } else if (reg->type == PTR_TO_STACK) {
>> + /* stack accesses must be at a fixed offset, so that we can
>> + * determine what type of data were returned.
>> + */
>> + if (reg->align.mask) {
>> + char tn_buf[48];
>> +
>> + tn_strn(tn_buf, sizeof(tn_buf), reg->align);
>> + verbose("variable stack access align=%s off=%d size=%d",
>> + tn_buf, off, size);
>> + return -EACCES;
> hmm. why this restriction?
> I thought one of key points of the diff that ptr+var tracking logic
> will now apply not only to map_value, but to stack_ptr as well?
As the comment above it says, we need to determine what was returned:
was it STACK_MISC or STACK_SPILL, and if the latter, what kind of pointer
was spilled there? See check_stack_read(), which I should probably
mention in the comment.
>> }
>>
>> - if (!err && size <= 2 && value_regno >= 0 && env->allow_ptr_leaks &&
>> - state->regs[value_regno].type == UNKNOWN_VALUE) {
>> - /* 1 or 2 byte load zero-extends, determine the number of
>> - * zero upper bits. Not doing it fo 4 byte load, since
>> - * such values cannot be added to ptr_to_packet anyway.
>> - */
>> - state->regs[value_regno].imm = 64 - size * 8;
>> + if (!err && size < BPF_REG_SIZE && value_regno >= 0 && t == BPF_READ &&
>> + state->regs[value_regno].type == SCALAR_VALUE) {
>> + /* b/h/w load zero-extends, mark upper bits as known 0 */
>> + state->regs[value_regno].align.value &= (1ULL << (size * 8)) - 1;
>> + state->regs[value_regno].align.mask &= (1ULL << (size * 8)) - 1;
> probably another helper from tnum.h is needed.
I could rewrite as
reg->align = tn_and(reg->align, tn_const((1ULL << (size * 8)) - 1))
or do you mean a helper that takes 'size' as an argument?
>> + /* sign bit is known zero, so we can bound the value */
>> + state->regs[value_regno].min_value = 0;
>> + state->regs[value_regno].max_value = min_t(u64,
>> + state->regs[value_regno].align.mask,
>> + BPF_REGISTER_MAX_RANGE);
> min_t with mask? should it be align.value?
Hmm, I think actually it should be (mask | value), because this is the
max (we're taking the min of two maxes to see which is tighter).
>> }
>> return err;
>> }
>> @@ -1000,9 +1068,18 @@ static int check_xadd(struct bpf_verifier_env *env, struct bpf_insn *insn)
>> BPF_SIZE(insn->code), BPF_WRITE, -1);
>> }
>>
>> +/* Does this register contain a constant zero? */
>> +static bool register_is_null(struct bpf_reg_state reg)
>> +{
>> + return reg.type == SCALAR_VALUE && reg.align.mask == 0 &&
>> + reg.align.value == 0;
> align.mask == 0 && align.value==0 into helper in tnum.h ?
Could do, but it seems unnecessary; I don't think anything but this
function would use it.
>> + /* Got here implies adding two SCALAR_VALUEs */
>> + if (WARN_ON_ONCE(ptr_reg)) {
>> + verbose("verifier internal error\n");
>> + return -EINVAL;
> ...
>> + if (WARN_ON(!src_reg)) {
>> + verbose("verifier internal error\n");
>> + return -EINVAL;
>> }
> i'm lost with these bits.
> Can you add a comment in what circumstances this can be hit
> and what would be the consequences?
It should be impossible to hit either of these cases. If we let the
first through, we'd probably do invalid pointer arithmetic (e.g. we
could multiply a pointer by two and think we'd just multiplied the
variable offset). As for the latter, we access through that pointer
so if it were NULL we would promptly oops.
>> +/* Returns true if (rold safe implies rcur safe) */
>> +static bool regsafe(struct bpf_reg_state *rold,
>> + struct bpf_reg_state *rcur,
>> + bool varlen_map_access)
>> +{
>> + if (memcmp(rold, rcur, sizeof(*rold)) == 0)
>> return true;
>> + if (rold->type == NOT_INIT)
>> + /* explored state can't have used this */
>> return true;
>> + if (rcur->type == NOT_INIT)
>> + return false;
>> + switch (rold->type) {
>> + case SCALAR_VALUE:
>> + if (rcur->type == SCALAR_VALUE) {
>> + /* new val must satisfy old val knowledge */
>> + return range_within(rold, rcur) &&
>> + tn_in(rold->align, rcur->align);
>> + } else {
>> + /* if we knew anything about the old value, we're not
>> + * equal, because we can't know anything about the
>> + * scalar value of the pointer in the new value.
>> + */
>> + return rold->min_value == BPF_REGISTER_MIN_RANGE &&
>> + rold->max_value == BPF_REGISTER_MAX_RANGE &&
>> + !~rold->align.mask;
>> + }
>> + case PTR_TO_MAP_VALUE:
>> + if (varlen_map_access) {
>> + /* If the new min/max/align satisfy the old ones and
>> + * everything else matches, we are OK.
>> + * We don't care about the 'id' value, because nothing
>> + * uses it for PTR_TO_MAP_VALUE (only for ..._OR_NULL)
>> + */
>> + return memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)) == 0 &&
>> + range_within(rold, rcur) &&
>> + tn_in(rold->align, rcur->align);
>> + } else {
>> + /* If the ranges/align were not the same, but
>> + * everything else was and we didn't do a variable
>> + * access into a map then we are a-ok.
>> + */
>> + return memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)) == 0;
>> + }
>> + case PTR_TO_MAP_VALUE_OR_NULL:
> does this new state comparison logic helps? Do you have any numbers before/after in the number of insns it had to process for the tests in selftests ?
I don't have the numbers, no (I'll try to collect them). This rewrite was
more because the data structures had changed so the old code needed changing
to match. It's mainly just a refactor and reimplementation of the existing
logic, I think, extended to cover the new 'align' member as well.
-Ed
^ permalink raw reply
* Re: next-20170608 build: 1 failures 4 warnings (next-20170608)
From: Sebastian Reichel @ 2017-06-08 14:53 UTC (permalink / raw)
To: David Miller
Cc: broonie, kernel-build-reports, linaro-kernel, linux-next, netdev
In-Reply-To: <20170608.101757.1257358800507365931.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 1059 bytes --]
Hi,
On Thu, Jun 08, 2017 at 10:17:57AM -0400, David Miller wrote:
> > On Thu, Jun 08, 2017 at 11:32:31AM +0100, Mark Brown wrote:
> >> On Thu, Jun 08, 2017 at 10:57:50AM +0100, Build bot for Mark Brown wrote:
> >>
> >> Today's -next fails to build an ARM allmodconfig due to:
> >>
> >> > arm-allmodconfig
> >> > ../drivers/hsi/clients/ssi_protocol.c:1069:5: error: 'struct net_device' has no member named 'destructor'
> >>
> >> due to cf124db566e6 (net: Fix inconsistent teardown and release of
> >> private netdev state.) which missed this instance, presumably due to a
> >> combination of it not being in one of the normal networking driver
> >> directories and only being buildable on ARM.
> >
> > I assume, that this will be fixed via the net tree.
> > If you Cc me on updated/new patch I will provide Acked-by.
>
> It is fixed in the 'net' tree, don't worry about it :-)
https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/?id=ed66e50d9587fc0bb032e276a2563c0068a5b63a
Looks good, thanks.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/2] net: emac: fix and unify emac_mdio functions
From: David Miller @ 2017-06-08 14:47 UTC (permalink / raw)
To: chunkeey; +Cc: netdev, ivan, andrew
In-Reply-To: <dc3b77e7ead819001e09589d1f95eb6c1061f381.1496773338.git.chunkeey@googlemail.com>
From: Christian Lamparter <chunkeey@googlemail.com>
Date: Wed, 7 Jun 2017 15:51:16 +0200
> emac_mdio_read_link() was not copying the requested phy settings
> back into the emac driver's own phy api. This has caused a link
> speed mismatch issue for the AR8035 as the emac driver kept
> trying to connect with 10/100MBps on a 1GBit/s link.
>
> This patch also unifies shared code between emac_setup_aneg()
> and emac_mdio_setup_forced(). And furthermore it removes
> a chunk of emac_mdio_init_phy(), that was copying the same
> data into itself.
>
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
Applied.
^ permalink raw reply
* Re: [PATCH v2 1/2] net: emac: fix reset timeout with AR8035 phy
From: David Miller @ 2017-06-08 14:47 UTC (permalink / raw)
To: chunkeey; +Cc: netdev, ivan, russell, andrew, chrisrblake93
In-Reply-To: <eefbc6df06c83b725c60997fc58cf1c0119334a1.1496773338.git.chunkeey@googlemail.com>
From: Christian Lamparter <chunkeey@googlemail.com>
Date: Wed, 7 Jun 2017 15:51:15 +0200
> This patch fixes a problem where the AR8035 PHY can't be
> detected on an Cisco Meraki MR24, if the ethernet cable is
> not connected on boot.
...
> Cc: Chris Blake <chrisrblake93@gmail.com>
> Reported-by: Russell Senior <russell@personaltelco.net>
> Fixes: 23fbb5a87c56e98 ("emac: Fix EMAC soft reset on 460EX/GT")
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
Applied.
^ permalink raw reply
* Re: [PATCH 25/44] arm: implement ->mapping_error
From: Russell King - ARM Linux @ 2017-06-08 14:43 UTC (permalink / raw)
To: Christoph Hellwig
Cc: x86, linux-arm-kernel, xen-devel, linux-c6x-dev, linux-hexagon,
linux-ia64, linux-mips, openrisc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, linux-xtensa, dmaengine, linux-tegra,
dri-devel, linux-samsung-soc, iommu, netdev, linux-kernel
In-Reply-To: <20170608132609.32662-26-hch@lst.de>
BOn Thu, Jun 08, 2017 at 03:25:50PM +0200, Christoph Hellwig wrote:
> +static int dmabounce_mapping_error(struct device *dev, dma_addr_t dma_addr)
> +{
> + if (dev->archdata.dmabounce)
> + return 0;
I'm not convinced that we need this check here:
dev->archdata.dmabounce = device_info;
set_dma_ops(dev, &dmabounce_ops);
There shouldn't be any chance of dev->archdata.dmabounce being NULL if
the dmabounce_ops has been set as the current device DMA ops. So I
think that test can be killed.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Re: [PATCH v2] decnet: dn_rtmsg: Improve input length sanitization in dnrmg_receive_user_skb
From: David Miller @ 2017-06-08 14:39 UTC (permalink / raw)
To: mjurczyk
Cc: fw, pablo, kadlec, netfilter-devel, coreteam, linux-decnet-user,
netdev, linux-kernel
In-Reply-To: <CAPr8fOQNP+y78xJqx5BNUfY6Rmjj2R8+jrz3Ett1SgdY+a5WLw@mail.gmail.com>
From: Mateusz Jurczyk <mjurczyk@google.com>
Date: Wed, 7 Jun 2017 16:41:57 +0200
> On Wed, Jun 7, 2017 at 4:18 PM, Florian Westphal <fw@strlen.de> wrote:
>> Mateusz Jurczyk <mjurczyk@google.com> wrote:
>>> Verify that the length of the socket buffer is sufficient to cover the
>>> nlmsghdr structure before accessing the nlh->nlmsg_len field for further
>>> input sanitization. If the client only supplies 1-3 bytes of data in
>>> sk_buff, then nlh->nlmsg_len remains partially uninitialized and
>>> contains leftover memory from the corresponding kernel allocation.
>>> Operating on such data may result in indeterminate evaluation of the
>>> nlmsg_len < sizeof(*nlh) expression.
>>>
>>> The bug was discovered by a runtime instrumentation designed to detect
>>> use of uninitialized memory in the kernel. The patch prevents this and
>>> other similar tools (e.g. KMSAN) from flagging this behavior in the future.
>>
>> Instead of changing all the internal users wouldn't it be better
>> to add this check once in netlink_unicast_kernel?
>>
>
> Perhaps. I must admit I'm not very familiar with this code
> area/interface, so I preferred to fix the few specific cases instead
> of submitting a general patch, which might have some unexpected side
> effects, e.g. behavior different from one of the internal clients etc.
>
> If you think one check in netlink_unicast_kernel is a better way to do
> it, I'm happy to implement it like that.
Until we decide to add the check to netlink_unicast_kernel(), I'm applying
this and queueing it up for -stable.
Thanks.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox