* [PATCH net-next v06 1/6] hinic3: Add ethtool queue ops
From: Fan Gong @ 2026-05-27 7:55 UTC (permalink / raw)
To: Fan Gong, Zhu Yikai, netdev, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Andrew Lunn,
Ioana Ciornei, Mohsin Bashir
Cc: linux-kernel, linux-doc, luosifu, Xin Guo, Zhou Shuai, Wu Like,
Shi Jing, Zheng Jiezhen, Maxime Chevallier
In-Reply-To: <cover.1779867397.git.zhuyikai1@h-partners.com>
Implement following ethtool callback function:
.get_ringparam
.set_ringparam
These callbacks allow users to utilize ethtool for detailed
queue depth configuration and monitoring.
Co-developed-by: Zhu Yikai <zhuyikai1@h-partners.com>
Signed-off-by: Zhu Yikai <zhuyikai1@h-partners.com>
Signed-off-by: Fan Gong <gongfan1@huawei.com>
---
.../ethernet/huawei/hinic3/hinic3_ethtool.c | 100 +++++++++++++++++
.../net/ethernet/huawei/hinic3/hinic3_irq.c | 3 +-
.../net/ethernet/huawei/hinic3/hinic3_main.c | 3 +
.../huawei/hinic3/hinic3_netdev_ops.c | 105 ++++++++++++++++--
.../ethernet/huawei/hinic3/hinic3_nic_dev.h | 8 ++
.../ethernet/huawei/hinic3/hinic3_nic_io.c | 4 +-
.../ethernet/huawei/hinic3/hinic3_nic_io.h | 8 +-
.../net/ethernet/huawei/hinic3/hinic3_rx.c | 2 +-
8 files changed, 219 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c b/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c
index 90fc16288de9..e594bcbc8153 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c
@@ -10,6 +10,7 @@
#include <linux/etherdevice.h>
#include <linux/netdevice.h>
#include <linux/ethtool.h>
+#include <net/devlink.h>
#include "hinic3_lld.h"
#include "hinic3_hw_comm.h"
@@ -409,6 +410,103 @@ hinic3_get_link_ksettings(struct net_device *netdev,
return 0;
}
+static void hinic3_get_ringparam(struct net_device *netdev,
+ struct ethtool_ringparam *ring,
+ struct kernel_ethtool_ringparam *kernel_ring,
+ struct netlink_ext_ack *extack)
+{
+ struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+
+ ring->rx_max_pending = HINIC3_MAX_RX_QUEUE_DEPTH;
+ ring->tx_max_pending = HINIC3_MAX_TX_QUEUE_DEPTH;
+ ring->rx_pending = nic_dev->rxqs[0].q_depth;
+ ring->tx_pending = nic_dev->txqs[0].q_depth;
+}
+
+static void hinic3_update_qp_depth(struct net_device *netdev,
+ u32 sq_depth, u32 rq_depth)
+{
+ struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+ u16 i;
+
+ nic_dev->q_params.sq_depth = sq_depth;
+ nic_dev->q_params.rq_depth = rq_depth;
+ for (i = 0; i < nic_dev->max_qps; i++) {
+ nic_dev->txqs[i].q_depth = sq_depth;
+ nic_dev->txqs[i].q_mask = sq_depth - 1;
+ nic_dev->rxqs[i].q_depth = rq_depth;
+ nic_dev->rxqs[i].q_mask = rq_depth - 1;
+ }
+}
+
+static int hinic3_check_ringparam_valid(struct net_device *netdev,
+ const struct ethtool_ringparam *ring,
+ struct netlink_ext_ack *extack)
+{
+ if (ring->tx_pending < HINIC3_MIN_QUEUE_DEPTH ||
+ ring->rx_pending < HINIC3_MIN_QUEUE_DEPTH) {
+ NL_SET_ERR_MSG_FMT_MOD(extack,
+ "Queue depth out of range tx[%d-%d] rx[%d-%d]",
+ HINIC3_MIN_QUEUE_DEPTH,
+ HINIC3_MAX_TX_QUEUE_DEPTH,
+ HINIC3_MIN_QUEUE_DEPTH,
+ HINIC3_MAX_RX_QUEUE_DEPTH);
+
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int hinic3_set_ringparam(struct net_device *netdev,
+ struct ethtool_ringparam *ring,
+ struct kernel_ethtool_ringparam *kernel_ring,
+ struct netlink_ext_ack *extack)
+{
+ struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+ struct hinic3_dyna_txrxq_params q_params = {};
+ u32 new_sq_depth, new_rq_depth;
+ int err;
+
+ err = hinic3_check_ringparam_valid(netdev, ring, extack);
+ if (err)
+ return err;
+
+ new_sq_depth = 1U << ilog2(ring->tx_pending);
+ new_rq_depth = 1U << ilog2(ring->rx_pending);
+ if (new_sq_depth == nic_dev->q_params.sq_depth &&
+ new_rq_depth == nic_dev->q_params.rq_depth)
+ return 0;
+
+ if (new_sq_depth != ring->tx_pending ||
+ new_rq_depth != ring->rx_pending)
+ NL_SET_ERR_MSG_FMT_MOD(extack,
+ "Requested Tx/Rx ring depth %u/%u trimmed to %u/%u",
+ ring->tx_pending, ring->rx_pending,
+ new_sq_depth, new_rq_depth);
+
+ netdev_info(netdev, "Change Tx/Rx ring depth from %u/%u to %u/%u\n",
+ nic_dev->q_params.sq_depth, nic_dev->q_params.rq_depth,
+ new_sq_depth, new_rq_depth);
+
+ if (!netif_running(netdev)) {
+ hinic3_update_qp_depth(netdev, new_sq_depth, new_rq_depth);
+ } else {
+ q_params = nic_dev->q_params;
+ q_params.sq_depth = new_sq_depth;
+ q_params.rq_depth = new_rq_depth;
+
+ err = hinic3_change_channel_settings(netdev, &q_params);
+ if (err) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Failed to change channel settings");
+ return err;
+ }
+ }
+
+ return 0;
+}
+
static const struct ethtool_ops hinic3_ethtool_ops = {
.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
ETHTOOL_COALESCE_PKT_RATE_RX_USECS,
@@ -417,6 +515,8 @@ static const struct ethtool_ops hinic3_ethtool_ops = {
.get_msglevel = hinic3_get_msglevel,
.set_msglevel = hinic3_set_msglevel,
.get_link = ethtool_op_get_link,
+ .get_ringparam = hinic3_get_ringparam,
+ .set_ringparam = hinic3_set_ringparam,
};
void hinic3_set_ethtool_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_irq.c b/drivers/net/ethernet/huawei/hinic3/hinic3_irq.c
index e7d6c2033b45..cc43773c1984 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_irq.c
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_irq.c
@@ -137,7 +137,8 @@ static int hinic3_set_interrupt_moder(struct net_device *netdev, u16 q_id,
struct hinic3_interrupt_info info = {};
int err;
- if (q_id >= nic_dev->q_params.num_qps)
+ if (test_bit(HINIC3_CHANGE_RES_INVALID, &nic_dev->flags) ||
+ q_id >= nic_dev->q_params.num_qps)
return 0;
info.interrupt_coalesc_set = 1;
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_main.c b/drivers/net/ethernet/huawei/hinic3/hinic3_main.c
index 0a888fe4c975..8cf605a0a5d2 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_main.c
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_main.c
@@ -315,6 +315,9 @@ static void hinic3_link_status_change(struct net_device *netdev,
{
struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+ if (test_bit(HINIC3_CHANGE_RES_INVALID, &nic_dev->flags))
+ return;
+
if (link_status_up) {
if (netif_carrier_ok(netdev))
return;
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c b/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c
index da73811641a9..b7f9ba8ce43f 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c
@@ -288,7 +288,8 @@ static void hinic3_free_channel_resources(struct net_device *netdev,
hinic3_free_qps(nic_dev, qp_params);
}
-static int hinic3_open_channel(struct net_device *netdev)
+static int hinic3_prepare_channel(struct net_device *netdev,
+ struct hinic3_dyna_txrxq_params *qp_params)
{
struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
int err;
@@ -299,16 +300,28 @@ static int hinic3_open_channel(struct net_device *netdev)
return err;
}
- err = hinic3_configure_txrxqs(netdev, &nic_dev->q_params);
+ err = hinic3_configure_txrxqs(netdev, qp_params);
if (err) {
netdev_err(netdev, "Failed to configure txrxqs\n");
goto err_free_qp_ctxts;
}
+ return 0;
+
+err_free_qp_ctxts:
+ hinic3_free_qp_ctxts(nic_dev);
+
+ return err;
+}
+
+static int hinic3_open_channel(struct net_device *netdev)
+{
+ int err;
+
err = hinic3_qps_irq_init(netdev);
if (err) {
netdev_err(netdev, "Failed to init txrxq irq\n");
- goto err_free_qp_ctxts;
+ return err;
}
err = hinic3_configure(netdev);
@@ -321,8 +334,6 @@ static int hinic3_open_channel(struct net_device *netdev)
err_uninit_qps_irq:
hinic3_qps_irq_uninit(netdev);
-err_free_qp_ctxts:
- hinic3_free_qp_ctxts(nic_dev);
return err;
}
@@ -428,6 +439,72 @@ static void hinic3_vport_down(struct net_device *netdev)
}
}
+int
+hinic3_change_channel_settings(struct net_device *netdev,
+ struct hinic3_dyna_txrxq_params *trxq_params)
+{
+ struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+ struct hinic3_dyna_txrxq_params cur_trxq_params = {};
+ struct hinic3_dyna_qp_params new_qp_params = {};
+ struct hinic3_dyna_qp_params cur_qp_params = {};
+ int err;
+
+ cur_trxq_params = nic_dev->q_params;
+
+ hinic3_config_num_qps(netdev, trxq_params);
+
+ err = hinic3_alloc_channel_resources(netdev, &new_qp_params,
+ trxq_params);
+ if (err) {
+ netdev_err(netdev, "Failed to alloc channel resources\n");
+ return err;
+ }
+
+ if (!test_and_set_bit(HINIC3_CHANGE_RES_INVALID, &nic_dev->flags)) {
+ hinic3_vport_down(netdev);
+ hinic3_close_channel(netdev);
+ hinic3_get_cur_qps(nic_dev, &cur_qp_params);
+ }
+
+ hinic3_init_qps(nic_dev, &new_qp_params);
+
+ err = hinic3_prepare_channel(netdev, trxq_params);
+ if (err)
+ goto err_uninit_qps;
+
+ if (nic_dev->num_qp_irq > trxq_params->num_qps)
+ hinic3_qp_irq_change(netdev, trxq_params->num_qps);
+
+ nic_dev->q_params = *trxq_params;
+
+ err = hinic3_open_channel(netdev);
+ if (err)
+ goto err_qp_irq_reset;
+
+ err = hinic3_vport_up(netdev);
+ if (err)
+ goto err_close_channel;
+
+ hinic3_free_channel_resources(netdev, &cur_qp_params, &cur_trxq_params);
+
+ clear_bit(HINIC3_CHANGE_RES_INVALID, &nic_dev->flags);
+
+ return 0;
+
+err_close_channel:
+ hinic3_close_channel(netdev);
+err_qp_irq_reset:
+ nic_dev->q_params = cur_trxq_params;
+
+ if (trxq_params->num_qps > cur_trxq_params.num_qps)
+ hinic3_qp_irq_change(netdev, cur_trxq_params.num_qps);
+err_uninit_qps:
+ hinic3_get_cur_qps(nic_dev, &new_qp_params);
+ hinic3_free_channel_resources(netdev, &new_qp_params, trxq_params);
+
+ return err;
+}
+
static int hinic3_open(struct net_device *netdev)
{
struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
@@ -458,6 +535,10 @@ static int hinic3_open(struct net_device *netdev)
hinic3_init_qps(nic_dev, &qp_params);
+ err = hinic3_prepare_channel(netdev, &nic_dev->q_params);
+ if (err)
+ goto err_uninit_qps;
+
err = hinic3_open_channel(netdev);
if (err)
goto err_uninit_qps;
@@ -473,7 +554,7 @@ static int hinic3_open(struct net_device *netdev)
err_close_channel:
hinic3_close_channel(netdev);
err_uninit_qps:
- hinic3_uninit_qps(nic_dev, &qp_params);
+ hinic3_get_cur_qps(nic_dev, &qp_params);
hinic3_free_channel_resources(netdev, &qp_params, &nic_dev->q_params);
err_destroy_num_qps:
hinic3_destroy_num_qps(netdev);
@@ -493,10 +574,18 @@ static int hinic3_close(struct net_device *netdev)
return 0;
}
+ if (test_and_clear_bit(HINIC3_CHANGE_RES_INVALID, &nic_dev->flags))
+ goto out;
+
hinic3_vport_down(netdev);
hinic3_close_channel(netdev);
- hinic3_uninit_qps(nic_dev, &qp_params);
- hinic3_free_channel_resources(netdev, &qp_params, &nic_dev->q_params);
+ hinic3_get_cur_qps(nic_dev, &qp_params);
+ hinic3_free_channel_resources(netdev, &qp_params,
+ &nic_dev->q_params);
+
+out:
+ hinic3_free_nicio_res(nic_dev);
+ hinic3_destroy_num_qps(netdev);
return 0;
}
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_dev.h b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_dev.h
index 9502293ff710..2322df552ee9 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_dev.h
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_dev.h
@@ -10,6 +10,9 @@
#include "hinic3_hw_cfg.h"
#include "hinic3_hwdev.h"
#include "hinic3_mgmt_interface.h"
+#include "hinic3_nic_io.h"
+#include "hinic3_tx.h"
+#include "hinic3_rx.h"
#define HINIC3_VLAN_BITMAP_BYTE_SIZE(nic_dev) (sizeof(*(nic_dev)->vlan_bitmap))
#define HINIC3_VLAN_BITMAP_SIZE(nic_dev) \
@@ -20,6 +23,7 @@ enum hinic3_flags {
HINIC3_MAC_FILTER_CHANGED,
HINIC3_RSS_ENABLE,
HINIC3_UPDATE_MAC_FILTER,
+ HINIC3_CHANGE_RES_INVALID,
};
enum hinic3_event_work_flags {
@@ -143,6 +147,10 @@ struct hinic3_nic_dev {
void hinic3_set_netdev_ops(struct net_device *netdev);
int hinic3_set_hw_features(struct net_device *netdev);
+int
+hinic3_change_channel_settings(struct net_device *netdev,
+ struct hinic3_dyna_txrxq_params *trxq_params);
+
int hinic3_qps_irq_init(struct net_device *netdev);
void hinic3_qps_irq_uninit(struct net_device *netdev);
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_io.c b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_io.c
index 87e736adba02..0e7a0ccfba98 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_io.c
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_io.c
@@ -484,8 +484,8 @@ void hinic3_init_qps(struct hinic3_nic_dev *nic_dev,
}
}
-void hinic3_uninit_qps(struct hinic3_nic_dev *nic_dev,
- struct hinic3_dyna_qp_params *qp_params)
+void hinic3_get_cur_qps(struct hinic3_nic_dev *nic_dev,
+ struct hinic3_dyna_qp_params *qp_params)
{
struct hinic3_nic_io *nic_io = nic_dev->nic_io;
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_io.h b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_io.h
index 12eefabcf1db..571b34d63950 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_io.h
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_io.h
@@ -14,6 +14,10 @@ struct hinic3_nic_dev;
#define HINIC3_RQ_WQEBB_SHIFT 3
#define HINIC3_SQ_WQEBB_SIZE BIT(HINIC3_SQ_WQEBB_SHIFT)
+#define HINIC3_MAX_TX_QUEUE_DEPTH 65536
+#define HINIC3_MAX_RX_QUEUE_DEPTH 16384
+#define HINIC3_MIN_QUEUE_DEPTH 128
+
/* ******************** RQ_CTRL ******************** */
enum hinic3_rq_wqe_type {
HINIC3_NORMAL_RQ_WQE = 1,
@@ -136,8 +140,8 @@ void hinic3_free_qps(struct hinic3_nic_dev *nic_dev,
struct hinic3_dyna_qp_params *qp_params);
void hinic3_init_qps(struct hinic3_nic_dev *nic_dev,
struct hinic3_dyna_qp_params *qp_params);
-void hinic3_uninit_qps(struct hinic3_nic_dev *nic_dev,
- struct hinic3_dyna_qp_params *qp_params);
+void hinic3_get_cur_qps(struct hinic3_nic_dev *nic_dev,
+ struct hinic3_dyna_qp_params *qp_params);
int hinic3_init_qp_ctxts(struct hinic3_nic_dev *nic_dev);
void hinic3_free_qp_ctxts(struct hinic3_nic_dev *nic_dev);
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_rx.c b/drivers/net/ethernet/huawei/hinic3/hinic3_rx.c
index 309ab5901379..b5b601469517 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_rx.c
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_rx.c
@@ -541,7 +541,7 @@ int hinic3_configure_rxqs(struct net_device *netdev, u16 num_rq,
rq_associate_cqes(rxq);
pkts = hinic3_rx_fill_buffers(rxq);
- if (!pkts) {
+ if (pkts < rxq->q_depth - 1) {
netdev_err(netdev, "Failed to fill Rx buffer\n");
return -ENOMEM;
}
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v06 0/6] net: hinic3: PF initialization
From: Fan Gong @ 2026-05-27 7:55 UTC (permalink / raw)
To: Fan Gong, Zhu Yikai, netdev, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Andrew Lunn,
Ioana Ciornei, Mohsin Bashir
Cc: linux-kernel, linux-doc, luosifu, Xin Guo, Zhou Shuai, Wu Like,
Shi Jing, Zheng Jiezhen, Maxime Chevallier
This is [3/3] part of hinic3 Ethernet driver second submission.
With this patch hinic3 becomes a complete Ethernet driver with
pf and vf.
Add 20 ethtool ops for information of queue, rss, coalesce and eth data.
Add MTU size validation
Config netdev watchdog timeout.
Remove unneed coalesce parameters.
Changes:
PATCH 03 V01: https://lore.kernel.org/netdev/cover.1773387649.git.zhuyikai1@h-partners.com/
* Add rmon/pause/phy/mac/ctrl stats (Ioana Ciornei)
PATCH 03 V02: https://lore.kernel.org/netdev/cover.1774684571.git.zhuyikai1@h-partners.com/
* Modify "return -EINVAL" intension problem (AI review)
* Use le16_to_cpu for rss_indir pair.out->buf (AI review)
* Use u32 instead of int in coalesce_limits to avoid overflow (AI review)
* Remove redundant u64_stats_update_begin/end when reading stats without
concurrent reader (AI review)
* Modify nic_dev->stats.syncp logic (AI review)
* Complete rxq/txq stats stats fileds in hinic3_rx/txq_get_stats (AI review)
* Remove statistics values in rtnl_link_stats64 from ethtool statistics
values (AI review)
* Add channel_cfg_lock & channel_res_lock to protect resources access (AI review)
* Remove OutOfRangeLengthField, FrameToolong and InRangeLengthErrors (Ioana Ciornei)
* Remove redundant mtu commit (Maxime Chevialler)
PATCH 03 V03: https://lore.kernel.org/netdev/cover.1774940117.git.zhuyikai1@h-partners.com/
* Change unnedd to unneeded (AI review)
* Remove packets,bytes,errors and dropped in hinic3_rx/tx_queue_stats (AI review)
* Remove duplicated entried in hinic3_port_stats[] (AI review)
* change stats_info.head.status to ps->head.status (AI review)
PATCH 03 V04: https://lore.kernel.org/netdev/cover.1775618797.git.zhuyikai1@h-partners.com/
* Remove restore_drop_sge in hinic3_rx_queue_stats (AI review)
* Remove hinic3_nic_stats (AI review)
* Use old_q_param to store old config and use it in error handling (Mohsin Bashir)
* Add netdev_info to inform the user that depth is trimmed (Mohsin Bashir)
* Remove const in hinic3_get_qp_stats_strings parameters (Mohsin Bashir)
* Change EOPNOTSUPP to ERANGE in is_coalesce_exceed_limit (Mohsin Bashir)
* Update nic_dev->rss_type after hinic3_set_rss_type (Mohsin Bashir)
* Modify MGMT_STATUS_CMD_UNSUPPORTED to EOPNOTSUPP for complying with the
error code specifications (Mohsin Bashir)
PATCH 03 V05: https://lore.kernel.org/netdev/cover.1775711066.git.zhuyikai1@h-partners.com/
* Clear HINIC3_CHANGE_RES_INVALID bit in error handling (AI review)
* Use low >= high to avoid low=high in is_coalesce_legal (AI review)
* As tx and rx share interrupts, we only use ETHTOOL_COALESCE_RX_USECS for
user setting to avoid user misunderstanding. So we do not add
ETHTOOL_COALESCE_TX_USECS. (Mohsin Bashir & AI review)
PATCH 03 V06:
* Remove redundant rx_jumbo_pending and rx_mini_pending judgement (Jakub Kicinski)
* Remove redundant max tx_pending judgement when .get_ringparam already got the
max value (Jakub Kicinski)
* Use extack instead of netdev_err/netdev_info/netdev_warning (Jakub Kicinski)
* Remove HINIC3_CHANNEL_RES_VALID and only use HINIC3_CHANGE_RES_INVALID
bit (Jakub Kicinski)
* Deference freed pointers in hinic3_change_channel_settings error
handling (Jakub Kicinski)
* Modify hinic3_open_channel (Jakub Kicinski)
Fan Gong (6):
hinic3: Add ethtool queue ops
hinic3: Add ethtool statistic ops
hinic3: Add ethtool coalesce ops
hinic3: Add ethtool rss ops
hinic3: Configure netdev->watchdog_timeo to set nic tx timeout
hinic3: Remove unneeded coalesce parameters
.../ethernet/huawei/hinic3/hinic3_ethtool.c | 835 +++++++++++++++++-
.../ethernet/huawei/hinic3/hinic3_hw_intf.h | 13 +-
.../net/ethernet/huawei/hinic3/hinic3_irq.c | 9 +-
.../net/ethernet/huawei/hinic3/hinic3_main.c | 7 +
.../huawei/hinic3/hinic3_mgmt_interface.h | 39 +
.../huawei/hinic3/hinic3_netdev_ops.c | 105 ++-
.../ethernet/huawei/hinic3/hinic3_nic_cfg.c | 64 ++
.../ethernet/huawei/hinic3/hinic3_nic_cfg.h | 109 +++
.../ethernet/huawei/hinic3/hinic3_nic_dev.h | 8 +
.../ethernet/huawei/hinic3/hinic3_nic_io.c | 4 +-
.../ethernet/huawei/hinic3/hinic3_nic_io.h | 8 +-
.../net/ethernet/huawei/hinic3/hinic3_rss.c | 487 +++++++++-
.../net/ethernet/huawei/hinic3/hinic3_rss.h | 19 +
.../net/ethernet/huawei/hinic3/hinic3_rx.c | 61 +-
.../net/ethernet/huawei/hinic3/hinic3_rx.h | 18 +-
.../net/ethernet/huawei/hinic3/hinic3_tx.c | 71 +-
.../net/ethernet/huawei/hinic3/hinic3_tx.h | 2 +
17 files changed, 1823 insertions(+), 36 deletions(-)
base-commit: aa064a614efcfa4c300609d1f01134e99a12ad10
--
2.43.0
^ permalink raw reply
* Re: [PATCH -next] mtd: spi-nor: testing locking, fix new doc build warnings
From: Miquel Raynal @ 2026-05-27 7:54 UTC (permalink / raw)
To: Randy Dunlap
Cc: linux-doc, Jonathan Corbet, Shuah Khan, Pratyush Yadav,
Michael Walle, Takahiro Kuwano, linux-mtd, Richard Weinberger,
Vignesh Raghavendra
In-Reply-To: <20260526172341.773398-1-rdunlap@infradead.org>
On 26/05/2026 at 10:23:41 -07, Randy Dunlap <rdunlap@infradead.org> wrote:
> Add a blank line to prevent documentation build warnings:
>
> Documentation/driver-api/mtd/spi-nor.rst:215: ERROR: Unexpected indentation. [docutils]
> Documentation/driver-api/mtd/spi-nor.rst:216: WARNING: Block quote ends without a blank line; unexpected unindent. [docutils]
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Thanks!
Miquèl
^ permalink raw reply
* Re: [PATCH v6 00/28] mtd: spi-nor: Enhance software protection
From: Miquel Raynal @ 2026-05-27 7:54 UTC (permalink / raw)
To: Pratyush Yadav
Cc: Michael Walle, Takahiro Kuwano, Richard Weinberger,
Vignesh Raghavendra, Jonathan Corbet, Tudor Ambarus, Shuah Khan,
Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
linux-kernel, linux-doc, stable
In-Reply-To: <2vxz8q964210.fsf@kernel.org>
> Applied to spi-nor/next. This should give us around 3 weeks to soak in
> linux next.
That's fine. Most of the build related breakages will usually be
discovered within one week. Testing feedback will anyway be much longer
to come.
> Expect the SPI NOR PR a little bit later than usual, around
> the start of the merge window so we can maximize the exposure of these
> patches.
I try to send the MTD PR between mid and end of the first week of the
merge window, getting your PR on the first days of the merge window is
still completely okay since the branches are already pulled into
linux-next.
> Apologies once again for the crappy contributor experience, I tried to
> get to this sooner, but wasn't much successful at it :-/
No worries :-)
Thanks,
Miquèl
^ permalink raw reply
* Re: [PATCH net-next 06/10] docs: net: refresh netdev feature guidance
From: Maxime Chevallier @ 2026-05-27 7:53 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, corbet,
vladimir.oltean, willemb, sdf.kernel, ecree.xilinx,
jesse.brandeburg, linux-doc
In-Reply-To: <20260526153532.7979b43c@kernel.org>
Hi Jakub
On 5/27/26 00:35, Jakub Kicinski wrote:
> On Tue, 26 May 2026 20:41:10 +0200 Maxime Chevallier wrote:
>>>
>>> 1. netdev->hw_features set contains features whose state may possibly
>>> be changed (enabled or disabled) for a particular device by user's
>>> - request. This set should be initialized in ndo_init callback and not
>>> - changed later.
>>> + request. Drivers normally initialize this set before registration or
>>> + in the ndo_init callback. Changes after registration should be made
>>> + very carefully as other parts of the code may assume hw_features are
>>> + static. At the very least changes must be made under rtnl_lock and
>>> + the netdev instance lock, and followed by netdev_update_features().
>> Feel free to keep this description as-is, but can we get somewhere the
>> actual meaning of "hw" in "hw_features" ? I've seen this cause confusion
>> before as this is sometimes wrongly interpreted as "Hardware features",
>> which isn't correct as the hardware may do stuff without allowing users
>> to change that behaviour.
>>
>> I vaguely recall something along the lines of "Host-Writeable features",
>> but I am not sure at all about that...
>
> Hm. I assumed the hw in hw_features stands for hardware.
> The magic behavior of host controllable vs hardwired was
> probably added later without renaming the field.
>
> As you indicate the usual confusion is that it's legal to have
> a feature in ->features which is not set in ->hw_features which
> means that features is hardwired "on", it can't be disabled by
> the user.
>
> The current text does say this: "features whose state may [..]
> be changed [..] by user's request". But perhaps it's not emphatic
> enough.
>
> Main question is whether this series should be clarifying
> this or our criteria is that the series doesn't _add_ confusion,
> even if it doesn't clarify all the potential confusion points? :)
Frankly I think your series is good as it is, the text is clear
enough, and we can redirect people to it if we spot an issue
on that point during review.
Thanks a lot for answering,
Maxime
^ permalink raw reply
* Re: [PATCH v5 04/28] mtd: spi-nor: swp: Improve locking user experience
From: Miquel Raynal @ 2026-05-27 7:51 UTC (permalink / raw)
To: Pratyush Yadav
Cc: Tudor Ambarus, Michael Walle, Takahiro Kuwano, Richard Weinberger,
Vignesh Raghavendra, Jonathan Corbet, Shuah Khan, Sean Anderson,
Thomas Petazzoni, Steam Lin, linux-mtd, linux-kernel, linux-doc,
stable
In-Reply-To: <2vxzcxyi42qh.fsf@kernel.org>
Hi Pratyush,
>> I know what the maintainer load can be, sometimes it does not play well
>> with the rest of the your personal and professional duties. But the
>> series has already been on the list for about 8 months, it's been looked
>> at by other people, the ones who had enough time to dedicate to it. From
>> my perspective, asking such contributions to wait indefinitely and then
>> suggesting partial application without a technical reason is not a
>> sustainable way to handle contributions. This series has not moved much,
>> it could have been applied *much* earlier. I've now addressed most of
>> the comments from Sashiko, v6 is coming, further improving the quality
>> for sure, as there were bugs - there are always. I am of course happy to
>> address further technical concerns, if there are any, but I would
>> strongly prefer merging the series as the coherent set it was intended
>> to be, rather than only taking the preparatory parts.
>
> Unfortunately I am only a patch monkey for SPI NOR these days and do not
> have any time to do reviews, especially for big series. I mainly check
> for reviewed patches and try to apply them. Even that is becoming harder
> these days since I am more busy at new $DAYJOB.
I fully understand that.
> I understand your frustration, and do take the blame for this, but
> unfortunately can't promise anything better in the future. We pretty
> much don't have any active reviewers in SPI NOR. Michael and Tudor are
> also short on time these days. If your employer cares about SPI NOR,
> perhaps you can encourage them to support developers in helping out with
> reviews and maintenance.
Well, I believe I already take a faire share in reviewing and
maintaining the subsystem :-) I even started getting a look into spi-nor
patches recently since I ramped up on the subsystem.
Michel and Tudor regularly make useful feedback, Takahiro is ramping up,
I try to keep enough availability for maintaining NAND, SPI NAND and
anything that is MTD but not in one of the three main subsystems. I
don't think we are in a bad maintainership situation.
> Applying the reviewed patches reduces the total patchset size and does
> make things easier to review and land. So I don't get why you are so
> opposed to the idea.
Applying series partially is okay; specifically for this series,
patches 1-8 could have been taken alone, but a bit earlier IMHO.
> But anyway, I'll bite the bullet here and apply the series. You
> generally have high quality patches so I am not super worried about this
> to begin with. We can deal with the bugs or shortcomings later I
> suppose...
Just to be clear, I am not asking for a fast path here. We need to
follow the contribution and review model, because that is the saniest
model that worked well for us until now. I am however opposed to the
statut quo situation that was progressively settling because of the lack
of availability.
Also, I am always following closely when patches get applied, so I can
respond quickly in case of breakage.
Thanks for your work Pratyush, don't get me wrong, your work is highly
appreciated, and your availability is a parameter we will comply with.
Cheers ;-)
Miquèl
^ permalink raw reply
* [PATCH] Update maintainer-handbooks
From: Amanda Corrêa @ 2026-05-27 7:48 UTC (permalink / raw)
To: danielmaraboo@gmail.com; +Cc: linux-doc@vger.kernel.org
[-- Attachment #1.1: Type: text/plain, Size: 2 bytes --]
[-- Attachment #1.2: Type: text/html, Size: 337 bytes --]
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-docs-pt-br-translate-maintainer-handbooks.patch --]
[-- Type: text/x-patch; name="0001-docs-pt-br-translate-maintainer-handbooks.patch", Size: 1662 bytes --]
From 13da7cb6de2e260055b9fb773a3281002edb26ff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Amanda=20Corr=C3=AAa?= <amandacorreades@hotmail.com>
Date: Wed, 27 May 2026 04:21:50 -0300
Subject: [PATCH] docs: pt-br: translate maintainer-handbooks
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Updated the maintainer-handbooks documentation to Brazilian
Portuguese.
Signed-off-by: Amanda Corrêa <amandacorreades@hotmail.com>
---
.../pt_BR/process/maintainer-handbooks.rst | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/Documentation/translations/pt_BR/process/maintainer-handbooks.rst b/Documentation/translations/pt_BR/process/maintainer-handbooks.rst
index bf7a38147..555fecae6 100644
--- a/Documentation/translations/pt_BR/process/maintainer-handbooks.rst
+++ b/Documentation/translations/pt_BR/process/maintainer-handbooks.rst
@@ -7,6 +7,17 @@ O propósito deste documento é fornecer informações específicas de
subsistemas que são suplementares ao manual geral do processo de
desenvolvimento.
+Para desenvolvedores, veja abaixo todos os guias específicos de
+subsistemas conhecidos. Se o subsistema para o qual você está
+contribuindo não tiver um guia listado aqui, é recomendável buscar
+esclarecimentos sobre as questões levantadas em
+Documentation/maintainer/maintainer-entry-profile.rst.
+
+Para mantenedores, considere documentar requisitos adicionais e
+expectativas caso as submissões frequentemente deixem de atender
+a critérios específicos de submissão. Veja
+Documentation/maintainer/maintainer-entry-profile.rst.
+
Conteúdos:
.. toctree::
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net-next] Documentation: networking: Add a test plan for ethtool pause validation
From: Maxime Chevallier @ 2026-05-27 7:07 UTC (permalink / raw)
To: Andrew Lunn, Jakub Kicinski
Cc: davem, Eric Dumazet, Paolo Abeni, Simon Horman, Russell King,
Heiner Kallweit, Jonathan Corbet, Shuah Khan, Oleksij Rempel,
Vladimir Oltean, Florian Fainelli, thomas.petazzoni, netdev,
linux-kernel, linux-doc
In-Reply-To: <5cb8e2b4-8eb6-4446-9b90-1cd4c7964cd9@lunn.ch>
On 5/27/26 04:47, Andrew Lunn wrote:
> On Tue, May 26, 2026 at 05:24:47PM -0700, Jakub Kicinski wrote:
>> On Fri, 22 May 2026 19:51:06 +0200 Maxime Chevallier (Netdev
>> Foundation) wrote:
>>> Documentation/networking/pause_test_plan.rst | 556 +++++++++++++++++++
>>
>> It'd be great to hear from others but IMHO in the current form this is
>> not suitable for Documentation/networking/ We can commit the "knowledge"
>> part but enumerating the test cases seems odd for Documentation/.
>
> Sorry, not looked too deeply at the actual content yet.
>
> What i was thinking was a python file, which sphinx can ingest to
> produce documentation, and place holders were code would be added to
> implement the actual test during the next phase.
>
> This is how i've done testing in the past. I would be the evil one who
> thought up the tests and described them in detail using sphinx markup
> in a python test template file. After some review they got passed off
> to a python developer for implementation. And when they got run and
> failed, sometimes the feature developer, the test developer and myself
> got together to figure who made the error.
>
> I'm not sure we even need sphinx. What i find important is that the
> test is documented. What kAPI calls should be made with what
> parameters. What results we are expected and why? So that when a test
> fails, a developer has the information they need to fix their
> code. The Why? is important, and often missing from the kernel tests.
I see, so we'd get an actual test plan as code (likely python), with for
now placeholder tests, that would work :)
As for the kAPI testing, I agree that the end goal is to get driver
authors to get their flow control implementation right running this
suite.
But I don't really see how we can validate kAPI itself, as we're down at
the ethnl level.
This is made more complex by the fact that there are 2 drivers involved
here, the MAC and the PHY one. On top of that, either the MAC driver
uses phylink, or it doesn't. The common issues are different depending
on the case.
Raw phylib usage is by far the trickiest, but even phylink-based MAC
drivers can get it wrong, by ignoring the pause params in their
mac_link_up() phylink callback, or getting their capabilities wrong.
The problem is we get an aggregated view of that in userspace. The
pauseparams come from the MAC driver, but the supported and advertised
modes are an aggregation of PHY + MAC stuff, which makes it very hard
to pinpoint where people are getting it wrong. Maybe people are writing
a MAC driver, but their tests fail because the PHY on their board has
a mis-behaving PHY driver, etc.
We're missing some information about what the MAC alone does, and what
the PHY driver alone also does, so that we can look at it from the
userspace perspective, and says for example "OK your MAC says that
it can only do Sym Pause, but I see that the interface reports
supporting Asym as well, and you're not using phylink, so you are
not calling phy_set_asymp_pause() correctly".
One possible path for that could be debugfs ? to let MAC/PHY expose
precise info, so we know who is wrong ?
Or, we go lower level and use the .self_test ethtool callback. We're no
longer validating the user-visible behaviour, but we have more control
on kAPI testing. We can poke into net_device, net_device->phydev, we may
even instrument phylib to validate what the MAC is doing ? But that
means we're drifting away from the netlink-level testing.
Maxime
>
> Andrew
^ permalink raw reply
* Re: [PATCH net-next 3/3] net/mlx5: Apply devlink default eswitch mode during init
From: Mark Bloch @ 2026-05-27 7:03 UTC (permalink / raw)
To: Jiri Pirko
Cc: Tariq Toukan, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Andrew Lunn, David S. Miller, Jonathan Corbet, Shuah Khan,
Simon Horman, Saeed Mahameed, Leon Romanovsky,
Borislav Petkov (AMD), Andrew Morton, Randy Dunlap,
Thomas Gleixner, Petr Mladek, Peter Zijlstra (Intel), Tejun Heo,
Vlastimil Babka, Feng Tang, Christian Brauner, Dave Hansen,
Dapeng Mi, Kees Cook, Marco Elver, Li RongQing, Eric Biggers,
Paul E. McKenney, linux-doc, linux-kernel, netdev, linux-rdma,
Gal Pressman, Dragos Tatulea, Jiri Pirko, Shay Drori,
Moshe Shemesh
In-Reply-To: <ahZ9CgIWdjny4N4D@FV6GYCPJ69>
On 27/05/2026 8:14, Jiri Pirko wrote:
> Tue, May 26, 2026 at 07:13:46PM +0200, mbloch@nvidia.com wrote:
>>
>>
>> On 26/05/2026 19:23, Jiri Pirko wrote:
>>> Tue, May 26, 2026 at 05:03:57PM +0200, mbloch@nvidia.com wrote:
>>>>
>>>>
>>>> On 26/05/2026 17:07, Jiri Pirko wrote:
>>>>> Tue, May 26, 2026 at 11:44:46AM +0200, mbloch@nvidia.com wrote:
>>>>>>
>>>>>>
>>>>>> On 26/05/2026 10:44, Jiri Pirko wrote:
>>>>>>> Thu, May 21, 2026 at 09:24:34AM +0200, tariqt@nvidia.com wrote:
>>>>>>>> From: Mark Bloch <mbloch@nvidia.com>
>>>>>>>>
>>>>>>>> Apply devlink default eswitch mode for mlx5 devices after successful
>>>>>>>> device initialization while holding the devlink instance lock.
>>>>>>>>
>>>>>>>> At this point the devlink instance is registered and the mlx5 devlink
>>>>>>>> operations are available, so the default eswitch mode can be applied to
>>>>>>>> the matching PCI devlink handle.
>>>>>>>>
>>>>>>>> Signed-off-by: Mark Bloch <mbloch@nvidia.com>
>>>>>>>> Reviewed-by: Shay Drori <shayd@nvidia.com>
>>>>>>>> Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
>>>>>>>> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
>>>>>>>> ---
>>>>>>>> drivers/net/ethernet/mellanox/mlx5/core/main.c | 17 +++++++++++++++++
>>>>>>>> 1 file changed, 17 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
>>>>>>>> index 0c6e4efe38c8..4528097f3d84 100644
>>>>>>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
>>>>>>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
>>>>>>>> @@ -1391,6 +1391,21 @@ static void mlx5_unload(struct mlx5_core_dev *dev)
>>>>>>>> mlx5_free_bfreg(dev, &dev->priv.bfreg);
>>>>>>>> }
>>>>>>>>
>>>>>>>> +static void mlx5_devl_apply_default_esw_mode(struct mlx5_core_dev *dev)
>>>>>>>> +{
>>>>>>>> + struct devlink *devlink = priv_to_devlink(dev);
>>>>>>>> + int err;
>>>>>>>> +
>>>>>>>> + if (!MLX5_ESWITCH_MANAGER(dev))
>>>>>>>> + return;
>>>>>>>> +
>>>>>>>> + devl_assert_locked(devlink);
>>>>>>>> + err = devl_apply_default_esw_mode(devlink);
>>>>>>>> + if (err)
>>>>>>>> + mlx5_core_warn(dev, "Couldn't apply default eswitch mode, err %d\n",
>>>>>>>> + err);
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> int mlx5_init_one_devl_locked(struct mlx5_core_dev *dev)
>>>>>>>> {
>>>>>>>> bool light_probe = mlx5_dev_is_lightweight(dev);
>>>>>>>> @@ -1437,6 +1452,7 @@ int mlx5_init_one_devl_locked(struct mlx5_core_dev *dev)
>>>>>>>> mlx5_core_err(dev, "mlx5_hwmon_dev_register failed with error code %d\n", err);
>>>>>>>>
>>>>>>>> mutex_unlock(&dev->intf_state_mutex);
>>>>>>>> + mlx5_devl_apply_default_esw_mode(dev);
>>>>>>>
>>>>>>> I wonder how we can make this work for all. I mean, other driver would
>>>>>>> silently ignore this command like arg, right? Any idea how to make all
>>>>>>> drivers follow the arg from very beginning?
>>>>>>>
>>>>>>
>>>>>> I have a follow-up series that adds the call to all drivers which support
>>>>>> setting eswitch mode. When going over the other drivers, what I found is
>>>>>> that the right point to apply the default is driver specific, drivers
>>>>>> I have patch for:
>>>>>>
>>>>>> 46e16c6d9836 net: Apply devlink esw mode defaults
>>>>>> ab4f54102ba9 bnxt_en: Apply devlink default eswitch mode during init
>>>>>> b48cce1607bb liquidio: Apply devlink default eswitch mode during init
>>>>>> 4ea54b0fe04a ice: Apply devlink default eswitch mode during init
>>>>>> b7faddaa1c90 octeontx2-af: Apply devlink default eswitch mode during init
>>>>>> 74b0c22c47b9 octeontx2-pf: Apply devlink default eswitch mode during init
>>>>>> 5000e4c3d768 nfp: Apply devlink default eswitch mode during init
>>>>>> 97a218e95e41 netdevsim: Apply devlink default eswitch mode during init
>>>>>>
>>>>>> I don't think doing this generically from devlink is realistic. devlink
>>>>>> doesn't really know when a given driver is ready to change eswitch mode.
>>>>>> Some drivers need SR-IOV state, representor setup, or other init pieces to
>>>>>> be ready first, and the locking is not identical across drivers either.
>>>>>
>>>>>
>>>>> Low hanging fruit would be just to call ops->eswitch_mode_set at the end
>>>>> of register. Multiple reasons:
>>>>>
>>>>> 1) end of devl_register is exactly the point userspace is free to issue
>>>>> the eswitch mode set. Driver should be ready to handle it.
>>>>> 2) all drivers would transparently get this functionality, without
>>>>> actually knowing this kernel command line arg ever existed, without
>>>>> odd wiring call of related exported function. I prefer that stongly.
>>>>> 3) you should add a there warning for the case this arg is passed yet
>>>>> the driver does not implement eswitch_mode_set. User should
>>>>> get a feedback like this, not silent ignore.
>>>>>
>>>>> The only loose end is see it the void return of devl_register().
>>>>> Multiple ways to handle the possibly failed eswitch_mode_set(). I would
>>>>> probably just go for pr_warn, seems to be the most correct.
>>>>>
>>>>> Make sense?
>>>>
>>>> I see the point, but I don't think devl_register() (at least not the only place)
>>>> is the right place.
>>>>
>>>> There is a small but important difference between userspace doing
>>>> "devlink eswitch set" after register is done, and devlink core calling
>>>> eswitch_mode_set() from inside the register flow.
>>>>
>>>> Some drivers call devlink_register() while holding the device lock.
>>>> liquidio is one example. If devlink core calls ops->eswitch_mode_set() from
>>>> there, we may start the full eswitch mode change while holding that lock.
>>>> That mode change can create representors, register netdevs, take rtnl,
>>>> allocate resources, etc. I don't think we want this to become an implicit
>>>> side effect of devlink registration.
>>>
>>> I believe your AI may untagle liquidio locking :)
>>
>> I didn't try to solve that one with ai. Most drivers were fairly simple
>> so I didn't use ai at all. bnxt was the one where I needed a bit of help :)
>>
>>>
>>>
>>>>
>>>> For mlx5, the placement after intf_state_mutex is also intentional:
>>>>
>>>> mutex_unlock(&dev->intf_state_mutex);
>>>> mlx5_devl_apply_default_esw_mode(dev);
>>>>
>>>> We can't call it while holding intf_state_mutex because the mode set path
>>>> takes it internally, and switchdev mode may also create IB representors.
>>>>
>>>> Also, devl_register() only covers the first registration. The mlx5 call in
>>>> mlx5_load_one_devl_locked() is for reload/fw reset recovery kind of flows.
>>>> In those flows devlink is already registered, so devl_register() is not
>>>> called again, but the driver state was rebuilt and we may need to apply the
>>>> default again.
>>>
>>> Call it from reload too, right?
>>
>> Yes, that was my first thought: apply it from devl_register() for the first
>> registration and from devlink_reload() after a successful DRIVER_REINIT.
>>
>> That covers the clean devlink reload path but....(see bellow)
>>
>>>
>>>
>>>>
>>>> Same for reload, fw reset and pci recovery in general. If the driver tears
>>>> down and rebuilds eswitch related state, the place to apply the default is
>>>> in that driver's reinit flow, not in devl_register().
>>>>
>>>> When I went over the other drivers, the right place was not always the same
>>>> as devlink registration. I'm not an expert in any of them, so I hope I got
>>>> the details right, but for example octeontx2 AF needs sr-iov and the
>>>> representor switch state to be initialized first. nfp can do it after
>>>> app/vNIC init while the devlink lock is already held. liquidio should do it
>>>> only after dropping the PCI device lock.
>>>
>>> Idk, perhaps do it from devlink_post_register_work of some kind? That
>>> would allow you to have the same locking ordering as a userspace cal
>> l.
>>
>> I thought about a workqueue too, it was actually my first idea.
>>
>> The problem is that then we race with userspace. In the mlx5 version here the
>> default is applied while the devlink lock is still held, before userspace can
>> come in and issue its own eswitch set. If we defer it to post-register work,
>> the devlink instance is already visible and userspace can get there first
>> and then we might change the user configuration.
>
> Figure that out and expose to user by setting xa_mark only after the
> work is done? This is doable.
I agree that if devlink can keep the instance hidden/unavailable until the
post register work is done, that solves the initial userspace race.
The other part is the reinit/recovery case. For that I think devlink core
needs some explicit indication from the driver that the device is now in
reinit. Something like (at least that's the code I had initially, but something
along those lines):
void devl_dev_reinit_begin(struct devlink *devlink);
void devl_dev_reinit_end(struct devlink *devlink);
void devl_dev_reinit_abort(struct devlink *devlink);
The core can then mark the instance as temporarily unavailable/in reinit
between begin/end, and the relevant lookup/dump paths, for example
devlink_get_from_attrs_lock() and devlink_nl_inst_iter_dumpit(), can reject
or skip it while reinit is in progress. devlink_reload() can probably mark
this state by itself around DRIVER_REINIT.
Then mlx5 would look more or less like:
devl_lock(devlink);
devl_dev_reinit_begin(devlink);
ret = mlx5_load_one_devl_locked(dev, recovery);
if (!ret)
devl_dev_reinit_end(devlink);
else
devl_dev_reinit_abort(devlink);
devl_unlock(devlink);
This gives devlink core a way to know that the devlink instance is registered,
but should not be used by userspace at the moment. It also allows keeping the
default/config apply logic in devlink instead of adding driver specific calls
to apply it in each init path.
But this still means the generic solution needs some driver help. Drivers need
to register devlink at a point where the post-register default apply is safe,
and full reinit paths need to be marked with this begin/end API.
>
>
>>
>> Also, the bigger issue for mlx5 is not only initial registration or devlink
>> reload. Some recovery paths, pci resume, and fw reset flows rebuild the driver
>> state without going through devlink at all. I did not find a clean way for
>> devlink core to infer all those points by itself.
>
> If you don't obey current configuration for example in pci resume, it is
> bug and you should fix it. All these flows should obey current eswitch
> mode configuration.
>
I agree that the device should come back according
to the intended high level policy. But I don't think full reinit can be treated
as restoring the whole previous runtime state. There may be user created
steering rules and other objects which the driver cannot keep or replay. Today
full reinit brings the device back to a clean initialized state, and that is
intentional.
So the split I have in mind is:
- full runtime state is not preserved across full reinit;
- high level devlink policy/configuration should be applied when the device is
initialized again;
- the command line default should not blindly override a later explicit
userspace eswitch mode selection.
I am not against moving this into devlink core, and I am willing to work on it.
But before I rework the series, I want to make sure we agree on the direction.
As I see it, doing this cleanly needs a devlink state like "registered but
unavailable/in reinit", plus driver annotations for the reinit paths.
If this is not the direction you want, I prefer to know now rather than spend
time on a version that will be rejected anyway.
Mark
>
>>
>> To handle that from devlink I would still need to add some api for the driver
>> to tell devlink "I just reinitialized, apply the default now". but nce I had
>> that driver call , it felt simpler and clearer to let the driver call
>> the helper directly at the points where it knows eswitch mode is safe.
>>
>> I agree that handling all of this inside devlink would be the better option.
>> I just couldn't make it work in a clean way.
>>
>> Mark
>>
>>>
>>>>
>>>> Mark
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> Also, since this knob is only about eswitch mode, I don't think we need to
>>>>>> touch every devlink driver. Drivers that don't implement eswitch_mode_set()
>>>>>> would just ignore it anyway. The follow-up only wires the default into
>>>>>> drivers that actually support changing eswitch mode.
>>>>>>
>>>>>> Mark
>>>>>>
>>>>>>>
>>>>>>>> return 0;
>>>>>>>>
>>>>>>>> err_register:
>>>>>>>> @@ -1538,6 +1554,7 @@ int mlx5_load_one_devl_locked(struct mlx5_core_dev *dev, bool recovery)
>>>>>>>> goto err_attach;
>>>>>>>>
>>>>>>>> mutex_unlock(&dev->intf_state_mutex);
>>>>>>>> + mlx5_devl_apply_default_esw_mode(dev);
>>>>>>>> return 0;
>>>>>>>>
>>>>>>>> err_attach:
>>>>>>>> --
>>>>>>>> 2.44.0
>>>>>>>>
>>>>>>
>>>>
>>
^ permalink raw reply
* Re: [PATCH net-next] Documentation: networking: Add a test plan for ethtool pause validation
From: Maxime Chevallier @ 2026-05-27 6:41 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Andrew Lunn, davem, Eric Dumazet, Paolo Abeni, Simon Horman,
Russell King, Heiner Kallweit, Jonathan Corbet, Shuah Khan,
Oleksij Rempel, Vladimir Oltean, Florian Fainelli,
thomas.petazzoni, netdev, linux-kernel, linux-doc
In-Reply-To: <20260526172447.10ca4b9e@kernel.org>
On 5/27/26 02:24, Jakub Kicinski wrote:
> On Fri, 22 May 2026 19:51:06 +0200 Maxime Chevallier (Netdev
> Foundation) wrote:
>> Documentation/networking/pause_test_plan.rst | 556 +++++++++++++++++++
>
> It'd be great to hear from others but IMHO in the current form this is
> not suitable for Documentation/networking/ We can commit the "knowledge"
> part but enumerating the test cases seems odd for Documentation/.
I think the same, I wasn't sure exactly how/where to send that to, so at
least in this form you can generate that as a nicer to read html form,
but this will likely change given Andrew's feedback
Maxime
^ permalink raw reply
* Re: [PATCH 1/3] ALSA: doc: usb-audio: Add doc for QUIRK_FLAG_IFB_SILENCE_ON_EMPTY
From: Takashi Iwai @ 2026-05-27 5:38 UTC (permalink / raw)
To: Rong Zhang
Cc: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Shuah Khan,
Gordon Chen, linux-sound, linux-doc, linux-kernel
In-Reply-To: <20260527-uac-quirk-get-cur-vol-v1-1-e9362b712e5e@rong.moe>
On Tue, 26 May 2026 19:49:23 +0200,
Rong Zhang wrote:
>
> QUIRK_FLAG_IFB_SILENCE_ON_EMPTY was introduced into usb-audio before
> without appropriate documentation, so add it.
>
> Fixes: a23812004228 ("ALSA: usb-audio: add IFB_SILENCE_ON_EMPTY quirk for Behringer Flow 8")
> Signed-off-by: Rong Zhang <i@rong.moe>
I take this patch now as it's basically irrelevant with your code
change itself.
thanks,
Takashi
^ permalink raw reply
* Re: [PATCH 2/3] ALSA: usb-audio: Add QUIRK_FLAG_MIXER_SKIP_GET_CUR_VOL
From: Takashi Iwai @ 2026-05-27 5:37 UTC (permalink / raw)
To: Rong Zhang
Cc: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Shuah Khan,
Gordon Chen, linux-sound, linux-doc, linux-kernel
In-Reply-To: <20260527-uac-quirk-get-cur-vol-v1-2-e9362b712e5e@rong.moe>
On Tue, 26 May 2026 19:49:24 +0200,
Rong Zhang wrote:
>
> Since commit 86aa1ea1f15c ("ALSA: usb-audio: Do not expose sticky
> mixers"), the UAC mixer core utilizes volume SET_CUR and GET_CUR to
> identify devices with sticky mixers. Unfortunately, even though most
> devices with sticky GET_CUR also have corresponding sticky SET_CUR,
> which I actually met more since the commit had been merged, there is
> also a rare case that some devices may have volume mixers that responds
> to SET_CUR properly but with its GET_CUR stubbed. This cause the sticky
> check to consider the mixer to be sticky and unnecessarily disable it.
>
> Add QUIRK_FLAG_MIXER_SKIP_GET_CUR_VOL to prevent sending GET_CUR to
> mixers by returning -ENXIO early. The error effectively skips the sticky
> check as it's only meaningful when the mixer has some sort of self-
> awareness. Similar to QUIRK_FLAG_GET_SAMPLE_RATE, this should also help
> if some unmet devices can't tolerate volume GET_CUR in other ways.
>
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
> Documentation/sound/alsa-configuration.rst | 4 ++++
> sound/usb/mixer.c | 5 +++++
> sound/usb/quirks.c | 1 +
> sound/usb/usbaudio.h | 6 ++++++
> 4 files changed, 16 insertions(+)
>
> diff --git a/Documentation/sound/alsa-configuration.rst b/Documentation/sound/alsa-configuration.rst
> index 4b30cd63c5a5..bc3bc65c379a 100644
> --- a/Documentation/sound/alsa-configuration.rst
> +++ b/Documentation/sound/alsa-configuration.rst
> @@ -2389,6 +2389,10 @@ quirk_flags
> from snd_usb_handle_sync_urb. Instead fall through and enqueue a
> packet_info containing only size-0 packets, so the OUT ring keeps
> moving (emits silence). Needed by Behringer Flow 8 (1397:050c).
> + * bit 30: ``mixer_skip_get_cur_vol``
> + Skip reading current volume for mixers, as some devices return
> + constant values or errors but otherwise works fine, i.e., setting
> + volume takes desired effect.
>
> This module supports multiple devices, autoprobe and hotplugging.
>
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index d61bde654219..3b745aebb181 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -420,6 +420,11 @@ static int get_cur_ctl_value(struct usb_mixer_elem_info *cval,
> static inline int get_cur_mix_raw(struct usb_mixer_elem_info *cval,
> int channel, int *value)
> {
> + struct snd_usb_audio *chip = cval->head.mixer->chip;
> +
> + if (chip->quirk_flags & QUIRK_FLAG_MIXER_SKIP_GET_CUR_VOL)
> + return -ENXIO;
So this workaround is applied to all mixer controls?
We can put it as a common quirk as you've done, but the question is
how many devices need this, too...
thanks,
Takashi
^ permalink raw reply
* Re: [PATCH net-next 3/3] net/mlx5: Apply devlink default eswitch mode during init
From: Jiri Pirko @ 2026-05-27 5:14 UTC (permalink / raw)
To: Mark Bloch
Cc: Tariq Toukan, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Andrew Lunn, David S. Miller, Jonathan Corbet, Shuah Khan,
Simon Horman, Saeed Mahameed, Leon Romanovsky,
Borislav Petkov (AMD), Andrew Morton, Randy Dunlap,
Thomas Gleixner, Petr Mladek, Peter Zijlstra (Intel), Tejun Heo,
Vlastimil Babka, Feng Tang, Christian Brauner, Dave Hansen,
Dapeng Mi, Kees Cook, Marco Elver, Li RongQing, Eric Biggers,
Paul E. McKenney, linux-doc, linux-kernel, netdev, linux-rdma,
Gal Pressman, Dragos Tatulea, Jiri Pirko, Shay Drori,
Moshe Shemesh
In-Reply-To: <b9105eb7-de56-496e-998f-7c49c660b880@nvidia.com>
Tue, May 26, 2026 at 07:13:46PM +0200, mbloch@nvidia.com wrote:
>
>
>On 26/05/2026 19:23, Jiri Pirko wrote:
>> Tue, May 26, 2026 at 05:03:57PM +0200, mbloch@nvidia.com wrote:
>>>
>>>
>>> On 26/05/2026 17:07, Jiri Pirko wrote:
>>>> Tue, May 26, 2026 at 11:44:46AM +0200, mbloch@nvidia.com wrote:
>>>>>
>>>>>
>>>>> On 26/05/2026 10:44, Jiri Pirko wrote:
>>>>>> Thu, May 21, 2026 at 09:24:34AM +0200, tariqt@nvidia.com wrote:
>>>>>>> From: Mark Bloch <mbloch@nvidia.com>
>>>>>>>
>>>>>>> Apply devlink default eswitch mode for mlx5 devices after successful
>>>>>>> device initialization while holding the devlink instance lock.
>>>>>>>
>>>>>>> At this point the devlink instance is registered and the mlx5 devlink
>>>>>>> operations are available, so the default eswitch mode can be applied to
>>>>>>> the matching PCI devlink handle.
>>>>>>>
>>>>>>> Signed-off-by: Mark Bloch <mbloch@nvidia.com>
>>>>>>> Reviewed-by: Shay Drori <shayd@nvidia.com>
>>>>>>> Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
>>>>>>> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
>>>>>>> ---
>>>>>>> drivers/net/ethernet/mellanox/mlx5/core/main.c | 17 +++++++++++++++++
>>>>>>> 1 file changed, 17 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
>>>>>>> index 0c6e4efe38c8..4528097f3d84 100644
>>>>>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
>>>>>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
>>>>>>> @@ -1391,6 +1391,21 @@ static void mlx5_unload(struct mlx5_core_dev *dev)
>>>>>>> mlx5_free_bfreg(dev, &dev->priv.bfreg);
>>>>>>> }
>>>>>>>
>>>>>>> +static void mlx5_devl_apply_default_esw_mode(struct mlx5_core_dev *dev)
>>>>>>> +{
>>>>>>> + struct devlink *devlink = priv_to_devlink(dev);
>>>>>>> + int err;
>>>>>>> +
>>>>>>> + if (!MLX5_ESWITCH_MANAGER(dev))
>>>>>>> + return;
>>>>>>> +
>>>>>>> + devl_assert_locked(devlink);
>>>>>>> + err = devl_apply_default_esw_mode(devlink);
>>>>>>> + if (err)
>>>>>>> + mlx5_core_warn(dev, "Couldn't apply default eswitch mode, err %d\n",
>>>>>>> + err);
>>>>>>> +}
>>>>>>> +
>>>>>>> int mlx5_init_one_devl_locked(struct mlx5_core_dev *dev)
>>>>>>> {
>>>>>>> bool light_probe = mlx5_dev_is_lightweight(dev);
>>>>>>> @@ -1437,6 +1452,7 @@ int mlx5_init_one_devl_locked(struct mlx5_core_dev *dev)
>>>>>>> mlx5_core_err(dev, "mlx5_hwmon_dev_register failed with error code %d\n", err);
>>>>>>>
>>>>>>> mutex_unlock(&dev->intf_state_mutex);
>>>>>>> + mlx5_devl_apply_default_esw_mode(dev);
>>>>>>
>>>>>> I wonder how we can make this work for all. I mean, other driver would
>>>>>> silently ignore this command like arg, right? Any idea how to make all
>>>>>> drivers follow the arg from very beginning?
>>>>>>
>>>>>
>>>>> I have a follow-up series that adds the call to all drivers which support
>>>>> setting eswitch mode. When going over the other drivers, what I found is
>>>>> that the right point to apply the default is driver specific, drivers
>>>>> I have patch for:
>>>>>
>>>>> 46e16c6d9836 net: Apply devlink esw mode defaults
>>>>> ab4f54102ba9 bnxt_en: Apply devlink default eswitch mode during init
>>>>> b48cce1607bb liquidio: Apply devlink default eswitch mode during init
>>>>> 4ea54b0fe04a ice: Apply devlink default eswitch mode during init
>>>>> b7faddaa1c90 octeontx2-af: Apply devlink default eswitch mode during init
>>>>> 74b0c22c47b9 octeontx2-pf: Apply devlink default eswitch mode during init
>>>>> 5000e4c3d768 nfp: Apply devlink default eswitch mode during init
>>>>> 97a218e95e41 netdevsim: Apply devlink default eswitch mode during init
>>>>>
>>>>> I don't think doing this generically from devlink is realistic. devlink
>>>>> doesn't really know when a given driver is ready to change eswitch mode.
>>>>> Some drivers need SR-IOV state, representor setup, or other init pieces to
>>>>> be ready first, and the locking is not identical across drivers either.
>>>>
>>>>
>>>> Low hanging fruit would be just to call ops->eswitch_mode_set at the end
>>>> of register. Multiple reasons:
>>>>
>>>> 1) end of devl_register is exactly the point userspace is free to issue
>>>> the eswitch mode set. Driver should be ready to handle it.
>>>> 2) all drivers would transparently get this functionality, without
>>>> actually knowing this kernel command line arg ever existed, without
>>>> odd wiring call of related exported function. I prefer that stongly.
>>>> 3) you should add a there warning for the case this arg is passed yet
>>>> the driver does not implement eswitch_mode_set. User should
>>>> get a feedback like this, not silent ignore.
>>>>
>>>> The only loose end is see it the void return of devl_register().
>>>> Multiple ways to handle the possibly failed eswitch_mode_set(). I would
>>>> probably just go for pr_warn, seems to be the most correct.
>>>>
>>>> Make sense?
>>>
>>> I see the point, but I don't think devl_register() (at least not the only place)
>>> is the right place.
>>>
>>> There is a small but important difference between userspace doing
>>> "devlink eswitch set" after register is done, and devlink core calling
>>> eswitch_mode_set() from inside the register flow.
>>>
>>> Some drivers call devlink_register() while holding the device lock.
>>> liquidio is one example. If devlink core calls ops->eswitch_mode_set() from
>>> there, we may start the full eswitch mode change while holding that lock.
>>> That mode change can create representors, register netdevs, take rtnl,
>>> allocate resources, etc. I don't think we want this to become an implicit
>>> side effect of devlink registration.
>>
>> I believe your AI may untagle liquidio locking :)
>
>I didn't try to solve that one with ai. Most drivers were fairly simple
>so I didn't use ai at all. bnxt was the one where I needed a bit of help :)
>
>>
>>
>>>
>>> For mlx5, the placement after intf_state_mutex is also intentional:
>>>
>>> mutex_unlock(&dev->intf_state_mutex);
>>> mlx5_devl_apply_default_esw_mode(dev);
>>>
>>> We can't call it while holding intf_state_mutex because the mode set path
>>> takes it internally, and switchdev mode may also create IB representors.
>>>
>>> Also, devl_register() only covers the first registration. The mlx5 call in
>>> mlx5_load_one_devl_locked() is for reload/fw reset recovery kind of flows.
>>> In those flows devlink is already registered, so devl_register() is not
>>> called again, but the driver state was rebuilt and we may need to apply the
>>> default again.
>>
>> Call it from reload too, right?
>
>Yes, that was my first thought: apply it from devl_register() for the first
>registration and from devlink_reload() after a successful DRIVER_REINIT.
>
>That covers the clean devlink reload path but....(see bellow)
>
>>
>>
>>>
>>> Same for reload, fw reset and pci recovery in general. If the driver tears
>>> down and rebuilds eswitch related state, the place to apply the default is
>>> in that driver's reinit flow, not in devl_register().
>>>
>>> When I went over the other drivers, the right place was not always the same
>>> as devlink registration. I'm not an expert in any of them, so I hope I got
>>> the details right, but for example octeontx2 AF needs sr-iov and the
>>> representor switch state to be initialized first. nfp can do it after
>>> app/vNIC init while the devlink lock is already held. liquidio should do it
>>> only after dropping the PCI device lock.
>>
>> Idk, perhaps do it from devlink_post_register_work of some kind? That
>> would allow you to have the same locking ordering as a userspace cal
>l.
>
>I thought about a workqueue too, it was actually my first idea.
>
>The problem is that then we race with userspace. In the mlx5 version here the
>default is applied while the devlink lock is still held, before userspace can
>come in and issue its own eswitch set. If we defer it to post-register work,
>the devlink instance is already visible and userspace can get there first
>and then we might change the user configuration.
Figure that out and expose to user by setting xa_mark only after the
work is done? This is doable.
>
>Also, the bigger issue for mlx5 is not only initial registration or devlink
>reload. Some recovery paths, pci resume, and fw reset flows rebuild the driver
>state without going through devlink at all. I did not find a clean way for
>devlink core to infer all those points by itself.
If you don't obey current configuration for example in pci resume, it is
bug and you should fix it. All these flows should obey current eswitch
mode configuration.
>
>To handle that from devlink I would still need to add some api for the driver
>to tell devlink "I just reinitialized, apply the default now". but nce I had
>that driver call , it felt simpler and clearer to let the driver call
>the helper directly at the points where it knows eswitch mode is safe.
>
>I agree that handling all of this inside devlink would be the better option.
>I just couldn't make it work in a clean way.
>
>Mark
>
>>
>>>
>>> Mark
>>>
>>>>
>>>>
>>>>>
>>>>> Also, since this knob is only about eswitch mode, I don't think we need to
>>>>> touch every devlink driver. Drivers that don't implement eswitch_mode_set()
>>>>> would just ignore it anyway. The follow-up only wires the default into
>>>>> drivers that actually support changing eswitch mode.
>>>>>
>>>>> Mark
>>>>>
>>>>>>
>>>>>>> return 0;
>>>>>>>
>>>>>>> err_register:
>>>>>>> @@ -1538,6 +1554,7 @@ int mlx5_load_one_devl_locked(struct mlx5_core_dev *dev, bool recovery)
>>>>>>> goto err_attach;
>>>>>>>
>>>>>>> mutex_unlock(&dev->intf_state_mutex);
>>>>>>> + mlx5_devl_apply_default_esw_mode(dev);
>>>>>>> return 0;
>>>>>>>
>>>>>>> err_attach:
>>>>>>> --
>>>>>>> 2.44.0
>>>>>>>
>>>>>
>>>
>
^ permalink raw reply
* Re: [PATCH net-next 0/4] docs: page_pool: tweaks and updates
From: Tariq Toukan @ 2026-05-27 5:11 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, corbet, tariqt,
dtatulea, linux-doc, hawk, ilias.apalodimas
In-Reply-To: <20260526155722.2790742-1-kuba@kernel.org>
On 26/05/2026 18:57, Jakub Kicinski wrote:
> I'm hoping to start feeding our docs into the AI review tools, instead
> of maintaining a separate repo with review prompts. To experiment with
> that we have to refresh the docs a little bit.
>
> This set exclusively focuses on the page pool API. First patch is
> a straightforward fix for information which is now out of date.
> Second one attempts to clarify the NAPI linking requirements.
> Third drops the dedicated section about the stats; the document
> is primarily developer-facing and the stats should require no
> development effort in most cases. Last but not least minor
> API cleanup.
>
> Jakub Kicinski (4):
> docs: net: page_pool: drop reference to removed PP_FLAG_PAGE_FRAG
> docs: clarify page pool NAPI consumer requirement
> docs: page_pool: drop the mention of the legacy stats API
> net: make page_pool_get_stats() void
>
> Documentation/networking/page_pool.rst | 60 +++++++------------
> include/net/page_pool/helpers.h | 2 +-
> .../ethernet/mellanox/mlx5/core/en_stats.c | 3 +-
> net/core/page_pool.c | 10 ++--
> net/core/page_pool_user.c | 3 +-
> 5 files changed, 28 insertions(+), 50 deletions(-)
>
For the series:
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
Thanks.
^ permalink raw reply
* [PATCH 2/2] hwmon: (pmbus/max20860a) Add driver for Analog Devices MAX20860A
From: Pradhan, Sanman @ 2026-05-27 4:54 UTC (permalink / raw)
To: linux-hwmon@vger.kernel.org
Cc: linux@roeck-us.net, robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, corbet@lwn.net, skhan@linuxfoundation.org,
devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, Sanman Pradhan
In-Reply-To: <20260527045409.9092-1-sanman.pradhan@hpe.com>
From: Sanman Pradhan <psanman@juniper.net>
Add a PMBus driver for the Analog Devices MAX20860A step-down DC-DC
switching regulator. The MAX20860A provides monitoring of input/output
voltage, output current, and temperature via the PMBus interface using
linear data format.
During probe, write protection is configured to level 0x20 (all writes
enabled except WRITE_PROTECT itself) to allow runtime configuration
access.
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
Documentation/hwmon/index.rst | 1 +
Documentation/hwmon/max20860a.rst | 60 ++++++++++++++++++++++++++
MAINTAINERS | 8 ++++
drivers/hwmon/pmbus/Kconfig | 9 ++++
drivers/hwmon/pmbus/Makefile | 1 +
drivers/hwmon/pmbus/max20860a.c | 70 +++++++++++++++++++++++++++++++
6 files changed, 149 insertions(+)
create mode 100644 Documentation/hwmon/max20860a.rst
create mode 100644 drivers/hwmon/pmbus/max20860a.c
diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index e880c6ca84f0..ffaacda416e7 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -163,6 +163,7 @@ Hardware Monitoring Kernel Drivers
max20730
max20751
max20830
+ max20860a
max31722
max31730
max31760
diff --git a/Documentation/hwmon/max20860a.rst b/Documentation/hwmon/max20860a.rst
new file mode 100644
index 000000000000..d9bf2ef90734
--- /dev/null
+++ b/Documentation/hwmon/max20860a.rst
@@ -0,0 +1,60 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Kernel driver max20860a
+=======================
+
+Supported chips:
+
+ * Analog Devices MAX20860A
+
+ Prefix: 'max20860a'
+
+ Addresses scanned: -
+
+ Datasheet: https://www.analog.com/en/products/max20860a.html
+
+Author:
+
+ - Sanman Pradhan <psanman@juniper.net>
+
+
+Description
+-----------
+
+This driver supports hardware monitoring for Analog Devices MAX20860A
+Step-Down Switching Regulator with PMBus Interface.
+
+The MAX20860A is a fully integrated step-down DC-DC switching regulator.
+Through the PMBus interface, the device can monitor input/output voltages,
+output current and temperature.
+
+The driver is a client driver to the core PMBus driver. Please see
+Documentation/hwmon/pmbus.rst for details on PMBus client drivers.
+
+Usage Notes
+-----------
+
+This driver does not auto-detect devices. You will have to instantiate
+the devices explicitly.
+
+The driver clears write protection (sets WRITE_PROTECT to 0x20) during
+probe to allow configuration access while keeping the WRITE_PROTECT
+command itself protected.
+
+Sysfs entries
+-------------
+
+================= ========================================
+in1_label "vin"
+in1_input Measured input voltage
+in1_alarm Input voltage alarm
+in2_label "vout1"
+in2_input Measured output voltage
+in2_alarm Output voltage alarm
+curr1_label "iout1"
+curr1_input Measured output current
+curr1_alarm Output current alarm
+temp1_input Measured temperature
+temp1_alarm Chip temperature alarm
+temp2_input Measured temperature (secondary)
+================= ========================================
diff --git a/MAINTAINERS b/MAINTAINERS
index b71acb130395..1d9651947ee3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15688,6 +15688,14 @@ F: Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
F: Documentation/hwmon/max20830.rst
F: drivers/hwmon/pmbus/max20830.c
+MAX20860A HARDWARE MONITOR DRIVER
+M: Sanman Pradhan <psanman@juniper.net>
+L: linux-hwmon@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/hwmon/pmbus/adi,max20860a.yaml
+F: Documentation/hwmon/max20860a.rst
+F: drivers/hwmon/pmbus/max20860a.c
+
MAX2175 SDR TUNER DRIVER
M: Ramesh Shanmugasundaram <rashanmu@gmail.com>
L: linux-media@vger.kernel.org
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index 64f38654f4e7..5825dda75f2c 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -402,6 +402,15 @@ config SENSORS_MAX20830
This driver can also be built as a module. If so, the module will
be called max20830.
+config SENSORS_MAX20860A
+ tristate "Analog Devices MAX20860A"
+ help
+ If you say yes here you get hardware monitoring support for Analog
+ Devices MAX20860A step-down converter.
+
+ This driver can also be built as a module. If so, the module will
+ be called max20860a.
+
config SENSORS_MAX31785
tristate "Maxim MAX31785 and compatibles"
help
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index 1f2c73b71953..ffc05f493213 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_SENSORS_MAX17616) += max17616.o
obj-$(CONFIG_SENSORS_MAX20730) += max20730.o
obj-$(CONFIG_SENSORS_MAX20751) += max20751.o
obj-$(CONFIG_SENSORS_MAX20830) += max20830.o
+obj-$(CONFIG_SENSORS_MAX20860A) += max20860a.o
obj-$(CONFIG_SENSORS_MAX31785) += max31785.o
obj-$(CONFIG_SENSORS_MAX34440) += max34440.o
obj-$(CONFIG_SENSORS_MAX8688) += max8688.o
diff --git a/drivers/hwmon/pmbus/max20860a.c b/drivers/hwmon/pmbus/max20860a.c
new file mode 100644
index 000000000000..9af6888ed07e
--- /dev/null
+++ b/drivers/hwmon/pmbus/max20860a.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Hardware monitoring driver for Analog Devices MAX20860A
+ *
+ * SPDX-FileCopyrightText: Copyright Hewlett Packard Enterprise Development LP
+ */
+
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include "pmbus.h"
+
+/*
+ * Writing 0x20 to WRITE_PROTECT disables all write protection except
+ * for the WRITE_PROTECT command itself, enabling configuration access.
+ */
+#define MAX20860A_WRITE_ENABLE 0x20
+
+static struct pmbus_driver_info max20860a_info = {
+ .pages = 1,
+ .format[PSC_VOLTAGE_IN] = linear,
+ .format[PSC_VOLTAGE_OUT] = linear,
+ .format[PSC_CURRENT_OUT] = linear,
+ .format[PSC_TEMPERATURE] = linear,
+ .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT |
+ PMBUS_HAVE_STATUS_VOUT |
+ PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
+ PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 |
+ PMBUS_HAVE_STATUS_TEMP | PMBUS_HAVE_STATUS_INPUT,
+};
+
+static int max20860a_probe(struct i2c_client *client)
+{
+ int ret;
+
+ ret = i2c_smbus_write_byte_data(client, PMBUS_WRITE_PROTECT,
+ MAX20860A_WRITE_ENABLE);
+ if (ret < 0)
+ return dev_err_probe(&client->dev, ret,
+ "failed to configure write protection\n");
+
+ return pmbus_do_probe(client, &max20860a_info);
+}
+
+static const struct i2c_device_id max20860a_id[] = {
+ {"max20860a"},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, max20860a_id);
+
+static const struct of_device_id max20860a_of_match[] = {
+ { .compatible = "adi,max20860a" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, max20860a_of_match);
+
+static struct i2c_driver max20860a_driver = {
+ .driver = {
+ .name = "max20860a",
+ .of_match_table = max20860a_of_match,
+ },
+ .probe = max20860a_probe,
+ .id_table = max20860a_id,
+};
+
+module_i2c_driver(max20860a_driver);
+
+MODULE_AUTHOR("Sanman Pradhan <psanman@juniper.net>");
+MODULE_DESCRIPTION("PMBus driver for Analog Devices MAX20860A");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("PMBUS");
--
2.34.1
^ permalink raw reply related
* [PATCH 1/2] dt-bindings: hwmon: pmbus: Add Analog Devices MAX20860A
From: Pradhan, Sanman @ 2026-05-27 4:54 UTC (permalink / raw)
To: linux-hwmon@vger.kernel.org
Cc: linux@roeck-us.net, robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, corbet@lwn.net, skhan@linuxfoundation.org,
devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, Sanman Pradhan
In-Reply-To: <20260527045409.9092-1-sanman.pradhan@hpe.com>
From: Sanman Pradhan <psanman@juniper.net>
Add devicetree binding documentation for the Analog Devices MAX20860A
step-down DC-DC switching regulator with PMBus interface.
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
.../bindings/hwmon/pmbus/adi,max20860a.yaml | 42 +++++++++++++++++++
1 file changed, 42 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/adi,max20860a.yaml
diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20860a.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20860a.yaml
new file mode 100644
index 000000000000..d864fef210b1
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20860a.yaml
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/pmbus/adi,max20860a.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices MAX20860A Step-Down Converter
+
+maintainers:
+ - Sanman Pradhan <psanman@juniper.net>
+
+description: |
+ The MAX20860A is a fully integrated step-down DC-DC switching regulator
+ with PMBus interface for monitoring input/output voltage, output current
+ and temperature.
+
+ Datasheet: https://www.analog.com/en/products/max20860a.html
+
+properties:
+ compatible:
+ const: adi,max20860a
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ regulator@40 {
+ compatible = "adi,max20860a";
+ reg = <0x40>;
+ };
+ };
--
2.34.1
^ permalink raw reply related
* [PATCH 0/2] hwmon: (pmbus/max20860a) Add driver for Analog Devices MAX20860A
From: Pradhan, Sanman @ 2026-05-27 4:54 UTC (permalink / raw)
To: linux-hwmon@vger.kernel.org
Cc: linux@roeck-us.net, robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, corbet@lwn.net, skhan@linuxfoundation.org,
devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, Sanman Pradhan
From: Sanman Pradhan <psanman@juniper.net>
Add PMBus hwmon driver and devicetree binding for the Analog Devices
MAX20860A step-down DC-DC switching regulator.
The MAX20860A uses linear data format and provides monitoring of input
voltage, output voltage, output current, and two temperature sensors
via the PMBus interface.
During probe, write protection is configured to level 0x20 to allow
runtime access to configuration and limit registers.
Tested on Juniper Networks hardware. All sensor readings (VIN, VOUT,
IOUT, TEMP1, TEMP2) report correct values in expected ranges, status
registers function correctly, and rapid-poll stress test completes
with zero errors.
Sanman Pradhan (2):
dt-bindings: hwmon: pmbus: Add Analog Devices MAX20860A
hwmon: (pmbus/max20860a) Add driver for Analog Devices MAX20860A
.../bindings/hwmon/pmbus/adi,max20860a.yaml | 42 +++++++++++
Documentation/hwmon/index.rst | 1 +
Documentation/hwmon/max20860a.rst | 60 ++++++++++++++++
MAINTAINERS | 8 +++
drivers/hwmon/pmbus/Kconfig | 9 +++
drivers/hwmon/pmbus/Makefile | 1 +
drivers/hwmon/pmbus/max20860a.c | 70 +++++++++++++++++++
7 files changed, 191 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/adi,max20860a.yaml
create mode 100644 Documentation/hwmon/max20860a.rst
create mode 100644 drivers/hwmon/pmbus/max20860a.c
--
2.34.1
^ permalink raw reply
* Re: [PATCH net-next] Documentation: networking: Add a test plan for ethtool pause validation
From: Andrew Lunn @ 2026-05-27 3:13 UTC (permalink / raw)
To: Maxime Chevallier (Netdev Foundation)
Cc: Jakub Kicinski, davem, Eric Dumazet, Paolo Abeni, Simon Horman,
Russell King, Heiner Kallweit, Jonathan Corbet, Shuah Khan,
Oleksij Rempel, Vladimir Oltean, Florian Fainelli,
thomas.petazzoni, netdev, linux-kernel, linux-doc
In-Reply-To: <20260522175109.198059-1-maxime.chevallier@bootlin.com>
> + These are used to allow the PHY do know what Pause settings the MAC supports, in
s/do/to/
> +Similarly, the link partner can advertise its capabilities through the same
> +bits. These parameters are exchanged through the negotiation process, but can
> +also be enforced locally by disabling **pause autoneg**, thus ignoring the
> +link partner's capabilities.
> +
> +The local resolution of the pause configuration after receiving the link-partner
> +abilities is done according to the following table, from 802.3 Annex 28B.3 :
> +
> ++-------------+--------------+--------------------------+
> +|Local device | Link partner | Pause settings resolution|
> ++------+------+-------+------+-----------+--------------+
> +|Pause | Asym | Pause | Asym | RX | TX |
> ++======+======+=======+======+===========+==============+
> +| 0 | 0 | Any | Any | No | No |
> ++------+------+-------+------+-----------+--------------+
> +| 0 | 1 | 0 | Any | No | No |
> ++------+------+-------+------+-----------+--------------+
> +| 0 | 1 | 1 | 0 | No | No |
> ++------+------+-------+------+-----------+--------------+
> +| 0 | 1 | 1 | 1 | No | Yes |
> ++------+------+-------+------+-----------+--------------+
> +| 1 | 0 | 0 | Any | No | No |
> ++------+------+-------+------+-----------+--------------+
> +| 1 | Any | 1 | Any | Yes | Yes |
> ++------+------+-------+------+-----------+--------------+
> +| 1 | 1 | 0 | 0 | No | No |
> ++------+------+-------+------+-----------+--------------+
> +| 1 | 1 | 0 | 1 | Yes | No |
> ++------+------+-------+------+-----------+--------------+
> +
> +The currently configured and advertised settings can be queried with ::
> +
> + ethtool <iface>
> +
> + Settings for eth0:
> + ...
> + Supported pause frame use: Symmetric Receive-only
> + ...
> + Advertised pause frame use: Symmetric Receive-only
> + ...
> + Link partner advertised pause frame use: Symmetric Receive-only
> +A : Standalone driver testing
> +=============================
> +
> +Requirements : The interface under test must be connected to a link-partner whose
> +interface is admin-up. We don't require the link-partner to be configured in any
> +other specific manner for these tests.
> +
> +A.1 : Sanity Checks
> +~~~~~~~~~~~~~~~~~~~
> +
> +Pause autoneg is set to off::
> +
> + ethtool -A <iface> autoneg off
> +
> +The 'supported' fields retrieved using the ETHTOOL_MSG_LINKMODES_GET includes
> +a "Pause" bit and an "Asym" bit.
> +
> +The ETHTOOL_MSG_PAUSE_GET command returns the currently configured pause modes
> +in the "tx" and "rx" attributes.
> +
> +These parameters must validate against the following truth table :
> +
> + +--------------+-----------------+
> + | linkmodes | pauseparam |
> + +-------+------+--------+--------+
> + | Pause | Asym | rx | tx |
> + +=======+======+========+========+
> + | 0 | 0 | 0 | 0 |
> + +-------+------+--------+--------+
> + | 0 | 1 | 0 | 0 or 1 |
> + +-------+------+--------+--------+
> + | 1 | 0 | rx == tx |
> + +-------+------+--------+--------+
> + | 1 | 1 | 0 or 1 | 0 or 1 |
> + +-------+------+--------+--------+
I think we need more sanity checks here, in order to know if the tests
that follow can be run.
If ETHTOOL_MSG_PAUSE_GET returns -EOPNOTSUPP, it is not a test error,
but all the following tests using forced pause should be skipped,
since it indicates forced pause is not supported.
When pause autoneg is on, and LP values are not reported, we probably
want a warning. I don't think we can say it is an error to report
local values but not LP values. There is probably firmware
implementations which don't make it available to user space. But we
should discourage such behaviour with a warning. And some of the tests
which follow will need to be skipped.
I would suggest looking through the tests and making a list of things
which must be implemented in order to actually perform the test. If
something is missing and returns -EOPNOTSUPP, we need to skip the
test.
> +
> +Test scenario
> +-------------
> +
> +Following the reported value from::
> +
> + ethtool <iface>
> + Settings for <iface>:
> + ...
> + Supported pause frame use: <value>
> +
> +Iterate over all the 4 combinations of rx and tx pause parameters::
> +
> + ethtool -A <iface> autoneg off rx <rx_val> tx <tx_val>
> +
> +The settings must be accepted or rejected, according to the above truth table.
-EOPNOTSUPP should also be consider a pass. Other codes should be a
fail.
Sorry, out of time now, i will continue later.
Andrew
^ permalink raw reply
* Re: [PATCH net-next] Documentation: networking: Add a test plan for ethtool pause validation
From: Andrew Lunn @ 2026-05-27 2:47 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Maxime Chevallier (Netdev Foundation), davem, Eric Dumazet,
Paolo Abeni, Simon Horman, Russell King, Heiner Kallweit,
Jonathan Corbet, Shuah Khan, Oleksij Rempel, Vladimir Oltean,
Florian Fainelli, thomas.petazzoni, netdev, linux-kernel,
linux-doc
In-Reply-To: <20260526172447.10ca4b9e@kernel.org>
On Tue, May 26, 2026 at 05:24:47PM -0700, Jakub Kicinski wrote:
> On Fri, 22 May 2026 19:51:06 +0200 Maxime Chevallier (Netdev
> Foundation) wrote:
> > Documentation/networking/pause_test_plan.rst | 556 +++++++++++++++++++
>
> It'd be great to hear from others but IMHO in the current form this is
> not suitable for Documentation/networking/ We can commit the "knowledge"
> part but enumerating the test cases seems odd for Documentation/.
Sorry, not looked too deeply at the actual content yet.
What i was thinking was a python file, which sphinx can ingest to
produce documentation, and place holders were code would be added to
implement the actual test during the next phase.
This is how i've done testing in the past. I would be the evil one who
thought up the tests and described them in detail using sphinx markup
in a python test template file. After some review they got passed off
to a python developer for implementation. And when they got run and
failed, sometimes the feature developer, the test developer and myself
got together to figure who made the error.
I'm not sure we even need sphinx. What i find important is that the
test is documented. What kAPI calls should be made with what
parameters. What results we are expected and why? So that when a test
fails, a developer has the information they need to fix their
code. The Why? is important, and often missing from the kernel tests.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 00/10] docs: net: updates for old and cobwebbed docs
From: Randy Dunlap @ 2026-05-27 2:39 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, corbet,
vladimir.oltean, willemb, sdf.kernel, ecree.xilinx,
jesse.brandeburg, linux-doc
In-Reply-To: <20260526181513.0393ca46@kernel.org>
On 5/26/26 6:15 PM, Jakub Kicinski wrote:
> On Tue, 26 May 2026 17:51:45 -0700 Randy Dunlap wrote:
>> It's in today's linux-next (20260526), line 1950 of
>> include/linux/netdevice.h:
>
> linux-next is useful, but we try not to merge broken code, not just
> notice it after the fact. So it'd be great if the fix made its way
> to Linus and therefore propagate to subsystem -next trees, not just
> linux-next.
OK. I don't quite know what you mean by that, but I just checked the
current net-next tree and it's still there at line 1950.
--
~Randy
^ permalink raw reply
* Re: [PATCH v2 0/4] mm/zswap: Implement per-cgroup proactive writeback
From: Hao Jia @ 2026-05-27 2:39 UTC (permalink / raw)
To: Yosry Ahmed
Cc: Andrew Morton, tj, hannes, shakeel.butt, mhocko, mkoutny, nphamcs,
chengming.zhou, muchun.song, roman.gushchin, cgroups, linux-mm,
linux-kernel, linux-doc, Hao Jia
In-Reply-To: <CAO9r8zO1+brQroYufMZ2K=ZH_PBBpzYPsdYm-DT3K2GxoKJs9A@mail.gmail.com>
On 2026/5/27 02:55, Yosry Ahmed wrote:
> On Tue, May 26, 2026 at 4:56 AM Hao Jia <jiahao.kernel@gmail.com> wrote:
>>
>>
>>
>> On 2026/5/26 03:24, Andrew Morton wrote:
>>> On Mon, 25 May 2026 20:22:38 +0800 Hao Jia <jiahao.kernel@gmail.com> wrote:
>>>
>>>> Zswap currently writes back pages to backing swap reactively, triggered
>>>> either by the shrinker or by the pool reaching its size limit. Although
>>>> proactive memory reclaim can automatically write back a portion of zswap
>>>> pages via the shrinker, it cannot explicitly control the amount of
>>>> writeback for a specific memory cgroup. Moreover, proactive memory reclaim
>>>> may not always be triggered during a steady state.
>>>>
>>>> In certain scenarios, it is desirable to trigger writeback in advance to
>>>> free up memory. For example, users may want to prepare for an upcoming
>>>> memory-intensive workload by flushing cold memory to the backing storage
>>>> when the system is relatively idle.
>>>>
>>>> This patch series introduces a "zswap_writeback_only" key to memory.reclaim
>>>> cgroup interface, allowing users to proactively write back cold compressed
>>>> pages from zswap to the backing swap device. When specified, this key
>>>> bypasses standard memory reclaim and exclusively performs proactive zswap
>>>> writeback up to the requested budget. If omitted, the default reclaim
>>>> behavior remains unchanged.
>>>
>>> Thanks. AI review found a few things to complain about, one of them
>>> described as "preexisting".
>>>
>>
>> Thanks Andrew. I have replied to the AI's review comments in a separate
>> email and posted v3.
>> https://lore.kernel.org/all/20260526114601.67041-1-jiahao.kernel@gmail.com
>
> Generally speaking, please give time for reviewers to take a look
> before sending a new version. Less than a day is usually too fast
> (unless you're iterating super fast with the reviewers). Review
> feedback does not have to be addressed immediately, usually wait for a
> bit to collect as much feedback as possible before spinning a new
> version.
>
Thanks for the advice, Yosry. Got it.
> I will take a look at v3 soon, thank you.
Appreciate you taking a look at v3.
Thakns,
Hao
^ permalink raw reply
* Re: [PATCH 1/3] net: Remove support for AIO on sockets
From: Jakub Kicinski @ 2026-05-27 1:40 UTC (permalink / raw)
To: Demi Marie Obenour via B4 Relay
Cc: demiobenour, Herbert Xu, David S. Miller, Eric Dumazet,
Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn, Jens Axboe,
Simon Horman, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Jonathan Corbet, Shuah Khan, Eric Biggers,
Ard Biesheuvel, linux-crypto, linux-kernel, io-uring, netdev,
linux-perf-users, linux-doc
In-Reply-To: <20260523-af-alg-harden-v1-1-c76755c3a5c5@gmail.com>
On Sat, 23 May 2026 15:43:02 -0400 Demi Marie Obenour via B4 Relay
wrote:
> The only user of msg->msg_iocb was AF_ALG, but that's deprecated.
> It can be removed entirely at the cost of only supporting synchronous
> operations. This doesn't break userspace, which will silently block
> (for a bounded amount of time) in io_submit instead of operating
> asynchronously.
>
> This also makes struct msghdr smaller, helping every other caller of
> sendmsg().
Acked-by: Jakub Kicinski <kuba@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 2/7] seg6: add End.M.GTP4.E behavior
From: Andrea Mayer @ 2026-05-27 1:09 UTC (permalink / raw)
To: Yuya Kusakabe
Cc: David S. Miller, Eric Dumazet, David Ahern, Jakub Kicinski,
Paolo Abeni, Simon Horman, Justin Iurman, Shuah Khan,
Jonathan Corbet, Shuah Khan, linux-kernel, netdev,
linux-kselftest, linux-doc, stefano.salsano, ahabdels,
Andrea Mayer
In-Reply-To: <20260505-seg6-mobile-v2-2-9e8022bdfdb6@gmail.com>
On Tue, 05 May 2026 01:30:12 +0900
Yuya Kusakabe <yuya.kusakabe@gmail.com> wrote:
Hi Yuya,
I do not repeat below the points already addressed in the cover letter
thread (drop reason granularity/prep series, SEG6_LOCAL_OIF/VRF removal,
selftest C helper) and in my reply to patch 1 (SRH validation semantics,
HMAC handling).
> Add the End.M.GTP4.E behavior (RFC 9433 Section 6.6), which
> decapsulates an inbound SRv6 packet and re-encapsulates the inner
> T-PDU in IPv4/UDP/GTP-U toward a legacy IPv4 receiver, including an
> optional PDU Session extension header.
>
> The SID layout per RFC 9433 Section 6.6 Figure 9 is:
>
> |<-- locator -->|<-- IPv4 DA -->|<-- Args.Mob.Session -->|<-pad->|
>
> The IPv4 destination is recovered from the SID; the IPv4 source is
> recovered from the inbound IPv6 source by overlaying the configured
> src_addr template with the v4_mask_len bits at bit offset
> v6_src_prefix_len (default 64) of the IPv6 SA, per RFC 9433
> Section 6.6 Figure 10. Args.Mob.Session is the 40-bit
> field defined in RFC 9433 Section 6.1: QFI(6) | R(1) | U(1) | PDU
> Session ID(32).
>
> DSCP / ECN / Hop Limit -> TTL are propagated from the inbound IPv6
> outer to the new IPv4 outer per RFC 6040. GSO packets that would
> not fit the egress route MTU after adding the outer headers are
> rejected explicitly because the GSO segmenter cannot fix this up
> after the network protocol has changed from IPv6 to IPv4.
>
> When net.netfilter.nf_hooks_lwtunnel=1, the inner T-PDU traverses
> NF_INET_PRE_ROUTING between the SRv6 strip and the GTP-U push,
> mirroring End.DX4 / End.DX6.
>
> Add four drop reasons used here to dropreason-core.h:
> SEG6_MOBILE_BAD_SID, SEG6_MOBILE_BAD_GTPU, SEG6_MOBILE_BAD_INNER,
> and SEG6_MOBILE_MTU_EXCEEDED.
>
> Configuration:
>
> ip -6 route add 2001:db8::/32 \
> encap seg6local action End.M.GTP4.E \
> src 2001:db8:2::1 v4_mask_len 32 \
> dev <dev>
There are three issues with the End.M.GTP4.E UAPI behavior:
(1) In the example, v4_mask_len = 32 and the src value is required but not
used.
seg6_mobile_v4_sa() builds the output IPv4 SA from two parts:
sa_bits = min(v4_mask_len, 32)
top sa_bits from the inbound IPv6 SA
low (32 - sa_bits) from src
With sa_bits = 32, all 32 bits of the IPv4 SA come from the inbound IPv6 SA
at bit offset v6_src_prefix_len, and no bit of src is part of the IPv4 SA.
The src "2001:db8:2::1" in the example is read and never used.
A required UAPI attribute that has no effect on the output cannot be
relaxed after merge, so this needs to be revised. Either v4_mask_len = 32
should not require src, or src should not be a required attribute.
(2) With v4_mask_len < 32, the low bits of the IPv4 SA that the inbound
packet does not carry come from src.
Configuration:
src 2001:db8::a00:05cd:0:0 v4_mask_len 24
v6_src_prefix_len 64 (default, as this attr is optional)
seg6_mobile_v4_sa() reads a 32-bit window from src at bit offset 64
(= v6_src_prefix_len), then replaces the top 24 bits with bits read from
the inbound IPv6 SA at the same offset.
src bytes: 20 01 0d b8 00 00 00 00 0a 00 05 cd 00 00 00 00
^^^^^^^^^^^
offset 64..95: 0a 00 05 cd
Inbound 2001:db8:1::0a00:05ff:0:0 bytes:
20 01 0d b8 00 01 00 00 0a 00 05 ff 00 00 00 00
^^^^^^^^
offset 64..87: 0a 00 05
IPv4 SA out = (top 24 bits from inbound) | (low 8 bits from src)
= 0a 00 05 | cd
= 10.0.5.205
The code only checks src is a 16-byte IPv6 address. For the same inbound
and v4_mask_len = 24, all of the following src values produce IPv4 SA out
10.0.5.205:
src 2001:db8::a00:05cd:0:0 (offset 64..95: 10.0.5.205)
src 2001:db8::110e:0bcd:0:0 (offset 64..95: 17.14.11.205)
src dead:beef::ffff:eecd:1234:5678 (offset 64..95: 255.255.238.205)
The 32-bit windows encode three different IPv4 prefixes (10.0.5, 17.14.11,
255.255.238). Only the shared last byte (.205, "cd") reaches the IPv4 SA
out. The prefix bits are overwritten by the inbound. The other 12 bytes of
src outside the 32-bit window are accepted but discarded.
A required UAPI attribute that consumes so few bits and accepts arbitrary
values for the rest cannot be tightened after merge, so this needs to be
revised.
(3) With v4_mask_len < 32, the low bits of the IPv4 DA that the SID does
not carry are set to zero.
seg6_mobile_parse_gtp4_sid() takes v4_mask_len bits from the IPv6 DA at
offset locator_bits:
IPv4 DA out = top v4_mask_len bits from IPv6 DA, low bits zero
With v4_mask_len = 24 the IPv4 DA out has the form X.Y.Z.0 and with
v4_mask_len = 16 it has the form X.Y.0.0. These look like network
addresses, not host addresses. RFC 9433 Section 6.6 does not specify how
the IPv4 DA is recovered when v4_mask_len < 32.
Together with the IPv4 SA behavior in (2), does it make sense to support
v4_mask_len values other than 32?
All three above are design issues in the End.M.GTP4.E UAPI.
They have to be addressed before this can land.
>
> Link: https://www.rfc-editor.org/rfc/rfc9433.html#section-6.6
> Link: https://www.rfc-editor.org/rfc/rfc6040
> Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
> ---
> include/net/dropreason-core.h | 28 +
> include/uapi/linux/seg6_local.h | 6 +
> net/ipv6/seg6_local.c | 711 +++++++++++++++++++++
> tools/testing/selftests/net/Makefile | 1 +
> .../selftests/net/srv6_end_m_gtp4_e_test.sh | 486 ++++++++++++++
> 5 files changed, 1232 insertions(+)
>
> + [snip]
> diff --git a/include/uapi/linux/seg6_local.h b/include/uapi/linux/seg6_local.h
> index 45386fdfa821..b42cb526bb81 100644
> --- a/include/uapi/linux/seg6_local.h
> +++ b/include/uapi/linux/seg6_local.h
> @@ -29,6 +29,10 @@ enum {
> SEG6_LOCAL_VRFTABLE,
> SEG6_LOCAL_COUNTERS,
> SEG6_LOCAL_FLAVORS,
> + SEG6_LOCAL_MOBILE_SRC_ADDR,
> + [snip]
> +/* Compose the IPv4 source address per RFC 9433 Section 6.6 Figure 10:
> + * the @v4_mask_len high bits are recovered from the inbound IPv6 SA at
> + * bit offset @v6_src_prefix_len (or /64 when 0); the remaining low bits
> + * come from @src_template at the same offset.
> + */
> +static __be32 seg6_mobile_v4_sa(const struct in6_addr *ip6_sa,
> + const struct in6_addr *src_template,
> + u8 v4_mask_len, u8 v6_src_prefix_len)
> +{
> + u8 p_bits = v6_src_prefix_len ? : SEG6_MOBILE_V6_SRC_PREFIX_LEN_DEFAULT;
> + u8 sa_bits = min_t(u8, v4_mask_len, 32);
> + u64 template_field, sa_field, mask;
> +
> + if ((unsigned int)p_bits + 32 > 128)
> + return 0;
This check cannot trigger at runtime.
seg6_mobile_v4_validate() already rejects v6_src_prefix_len > 96 at install
time, and the default (64) leaves p_bits at most 96.
If kept for defense in depth, or if you plan to reuse this helper
elsewhere, the silent return of 0 produces an outgoing IPv4 packet with SA
= 0.0.0.0 without any error.
Nit: the (unsigned int) cast on p_bits can be removed.
> + [snip]
> +/* Return the bit length of the routing prefix that delivered @skb to
> + * the current End.* handler (i.e. the prefix length of the matched FIB
> + * entry). This is the locator length used to position v4DA /
> + * Args.Mob.Session inside the SID per RFC 9433 Section 6.6.
> + */
> +static unsigned int seg6_mobile_skb_prefix_bits(const struct sk_buff *skb)
> +{
> + struct dst_entry *dst = skb_dst(skb);
> + struct rt6_info *rt;
> + struct fib6_info *fib6;
> + u8 plen = 128;
> +
> + /* container_of() below requires an IPv6 dst. */
> + if (!dst || dst->ops->family != AF_INET6)
> + return 128;
> +
> + rt = container_of(dst, struct rt6_info, dst);
> + rcu_read_lock();
> + fib6 = rcu_dereference(rt->from);
> + if (fib6)
> + plen = fib6->fib6_dst.plen;
> + rcu_read_unlock();
> +
> + return plen;
> +}
seg6_mobile_skb_prefix_bits() reads the matched route prefix length from
the FIB on every packet. The same value is fc_dst_len in struct
fib6_config, accessible from the cfg argument of the build_state callback.
Caching it in slwt->mobile_info at validate would let the data path read
minfo->locator_bits directly.
That avoids the rcu_read_lock/rcu_dereference per packet, the
container_of(dst, struct rt6_info, dst) cast, and the 128 fallback. It also
removes one dependency on skb_dst at runtime.
The operator cannot configure the IPv4 DA to start at a position different
from where the route prefix ends. Is this intentional?
> + [snip]
> +
> +/* Push a GTP-U header on top of @skb. When @pdu_type_set is true the
> + * GTPv1 long header (with the EH bit set) is followed by a 4-byte
> + * PDU Session extension header (3GPP TS 38.415); @pdu_type selects
> + * the PDU Type field (0 for downlink, 1 for uplink, 2..15 reserved).
> + * When @pdu_type_set is false the GTPv1 short header is emitted with
> + * no PDU Session Container, regardless of @qfi.
> + */
> +static int seg6_mobile_push_gtpu(struct sk_buff *skb, u32 teid, u8 qfi,
> + u8 pdu_type, bool pdu_type_set)
> +{
> + struct gtp1_header_long *gtphl;
> + struct gtp1_header *gtph;
> + struct seg6_mobile_pdu_session_ext *pdu_session;
> +
> + if (!pdu_type_set) {
> + if (skb_cow_head(skb, sizeof(*gtph)))
> + return -ENOMEM;
> +
> + gtph = (struct gtp1_header *)skb_push(skb, sizeof(*gtph));
> + gtph->flags = SEG6_MOBILE_GTP1U_FLAGS_BASE;
> + gtph->type = GTP_TPDU;
> + gtph->length = htons(skb->len - sizeof(*gtph));
> + gtph->tid = htonl(teid);
> + return 0;
> + }
> +
> + if (skb_cow_head(skb, sizeof(*gtphl) + sizeof(*pdu_session)))
> + return -ENOMEM;
> +
> + pdu_session = skb_push(skb, sizeof(*pdu_session));
> + pdu_session->ext_len = 1;
> + pdu_session->pdu_type_spare = (pdu_type & 0xf) << 4;
> + pdu_session->spare_qfi = qfi & SEG6_MOBILE_PDU_SESSION_QFI_MASK;
> + pdu_session->next_ext = 0;
> +
> + gtphl = (struct gtp1_header_long *)skb_push(skb, sizeof(*gtphl));
> + gtphl->flags = SEG6_MOBILE_GTP1U_FLAGS_BASE | GTP1_F_EXTHDR;
> + gtphl->type = GTP_TPDU;
> + gtphl->length = htons(skb->len - sizeof(struct gtp1_header));
> + gtphl->tid = htonl(teid);
> + gtphl->seq = 0;
> + gtphl->npdu = 0;
> + gtphl->next = SEG6_MOBILE_PDU_SESSION_NH;
> +
> + return 0;
> +}
Nit: skb_push() returns void * so the explicit casts on two of the three
calls are unnecessary.
> +
> +/* Per-skb context preserved across the NF_INET_PRE_ROUTING hook on
> + * the inner T-PDU exposed by End.M.GTP4.E. After the outer SRv6 has
> + * been popped the inner IP is briefly visible to netfilter; the
> + * finish half then needs the synthesised IPv4 outer fields and the
> + * GTP-U identifiers to rebuild the packet.
> + */
> +struct seg6_mobile_gtp4_e_cb {
> + __be32 v4_da;
> + __be32 v4_sa;
> + u32 teid;
> + u8 qfi;
> + u8 outer_tclass;
> + u8 outer_hoplimit;
> + u8 pdu_type;
> + bool pdu_type_set;
> +};
> +
> +#define SEG6_MOBILE_GTP4_E_CB(skb) \
> + ((struct seg6_mobile_gtp4_e_cb *)((skb)->cb))
> +
> +static int input_action_end_m_gtp4_e_finish(struct net *net,
> + struct sock *sk,
> + struct sk_buff *skb)
> +{
> + struct seg6_mobile_gtp4_e_cb cb = *SEG6_MOBILE_GTP4_E_CB(skb);
> + struct dst_entry *orig_dst = skb_dst(skb);
> + enum skb_drop_reason reason = SKB_DROP_REASON_SEG6_MOBILE_NOMEM;
> + struct seg6_local_lwt *slwt;
> + struct iphdr *iph;
> + struct udphdr *uh;
> +
> + slwt = seg6_local_lwtunnel(orig_dst->lwtstate);
orig_dst may be NULL or stale. This is an NF_HOOK finish callback, and
Netfilter processing during NF_INET_PRE_ROUTING can drop or replace the
skb dst. skb->cb is not guaranteed to be untouched (IPCB/IP6CB aliases it).
Noting this here only to track the issue we already discussed in the cover
letter thread. It is pre-existing, not introduced by your patchset.
> +
> + /* Reject GSO packets that would not fit the egress IPv4 path after
> + * adding our outer headers; the GSO segmenter cannot fix this up
> + * once we have changed the network protocol from IPv6 to IPv4.
> + * The MTU check uses the inbound IPv6 dst as a conservative bound
> + * (the outbound IPv4 route is not known until ip_route_output_key()
> + * below); skip the check entirely when no MTU is known on the
> + * current dst.
> + */
> + if (skb_is_gso(skb)) {
> + unsigned int ovhd = sizeof(*iph) + sizeof(*uh) +
> + sizeof(struct gtp1_header_long) +
> + sizeof(struct seg6_mobile_pdu_session_ext);
> + unsigned int mtu = dst_mtu(skb_dst(skb));
> +
> + if (mtu && (mtu <= ovhd ||
> + !skb_gso_validate_network_len(skb, mtu - ovhd))) {
> + reason = SKB_DROP_REASON_SEG6_MOBILE_MTU_EXCEEDED;
> + goto drop;
> + }
> + }
> +
> + /* Reserve worst-case headroom for the entire outer chain we are about
> + * to push: IPv4 + UDP + GTP-U long header + PDU Session extension.
> + * Subsequent skb_cow_head() calls inside seg6_mobile_push_gtpu() then
> + * become no-ops.
> + */
> + if (skb_cow_head(skb,
> + sizeof(*iph) + sizeof(*uh) +
> + sizeof(struct gtp1_header_long) +
> + sizeof(struct seg6_mobile_pdu_session_ext)))
Same four-sizeof sum as ovhd above. Move ovhd to function scope to avoid
the repetition.
> + goto drop;
input_action_end_m_gtp4_e_finish() never calls iptunnel_handle_offloads(),
so GSO of the outer UDP tunnel cannot operate correctly. The call should
go between skb_cow_head() and seg6_mobile_push_gtpu().
iptunnel_handle_offloads() snapshots network_header, transport_header,
and mac_header into the inner_* fields via skb_reset_inner_headers(),
and marks the skb as encapsulated. For the snapshot to capture the
inner correctly:
i) transport_header has to be advanced past the inner IP header (the
outer pull left it at the same position as network_header);
ii) mac_header has to be reset to the inner via skb_reset_mac_header()
(the outer pull did not touch it, so it still points at the
original outer link-layer position).
The inner protocol is not snapshotted by skb_reset_inner_headers() and
needs to be recorded separately, e.g. via skb_set_inner_protocol(skb,
htons(ETH_P_IP/IPV6)) after iptunnel_handle_offloads().
The fix needs end-to-end testing to confirm it works correctly.
> +
> + if (seg6_mobile_push_gtpu(skb, cb.teid, cb.qfi, cb.pdu_type,
> + cb.pdu_type_set))
> + goto drop;
The caller already reserves worst-case headroom, so the skb_cow_head()
calls inside seg6_mobile_push_gtpu() are always no-ops. Could they be
removed?
> +
> + uh = skb_push(skb, sizeof(*uh));
> + skb_reset_transport_header(skb);
> + uh->source = htons(GTP1U_PORT);
> + uh->dest = htons(GTP1U_PORT);
Does this mirror the drivers/net/gtp.c data-path convention
(src == dst == 2152)?
> + uh->len = htons(skb->len);
> + uh->check = 0; /* IPv4 UDP checksum optional; offload may set later */
> +
> + iph = skb_push(skb, sizeof(*iph));
> + skb_reset_network_header(skb);
> + iph->version = 4;
> + iph->ihl = sizeof(*iph) >> 2;
> + iph->tos = cb.outer_tclass;
> + iph->tot_len = htons(skb->len);
> + iph->frag_off = htons(IP_DF);
> + iph->ttl = cb.outer_hoplimit;
> + iph->protocol = IPPROTO_UDP;
> + iph->saddr = cb.v4_sa;
> + iph->daddr = cb.v4_da;
> + __ip_select_ident(net, iph, 1);
> + iph->check = 0;
> + iph->check = ip_fast_csum((unsigned char *)iph, iph->ihl);
> +
> + skb->protocol = htons(ETH_P_IP);
> + nf_reset_ct(skb);
> + skb_dst_drop(skb);
> +
> + /* The IPv4 outer is constructed locally from the SRv6 SID and the
> + * inbound IPv6 outer (RFC 9433 Section 6.6 Figure 9 / 10). The
> + * IPv4 source address is synthesised, so it is not guaranteed to
> + * have a reverse route; ip_route_input() + dst_input() with
> + * rp_filter enabled would drop the packet. Use an output route
> + * lookup with FLOWI_FLAG_ANYSRC and emit via dst_output(): the
> + * packet is locally originated from the IPv4 stack's perspective
> + * and traverses NF_INET_LOCAL_OUT.
> + */
The comment says the packet "traverses NF_INET_LOCAL_OUT", but dst_output()
goes through NF_INET_POST_ROUTING only. ip_local_out() is the one that
traverses LOCAL_OUT first.
This is also the only behavior in seg6_local.c that uses the output path,
bypassing NF_INET_FORWARD entirely. All the other behaviors use
ip_route_input() or seg6_lookup_any_nexthop() with dst_input(), including
the four other MUP behaviors in this patchset (End.M.GTP6.E, End.M.GTP6.D,
End.M.GTP6.D.Di, H.M.GTP4.D), so iptables/nftables FORWARD rules see the
traffic.
IMHO, gateways that do protocol transformation can set rp_filter=0. Using
ip_route_input() + dst_input() and documenting the rp_filter requirement is
preferable to silently bypassing the FORWARD chain.
> + {
The anonymous { } scope block should be avoided. rt and fl4 should be
declared at the function top. The same pattern appears in other patches
of this series.
> + struct rtable *rt;
> + struct flowi4 fl4 = {
> + .daddr = cb.v4_da,
> + .saddr = cb.v4_sa,
> + .flowi4_proto = IPPROTO_UDP,
> + .flowi4_flags = FLOWI_FLAG_ANYSRC,
> + .flowi4_oif = slwt->oif,
> + };
> +
> + rt = ip_route_output_key(net, &fl4);
> + if (IS_ERR(rt)) {
> + reason = SKB_DROP_REASON_IP_OUTNOROUTES;
> + goto drop;
> + }
> + skb_dst_set(skb, &rt->dst);
> + return dst_output(net, NULL, skb);
> + }
> +
> +drop:
> + kfree_skb_reason(skb, reason);
> + return -EINVAL;
> +}
> +
> +/* RFC 9433 Section 6.6 -- End.M.GTP4.E
> + * Receives an SRv6 packet and re-encapsulates the inner payload in
> + * IPv4/UDP/GTP-U (with an optional PDU Session extension header)
> + * toward a legacy IPv4 gNB.
> + *
> + * When net.netfilter.nf_hooks_lwtunnel=1 the inner T-PDU is exposed
> + * to NF_INET_PRE_ROUTING after the outer IPv6/SRH is popped and
> + * before the GTP-U header is pushed. This lets nftables / conntrack
> + * apply policy on the inner 5-tuple at the SR Gateway.
> + */
> +static int input_action_end_m_gtp4_e(struct sk_buff *skb,
> + struct seg6_local_lwt *slwt)
> +{
> + u8 qfi, outer_tclass, outer_hoplimit;
> + unsigned int outer_len;
> + struct ipv6_sr_hdr *srh;
> + struct in6_addr ip6_sa;
> + struct seg6_mobile_gtp4_e_cb *cb;
> + bool no_srh = false;
> + int inner_nfproto;
> + __be16 frag_off;
> + __be32 v4_da, v4_sa;
> + struct ipv6hdr *ip6h;
> + u64 args_mob;
> + u32 teid;
> + u8 nh;
> + int off;
> + const struct seg6_mobile_info *minfo = &slwt->mobile_info;
> + enum skb_drop_reason reason = SKB_DROP_REASON_SEG6_MOBILE_BAD_SID;
Variable declarations are not in reverse Christmas tree order. Same issue
in the other functions introduced by this patch.
> +
> + BUILD_BUG_ON(sizeof(struct seg6_mobile_gtp4_e_cb) >
> + sizeof_field(struct sk_buff, cb));
> +
> + if (!pskb_may_pull(skb, sizeof(*ip6h))) {
> + reason = SKB_DROP_REASON_SEG6_MOBILE_BAD_INNER;
> + goto drop;
> + }
This pskb_may_pull pulls the OUTER IPv6 header, not the inner T-PDU.
BAD_INNER is the wrong drop reason here. BAD_SID or a general "packet too
short" would be more accurate.
> +
> + ip6h = ipv6_hdr(skb);
> + ip6_sa = ip6h->saddr;
> +
> + /* Snapshot fields read from the IPv6 outer before any pskb_may_pull()
> + * call below: seg6_mobile_get_validated_srh() invokes pskb_may_pull()
> + * internally and may reallocate skb->head, invalidating @ip6h. RFC
> + * 6040 outer-to-outer propagation: DSCP+ECN to TOS, HopLimit to TTL.
> + */
> + outer_tclass = ipv6_get_dsfield(ip6h);
> + outer_hoplimit = ip6h->hop_limit;
RFC 6040 Section 1.1 scopes the document to ECN field processing in
IP-in-IP tunnels. The patch cites it for DSCP and Hop Limit/TTL on a
protocol conversion (the IPv6 outer is popped, not encapsulated). Could
you clarify the RFC 6040 reference?
> +
> + if (!seg6_mobile_parse_gtp4_sid(&ip6h->daddr,
> + seg6_mobile_skb_prefix_bits(skb),
> + minfo->v4_mask_len,
> + &v4_da, &args_mob))
> + goto drop;
See issue (3) above on the IPv4 DA zero-fill behavior of
seg6_mobile_parse_gtp4_sid() when v4_mask_len < 32.
> +
> + /* Validate SRH (if present) per RFC 9433 Section 6.6 S01-S04: SL
> + * must be 0. HMAC is enforced when the SRH is present and the
> + * kernel was built with CONFIG_IPV6_SEG6_HMAC.
> + */
> + srh = seg6_mobile_get_validated_srh(skb, &no_srh);
> + if (!srh && !no_srh) {
> + reason = SKB_DROP_REASON_SEG6_MOBILE_INVALID_SRH_SL;
> + goto drop;
> + }
Same seg6_mobile_get_validated_srh() bug described in my reply to patch 1.
> + if (srh && srh->segments_left != 0) {
> + reason = SKB_DROP_REASON_SEG6_MOBILE_INVALID_SRH_SL;
> + goto drop;
> + }
> +
> + /* @ip6h may have been invalidated by pskb_may_pull() inside
> + * seg6_mobile_get_validated_srh(); re-evaluate before any further
> + * dereference.
> + */
> + ip6h = ipv6_hdr(skb);
> +
> + teid = seg6_mobile_teid_from_args(args_mob);
> + qfi = seg6_mobile_qfi_from_args(args_mob);
> +
> + v4_sa = seg6_mobile_v4_sa(&ip6_sa, &minfo->src_addr, minfo->v4_mask_len,
> + minfo->v6_src_prefix_len);
> +
> + /* RFC 9433 Section 6.6 upper-layer S02 mandates "Pop the IPv6
> + * header and all its extension headers". Use ipv6_skip_exthdr()
> + * so HBH / Routing / Dest-Opts / Fragment headers are accounted
> + * for in addition to the SRH. The terminal next-header value
> + * also selects NFPROTO_IPV4 / NFPROTO_IPV6 for the
> + * NF_INET_PRE_ROUTING hook below.
> + */
> + nh = ip6h->nexthdr;
> + off = ipv6_skip_exthdr(skb, sizeof(*ip6h), &nh, &frag_off);
> + if (off < 0) {
> + reason = SKB_DROP_REASON_SEG6_MOBILE_BAD_INNER;
> + goto drop;
> + }
> + outer_len = off;
Same BAD_INNER misuse: ipv6_skip_exthdr() is parsing the OUTER IPv6
extension headers to find where the inner T-PDU starts. If it fails, the
outer extension header chain is malformed or truncated, not the inner
payload.
frag_off is not checked after ipv6_skip_exthdr(). If the outer IPv6 packet
carries a Fragment header, the code continues to parse what follows as a
complete inner T-PDU, but the payload may be truncated or absent. Adding
"if (frag_off) goto drop;" after the ipv6_skip_exthdr() call would handle
this.
> +
> + switch (nh) {
> + case IPPROTO_IPIP:
> + inner_nfproto = NFPROTO_IPV4;
> + break;
> + case IPPROTO_IPV6:
> + inner_nfproto = NFPROTO_IPV6;
> + break;
> + default:
> + inner_nfproto = -1;
> + break;
> + }
> +
> + /* For inner IP traffic that may traverse NF_INET_PRE_ROUTING below,
> + * pull the full inner IP header into the linear area so a netfilter
> + * hook reading skb_transport_header() does not access stale data.
> + * Non-IP inner is forwarded as-is via the GTP-U T-PDU payload.
> + */
> + if (!pskb_may_pull(skb, outer_len + ((inner_nfproto == NFPROTO_IPV4) ?
> + sizeof(struct iphdr) :
> + (inner_nfproto == NFPROTO_IPV6) ?
> + sizeof(struct ipv6hdr) : 0))) {
> + reason = SKB_DROP_REASON_SEG6_MOBILE_BAD_INNER;
> + goto drop;
> + }
The inner_nfproto-based size selection appears several times:
pskb_may_pull, skb->protocol, and skb_set_transport_header. Computing a
local inner_hdr_len once inside the switch would replace all three. Same
pattern in every other behavior.
> + [snip]
> +static int parse_nla_mobile_pdu_type(struct nlattr **attrs,
> + struct seg6_local_lwt *slwt,
> + struct netlink_ext_ack *extack)
> +{
> + u8 t = nla_get_u8(attrs[SEG6_LOCAL_MOBILE_PDU_TYPE]);
> +
> + /* 3GPP TS 38.415: PDU Type is a 4-bit field. */
> + if (t > 0xf) {
> + [snip]
parse_nla_mobile_pdu_type() accepts the full 4-bit range 0..15, but the
function comment in seg6_mobile_push_gtpu() notes that only 0 (downlink)
and 1 (uplink) have a defined meaning.
RFC 9433 describes the E behaviors in the downlink packet flows (Section
5.3.1.2 and 5.3.2.2). How are PDU Type 1 (uplink) and the reserved values
supposed to be used with the GTP*.E behaviors?
UAPI cannot be tightened after merge. IMHO, if a type is not supported
yet, the parser should reject it and notify userspace.
> + [snip]
Thanks,
Ciao,
Andrea
^ permalink raw reply
* Re: [PATCH net-next 00/10] docs: net: updates for old and cobwebbed docs
From: Jakub Kicinski @ 2026-05-27 1:15 UTC (permalink / raw)
To: Randy Dunlap
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, corbet,
vladimir.oltean, willemb, sdf.kernel, ecree.xilinx,
jesse.brandeburg, linux-doc
In-Reply-To: <9757813c-dfc2-4979-80da-c3bbf962dc4c@infradead.org>
On Tue, 26 May 2026 17:51:45 -0700 Randy Dunlap wrote:
> It's in today's linux-next (20260526), line 1950 of
> include/linux/netdevice.h:
linux-next is useful, but we try not to merge broken code, not just
notice it after the fact. So it'd be great if the fix made its way
to Linus and therefore propagate to subsystem -next trees, not just
linux-next.
^ permalink raw reply
* Re: [PATCH net-next 00/10] docs: net: updates for old and cobwebbed docs
From: Randy Dunlap @ 2026-05-27 0:51 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, corbet,
vladimir.oltean, willemb, sdf.kernel, ecree.xilinx,
jesse.brandeburg, linux-doc
In-Reply-To: <20260526154001.4a8d30bf@kernel.org>
On 5/26/26 3:40 PM, Jakub Kicinski wrote:
> On Tue, 26 May 2026 15:37:19 -0700 Jakub Kicinski wrote:
>> On Tue, 26 May 2026 11:48:41 -0700 Randy Dunlap wrote:
>>> WARNING: ../include/linux/netdevice.h:2622 Excess struct member 'ax25_ptr' description in 'net_device'
>>
>> I wonder how that sneaked in? ;) ;)
>>
>> I'll clean this up separately, hopefully we haven't regressed too many
>> things while the script was broken :(
>
> Ugh, I'm still not seeing this on Linus's tree.
> Is the fix for kernel-doc skipping such warnings on its way to Linus,
> or queue for -next?
It's in today's linux-next (20260526), line 1950 of
include/linux/netdevice.h:
* @ax25_ptr: AX.25 specific data
from 2014:
commit 536721b1cb3f
Author: Karoly Kemeny <karoly.kemeny@gmail.com>
Date: Wed Jul 30 20:27:36 2014 +0200
net: kernel-doc compliant documentation for net_device
--
~Randy
^ permalink raw reply
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