Netdev List
 help / color / mirror / Atom feed
* [PATCH net] net: qualcomm: emac: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-12 15:49 UTC (permalink / raw)
  To: netdev; +Cc: timur, davem, yang.wei9, albin_yang

From: Yang Wei <yang.wei9@zte.com.cn>

dev_consume_skb_irq() should be called in emac_mac_tx_process() when
skb xmit done. It makes drop profiles(dropwatch, perf) more friendly.

Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
 drivers/net/ethernet/qualcomm/emac/emac-mac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.c b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
index 8d79031..20d2400 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-mac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
@@ -1204,7 +1204,7 @@ void emac_mac_tx_process(struct emac_adapter *adpt, struct emac_tx_queue *tx_q)
 		if (tpbuf->skb) {
 			pkts_compl++;
 			bytes_compl += tpbuf->skb->len;
-			dev_kfree_skb_irq(tpbuf->skb);
+			dev_consume_skb_irq(tpbuf->skb);
 			tpbuf->skb = NULL;
 		}
 
-- 
2.7.4



^ permalink raw reply related

* [PATCH net] net: moxa: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-12 15:56 UTC (permalink / raw)
  To: netdev; +Cc: davem, keescook, yang.wei9, albin_yang

From: Yang Wei <yang.wei9@zte.com.cn>

dev_consume_skb_irq() should be called in moxart_tx_finished() when
skb xmit done. It makes drop profiles(dropwatch, perf) more friendly.

Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
 drivers/net/ethernet/moxa/moxart_ether.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index b34055a..b966815 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -298,7 +298,7 @@ static void moxart_tx_finished(struct net_device *ndev)
 		ndev->stats.tx_packets++;
 		ndev->stats.tx_bytes += priv->tx_skb[tx_tail]->len;
 
-		dev_kfree_skb_irq(priv->tx_skb[tx_tail]);
+		dev_consume_skb_irq(priv->tx_skb[tx_tail]);
 		priv->tx_skb[tx_tail] = NULL;
 
 		tx_tail = TX_NEXT(tx_tail);
-- 
2.7.4



^ permalink raw reply related

* [PATCH net] net: sis: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-12 15:59 UTC (permalink / raw)
  To: netdev; +Cc: romieu, davem, venza, yang.wei9, albin_yang

From: Yang Wei <yang.wei9@zte.com.cn>

dev_consume_skb_irq() should be called when skb xmit done. It makes
drop profiles(dropwatch, perf) more friendly.

Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
 drivers/net/ethernet/sis/sis190.c | 2 +-
 drivers/net/ethernet/sis/sis900.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/sis/sis190.c b/drivers/net/ethernet/sis/sis190.c
