Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 1/7] fix hnode refcounting
From: Jamal Hadi Salim @ 2018-09-07 12:13 UTC (permalink / raw)
  To: Al Viro; +Cc: netdev, Cong Wang, Jiri Pirko, stable
In-Reply-To: <20180907023529.GV19965@ZenIV.linux.org.uk>

On 2018-09-06 10:35 p.m., Al Viro wrote:
> On Thu, Sep 06, 2018 at 06:21:09AM -0400, Jamal Hadi Salim wrote:
[..]
> 
> Argh...  Unfortunately, there's this: in u32_delete() we have
>          if (root_ht) {
>                  if (root_ht->refcnt > 1) {
>                          *last = false;
>                          goto ret;
>                  }
>                  if (root_ht->refcnt == 1) {
>                          if (!ht_empty(root_ht)) {
>                                  *last = false;
>                                  goto ret;
>                          }
>                  }
>          }
> and that would need to be updated.  

It is not detrimental as you have it right now but
you are right an adjustment is needed...

Deleting of a root directly should not be allowed. But
you can flush a whole tp. Consider this:
--
sudo tc qdisc add dev $P ingress
sudo tc filter add dev $P parent ffff: protocol ip prio 10 \
u32 match ip protocol 1 0xff

Which creates root ht 800

You shouldnt be allowed to do this:
--
tc filter delete dev $P parent ffff: protocol ip prio 10 handle 800: u32
---

But you can delete the tp entirely as such:
---
tc filter delete dev $P parent ffff: protocol ip prio 10 u32
--

The later will go via the destroy() path and flush all filters.

You should also be able to delete individual filters. ex:
$tc filter del dev $P parent ffff: prio 10 handle 800:0:800 u32

Where that code you are referring to is important is when
the last filter deleted - we need the caller to know
and it destroys root.

i.e you should return last=true when the last filter is
deleted so root gets auto deleted (just like it was autocreated)

> However, that logics is bloody odd
> to start with.  First of all, root_ht has come from
>         struct tc_u_hnode *root_ht = rtnl_dereference(tp->root);
> and the only place where it's ever modified is
>          rcu_assign_pointer(tp->root, root_ht);
> in u32_init(), where we'd bloody well checked that root_ht is non-NULL
> (see
>          if (root_ht == NULL)
>                  return -ENOBUFS;
> upstream of that place) and where that assignment is inevitable on the
> way to returning 0.  No matter what, if tp has passed u32_init() it
> will have non-NULL ->root, forever.  And there is no way for tcf_proto
> to be seen outside of tcf_proto_create() without ->init() having returned
> 0 - it gets freed before anyone sees it.
> 

Yes, the check for root_ht is not necessary - but the check for the
last filter (and testing for last) is needed.

> So this 'if (root_ht)' can't be false.  What's more, what the hell is the
> whole thing checking?  We are in u32_delete().  It's called (as ->delete())
> from tfilter_del_notify(), which is called from tc_del_tfilter().  If we
> return 0 with *last true, we follow up calling tcf_proto_destroy().
> OK, let's look at the logics in there:
> 	* if there are links to root hnode => false
> 	* if there's no links to root hnode and it has knodes => false
> (BTW, if we ever get there with root_ht->refcnt < 1, we are obviously screwed)
> 	* if there is a tcf_proto sharing tp->data => false (i.e. any filters
> with different prio - don't bother)
> 	* if tp is the only one with reference to tp->data and there are *any*
> knodes => false.
> 
> Any extra links can come only from knodes in a non-empty hnode.  And it's not
> a common case.  Shouldn't thIe whole thing be
> 	* shared tp->data => false
> 	* any non-empty hnode => false
> instead?  Perhaps even with the knode counter in tp->data, avoiding any loops
> in there, as well as the entire ht_empty()...
> 
> Now, in the very beginning of u32_delete() we have this:
>          struct tc_u_hnode *ht = arg;
> 	
>          if (ht == NULL)
>                  goto out;
> OK, but the call of ->delete() is
>          err = tp->ops->delete(tp, fh, last, extack);
> and arg == NULL seen in u32_delete() means fh == NULL in tfilter_del_notify().
> Which is called in
>          if (!fh) {
> 		...
> 	} else {
>                  bool last;
> 
>                  err = tfilter_del_notify(net, skb, n, tp, block,
>                                           q, parent, fh, false, &last,
>                                           extack);
> How can we ever get there with NULL fh?
>

Try:
tc filter delete dev $P parent ffff: protocol ip prio 10 u32
tcm handle is 0, so will hit that code path.

> The whole thing makes very little sense; looks like it used to live in
> u32_destroy() prior to commit 763dbf6328e41 ("net_sched: move the empty tp
> check from ->destroy() to ->delete()"), but looking at the rationale in
> that commit...  I don't see how it fixes anything - sure, now we remove
> tcf_proto from the list before calling ->destroy().  Without any RCU delays
> in between.  How could it possibly solve any issues with ->classify()
> called in parallel with ->destroy()?  cls_u32 (at least these days)
> does try to survive u32_destroy() in parallel with u32_classify();
> if any other classifiers do not, they are still broken and that commit
> has not done anything for them.
> 
> Anyway, adjusting 1/7 for that is trivial, but I would really like to
> understand what that code is doing...  Comments?
> 

Refer to above.

cheers,
jamal

^ permalink raw reply

* [PATCH] nfp: replace spin_lock_bh with spin_lock in tasklet callback
From: jun qian @ 2018-09-07 17:01 UTC (permalink / raw)
  To: Jakub Kicinski, Dirk van der Merwe, Daniel Borkmann,
	Quentin Monnet
  Cc: oss-drivers, netdev, linux-kernel, jun qian

As you are already in a tasklet, it is unnecessary to call spin_lock_bh.

Signed-off-by: jun qian <hangdianqj@163.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index a8b9fbab5f73..084c983ec3c2 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -2075,10 +2075,10 @@ static void nfp_ctrl_poll(unsigned long arg)
 {
 	struct nfp_net_r_vector *r_vec = (void *)arg;
 
-	spin_lock_bh(&r_vec->lock);
+	spin_lock(&r_vec->lock);
 	nfp_net_tx_complete(r_vec->tx_ring, 0);
 	__nfp_ctrl_tx_queued(r_vec);
-	spin_unlock_bh(&r_vec->lock);
+	spin_unlock(&r_vec->lock);
 
 	nfp_ctrl_rx(r_vec);
 
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH v3 2/2] net: ethernet: i40evf: fix underlying build error
From: Wang, Dongsheng @ 2018-09-07 17:14 UTC (permalink / raw)
  To: Sergei Shtylyov, jeffrey.t.kirsher@intel.com
  Cc: jacob.e.keller@intel.com, davem@davemloft.net,
	intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <59fd149c-bacc-c39c-b0d8-a4fb9366f26a@cogentembedded.com>

On 9/7/2018 11:33 PM, Sergei Shtylyov wrote:
> On 09/07/2018 02:19 PM, Wang Dongsheng wrote:
>
>> Can't have non-inline function in a header file.
>> There is a risk of "Multiple definition" from cross-including.
>>
>> Tested on: x86_64, make ARCH=i386
>>
>> Modules section .text:
>> i40e: 00019380 <__i40e_add_stat_strings>:
>> i40evf: 00006b00 <__i40e_add_stat_strings>:
>>
>> Buildin section .text:
>> i40e: c351ca60 <__i40e_add_stat_strings>:
>> i40evf: c354f2c0 <__i40e_add_stat_strings>:
>>
>> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>
>> ---
>> V3: add static 
>> ---
>>  .../intel/i40evf/i40e_ethtool_stats.h         | 23 +-----------------
>>  .../ethernet/intel/i40evf/i40evf_ethtool.c    | 24 +++++++++++++++++++
>>  2 files changed, 25 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h b/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
>> index 60b595dd8c39..62ab67a77753 100644
>> --- a/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
>> +++ b/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
>> @@ -181,29 +181,8 @@ i40evf_add_queue_stats(u64 **data, struct i40e_ring *ring)
>>  	*data += size;
>>  }
>>  
>> -/**
>> - * __i40e_add_stat_strings - copy stat strings into ethtool buffer
>> - * @p: ethtool supplied buffer
>> - * @stats: stat definitions array
>> - * @size: size of the stats array
>> - *
>> - * Format and copy the strings described by stats into the buffer pointed at
>> - * by p.
>> - **/
>>  static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
>    There's no point to keeping *static* function in the header file (unless it's
> also *inline*).

Yes, we need it for now. Because there is a copy file at i40e dir, and
a "Multiple definition" will show up when we buildin i40e&i40evf and
remove this *static* .

Cause the header file is only used  in ethtool.c so we can keep this
static, and another option is not touch this header.

As I replied to Jacob's email earlier, we can do without touch i40evf at
all. Because this header is only for one and not included in another.


Cheers,

Dongsheng

>> -				    const unsigned int size, ...)
>> -{
>> -	unsigned int i;
>> -
>> -	for (i = 0; i < size; i++) {
>> -		va_list args;
>> -
>> -		va_start(args, size);
>> -		vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
>> -		*p += ETH_GSTRING_LEN;
>> -		va_end(args);
>> -	}
>> -}
>> +				    const unsigned int size, ...);
>>  
>>  /**
>>   * 40e_add_stat_strings - copy stat strings into ethtool buffer
> [...]
>
> MBR, Sergei
>

^ permalink raw reply

* [PATCH] wimax: i2400m: remove unnecessary unlikely()
From: Igor Stoppa @ 2018-09-07 17:22 UTC (permalink / raw)
  To: Inaky Perez-Gonzalez
  Cc: igor.stoppa, Igor Stoppa, linux-wimax, David S. Miller, netdev,
	linux-kernel

WARN_ON() already contains an unlikely(), so it's not necessary to
wrap it into another.

Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>
Cc: Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>
Cc: linux-wimax@intel.com
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/net/wimax/i2400m/tx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wimax/i2400m/tx.c b/drivers/net/wimax/i2400m/tx.c
index f20886ade1cc..59c70d928249 100644
--- a/drivers/net/wimax/i2400m/tx.c
+++ b/drivers/net/wimax/i2400m/tx.c
@@ -655,8 +655,7 @@ void i2400m_tx_close(struct i2400m *i2400m)
 	padding = aligned_size - tx_msg_moved->size;
 	if (padding > 0) {
 		pad_buf = i2400m_tx_fifo_push(i2400m, padding, 0, 0);
-		if (unlikely(WARN_ON(pad_buf == NULL
-				     || pad_buf == TAIL_FULL))) {
+		if (WARN_ON(!pad_buf || pad_buf == TAIL_FULL)) {
 			/* This should not happen -- append should verify
 			 * there is always space left at least to append
 			 * tx_block_size */
-- 
2.17.1

^ permalink raw reply related

* [PATCH] freescale: ethernet: remove unnecessary unlikely()
From: Igor Stoppa @ 2018-09-07 17:23 UTC (permalink / raw)
  To: Madalin Bucur
  Cc: igor.stoppa, Igor Stoppa, David S. Miller, netdev, linux-kernel

