* Re: [PATCH v20 net-next 06/10] net/nebula-matrix: add intr resource implementation
From: Paolo Abeni @ 2026-07-02 9:42 UTC (permalink / raw)
To: illusion.wang
Cc: dimon.zhao, alvin.wang, sam.chen, netdev, andrew+netdev, corbet,
kuba, horms, linux-doc, pabeni, vadim.fedorenko, lukas.bulwahn,
edumazet, enelsonmoore, skhan, hkallweit1, linux-kernel
In-Reply-To: <20260630010718.4346-7-illusion.wang@nebula-matrix.com>
From: AI Reviewer <ai@example.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/
---
net/nebula-matrix: add intr resource implementation
This patch adds the nbl_interrupt module for management of driver-private
MSI-X indices. It adds hw_ops callbacks to program the function MSI-X mapping
registers, configure MSI-X info, and enable mailbox interrupts.
> diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_interrupt.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_interrupt.c
> new file mode 100644
> index 0000000000000..53a657aef9ca0
> --- /dev/null
> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_interrupt.c
> @@ -0,0 +1,267 @@
[ ... ]
> +/*
> + * This function tears down the MSI-X mapping for a function by clearing the
> + * interrupt bitmaps, disabling MSI-X via HW callbacks, and resetting the
> + * bookkeeping state. The DMA buffer and interrupts[] array are intentionally
> + * left allocated for potential reuse on reset/hot-add paths. Caller must
> + * ensure this is only invoked when the HW cannot issue in-flight DMA to
> + * the region (IRQs already freed or not yet configured).
> + */
> +int nbl_res_intr_destroy_msix_map(struct nbl_resource_mgt *res_mgt,
> + u16 func_id)
> +{
[ ... ]
> + /* use ctrl dev bdf */
> + hw_ops->configure_msix_map(res_mgt->hw_ops_tbl->priv, func_id, false, 0,
> + 0, 0, 0);
> +
> + intr_mgt->func_intr_res[func_id].interrupts = NULL;
> + intr_mgt->func_intr_res[func_id].num_interrupts = 0;
> + intr_mgt->func_intr_res[func_id].msix_map_table.base_addr = NULL;
> + intr_mgt->func_intr_res[func_id].msix_map_table.size = 0;
> + hw_ops->flush_write(res_mgt->hw_ops_tbl->priv);
> +
> + return 0;
> +}
[Severity: High]
Does this code leak the devm-managed resources? The comment mentions that
the DMA buffer and interrupts array are left allocated for potential reuse,
but the pointers are set to NULL here. This causes the pointers to be lost
so they cannot be reused or explicitly freed later.
Because nbl_res_intr_configure_msix_map() unconditionally allocates new devres
memory on every call, won't this cause the devres list to accumulate leaked
allocations (dmam_alloc_coherent and devm_kcalloc) each time the device is
reset or a VF is reconfigured?
[ ... ]
> +int nbl_res_intr_configure_msix_map(struct nbl_resource_mgt *res_mgt,
> + u16 func_id, u16 num_net_msix,
> + u16 num_others_msix,
> + bool net_msix_mask_en)
> +{
[ ... ]
> + nbl_res_intr_destroy_msix_map(res_mgt, func_id);
> +
> + msix_map_table = &intr_mgt->func_intr_res[func_id].msix_map_table;
> + WARN_ON(msix_map_table->base_addr);
> + msix_map_table->size =
> + sizeof(struct nbl_msix_map) * NBL_MSIX_MAP_TABLE_MAX_ENTRIES;
> + /* HW indexes the MSI-X map table with a fixed stride of 1024 entries,
> + * so we must allocate the full table even if fewer vectors are used.
> + */
> + msix_map_table->base_addr = dmam_alloc_coherent(dev,
> + msix_map_table->size,
> + &msix_map_table->dma,
> + GFP_KERNEL);
> + if (!msix_map_table->base_addr) {
> + dev_err(dev, "Allocate DMA memory for function msix map table failed\n");
> + msix_map_table->size = 0;
> + return -ENOMEM;
> + }
> +
> + interrupts = devm_kcalloc(dev, requested, sizeof(interrupts[0]),
> + GFP_KERNEL);
> + if (!interrupts) {
> + ret = -ENOMEM;
> + goto alloc_interrupts_err;
> + }
[Severity: High]
If devm_kcalloc() fails, the goto alloc_interrupts_err path bypasses
clearing msix_map_table->base_addr and size.
While nbl_res_intr_destroy_msix_map() clears it unconditionally on the next
call, doesn't this leave the internal state inconsistent by keeping base_addr
populated when interrupts is NULL?
[ ... ]
> +get_interrupt_err:
> + while (i--) {
> + intr_index = interrupts[i];
> + if (intr_index >= NBL_NET_INTR_BASE)
> + clear_bit(intr_index - NBL_NET_INTR_BASE,
> + intr_mgt->interrupt_net_bitmap);
> + else
> + clear_bit(intr_index,
> + intr_mgt->interrupt_others_bitmap);
> + }
> +
> + intr_mgt->func_intr_res[func_id].num_interrupts = 0;
> + intr_mgt->func_intr_res[func_id].interrupts = NULL;
> + intr_mgt->func_intr_res[func_id].num_net_interrupts = 0;
> + intr_mgt->func_intr_res[func_id].msix_map_table.base_addr = NULL;
> + intr_mgt->func_intr_res[func_id].msix_map_table.size = 0;
> +alloc_interrupts_err:
> + return ret;
> +}
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH v20 net-next 05/10] net/nebula-matrix: add common resource implementation
From: Paolo Abeni @ 2026-07-02 9:42 UTC (permalink / raw)
To: illusion.wang
Cc: dimon.zhao, alvin.wang, sam.chen, netdev, andrew+netdev, corbet,
kuba, horms, linux-doc, pabeni, vadim.fedorenko, lukas.bulwahn,
edumazet, enelsonmoore, skhan, hkallweit1, linux-kernel
In-Reply-To: <20260630010718.4346-6-illusion.wang@nebula-matrix.com>
From: AI Reviewer <ai@example.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/
---
net/nebula-matrix: add common resource implementation
The patch adds the common part of the resource layer for the Nebula Matrix
driver, including conversion relationships among vsi_id, func_id, eth_id,
and pf_id.
> diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_resource.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_resource.c
> new file mode 100644
> index 0000000000000..6742da7c66376
> --- /dev/null
> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_resource.c
[ ... ]
> +int nbl_res_func_id_to_vsi_id(struct nbl_resource_mgt *res_mgt, u16 func_id,
> + u16 type, u16 *vsi_id)
> +{
[ ... ]
> + nbl_res_pf_dev_vsi_type_to_hw_vsi_type(type, &dst_type);
> + *vsi_id = vsi_info->serv_info[diff][dst_type].base_id;
> + return 0;
> +}
[ ... ]
> +void nbl_res_pf_dev_vsi_type_to_hw_vsi_type(u16 src_type,
> + enum nbl_vsi_serv_type *dst_type)
> +{
> + switch (src_type) {
> + case NBL_VSI_DATA:
> + *dst_type = NBL_VSI_SERV_PF_DATA_TYPE;
> + break;
> + default:
> + WARN_ON_ONCE(1);
[Severity: High]
Can a malicious or buggy Virtual Function (VF) trigger this warning by
passing an invalid type?
Because nbl_res_pf_dev_vsi_type_to_hw_vsi_type() returns void, it cannot
propagate an error back to the caller. When an invalid type is processed,
nbl_res_func_id_to_vsi_id() silently defaults to NBL_VSI_SERV_PF_DATA_TYPE
and returns 0 for success.
Should this function return an error code so that nbl_res_func_id_to_vsi_id()
can properly reject the invalid type instead of reporting a false success?
> + break;
> + }
> +}
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH v20 net-next 04/10] net/nebula-matrix: add channel layer
From: Paolo Abeni @ 2026-07-02 9:42 UTC (permalink / raw)
To: illusion.wang
Cc: dimon.zhao, alvin.wang, sam.chen, netdev, andrew+netdev, corbet,
kuba, horms, linux-doc, pabeni, vadim.fedorenko, lukas.bulwahn,
edumazet, enelsonmoore, skhan, hkallweit1, linux-kernel
In-Reply-To: <20260630010718.4346-5-illusion.wang@nebula-matrix.com>
From: AI Reviewer <ai@example.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/
---
Subject: net/nebula-matrix: add channel layer
This patch introduces a channel management layer for structured communication
between components and drivers. It implements message registration, sending,
and acknowledgment via hardware mailbox queues.
> diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_channel/nbl_channel.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_channel/nbl_channel.c
> index c7689f0e40293..eca9495449938 100644
> --- a/drivers/net/ethernet/nebula-matrix/nbl/nbl_channel/nbl_channel.c
> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_channel/nbl_channel.c
[ ... ]
> @@ -2,12 +2,997 @@
[ ... ]
> +static int nbl_chan_teardown_queue(struct nbl_channel_mgt *chan_mgt,
> + u8 chan_type)
> +{
[ ... ]
> + /*
> + * Lock and unlock to ensure all in-flight callers have released the
> + * lock. After unlock returns, we're guaranteed no other thread is
> + * using the lock, so it's safe to destroy it.
> + */
> + mutex_lock(&chan_info->txq_lock);
> + mutex_unlock(&chan_info->txq_lock);
> + wait_for_completion(&chan_info->teardown_done);
> + mutex_destroy(&chan_info->txq_lock);
> +
> + return 0;
> +}
[Severity: High]
Does this code deadlock or fail to synchronize?
Because nbl_chan_send_msg() calls complete(&chan_info->teardown_done) on
every exit path, it increments the completion counter for every message sent.
If no messages were ever sent, the counter is 0 and this wait_for_completion()
will block forever.
If messages were sent, the counter is greater than 0, so this returns
immediately without waiting for active senders. It then destroys txq_lock
while other threads might still be using it, which can cause a use-after-free
or lock corruption.
[ ... ]
> +static int nbl_chan_update_txqueue(struct nbl_channel_mgt *chan_mgt,
> + struct nbl_chan_info *chan_info,
> + struct nbl_chan_tx_param *param)
> +{
[ ... ]
> + if (param->arg_len > NBL_CHAN_TX_DESC_EMBEDDED_DATA_LEN) {
> + memcpy(tx_buf->va, param->arg, param->arg_len);
> + tx_desc->buf_addr = cpu_to_le64(tx_buf->pa);
> + tx_desc->buf_len = cpu_to_le16(param->arg_len);
> + tx_desc->data_len = 0;
> + memset(tx_desc->data, 0, sizeof(tx_desc->data));
> + } else {
> + memset(tx_desc->data, 0, sizeof(tx_desc->data));
> + memcpy(tx_desc->data, param->arg, param->arg_len);
> + tx_desc->buf_len = 0;
> + tx_desc->data_len = cpu_to_le16(param->arg_len);
> + }
[Severity: High]
Does this code leak host physical memory addresses to the receiving endpoint?
In the else branch (when embedding data), tx_desc->buf_addr is not cleared.
Since the TX descriptor ring is reused circularly, if this slot previously held
a large message, buf_addr still contains the physical DMA address of the host's
buffer.
This leaked physical address is written to memory shared with the receiving
endpoint, potentially crossing a security boundary.
[ ... ]
> +static void nbl_chan_clean_queue(struct nbl_channel_mgt *chan_mgt,
> + struct nbl_chan_info *chan_info)
> +{
[ ... ]
> + while (le16_to_cpu(rx_desc->flags) & BIT(NBL_CHAN_RX_DESC_USED)) {
[ ... ]
> + if (--budget == 0)
> + break;
> + }
> + rxq->next_to_clean = next_to_clean;
> +}
[Severity: High]
Can this permanently stall the RX queue?
The loop exits when the budget is exhausted, but nbl_chan_clean_queue()
returns void and doesn't indicate that more packets remain.
The workqueue handler does not check if the RX queue was completely drained
and won't reschedule the work item. If the ring wasn't completely drained,
edge-triggered MSI-X interrupts won't fire again until a new packet arrives,
leading to a stalled queue and missed messages.
[ ... ]
> +static int nbl_chan_send_msg(struct nbl_channel_mgt *chan_mgt,
> + struct nbl_chan_send_info *chan_send)
> +{
[ ... ]
> + if (!chan_send->ack) {
> + complete(&chan_info->teardown_done);
> + return 0;
> + }
[Severity: High]
Is it intentional to call complete(&chan_info->teardown_done) here and on
all other exit paths of nbl_chan_send_msg()?
This increments the completion counter on every message sent, which breaks the
synchronization in nbl_chan_teardown_queue() (as mentioned above).
[ ... ]
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH v3] net/sched: cake: reject overhead values that underflow length
From: Toke Høiland-Jørgensen @ 2026-07-02 9:40 UTC (permalink / raw)
To: Samuel Moelius
Cc: Samuel Moelius, Jamal Hadi Salim, Jiri Pirko, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
moderated list:CAKE QDISC, open list:TC subsystem, open list
In-Reply-To: <20260702000758.297407.e5c888d9d99d.cake-overhead-underflow@trailofbits.com>
Samuel Moelius <sam.moelius@trailofbits.com> writes:
> CAKE accepts signed overhead values and stores them in an s16, but the
> adjusted packet length calculation uses unsigned arithmetic. A negative
> effective length can therefore wrap to a large value.
>
> Such configurations make rate accounting depend on integer wraparound
> rather than on the packet size userspace intended to model. A static
> netlink lower bound is not enough because packets reaching CAKE can be
> smaller than any reasonable manual-overhead allowance.
>
> Fold the signed overhead adjustment into the existing datapath MPU clamp
> so negative adjusted lengths are clamped before link-layer framing
> adjustments.
>
> Fixes: a729b7f0bd5b ("sch_cake: Add overhead compensation support to the rate shaper")
> Assisted-by: Codex:gpt-5.5-cyber-preview
> Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>
Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
^ permalink raw reply
* Re: [PATCH net-next v2 0/5] net: dsa: realtek: rtl8366rb: Use generic RTL83xx code
From: patchwork-bot+netdevbpf @ 2026-07-02 9:40 UTC (permalink / raw)
To: Linus Walleij
Cc: luizluca, alsi, andrew, olteanv, davem, edumazet, kuba, pabeni,
netdev
In-Reply-To: <20260630-rtl8366rb-improvements-v2-0-05eb9d6a37f5@kernel.org>
Hello:
This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Tue, 30 Jun 2026 13:19:40 +0200 you wrote:
> As a follow-up to Luiz's and Alvin's series improvining the
> generic handling of the Realtek DSA switches, this small
> series brings the RTL8366RB closer to the way things are done
> in the RTL8365MB driver.
>
> This patch series switches over to using the generic helpers
> for:
>
> [...]
Here is the summary with links:
- [net-next,v2,1/5] net: dsa: realtek: rtl83xx: Make learning optional in join/leave
https://git.kernel.org/netdev/net-next/c/69bf497cfa79
- [net-next,v2,2/5] net: dsa: realtek: rtl8366rb: Switch to generic port_bridge* handlers
https://git.kernel.org/netdev/net-next/c/82ddf181448b
- [net-next,v2,3/5] net: dsa: realtek: rtl8366rb: Use DSA port iterators
https://git.kernel.org/netdev/net-next/c/cc61d5d7c205
- [net-next,v2,4/5] net: dsa: realtek: rtl8366rb: Disable STP learning on all ports in setup
https://git.kernel.org/netdev/net-next/c/e058ab0c4616
- [net-next,v2,5/5] net: dsa: realtek: rtl8366rb: Switch to generic learning enablement
https://git.kernel.org/netdev/net-next/c/b269a0596191
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH] usb: atm: ueagle: fix use-after-free in uea_upload_pre_firmware()
From: Stanislaw Gruszka @ 2026-07-02 9:37 UTC (permalink / raw)
To: Deepanshu Kartikey
Cc: castet.matthieu, 3chas3, gregkh, linux-atm-general, netdev,
linux-usb, linux-kernel, syzbot+3d45d763d18796f97412,
Mauricio Faria de Oliveira
In-Reply-To: <20260630041716.97102-1-kartikey406@gmail.com>
Hi, thanks for working at this,
On Tue, Jun 30, 2026 at 09:47:16AM +0530, Deepanshu Kartikey wrote:
> uea_load_firmware() calls request_firmware_nowait() passing a raw
> struct usb_device pointer as context, without holding a reference
> to it.
>
> If the USB device is disconnected before the firmware workqueue
> fires, the usb_device and its usb_interface objects are freed while
> uea_upload_pre_firmware() is still pending on the workqueue. When
> the callback eventually runs, it accesses the freed memory causing
> a slab-use-after-free:
>
> BUG: KASAN: slab-use-after-free in __intf_to_usbdev
> include/linux/usb.h:752 [inline]
> BUG: KASAN: slab-use-after-free in uea_upload_pre_firmware+0x8d/0x640
> drivers/usb/atm/ueagle-atm.c:598
> Read of size 8 at addr ffff88802b0710b8 by task kworker/0:2/1664
>
> Fix by calling usb_get_dev() before queuing the firmware request to
> pin the usb_device in memory for the lifetime of the async operation,
> and usb_put_dev() in the callback once it is finished with the
> pointer. On the error path where request_firmware_nowait() itself
> fails, drop the reference immediately since the callback will never
> fire.
> Reported-by: syzbot+3d45d763d18796f97412@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3d45d763d18796f97412
I think the problem is not lack of usb device reference.
request_firmware_nowait() does get_device() and after fw work
finish - put_device().
I suspect the issue is that syskaller corrupt descriptor such
the below condition:
else if (usb->config->desc.bNumInterfaces == 1)
is not met for pre-firmware device.
Adding Mauricio, who has setup for reproducing syskaller bugs on ueagle.
Hopefully he can confirm the diagnostic. If it's correct, we could
either save flag to recognize pre-firmware device, or separate driver
probe/disconnect for pre-firmware and post-firmware, to fix the issue.
Regards
Stanislaw
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
> drivers/usb/atm/ueagle-atm.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
> index d610cdcef7d0..686cc58fb89f 100644
> --- a/drivers/usb/atm/ueagle-atm.c
> +++ b/drivers/usb/atm/ueagle-atm.c
> @@ -663,6 +663,7 @@ static void uea_upload_pre_firmware(const struct firmware *fw_entry,
> uea_err(usb, "firmware is corrupted\n");
> err:
> release_firmware(fw_entry);
> + usb_put_dev(usb);
> }
>
> /*
> @@ -693,12 +694,14 @@ static int uea_load_firmware(struct usb_device *usb, unsigned int ver)
> break;
> }
>
> + usb_get_dev(usb);
> ret = request_firmware_nowait(THIS_MODULE, 1, fw_name, &usb->dev,
> GFP_KERNEL, usb,
> uea_upload_pre_firmware);
> - if (ret)
> + if (ret) {
> uea_err(usb, "firmware %s is not available\n", fw_name);
> - else
> + usb_put_dev(usb);
> + } else
> uea_info(usb, "loading firmware %s\n", fw_name);
>
> return ret;
> --
> 2.43.0
>
^ permalink raw reply
* [PATCH iwl-next v1 6/6] ixgbe: take rtnl lock before ixgbe_reset() is called
From: Jedrzej Jagielski @ 2026-07-02 9:15 UTC (permalink / raw)
To: intel-wired-lan; +Cc: anthony.l.nguyen, netdev, Jedrzej Jagielski
In-Reply-To: <20260702091553.57112-1-jedrzej.jagielski@intel.com>
Previous commit introduced ixgbe_mac_addr_refresh which touches netdev
struct by updating mac addr. It should operate after taking rtnl lock.
One of the callers is ixgbe_reset(). Most of scenarios when ixgbe_reset()
is called met taking lock requirement, but there is a ixgbe_resume() path
which calls ixgbe_reset() -> ixgbe_mac_addr_refresh() without taking
the lock. So there is a risk of race.
Move rtnl_lock() before ixgbe_reset() is called.
Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index ce2b1e208c0f..c7261eb0e9b0 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7574,11 +7574,11 @@ static int ixgbe_resume(struct device *dev_d)
device_wakeup_disable(dev_d);
+ rtnl_lock();
ixgbe_reset(adapter);
IXGBE_WRITE_REG(&adapter->hw, IXGBE_WUS, ~0);
- rtnl_lock();
err = ixgbe_init_interrupt_scheme(adapter);
if (!err && netif_running(netdev))
err = ixgbe_open(netdev);
--
2.31.1
^ permalink raw reply related
* [PATCH iwl-next v1 5/6] ixgbe: E610: add MAC address runtime refresh
From: Jedrzej Jagielski @ 2026-07-02 9:15 UTC (permalink / raw)
To: intel-wired-lan; +Cc: anthony.l.nguyen, netdev, Jedrzej Jagielski
In-Reply-To: <20260702091553.57112-1-jedrzej.jagielski@intel.com>
Whenever MGMT requests MAC addr change and FW does it, driver does not
get aligned to that change. Current - legacy approach doesn't handle
such scenario.
Poll RAR0 (Receive Address Register) each service task cycle and update
driver and netdev structs once new MAC addr is detected. It may happen
that FW updates MAC address stored in RAR0 during runtime so SW shall fetch
the new address.
Refresh addr also during reset path to ensure the address survives RAR0
clearing during init_hw().
Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 37 +++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d1cfe913081f..ce2b1e208c0f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6499,6 +6499,36 @@ void ixgbe_disable_tx(struct ixgbe_adapter *adapter)
}
}
+static void ixgbe_mac_addr_refresh(struct ixgbe_adapter *adapter)
+{
+ struct net_device *netdev = adapter->netdev;
+ struct ixgbe_hw *hw = &adapter->hw;
+ int err;
+
+ if (hw->mac.type != ixgbe_mac_e610)
+ return;
+
+ /* fetch address stored currently in RAR0 in case the addr has been
+ * altered by FW; if so, use it as the default one
+ */
+ err = hw->mac.ops.get_mac_addr(hw, hw->mac.addr);
+ if (err) {
+ e_dev_warn("Cannot get MAC address\n");
+ return;
+ }
+
+ if (ether_addr_equal(netdev->dev_addr, hw->mac.addr) ||
+ !is_valid_ether_addr(hw->mac.addr))
+ return;
+
+ ASSERT_RTNL();
+
+ eth_hw_addr_set(netdev, hw->mac.addr);
+ ether_addr_copy(adapter->mac_table[0].addr, hw->mac.addr);
+
+ call_netdevice_notifiers(NETDEV_CHANGEADDR, netdev);
+}
+
void ixgbe_reset(struct ixgbe_adapter *adapter)
{
struct ixgbe_hw *hw = &adapter->hw;
@@ -6516,6 +6546,8 @@ void ixgbe_reset(struct ixgbe_adapter *adapter)
IXGBE_FLAG2_SFP_NEEDS_RESET);
adapter->flags &= ~IXGBE_FLAG_NEED_LINK_CONFIG;
+ ixgbe_mac_addr_refresh(adapter);
+
err = hw->mac.ops.init_hw(hw);
switch (err) {
case 0:
@@ -8701,6 +8733,11 @@ static void ixgbe_service_task(struct work_struct *work)
ixgbe_handle_fw_event(adapter);
ixgbe_check_media_subtask(adapter);
}
+
+ rtnl_lock();
+ ixgbe_mac_addr_refresh(adapter);
+ rtnl_unlock();
+
ixgbe_reset_subtask(adapter);
ixgbe_phy_interrupt_subtask(adapter);
ixgbe_sfp_detection_subtask(adapter);
--
2.31.1
^ permalink raw reply related
* [PATCH iwl-next v1 4/6] ixgbe: E610: re-enable LSE unconditionally
From: Jedrzej Jagielski @ 2026-07-02 9:15 UTC (permalink / raw)
To: intel-wired-lan
Cc: anthony.l.nguyen, netdev, Jedrzej Jagielski, Aleksandr Loktionov
In-Reply-To: <20260702091553.57112-1-jedrzej.jagielski@intel.com>
ixgbe_aci_get_link_info() currently allows to [en/dis]able LSE by @ena_lse
param. This isn't proper approach as this function objective is completely
different than toggling LSE feature. Creating such parameter was probably
dictated by the fact the flag corresponding for LSE enablement was placed
within the ACI command used by the function.
LSE should not be switched off while the ixgbe driver is running.
Current driver behavior doesn't provide such possibility for the user.
Enable LSE by default whenever 0x0607 ACI command is sent. Such change
corresponds to the fact that FW disables LSE whenever new event is
triggered. ixgbe_aci_get_link_info() is a part of routine handling LSE
events, so it ensures that LSE remain enabled.
Remove wrappers utilizing @ena_lse param to [en/dis]able LSE as they
are no longer valid.
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 25 ++++---------------
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h | 3 +--
.../net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 23 +----------------
4 files changed, 8 insertions(+), 45 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
index 54cc0f116e88..ec7d8073efe6 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
@@ -1391,7 +1391,7 @@ int ixgbe_update_link_info(struct ixgbe_hw *hw)
li = &hw->link.link_info;
- err = ixgbe_aci_get_link_info(hw, true, NULL);
+ err = ixgbe_aci_get_link_info(hw, NULL);
if (err)
return err;
@@ -1445,7 +1445,6 @@ int ixgbe_get_link_status(struct ixgbe_hw *hw, bool *link_up)
/**
* ixgbe_aci_get_link_info - get the link status
* @hw: pointer to the HW struct
- * @ena_lse: enable/disable LinkStatusEvent reporting
* @link: pointer to link status structure - optional
*
* Get the current Link Status using ACI command (0x607).
@@ -1454,8 +1453,7 @@ int ixgbe_get_link_status(struct ixgbe_hw *hw, bool *link_up)
*
* Return: the link status of the adapter.
*/
-int ixgbe_aci_get_link_info(struct ixgbe_hw *hw, bool ena_lse,
- struct ixgbe_link_status *link)
+int ixgbe_aci_get_link_info(struct ixgbe_hw *hw, struct ixgbe_link_status *link)
{
struct ixgbe_aci_cmd_get_link_status_data link_data = {};
struct ixgbe_aci_cmd_get_link_status *resp;
@@ -1474,7 +1472,7 @@ int ixgbe_aci_get_link_info(struct ixgbe_hw *hw, bool ena_lse,
hw_fc_info = &hw->fc;
ixgbe_fill_dflt_direct_cmd_desc(&desc, ixgbe_aci_opc_get_link_status);
- cmd_flags = (ena_lse) ? IXGBE_ACI_LSE_ENA : IXGBE_ACI_LSE_DIS;
+ cmd_flags = IXGBE_ACI_LSE_ENA;
resp = libie_aq_raw(&desc);
resp->cmd_flags = cpu_to_le16(cmd_flags);
resp->lport_num = hw->bus.func;
@@ -1561,19 +1559,6 @@ static int ixgbe_aci_init_event_mask(struct ixgbe_hw *hw)
return ixgbe_aci_set_event_mask(hw, (u8)hw->bus.func, mask);
}
-/**
- * ixgbe_configure_lse - enable/disable link status events
- * @hw: pointer to the HW struct
- * @activate: true for enable lse, false otherwise
- *
- * Return: the exit code of the operation.
- */
-int ixgbe_configure_lse(struct ixgbe_hw *hw, bool activate)
-{
- /* Enabling link status events generation by fw. */
- return ixgbe_aci_get_link_info(hw, activate, NULL);
-}
-
/**
* ixgbe_start_hw_e610 - Prepare hardware for Tx/Rx
* @hw: pointer to hardware structure
@@ -1913,7 +1898,7 @@ void ixgbe_fc_autoneg_e610(struct ixgbe_hw *hw)
/* Get current link err.
* Current FC mode will be stored in the hw context.
*/
- err = ixgbe_aci_get_link_info(hw, true, NULL);
+ err = ixgbe_aci_get_link_info(hw, NULL);
if (err)
goto no_autoneg;
@@ -2219,7 +2204,7 @@ int ixgbe_setup_phy_link_e610(struct ixgbe_hw *hw)
u64 phy_type_low = 0, phy_type_high = 0;
int err;
- err = ixgbe_aci_get_link_info(hw, true, NULL);
+ err = ixgbe_aci_get_link_info(hw, NULL);
if (err)
return err;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
index ecda35a77adb..4fb4ab99d458 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
@@ -32,9 +32,8 @@ int ixgbe_aci_set_phy_cfg(struct ixgbe_hw *hw,
int ixgbe_aci_set_link_restart_an(struct ixgbe_hw *hw, bool ena_link);
int ixgbe_update_link_info(struct ixgbe_hw *hw);
int ixgbe_get_link_status(struct ixgbe_hw *hw, bool *link_up);
-int ixgbe_aci_get_link_info(struct ixgbe_hw *hw, bool ena_lse,
+int ixgbe_aci_get_link_info(struct ixgbe_hw *hw,
struct ixgbe_link_status *link);
-int ixgbe_configure_lse(struct ixgbe_hw *hw, bool activate);
int ixgbe_aci_set_port_id_led(struct ixgbe_hw *hw, bool orig_mode);
enum ixgbe_media_type ixgbe_get_media_type_e610(struct ixgbe_hw *hw);
int ixgbe_setup_link_e610(struct ixgbe_hw *hw, ixgbe_link_speed speed,
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index b8e85bc91a27..5d71c1011cf4 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -3647,7 +3647,7 @@ static int ixgbe_get_eee_e610(struct net_device *netdev,
linkmode_zero(kedata->supported);
linkmode_zero(kedata->advertised);
- err = ixgbe_aci_get_link_info(hw, true, &link);
+ err = ixgbe_aci_get_link_info(hw, &link);
if (err)
return err;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index f33a534490b5..d1cfe913081f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5966,19 +5966,6 @@ static void ixgbe_configure(struct ixgbe_adapter *adapter)
ixgbe_configure_dfwd(adapter);
}
-/**
- * ixgbe_enable_link_status_events - enable link status events
- * @adapter: pointer to the adapter structure
- *
- * Enables link status events by invoking ixgbe_configure_lse()
- *
- * Return: the exit code of the operation.
- */
-static int ixgbe_enable_link_status_events(struct ixgbe_adapter *adapter)
-{
- return ixgbe_configure_lse(&adapter->hw, true);
-}
-
/**
* ixgbe_sfp_link_config - set up SFP+ link
* @adapter: pointer to private adapter struct
@@ -6008,8 +5995,6 @@ static void ixgbe_sfp_link_config(struct ixgbe_adapter *adapter)
**/
static int ixgbe_non_sfp_link_config(struct ixgbe_hw *hw)
{
- struct ixgbe_adapter *adapter = container_of(hw, struct ixgbe_adapter,
- hw);
bool autoneg, link_up = false;
int ret = -EIO;
u32 speed;
@@ -6035,14 +6020,8 @@ static int ixgbe_non_sfp_link_config(struct ixgbe_hw *hw)
if (ret)
return ret;
- if (hw->mac.ops.setup_link) {
- if (adapter->hw.mac.type == ixgbe_mac_e610) {
- ret = ixgbe_enable_link_status_events(adapter);
- if (ret)
- return ret;
- }
+ if (hw->mac.ops.setup_link)
ret = hw->mac.ops.setup_link(hw, speed, link_up);
- }
return ret;
}
--
2.31.1
^ permalink raw reply related
* [PATCH iwl-next v1 3/6] ixgbe: E610: do not disable LSE on driver down/remove
From: Jedrzej Jagielski @ 2026-07-02 9:15 UTC (permalink / raw)
To: intel-wired-lan
Cc: anthony.l.nguyen, netdev, Jedrzej Jagielski, Aleksandr Loktionov
In-Reply-To: <20260702091553.57112-1-jedrzej.jagielski@intel.com>
For the first case:
LSE especially shall remain enabled when interface is down and link watchdog
cannot track status of the link. Then the only way for link update is by
handling LSE.
For the second case:
There's no value in disabling LSE when the driver's unloading is about to be
terminated.
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 2c8968b9b86c..f33a534490b5 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5979,19 +5979,6 @@ static int ixgbe_enable_link_status_events(struct ixgbe_adapter *adapter)
return ixgbe_configure_lse(&adapter->hw, true);
}
-/**
- * ixgbe_disable_link_status_events - disable link status events
- * @adapter: pointer to the adapter structure
- *
- * Disables link status events by invoking ixgbe_configure_lse()
- *
- * Return: the exit code of the operation.
- */
-static int ixgbe_disable_link_status_events(struct ixgbe_adapter *adapter)
-{
- return ixgbe_configure_lse(&adapter->hw, false);
-}
-
/**
* ixgbe_sfp_link_config - set up SFP+ link
* @adapter: pointer to private adapter struct
@@ -6775,8 +6762,6 @@ void ixgbe_down(struct ixgbe_adapter *adapter)
ixgbe_clean_all_tx_rings(adapter);
ixgbe_clean_all_rx_rings(adapter);
- if (adapter->hw.mac.type == ixgbe_mac_e610)
- ixgbe_disable_link_status_events(adapter);
}
/**
@@ -12143,9 +12128,6 @@ static void ixgbe_remove(struct pci_dev *pdev)
set_bit(__IXGBE_REMOVING, &adapter->state);
cancel_work_sync(&adapter->service_task);
- if (adapter->hw.mac.type == ixgbe_mac_e610)
- ixgbe_disable_link_status_events(adapter);
-
if (adapter->mii_bus)
mdiobus_unregister(adapter->mii_bus);
--
2.31.1
^ permalink raw reply related
* [PATCH iwl-next v1 2/6] ixgbe: E610: prevent from disabling LSE
From: Jedrzej Jagielski @ 2026-07-02 9:15 UTC (permalink / raw)
To: intel-wired-lan
Cc: anthony.l.nguyen, netdev, Jedrzej Jagielski, Aleksandr Loktionov
In-Reply-To: <20260702091553.57112-1-jedrzej.jagielski@intel.com>
LSE is not allowed to be switched off while the ixgbe driver is running.
Some of the ixgbe_aci_get_link_info() callers called it with @ena_lse
equal to false what was causing issues.
There was a known issue when FW after being called by Redfish MGMT request
to disable an interface wasn't able to notify the driver about link status
change due to disabled LSE reception on the driver side. As a result driver
exposed incorrect link status.
There's no any requirement forcing to disable LSE either during configuring
FC, either during setting PHY params.
Force calling ixgbe_aci_get_link_info() with @ena_lse == true.
Fixes: 4600cdf9f5ac ("ixgbe: Enable link management in E610 device")
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
index d451da68fd6d..54cc0f116e88 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
@@ -1913,7 +1913,7 @@ void ixgbe_fc_autoneg_e610(struct ixgbe_hw *hw)
/* Get current link err.
* Current FC mode will be stored in the hw context.
*/
- err = ixgbe_aci_get_link_info(hw, false, NULL);
+ err = ixgbe_aci_get_link_info(hw, true, NULL);
if (err)
goto no_autoneg;
@@ -2219,7 +2219,7 @@ int ixgbe_setup_phy_link_e610(struct ixgbe_hw *hw)
u64 phy_type_low = 0, phy_type_high = 0;
int err;
- err = ixgbe_aci_get_link_info(hw, false, NULL);
+ err = ixgbe_aci_get_link_info(hw, true, NULL);
if (err)
return err;
--
2.31.1
^ permalink raw reply related
* [PATCH iwl-next v1 1/6] ixgbe: E610: init Link Status Events mask just once
From: Jedrzej Jagielski @ 2026-07-02 9:15 UTC (permalink / raw)
To: intel-wired-lan
Cc: anthony.l.nguyen, netdev, Jedrzej Jagielski, Aleksandr Loktionov
In-Reply-To: <20260702091553.57112-1-jedrzej.jagielski@intel.com>
Current approach is that LSE mask is configured each time the LSE feature
itself is toggled. The set of bits corresponding to LSE trigger causes
remain the same for the whole ixgbe lifecycle. There's no support for
enabling or disabling specific bits within LSE mask.
This is redundant; there's no need to configure LSE mask over and over.
The mask persists in hardware and only needs to be set once so should
be separated from the LSE toggling logic.
Do it just once at the init phase.
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 -
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 34 ++++++++++++-------
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h | 3 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 28 +++------------
4 files changed, 27 insertions(+), 39 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 30f62174acf2..17e0a9824777 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -821,7 +821,6 @@ struct ixgbe_adapter {
struct ixgbe_mac_addr *mac_table;
u8 tx_hang_count[IXGBE_MAX_TX_QUEUES];
struct kobject *info_kobj;
- u16 lse_mask;
#ifdef CONFIG_IXGBE_HWMON
struct hwmon_buff *ixgbe_hwmon_buff;
#endif /* CONFIG_IXGBE_HWMON */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
index 831cfe9a4697..d451da68fd6d 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
@@ -1536,7 +1536,7 @@ int ixgbe_aci_get_link_info(struct ixgbe_hw *hw, bool ena_lse,
*
* Return: the exit code of the operation.
*/
-int ixgbe_aci_set_event_mask(struct ixgbe_hw *hw, u8 port_num, u16 mask)
+static int ixgbe_aci_set_event_mask(struct ixgbe_hw *hw, u8 port_num, u16 mask)
{
struct ixgbe_aci_cmd_set_event_mask *cmd;
struct libie_aq_desc desc;
@@ -1551,25 +1551,25 @@ int ixgbe_aci_set_event_mask(struct ixgbe_hw *hw, u8 port_num, u16 mask)
return ixgbe_aci_send_cmd(hw, &desc, NULL, 0);
}
+static int ixgbe_aci_init_event_mask(struct ixgbe_hw *hw)
+{
+ u16 mask = ~((u16)(IXGBE_ACI_LINK_EVENT_UPDOWN |
+ IXGBE_ACI_LINK_EVENT_MEDIA_NA |
+ IXGBE_ACI_LINK_EVENT_MODULE_QUAL_FAIL |
+ IXGBE_ACI_LINK_EVENT_PHY_FW_LOAD_FAIL));
+
+ return ixgbe_aci_set_event_mask(hw, (u8)hw->bus.func, mask);
+}
+
/**
* ixgbe_configure_lse - enable/disable link status events
* @hw: pointer to the HW struct
* @activate: true for enable lse, false otherwise
- * @mask: event mask to be set; a set bit means deactivation of the
- * corresponding event
- *
- * Set the event mask and then enable or disable link status events
*
* Return: the exit code of the operation.
*/
-int ixgbe_configure_lse(struct ixgbe_hw *hw, bool activate, u16 mask)
+int ixgbe_configure_lse(struct ixgbe_hw *hw, bool activate)
{
- int err;
-
- err = ixgbe_aci_set_event_mask(hw, (u8)hw->bus.func, mask);
- if (err)
- return err;
-
/* Enabling link status events generation by fw. */
return ixgbe_aci_get_link_info(hw, activate, NULL);
}
@@ -1597,6 +1597,16 @@ static int ixgbe_start_hw_e610(struct ixgbe_hw *hw)
ixgbe_start_hw_gen2(hw);
+ err = ixgbe_aci_init_event_mask(hw);
+ /* In case of error just log it, don't fail the whole init */
+ if (err) {
+ struct ixgbe_adapter *adapter =
+ container_of(hw, struct ixgbe_adapter, hw);
+
+ e_dev_err("Cannot configure Link Status Event mask, err = %d\n",
+ err);
+ }
+
return 0;
}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
index 2cb76a3d30ae..ecda35a77adb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
@@ -34,8 +34,7 @@ int ixgbe_update_link_info(struct ixgbe_hw *hw);
int ixgbe_get_link_status(struct ixgbe_hw *hw, bool *link_up);
int ixgbe_aci_get_link_info(struct ixgbe_hw *hw, bool ena_lse,
struct ixgbe_link_status *link);
-int ixgbe_aci_set_event_mask(struct ixgbe_hw *hw, u8 port_num, u16 mask);
-int ixgbe_configure_lse(struct ixgbe_hw *hw, bool activate, u16 mask);
+int ixgbe_configure_lse(struct ixgbe_hw *hw, bool activate);
int ixgbe_aci_set_port_id_led(struct ixgbe_hw *hw, bool orig_mode);
enum ixgbe_media_type ixgbe_get_media_type_e610(struct ixgbe_hw *hw);
int ixgbe_setup_link_e610(struct ixgbe_hw *hw, ixgbe_link_speed speed,
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 7a0783c1abc1..2c8968b9b86c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5969,23 +5969,14 @@ static void ixgbe_configure(struct ixgbe_adapter *adapter)
/**
* ixgbe_enable_link_status_events - enable link status events
* @adapter: pointer to the adapter structure
- * @mask: event mask to be set
*
* Enables link status events by invoking ixgbe_configure_lse()
*
* Return: the exit code of the operation.
*/
-static int ixgbe_enable_link_status_events(struct ixgbe_adapter *adapter,
- u16 mask)
+static int ixgbe_enable_link_status_events(struct ixgbe_adapter *adapter)
{
- int err;
-
- err = ixgbe_configure_lse(&adapter->hw, true, mask);
- if (err)
- return err;
-
- adapter->lse_mask = mask;
- return 0;
+ return ixgbe_configure_lse(&adapter->hw, true);
}
/**
@@ -5998,14 +5989,7 @@ static int ixgbe_enable_link_status_events(struct ixgbe_adapter *adapter,
*/
static int ixgbe_disable_link_status_events(struct ixgbe_adapter *adapter)
{
- int err;
-
- err = ixgbe_configure_lse(&adapter->hw, false, adapter->lse_mask);
- if (err)
- return err;
-
- adapter->lse_mask = 0;
- return 0;
+ return ixgbe_configure_lse(&adapter->hw, false);
}
/**
@@ -6039,10 +6023,6 @@ static int ixgbe_non_sfp_link_config(struct ixgbe_hw *hw)
{
struct ixgbe_adapter *adapter = container_of(hw, struct ixgbe_adapter,
hw);
- u16 mask = ~((u16)(IXGBE_ACI_LINK_EVENT_UPDOWN |
- IXGBE_ACI_LINK_EVENT_MEDIA_NA |
- IXGBE_ACI_LINK_EVENT_MODULE_QUAL_FAIL |
- IXGBE_ACI_LINK_EVENT_PHY_FW_LOAD_FAIL));
bool autoneg, link_up = false;
int ret = -EIO;
u32 speed;
@@ -6070,7 +6050,7 @@ static int ixgbe_non_sfp_link_config(struct ixgbe_hw *hw)
if (hw->mac.ops.setup_link) {
if (adapter->hw.mac.type == ixgbe_mac_e610) {
- ret = ixgbe_enable_link_status_events(adapter, mask);
+ ret = ixgbe_enable_link_status_events(adapter);
if (ret)
return ret;
}
--
2.31.1
^ permalink raw reply related
* [PATCH iwl-next v1 0/6] ixgbe: improve FW/SW data synchronization
From: Jedrzej Jagielski @ 2026-07-02 9:15 UTC (permalink / raw)
To: intel-wired-lan; +Cc: anthony.l.nguyen, netdev, Jedrzej Jagielski
There are some possible discrepancies between data shown by the driver
with the actual NIC/FW state.
First 4 patches address problem with link state correctness. In some
scenarios FW may not be able to notify the driver about link change.
Along with fix, refactor the whole approach to LSE (Link Status Events)
enablement. As there's no any reasonable argument for disabling LSE
during driver life cycle, make them constantly enabled.
5th and 6th patches address problem occurring when FW changes MAC address.
There's no any kind of notify/event accompanying that change. Therefore let the
driver to poll for the new address.
Jedrzej Jagielski (6):
ixgbe: E610: init Link Status Events mask just once
ixgbe: E610: prevent from disabling LSE
ixgbe: E610: do not disable LSE on driver down/remove
ixgbe: E610: re-enable LSE unconditionally
ixgbe: E610: add MAC address runtime refresh
ixgbe: take rtnl lock before ixgbe_reset() is called
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 -
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 49 ++++-----
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h | 4 +-
.../net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 100 +++++++-----------
5 files changed, 63 insertions(+), 93 deletions(-)
--
2.31.1
^ permalink raw reply
* Re: [PATCH net-next v2 3/5] net: dsa: realtek: rtl8366rb: Use DSA port iterators
From: Paolo Abeni @ 2026-07-02 9:31 UTC (permalink / raw)
To: Linus Walleij, Luiz Angelo Daros de Luca, Alvin Šipraga,
Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski
Cc: netdev
In-Reply-To: <20260630-rtl8366rb-improvements-v2-3-05eb9d6a37f5@kernel.org>
On 6/30/26 1:19 PM, Linus Walleij wrote:
> @@ -895,17 +898,47 @@ static int rtl8366rb_setup(struct dsa_switch *ds)
> if (ret)
> return ret;
>
> - /* Isolate all user ports so they can only send packets to itself and the CPU port */
> - for (i = 0; i < RTL8366RB_PORT_NUM_CPU; i++) {
> - ret = rtl8366rb_port_set_isolation(priv, i, BIT(RTL8366RB_PORT_NUM_CPU));
> + /* Start with all ports blocked, including unused ports */
> + dsa_switch_for_each_port(dp, ds) {
> + /* Start with all ports completely isolated */
> + ret = rtl8366rb_port_set_isolation(priv, dp->index, 0);
Sashiko (sigh!) noted that the above actually disables isolation entirely.
Since I guess/think/hope there will be at least a CPU port and an user
port per DSA, and later loops will finalize the setup correctly in such
a case, I think this is better handled as a follow-up.
/P
^ permalink raw reply
* [PATCH v2 1/2] net: fman: move IRQ registration after init to prevent NULL deref and UAF
From: ZhaoJinming @ 2026-07-02 9:28 UTC (permalink / raw)
To: pabeni
Cc: andrew+netdev, andrew, davem, edumazet, horms, kuba, linux-kernel,
madalin.bucur, netdev, sean.anderson, zhaojinming
In-Reply-To: <20260702092815.1206704-1-zhaojinming@uniontech.com>
read_dts_node() registers shared interrupt handlers via
devm_request_irq() with fman as dev_id. Two bugs exist in the
current code:
1) Pre-init NULL dereference: at registration time fman is only
partially initialized -- kzalloc_obj() zero-initializes all fields,
so fman->cfg and fman->fpm_regs are NULL. The handlers check
is_init_done(fman->cfg) to guard against incomplete init, but
is_init_done(NULL) returns true (intended to mean cfg was freed
after successful init), so the guard is bypassed and fpm_regs is
dereferenced. If another device on the same shared IRQ line fires
during the window between devm_request_irq() and fman_init(), the
handler accesses NULL fpm_regs via ioread32be(), causing a crash.
2) Use-after-free on probe failure: fman is allocated with
kzalloc_obj() (not devm), so on error paths in read_dts_node()
(ioremap failure, of_platform_populate failure) and fman_config(),
kfree(fman) is called while the devm IRQ handlers remain
registered. The driver core's subsequent devres_release_all() frees
the IRQ handlers, but during the window between kfree(fman) and
devm_free_irq(), a shared-IRQ spurious firing will dereference
the already-freed fman.
A previous attempt to fix issue #1 with an irq_ready flag protected
by READ_ONCE()/WRITE_ONCE() is insufficient on weakly-ordered
architectures. READ_ONCE()/WRITE_ONCE() only prevent compiler
optimization; they do not provide the memory ordering guarantees
(e.g., smp_store_release/smp_load_acquire) needed to ensure that
writes to register pointers are visible to the IRQ handler before
it observes the flag as true.
Fix both issues by moving devm_request_irq() out of read_dts_node()
and into fman_probe(), after both fman_config() and fman_init()
have completed. This eliminates both race windows: by the time the
handlers are registered, all register pointers are initialized
(preventing the NULL dereference), and since fman is never freed
after this point, the use-after-free cannot occur either.
Add an 'irq' field to struct fman_dts_params so that the primary IRQ
number parsed in read_dts_node() is available to fman_probe().
This replaces the previous approach (irq_ready flag with READ_ONCE)
and also supersedes the separate UAF fix patch ("fsl_fman: fix
use-after-free on IRQF_SHARED handler after probe failure" v3),
as moving IRQ registration after init resolves both issues
in a single change.
Fixes: 414fd46e7762 ("fsl/fman: Add FMan support")
Link: https://lore.kernel.org/netdev/20260626162323.GE1310988@horms.kernel.org/
Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
drivers/net/ethernet/freescale/fman/fman.c | 52 +++++++++++++---------
drivers/net/ethernet/freescale/fman/fman.h | 1 +
2 files changed, 32 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fman/fman.c b/drivers/net/ethernet/freescale/fman/fman.c
index 013273a2de32..a33c9c26f99c 100644
--- a/drivers/net/ethernet/freescale/fman/fman.c
+++ b/drivers/net/ethernet/freescale/fman/fman.c
@@ -2693,7 +2693,7 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
void __iomem *base_addr;
struct resource *res;
u32 val, range[2];
- int err, irq;
+ int err;
struct clk *clk;
u32 clk_rate;
@@ -2715,7 +2715,7 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
err = platform_get_irq(of_dev, 0);
if (err < 0)
goto fman_node_put;
- irq = err;
+ fman->dts_params.irq = err;
/* Get the FM error interrupt */
err = platform_get_irq(of_dev, 1);
@@ -2771,25 +2771,6 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
of_node_put(muram_node);
- err = devm_request_irq(&of_dev->dev, irq, fman_irq, IRQF_SHARED,
- "fman", fman);
- if (err < 0) {
- dev_err(&of_dev->dev, "%s: irq %d allocation failed (error = %d)\n",
- __func__, irq, err);
- goto fman_free;
- }
-
- if (fman->dts_params.err_irq != 0) {
- err = devm_request_irq(&of_dev->dev, fman->dts_params.err_irq,
- fman_err_irq, IRQF_SHARED,
- "fman-err", fman);
- if (err < 0) {
- dev_err(&of_dev->dev, "%s: irq %d allocation failed (error = %d)\n",
- __func__, fman->dts_params.err_irq, err);
- goto fman_free;
- }
- }
-
base_addr = devm_platform_get_and_ioremap_resource(of_dev, 0, &res);
if (IS_ERR(base_addr)) {
err = PTR_ERR(base_addr);
@@ -2846,6 +2827,35 @@ static int fman_probe(struct platform_device *of_dev)
return -EINVAL;
}
+ /* Register IRQ handlers only after initialization is complete.
+ * This prevents two issues:
+ * 1) Pre-init NULL dereference: is_init_done(NULL) returns true,
+ * so a shared-IRQ spurious firing before fpm_regs is set would
+ * dereference NULL.
+ * 2) Use-after-free on probe failure: fman was kzalloc'd (not devm),
+ * so on error paths kfree(fman) ran before devm_free_irq, leaving
+ * a window where the handler could fire with a freed dev_id.
+ * By registering here, both problems are eliminated.
+ */
+ err = devm_request_irq(dev, fman->dts_params.irq, fman_irq,
+ IRQF_SHARED, "fman", fman);
+ if (err < 0) {
+ dev_err(dev, "%s: irq %d allocation failed (error = %d)\n",
+ __func__, fman->dts_params.irq, err);
+ return err;
+ }
+
+ if (fman->dts_params.err_irq != 0) {
+ err = devm_request_irq(dev, fman->dts_params.err_irq,
+ fman_err_irq, IRQF_SHARED,
+ "fman-err", fman);
+ if (err < 0) {
+ dev_err(dev, "%s: irq %d allocation failed (error = %d)\n",
+ __func__, fman->dts_params.err_irq, err);
+ return err;
+ }
+ }
+
if (fman->dts_params.err_irq == 0) {
fman_set_exception(fman, FMAN_EX_DMA_BUS_ERROR, false);
fman_set_exception(fman, FMAN_EX_DMA_READ_ECC, false);
diff --git a/drivers/net/ethernet/freescale/fman/fman.h b/drivers/net/ethernet/freescale/fman/fman.h
index 74eb62eba0d7..630d57c3144c 100644
--- a/drivers/net/ethernet/freescale/fman/fman.h
+++ b/drivers/net/ethernet/freescale/fman/fman.h
@@ -286,6 +286,7 @@ struct fman_dts_params {
struct resource *res; /* FMan memory resource */
u8 id; /* FMan ID */
+ int irq; /* FMan IRQ */
int err_irq; /* FMan Error IRQ */
u16 clk_freq; /* FMan clock freq (In Mhz) */
--
2.20.1
From 6f2bd949c043ba7a3bb650430014ef933c698573 Mon Sep 17 00:00:00 2001
From: ZhaoJinming <zhaojinming@uniontech.com>
Date: Thu, 2 Jul 2026 16:34:06 +0800
Subject: [PATCH v2 2/2] net: fman: add error cleanup path in fman_probe
fman_init() and devm_request_irq() failure paths in fman_probe()
do not free fman and its sub-resources (keygen, muram allocations,
state, cfg), causing memory leaks on probe failure.
Add fman_muram_finish() to properly tear down a MURAM partition
(gen_pool_destroy + iounmap + kfree), complementing the existing
fman_muram_init().
Add fman_free_resources() that releases all fman sub-resources
in the correct order:
- devm_free_irq() for any already-registered IRQ handlers
- kfree(fman->keygen)
- free_init_resources() for MURAM CAM/FIFO allocations
- kfree(fman->cfg)
- fman_muram_finish(fman->muram) for the MURAM management object
- kfree(fman->state)
- kfree(fman)
Use two goto labels in fman_probe():
- err_irq: main IRQ registered but err_irq failed -- free main IRQ
then fall through to release resources
- err_no_irq: no IRQ registered -- just release resources
The IRQ handlers must be explicitly freed before kfree(fman) to
avoid a window where a shared-IRQ spurious firing could dereference
the freed dev_id.
Note: fman_config() is not changed -- it already frees fman
internally on all its error paths, so fman_probe() must not touch
fman after fman_config() fails.
Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
drivers/net/ethernet/freescale/fman/fman.c | 31 +++++++++++++++++--
.../net/ethernet/freescale/fman/fman_muram.c | 15 +++++++++
.../net/ethernet/freescale/fman/fman_muram.h | 2 ++
3 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fman/fman.c b/drivers/net/ethernet/freescale/fman/fman.c
index a33c9c26f99c..a8ca0998b752 100644
--- a/drivers/net/ethernet/freescale/fman/fman.c
+++ b/drivers/net/ethernet/freescale/fman/fman.c
@@ -2804,6 +2804,24 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
return ERR_PTR(err);
}
+static void fman_free_resources(struct fman *fman, struct device *dev,
+ bool irq_registered)
+{
+ /* Free IRQs first while fman is still valid */
+ if (irq_registered) {
+ if (fman->dts_params.err_irq != 0)
+ devm_free_irq(dev, fman->dts_params.err_irq, fman);
+ devm_free_irq(dev, fman->dts_params.irq, fman);
+ }
+
+ kfree(fman->keygen);
+ free_init_resources(fman);
+ kfree(fman->cfg);
+ fman_muram_finish(fman->muram);
+ kfree(fman->state);
+ kfree(fman);
+}
+
static int fman_probe(struct platform_device *of_dev)
{
struct fman *fman;
@@ -2819,12 +2837,13 @@ static int fman_probe(struct platform_device *of_dev)
err = fman_config(fman);
if (err) {
dev_err(dev, "%s: FMan config failed\n", __func__);
+ /* fman_config() frees fman internally on all error paths */
return -EINVAL;
}
if (fman_init(fman) != 0) {
dev_err(dev, "%s: FMan init failed\n", __func__);
- return -EINVAL;
+ goto err_no_irq;
}
/* Register IRQ handlers only after initialization is complete.
@@ -2842,7 +2861,7 @@ static int fman_probe(struct platform_device *of_dev)
if (err < 0) {
dev_err(dev, "%s: irq %d allocation failed (error = %d)\n",
__func__, fman->dts_params.irq, err);
- return err;
+ goto err_no_irq;
}
if (fman->dts_params.err_irq != 0) {
@@ -2852,7 +2871,7 @@ static int fman_probe(struct platform_device *of_dev)
if (err < 0) {
dev_err(dev, "%s: irq %d allocation failed (error = %d)\n",
__func__, fman->dts_params.err_irq, err);
- return err;
+ goto err_irq;
}
}
@@ -2881,6 +2900,12 @@ static int fman_probe(struct platform_device *of_dev)
dev_dbg(dev, "FMan%d probed\n", fman->dts_params.id);
return 0;
+
+err_irq:
+ devm_free_irq(dev, fman->dts_params.irq, fman);
+err_no_irq:
+ fman_free_resources(fman, dev, false);
+ return err ?: -EINVAL;
}
static const struct of_device_id fman_match[] = {
diff --git a/drivers/net/ethernet/freescale/fman/fman_muram.c b/drivers/net/ethernet/freescale/fman/fman_muram.c
index 6ac7c2b0cb19..6c2b4f7a02b8 100644
--- a/drivers/net/ethernet/freescale/fman/fman_muram.c
+++ b/drivers/net/ethernet/freescale/fman/fman_muram.c
@@ -129,3 +129,18 @@ void fman_muram_free_mem(struct muram_info *muram, unsigned long offset,
gen_pool_free(muram->pool, addr, size);
}
+
+/**
+ * fman_muram_finish
+ * @muram: FM-MURAM module pointer.
+ *
+ * Frees all resources associated with a MURAM partition.
+ */
+void fman_muram_finish(struct muram_info *muram)
+{
+ if (!muram)
+ return;
+ iounmap(muram->vbase);
+ gen_pool_destroy(muram->pool);
+ kfree(muram);
+}
diff --git a/drivers/net/ethernet/freescale/fman/fman_muram.h b/drivers/net/ethernet/freescale/fman/fman_muram.h
index 3643af61bae2..a5cb544c0f08 100644
--- a/drivers/net/ethernet/freescale/fman/fman_muram.h
+++ b/drivers/net/ethernet/freescale/fman/fman_muram.h
@@ -23,4 +23,6 @@ unsigned long fman_muram_alloc(struct muram_info *muram, size_t size);
void fman_muram_free_mem(struct muram_info *muram, unsigned long offset,
size_t size);
+void fman_muram_finish(struct muram_info *muram);
+
#endif /* __FM_MURAM_EXT */
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] net: fman: guard IRQ handlers against pre-init interrupt
From: ZhaoJinming @ 2026-07-02 9:28 UTC (permalink / raw)
To: pabeni
Cc: andrew+netdev, andrew, davem, edumazet, horms, kuba, linux-kernel,
madalin.bucur, netdev, sean.anderson, zhaojinming
In-Reply-To: <e1f8aec9-751b-4a51-ab09-ca3c37522e30@redhat.com>
Hi Paolo,
Thanks for the review. You are right -- the irq_ready approach was
fundamentally flawed, and moving IRQ registration after init is the
correct fix.
I have updated the series to address your feedback:
Patch 1/2: Move devm_request_irq() out of read_dts_node() and into
fman_probe(), after fman_config() and fman_init() have completed.
This eliminates both the pre-init NULL dereference and the UAF on
probe failure in a single change. A separate UAF fix patch is no
longer needed.
Patch 2/2: Add proper error cleanup in fman_probe(). The existing
driver had no unified cleanup path -- fman_init() and devm_request_irq()
failure paths leaked fman and its sub-resources. This patch adds
fman_free_resources() and fman_muram_finish() to release everything
correctly, including explicitly freeing IRQs before kfree(fman) to
avoid reintroducing the UAF window on the cleanup path itself.
Patches are against net-next/master.
v1 patch (irq_ready + READ_ONCE) is dropped. v3 UAF fix patch is
superseded.
Thanks,
Jinming
^ permalink raw reply
* Re: [PATCH net] net/stmmac: Set Rx queue page_pool to NULL when freeing DMA resources
From: Maxime Chevallier @ 2026-07-02 9:24 UTC (permalink / raw)
To: Jakub Raczynski, netdev
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, mcoquelin.stm32,
linux-kernel, k.tegowski, k.domagalski, Yashwant Varur
In-Reply-To: <20260630100953.747868-1-j.raczynski@samsung.com>
Hi,
On 6/30/26 12:09, Jakub Raczynski wrote:
> When freeing RX descriptor resources, there is standard clearing of
> descriptor page_pool via page_pool_destroy() which does destroy
> page but does not set its pointer to NULL, which must be done by driver
> calling this function.
> It is not done in __free_dma_rx_desc_resources() when stopping interface,
> which is generally not an issue, because __alloc_dma_rx_desc_resources() does
> setup this regardless of previous state.
> But above is true assuming reinitialization is successful.
>
> In case of failure of page_pool_create() in __alloc_dma_rx_desc_resources(),
> all non-NULL pages will be freed, including those already cleared.
> So there is possible kernel panic due to wrong paging request at address.
>
> Fix this by assigning NULL to page_pool pointer on free
>
> Fixes: da5ec7f22a0f1 ("net: stmmac: refactor stmmac_init_rx_buffers for stmmac_reinit_rx_buffers")
> Signed-off-by: Yashwant Varur <yashwant.v@samsung.com>
> Signed-off-by: Jakub Raczynski <j.raczynski@samsung.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 3591755ea30b..4b8f2814d3b5 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2172,8 +2172,10 @@ static void __free_dma_rx_desc_resources(struct stmmac_priv *priv,
> xdp_rxq_info_unreg(&rx_q->xdp_rxq);
>
> kfree(rx_q->buf_pool);
> - if (rx_q->page_pool)
> + if (rx_q->page_pool) {
> page_pool_destroy(rx_q->page_pool);
> + rx_q->page_pool = NULL;
> + }
page_pool_destroy can be passed NULL, as you're fixing this I think you might as well
drop the if(rx_q->page_pool) at the same time :) I agree with the change otherwise.
with that changed,
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Maxime
^ permalink raw reply
* Re: [PATCH net-next v11 1/7] dt-bindings: phy: document the serdes PHY on sa8255p
From: Geert Uytterhoeven @ 2026-07-02 9:16 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Vinod Koul, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin,
Alexandre Torgue, Giuseppe Cavallaro, Chen-Yu Tsai,
Jernej Skrabec, Neil Armstrong, Kevin Hilman, Jerome Brunet,
Shawn Guo, Fabio Estevam, Jan Petrous, s32, Mohd Ayaan Anwar,
Romain Gantois, Magnus Damm, Maxime Ripard, Christophe Roullier,
Radu Rendec, linux-arm-msm, devicetree, linux-kernel, netdev,
linux-stm32, linux-arm-kernel, Drew Fustini, linux-sunxi,
linux-amlogic, linux-mips, imx, linux-renesas-soc, linux-rockchip,
sophgo, linux-riscv, Bartosz Golaszewski, Bartosz Golaszewski
In-Reply-To: <CAMRc=MfBcOhbj=ETxy2Xz9o_nyzykrEhKAc3jYi6V5_jMZEE3Q@mail.gmail.com>
Hi Bartosz,
On Thu, 2 Jul 2026 at 11:12, Bartosz Golaszewski <brgl@kernel.org> wrote:
> On Tue, 30 Jun 2026 12:23:16 +0200, Vinod Koul <vkoul@kernel.org> said:
> > On 29-06-26, 16:51, Geert Uytterhoeven wrote:
> >> > Russell King asked me to put the PHY logic for SCMI pm domains into the PHY
> >> > driver instead of the MAC driver where it was previously. Instead of cramming
> >> > both HLOS and firmware handling into the same driver, I figured it makes more
> >> > sense to have a dedicated, cleaner driver as the two share very little code (if
> >> > any).
> >>
> >> I think you are mixing up DT bindings and driver implementation?
> >
> > Should the bindings change if we have different driver and firmware
> > implementations? Isn't binding supposed to be agnostic of
> > implementations..?
>
> I've thought about it some more and I believe this question is philosophical in
> nature.
>
> sa8775p and sa8255p are *the same* hardware. I can flash different firmware on
> the same Lemans Ride board and it becomes one or the other. Yet they are not
> described by the same DTS and the bindings differ as well. I don't see why we
> wouldn't allow the same approach for the this PHY.
>
> We treat it as different HW variant when it's managed by firmware - just like
> we do with the rest of the SoC.
DT describes hardware, not software policy.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH net-next v2] ionic: Change list definition method
From: patchwork-bot+netdevbpf @ 2026-07-02 9:20 UTC (permalink / raw)
To: zhulei; +Cc: brett.creeley, andrew+netdev, davem, edumazet, kuba, pabeni,
netdev
In-Reply-To: <20260630065457.160081-1-zhulei_szu@163.com>
Hello:
This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Tue, 30 Jun 2026 14:54:57 +0800 you wrote:
> From: Lei Zhu <zhulei@kylinos.cn>
>
> The LIST_HEAD macro can both define a linked list and initialize
> it in one step. To simplify code, we replace the separate operations
> of linked list definition and manual initialization with the LIST_HEAD
> macro.
>
> [...]
Here is the summary with links:
- [net-next,v2] ionic: Change list definition method
https://git.kernel.org/netdev/net-next/c/09cfee6a8004
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH iwl-next v1] ixgbe: E610: force phy link to get down when interface is down
From: Jagielski, Jedrzej @ 2026-07-02 9:13 UTC (permalink / raw)
To: Paul Menzel
Cc: intel-wired-lan@lists.osuosl.org, Nguyen, Anthony L,
netdev@vger.kernel.org, Loktionov, Aleksandr
In-Reply-To: <fb13640b-0171-457a-84c5-f6dbdd6d7471@molgen.mpg.de>
From: Paul Menzel <pmenzel@molgen.mpg.de>
Sent: Wednesday, July 1, 2026 2:27 PM
>Dear Jedrzej,
>
>
>Thank you for your patch.
>
>Am 01.07.26 um 13:35 schrieb Jedrzej Jagielski:
>> For the E610 family, similarly to the E8xx adapters, the default behavior
>> is for the PHY link to remain up even when the corresponding OS interface
>> is down.
>>
>> Add function setting down the PHY config IXGBE_ACI_PHY_ENA_LINK bit
>> what leads to disabling PHY link.
>
>It’d extend it a little:
>
>… by factoring the code out into ixgbe_handle_link_down(), and call it
>in ixgbe_close().
Hi Paul
thanks for suggestions,
sure, the commit msg will be extended.
>
>> Align functionality with the implementation of the ice driver.
>
>Please add a paragraph detailing regression potential. Are there users
>that might depend on the current default, as uncommon it might be?
There's no regression potential imho
changing behavior is volountary and is done via ethtool
i will rephrase it if that's unclear from the commit msg
>
>> Let user to configure link-down-on-close enablement through ethtool.
>
>Please provide examples, and how to test your change. Doing this you can
>also paste the new log messages.
>
>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
>> ---
>> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 +
>> drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 35 ++++++++++++++++++-
>> drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h | 1 +
>> .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 15 ++++++++
>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 27 +++++++++++---
>> 5 files changed, 73 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>> index 30f62174acf2..7bbb82dd962c 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>> @@ -685,6 +685,7 @@ struct ixgbe_adapter {
>> #define IXGBE_FLAG2_MOD_POWER_UNSUPPORTED BIT(22)
>> #define IXGBE_FLAG2_API_MISMATCH BIT(23)
>> #define IXGBE_FLAG2_FW_ROLLBACK BIT(24)
>> +#define IXGBE_FLAG2_LINK_DOWN_ON_CLOSE BIT(25)
>>
>> /* Tx fast path data */
>> int num_tx_queues;
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
>> index da445fb673fc..46d8a3ea86b8 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
>> @@ -1923,6 +1923,33 @@ void ixgbe_fc_autoneg_e610(struct ixgbe_hw *hw)
>> hw->fc.current_mode = hw->fc.requested_mode;
>> }
>>
>> +/**
>> + * ixgbe_disable_phy_link - force phy link to get down
>> + * @hw: pointer to hardware structure
>> + *
>> + * Send 0x0601 with the IXGBE_ACI_PHY_ENA_LINK bit set down.
>> + *
>> + * Return: the exit code of the operation.
>
>At least for me it’s not that helpful. Shouldn’t the return values be
>listed? What is success? What is failure?
Yeah i know that's not ideal but i also don't think mentioning all possible
retvals of ixgbe_aci_get_phy_caps and ixgbe_aci_set_phy_cfg is the best way
>
>> + */
>> +int ixgbe_disable_phy_link(struct ixgbe_hw *hw)
>> +{
>> + struct ixgbe_aci_cmd_get_phy_caps_data pcaps = {};
>> + struct ixgbe_aci_cmd_set_phy_cfg_data pcfg = {};
>> + int err;
>> +
>> + err = ixgbe_aci_get_phy_caps(hw, false, IXGBE_ACI_REPORT_ACTIVE_CFG,
>> + &pcaps);
>> + if (err)
>> + return err;
>> +
>> + ixgbe_copy_phy_caps_to_cfg(&pcaps, &pcfg);
>> +
>> + pcfg.caps &= ~IXGBE_ACI_PHY_ENA_LINK;
>> + pcfg.caps |= IXGBE_ACI_PHY_ENA_AUTO_LINK_UPDT;
>> +
>> + return ixgbe_aci_set_phy_cfg(hw, &pcfg);
>> +}
>> +
>> /**
>> * ixgbe_disable_rx_e610 - Disable RX unit
>> * @hw: pointer to hardware structure
>> @@ -2207,6 +2234,7 @@ int ixgbe_setup_phy_link_e610(struct ixgbe_hw *hw)
>> u8 rmode = IXGBE_ACI_REPORT_TOPO_CAP_MEDIA;
>> u64 sup_phy_type_low, sup_phy_type_high;
>> u64 phy_type_low = 0, phy_type_high = 0;
>> + bool force_on_required;
>> int err;
>>
>> err = ixgbe_aci_get_link_info(hw, false, NULL);
>> @@ -2272,6 +2300,11 @@ int ixgbe_setup_phy_link_e610(struct ixgbe_hw *hw)
>> phy_type_high |= IXGBE_PHY_TYPE_HIGH_10G_USXGMII;
>> }
>>
>> + /* If IXGBE_ACI_PHY_ENA_LINK has been explicitly disabled that means
>> + * we need to force interface enablement after reaching that point
>
>It’d be great, if you rephrased “that point”.
ok, will try
>
>> + */
>> + force_on_required = !(pcfg.caps & IXGBE_ACI_PHY_ENA_LINK);
>> +
>> /* Mask the set values to avoid requesting unsupported link types. */
>> phy_type_low &= sup_phy_type_low;
>> pcfg.phy_type_low = cpu_to_le64(phy_type_low);
>> @@ -2280,7 +2313,7 @@ int ixgbe_setup_phy_link_e610(struct ixgbe_hw *hw)
>>
>> if (pcfg.phy_type_high != pcaps.phy_type_high ||
>> pcfg.phy_type_low != pcaps.phy_type_low ||
>> - pcfg.caps != pcaps.caps) {
>> + pcfg.caps != pcaps.caps || force_on_required) {
>> pcfg.caps |= IXGBE_ACI_PHY_ENA_LINK;
>> pcfg.caps |= IXGBE_ACI_PHY_ENA_AUTO_LINK_UPDT;
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
>> index 2cb76a3d30ae..59044d67ebeb 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
>> @@ -50,6 +50,7 @@ int ixgbe_cfg_phy_fc(struct ixgbe_hw *hw,
>> enum ixgbe_fc_mode req_mode);
>> int ixgbe_setup_fc_e610(struct ixgbe_hw *hw);
>> void ixgbe_fc_autoneg_e610(struct ixgbe_hw *hw);
>> +int ixgbe_disable_phy_link(struct ixgbe_hw *hw);
>> void ixgbe_disable_rx_e610(struct ixgbe_hw *hw);
>> int ixgbe_init_phy_ops_e610(struct ixgbe_hw *hw);
>> int ixgbe_identify_phy_e610(struct ixgbe_hw *hw);
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
>> index 4dfae53b4ea1..0fcb9d738984 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
>> @@ -139,6 +139,8 @@ static const char ixgbe_priv_flags_strings[][ETH_GSTRING_LEN] = {
>> "vf-ipsec",
>> #define IXGBE_PRIV_FLAGS_AUTO_DISABLE_VF BIT(2)
>> "mdd-disable-vf",
>> +#define IXGBE_PRIV_LINK_DOWN_ON_CLOSE BIT(3)
>> + "link-down-on-close",
>> };
>>
>> #define IXGBE_PRIV_FLAGS_STR_LEN ARRAY_SIZE(ixgbe_priv_flags_strings)
>> @@ -3842,6 +3844,9 @@ static u32 ixgbe_get_priv_flags(struct net_device *netdev)
>> if (adapter->flags2 & IXGBE_FLAG2_AUTO_DISABLE_VF)
>> priv_flags |= IXGBE_PRIV_FLAGS_AUTO_DISABLE_VF;
>>
>> + if (adapter->flags2 & IXGBE_FLAG2_LINK_DOWN_ON_CLOSE)
>> + priv_flags |= IXGBE_PRIV_LINK_DOWN_ON_CLOSE;
>> +
>> return priv_flags;
>> }
>>
>> @@ -3879,6 +3884,16 @@ static int ixgbe_set_priv_flags(struct net_device *netdev, u32 priv_flags)
>> }
>> }
>>
>> + flags2 &= ~IXGBE_FLAG2_LINK_DOWN_ON_CLOSE;
>> + if (priv_flags & IXGBE_PRIV_LINK_DOWN_ON_CLOSE) {
>> + if (adapter->hw.mac.type == ixgbe_mac_e610) {
>> + flags2 |= IXGBE_FLAG2_LINK_DOWN_ON_CLOSE;
>> + } else {
>> + e_info(probe, "Cannot set private flags: Unsupported hardware\n");
>
>Please print hw.mac.type, and mention, that it’s only supported on E610.
yeah why not
>
>> + return -EOPNOTSUPP;
>> + }
>> + }
>> +
>> if (flags2 != adapter->flags2) {
>> adapter->flags2 = flags2;
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> index 62c2d83e1577..58ee4a186039 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> @@ -7544,6 +7544,17 @@ static void ixgbe_close_suspend(struct ixgbe_adapter *adapter)
>> ixgbe_free_all_rx_resources(adapter);
>> }
>>
>> +static void ixgbe_handle_link_down(struct ixgbe_adapter *adapter)
>> +{
>> + struct net_device *netdev = adapter->netdev;
>> +
>> + if (test_bit(__IXGBE_PTP_RUNNING, &adapter->state))
>> + ixgbe_ptp_start_cyclecounter(adapter);
>> +
>> + e_info(drv, "NIC Link is Down\n");
>> + netif_carrier_off(netdev);
>> +}
>> +
>> /**
>> * ixgbe_close - Disables a network interface
>> * @netdev: network interface device structure
>> @@ -7566,6 +7577,16 @@ int ixgbe_close(struct net_device *netdev)
>>
>> ixgbe_fdir_filter_exit(adapter);
>>
>> + if (adapter->flags2 & IXGBE_FLAG2_LINK_DOWN_ON_CLOSE) {
>> + int err;
>> +
>> + err = ixgbe_disable_phy_link(&adapter->hw);
>> + if (err)
>> + e_warn(drv, "Cannot set PHY link down\n");
>
>Log the error?
you mean to change the log lvl?
Thanks for your review!
>
>> +
>> + ixgbe_handle_link_down(adapter);
>> + }
>> +
>> ixgbe_release_hw_control(adapter);
>>
>> return 0;
>> @@ -8244,11 +8265,7 @@ static void ixgbe_watchdog_link_is_down(struct ixgbe_adapter *adapter)
>> if (ixgbe_is_sfp(hw) && hw->mac.type == ixgbe_mac_82598EB)
>> adapter->flags2 |= IXGBE_FLAG2_SEARCH_FOR_SFP;
>>
>> - if (test_bit(__IXGBE_PTP_RUNNING, &adapter->state))
>> - ixgbe_ptp_start_cyclecounter(adapter);
>> -
>> - e_info(drv, "NIC Link is Down\n");
>> - netif_carrier_off(netdev);
>> + ixgbe_handle_link_down(adapter);
>> }
>>
>> static bool ixgbe_ring_tx_pending(struct ixgbe_adapter *adapter)
>
>
>Kind regards,
>
>Paul
^ permalink raw reply
* Re: [PATCH net-next v11 1/7] dt-bindings: phy: document the serdes PHY on sa8255p
From: Bartosz Golaszewski @ 2026-07-02 9:12 UTC (permalink / raw)
To: Vinod Koul
Cc: Bartosz Golaszewski, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin,
Alexandre Torgue, Giuseppe Cavallaro, Chen-Yu Tsai,
Jernej Skrabec, Neil Armstrong, Kevin Hilman, Jerome Brunet,
Shawn Guo, Fabio Estevam, Jan Petrous, s32, Mohd Ayaan Anwar,
Romain Gantois, Magnus Damm, Maxime Ripard, Christophe Roullier,
Radu Rendec, linux-arm-msm, devicetree, linux-kernel, netdev,
linux-stm32, linux-arm-kernel, Drew Fustini, linux-sunxi,
linux-amlogic, linux-mips, imx, linux-renesas-soc, linux-rockchip,
sophgo, linux-riscv, Bartosz Golaszewski, Bartosz Golaszewski,
Geert Uytterhoeven
In-Reply-To: <akOZFIowVvprnAMf@vaman>
On Tue, 30 Jun 2026 12:23:16 +0200, Vinod Koul <vkoul@kernel.org> said:
> On 29-06-26, 16:51, Geert Uytterhoeven wrote:
>> > Russell King asked me to put the PHY logic for SCMI pm domains into the PHY
>> > driver instead of the MAC driver where it was previously. Instead of cramming
>> > both HLOS and firmware handling into the same driver, I figured it makes more
>> > sense to have a dedicated, cleaner driver as the two share very little code (if
>> > any).
>>
>> I think you are mixing up DT bindings and driver implementation?
>
> Should the bindings change if we have different driver and firmware
> implementations? Isn't binding supposed to be agnostic of
> implementations..?
>
I've thought about it some more and I believe this question is philosophical in
nature.
sa8775p and sa8255p are *the same* hardware. I can flash different firmware on
the same Lemans Ride board and it becomes one or the other. Yet they are not
described by the same DTS and the bindings differ as well. I don't see why we
wouldn't allow the same approach for the this PHY.
We treat it as different HW variant when it's managed by firmware - just like
we do with the rest of the SoC.
Bart
^ permalink raw reply
* Re: [PATCH net-next v6 4/6] pds_core: add PLDM component info display
From: Paolo Abeni @ 2026-07-02 9:11 UTC (permalink / raw)
To: nikhil.rao
Cc: netdev, kuba, brett.creeley, eric.joyner, andrew+netdev, davem,
edumazet, pabeni, jacob.e.keller
In-Reply-To: <20260629230200.82092-5-nikhil.rao@amd.com>
From: AI Reviewer <ai@example.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
pds_core: add PLDM component info display
This change adds detailed PLDM component info to devlink info on
identity v2 devices, introduces a pdsc_fw_components_invalidate()
helper used during firmware recovery and after firmware update, and
maps PDS_CORE_FW_TYPE_MAIN to the generic "fw" name in the PLDM
flash path.
> diff --git a/drivers/net/ethernet/amd/pds_core/core.c b/drivers/net/ethernet/amd/pds_core/core.c
> index 38a2446571af..4d9ab8b74ec6 100644
> --- a/drivers/net/ethernet/amd/pds_core/core.c
> +++ b/drivers/net/ethernet/amd/pds_core/core.c
> @@ -580,6 +580,8 @@ void pdsc_fw_up(struct pdsc *pdsc)
> return;
> }
>
> + pdsc_fw_components_invalidate(pdsc);
> +
> err = pdsc_setup(pdsc, PDSC_SETUP_RECOVERY);
> if (err)
> goto err_out;
[Medium]
Can concurrent access to pdsc->fw_components race here?
pdsc_fw_up() runs from pdsc_health_thread (workqueue, holding only
pdsc->config_lock) and from pdsc_reset_done (PCI reset handler, holding
neither), so pdsc_fw_components_invalidate() writes num_components = 0
without the devlink instance lock.
Meanwhile, pdsc_dl_component_info_get() reads pdsc->fw_components from
the devlink info_get callback under only the devlink lock, never
config_lock. The two domains do not exclude each other.
In pdsc_dl_component_info_get():
if (!pdsc->fw_components.num_components) {
err = pdsc_get_component_info(pdsc);
if (err || !pdsc->fw_components.num_components)
return pdsc_dl_report_fw_ver(req,
pdsc->dev_info.fw_version);
}
list_info = &pdsc->fw_components;
num_components = min_t(u16, list_info->num_components,
le16_to_cpu(pdsc->dev_ident.max_fw_slots));
The reader can observe num_components > 0 on the first load, then
re-read it inside min_t() and observe 0 after pdsc_fw_up() invalidates
the cache, producing zero iterations and no fallback. Conversely, a
stale non-zero count could be iterated over info[] entries describing
the previous firmware.
No READ_ONCE/WRITE_ONCE or memory barriers are used. Would a lock or
RCU be a better fit here?
> diff --git a/drivers/net/ethernet/amd/pds_core/devlink.c b/drivers/net/ethernet/amd/pds_core/devlink.c
> index 3b763ee1715e..bd234c3c50b2 100644
> --- a/drivers/net/ethernet/amd/pds_core/devlink.c
> +++ b/drivers/net/ethernet/amd/pds_core/devlink.c
> @@ -93,14 +93,110 @@ int pdsc_dl_flash_update(struct devlink *dl,
> return pdsc_firmware_update(pdsc, params, extack);
> }
>
> +static int pdsc_dl_report_component(struct devlink_info_req *req,
> + struct pds_core_fw_component_info *info)
> +{
> + enum devlink_info_version_type ver_type;
> + u16 flags = le16_to_cpu(info->flags);
> + char *ver = info->version;
> + const char *name;
> + char buf[32];
> +
> + /* Main firmware is reported as generic "fw" */
> + if (info->component_type == PDS_CORE_FW_TYPE_MAIN) {
> + if (info->slot_id == PDS_CORE_FW_SLOT_GOLD)
> + snprintf(buf, sizeof(buf), "fw.gold");
> + else
> + snprintf(buf, sizeof(buf), "fw");
> + } else {
> + name = pdsc_fw_type_to_name(info->component_type);
> + if (!name)
> + return 0;
> +
> + if (info->slot_id == PDS_CORE_FW_SLOT_GOLD)
> + snprintf(buf, sizeof(buf), "fw.%s.gold", name);
> + else
> + snprintf(buf, sizeof(buf), "fw.%s", name);
> + }
[Medium]
For PDS_CORE_FW_TYPE_MAIN, only PDS_CORE_FW_SLOT_GOLD is special-cased.
PDS_CORE_FW_SLOT_INVALID, PDS_CORE_FW_SLOT_A, and PDS_CORE_FW_SLOT_B all
end up emitting the bare name "fw".
The legacy v1 path used distinct names fw.mainfwa and fw.mainfwb for
these two slots, and devlink_info_version_*_put() does not deduplicate
names. If firmware reports two MAIN components (which the v1
pds_core_fw_list_info already exposed as A/B), will userspace see two
entries both named "fw" with no way to distinguish them?
[Medium]
Are the new component names documented anywhere? The driver introduces
fw.bootloader, fw.cpld, fw.secure, fw.fpga, fw.suc, fw.suc.bootloader,
fw.uboot, plus a .gold variant of each, and fw.gold.
Documentation/networking/devlink/devlink-info.rst says:
It is expected that drivers use the following generic names ... As
last resort, if a component is truly unique, using driver-specific
names is allowed, but these should be documented in the driver-specific
file.
Of those names only fw and fw.bootloader correspond to documented
generic suffixes; the rest appear to be driver-invented, and there is
no Documentation/networking/devlink/pds_core.rst introduced by this
patch. The .gold suffix in particular is not part of the generic
vocabulary (different copies of the same component are normally handled
via bundle_id or by reporting the same name under both running and
stored). Composite names like fw.suc.bootloader.gold do not have any
documented dot-separation semantics either.
> +
> + ver_type = DEVLINK_INFO_VERSION_TYPE_NONE;
> + if (flags & PDS_CORE_FW_COMPONENT_INFO_F_UPDATE_BY_NAME)
> + ver_type = DEVLINK_INFO_VERSION_TYPE_COMPONENT;
> +
> + if (flags & PDS_CORE_FW_COMPONENT_INFO_F_FIXED) {
> + int err;
> +
> + err = devlink_info_version_fixed_put(req, buf, ver);
> + if (err)
> + return err;
> + }
> +
> + if (flags & PDS_CORE_FW_COMPONENT_INFO_F_RUNNING) {
> + int err;
> +
> + err = devlink_info_version_running_put_ext(req, buf,
> + ver, ver_type);
> + if (err)
> + return err;
> + }
> +
> + if (flags & PDS_CORE_FW_COMPONENT_INFO_F_STARTUP) {
> + int err;
> +
> + err = devlink_info_version_stored_put_ext(req, buf,
> + ver, ver_type);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
[Medium]
Is the generic running "fw" entry still guaranteed on the v2 path?
It is produced only as a side effect of pdsc_dl_report_component()
seeing component_type == PDS_CORE_FW_TYPE_MAIN with
PDS_CORE_FW_COMPONENT_INFO_F_RUNNING set.
The v1 path always called:
devlink_info_version_running_put(req,
DEVLINK_INFO_VERSION_GENERIC_FW,
pdsc->dev_info.fw_version);
unconditionally. If firmware reports MAIN only with F_STARTUP/F_FIXED
(no F_RUNNING), or reports no MAIN component at all, does the user
silently lose the generic running fw version that monitoring tools rely
on to identify the active firmware?
> +
> +static int pdsc_dl_report_fw_ver(struct devlink_info_req *req, char *fw_ver)
> +{
> + return devlink_info_version_running_put(req,
> + DEVLINK_INFO_VERSION_GENERIC_FW,
> + fw_ver);
> +}
> +
> +static int pdsc_dl_component_info_get(struct devlink *dl,
> + struct devlink_info_req *req,
> + struct netlink_ext_ack *extack)
> +{
> + struct pds_core_component_list_info *list_info;
> + struct pdsc *pdsc = devlink_priv(dl);
> + u8 num_components;
> + int err;
> + int i;
> +
> + if (!pdsc->fw_components.num_components) {
> + err = pdsc_get_component_info(pdsc);
> + if (err || !pdsc->fw_components.num_components)
> + return pdsc_dl_report_fw_ver(req,
> + pdsc->dev_info.fw_version);
> + }
[High]
Can devlink info polling accumulate deferred DMA buffers when the
firmware is hung?
pdsc_dl_component_info_get() is reachable from any devlink dev info
request. On -EAGAIN or -ETIMEDOUT, pdsc_get_component_info() calls
pdsc_deferred_dma_add() and returns the error without setting
pdsc->fw_components.num_components:
err = pdsc_devcmd(pdsc, &cmd, &comp, pdsc->devcmd_timeout * 2);
if (err == -ETIMEDOUT || err == -EAGAIN) {
pdsc_deferred_dma_add(pdsc, deferred, dma_addr, list_info,
PDS_PAGE_SIZE, DMA_FROM_DEVICE);
return err;
}
The cached count therefore stays 0, so the next devlink info request
re-enters the same path and queues another PDS_PAGE_SIZE allocation
plus DMA mapping. The deferred list is only drained by
pdsc_deferred_dma_free() in __pdsc_devcmd_locked() on a non-EAGAIN
return.
Before this patch, pdsc_get_component_info() was reachable only from
the privileged flash path. Is it intentional that any monitoring agent
polling devlink dev info while firmware is wedged can grow the deferred
DMA list unbounded?
[Medium]
On the v2 branch, the function falls back to a single
pdsc_dl_report_fw_ver() call whenever pdsc_get_component_info() returns
an error or num_components is still 0 (which also happens when the
firmware reports comp.get_component_info.ver == 0 and the function
returns 0).
Before this patch, pdsc_dl_info_get() unconditionally invoked
PDS_CORE_FW_GET_LIST and emitted fw.goldfw, fw.mainfwa, fw.mainfwb.
On v2 devices that branch is unreachable, since pdsc_dl_fw_list_info_get()
is only called from pdsc_dl_info_get_v1().
For transitional firmware that advertises identity v2 but does not yet
support PLDM/GET_COMPONENT_INFO, would calling pdsc_dl_fw_list_info_get()
as a v2 fallback preserve the previous four-version output rather than
silently collapsing it to one?
> +
> + list_info = &pdsc->fw_components;
> + num_components = min_t(u16, list_info->num_components,
> + le16_to_cpu(pdsc->dev_ident.max_fw_slots));
> + for (i = 0; i < num_components; i++) {
> + err = pdsc_dl_report_component(req, &list_info->info[i]);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
[Medium]
What happens if dev_ident.max_fw_slots is 0? pdsc_get_component_info()
populates pdsc->fw_components.num_components from the firmware list
(bounded by PDS_CORE_FW_COMPONENT_LIST_LEN), but the loop bound clamps
that against the separate max_fw_slots identity field:
num_components = min_t(u16, list_info->num_components,
le16_to_cpu(pdsc->dev_ident.max_fw_slots));
If firmware advertises identity v2 but reports max_fw_slots == 0, the
earlier !pdsc->fw_components.num_components check does not fire (the
count is non-zero), min_t() yields 0, the loop body is skipped, and
pdsc_dl_info_get() proceeds straight to ASIC ID/REV/serial. Would
adding a fallback to pdsc_dl_report_fw_ver() in this case avoid
emitting devlink info with no fw entry at all?
> +
> static char *fw_slotnames[] = {
> "fw.goldfw",
> "fw.mainfwa",
> "fw.mainfwb",
> };
>
> -int pdsc_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
> - struct netlink_ext_ack *extack)
> +static int pdsc_dl_fw_list_info_get(struct devlink *dl,
> + struct devlink_info_req *req,
> + struct netlink_ext_ack *extack)
> {
[ ... ]
> @@ -134,12 +230,41 @@ int pdsc_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
> return err;
> }
>
> - err = devlink_info_version_running_put(req,
> - DEVLINK_INFO_VERSION_GENERIC_FW,
> - pdsc->dev_info.fw_version);
> + return 0;
> +}
> +
> +static int pdsc_dl_info_get_v1(struct devlink *dl,
> + struct devlink_info_req *req,
> + struct netlink_ext_ack *extack)
> +{
> + struct pdsc *pdsc = devlink_priv(dl);
> + int err;
> +
> + err = pdsc_dl_fw_list_info_get(dl, req, extack);
> if (err)
> return err;
>
> + /* Version 1: report fw from dev_info (running only) */
> + return pdsc_dl_report_fw_ver(req, pdsc->dev_info.fw_version);
> +}
> +
> +int pdsc_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
> + struct netlink_ext_ack *extack)
> +{
> + struct pdsc *pdsc = devlink_priv(dl);
> + char buf[32];
> + int err;
> +
> + if (pdsc->dev_ident.version >= PDS_CORE_IDENTITY_VERSION_2) {
> + err = pdsc_dl_component_info_get(dl, req, extack);
> + if (err)
> + return err;
> + } else {
> + err = pdsc_dl_info_get_v1(dl, req, extack);
> + if (err)
> + return err;
> + }
> +
[Medium]
Is the devlink-info naming change across the identity-version boundary
intentional, and is it captured in the commit message or a Documentation
update?
Before this patch every device emitted stored slot names from
fw_slotnames[] (fw.goldfw, fw.mainfwa, fw.mainfwb) via the unconditional
PDS_CORE_FW_GET_LIST devcmd. After this patch, devices that advertise
identity >= PDS_CORE_IDENTITY_VERSION_2 take the
pdsc_dl_component_info_get() branch and emit a different set of names
(fw, fw.gold, fw.bootloader, fw.uboot, fw.cpld, ...), with no alias for
the old names.
A firmware-only upgrade can therefore silently rename the user-visible
stored entries. Should the commit message call out this UAPI rename,
and should the legacy names remain emitted as aliases for compatibility
with existing fleet tooling that parses devlink dev info output?
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH net-next v6 3/6] pds_core: add PLDM firmware update support via devlink flash
From: Paolo Abeni @ 2026-07-02 9:11 UTC (permalink / raw)
To: nikhil.rao
Cc: netdev, kuba, brett.creeley, eric.joyner, andrew+netdev, davem,
edumazet, pabeni, jacob.e.keller
In-Reply-To: <20260629230200.82092-4-nikhil.rao@amd.com>
From: AI Reviewer <ai@example.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
pds_core: add PLDM firmware update support via devlink flash
Implements PLDM firmware update via devlink flash using the upstream
pldmfw API. Adds whole-package and per-component update paths, and
introduces a deferred-DMA list that releases buffers after timed-out
devcmds. A few questions inline.
> diff --git a/drivers/net/ethernet/amd/pds_core/core.h b/drivers/net/ethernet/amd/pds_core/core.h
> index b7fe9ad73349..c686f0bbbaeb 100644
> --- a/drivers/net/ethernet/amd/pds_core/core.h
> +++ b/drivers/net/ethernet/amd/pds_core/core.h
> @@ -23,6 +23,14 @@
> #define PDSC_SETUP_RECOVERY false
> #define PDSC_SETUP_INIT true
>
> +struct pdsc_deferred_dma {
> + struct list_head list;
> + dma_addr_t dma_addr;
> + void *va;
> + size_t size;
> + enum dma_data_direction dir;
> +};
> +
[ ... ]
> diff --git a/drivers/net/ethernet/amd/pds_core/dev.c b/drivers/net/ethernet/amd/pds_core/dev.c
> index 5c0ca3d0b000..6082b28915db 100644
> --- a/drivers/net/ethernet/amd/pds_core/dev.c
> +++ b/drivers/net/ethernet/amd/pds_core/dev.c
> @@ -206,15 +206,53 @@ static int __pdsc_devcmd_locked(struct pdsc *pdsc, union pds_core_dev_cmd *cmd,
> else
> memcpy_fromio(comp, &pdsc->cmd_regs->comp, sizeof(*comp));
>
> + if (err != -ETIMEDOUT && err != -EAGAIN)
> + pdsc_deferred_dma_free(pdsc);
> +
> return err;
> }
[High]
Can the deferred-DMA scheme actually guarantee that the device has
finished accessing the buffer before it is unmapped and freed?
The trigger for releasing a deferred entry is "the next devcmd returned
something other than -ETIMEDOUT/-EAGAIN", plus pdsc_remove() and
pdsc_reset_prepare(). None of these provide a hardware-level fence that
the previous in-flight DMA has drained. In the remove/reset paths the
free runs after pci_disable_device(), which clears bus-master but does
not synchronously drain pending transactions.
The DMA_FROM_DEVICE case in pdsc_get_component_info() is particularly
exposed because the buffer is plain kernel slab memory: a late device
write after kfree would corrupt unrelated allocations.
Also note that pdsc_send_package_data(), pdsc_get_component_info(), and
pdsc_flash_component_chunk() only defer on -EAGAIN/-ETIMEDOUT and
immediately unmap+kfree on any other error (-EIO, -ENXIO, ...). Is the
device guaranteed not to have started any DMA in those cases?
> +void pdsc_deferred_dma_add(struct pdsc *pdsc, struct pdsc_deferred_dma *entry,
> + dma_addr_t dma_addr, void *va, size_t size,
> + enum dma_data_direction dir)
> +{
> + entry->dma_addr = dma_addr;
> + entry->va = va;
> + entry->size = size;
> + entry->dir = dir;
> +
> + spin_lock(&pdsc->deferred_dma_lock);
> + list_add_tail(&entry->list, &pdsc->deferred_dma_list);
> + spin_unlock(&pdsc->deferred_dma_lock);
> +}
> +
> +void pdsc_deferred_dma_free(struct pdsc *pdsc)
> +{
> + struct pdsc_deferred_dma *entry, *tmp;
> +
> + spin_lock(&pdsc->deferred_dma_lock);
> + list_for_each_entry_safe(entry, tmp, &pdsc->deferred_dma_list, list) {
> + dma_unmap_single(pdsc->dev, entry->dma_addr,
> + entry->size, entry->dir);
> + kfree(entry->va);
> + list_del(&entry->list);
> + kfree(entry);
> + }
> + spin_unlock(&pdsc->deferred_dma_lock);
> +}
[Medium]
Is it intentional to hold deferred_dma_lock across dma_unmap_single()
and two kfree()s per entry? This runs from __pdsc_devcmd_locked() on
every successful devcmd while devcmd_lock is also held, and
dma_unmap_single() can trigger IOMMU IOTLB invalidations. Would
list_splice_init() under the lock followed by iteration on a detached
list scale better when many entries have accumulated?
[ ... ]
> diff --git a/drivers/net/ethernet/amd/pds_core/fw.c b/drivers/net/ethernet/amd/pds_core/fw.c
> index fa626719e68d..433b0a8df4c1 100644
> --- a/drivers/net/ethernet/amd/pds_core/fw.c
> +++ b/drivers/net/ethernet/amd/pds_core/fw.c
[ ... ]
> +#define PDSC_FW_COMPONENT_PREFIX "fw."
> +#define PDSC_FW_COMPONENT_FULL_NAME_BUFLEN \
> + (sizeof(PDSC_FW_COMPONENT_PREFIX) + PDS_CORE_FW_COMPONENT_NAME_BUFLEN)
> +
> +/* Driver-defined component type to name mapping.
> + * PDS_CORE_FW_TYPE_MAIN is NULL - handled specially as "fw" without prefix.
> + */
> +static const char * const pdsc_fw_type_names[] = {
> + [PDS_CORE_FW_TYPE_MAIN] = NULL,
> + [PDS_CORE_FW_TYPE_BOOT] = "bootloader",
> + [PDS_CORE_FW_TYPE_CPLD] = "cpld",
> + [PDS_CORE_FW_TYPE_SECURE] = "secure",
> + [PDS_CORE_FW_TYPE_FPGA] = "fpga",
> + [PDS_CORE_FW_TYPE_SUC_MAIN] = "suc",
> + [PDS_CORE_FW_TYPE_SUC_BOOT] = "suc.bootloader",
> + [PDS_CORE_FW_TYPE_UBOOT] = "uboot",
> +};
[Low, Low]
The documentation table added in this same patch lists fw.gold as a
driver-defined component name, and the kerneldoc on
enum pds_core_fw_component_type says gold variants are "reported with
a .gold suffix (e.g., fw.gold)". The table above has no gold entry, so
pdsc_name_to_fw_type("fw.gold") returns 0 and pdsc_pldm_firmware_update()
rejects the request with -ENOENT. Should fw.gold either be wired up or
the docs clarify that gold is read-only?
PDSC_FW_COMPONENT_FULL_NAME_BUFLEN is defined here but never used;
pdsc_flash_component() uses an ad-hoc sizeof(PDSC_FW_COMPONENT_PREFIX) +
16 instead. Should this macro be wired up or dropped?
[ ... ]
> +int pdsc_get_component_info(struct pdsc *pdsc)
> +{
> + union pds_core_dev_cmd cmd = {
> + .get_component_info.opcode = PDS_CORE_CMD_GET_COMPONENT_INFO,
> + .get_component_info.ver = 1,
> + };
[ ... ]
> + if (comp.get_component_info.ver == 0) {
> + /* Don't support backward compatibility as version 0 has
> + * alignment issues, so give a hint to users to update
> + * their firmware
> + */
> + dev_warn_once(pdsc->dev,
> + "Incompatible get_component_info version %u reported by firmware\n",
> + comp.get_component_info.ver);
> + err = 0;
> + goto out;
> + }
[Low]
This branch returns 0 without populating pdsc->fw_components. The caller
pdsc_pldm_firmware_update() uses:
if (!pdsc->fw_components.num_components) {
err = pdsc_get_component_info(pdsc);
if (err) { ... return err; }
}
if (params->component) {
u8 type = pdsc_name_to_fw_type(params->component);
if (!type || !pdsc_component_type_exists(pdsc, type))
return -ENOENT;
}
so on a device that reports ver == 0, the cache never populates and
every per-component flash returns -ENOENT. Should this branch return an
explicit error instead, or otherwise prevent the per-component lookup
from running?
[ ... ]
> +static int pdsc_devcmd_send_component(struct pdsc *pdsc,
> + struct pds_core_flash_component *info,
> + u16 info_sz, dma_addr_t addr, u32 length,
> + u32 offset, u16 slot_id,
> + union pds_core_dev_comp *comp)
> +{
> + union pds_core_dev_cmd cmd = {
> + .send_component.opcode = PDS_CORE_CMD_SEND_COMPONENT,
> + .send_component.ver = 1,
> + .send_component.operation = PDS_CORE_SEND_COMPONENT_START,
> + .send_component.data_pa = cpu_to_le64(addr),
> + .send_component.data_len = cpu_to_le32(length),
> + .send_component.offset = cpu_to_le32(offset),
> + .send_component.slot_id = slot_id,
> + };
> + unsigned long timeout = 300 * HZ;
> + unsigned long start_time;
> + unsigned long end_time;
> + int err;
> +
> + start_time = jiffies;
> + end_time = start_time + timeout;
> + do {
> + /* prevent noisy/benign devcmd failures */
> + err = pdsc_devcmd_with_data_nomsg(pdsc, &cmd, info, info_sz,
> + comp, 60);
> + if (err != -EAGAIN)
> + break;
> +
> + /* if required, subsequent commands check status of
> + * PDS_CORE_CMD_SEND_COMPONENT command, which returns
> + * EAGAIN while the command is still running,
> + * else we get the final command status.
> + */
> + cmd.send_component.operation = PDS_CORE_SEND_COMPONENT_STATUS;
> + msleep(20);
> + } while (time_before(jiffies, end_time));
[Low]
In the STATUS retry phase, only cmd.send_component.operation changes
between iterations; data_pa, data_len, offset, and slot_id remain set
to the original START values, and pdsc_devcmd_with_data_nomsg() also
re-issues memcpy_toio(&pdsc->cmd_regs->data, info, info_sz) on every
poll (potentially up to ~15000 iterations over the 300s timeout at 20ms
cadence). Does the firmware ignore data_pa/data_len on the STATUS
opcode, or could it re-initiate DMA from the same buffer? Could the
status poll send just the operation field?
[ ... ]
> +static int pdsc_finalize_update(struct pldmfw *context)
> +{
[ ... ]
> + start_time = jiffies;
> + end_time = start_time + (PDSC_FW_INSTALL_TIMEOUT * HZ);
> + do {
> + err = pdsc_devcmd_finalize_update(pdsc);
> + if (!err || err != -EAGAIN)
> + break;
> +
> + dev_dbg(dev, "retrying finalize_update: %pe\n", ERR_PTR(err));
> + msleep(20);
> + } while (time_before(jiffies, end_time) && err == -EAGAIN);
[Low]
Is the !err in if (!err || err != -EAGAIN) intentional? err == 0 already
satisfies err != -EAGAIN, so the condition reduces to err != -EAGAIN.
[ ... ]
> +static int pdsc_pldm_firmware_update(struct pdsc *pdsc,
> + struct devlink_flash_update_params *params,
> + struct netlink_ext_ack *extack,
> + const struct firmware *fw)
> +{
> + struct pds_core_fwu_priv priv = {};
> + int err;
> +
> + if (!pdsc->fw_components.num_components) {
> + err = pdsc_get_component_info(pdsc);
> + if (err) {
> + dev_err(pdsc->dev,
> + "Failed to get component info: %pe\n",
> + ERR_PTR(err));
> + return err;
> + }
> + }
> +
> + if (params->component) {
> + u8 type = pdsc_name_to_fw_type(params->component);
> +
> + if (!type || !pdsc_component_type_exists(pdsc, type))
> + return -ENOENT;
> + }
> +
> + INIT_LIST_HEAD(&priv.components);
> + priv.context.ops = &pdsc_pldmfw_ops;
> + priv.context.dev = pdsc->dev;
> + priv.params = params;
> + priv.pdsc = pdsc;
> +
> + err = pldmfw_flash_image(&priv.context, fw);
> + pdsc_free_fwu_priv(&priv);
> +
> + return err;
> +}
[Medium]
The extack parameter is accepted here and struct pds_core_fwu_priv
declares a netlink_ext_ack *extack field, but priv.extack is never
assigned, and none of the PLDM callbacks (pdsc_match_record_descs,
pdsc_send_package_data, pdsc_send_component_table, pdsc_flash_component,
pdsc_finalize_update) call NL_SET_ERR_MSG_MOD(). The legacy path uses
extack at multiple call sites ("Firmware wait timed out", "Segment
download failed", ...). Was the intent to propagate extack diagnostics
on the PLDM path too, including for the -ENOENT returned above?
> +int pdsc_firmware_update(struct pdsc *pdsc,
> + struct devlink_flash_update_params *params,
> + struct netlink_ext_ack *extack)
> +{
> + int err;
> +
> + if (pdsc->dev_ident.version >= PDS_CORE_IDENTITY_VERSION_2 &&
> + pdsc->dev_ident.capabilities &
> + cpu_to_le64(PDS_CORE_DEV_CAP_PLDM_FW_UPDATE))
> + err = pdsc_pldm_firmware_update(pdsc, params, extack,
> + params->fw);
> + else
> + err = pdsc_legacy_firmware_update(pdsc, params->fw, extack);
> +
> + /* Invalidate cached component info so next info_get refreshes */
> + pdsc->fw_components.num_components = 0;
> +
> + return err;
> +}
[Medium]
What happens here when a user runs:
devlink dev flash pci/0000:b5:00.0 file foo component fw.cpld
against a device that does not advertise PDS_CORE_DEV_CAP_PLDM_FW_UPDATE?
The dispatcher falls through to pdsc_legacy_firmware_update(pdsc,
params->fw, extack), which has no notion of components and proceeds to
flash the entire image.
The commit message says:
Not all components support per-component update - devlink will
reject the request if the specified component cannot be updated.
Should the dispatcher return -EOPNOTSUPP (or similar) when
params->component != NULL on the legacy path, to match that promise?
> diff --git a/include/linux/pds/pds_core_if.h b/include/linux/pds/pds_core_if.h
> index 619186f26b5b..5a1fafaccf20 100644
> --- a/include/linux/pds/pds_core_if.h
> +++ b/include/linux/pds/pds_core_if.h
> @@ -279,11 +299,20 @@ enum pds_core_fw_control_oper {
> PDS_CORE_FW_GET_LIST = 7,
> };
>
> +/**
> + * enum pds_core_fw_slot - Firmware slot identifiers
> + * @PDS_CORE_FW_SLOT_INVALID: Let firmware select slot based on package metadata
> + * @PDS_CORE_FW_SLOT_A: Primary firmware slot A
> + * @PDS_CORE_FW_SLOT_B: Primary firmware slot B
> + * @PDS_CORE_FW_SLOT_GOLD: Gold/recovery firmware slot
> + * @PDS_CORE_FW_SLOT_MAX: Sentinel value indicating no slot resolved
> + */
> enum pds_core_fw_slot {
> PDS_CORE_FW_SLOT_INVALID = 0,
> PDS_CORE_FW_SLOT_A = 1,
> PDS_CORE_FW_SLOT_B = 2,
> PDS_CORE_FW_SLOT_GOLD = 3,
> + PDS_CORE_FW_SLOT_MAX = 0xff,
> };
[Low]
PDS_CORE_FW_SLOT_MAX is added with value 0xff to this wire-protocol
enum but has no references anywhere. Should it be wired up to something
or dropped before the enum value is locked in by firmware?
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH net-next v4] net: usb: rtl8150: handle link status read failures
From: patchwork-bot+netdevbpf @ 2026-07-02 9:10 UTC (permalink / raw)
To: Yousef Alhouseen
Cc: petkan, andrew+netdev, davem, edumazet, kuba, pabeni, linux-usb,
netdev, linux-kernel, syzbot+9db6c624635564ad813c
In-Reply-To: <20260630101216.10365-1-alhouseenyousef@gmail.com>
Hello:
This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Tue, 30 Jun 2026 12:12:16 +0200 you wrote:
> set_carrier() ignores the result of the USB control transfer and tests
> the stack variable supplied as its receive buffer. If the device rejects
> or aborts the request, that variable remains uninitialized and the driver
> chooses an arbitrary carrier state.
>
> Leave the existing carrier state unchanged when the link status cannot be
> read. A transient USB error should not be treated as link loss.
>
> [...]
Here is the summary with links:
- [net-next,v4] net: usb: rtl8150: handle link status read failures
https://git.kernel.org/netdev/net-next/c/8c9c5b9a6896
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [PATCH net-next] selftests/net/openvswitch: add SCTP flow key test
From: Minxi Hou @ 2026-07-02 9:09 UTC (permalink / raw)
To: netdev
Cc: Minxi Hou, aconole, echaudro, i.maximets, davem, edumazet, kuba,
pabeni, horms, shuah, dev, linux-kselftest
Register OVS_KEY_ATTR_SCTP in the flow key parser so that sctp()
can be used in flow specifications. The ovs_key_sctp class already
exists (with src/dst fields matching the TCP/UDP siblings) but was
not wired into the parser, so the token was silently dropped and the
kernel rejected the flow.
Add test_sctp_connect_v4 exercising the SCTP flow key with
port-specific matching: sctp(dst=4443) for client-to-server and
sctp(src=4443) for server-to-client, mirroring the SCTP four-way
handshake direction. The test verifies connectivity with flows
installed, confirms failure after flow removal, then reinstalls
and verifies recovery.
Signed-off-by: Minxi Hou <houminxi@gmail.com>
---
.../selftests/net/openvswitch/openvswitch.sh | 92 +++++++++++++++++++
.../selftests/net/openvswitch/ovs-dpctl.py | 5 +
2 files changed, 97 insertions(+)
diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index 2954245129a2..dd4d0b3bcc23 100755
--- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
+++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
@@ -32,6 +32,7 @@ tests="
dec_ttl ttl: dec_ttl decrements IP TTL
flow_set flow-set: Flow modify
action_set set: SET action rewrites fields
+ sctp_connect_v4 sctp: SCTP flow key matching
psample psample: Sampling packets with psample"
info() {
@@ -443,6 +444,97 @@ test_action_set() {
return 0
}
+# sctp_connect_v4 test
+# - sctp(dst=4443) matches client-to-server INIT
+# - sctp(src=4443) matches server-to-client INIT-ACK
+# - remove flows and verify connection fails, reinstall and recover
+test_sctp_connect_v4() {
+ local t="test_sctp_connect_v4"
+
+ which ncat >/dev/null 2>&1 || return $ksft_skip
+ modprobe -q sctp 2>/dev/null || return $ksft_skip
+
+ sbx_add "$t" || return $?
+ ovs_add_dp "$t" sctp4 || return 1
+
+ info "create namespaces"
+ for ns in client server; do
+ ovs_add_netns_and_veths "$t" "sctp4" "$ns" \
+ "${ns:0:1}0" "${ns:0:1}1" || return 1
+ done
+
+ ip netns exec client ip addr add 172.31.110.10/24 dev c1
+ ip netns exec client ip link set c1 up
+ ip netns exec server ip addr add 172.31.110.20/24 dev s1
+ ip netns exec server ip link set s1 up
+
+ # ARP forwarding
+ ovs_add_flow "$t" sctp4 \
+ 'in_port(1),eth(),eth_type(0x0806),arp()' \
+ '2' || return 1
+ ovs_add_flow "$t" sctp4 \
+ 'in_port(2),eth(),eth_type(0x0806),arp()' \
+ '1' || return 1
+
+ # SCTP port matching: dst for request, src for reply
+ ovs_add_flow "$t" sctp4 \
+ 'in_port(1),eth(),eth_type(0x0800),ipv4(proto=132),sctp(dst=4443)' \
+ '2' || return 1
+ ovs_add_flow "$t" sctp4 \
+ 'in_port(2),eth(),eth_type(0x0800),ipv4(proto=132),sctp(src=4443)' \
+ '1' || return 1
+
+ echo "server" | \
+ ovs_netns_spawn_daemon "$t" "server" \
+ ncat --sctp -l 172.31.110.20 -vn 4443
+ sleep 0.1
+
+ info "verify SCTP association with port-keyed flows"
+ ovs_sbx "$t" ip netns exec client \
+ ncat --sctp -i 1 -zv 172.31.110.20 4443 \
+ || return 1
+
+ ovs_del_flows "$t" sctp4
+
+ info "verify connection fails without flows"
+ ovs_add_flow "$t" sctp4 \
+ 'in_port(1),eth(),eth_type(0x0806),arp()' \
+ '2' || return 1
+ ovs_add_flow "$t" sctp4 \
+ 'in_port(2),eth(),eth_type(0x0806),arp()' \
+ '1' || return 1
+
+ echo "server2" | \
+ ovs_netns_spawn_daemon "$t" "server" \
+ ncat --sctp -l 172.31.110.20 -vn 4443
+ sleep 0.1
+
+ ovs_sbx "$t" ip netns exec client \
+ ncat --sctp -w 2 -zv 172.31.110.20 4443 \
+ >/dev/null 2>&1 \
+ && { info "FAIL: connection should fail without flows"
+ return 1; }
+
+ info "reinstall flows and verify recovery"
+ ovs_add_flow "$t" sctp4 \
+ 'in_port(1),eth(),eth_type(0x0800),ipv4(proto=132),sctp(dst=4443)' \
+ '2' || return 1
+ ovs_add_flow "$t" sctp4 \
+ 'in_port(2),eth(),eth_type(0x0800),ipv4(proto=132),sctp(src=4443)' \
+ '1' || return 1
+
+ echo "server3" | \
+ ovs_netns_spawn_daemon "$t" "server" \
+ ncat --sctp -l 172.31.110.20 -vn 4443
+ sleep 0.1
+
+ ovs_sbx "$t" ip netns exec client \
+ ncat --sctp -i 1 -zv 172.31.110.20 4443 \
+ || return 1
+
+ return 0
+}
+
# psample test
# - use psample to observe packets
test_psample() {
diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
index e1ecfad2c03e..7cfc29ec7e59 100644
--- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
+++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
@@ -1982,6 +1982,11 @@ class ovskey(nla):
"icmp",
ovskey.ovs_key_icmp,
),
+ (
+ "OVS_KEY_ATTR_SCTP",
+ "sctp",
+ ovskey.ovs_key_sctp,
+ ),
(
"OVS_KEY_ATTR_TCP_FLAGS",
"tcp_flags",
--
2.54.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox