Netdev List
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 5.4 6/9] net: annotate data-races around sk->sk_dst_pending_confirm
From: Sasha Levin @ 2023-11-07 12:12 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Eric Dumazet, David S . Miller, Sasha Levin, kuba, pabeni,
	dsahern, kuniyu, wuyun.abel, leitao, alexander, dhowells, netdev
In-Reply-To: <20231107121256.3758858-1-sashal@kernel.org>

From: Eric Dumazet <edumazet@google.com>

[ Upstream commit eb44ad4e635132754bfbcb18103f1dcb7058aedd ]

This field can be read or written without socket lock being held.

Add annotations to avoid load-store tearing.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 include/net/sock.h    | 6 +++---
 net/core/sock.c       | 2 +-
 net/ipv4/tcp_output.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index b021c8912e2cf..5293f2b65fb55 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1941,7 +1941,7 @@ static inline void dst_negative_advice(struct sock *sk)
 		if (ndst != dst) {
 			rcu_assign_pointer(sk->sk_dst_cache, ndst);
 			sk_tx_queue_clear(sk);
-			sk->sk_dst_pending_confirm = 0;
+			WRITE_ONCE(sk->sk_dst_pending_confirm, 0);
 		}
 	}
 }
@@ -1952,7 +1952,7 @@ __sk_dst_set(struct sock *sk, struct dst_entry *dst)
 	struct dst_entry *old_dst;
 
 	sk_tx_queue_clear(sk);
-	sk->sk_dst_pending_confirm = 0;
+	WRITE_ONCE(sk->sk_dst_pending_confirm, 0);
 	old_dst = rcu_dereference_protected(sk->sk_dst_cache,
 					    lockdep_sock_is_held(sk));
 	rcu_assign_pointer(sk->sk_dst_cache, dst);
@@ -1965,7 +1965,7 @@ sk_dst_set(struct sock *sk, struct dst_entry *dst)
 	struct dst_entry *old_dst;
 
 	sk_tx_queue_clear(sk);
-	sk->sk_dst_pending_confirm = 0;
+	WRITE_ONCE(sk->sk_dst_pending_confirm, 0);
 	old_dst = xchg((__force struct dst_entry **)&sk->sk_dst_cache, dst);
 	dst_release(old_dst);
 }
diff --git a/net/core/sock.c b/net/core/sock.c
index 9979cd602dfac..2c3c5df139345 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -545,7 +545,7 @@ struct dst_entry *__sk_dst_check(struct sock *sk, u32 cookie)
 
 	if (dst && dst->obsolete && dst->ops->check(dst, cookie) == NULL) {
 		sk_tx_queue_clear(sk);
-		sk->sk_dst_pending_confirm = 0;
+		WRITE_ONCE(sk->sk_dst_pending_confirm, 0);
 		RCU_INIT_POINTER(sk->sk_dst_cache, NULL);
 		dst_release(dst);
 		return NULL;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 16e0249b11f6c..97bee820799ee 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1103,7 +1103,7 @@ static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb,
 	skb_set_hash_from_sk(skb, sk);
 	refcount_add(skb->truesize, &sk->sk_wmem_alloc);
 
-	skb_set_dst_pending_confirm(skb, sk->sk_dst_pending_confirm);
+	skb_set_dst_pending_confirm(skb, READ_ONCE(sk->sk_dst_pending_confirm));
 
 	/* Build TCP header and checksum it. */
 	th = (struct tcphdr *)skb->data;
-- 
2.42.0


^ permalink raw reply related

* [PATCH AUTOSEL 4.19 1/7] wifi: mac80211: don't return unset power in ieee80211_get_tx_power()
From: Sasha Levin @ 2023-11-07 12:13 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Ping-Ke Shih, Zong-Zhe Yang, Johannes Berg, Sasha Levin, johannes,
	davem, edumazet, kuba, pabeni, linux-wireless, netdev

From: Ping-Ke Shih <pkshih@realtek.com>

[ Upstream commit e160ab85166e77347d0cbe5149045cb25e83937f ]

We can get a UBSAN warning if ieee80211_get_tx_power() returns the
INT_MIN value mac80211 internally uses for "unset power level".

 UBSAN: signed-integer-overflow in net/wireless/nl80211.c:3816:5
 -2147483648 * 100 cannot be represented in type 'int'
 CPU: 0 PID: 20433 Comm: insmod Tainted: G        WC OE
 Call Trace:
  dump_stack+0x74/0x92
  ubsan_epilogue+0x9/0x50
  handle_overflow+0x8d/0xd0
  __ubsan_handle_mul_overflow+0xe/0x10
  nl80211_send_iface+0x688/0x6b0 [cfg80211]
  [...]
  cfg80211_register_wdev+0x78/0xb0 [cfg80211]
  cfg80211_netdev_notifier_call+0x200/0x620 [cfg80211]
  [...]
  ieee80211_if_add+0x60e/0x8f0 [mac80211]
  ieee80211_register_hw+0xda5/0x1170 [mac80211]

In this case, simply return an error instead, to indicate
that no data is available.

Cc: Zong-Zhe Yang <kevin_yang@realtek.com>
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
Link: https://lore.kernel.org/r/20230203023636.4418-1-pkshih@realtek.com
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 net/mac80211/cfg.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 5659af1bec179..77d8ed184c1c4 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2452,6 +2452,10 @@ static int ieee80211_get_tx_power(struct wiphy *wiphy,
 	else
 		*dbm = sdata->vif.bss_conf.txpower;
 
+	/* INT_MIN indicates no power level was set yet */
+	if (*dbm == INT_MIN)
+		return -EINVAL;
+
 	return 0;
 }
 
-- 
2.42.0


^ permalink raw reply related

* [PATCH AUTOSEL 4.19 4/7] net: annotate data-races around sk->sk_tx_queue_mapping
From: Sasha Levin @ 2023-11-07 12:13 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Eric Dumazet, David S . Miller, Sasha Levin, kuba, pabeni, netdev
In-Reply-To: <20231107121318.3759058-1-sashal@kernel.org>

From: Eric Dumazet <edumazet@google.com>

[ Upstream commit 0bb4d124d34044179b42a769a0c76f389ae973b6 ]

This field can be read or written without socket lock being held.

Add annotations to avoid load-store tearing.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 include/net/sock.h | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 373e34b46a3c9..c0df14e5a0754 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1759,21 +1759,33 @@ static inline void sk_tx_queue_set(struct sock *sk, int tx_queue)
 	/* sk_tx_queue_mapping accept only upto a 16-bit value */
 	if (WARN_ON_ONCE((unsigned short)tx_queue >= USHRT_MAX))
 		return;
-	sk->sk_tx_queue_mapping = tx_queue;
+	/* Paired with READ_ONCE() in sk_tx_queue_get() and
+	 * other WRITE_ONCE() because socket lock might be not held.
+	 */
+	WRITE_ONCE(sk->sk_tx_queue_mapping, tx_queue);
 }
 
 #define NO_QUEUE_MAPPING	USHRT_MAX
 
 static inline void sk_tx_queue_clear(struct sock *sk)
 {
-	sk->sk_tx_queue_mapping = NO_QUEUE_MAPPING;
+	/* Paired with READ_ONCE() in sk_tx_queue_get() and
+	 * other WRITE_ONCE() because socket lock might be not held.
+	 */
+	WRITE_ONCE(sk->sk_tx_queue_mapping, NO_QUEUE_MAPPING);
 }
 
 static inline int sk_tx_queue_get(const struct sock *sk)
 {
-	if (sk && sk->sk_tx_queue_mapping != NO_QUEUE_MAPPING)
-		return sk->sk_tx_queue_mapping;
+	if (sk) {
+		/* Paired with WRITE_ONCE() in sk_tx_queue_clear()
+		 * and sk_tx_queue_set().
+		 */
+		int val = READ_ONCE(sk->sk_tx_queue_mapping);
 
+		if (val != NO_QUEUE_MAPPING)
+			return val;
+	}
 	return -1;
 }
 
-- 
2.42.0


^ permalink raw reply related

* [PATCH AUTOSEL 4.19 5/7] net: annotate data-races around sk->sk_dst_pending_confirm
From: Sasha Levin @ 2023-11-07 12:13 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Eric Dumazet, David S . Miller, Sasha Levin, kuba, pabeni,
	dsahern, kuniyu, wuyun.abel, leitao, alexander, dhowells, netdev
In-Reply-To: <20231107121318.3759058-1-sashal@kernel.org>

From: Eric Dumazet <edumazet@google.com>

[ Upstream commit eb44ad4e635132754bfbcb18103f1dcb7058aedd ]

This field can be read or written without socket lock being held.

Add annotations to avoid load-store tearing.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 include/net/sock.h    | 6 +++---
 net/core/sock.c       | 2 +-
 net/ipv4/tcp_output.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index c0df14e5a0754..81888513b3b93 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1918,7 +1918,7 @@ static inline void dst_negative_advice(struct sock *sk)
 		if (ndst != dst) {
 			rcu_assign_pointer(sk->sk_dst_cache, ndst);
 			sk_tx_queue_clear(sk);
-			sk->sk_dst_pending_confirm = 0;
+			WRITE_ONCE(sk->sk_dst_pending_confirm, 0);
 		}
 	}
 }
@@ -1929,7 +1929,7 @@ __sk_dst_set(struct sock *sk, struct dst_entry *dst)
 	struct dst_entry *old_dst;
 
 	sk_tx_queue_clear(sk);
-	sk->sk_dst_pending_confirm = 0;
+	WRITE_ONCE(sk->sk_dst_pending_confirm, 0);
 	old_dst = rcu_dereference_protected(sk->sk_dst_cache,
 					    lockdep_sock_is_held(sk));
 	rcu_assign_pointer(sk->sk_dst_cache, dst);
@@ -1942,7 +1942,7 @@ sk_dst_set(struct sock *sk, struct dst_entry *dst)
 	struct dst_entry *old_dst;
 
 	sk_tx_queue_clear(sk);
-	sk->sk_dst_pending_confirm = 0;
+	WRITE_ONCE(sk->sk_dst_pending_confirm, 0);
 	old_dst = xchg((__force struct dst_entry **)&sk->sk_dst_cache, dst);
 	dst_release(old_dst);
 }
diff --git a/net/core/sock.c b/net/core/sock.c
index e1d0c8c715b87..62d169bcfcfa1 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -496,7 +496,7 @@ struct dst_entry *__sk_dst_check(struct sock *sk, u32 cookie)
 
 	if (dst && dst->obsolete && dst->ops->check(dst, cookie) == NULL) {
 		sk_tx_queue_clear(sk);
-		sk->sk_dst_pending_confirm = 0;
+		WRITE_ONCE(sk->sk_dst_pending_confirm, 0);
 		RCU_INIT_POINTER(sk->sk_dst_cache, NULL);
 		dst_release(dst);
 		return NULL;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 3dd62cf739e32..a0875dc60e08f 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1090,7 +1090,7 @@ static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb,
 	skb_set_hash_from_sk(skb, sk);
 	refcount_add(skb->truesize, &sk->sk_wmem_alloc);
 
-	skb_set_dst_pending_confirm(skb, sk->sk_dst_pending_confirm);
+	skb_set_dst_pending_confirm(skb, READ_ONCE(sk->sk_dst_pending_confirm));
 
 	/* Build TCP header and checksum it. */
 	th = (struct tcphdr *)skb->data;
-- 
2.42.0


^ permalink raw reply related

* [PATCH AUTOSEL 4.14 1/4] wifi: mac80211: don't return unset power in ieee80211_get_tx_power()
From: Sasha Levin @ 2023-11-07 12:13 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Ping-Ke Shih, Zong-Zhe Yang, Johannes Berg, Sasha Levin, johannes,
	davem, edumazet, kuba, pabeni, linux-wireless, netdev

From: Ping-Ke Shih <pkshih@realtek.com>

[ Upstream commit e160ab85166e77347d0cbe5149045cb25e83937f ]

We can get a UBSAN warning if ieee80211_get_tx_power() returns the
INT_MIN value mac80211 internally uses for "unset power level".

 UBSAN: signed-integer-overflow in net/wireless/nl80211.c:3816:5
 -2147483648 * 100 cannot be represented in type 'int'
 CPU: 0 PID: 20433 Comm: insmod Tainted: G        WC OE
 Call Trace:
  dump_stack+0x74/0x92
  ubsan_epilogue+0x9/0x50
  handle_overflow+0x8d/0xd0
  __ubsan_handle_mul_overflow+0xe/0x10
  nl80211_send_iface+0x688/0x6b0 [cfg80211]
  [...]
  cfg80211_register_wdev+0x78/0xb0 [cfg80211]
  cfg80211_netdev_notifier_call+0x200/0x620 [cfg80211]
  [...]
  ieee80211_if_add+0x60e/0x8f0 [mac80211]
  ieee80211_register_hw+0xda5/0x1170 [mac80211]

In this case, simply return an error instead, to indicate
that no data is available.

Cc: Zong-Zhe Yang <kevin_yang@realtek.com>
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
Link: https://lore.kernel.org/r/20230203023636.4418-1-pkshih@realtek.com
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 net/mac80211/cfg.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 94293b57f1b23..05e74004376fb 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2428,6 +2428,10 @@ static int ieee80211_get_tx_power(struct wiphy *wiphy,
 	else
 		*dbm = sdata->vif.bss_conf.txpower;
 
+	/* INT_MIN indicates no power level was set yet */
+	if (*dbm == INT_MIN)
+		return -EINVAL;
+
 	return 0;
 }
 
-- 
2.42.0


^ permalink raw reply related

* [PATCH AUTOSEL 4.14 4/4] net: annotate data-races around sk->sk_dst_pending_confirm
From: Sasha Levin @ 2023-11-07 12:13 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Eric Dumazet, David S . Miller, Sasha Levin, kuba, pabeni,
	dsahern, kuniyu, wuyun.abel, leitao, alexander, dhowells, netdev
In-Reply-To: <20231107121337.3759240-1-sashal@kernel.org>

From: Eric Dumazet <edumazet@google.com>

[ Upstream commit eb44ad4e635132754bfbcb18103f1dcb7058aedd ]

This field can be read or written without socket lock being held.

Add annotations to avoid load-store tearing.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 include/net/sock.h    | 6 +++---
 net/core/sock.c       | 2 +-
 net/ipv4/tcp_output.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 7b42ddca4decb..f974b548e1199 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1804,7 +1804,7 @@ static inline void dst_negative_advice(struct sock *sk)
 		if (ndst != dst) {
 			rcu_assign_pointer(sk->sk_dst_cache, ndst);
 			sk_tx_queue_clear(sk);
-			sk->sk_dst_pending_confirm = 0;
+			WRITE_ONCE(sk->sk_dst_pending_confirm, 0);
 		}
 	}
 }
@@ -1815,7 +1815,7 @@ __sk_dst_set(struct sock *sk, struct dst_entry *dst)
 	struct dst_entry *old_dst;
 
 	sk_tx_queue_clear(sk);
-	sk->sk_dst_pending_confirm = 0;
+	WRITE_ONCE(sk->sk_dst_pending_confirm, 0);
 	old_dst = rcu_dereference_protected(sk->sk_dst_cache,
 					    lockdep_sock_is_held(sk));
 	rcu_assign_pointer(sk->sk_dst_cache, dst);
@@ -1828,7 +1828,7 @@ sk_dst_set(struct sock *sk, struct dst_entry *dst)
 	struct dst_entry *old_dst;
 
 	sk_tx_queue_clear(sk);
-	sk->sk_dst_pending_confirm = 0;
+	WRITE_ONCE(sk->sk_dst_pending_confirm, 0);
 	old_dst = xchg((__force struct dst_entry **)&sk->sk_dst_cache, dst);
 	dst_release(old_dst);
 }
diff --git a/net/core/sock.c b/net/core/sock.c
index 5b9f51a27dc0d..e8b5742d91492 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -534,7 +534,7 @@ struct dst_entry *__sk_dst_check(struct sock *sk, u32 cookie)
 
 	if (dst && dst->obsolete && dst->ops->check(dst, cookie) == NULL) {
 		sk_tx_queue_clear(sk);
-		sk->sk_dst_pending_confirm = 0;
+		WRITE_ONCE(sk->sk_dst_pending_confirm, 0);
 		RCU_INIT_POINTER(sk->sk_dst_cache, NULL);
 		dst_release(dst);
 		return NULL;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 8b2d49120ce23..67636017f275a 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1059,7 +1059,7 @@ static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb,
 	skb_set_hash_from_sk(skb, sk);
 	refcount_add(skb->truesize, &sk->sk_wmem_alloc);
 
-	skb_set_dst_pending_confirm(skb, sk->sk_dst_pending_confirm);
+	skb_set_dst_pending_confirm(skb, READ_ONCE(sk->sk_dst_pending_confirm));
 
 	/* Build TCP header and checksum it. */
 	th = (struct tcphdr *)skb->data;
-- 
2.42.0


^ permalink raw reply related

* Re: [PATCH net] page_pool: Add myself as page pool reviewer in MAINTAINERS
From: Jesper Dangaard Brouer @ 2023-11-07 12:28 UTC (permalink / raw)
  To: Yunsheng Lin, davem, kuba, pabeni
  Cc: hawk, netdev, linux-kernel, Ilias Apalodimas
In-Reply-To: <20231107113440.59794-1-linyunsheng@huawei.com>



On 07/11/2023 12.34, Yunsheng Lin wrote:
> I have added frag support for page pool, made some improvement
> for it recently, and reviewed some related patches too.
> 

Yes, notice your frag stuff was applied while I was on vacation.
Thanks to Ilias, Jakub and other reviewers for handling this.

> So add myself as reviewer so that future patch will be cc'ed
> to my email.

I think is a good idea and I appreciate that you will review your
changes to page_pool.

There is a format issue below in patch.


> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
> CC: Jesper Dangaard Brouer <hawk@kernel.org>
> CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> CC: David S. Miller <davem@davemloft.net>
> CC: Jakub Kicinski <kuba@kernel.org>
> CC: Paolo Abeni <pabeni@redhat.com>
> CC: Netdev <netdev@vger.kernel.org>
> ---
>   MAINTAINERS | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 14e1194faa4b..5d20efb9021a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16242,6 +16242,7 @@ F:	mm/truncate.c
>   PAGE POOL
>   M:	Jesper Dangaard Brouer <hawk@kernel.org>
>   M:	Ilias Apalodimas <ilias.apalodimas@linaro.org>
> +R	Yunsheng Lin <linyunsheng@huawei.com>

I think there is missing a colon ":" after "R".

>   L:	netdev@vger.kernel.org
>   S:	Supported
>   F:	Documentation/networking/page_pool.rst

^ permalink raw reply

* [PATCH] iphase: Adding a null pointer check
From: Andrey Shumilin @ 2023-11-07 12:36 UTC (permalink / raw)
  To: 3chas3
  Cc: Andrey Shumilin, linux-atm-general, netdev, linux-kernel,
	lvc-project, khoroshilov, ykarpov, vmerzlyakov, vefanov

The pointer <dev->desc_tbl[i].iavcc> is dereferenced on line 195.
Further in the code, it is checked for null on line 204.
It is proposed to add a check before dereferencing the pointer.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Andrey Shumilin <shum.sdl@nppct.ru>
---
 drivers/atm/iphase.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/atm/iphase.c b/drivers/atm/iphase.c
index 324148686953..596422fbfacc 100644
--- a/drivers/atm/iphase.c
+++ b/drivers/atm/iphase.c
@@ -192,6 +192,11 @@ static u16 get_desc (IADEV *dev, struct ia_vcc *iavcc) {
            i++;
            continue;
         }
+       if (!(iavcc_r = dev->desc_tbl[i].iavcc)) {
+	   printk("Fatal err, desc table vcc or skb is NULL\n");
+	   i++;
+	   continue;
+	}
         ltimeout = dev->desc_tbl[i].iavcc->ltimeout; 
         delta = jiffies - dev->desc_tbl[i].timestamp;
         if (delta >= ltimeout) {
-- 
2.30.2


^ permalink raw reply related

* [PATCH net v2] page_pool: Add myself as page pool reviewer in MAINTAINERS
From: Yunsheng Lin @ 2023-11-07 12:38 UTC (permalink / raw)
  To: davem, kuba, pabeni
  Cc: netdev, linux-kernel, Yunsheng Lin, Jesper Dangaard Brouer,
	Ilias Apalodimas

I have added frag support for page pool, made some improvement
for it recently, and reviewed some related patches too.

So add myself as reviewer so that future patch will be cc'ed
to my email.

Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
CC: Jesper Dangaard Brouer <hawk@kernel.org>
CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
CC: David S. Miller <davem@davemloft.net>
CC: Jakub Kicinski <kuba@kernel.org>
CC: Paolo Abeni <pabeni@redhat.com>
CC: Netdev <netdev@vger.kernel.org>
---
V2: add missing ":" as pointed out by Jesper
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 14e1194faa4b..67817d80c9cc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16242,6 +16242,7 @@ F:	mm/truncate.c
 PAGE POOL
 M:	Jesper Dangaard Brouer <hawk@kernel.org>
 M:	Ilias Apalodimas <ilias.apalodimas@linaro.org>
+R:	Yunsheng Lin <linyunsheng@huawei.com>
 L:	netdev@vger.kernel.org
 S:	Supported
 F:	Documentation/networking/page_pool.rst
-- 
2.33.0


^ permalink raw reply related

* Re: [PATCH net] page_pool: Add myself as page pool reviewer in MAINTAINERS
From: Yunsheng Lin @ 2023-11-07 12:40 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, davem, kuba, pabeni
  Cc: netdev, linux-kernel, Ilias Apalodimas
In-Reply-To: <973bcee0-a382-4a8d-8a2c-1be9b6d9d7ad@kernel.org>

On 2023/11/7 20:28, Jesper Dangaard Brouer wrote:

>> ---
>>   MAINTAINERS | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 14e1194faa4b..5d20efb9021a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -16242,6 +16242,7 @@ F:    mm/truncate.c
>>   PAGE POOL
>>   M:    Jesper Dangaard Brouer <hawk@kernel.org>
>>   M:    Ilias Apalodimas <ilias.apalodimas@linaro.org>
>> +R    Yunsheng Lin <linyunsheng@huawei.com>
> 
> I think there is missing a colon ":" after "R".

Yes, thanks for pointing out.
Fixed and sent.

> 
>>   L:    netdev@vger.kernel.org
>>   S:    Supported
>>   F:    Documentation/networking/page_pool.rst
> 
> .
> 

^ permalink raw reply

* Re: [RFC v1 0/8] vhost-vdpa: add support for iommufd
From: Jason Gunthorpe @ 2023-11-07 12:49 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Cindy Lu, jasowang, yi.l.liu, linux-kernel, virtualization,
	netdev
In-Reply-To: <20231107022847-mutt-send-email-mst@kernel.org>

On Tue, Nov 07, 2023 at 02:30:34AM -0500, Michael S. Tsirkin wrote:
> On Sat, Nov 04, 2023 at 01:16:33AM +0800, Cindy Lu wrote:
> > 
> > Hi All
> > This code provides the iommufd support for vdpa device
> > This code fixes the bugs from the last version and also add the asid support. rebase on kernel
> > v6,6-rc3
> > Test passed in the physical device (vp_vdpa), but  there are still some problems in the emulated device (vdpa_sim_net), 
> 
> What kind of problems? Understanding that will make it easier
> to figure out whether this is worth reviewing.

IMHO, this patch series needs to spend more time internally to Red Hat
before it is presented to the community. It is too far away from
something worth reviewing at this point.

Jason

^ permalink raw reply

* Re: [RFC v1 0/8] vhost-vdpa: add support for iommufd
From: Michael S. Tsirkin @ 2023-11-07 13:23 UTC (permalink / raw)
  To: Cindy Lu; +Cc: jasowang, yi.l.liu, jgg, linux-kernel, virtualization, netdev
In-Reply-To: <20231103171641.1703146-1-lulu@redhat.com>

On Sat, Nov 04, 2023 at 01:16:33AM +0800, Cindy Lu wrote:
> Test passed in the physical device (vp_vdpa), but  there are still some problems in the emulated device (vdpa_sim_net), 

I'm not sure there's even value in bothering with iommufd for the
simulator. Just find a way to disable it and fail gracefully.

-- 
MST


^ permalink raw reply

* Re: [PATCH v8 2/2] can: esd: add support for esd GmbH PCIe/402 CAN interface family
From: Stefan Mätje @ 2023-11-07 13:27 UTC (permalink / raw)
  To: horms@kernel.org
  Cc: wg@grandegger.com, davem@davemloft.net, linux-can@vger.kernel.org,
	linux-kernel@vger.kernel.org, mkl@pengutronix.de, kuba@kernel.org,
	netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com
In-Reply-To: <20231103164839.GA714036@kernel.org>

Am Freitag, dem 03.11.2023 um 16:48 +0000 schrieb Simon Horman:
> On Wed, Oct 25, 2023 at 04:16:35PM +0200, Stefan Mätje wrote:
> > This patch adds support for the PCI based PCIe/402 CAN interface family
> > from esd GmbH that is available with various form factors
> > (https://esd.eu/en/products/402-series-can-interfaces).
> > 
> > All boards utilize a FPGA based CAN controller solution developed
> > by esd (esdACC). For more information on the esdACC see
> > https://esd.eu/en/products/esdacc.
> > 
> > This driver detects all available CAN interface board variants of
> > the family but atm. operates the CAN-FD capable devices in
> > Classic-CAN mode only! A later patch will introduce the CAN-FD
> > functionality in this driver.
> > 
> > Co-developed-by: Thomas Körper <thomas.koerper@esd.eu>
> > Signed-off-by: Thomas Körper <thomas.koerper@esd.eu>
> > Signed-off-by: Stefan Mätje <stefan.maetje@esd.eu>
> 
> ...
> 
> > +static int pci402_probe(struct pci_dev *pdev, const struct pci_device_id
> > *ent)
> > +{
> > +       struct pci402_card *card = NULL;
> > +       int err;
> > +
> > +       err = pci_enable_device(pdev);
> > +       if (err)
> > +               return err;
> > +
> > +       card = devm_kzalloc(&pdev->dev, sizeof(*card), GFP_KERNEL);
> > +       if (!card)
> 
> Hi Thomas and Stefan,
> 
> If this condition is met then the function will return err,
> but err is set to 0. Perhaps it should be set to an error value here?
> 
> Flagged by Smatch.

Hi Simon,

thank you for reviewing this. Looking at the code it is apparently wrong.

I was not aware of smatch. I got a copy and could reproduce the error report.
This will add another tool of static code analysis to my release routine.

An upgraded patch with a fix will follow.

Best regards,
    Stefan

> > +               goto failure_disable_pci;
> > +
> > +       pci_set_drvdata(pdev, card);
> > +
> > +       err = pci_request_regions(pdev, pci_name(pdev));
> > +       if (err)
> > +               goto failure_disable_pci;
> > +
> > +       card->addr = pci_iomap(pdev, PCI402_BAR, PCI402_IO_LEN_TOTAL);
> > +       if (!card->addr) {
> > +               err = -ENOMEM;
> > +               goto failure_release_regions;
> > +       }
> > +
> > +       err = pci402_init_card(pdev);
> > +       if (err)
> > +               goto failure_unmap;
> > +
> > +       err = pci402_init_dma(pdev);
> > +       if (err)
> > +               goto failure_unmap;
> > +
> > +       err = pci402_init_interrupt(pdev);
> > +       if (err)
> > +               goto failure_finish_dma;
> > +
> > +       err = pci402_init_cores(pdev);
> > +       if (err)
> > +               goto failure_finish_interrupt;
> > +
> > +       return 0;
> > +
> > +failure_finish_interrupt:
> > +       pci402_finish_interrupt(pdev);
> > +
> > +failure_finish_dma:
> > +       pci402_finish_dma(pdev);
> > +
> > +failure_unmap:
> > +       pci_iounmap(pdev, card->addr);
> > +
> > +failure_release_regions:
> > +       pci_release_regions(pdev);
> > +
> > +failure_disable_pci:
> > +       pci_disable_device(pdev);
> > +
> > +       return err;
> > +}
> 
> ...


^ permalink raw reply

* Re: [RFC v1 0/8] vhost-vdpa: add support for iommufd
From: Michael S. Tsirkin @ 2023-11-07 13:28 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Cindy Lu, jasowang, yi.l.liu, linux-kernel, virtualization,
	netdev
In-Reply-To: <20231107124902.GJ4488@nvidia.com>

On Tue, Nov 07, 2023 at 08:49:02AM -0400, Jason Gunthorpe wrote:
> On Tue, Nov 07, 2023 at 02:30:34AM -0500, Michael S. Tsirkin wrote:
> > On Sat, Nov 04, 2023 at 01:16:33AM +0800, Cindy Lu wrote:
> > > 
> > > Hi All
> > > This code provides the iommufd support for vdpa device
> > > This code fixes the bugs from the last version and also add the asid support. rebase on kernel
> > > v6,6-rc3
> > > Test passed in the physical device (vp_vdpa), but  there are still some problems in the emulated device (vdpa_sim_net), 
> > 
> > What kind of problems? Understanding that will make it easier
> > to figure out whether this is worth reviewing.
> 
> IMHO, this patch series needs to spend more time internally to Red Hat
> before it is presented to the community. It is too far away from
> something worth reviewing at this point.
> 
> Jason

I am always trying to convince people to post RFCs early
instead of working for months behind closed doors only
to be told to rewrite everything in Rust.

Why does it have to be internal to a specific company?
I see Yi Liu from Intel is helping Cindy get it into shape
and that's classic open source ethos.

I know some subsystems ignore the RFC tag but I didn't realize
iommu is one of these. Is that really true?

-- 
MST


^ permalink raw reply

* [PATCH iwl-net] ice: Restore fix disabling RX VLAN filtering
From: Marcin Szycik @ 2023-11-07 13:51 UTC (permalink / raw)
  To: intel-wired-lan; +Cc: netdev, Marcin Szycik, Wojciech Drewek

Fix setting dis_rx_filtering depending on whether port vlan is being
turned on or off. This was originally fixed in commit c793f8ea15e3 ("ice:
Fix disabling Rx VLAN filtering with port VLAN enabled"), but while
refactoring ice_vf_vsi_init_vlan_ops(), the fix has been lost. Restore the
fix along with the original comment from that change.

Also delete duplicate lines in ice_port_vlan_on().

Fixes: 2946204b3fa8 ("ice: implement bridge port vlan")
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Signed-off-by: Marcin Szycik <marcin.szycik@linux.intel.com>
---
 drivers/net/ethernet/intel/ice/ice_vf_vsi_vlan_ops.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_vf_vsi_vlan_ops.c b/drivers/net/ethernet/intel/ice/ice_vf_vsi_vlan_ops.c
index d7b10dc67f03..80dc4bcdd3a4 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_vsi_vlan_ops.c
+++ b/drivers/net/ethernet/intel/ice/ice_vf_vsi_vlan_ops.c
@@ -32,7 +32,6 @@ static void ice_port_vlan_on(struct ice_vsi *vsi)
 		/* setup outer VLAN ops */
 		vlan_ops->set_port_vlan = ice_vsi_set_outer_port_vlan;
 		vlan_ops->clear_port_vlan = ice_vsi_clear_outer_port_vlan;
-		vlan_ops->clear_port_vlan = ice_vsi_clear_outer_port_vlan;
 
 		/* setup inner VLAN ops */
 		vlan_ops = &vsi->inner_vlan_ops;
@@ -47,8 +46,13 @@ static void ice_port_vlan_on(struct ice_vsi *vsi)
 
 		vlan_ops->set_port_vlan = ice_vsi_set_inner_port_vlan;
 		vlan_ops->clear_port_vlan = ice_vsi_clear_inner_port_vlan;
-		vlan_ops->clear_port_vlan = ice_vsi_clear_inner_port_vlan;
 	}
+
+	/* all Rx traffic should be in the domain of the assigned port VLAN,
+	 * so prevent disabling Rx VLAN filtering
+	 */
+	vlan_ops->dis_rx_filtering = noop_vlan;
+
 	vlan_ops->ena_rx_filtering = ice_vsi_ena_rx_vlan_filtering;
 }
 
@@ -77,6 +81,8 @@ static void ice_port_vlan_off(struct ice_vsi *vsi)
 		vlan_ops->del_vlan = ice_vsi_del_vlan;
 	}
 
+	vlan_ops->dis_rx_filtering = ice_vsi_dis_rx_vlan_filtering;
+
 	if (!test_bit(ICE_FLAG_VF_VLAN_PRUNING, pf->flags))
 		vlan_ops->ena_rx_filtering = noop_vlan;
 	else
@@ -141,7 +147,6 @@ void ice_vf_vsi_init_vlan_ops(struct ice_vsi *vsi)
 		&vsi->outer_vlan_ops : &vsi->inner_vlan_ops;
 
 	vlan_ops->add_vlan = ice_vsi_add_vlan;
-	vlan_ops->dis_rx_filtering = ice_vsi_dis_rx_vlan_filtering;
 	vlan_ops->ena_tx_filtering = ice_vsi_ena_tx_vlan_filtering;
 	vlan_ops->dis_tx_filtering = ice_vsi_dis_tx_vlan_filtering;
 }
-- 
2.31.1


^ permalink raw reply related

* Re: [PATCH net-next v2 3/3] net: dsa: realtek: support reset controller
From: Luiz Angelo Daros de Luca @ 2023-11-07 13:54 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Vladimir Oltean, netdev, alsi, andrew, vivien.didelot, f.fainelli,
	davem, kuba, pabeni, robh+dt, krzk+dt, arinc.unal
In-Reply-To: <CACRpkdaBC7GeeGYoZ+CYjSVV657yFm=B2L6U2mNyh+AVsLbnsA@mail.gmail.com>

> > Your proposed Kconfig does not attempt to avoid a realtek-interface
> > without both interfaces or without support for both switch families.
> > Is it possible in Kconfig to force it to, at least, select one of the
> > interfaces and one of the switches? Is it okay to leave it
> > unconstrained?
>
> Can't you just remove the help text under
> NET_DSA_REALTEK_INTERFACE so it becomes a hidden
> option? The other options just select it anyway.

Without a text after the tristate, it will already be hidden. However,
we can still ask to build a module with no SMI and MDIO.

> > If merging the modules is the accepted solution, it makes me wonder if
> > rtl8365mb.ko and rtl8366.ko should get merged as well into a single
> > realtek-switch.ko. They are a hard dependency for realtek-interface.ko
> > (previously on each interface module). If the kernel is custom-built,
> > it would still be possible to exclude one switch family at build time.
>
> That's not a good idea, because we want to be able to load
> a single module into the kernel to support a single switch
> family at runtime. If you have a kernel that boots on several
> systems and some of them have one of the switches and
> some of them have another switch, I think you see the problem
> with this approach.

We already have this situation. As the interface module uses
rtl8366rb_variant and rtl8365mb_variant, we cannot select one or the
other at runtime.

rtl8365mb              14802  1 realtek_smi
rtl8366                20870  1 realtek_smi
tag_rtl4_a              1522  1

If we build it with support for both switches, both modules need to be
loaded together.

Somehow initializing the switch selectively autoloads the tag module.
Is it possible to have something like this for subdrivers?

> > I'll use these modules in OpenWrt, which builds a single kernel for a
> > bunch of devices. Is there a way to weakly depend on a module,
> > allowing the system to load only a single subdriver? Is it worth it?
>
> Last time I looked actually having DSA:s as loadable modules
> didn't work so well, so they are all compiled in. In OpenWrt
> I didn't find any DSA modules packaged as modules. But maybe
> I didn't try hard enough. IIRC the problem is that it needs to
> also have a tag module (for NET_DSA_TAG_*) and that didn't
> modularize so well.

It does work, even the tag module. As I mentioned, the tag modules are
even loaded on demand. You just need to load it in the correct
sequence.

>
> Yours,
> Linus Walleij

^ permalink raw reply

* Re: [RFC v1 0/8] vhost-vdpa: add support for iommufd
From: Jason Gunthorpe @ 2023-11-07 14:12 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Cindy Lu, jasowang, yi.l.liu, linux-kernel, virtualization,
	netdev
In-Reply-To: <20231107082343-mutt-send-email-mst@kernel.org>

On Tue, Nov 07, 2023 at 08:28:32AM -0500, Michael S. Tsirkin wrote:
> On Tue, Nov 07, 2023 at 08:49:02AM -0400, Jason Gunthorpe wrote:
> > On Tue, Nov 07, 2023 at 02:30:34AM -0500, Michael S. Tsirkin wrote:
> > > On Sat, Nov 04, 2023 at 01:16:33AM +0800, Cindy Lu wrote:
> > > > 
> > > > Hi All
> > > > This code provides the iommufd support for vdpa device
> > > > This code fixes the bugs from the last version and also add the asid support. rebase on kernel
> > > > v6,6-rc3
> > > > Test passed in the physical device (vp_vdpa), but  there are still some problems in the emulated device (vdpa_sim_net), 
> > > 
> > > What kind of problems? Understanding that will make it easier
> > > to figure out whether this is worth reviewing.
> > 
> > IMHO, this patch series needs to spend more time internally to Red Hat
> > before it is presented to the community. It is too far away from
> > something worth reviewing at this point.
> > 
> > Jason
> 
> I am always trying to convince people to post RFCs early
> instead of working for months behind closed doors only
> to be told to rewrite everything in Rust.

The community has a limited review bandwidth, things should meet a
minimum standard before there is an expectation that other people
should review it on the list.

> Why does it have to be internal to a specific company?
> I see Yi Liu from Intel is helping Cindy get it into shape
> and that's classic open source ethos.

Big company's should take the responsibility to train and provide
skill development for their own staff.

> I know some subsystems ignore the RFC tag but I didn't realize
> iommu is one of these. Is that really true?

At least I've looked at this twice now and been disappointed. Neither
have been a well thought out RFC, this is a brain dump of unfinished
work.

Jason

^ permalink raw reply

* Re: [RFC v1 0/8] vhost-vdpa: add support for iommufd
From: Michael S. Tsirkin @ 2023-11-07 14:30 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Cindy Lu, jasowang, yi.l.liu, linux-kernel, virtualization,
	netdev
In-Reply-To: <20231107141237.GO4488@nvidia.com>

On Tue, Nov 07, 2023 at 10:12:37AM -0400, Jason Gunthorpe wrote:
> Big company's should take the responsibility to train and provide
> skill development for their own staff.

That would result in a beautiful cathedral of a patch. I know this is
how some companies work. We are doing more of a bazaar thing here,
though. In a bunch of subsystems it seems that you don't get the
necessary skills until you have been publically shouted at by
maintainers - better to start early ;). Not a nice environment for
novices, for sure.

-- 
MST


^ permalink raw reply

* Re: [RFC v1 0/8] vhost-vdpa: add support for iommufd
From: Michael S. Tsirkin @ 2023-11-07 14:55 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Cindy Lu, jasowang, yi.l.liu, linux-kernel, virtualization,
	netdev
In-Reply-To: <20231107124902.GJ4488@nvidia.com>

On Tue, Nov 07, 2023 at 08:49:02AM -0400, Jason Gunthorpe wrote:
> IMHO, this patch series needs to spend more time internally to Red Hat
> before it is presented to the community.

Just to add an example why I think this "internal review" is a bad idea
I seem to recall that someone internal to nvidia at some point
attempted to implement this already. The only output from that
work we have is that "it's tough" - no pointers to what's tough,
no code to study even as a bad path to follow.
And while Red Hat might be big, the virt team is rather smaller.

-- 
MST


^ permalink raw reply

* Re: [RFC PATCH v3 00/12] Device Memory TCP
From: David Ahern @ 2023-11-07 15:18 UTC (permalink / raw)
  To: Jakub Kicinski, Paolo Abeni
  Cc: David S. Miller, Eric Dumazet, Jesper Dangaard Brouer,
	Ilias Apalodimas, Arnd Bergmann, Willem de Bruijn, Shuah Khan,
	Sumit Semwal, Christian König, Shakeel Butt, Jeroen de Borst,
	Praveen Kaligineedi, Mina Almasry, netdev, linux-kernel,
	linux-arch, linux-kselftest, linux-media, dri-devel,
	linaro-mm-sig
In-Reply-To: <20231106024413.2801438-1-almasrymina@google.com>

Is there a policy about cc'ing moderated lists on patch sets? I thought
there was, but not finding anything under Documentation/. Getting a
'needs moderator approval response' on every message is rather annoying.

^ permalink raw reply

* Re: BPF/XDP: kernel panic when removing an interface that is an xdp_redirect target
From: Toke Høiland-Jørgensen @ 2023-11-07 15:31 UTC (permalink / raw)
  To: Nelson, Shannon, Jesper Dangaard Brouer, David Ahern,
	Jakub Kicinski, netdev, bpf, Daniel Borkmann, Alexei Starovoitov,
	Andrii Nakryiko
In-Reply-To: <e3085c47-7452-4302-8401-1bda052a3714@amd.com>

"Nelson, Shannon" <shannon.nelson@amd.com> writes:

> While testing new code to support XDP in the ionic driver we found that 
> we could panic the kernel by running a bind/unbind loop on the target 
> interface of an xdp_redirect action.  Obviously this is a stress test 
> that is abusing the system, but it does point to a window of opportunity 
> in bq_enqueue() and bq_xmit_all().  I believe that while the validity of 
> the target interface has been checked in __xdp_enqueue(), the interface 
> can be unbound by the time either bq_enqueue() or bq_xmit_all() tries to 
> use the interface.  There is no locking or reference taken on the 
> interface to hold it in place before the target’s ndo_xdp_xmit() is called.
>
> Below is a stack trace that our tester captured while running our test 
> code on a RHEL 9.2 kernel – yes, I know, unpublished driver code on a 
> non-upstream kernel.  But if you look at the current upstream code in 
> kernel/bpf/devmap.c I think you can see what we ran into.
>
> Other than telling users to not abuse the system with a bind/unbind 
> loop, is there something we can do to limit the potential pain here? 
> Without knowing what interfaces might be targeted by the users’ XDP 
> programs, is there a step the originating driver can do to take 
> precautions?  Did we simply miss a step in the driver, or is this an 
> actual problem in the devmap code?

Sounds like a driver bug :)

The XDP redirect flow guarantees that all outstanding packets are
flushed within a single NAPI cycle, as documented here:
https://docs.kernel.org/bpf/redirect.html

So basically, the driver should be doing a two-step teardown: remove
global visibility of the resource in question, wait for all concurrent
users to finish, and *then* free the data structure. This corresponds to
the usual RCU protection: resources should be kept alive until all
concurrent RCU critical sections have exited on all CPUs. So if your
driver is removing an interface's data structure without waiting for
concurrent NAPI cycles to finish, that's a bug in the driver.

This kind of thing is what the synchronize_net() function is for; for a
usage example, see veth_napi_del_range(). My guess would be that you're
missing this as part of your driver teardown flow?

Another source of a bug like this could be that your driver does not in
fact call xdp_do_flush() before exiting its NAPI cycle, so that there
will be packets from the previous cycle in the bq queue, in which case
the assumption mentioned in the linked document obviously breaks down.
But that would also be a driver bug :)

-Toke


^ permalink raw reply

* [PATCH AUTOSEL 6.6 22/36] atm: iphase: Do PCI error checks on own line
From: Sasha Levin @ 2023-11-07 15:46 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Ilpo Järvinen, Bjorn Helgaas, Sasha Levin, 3chas3,
	linux-atm-general, netdev
In-Reply-To: <20231107154654.3765336-1-sashal@kernel.org>

From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

[ Upstream commit c28742447ca9879b52fbaf022ad844f0ffcd749c ]

In get_esi() PCI errors are checked inside line-split "if" conditions (in
addition to the file not following the coding style). To make the code in
get_esi() more readable, fix the coding style and use the usual error
handling pattern with a separate variable.

In addition, initialization of 'error' variable at declaration is not
needed.

No functional changes intended.

Link: https://lore.kernel.org/r/20230911125354.25501-4-ilpo.jarvinen@linux.intel.com
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/atm/iphase.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/atm/iphase.c b/drivers/atm/iphase.c
index 3241486869530..9bba8f280a4d4 100644
--- a/drivers/atm/iphase.c
+++ b/drivers/atm/iphase.c
@@ -2291,19 +2291,21 @@ static int get_esi(struct atm_dev *dev)
 static int reset_sar(struct atm_dev *dev)  
 {  
 	IADEV *iadev;  
-	int i, error = 1;  
+	int i, error;
 	unsigned int pci[64];  
 	  
 	iadev = INPH_IA_DEV(dev);  
-	for(i=0; i<64; i++)  
-	  if ((error = pci_read_config_dword(iadev->pci,  
-				i*4, &pci[i])) != PCIBIOS_SUCCESSFUL)  
-  	      return error;  
+	for (i = 0; i < 64; i++) {
+		error = pci_read_config_dword(iadev->pci, i * 4, &pci[i]);
+		if (error != PCIBIOS_SUCCESSFUL)
+			return error;
+	}
 	writel(0, iadev->reg+IPHASE5575_EXT_RESET);  
-	for(i=0; i<64; i++)  
-	  if ((error = pci_write_config_dword(iadev->pci,  
-					i*4, pci[i])) != PCIBIOS_SUCCESSFUL)  
-	    return error;  
+	for (i = 0; i < 64; i++) {
+		error = pci_write_config_dword(iadev->pci, i * 4, pci[i]);
+		if (error != PCIBIOS_SUCCESSFUL)
+			return error;
+	}
 	udelay(5);  
 	return 0;  
 }  
-- 
2.42.0


^ permalink raw reply related

* [PATCH AUTOSEL 6.6 24/36] scsi: libfc: Fix potential NULL pointer dereference in fc_lport_ptp_setup()
From: Sasha Levin @ 2023-11-07 15:46 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Wenchao Hao, Simon Horman, Martin K . Petersen, Sasha Levin, hare,
	jejb, richardcochran, linux-scsi, netdev
In-Reply-To: <20231107154654.3765336-1-sashal@kernel.org>

From: Wenchao Hao <haowenchao2@huawei.com>

[ Upstream commit 4df105f0ce9f6f30cda4e99f577150d23f0c9c5f ]

fc_lport_ptp_setup() did not check the return value of fc_rport_create()
which can return NULL and would cause a NULL pointer dereference. Address
this issue by checking return value of fc_rport_create() and log error
message on fc_rport_create() failed.

Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
Link: https://lore.kernel.org/r/20231011130350.819571-1-haowenchao2@huawei.com
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/scsi/libfc/fc_lport.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c
index 9c02c9523c4d4..ab06e9aeb613e 100644
--- a/drivers/scsi/libfc/fc_lport.c
+++ b/drivers/scsi/libfc/fc_lport.c
@@ -241,6 +241,12 @@ static void fc_lport_ptp_setup(struct fc_lport *lport,
 	}
 	mutex_lock(&lport->disc.disc_mutex);
 	lport->ptp_rdata = fc_rport_create(lport, remote_fid);
+	if (!lport->ptp_rdata) {
+		printk(KERN_WARNING "libfc: Failed to setup lport 0x%x\n",
+			lport->port_id);
+		mutex_unlock(&lport->disc.disc_mutex);
+		return;
+	}
 	kref_get(&lport->ptp_rdata->kref);
 	lport->ptp_rdata->ids.port_name = remote_wwpn;
 	lport->ptp_rdata->ids.node_name = remote_wwnn;
-- 
2.42.0


^ permalink raw reply related

* Re: [RFC v1 0/8] vhost-vdpa: add support for iommufd
From: Jason Gunthorpe @ 2023-11-07 15:48 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Cindy Lu, jasowang, yi.l.liu, linux-kernel, virtualization,
	netdev
In-Reply-To: <20231107094818-mutt-send-email-mst@kernel.org>

On Tue, Nov 07, 2023 at 09:55:26AM -0500, Michael S. Tsirkin wrote:
> On Tue, Nov 07, 2023 at 08:49:02AM -0400, Jason Gunthorpe wrote:
> > IMHO, this patch series needs to spend more time internally to Red Hat
> > before it is presented to the community.
> 
> Just to add an example why I think this "internal review" is a bad idea
> I seem to recall that someone internal to nvidia at some point
> attempted to implement this already. The only output from that
> work we have is that "it's tough" - no pointers to what's tough,
> no code to study even as a bad path to follow.
> And while Red Hat might be big, the virt team is rather smaller.

I don't think Nicolin got to a presentable code point.

But you can start to see the issues even in this series, like
simulator is complicated. mlx5 is complicated. Deciding to omit those
is one path. Come with a proposal and justification to take it out,
not a patch with an unexplained #ifdef.

Again, I'm not talking about big impactful decisions I'm saying RH
should take it internally to get a RFC proposal to the level where it
is actually an RFC proposal and not a brain dump. Make sure it has
logical commit messages, make sure the basic thinking about the idea
is done and the proposal is self consistent and explained. Make sure
the patches and series construction meet a kernel standard.

The purpose of the RFC is to clearly articulate what it is you are
asking to do, why you want to do it, and how you intend to get
there. There is still alot of basic work to achieve this and properly
communicate it.

Training to do that should rightly come from the employeer, not the
community. We've seen some big blow ups because some companies have
been trying to externalize their training to the community.

Jason

^ permalink raw reply

* [PATCH AUTOSEL 6.5 20/34] atm: iphase: Do PCI error checks on own line
From: Sasha Levin @ 2023-11-07 15:48 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Ilpo Järvinen, Bjorn Helgaas, Sasha Levin, 3chas3,
	linux-atm-general, netdev
In-Reply-To: <20231107154846.3766119-1-sashal@kernel.org>

From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

[ Upstream commit c28742447ca9879b52fbaf022ad844f0ffcd749c ]

In get_esi() PCI errors are checked inside line-split "if" conditions (in
addition to the file not following the coding style). To make the code in
get_esi() more readable, fix the coding style and use the usual error
handling pattern with a separate variable.

In addition, initialization of 'error' variable at declaration is not
needed.

No functional changes intended.

Link: https://lore.kernel.org/r/20230911125354.25501-4-ilpo.jarvinen@linux.intel.com
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/atm/iphase.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/atm/iphase.c b/drivers/atm/iphase.c
index 3241486869530..9bba8f280a4d4 100644
--- a/drivers/atm/iphase.c
+++ b/drivers/atm/iphase.c
@@ -2291,19 +2291,21 @@ static int get_esi(struct atm_dev *dev)
 static int reset_sar(struct atm_dev *dev)  
 {  
 	IADEV *iadev;  
-	int i, error = 1;  
+	int i, error;
 	unsigned int pci[64];  
 	  
 	iadev = INPH_IA_DEV(dev);  
-	for(i=0; i<64; i++)  
-	  if ((error = pci_read_config_dword(iadev->pci,  
-				i*4, &pci[i])) != PCIBIOS_SUCCESSFUL)  
-  	      return error;  
+	for (i = 0; i < 64; i++) {
+		error = pci_read_config_dword(iadev->pci, i * 4, &pci[i]);
+		if (error != PCIBIOS_SUCCESSFUL)
+			return error;
+	}
 	writel(0, iadev->reg+IPHASE5575_EXT_RESET);  
-	for(i=0; i<64; i++)  
-	  if ((error = pci_write_config_dword(iadev->pci,  
-					i*4, pci[i])) != PCIBIOS_SUCCESSFUL)  
-	    return error;  
+	for (i = 0; i < 64; i++) {
+		error = pci_write_config_dword(iadev->pci, i * 4, pci[i]);
+		if (error != PCIBIOS_SUCCESSFUL)
+			return error;
+	}
 	udelay(5);  
 	return 0;  
 }  
-- 
2.42.0


^ 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