Both WARN_ON() and WARN_ONCE() already contain an unlikely(), so it's not
necessary to wrap it into another.

Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>
Cc: Madalin Bucur <madalin.bucur@nxp.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 65a22cd9aef2..783134f1b779 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -1280,7 +1280,7 @@ static int dpaa_bman_release(const struct dpaa_bp *dpaa_bp,
 
 	err = bman_release(dpaa_bp->pool, bmb, cnt);
 	/* Should never occur, address anyway to avoid leaking the buffers */
-	if (unlikely(WARN_ON(err)) && dpaa_bp->free_buf_cb)
+	if (WARN_ON(err) && dpaa_bp->free_buf_cb)
 		while (cnt-- > 0)
 			dpaa_bp->free_buf_cb(dpaa_bp, &bmb[cnt]);
 
@@ -1704,10 +1704,8 @@ static struct sk_buff *contig_fd_to_skb(const struct dpaa_priv *priv,
 
 	skb = build_skb(vaddr, dpaa_bp->size +
 			SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
-	if (unlikely(!skb)) {
-		WARN_ONCE(1, "Build skb failure on Rx\n");
+	if (WARN_ONCE(!skb, "Build skb failure on Rx\n"))
 		goto free_buffer;
-	}
 	WARN_ON(fd_off != priv->rx_headroom);
 	skb_reserve(skb, fd_off);
 	skb_put(skb, qm_fd_get_length(fd));
@@ -1770,7 +1768,7 @@ static struct sk_buff *sg_fd_to_skb(const struct dpaa_priv *priv,
 			sz = dpaa_bp->size +
 				SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
 			skb = build_skb(sg_vaddr, sz);
-			if (WARN_ON(unlikely(!skb)))
+			if (WARN_ON(!skb))
 				goto free_buffers;
 
 			skb->ip_summed = rx_csum_offload(priv, fd);
-- 
2.17.1

^ permalink raw reply related

* [PATCH] ethernet: hnae: add unlikely() to assert()
From: Igor Stoppa @ 2018-09-07 17:26 UTC (permalink / raw)
  To: huangdaode
  Cc: igor.stoppa, Igor Stoppa, Yisen Zhuang, Salil Mehta,
	David S. Miller, netdev, linux-kernel

The assert() condition is likely to be true.

Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>
Cc: huangdaode <huangdaode@hisilicon.com>
Cc: Yisen Zhuang <yisen.zhuang@huawei.com>
Cc: Salil Mehta <salil.mehta@huawei.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/net/ethernet/hisilicon/hns/hnae.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hnae.h b/drivers/net/ethernet/hisilicon/hns/hnae.h
index 08a750fb60c4..bd3c180a3fe9 100644
--- a/drivers/net/ethernet/hisilicon/hns/hnae.h
+++ b/drivers/net/ethernet/hisilicon/hns/hnae.h
@@ -47,7 +47,7 @@
 #ifndef assert
 #define assert(expr) \
 do { \
-	if (!(expr)) { \
+	if (unlikely(!(expr))) { \
 		pr_err("Assertion failed! %s, %s, %s, line %d\n", \
 			   #expr, __FILE__, __func__, __LINE__); \
 	} \
-- 
2.17.1

^ permalink raw reply related

* [PATCH] net/core/filter: fix unused-variable warning
From: Anders Roxell @ 2018-09-07 12:50 UTC (permalink / raw)
  To: davem, ast, daniel, tehnerd; +Cc: netdev, linux-kernel, Anders Roxell

Building with CONFIG_INET=n will show the warning below:
net/core/filter.c: In function ‘____bpf_getsockopt’:
net/core/filter.c:4048:19: warning: unused variable ‘tp’ [-Wunused-variable]
  struct tcp_sock *tp;
                   ^~
net/core/filter.c:4046:31: warning: unused variable ‘icsk’ [-Wunused-variable]
  struct inet_connection_sock *icsk;
                               ^~~~
Move the variable declarations inside the {} block where they are used.

Fixes: 1e215300f138 ("bpf: add TCP_SAVE_SYN/TCP_SAVED_SYN options for bpf_(set|get)sockopt")
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
 net/core/filter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index d301134bca3a..0ae7185b2207 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4043,14 +4043,14 @@ static const struct bpf_func_proto bpf_setsockopt_proto = {
 BPF_CALL_5(bpf_getsockopt, struct bpf_sock_ops_kern *, bpf_sock,
 	   int, level, int, optname, char *, optval, int, optlen)
 {
-	struct inet_connection_sock *icsk;
 	struct sock *sk = bpf_sock->sk;
-	struct tcp_sock *tp;
 
 	if (!sk_fullsock(sk))
 		goto err_clear;
 #ifdef CONFIG_INET
 	if (level == SOL_TCP && sk->sk_prot->getsockopt == tcp_getsockopt) {
+		struct inet_connection_sock *icsk;
+		struct tcp_sock *tp;
 		switch (optname) {
 		case TCP_CONGESTION:
 			icsk = inet_csk(sk);
-- 
2.18.0

^ permalink raw reply related

* Re: linux-next: build failure after merge of the net-next tree
From: David Miller @ 2018-09-07 17:31 UTC (permalink / raw)
  To: jacob.e.keller
  Cc: sfr, netdev, linux-next, linux-kernel, jeffrey.t.kirsher,
	andrewx.bowers
In-Reply-To: <02874ECE860811409154E81DA85FBB5884C7B651@ORSMSX115.amr.corp.intel.com>

From: "Keller, Jacob E" <jacob.e.keller@intel.com>
Date: Fri, 7 Sep 2018 15:30:42 +0000

> There's some discussion about this going on in the intel-wired-lan
> mailing list.

I really want to see a pull request in my inbox fixing this by the end
of today or I'll apply a fix directly at my discretion.

^ permalink raw reply

* Re: [PATCH] ethernet:netronome:nfp:move spin_lock_bh to spin_lock in tasklet
From: David Miller @ 2018-09-07 17:35 UTC (permalink / raw)
  To: jakub.kicinski
  Cc: hangdianqj, dirk.vandermerwe, daniel, quentin.monnet, oss-drivers,
	netdev, linux-kernel
In-Reply-To: <20180907183636.48a681f4@cakuba>

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Fri, 7 Sep 2018 18:36:36 +0200

> On Fri,  7 Sep 2018 09:21:17 -0700, jun qian wrote:
>> As you are already in a tasklet, it is unnecessary to call spin_lock_bh.
>> 
>> Signed-off-by: jun qian <hangdianqj@163.com>
> 
> FWIW you should put spaces after the colons.  It's generally a good
> practice to look at the prefix previous authors used for a given piece
> of code with 
> 
> git log -- $file_path
> 
> This would be a better subject:
> 
> nfp: replace spin_lock_bh with spin_lock in tasklet callback

Yes, please fix your subject styling.

^ permalink raw reply

* Re: [PATCH] nfp: replace spin_lock_bh with spin_lock in tasklet callback
From: Jakub Kicinski @ 2018-09-07 17:42 UTC (permalink / raw)
  To: jun qian
  Cc: Dirk van der Merwe, Daniel Borkmann, Quentin Monnet, oss-drivers,
	netdev, linux-kernel
In-Reply-To: <20180907170109.48150-1-hangdianqj@163.com>

On Fri,  7 Sep 2018 10:01:09 -0700, jun qian wrote:
> As you are already in a tasklet, it is unnecessary to call spin_lock_bh.
> 
> Signed-off-by: jun qian <hangdianqj@163.com>

Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>

^ permalink raw reply

* [PATCH net-next] net: dsa: Expose tagging protocol to user-space
From: Florian Fainelli @ 2018-09-07 18:09 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S. Miller,
	open list

There is no way for user-space to know what a given DSA network device's
tagging protocol is. Expose this information through a dsa/tagging
attribute which reflects the tagging protocol currently in use.

This is helpful for configuration (e.g: none behaves dramatically
different wrt. bridges) as well as for packet capture tools when there
is not a proper Ethernet type available.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 Documentation/ABI/testing/sysfs-class-net-dsa |  7 +++
 net/dsa/dsa.c                                 | 43 +++++++++++++++++++
 net/dsa/dsa_priv.h                            |  1 +
 net/dsa/slave.c                               | 28 ++++++++++++
 4 files changed, 79 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-net-dsa

diff --git a/Documentation/ABI/testing/sysfs-class-net-dsa b/Documentation/ABI/testing/sysfs-class-net-dsa
new file mode 100644
index 000000000000..f240221e071e
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-net-dsa
@@ -0,0 +1,7 @@
+What:		/sys/class/net/<iface>/tagging
+Date:		August 2018
+KernelVersion:	4.20
+Contact:	netdev@vger.kernel.org
+Description:
+		String indicating the type of tagging protocol used by the
+		DSA slave network device.
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 9f3209ff7ffd..45f70859f550 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -70,6 +70,49 @@ const struct dsa_device_ops *dsa_device_ops[DSA_TAG_LAST] = {
 	[DSA_TAG_PROTO_NONE] = &none_ops,
 };
 
+const char *dsa_tag_protocol_to_str(const struct dsa_device_ops *ops)
+{
+	const char *protocol_name[DSA_TAG_LAST] = {
+#ifdef CONFIG_NET_DSA_TAG_BRCM
+		[DSA_TAG_PROTO_BRCM] = "brcm",
+#endif
+#ifdef CONFIG_NET_DSA_TAG_BRCM_PREPEND
+		[DSA_TAG_PROTO_BRCM_PREPEND] = "brcm-prepend",
+#endif
+#ifdef CONFIG_NET_DSA_TAG_DSA
+		[DSA_TAG_PROTO_DSA] = "dsa",
+#endif
+#ifdef CONFIG_NET_DSA_TAG_EDSA
+		[DSA_TAG_PROTO_EDSA] = "edsa",
+#endif
+#ifdef CONFIG_NET_DSA_TAG_KSZ
+		[DSA_TAG_PROTO_KSZ] = "ksz",
+#endif
+#ifdef CONFIG_NET_DSA_TAG_LAN9303
+		[DSA_TAG_PROTO_LAN9303] = "lan9303",
+#endif
+#ifdef CONFIG_NET_DSA_TAG_MTK
+		[DSA_TAG_PROTO_MTK] = "mtk",
+#endif
+#ifdef CONFIG_NET_DSA_TAG_QCA
+		[DSA_TAG_PROTO_QCA] = "qca",
+#endif
+#ifdef CONFIG_NET_DSA_TAG_TRAILER
+		[DSA_TAG_PROTO_TRAILER] = "trailer",
+#endif
+		[DSA_TAG_PROTO_NONE] = "none",
+	};
+	unsigned int i;
+
+	BUILD_BUG_ON(ARRAY_SIZE(protocol_name) != DSA_TAG_LAST);
+
+	for (i = 0; i < ARRAY_SIZE(dsa_device_ops); i++)
+		if (ops == dsa_device_ops[i])
+			return protocol_name[i];
+
+	return protocol_name[DSA_TAG_PROTO_NONE];
+};
+
 const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol)
 {
 	const struct dsa_device_ops *ops;
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 3964c6f7a7c0..2868b5bb7e7d 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -86,6 +86,7 @@ struct dsa_slave_priv {
 /* dsa.c */
 const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol);
 bool dsa_schedule_work(struct work_struct *work);
+const char *dsa_tag_protocol_to_str(const struct dsa_device_ops *ops);
 
 /* legacy.c */
 #if IS_ENABLED(CONFIG_NET_DSA_LEGACY)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 1c45c1d6d241..3f840b6eea69 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1058,6 +1058,27 @@ static struct device_type dsa_type = {
 	.name	= "dsa",
 };
 
+static ssize_t tagging_show(struct device *d, struct device_attribute *attr,
+			    char *buf)
+{
+	struct net_device *dev = to_net_dev(d);
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+
+	return sprintf(buf, "%s\n",
+		       dsa_tag_protocol_to_str(dp->cpu_dp->tag_ops));
+}
+static DEVICE_ATTR_RO(tagging);
+
+static struct attribute *dsa_slave_attrs[] = {
+	&dev_attr_tagging.attr,
+	NULL
+};
+
+static const struct attribute_group dsa_group = {
+	.name	= "dsa",
+	.attrs	= dsa_slave_attrs,
+};
+
 static void dsa_slave_phylink_validate(struct net_device *dev,
 				       unsigned long *supported,
 				       struct phylink_link_state *state)
@@ -1353,8 +1374,14 @@ int dsa_slave_create(struct dsa_port *port)
 		goto out_phy;
 	}
 
+	ret = sysfs_create_group(&slave_dev->dev.kobj, &dsa_group);
+	if (ret)
+		goto out_unreg;
+
 	return 0;
 
+out_unreg:
+	unregister_netdev(slave_dev);
 out_phy:
 	rtnl_lock();
 	phylink_disconnect_phy(p->dp->pl);
@@ -1378,6 +1405,7 @@ void dsa_slave_destroy(struct net_device *slave_dev)
 	rtnl_unlock();
 
 	dsa_slave_notify(slave_dev, DSA_PORT_UNREGISTER);
+	sysfs_remove_group(&slave_dev->dev.kobj, &dsa_group);
 	unregister_netdev(slave_dev);
 	phylink_destroy(dp->pl);
 	free_percpu(p->stats64);
-- 
2.17.1

^ permalink raw reply related

* [PATCH] r8169: set TxConfig register after TX / RX is enabled, just like RxConfig
From: Maciej S. Szmigiero @ 2018-09-07 18:15 UTC (permalink / raw)
  To: Realtek linux nic maintainers, David S. Miller
  Cc: netdev, linux-kernel, Azat Khuzhin, Heiner Kallweit

Commit 3559d81e76bf ("r8169: simplify rtl_hw_start_8169") changed order of
two register writes:
1) Caused RxConfig to be written before TX / RX is enabled,
2) Caused TxConfig to be written before TX / RX is enabled.

At least on XIDs 10000000 ("RTL8169sb/8110sb") and
18000000 ("RTL8169sc/8110sc") such writes are ignored by the chip, leaving
values in these registers intact.

Change 1) was reverted by
commit 05212ba8132b42 ("r8169: set RxConfig after tx/rx is enabled for RTL8169sb/8110sb devices"),
however change 2) wasn't.

In practice, this caused TxConfig's "InterFrameGap time" and "Max DMA Burst
Size per Tx DMA Burst" bits to be zero dramatically reducing TX performance
(in my tests it dropped from around 500Mbps to around 50Mbps).

This patch fixes the issue by moving TxConfig register write a bit later in
the code so it happens after TX / RX is already enabled.

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Fixes: 05212ba8132b42 ("r8169: set RxConfig after tx/rx is enabled for RTL8169sb/8110sb devices")
---
"Fixes" tag points to the RxConfig fix instead of the actual commit that
introduced this regression to maintain patch ordering since the RxConfig fix
partially affects the same code lines as this fix.

 drivers/net/ethernet/realtek/r8169.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index b935a18358cb..2ade3a27d7f1 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -4634,13 +4634,13 @@ static void rtl_hw_start(struct  rtl8169_private *tp)
 
 	rtl_set_rx_max_size(tp);
 	rtl_set_rx_tx_desc_registers(tp);
-	rtl_set_tx_config_registers(tp);
 	RTL_W8(tp, Cfg9346, Cfg9346_Lock);
 
 	/* Initially a 10 us delay. Turned it into a PCI commit. - FR */
 	RTL_R8(tp, IntrMask);
 	RTL_W8(tp, ChipCmd, CmdTxEnb | CmdRxEnb);
 	rtl_init_rxcfg(tp);
+	rtl_set_tx_config_registers(tp);
 
 	rtl_set_rx_mode(tp->dev);
 	/* no early-rx interrupts */
-- 
2.17.0

^ permalink raw reply related

* [PATCH net-next v2] net: sched: change tcf_del_walker() to take idrinfo->lock
From: Vlad Buslov @ 2018-09-07 13:51 UTC (permalink / raw)
  To: netdev, xiyou.wangcong; +Cc: jhs, jiri, davem, Vlad Buslov
In-Reply-To: <CAM_iQpVhTeKMQmt55zuZ3vTgtrMajzDo3H-3Nw7+UdrGJCJDrA@mail.gmail.com>

Action API was changed to work with actions and action_idr in concurrency
safe manner, however tcf_del_walker() still uses actions without taking a
reference or idrinfo->lock first, and deletes them directly, disregarding
possible concurrent delete.

Add tc_action_wq workqueue to action API. Implement
tcf_idr_release_unsafe() that assumes external synchronization by caller
and delays blocking action cleanup part to tc_action_wq workqueue. Extend
tcf_action_cleanup() with 'async' argument to indicate that function should
free action asynchronously.

Change tcf_del_walker() to take idrinfo->lock while iterating over actions
and use new tcf_idr_release_unsafe() to release them while holding the
lock.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
 include/net/act_api.h |  1 +
 net/sched/act_api.c   | 73 ++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 65 insertions(+), 9 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index c6f195b3c706..4c5117bc4afb 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -38,6 +38,7 @@ struct tc_action {
 	struct gnet_stats_queue __percpu *cpu_qstats;
 	struct tc_cookie	__rcu *act_cookie;
 	struct tcf_chain	*goto_chain;
+	struct work_struct	work;
 };
 #define tcf_index	common.tcfa_index
 #define tcf_refcnt	common.tcfa_refcnt
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 6f118d62c731..4ad9062c34b3 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -90,13 +90,38 @@ static void free_tcf(struct tc_action *p)
 	kfree(p);
 }
 
-static void tcf_action_cleanup(struct tc_action *p)
+static void tcf_action_free(struct tc_action *p)
+{
+	gen_kill_estimator(&p->tcfa_rate_est);
+	free_tcf(p);
+}
+
+static void tcf_action_free_work(struct work_struct *work)
+{
+	struct tc_action *p = container_of(work,
+					   struct tc_action,
+					   work);
+
+	tcf_action_free(p);
+}
+
+static struct workqueue_struct *tc_action_wq;
+
+static bool tcf_action_queue_work(struct work_struct *work, work_func_t func)
+{
+	INIT_WORK(work, func);
+	return queue_work(tc_action_wq, work);
+}
+
+static void tcf_action_cleanup(struct tc_action *p, bool async)
 {
 	if (p->ops->cleanup)
 		p->ops->cleanup(p);
 
-	gen_kill_estimator(&p->tcfa_rate_est);
-	free_tcf(p);
+	if (async)
+		tcf_action_queue_work(&p->work, tcf_action_free_work);
+	else
+		tcf_action_free(p);
 }
 
 static int __tcf_action_put(struct tc_action *p, bool bind)
@@ -109,7 +134,7 @@ static int __tcf_action_put(struct tc_action *p, bool bind)
 		idr_remove(&idrinfo->action_idr, p->tcfa_index);
 		spin_unlock(&idrinfo->lock);
 
-		tcf_action_cleanup(p);
+		tcf_action_cleanup(p, false);
 		return 1;
 	}
 
@@ -147,6 +172,24 @@ int __tcf_idr_release(struct tc_action *p, bool bind, bool strict)
 }
 EXPORT_SYMBOL(__tcf_idr_release);
 
+/* Release idr without obtaining idrinfo->lock. Caller must prevent any
+ * concurrent modifications of idrinfo->action_idr!
+ */
+
+static int tcf_idr_release_unsafe(struct tc_action *p)
+{
+	if (atomic_read(&p->tcfa_bindcnt) > 0)
+		return -EPERM;
+
+	if (refcount_dec_and_test(&p->tcfa_refcnt)) {
+		idr_remove(&p->idrinfo->action_idr, p->tcfa_index);
+		tcf_action_cleanup(p, true);
+		return ACT_P_DELETED;
+	}
+
+	return 0;
+}
+
 static size_t tcf_action_shared_attrs_size(const struct tc_action *act)
 {
 	struct tc_cookie *act_cookie;
@@ -262,20 +305,25 @@ static int tcf_del_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
 	if (nla_put_string(skb, TCA_KIND, ops->kind))
 		goto nla_put_failure;
 
+	spin_lock(&idrinfo->lock);
 	idr_for_each_entry_ul(idr, p, id) {
-		ret = __tcf_idr_release(p, false, true);
+		ret = tcf_idr_release_unsafe(p);
 		if (ret == ACT_P_DELETED) {
 			module_put(ops->owner);
 			n_i++;
 		} else if (ret < 0) {
-			goto nla_put_failure;
+			goto nla_put_failure_locked;
 		}
 	}
+	spin_unlock(&idrinfo->lock);
+
 	if (nla_put_u32(skb, TCA_FCNT, n_i))
 		goto nla_put_failure;
 	nla_nest_end(skb, nest);
 
 	return n_i;
+nla_put_failure_locked:
+	spin_unlock(&idrinfo->lock);
 nla_put_failure:
 	nla_nest_cancel(skb, nest);
 	return ret;
@@ -341,7 +389,7 @@ static int tcf_idr_delete_index(struct tcf_idrinfo *idrinfo, u32 index)
 						p->tcfa_index));
 			spin_unlock(&idrinfo->lock);
 
-			tcf_action_cleanup(p);
+			tcf_action_cleanup(p, false);
 			module_put(owner);
 			return 0;
 		}
@@ -1713,16 +1761,23 @@ static int __init tc_action_init(void)
 {
 	int err;
 
+	tc_action_wq = alloc_ordered_workqueue("tc_action_workqueue", 0);
+	if (!tc_action_wq)
+		return -ENOMEM;
+
 	err = register_pernet_subsys(&tcf_action_net_ops);
 	if (err)
-		return err;
+		goto err_register_pernet_subsys;
 
 	rtnl_register(PF_UNSPEC, RTM_NEWACTION, tc_ctl_action, NULL, 0);
 	rtnl_register(PF_UNSPEC, RTM_DELACTION, tc_ctl_action, NULL, 0);
 	rtnl_register(PF_UNSPEC, RTM_GETACTION, tc_ctl_action, tc_dump_action,
 		      0);
+	return err;
 
-	return 0;
+err_register_pernet_subsys:
+	destroy_workqueue(tc_action_wq);
+	return err;
 }
 
 subsys_initcall(tc_action_init);
-- 
2.7.5

^ permalink raw reply related

* [iproute2 PATCH] bridge: Correct json output
From: Tobias Jungel @ 2018-09-07 13:42 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

The current implementation adds configured vlans as "vlan": [ ... ] into
an array. This is malformed json and fails to be parsed. This patch
creates an object to include this key value pair.

Test with:

ip l a type bridge
./bridge/bridge -j vlan | jq

fixes c7c1a1ef5

Signed-off-by: Tobias Jungel <tobias.jungel@bisdn.de>
---
 bridge/vlan.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/bridge/vlan.c b/bridge/vlan.c
index 19a36b80..9376b985 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -632,6 +632,7 @@ void print_vlan_info(FILE *fp, struct rtattr *tb)
 	if (!is_json_context())
 		fprintf(fp, "%s", _SL_);
 
+	open_json_object(NULL);
 	open_json_array(PRINT_JSON, "vlan");
 
 	for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
@@ -659,6 +660,7 @@ void print_vlan_info(FILE *fp, struct rtattr *tb)
 	}
 
 	close_json_array(PRINT_ANY, "\n");
+	close_json_object();
 }
 
 int do_vlan(int argc, char **argv)

^ permalink raw reply related

* [PATCH net-next v2] net: sched: cls_flower: dump offload count value
From: Vlad Buslov @ 2018-09-07 14:22 UTC (permalink / raw)
  To: netdev; +Cc: jakub.kicinski, jhs, xiyou.wangcong, jiri, davem, Vlad Buslov

Change flower in_hw_count type to fixed-size u32 and dump it as
TCA_FLOWER_IN_HW_COUNT. This change is necessary to properly test shared
blocks and re-offload functionality.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/sch_generic.h    | 2 +-
 include/uapi/linux/pkt_cls.h | 2 ++
 net/sched/cls_flower.c       | 5 ++++-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index a6d00093f35e..d68ac55539a5 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -362,7 +362,7 @@ static inline void tcf_block_offload_dec(struct tcf_block *block, u32 *flags)
 }
 
 static inline void
-tc_cls_offload_cnt_update(struct tcf_block *block, unsigned int *cnt,
+tc_cls_offload_cnt_update(struct tcf_block *block, u32 *cnt,
 			  u32 *flags, bool add)
 {
 	if (add) {
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index be382fb0592d..401d0c1e612d 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -483,6 +483,8 @@ enum {
 	TCA_FLOWER_KEY_ENC_OPTS,
 	TCA_FLOWER_KEY_ENC_OPTS_MASK,
 
+	TCA_FLOWER_IN_HW_COUNT,
+
 	__TCA_FLOWER_MAX,
 };
 
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 6fd9bdd93796..4b8dd37dd4f8 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -98,7 +98,7 @@ struct cls_fl_filter {
 	struct list_head list;
 	u32 handle;
 	u32 flags;
-	unsigned int in_hw_count;
+	u32 in_hw_count;
 	struct rcu_work rwork;
 	struct net_device *hw_dev;
 };
@@ -1880,6 +1880,9 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh,
 	if (f->flags && nla_put_u32(skb, TCA_FLOWER_FLAGS, f->flags))
 		goto nla_put_failure;
 
+	if (nla_put_u32(skb, TCA_FLOWER_IN_HW_COUNT, f->in_hw_count))
+		goto nla_put_failure;
+
 	if (tcf_exts_dump(skb, &f->exts))
 		goto nla_put_failure;
 
-- 
2.7.5

^ permalink raw reply related

* [PATCH] mt76: another missing 'select' in Kconfig
From: valdis.kletnieks @ 2018-09-07 19:08 UTC (permalink / raw)
  To: Kalle Valo, David S. Miller, Lorenzo Bianconi
  Cc: netdev, linux-kernel, linux-wireless

commit b40b15e1521f7764ea8c68d5a00ecc971b673d21
Author: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Date:   Tue Jul 31 10:09:19 2018 +0200

    mt76: add usb support to mt76 layer

add a new mt76-usb.c for MT76x2U USB devices, but failed to wire
it up for MT76x0U USB devices.

Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
---
diff --git a/drivers/net/wireless/mediatek/mt76/Kconfig b/drivers/net/wireless/mediatek/mt76/Kconfig
index 6a270e759006..cec977b81305 100644
--- a/drivers/net/wireless/mediatek/mt76/Kconfig
+++ b/drivers/net/wireless/mediatek/mt76/Kconfig
@@ -17,6 +17,7 @@ config MT76x2_COMMON
 config MT76x0U
 	tristate "MediaTek MT76x0U (USB) support"
 	select MT76_CORE
+	select MT76_USB
 	depends on MAC80211
 	depends on USB
 	select MT76x02_LIB

^ permalink raw reply related

* Re: [PATCH net-next v2] net: sched: cls_flower: dump offload count value
From: Jakub Kicinski @ 2018-09-07 14:49 UTC (permalink / raw)
  To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, jiri, davem
In-Reply-To: <1536330141-10354-1-git-send-email-vladbu@mellanox.com>

On Fri,  7 Sep 2018 17:22:21 +0300, Vlad Buslov wrote:
> Change flower in_hw_count type to fixed-size u32 and dump it as
> TCA_FLOWER_IN_HW_COUNT. This change is necessary to properly test shared
> blocks and re-offload functionality.
> 
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
> Acked-by: Jiri Pirko <jiri@mellanox.com>

LGTM, thanks for the respin!

^ permalink raw reply

* Allow bpf_perf_event_output to access packet data
From: Lorenz Bauer @ 2018-09-07 14:56 UTC (permalink / raw)
  To: netdev

Re-sent due to HTML e-mail mess up, apologies.

---------- Forwarded message ----------
From: Lorenz Bauer <lmb@cloudflare.com>
Date: 7 September 2018 at 15:53
Subject: Allow bpf_perf_event_output to access packet data
To: netdev@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>


Hello list,

I'm attempting to use bpf_perf_event_output to do packet sampling from XDP.

The code basically runs before our other XDP code, does a
perf_event_output with the full packet (for now) and then tail calls
into DDoS mitigation, etc.

I've just discovered that perf_event_output isn't allowed to access
packet data by the verifier. Is this something that could be allowed?

Best
Lorenz

-- 
Lorenz Bauer  |  Systems Engineer
25 Lavington St., London SE1 0NZ

www.cloudflare.com

^ permalink raw reply

* [PATCH 0/5] introduce setbits32/clrbits32/clrsetbits32/setbits64/clrbits64/clrsetbits64 functions
From: Corentin Labbe @ 2018-09-07 19:41 UTC (permalink / raw)
  To: Gilles.Muller, Julia.Lawall, agust, alexandre.torgue, alistair,
	benh, carlo, davem, galak, joabreu, khilman, maxime.ripard,
	michal.lkml, mpe, mporter, nicolas.palix, oss, paulus,
	peppe.cavallaro, tj, vitb, wens
  Cc: cocci, linux-amlogic, linux-arm-kernel, linux-ide, linux-kernel,
	linuxppc-dev, netdev, linux-sunxi, Corentin Labbe

Hello

This patchset adds a new set of functions which are open-coded in lot of
place.
Basicly the pattern is always the same, "read, modify a bit, write"
some driver already have thoses pattern them as functions. (like ahci_sunxi.c or dwmac-meson8b)

The first patch rename some powerpc funtions which already use the same name (xxxbits32)
but with only bigendian values.

The second patch adds the header.
The third patch is an ugly try to implement a coccinelle semantic patch to
find all place where xxxbits function could be used.
Probably this spatch could be better written and I didnt found an easy way to add the "linux/setbits" header.

The two last patch are example of convertion of two drivers.
Thoses patchs give an example of the reduction of code won by using xxxbits32.

This patchset is tested with the ahci_sunxi and dwmac-sun8i drivers.

Regards

Corentin Labbe (5):
  powerpc: rename setbits32/clrbits32 to setbits32_be/clrbits32_be
  include: add
    setbits32/clrbits32/clrsetbits32/setbits64/clrbits64/clrsetbits64 in
    linux/setbits.h
  coccinelle: add xxxsetbitsXX converting spatch
  net: ethernet: stmmac: use xxxsetbits32
  ata: ahci_sunxi: use xxxsetbits32 functions

 arch/powerpc/include/asm/fsl_lbc.h                 |   2 +-
 arch/powerpc/include/asm/io.h                      |   5 +-
 arch/powerpc/platforms/44x/canyonlands.c           |   4 +-
 arch/powerpc/platforms/4xx/gpio.c                  |  28 +-
 arch/powerpc/platforms/512x/pdm360ng.c             |   6 +-
 arch/powerpc/platforms/52xx/mpc52xx_common.c       |   6 +-
 arch/powerpc/platforms/52xx/mpc52xx_gpt.c          |  10 +-
 arch/powerpc/platforms/82xx/ep8248e.c              |   2 +-
 arch/powerpc/platforms/82xx/km82xx.c               |   6 +-
 arch/powerpc/platforms/82xx/mpc8272_ads.c          |  10 +-
 arch/powerpc/platforms/82xx/pq2.c                  |   2 +-
 arch/powerpc/platforms/82xx/pq2ads-pci-pic.c       |   4 +-
 arch/powerpc/platforms/82xx/pq2fads.c              |  10 +-
 arch/powerpc/platforms/83xx/km83xx.c               |   6 +-
 arch/powerpc/platforms/83xx/mpc836x_mds.c          |   2 +-
 arch/powerpc/platforms/85xx/mpc85xx_mds.c          |   2 +-
 arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c       |   4 +-
 arch/powerpc/platforms/85xx/mpc85xx_rdb.c          |   2 +-
 arch/powerpc/platforms/85xx/p1022_ds.c             |   4 +-
 arch/powerpc/platforms/85xx/p1022_rdk.c            |   4 +-
 arch/powerpc/platforms/85xx/t1042rdb_diu.c         |   4 +-
 arch/powerpc/platforms/85xx/twr_p102x.c            |   2 +-
 arch/powerpc/platforms/86xx/mpc8610_hpcd.c         |   4 +-
 arch/powerpc/platforms/8xx/adder875.c              |   2 +-
 arch/powerpc/platforms/8xx/m8xx_setup.c            |   4 +-
 arch/powerpc/platforms/8xx/mpc86xads_setup.c       |   4 +-
 arch/powerpc/platforms/8xx/mpc885ads_setup.c       |  28 +-
 arch/powerpc/platforms/embedded6xx/flipper-pic.c   |   6 +-
 arch/powerpc/platforms/embedded6xx/hlwd-pic.c      |   8 +-
 arch/powerpc/platforms/embedded6xx/wii.c           |  10 +-
 arch/powerpc/sysdev/cpm1.c                         |  26 +-
 arch/powerpc/sysdev/cpm2.c                         |  16 +-
 arch/powerpc/sysdev/cpm_common.c                   |   4 +-
 arch/powerpc/sysdev/fsl_85xx_l2ctlr.c              |   8 +-
 arch/powerpc/sysdev/fsl_lbc.c                      |   2 +-
 arch/powerpc/sysdev/fsl_pci.c                      |   8 +-
 arch/powerpc/sysdev/fsl_pmc.c                      |   2 +-
 arch/powerpc/sysdev/fsl_rcpm.c                     |  74 ++--
 arch/powerpc/sysdev/fsl_rio.c                      |   4 +-
 arch/powerpc/sysdev/fsl_rmu.c                      |   8 +-
 arch/powerpc/sysdev/mpic_timer.c                   |  12 +-
 drivers/ata/ahci_sunxi.c                           |  51 +--
 .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    |  54 +--
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c  |  55 +--
 .../net/ethernet/stmicro/stmmac/dwmac1000_core.c   |  21 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c  |  51 +--
 drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c   |  13 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c   |  42 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac5.c       |  11 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c    |  17 +-
 .../net/ethernet/stmicro/stmmac/dwxgmac2_core.c    |  30 +-
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c |  69 +---
 .../net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c  |  11 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c  |   7 +-
 include/linux/setbits.h                            |  55 +++
 scripts/coccinelle/misc/setbits.cocci              | 423 +++++++++++++++++++++
 56 files changed, 776 insertions(+), 489 deletions(-)
 create mode 100644 include/linux/setbits.h
 create mode 100644 scripts/coccinelle/misc/setbits.cocci

-- 
2.16.4

^ permalink raw reply

* [PATCH 4/5] net: ethernet: stmmac: use xxxsetbits32
From: Corentin Labbe @ 2018-09-07 19:41 UTC (permalink / raw)
  To: Gilles.Muller, Julia.Lawall, agust, alexandre.torgue, alistair,
	benh, carlo, davem, galak, joabreu, khilman, maxime.ripard,
	michal.lkml, mpe, mporter, nicolas.palix, oss, paulus,
	peppe.cavallaro, tj, vitb, wens
  Cc: cocci, linux-amlogic, linux-arm-kernel, linux-ide, linux-kernel,
	linuxppc-dev, netdev, linux-sunxi, Corentin Labbe
In-Reply-To: <1536349307-20714-1-git-send-email-clabbe@baylibre.com>

This patch convert stmmac driver to use all xxxsetbits32 functions.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 54 +++++++----------
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c  | 55 ++++-------------
 .../net/ethernet/stmicro/stmmac/dwmac1000_core.c   | 21 +++----
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c  | 51 ++++++----------
 drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c   | 13 ++--
 drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c   | 42 +++----------
 drivers/net/ethernet/stmicro/stmmac/dwmac5.c       | 11 +---
 drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c    | 17 ++----
 .../net/ethernet/stmicro/stmmac/dwxgmac2_core.c    | 30 ++++------
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 69 +++++-----------------
 .../net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c  | 11 +---
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c  |  7 +--
 12 files changed, 108 insertions(+), 273 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index c5979569fd60..035a2ab7b479 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -22,6 +22,7 @@
 #include <linux/of_net.h>
 #include <linux/mfd/syscon.h>
 #include <linux/platform_device.h>
+#include <linux/setbits.h>
 #include <linux/stmmac.h>
 
 #include "stmmac_platform.h"
@@ -75,18 +76,6 @@ struct meson8b_dwmac_clk_configs {
 	struct clk_gate		rgmii_tx_en;
 };
 
-static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
-				    u32 mask, u32 value)
-{
-	u32 data;
-
-	data = readl(dwmac->regs + reg);
-	data &= ~mask;
-	data |= (value & mask);
-
-	writel(data, dwmac->regs + reg);
-}
-
 static struct clk *meson8b_dwmac_register_clk(struct meson8b_dwmac *dwmac,
 					      const char *name_suffix,
 					      const char **parent_names,
@@ -192,14 +181,12 @@ static int meson8b_set_phy_mode(struct meson8b_dwmac *dwmac)
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
 		/* enable RGMII mode */
-		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
-					PRG_ETH0_RGMII_MODE,
-					PRG_ETH0_RGMII_MODE);
+		clrsetbits32(dwmac->regs + PRG_ETH0, PRG_ETH0_RGMII_MODE,
+			     PRG_ETH0_RGMII_MODE);
 		break;
 	case PHY_INTERFACE_MODE_RMII:
 		/* disable RGMII mode -> enables RMII mode */
-		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
-					PRG_ETH0_RGMII_MODE, 0);
+		clrsetbits32(dwmac->regs + PRG_ETH0, PRG_ETH0_RGMII_MODE, 0);
 		break;
 	default:
 		dev_err(dwmac->dev, "fail to set phy-mode %s\n",
@@ -218,15 +205,15 @@ static int meson_axg_set_phy_mode(struct meson8b_dwmac *dwmac)
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
 		/* enable RGMII mode */
-		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
-					PRG_ETH0_EXT_PHY_MODE_MASK,
-					PRG_ETH0_EXT_RGMII_MODE);
+		clrsetbits32(dwmac->regs + PRG_ETH0,
+			     PRG_ETH0_EXT_PHY_MODE_MASK,
+			     PRG_ETH0_EXT_RGMII_MODE);
 		break;
 	case PHY_INTERFACE_MODE_RMII:
 		/* disable RGMII mode -> enables RMII mode */
-		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
-					PRG_ETH0_EXT_PHY_MODE_MASK,
-					PRG_ETH0_EXT_RMII_MODE);
+		clrsetbits32(dwmac->regs + PRG_ETH0,
+			     PRG_ETH0_EXT_PHY_MODE_MASK,
+			     PRG_ETH0_EXT_RMII_MODE);
 		break;
 	default:
 		dev_err(dwmac->dev, "fail to set phy-mode %s\n",
@@ -255,11 +242,11 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
 		/* only relevant for RMII mode -> disable in RGMII mode */
-		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
-					PRG_ETH0_INVERTED_RMII_CLK, 0);
+		clrsetbits32(dwmac->regs + PRG_ETH0,
+			     PRG_ETH0_INVERTED_RMII_CLK, 0);
 
-		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
-					tx_dly_val << PRG_ETH0_TXDLY_SHIFT);
+		clrsetbits32(dwmac->regs + PRG_ETH0, PRG_ETH0_TXDLY_MASK,
+			     tx_dly_val << PRG_ETH0_TXDLY_SHIFT);
 
 		/* Configure the 125MHz RGMII TX clock, the IP block changes
 		 * the output automatically (= without us having to configure
@@ -287,13 +274,12 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 
 	case PHY_INTERFACE_MODE_RMII:
 		/* invert internal clk_rmii_i to generate 25/2.5 tx_rx_clk */
-		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
-					PRG_ETH0_INVERTED_RMII_CLK,
-					PRG_ETH0_INVERTED_RMII_CLK);
+		clrsetbits32(dwmac->regs + PRG_ETH0,
+			     PRG_ETH0_INVERTED_RMII_CLK,
+			     PRG_ETH0_INVERTED_RMII_CLK);
 
 		/* TX clock delay cannot be configured in RMII mode */
-		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
-					0);
+		clrsetbits32(dwmac->regs + PRG_ETH0, PRG_ETH0_TXDLY_MASK, 0);
 
 		break;
 
@@ -304,8 +290,8 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 	}
 
 	/* enable TX_CLK and PHY_REF_CLK generator */
-	meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TX_AND_PHY_REF_CLK,
-				PRG_ETH0_TX_AND_PHY_REF_CLK);
+	clrsetbits32(dwmac->regs + PRG_ETH0, PRG_ETH0_TX_AND_PHY_REF_CLK,
+		     PRG_ETH0_TX_AND_PHY_REF_CLK);
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 0f660af01a4b..3c7f531feadf 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -27,6 +27,7 @@
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 #include <linux/regmap.h>
+#include <linux/setbits.h>
 #include <linux/stmmac.h>
 
 #include "stmmac.h"