index 808cf98..5b351be 100644
--- a/drivers/net/ethernet/sis/sis190.c
+++ b/drivers/net/ethernet/sis/sis190.c
@@ -714,7 +714,7 @@ static void sis190_tx_interrupt(struct net_device *dev,
 
 		sis190_unmap_tx_skb(tp->pci_dev, skb, txd);
 		tp->Tx_skbuff[entry] = NULL;
-		dev_kfree_skb_irq(skb);
+		dev_consume_skb_irq(skb);
 	}
 
 	if (tp->dirty_tx != dirty_tx) {
diff --git a/drivers/net/ethernet/sis/sis900.c b/drivers/net/ethernet/sis/sis900.c
index 4bb89f7..6073387 100644
--- a/drivers/net/ethernet/sis/sis900.c
+++ b/drivers/net/ethernet/sis/sis900.c
@@ -1927,7 +1927,7 @@ static void sis900_finish_xmit (struct net_device *net_dev)
 		pci_unmap_single(sis_priv->pci_dev,
 			sis_priv->tx_ring[entry].bufptr, skb->len,
 			PCI_DMA_TODEVICE);
-		dev_kfree_skb_irq(skb);
+		dev_consume_skb_irq(skb);
 		sis_priv->tx_skbuff[entry] = NULL;
 		sis_priv->tx_ring[entry].bufptr = 0;
 		sis_priv->tx_ring[entry].cmdsts = 0;
-- 
2.7.4



^ permalink raw reply related

* [PATCH net] net: macb: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-12 16:00 UTC (permalink / raw)
  To: netdev; +Cc: nicolas.ferre, davem, yang.wei9, albin_yang

From: Yang Wei <yang.wei9@zte.com.cn>

dev_consume_skb_irq() should be called in at91ether_interrupt() when
skb xmit done. It makes drop profiles(dropwatch, perf) more friendly.

Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
 drivers/net/ethernet/cadence/macb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 2b28826..835cc58 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -3763,7 +3763,7 @@ static irqreturn_t at91ether_interrupt(int irq, void *dev_id)
 			dev->stats.tx_errors++;
 
 		if (lp->skb) {
-			dev_kfree_skb_irq(lp->skb);
+			dev_consume_skb_irq(lp->skb);
 			lp->skb = NULL;
 			dma_unmap_single(NULL, lp->skb_physaddr,
 					 lp->skb_length, DMA_TO_DEVICE);
-- 
2.7.4



^ permalink raw reply related

* [PATCH net] net: ixp4xx_eth: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-12 16:01 UTC (permalink / raw)
  To: netdev; +Cc: khalasa, davem, yang.wei9, albin_yang

From: Yang Wei <yang.wei9@zte.com.cn>

dev_consume_skb_irq() should be called in eth_txdone_irq() when skb
xmit done. It makes drop profiles(dropwatch, perf) more friendly.

Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
 drivers/net/ethernet/xscale/ixp4xx_eth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/xscale/ixp4xx_eth.c b/drivers/net/ethernet/xscale/ixp4xx_eth.c
index aee55c0..ed6623a 100644
--- a/drivers/net/ethernet/xscale/ixp4xx_eth.c
+++ b/drivers/net/ethernet/xscale/ixp4xx_eth.c
@@ -139,7 +139,7 @@
 #ifdef __ARMEB__
 typedef struct sk_buff buffer_t;
 #define free_buffer dev_kfree_skb
-#define free_buffer_irq dev_kfree_skb_irq
+#define free_buffer_irq dev_consume_skb_irq
 #else
 typedef void buffer_t;
 #define free_buffer kfree
-- 
2.7.4



^ permalink raw reply related

* [PATCH] qed: fix indentation issue with statements in an if-block
From: Colin King @ 2019-02-12 16:01 UTC (permalink / raw)
  To: Ariel Elior, GR-everest-linux-l2, David S . Miller, netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

There are some statements in an if-block that are not correctly
indented. Fix these.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/ethernet/qlogic/qed/qed_cxt.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_cxt.c b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
index 35c9f484eb9f..e61d1d905415 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_cxt.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
@@ -2135,12 +2135,12 @@ int qed_cxt_set_pf_params(struct qed_hwfn *p_hwfn, u32 rdma_tasks)
 		struct qed_eth_pf_params *p_params =
 		    &p_hwfn->pf_params.eth_pf_params;
 
-			if (!p_params->num_vf_cons)
-				p_params->num_vf_cons =
-				    ETH_PF_PARAMS_VF_CONS_DEFAULT;
-			qed_cxt_set_proto_cid_count(p_hwfn, PROTOCOLID_ETH,
-						    p_params->num_cons,
-						    p_params->num_vf_cons);
+		if (!p_params->num_vf_cons)
+			p_params->num_vf_cons =
+			    ETH_PF_PARAMS_VF_CONS_DEFAULT;
+		qed_cxt_set_proto_cid_count(p_hwfn, PROTOCOLID_ETH,
+					    p_params->num_cons,
+					    p_params->num_vf_cons);
 		p_hwfn->p_cxt_mngr->arfs_count = p_params->num_arfs_filters;
 		break;
 	}
-- 
2.20.1


^ permalink raw reply related

* RE: [PATCH net-next 2/2] devlink: Fix list access without lock while reading region
From: Parav Pandit @ 2019-02-12 16:04 UTC (permalink / raw)
  To: Sergei Shtylyov, Jiri Pirko, davem@davemloft.net,
	netdev@vger.kernel.org
In-Reply-To: <a8f3f707-bb7a-0cb3-201f-6fcc06169677@cogentembedded.com>



> -----Original Message-----
> From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Sent: Tuesday, February 12, 2019 3:01 AM
> To: Parav Pandit <parav@mellanox.com>; Jiri Pirko <jiri@mellanox.com>;
> davem@davemloft.net; netdev@vger.kernel.org
> Subject: Re: [PATCH net-next 2/2] devlink: Fix list access without lock while
> reading region
> 
> Hello!
> 
> On 12.02.2019 10:09, Parav Pandit wrote:
> 
> > While finding the devlink device during region reading, devlink device
> > list is accessed and devlink device is returned without holding a
> > lock. This could lead to user-after-free
> 
>     Use-after-free, perhaps?
> 
Yep. Sending v1 to fix typo.

> > accesses.
> >
> > While at it, add lockdep assert to ensure that all future callers hold
> > the lock when calling devlink_get_from_attrs().
> >
> > Fixes: 4e54795a27f5 ("devlink: Add support for region snapshot read
> > command")
> > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > Acked-by: Jiri Pirko <jiri@mellanox.com>
> [...]
> 
> MBR, Sergei

^ permalink raw reply

* Re: [PATCH net] sctp: set stream ext to NULL after freeing it in sctp_stream_outq_migrate
From: Marcelo Ricardo Leitner @ 2019-02-12 16:07 UTC (permalink / raw)
  To: Xin Long; +Cc: linux-kernel, network dev, linux-sctp, davem, Neil Horman
In-Reply-To: <0cb9e543c21495df48c3723044d6c9f64f238eca.1549968661.git.lucien.xin@gmail.com>

On Tue, Feb 12, 2019 at 06:51:01PM +0800, Xin Long wrote:
> In sctp_stream_init(), after sctp_stream_outq_migrate() freed the
> surplus streams' ext, but sctp_stream_alloc_out() returns -ENOMEM,
> stream->outcnt will not be set to 'outcnt'.
> 
> With the bigger value on stream->outcnt, when closing the assoc and
> freeing its streams, the ext of those surplus streams will be freed
> again since those stream exts were not set to NULL after freeing in
> sctp_stream_outq_migrate(). Then the invalid-free issue reported by
> syzbot would be triggered.
> 
> We fix it by simply setting them to NULL after freeing.
> 
> Fixes: 5bbbbe32a431 ("sctp: introduce stream scheduler foundations")
> Reported-by: syzbot+58e480e7b28f2d890bfd@syzkaller.appspotmail.com
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  net/sctp/stream.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> index f246331..2936ed1 100644
> --- a/net/sctp/stream.c
> +++ b/net/sctp/stream.c
> @@ -144,8 +144,10 @@ static void sctp_stream_outq_migrate(struct sctp_stream *stream,
>  		}
>  	}
>  
> -	for (i = outcnt; i < stream->outcnt; i++)
> +	for (i = outcnt; i < stream->outcnt; i++) {
>  		kfree(SCTP_SO(stream, i)->ext);
> +		SCTP_SO(stream, i)->ext = NULL;
> +	}
>  }
>  
>  static int sctp_stream_alloc_out(struct sctp_stream *stream, __u16 outcnt,
> -- 
> 2.1.0
> 

^ permalink raw reply

* [PATCH] qlge: fix some indentation issues
From: Colin King @ 2019-02-12 16:08 UTC (permalink / raw)
  To: Manish Chopra, GR-Linux-NIC-Dev, David S . Miller, netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

There are some statements that are indented incorrectly. Fix these.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c | 4 ++--
 drivers/net/ethernet/qlogic/qlge/qlge_main.c    | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c b/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
index 5edbd532127d..a6886cc5654c 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
@@ -249,8 +249,8 @@ static void ql_update_stats(struct ql_adapter *qdev)
 
 	spin_lock(&qdev->stats_lock);
 	if (ql_sem_spinlock(qdev, qdev->xg_sem_mask)) {
-			netif_err(qdev, drv, qdev->ndev,
-				  "Couldn't get xgmac sem.\n");
+		netif_err(qdev, drv, qdev->ndev,
+			  "Couldn't get xgmac sem.\n");
 		goto quit;
 	}
 	/*
diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
index 059ba9429e51..096515c27263 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
@@ -1159,10 +1159,10 @@ static void ql_update_lbq(struct ql_adapter *qdev, struct rx_ring *rx_ring)
 
 			map = lbq_desc->p.pg_chunk.map +
 				lbq_desc->p.pg_chunk.offset;
-				dma_unmap_addr_set(lbq_desc, mapaddr, map);
+			dma_unmap_addr_set(lbq_desc, mapaddr, map);
 			dma_unmap_len_set(lbq_desc, maplen,
 					rx_ring->lbq_buf_size);
-				*lbq_desc->addr = cpu_to_le64(map);
+			*lbq_desc->addr = cpu_to_le64(map);
 
 			pci_dma_sync_single_for_device(qdev->pdev, map,
 						rx_ring->lbq_buf_size,
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH bpf] xsk: do not remove umem from netdevice on fall-back to copy-mode
From: Daniel Borkmann @ 2019-02-12 16:09 UTC (permalink / raw)
  To: Björn Töpel, ast, netdev
  Cc: Björn Töpel, magnus.karlsson, magnus.karlsson,
	jan.sokolowski
In-Reply-To: <20190212075114.6628-1-bjorn.topel@gmail.com>

On 02/12/2019 08:51 AM, Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
> 
> Commit c9b47cc1fabc ("xsk: fix bug when trying to use both copy and
> zero-copy on one queue id") stores the umem into the netdev._rx
> struct. However, the patch incorrectly removed the umem from the
> netdev._rx struct when user-space passed "best-effort" mode
> (i.e. select the fastest possible option available), and zero-copy
> mode was not available. This commit fixes that.
> 
> Fixes: c9b47cc1fabc ("xsk: fix bug when trying to use both copy and zero-copy on one queue id")
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>

Applied, thanks!

^ permalink raw reply

* Re: [PATCH v2 bpf-next] tools: bpftool: doc, add text about feature-subcommand
From: Daniel Borkmann @ 2019-02-12 16:09 UTC (permalink / raw)
  To: Quentin Monnet, Prashant Bhole, Alexei Starovoitov; +Cc: netdev
In-Reply-To: <4b2c8979-9023-a82e-12ba-b1783f77a8ab@netronome.com>

On 02/12/2019 11:12 AM, Quentin Monnet wrote:
> 2019-02-12 10:25 UTC+0900 ~ Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
>> This patch adds missing information about feature-subcommand in
>> bpftool.rst
>>
>> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
>> ---
>>
>> v2: used tabs instead of spaces
> 
> Thanks a lot!
> 
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>

Applied, thanks!

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: offload: add priv field for drivers
From: Daniel Borkmann @ 2019-02-12 16:10 UTC (permalink / raw)
  To: Jakub Kicinski, alexei.starovoitov; +Cc: oss-drivers, netdev
In-Reply-To: <20190212082039.31109-1-jakub.kicinski@netronome.com>

On 02/12/2019 09:20 AM, Jakub Kicinski wrote:
> Currently bpf_offload_dev does not have any priv pointer, forcing
> the drivers to work backwards from the netdev in program metadata.
> This is not great given programs are conceptually associated with
> the offload device, and it means one or two unnecessary deferences.
> Add a priv pointer to bpf_offload_dev.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>

Applied, thanks!

^ permalink raw reply

* Re: [PATCH net] sctp: call gso_reset_checksum when computing checksum in sctp_gso_segment
From: Marcelo Ricardo Leitner @ 2019-02-12 16:15 UTC (permalink / raw)
  To: Xin Long; +Cc: linux-kernel, network dev, linux-sctp, davem, Neil Horman
In-Reply-To: <5b8187d1eabd52e4db7d3e4506d98c33571c1c83.1549968450.git.lucien.xin@gmail.com>

On Tue, Feb 12, 2019 at 06:47:30PM +0800, Xin Long wrote:
> Jianlin reported a panic when running sctp gso over gre over vlan device:
> 
>   [   84.772930] RIP: 0010:do_csum+0x6d/0x170
>   [   84.790605] Call Trace:
>   [   84.791054]  csum_partial+0xd/0x20
>   [   84.791657]  gre_gso_segment+0x2c3/0x390
>   [   84.792364]  inet_gso_segment+0x161/0x3e0
>   [   84.793071]  skb_mac_gso_segment+0xb8/0x120
>   [   84.793846]  __skb_gso_segment+0x7e/0x180
>   [   84.794581]  validate_xmit_skb+0x141/0x2e0
>   [   84.795297]  __dev_queue_xmit+0x258/0x8f0
>   [   84.795949]  ? eth_header+0x26/0xc0
>   [   84.796581]  ip_finish_output2+0x196/0x430
>   [   84.797295]  ? skb_gso_validate_network_len+0x11/0x80
>   [   84.798183]  ? ip_finish_output+0x169/0x270
>   [   84.798875]  ip_output+0x6c/0xe0
>   [   84.799413]  ? ip_append_data.part.50+0xc0/0xc0
>   [   84.800145]  iptunnel_xmit+0x144/0x1c0
>   [   84.800814]  ip_tunnel_xmit+0x62d/0x930 [ip_tunnel]
>   [   84.801699]  gre_tap_xmit+0xac/0xf0 [ip_gre]
>   [   84.802395]  dev_hard_start_xmit+0xa5/0x210
>   [   84.803086]  sch_direct_xmit+0x14f/0x340
>   [   84.803733]  __dev_queue_xmit+0x799/0x8f0
>   [   84.804472]  ip_finish_output2+0x2e0/0x430
>   [   84.805255]  ? skb_gso_validate_network_len+0x11/0x80
>   [   84.806154]  ip_output+0x6c/0xe0
>   [   84.806721]  ? ip_append_data.part.50+0xc0/0xc0
>   [   84.807516]  sctp_packet_transmit+0x716/0xa10 [sctp]
>   [   84.808337]  sctp_outq_flush+0xd7/0x880 [sctp]
> 
> It was caused by SKB_GSO_CB(skb)->csum_start not set in sctp_gso_segment.
> sctp_gso_segment() calls skb_segment() with 'feature | NETIF_F_HW_CSUM',
> which causes SKB_GSO_CB(skb)->csum_start not to be set in skb_segment().
> 
> For TCP/UDP, when feature supports HW_CSUM, CHECKSUM_PARTIAL will be set
> and gso_reset_checksum will be called to set SKB_GSO_CB(skb)->csum_start.
> 
> So SCTP should do the same as TCP/UDP, to call gso_reset_checksum() when
> computing checksum in sctp_gso_segment.
> 
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  net/sctp/offload.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/sctp/offload.c b/net/sctp/offload.c
> index 123e9f2..edfcf16 100644
> --- a/net/sctp/offload.c
> +++ b/net/sctp/offload.c
> @@ -36,6 +36,7 @@ static __le32 sctp_gso_make_checksum(struct sk_buff *skb)
>  {
>  	skb->ip_summed = CHECKSUM_NONE;
>  	skb->csum_not_inet = 0;
> +	gso_reset_checksum(skb, ~0);
>  	return sctp_compute_cksum(skb, skb_transport_offset(skb));
>  }
>  
> -- 
> 2.1.0
> 

^ permalink raw reply

* [PATCH net] net: fealnx: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-12 15:56 UTC (permalink / raw)
  To: netdev; +Cc: davem, yang.wei9, albin_yang

From: Yang Wei <yang.wei9@zte.com.cn>

dev_consume_skb_irq() should be called in intr_handler() when skb
xmit done. It makes drop profiles(dropwatch, perf) more friendly.

Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
 drivers/net/ethernet/fealnx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c
index ae55da6..c24fd56 100644
--- a/drivers/net/ethernet/fealnx.c
+++ b/drivers/net/ethernet/fealnx.c
@@ -1531,7 +1531,7 @@ static irqreturn_t intr_handler(int irq, void *dev_instance)
 			/* Free the original skb. */
 			pci_unmap_single(np->pci_dev, np->cur_tx->buffer,
 				np->cur_tx->skbuff->len, PCI_DMA_TODEVICE);
-			dev_kfree_skb_irq(np->cur_tx->skbuff);
+			dev_consume_skb_irq(np->cur_tx->skbuff);
 			np->cur_tx->skbuff = NULL;
 			--np->really_tx_count;
 			if (np->cur_tx->control & TXLD) {
-- 
2.7.4



^ permalink raw reply related

* [PATCH] net: stmmac: Add SMC support for EMAC System Manager register
From: Ooi, Joyce @ 2019-02-12 16:24 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S. Miller,
	Maxime Coquelin
  Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel,
	See Chin Liang, Joyce Ooi

As there is restriction to access to EMAC System Manager registers in
the kernel for Intel Stratix10, the use of SMC calls are required and
added in dwmac-socfpga driver.

Signed-off-by: Ooi, Joyce <joyce.ooi@intel.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    |  101 ++++++++++++++++++++
 1 files changed, 101 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 5b3b06a..55cce97 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -15,6 +15,10 @@
  * Adopted from dwmac-sti.c
  */
 
+#if defined CONFIG_HAVE_ARM_SMCCC && defined CONFIG_ARCH_STRATIX10
+#include <linux/arm-smccc.h>
+#include <linux/firmware/intel/stratix10-smc.h>
+#endif
 #include <linux/mfd/syscon.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
@@ -52,6 +56,9 @@ struct socfpga_dwmac {
 	int	interface;
 	u32	reg_offset;
 	u32	reg_shift;
+#if defined CONFIG_HAVE_ARM_SMCCC && defined CONFIG_ARCH_STRATIX10
+	u32	sysmgr_reg;
+#endif
 	struct	device *dev;
 	struct regmap *sys_mgr_base_addr;
 	struct reset_control *stmmac_rst;
@@ -61,6 +68,63 @@ struct socfpga_dwmac {
 	struct tse_pcs pcs;
 };
 
+#if defined CONFIG_HAVE_ARM_SMCCC && defined CONFIG_ARCH_STRATIX10
+/**************** Stratix 10 EMAC Memory Controller Functions ************/
+
+/* s10_protected_reg_write
+ * Write to a protected SMC register.
+ * @context: Not used
+ * @reg: Address of register
+ * @value: Value to write
+ * Return: INTEL_SIP_SMC_STATUS_OK (0) on success
+ *         INTEL_SIP_SMC_REG_ERROR on error
+ *         INTEL_SIP_SMC_RETURN_UNKNOWN_FUNCTION if not supported
+ */
+static int s10_protected_reg_write(void *context, unsigned int reg,
+				   unsigned int val)
+{
+	struct arm_smccc_res result;
+
+	arm_smccc_smc(INTEL_SIP_SMC_REG_WRITE, reg, val, 0, 0,
+		      0, 0, 0, &result);
+
+	return (int)result.a0;
+}
+
+/* s10_protected_reg_read
+ * Read the status of a protected SMC register
+ * @context: Not used
+ * @reg: Address of register
+ * @value: Value read.
+ * Return: INTEL_SIP_SMC_STATUS_OK (0) on success
+ *         INTEL_SIP_SMC_REG_ERROR on error
+ *         INTEL_SIP_SMC_RETURN_UNKNOWN_FUNCTION if not supported
+ */
+static int s10_protected_reg_read(void *context, unsigned int reg,
+				  unsigned int *val)
+{
+	struct arm_smccc_res result;
+
+	arm_smccc_smc(INTEL_SIP_SMC_REG_READ, reg, 0, 0, 0,
+		      0, 0, 0, &result);
+
+	*val = (unsigned int)result.a1;
+
+	return (int)result.a0;
+}
+
+static const struct regmap_config s10_emac_regmap_cfg = {
+	.name = "s10_emac",
+	.reg_bits = 32,
+	.val_bits = 32,
+	.max_register = 0xffffffff,
+	.reg_read = s10_protected_reg_read,
+	.reg_write = s10_protected_reg_write,
+	.use_single_read = true,
+	.use_single_write = true,
+};
+#endif
+
 static void socfpga_dwmac_fix_mac_speed(void *priv, unsigned int speed)
 {
 	struct socfpga_dwmac *dwmac = (struct socfpga_dwmac *)priv;
@@ -105,20 +169,43 @@ static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, struct device *
 	struct device_node *np = dev->of_node;
 	struct regmap *sys_mgr_base_addr;
 	u32 reg_offset, reg_shift;
+#if defined CONFIG_HAVE_ARM_SMCCC && defined CONFIG_ARCH_STRATIX10
+	u32 sysmgr_reg = 0;
+#endif
 	int ret, index;
 	struct device_node *np_splitter = NULL;
 	struct device_node *np_sgmii_adapter = NULL;
+#if defined CONFIG_HAVE_ARM_SMCCC && defined CONFIG_ARCH_STRATIX10
+	struct device_node *np_sysmgr = NULL;
+#endif
 	struct resource res_splitter;
 	struct resource res_tse_pcs;
 	struct resource res_sgmii_adapter;
 
 	dwmac->interface = of_get_phy_mode(np);
 
+#if defined CONFIG_HAVE_ARM_SMCCC && defined CONFIG_ARCH_STRATIX10
+	sys_mgr_base_addr = devm_regmap_init(dev, NULL, (void *)dwmac,
+					     &s10_emac_regmap_cfg);
+	if (IS_ERR(sys_mgr_base_addr))
+		return PTR_ERR(sys_mgr_base_addr);
+
+	np_sysmgr = of_parse_phandle(np, "altr,sysmgr-syscon", 0);
+	if (np_sysmgr) {
+		ret = of_property_read_u32_index(np_sysmgr, "reg", 0,
+						 &sysmgr_reg);
+		if (ret) {
+			dev_info(dev, "Could not read sysmgr register address\n");
+			return -EINVAL;
+		}
+	}
+#else
 	sys_mgr_base_addr = syscon_regmap_lookup_by_phandle(np, "altr,sysmgr-syscon");
 	if (IS_ERR(sys_mgr_base_addr)) {
 		dev_info(dev, "No sysmgr-syscon node found\n");
 		return PTR_ERR(sys_mgr_base_addr);
 	}
+#endif
 
 	ret = of_property_read_u32_index(np, "altr,sysmgr-syscon", 1, &reg_offset);
 	if (ret) {
@@ -222,6 +309,9 @@ static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, struct device *
 	dwmac->reg_offset = reg_offset;
 	dwmac->reg_shift = reg_shift;
 	dwmac->sys_mgr_base_addr = sys_mgr_base_addr;
+#if defined CONFIG_HAVE_ARM_SMCCC && defined CONFIG_ARCH_STRATIX10
+	dwmac->sysmgr_reg = sysmgr_reg;
+#endif
 	dwmac->dev = dev;
 	of_node_put(np_sgmii_adapter);
 
@@ -238,6 +328,9 @@ static int socfpga_dwmac_set_phy_mode(struct socfpga_dwmac *dwmac)
 	int phymode = dwmac->interface;
 	u32 reg_offset = dwmac->reg_offset;
 	u32 reg_shift = dwmac->reg_shift;
+#if defined CONFIG_HAVE_ARM_SMCCC && defined CONFIG_ARCH_STRATIX10
+	u32 sysmgr_reg = dwmac->sysmgr_reg;
+#endif
 	u32 ctrl, val, module;
 
 	switch (phymode) {
@@ -266,7 +359,11 @@ static int socfpga_dwmac_set_phy_mode(struct socfpga_dwmac *dwmac)
 	reset_control_assert(dwmac->stmmac_ocp_rst);
 	reset_control_assert(dwmac->stmmac_rst);
 
+#if defined CONFIG_HAVE_ARM_SMCCC && defined CONFIG_ARCH_STRATIX10
+	regmap_read(sys_mgr_base_addr, sysmgr_reg + reg_offset, &ctrl);
+#else
 	regmap_read(sys_mgr_base_addr, reg_offset, &ctrl);
+#endif
 	ctrl &= ~(SYSMGR_EMACGRP_CTRL_PHYSEL_MASK << reg_shift);
 	ctrl |= val << reg_shift;
 
@@ -284,7 +381,11 @@ static int socfpga_dwmac_set_phy_mode(struct socfpga_dwmac *dwmac)
 		ctrl &= ~(SYSMGR_EMACGRP_CTRL_PTP_REF_CLK_MASK << (reg_shift / 2));
 	}
 
+#if defined CONFIG_HAVE_ARM_SMCCC && defined CONFIG_ARCH_STRATIX10
+	regmap_write(sys_mgr_base_addr, sysmgr_reg + reg_offset, ctrl);
+#else
 	regmap_write(sys_mgr_base_addr, reg_offset, ctrl);
+#endif
 
 	/* Deassert reset for the phy configuration to be sampled by
 	 * the enet controller, and operation to start in requested mode
-- 
1.7.1


^ permalink raw reply related

* [PATCH net-next 0/4] mlxsw: Several updates
From: Ido Schimmel @ 2019-02-12 16:29 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: davem@davemloft.net, Jiri Pirko, Nir Dotan, mlxsw, Ido Schimmel

Patches #1-#3 contain misc updates for the mlxsw driver, one of which is
a fix following recent introduction of flow_rule infrastructure.

Patch #4 avoids double sourcing of lib.sh in forwarding selftests.

Ido Schimmel (2):
  mlxsw: spectrum_router: Drop unnecessary WARN_ON_ONCE()
  mlxsw: spectrum_flower: Fix VLAN modify action support

Jiri Pirko (1):
  selftests: mlxsw: avoid double sourcing of lib.sh

Nir Dotan (1):
  mlxsw: spectrum: Set LAG port collector only when active

 .../net/ethernet/mellanox/mlxsw/spectrum.c    | 62 ++++++++++++++-----
 .../ethernet/mellanox/mlxsw/spectrum_flower.c |  3 +-
 .../ethernet/mellanox/mlxsw/spectrum_router.c |  2 +-
 .../net/mlxsw/spectrum/resource_scale.sh      |  1 -
 4 files changed, 47 insertions(+), 21 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH net-next 1/4] mlxsw: spectrum: Set LAG port collector only when active
From: Ido Schimmel @ 2019-02-12 16:29 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: davem@davemloft.net, Jiri Pirko, Nir Dotan, mlxsw, Ido Schimmel
In-Reply-To: <20190212162924.29777-1-idosch@mellanox.com>

From: Nir Dotan <nird@mellanox.com>

The LAG port collecting (receive) function was mistakenly set when the
port was registered as a LAG member, while it should be set only when
the port collection state is set to true. Set LAG port to collecting
when it is set to distributing, as described in the IEEE link
aggregation standard coupled control mux machine state diagram.

Signed-off-by: Nir Dotan <nird@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum.c    | 62 ++++++++++++++-----
 1 file changed, 45 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 7c9745cecbbd..9686d3822b92 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4771,9 +4771,6 @@ static int mlxsw_sp_port_lag_join(struct mlxsw_sp_port *mlxsw_sp_port,
 	err = mlxsw_sp_lag_col_port_add(mlxsw_sp_port, lag_id, port_index);
 	if (err)
 		goto err_col_port_add;
-	err = mlxsw_sp_lag_col_port_enable(mlxsw_sp_port, lag_id);
-	if (err)
-		goto err_col_port_enable;
 
 	mlxsw_core_lag_mapping_set(mlxsw_sp->core, lag_id, port_index,
 				   mlxsw_sp_port->local_port);
@@ -4787,8 +4784,6 @@ static int mlxsw_sp_port_lag_join(struct mlxsw_sp_port *mlxsw_sp_port,
 
 	return 0;
 
-err_col_port_enable:
-	mlxsw_sp_lag_col_port_remove(mlxsw_sp_port, lag_id);
 err_col_port_add:
 	if (!lag->ref_count)
 		mlxsw_sp_lag_destroy(mlxsw_sp, lag_id);
@@ -4807,7 +4802,6 @@ static void mlxsw_sp_port_lag_leave(struct mlxsw_sp_port *mlxsw_sp_port,
 	lag = mlxsw_sp_lag_get(mlxsw_sp, lag_id);
 	WARN_ON(lag->ref_count == 0);
 
-	mlxsw_sp_lag_col_port_disable(mlxsw_sp_port, lag_id);
 	mlxsw_sp_lag_col_port_remove(mlxsw_sp_port, lag_id);
 
 	/* Any VLANs configured on the port are no longer valid */
@@ -4852,21 +4846,56 @@ static int mlxsw_sp_lag_dist_port_remove(struct mlxsw_sp_port *mlxsw_sp_port,
 	return mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(sldr), sldr_pl);
 }
 
-static int mlxsw_sp_port_lag_tx_en_set(struct mlxsw_sp_port *mlxsw_sp_port,
-				       bool lag_tx_enabled)
+static int
+mlxsw_sp_port_lag_col_dist_enable(struct mlxsw_sp_port *mlxsw_sp_port)
 {
-	if (lag_tx_enabled)
-		return mlxsw_sp_lag_dist_port_add(mlxsw_sp_port,
-						  mlxsw_sp_port->lag_id);
-	else
-		return mlxsw_sp_lag_dist_port_remove(mlxsw_sp_port,
-						     mlxsw_sp_port->lag_id);
+	int err;
+
+	err = mlxsw_sp_lag_col_port_enable(mlxsw_sp_port,
+					   mlxsw_sp_port->lag_id);
+	if (err)
+		return err;
+
+	err = mlxsw_sp_lag_dist_port_add(mlxsw_sp_port, mlxsw_sp_port->lag_id);
+	if (err)
+		goto err_dist_port_add;
+
+	return 0;
+
+err_dist_port_add:
+	mlxsw_sp_lag_col_port_disable(mlxsw_sp_port, mlxsw_sp_port->lag_id);
+	return err;
+}
+
+static int
+mlxsw_sp_port_lag_col_dist_disable(struct mlxsw_sp_port *mlxsw_sp_port)
+{
+	int err;
+
+	err = mlxsw_sp_lag_dist_port_remove(mlxsw_sp_port,
+					    mlxsw_sp_port->lag_id);
+	if (err)
+		return err;
+
+	err = mlxsw_sp_lag_col_port_disable(mlxsw_sp_port,
+					    mlxsw_sp_port->lag_id);
+	if (err)
+		goto err_col_port_disable;
+
+	return 0;
+
+err_col_port_disable:
+	mlxsw_sp_lag_dist_port_add(mlxsw_sp_port, mlxsw_sp_port->lag_id);
+	return err;
 }
 
 static int mlxsw_sp_port_lag_changed(struct mlxsw_sp_port *mlxsw_sp_port,
 				     struct netdev_lag_lower_state_info *info)
 {
-	return mlxsw_sp_port_lag_tx_en_set(mlxsw_sp_port, info->tx_enabled);
+	if (info->tx_enabled)
+		return mlxsw_sp_port_lag_col_dist_enable(mlxsw_sp_port);
+	else
+		return mlxsw_sp_port_lag_col_dist_disable(mlxsw_sp_port);
 }
 
 static int mlxsw_sp_port_stp_set(struct mlxsw_sp_port *mlxsw_sp_port,
@@ -5089,8 +5118,7 @@ static int mlxsw_sp_netdevice_port_upper_event(struct net_device *lower_dev,
 				err = mlxsw_sp_port_lag_join(mlxsw_sp_port,
 							     upper_dev);
 			} else {
-				mlxsw_sp_port_lag_tx_en_set(mlxsw_sp_port,
-							    false);
+				mlxsw_sp_port_lag_col_dist_disable(mlxsw_sp_port);
 				mlxsw_sp_port_lag_leave(mlxsw_sp_port,
 							upper_dev);
 			}
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next 2/4] mlxsw: spectrum_router: Drop unnecessary WARN_ON_ONCE()
From: Ido Schimmel @ 2019-02-12 16:29 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: davem@davemloft.net, Jiri Pirko, Nir Dotan, mlxsw, Ido Schimmel
In-Reply-To: <20190212162924.29777-1-idosch@mellanox.com>

In case the register access failed an error would be logged anyway, so
we can drop the warning.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 818040ce4d68..52fed8c7bf1e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -6142,7 +6142,7 @@ static int mlxsw_sp_router_rif_disable(struct mlxsw_sp *mlxsw_sp, u16 rif)
 
 	mlxsw_reg_ritr_rif_pack(ritr_pl, rif);
 	err = mlxsw_reg_query(mlxsw_sp->core, MLXSW_REG(ritr), ritr_pl);
-	if (WARN_ON_ONCE(err))
+	if (err)
 		return err;
 
 	mlxsw_reg_ritr_enable_set(ritr_pl, false);
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next 3/4] mlxsw: spectrum_flower: Fix VLAN modify action support
From: Ido Schimmel @ 2019-02-12 16:29 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: davem@davemloft.net, Jiri Pirko, Nir Dotan, mlxsw, Ido Schimmel,
	Pablo Neira Ayuso
In-Reply-To: <20190212162924.29777-1-idosch@mellanox.com>

The driver does not support VLAN push and pop, but only VLAN modify.

Fixes: 738678817573 ("drivers: net: use flow action infrastructure")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
index 9af9f5c1b25c..15f804453cd6 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
@@ -102,8 +102,7 @@ static int mlxsw_sp_flower_parse_actions(struct mlxsw_sp *mlxsw_sp,
 				return err;
 			}
 			break;
-		case FLOW_ACTION_VLAN_PUSH:
-		case FLOW_ACTION_VLAN_POP: {
+		case FLOW_ACTION_VLAN_MANGLE: {
 			u16 proto = be16_to_cpu(act->vlan.proto);
 			u8 prio = act->vlan.prio;
 			u16 vid = act->vlan.vid;
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next 4/4] selftests: mlxsw: avoid double sourcing of lib.sh
From: Ido Schimmel @ 2019-02-12 16:29 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: davem@davemloft.net, Jiri Pirko, Nir Dotan, mlxsw, Ido Schimmel
In-Reply-To: <20190212162924.29777-1-idosch@mellanox.com>

From: Jiri Pirko <jiri@mellanox.com>

Don't source lib.sh 2 times and make the script work with ifnames
passed on the command line.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../selftests/drivers/net/mlxsw/spectrum/resource_scale.sh       | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/spectrum/resource_scale.sh b/tools/testing/selftests/drivers/net/mlxsw/spectrum/resource_scale.sh
index a0a80e1a69e8..e7ffc79561b7 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/spectrum/resource_scale.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/spectrum/resource_scale.sh
@@ -2,7 +2,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
 NUM_NETIFS=6
-source ../../../../net/forwarding/lib.sh
 source ../../../../net/forwarding/tc_common.sh
 source devlink_lib_spectrum.sh
 
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH net] dsa: mv88e6xxx: Ensure all pending interrupts are handled prior to exit
From: Russell King - ARM Linux admin @ 2019-02-12 16:30 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Andrew Lunn, John David Anglin, Vivien Didelot, Florian Fainelli,
	netdev
In-Reply-To: <13c1e6d5-c287-0091-3b24-1978f9a18e7e@gmail.com>

On Tue, Feb 12, 2019 at 07:51:05AM +0100, Heiner Kallweit wrote:
> On 12.02.2019 04:58, Andrew Lunn wrote:
> > That change means we don't check the PHY device if it caused an
> > interrupt when its state is less than UP.
> > 
> > What i'm seeing is that the PHY is interrupting pretty early on after
> > a reboot when the previous boot had the interface up.
> > 
> So this means that when going down for reboot the interrupts are not
> properly masked / disabled? Because (at least for net-next) we enable
> interrupts in phy_start() only.

Looking at Linus' tree as opposed to net-next, things do look rather
broken wrt interrupts:

+-phy_attach_direct
  `-phydev->state = PHY_READY
+-phy_prepare_link
+-phy_start_machine
  `-phy_trigger_machine()
`-phy_start_interrupts
  +-request_threaded_irq()
  `-phy_enable_interrupts()
    +-phy_clear_interrupt()
    `-phy_config_interrupt(, PHY_INTERRUPT_ENABLED)

At this point, the PHY is then able to generate interrupts, which,
because phy_start() has not been called and phy_interrupt() checks
that phydev->state >= PHY_UP, get ignored by the interrupt handler
exactly as Andrew is finding.

So it looks like 5.0-rc is already in need of this being fixed.

In looking at this, I came across this chunk of code:

static inline bool __phy_is_started(struct phy_device *phydev)
{
        WARN_ON(!mutex_is_locked(&phydev->lock));

        return phydev->state >= PHY_UP;
}

/**
 * phy_is_started - Convenience function to check whether PHY is started
 * @phydev: The phy_device struct
 */
static inline bool phy_is_started(struct phy_device *phydev)
{
        bool started;

        mutex_lock(&phydev->lock);
        started = __phy_is_started(phydev);
        mutex_unlock(&phydev->lock);

        return started;
}

which looks to me like over-complication.  The mutex locking there is
completely pointless - what are you trying to achieve with it?

Let's go through this.  The above is exactly equivalent to:

bool phy_is_started(phydev)
{
	int state;

	mutex_lock(&phydev->lock);
	state = phydev->state;
	mutex_unlock(&phydev->lock);

	return state >= PHY_UP;
}

since when we do the test is irrelevant.  Architectures that Linux
runs on are single-copy atomic, which means that reading phydev->state
itself is an atomic operation.  So, the mutex locking around that
doesn't add to the atomicity of the entire operation.

How, depending on what you do with the rest of this function depends
whether the entire operation is safe or not.  For example, let's take
this code at the end of phy_state_machine():

        if (phy_polling_mode(phydev) && phy_is_started(phydev))
                phy_queue_state_machine(phydev, PHY_STATE_TIME);

state = PHY_UP
		thread 0			thread 1
						phy_disconnect()
						+-phy_is_started()
		phy_is_started()                |
						`-phy_stop()
						  +-phydev->state = PHY_HALTED
						  `-phy_stop_machine()
						    `-cancel_delayed_work_sync()
		phy_queue_state_machine()
		`-mod_delayed_work()

At this point, the phydev->state_queue() has been added back onto the
system workqueue despite phy_stop_machine() having been called and
cancel_delayed_work_sync() called on it.

The original code in 4.20 did not have this race condition.

Basically, the lock inside phy_is_started() does nothing useful, and
I'd say is dangerously misleading.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* Re: [PATCH net-next] nfp: flower: remove double new line
From: David Miller @ 2019-02-12 16:39 UTC (permalink / raw)
  To: jakub.kicinski; +Cc: netdev, oss-drivers
In-Reply-To: <20190212081802.30685-1-jakub.kicinski@netronome.com>

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Tue, 12 Feb 2019 00:18:02 -0800

> Recent cls_flower offload rewrite added a double new line.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>

Applied.

^ permalink raw reply

* Re: pull-request: mac80211 2019-02-12
From: David Miller @ 2019-02-12 16:43 UTC (permalink / raw)
  To: johannes; +Cc: netdev, linux-wireless
In-Reply-To: <20190212115121.27086-1-johannes@sipsolutions.net>

From: Johannes Berg <johannes@sipsolutions.net>
Date: Tue, 12 Feb 2019 12:51:20 +0100

> We have few more fixes, mostly one-liners; two are bigger:
>  * the speculation one, only because the function had multiple
>    return points and that had to change, and
>  * the peer measurement locking one, because I had to refactor
>    a function to be able to call it with or without locking
>    (depending on context).
> 
> Please pull and let me know if there's any problem.

Pulled, thanks Johannes.

^ permalink raw reply

* Re: [PATCH net] sfc: initialise found bitmap in efx_ef10_mtd_probe
From: David Miller @ 2019-02-12 16:50 UTC (permalink / raw)
  To: bkenward; +Cc: linux-net-drivers, netdev
In-Reply-To: <b38d9512-b88f-e07c-b559-0150cc486441@solarflare.com>

From: Bert Kenward <bkenward@solarflare.com>
Date: Tue, 12 Feb 2019 13:10:00 +0000

> The bitmap of found partitions in efx_ef10_mtd_probe was not
> initialised, causing partitions to be suppressed based off whatever
> value was in the bitmap at the start.
> 
> Fixes: 3366463513f5 ("sfc: suppress duplicate nvmem partition types in efx_ef10_mtd_probe")
> Signed-off-by: Bert Kenward <bkenward@solarflare.com>

Applied, thanks.

^ permalink raw reply

* [PATCHv1 net-next 0/2] devlink: 2 fixes for devlink region read
From: Parav Pandit @ 2019-02-12 16:51 UTC (permalink / raw)
  To: jiri, davem, netdev; +Cc: parav

This 2 patches consist of fixes for devlink region read handling.

Signed-off-by: Parav Pandit <parav@mellanox.com>
---

Parav Pandit (2):
  devlink: Return right error code in case of errors for region read
  devlink: Fix list access without lock while reading region

 net/core/devlink.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

-- 
1.8.3.1


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox