Netdev List
 help / color / mirror / Atom feed
* [bug report] octeontx2-af: npc: cn20k: virtual index support
From: Dan Carpenter @ 2026-04-10 10:12 UTC (permalink / raw)
  To: Ratheesh Kannoth; +Cc: netdev

Hello Ratheesh Kannoth,

Commit 645c6e3c1999 ("octeontx2-af: npc: cn20k: virtual index
support") from Feb 24, 2026 (linux-next), leads to the following
Smatch static checker warning:

	drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c:3534 npc_defrag_alloc_free_slots()
	warn: missing error code 'rc'

drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
    3479 static int npc_defrag_alloc_free_slots(struct rvu *rvu,
    3480                                        struct npc_defrag_node *f,
    3481                                        int cnt, u16 *save)
    3482 {
    3483         int alloc_cnt1, alloc_cnt2;
    3484         struct npc_subbank *sb;
    3485         int rc, sb_off, i;
    3486         bool deleted;
    3487 
    3488         sb = &npc_priv.sb[f->idx];
    3489 
    3490         alloc_cnt1 = 0;
    3491         alloc_cnt2 = 0;
    3492 
    3493         rc = __npc_subbank_alloc(rvu, sb,
    3494                                  NPC_MCAM_KEY_X2, sb->b0b,
    3495                                  sb->b0t,
    3496                                  NPC_MCAM_LOWER_PRIO,
    3497                                  false, cnt, save, cnt, true,
    3498                                  &alloc_cnt1);
    3499         if (alloc_cnt1 < cnt) {
    3500                 rc = __npc_subbank_alloc(rvu, sb,
    3501                                          NPC_MCAM_KEY_X2, sb->b1b,
    3502                                          sb->b1t,
    3503                                          NPC_MCAM_LOWER_PRIO,
    3504                                          false, cnt - alloc_cnt1,
    3505                                          save + alloc_cnt1,
    3506                                          cnt - alloc_cnt1,
    3507                                          true, &alloc_cnt2);
    3508         }
    3509 
    3510         if (alloc_cnt1 + alloc_cnt2 != cnt) {
    3511                 dev_err(rvu->dev,
    3512                         "%s: Failed to alloc cnt=%u alloc_cnt1=%u alloc_cnt2=%u\n",
    3513                         __func__, cnt, alloc_cnt1, alloc_cnt2);
    3514                 goto fail_free_alloc;
    3515         }
    3516         return 0;
    3517 
    3518 fail_free_alloc:
    3519         for (i = 0; i < alloc_cnt1 + alloc_cnt2; i++) {
    3520                 rc =  npc_mcam_idx_2_subbank_idx(rvu, save[i],
    3521                                                  &sb, &sb_off);
    3522                 if (rc) {
    3523                         dev_err(rvu->dev,
    3524                                 "%s: Error to find subbank for mcam idx=%u\n",
    3525                                 __func__, save[i]);
    3526                         break;
    3527                 }
    3528 
    3529                 deleted = __npc_subbank_free(rvu, sb, sb_off);
    3530                 if (!deleted) {
    3531                         dev_err(rvu->dev,
    3532                                 "%s: Error to free mcam idx=%u\n",
    3533                                 __func__, save[i]);
--> 3534                         break;

Set an error code here?

    3535                 }
    3536         }
    3537 
    3538         return rc;
    3539 }

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter

^ permalink raw reply

* [bug report] octeontx2-af: npc: cn20k: add debugfs support
From: Dan Carpenter @ 2026-04-10 10:12 UTC (permalink / raw)
  To: Ratheesh Kannoth; +Cc: netdev

Hello Ratheesh Kannoth,

Commit 528530dff56b ("octeontx2-af: npc: cn20k: add debugfs support")
from Feb 24, 2026 (linux-next), leads to the following Smatch static
checker warning:

drivers/net/ethernet/marvell/octeontx2/af/cn20k/debugfs.c:257 npc_cn20k_debugfs_init() warn: 'npc_dentry' is an error pointer or valid
drivers/net/ethernet/marvell/octeontx2/af/cn20k/debugfs.c:263 npc_cn20k_debugfs_init() warn: 'npc_dentry' is an error pointer or valid
drivers/net/ethernet/marvell/octeontx2/af/cn20k/debugfs.c:268 npc_cn20k_debugfs_init() warn: 'npc_dentry' is an error pointer or valid
drivers/net/ethernet/marvell/octeontx2/af/cn20k/debugfs.c:273 npc_cn20k_debugfs_init() warn: 'npc_dentry' is an error pointer or valid
drivers/net/ethernet/marvell/octeontx2/af/cn20k/debugfs.c:278 npc_cn20k_debugfs_init() warn: 'npc_dentry' is an error pointer or valid

drivers/net/ethernet/marvell/octeontx2/af/cn20k/debugfs.c
    249 int npc_cn20k_debugfs_init(struct rvu *rvu)
    250 {
    251         struct npc_priv_t *npc_priv = npc_priv_get();
    252         struct dentry *npc_dentry;
    253 
    254         npc_dentry = debugfs_create_file("mcam_layout", 0444, rvu->rvu_dbg.npc,
    255                                          npc_priv, &npc_mcam_layout_fops);
    256 
    257         if (!npc_dentry)
    258                 return -EFAULT;

This error checking is wrong, but instead of fixing it, just delete it.
See my blog for details:

https://staticthinking.wordpress.com/2023/07/24/debugfs-functions-are-not-supposed-to-be-checked/

    259 
    260         npc_dentry = debugfs_create_file("mcam_default", 0444, rvu->rvu_dbg.npc,
    261                                          rvu, &npc_mcam_default_fops);
    262 
    263         if (!npc_dentry)
    264                 return -EFAULT;
    265 
    266         npc_dentry = debugfs_create_file("vidx2idx", 0444, rvu->rvu_dbg.npc,
    267                                          npc_priv, &npc_vidx2idx_map_fops);
    268         if (!npc_dentry)
    269                 return -EFAULT;
    270 
    271         npc_dentry = debugfs_create_file("idx2vidx", 0444, rvu->rvu_dbg.npc,
    272                                          npc_priv, &npc_idx2vidx_map_fops);
    273         if (!npc_dentry)
    274                 return -EFAULT;
    275 
    276         npc_dentry = debugfs_create_file("defrag", 0444, rvu->rvu_dbg.npc,
    277                                          npc_priv, &npc_defrag_fops);
--> 278         if (!npc_dentry)
    279                 return -EFAULT;
    280 
    281         return 0;
    282 }

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter

^ permalink raw reply

* Re: [net-next PATCH v5 1/4] octeontx2-af: npa: cn20k: Add NPA Halo support
From: Subbaraya Sundeep @ 2026-04-10 10:11 UTC (permalink / raw)
  To: Alexander Lobakin
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, sgoutham, gakula,
	bbhushan2, netdev, linux-kernel, Linu Cherian
In-Reply-To: <1dc56269-d6d5-48da-a4c2-0686ce4fd1f6@intel.com>

On 2026-04-10 at 15:06:56, Alexander Lobakin (aleksander.lobakin@intel.com) wrote:
> From: Subbaraya Sundeep <sbhatta@marvell.com>
> Date: Fri, 10 Apr 2026 15:05:36 +0530
> 
> > On 2026-04-09 at 20:39:02, Alexander Lobakin (aleksander.lobakin@intel.com) wrote:
> >> From: Subbaraya Sundeep <sbhatta@marvell.com>
> >> Date: Thu, 9 Apr 2026 15:23:21 +0530
> >>
> >>> From: Linu Cherian <lcherian@marvell.com>
> >>>
> >>> CN20K silicon implements unified aura and pool context
> >>> type called Halo for better resource usage. Add support to
> >>> handle Halo context type operations.
> >>>
> >>> Signed-off-by: Linu Cherian <lcherian@marvell.com>
> >>> Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
> >>
> >> [...]
> >>
> >>> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/struct.h b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/struct.h
> >>> index 763f6cabd7c2..2364bafd329d 100644
> >>> --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/struct.h
> >>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/struct.h
> >>> @@ -377,4 +377,85 @@ struct npa_cn20k_pool_s {
> >>>  
> >>>  static_assert(sizeof(struct npa_cn20k_pool_s) == NIX_MAX_CTX_SIZE);
> >>>  
> >>> +struct npa_cn20k_halo_s {
> >>> +	u64 stack_base                  : 64;
> >>
> >> It's redundant to add : 64 to a 64-bit field.
> >  Agreed. But this is for readability, it helps when checking HRM. For
> >  instance HRM says [703:640] and we define as u64 reserved_640_703 : 64;
> >  so that we do not have to count bits in mind.
> >> Moreover, on 32-bit systems, the compilers sometimes complain on
> >> bitfields > 32 bits.
> >  This driver depends on 64BIT.
> >>
> >>> +	u64 ena                         :  1;
> >>> +	u64 nat_align                   :  1;
> >>> +	u64 reserved_66_67              :  2;
> >>> +	u64 stack_caching               :  1;
> >>> +	u64 reserved_69_71              :  3;
> >>> +	u64 aura_drop_ena               :  1;
> >>> +	u64 reserved_73_79              :  7;
> >>> +	u64 aura_drop                   :  8;
> >>> +	u64 buf_offset                  : 12;
> >>> +	u64 reserved_100_103            :  4;
> >>> +	u64 buf_size                    : 12;
> >>> +	u64 reserved_116_119            :  4;
> >>> +	u64 ref_cnt_prof                :  3;
> >>> +	u64 reserved_123_127            :  5;
> >>> +	u64 stack_max_pages             : 32;
> >>> +	u64 stack_pages                 : 32;
> >>> +	u64 bp_0                        :  7;
> >>> +	u64 bp_1                        :  7;
> >>> +	u64 bp_2                        :  7;
> >>> +	u64 bp_3                        :  7;
> >>> +	u64 bp_4                        :  7;
> >>> +	u64 bp_5                        :  7;
> >>> +	u64 bp_6                        :  7;
> >>> +	u64 bp_7                        :  7;
> >>> +	u64 bp_ena_0                    :  1;
> >>> +	u64 bp_ena_1                    :  1;
> >>> +	u64 bp_ena_2                    :  1;
> >>> +	u64 bp_ena_3                    :  1;
> >>> +	u64 bp_ena_4                    :  1;
> >>> +	u64 bp_ena_5                    :  1;
> >>> +	u64 bp_ena_6                    :  1;
> >>> +	u64 bp_ena_7                    :  1;
> >>> +	u64 stack_offset                :  4;
> >>> +	u64 reserved_260_263            :  4;
> >>> +	u64 shift                       :  6;
> >>> +	u64 reserved_270_271            :  2;
> >>> +	u64 avg_level                   :  8;
> >>> +	u64 avg_con                     :  9;
> >>> +	u64 fc_ena                      :  1;
> >>> +	u64 fc_stype                    :  2;
> >>> +	u64 fc_hyst_bits                :  4;
> >>> +	u64 fc_up_crossing              :  1;
> >>> +	u64 reserved_297_299            :  3;
> >>> +	u64 update_time                 : 16;
> >>> +	u64 reserved_316_319            :  4;
> >>> +	u64 fc_addr                     : 64;
> >>> +	u64 ptr_start                   : 64;
> >>> +	u64 ptr_end                     : 64;
> >>> +	u64 bpid_0                      : 12;
> >>> +	u64 reserved_524_535            : 12;
> >>> +	u64 err_int                     :  8;
> >>> +	u64 err_int_ena                 :  8;
> >>> +	u64 thresh_int                  :  1;
> >>> +	u64 thresh_int_ena              :  1;
> >>> +	u64 thresh_up                   :  1;
> >>> +	u64 reserved_555                :  1;
> >>> +	u64 thresh_qint_idx             :  7;
> >>> +	u64 reserved_563                :  1;
> >>> +	u64 err_qint_idx                :  7;
> >>> +	u64 reserved_571_575            :  5;
> >>> +	u64 thresh                      : 36;
> >>> +	u64 reserved_612_615            :  4;
> >>> +	u64 fc_msh_dst                  : 11;
> >>> +	u64 reserved_627_630            :  4;
> >>> +	u64 op_dpc_ena                  :  1;
> >>> +	u64 op_dpc_set                  :  5;
> >>> +	u64 reserved_637_637            :  1;
> >>> +	u64 stream_ctx                  :  1;
> >>> +	u64 unified_ctx                 :  1;
> >>> +	u64 reserved_640_703            : 64;
> >>> +	u64 reserved_704_767            : 64;
> >>> +	u64 reserved_768_831            : 64;
> >>> +	u64 reserved_832_895            : 64;
> >>> +	u64 reserved_896_959            : 64;
> >>> +	u64 reserved_960_1023           : 64;
> >>> +};
> >>> +
> >>> +static_assert(sizeof(struct npa_cn20k_halo_s) == NIX_MAX_CTX_SIZE);
> >>
> >> Now the main question:
> >>
> >> Is mailbox's Endianness fixed (LE/BE)? Or is it always the same as the
> >> host's ones (I doubt so)?
> >> If not, these need to be __le{8,16,32,64} (or __be if it's Big Endian)
> >> and you need to handle the conversions manually.
> >>
> >  Yes endianness is LE and fixed. This is NOT a host side driver for an
> >  endpoint card. This is driver for on chip PCI device of CN20K soc.
> >  Hope I answered your question wrt host.
> 
> But the mailbox is shared between the SoC and the host or HW or not? Is
 In hardware it is just shared DDR region between two on chip devices and both
 devices access shared region using their BARs. 
> it possible that one client of the mailbox will have LE and the second
> will have BE?
 No not possible.

 Thanks,
 Sundeep
> 
> > 
> >  Thanks,
> >  Sundeep
> 
> Thanks,
> Olek

^ permalink raw reply

* [PATCH v3] selftests: vsock: avoid races creating Unix socket paths
From: Cao Ruichuang @ 2026-04-10 10:07 UTC (permalink / raw)
  To: Stefano Garzarella, Shuah Khan
  Cc: Stefano Garzarella, Simon Horman, Bobby Eshleman, virtualization,
	netdev, linux-kselftest, linux-kernel
In-Reply-To: <20260410035237.59644-1-create0818@163.com>

vmtest.sh currently uses mktemp -u to precompute Unix socket paths for the
namespace bridge helpers. That only returns an unused pathname and leaves a
time-of-check/time-of-use window before socat binds or connects to it.

Create a private temporary directory with mktemp -d and place the
socket path inside it instead. This removes the pathname race while
keeping cleanup straightforward.

Signed-off-by: Cao Ruichuang <create0818@163.com>
---
v3:
- restore the missing patch description
- add Bobby Eshleman to Cc

 tools/testing/selftests/vsock/vmtest.sh | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
index 86e338886b3..c345fa539d3 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -718,6 +718,7 @@ test_ns_diff_global_host_connect_to_global_vm_ok() {
 	local pids pid pidfile
 	local ns0 ns1 port
 	declare -a pids
+	local unixdir
 	local unixfile
 	ns0="global0"
 	ns1="global1"
@@ -736,7 +737,8 @@ test_ns_diff_global_host_connect_to_global_vm_ok() {
 	oops_before=$(vm_dmesg_oops_count "${ns0}")
 	warn_before=$(vm_dmesg_warn_count "${ns0}")
 
-	unixfile=$(mktemp -u /tmp/XXXX.sock)
+	unixdir=$(mktemp -d /tmp/vsock_vmtest_XXXXXX)
+	unixfile="${unixdir}/sock"
 	ip netns exec "${ns1}" \
 		socat TCP-LISTEN:"${TEST_HOST_PORT}",fork \
 			UNIX-CONNECT:"${unixfile}" &
@@ -758,6 +760,8 @@ test_ns_diff_global_host_connect_to_global_vm_ok() {
 
 	terminate_pids "${pids[@]}"
 	terminate_pidfiles "${pidfile}"
+	rm "${unixfile}"
+	rmdir "${unixdir}"
 
 	if [[ "${rc}" -ne 0 ]] || [[ "${dmesg_rc}" -ne 0 ]]; then
 		return "${KSFT_FAIL}"
@@ -814,6 +818,7 @@ test_ns_diff_global_vm_connect_to_global_host_ok() {
 	local ns0="global0"
 	local ns1="global1"
 	local port=12345
+	local unixdir
 	local unixfile
 	local dmesg_rc
 	local pidfile
@@ -826,7 +831,8 @@ test_ns_diff_global_vm_connect_to_global_host_ok() {
 
 	log_host "Setup socat bridge from ns ${ns0} to ns ${ns1} over port ${port}"
 
-	unixfile=$(mktemp -u /tmp/XXXX.sock)
+	unixdir=$(mktemp -d /tmp/vsock_vmtest_XXXXXX)
+	unixfile="${unixdir}/sock"
 
 	ip netns exec "${ns0}" \
 		socat TCP-LISTEN:"${port}" UNIX-CONNECT:"${unixfile}" &
@@ -845,7 +851,8 @@ test_ns_diff_global_vm_connect_to_global_host_ok() {
 	if ! vm_start "${pidfile}" "${ns0}"; then
 		log_host "failed to start vm (cid=${cid}, ns=${ns0})"
 		terminate_pids "${pids[@]}"
-		rm -f "${unixfile}"
+		rm "${unixfile}"
+		rmdir "${unixdir}"
 		return "${KSFT_FAIL}"
 	fi
 
@@ -862,7 +869,8 @@ test_ns_diff_global_vm_connect_to_global_host_ok() {
 
 	terminate_pidfiles "${pidfile}"
 	terminate_pids "${pids[@]}"
-	rm -f "${unixfile}"
+	rm "${unixfile}"
+	rmdir "${unixdir}"
 
 	if [[ "${rc}" -ne 0 ]] || [[ "${dmesg_rc}" -ne 0 ]]; then
 		return "${KSFT_FAIL}"
-- 
2.39.5 (Apple Git-154)


^ permalink raw reply related

* Re: [PATCH v2] selftests: vsock: avoid races creating Unix socket paths
From: Cao Ruichuang @ 2026-04-10 10:05 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: Stefano Garzarella, Simon Horman, Shuah Khan, Bobby Eshleman,
	virtualization, netdev, linux-kselftest, linux-kernel
In-Reply-To: <adi1X_3wVUsDhRmR@sgarzare-redhat>

Sorry, that was my mistake.

I accidentally sent v2 without the patch description. I will resend it as v3
with the proper changelog and Cc Bobby as suggested.

Thanks for catching this.


^ permalink raw reply

* Re: [PATCH v2 1/2] mfd: nct6694: Switch to devm_mfd_add_devices() and drop IDA
From: Ming Yu @ 2026-04-10  9:59 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: tmyu0, linusw, linux, andi.shyti, lee, mkl, mailhol,
	alexandre.belloni, wim, linux-kernel, linux-gpio, linux-i2c,
	linux-can, netdev, linux-watchdog, linux-hwmon, linux-rtc,
	linux-usb
In-Reply-To: <CAMRc=MeJL_po8HuBa4btVowR-e0i2FyzbDgNVo2u54iPKyuvWw@mail.gmail.com>

Hi Bart, all,

Thanks for the review.

Bartosz Golaszewski <brgl@kernel.org> 於 2026年4月8日週三 下午3:25寫道:
>
> On Wed, Apr 8, 2026 at 7:31 AM <a0282524688@gmail.com> wrote:
> >
> > From: Ming Yu <a0282524688@gmail.com>
> >
> > Currently, the nct6694 core driver uses mfd_add_hotplug_devices()
> > and an IDA to manage subdevice IDs.
> >
> > Switch the core implementation to use the managed
> > devm_mfd_add_devices() API, which simplifies the error handling and
> > device lifecycle management. Concurrently, drop the custom IDA
> > implementation and transition to using pdev->id.
> >
> > Signed-off-by: Ming Yu <a0282524688@gmail.com>
> > ---
>
> This does result in a nice code shrink but I'd split this commit into
> two: one switching to using MFD_CELL_BASIC() with hard-coded devices
> IDs and one completing the transition to devres.
>


You are right that this change is trying to do too much at once, and
splitting it as you suggested would make the series much cleaner.

After looking more closely at the ID handling and hotplug
implications, I realized that switching to devm_mfd_add_devices() and
dropping the IDA is not a good fit for this driver. The current
mfd_add_hotplug_devices() path uses PLATFORM_DEVID_AUTO, which gives
globally unique device IDs and avoids sysfs name collisions. If we
switch to devm_mfd_add_devices() with fixed IDs, multiple identical
NCT6694 devices can end up registering subdevices with the same
platform device names, which would break hotplug support when more
than one device is present.

So I think it is better not to pursue this direction further.

For the next revision, I will drop this part of the change and keep
the existing MFD core logic, including the IDA usage. The series will
focus on adding the nct6694-hif MFD driver only, and I will add the
IDA initialization there as needed.

Thanks again for the suggestion and review.


Best regards,
Ming

^ permalink raw reply

* Re: [bug report] octeontx2-af: npc: cn20k: Use common APIs
From: Ratheesh Kannoth @ 2026-04-10  9:52 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Suman Ghosh, netdev
In-Reply-To: <adiQJvuKlEhq2ILx@stanley.mountain>

On 2026-04-10 at 11:22:38, Dan Carpenter (error27@gmail.com) wrote:
> Hello Suman Ghosh,
>
> Commit 6d1e70282f76 ("octeontx2-af: npc: cn20k: Use common APIs")
> from Feb 24, 2026 (linux-next), leads to the following Smatch static
> checker warning:
>
> 	drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c:1065 npc_cn20k_config_mcam_entry()
> 	error: uninitialized symbol 'kw_type'.
>
> drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
>     1045 void npc_cn20k_config_mcam_entry(struct rvu *rvu, int blkaddr, int index,
>     1046                                  u8 intf, struct cn20k_mcam_entry *entry,
>     1047                                  bool enable, u8 hw_prio, u8 req_kw_type)
>     1048 {
>     1049         struct npc_mcam *mcam = &rvu->hw->mcam;
>     1050         int mcam_idx = index % mcam->banksize;
>     1051         int bank = index / mcam->banksize;
>     1052         int kw = 0;
>     1053         u8 kw_type;
>     1054
>     1055         /* Disable before mcam entry update */
>     1056         npc_cn20k_enable_mcam_entry(rvu, blkaddr, index, false);
>     1057
>     1058         npc_mcam_idx_2_key_type(rvu, index, &kw_type);
>
> No error checking?
ACK. will push
>
>     1059         /* CAM1 takes the comparison value and
>     1060          * CAM0 specifies match for a bit in key being '0' or '1' or 'dontcare'.
>     1061          * CAM1<n> = 0 & CAM0<n> = 1 => match if key<n> = 0
>     1062          * CAM1<n> = 1 & CAM0<n> = 0 => match if key<n> = 1
>     1063          * CAM1<n> = 0 & CAM0<n> = 0 => always match i.e dontcare.
>     1064          */
> --> 1065         if (kw_type == NPC_MCAM_KEY_X2) {
>                      ^^^^^^^
kw_type is set by npc_mcam_idx_2_key_type() call above.
> Uninitialized
>

^ permalink raw reply

* Re: [PATCH net] ice: fix netdev bring-up and bring-down in self-test
From: Alexander Lobakin @ 2026-04-10  9:42 UTC (permalink / raw)
  To: Tony Nguyen, Jakub Kicinski
  Cc: Aleksandr Loktionov, intel-wired-lan, netdev, Konstantin Ilichev,
	Grzegorz Nitka
In-Reply-To: <c5f6796a-66de-40df-9f12-2bc7ff04be34@intel.com>

From: Anthony Nguyen <anthony.l.nguyen@intel.com>
Date: Wed, 8 Apr 2026 14:12:28 -0700

> 
> 
> On 3/27/2026 12:23 AM, Aleksandr Loktionov wrote:
>> From: Konstantin Ilichev <konstantin.ilichev@intel.com>
>>
>> When an offline self-test is initiated with ethtool -t, any ongoing
>> traffic could get stuck because ice_stop() and ice_open() are called
>> without letting the OS know about state transitions.  In most cases
>> a write() system call would block.
>>
>> Fix this by calling dev_change_flags() to bring the netdev up and
>> down, which ensures ndo_open()/ndo_stop() are called and all watchers
>> are notified correctly.
> 
> + Olek
> 
> AI review reports:
> 
> The ethtool core acquires the per-netdev mutex via netdev_lock_ops(dev)
> before invoking the driver's .self_test callback. dev_change_flags() is
> an exported API that explicitly re-acquires this exact same lock:
> net/core/dev_api.c:dev_change_flags() {
>     ...
>     netdev_lock_ops(dev);
>     ret = netif_change_flags(dev, flags, extack);
>     netdev_unlock_ops(dev);
>     ...
> }
> Because dev->lock is a standard, non-recursive mutex, this will result
> in a hard deadlock for any driver that opts into request_ops_lock. While
> ice might not currently set this flag, introducing nested lock
> acquisitions of the same mutex guarantees a deadlock as the subsystem
> migrates toward per-netdev locking.
> 
> 
> With ice netdev lock changes in progress [1], this would soon become an
> issue.

Hmmm, seems like we'll need to either export netif_change_flags() or
introduce something like dev_change_flags_locked()...

> 
> Thanks,
> Tony
> 
> [1] https://lore.kernel.org/netdev/20260325200644.2528726-4-
> anthony.l.nguyen@intel.com/
> 
>> Fixes: 0e674aeb0b77 ("ice: Add handler for ethtool selftest")
>> Cc: stable@vger.kernel.org
>> Co-developed-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
>> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
>> Signed-off-by: Konstantin Ilichev <konstantin.ilichev@intel.com>
>> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>> ---
>>
>>   drivers/net/ethernet/intel/ice/ice_ethtool.c | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/
>> net/ethernet/intel/ice/ice_ethtool.c
>> index 96d95af..2a4f06f 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
>> @@ -1416,7 +1416,7 @@ ice_self_test(struct net_device *netdev, struct
>> ethtool_test *eth_test,
>>           /* If the device is online then take it offline */
>>           if (if_running)
>>               /* indicate we're in test mode */
>> -            ice_stop(netdev);
>> +            dev_change_flags(netdev, netdev->flags & ~IFF_UP, NULL);
>>             data[ICE_ETH_TEST_LINK] = ice_link_test(netdev);
>>           data[ICE_ETH_TEST_EEPROM] = ice_eeprom_test(netdev);
>> @@ -1434,10 +1434,12 @@ ice_self_test(struct net_device *netdev,
>> struct ethtool_test *eth_test,
>>           clear_bit(ICE_TESTING, pf->state);
>>             if (if_running) {
>> -            int status = ice_open(netdev);
>> +            int status = dev_change_flags(netdev,
>> +                              netdev->flags | IFF_UP,
>> +                              NULL);

BTW are you sure there's no other way to do an ifdown/ifup cycle other
than sneaking into the flags? Have you checked whether there's an
exported generic function which calls .ndo_open()?

>>                 if (status) {
>> -                dev_err(dev, "Could not open device %s, err %d\n",
>> +                dev_err(dev, "Could not bring up device %s, err %d\n",
>>                       pf->int_name, status);
>>               }
>>           }

Thanks,
Olek

^ permalink raw reply

* [PATCH net-next] ppp: tear down bridge before clearing pch->chan
From: Qingfang Deng @ 2026-04-10  9:38 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Qingfang Deng, Yue Haibing, Kuniyuki Iwashima,
	Kees Cook, Sebastian Andrzej Siewior, linux-ppp, netdev,
	linux-kernel
  Cc: Tom Parkin, James Chapman, Guillaume Nault

As we previously did to ppp_disconnect_channel(), also move
ppp_unbridge_channels() before pch->chan is set to NULL in
ppp_unregister_channel().

ppp_unbridge_channels() calls synchronize_rcu(), so no concurrent RCU
readers in ppp_channel_bridge_input() can observe the channel after its
chan pointer is cleared.

This makes the !pchb->chan check in ppp_channel_bridge_input()
redundant and can be safely removed.

Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
---
 drivers/net/ppp/ppp_generic.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index b097d1b38ac9..3a609d48a424 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -2285,17 +2285,11 @@ static bool ppp_channel_bridge_input(struct channel *pch, struct sk_buff *skb)
 		goto out_rcu;
 
 	spin_lock_bh(&pchb->downl);
-	if (!pchb->chan) {
-		/* channel got unregistered */
-		kfree_skb(skb);
-		goto outl;
-	}
 
 	skb_scrub_packet(skb, !net_eq(pch->chan_net, pchb->chan_net));
 	if (!pchb->chan->ops->start_xmit(pchb->chan, skb))
 		kfree_skb(skb);
 
-outl:
 	spin_unlock_bh(&pchb->downl);
 out_rcu:
 	rcu_read_unlock();
@@ -2997,6 +2991,8 @@ ppp_unregister_channel(struct ppp_channel *chan)
 	 * the channel's start_xmit or ioctl routine before we proceed.
 	 */
 	ppp_disconnect_channel(pch);
+	ppp_unbridge_channels(pch);
+
 	down_write(&pch->chan_sem);
 	spin_lock_bh(&pch->downl);
 	pch->chan = NULL;
@@ -3008,8 +3004,6 @@ ppp_unregister_channel(struct ppp_channel *chan)
 	list_del(&pch->list);
 	spin_unlock_bh(&pn->all_channels_lock);
 
-	ppp_unbridge_channels(pch);
-
 	pch->file.dead = 1;
 	wake_up_interruptible(&pch->file.rwait);
 
-- 
2.43.0


^ permalink raw reply related

* Re: [net-next PATCH v5 1/4] octeontx2-af: npa: cn20k: Add NPA Halo support
From: Alexander Lobakin @ 2026-04-10  9:36 UTC (permalink / raw)
  To: Subbaraya Sundeep
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, sgoutham, gakula,
	bbhushan2, netdev, linux-kernel, Linu Cherian
In-Reply-To: <20260410093536.GA1783667@kernel-ep2>

From: Subbaraya Sundeep <sbhatta@marvell.com>
Date: Fri, 10 Apr 2026 15:05:36 +0530

> On 2026-04-09 at 20:39:02, Alexander Lobakin (aleksander.lobakin@intel.com) wrote:
>> From: Subbaraya Sundeep <sbhatta@marvell.com>
>> Date: Thu, 9 Apr 2026 15:23:21 +0530
>>
>>> From: Linu Cherian <lcherian@marvell.com>
>>>
>>> CN20K silicon implements unified aura and pool context
>>> type called Halo for better resource usage. Add support to
>>> handle Halo context type operations.
>>>
>>> Signed-off-by: Linu Cherian <lcherian@marvell.com>
>>> Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
>>
>> [...]
>>
>>> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/struct.h b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/struct.h
>>> index 763f6cabd7c2..2364bafd329d 100644
>>> --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/struct.h
>>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/struct.h
>>> @@ -377,4 +377,85 @@ struct npa_cn20k_pool_s {
>>>  
>>>  static_assert(sizeof(struct npa_cn20k_pool_s) == NIX_MAX_CTX_SIZE);
>>>  
>>> +struct npa_cn20k_halo_s {
>>> +	u64 stack_base                  : 64;
>>
>> It's redundant to add : 64 to a 64-bit field.
>  Agreed. But this is for readability, it helps when checking HRM. For
>  instance HRM says [703:640] and we define as u64 reserved_640_703 : 64;
>  so that we do not have to count bits in mind.
>> Moreover, on 32-bit systems, the compilers sometimes complain on
>> bitfields > 32 bits.
>  This driver depends on 64BIT.
>>
>>> +	u64 ena                         :  1;
>>> +	u64 nat_align                   :  1;
>>> +	u64 reserved_66_67              :  2;
>>> +	u64 stack_caching               :  1;
>>> +	u64 reserved_69_71              :  3;
>>> +	u64 aura_drop_ena               :  1;
>>> +	u64 reserved_73_79              :  7;
>>> +	u64 aura_drop                   :  8;
>>> +	u64 buf_offset                  : 12;
>>> +	u64 reserved_100_103            :  4;
>>> +	u64 buf_size                    : 12;
>>> +	u64 reserved_116_119            :  4;
>>> +	u64 ref_cnt_prof                :  3;
>>> +	u64 reserved_123_127            :  5;
>>> +	u64 stack_max_pages             : 32;
>>> +	u64 stack_pages                 : 32;
>>> +	u64 bp_0                        :  7;
>>> +	u64 bp_1                        :  7;
>>> +	u64 bp_2                        :  7;
>>> +	u64 bp_3                        :  7;
>>> +	u64 bp_4                        :  7;
>>> +	u64 bp_5                        :  7;
>>> +	u64 bp_6                        :  7;
>>> +	u64 bp_7                        :  7;
>>> +	u64 bp_ena_0                    :  1;
>>> +	u64 bp_ena_1                    :  1;
>>> +	u64 bp_ena_2                    :  1;
>>> +	u64 bp_ena_3                    :  1;
>>> +	u64 bp_ena_4                    :  1;
>>> +	u64 bp_ena_5                    :  1;
>>> +	u64 bp_ena_6                    :  1;
>>> +	u64 bp_ena_7                    :  1;
>>> +	u64 stack_offset                :  4;
>>> +	u64 reserved_260_263            :  4;
>>> +	u64 shift                       :  6;
>>> +	u64 reserved_270_271            :  2;
>>> +	u64 avg_level                   :  8;
>>> +	u64 avg_con                     :  9;
>>> +	u64 fc_ena                      :  1;
>>> +	u64 fc_stype                    :  2;
>>> +	u64 fc_hyst_bits                :  4;
>>> +	u64 fc_up_crossing              :  1;
>>> +	u64 reserved_297_299            :  3;
>>> +	u64 update_time                 : 16;
>>> +	u64 reserved_316_319            :  4;
>>> +	u64 fc_addr                     : 64;
>>> +	u64 ptr_start                   : 64;
>>> +	u64 ptr_end                     : 64;
>>> +	u64 bpid_0                      : 12;
>>> +	u64 reserved_524_535            : 12;
>>> +	u64 err_int                     :  8;
>>> +	u64 err_int_ena                 :  8;
>>> +	u64 thresh_int                  :  1;
>>> +	u64 thresh_int_ena              :  1;
>>> +	u64 thresh_up                   :  1;
>>> +	u64 reserved_555                :  1;
>>> +	u64 thresh_qint_idx             :  7;
>>> +	u64 reserved_563                :  1;
>>> +	u64 err_qint_idx                :  7;
>>> +	u64 reserved_571_575            :  5;
>>> +	u64 thresh                      : 36;
>>> +	u64 reserved_612_615            :  4;
>>> +	u64 fc_msh_dst                  : 11;
>>> +	u64 reserved_627_630            :  4;
>>> +	u64 op_dpc_ena                  :  1;
>>> +	u64 op_dpc_set                  :  5;
>>> +	u64 reserved_637_637            :  1;
>>> +	u64 stream_ctx                  :  1;
>>> +	u64 unified_ctx                 :  1;
>>> +	u64 reserved_640_703            : 64;
>>> +	u64 reserved_704_767            : 64;
>>> +	u64 reserved_768_831            : 64;
>>> +	u64 reserved_832_895            : 64;
>>> +	u64 reserved_896_959            : 64;
>>> +	u64 reserved_960_1023           : 64;
>>> +};
>>> +
>>> +static_assert(sizeof(struct npa_cn20k_halo_s) == NIX_MAX_CTX_SIZE);
>>
>> Now the main question:
>>
>> Is mailbox's Endianness fixed (LE/BE)? Or is it always the same as the
>> host's ones (I doubt so)?
>> If not, these need to be __le{8,16,32,64} (or __be if it's Big Endian)
>> and you need to handle the conversions manually.
>>
>  Yes endianness is LE and fixed. This is NOT a host side driver for an
>  endpoint card. This is driver for on chip PCI device of CN20K soc.
>  Hope I answered your question wrt host.

But the mailbox is shared between the SoC and the host or HW or not? Is
it possible that one client of the mailbox will have LE and the second
will have BE?

> 
>  Thanks,
>  Sundeep

Thanks,
Olek

^ permalink raw reply

* Re: [net-next PATCH v5 1/4] octeontx2-af: npa: cn20k: Add NPA Halo support
From: Subbaraya Sundeep @ 2026-04-10  9:35 UTC (permalink / raw)
  To: Alexander Lobakin
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, sgoutham, gakula,
	bbhushan2, netdev, linux-kernel, Linu Cherian
In-Reply-To: <00fc6c1e-638a-4e80-b3c4-14e4dfa3651a@intel.com>

On 2026-04-09 at 20:39:02, Alexander Lobakin (aleksander.lobakin@intel.com) wrote:
> From: Subbaraya Sundeep <sbhatta@marvell.com>
> Date: Thu, 9 Apr 2026 15:23:21 +0530
> 
> > From: Linu Cherian <lcherian@marvell.com>
> > 
> > CN20K silicon implements unified aura and pool context
> > type called Halo for better resource usage. Add support to
> > handle Halo context type operations.
> > 
> > Signed-off-by: Linu Cherian <lcherian@marvell.com>
> > Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
> 
> [...]
> 
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/struct.h b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/struct.h
> > index 763f6cabd7c2..2364bafd329d 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/struct.h
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/struct.h
> > @@ -377,4 +377,85 @@ struct npa_cn20k_pool_s {
> >  
> >  static_assert(sizeof(struct npa_cn20k_pool_s) == NIX_MAX_CTX_SIZE);
> >  
> > +struct npa_cn20k_halo_s {
> > +	u64 stack_base                  : 64;
> 
> It's redundant to add : 64 to a 64-bit field.
 Agreed. But this is for readability, it helps when checking HRM. For
 instance HRM says [703:640] and we define as u64 reserved_640_703 : 64;
 so that we do not have to count bits in mind.
> Moreover, on 32-bit systems, the compilers sometimes complain on
> bitfields > 32 bits.
 This driver depends on 64BIT.
> 
> > +	u64 ena                         :  1;
> > +	u64 nat_align                   :  1;
> > +	u64 reserved_66_67              :  2;
> > +	u64 stack_caching               :  1;
> > +	u64 reserved_69_71              :  3;
> > +	u64 aura_drop_ena               :  1;
> > +	u64 reserved_73_79              :  7;
> > +	u64 aura_drop                   :  8;
> > +	u64 buf_offset                  : 12;
> > +	u64 reserved_100_103            :  4;
> > +	u64 buf_size                    : 12;
> > +	u64 reserved_116_119            :  4;
> > +	u64 ref_cnt_prof                :  3;
> > +	u64 reserved_123_127            :  5;
> > +	u64 stack_max_pages             : 32;
> > +	u64 stack_pages                 : 32;
> > +	u64 bp_0                        :  7;
> > +	u64 bp_1                        :  7;
> > +	u64 bp_2                        :  7;
> > +	u64 bp_3                        :  7;
> > +	u64 bp_4                        :  7;
> > +	u64 bp_5                        :  7;
> > +	u64 bp_6                        :  7;
> > +	u64 bp_7                        :  7;
> > +	u64 bp_ena_0                    :  1;
> > +	u64 bp_ena_1                    :  1;
> > +	u64 bp_ena_2                    :  1;
> > +	u64 bp_ena_3                    :  1;
> > +	u64 bp_ena_4                    :  1;
> > +	u64 bp_ena_5                    :  1;
> > +	u64 bp_ena_6                    :  1;
> > +	u64 bp_ena_7                    :  1;
> > +	u64 stack_offset                :  4;
> > +	u64 reserved_260_263            :  4;
> > +	u64 shift                       :  6;
> > +	u64 reserved_270_271            :  2;
> > +	u64 avg_level                   :  8;
> > +	u64 avg_con                     :  9;
> > +	u64 fc_ena                      :  1;
> > +	u64 fc_stype                    :  2;
> > +	u64 fc_hyst_bits                :  4;
> > +	u64 fc_up_crossing              :  1;
> > +	u64 reserved_297_299            :  3;
> > +	u64 update_time                 : 16;
> > +	u64 reserved_316_319            :  4;
> > +	u64 fc_addr                     : 64;
> > +	u64 ptr_start                   : 64;
> > +	u64 ptr_end                     : 64;
> > +	u64 bpid_0                      : 12;
> > +	u64 reserved_524_535            : 12;
> > +	u64 err_int                     :  8;
> > +	u64 err_int_ena                 :  8;
> > +	u64 thresh_int                  :  1;
> > +	u64 thresh_int_ena              :  1;
> > +	u64 thresh_up                   :  1;
> > +	u64 reserved_555                :  1;
> > +	u64 thresh_qint_idx             :  7;
> > +	u64 reserved_563                :  1;
> > +	u64 err_qint_idx                :  7;
> > +	u64 reserved_571_575            :  5;
> > +	u64 thresh                      : 36;
> > +	u64 reserved_612_615            :  4;
> > +	u64 fc_msh_dst                  : 11;
> > +	u64 reserved_627_630            :  4;
> > +	u64 op_dpc_ena                  :  1;
> > +	u64 op_dpc_set                  :  5;
> > +	u64 reserved_637_637            :  1;
> > +	u64 stream_ctx                  :  1;
> > +	u64 unified_ctx                 :  1;
> > +	u64 reserved_640_703            : 64;
> > +	u64 reserved_704_767            : 64;
> > +	u64 reserved_768_831            : 64;
> > +	u64 reserved_832_895            : 64;
> > +	u64 reserved_896_959            : 64;
> > +	u64 reserved_960_1023           : 64;
> > +};
> > +
> > +static_assert(sizeof(struct npa_cn20k_halo_s) == NIX_MAX_CTX_SIZE);
> 
> Now the main question:
> 
> Is mailbox's Endianness fixed (LE/BE)? Or is it always the same as the
> host's ones (I doubt so)?
> If not, these need to be __le{8,16,32,64} (or __be if it's Big Endian)
> and you need to handle the conversions manually.
>
 Yes endianness is LE and fixed. This is NOT a host side driver for an
 endpoint card. This is driver for on chip PCI device of CN20K soc.
 Hope I answered your question wrt host.

 Thanks,
 Sundeep

> Thanks,
> Olek

^ permalink raw reply

* Re: [Intel-wired-lan] [PATCH iwl-net 4/4] ice: fix ice_ptp_read_tx_hwtstamp_status_eth56g
From: Petr Oros @ 2026-04-10  9:34 UTC (permalink / raw)
  To: Jacob Keller, Anthony Nguyen, Intel Wired LAN, netdev
  Cc: Aleksandr Loktionov, Timothy Miskell
In-Reply-To: <20260408-jk-even-more-e825c-fixes-v1-4-b959da91a81f@intel.com>


On 4/8/26 20:46, Jacob Keller wrote:
> The ice_ptp_read_tx_hwtstamp_status_eth56g function calls
> ice_read_phy_eth56g with a PHY index. However the function actually expects
> a port index. This causes the function to read the wrong PHY_PTP_INT_STATUS
> registers, and effectively makes the status wrong for the second set of
> ports from 4 to 7.
>
> The ice_read_phy_eth56g function uses the provided port index to determine
> which PHY device to read. We could refactor the entire chain to take a PHY
> index, but this would impact many code sites. Instead, multiply the PHY
> index by the number of ports, so that we read from the first port of each
> PHY.
>
> Fixes: 7cab44f1c35f ("ice: Introduce ETH56G PHY model for E825C products")
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> index 64ad5ed5c688..672218e5d1f9 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> @@ -2219,13 +2219,19 @@ int ice_ptp_read_tx_hwtstamp_status_eth56g(struct ice_hw *hw, u32 *ts_status)
>   	*ts_status = 0;
>   
>   	for (phy = 0; phy < params->num_phys; phy++) {
> +		u8 port;
>   		int err;
>   
> -		err = ice_read_phy_eth56g(hw, phy, PHY_PTP_INT_STATUS, &status);
> +		/* ice_read_phy_eth56g expects a port index, so use the first
> +		 * port of the PHY
> +		 */
> +		port = phy * hw->ptp.ports_per_phy;
> +
> +		err = ice_read_phy_eth56g(hw, port, PHY_PTP_INT_STATUS, &status);
>   		if (err)
>   			return err;
>   
> -		*ts_status |= (status & mask) << (phy * hw->ptp.ports_per_phy);
> +		*ts_status |= (status & mask) << port;
>   	}
>   
>   	ice_debug(hw, ICE_DBG_PTP, "PHY interrupt err: %x\n", *ts_status);
Reviewed-by: Petr Oros <poros@redhat.com>


^ permalink raw reply

* Re: [Intel-wired-lan] [PATCH iwl-net 3/4] ice: fix ready bitmap check for non-E822 devices
From: Petr Oros @ 2026-04-10  9:34 UTC (permalink / raw)
  To: Jacob Keller, Anthony Nguyen, Intel Wired LAN, netdev
  Cc: Aleksandr Loktionov, Timothy Miskell
In-Reply-To: <20260408-jk-even-more-e825c-fixes-v1-3-b959da91a81f@intel.com>


On 4/8/26 20:46, Jacob Keller wrote:
> The E800 hardware (apart from E810) has a ready bitmap for the PHY
> indicating which timestamp slots currently have an outstanding timestamp
> waiting to be read by software.
>
> This bitmap is checked in multiple places using the
> ice_get_phy_tx_tstamp_ready():
>
>   * ice_ptp_process_tx_tstamp() calls it to determine which timestamps to
>     attempt reading from the PHY
>   * ice_ptp_tx_tstamps_pending() calls it in a loop at the end of the
>     miscellaneous IRQ to check if new timestamps came in while the interrupt
>     handler was executing.
>   * ice_ptp_maybe_trigger_tx_interrupt() calls it in the auxiliary work task
>     to trigger a software interrupt in the event that the hardware logic
>     gets stuck.
>
> For E82X devices, multiple PHYs share the same block, and the parameter
> passed to the ready bitmap is a block number associated with the given
> port. For E825-C devices, the PHYs have their own independent blocks and do
> not share, so the parameter passed needs to be the port number. For E810
> devices, the ice_get_phy_tx_tstamp_ready() always returns all 1s regardless
> of what port, since this hardware does not have a ready bitmap. Finally,
> for E830 devices, each PF has its own ready bitmap accessible via register,
> and the block parameter is unused.
>
> The first call correctly uses the Tx timestamp tracker block parameter to
> check the appropriate timestamp block. This works because the tracker is
> setup correctly for each timestamp device type.
>
> The second two callers behave incorrectly for all device types other than
> the older E822 devices. They both iterate in a loop using
> ICE_GET_QUAD_NUM() which is a macro only used by E822 devices. This logic
> is incorrect for devices other than the E822 devices.
>
> For E810 the calls would always return true, causing E810 devices to always
> attempt to trigger a software interrupt even when they have no reason to.
> For E830, this results in duplicate work as the ready bitmap is checked
> once per number of quads. Finally, for E825-C, this results in the pending
> checks failing to detect timestamps on ports other than the first two.
>
> Fix this by introducing a new hardware API function to ice_ptp_hw.c,
> ice_check_phy_tx_tstamp_ready(). This function will check if any timestamps
> are available and returns a positive value if any timestamps are pending.
> For E810, the function always returns false, so that the re-trigger checks
> never happen. For E830, check the ready bitmap just once. For E82x
> hardware, check each quad. Finally, for E825-C, check every port.
>
> The interface function returns an integer to enable reporting of error code
> if the driver is unable read the ready bitmap. This enables callers to
> handle this case properly. The previous implementation assumed that
> timestamps are available if they failed to read the bitmap. This is
> problematic as it could lead to continuous software IRQ triggering if the
> PHY timestamp registers somehow become inaccessible.
>
> This change is especially important for E825-C devices, as the missing
> checks could leave a window open where a new timestamp could arrive while
> the existing timestamps aren't completed. As a result, the hardware
> threshold logic would not trigger a new interrupt. Without the check, the
> timestamp is left unhandled, and new timestamps will not cause an interrupt
> again until the timestamp is handled. Since both the interrupt check and
> the backup check in the auxiliary task do not function properly, the device
> may have Tx timestamps permanently stuck failing on a given port.
>
> The faulty checks originate from commit d938a8cca88a ("ice: Auxbus devices
> & driver for E822 TS") and commit 712e876371f8 ("ice: periodically kick Tx
> timestamp interrupt"), however at the time of the original coding, both
> functions only operated on E822 hardware. This is no longer the case, and
> hasn't been since the introduction of the ETH56G PHY model in commit
> 7cab44f1c35f ("ice: Introduce ETH56G PHY model for E825C products")
>
> Fixes: 7cab44f1c35f ("ice: Introduce ETH56G PHY model for E825C products")
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_ptp_hw.h |   1 +
>   drivers/net/ethernet/intel/ice/ice_ptp.c    |  40 ++++------
>   drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 117 ++++++++++++++++++++++++++++
>   3 files changed, 132 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> index 9d7acc7eb2ce..1b58b054f4a5 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> @@ -300,6 +300,7 @@ void ice_ptp_reset_ts_memory(struct ice_hw *hw);
>   int ice_ptp_init_phc(struct ice_hw *hw);
>   void ice_ptp_init_hw(struct ice_hw *hw);
>   int ice_get_phy_tx_tstamp_ready(struct ice_hw *hw, u8 block, u64 *tstamp_ready);
> +int ice_check_phy_tx_tstamp_ready(struct ice_hw *hw);
>   int ice_ptp_one_port_cmd(struct ice_hw *hw, u8 configured_port,
>   			 enum ice_ptp_tmr_cmd configured_cmd);
>   
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index ada42bcc4d0b..34906f972d17 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -2718,7 +2718,7 @@ static bool ice_any_port_has_timestamps(struct ice_pf *pf)
>   bool ice_ptp_tx_tstamps_pending(struct ice_pf *pf)
>   {
>   	struct ice_hw *hw = &pf->hw;
> -	unsigned int i;
> +	int ret;
>   
>   	/* Check software indicator */
>   	switch (pf->ptp.tx_interrupt_mode) {
> @@ -2739,16 +2739,15 @@ bool ice_ptp_tx_tstamps_pending(struct ice_pf *pf)
>   	}
>   
>   	/* Check hardware indicator */
> -	for (i = 0; i < ICE_GET_QUAD_NUM(hw->ptp.num_lports); i++) {
> -		u64 tstamp_ready = 0;
> -		int err;
> -
> -		err = ice_get_phy_tx_tstamp_ready(&pf->hw, i, &tstamp_ready);
> -		if (err || tstamp_ready)
> -			return true;
> +	ret = ice_check_phy_tx_tstamp_ready(hw);
> +	if (ret < 0) {
> +		dev_dbg(ice_pf_to_dev(pf), "Unable to read PHY Tx timestamp ready bitmap, err %d\n",
> +			ret);
> +		/* Stop triggering IRQs if we're unable to read PHY */
> +		return false;
>   	}
>   
> -	return false;
> +	return ret;
>   }
>   
>   /**
> @@ -2832,8 +2831,7 @@ static void ice_ptp_maybe_trigger_tx_interrupt(struct ice_pf *pf)
>   {
>   	struct device *dev = ice_pf_to_dev(pf);
>   	struct ice_hw *hw = &pf->hw;
> -	bool trigger_oicr = false;
> -	unsigned int i;
> +	int ret;
>   
>   	if (!pf->ptp.port.tx.has_ready_bitmap)
>   		return;
> @@ -2841,21 +2839,11 @@ static void ice_ptp_maybe_trigger_tx_interrupt(struct ice_pf *pf)
>   	if (!ice_pf_src_tmr_owned(pf))
>   		return;
>   
> -	for (i = 0; i < ICE_GET_QUAD_NUM(hw->ptp.num_lports); i++) {
> -		u64 tstamp_ready;
> -		int err;
> -
> -		err = ice_get_phy_tx_tstamp_ready(&pf->hw, i, &tstamp_ready);
> -		if (!err && tstamp_ready) {
> -			trigger_oicr = true;
> -			break;
> -		}
> -	}
> -
> -	if (trigger_oicr) {
> -		/* Trigger a software interrupt, to ensure this data
> -		 * gets processed.
> -		 */
> +	ret = ice_check_phy_tx_tstamp_ready(hw);
> +	if (ret < 0) {
> +		dev_dbg(dev, "PTP periodic task unable to read PHY timestamp ready bitmap, err %d\n",
> +			ret);
> +	} else if (ret) {
>   		dev_dbg(dev, "PTP periodic task detected waiting timestamps. Triggering Tx timestamp interrupt now.\n");
>   
>   		wr32(hw, PFINT_OICR, PFINT_OICR_TSYN_TX_M);
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> index 441b5f10e4bb..64ad5ed5c688 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> @@ -2168,6 +2168,35 @@ int ice_start_phy_timer_eth56g(struct ice_hw *hw, u8 port)
>   	return 0;
>   }
>   
> +/**
> + * ice_check_phy_tx_tstamp_ready_eth56g - Check Tx memory status for all ports
> + * @hw: pointer to the HW struct
> + *
> + * Check the PHY_REG_TX_MEMORY_STATUS for all ports. A set bit indicates
> + * a waiting timestamp.
> + *
> + * Return: 1 if any port has at least one timestamp ready bit set,
> + * 0 otherwise, and a negative error code if unable to read the bitmap.
> + */
> +static int ice_check_phy_tx_tstamp_ready_eth56g(struct ice_hw *hw)
> +{
> +	int port;
> +
> +	for (port = 0; port < hw->ptp.num_lports; port++) {
> +		u64 tstamp_ready;
> +		int err;
> +
> +		err = ice_get_phy_tx_tstamp_ready(hw, port, &tstamp_ready);
> +		if (err)
> +			return err;
> +
> +		if (tstamp_ready)
> +			return 1;
> +	}
> +
> +	return 0;
> +}
> +
>   /**
>    * ice_ptp_read_tx_hwtstamp_status_eth56g - Get TX timestamp status
>    * @hw: pointer to the HW struct
> @@ -4318,6 +4347,35 @@ ice_get_phy_tx_tstamp_ready_e82x(struct ice_hw *hw, u8 quad, u64 *tstamp_ready)
>   	return 0;
>   }
>   
> +/**
> + * ice_check_phy_tx_tstamp_ready_e82x - Check Tx memory status for all quads
> + * @hw: pointer to the HW struct
> + *
> + * Check the Q_REG_TX_MEMORY_STATUS for all quads. A set bit indicates
> + * a waiting timestamp.
> + *
> + * Return: 1 if any quad has at least one timestamp ready bit set,
> + * 0 otherwise, and a negative error value if unable to read the bitmap.
> + */
> +static int ice_check_phy_tx_tstamp_ready_e82x(struct ice_hw *hw)
> +{
> +	int quad;
> +
> +	for (quad = 0; quad < ICE_GET_QUAD_NUM(hw->ptp.num_lports); quad++) {
> +		u64 tstamp_ready;
> +		int err;
> +
> +		err = ice_get_phy_tx_tstamp_ready(hw, quad, &tstamp_ready);
> +		if (err)
> +			return err;
> +
> +		if (tstamp_ready)
> +			return 1;
> +	}
> +
> +	return 0;
> +}
> +
>   /**
>    * ice_phy_cfg_intr_e82x - Configure TX timestamp interrupt
>    * @hw: pointer to the HW struct
> @@ -4870,6 +4928,23 @@ ice_get_phy_tx_tstamp_ready_e810(struct ice_hw *hw, u8 port, u64 *tstamp_ready)
>   	return 0;
>   }
>   
> +/**
> + * ice_check_phy_tx_tstamp_ready_e810 - Check Tx memory status register
> + * @hw: pointer to the HW struct
> + *
> + * The E810 devices do not have a Tx memory status register. Note this is
> + * intentionally different behavior from ice_get_phy_tx_tstamp_ready_e810
> + * which always says that all bits are ready. This function is called in cases
> + * where code will trigger interrupts if timestamps are waiting, and should
> + * not be called for E810 hardware.
> + *
> + * Return: 0.
> + */
> +static int ice_check_phy_tx_tstamp_ready_e810(struct ice_hw *hw)
> +{
> +	return 0;
> +}
> +
>   /* E810 SMA functions
>    *
>    * The following functions operate specifically on E810 hardware and are used
> @@ -5124,6 +5199,21 @@ static void ice_get_phy_tx_tstamp_ready_e830(const struct ice_hw *hw, u8 port,
>   	*tstamp_ready |= rd32(hw, E830_PRTMAC_TS_TX_MEM_VALID_L);
>   }
>   
> +/**
> + * ice_check_phy_tx_tstamp_ready_e830 - Check Tx memory status register
> + * @hw: pointer to the HW struct
> + *
> + * Return: 1 if the device has waiting timestamps, 0 otherwise.
> + */
> +static int ice_check_phy_tx_tstamp_ready_e830(struct ice_hw *hw)
> +{
> +	u64 tstamp_ready;
> +
> +	ice_get_phy_tx_tstamp_ready_e830(hw, 0, &tstamp_ready);
> +
> +	return !!tstamp_ready;
> +}
> +
>   /**
>    * ice_ptp_init_phy_e830 - initialize PHY parameters
>    * @ptp: pointer to the PTP HW struct
> @@ -5716,6 +5806,33 @@ int ice_get_phy_tx_tstamp_ready(struct ice_hw *hw, u8 block, u64 *tstamp_ready)
>   	}
>   }
>   
> +/**
> + * ice_check_phy_tx_tstamp_ready - Check PHY Tx timestamp memory status
> + * @hw: pointer to the HW struct
> + *
> + * Check the PHY for Tx timestamp memory status on all ports. If you need to
> + * see individual timestamp status for each index, use
> + * ice_get_phy_tx_tstamp_ready() instead.
> + *
> + * Return: 1 if any port has timestamps available, 0 if there are no timestamps
> + * available, and a negative error code on failure.
> + */
> +int ice_check_phy_tx_tstamp_ready(struct ice_hw *hw)
> +{
> +	switch (hw->mac_type) {
> +	case ICE_MAC_E810:
> +		return ice_check_phy_tx_tstamp_ready_e810(hw);
> +	case ICE_MAC_E830:
> +		return ice_check_phy_tx_tstamp_ready_e830(hw);
> +	case ICE_MAC_GENERIC:
> +		return ice_check_phy_tx_tstamp_ready_e82x(hw);
> +	case ICE_MAC_GENERIC_3K_E825:
> +		return ice_check_phy_tx_tstamp_ready_eth56g(hw);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
>   /**
>    * ice_cgu_get_pin_desc_e823 - get pin description array
>    * @hw: pointer to the hw struct
>
Reviewed-by: Petr Oros <poros@redhat.com>


^ permalink raw reply

* Re: [Intel-wired-lan] [PATCH iwl-net 2/4] ice: perform PHY soft reset for E825C ports at initialization
From: Petr Oros @ 2026-04-10  9:33 UTC (permalink / raw)
  To: Jacob Keller, Anthony Nguyen, Intel Wired LAN, netdev
  Cc: Aleksandr Loktionov, Timothy Miskell
In-Reply-To: <20260408-jk-even-more-e825c-fixes-v1-2-b959da91a81f@intel.com>


On 4/8/26 20:46, Jacob Keller wrote:
> From: Grzegorz Nitka <grzegorz.nitka@intel.com>
>
> In some cases the PHY timestamp block of the E825C can become stuck. This
> is known to occur if the software writes 0 to the Tx timestamp threshold,
> and with older versions of the ice driver the threshold configuration is
> buggy and can race in such that hardware briefly operates with a zero
> threshold enabled. There are no other known ways to trigger this behavior,
> but once it occurs, the hardware is not recovered by normal reset, a driver
> reload, or even a warm power cycle of the system. A cold power cycle is
> sufficient to recover hardware, but this is extremely invasive and can
> result in significant downtime on customer deployments.
>
> The PHY for each port has a timestamping block which has its own reset
> functionality accessible by programming the PHY_REG_GLOBAL register.
> Writing to the PHY_REG_GLOBAL_SOFT_RESET_BIT triggers the hardware to
> perform a complete reset of the timestamping block of the PHY. This
> includes clearing the timestamp status for the port, clearing all
> outstanding timestamps in the memory bank, and resetting the PHY timer.
>
> The new ice_ptp_phy_soft_reset_eth56g() function toggles the
> PHY_REG_GLOBAL soft reset bit with the required delays, ensuring the
> PHY is properly reinitialized without requiring a full device reset.
> The sequence clears the reset bit, asserts it, then clears it again,
> with short waits between transitions to allow hardware stabilization.
>
> Call this function in the new ice_ptp_init_phc_e825c(), implementing the
> E825C device specific variant of the ice_ptp_init_phc(). Note that if
> ice_ptp_init_phc() fails, PTP functionality may be disabled, but the driver
> will still load to allow basic functionality to continue.
>
> This causes the clock owning PF driver to perform a PHY soft reset for
> every port during initialization. This ensures the driver begins life in a
> known functional state regardless of how it was previously programmed.
>
> This ensures that we properly reconfigure the hardware after a device reset
> or when loading the driver, even if it was previously misconfigured with an
> out-of-date or modified driver.
>
> Fixes: 7cab44f1c35f ("ice: Introduce ETH56G PHY model for E825C products")
> Signed-off-by: Timothy Miskell <timothy.miskell@intel.com>
> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_ptp_hw.h |  4 ++
>   drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 90 ++++++++++++++++++++++++++++-
>   2 files changed, 93 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> index 5896b346e579..9d7acc7eb2ce 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> @@ -374,6 +374,7 @@ int ice_stop_phy_timer_eth56g(struct ice_hw *hw, u8 port, bool soft_reset);
>   int ice_start_phy_timer_eth56g(struct ice_hw *hw, u8 port);
>   int ice_phy_cfg_intr_eth56g(struct ice_hw *hw, u8 port, bool ena, u8 threshold);
>   int ice_phy_cfg_ptp_1step_eth56g(struct ice_hw *hw, u8 port);
> +int ice_ptp_phy_soft_reset_eth56g(struct ice_hw *hw, u8 port);
>   
>   #define ICE_ETH56G_NOMINAL_INCVAL	0x140000000ULL
>   #define ICE_ETH56G_NOMINAL_PCS_REF_TUS	0x100000000ULL
> @@ -676,6 +677,9 @@ static inline u64 ice_get_base_incval(struct ice_hw *hw)
>   #define ICE_P0_GNSS_PRSNT_N	BIT(4)
>   
>   /* ETH56G PHY register addresses */
> +#define PHY_REG_GLOBAL			0x0
> +#define PHY_REG_GLOBAL_SOFT_RESET_M	BIT(11)
> +
>   /* Timestamp PHY incval registers */
>   #define PHY_REG_TIMETUS_L		0x8
>   #define PHY_REG_TIMETUS_U		0xC
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> index 67775beb9449..441b5f10e4bb 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> @@ -377,6 +377,31 @@ static void ice_ptp_cfg_sync_delay(const struct ice_hw *hw, u32 delay)
>    * The following functions operate on devices with the ETH 56G PHY.
>    */
>   
> +/**
> + * ice_ptp_init_phc_e825c - Perform E825C specific PHC initialization
> + * @hw: pointer to HW struct
> + *
> + * Perform E825C-specific PTP hardware clock initialization steps.
> + *
> + * Return: 0 on success, or a negative error value on failure.
> + */
> +static int ice_ptp_init_phc_e825c(struct ice_hw *hw)
> +{
> +	int err;
> +
> +	/* Soft reset all ports, to ensure everything is at a clean state */
> +	for (int port = 0; port < hw->ptp.num_lports; port++) {
> +		err = ice_ptp_phy_soft_reset_eth56g(hw, port);
> +		if (err) {
> +			ice_debug(hw, ICE_DBG_PTP, "Failed to soft reset port %d, err %d\n",
> +				  port, err);
> +			return err;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>   /**
>    * ice_ptp_get_dest_dev_e825 - get destination PHY for given port number
>    * @hw: pointer to the HW struct
> @@ -2179,6 +2204,69 @@ int ice_ptp_read_tx_hwtstamp_status_eth56g(struct ice_hw *hw, u32 *ts_status)
>   	return 0;
>   }
>   
> +/**
> + * ice_ptp_phy_soft_reset_eth56g - Perform a PHY soft reset on ETH56G
> + * @hw: pointer to the HW structure
> + * @port: PHY port number
> + *
> + * Trigger a soft reset of the ETH56G PHY by toggling the soft reset
> + * bit in the PHY global register. The reset sequence consists of:
> + *   1. Clearing the soft reset bit
> + *   2. Asserting the soft reset bit
> + *   3. Clearing the soft reset bit again
> + *
> + * Short delays are inserted between each step to allow the hardware
> + * to settle. This provides a controlled way to reinitialize the PHY
> + * without requiring a full device reset.
> + *
> + * Return: 0 on success, or a negative error code on failure when
> + *         reading or writing the PHY register.
> + */
> +int ice_ptp_phy_soft_reset_eth56g(struct ice_hw *hw, u8 port)
> +{
> +	u32 global_val;
> +	int err;
> +
> +	err = ice_read_ptp_reg_eth56g(hw, port, PHY_REG_GLOBAL, &global_val);
> +	if (err) {
> +		ice_debug(hw, ICE_DBG_PTP, "Failed to read PHY_REG_GLOBAL for port %d, err %d\n",
> +			  port, err);
> +		return err;
> +	}
> +
> +	global_val &= ~PHY_REG_GLOBAL_SOFT_RESET_M;
> +	ice_debug(hw, ICE_DBG_PTP, "Clearing soft reset bit for port %d, val: 0x%x\n",
> +		  port, global_val);
> +	err = ice_write_ptp_reg_eth56g(hw, port, PHY_REG_GLOBAL, global_val);
> +	if (err) {
> +		ice_debug(hw, ICE_DBG_PTP, "Failed to write PHY_REG_GLOBAL for port %d, err %d\n",
> +			  port, err);
> +		return err;
> +	}
> +
> +	usleep_range(5000, 6000);
> +
> +	global_val |= PHY_REG_GLOBAL_SOFT_RESET_M;
> +	ice_debug(hw, ICE_DBG_PTP, "Set soft reset bit for port %d, val: 0x%x\n",
> +		  port, global_val);
> +	err = ice_write_ptp_reg_eth56g(hw, port, PHY_REG_GLOBAL, global_val);
> +	if (err) {
> +		ice_debug(hw, ICE_DBG_PTP, "Failed to write PHY_REG_GLOBAL for port %d, err %d\n",
> +			  port, err);
> +		return err;
> +	}
> +	usleep_range(5000, 6000);
> +
> +	global_val &= ~PHY_REG_GLOBAL_SOFT_RESET_M;
> +	ice_debug(hw, ICE_DBG_PTP, "Clear soft reset bit for port %d, val: 0x%x\n",
> +		  port, global_val);
> +	err = ice_write_ptp_reg_eth56g(hw, port, PHY_REG_GLOBAL, global_val);
> +	if (err)
> +		ice_debug(hw, ICE_DBG_PTP, "Failed to write PHY_REG_GLOBAL for port %d, err %d\n",
> +			  port, err);
> +	return err;
> +}
> +
>   /**
>    * ice_get_phy_tx_tstamp_ready_eth56g - Read the Tx memory status register
>    * @hw: pointer to the HW struct
> @@ -5591,7 +5679,7 @@ int ice_ptp_init_phc(struct ice_hw *hw)
>   	case ICE_MAC_GENERIC:
>   		return ice_ptp_init_phc_e82x(hw);
>   	case ICE_MAC_GENERIC_3K_E825:
> -		return 0;
> +		return ice_ptp_init_phc_e825c(hw);
>   	default:
>   		return -EOPNOTSUPP;
>   	}
>
Reviewed-by: Petr Oros <poros@redhat.com>


^ permalink raw reply

* Re: [Intel-wired-lan] [PATCH iwl-net 1/4] ice: fix timestamp interrupt configuration for E825C
From: Petr Oros @ 2026-04-10  9:33 UTC (permalink / raw)
  To: Jacob Keller, Anthony Nguyen, Intel Wired LAN, netdev
  Cc: Aleksandr Loktionov, Timothy Miskell
In-Reply-To: <20260408-jk-even-more-e825c-fixes-v1-1-b959da91a81f@intel.com>


On 4/8/26 20:46, Jacob Keller wrote:
> From: Grzegorz Nitka <grzegorz.nitka@intel.com>
>
> The E825C ice_phy_cfg_intr_eth56g() function is responsible for programming
> the PHY interrupt for a given port. This function writes to the
> PHY_REG_TS_INT_CONFIG register of the port. The register is responsible for
> configuring whether the port interrupt logic is enabled, as well as
> programming the threshold of waiting timestamps that will trigger an
> interrupt from this port.
>
> This threshold value must not be programmed to zero while the interrupt is
> enabled. Doing so puts the port in a misconfigured state where the PHY
> timestamp interrupt for the quad of connected ports will become stuck.
>
> This occurs, because a threshold of zero results in the timestamp interrupt
> status for the port becoming stuck high. The four ports in the connected
> quad have their timestamp status indicators muxed together. A new interrupt
> cannot be generated until the timestamp status indicators return low for
> all four ports.
>
> Normally, the timestamp status for a port will clear once there are fewer
> timestamps in that ports timestamp memory bank than the threshold. A
> threshold of zero makes this impossible, so the timestamp status for the
> port does not clear.
>
> The ice driver never intentionally programs the threshold to zero, indeed
> the driver always programs it to a value of 1, intending to get an
> interrupt immediately as soon as even a single packet is waiting for a
> timestamp.
>
> However, there is a subtle flaw in the programming logic in the
> ice_phy_cfg_intr_eth56g() function. Due to the way that the hardware
> handles enabling the PHY interrupt. If the threshold value is modified at
> the same time as the interrupt is enabled, the HW PHY state machine might
> enable the interrupt before the new threshold value is actually updated.
> This leaves a potential race condition caused by the hardware logic where
> a PHY timestamp interrupt might be triggered before the non-zero threshold
> is written, resulting in the PHY timestamp logic becoming stuck.
>
> Once the PHY timestamp status is stuck high, it will remain stuck even
> after attempting to reprogram the PHY block by changing its threshold or
> disabling the interrupt. Even a typical PF or CORE reset will not reset the
> particular block of the PHY that becomes stuck. Even a warm power cycle is
> not guaranteed to cause the PHY block to reset, and a cold power cycle is
> required.
>
> Prevent this by always writing the PHY_REG_TS_INT_CONFIG in two stages.
> First write the threshold value with the interrupt disabled, and only write
> the enable bit after the threshold has been programmed. When disabling the
> interrupt, leave the threshold unchanged. Additionally, re-read the
> register after writing it to guarantee that the write to the PHY has been
> flushed upon exit of the function.
>
> While we're modifying this function implementation, explicitly reject
> programming a threshold of 0 when enabling the interrupt. No caller does
> this today, but the consequences of doing so are significant. An explicit
> rejection in the code makes this clear.
>
> Fixes: 7cab44f1c35f ("ice: Introduce ETH56G PHY model for E825C products")
> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 36 +++++++++++++++++++++++++----
>   1 file changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> index e3db252c3918..67775beb9449 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> @@ -1847,6 +1847,8 @@ static int ice_phy_cfg_mac_eth56g(struct ice_hw *hw, u8 port)
>    * @ena: enable or disable interrupt
>    * @threshold: interrupt threshold
>    *
> + * The threshold cannot be 0 while the interrupt is enabled.
> + *
>    * Configure TX timestamp interrupt for the specified port
>    *
>    * Return:
> @@ -1858,19 +1860,45 @@ int ice_phy_cfg_intr_eth56g(struct ice_hw *hw, u8 port, bool ena, u8 threshold)
>   	int err;
>   	u32 val;
>   
> +	if (ena && !threshold)
> +		return -EINVAL;
> +
>   	err = ice_read_ptp_reg_eth56g(hw, port, PHY_REG_TS_INT_CONFIG, &val);
>   	if (err)
>   		return err;
>   
> +	val &= ~PHY_TS_INT_CONFIG_ENA_M;
>   	if (ena) {
> -		val |= PHY_TS_INT_CONFIG_ENA_M;
>   		val &= ~PHY_TS_INT_CONFIG_THRESHOLD_M;
>   		val |= FIELD_PREP(PHY_TS_INT_CONFIG_THRESHOLD_M, threshold);
> -	} else {
> -		val &= ~PHY_TS_INT_CONFIG_ENA_M;
> +		err = ice_write_ptp_reg_eth56g(hw, port, PHY_REG_TS_INT_CONFIG,
> +					       val);
> +		if (err) {
> +			ice_debug(hw, ICE_DBG_PTP,
> +				  "Failed to update 'threshold' PHY_REG_TS_INT_CONFIG port=%u ena=%u threshold=%u\n",
> +				  port, !!ena, threshold);
> +			return err;
> +		}
> +		val |= PHY_TS_INT_CONFIG_ENA_M;
>   	}
>   
> -	return ice_write_ptp_reg_eth56g(hw, port, PHY_REG_TS_INT_CONFIG, val);
> +	err = ice_write_ptp_reg_eth56g(hw, port, PHY_REG_TS_INT_CONFIG, val);
> +	if (err) {
> +		ice_debug(hw, ICE_DBG_PTP,
> +			  "Failed to update 'ena' PHY_REG_TS_INT_CONFIG port=%u ena=%u threshold=%u\n",
> +			  port, !!ena, threshold);
> +		return err;
> +	}
> +
> +	err = ice_read_ptp_reg_eth56g(hw, port, PHY_REG_TS_INT_CONFIG, &val);
> +	if (err) {
> +		ice_debug(hw, ICE_DBG_PTP,
> +			  "Failed to read PHY_REG_TS_INT_CONFIG port=%u ena=%u threshold=%u\n",
> +			  port, !!ena, threshold);
> +		return err;
> +	}
> +
> +	return 0;
>   }
>   
>   /**
>
Reviewed-by: Petr Oros <poros@redhat.com>



^ permalink raw reply

* Re: [PATCH v2] selftests: vsock: avoid races creating Unix socket paths
From: Stefano Garzarella @ 2026-04-10  9:06 UTC (permalink / raw)
  To: Cao Ruichuang, Bobby Eshleman
  Cc: stefano.garzarella, shuah, virtualization, netdev,
	linux-kselftest, linux-kernel, horms
In-Reply-To: <adi1X_3wVUsDhRmR@sgarzare-redhat>

On Fri, 10 Apr 2026 at 10:33, Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Fri, Apr 10, 2026 at 11:52:37AM +0800, Cao Ruichuang wrote:
>
> No patch description at all?

Also, maybe better to CC Bobby.

Stefano

>
> >Signed-off-by: Cao Ruichuang <create0818@163.com>
> >---
> >v2:
> >- retitle the patch to describe the race being fixed
> >- replace rm -rf with explicit rm and rmdir cleanup
> >
> > tools/testing/selftests/vsock/vmtest.sh | 16 ++++++++++++----
> > 1 file changed, 12 insertions(+), 4 deletions(-)
> >
> >diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
> >index 86e338886b3..c345fa539d3 100755
> >--- a/tools/testing/selftests/vsock/vmtest.sh
> >+++ b/tools/testing/selftests/vsock/vmtest.sh
> >@@ -718,6 +718,7 @@ test_ns_diff_global_host_connect_to_global_vm_ok() {
> >       local pids pid pidfile
> >       local ns0 ns1 port
> >       declare -a pids
> >+      local unixdir
> >       local unixfile
> >       ns0="global0"
> >       ns1="global1"
> >@@ -736,7 +737,8 @@ test_ns_diff_global_host_connect_to_global_vm_ok() {
> >       oops_before=$(vm_dmesg_oops_count "${ns0}")
> >       warn_before=$(vm_dmesg_warn_count "${ns0}")
> >
> >-      unixfile=$(mktemp -u /tmp/XXXX.sock)
> >+      unixdir=$(mktemp -d /tmp/vsock_vmtest_XXXXXX)
> >+      unixfile="${unixdir}/sock"
> >       ip netns exec "${ns1}" \
> >               socat TCP-LISTEN:"${TEST_HOST_PORT}",fork \
> >                       UNIX-CONNECT:"${unixfile}" &
> >@@ -758,6 +760,8 @@ test_ns_diff_global_host_connect_to_global_vm_ok() {
> >
> >       terminate_pids "${pids[@]}"
> >       terminate_pidfiles "${pidfile}"
> >+      rm "${unixfile}"
> >+      rmdir "${unixdir}"
> >
> >       if [[ "${rc}" -ne 0 ]] || [[ "${dmesg_rc}" -ne 0 ]]; then
> >               return "${KSFT_FAIL}"
> >@@ -814,6 +818,7 @@ test_ns_diff_global_vm_connect_to_global_host_ok() {
> >       local ns0="global0"
> >       local ns1="global1"
> >       local port=12345
> >+      local unixdir
> >       local unixfile
> >       local dmesg_rc
> >       local pidfile
> >@@ -826,7 +831,8 @@ test_ns_diff_global_vm_connect_to_global_host_ok() {
> >
> >       log_host "Setup socat bridge from ns ${ns0} to ns ${ns1} over port ${port}"
> >
> >-      unixfile=$(mktemp -u /tmp/XXXX.sock)
> >+      unixdir=$(mktemp -d /tmp/vsock_vmtest_XXXXXX)
> >+      unixfile="${unixdir}/sock"
> >
> >       ip netns exec "${ns0}" \
> >               socat TCP-LISTEN:"${port}" UNIX-CONNECT:"${unixfile}" &
> >@@ -845,7 +851,8 @@ test_ns_diff_global_vm_connect_to_global_host_ok() {
> >       if ! vm_start "${pidfile}" "${ns0}"; then
> >               log_host "failed to start vm (cid=${cid}, ns=${ns0})"
> >               terminate_pids "${pids[@]}"
> >-              rm -f "${unixfile}"
> >+              rm "${unixfile}"
> >+              rmdir "${unixdir}"
> >               return "${KSFT_FAIL}"
> >       fi
> >
> >@@ -862,7 +869,8 @@ test_ns_diff_global_vm_connect_to_global_host_ok() {
> >
> >       terminate_pidfiles "${pidfile}"
> >       terminate_pids "${pids[@]}"
> >-      rm -f "${unixfile}"
> >+      rm "${unixfile}"
> >+      rmdir "${unixdir}"
> >
> >       if [[ "${rc}" -ne 0 ]] || [[ "${dmesg_rc}" -ne 0 ]]; then
> >               return "${KSFT_FAIL}"
> >--
> >2.39.5 (Apple Git-154)
> >
> >


^ permalink raw reply

* [PATCH net-next] net: lan743x: rename chip_rev to fpga_rev
From: Thangaraj Samynathan @ 2026-04-10  8:57 UTC (permalink / raw)
  To: bryan.whitehead
  Cc: UNGLinuxDriver, andrew+netdev, davem, edumazet, kuba, pabeni,
	netdev, linux-kernel

The variable chip_rev stores the value read from the FPGA_REV
register and represents the FPGA revision. Rename it to fpga_rev
to better reflect its meaning.

No functional change intended.

Signed-off-by: Thangaraj Samynathan <thangaraj.s@microchip.com>
---
 drivers/net/ethernet/microchip/lan743x_main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index b4cabde6625a..f3332417162e 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -36,7 +36,7 @@ static bool pci11x1x_is_a0(struct lan743x_adapter *adapter)
 
 static void pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
 {
-	u32 chip_rev;
+	u32 fpga_rev;
 	u32 cfg_load;
 	u32 hw_cfg;
 	u32 strap;
@@ -63,9 +63,9 @@ static void pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
 		else
 			adapter->is_sgmii_en = false;
 	} else {
-		chip_rev = lan743x_csr_read(adapter, FPGA_REV);
-		if (chip_rev) {
-			if (chip_rev & FPGA_SGMII_OP)
+		fpga_rev = lan743x_csr_read(adapter, FPGA_REV);
+		if (fpga_rev) {
+			if (fpga_rev & FPGA_SGMII_OP)
 				adapter->is_sgmii_en = true;
 			else
 				adapter->is_sgmii_en = false;
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH net v2] net: fix __this_cpu_add() in preemptible code in dev_xmit_recursion_inc/dec
From: Eric Dumazet @ 2026-04-10  8:55 UTC (permalink / raw)
  To: Jiayuan Chen
  Cc: netdev, David S. Miller, David Ahern, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Weiming Shi, linux-kernel
In-Reply-To: <20260410020631.191786-1-jiayuan.chen@linux.dev>

On Thu, Apr 9, 2026 at 7:06 PM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
> dev_xmit_recursion_{inc,dec}() use __this_cpu_{inc,dec}() which requires
> the caller to be non-preemptible in order to avoid cpu migration. However,
> some callers like SCTP's UDP encapsulation path invoke iptunnel_xmit()
> from process context without disabling BH or preemption:
>
>   sctp_inet_connect -> __sctp_connect -> sctp_do_sm ->
>   sctp_outq_flush -> sctp_packet_transmit -> sctp_v4_xmit ->
>   udp_tunnel_xmit_skb -> iptunnel_xmit -> dev_xmit_recursion_inc
>
> This triggers the following warning on PREEMPT(full) kernels:
>
>

> All other callers of dev_xmit_recursion_{inc,dec}() are fine: those in
> net/core/dev.c and net/core/filter.c run under local_bh_disable(), and
> lwtunnel_input() asserts in_softirq() context. Currently only
> iptunnel_xmit() and ip6tunnel_xmit() can be reached from process
> context via the SCTP UDP encapsulation path.
>
> Fix this by adding guard(migrate)() at the top of iptunnel_xmit() and
> ip6tunnel_xmit() to ensure dev_recursion_level(), dev_xmit_recursion_inc()
> and dev_xmit_recursion_dec() all run on the same CPU.
>
> Fixes: 6f1a9140ecda ("net: add xmit recursion limit to tunnel xmit functions")
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> ---

Reviewed-by: Eric Dumazet <edumazet@google.com>

Thanks!

^ permalink raw reply

* Re: [PATCH net 1/1] af_unix: Hold receive queue lock in ioctl(SIOCATMARK)
From: Eric Dumazet @ 2026-04-10  8:48 UTC (permalink / raw)
  To: Ren Wei
  Cc: netdev, kuniyu, davem, kuba, pabeni, horms, rao.shoaib, yifanwucs,
	tomapufckgml, yuantan098, bird, enjou1224z, wangjiexun2025
In-Reply-To: <f6cbbc8da90e95584847b5ceb60aae830d1631c2.1775731983.git.wangjiexun2025@gmail.com>

On Thu, Apr 9, 2026 at 11:32 PM Ren Wei <n05ec@lzu.edu.cn> wrote:
>
> From: Jiexun Wang <wangjiexun2025@gmail.com>
>
> unix_ioctl() peeks at the receive queue and may check both the head skb
> and its successor while deciding whether SIOCATMARK should report the
> mark. However, u->iolock does not stabilize receive-queue element
> lifetime. Queue teardown paths can purge or splice the queue under
> sk->sk_receive_queue.lock and free the skb while unix_ioctl() still
> uses it.
>
> Take sk->sk_receive_queue.lock while inspecting the queue so the skb
> and next_skb stay alive for the whole decision.
>
> Fixes: 314001f0bf92 ("af_unix: Add OOB support")
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Co-developed-by: Yuan Tan <yuantan098@gmail.com>
> Signed-off-by: Yuan Tan <yuantan098@gmail.com>
> Suggested-by: Xin Liu <bird@lzu.edu.cn>
> Tested-by: Ren Wei <enjou1224z@gmail.com>
> Signed-off-by: Jiexun Wang <wangjiexun2025@gmail.com>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> ---
>  net/unix/af_unix.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index b23c33df8b46..54f12d5cda37 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -3301,6 +3301,8 @@ static int unix_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
>                         int answ = 0;
>
>                         mutex_lock(&u->iolock);
> +                       /* The receive queue lock keeps skb and next_skb alive. */
> +                       spin_lock(&sk->sk_receive_queue.lock);
>
>                         skb = skb_peek(&sk->sk_receive_queue);
>                         if (skb) {
> @@ -3315,6 +3317,7 @@ static int unix_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
>                                         answ = 1;
>                         }
>
> +                       spin_unlock(&sk->sk_receive_queue.lock);
>                         mutex_unlock(&u->iolock);
>
>                         err = put_user(answ, (int __user *)arg);

Ok, but I think we also should remove the READ_ONCE() now we have
proper locking.

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 5d4baac499af8d0a9cc36f652acf129a2e3619b2..1c3b80b25ef8b37d8e6553066fb7da20d81294a5
100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -3301,10 +3301,12 @@ static int unix_ioctl(struct socket *sock,
unsigned int cmd, unsigned long arg)
                        int answ = 0;

                        mutex_lock(&u->iolock);
+                       /* The receive queue lock keeps skb and
next_skb alive. */
+                       spin_lock(&sk->sk_receive_queue.lock);

                        skb = skb_peek(&sk->sk_receive_queue);
                        if (skb) {
-                               struct sk_buff *oob_skb = READ_ONCE(u->oob_skb);
+                               struct sk_buff *oob_skb = u->oob_skb;
                                struct sk_buff *next_skb;

                                next_skb = skb_peek_next(skb,
&sk->sk_receive_queue);
@@ -3314,7 +3316,7 @@ static int unix_ioctl(struct socket *sock,
unsigned int cmd, unsigned long arg)
                                     (!oob_skb || next_skb == oob_skb)))
                                        answ = 1;
                        }
-
+                       spin_unlock(&sk->sk_receive_queue.lock);
                        mutex_unlock(&u->iolock);

                        err = put_user(answ, (int __user *)arg);

^ permalink raw reply

* Re: [PATCH RFC net-next 02/10] net: stmmac: rename dev_id to userver
From: Russell King (Oracle) @ 2026-04-10  8:39 UTC (permalink / raw)
  To: Jitendra Vegiraju
  Cc: Andrew Lunn, Alexandre Torgue, Andrew Lunn, Chen-Yu Tsai,
	David S. Miller, Eric Dumazet, Jakub Kicinski, linux-arm-kernel,
	linux-stm32, linux-sunxi, netdev, Paolo Abeni, Samuel Holland
In-Reply-To: <CAMdnO-+TK65AxjTsDd017Mhop+VC3Xf8jtfaTXYpE6wBNZOt4g@mail.gmail.com>

On Thu, Apr 09, 2026 at 04:07:42PM -0700, Jitendra Vegiraju wrote:
> Hi Russell,
> 
> On Wed, Apr 8, 2026 at 2:27 AM Russell King (Oracle)
> <rmk+kernel@armlinux.org.uk> wrote:
> >
> > The Synopsys Databook and several implementation TRMs identify bits
> > 15:8 of the version register in dwmac v3.xx and v4.xx as "userver".
> > We even print its value with "User ID". Rather than using "dev_id",
> > use "userver" instead.
> >
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/hwif.c | 18 +++++++++---------
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.c b/drivers/net/ethernet/stmicro/stmmac/hwif.c
> > index 3774af66db48..830ff816ab4f 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/hwif.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.c
> > @@ -15,7 +15,7 @@
> >
> >  struct stmmac_version {
> >         u8 snpsver;
> > -       u8 dev_id;
> > +       u8 userver;
> >  };
> From the XGMAC databook that I have access to bits(15:8) identify the
> DEVID field of MAC_version register.
> The userver field is from bits(23:16) of the same register. This is a
> customer defined field (configured with coreConsultant).
> Currently stmmac doesn't care about bits(23:16).

Thanks for the additional information.

I don't have any XGMAC documentation, but this indicates that it differs
between XGMAC and previous cores - GMAC and GMAC4 cores, 15:8 are
documented as userver, and 31:16 are marked as reserved.

Note that the dev_info() also prints 15:8 as "User ID" not "Device ID".

To confirm, is the XGMAC version register at offset 0x20 ? Later GMAC
cores moved it to 0x110.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply

* Re: [PATCH v3 net] vsock: fix buffer size clamping order
From: Stefano Garzarella @ 2026-04-10  8:36 UTC (permalink / raw)
  To: Norbert Szetei
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, virtualization, netdev, linux-kernel
In-Reply-To: <180118C5-8BCF-4A63-A305-4EE53A34AB9C@doyensec.com>

On Thu, Apr 09, 2026 at 06:34:12PM +0200, Norbert Szetei wrote:
>In vsock_update_buffer_size(), the buffer size was being clamped to the
>maximum first, and then to the minimum. If a user sets a minimum buffer
>size larger than the maximum, the minimum check overrides the maximum
>check, inverting the constraint.
>
>This breaks the intended socket memory boundaries by allowing the
>vsk->buffer_size to grow beyond the configured vsk->buffer_max_size.
>
>Fix this by checking the minimum first, and then the maximum. This
>ensures the buffer size never exceeds the buffer_max_size.
>
>Fixes: b9f2b0ffde0c ("vsock: handle buffer_size sockopts in the core")
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>Signed-off-by: Norbert Szetei <norbert@doyensec.com>
>---
>v3:
> - Added Fixes and Suggested-by tags.
>
> net/vmw_vsock/af_vsock.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>


^ permalink raw reply

* Re: [PATCH v2] selftests: vsock: avoid races creating Unix socket paths
From: Stefano Garzarella @ 2026-04-10  8:33 UTC (permalink / raw)
  To: Cao Ruichuang
  Cc: stefano.garzarella, shuah, virtualization, netdev,
	linux-kselftest, linux-kernel, horms
In-Reply-To: <20260410035237.59644-1-create0818@163.com>

On Fri, Apr 10, 2026 at 11:52:37AM +0800, Cao Ruichuang wrote:

No patch description at all?

>Signed-off-by: Cao Ruichuang <create0818@163.com>
>---
>v2:
>- retitle the patch to describe the race being fixed
>- replace rm -rf with explicit rm and rmdir cleanup
>
> tools/testing/selftests/vsock/vmtest.sh | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
>diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
>index 86e338886b3..c345fa539d3 100755
>--- a/tools/testing/selftests/vsock/vmtest.sh
>+++ b/tools/testing/selftests/vsock/vmtest.sh
>@@ -718,6 +718,7 @@ test_ns_diff_global_host_connect_to_global_vm_ok() {
> 	local pids pid pidfile
> 	local ns0 ns1 port
> 	declare -a pids
>+	local unixdir
> 	local unixfile
> 	ns0="global0"
> 	ns1="global1"
>@@ -736,7 +737,8 @@ test_ns_diff_global_host_connect_to_global_vm_ok() {
> 	oops_before=$(vm_dmesg_oops_count "${ns0}")
> 	warn_before=$(vm_dmesg_warn_count "${ns0}")
>
>-	unixfile=$(mktemp -u /tmp/XXXX.sock)
>+	unixdir=$(mktemp -d /tmp/vsock_vmtest_XXXXXX)
>+	unixfile="${unixdir}/sock"
> 	ip netns exec "${ns1}" \
> 		socat TCP-LISTEN:"${TEST_HOST_PORT}",fork \
> 			UNIX-CONNECT:"${unixfile}" &
>@@ -758,6 +760,8 @@ test_ns_diff_global_host_connect_to_global_vm_ok() {
>
> 	terminate_pids "${pids[@]}"
> 	terminate_pidfiles "${pidfile}"
>+	rm "${unixfile}"
>+	rmdir "${unixdir}"
>
> 	if [[ "${rc}" -ne 0 ]] || [[ "${dmesg_rc}" -ne 0 ]]; then
> 		return "${KSFT_FAIL}"
>@@ -814,6 +818,7 @@ test_ns_diff_global_vm_connect_to_global_host_ok() {
> 	local ns0="global0"
> 	local ns1="global1"
> 	local port=12345
>+	local unixdir
> 	local unixfile
> 	local dmesg_rc
> 	local pidfile
>@@ -826,7 +831,8 @@ test_ns_diff_global_vm_connect_to_global_host_ok() {
>
> 	log_host "Setup socat bridge from ns ${ns0} to ns ${ns1} over port ${port}"
>
>-	unixfile=$(mktemp -u /tmp/XXXX.sock)
>+	unixdir=$(mktemp -d /tmp/vsock_vmtest_XXXXXX)
>+	unixfile="${unixdir}/sock"
>
> 	ip netns exec "${ns0}" \
> 		socat TCP-LISTEN:"${port}" UNIX-CONNECT:"${unixfile}" &
>@@ -845,7 +851,8 @@ test_ns_diff_global_vm_connect_to_global_host_ok() {
> 	if ! vm_start "${pidfile}" "${ns0}"; then
> 		log_host "failed to start vm (cid=${cid}, ns=${ns0})"
> 		terminate_pids "${pids[@]}"
>-		rm -f "${unixfile}"
>+		rm "${unixfile}"
>+		rmdir "${unixdir}"
> 		return "${KSFT_FAIL}"
> 	fi
>
>@@ -862,7 +869,8 @@ test_ns_diff_global_vm_connect_to_global_host_ok() {
>
> 	terminate_pidfiles "${pidfile}"
> 	terminate_pids "${pids[@]}"
>-	rm -f "${unixfile}"
>+	rm "${unixfile}"
>+	rmdir "${unixdir}"
>
> 	if [[ "${rc}" -ne 0 ]] || [[ "${dmesg_rc}" -ne 0 ]]; then
> 		return "${KSFT_FAIL}"
>-- 
>2.39.5 (Apple Git-154)
>
>


^ permalink raw reply

* Re: [PATCH net 1/1] net: stmmac: Update default_an_inband before passing value to phylink_config
From: KhaiWenTan @ 2026-04-10  7:53 UTC (permalink / raw)
  To: linux
  Cc: alexandre.torgue, andrew+netdev, davem, edumazet, hong.aun.looi,
	khai.wen.tan, khai.wen.tan, kuba, linux-arm-kernel, linux-kernel,
	linux-stm32, maxime.chevallier, mcoquelin.stm32, netdev,
	ovidiu.panait.rb, pabeni, vladimir.oltean, yoong.siang.song
In-Reply-To: <adirrTJujEsrsK4F@shell.armlinux.org.uk>

Hi Russell,

Thanks for the review. I will address the comments in v2.

^ permalink raw reply

* RE: [Intel-wired-lan] [PATCH net] ice: fix double free in ice_sf_eth_activate() error path
From: Loktionov, Aleksandr @ 2026-04-10  8:12 UTC (permalink / raw)
  To: Greg Kroah-Hartman, intel-wired-lan@lists.osuosl.org,
	netdev@vger.kernel.org
  Cc: linux-kernel@vger.kernel.org, Nguyen, Anthony L,
	Kitszel, Przemyslaw, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Piotr Raczynski, Jiri Pirko,
	Simon Horman, Michal Swiatkowski, stable
In-Reply-To: <2026040919-junior-glue-10d0@gregkh>



> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Greg Kroah-Hartman
> Sent: Thursday, April 9, 2026 5:11 PM
> To: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Andrew Lunn <andrew+netdev@lunn.ch>;
> David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Piotr Raczynski <piotr.raczynski@intel.com>; Jiri
> Pirko <jiri@resnulli.us>; Simon Horman <horms@kernel.org>; Michal
> Swiatkowski <michal.swiatkowski@linux.intel.com>; stable
> <stable@kernel.org>
> Subject: [Intel-wired-lan] [PATCH net] ice: fix double free in
> ice_sf_eth_activate() error path
> 
> When auxiliary_device_add() fails, the aux_dev_uninit label calls
> auxiliary_device_uninit() and falls through to sf_dev_free and
> xa_erase.
> The uninit invokes ice_sf_dev_release(), which already frees sf_dev
> via
> kfree() and erases the entry from ice_sf_aux_id.  The fall-through
> then double-frees sf_dev and double-erases the id.
> 
> This is reachable from userspace via the devlink port function state-
> set netlink command.
> 
> Fix this by returning right after uninit because the release callback
> handles all cleanup correctly.
> 
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Cc: Andrew Lunn <andrew+netdev@lunn.ch>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Piotr Raczynski <piotr.raczynski@intel.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Cc: Simon Horman <horms@kernel.org>
> Cc: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Fixes: 177ef7f1e2a0 ("ice: base subfunction aux driver")
> Cc: stable <stable@kernel.org>
> Assisted-by: gregkh_clanker_t1000
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/net/ethernet/intel/ice/ice_sf_eth.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_sf_eth.c
> b/drivers/net/ethernet/intel/ice/ice_sf_eth.c
> index 2cf04bc6edce..6bc8aa896762 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sf_eth.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sf_eth.c
> @@ -304,7 +304,9 @@ ice_sf_eth_activate(struct ice_dynamic_port
> *dyn_port,
>  	return 0;
> 
>  aux_dev_uninit:
> +	/* ice_sf_dev_release() frees sf_dev and erases the xa entry */
>  	auxiliary_device_uninit(&sf_dev->adev);
> +	return err;
>  sf_dev_free:
>  	kfree(sf_dev);
>  xa_erase:
> --
> 2.53.0

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>


^ permalink raw reply

* Re: [PATCH net 1/1] net: stmmac: Update default_an_inband before passing value to phylink_config
From: Russell King (Oracle) @ 2026-04-10  7:50 UTC (permalink / raw)
  To: KhaiWenTan
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, mcoquelin.stm32,
	alexandre.torgue, maxime.chevallier, ovidiu.panait.rb,
	vladimir.oltean, netdev, linux-stm32, linux-arm-kernel,
	linux-kernel, yoong.siang.song, hong.aun.looi, khai.wen.tan
In-Reply-To: <20260410020735.327590-1-khai.wen.tan@linux.intel.com>

On Fri, Apr 10, 2026 at 10:07:35AM +0800, KhaiWenTan wrote:
> get_interfaces() will update both the plat->phy_interfaces and
> mdio_bus_data->default_an_inband based on reading a SERDES register.
> 
> Therefore, we moved the priv->plat->get_interfaces() to be executed
> first before assigning mdio_bus_data->default_an_inband to
> config->default_an_inband to ensure default_an_inband is in correct
> value during PHY setup.
> 
> Fixes: ca732e990fc8 ("net: stmmac: add get_interfaces() platform method")
> Signed-off-by: KhaiWenTan <khai.wen.tan@linux.intel.com>

Patch looks good, but the blamed commit is wrong.

d3836052fe09 ("net: stmmac: intel: convert speed_mode_2500() to get_interfaces()")

introduced the problem.

The commit message could also do with mentioning that dwmac-intel
regressed result of that commit.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply


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