@@ -342,50 +343,27 @@ static void sun8i_dwmac_disable_dma_irq(void __iomem *ioaddr, u32 chan)
 
 static void sun8i_dwmac_dma_start_tx(void __iomem *ioaddr, u32 chan)
 {
-	u32 v;
-
-	v = readl(ioaddr + EMAC_TX_CTL1);
-	v |= EMAC_TX_DMA_START;
-	v |= EMAC_TX_DMA_EN;
-	writel(v, ioaddr + EMAC_TX_CTL1);
+	setbits32(ioaddr + EMAC_TX_CTL1, EMAC_TX_DMA_START | EMAC_TX_DMA_EN);
 }
 
 static void sun8i_dwmac_enable_dma_transmission(void __iomem *ioaddr)
 {
-	u32 v;
-
-	v = readl(ioaddr + EMAC_TX_CTL1);
-	v |= EMAC_TX_DMA_START;
-	v |= EMAC_TX_DMA_EN;
-	writel(v, ioaddr + EMAC_TX_CTL1);
+	setbits32(ioaddr + EMAC_TX_CTL1, EMAC_TX_DMA_START | EMAC_TX_DMA_EN);
 }
 
 static void sun8i_dwmac_dma_stop_tx(void __iomem *ioaddr, u32 chan)
 {
-	u32 v;
-
-	v = readl(ioaddr + EMAC_TX_CTL1);
-	v &= ~EMAC_TX_DMA_EN;
-	writel(v, ioaddr + EMAC_TX_CTL1);
+	clrbits32(ioaddr + EMAC_TX_CTL1, EMAC_TX_DMA_EN);
 }
 
 static void sun8i_dwmac_dma_start_rx(void __iomem *ioaddr, u32 chan)
 {
-	u32 v;
-
-	v = readl(ioaddr + EMAC_RX_CTL1);
-	v |= EMAC_RX_DMA_START;
-	v |= EMAC_RX_DMA_EN;
-	writel(v, ioaddr + EMAC_RX_CTL1);
+	setbits32(ioaddr + EMAC_RX_CTL1, EMAC_RX_DMA_START | EMAC_RX_DMA_EN);
 }
 
 static void sun8i_dwmac_dma_stop_rx(void __iomem *ioaddr, u32 chan)
 {
-	u32 v;
-
-	v = readl(ioaddr + EMAC_RX_CTL1);
-	v &= ~EMAC_RX_DMA_EN;
-	writel(v, ioaddr + EMAC_RX_CTL1);
+	clrbits32(ioaddr + EMAC_RX_CTL1, EMAC_RX_DMA_EN);
 }
 
 static int sun8i_dwmac_dma_interrupt(void __iomem *ioaddr,
@@ -608,11 +586,8 @@ static void sun8i_dwmac_get_umac_addr(struct mac_device_info *hw,
 static int sun8i_dwmac_rx_ipc_enable(struct mac_device_info *hw)
 {
 	void __iomem *ioaddr = hw->pcsr;
-	u32 v;
 
-	v = readl(ioaddr + EMAC_RX_CTL0);
-	v |= EMAC_RX_DO_CRC;
-	writel(v, ioaddr + EMAC_RX_CTL0);
+	setbits32(ioaddr + EMAC_RX_CTL0, EMAC_RX_DO_CRC);
 
 	return 1;
 }
@@ -662,21 +637,16 @@ static void sun8i_dwmac_flow_ctrl(struct mac_device_info *hw,
 				  unsigned int pause_time, u32 tx_cnt)
 {
 	void __iomem *ioaddr = hw->pcsr;
-	u32 v;
 
-	v = readl(ioaddr + EMAC_RX_CTL0);
 	if (fc == FLOW_AUTO)
-		v |= EMAC_RX_FLOW_CTL_EN;
+		setbits32(ioaddr + EMAC_RX_CTL0, EMAC_RX_FLOW_CTL_EN);
 	else
-		v &= ~EMAC_RX_FLOW_CTL_EN;
-	writel(v, ioaddr + EMAC_RX_CTL0);
+		clrbits32(ioaddr + EMAC_RX_CTL0, EMAC_RX_FLOW_CTL_EN);
 
-	v = readl(ioaddr + EMAC_TX_FLOW_CTL);
 	if (fc == FLOW_AUTO)
-		v |= EMAC_TX_FLOW_CTL_EN;
+		setbits32(ioaddr + EMAC_TX_FLOW_CTL, EMAC_TX_FLOW_CTL_EN);
 	else
-		v &= ~EMAC_TX_FLOW_CTL_EN;
-	writel(v, ioaddr + EMAC_TX_FLOW_CTL);
+		clrbits32(ioaddr + EMAC_TX_FLOW_CTL, EMAC_TX_FLOW_CTL_EN);
 }
 
 static int sun8i_dwmac_reset(struct stmmac_priv *priv)
@@ -684,8 +654,7 @@ static int sun8i_dwmac_reset(struct stmmac_priv *priv)
 	u32 v;
 	int err;
 
-	v = readl(priv->ioaddr + EMAC_BASIC_CTL1);
-	writel(v | 0x01, priv->ioaddr + EMAC_BASIC_CTL1);
+	setbits32(priv->ioaddr + EMAC_BASIC_CTL1, 0x01);
 
 	/* The timeout was previoulsy set to 10ms, but some board (OrangePI0)
 	 * need more if no cable plugged. 100ms seems OK
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index 0877bde6e860..ca864c3d7ff3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -25,6 +25,7 @@
 #include <linux/crc32.h>
 #include <linux/slab.h>
 #include <linux/ethtool.h>
+#include <linux/setbits.h>
 #include <net/dsa.h>
 #include <asm/io.h>
 #include "stmmac.h"
@@ -355,7 +356,6 @@ static void dwmac1000_set_eee_mode(struct mac_device_info *hw,
 				   bool en_tx_lpi_clockgating)
 {
 	void __iomem *ioaddr = hw->pcsr;
-	u32 value;
 
 	/*TODO - en_tx_lpi_clockgating treatment */
 
@@ -363,19 +363,16 @@ static void dwmac1000_set_eee_mode(struct mac_device_info *hw,
 	 * receive path and instruct the transmit to enter in LPI
 	 * state.
 	 */
-	value = readl(ioaddr + LPI_CTRL_STATUS);
-	value |= LPI_CTRL_STATUS_LPIEN | LPI_CTRL_STATUS_LPITXA;
-	writel(value, ioaddr + LPI_CTRL_STATUS);
+	setbits32(ioaddr + LPI_CTRL_STATUS,
+		  LPI_CTRL_STATUS_LPIEN | LPI_CTRL_STATUS_LPITXA);
 }
 
 static void dwmac1000_reset_eee_mode(struct mac_device_info *hw)
 {
 	void __iomem *ioaddr = hw->pcsr;
-	u32 value;
 
-	value = readl(ioaddr + LPI_CTRL_STATUS);
-	value &= ~(LPI_CTRL_STATUS_LPIEN | LPI_CTRL_STATUS_LPITXA);
-	writel(value, ioaddr + LPI_CTRL_STATUS);
+	clrbits32(ioaddr + LPI_CTRL_STATUS,
+		  (LPI_CTRL_STATUS_LPIEN | LPI_CTRL_STATUS_LPITXA));
 }
 
 static void dwmac1000_set_eee_pls(struct mac_device_info *hw, int link)
@@ -383,14 +380,10 @@ static void dwmac1000_set_eee_pls(struct mac_device_info *hw, int link)
 	void __iomem *ioaddr = hw->pcsr;
 	u32 value;
 
-	value = readl(ioaddr + LPI_CTRL_STATUS);
-
 	if (link)
-		value |= LPI_CTRL_STATUS_PLS;
+		setbits32(ioaddr + LPI_CTRL_STATUS, LPI_CTRL_STATUS_PLS);
 	else
-		value &= ~LPI_CTRL_STATUS_PLS;
-
-	writel(value, ioaddr + LPI_CTRL_STATUS);
+		clrbits32(ioaddr + LPI_CTRL_STATUS, LPI_CTRL_STATUS_PLS);
 }
 
 static void dwmac1000_set_eee_timer(struct mac_device_info *hw, int ls, int tw)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 7e5d5db0d516..998695cbf3c2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -17,6 +17,8 @@
 #include <linux/slab.h>
 #include <linux/ethtool.h>
 #include <linux/io.h>
+#include <linux/setbits.h>
+#include <linux/setbits.h>
 #include <net/dsa.h>
 #include "stmmac.h"
 #include "stmmac_pcs.h"
@@ -85,16 +87,11 @@ static void dwmac4_rx_queue_priority(struct mac_device_info *hw,
 {
 	void __iomem *ioaddr = hw->pcsr;
 	u32 base_register;
-	u32 value;
 
 	base_register = (queue < 4) ? GMAC_RXQ_CTRL2 : GMAC_RXQ_CTRL3;
 
-	value = readl(ioaddr + base_register);
-
-	value &= ~GMAC_RXQCTRL_PSRQX_MASK(queue);
-	value |= (prio << GMAC_RXQCTRL_PSRQX_SHIFT(queue)) &
-						GMAC_RXQCTRL_PSRQX_MASK(queue);
-	writel(value, ioaddr + base_register);
+	clrsetbits32(ioaddr + base_register, GMAC_RXQCTRL_PSRQX_MASK(queue),
+		     (prio << GMAC_RXQCTRL_PSRQX_SHIFT(queue)) & GMAC_RXQCTRL_PSRQX_MASK(queue));
 }
 
 static void dwmac4_tx_queue_priority(struct mac_device_info *hw,
@@ -102,17 +99,11 @@ static void dwmac4_tx_queue_priority(struct mac_device_info *hw,
 {
 	void __iomem *ioaddr = hw->pcsr;
 	u32 base_register;
-	u32 value;
 
 	base_register = (queue < 4) ? GMAC_TXQ_PRTY_MAP0 : GMAC_TXQ_PRTY_MAP1;
 
-	value = readl(ioaddr + base_register);
-
-	value &= ~GMAC_TXQCTRL_PSTQX_MASK(queue);
-	value |= (prio << GMAC_TXQCTRL_PSTQX_SHIFT(queue)) &
-						GMAC_TXQCTRL_PSTQX_MASK(queue);
-
-	writel(value, ioaddr + base_register);
+	clrsetbits32(ioaddr + base_register, GMAC_TXQCTRL_PSTQX_MASK(queue),
+		     (prio << GMAC_TXQCTRL_PSTQX_SHIFT(queue)) & GMAC_TXQCTRL_PSTQX_MASK(queue));
 }
 
 static void dwmac4_rx_queue_routing(struct mac_device_info *hw,
@@ -198,11 +189,9 @@ static void dwmac4_set_mtl_tx_queue_weight(struct mac_device_info *hw,
 					   u32 weight, u32 queue)
 {
 	void __iomem *ioaddr = hw->pcsr;
-	u32 value = readl(ioaddr + MTL_TXQX_WEIGHT_BASE_ADDR(queue));
-
-	value &= ~MTL_TXQ_WEIGHT_ISCQW_MASK;
-	value |= weight & MTL_TXQ_WEIGHT_ISCQW_MASK;
-	writel(value, ioaddr + MTL_TXQX_WEIGHT_BASE_ADDR(queue));
+	clrsetbits32(ioaddr + MTL_TXQX_WEIGHT_BASE_ADDR(queue),
+		     MTL_TXQ_WEIGHT_ISCQW_MASK,
+		     weight & MTL_TXQ_WEIGHT_ISCQW_MASK);
 }
 
 static void dwmac4_map_mtl_dma(struct mac_device_info *hw, u32 queue, u32 chan)
@@ -243,10 +232,8 @@ static void dwmac4_config_cbs(struct mac_device_info *hw,
 	pr_debug("\tlow_credit: 0x%08x\n", low_credit);
 
 	/* enable AV algorithm */
-	value = readl(ioaddr + MTL_ETSX_CTRL_BASE_ADDR(queue));
-	value |= MTL_ETS_CTRL_AVALG;
-	value |= MTL_ETS_CTRL_CC;
-	writel(value, ioaddr + MTL_ETSX_CTRL_BASE_ADDR(queue));
+	setbits32(ioaddr + MTL_ETSX_CTRL_BASE_ADDR(queue),
+		  MTL_ETS_CTRL_AVALG | MTL_ETS_CTRL_CC);
 
 	/* configure send slope */
 	value = readl(ioaddr + MTL_SEND_SLP_CREDX_BASE_ADDR(queue));
@@ -360,11 +347,9 @@ static void dwmac4_set_eee_mode(struct mac_device_info *hw,
 static void dwmac4_reset_eee_mode(struct mac_device_info *hw)
 {
 	void __iomem *ioaddr = hw->pcsr;
-	u32 value;
 
-	value = readl(ioaddr + GMAC4_LPI_CTRL_STATUS);
-	value &= ~(GMAC4_LPI_CTRL_STATUS_LPIEN | GMAC4_LPI_CTRL_STATUS_LPITXA);
-	writel(value, ioaddr + GMAC4_LPI_CTRL_STATUS);
+	clrbits32(ioaddr + GMAC4_LPI_CTRL_STATUS,
+		  (GMAC4_LPI_CTRL_STATUS_LPIEN | GMAC4_LPI_CTRL_STATUS_LPITXA));
 }
 
 static void dwmac4_set_eee_pls(struct mac_device_info *hw, int link)
@@ -372,14 +357,12 @@ static void dwmac4_set_eee_pls(struct mac_device_info *hw, int link)
 	void __iomem *ioaddr = hw->pcsr;
 	u32 value;
 
-	value = readl(ioaddr + GMAC4_LPI_CTRL_STATUS);
-
 	if (link)
-		value |= GMAC4_LPI_CTRL_STATUS_PLS;
+		setbits32(ioaddr + GMAC4_LPI_CTRL_STATUS,
+			  GMAC4_LPI_CTRL_STATUS_PLS);
 	else
-		value &= ~GMAC4_LPI_CTRL_STATUS_PLS;
-
-	writel(value, ioaddr + GMAC4_LPI_CTRL_STATUS);
+		clrbits32(ioaddr + GMAC4_LPI_CTRL_STATUS,
+			  GMAC4_LPI_CTRL_STATUS_PLS);
 }
 
 static void dwmac4_set_eee_timer(struct mac_device_info *hw, int ls, int tw)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
index edb6053bd980..63c582ff24a1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
@@ -14,6 +14,7 @@
  */
 
 #include <linux/io.h>
+#include <linux/setbits.h>
 #include "dwmac4.h"
 #include "dwmac4_dma.h"
 
@@ -270,9 +271,7 @@ static void dwmac4_dma_rx_chan_op_mode(void __iomem *ioaddr, int mode,
 	writel(mtl_rx_op, ioaddr + MTL_CHAN_RX_OP_MODE(channel));
 
 	/* Enable MTL RX overflow */
-	mtl_rx_int = readl(ioaddr + MTL_CHAN_INT_CTRL(channel));
-	writel(mtl_rx_int | MTL_RX_OVERFLOW_INT_EN,
-	       ioaddr + MTL_CHAN_INT_CTRL(channel));
+	setbits32(ioaddr + MTL_CHAN_INT_CTRL(channel), MTL_RX_OVERFLOW_INT_EN);
 }
 
 static void dwmac4_dma_tx_chan_op_mode(void __iomem *ioaddr, int mode,
@@ -422,12 +421,8 @@ static void dwmac4_qmode(void __iomem *ioaddr, u32 channel, u8 qmode)
 
 static void dwmac4_set_bfsize(void __iomem *ioaddr, int bfsize, u32 chan)
 {
-	u32 value = readl(ioaddr + DMA_CHAN_RX_CONTROL(chan));
-
-	value &= ~DMA_RBSZ_MASK;
-	value |= (bfsize << DMA_RBSZ_SHIFT) & DMA_RBSZ_MASK;
-
-	writel(value, ioaddr + DMA_CHAN_RX_CONTROL(chan));
+	clrsetbits32(ioaddr + DMA_CHAN_RX_CONTROL(chan), DMA_RBSZ_MASK,
+		     (bfsize << DMA_RBSZ_SHIFT) & DMA_RBSZ_MASK);
 }
 
 const struct stmmac_dma_ops dwmac4_dma_ops = {
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
index 49f5687879df..5f699cf54e17 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
@@ -10,6 +10,7 @@
 
 #include <linux/io.h>
 #include <linux/delay.h>
+#include <linux/setbits.h>
 #include "common.h"
 #include "dwmac4_dma.h"
 #include "dwmac4.h"
@@ -47,51 +48,26 @@ void dwmac4_set_tx_tail_ptr(void __iomem *ioaddr, u32 tail_ptr, u32 chan)
 
 void dwmac4_dma_start_tx(void __iomem *ioaddr, u32 chan)
 {
-	u32 value = readl(ioaddr + DMA_CHAN_TX_CONTROL(chan));
-
-	value |= DMA_CONTROL_ST;
-	writel(value, ioaddr + DMA_CHAN_TX_CONTROL(chan));
-
-	value = readl(ioaddr + GMAC_CONFIG);
-	value |= GMAC_CONFIG_TE;
-	writel(value, ioaddr + GMAC_CONFIG);
+	setbits32(ioaddr + DMA_CHAN_TX_CONTROL(chan), DMA_CONTROL_ST);
+	setbits32(ioaddr + GMAC_CONFIG, GMAC_CONFIG_TE);
 }
 
 void dwmac4_dma_stop_tx(void __iomem *ioaddr, u32 chan)
 {
-	u32 value = readl(ioaddr + DMA_CHAN_TX_CONTROL(chan));
-
-	value &= ~DMA_CONTROL_ST;
-	writel(value, ioaddr + DMA_CHAN_TX_CONTROL(chan));
-
-	value = readl(ioaddr + GMAC_CONFIG);
-	value &= ~GMAC_CONFIG_TE;
-	writel(value, ioaddr + GMAC_CONFIG);
+	clrbits32(ioaddr + DMA_CHAN_TX_CONTROL(chan), DMA_CONTROL_ST);
+	clrbits32(ioaddr + GMAC_CONFIG, GMAC_CONFIG_TE);
 }
 
 void dwmac4_dma_start_rx(void __iomem *ioaddr, u32 chan)
 {
-	u32 value = readl(ioaddr + DMA_CHAN_RX_CONTROL(chan));
-
-	value |= DMA_CONTROL_SR;
-
-	writel(value, ioaddr + DMA_CHAN_RX_CONTROL(chan));
-
-	value = readl(ioaddr + GMAC_CONFIG);
-	value |= GMAC_CONFIG_RE;
-	writel(value, ioaddr + GMAC_CONFIG);
+	setbits32(ioaddr + DMA_CHAN_RX_CONTROL(chan), DMA_CONTROL_SR);
+	setbits32(ioaddr + GMAC_CONFIG, GMAC_CONFIG_RE);
 }
 
 void dwmac4_dma_stop_rx(void __iomem *ioaddr, u32 chan)
 {
-	u32 value = readl(ioaddr + DMA_CHAN_RX_CONTROL(chan));
-
-	value &= ~DMA_CONTROL_SR;
-	writel(value, ioaddr + DMA_CHAN_RX_CONTROL(chan));
-
-	value = readl(ioaddr + GMAC_CONFIG);
-	value &= ~GMAC_CONFIG_RE;
-	writel(value, ioaddr + GMAC_CONFIG);
+	clrbits32(ioaddr + DMA_CHAN_RX_CONTROL(chan), DMA_CONTROL_SR);
+	clrbits32(ioaddr + GMAC_CONFIG, GMAC_CONFIG_RE);
 }
 
 void dwmac4_set_tx_ring_len(void __iomem *ioaddr, u32 len, u32 chan)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
index 3f4f3132e16b..aec2fb884477 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
@@ -4,6 +4,7 @@
 
 #include <linux/bitops.h>
 #include <linux/iopoll.h>
+#include <linux/setbits.h>
 #include "common.h"
 #include "dwmac4.h"
 #include "dwmac5.h"
@@ -307,9 +308,7 @@ static int dwmac5_rxp_disable(void __iomem *ioaddr)
 	u32 val;
 	int ret;
 
-	val = readl(ioaddr + MTL_OPERATION_MODE);
-	val &= ~MTL_FRPE;
-	writel(val, ioaddr + MTL_OPERATION_MODE);
+	clrbits32(ioaddr + MTL_OPERATION_MODE, MTL_FRPE);
 
 	ret = readl_poll_timeout(ioaddr + MTL_RXP_CONTROL_STATUS, val,
 			val & RXPI, 1, 10000);
@@ -320,11 +319,7 @@ static int dwmac5_rxp_disable(void __iomem *ioaddr)
 
 static void dwmac5_rxp_enable(void __iomem *ioaddr)
 {
-	u32 val;
-
-	val = readl(ioaddr + MTL_OPERATION_MODE);
-	val |= MTL_FRPE;
-	writel(val, ioaddr + MTL_OPERATION_MODE);
+	setbits32(ioaddr + MTL_OPERATION_MODE, MTL_FRPE);
 }
 
 static int dwmac5_rxp_update_single_entry(void __iomem *ioaddr,
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
index 7516ca210855..acecb9f0ee4b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
@@ -18,6 +18,7 @@
 
 #include <linux/io.h>
 #include <linux/iopoll.h>
+#include <linux/setbits.h>
 #include "common.h"
 #include "dwmac_dma.h"
 
@@ -59,30 +60,22 @@ void dwmac_disable_dma_irq(void __iomem *ioaddr, u32 chan)
 
 void dwmac_dma_start_tx(void __iomem *ioaddr, u32 chan)
 {
-	u32 value = readl(ioaddr + DMA_CONTROL);
-	value |= DMA_CONTROL_ST;
-	writel(value, ioaddr + DMA_CONTROL);
+	setbits32(ioaddr + DMA_CONTROL, DMA_CONTROL_ST);
 }
 
 void dwmac_dma_stop_tx(void __iomem *ioaddr, u32 chan)
 {
-	u32 value = readl(ioaddr + DMA_CONTROL);
-	value &= ~DMA_CONTROL_ST;
-	writel(value, ioaddr + DMA_CONTROL);
+	clrbits32(ioaddr + DMA_CONTROL, DMA_CONTROL_ST);
 }
 
 void dwmac_dma_start_rx(void __iomem *ioaddr, u32 chan)
 {
-	u32 value = readl(ioaddr + DMA_CONTROL);
-	value |= DMA_CONTROL_SR;
-	writel(value, ioaddr + DMA_CONTROL);
+	setbits32(ioaddr + DMA_CONTROL, DMA_CONTROL_SR);
 }
 
 void dwmac_dma_stop_rx(void __iomem *ioaddr, u32 chan)
 {
-	u32 value = readl(ioaddr + DMA_CONTROL);
-	value &= ~DMA_CONTROL_SR;
-	writel(value, ioaddr + DMA_CONTROL);
+	clrbits32(ioaddr + DMA_CONTROL, DMA_CONTROL_SR);
 }
 
 #ifdef DWMAC_DMA_DEBUG
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
index 64b8cb88ea45..fc7df8de2ba7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
@@ -4,6 +4,7 @@
  * stmmac XGMAC support.
  */
 
+#include <linux/setbits.h>
 #include "stmmac.h"
 #include "dwxgmac2.h"
 
@@ -75,12 +76,10 @@ static int dwxgmac2_rx_ipc(struct mac_device_info *hw)
 	void __iomem *ioaddr = hw->pcsr;
 	u32 value;
 
-	value = readl(ioaddr + XGMAC_RX_CONFIG);
 	if (hw->rx_csum)
-		value |= XGMAC_CONFIG_IPC;
+		setbits32(ioaddr + XGMAC_RX_CONFIG, XGMAC_CONFIG_IPC);
 	else
-		value &= ~XGMAC_CONFIG_IPC;
-	writel(value, ioaddr + XGMAC_RX_CONFIG);
+		clrbits32(ioaddr + XGMAC_RX_CONFIG, XGMAC_CONFIG_IPC);
 
 	return !!(readl(ioaddr + XGMAC_RX_CONFIG) & XGMAC_CONFIG_IPC);
 }
@@ -107,11 +106,8 @@ static void dwxgmac2_rx_queue_prio(struct mac_device_info *hw, u32 prio,
 
 	reg = (queue < 4) ? XGMAC_RXQ_CTRL2 : XGMAC_RXQ_CTRL3;
 
-	value = readl(ioaddr + reg);
-	value &= ~XGMAC_PSRQ(queue);
-	value |= (prio << XGMAC_PSRQ_SHIFT(queue)) & XGMAC_PSRQ(queue);
-
-	writel(value, ioaddr + reg);
+	clrsetbits32(ioaddr + reg, XGMAC_PSRQ(queue),
+		     (prio << XGMAC_PSRQ_SHIFT(queue)) & XGMAC_PSRQ(queue));
 }
 
 static void dwxgmac2_prog_mtl_rx_algorithms(struct mac_device_info *hw,
@@ -170,11 +166,8 @@ static void dwxgmac2_map_mtl_to_dma(struct mac_device_info *hw, u32 queue,
 
 	reg = (queue < 4) ? XGMAC_MTL_RXQ_DMA_MAP0 : XGMAC_MTL_RXQ_DMA_MAP1;
 
-	value = readl(ioaddr + reg);
-	value &= ~XGMAC_QxMDMACH(queue);
-	value |= (chan << XGMAC_QxMDMACH_SHIFT(queue)) & XGMAC_QxMDMACH(queue);
-
-	writel(value, ioaddr + reg);
+	clrsetbits32(ioaddr + reg, XGMAC_QxMDMACH(queue),
+		     (chan << XGMAC_QxMDMACH_SHIFT(queue)) & XGMAC_QxMDMACH(queue));
 }
 
 static void dwxgmac2_config_cbs(struct mac_device_info *hw,
@@ -189,9 +182,8 @@ static void dwxgmac2_config_cbs(struct mac_device_info *hw,
 	writel(high_credit, ioaddr + XGMAC_MTL_TCx_HICREDIT(queue));
 	writel(low_credit, ioaddr + XGMAC_MTL_TCx_LOCREDIT(queue));
 
-	value = readl(ioaddr + XGMAC_MTL_TCx_ETS_CONTROL(queue));
-	value |= XGMAC_CC | XGMAC_CBS;
-	writel(value, ioaddr + XGMAC_MTL_TCx_ETS_CONTROL(queue));
+	setbits32(ioaddr + XGMAC_MTL_TCx_ETS_CONTROL(queue),
+		  XGMAC_CC | XGMAC_CBS);
 }
 
 static int dwxgmac2_host_irq_status(struct mac_device_info *hw,
@@ -263,9 +255,7 @@ static void dwxgmac2_pmt(struct mac_device_info *hw, unsigned long mode)
 	if (mode & WAKE_UCAST)
 		val |= XGMAC_PWRDWN | XGMAC_GLBLUCAST | XGMAC_RWKPKTEN;
 	if (val) {
-		u32 cfg = readl(ioaddr + XGMAC_RX_CONFIG);
-		cfg |= XGMAC_CONFIG_RE;
-		writel(cfg, ioaddr + XGMAC_RX_CONFIG);
+		setbits32(ioaddr + XGMAC_RX_CONFIG, XGMAC_CONFIG_RE);
 	}
 
 	writel(val, ioaddr + XGMAC_PMT);
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 6c5092e7771c..7a7d584211e9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/iopoll.h>
+#include <linux/setbits.h>
 #include "stmmac.h"
 #include "dwxgmac2.h"
 
@@ -47,12 +48,9 @@ static void dwxgmac2_dma_init_rx_chan(void __iomem *ioaddr,
 				      u32 dma_rx_phy, u32 chan)
 {
 	u32 rxpbl = dma_cfg->rxpbl ?: dma_cfg->pbl;
-	u32 value;
 
-	value = readl(ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
-	value &= ~XGMAC_RxPBL;
-	value |= (rxpbl << XGMAC_RxPBL_SHIFT) & XGMAC_RxPBL;
-	writel(value, ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
+	clrsetbits32(ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan), XGMAC_RxPBL,
+		     (rxpbl << XGMAC_RxPBL_SHIFT) & XGMAC_RxPBL);
 
 	writel(dma_rx_phy, ioaddr + XGMAC_DMA_CH_RxDESC_LADDR(chan));
 }
@@ -150,8 +148,7 @@ static void dwxgmac2_dma_rx_mode(void __iomem *ioaddr, int mode,
 	writel(value, ioaddr + XGMAC_MTL_RXQ_OPMODE(channel));
 
 	/* Enable MTL RX overflow */
-	value = readl(ioaddr + XGMAC_MTL_QINTEN(channel));
-	writel(value | XGMAC_RXOIE, ioaddr + XGMAC_MTL_QINTEN(channel));
+	setbits32(ioaddr + XGMAC_MTL_QINTEN(channel), XGMAC_RXOIE);
 }
 
 static void dwxgmac2_dma_tx_mode(void __iomem *ioaddr, int mode,
@@ -209,54 +206,26 @@ static void dwxgmac2_disable_dma_irq(void __iomem *ioaddr, u32 chan)
 
 static void dwxgmac2_dma_start_tx(void __iomem *ioaddr, u32 chan)
 {
-	u32 value;
-
-	value = readl(ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
-	value |= XGMAC_TXST;
-	writel(value, ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
-
-	value = readl(ioaddr + XGMAC_TX_CONFIG);
-	value |= XGMAC_CONFIG_TE;
-	writel(value, ioaddr + XGMAC_TX_CONFIG);
+	setbits32(ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan), XGMAC_TXST);
+	setbits32(ioaddr + XGMAC_TX_CONFIG, XGMAC_CONFIG_TE);
 }
 
 static void dwxgmac2_dma_stop_tx(void __iomem *ioaddr, u32 chan)
 {
-	u32 value;
-
-	value = readl(ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
-	value &= ~XGMAC_TXST;
-	writel(value, ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
-
-	value = readl(ioaddr + XGMAC_TX_CONFIG);
-	value &= ~XGMAC_CONFIG_TE;
-	writel(value, ioaddr + XGMAC_TX_CONFIG);
+	clrbits32(ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan), XGMAC_TXST);
+	clrbits32(ioaddr + XGMAC_TX_CONFIG, XGMAC_CONFIG_TE);
 }
 
 static void dwxgmac2_dma_start_rx(void __iomem *ioaddr, u32 chan)
 {
-	u32 value;
-
-	value = readl(ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
-	value |= XGMAC_RXST;
-	writel(value, ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
-
-	value = readl(ioaddr + XGMAC_RX_CONFIG);
-	value |= XGMAC_CONFIG_RE;
-	writel(value, ioaddr + XGMAC_RX_CONFIG);
+	setbits32(ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan), XGMAC_RXST);
+	setbits32(ioaddr + XGMAC_RX_CONFIG, XGMAC_CONFIG_RE);
 }
 
 static void dwxgmac2_dma_stop_rx(void __iomem *ioaddr, u32 chan)
 {
-	u32 value;
-
-	value = readl(ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
-	value &= ~XGMAC_RXST;
-	writel(value, ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
-
-	value = readl(ioaddr + XGMAC_RX_CONFIG);
-	value &= ~XGMAC_CONFIG_RE;
-	writel(value, ioaddr + XGMAC_RX_CONFIG);
+	clrbits32(ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan), XGMAC_RXST);
+	clrbits32(ioaddr + XGMAC_RX_CONFIG, XGMAC_CONFIG_RE);
 }
 
 static int dwxgmac2_dma_interrupt(void __iomem *ioaddr,
@@ -367,14 +336,10 @@ static void dwxgmac2_set_tx_tail_ptr(void __iomem *ioaddr, u32 ptr, u32 chan)
 
 static void dwxgmac2_enable_tso(void __iomem *ioaddr, bool en, u32 chan)
 {
-	u32 value = readl(ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
-
 	if (en)
-		value |= XGMAC_TSE;
+		setbits32(ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan), XGMAC_TSE);
 	else
-		value &= ~XGMAC_TSE;
-
-	writel(value, ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
+		clrbits32(ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan), XGMAC_TSE);
 }
 
 static void dwxgmac2_qmode(void __iomem *ioaddr, u32 channel, u8 qmode)
@@ -394,11 +359,7 @@ static void dwxgmac2_qmode(void __iomem *ioaddr, u32 channel, u8 qmode)
 
 static void dwxgmac2_set_bfsize(void __iomem *ioaddr, int bfsize, u32 chan)
 {
-	u32 value;
-
-	value = readl(ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
-	value |= bfsize << 1;
-	writel(value, ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
+	setbits32(ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan), bfsize << 1);
 }
 
 const struct stmmac_dma_ops dwxgmac210_dma_ops = {
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
index 8d9cc2157afd..8680fb4b1fa8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
@@ -21,6 +21,7 @@
 
 #include <linux/io.h>
 #include <linux/delay.h>
+#include <linux/setbits.h>
 #include "common.h"
 #include "stmmac_ptp.h"
 
@@ -64,14 +65,11 @@ static void config_sub_second_increment(void __iomem *ioaddr,
 static int init_systime(void __iomem *ioaddr, u32 sec, u32 nsec)
 {
 	int limit;
-	u32 value;
 
 	writel(sec, ioaddr + PTP_STSUR);
 	writel(nsec, ioaddr + PTP_STNSUR);
 	/* issue command to initialize the system time value */
-	value = readl(ioaddr + PTP_TCR);
-	value |= PTP_TCR_TSINIT;
-	writel(value, ioaddr + PTP_TCR);
+	setbits32(ioaddr + PTP_TCR, PTP_TCR_TSINIT);
 
 	/* wait for present system time initialize to complete */
 	limit = 10;
@@ -88,14 +86,11 @@ static int init_systime(void __iomem *ioaddr, u32 sec, u32 nsec)
 
 static int config_addend(void __iomem *ioaddr, u32 addend)
 {
-	u32 value;
 	int limit;
 
 	writel(addend, ioaddr + PTP_TAR);
 	/* issue command to update the addend value */
-	value = readl(ioaddr + PTP_TCR);
-	value |= PTP_TCR_TSADDREG;
-	writel(value, ioaddr + PTP_TCR);
+	setbits32(ioaddr + PTP_TCR, PTP_TCR_TSADDREG);
 
 	/* wait for present addend update to complete */
 	limit = 10;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index b72ef171477e..b9cdf951eda6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -27,6 +27,7 @@
 #include <linux/of_gpio.h>
 #include <linux/of_mdio.h>
 #include <linux/phy.h>
+#include <linux/setbits.h>
 #include <linux/slab.h>
 
 #include "dwxgmac2.h"
@@ -64,10 +65,8 @@ static int stmmac_xgmac2_c22_format(struct stmmac_priv *priv, int phyaddr,
 		return -EBUSY;
 
 	/* Set port as Clause 22 */
-	tmp = readl(priv->ioaddr + XGMAC_MDIO_C22P);
-	tmp &= ~MII_XGMAC_C22P_MASK;
-	tmp |= BIT(phyaddr);
-	writel(tmp, priv->ioaddr + XGMAC_MDIO_C22P);
+	clrsetbits32(priv->ioaddr + XGMAC_MDIO_C22P, MII_XGMAC_C22P_MASK,
+		     BIT(phyaddr));
 
 	*hw_addr = (phyaddr << 16) | (phyreg & 0x1f);
 	return 0;
-- 
2.16.4

^ permalink raw reply related

* for_each_netdev_feature() broken on big endian
From: Mehrtens, Hauke @ 2018-09-07 15:10 UTC (permalink / raw)
  To: netdev@vger.kernel.org; +Cc: Doychev, Zahari, Langer, Thomas

Hi,

On a MIPS 32 Big endian system the netdev_sync_upper_features() function does not work correctly.
It does not disbale bit 15 (NETIF_F_LRO, 0x0000000000008000), but 47 (NETIF_F_HW_TC, 0x0000800000000000).

The for_each_netdev_feature() macro is used to go over all netdev feature flags and calls for_each_set_bit() with a u64.
This is the code:
#define for_each_netdev_feature(mask_addr, bit)	\
	for_each_set_bit(bit, (unsigned long *)mask_addr, NETDEV_FEATURE_COUNT)
https://elixir.bootlin.com/linux/v4.19-rc2/source/include/linux/netdev_features.h#L157

The for_each_set_bit() macro calls the find_first_bit() macro and works directly on the bits in the memory
organized in 32 bit values in the generic implementation, see here:
https://elixir.bootlin.com/linux/v4.19-rc2/source/lib/find_bit.c#L102

On big endian systems the u64 value is stored in a different order in memory compared to little endian systems.

When I execute this code: 
	netdev_features_t upper_disables = NETIF_F_UPPER_DISABLES;
	printk("%s:%i: addr: 0x%llx, size: %i\n", __func__, __LINE__, upper_disables, NETDEV_FEATURE_COUNT);
	ret = find_first_bit((unsigned long *)&upper_disables, NETDEV_FEATURE_COUNT);
	printk("%s:%i: addr: 0x%llx, size: %i, ret: %li\n", __func__, __LINE__, upper_disables, NETDEV_FEATURE_COUNT, ret);
I get this result:
	[   35.227140] netdev_sync_upper_features:6912: addr: 0x8000, size: 48
	[   35.233337] netdev_sync_upper_features:6914: addr: 0x8000, size: 48, ret: 47
Expected would be a ret: 15.

As I understood the documentation for for_each_set_bit() this should find the first bit in memory and not in a integer,
so for_each_netdev_feature() should not use for_each_set_bit().
I do not really know what a good fix would be, I could convert the feature flags to little endian before calling for_each_netdev_feature(), would this be ok?

Hauke

^ permalink raw reply

* Re: [PATCH 2/5] include: add setbits32/clrbits32/clrsetbits32/setbits64/clrbits64/clrsetbits64 in linux/setbits.h
From: Scott Wood @ 2018-09-07 20:00 UTC (permalink / raw)
  To: Corentin Labbe, Gilles.Muller, Julia.Lawall, agust,
	alexandre.torgue, alistair, benh, carlo, davem, galak, joabreu,
	khilman, maxime.ripard, michal.lkml, mpe, mporter, nicolas.palix,
	paulus, peppe.cavallaro, tj, vitb, wens
  Cc: cocci, linux-amlogic, linux-arm-kernel, linux-ide, linux-kernel,
	linuxppc-dev, netdev, linux-sunxi
In-Reply-To: <1536349307-20714-3-git-send-email-clabbe@baylibre.com>

On Fri, 2018-09-07 at 19:41 +0000, Corentin Labbe wrote:
> This patch adds setbits32/clrbits32/clrsetbits32 and
> setbits64/clrbits64/clrsetbits64 in linux/setbits.h header.
> 
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
>  include/linux/setbits.h | 55
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 include/linux/setbits.h
> 
> diff --git a/include/linux/setbits.h b/include/linux/setbits.h
> new file mode 100644
> index 000000000000..3e1e273551bb
> --- /dev/null
> +++ b/include/linux/setbits.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __LINUX_SETBITS_H
> +#define __LINUX_SETBITS_H
> +
> +#include <linux/io.h>
> +
> +#define __setbits(readfunction, writefunction, addr, set) \
> +	writefunction((readfunction(addr) | (set)), addr)
> +#define __clrbits(readfunction, writefunction, addr, mask) \
> +	writefunction((readfunction(addr) & ~(mask)), addr)
> +#define __clrsetbits(readfunction, writefunction, addr, mask, set) \
> +	writefunction(((readfunction(addr) & ~(mask)) | (set)), addr)
> +#define __setclrbits(readfunction, writefunction, addr, mask, set) \
> +	writefunction(((readfunction(addr) | (seti)) & ~(mask)), addr)
> +
> +#define setbits32(addr, set) __setbits(readl, writel, addr, set)
> +#define setbits32_relaxed(addr, set) __setbits(readl_relaxed,
> writel_relaxed, \
> +					       addr, set)
> +
> +#define clrbits32(addr, mask) __clrbits(readl, writel, addr, mask)
> +#define clrbits32_relaxed(addr, mask) __clrbits(readl_relaxed,
> writel_relaxed, \
> +						addr, mask)

So now setbits32/clrbits32 is implicitly little-endian?  Introducing new
implicit-endian accessors is probably a bad thing in general, but doing it
with a name that until this patchset was implicitly big-endian (at least on
powerpc) seems worse.  Why not setbits32_le()?


> +
> +#define clrsetbits32(addr, mask, set) __clrsetbits(readl, writel, addr,
> mask, set)
> +#define clrsetbits32_relaxed(addr, mask, set) __clrsetbits(readl_relaxed, \
> +							   writel_relaxed,
> \
> +							   addr, mask, set)
> +
> +#define setclrbits32(addr, mask, set) __setclrbits(readl, writel, addr,
> mask, set)
> +#define setclrbits32_relaxed(addr, mask, set) __setclrbits(readl_relaxed, \
> +							   writel_relaxed,
> \
> +							   addr, mask, set)

What's the use case for setclrbits?  I don't see it used anywhere in this
patchset (not even in the coccinelle patterns), it doesn't seem like it would
be a common pattern, and it could easily get confused with clrsetbits.

-Scott

^ permalink raw reply

* Re: for_each_netdev_feature() broken on big endian
From: David Miller @ 2018-09-07 15:25 UTC (permalink / raw)
  To: hauke.mehrtens; +Cc: netdev, zahari.doychev, thomas.langer
In-Reply-To: <9231D502B07C5E4A8B32D5115C9F19991EE57330@IRSMSX101.ger.corp.intel.com>

From: "Mehrtens, Hauke" <hauke.mehrtens@intel.com>
Date: Fri, 7 Sep 2018 15:10:53 +0000

> On a MIPS 32 Big endian system the netdev_sync_upper_features() function does not work correctly.
> It does not disbale bit 15 (NETIF_F_LRO, 0x0000000000008000), but 47 (NETIF_F_HW_TC, 0x0000800000000000).
> 
> The for_each_netdev_feature() macro is used to go over all netdev feature flags and calls for_each_set_bit() with a u64.
> This is the code:
> #define for_each_netdev_feature(mask_addr, bit)	\
> 	for_each_set_bit(bit, (unsigned long *)mask_addr, NETDEV_FEATURE_COUNT)
> https://elixir.bootlin.com/linux/v4.19-rc2/source/include/linux/netdev_features.h#L157

Good catch, yes we cannot use the the generic bit handling macros on the netdev
feature flags because the feature flags are a u64 operated on as a unit whereas
"long" may be 32-bit or 64-bit depending upon the architecture.

^ permalink raw reply

* Re: [PATCH v4 3/3] IB/ipoib: Log sysfs 'dev_id' accesses from userspace
From: Jason Gunthorpe @ 2018-09-07 15:43 UTC (permalink / raw)
  To: Arseny Maslennikov; +Cc: linux-rdma, Doug Ledford, netdev
In-Reply-To: <20180906145112.29245-4-ar@cs.msu.ru>

On Thu, Sep 06, 2018 at 05:51:12PM +0300, Arseny Maslennikov wrote:
> Some tools may currently be using only the deprecated attribute;
> let's print an elaborate and clear deprecation notice to kmsg.
> 
> To do that, we have to replace the whole sysfs file, since we inherit
> the original one from netdev.
> 
> Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
>  drivers/infiniband/ulp/ipoib/ipoib_main.c | 31 +++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> index 30f840f874b3..74732726ec6f 100644
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -2386,6 +2386,35 @@ int ipoib_add_pkey_attr(struct net_device *dev)
>  	return device_create_file(&dev->dev, &dev_attr_pkey);
>  }
>  
> +/*
> + * We erroneously exposed the iface's port number in the dev_id
> + * sysfs field long after dev_port was introduced for that purpose[1],
> + * and we need to stop everyone from relying on that.
> + * Let's overload the shower routine for the dev_id file here
> + * to gently bring the issue up.
> + *
> + * [1] https://www.spinics.net/lists/netdev/msg272123.html
> + */
> +static ssize_t dev_id_show(struct device *dev,
> +			   struct device_attribute *attr, char *buf)
> +{
> +	struct net_device *ndev = to_net_dev(dev);
> +
> +	if (ndev->dev_id == ndev->dev_port)
> +		netdev_info_once(ndev,
> +			"\"%s\" wants to know my dev_id. Should it look at dev_port instead? See Documentation/ABI/testing/sysfs-class-net for more info.\n",
> +			current->comm);
> +
> +	return sprintf(buf, "%#x\n", ndev->dev_id);
> +}
> +static DEVICE_ATTR_RO(dev_id);
> +
> +int ipoib_intercept_dev_id_attr(struct net_device *dev)
> +{
> +	device_remove_file(&dev->dev, &dev_attr_dev_id);
> +	return device_create_file(&dev->dev, &dev_attr_dev_id);
> +}

Isn't this racey with userspace? Ie what happens if udev is querying
the dev_id right here?

Do we know there is no userspace doing this?

>  static struct net_device *ipoib_add_port(const char *format,
>  					 struct ib_device *hca, u8 port)
>  {
> @@ -2427,6 +2456,8 @@ static struct net_device *ipoib_add_port(const char *format,
>  	 */
>  	ndev->priv_destructor = ipoib_intf_free;
>  
> +	if (ipoib_intercept_dev_id_attr(ndev))
> +		goto sysfs_failed;

No device_remove_file needed?

Jason

^ permalink raw reply

* Re: [**EXTERNAL**] Re: VRF with enslaved L3 enabled bridge
From: David Ahern @ 2018-09-07 16:09 UTC (permalink / raw)
  To: D'Souza, Nelson, netdev@vger.kernel.org; +Cc: Ido Schimmel
In-Reply-To: <BYAPR04MB3797F5219C2164F3C0B6DE57AD000@BYAPR04MB3797.namprd04.prod.outlook.com>

On 9/7/18 9:56 AM, D'Souza, Nelson wrote:

> ------------------------------------------------------------------------
> *From:* David Ahern <dsa@cumulusnetworks.com>
> *Sent:* Thursday, September 6, 2018 5:27 PM
> *To:* D'Souza, Nelson; netdev@vger.kernel.org
> *Subject:* Re: [**EXTERNAL**] Re: VRF with enslaved L3 enabled bridge
>  
> On 9/5/18 12:00 PM, D'Souza, Nelson wrote:
>> Just following up.... would you be able to confirm that this is a
> Linux VRF issue?
> 
> I can confirm that I can reproduce the problem. Need to find time to dig
> into it.

bridge's netfilter hook is dropping the packet.

bridge's netfilter code registers hook operations that are invoked when
nh_hook is called. It then sees all subsequent calls to nf_hook.

Packet wise, the bridge netfilter hook runs first. br_nf_pre_routing
allocates nf_bridge, sets in_prerouting to 1 and calls NF_HOOK for
NF_INET_PRE_ROUTING. It's finish function, br_nf_pre_routing_finish,
then resets in_prerouting flag to 0. Any subsequent calls to nf_hook
invoke ip_sabotage_in. That function sees in_prerouting is not
set and steals (drops) the packet.

The simplest change is to have ip_sabotage_in recognize that the bridge
can be enslaved to a VRF (L3 master device) and allow the packet to
continue.

Thanks to Ido for the hint on ip_sabotage_in.

This patch works for me:

diff --git a/net/bridge/br_netfilter_hooks.c
b/net/bridge/br_netfilter_hooks.c
index 6e0dc6bcd32a..37278dc280eb 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -835,7 +835,8 @@ static unsigned int ip_sabotage_in(void *priv,
                                   struct sk_buff *skb,
                                   const struct nf_hook_state *state)
 {
-       if (skb->nf_bridge && !skb->nf_bridge->in_prerouting) {
+       if (skb->nf_bridge && !skb->nf_bridge->in_prerouting &&
+           !netif_is_l3_master(skb->dev)) {
                state->okfn(state->net, state->sk, skb);
                return NF_STOLEN;
        }

^ permalink raw reply related


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