* Please backport bridge multicast exponential field encoding fix series to 6.1.y/6.6.y/6.12.y/6.18.y/7.0.y
From: Ujjal Roy @ 2026-07-09 10:13 UTC (permalink / raw)
To: Linux Stable, Greg KH, Greg Kroah-Hartman, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Nikolay Aleksandrov, Ido Schimmel, David Ahern, Shuah Khan,
Andy Roulin, Yong Wang, Petr Machata
Cc: Ujjal Roy, bridge, netdev, linux-kernel, linux-kselftest
Hi Greg,
Please consider backporting the following bridge multicast fix series to 6.1.y, 6.6.y, 6.12.y, 6.18.y and 7.0.y.
726fa7da2d8c ("ipv4: igmp: get rid of IGMPV3_{QQIC,MRC} and simplify calculation")
12cfb4ecc471 ("ipv6: mld: rename mldv2_mrc() and add mldv2_qqi()")
95bfd196f0dc ("ipv4: igmp: encode multicast exponential fields")
e51560f4220a ("ipv6: mld: encode multicast exponential fields")
529dbe762de0 ("selftests: net: bridge: add MRC and QQIC field encoding tests")
This series was merged via: db314398f618 ("net: bridge: mcast: support
exponential field encoding")
I checked that the above upstream commits cherry-picked cleanly onto the following stable branches:
linux-7.0.y
linux-6.18.y
linux-6.12.y
linux-6.6.y
linux-6.1.y
Thanks,
Ujjal
^ permalink raw reply
* Re: [PATCH net v3 1/1] tcp: bound SYN-ACK timers to reqsk timeout range
From: Paolo Abeni @ 2026-07-09 10:13 UTC (permalink / raw)
To: Ren Wei, netdev
Cc: edumazet, ncardwell, kuniyu, davem, horms, chia-yu.chang, ij,
idosch, fmancera, bronzed_45_vested, yuuchihsu, yuantan098,
yifanwucs, tomapufckgml, bird, roxy520tt
In-Reply-To: <20260702095324.2995243-1-n05ec@lzu.edu.cn>
On 7/2/26 11:52 AM, Ren Wei wrote:
> @@ -1034,6 +1035,7 @@ static struct ctl_table ipv4_net_table[] = {
> .maxlen = sizeof(u8),
> .mode = 0644,
> .proc_handler = proc_dou8vec_minmax,
> + .extra2 = &tcp_synack_retries_max
This new restriction, even if correct, may break existing scripts.
Since the rest of the patch enforces the upper bound at every
sysctl_tcp_synack_retries usage, what about dropping this chunk?
/P
^ permalink raw reply
* Re: [PATCH net v3 0/1] octeontx2-af: Bug fixes for VF RX mode
From: patchwork-bot+netdevbpf @ 2026-07-09 10:10 UTC (permalink / raw)
To: Nitin Shetty J
Cc: netdev, linux-kernel, sgoutham, lcherian, gakula, hkelam, sbhatta,
andrew+netdev, davem, edumazet, kuba, pabeni, naveenm, rkannoth,
kirankumark
In-Reply-To: <20260702045616.3002773-1-nshettyj@marvell.com>
Hello:
This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Thu, 2 Jul 2026 10:26:15 +0530 you wrote:
> From: Kiran Kumar K <kirankumark@marvell.com>
>
> Hello,
>
> The patch resolves an issue where a VF changing its interface
> state could inadvertently delete the RX promiscuous and all-multicast
> MCAM rules belonging to the host PF.
>
> [...]
Here is the summary with links:
- [net,v3,1/1] octeontx2-af: fix VF bringup affecting PF promiscuous state
https://git.kernel.org/netdev/net/c/fabb881df322
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [PATCH net-next v6 7/7] r8169: fix RTL8116af can not enter s0idle and c10
From: javen @ 2026-07-09 10:02 UTC (permalink / raw)
To: hkallweit1, nic_swsd, andrew+netdev, davem, edumazet, kuba,
pabeni, maxime.chevallier, horms
Cc: netdev, linux-kernel, daniel, linux, enelsonmoore, daniel,
Javen Xu
In-Reply-To: <20260709100237.541-1-javen_xu@realsil.com.cn>
From: Javen Xu <javen_xu@realsil.com.cn>
RTL8116AF is a multi-function device. Functions 2 to 7 are hidden from
the PCI core and return an all-ones response when their vendor ID is read,
so they are not enumerated as normal PCI functions.
However, these hidden functions can still affect platform power
management. If they are left in D0 or keep ASPM disabled, the platform may
fail to enter the low-power s0ix state and the CPU package may fail to
enter Package C10.
Put functions 2 to 7 into D3hot and enable ASPM on their PCIe link control
register. Since these functions are hidden, access their configuration
space through pci_bus_read_config_dword() / pci_bus_write_config_dword()
using the same slot and the target function numbers.
Ignore functions that return a PCI error response when reading their
configuration space.
Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
Changes in v2:
- no changes
Changes in v3:
- no changes
Changes in v4:
- add gate for rtl_lowpower_hidden_functions, only for RTL8116af
Changes in v5:
- no changes
Changes in v6:
- no changes
---
drivers/net/ethernet/realtek/r8169_main.c | 42 +++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index c279beb6fdb5..c0b0c2bc5a48 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -3771,6 +3771,42 @@ static void rtl_hw_start_8168ep_3(struct rtl8169_private *tp)
r8168_mac_ocp_modify(tp, 0xe860, 0x0000, 0x0080);
}
+static void rtl_lowpower_hidden_functions(struct pci_dev *pdev)
+{
+ unsigned int slot = PCI_SLOT(pdev->devfn);
+ struct pci_bus *bus = pdev->bus;
+ int func, pos;
+ u16 val;
+
+ for (func = 2; func < 8; func++) {
+ unsigned int devfn = PCI_DEVFN(slot, func);
+
+ pos = pci_bus_find_capability(bus, devfn, PCI_CAP_ID_EXP);
+ if (pos) {
+ pci_bus_read_config_word(bus, devfn, pos + PCI_EXP_LNKCTL, &val);
+
+ if (PCI_POSSIBLE_ERROR(val))
+ continue;
+
+ val |= (PCI_EXP_LNKCTL_ASPMC | PCI_EXP_LNKCTL_CLKREQ_EN);
+ pci_bus_write_config_word(bus, devfn, pos + PCI_EXP_LNKCTL, val);
+ }
+
+ pos = pci_bus_find_capability(bus, devfn, PCI_CAP_ID_PM);
+ if (pos) {
+ pci_bus_read_config_word(bus, devfn, pos + PCI_PM_CTRL, &val);
+
+ if (PCI_POSSIBLE_ERROR(val))
+ continue;
+
+ val &= ~PCI_PM_CTRL_STATE_MASK;
+ val |= PCI_D3hot;
+ val |= PCI_PM_CTRL_PME_STATUS;
+ pci_bus_write_config_word(bus, devfn, pos + PCI_PM_CTRL, val);
+ }
+ }
+}
+
static void rtl_hw_start_8117(struct rtl8169_private *tp)
{
static const struct ephy_info e_info_8117[] = {
@@ -5326,6 +5362,9 @@ static int rtl8169_resume(struct device *device)
/* Some chip versions may truncate packets without this initialization */
rtl_init_rxcfg(tp);
+ if (rtl_is_8116af(tp))
+ rtl_lowpower_hidden_functions(tp->pci_dev);
+
return rtl8169_runtime_resume(device);
}
@@ -6160,6 +6199,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
rtl8168_driver_start(tp);
}
+ if (rtl_is_8116af(tp))
+ rtl_lowpower_hidden_functions(tp->pci_dev);
+
if (pci_dev_run_wake(pdev))
pm_runtime_put_sync(&pdev->dev);
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v6 4/7] r8169: add support for RTL8116af
From: javen @ 2026-07-09 10:02 UTC (permalink / raw)
To: hkallweit1, nic_swsd, andrew+netdev, davem, edumazet, kuba,
pabeni, maxime.chevallier, horms
Cc: netdev, linux-kernel, daniel, linux, enelsonmoore, daniel,
Javen Xu
In-Reply-To: <20260709100237.541-1-javen_xu@realsil.com.cn>
From: Javen Xu <javen_xu@realsil.com.cn>
RTL8116af is sfp mode. Phylink uses pcs to get the link status from its
serdes reg, instead of standard phy reg. Speed and duplex are hardcoded
to 1000Mbps Full-Duplex. Also, RTL8116af doesn't have internal phy, so
we add some checks to ensure that tp->phydev is not empty when we need it.
In rtl_hw_start_8117(), the MAC calibration for register 0xd412 relies
on reading the internal PHY register 0x0c42. Since RTL8116af does not
have an internal PHY, this calibration step is intentionally bypassed.
Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
Changes in v2:
- replace some magic numbers with macro
Changes in v3:
- change commit message
- add lock when we do rtl8169_sds_read
- use phylink_mii_c22_pcs_decode_state to get status
- add phylink_mac_change for RTL8116af for it doesn't have phy
Changes in v4:
- if tp->pcs.ops is not initial, just return NULL in rtl_mac_select_pcs
Changes in v5:
- no changes
Changes in v6:
- no changes
---
drivers/net/ethernet/realtek/r8169_main.c | 187 ++++++++++++++++++----
1 file changed, 153 insertions(+), 34 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 7a6c734df3d2..570801408f22 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -97,6 +97,18 @@
#define JUMBO_9K (9 * SZ_1K - VLAN_ETH_HLEN - ETH_FCS_LEN)
#define JUMBO_16K (SZ_16K - VLAN_ETH_HLEN - ETH_FCS_LEN)
+#define OCP_SDS_ADDR_REG 0xeb10
+#define OCP_SDS_CMD_REG 0xeb0e
+#define OCP_SDS_DATA_REG 0xeb14
+#define SDS_CMD_READ 0x0001
+#define RTL_SDS_C22_BASE 0x40
+#define RTL_PKG_DETECT 0xdc00
+#define RTL_PKG_DETECT_MASK 0x0078
+#define RTL_PKG_DETECT_8116AF 0x0030
+#define RTL_INT_HW_ID 0xd006
+#define RTL_INT_HW_ID_MASK 0x00ff
+#define RTL_INT_HW_ID_8116AF 0x0000
+
static const struct rtl_chip_info {
u32 mask;
u32 val;
@@ -729,6 +741,12 @@ enum rtl_dash_type {
RTL_DASH_25_BP,
};
+enum rtl_sfp_mode {
+ RTL_SFP_NONE,
+ RTL_SFP_8168_AF,
+ RTL_SFP_8127_ATF,
+};
+
struct rtl8169_private {
void __iomem *mmio_addr; /* memory map physical address */
struct pci_dev *pci_dev;
@@ -737,6 +755,7 @@ struct rtl8169_private {
struct napi_struct napi;
enum mac_version mac_version;
enum rtl_dash_type dash_type;
+ enum rtl_sfp_mode sfp_mode;
u32 cur_rx; /* Index into the Rx descriptor buffer of next Rx pkt. */
u32 cur_tx; /* Index into the Tx descriptor buffer of next Rx pkt. */
u32 dirty_tx;
@@ -764,7 +783,6 @@ struct rtl8169_private {
unsigned supports_gmii:1;
unsigned aspm_manageable:1;
unsigned dash_enabled:1;
- bool sfp_mode:1;
dma_addr_t counters_phys_addr;
struct rtl8169_counters *counters;
struct rtl8169_tc_offsets tc_offset;
@@ -778,6 +796,7 @@ struct rtl8169_private {
u32 ocp_base;
struct phylink *phylink;
struct phylink_config phylink_config;
+ struct phylink_pcs pcs;
};
typedef void (*rtl_generic_fct)(struct rtl8169_private *tp);
@@ -1133,7 +1152,7 @@ static int r8168_phy_ocp_read(struct rtl8169_private *tp, u32 reg)
return 0;
/* Return dummy MII_PHYSID2 in SFP mode to match SFP PHY driver */
- if (tp->sfp_mode && reg == (OCP_STD_PHY_BASE + 2 * MII_PHYSID2))
+ if (tp->sfp_mode == RTL_SFP_8127_ATF && reg == (OCP_STD_PHY_BASE + 2 * MII_PHYSID2))
return PHY_ID_RTL_DUMMY_SFP & 0xffff;
RTL_W32(tp, GPHY_OCP, reg << 15);
@@ -1287,6 +1306,15 @@ static void mac_mcu_write(struct rtl8169_private *tp, int reg, int value)
r8168_mac_ocp_write(tp, tp->ocp_base + reg, value);
}
+static bool rtl_is_8116af(struct rtl8169_private *tp)
+{
+ return tp->mac_version == RTL_GIGA_MAC_VER_52 &&
+ (r8168_mac_ocp_read(tp, RTL_PKG_DETECT) & RTL_PKG_DETECT_MASK) ==
+ RTL_PKG_DETECT_8116AF &&
+ (r8168_mac_ocp_read(tp, RTL_INT_HW_ID) & RTL_INT_HW_ID_MASK) ==
+ RTL_INT_HW_ID_8116AF;
+}
+
static int mac_mcu_read(struct rtl8169_private *tp, int reg)
{
return r8168_mac_ocp_read(tp, tp->ocp_base + reg);
@@ -1582,6 +1610,20 @@ static bool rtl_dash_is_enabled(struct rtl8169_private *tp)
}
}
+static enum rtl_sfp_mode rtl_get_sfp_mode(struct rtl8169_private *tp)
+{
+ if (rtl_is_8125(tp)) {
+ u16 data = r8168_mac_ocp_read(tp, RTL_INT_HW_ID);
+
+ if ((data & 0xff) == 0x07)
+ return RTL_SFP_8127_ATF;
+ } else if (rtl_is_8116af(tp)) {
+ return RTL_SFP_8168_AF;
+ }
+
+ return RTL_SFP_NONE;
+}
+
static enum rtl_dash_type rtl_get_dash_type(struct rtl8169_private *tp)
{
switch (tp->mac_version) {
@@ -2397,7 +2439,7 @@ static int rtl8169_set_link_ksettings(struct net_device *ndev,
int duplex = cmd->base.duplex;
int speed = cmd->base.speed;
- if (!tp->sfp_mode)
+ if (tp->sfp_mode != RTL_SFP_8127_ATF)
return phylink_ethtool_ksettings_set(tp->phylink, cmd);
if (cmd->base.autoneg != AUTONEG_DISABLE)
@@ -2509,9 +2551,10 @@ void r8169_apply_firmware(struct rtl8169_private *tp)
tp->ocp_base = OCP_STD_PHY_BASE;
/* PHY soft reset may still be in progress */
- phy_read_poll_timeout(tp->phydev, MII_BMCR, val,
- !(val & BMCR_RESET),
- 50000, 600000, true);
+ if (tp->phydev)
+ phy_read_poll_timeout(tp->phydev, MII_BMCR, val,
+ !(val & BMCR_RESET),
+ 50000, 600000, true);
}
}
@@ -2548,6 +2591,8 @@ static void rtl_schedule_task(struct rtl8169_private *tp, enum rtl_flag flag)
static void rtl8169_init_phy(struct rtl8169_private *tp)
{
+ phy_init_hw(tp->phydev);
+ phy_resume(tp->phydev);
r8169_hw_phy_config(tp, tp->phydev, tp->mac_version);
if (tp->mac_version <= RTL_GIGA_MAC_VER_06) {
@@ -2562,7 +2607,7 @@ static void rtl8169_init_phy(struct rtl8169_private *tp)
tp->pci_dev->subsystem_device == 0xe000)
phy_write_paged(tp->phydev, 0x0001, 0x10, 0xf01b);
- if (tp->sfp_mode)
+ if (tp->sfp_mode == RTL_SFP_8127_ATF)
rtl_sfp_init(tp);
/* We may have called phy_speed_down before */
@@ -3704,12 +3749,14 @@ static void rtl_hw_start_8117(struct rtl8169_private *tp)
rtl_pcie_state_l2l3_disable(tp);
- rg_saw_cnt = phy_read_paged(tp->phydev, 0x0c42, 0x13) & 0x3fff;
- if (rg_saw_cnt > 0) {
- u16 sw_cnt_1ms_ini;
+ if (tp->phydev) {
+ rg_saw_cnt = phy_read_paged(tp->phydev, 0x0c42, 0x13) & 0x3fff;
+ if (rg_saw_cnt > 0) {
+ u16 sw_cnt_1ms_ini;
- sw_cnt_1ms_ini = (16000000 / rg_saw_cnt) & 0x0fff;
- r8168_mac_ocp_modify(tp, 0xd412, 0x0fff, sw_cnt_1ms_ini);
+ sw_cnt_1ms_ini = (16000000 / rg_saw_cnt) & 0x0fff;
+ r8168_mac_ocp_modify(tp, 0xd412, 0x0fff, sw_cnt_1ms_ini);
+ }
}
r8168_mac_ocp_modify(tp, 0xe056, 0x00f0, 0x0000);
@@ -4893,8 +4940,13 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
goto out;
}
- if (status & LinkChg)
- phy_mac_interrupt(tp->phydev);
+ if (status & LinkChg) {
+ if (tp->phydev)
+ phy_mac_interrupt(tp->phydev);
+ else if (tp->sfp_mode == RTL_SFP_8168_AF)
+ phylink_mac_change(tp->phylink,
+ !!(RTL_R8(tp, PHYstatus) & LinkStatus));
+ }
rtl_irq_disable(tp);
napi_schedule(&tp->napi);
@@ -5004,7 +5056,7 @@ static void rtl8169_down(struct rtl8169_private *tp)
bitmap_zero(tp->wk.flags, RTL_FLAG_MAX);
/* Reset SerDes PHY to bring down fiber link */
- if (tp->sfp_mode)
+ if (tp->sfp_mode == RTL_SFP_8127_ATF)
rtl_sfp_reset(tp);
rtl8169_update_counters(tp);
@@ -5026,9 +5078,9 @@ static void rtl8169_up(struct rtl8169_private *tp)
rtl8168_driver_start(tp);
pci_set_master(tp->pci_dev);
- phy_init_hw(tp->phydev);
- phy_resume(tp->phydev);
- rtl8169_init_phy(tp);
+ if (tp->phydev)
+ rtl8169_init_phy(tp);
+
napi_enable(&tp->napi);
enable_work(&tp->wk.work);
rtl_reset_work(tp);
@@ -5105,10 +5157,11 @@ static int rtl_open(struct net_device *dev)
if (retval < 0)
goto err_release_fw_2;
- retval = r8169_phy_connect(tp);
- if (retval)
- goto err_free_irq;
-
+ if (tp->phydev) {
+ retval = r8169_phy_connect(tp);
+ if (retval)
+ goto err_free_irq;
+ }
rtl8169_up(tp);
phylink_start(tp->phylink);
rtl8169_init_counter_offsets(tp);
@@ -5609,6 +5662,14 @@ static void rtl_mac_link_up(struct phylink_config *config, struct phy_device *ph
static struct phylink_pcs *rtl_mac_select_pcs(struct phylink_config *config,
phy_interface_t interface)
{
+ struct rtl8169_private *tp = container_of(config, struct rtl8169_private, phylink_config);
+
+ if (!tp->pcs.ops)
+ return NULL;
+
+ if (interface == PHY_INTERFACE_MODE_1000BASEX)
+ return &tp->pcs;
+
return NULL;
}
@@ -5617,6 +5678,51 @@ static void rtl_mac_config(struct phylink_config *config, unsigned int mode,
{
}
+static u16 rtl8169_sds_read(struct rtl8169_private *tp, u16 sds_reg)
+{
+ unsigned long flags;
+ u16 val = 0;
+
+ raw_spin_lock_irqsave(&tp->mac_ocp_lock, flags);
+ __r8168_mac_ocp_write(tp, OCP_SDS_ADDR_REG, sds_reg);
+ __r8168_mac_ocp_write(tp, OCP_SDS_CMD_REG, SDS_CMD_READ);
+ val = __r8168_mac_ocp_read(tp, OCP_SDS_DATA_REG);
+ raw_spin_unlock_irqrestore(&tp->mac_ocp_lock, flags);
+
+ return val;
+}
+
+static void rtl8169_pcs_get_state(struct phylink_pcs *pcs,
+ unsigned int neg_mode,
+ struct phylink_link_state *state)
+{
+ struct rtl8169_private *tp = container_of(pcs, struct rtl8169_private, pcs);
+ u16 bmsr, lpa;
+
+ bmsr = rtl8169_sds_read(tp, RTL_SDS_C22_BASE + MII_BMSR);
+ lpa = rtl8169_sds_read(tp, RTL_SDS_C22_BASE + MII_LPA);
+
+ phylink_mii_c22_pcs_decode_state(state, neg_mode, bmsr, lpa);
+}
+
+static int rtl8169_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
+ phy_interface_t interface,
+ const unsigned long *advertising,
+ bool permit_pause_to_mac)
+{
+ return 0;
+}
+
+static int rtl8169_pcs_validate(struct phylink_pcs *pcs, unsigned long *supported,
+ const struct phylink_link_state *state)
+{
+ return 0;
+}
+
+static void rtl8169_pcs_an_restart(struct phylink_pcs *pcs)
+{
+}
+
static void rtl_mac_disable_tx_lpi(struct phylink_config *config)
{
struct rtl8169_private *tp = container_of(config, struct rtl8169_private, phylink_config);
@@ -5664,6 +5770,13 @@ static unsigned long rtl8169_get_lpi_caps(struct rtl8169_private *tp)
return caps;
}
+static const struct phylink_pcs_ops r8169_pcs_ops = {
+ .pcs_validate = rtl8169_pcs_validate,
+ .pcs_get_state = rtl8169_pcs_get_state,
+ .pcs_config = rtl8169_pcs_config,
+ .pcs_an_restart = rtl8169_pcs_an_restart,
+};
+
static int rtl_init_phylink(struct rtl8169_private *tp)
{
struct phylink *pl;
@@ -5675,10 +5788,18 @@ static int rtl_init_phylink(struct rtl8169_private *tp)
tp->phylink_config.lpi_capabilities = rtl8169_get_lpi_caps(tp);
tp->phylink_config.mac_capabilities |= MAC_ASYM_PAUSE | MAC_SYM_PAUSE;
- if (tp->sfp_mode) {
+ switch (tp->sfp_mode) {
+ case RTL_SFP_8168_AF:
+ tp->pcs.ops = &r8169_pcs_ops;
+ tp->phylink_config.default_an_inband = true;
+ phy_mode = PHY_INTERFACE_MODE_1000BASEX;
+ tp->phylink_config.mac_capabilities |= MAC_1000FD;
+ break;
+ case RTL_SFP_8127_ATF:
phy_mode = PHY_INTERFACE_MODE_INTERNAL;
tp->phylink_config.mac_capabilities |= MAC_10000FD;
- } else {
+ break;
+ default:
tp->phylink_config.mac_capabilities |= MAC_10 | MAC_100;
if (tp->mac_version == RTL_GIGA_MAC_VER_80)
@@ -5698,6 +5819,7 @@ static int rtl_init_phylink(struct rtl8169_private *tp)
PHY_INTERFACE_MODE_MII;
else
phy_mode = PHY_INTERFACE_MODE_INTERNAL;
+ break;
}
__set_bit(phy_mode, tp->phylink_config.supported_interfaces);
@@ -5795,12 +5917,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
}
tp->aspm_manageable = !rc;
- if (rtl_is_8125(tp)) {
- u16 data = r8168_mac_ocp_read(tp, 0xd006);
-
- if ((data & 0xff) == 0x07)
- tp->sfp_mode = true;
- }
+ tp->sfp_mode = rtl_get_sfp_mode(tp);
tp->dash_type = rtl_get_dash_type(tp);
tp->dash_enabled = rtl_dash_is_enabled(tp);
@@ -5908,10 +6025,12 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
if (rc)
return rc;
- rc = r8169_mdio_register(tp);
- if (rc) {
- phylink_destroy(tp->phylink);
- return rc;
+ if (tp->sfp_mode != RTL_SFP_8168_AF) {
+ rc = r8169_mdio_register(tp);
+ if (rc) {
+ phylink_destroy(tp->phylink);
+ return rc;
+ }
}
rc = register_netdev(dev);
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v6 1/7] r8169: add speed in private struct
From: javen @ 2026-07-09 10:02 UTC (permalink / raw)
To: hkallweit1, nic_swsd, andrew+netdev, davem, edumazet, kuba,
pabeni, maxime.chevallier, horms
Cc: netdev, linux-kernel, daniel, linux, enelsonmoore, daniel,
Javen Xu, Andrew Lunn
In-Reply-To: <20260709100237.541-1-javen_xu@realsil.com.cn>
From: Javen Xu <javen_xu@realsil.com.cn>
This patch adds speed in private struct in order to decouple
from phydev in the following patch supporting for phylink.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
Changes in v2:
- repalce current_speed with speed
Changes in v3:
- update tp->speed in rtl8169_set_link_ksettings()
Changes in v4:
- no changes
Changes in v5:
- no changes
Changes in v6:
- no changes
---
drivers/net/ethernet/realtek/r8169_main.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index ec4fc21fa21f..c60710f9bd21 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -750,6 +750,7 @@ struct rtl8169_private {
u32 irq_mask;
int irq;
struct clk *clk;
+ int speed;
struct {
DECLARE_BITMAP(flags, RTL_FLAG_MAX);
@@ -1673,16 +1674,14 @@ static void rtl8169_irq_mask_and_ack(struct rtl8169_private *tp)
rtl_pci_commit(tp);
}
-static void rtl_link_chg_patch(struct rtl8169_private *tp)
+static void rtl_link_chg_patch(struct rtl8169_private *tp, int speed)
{
- struct phy_device *phydev = tp->phydev;
-
if (tp->mac_version == RTL_GIGA_MAC_VER_34 ||
tp->mac_version == RTL_GIGA_MAC_VER_38) {
- if (phydev->speed == SPEED_1000) {
+ if (speed == SPEED_1000) {
rtl_eri_write(tp, 0x1bc, ERIAR_MASK_1111, 0x00000011);
rtl_eri_write(tp, 0x1dc, ERIAR_MASK_1111, 0x00000005);
- } else if (phydev->speed == SPEED_100) {
+ } else if (speed == SPEED_100) {
rtl_eri_write(tp, 0x1bc, ERIAR_MASK_1111, 0x0000001f);
rtl_eri_write(tp, 0x1dc, ERIAR_MASK_1111, 0x00000005);
} else {
@@ -1692,7 +1691,7 @@ static void rtl_link_chg_patch(struct rtl8169_private *tp)
rtl_reset_packet_filter(tp);
} else if (tp->mac_version == RTL_GIGA_MAC_VER_35 ||
tp->mac_version == RTL_GIGA_MAC_VER_36) {
- if (phydev->speed == SPEED_1000) {
+ if (speed == SPEED_1000) {
rtl_eri_write(tp, 0x1bc, ERIAR_MASK_1111, 0x00000011);
rtl_eri_write(tp, 0x1dc, ERIAR_MASK_1111, 0x00000005);
} else {
@@ -1700,7 +1699,7 @@ static void rtl_link_chg_patch(struct rtl8169_private *tp)
rtl_eri_write(tp, 0x1dc, ERIAR_MASK_1111, 0x0000003f);
}
} else if (tp->mac_version == RTL_GIGA_MAC_VER_37) {
- if (phydev->speed == SPEED_10) {
+ if (speed == SPEED_10) {
rtl_eri_write(tp, 0x1d0, ERIAR_MASK_0011, 0x4d02);
rtl_eri_write(tp, 0x1dc, ERIAR_MASK_0011, 0x0060a);
} else {
@@ -2074,11 +2073,11 @@ rtl_coalesce_info(struct rtl8169_private *tp)
ci = rtl_coalesce_info_8168_8136;
/* if speed is unknown assume highest one */
- if (tp->phydev->speed == SPEED_UNKNOWN)
+ if (tp->speed == SPEED_UNKNOWN)
return ci;
for (; ci->speed; ci++) {
- if (tp->phydev->speed == ci->speed)
+ if (tp->speed == ci->speed)
return ci;
}
@@ -2236,7 +2235,7 @@ static void rtl_set_eee_txidle_timer(struct rtl8169_private *tp)
static unsigned int r8169_get_tx_lpi_timer_us(struct rtl8169_private *tp)
{
- unsigned int speed = tp->phydev->speed;
+ unsigned int speed = tp->speed;
unsigned int timer = tp->tx_lpi_timer;
if (!timer || speed == SPEED_UNKNOWN)
@@ -4968,8 +4967,9 @@ static void r8169_phylink_handler(struct net_device *ndev)
struct rtl8169_private *tp = netdev_priv(ndev);
struct device *d = tp_to_dev(tp);
+ tp->speed = tp->phydev->speed;
if (netif_carrier_ok(ndev)) {
- rtl_link_chg_patch(tp);
+ rtl_link_chg_patch(tp, tp->speed);
rtl_enable_tx_lpi(tp, tp->phydev->enable_tx_lpi);
pm_request_resume(d);
} else {
@@ -5667,6 +5667,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
ext_xid_str, xid);
tp->mac_version = chip->mac_version;
tp->fw_name = chip->fw_name;
+ tp->speed = SPEED_UNKNOWN;
/* Disable ASPM L1 as that cause random device stop working
* problems as well as full system hangs for some PCIe devices users.
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v6 6/7] r8169: add ltr support for RTL8117 series
From: javen @ 2026-07-09 10:02 UTC (permalink / raw)
To: hkallweit1, nic_swsd, andrew+netdev, davem, edumazet, kuba,
pabeni, maxime.chevallier, horms
Cc: netdev, linux-kernel, daniel, linux, enelsonmoore, daniel,
Javen Xu
In-Reply-To: <20260709100237.541-1-javen_xu@realsil.com.cn>
From: Javen Xu <javen_xu@realsil.com.cn>
This patch adds ltr support for RTL8117 series, enables RTL8117 series
enter l1.2 state. This makes sense for the system to enter c10 state.
This patch drops the COMBO_LTR_EXTEND_EN write for VER_52. This is
intentional, as this bit is not required for RTL8116af.
Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
Changes in v2:
- no changes
Changes in v3:
- no changes
Changes in v4:
- no changes
Changes in v5:
- no changes
Changes in v6:
- no changes
---
drivers/net/ethernet/realtek/r8169_main.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index db2f3d9d6bf1..c279beb6fdb5 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -353,11 +353,13 @@ enum rtl_registers {
ALDPS_LTR = 0xe0a2,
LTR_OBFF_LOCK = 0xe032,
LTR_SNOOP = 0xe034,
+ SEND_LTR_MSG = 0xe038,
#define ALDPS_LTR_EN BIT(0)
#define LTR_OBFF_LOCK_EN BIT(0)
#define LINK_SPEED_CHANGE_EN BIT(14)
#define LTR_SNOOP_EN GENMASK(15, 14)
+#define LTR_MSG_EN BIT(0)
};
enum rtl8168_8101_registers {
@@ -3192,8 +3194,23 @@ static void rtl_enable_ltr(struct rtl8169_private *tp)
r8168_mac_ocp_write(tp, 0xcdf2, 0x9003);
r8168_mac_ocp_modify(tp, LTR_OBFF_LOCK, 0x0000, LINK_SPEED_CHANGE_EN);
break;
- case RTL_GIGA_MAC_VER_46 ... RTL_GIGA_MAC_VER_48:
case RTL_GIGA_MAC_VER_52:
+ r8168_mac_ocp_write(tp, 0xcdd0, 0x9003);
+ r8168_mac_ocp_modify(tp, LTR_SNOOP, 0x0000, LTR_SNOOP_EN);
+ r8168_mac_ocp_write(tp, 0xe02c, 0x1880);
+ r8168_mac_ocp_write(tp, 0xe02e, 0x4880);
+ r8168_mac_ocp_modify(tp, ALDPS_LTR, 0x0000, ALDPS_LTR_EN);
+ r8168_mac_ocp_write(tp, 0xcdd8, 0x9003);
+ r8168_mac_ocp_write(tp, 0xcdda, 0x9003);
+ r8168_mac_ocp_write(tp, 0xcddc, 0x9003);
+ r8168_mac_ocp_write(tp, 0xcdd2, 0x883c);
+ r8168_mac_ocp_write(tp, 0xcdd4, 0x8c12);
+ r8168_mac_ocp_write(tp, 0xcdd6, 0x9003);
+ r8168_mac_ocp_write(tp, 0xe0a6, 0x9003);
+ r8168_mac_ocp_write(tp, 0xe0a8, 0x9003);
+ r8168_mac_ocp_modify(tp, LTR_OBFF_LOCK, 0x0000, LINK_SPEED_CHANGE_EN);
+ break;
+ case RTL_GIGA_MAC_VER_46 ... RTL_GIGA_MAC_VER_48:
r8168_mac_ocp_modify(tp, ALDPS_LTR, 0x0000, ALDPS_LTR_EN);
RTL_W8(tp, COMBO_LTR_EXTEND, RTL_R8(tp, COMBO_LTR_EXTEND) | COMBO_LTR_EXTEND_EN);
fallthrough;
@@ -3213,6 +3230,7 @@ static void rtl_enable_ltr(struct rtl8169_private *tp)
}
/* chip can trigger LTR */
r8168_mac_ocp_modify(tp, LTR_OBFF_LOCK, 0x0003, LTR_OBFF_LOCK_EN);
+ r8168_mac_ocp_modify(tp, SEND_LTR_MSG, 0x0000, LTR_MSG_EN);
}
static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable)
@@ -3246,6 +3264,7 @@ static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable)
rtl_enable_ltr(tp);
switch (tp->mac_version) {
case RTL_GIGA_MAC_VER_46 ... RTL_GIGA_MAC_VER_48:
+ case RTL_GIGA_MAC_VER_52:
case RTL_GIGA_MAC_VER_61 ... RTL_GIGA_MAC_VER_LAST:
/* reset ephy tx/rx disable timer */
r8168_mac_ocp_modify(tp, 0xe094, 0xff00, 0);
@@ -3258,6 +3277,7 @@ static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable)
} else {
switch (tp->mac_version) {
case RTL_GIGA_MAC_VER_46 ... RTL_GIGA_MAC_VER_48:
+ case RTL_GIGA_MAC_VER_52:
case RTL_GIGA_MAC_VER_61 ... RTL_GIGA_MAC_VER_LAST:
r8168_mac_ocp_modify(tp, 0xe092, 0x00ff, 0);
break;
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v6 5/7] r8169: add support for RTL8127atf
From: javen @ 2026-07-09 10:02 UTC (permalink / raw)
To: hkallweit1, nic_swsd, andrew+netdev, davem, edumazet, kuba,
pabeni, maxime.chevallier, horms
Cc: netdev, linux-kernel, daniel, linux, enelsonmoore, daniel,
Javen Xu
In-Reply-To: <20260709100237.541-1-javen_xu@realsil.com.cn>
From: Javen Xu <javen_xu@realsil.com.cn>
RTL8127atf is also a fiber mode card, but its sds reg base addr is
0x0080, which is different from RTL8116af. Add 10g and 1g support for
RTL8127atf in this patch.
Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
Changes in v3:
- No changes. New file.
Changes in v4:
- remove DUMMY_PHY in driver/net/phy/realtek/realtek_main.c and related
function
Changes in v5:
- no changes, 1G support will be submitted by a following patch
Changes in v6:
- no changes, merge patch 7/8 and 8/8 in v5
- r8127_sds_read/write, return -ETIMEDOUT when time out
- rtl8169_pcs_get_state set speed according to state->interface
- register PHY_INTERFACE_MODE_10GBASER and PHY_INTERFACE_MODE_1000BASEX
to supported_interfaces for RTL8127atf
---
drivers/net/ethernet/realtek/r8169_main.c | 174 ++++++++++++++++------
drivers/net/phy/realtek/realtek_main.c | 54 -------
include/net/phy/realtek_phy.h | 7 -
3 files changed, 127 insertions(+), 108 deletions(-)
delete mode 100644 include/net/phy/realtek_phy.h
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 570801408f22..db2f3d9d6bf1 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -32,7 +32,6 @@
#include <linux/unaligned.h>
#include <net/ip6_checksum.h>
#include <net/netdev_queues.h>
-#include <net/phy/realtek_phy.h>
#include "r8169.h"
#include "r8169_firmware.h"
@@ -97,11 +96,18 @@
#define JUMBO_9K (9 * SZ_1K - VLAN_ETH_HLEN - ETH_FCS_LEN)
#define JUMBO_16K (SZ_16K - VLAN_ETH_HLEN - ETH_FCS_LEN)
+#define R8127_SDS_CMD 0x2348
+#define R8127_SDS_ADDR 0x234a
+#define R8127_SDS_DATA_IN 0x234c
+#define R8127_SDS_DATA_OUT 0x234e
+#define R8127_SDS_CMD_EXEC BIT(0)
+#define R8127_SDS_CMD_WE BIT(1)
#define OCP_SDS_ADDR_REG 0xeb10
#define OCP_SDS_CMD_REG 0xeb0e
#define OCP_SDS_DATA_REG 0xeb14
#define SDS_CMD_READ 0x0001
#define RTL_SDS_C22_BASE 0x40
+#define RTL_SDS_C45_BASE 0x0080
#define RTL_PKG_DETECT 0xdc00
#define RTL_PKG_DETECT_MASK 0x0078
#define RTL_PKG_DETECT_8116AF 0x0030
@@ -1151,10 +1157,6 @@ static int r8168_phy_ocp_read(struct rtl8169_private *tp, u32 reg)
if (rtl_ocp_reg_failure(reg))
return 0;
- /* Return dummy MII_PHYSID2 in SFP mode to match SFP PHY driver */
- if (tp->sfp_mode == RTL_SFP_8127_ATF && reg == (OCP_STD_PHY_BASE + 2 * MII_PHYSID2))
- return PHY_ID_RTL_DUMMY_SFP & 0xffff;
-
RTL_W32(tp, GPHY_OCP, reg << 15);
return rtl_loop_wait_high(tp, &rtl_ocp_gphy_cond, 25, 10) ?
@@ -1222,6 +1224,73 @@ static void r8127_sfp_sds_phy_reset(struct rtl8169_private *tp)
usleep_range(10, 20);
}
+DECLARE_RTL_COND(r8127_sds_cmd_cond)
+{
+ return RTL_R16(tp, R8127_SDS_CMD) & R8127_SDS_CMD_EXEC;
+}
+
+static int r8127_sds_read(struct rtl8169_private *tp, u16 index, u16 page, u16 reg)
+{
+ u16 addr = (index << 11) | (page << 5) | reg;
+
+ RTL_W16(tp, R8127_SDS_ADDR, addr);
+ RTL_W16(tp, R8127_SDS_CMD, R8127_SDS_CMD_EXEC);
+
+ if (rtl_loop_wait_low(tp, &r8127_sds_cmd_cond, 10, 100))
+ return RTL_R16(tp, R8127_SDS_DATA_OUT);
+
+ return -ETIMEDOUT;
+}
+
+static int r8127_sds_write(struct rtl8169_private *tp, u16 index, u16 page,
+ u16 reg, u16 val)
+{
+ u16 addr = (index << 11) | (page << 5) | reg;
+
+ RTL_W16(tp, R8127_SDS_DATA_IN, val);
+ RTL_W16(tp, R8127_SDS_ADDR, addr);
+ RTL_W16(tp, R8127_SDS_CMD, R8127_SDS_CMD_EXEC | R8127_SDS_CMD_WE);
+
+ if (rtl_loop_wait_low(tp, &r8127_sds_cmd_cond, 10, 100))
+ return 0;
+
+ return -ETIMEDOUT;
+}
+
+static void r8127_sds_modify(struct rtl8169_private *tp, u16 index, u16 page,
+ u16 reg, u16 clearmask, u16 setmask)
+{
+ int val = r8127_sds_read(tp, index, page, reg);
+
+ if (val < 0)
+ return;
+
+ val = (val & ~clearmask) | setmask;
+ r8127_sds_write(tp, index, page, reg, val);
+}
+
+static void r8127_sfp_init_1g(struct rtl8169_private *tp)
+{
+ int val;
+
+ r8127_sfp_sds_phy_reset(tp);
+
+ r8127_sds_modify(tp, 0, 1, 31, 0, BIT(3));
+ r8127_sds_modify(tp, 0, 2, 0, BIT(13) | BIT(12) | BIT(6), BIT(12) | BIT(6));
+ r8127_sds_modify(tp, 0, 0, 4, 0, BIT(2));
+
+ RTL_W16(tp, 0x233a, 0x8004);
+ RTL_W16(tp, 0x233e, (RTL_R16(tp, 0x233e) & ~0x3003) | 0x0002);
+
+ r8168_phy_ocp_write(tp, 0xc40a, 0x0000);
+ r8168_phy_ocp_write(tp, 0xc466, 0x0000);
+ r8168_phy_ocp_write(tp, 0xc808, 0x0000);
+ r8168_phy_ocp_write(tp, 0xc80a, 0x0000);
+
+ val = r8168_phy_ocp_read(tp, 0xc804);
+ r8168_phy_ocp_write(tp, 0xc804, (val & ~0x000f) | 0x000c);
+}
+
static void r8127_sfp_init_10g(struct rtl8169_private *tp)
{
int val;
@@ -1240,12 +1309,6 @@ static void r8127_sfp_init_10g(struct rtl8169_private *tp)
r8168_phy_ocp_write(tp, 0xc804, (val & ~0x000f) | 0x000c);
}
-static void rtl_sfp_init(struct rtl8169_private *tp)
-{
- if (tp->mac_version == RTL_GIGA_MAC_VER_80)
- r8127_sfp_init_10g(tp);
-}
-
static void rtl_sfp_reset(struct rtl8169_private *tp)
{
if (tp->mac_version == RTL_GIGA_MAC_VER_80)
@@ -2435,30 +2498,8 @@ static int rtl8169_set_link_ksettings(struct net_device *ndev,
const struct ethtool_link_ksettings *cmd)
{
struct rtl8169_private *tp = netdev_priv(ndev);
- struct phy_device *phydev = tp->phydev;
- int duplex = cmd->base.duplex;
- int speed = cmd->base.speed;
-
- if (tp->sfp_mode != RTL_SFP_8127_ATF)
- return phylink_ethtool_ksettings_set(tp->phylink, cmd);
-
- if (cmd->base.autoneg != AUTONEG_DISABLE)
- return -EINVAL;
-
- if (!phy_check_valid(speed, duplex, phydev->supported))
- return -EINVAL;
- mutex_lock(&phydev->lock);
-
- phydev->autoneg = AUTONEG_DISABLE;
- phydev->speed = speed;
- phydev->duplex = duplex;
-
- rtl_sfp_init(tp);
-
- mutex_unlock(&phydev->lock);
-
- return 0;
+ return phylink_ethtool_ksettings_set(tp->phylink, cmd);
}
static int rtl8169_nway_reset(struct net_device *dev)
@@ -2607,9 +2648,6 @@ static void rtl8169_init_phy(struct rtl8169_private *tp)
tp->pci_dev->subsystem_device == 0xe000)
phy_write_paged(tp->phydev, 0x0001, 0x10, 0xf01b);
- if (tp->sfp_mode == RTL_SFP_8127_ATF)
- rtl_sfp_init(tp);
-
/* We may have called phy_speed_down before */
phy_speed_up(tp->phydev);
@@ -4943,7 +4981,7 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
if (status & LinkChg) {
if (tp->phydev)
phy_mac_interrupt(tp->phydev);
- else if (tp->sfp_mode == RTL_SFP_8168_AF)
+ else if (tp->sfp_mode)
phylink_mac_change(tp->phylink,
!!(RTL_R8(tp, PHYstatus) & LinkStatus));
}
@@ -5667,7 +5705,8 @@ static struct phylink_pcs *rtl_mac_select_pcs(struct phylink_config *config,
if (!tp->pcs.ops)
return NULL;
- if (interface == PHY_INTERFACE_MODE_1000BASEX)
+ if (interface == PHY_INTERFACE_MODE_1000BASEX ||
+ interface == PHY_INTERFACE_MODE_10GBASER)
return &tp->pcs;
return NULL;
@@ -5697,12 +5736,32 @@ static void rtl8169_pcs_get_state(struct phylink_pcs *pcs,
struct phylink_link_state *state)
{
struct rtl8169_private *tp = container_of(pcs, struct rtl8169_private, pcs);
- u16 bmsr, lpa;
- bmsr = rtl8169_sds_read(tp, RTL_SDS_C22_BASE + MII_BMSR);
- lpa = rtl8169_sds_read(tp, RTL_SDS_C22_BASE + MII_LPA);
+ if (tp->sfp_mode == RTL_SFP_8127_ATF) {
+ u16 stat1;
+
+ stat1 = rtl8169_sds_read(tp, RTL_SDS_C45_BASE + MDIO_STAT1);
+
+ if (!(stat1 & MDIO_STAT1_LSTATUS))
+ stat1 = rtl8169_sds_read(tp, RTL_SDS_C45_BASE + MDIO_STAT1);
- phylink_mii_c22_pcs_decode_state(state, neg_mode, bmsr, lpa);
+ state->link = !!(stat1 & MDIO_STAT1_LSTATUS);
+ if (!state->link)
+ return;
+
+ if (state->interface == PHY_INTERFACE_MODE_1000BASEX)
+ state->speed = SPEED_1000;
+ else
+ state->speed = SPEED_10000;
+
+ state->duplex = DUPLEX_FULL;
+ } else {
+ u16 bmsr, lpa;
+
+ bmsr = rtl8169_sds_read(tp, RTL_SDS_C22_BASE + MII_BMSR);
+ lpa = rtl8169_sds_read(tp, RTL_SDS_C22_BASE + MII_LPA);
+ phylink_mii_c22_pcs_decode_state(state, neg_mode, bmsr, lpa);
+ }
}
static int rtl8169_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
@@ -5710,6 +5769,23 @@ static int rtl8169_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
const unsigned long *advertising,
bool permit_pause_to_mac)
{
+ struct rtl8169_private *tp = container_of(pcs, struct rtl8169_private, pcs);
+
+ if (tp->sfp_mode == RTL_SFP_8127_ATF) {
+ switch (interface) {
+ case PHY_INTERFACE_MODE_10GBASER:
+ r8127_sfp_init_10g(tp);
+ break;
+ case PHY_INTERFACE_MODE_1000BASEX:
+ r8127_sfp_init_1g(tp);
+ break;
+ default:
+ netdev_err(tp->dev, "Unsupported SFP interface mode: %s\n",
+ phy_modes(interface));
+ return -EOPNOTSUPP;
+ }
+ }
+
return 0;
}
@@ -5752,7 +5828,7 @@ static unsigned long rtl8169_get_lpi_caps(struct rtl8169_private *tp)
{
unsigned long caps = 0;
- if (!rtl_supports_eee(tp))
+ if (!rtl_supports_eee(tp) || tp->sfp_mode == RTL_SFP_8127_ATF)
return 0;
caps |= MAC_100FD | MAC_1000FD;
@@ -5796,8 +5872,12 @@ static int rtl_init_phylink(struct rtl8169_private *tp)
tp->phylink_config.mac_capabilities |= MAC_1000FD;
break;
case RTL_SFP_8127_ATF:
- phy_mode = PHY_INTERFACE_MODE_INTERNAL;
- tp->phylink_config.mac_capabilities |= MAC_10000FD;
+ tp->pcs.ops = &r8169_pcs_ops;
+ phy_mode = PHY_INTERFACE_MODE_10GBASER;
+ tp->phylink_config.default_an_inband = true;
+ tp->phylink_config.mac_capabilities |= MAC_1000FD | MAC_10000FD;
+ __set_bit(PHY_INTERFACE_MODE_10GBASER, tp->phylink_config.supported_interfaces);
+ __set_bit(PHY_INTERFACE_MODE_1000BASEX, tp->phylink_config.supported_interfaces);
break;
default:
tp->phylink_config.mac_capabilities |= MAC_10 | MAC_100;
@@ -6025,7 +6105,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
if (rc)
return rc;
- if (tp->sfp_mode != RTL_SFP_8168_AF) {
+ if (tp->sfp_mode == RTL_SFP_NONE) {
rc = r8169_mdio_register(tp);
if (rc) {
phylink_destroy(tp->phylink);
diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index b65d0f5fa1a0..4721ba071cbb 100644
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -17,7 +17,6 @@
#include <linux/delay.h>
#include <linux/clk.h>
#include <linux/string_choices.h>
-#include <net/phy/realtek_phy.h>
#include "../phylib.h"
#include "realtek.h"
@@ -2646,45 +2645,6 @@ static irqreturn_t rtl8221b_handle_interrupt(struct phy_device *phydev)
return IRQ_HANDLED;
}
-static int rtlgen_sfp_get_features(struct phy_device *phydev)
-{
- linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
- phydev->supported);
-
- /* set default mode */
- phydev->speed = SPEED_10000;
- phydev->duplex = DUPLEX_FULL;
-
- phydev->port = PORT_FIBRE;
-
- return 0;
-}
-
-static int rtlgen_sfp_read_status(struct phy_device *phydev)
-{
- int val, err;
-
- err = genphy_update_link(phydev);
- if (err)
- return err;
-
- if (!phydev->link)
- return 0;
-
- val = phy_read(phydev, RTL_PHYSR);
- if (val < 0)
- return val;
-
- rtlgen_decode_physr(phydev, val);
-
- return 0;
-}
-
-static int rtlgen_sfp_config_aneg(struct phy_device *phydev)
-{
- return 0;
-}
-
static struct phy_driver realtek_drvs[] = {
{
PHY_ID_MATCH_EXACT(0x00008201),
@@ -2934,20 +2894,6 @@ static struct phy_driver realtek_drvs[] = {
.write_page = rtl821x_write_page,
.read_mmd = rtl822x_read_mmd,
.write_mmd = rtl822x_write_mmd,
- }, {
- PHY_ID_MATCH_EXACT(PHY_ID_RTL_DUMMY_SFP),
- .name = "Realtek SFP PHY Mode",
- .flags = PHY_IS_INTERNAL,
- .probe = rtl822x_probe,
- .get_features = rtlgen_sfp_get_features,
- .config_aneg = rtlgen_sfp_config_aneg,
- .read_status = rtlgen_sfp_read_status,
- .suspend = genphy_suspend,
- .resume = rtlgen_resume,
- .read_page = rtl821x_read_page,
- .write_page = rtl821x_write_page,
- .read_mmd = rtl822x_read_mmd,
- .write_mmd = rtl822x_write_mmd,
}, {
PHY_ID_MATCH_EXACT(0x001ccad0),
.name = "RTL8224 2.5Gbps PHY",
diff --git a/include/net/phy/realtek_phy.h b/include/net/phy/realtek_phy.h
deleted file mode 100644
index d683bc1b0659..000000000000
--- a/include/net/phy/realtek_phy.h
+++ /dev/null
@@ -1,7 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _REALTEK_PHY_H
-#define _REALTEK_PHY_H
-
-#define PHY_ID_RTL_DUMMY_SFP 0x001ccbff
-
-#endif /* _REALTEK_PHY_H */
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v6 3/7] r8169: add support for phylink
From: javen @ 2026-07-09 10:02 UTC (permalink / raw)
To: hkallweit1, nic_swsd, andrew+netdev, davem, edumazet, kuba,
pabeni, maxime.chevallier, horms
Cc: netdev, linux-kernel, daniel, linux, enelsonmoore, daniel,
Javen Xu
In-Reply-To: <20260709100237.541-1-javen_xu@realsil.com.cn>
From: Javen Xu <javen_xu@realsil.com.cn>
Transfer old framework to phylink. Phylink can support fiber mode card
which can not get link status or link speed from standard phy registers.
Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
Changes in v2:
- merge patch v1 3/6 and v1 4/6.
- add helper rtl_mac_enable_tx_lpi(), rtl_mac_disable_tx_lpi()
and rtl8169_get_lpi_caps()
Changes in v3:
- use phylink_ethtool_set_pauseparam to set pause status when change
mtu
- replace phy_do_ioctl_running with rtl8169_ioctl
- recover phy_mode according to tp->supports_gmii for 1G nics
Changes in v4:
- set lpi_interface if support eee
- Only if jumbo changes, set pause
Changes in v5:
- no changes
Changes in v6:
- remove PHYLIB in KConfig
- remove dead code, saved_pause and jumbo_pause_saved
- move phylink_start to rtl8169_open, phylink_stop to rtl8169_close
---
drivers/net/ethernet/realtek/Kconfig | 2 +-
drivers/net/ethernet/realtek/r8169_main.c | 273 ++++++++++++++++------
2 files changed, 199 insertions(+), 76 deletions(-)
diff --git a/drivers/net/ethernet/realtek/Kconfig b/drivers/net/ethernet/realtek/Kconfig
index 9b0f4f9631db..5818a9a500a8 100644
--- a/drivers/net/ethernet/realtek/Kconfig
+++ b/drivers/net/ethernet/realtek/Kconfig
@@ -86,8 +86,8 @@ config R8169
depends on PCI
select FW_LOADER
select CRC32
- select PHYLIB
select REALTEK_PHY
+ select PHYLINK
help
Say Y here if you have a Realtek Ethernet adapter belonging to
the following families:
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index c60710f9bd21..7a6c734df3d2 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -26,6 +26,7 @@
#include <linux/dma-mapping.h>
#include <linux/pm_runtime.h>
#include <linux/bitfield.h>
+#include <linux/phylink.h>
#include <linux/prefetch.h>
#include <linux/ipv6.h>
#include <linux/unaligned.h>
@@ -775,6 +776,8 @@ struct rtl8169_private {
struct r8169_led_classdev *leds;
u32 ocp_base;
+ struct phylink *phylink;
+ struct phylink_config phylink_config;
};
typedef void (*rtl_generic_fct)(struct rtl8169_private *tp);
@@ -2253,7 +2256,7 @@ static int rtl8169_get_eee(struct net_device *dev, struct ethtool_keee *data)
if (!rtl_supports_eee(tp))
return -EOPNOTSUPP;
- ret = phy_ethtool_get_eee(tp->phydev, data);
+ ret = phylink_ethtool_get_eee(tp->phylink, data);
if (ret)
return ret;
@@ -2269,7 +2272,7 @@ static int rtl8169_set_eee(struct net_device *dev, struct ethtool_keee *data)
if (!rtl_supports_eee(tp))
return -EOPNOTSUPP;
- return phy_ethtool_set_eee(tp->phydev, data);
+ return phylink_ethtool_set_eee(tp->phylink, data);
}
static void rtl8169_get_ringparam(struct net_device *dev,
@@ -2300,13 +2303,8 @@ static void rtl8169_get_pauseparam(struct net_device *dev,
struct ethtool_pauseparam *data)
{
struct rtl8169_private *tp = netdev_priv(dev);
- bool tx_pause, rx_pause;
- phy_get_pause(tp->phydev, &tx_pause, &rx_pause);
-
- data->autoneg = tp->phydev->autoneg;
- data->tx_pause = tx_pause ? 1 : 0;
- data->rx_pause = rx_pause ? 1 : 0;
+ phylink_ethtool_get_pauseparam(tp->phylink, data);
}
static int rtl8169_set_pauseparam(struct net_device *dev,
@@ -2317,9 +2315,7 @@ static int rtl8169_set_pauseparam(struct net_device *dev,
if (dev->mtu > ETH_DATA_LEN)
return -EOPNOTSUPP;
- phy_set_asym_pause(tp->phydev, data->rx_pause, data->tx_pause);
-
- return 0;
+ return phylink_ethtool_set_pauseparam(tp->phylink, data);
}
static void rtl8169_get_eth_mac_stats(struct net_device *dev,
@@ -2385,6 +2381,14 @@ static void rtl8169_get_eth_ctrl_stats(struct net_device *dev,
le32_to_cpu(tp->counters->rx_unknown_opcode);
}
+static int rtl8169_get_link_ksettings(struct net_device *ndev,
+ struct ethtool_link_ksettings *cmd)
+{
+ struct rtl8169_private *tp = netdev_priv(ndev);
+
+ return phylink_ethtool_ksettings_get(tp->phylink, cmd);
+}
+
static int rtl8169_set_link_ksettings(struct net_device *ndev,
const struct ethtool_link_ksettings *cmd)
{
@@ -2394,7 +2398,7 @@ static int rtl8169_set_link_ksettings(struct net_device *ndev,
int speed = cmd->base.speed;
if (!tp->sfp_mode)
- return phy_ethtool_ksettings_set(phydev, cmd);
+ return phylink_ethtool_ksettings_set(tp->phylink, cmd);
if (cmd->base.autoneg != AUTONEG_DISABLE)
return -EINVAL;
@@ -2415,6 +2419,13 @@ static int rtl8169_set_link_ksettings(struct net_device *ndev,
return 0;
}
+static int rtl8169_nway_reset(struct net_device *dev)
+{
+ struct rtl8169_private *tp = netdev_priv(dev);
+
+ return phylink_ethtool_nway_reset(tp->phylink);
+}
+
static const struct ethtool_ops rtl8169_ethtool_ops = {
.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
ETHTOOL_COALESCE_MAX_FRAMES,
@@ -2430,10 +2441,10 @@ static const struct ethtool_ops rtl8169_ethtool_ops = {
.get_sset_count = rtl8169_get_sset_count,
.get_ethtool_stats = rtl8169_get_ethtool_stats,
.get_ts_info = ethtool_op_get_ts_info,
- .nway_reset = phy_ethtool_nway_reset,
+ .nway_reset = rtl8169_nway_reset,
.get_eee = rtl8169_get_eee,
.set_eee = rtl8169_set_eee,
- .get_link_ksettings = phy_ethtool_get_link_ksettings,
+ .get_link_ksettings = rtl8169_get_link_ksettings,
.set_link_ksettings = rtl8169_set_link_ksettings,
.get_ringparam = rtl8169_get_ringparam,
.get_pause_stats = rtl8169_get_pause_stats,
@@ -2656,15 +2667,6 @@ static void rtl_jumbo_config(struct rtl8169_private *tp)
if (pci_is_pcie(tp->pci_dev) && tp->supports_gmii)
pcie_set_readrq(tp->pci_dev, readrq);
-
- /* Chip doesn't support pause in jumbo mode */
- if (jumbo) {
- linkmode_clear_bit(ETHTOOL_LINK_MODE_Pause_BIT,
- tp->phydev->advertising);
- linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
- tp->phydev->advertising);
- phy_start_aneg(tp->phydev);
- }
}
DECLARE_RTL_COND(rtl_chipcmd_cond)
@@ -2779,7 +2781,7 @@ static void rtl_prepare_power_down(struct rtl8169_private *tp)
rtl_ephy_write(tp, 0x19, 0xff64);
if (device_may_wakeup(tp_to_dev(tp))) {
- phy_speed_down(tp->phydev, false);
+ phylink_speed_down(tp->phylink, false);
rtl_wol_enable_rx(tp);
}
}
@@ -2831,6 +2833,16 @@ static void rtl8169_set_magic_reg(struct rtl8169_private *tp)
RTL_W32(tp, 0x7c, val);
}
+static int rtl8169_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
+{
+ struct rtl8169_private *tp = netdev_priv(dev);
+
+ if (!netif_running(dev))
+ return -ENODEV;
+
+ return phylink_mii_ioctl(tp->phylink, ifr, cmd);
+}
+
static void rtl_set_rx_mode(struct net_device *dev)
{
u32 rx_mode = AcceptBroadcast | AcceptMyPhys | AcceptMulticast;
@@ -4138,12 +4150,25 @@ static void rtl_hw_start(struct rtl8169_private *tp)
static int rtl8169_change_mtu(struct net_device *dev, int new_mtu)
{
struct rtl8169_private *tp = netdev_priv(dev);
+ bool jumbo_before = dev->mtu > ETH_DATA_LEN;
+ bool jumbo_after = new_mtu > ETH_DATA_LEN;
WRITE_ONCE(dev->mtu, new_mtu);
netdev_update_features(dev);
rtl_jumbo_config(tp);
rtl_set_eee_txidle_timer(tp);
+ if (jumbo_before != jumbo_after) {
+ unsigned long caps = tp->phylink_config.mac_capabilities;
+
+ if (jumbo_after)
+ caps &= ~(MAC_SYM_PAUSE | MAC_ASYM_PAUSE);
+ else
+ caps |= (MAC_SYM_PAUSE | MAC_ASYM_PAUSE);
+
+ phylink_update_mac_pause_capabilities(tp->phylink, caps);
+ }
+
return 0;
}
@@ -4929,9 +4954,6 @@ static int rtl8169_poll(struct napi_struct *napi, int budget)
static void rtl_enable_tx_lpi(struct rtl8169_private *tp, bool enable)
{
- if (!rtl_supports_eee(tp))
- return;
-
switch (tp->mac_version) {
case RTL_GIGA_MAC_VER_34 ... RTL_GIGA_MAC_VER_52:
/* Adjust EEE LED frequency */
@@ -4962,41 +4984,15 @@ static void rtl_enable_tx_lpi(struct rtl8169_private *tp, bool enable)
}
}
-static void r8169_phylink_handler(struct net_device *ndev)
-{
- struct rtl8169_private *tp = netdev_priv(ndev);
- struct device *d = tp_to_dev(tp);
-
- tp->speed = tp->phydev->speed;
- if (netif_carrier_ok(ndev)) {
- rtl_link_chg_patch(tp, tp->speed);
- rtl_enable_tx_lpi(tp, tp->phydev->enable_tx_lpi);
- pm_request_resume(d);
- } else {
- pm_runtime_idle(d);
- }
-
- phy_print_status(tp->phydev);
-}
-
static int r8169_phy_connect(struct rtl8169_private *tp)
{
- struct phy_device *phydev = tp->phydev;
- phy_interface_t phy_mode;
int ret;
- phy_mode = tp->supports_gmii ? PHY_INTERFACE_MODE_GMII :
- PHY_INTERFACE_MODE_MII;
-
- ret = phy_connect_direct(tp->dev, phydev, r8169_phylink_handler,
- phy_mode);
- if (ret)
+ ret = phylink_connect_phy(tp->phylink, tp->phydev);
+ if (ret) {
+ netdev_err(tp->dev, "failed to connect phy\n");
return ret;
-
- if (!tp->supports_gmii)
- phy_set_max_speed(phydev, SPEED_100);
-
- phy_attached_info(phydev);
+ }
return 0;
}
@@ -5007,8 +5003,6 @@ static void rtl8169_down(struct rtl8169_private *tp)
/* Clear all task flags */
bitmap_zero(tp->wk.flags, RTL_FLAG_MAX);
- phy_stop(tp->phydev);
-
/* Reset SerDes PHY to bring down fiber link */
if (tp->sfp_mode)
rtl_sfp_reset(tp);
@@ -5038,8 +5032,6 @@ static void rtl8169_up(struct rtl8169_private *tp)
napi_enable(&tp->napi);
enable_work(&tp->wk.work);
rtl_reset_work(tp);
-
- phy_start(tp->phydev);
}
static int rtl8169_close(struct net_device *dev)
@@ -5049,13 +5041,14 @@ static int rtl8169_close(struct net_device *dev)
pm_runtime_get_sync(&pdev->dev);
+ phylink_stop(tp->phylink);
netif_stop_queue(dev);
rtl8169_down(tp);
rtl8169_rx_clear(tp);
free_irq(tp->irq, tp);
- phy_disconnect(tp->phydev);
+ phylink_disconnect_phy(tp->phylink);
dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
tp->RxPhyAddr);
@@ -5117,6 +5110,7 @@ static int rtl_open(struct net_device *dev)
goto err_free_irq;
rtl8169_up(tp);
+ phylink_start(tp->phylink);
rtl8169_init_counter_offsets(tp);
netif_start_queue(dev);
out:
@@ -5288,6 +5282,7 @@ static void rtl_remove_one(struct pci_dev *pdev)
r8169_remove_leds(tp->leds);
unregister_netdev(tp->dev);
+ phylink_destroy(tp->phylink);
if (tp->dash_type != RTL_DASH_NONE)
rtl8168_driver_stop(tp);
@@ -5310,7 +5305,7 @@ static const struct net_device_ops rtl_netdev_ops = {
.ndo_fix_features = rtl8169_fix_features,
.ndo_set_features = rtl8169_set_features,
.ndo_set_mac_address = rtl_set_mac_address,
- .ndo_eth_ioctl = phy_do_ioctl_running,
+ .ndo_eth_ioctl = rtl8169_ioctl,
.ndo_set_rx_mode = rtl_set_rx_mode,
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller = rtl8169_netpoll,
@@ -5474,16 +5469,6 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
return -EUNATCH;
}
- tp->phydev->mac_managed_pm = true;
- if (rtl_supports_eee(tp))
- phy_support_eee(tp->phydev);
- phy_support_asym_pause(tp->phydev);
-
- /* mimic behavior of r8125/r8126 vendor drivers */
- if (tp->mac_version == RTL_GIGA_MAC_VER_61)
- phy_disable_eee_mode(tp->phydev,
- ETHTOOL_LINK_MODE_2500baseT_Full_BIT);
-
/* PHY will be woken up in rtl_open() */
phy_suspend(tp->phydev);
@@ -5599,6 +5584,136 @@ static bool rtl_aspm_is_safe(struct rtl8169_private *tp)
return false;
}
+static void rtl_mac_link_down(struct phylink_config *config, unsigned int mode,
+ phy_interface_t interface)
+{
+ struct rtl8169_private *tp = container_of(config, struct rtl8169_private, phylink_config);
+
+ tp->speed = SPEED_UNKNOWN;
+ pm_runtime_idle(tp_to_dev(tp));
+}
+
+static void rtl_mac_link_up(struct phylink_config *config, struct phy_device *phydev,
+ unsigned int mode, phy_interface_t interface,
+ int speed, int duplex, bool tx_pause, bool rx_pause)
+{
+ struct rtl8169_private *tp = container_of(config, struct rtl8169_private, phylink_config);
+ struct device *d = tp_to_dev(tp);
+
+ tp->speed = speed;
+ rtl_link_chg_patch(tp, speed);
+
+ pm_request_resume(d);
+}
+
+static struct phylink_pcs *rtl_mac_select_pcs(struct phylink_config *config,
+ phy_interface_t interface)
+{
+ return NULL;
+}
+
+static void rtl_mac_config(struct phylink_config *config, unsigned int mode,
+ const struct phylink_link_state *state)
+{
+}
+
+static void rtl_mac_disable_tx_lpi(struct phylink_config *config)
+{
+ struct rtl8169_private *tp = container_of(config, struct rtl8169_private, phylink_config);
+
+ rtl_enable_tx_lpi(tp, false);
+}
+
+static int rtl_mac_enable_tx_lpi(struct phylink_config *config, u32 timer, bool tx_clk_stop)
+{
+ struct rtl8169_private *tp = container_of(config, struct rtl8169_private, phylink_config);
+
+ rtl_enable_tx_lpi(tp, true);
+
+ return 0;
+}
+
+static const struct phylink_mac_ops rtl_phylink_mac_ops = {
+ .mac_select_pcs = rtl_mac_select_pcs,
+ .mac_config = rtl_mac_config,
+ .mac_link_down = rtl_mac_link_down,
+ .mac_link_up = rtl_mac_link_up,
+ .mac_disable_tx_lpi = rtl_mac_disable_tx_lpi,
+ .mac_enable_tx_lpi = rtl_mac_enable_tx_lpi,
+};
+
+static unsigned long rtl8169_get_lpi_caps(struct rtl8169_private *tp)
+{
+ unsigned long caps = 0;
+
+ if (!rtl_supports_eee(tp))
+ return 0;
+
+ caps |= MAC_100FD | MAC_1000FD;
+
+ /* mimic behavior of r8125/r8126 vendor drivers
+ * RTL_GIGA_MAC_VER_61 doesn't support 2.5G eee
+ */
+ if (tp->mac_version >= RTL_GIGA_MAC_VER_63)
+ caps |= MAC_2500FD;
+ if (tp->mac_version >= RTL_GIGA_MAC_VER_70)
+ caps |= MAC_5000FD;
+ if (tp->mac_version == RTL_GIGA_MAC_VER_80)
+ caps |= MAC_10000FD;
+
+ return caps;
+}
+
+static int rtl_init_phylink(struct rtl8169_private *tp)
+{
+ struct phylink *pl;
+ phy_interface_t phy_mode;
+
+ tp->phylink_config.dev = &tp->dev->dev;
+ tp->phylink_config.type = PHYLINK_NETDEV;
+ tp->phylink_config.mac_managed_pm = true;
+ tp->phylink_config.lpi_capabilities = rtl8169_get_lpi_caps(tp);
+ tp->phylink_config.mac_capabilities |= MAC_ASYM_PAUSE | MAC_SYM_PAUSE;
+
+ if (tp->sfp_mode) {
+ phy_mode = PHY_INTERFACE_MODE_INTERNAL;
+ tp->phylink_config.mac_capabilities |= MAC_10000FD;
+ } else {
+ tp->phylink_config.mac_capabilities |= MAC_10 | MAC_100;
+
+ if (tp->mac_version == RTL_GIGA_MAC_VER_80)
+ tp->phylink_config.mac_capabilities |= MAC_1000FD | MAC_2500FD |
+ MAC_5000FD | MAC_10000FD;
+ else if (tp->mac_version == RTL_GIGA_MAC_VER_70)
+ tp->phylink_config.mac_capabilities |= MAC_1000FD |
+ MAC_2500FD | MAC_5000FD;
+ else if (tp->mac_version >= RTL_GIGA_MAC_VER_61)
+ tp->phylink_config.mac_capabilities |= MAC_1000FD | MAC_2500FD;
+ else
+ if (tp->supports_gmii)
+ tp->phylink_config.mac_capabilities |= MAC_1000FD;
+
+ if (tp->mac_version < RTL_GIGA_MAC_VER_61)
+ phy_mode = tp->supports_gmii ? PHY_INTERFACE_MODE_GMII :
+ PHY_INTERFACE_MODE_MII;
+ else
+ phy_mode = PHY_INTERFACE_MODE_INTERNAL;
+ }
+
+ __set_bit(phy_mode, tp->phylink_config.supported_interfaces);
+ if (tp->phylink_config.lpi_capabilities)
+ __set_bit(phy_mode, tp->phylink_config.lpi_interfaces);
+
+ pl = phylink_create(&tp->phylink_config, tp_to_dev(tp)->fwnode,
+ phy_mode, &rtl_phylink_mac_ops);
+ if (IS_ERR(pl))
+ return PTR_ERR(pl);
+
+ tp->phylink = pl;
+
+ return 0;
+}
+
static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
{
const struct rtl_chip_info *chip;
@@ -5789,13 +5904,21 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
pci_set_drvdata(pdev, tp);
- rc = r8169_mdio_register(tp);
+ rc = rtl_init_phylink(tp);
if (rc)
return rc;
+ rc = r8169_mdio_register(tp);
+ if (rc) {
+ phylink_destroy(tp->phylink);
+ return rc;
+ }
+
rc = register_netdev(dev);
- if (rc)
+ if (rc) {
+ phylink_destroy(tp->phylink);
return rc;
+ }
if (IS_ENABLED(CONFIG_R8169_LEDS)) {
if (rtl_is_8125(tp))
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v6 2/7] net: phy: phylink: add helper to modify pause
From: javen @ 2026-07-09 10:02 UTC (permalink / raw)
To: hkallweit1, nic_swsd, andrew+netdev, davem, edumazet, kuba,
pabeni, maxime.chevallier, horms
Cc: netdev, linux-kernel, daniel, linux, enelsonmoore, daniel,
Javen Xu
In-Reply-To: <20260709100237.541-1-javen_xu@realsil.com.cn>
From: Javen Xu <javen_xu@realsil.com.cn>
For Realtek nics, when we enable jumbo, pause are not supported. So we
must check the pause capabilities from ourself and lp.
Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
Changes in v5:
- no changes, new file
Changes in v6:
- rename phylink_update_mac_pause_capabilities(), this function only
changes mac pause capability
- set asym pause and pause according to config->pause tx and rx
- add phydev->lock when change pl->phydev->advertising
---
drivers/net/phy/phylink.c | 87 +++++++++++++++++++++++++++++++++++++++
include/linux/phylink.h | 2 +
2 files changed, 89 insertions(+)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 59dfe35afa54..9e9cd79301d6 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1828,6 +1828,93 @@ int phylink_set_fixed_link(struct phylink *pl,
}
EXPORT_SYMBOL_GPL(phylink_set_fixed_link);
+/**
+ * phylink_update_mac_pause_capabilities() - Dynamically update MAC pause
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ * @mac_pause: the new MAC pause capabilities mask
+ *
+ * This function allows a MAC driver to dynamically change its pause state,
+ * such as losing/gaining Pause frame support based on MTU size.
+ * It recalculates supported link modes and triggers renegotiation if needed.
+ */
+void phylink_update_mac_pause_capabilities(struct phylink *pl, unsigned long mac_pause)
+{
+ struct phylink_link_state *config = &pl->link_config;
+ unsigned long old_pause, caps_added, caps_removed;
+ bool pause_adv, asym_adv;
+
+ ASSERT_RTNL();
+
+ if (mac_pause & ~(MAC_SYM_PAUSE | MAC_ASYM_PAUSE)) {
+ phylink_err(pl, "Attempted to dynamically change non-pause MAC capabilities\n");
+ return;
+ }
+
+ old_pause = pl->config->mac_capabilities & (MAC_SYM_PAUSE | MAC_ASYM_PAUSE);
+ caps_added = mac_pause & ~old_pause;
+ caps_removed = old_pause & ~mac_pause;
+
+ if (!caps_added && !caps_removed)
+ return;
+
+ mutex_lock(&pl->state_mutex);
+
+ pl->config->mac_capabilities &= ~(MAC_SYM_PAUSE | MAC_ASYM_PAUSE);
+ pl->config->mac_capabilities |= mac_pause;
+
+ if (caps_removed & MAC_SYM_PAUSE)
+ linkmode_clear_bit(ETHTOOL_LINK_MODE_Pause_BIT, pl->supported);
+ if (caps_removed & MAC_ASYM_PAUSE)
+ linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, pl->supported);
+
+ linkmode_and(config->advertising, config->advertising, pl->supported);
+
+ if (caps_added & MAC_SYM_PAUSE) {
+ linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, pl->supported);
+ if (pl->phydev && !phylink_test(pl->phydev->supported, Pause))
+ linkmode_clear_bit(ETHTOOL_LINK_MODE_Pause_BIT, pl->supported);
+ }
+ if (caps_added & MAC_ASYM_PAUSE) {
+ linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, pl->supported);
+ if (pl->phydev && !phylink_test(pl->phydev->supported, Asym_Pause))
+ linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, pl->supported);
+ }
+
+ linkmode_and(config->advertising, config->advertising, pl->supported);
+
+ if (config->pause & MLO_PAUSE_AN) {
+ if (phylink_test(pl->supported, Pause) &&
+ (config->pause & MLO_PAUSE_RX) && (config->pause & MLO_PAUSE_TX))
+ linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, config->advertising);
+
+ if (phylink_test(pl->supported, Asym_Pause) &&
+ ((config->pause & MLO_PAUSE_RX) ^ (config->pause & MLO_PAUSE_TX)))
+ linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, config->advertising);
+ }
+
+ if (!pl->phydev)
+ phylink_change_inband_advert(pl);
+
+ mutex_unlock(&pl->state_mutex);
+
+ if (pl->phydev) {
+ pause_adv = linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
+ config->advertising);
+ asym_adv = linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+ config->advertising);
+
+ mutex_lock(&pl->phydev->lock);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_Pause_BIT,
+ pl->phydev->advertising, pause_adv);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+ pl->phydev->advertising, asym_adv);
+ mutex_unlock(&pl->phydev->lock);
+
+ phy_start_aneg(pl->phydev);
+ }
+}
+EXPORT_SYMBOL_GPL(phylink_update_mac_pause_capabilities);
+
/**
* phylink_create() - create a phylink instance
* @config: a pointer to the target &struct phylink_config
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 2bc0db3d52ac..e3c7822100ab 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -842,4 +842,6 @@ void phylink_replay_link_begin(struct phylink *pl);
void phylink_replay_link_end(struct phylink *pl);
+void phylink_update_mac_pause_capabilities(struct phylink *pl, unsigned long mac_pause);
+
#endif
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v6 0/7] r8169: add support for phylink
From: javen @ 2026-07-09 10:02 UTC (permalink / raw)
To: hkallweit1, nic_swsd, andrew+netdev, davem, edumazet, kuba,
pabeni, maxime.chevallier, horms
Cc: netdev, linux-kernel, daniel, linux, enelsonmoore, daniel,
Javen Xu
From: Javen Xu <javen_xu@realsil.com.cn>
This series patch adds support for phylink. RTL8116af is a fiber mode
card, link status and speed can not be read from standard phy reg. So
we read link status and speed from serdes reg by pcs. So as RTL8127atf.
Javen Xu (7):
r8169: add speed in private struct
net: phy: phylink: add helper to modify pause
r8169: add support for phylink
r8169: add support for RTL8116af
r8169: add support for RTL8127atf
r8169: add ltr support for RTL8117 series
r8169: fix RTL8116af can not enter s0idle and c10
drivers/net/ethernet/realtek/Kconfig | 2 +-
drivers/net/ethernet/realtek/r8169_main.c | 673 +++++++++++++++++-----
drivers/net/phy/phylink.c | 87 +++
drivers/net/phy/realtek/realtek_main.c | 54 --
include/linux/phylink.h | 2 +
include/net/phy/realtek_phy.h | 7 -
6 files changed, 619 insertions(+), 206 deletions(-)
delete mode 100644 include/net/phy/realtek_phy.h
--
2.43.0
^ permalink raw reply
* [PATCH v2 1/1] macvlan: allow source mode devices along with passthru
From: Thomas Martitz @ 2026-07-09 10:05 UTC (permalink / raw)
To: Simon Horman, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, open list:NETWORKING DRIVERS,
open list
Cc: Thomas Martitz, open list:NETWORKING DRIVERS, open list
In-Reply-To: <20260709100512.1383421-1-t.martitz@fritz.com>
This allows for configurations where there are a few
known senders in the system (e.g. multiple SoCs on the same
board) along with unlimited external senders.
The source mode devices represent the known senders while
all external senders terminate on passthru device.
Although you could still receive packets on the lower device
without the need for the passthru vlan device, there
are use cases where you need additional packet processing
in the pipeline that hooks via rx_handler. But the rx_handler
is already bound to the macvlan port.
With this patch the rx_handler can be attached to the passthru
device while macvlan itself remains attached to the lower device.
We use this to use the share the only available high-speed interface
for inter-SoC networking and external networking (user traffic).
Inter-SoC packets are received on the source mode interface, identified
by a well-known source address. Some of our chips have simply no other
viable link for inter-SoC traffic.
Signed-off-by: Thomas Martitz <t.martitz@fritz.com>
---
drivers/net/macvlan.c | 107 ++++++++++++++++++++++++++++--------------
1 file changed, 73 insertions(+), 34 deletions(-)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 9a4bc99dbf53b..2505f5f2e9d30 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -83,6 +83,11 @@ static inline void macvlan_set_passthru(struct macvlan_port *port)
port->flags |= MACVLAN_F_PASSTHRU;
}
+static inline void macvlan_clear_passthru(struct macvlan_port *port)
+{
+ port->flags &= ~MACVLAN_F_PASSTHRU;
+}
+
static inline bool macvlan_addr_change(const struct macvlan_port *port)
{
return port->flags & MACVLAN_F_ADDRCHANGE;
@@ -637,7 +642,7 @@ static int macvlan_open(struct net_device *dev)
struct net_device *lowerdev = vlan->lowerdev;
int err;
- if (macvlan_passthru(vlan->port)) {
+ if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
if (!(vlan->flags & MACVLAN_FLAG_NOPROMISC)) {
err = dev_set_promiscuity(lowerdev, 1);
if (err < 0)
@@ -712,7 +717,7 @@ static int macvlan_stop(struct net_device *dev)
dev_uc_unsync(lowerdev, dev);
dev_mc_unsync(lowerdev, dev);
- if (macvlan_passthru(vlan->port)) {
+ if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
if (!(vlan->flags & MACVLAN_FLAG_NOPROMISC))
dev_set_promiscuity(lowerdev, -1);
goto hash_del;
@@ -792,6 +797,23 @@ static int macvlan_set_mac_address(struct net_device *dev, void *p)
return macvlan_sync_address(dev, addr->__data);
}
+static void macvlan_port_release_mac(struct net_device *dev)
+{
+ struct macvlan_port *port = macvlan_port_get_rtnl(dev);
+
+ /* If the lower device address has been changed by passthru
+ * macvlan, put it back.
+ */
+ if (macvlan_passthru(port) &&
+ !ether_addr_equal(port->dev->dev_addr, port->perm_addr)) {
+ struct sockaddr_storage ss;
+
+ ss.ss_family = port->dev->type;
+ memcpy(&ss.__data, port->perm_addr, port->dev->addr_len);
+ dev_set_mac_address(port->dev, &ss, NULL);
+ }
+}
+
static void macvlan_change_rx_flags(struct net_device *dev, int change)
{
struct macvlan_dev *vlan = netdev_priv(dev);
@@ -977,8 +999,18 @@ static void macvlan_uninit(struct net_device *dev)
macvlan_flush_sources(port, vlan);
port->count -= 1;
- if (!port->count)
- macvlan_port_destroy(port->dev);
+ if (port->count) {
+ /* In case of remaining source interfaces undo
+ * passthru-specific properties.
+ */
+ if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
+ macvlan_port_release_mac(dev);
+ macvlan_clear_passthru(vlan->port);
+ }
+ return;
+ }
+
+ macvlan_port_destroy(port->dev);
}
static void macvlan_dev_get_stats64(struct net_device *dev,
@@ -1052,7 +1084,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
/* Support unicast filter only on passthru devices.
* Multicast filter should be allowed on all devices.
*/
- if (!macvlan_passthru(vlan->port) && is_unicast_ether_addr(addr))
+ if (vlan->mode != MACVLAN_MODE_PASSTHRU && is_unicast_ether_addr(addr))
return -EOPNOTSUPP;
if (flags & NLM_F_REPLACE)
@@ -1077,7 +1109,7 @@ static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
/* Support unicast filter only on passthru devices.
* Multicast filter should be allowed on all devices.
*/
- if (!macvlan_passthru(vlan->port) && is_unicast_ether_addr(addr))
+ if (vlan->mode != MACVLAN_MODE_PASSTHRU && is_unicast_ether_addr(addr))
return -EOPNOTSUPP;
if (is_unicast_ether_addr(addr))
@@ -1308,17 +1340,7 @@ static void macvlan_port_destroy(struct net_device *dev)
kfree_skb(skb);
}
- /* If the lower device address has been changed by passthru
- * macvlan, put it back.
- */
- if (macvlan_passthru(port) &&
- !ether_addr_equal(port->dev->dev_addr, port->perm_addr)) {
- struct sockaddr_storage ss;
-
- ss.ss_family = port->dev->type;
- memcpy(&ss.__data, port->perm_addr, port->dev->addr_len);
- dev_set_mac_address(port->dev, &ss, NULL);
- }
+ macvlan_port_release_mac(dev);
kfree(port);
}
@@ -1506,15 +1528,6 @@ int macvlan_common_newlink(struct net_device *dev,
}
port = macvlan_port_get_rtnl(lowerdev);
- /* Only 1 macvlan device can be created in passthru mode */
- if (macvlan_passthru(port)) {
- /* The macvlan port must be not created this time,
- * still goto destroy_macvlan_port for readability.
- */
- err = -EINVAL;
- goto destroy_macvlan_port;
- }
-
vlan->lowerdev = lowerdev;
vlan->dev = dev;
vlan->port = port;
@@ -1527,10 +1540,30 @@ int macvlan_common_newlink(struct net_device *dev,
if (data && data[IFLA_MACVLAN_FLAGS])
vlan->flags = nla_get_u16(data[IFLA_MACVLAN_FLAGS]);
+ /* Only 1 macvlan device can be created in passthru mode. There may be
+ * additional source mode devices but nothing else at the moment.
+ *
+ * First check if adding a source mode device to an existing passthru vlan.
+ */
+ if (macvlan_passthru(port) && vlan->mode != MACVLAN_MODE_SOURCE) {
+ /* The macvlan port must be not created this time,
+ * still goto destroy_macvlan_port for readability.
+ */
+ err = -EINVAL;
+ goto destroy_macvlan_port;
+ }
+
+ /* Now check if adding a passthru device to an existing set of source mode
+ * devices.
+ */
if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
- if (port->count) {
- err = -EINVAL;
- goto destroy_macvlan_port;
+ struct macvlan_dev *p;
+
+ list_for_each_entry(p, &port->vlans, list) {
+ if (p->mode != MACVLAN_MODE_SOURCE) {
+ err = -EINVAL;
+ goto destroy_macvlan_port;
+ }
}
macvlan_set_passthru(port);
eth_hw_addr_inherit(dev, lowerdev);
@@ -1564,7 +1597,11 @@ int macvlan_common_newlink(struct net_device *dev,
if (err)
goto unregister_netdev;
- list_add_tail_rcu(&vlan->list, &port->vlans);
+ /* macvlan_handle_frame expects the (one and only) passthru device first. */
+ if (vlan->mode == MACVLAN_MODE_PASSTHRU)
+ list_add_rcu(&vlan->list, &port->vlans);
+ else
+ list_add_tail_rcu(&vlan->list, &port->vlans);
update_port_bc_queue_len(vlan->port);
netif_stacked_transfer_operstate(lowerdev, dev);
linkwatch_fire_event(dev);
@@ -1627,9 +1664,11 @@ static int macvlan_changelink(struct net_device *dev,
if (data && data[IFLA_MACVLAN_MODE]) {
set_mode = true;
mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
- /* Passthrough mode can't be set or cleared dynamically */
- if ((mode == MACVLAN_MODE_PASSTHRU) !=
- (vlan->mode == MACVLAN_MODE_PASSTHRU))
+ /* Passthrough mode can't be set or cleared dynamically,
+ * regardless of existing source interfaces. Furthermore, source
+ * interfaces can't switch modes within a passhtrough port.
+ */
+ if (macvlan_passthru(vlan->port) && mode != vlan->mode)
return -EINVAL;
if (vlan->mode == MACVLAN_MODE_SOURCE &&
vlan->mode != mode)
@@ -1639,7 +1678,7 @@ static int macvlan_changelink(struct net_device *dev,
if (data && data[IFLA_MACVLAN_FLAGS]) {
__u16 flags = nla_get_u16(data[IFLA_MACVLAN_FLAGS]);
bool promisc = (flags ^ vlan->flags) & MACVLAN_FLAG_NOPROMISC;
- if (macvlan_passthru(vlan->port) && promisc) {
+ if (vlan->mode == MACVLAN_MODE_PASSTHRU && promisc) {
int err;
if (flags & MACVLAN_FLAG_NOPROMISC)
--
2.54.0
^ permalink raw reply related
* [PATCH v2 0/1] macvlan: allow source mode devices along with passthru
From: Thomas Martitz @ 2026-07-09 10:05 UTC (permalink / raw)
To: Simon Horman, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, open list:NETWORKING DRIVERS,
open list
Cc: Thomas Martitz, open list:NETWORKING DRIVERS, open list
Hello,
we're trying to solve a use case on our devices where two SoC are
connected on the same board, using the only available high-speed interface.
One SoC runs the main Linux system including the full routing stack
(FRITZ!OS) and the other SoC implements most of the GPON ONT side.
The high-speed interface is of course also used for the user traffic.
Therefore we must tell the inter-SoC traffic apart from the user traffic.
We achieve this by matching the well-known MAC address of the ONT SoC.
The user traffic passes through the ONT SoC without modifying MAC headers.
Now we would like to use macvlan (with source mode devices) on the main
SoC side for this but our routing stack requires the rx_handler to be
available. Therefore macvlan is currently not an option.
With this patch macvlan becomes an option because the current limitation
of either "one passthru device" or "any other configuration" is relaxed
for the combination of passthru and any number of source mode devices.
This allows us to configure a source mode device for the other SoC and
register an rx_handler for further processing on the passthru device.
Thanks in advance!
Thomas Martitz (1):
macvlan: allow source mode devices along with passthru
drivers/net/macvlan.c | 107 ++++++++++++++++++++++++++++--------------
1 file changed, 73 insertions(+), 34 deletions(-)
--
2.54.0
^ permalink raw reply
* Re: [PATCH net-next v7 5/5] veth: time-based BQL completion coalescing via ethtool tx-usecs
From: Jonas Köppeler @ 2026-07-09 10:03 UTC (permalink / raw)
To: Simon Schippers, hawk, netdev
Cc: kernel-team, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
John Fastabend, Stanislav Fomichev, linux-kernel, bpf
In-Reply-To: <9205d83f-2c9e-4abb-91a0-488704bcdef3@tu-dortmund.de>
On 6/30/26 21:07, Simon Schippers wrote:
> On 6/30/26 16:00, Jonas Köppeler wrote:
>> On 6/13/26 4:14 PM, Simon Schippers wrote:
>>> On 6/12/26 10:35, hawk@kernel.org wrote:
>>>> From: Simon Schippers <simon.schippers@tu-dortmund.de>
>>>>
>>>> Per-packet BQL completion forces DQL to converge on limit=2, causing
>>>> excessive NAPI scheduling overhead and qdisc requeues.
>>>>
>>>> Accumulate BQL completions and flush them when a configurable time
>>>> threshold (tx-usecs) is exceeded, letting DQL discover a limit that
>>>> bounds actual queuing delay to the configured interval. Coalescing
>>>> state persists across NAPI polls in struct veth_rq so completions can
>>>> accumulate beyond a single budget=64 cycle.
>>>>
>>>> The flush condition is:
>>>>
>>>> state->time + bql_flush_ns <= current_time || state->n_bql > dql.limit
>>>>
>>>> Flushing when n_bql exceeds dql.limit handles BQL starvation.
>>>>
>>>> The comparison is strictly greater-than because netdev_tx_sent_queue()
>>>> always lets the producer exceed the limit by one before it stops, so
>>>> n_bql == dql.limit is a normal in-flight state. dql.limit lives in
>>>> the same cacheline as the completion path, so the check is cheap.
>>>>
>>>> Add ethtool tx-usecs support for runtime tuning. Default is 100 us;
>>>> setting tx-usecs to 0 disables coalescing and falls back to per-packet
>>>> completion.
>>>>
>>>> ethtool -C <veth-dev> tx-usecs 500 # 500us coalescing
>>>> ethtool -C <veth-dev> tx-usecs 0 # per-packet (no coalescing)
>>>>
>>>> Co-developed-by: Jesper Dangaard Brouer <hawk@kernel.org>
>>>> Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
>>>> Co-developed-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
>>>> Signed-off-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
>>>> Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
>>>> ---
>>>> drivers/net/veth.c | 123 ++++++++++++++++++++++++++++++++++++++++++---
>>>> 1 file changed, 117 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
>>>> index 2473f730734b..c62d87a8402c 100644
>>>> --- a/drivers/net/veth.c
>>>> +++ b/drivers/net/veth.c
>>>> @@ -28,6 +28,7 @@
>>>> #include <linux/bpf_trace.h>
>>>> #include <linux/net_tstamp.h>
>>>> #include <linux/skbuff_ref.h>
>>>> +#include <linux/sched/clock.h>
>>>> #include <net/page_pool/helpers.h>
>>>> #define DRV_NAME "veth"
>>>> @@ -50,6 +51,7 @@
>>>> * delay => 64 * 250 ms = 16 s.
>>>> */
>>>> #define VETH_WATCHDOG_TIMEOUT_MS (64 * 250)
>>>> +#define VETH_BQL_COAL_TX_USECS 100 /* default tx-usecs for BQL batching*/
>>>> struct veth_stats {
>>>> u64 rx_drops;
>>>> @@ -69,6 +71,11 @@ struct veth_rq_stats {
>>>> struct u64_stats_sync syncp;
>>>> };
>>>> +struct veth_bql_state {
>>>> + u64 time; /* sched_clock() when current coalescing window started */
>>>> + uint n_bql; /* BQL completions batched in the current window */
>>>> +};
>>>> +
>>>> struct veth_rq {
>>>> struct napi_struct xdp_napi;
>>>> struct napi_struct __rcu *napi; /* points to xdp_napi when the latteris initialized */
>>>> @@ -76,6 +83,7 @@ struct veth_rq {
>>>> struct bpf_prog __rcu *xdp_prog;
>>>> struct xdp_mem_info xdp_mem;
>>>> struct veth_rq_stats stats;
>>>> + struct veth_bql_state bql_state;
>>>> bool rx_notify_masked;
>>>> struct ptr_ring xdp_ring;
>>>> struct xdp_rxq_info xdp_rxq;
>>>> @@ -88,6 +96,7 @@ struct veth_priv {
>>>> struct bpf_prog *_xdp_prog;
>>>> struct veth_rq *rq;
>>>> unsigned int requested_headroom;
>>>> + unsigned int tx_coal_usecs; /* BQL completion coalescing */
>>>> };
>>>> struct veth_xdp_tx_bq {
>>>> @@ -272,7 +281,56 @@ static void veth_get_channels(struct net_device *dev,
>>>> static int veth_set_channels(struct net_device *dev,
>>>> struct ethtool_channels *ch);
>>>> +static int veth_get_coalesce(struct net_device *dev,
>>>> + struct ethtool_coalesce *ec,
>>>> + struct kernel_ethtool_coalesce *kernel_coal,
>>>> + struct netlink_ext_ack *extack)
>>>> +{
>>>> + struct veth_priv *priv = netdev_priv(dev);
>>>> +
>>>> + ec->tx_coalesce_usecs = priv->tx_coal_usecs;
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int veth_set_coalesce(struct net_device *dev,
>>>> + struct ethtool_coalesce *ec,
>>>> + struct kernel_ethtool_coalesce *kernel_coal,
>>>> + struct netlink_ext_ack *extack)
>>>> +{
>>>> + struct veth_priv *priv = netdev_priv(dev);
>>>> + struct net_device *peer;
>>>> +
>>>> + /* The coalescing window delays BQL completions, so keep tx-usecs well
>>>> + * below the tx_timeout watchdog; otherwise a large value could stall a
>>>> + * stopped queue long enough to trip a false watchdog timeout. Cap at
>>>> + * half the watchdog to leave a generous safety margin. tx-usecs is
>>>> + * microseconds, the watchdog is milliseconds.
>>>> + */
>>>> + if (ec->tx_coalesce_usecs > VETH_WATCHDOG_TIMEOUT_MS / 2 * USEC_PER_MSEC) {
>>>> + NL_SET_ERR_MSG_MOD(extack,
>>>> + "tx-usecs must stay below half the tx_timeout watchdog");
>>>> + return -ERANGE;
>>>> + }
>>>> +
>>>> + /* Paired with READ_ONCE in veth_xdp_rcv(). */
>>>> + WRITE_ONCE(priv->tx_coal_usecs, ec->tx_coalesce_usecs);
>>>> +
>>>> + /* veth_xdp_rcv() reads each device's own value, so mirror it onto
>>>> + * the peer to keep the pair symmetric: both directions coalesce
>>>> + * with the same tx-usecs. Called under RTNL, rtnl_dereference() is safe.
>>>> + */
>>>> + peer = rtnl_dereference(priv->peer);
>>>> + if (peer) {
>>>> + struct veth_priv *peer_priv = netdev_priv(peer);
>>>> +
>>>> + WRITE_ONCE(peer_priv->tx_coal_usecs, ec->tx_coalesce_usecs);
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> static const struct ethtool_ops veth_ethtool_ops = {
>>>> + .supported_coalesce_params = ETHTOOL_COALESCE_TX_USECS,
>>>> .get_drvinfo = veth_get_drvinfo,
>>>> .get_link = ethtool_op_get_link,
>>>> .get_strings = veth_get_strings,
>>>> @@ -282,6 +340,8 @@ static const struct ethtool_ops veth_ethtool_ops ={
>>>> .get_ts_info = ethtool_op_get_ts_info,
>>>> .get_channels = veth_get_channels,
>>>> .set_channels = veth_set_channels,
>>>> + .get_coalesce = veth_get_coalesce,
>>>> + .set_coalesce = veth_set_coalesce,
>>>> };
>>>> /* general routines */
>>>> @@ -969,13 +1029,54 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,
>>>> return NULL;
>>>> }
>>>> +static void veth_bql_maybe_complete(struct veth_bql_state *state,
>>>> + struct netdev_queue *peer_txq,
>>>> + u64 bql_flush_ns)
>>>> +{
>>>> + u64 current_time;
>>>> +
>>>> + /* There is no reason to complete with 0 and
>>>> + * peer_txq could go away.
>>>> + */
>>>> + if (!state->n_bql || !peer_txq)
>>>> + return;
>>>> +
>>>> + current_time = sched_clock();
>>>> +
>>>> + /* We complete if:
>>>> + * 1. We reach bql_flush_ns.
>>>> + * 2. We potentially have BQL starvation.
>>>> + */
>>>> + if (state->time + bql_flush_ns <= current_time ||
>>>> + state->n_bql > peer_txq->dql.limit) {
>>>
>> Indeed, this does not compile when CONFIG_BQL is not set. I think we should just bring back the 'queue is empty + queue is stopped' check from v6 back at the end of the poll and remove the n_bql > dql.limit check.
>
> We would put #ifdef CONFIG_BQL around that logic aswell.
>
>> It also feels not obvious why this is handling the starvation case. This only works, because the producer has went overlimit previously and was stopped. So more than 'limit' packets have been enqueued to the ring, and they are eventually drained when this check is true.
>
> I think it just needs some comment tweaking:
>
> /* We complete if:
> * 1. We reach bql_flush_ns.
> * 2. We have BQL starvation. This means that the queue was over-limit
> * in the last interval, and there is no more data in the queue,
> * which is equivalent to we consumed more than limit items.
> */
>
>> By removing this we can also avoid accessing dql internal members, but if you don't think that's a problem we can leave as is.
>
> I agree accessing dql internal variables is not perfect.
>
> That is why I have locally implemented DQL for software interfaces in
> a generic way inside dynamic_queue_limits.{h,c}.
> I was able to squeeze the time and n_bql variables into the completion
> cacheline of the dql struct by moving around variables.
> The logic applies inside dql_completed() if enabled.
> With this we just have to call netdev_completed_queue().
> Also it allows for per-queue tweaking of tx_usecs via sysfs.
> Works well for me, can share it if we want to use it.
>
>>
>> Further, this is only works if VETH_BQL_UNIT stays 1, otherwise it will never fire. Anyway, still its necessary to check for CONFIG_BQL. But we could solve this by adding VETH_BQL_UNIT to n_bql instead of 1. This is also safe from any overflows, since limit is bound to limit_max, inflight is always less than limit + 1*VETH_BQL_UNIT and n_bql <= inflight.
>
> You are right.
>
> But I think there is no reason for VETH_BQL_UNIT anyway.
> There should be no difference in the BQL algorithm, I personally
> would replace VETH_BQL_UNIT with a hard-coded 1.
>
>>
>> In a version of bringing back the 'queue-empty' check and keeping most of the current logic (so a mixture of v6 and v7) resulted in the same performance on an x86_64 architecture.
>>
>>> Both Sashiko-Nipa and Sashiko-Gemini are right, this is missing a
>>> #ifdef CONFIG_BQL. Not sure what is the best way to add them.
>>> And for the struct we could maybe do:
>>>
>>> #ifdef CONFIG_BQL
>>> struct veth_bql_state {
>>> u64 time; /* sched_clock() when current coalescing window started */
>>> uint n_bql; /* BQL completions batched in the current window */
>>> };
>>> #else
>>> struct veth_bql_state {};
>>> #endif
>> Regarding the configs: we can just do something along those lines.
>> struct veth_rq {
>> ...
>> #ifdef CONFIG_BQL
>> struct veth_bql_state dql;
>> #endif
>> ...
>> }
>>
>> and we put the rest of the code that accesses or performs an action regarding bql in some functions and do it like in netdev_* functions with
>>
>> Function-Signature()
>> {
>> #ifdef CONFIG_BQL
>> // Code
>> #endif
>> }
>>
>> Wdyt?
>> - Jonas
>
> Yes, we have to. Unless we put it into dynamic_queue_limits.{h,c}
> of course :^)
>
> Thanks,
> Simon
> I did implement the CONFIG_BQL guard, and reordered the completion call,
dropping the pre-loop completion call and moved the in-loop
completion call in front of the packet processing. I think we can drop
one of the completion calls, since the there is only a difference of one
packet more or less that is completed.
Two patches on top of 5/5, inline below as RFC (not for application):
1) veth: simplify BQL completion condition
2) veth: Add CONFIG_BQL guards
Patch 1 is the "bring back the queue-empty check" idea: it drops the
state->n_bql > dql.limit test and splits the flush into (a) the
time-based completion, kept per packet, and (b) an explicit post-loop
"ring drained + peer stopped" wake. So veth no longer touches dql.limit.
Basically the same we had in v6.
However, if we like we can just replace it in the eth_bql_flush_starved
with the barrier-free alternative:
if (peer_txq &&
(u64)state->n_bql * VETH_BQL_UNIT > peer_txq->dql.limit)
veth_bql_complete(...);
Once the producer is stopped, num_queued/num_completed/limit are frozen,
so as the ring drains n_bql rises to inflight and this becomes true
exactly when the ring empties under backpressure — same event, no
barrier.
So there are three options how to handle this case:
a) barrier + STACK_XOFF (patch 1 as posted).
b) n_bql * VETH_BQL_UNIT > dql.limit.
c) Simon's generic DQL-for-software-interfaces in
dynamic_queue_limits.{h,c}
I do not have a strong opinion. For a and b I could not measure any
performance difference.
Patch 2 does the CONFIG_BQL wrapping the way I sketched (BQL-only
helpers with no-op stubs so veth_xdp_rcv()/teardown stay ifdef-free) and
rejects `ethtool -C tx-usecs` with -EOPNOTSUPP when BQL is compiled out.
Performance of !CONFIG_BQL+v7+patch-2 and net-next/main is the same.
Full diffs below.
- Jonas
---8<--- patch 1 ---8<---
From: =?UTF-8?q?Jonas=20K=C3=B6ppeler?= <j.koeppeler@tu-berlin.de>
Date: Sun, 28 Jun 2026 10:26:15 +0200
Subject: [PATCH] veth: simplify BQL completion condition
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The previous patch flushed batched BQL completions when either the
coalescing window elapsed or the batch grew past the DQL limit
(state->n_bql > peer_txq->dql.limit). The latter test reaches into DQL
internals and is unit-fragile: it compares the raw count n_bql against
dql.limit, dropping the VETH_BQL_UNIT factor of the charge
(n_bql * VETH_BQL_UNIT), so it only holds because that unit is 1.
Replace that test. The time-based completion stays per packet in
veth_xdp_rcv(), issued before veth_xdp_rcv_skb() so the producer wake
overlaps with the first skb processing. The wake-a-stalled-producer case
becomes an explicit post-loop block: once the ring has drained, if the
peer TX queue is stopped by BQL backpressure (STACK_XOFF), release the
batched completions to unblock it. DRV_XOFF is left to the existing wake
in veth_poll().
Reading STACK_XOFF after the drain needs an smp_rmb(): the producer sets
STACK_XOFF before publishing into the ring, so a consumer on another CPU
that observed the packet must order its ring read ahead of the state
read, or it may read a stale, un-stopped state and drop the wakeup.
Pairs with the set_bit()/smp_wmb() on the producer side.
Signed-off-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
---
drivers/net/veth.c | 53 ++++++++++++++++++++++++++++++----------------
1 file changed, 35 insertions(+), 18 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index c62d87a8402c..2963f190988f 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -1029,11 +1029,21 @@ static struct sk_buff *veth_xdp_rcv_skb(struct
veth_rq *rq,
return NULL;
}
+static void veth_bql_complete(struct veth_bql_state *state,
+ struct netdev_queue *peer_txq,
+ u64 now)
+{
+ netdev_tx_completed_queue(peer_txq, state->n_bql,
+ state->n_bql * VETH_BQL_UNIT);
+ state->time = now;
+ state->n_bql = 0;
+}
+
static void veth_bql_maybe_complete(struct veth_bql_state *state,
struct netdev_queue *peer_txq,
u64 bql_flush_ns)
{
- u64 current_time;
+ u64 now;
/* There is no reason to complete with 0 and
* peer_txq could go away.
@@ -1041,19 +1051,12 @@ static void veth_bql_maybe_complete(struct
veth_bql_state *state,
if (!state->n_bql || !peer_txq)
return;
- current_time = sched_clock();
-
- /* We complete if:
- * 1. We reach bql_flush_ns.
- * 2. We potentially have BQL starvation.
+ /* Release the batched completions once the coalescing window has
+ * elapsed.
*/
- if (state->time + bql_flush_ns <= current_time ||
- state->n_bql > peer_txq->dql.limit) {
- netdev_tx_completed_queue(peer_txq, state->n_bql,
- state->n_bql * VETH_BQL_UNIT);
- state->time = current_time;
- state->n_bql = 0;
- }
+ now = sched_clock();
+ if (state->time + bql_flush_ns <= now)
+ veth_bql_complete(state, peer_txq, now);
}
static int veth_xdp_rcv(struct veth_rq *rq, int budget,
@@ -1075,9 +1078,6 @@ static int veth_xdp_rcv(struct veth_rq *rq, int
budget,
*/
state->time = min(state->time, sched_clock());
- /* Flush completions that timed out since the previous NAPI poll. */
- veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);
-
for (i = 0; i < budget; i++) {
void *ptr = __ptr_ring_consume(&rq->xdp_ring);
@@ -1105,8 +1105,12 @@ static int veth_xdp_rcv(struct veth_rq *rq, int
budget,
if (veth_ptr_is_bql(ptr))
state->n_bql++;
- stats->xdp_bytes += skb->len;
+ /* Complete before processing so the producer wakes
+ * sooner; ring-empty case handled after the loop.
+ */
+ veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);
+ stats->xdp_bytes += skb->len;
skb = veth_xdp_rcv_skb(rq, skb, bq, stats);
if (skb) {
if (skb_shared(skb) || skb_unclone(skb, GFP_ATOMIC))
@@ -1115,13 +1119,26 @@ static int veth_xdp_rcv(struct veth_rq *rq, int
budget,
napi_gro_receive(&rq->xdp_napi, skb);
}
}
- veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);
done++;
}
if (n_xdpf)
veth_xdp_rcv_bulk_skb(rq, xdpf, n_xdpf, bq, stats);
+ /* If the ring drained and the peer TX queue is stalled by BQL
+ * backpressure (STACK_XOFF), release the batched completions now to
+ * unblock the producer. DRV_XOFF is handled by the wake in veth_poll().
+ */
+ if (peer_txq && state->n_bql && __ptr_ring_empty(&rq->xdp_ring)) {
+ /* The consume above observed the producer's publish; order it
+ * before reading STACK_XOFF. Pairs with the smp_wmb() and XOFF
+ * set_bit() on the producer side.
+ */
+ smp_rmb();
+ if (test_bit(__QUEUE_STATE_STACK_XOFF, &peer_txq->state))
+ veth_bql_complete(state, peer_txq, sched_clock());
+ }
+
u64_stats_update_begin(&rq->stats.syncp);
rq->stats.vs.xdp_redirect += stats->xdp_redirect;
rq->stats.vs.xdp_bytes += stats->xdp_bytes;
--
2.53.0
---8<--- patch 2 ---8<---
From: =?UTF-8?q?Jonas=20K=C3=B6ppeler?= <j.koeppeler@tu-berlin.de>
Date: Thu, 2 Jul 2026 10:13:02 +0000
Subject: [PATCH] veth: Add CONFIG_BQL guards
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Wrap the BQL-only code under CONFIG_BQL and expose it to the driver
through a small set of helpers with no-op stubs for the !CONFIG_BQL
case, so veth_xdp_rcv() and the NAPI teardown path stay free of ifdefs:
veth_bql_poll_prepare() per-poll setup: clamps the coalescing window
timestamp and returns the interval length, so
!CONFIG_BQL builds never read tx_coal_usecs
veth_bql_account() counts a consumed BQL-tagged skb and releases
the batch once the interval window has elapsed
veth_bql_flush_starved() releases the batch early when the ring has
drained and the peer txq is stopped by BQL
backpressure (STACK_XOFF)
veth_bql_state_init()
veth_bql_drain_and_reset() per-queue state setup and teardown
Since tx-usecs only batches BQL completions, reject ethtool -C tx-usecs
with -EOPNOTSUPP when BQL is compiled out instead of silently storing an
inert value.
Signed-off-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
---
drivers/net/veth.c | 173 +++++++++++++++++++++++++++++----------------
1 file changed, 111 insertions(+), 62 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 2963f190988f..5c3b7820c55c 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -73,7 +73,7 @@ struct veth_rq_stats {
struct veth_bql_state {
u64 time; /* sched_clock() when current coalescing window started */
- uint n_bql; /* BQL completions batched in the current window */
+ unsigned int n_bql; /* BQL completions batched in the current window */
};
struct veth_rq {
@@ -83,7 +83,9 @@ struct veth_rq {
struct bpf_prog __rcu *xdp_prog;
struct xdp_mem_info xdp_mem;
struct veth_rq_stats stats;
+#ifdef CONFIG_BQL
struct veth_bql_state bql_state;
+#endif
bool rx_notify_masked;
struct ptr_ring xdp_ring;
struct xdp_rxq_info xdp_rxq;
@@ -300,6 +302,12 @@ static int veth_set_coalesce(struct net_device *dev,
struct veth_priv *priv = netdev_priv(dev);
struct net_device *peer;
+ /* tx-usecs only batches BQL completions; without BQL it is inert. */
+ if (!IS_ENABLED(CONFIG_BQL)) {
+ NL_SET_ERR_MSG_MOD(extack, "tx-usecs requires CONFIG_BQL");
+ return -EOPNOTSUPP;
+ }
+
/* The coalescing window delays BQL completions, so keep tx-usecs well
* below the tx_timeout watchdog; otherwise a large value could stall a
* stopped queue long enough to trip a false watchdog timeout. Cap at
@@ -351,11 +359,6 @@ static bool veth_is_xdp_frame(void *ptr)
return (unsigned long)ptr & VETH_XDP_FLAG;
}
-static bool veth_ptr_is_bql(void *ptr)
-{
- return (unsigned long)ptr & VETH_BQL_FLAG;
-}
-
static struct sk_buff *veth_ptr_to_skb(void *ptr)
{
return (void *)((unsigned long)ptr & ~VETH_BQL_FLAG);
@@ -384,25 +387,6 @@ static void veth_ptr_free(void *ptr)
kfree_skb(veth_ptr_to_skb(ptr));
}
-/* Drain frames left in the ptr_ring at teardown, freeing each one and
- * returning the number of BQL-charged SKBs. The caller completes these
- * via netdev_tx_completed_queue() to balance the DQL accounting, avoiding
- * the racy netdev_tx_reset_queue()/dql_reset().
- */
-static unsigned int veth_ptr_ring_drain(struct ptr_ring *ring)
-{
- unsigned int n_bql = 0;
- void *ptr;
-
- while ((ptr = ptr_ring_consume(ring))) {
- if (veth_ptr_is_bql(ptr))
- n_bql++;
- veth_ptr_free(ptr);
- }
-
- return n_bql;
-}
-
static void __veth_xdp_flush(struct veth_rq *rq)
{
/* Write ptr_ring before reading rx_notify_masked */
@@ -1029,6 +1013,31 @@ static struct sk_buff *veth_xdp_rcv_skb(struct
veth_rq *rq,
return NULL;
}
+#ifdef CONFIG_BQL
+static bool veth_ptr_is_bql(void *ptr)
+{
+ return (unsigned long)ptr & VETH_BQL_FLAG;
+}
+
+/* Drain frames left in the ptr_ring at teardown, freeing each one and
+ * returning the number of BQL-charged SKBs. The caller completes these
+ * via netdev_tx_completed_queue() to balance the DQL accounting, avoiding
+ * the racy netdev_tx_reset_queue()/dql_reset().
+ */
+static unsigned int veth_bql_ring_drain(struct ptr_ring *ring)
+{
+ unsigned int n_bql = 0;
+ void *ptr;
+
+ while ((ptr = ptr_ring_consume(ring))) {
+ if (veth_ptr_is_bql(ptr))
+ n_bql++;
+ veth_ptr_free(ptr);
+ }
+
+ return n_bql;
+}
+
static void veth_bql_complete(struct veth_bql_state *state,
struct netdev_queue *peer_txq,
u64 now)
@@ -1039,18 +1048,18 @@ static void veth_bql_complete(struct
veth_bql_state *state,
state->n_bql = 0;
}
-static void veth_bql_maybe_complete(struct veth_bql_state *state,
- struct netdev_queue *peer_txq,
- u64 bql_flush_ns)
+static void veth_bql_account(struct veth_rq *rq,
+ struct netdev_queue *peer_txq,
+ void *ptr, u64 bql_flush_ns)
{
+ struct veth_bql_state *state = &rq->bql_state;
u64 now;
- /* There is no reason to complete with 0 and
- * peer_txq could go away.
- */
- if (!state->n_bql || !peer_txq)
+ if (!peer_txq || !veth_ptr_is_bql(ptr))
return;
+ state->n_bql++;
+
/* Release the batched completions once the coalescing window has
* elapsed.
*/
@@ -1059,24 +1068,81 @@ static void veth_bql_maybe_complete(struct
veth_bql_state *state,
veth_bql_complete(state, peer_txq, now);
}
+/* Per-poll setup: clamp the window timestamp and return the length of the
+ * coalescing window in ns.
+ */
+static u64 veth_bql_poll_prepare(struct veth_rq *rq)
+{
+ struct veth_priv *priv = netdev_priv(rq->dev);
+ struct veth_bql_state *state = &rq->bql_state;
+
+ /* Clamp stored timestamp in case we migrated to a CPU with a behind
+ * sched_clock(); tries to reduce late BQL flushes.
+ */
+ state->time = min(state->time, sched_clock());
+
+ /* Mirrored to both peers; paired with WRITE_ONCE() in
veth_set_coalesce */
+ return (u64)READ_ONCE(priv->tx_coal_usecs) * NSEC_PER_USEC;
+}
+
+static void veth_bql_flush_starved(struct veth_rq *rq,
+ struct netdev_queue *peer_txq)
+{
+ struct veth_bql_state *state = &rq->bql_state;
+
+ if (!peer_txq)
+ return;
+
+ /* If the ring drained and the peer TX queue is stalled by BQL
+ * backpressure (STACK_XOFF), release the batched completions now to
+ * unblock the producer. DRV_XOFF is handled by the wake in veth_poll().
+ */
+ if (state->n_bql && __ptr_ring_empty(&rq->xdp_ring)) {
+ /* The consume above observed the producer's publish; order it
+ * before reading STACK_XOFF. Pairs with the smp_wmb() and XOFF
+ * set_bit() on the producer side.
+ */
+ smp_rmb();
+ if (test_bit(__QUEUE_STATE_STACK_XOFF, &peer_txq->state))
+ veth_bql_complete(state, peer_txq, sched_clock());
+ }
+}
+
+static void veth_bql_state_init(struct veth_rq *rq)
+{
+ rq->bql_state.time = sched_clock();
+ rq->bql_state.n_bql = 0;
+}
+
+static unsigned int veth_bql_drain_and_reset(struct veth_rq *rq)
+{
+ unsigned int n_bql = veth_bql_ring_drain(&rq->xdp_ring) +
rq->bql_state.n_bql;
+
+ rq->bql_state.n_bql = 0;
+ rq->bql_state.time = 0;
+ return n_bql;
+}
+#else
+static inline void veth_bql_account(struct veth_rq *rq,
+ struct netdev_queue *peer_txq,
+ void *ptr, u64 bql_flush_ns) {}
+static inline u64 veth_bql_poll_prepare(struct veth_rq *rq) { return 0; }
+static inline void veth_bql_flush_starved(struct veth_rq *rq,
+ struct netdev_queue *peer_txq) {}
+static inline void veth_bql_state_init(struct veth_rq *rq) {}
+static inline unsigned int veth_bql_drain_and_reset(struct veth_rq *rq)
{ return 0; }
+#endif
+
static int veth_xdp_rcv(struct veth_rq *rq, int budget,
struct veth_xdp_tx_bq *bq,
struct veth_stats *stats,
struct netdev_queue *peer_txq)
{
- struct veth_priv *priv = netdev_priv(rq->dev);
- struct veth_bql_state *state = &rq->bql_state;
int i, done = 0, n_xdpf = 0;
void *xdpf[VETH_XDP_BATCH];
u64 bql_flush_ns;
- /* Mirrored to both peers; paired with WRITE_ONCE() in
veth_set_coalesce */
- bql_flush_ns = (u64)READ_ONCE(priv->tx_coal_usecs) * 1000;
-
- /* Clamp stored timestamp in case we migrated to a CPU with a behind
- * sched_clock(); tries to reduce late BQL flushes.
- */
- state->time = min(state->time, sched_clock());
+ bql_flush_ns = veth_bql_poll_prepare(rq);
for (i = 0; i < budget; i++) {
void *ptr = __ptr_ring_consume(&rq->xdp_ring);
@@ -1103,12 +1169,10 @@ static int veth_xdp_rcv(struct veth_rq *rq, int
budget,
/* ndo_start_xmit */
struct sk_buff *skb = veth_ptr_to_skb(ptr);
- if (veth_ptr_is_bql(ptr))
- state->n_bql++;
/* Complete before processing so the producer wakes
* sooner; ring-empty case handled after the loop.
*/
- veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);
+ veth_bql_account(rq, peer_txq, ptr, bql_flush_ns);
stats->xdp_bytes += skb->len;
skb = veth_xdp_rcv_skb(rq, skb, bq, stats);
@@ -1125,19 +1189,7 @@ static int veth_xdp_rcv(struct veth_rq *rq, int
budget,
if (n_xdpf)
veth_xdp_rcv_bulk_skb(rq, xdpf, n_xdpf, bq, stats);
- /* If the ring drained and the peer TX queue is stalled by BQL
- * backpressure (STACK_XOFF), release the batched completions now to
- * unblock the producer. DRV_XOFF is handled by the wake in veth_poll().
- */
- if (peer_txq && state->n_bql && __ptr_ring_empty(&rq->xdp_ring)) {
- /* The consume above observed the producer's publish; order it
- * before reading STACK_XOFF. Pairs with the smp_wmb() and XOFF
- * set_bit() on the producer side.
- */
- smp_rmb();
- if (test_bit(__QUEUE_STATE_STACK_XOFF, &peer_txq->state))
- veth_bql_complete(state, peer_txq, sched_clock());
- }
+ veth_bql_flush_starved(rq, peer_txq);
u64_stats_update_begin(&rq->stats.syncp);
rq->stats.vs.xdp_redirect += stats->xdp_redirect;
@@ -1241,8 +1293,7 @@ static int __veth_napi_enable_range(struct
net_device *dev, int start, int end)
for (i = start; i < end; i++) {
struct veth_rq *rq = &priv->rq[i];
- rq->bql_state.time = sched_clock();
- rq->bql_state.n_bql = 0;
+ veth_bql_state_init(rq);
napi_enable(&rq->xdp_napi);
rcu_assign_pointer(priv->rq[i].napi, &priv->rq[i].xdp_napi);
@@ -1298,10 +1349,8 @@ static void veth_napi_del_range(struct net_device
*dev, int start, int end)
* (consumed by NAPI but not yet flushed). Both were charged
* via netdev_tx_sent_queue() and are still outstanding.
*/
- n_bql = veth_ptr_ring_drain(&rq->xdp_ring) + rq->bql_state.n_bql;
+ n_bql = veth_bql_drain_and_reset(rq);
ptr_ring_cleanup(&rq->xdp_ring, veth_ptr_free);
- rq->bql_state.n_bql = 0;
- rq->bql_state.time = 0;
if (!peer || i >= peer->num_tx_queues)
continue;
--
2.53.0
>>>
>>>> + netdev_tx_completed_queue(peer_txq, state->n_bql,
>>>> + state->n_bql * VETH_BQL_UNIT);
>>>> + state->time = current_time;
>>>> + state->n_bql = 0;
>>>> + }
>>>> +}
>>>> +
>>>> static int veth_xdp_rcv(struct veth_rq *rq, int budget,
>>>> struct veth_xdp_tx_bq *bq,
>>>> struct veth_stats *stats,
>>>> struct netdev_queue *peer_txq)
>>>> {
>>>> + struct veth_priv *priv = netdev_priv(rq->dev);
>>>> + struct veth_bql_state *state = &rq->bql_state;
>>>> int i, done = 0, n_xdpf = 0;
>>>> void *xdpf[VETH_XDP_BATCH];
>>>> + u64 bql_flush_ns;
>>>> +
>>>> + /* Mirrored to both peers; paired with WRITE_ONCE() in veth_set_coalesce */
>>>> + bql_flush_ns = (u64)READ_ONCE(priv->tx_coal_usecs) * 1000;
>>>> +
>>>> + /* Clamp stored timestamp in case we migrated to a CPU with a behind
>>>> + * sched_clock(); tries to reduce late BQL flushes.
>>>> + */
>>>> + state->time = min(state->time, sched_clock());
>>>> +
>>>> + /* Flush completions that timed out since the previous NAPI poll. */
>>>> + veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);>>
>>>> for (i = 0; i < budget; i++) {
>>>> void *ptr = __ptr_ring_consume(&rq->xdp_ring);
>>>> @@ -1000,12 +1101,11 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
>>>> }
>>>> } else {
>>>> /* ndo_start_xmit */
>>>> - bool bql_charged = veth_ptr_is_bql(ptr);
>>>> struct sk_buff *skb = veth_ptr_to_skb(ptr);
>>>> + if (veth_ptr_is_bql(ptr))
>>>> + state->n_bql++;
>>>> stats->xdp_bytes += skb->len;
>>>> - if (peer_txq && bql_charged)
>>>> - netdev_tx_completed_queue(peer_txq, 1, VETH_BQL_UNIT);
>>>> skb = veth_xdp_rcv_skb(rq, skb, bq, stats);
>>>> if (skb) {
>>>> @@ -1015,6 +1115,7 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
>>>> napi_gro_receive(&rq->xdp_napi, skb);
>>>> }
>>>> }
>>>> + veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);
>>>> done++;
>>>
>>> Sashiko-Nipa reports:
>>>
>>> "If veth_xdp_rcv() finishes and returns a done count less than the budget,
>>> NAPI will go to sleep in veth_poll(). Do we need to unconditionally flush
>>> any stranded BQL completions in veth_poll() before sleeping?
>>> If completions are left in rq->bql_state indefinitely across NAPI idle
>>> periods, it might present an artificially massive delay to DQL. This could
>>> cause DQL to mistakenly conclude the hardware is extremely slow and
>>> aggressively shrink dql.limit to its minimum, crippling throughput on
>>> subsequent bursts."
>>>
>>> Again the issue that I found to be non-problematic in [1] and can be
>>> seen by an BQL inflight > 0 when for example pktgen suddenly stops.
>>>
>>> If we would "unconditionally flush any stranded BQL completions in
>>> veth_poll() before sleeping" we would *not* accumulate BQL completions
>>> across NAPI polls but we want to do that.
>>>
>>> Do you agree?
>>>
>>> [1] https://lore.kernel.org/netdev/c8650d3a-e488-4279-b28f-549d766c23a1@tu-dortmund.de/
>>
^ permalink raw reply related
* Re: [PATCH v4 3/3] drm/xe/xe_ras: Add error-event support for CRI
From: Raag Jadav @ 2026-07-09 10:04 UTC (permalink / raw)
To: Riana Tauro
Cc: intel-xe, dri-devel, netdev, aravind.iddamsetty, anshuman.gupta,
rodrigo.vivi, joonas.lahtinen, kuba, simona.vetter, airlied,
pratik.bari, joshua.santosh.ranjan, ashwin.kumar.kulkarni,
shubham.kumar, ravi.kishore.koppuravuri, maarten.lankhorst,
mallesh.koujalagi, soham.purkait, Michal Wajdeczko
In-Reply-To: <20260701094409.129131-8-riana.tauro@intel.com>
On Wed, Jul 01, 2026 at 03:14:13PM +0530, Riana Tauro wrote:
> Add error-event support for Correctable errors in CRI. Report an error
> event to userspace for every component that has crossed the threshold on
> receiving an interrupt.
...
> +static void ras_send_error_event(struct xe_device *xe, u8 severity, u8 component)
> +{
> + u8 drm_severity, drm_component;
> + u32 value;
> + int ret;
> +
> + drm_severity = xe_to_drm_ras_severity(severity);
> + if (drm_severity == DRM_XE_RAS_ERR_SEV_MAX) {
> + xe_warn(xe, "sysctrl: unexpected severity %u\n", severity);
This is uapi and not coming from sysctrl, so the message is a bit
misleading. But if at all it needs validation, it should be done in
drm_ras layer.
> + return;
> + }
> +
> + drm_component = xe_to_drm_ras_component(component);
> + if (drm_component == DRM_XE_RAS_ERR_COMP_MAX) {
> + xe_warn(xe, "sysctrl: unexpected component %u\n", component);
Ditto.
> + return;
> + }
> +
> + ret = xe_ras_get_counter(xe, drm_severity, drm_component, &value);
No, instead of converting back and forth just do get_counter() using
sysctrl values and send_event() afterwards.
> + if (ret)
> + return;
> +
> + xe_drm_ras_event(xe, drm_component, drm_severity, value, GFP_KERNEL);
> +}
> +
> void xe_ras_counter_threshold_crossed(struct xe_device *xe,
> struct xe_sysctrl_event_response *response)
> {
> struct xe_ras_threshold_crossed *pending = (void *)&response->data;
> struct xe_ras_error_class *errors = pending->counters;
> u32 id, ncounters = pending->ncounters;
> + u8 sent = 0;
>
> BUILD_BUG_ON(sizeof(response->data) < sizeof(*pending));
> + BUILD_BUG_ON(XE_RAS_COMP_MAX > (BITS_PER_BYTE * sizeof(sent)));
I prefer it the other way around. Also, have you tried using BITS_PER_TYPE()?
BUILD_BUG_ON(BITS_PER_TYPE(sent) < XE_RAS_COMP_MAX));
> xe_device_assert_mem_access(xe);
>
> if (!ncounters || ncounters > XE_RAS_NUM_COUNTERS)
> @@ -154,6 +211,24 @@ void xe_ras_counter_threshold_crossed(struct xe_device *xe,
>
> xe_warn(xe, "[RAS]: %s %s detected\n",
> comp_to_str(component), sev_to_str(severity));
> +
> + if (severity != XE_RAS_SEV_CORRECTABLE) {
> + xe_warn(xe, "sysctrl: unexpected severity %s (%u)\n", sev_to_str(severity),
> + severity);
This should be before "detected" log above.
> + continue;
> + }
> +
> + if (component >= XE_RAS_COMP_MAX) {
> + xe_warn(xe, "sysctrl: unexpected component %u\n", component);
Ditto. Also, use xx_to_str() in both cases or don't but be consistent.
Raag
> + continue;
> + }
> +
> + /* Send event once per component */
> + if (sent & BIT(component))
> + continue;
> + sent |= BIT(component);
> +
> + ras_send_error_event(xe, severity, component);
> }
> }
>
> --
> 2.47.1
>
^ permalink raw reply
* [PATCH net v3] tun/tap & vhost-net: make qdisc backpressure opt-in via IFF_BACKPRESSURE
From: Simon Schippers @ 2026-07-09 9:55 UTC (permalink / raw)
To: Willem de Bruijn, Jason Wang, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Michael S . Tsirkin, netdev
Cc: Simon Horman, Jonathan Corbet, Shuah Khan, Andrew Lunn,
Tim Gebauer, Brett Sheffield, linux-doc, linux-kernel,
Simon Schippers
Commit 1d6e569b7d0c ("tun/tap & vhost-net: avoid ptr_ring tail-drop
when a qdisc is present") did not show a relevant performance regression
in my testing but on Brett Sheffield's librecast testbed it shows a
significant performance drop in a IPv6 multicast testcase. The regression
can be pinpointed when multiple iperf3 TCP threads are sending. For 8
threads the performance dropped from 13.5 Gbit/s to 9.13 Gbit/s. This is
the reason why this patch makes the qdisc backpressure behavior opt-in.
One option to accomplish the opt-in would be to set the default qdisc to
noqueue at init. However this may also break userspace as users might
have chosen a custom qdisc even though most of the qdiscs did nothing
for tun/tap in the past due to missing backpressure...
This is the reason why in this patch, the flag IFF_BACKPRESSURE is
introduced instead which is required to enable the backpressure logic.
This means the stopping logic in tun_net_xmit() and the waking logic in
__tun_wake_queue() are skipped if the flag is disabled. Setting
IFF_BACKPRESSURE makes an attached qdisc effective by stopping the queue
instead of tail-dropping when the internal ring is full.
To avoid a possible stall due to disabling IFF_BACKPRESSURE, the new
helper tun_force_wake_queue() is implemented. The helper safely wakes the
respective netdev queue and resets cons_cnt while the consumer_lock and
the producer_lock of the ring are held. The helper is run in tun_attach()
when a queue (re)attaches, in tun_set_iff() for attached tfiles, and
in tun_queue_resize().
The documentation in tuntap.rst is updated accordingly.
Fixes: 1d6e569b7d0c ("tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present")
Reported-by: Brett Sheffield <brett@librecast.net>
Closes: https://lore.kernel.org/netdev/akVnoOYQOrt8k-Gu@karahi.librecast.net/T/#u
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
V2 -> V3:
- As suggested by MST: Clarify in tuntap.rst and the UAPI header what
enabling IFF_BACKPRESSURE opts into: an attached qdisc becomes effective
instead of the driver tail-dropping when the internal ring is full.
- Avoid lines over 75 characters.
- Update comment in tun_net_xmit() to include IFF_BACKPRESSURE.
- Brett: Update in commit message that the referenced tests were TCP.
V1 -> V2:
- Sashiko: Ensure detached queues are woken on re-attach by calling the
new tun_force_wake_queue() helper from tun_attach(), and reuse it
across the existing wake paths.
- Specify the failing test case in the commit message.
V1: https://lore.kernel.org/netdev/20260704112058.95421-1-simon.schippers@tu-dortmund.de/T/#u
V2: https://lore.kernel.org/netdev/20260706094242.115992-1-simon.schippers@tu-dortmund.de/T/#u
Documentation/networking/tuntap.rst | 22 +++++++++++++
drivers/net/tun.c | 51 ++++++++++++++++++++---------
include/uapi/linux/if_tun.h | 4 +++
tools/include/uapi/linux/if_tun.h | 1 +
4 files changed, 62 insertions(+), 16 deletions(-)
diff --git a/Documentation/networking/tuntap.rst b/Documentation/networking/tuntap.rst
index 4d7087f727be..5921a924c2ae 100644
--- a/Documentation/networking/tuntap.rst
+++ b/Documentation/networking/tuntap.rst
@@ -206,6 +206,28 @@ enable is true we enable it, otherwise we disable it::
return ioctl(fd, TUNSETQUEUE, (void *)&ifr);
}
+3.4 qdisc backpressure
+----------------------
+
+Starting with Linux 7.2, IFF_BACKPRESSURE can be set to enable qdisc
+backpressure. Without it, TX drops occur when the internal ring buffer
+is full, so any attached qdisc is effectively bypassed and applications
+only learn about congestion through those drops.
+
+With it, the kernel stops instead, letting the qdisc hold and schedule
+packets, so its AQM, shaping and fairness actually apply. This helps
+protocols like TCP, which cut throughput in reaction to packet drops.
+With IFF_BACKPRESSURE, drops then only occur as a rare race. Backpressure
+requires a qdisc to be attached and has no effect with noqueue.
+
+The txqueuelen can be reduced alongside this flag to further shift
+buffering into the qdisc and reduce bufferbloat, but comes at possible
+performance cost.
+
+When running multiple network streams in parallel through a single
+TUN/TAP queue, the flag may reduce performance due to the extra overhead
+of the backpressure mechanism.
+
Universal TUN/TAP device driver Frequently Asked Question
=========================================================
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index ffbe6f13fb1f..5941e8f302ea 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -98,7 +98,8 @@ static void tun_default_link_ksettings(struct net_device *dev,
#define TUN_FASYNC IFF_ATTACH_QUEUE
#define TUN_FEATURES (IFF_NO_PI | IFF_ONE_QUEUE | IFF_VNET_HDR | \
- IFF_MULTI_QUEUE | IFF_NAPI | IFF_NAPI_FRAGS)
+ IFF_MULTI_QUEUE | IFF_NAPI | IFF_NAPI_FRAGS | \
+ IFF_BACKPRESSURE)
#define GOODCOPY_LEN 128
@@ -694,6 +695,20 @@ static void tun_detach_all(struct net_device *dev)
module_put(THIS_MODULE);
}
+static void tun_force_wake_queue(struct tun_struct *tun,
+ struct tun_file *tfile)
+{
+ /* Ensure that the producer can not stop the
+ * queue concurrently by taking locks.
+ */
+ spin_lock_bh(&tfile->tx_ring.consumer_lock);
+ spin_lock(&tfile->tx_ring.producer_lock);
+ netif_wake_subqueue(tun->dev, tfile->queue_index);
+ tfile->cons_cnt = 0;
+ spin_unlock(&tfile->tx_ring.producer_lock);
+ spin_unlock_bh(&tfile->tx_ring.consumer_lock);
+}
+
static int tun_attach(struct tun_struct *tun, struct file *file,
bool skip_filter, bool napi, bool napi_frags,
bool publish_tun)
@@ -737,11 +752,9 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
goto out;
}
- spin_lock(&tfile->tx_ring.consumer_lock);
- tfile->cons_cnt = 0;
- spin_unlock(&tfile->tx_ring.consumer_lock);
tfile->queue_index = tun->numqueues;
tfile->socket.sk->sk_shutdown &= ~RCV_SHUTDOWN;
+ tun_force_wake_queue(tun, tfile);
if (tfile->detached) {
/* Re-attach detached tfile, updating XDP queue_index */
@@ -1077,7 +1090,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
spin_lock(&tfile->tx_ring.producer_lock);
ret = __ptr_ring_produce(&tfile->tx_ring, skb);
- if (!qdisc_txq_has_no_queue(queue) &&
+ if ((tun->flags & IFF_BACKPRESSURE) &&
+ !qdisc_txq_has_no_queue(queue) &&
__ptr_ring_check_produce(&tfile->tx_ring) == -ENOSPC) {
netif_tx_stop_queue(queue);
/* Paired with smp_mb() in __tun_wake_queue() */
@@ -1088,8 +1102,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
spin_unlock(&tfile->tx_ring.producer_lock);
if (ret) {
- /* This should be a rare case if a qdisc is present, but
- * can happen due to lltx.
+ /* This should be a rare case if IFF_BACKPRESSURE is enabled and
+ * a qdisc is present, but can happen due to lltx.
* Since skb_tx_timestamp(), skb_orphan(),
* run_ebpf_filter() and pskb_trim() could have tinkered
* with the SKB, returning NETDEV_TX_BUSY is unsafe and
@@ -2151,8 +2165,12 @@ static ssize_t tun_put_user(struct tun_struct *tun,
static void __tun_wake_queue(struct tun_struct *tun,
struct tun_file *tfile, int consumed)
{
- struct netdev_queue *txq = netdev_get_tx_queue(tun->dev,
- tfile->queue_index);
+ struct netdev_queue *txq;
+
+ if (!(tun->flags & IFF_BACKPRESSURE))
+ return;
+
+ txq = netdev_get_tx_queue(tun->dev, tfile->queue_index);
/* Paired with smp_mb__after_atomic() in tun_net_xmit() */
smp_mb();
@@ -2764,7 +2782,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
struct tun_struct *tun;
struct tun_file *tfile = file->private_data;
struct net_device *dev;
- int err;
+ int err, i;
if (tfile->detached)
return -EINVAL;
@@ -2893,8 +2911,12 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
/* Make sure persistent devices do not get stuck in
* xoff state.
*/
- if (netif_running(tun->dev))
- netif_tx_wake_all_queues(tun->dev);
+ if (netif_running(tun->dev)) {
+ for (i = 0; i < tun->numqueues; i++) {
+ tfile = rtnl_dereference(tun->tfiles[i]);
+ tun_force_wake_queue(tun, tfile);
+ }
+ }
strscpy(ifr->ifr_name, tun->dev->name);
return 0;
@@ -3693,10 +3715,7 @@ static int tun_queue_resize(struct tun_struct *tun)
if (!ret) {
for (i = 0; i < tun->numqueues; i++) {
tfile = rtnl_dereference(tun->tfiles[i]);
- spin_lock(&tfile->tx_ring.consumer_lock);
- netif_wake_subqueue(tun->dev, tfile->queue_index);
- tfile->cons_cnt = 0;
- spin_unlock(&tfile->tx_ring.consumer_lock);
+ tun_force_wake_queue(tun, tfile);
}
}
diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
index 79d53c7a1ebd..a0ddc50a7534 100644
--- a/include/uapi/linux/if_tun.h
+++ b/include/uapi/linux/if_tun.h
@@ -69,6 +69,10 @@
#define IFF_NAPI_FRAGS 0x0020
/* Used in TUNSETIFF to bring up tun/tap without carrier */
#define IFF_NO_CARRIER 0x0040
+/* Stop the queue instead of dropping when the internal ring is full, so an
+ * attached qdisc applies backpressure instead of being bypassed.
+ */
+#define IFF_BACKPRESSURE 0x0080
#define IFF_NO_PI 0x1000
/* This flag has no real effect */
#define IFF_ONE_QUEUE 0x2000
diff --git a/tools/include/uapi/linux/if_tun.h b/tools/include/uapi/linux/if_tun.h
index 2ec07de1d73b..97b670f5bc0a 100644
--- a/tools/include/uapi/linux/if_tun.h
+++ b/tools/include/uapi/linux/if_tun.h
@@ -67,6 +67,7 @@
#define IFF_TAP 0x0002
#define IFF_NAPI 0x0010
#define IFF_NAPI_FRAGS 0x0020
+#define IFF_BACKPRESSURE 0x0080
#define IFF_NO_PI 0x1000
/* This flag has no real effect */
#define IFF_ONE_QUEUE 0x2000
--
2.43.0
^ permalink raw reply related
* Re: Question: net: phy: realtek: RTL8211FS SGMII bridge mode support
From: Maxime Chevallier @ 2026-07-09 9:57 UTC (permalink / raw)
To: Jacky Huang, Andrew Lunn, Heiner Kallweit
Cc: Russell King, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Daniel Golle, Vladimir Oltean, netdev, linux-kernel
In-Reply-To: <20260709035244.3008436-1-jackyhuang@nvidia.com>
Hi,
On 7/9/26 05:52, Jacky Huang wrote:
> Hi,
>
> RTL8211FS is the SGMII-capable variant of RTL8211F. Besides the standard
> copper/RGMII mode, it supports hardware-strapped RGMII-to-SGMII bridge mode,
> where the MAC side uses RGMII and the line side uses SGMII.
>
> The in-tree realtek driver currently does not handle this mode. RTL8211FS
> shares the same PHY ID as RTL8211F (0x001cc916), so the driver binds it
> as a standard RTL8211F and follows the existing copper/RGMII path. In
> RGMII-to-SGMII bridge mode, this means read_status() does not observe the
> SGMII-side link and the link stays down.
>
> We have discussed this with Realtek. They confirmed that the current mode
> can be detected from a vendor mode-selection register shared by RTL8211F
> variants, and that this case is application-dependent rather than generic
> copper PHY behavior.
>
> Our plan is to add RTL8211FS bridge mode support inside the realtek
> driver, gated on the detected mode. The driver would only read this
> register for detection and would not use it to override the hardware
> strapping. For that mode, config_init() would program the SGMII side as
> needed, and read_status() would read the SGMII-side link status instead
> of relying on the copper path. The existing RTL8211F copper/RGMII behavior
> would be left unchanged.
>
> Does this approach sound acceptable for upstream? If you prefer a
> different model, or if there is already related work that we should build
> on, please let me know.
If you can tell the mode based on the straps, that's OK :) There are other drivers
that do the same, for example the TI dp83869 (it's not based on a strap value,
but rather a custom DT property, but it's the same idea), or some broadcom
drivers that read strap state from registers and switch to fiber mode.
What's missing is proper support for what comes after the SGMII bridge, if you
have an SFP after for example, there's all the plumbing to support handling that
SFP module, however there's no way (yet) to handle the PHY within that module,
as you'll have :
MAC - RGMII - PHY - SGMII - PHY
^
|
nothing handles the state machine of that
one PHY
If you have a DSA switch on that SGMII link though, it may "just work" as long as
you correctly read the status and configure inband aned on the SGMII side.
Maxime
^ permalink raw reply
* Re: [PATCH v2 net-next 9/9] octeontx2: add TC flow offload path for switch flows
From: Paolo Abeni @ 2026-07-09 9:56 UTC (permalink / raw)
To: rkannoth
Cc: linux-kernel, netdev, andrew+netdev, davem, edumazet, kuba,
pabeni, sgoutham
In-Reply-To: <20260702045026.2914748-10-rkannoth@marvell.com>
From: AI Reviewer <ai@example.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
octeontx2: add TC flow offload path for switch flows
This patch registers a TC flow offload path that translates flower rules
into fl_tuple state, resolves ingress/egress pcifunc via FIB, and forwards
the results to the AF over the PF mailbox. A few observations follow
inline.
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw.c b/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw.c
> --- a/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw.c
> @@ -7,14 +7,14 @@
> u32 rvu_sw_port_id(struct rvu *rvu, u16 pcifunc)
> {
> - u32 port_id;
> + u16 port_id;
> u16 rep_id;
>
> rep_id = rvu_rep_get_vlan_id(rvu, pcifunc);
[High]
Is the change from u32 to u16 here intentional? The next line builds
a value that spans bits 0..31:
port_id = FIELD_PREP(GENMASK_ULL(31, 16), rep_id) |
FIELD_PREP(GENMASK_ULL(15, 0), pcifunc);
Storing that back into a u16 port_id drops the rep_id (bits 16..31)
before the return, so callers that stash the result as u32 (l2_entry,
l3_entry, fl_entry->port_id) always see zero in the upper half. Would
that not defeat rep_id-based port disambiguation once rep_id != 0?
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw_fl.c b/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw_fl.c
> --- a/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw_fl.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw_fl.c
> @@ -4,12 +4,257 @@
> +static struct workqueue_struct *sw_fl_offl_wq;
> +
> +struct fl_entry {
[ ... ]
> +};
> +
> +static DEFINE_MUTEX(fl_offl_llock);
> +static LIST_HEAD(fl_offl_lh);
> +static bool fl_offl_work_running;
> +
> +static struct workqueue_struct *sw_fl_offl_wq;
[Low]
sw_fl_offl_wq is declared twice as a static in the same translation
unit. Both declarations resolve to the same tentative definition, but
was the duplication intentional?
> +int rvu_sw_fl_stats_sync2db(struct rvu *rvu, struct fl_info *fl, int cnt)
> +{
> + struct npc_mcam_get_mul_stats_req *req = NULL;
> + struct npc_mcam_get_mul_stats_rsp *rsp = NULL;
> + int tot = 0;
> + u16 i2idx_map[256];
> + int rc = 0;
> + u64 pkts;
> + int idx;
> +
> + if (cnt > 256)
> + return -ENOMEM;
> +
> + for (int i = 0; i < cnt; i++) {
> + tot++;
> + if (fl[i].uni_di)
> + continue;
> +
> + tot++;
> + }
[Critical]
Can this overflow the fixed-size arrays sized by cnt? The guard only
bounds cnt at 256, but tot can grow up to 2 * cnt = 512 when every
entry has uni_di == 0. Three arrays are then indexed by tot:
u16 i2idx_map[256]; /* stack */
req->entry[256]; /* struct npc_mcam_get_mul_stats_req */
rsp->stat[256]; /* struct npc_mcam_get_mul_stats_rsp */
> + req->cnt = tot;
> + idx = 0;
> + for (int i = 0; i < tot; idx++) {
> + i2idx_map[i] = idx;
> + req->entry[i++] = fl[idx].mcam_idx[0];
> + if (fl[idx].uni_di)
> + continue;
> +
> + i2idx_map[i] = idx;
> + req->entry[i++] = fl[idx].mcam_idx[1];
> + }
[Critical]
With cnt = 256 and all uni_di = 0, would this loop write i2idx_map[0..511]
and req->entry[0..511], corrupting the kernel stack and the adjacent heap
region? The subsequent read loop then reads rsp->stat[i++] beyond its
256-slot array too. rvu_mbox_handler_npc_mcam_mul_stats() rejects
req->cnt > 256 later, but the OOB writes happen before that check.
Should the bound be cnt <= 128 (so tot <= 256), or should tot itself be
capped?
[Critical]
Separately, is cnt validated against the wire format? struct
swdev2af_notify_req embeds struct fl_info fl[64], and
rvu_mbox_handler_swdev2af_notify() forwards req->cnt to this helper
without checking it. For any 64 < cnt <= 256 the loops here read
fl[64..cnt-1] out of the mailbox payload into adjacent memory, then
propagate those bytes into req->entry[] and back through the mailbox.
Should the guard be cnt < 0 || cnt > ARRAY_SIZE(fl) rather than
cnt > 256 (cnt is a signed int, so negative values also slip through
today)?
> +int rvu_mbox_handler_fl_notify(struct rvu *rvu,
> + struct fl_notify_req *req,
> + struct msg_rsp *rsp)
> {
> + struct fl_entry *fl_entry;
[ ... ]
> + mutex_lock(&fl_offl_llock);
> + list_add_tail(&fl_entry->list, &fl_offl_lh);
> +
> + if (!fl_offl_work_running) {
> + sw_fl_offl_wq = alloc_workqueue("sw_af_fl_wq", 0, 0);
> + if (sw_fl_offl_wq)
> + fl_offl_work_running = true;
> + }
> + mutex_unlock(&fl_offl_llock);
> + queue_delayed_work(sw_fl_offl_wq, &fl_offl_work, msecs_to_jiffies(10));
> +
> return 0;
> }
[High]
On the very first invocation, if alloc_workqueue() returns NULL (for
example under memory pressure), fl_offl_work_running stays false and
sw_fl_offl_wq stays NULL, and queue_delayed_work(NULL, ...) is then
dispatched unconditionally after the mutex is dropped. Does that not
NULL-deref inside __queue_delayed_work()? Also, fl_entry has already
been list_add_tail'd, so if this path returns early there is nothing to
drain the list, and rvu_sw_fl_shutdown() early-returns on
!sw_fl_offl_wq, so the queued entry looks like it would leak.
> +void rvu_sw_fl_shutdown(void)
> +{
> + struct sw_fl_stats_node *snode, *tmp;
> + struct fl_entry *entry;
> + LIST_HEAD(tlist);
> +
> + if (!sw_fl_offl_wq)
> + return;
> +
> + cancel_delayed_work_sync(&fl_offl_work);
> + destroy_workqueue(sw_fl_offl_wq);
[ ... ]
> + mutex_lock(&sw_fl_stats_lock);
> + list_for_each_entry_safe(snode, tmp, &sw_fl_stats_lh, list) {
> + list_del_init(&snode->list);
> + kfree(snode);
> + }
> + mutex_unlock(&sw_fl_stats_lock);
> +}
[High]
After destroy_workqueue(), sw_fl_offl_wq is not set to NULL and
fl_offl_work_running is not reset. If a concurrent (or later)
rvu_mbox_handler_fl_notify() runs, it observes fl_offl_work_running ==
true, skips reallocation, and calls queue_delayed_work(sw_fl_offl_wq,
...) on a freed workqueue. Is that intentional, or should shutdown
clear both under fl_offl_llock so subsequent handlers either reallocate
or bail out?
[Medium]
Also, the early return on !sw_fl_offl_wq skips the cleanup of
sw_fl_stats_lh below. Nodes on that list are allocated by
rvu_sw_fl_stats_sync2db_one_entry() independently of whether the
offload workqueue ever came up, so on a shutdown where the workqueue
was never allocated they leak. Would moving the stats-list cleanup
above the early return match the allocation lifetime?
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_fl.c b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_fl.c
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_fl.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_fl.c
> @@ -4,13 +4,578 @@
[ ... ]
> +static int sw_fl_parse_actions(struct otx2_nic *nic,
[ ... ]
> + case FLOW_ACTION_CT:
> + trace_sw_act_dump(__func__, __LINE__, act->id);
> + err = nf_flow_table_offload_add_cb(act->ct.flow_table,
> + sw_fl_setup_ft_block_ingress_cb,
> + nic);
> + if (err != -EEXIST && err) {
> + netdev_err(nic->netdev,
> + "%s:%d Error to offload flow, err=%d\n",
> + __func__, __LINE__, err);
> + break;
> + }
> +
> + *op |= BIT_ULL(FLOW_ACTION_CT);
> + break;
[Medium]
On a CT offload error that is not -EEXIST, the code only prints and
breaks out of the switch without setting the CT bit and without
returning the error. Processing continues; if any later action sets
*op (for example FLOW_ACTION_REDIRECT), sw_fl_add() sees a non-zero op
that lacks the CT bit, skips the CT early-return, and offloads the
rule as a plain REDIRECT. Should the error be returned to the caller
so the block callback surfaces it, rather than silently converting a
CT rule into a non-CT one?
> +static int sw_fl_stats(struct otx2_nic *nic, struct flow_cls_offload *f)
> +{
> + struct fl_get_stats_req *req;
> + struct fl_get_stats_rsp *rsp;
[ ... ]
> + rc = otx2_sync_mbox_msg(&nic->mbox);
> + if (rc)
> + goto fail;
> +
> + rsp = (struct fl_get_stats_rsp *)otx2_mbox_get_rsp
> + (&nic->mbox.mbox, 0, &req->hdr);
> + pkts_diff = rsp->pkts_diff;
[High]
Is an IS_ERR(rsp) check missing here? otx2_mbox_get_rsp() returns
ERR_PTR(-ENODEV) on failure, and every other caller in this driver
(cn10k.c, cn10k_macsec.c, otx2_common.c, otx2_tc.c, ...) uses
IS_ERR(rsp) before dereferencing. Without the check, an error return
here is dereferenced as a valid pointer.
> +static bool init_done;
> +
> +int sw_fl_setup_ft_block_ingress_cb(enum tc_setup_type type,
> + void *type_data, void *cb_priv)
> +{
> + struct flow_cls_offload *cls = type_data;
> + struct otx2_nic *nic = cb_priv;
> +
> + if (!init_done)
> + return 0;
[ ... ]
> +}
> +
> int sw_fl_init(void)
> {
> + INIT_WORK(&sw_fl_work, sw_fl_wq_handler);
> + sw_fl_wq = alloc_workqueue("sw_fl_wq", 0, 0);
> + if (!sw_fl_wq)
> + return -ENOMEM;
> +
> + init_done = true;
> return 0;
> }
>
> void sw_fl_deinit(void)
> {
[ ... ]
> + cancel_work_sync(&sw_fl_work);
> + destroy_workqueue(sw_fl_wq);
[ ... ]
> }
[Medium]
init_done and sw_fl_wq are used together as an ad-hoc gate: the
callback reads init_done as a plain load and then paths off that read
call queue_work(sw_fl_wq, ...). Since the two stores are plain writes
in sw_fl_init(), can a weakly ordered CPU see init_done == true while
still seeing sw_fl_wq as NULL? A smp_store_release() / smp_load_acquire()
pair, or moving the flag/wq under a real lock, would remove the
ordering hole.
[High]
Also, sw_fl_deinit() destroys sw_fl_wq without clearing init_done or
NULLing sw_fl_wq. Since the callback is registered via
nf_flow_table_offload_add_cb() and TC block infrastructure, callbacks
that arrive after deinit will still see init_done == true and can
queue_work() on a freed workqueue. Is there a missing callback
unregistration step here?
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_trace.h b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_trace.h
> new file mode 100644
> --- /dev/null
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_trace.h
> @@ -0,0 +1,82 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM rvu
[Medium]
The trace system name rvu is already owned by
drivers/net/ethernet/marvell/octeontx2/af/rvu_trace.h, which files the
core AF-PF mailbox protocol events (otx2_msg_alloc / otx2_msg_send /
otx2_msg_process) under /sys/kernel/tracing/events/rvu/. Adding a
second CREATE_TRACE_POINTS site in nic/switch/sw_trace.c under the
same system name mixes these driver-private debug events with the
pre-existing protocol events in user-visible tracefs. Would a
distinct name (rvu_sw, octeontx2_sw, ...) avoid the collision?
> +TRACE_EVENT(sw_fl_dump,
> + TP_PROTO(const char *fname, int line, struct fl_tuple *ftuple),
[ ... ]
> + TP_printk("[%s:%d] %pM %pI4:%u to %pM %pI4:%u eth_type=%#x proto=%u uni=%u in=%#x out=%#x",
> + __get_str(f), __entry->l, __entry->smac, &__entry->sip, __entry->sport,
[ ... ]
> +);
> +
> +TRACE_EVENT(sw_act_dump,
> + TP_PROTO(const char *fname, int line, u32 act),
[ ... ]
> + TP_printk("[%s:%d] %u",
> + __get_str(fname), __entry->line, __entry->act)
> +);
[Low]
These tracepoints capture __func__ and __LINE__ from the call site and
render them in TP_printk. Since tracepoint output is treated as
stable ABI (see Documentation/trace/tracepoints.rst), any future
refactor in sw_fl.c will silently change the visible trace output.
Since the events are also EXPORT_TRACEPOINT_SYMBOL'd despite only being
used from this driver, would netdev_dbg / dynamic-debug fit the intent
better than TRACE_EVENT here?
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH v2 net-next 8/9] octeontx2: offload host FIB updates to switch via AF mailbox
From: Paolo Abeni @ 2026-07-09 9:56 UTC (permalink / raw)
To: rkannoth
Cc: linux-kernel, netdev, andrew+netdev, davem, edumazet, kuba,
pabeni, sgoutham
In-Reply-To: <20260702045026.2914748-9-rkannoth@marvell.com>
From: AI Reviewer <ai@example.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
octeontx2: offload host FIB updates to switch via AF mailbox
This change wires up an IPv4/IPv6 FIB offload path: the switch PF pushes
fib_entry batches to the RVU AF via a fib_notify mbox, and the AF forwards
them back down to the switch PF via a new af2swdev FIB_CMD. A pair of
workqueues on both ends batch and post the entries.
Some questions follow inline.
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> index 28ebf0a8d3d6..ae6f145a0db0 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> @@ -1968,7 +1968,7 @@ struct af2swdev_notify_req {
> };
> struct {
> u8 cnt;
> - struct fib_entry entry[16];
> + struct fib_entry entry[12];
> };
[Critical]
Can this shrink cause an out-of-bounds write in
rvu_sw_l3_offl_rule_push()? The AF-side handler still accepts up to 16
entries per fib_notify:
if (req->cnt > 16)
return -EINVAL;
and the batching guard in sw_l3_offl_work_handler()
if (lcnt + l3_entry->cnt > 8 && !list_empty(&l3_local_lh))
only splits when the local list is already non-empty, so the first
l3_entry in a batch is always accepted regardless of its cnt. A single
fib_notify with cnt in [13,16] therefore reaches
memcpy(dst, entry, sizeof(*entry) * cnt);
writing up to 16 struct fib_entry into req->entry[12], and overflows the
mbox message by 1..4 entries (roughly 60..240 bytes) into adjacent mbox
memory.
Also, req->cnt is a u8 while tot_cnt is int, so any total that batches
past 255 would silently truncate on the assignment
req->cnt = tot_cnt;
Would it be clearer to either raise entry[] back to 16, cap fib_notify at
12, or clamp/split at the memcpy site with tot_cnt validated against a
u8?
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw_l3.c b/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw_l3.c
> index 2b798d5f0644..bd4ec3b20d4a 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw_l3.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw_l3.c
[ ... ]
> +static struct workqueue_struct *sw_l3_offl_wq;
> +
> +struct l3_entry {
> + struct list_head list;
> + struct rvu *rvu;
> + u32 port_id;
> + int cnt;
> + struct fib_entry entry[];
> +};
> +
> +static DEFINE_MUTEX(l3_offl_llock);
> +static LIST_HEAD(l3_offl_lh);
> +static bool l3_offl_work_running;
> +
> +static struct workqueue_struct *sw_l3_offl_wq;
[Low]
Is the second declaration of sw_l3_offl_wq intentional? It appears once
above the struct l3_entry definition and again immediately after, so the
translation unit has two tentative definitions of the same object.
> +static void sw_l3_offl_work_handler(struct work_struct *work);
> +static DECLARE_DELAYED_WORK(l3_offl_work, sw_l3_offl_work_handler);
> +
> +static int rvu_sw_l3_offl_rule_push(struct list_head *lh)
> +{
[ ... ]
> + dst = &req->entry[0];
> + while ((l3_entry =
> + list_first_entry_or_null(lh,
> + struct l3_entry, list)) != NULL) {
> + entry = l3_entry->entry;
> + cnt = l3_entry->cnt;
> + sz = sizeof(*entry) * cnt;
> +
> + memcpy(dst, entry, sz);
> + tot_cnt += cnt;
> + dst += cnt;
> +
> + list_del_init(&l3_entry->list);
> + kfree(l3_entry);
> + }
> + req->flags = FIB_CMD;
> + req->cnt = tot_cnt;
> +
> + rc = otx2_mbox_wait_for_zero(&rvu->afpf_wq_info.mbox_up, swdev_pf);
> + if (rc)
> + otx2_mbox_msg_send_up(&rvu->afpf_wq_info.mbox_up, swdev_pf);
> +
> + mutex_unlock(&rvu->mbox_lock);
> + return rc ? 0 : -EFAULT;
> +}
[High]
When otx2_mbox_wait_for_zero() returns false (mbox still busy after the
~1ms wait), otx2_mbox_msg_send_up() is skipped but the loop above has
already drained every l3_entry from lh and kfree()d them.
Does this silently drop the batch? The allocated mbox message is left
dangling in the up-channel, the fib_entry payload has been freed, and
the caller only logs pr_err() without any re-queue, so the switch state
appears to permanently diverge from the host FIB whenever the mbox is
momentarily busy.
> +static atomic64_t req_cnt;
> +static atomic64_t ack_cnt;
> +static atomic64_t req_processed;
> +static LIST_HEAD(l3_local_lh);
> +static int lcnt;
> +
> +static void sw_l3_offl_work_handler(struct work_struct *work)
> +{
> + struct l3_entry *l3_entry;
> + struct list_head l3lh;
> + u64 req, ack, proc;
> +
> + INIT_LIST_HEAD(&l3lh);
> +
> + mutex_lock(&l3_offl_llock);
> + while (1) {
> + l3_entry = list_first_entry_or_null(&l3_offl_lh, struct l3_entry, list);
> +
> + if (!l3_entry)
> + break;
> +
> + if (lcnt + l3_entry->cnt > 8 && !list_empty(&l3_local_lh)) {
> + req = atomic64_read(&req_cnt);
> + atomic64_set(&ack_cnt, req);
> + atomic64_set(&req_processed, req);
> + mutex_unlock(&l3_offl_llock);
> + goto process;
> + }
> +
> + lcnt += l3_entry->cnt;
> +
> + atomic64_inc(&req_cnt);
> + list_del_init(&l3_entry->list);
> + list_add_tail(&l3_entry->list, &l3_local_lh);
> + }
> + mutex_unlock(&l3_offl_llock);
> +
> + req = atomic64_read(&req_cnt);
> + ack = atomic64_read(&ack_cnt);
> +
> + if (req > ack) {
> + atomic64_set(&ack_cnt, req);
> + queue_delayed_work(sw_l3_offl_wq, &l3_offl_work,
> + msecs_to_jiffies(100));
> + return;
> + }
> +
> + proc = atomic64_read(&req_processed);
> + if (req == proc) {
> + queue_delayed_work(sw_l3_offl_wq, &l3_offl_work,
> + msecs_to_jiffies(1000));
> + return;
> + }
> +
> + atomic64_set(&req_processed, req);
[Medium]
Do req_cnt, ack_cnt, and req_processed model any real acknowledgement
protocol? They are only ever written from inside this function and
neither otx2_mbox_wait_for_zero() nor otx2_mbox_msg_send_up() touches
them, so the (req > ack) branch effectively only ever fires on the first
invocation after new entries arrive and then
atomic64_set(&ack_cnt, req);
immediately equalizes them. Would this be clearer as a plain "if new
work arrived, re-poll shortly, otherwise poll slowly" without the ack
counters?
[Medium]
Do the two early-return branches leak the entries parked in l3_local_lh?
The while loop above moves items from l3_offl_lh into the module-static
l3_local_lh, and then both
if (req > ack) { ... return; }
if (req == proc) { ... return; }
return without splicing l3_local_lh into l3lh for processing. If
rvu_sw_l3_shutdown() then runs, it only drains l3_offl_lh:
while (1) {
entry = list_first_entry_or_null(&l3_offl_lh,
struct l3_entry, list);
so entries parked in l3_local_lh remain allocated. Would it be safer to
splice l3_local_lh back into l3_offl_lh before returning, or drain both
lists in the shutdown path?
> +
> +process:
> + lcnt = 0;
[ ... ]
> +int rvu_mbox_handler_fib_notify(struct rvu *rvu,
> struct fib_notify_req *req,
> struct msg_rsp *rsp)
> {
> + struct l3_entry *l3_entry;
> + int sz;
> +
> + if (!(rvu->rswitch.flags & RVU_SWITCH_FLAG_FW_READY))
> + return 0;
[High]
Should this return an error instead of 0 when firmware is not ready?
Returning 0 tells the caller the update was accepted, but nothing is
buffered or resynced later, so any FIB event that arrives during the
pre-ready window is silently dropped. Because FIB deltas are
edge-triggered, a single missed add/del leaves the offload permanently
divergent from the host FIB with no user-visible signal.
> +
> + if (req->cnt > 16)
> + return -EINVAL;
> +
> + sz = req->cnt * sizeof(struct fib_entry);
> +
> + l3_entry = kcalloc(1, sizeof(*l3_entry) + sz, GFP_KERNEL);
> + if (!l3_entry)
> + return -ENOMEM;
> +
> + l3_entry->port_id = rvu_sw_port_id(rvu, req->hdr.pcifunc);
> + l3_entry->rvu = rvu;
> + l3_entry->cnt = req->cnt;
> + INIT_LIST_HEAD(&l3_entry->list);
> + memcpy(l3_entry->entry, req->entry, sz);
> +
> + mutex_lock(&l3_offl_llock);
> + list_add_tail(&l3_entry->list, &l3_offl_lh);
> + mutex_unlock(&l3_offl_llock);
> +
> + if (!l3_offl_work_running) {
> + sw_l3_offl_wq = alloc_workqueue("sw_af_fib_wq", 0, 0);
> + l3_offl_work_running = true;
> + queue_delayed_work(sw_l3_offl_wq, &l3_offl_work,
> + msecs_to_jiffies(1000));
> + }
> +
> return 0;
> }
[High]
Is the return value of alloc_workqueue() handled anywhere? Under memory
pressure it can return NULL and queue_delayed_work() will then
dereference a NULL pool. Additionally l3_offl_work_running is set to
true unconditionally, so on the next invocation the branch is skipped
and every future fib_notify piles a new l3_entry onto l3_offl_lh with
no worker to drain it.
[High]
Is this check-then-set race-safe? It runs outside l3_offl_llock, and
mbox handlers can dispatch from separate PF worker contexts. Two
concurrent invocations can both observe l3_offl_work_running == false,
both call alloc_workqueue(), and one of the pointers is then overwritten
into sw_l3_offl_wq while the other workqueue leaks along with any
delayed work already queued on it.
Related: rvu_sw_l3_shutdown() destroys sw_l3_offl_wq but does not clear
sw_l3_offl_wq to NULL or reset l3_offl_work_running:
> +void rvu_sw_l3_shutdown(void)
> +{
> + struct l3_entry *entry;
> + LIST_HEAD(tlist);
> +
> + if (!sw_l3_offl_wq)
> + return;
> +
> + cancel_delayed_work_sync(&l3_offl_work);
> + destroy_workqueue(sw_l3_offl_wq);
> +
> + mutex_lock(&l3_offl_llock);
> + while (1) {
> + entry = list_first_entry_or_null(&l3_offl_lh,
> + struct l3_entry, list);
> + if (!entry)
> + break;
> +
> + list_del_init(&entry->list);
> + kfree(entry);
> + }
> + mutex_unlock(&l3_offl_llock);
> +}
After shutdown, if rvu_mbox_handler_fib_notify() runs again it sees
l3_offl_work_running still true and calls queue_delayed_work() on the
destroyed workqueue. Would it help to move the workqueue lifetime to
subsystem init/deinit (like rvu_sw_l2) and always clear both pieces of
state here?
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_fib.c b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_fib.c
> index 12ddf8119372..54b854aa7ffa 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_fib.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_fib.c
[ ... ]
> +static void sw_fib_work_handler(struct work_struct *work)
> +{
> + struct sw_fib_list_entry *lentry;
> + LIST_HEAD(tlist);
> +
> + spin_lock_bh(&sw_fib_llock);
> + list_splice_init(&sw_fib_lh, &tlist);
> + spin_unlock_bh(&sw_fib_llock);
> +
> + while ((lentry =
> + list_first_entry_or_null(&tlist,
> + struct sw_fib_list_entry, lh)) != NULL) {
> + list_del_init(&lentry->lh);
> + sw_fib_notify(lentry->pf, lentry->cnt, lentry->entry);
> + kfree(lentry->entry);
> + kfree(lentry);
> + }
[High]
Should the return value of sw_fib_notify() be checked? It can fail via
cnt > 16 returning -EINVAL, mbox alloc returning -ENOMEM, or
otx2_sync_mbox_msg() transport failure, and all three paths simply drop
the FIB delta with no retry and no diagnostic. Because FIB updates are
edge-triggered, a single lost message leaves the switch state
permanently out of sync.
> +
> + spin_lock_bh(&sw_fib_llock);
> + if (!list_empty(&sw_fib_lh))
> + queue_delayed_work(sw_fib_wq, &sw_fib_work,
> + msecs_to_jiffies(10));
> + spin_unlock_bh(&sw_fib_llock);
> +}
> +
> +int sw_fib_add_to_list(struct net_device *dev,
> + struct fib_entry *entry, int cnt)
> +{
> + struct otx2_nic *pf = netdev_priv(dev);
> + struct sw_fib_list_entry *lentry;
> +
> + lentry = kcalloc(1, sizeof(*lentry), GFP_ATOMIC);
> + if (!lentry)
> + return -ENOMEM;
[High]
Does this leak the caller's entry on ENOMEM? Every call site in this
patch (sw_nb_v4_netdev_event, sw_nb_v4_inetaddr_event,
sw_nb_v4_fib_event x2, sw_nb_net_v4_neigh_update, sw_nb_v6_netdev_event,
sw_nb_v6_fib_event, sw_nb_net_v6_neigh_update, sw_nb_v6_inetaddr_event)
hands ownership of the entry to this function and drops the reference:
- kfree(entry);
+ sw_fib_add_to_list(pf_dev, entry, 1);
The return value is ignored in every caller, so on GFP_ATOMIC failure
the fib_entry buffer is stranded. Would it make sense to kfree(entry)
on the error path (or take ownership only after the lentry allocation
succeeds)?
[Medium]
Is there a cnt upper bound? sw_fib_notify() rejects cnt > 16 but
sw_fib_add_to_list() does not, and sw_nb_v4_fib_event() computes
cnt = iter - entries;
bounded by fi->fib_nhs. For multipath configurations with more than 16
next-hops the entry is happily queued and then silently discarded inside
the worker. Should the notifier either split the batch or refuse early
so the sender can adjust?
> +
> + lentry->pf = pf;
> + lentry->cnt = cnt;
> + lentry->entry = entry;
> + INIT_LIST_HEAD(&lentry->lh);
> +
> + spin_lock(&sw_fib_llock);
> + list_add_tail(&lentry->lh, &sw_fib_lh);
> + queue_delayed_work(sw_fib_wq, &sw_fib_work,
> + msecs_to_jiffies(10));
> + spin_unlock(&sw_fib_llock);
> +
> + return 0;
> +}
[High]
Is sw_fib_llock protected against softirq reentry here? This function
uses plain spin_lock()/spin_unlock() but sw_fib_work_handler() and
sw_fib_deinit() both acquire the same lock with spin_lock_bh().
sw_nb_net_v4_neigh_update() and sw_nb_net_v6_neigh_update() are called
from the netevent chain, which the neighbour subsystem can invoke in
softirq context (e.g., from __neigh_update() paths driven by RX
processing). If CPU X is holding sw_fib_llock via spin_lock() in
process context and a softirq on the same CPU reaches
sw_fib_add_to_list() as well, the softirq spins waiting on a lock that
only its preempted process context can release.
Would spin_lock_bh() be more appropriate here?
[ ... ]
> +void sw_fib_deinit(void)
> {
> + struct sw_fib_list_entry *lentry;
> + LIST_HEAD(tlist);
> +
> + cancel_delayed_work_sync(&sw_fib_work);
> + destroy_workqueue(sw_fib_wq);
> +
> + spin_lock_bh(&sw_fib_llock);
> + list_splice_init(&sw_fib_lh, &tlist);
> + spin_unlock_bh(&sw_fib_llock);
> +
> + while ((lentry =
> + list_first_entry_or_null(&tlist,
> + struct sw_fib_list_entry, lh)) != NULL) {
> + list_del_init(&lentry->lh);
> + kfree(lentry->entry);
> + kfree(lentry);
> + }
> }
[High]
Is there anything in this module that prevents sw_fib_add_to_list() from
being called during or after sw_fib_deinit()? The current caller order
in sw_nb_unregister() unregisters notifiers first, but sw_fib.c has no
self-contained shutdown flag or nulling of sw_fib_wq. A stray notifier
callback that reaches sw_fib_add_to_list() after destroy_workqueue()
would call queue_delayed_work() on a freed workqueue pointer. Would
setting sw_fib_wq to NULL under the lock and checking it in
sw_fib_add_to_list() be robust against future ordering changes?
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb.c b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb.c
> index 99b8b9fdfe8a..61f0ed26adfd 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb.c
> @@ -301,11 +305,9 @@ static int sw_nb_netdev_event(struct notifier_block *unused,
> if (idev)
> sw_nb_v4_netdev_event(unused, event, ptr);
>
> -#if IS_ENABLED(CONFIG_IPV6)
> i6dev = __in6_dev_get(dev);
> if (i6dev)
> sw_nb_v6_netdev_event(unused, event, ptr);
> -#endif
>
> return NOTIFY_DONE;
> }
[High]
Does this drop the guard on the wrong caller? The other callers of the
v6 helpers in this file (register/unregister, sw_nb_v6_fib_event,
sw_nb_v6_inetaddr_event dispatch, netevent v6 dispatch) still sit under
their own IS_ENABLED(CONFIG_IPV6) blocks, and drivers/net/ethernet/
marvell/octeontx2/nic/Makefile only compiles sw_nb_v6.o under
ifdef CONFIG_IPV6
so sw_nb_v6.c's body is wrapped in the same #if. On
CONFIG_OCTEONTX_SWITCH=y with CONFIG_IPV6=n, Kconfig does not select or
depend on IPV6, so the reference to sw_nb_v6_netdev_event() would remain
in the object file with no definition to link against. Should this
guard be restored, or should sw_nb_v6.h grow a static inline stub for
!CONFIG_IPV6?
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb_v4.c b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb_v4.c
> index 947dafe586a0..8fd02edaa90e 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb_v4.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb_v4.c
> @@ -134,7 +134,7 @@ int sw_nb_v4_inetaddr_event(struct notifier_block *nb,
> netdev_dbg(dev, "%s: pushing inetaddr event from HOST interface address %#x, %pM, %s\n",
> __func__, entry->dst, entry->mac, dev->name);
>
> - kfree(entry);
> + sw_fib_add_to_list(pf_dev, entry, 1);
> return NOTIFY_DONE;
> }
[High]
Not introduced by this patch, but the switch to sw_fib_add_to_list()
lengthens the exposure of a pre-existing kcalloc(GFP_ATOMIC) that is
never NULL-checked:
entry = kcalloc(1, sizeof(*entry), GFP_ATOMIC);
entry->cmd = sw_nb_inetaddr_event_to_otx2_event(event, dev);
Previously a NULL entry crashed here at the immediate dereference;
now the pointer also gets stored in a work item. The same pattern
exists in sw_nb_net_v4_neigh_update(), sw_nb_v6_netdev_event()
(GFP_KERNEL), sw_nb_net_v6_neigh_update(), and
sw_nb_v6_inetaddr_event(). Could the NULL checks be added while
touching these paths?
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH v2 net-next 7/9] octeontx2: plumb bridge FDB updates through AF and switchdev
From: Paolo Abeni @ 2026-07-09 9:56 UTC (permalink / raw)
To: rkannoth
Cc: linux-kernel, netdev, andrew+netdev, davem, edumazet, kuba,
pabeni, sgoutham
In-Reply-To: <20260702045026.2914748-8-rkannoth@marvell.com>
From: AI Reviewer <ai@example.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
octeontx2: plumb bridge FDB updates through AF and switchdev
This patch wires switchdev FDB add/delete notifications on the PF into
mailbox fdb_notify messages toward the AF, adds an AF worker to push L2
rules to the switchdev firmware, and introduces a new AF-to-PF/VF
MBOX_MSG_AF2PF_FDB_REFRESH message that ends up calling
call_switchdev_notifiers(SWITCHDEV_FDB_ADD_TO_BRIDGE, ...) on the target.
A few questions inline below.
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
> index 6e10a58a0421..0f751555338e 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
[ ... ]
> @@ -2579,6 +2580,7 @@ static void __rvu_mbox_up_handler(struct rvu_work *mwork, int type)
>
> switch (msg->id) {
> case MBOX_MSG_CGX_LINK_EVENT:
> + case MBOX_MSG_AF2PF_FDB_REFRESH:
> break;
> default:
> if (msg->rc)
[Medium]
Adding MBOX_MSG_AF2PF_FDB_REFRESH to the same case as MBOX_MSG_CGX_LINK_EVENT
suppresses the msg->rc check. If the PF returns a non-zero rc for
AF2PF_FDB_REFRESH (for example MBOX_MSG_INVALID, see the next comment about
PF-side dispatch), is the intent to silently swallow it here or would it
be preferable to log it like every other message?
> @@ -3821,6 +3823,8 @@ static void rvu_remove(struct pci_dev *pdev)
> {
> struct rvu *rvu = pci_get_drvdata(pdev);
>
> + rvu_sw_shutdown();
> +
> rvu_dbg_exit(rvu);
> rvu_unregister_dl(rvu);
> rvu_unregister_interrupts(rvu);
[High]
rvu_sw_shutdown() runs at the top of rvu_remove(), well before
rvu_mbox_destroy(&rvu->afpf_wq_info). While rvu_sw_l2_shutdown() does
cancel_work_sync() and destroy_workqueue() on both workqueues, it never
clears RVU_SWITCH_FLAG_FW_READY, never nulls the file-static
rvu_sw_l2_offl_wq / fdb_refresh_wq pointers, and does not stop new mbox
handlers from producing.
Can this race happen?
CPU0 (rvu_remove) CPU1 (afpf mbox worker)
rvu_sw_l2_shutdown()
cancel_work_sync(&fdb_...)
destroy_workqueue(fdb_refresh_wq) /* wq freed */
rvu_mbox_handler_fdb_notify()
if (flags & FW_READY) /* still set */
queue_work(rvu_sw_l2_offl_wq, ...);
/* UAF on freed wq */
Should the shutdown order be inverted (tear down mbox producers first, or
clear FW_READY under a lock that mbox handlers observe, then flush and
destroy the workqueues, then null the pointers)?
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw.c b/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw.c
> index 28f8281e9219..6fc13aeff45f 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw.c
[ ... ]
> @@ -26,5 +28,22 @@ int rvu_mbox_handler_swdev2af_notify(struct rvu *rvu,
> struct swdev2af_notify_req *req,
> struct msg_rsp *rsp)
> {
> - return 0;
> + int rc = 0;
> +
> + switch (req->msg_type) {
> + case SWDEV2AF_MSG_TYPE_FW_STATUS:
> + rc = rvu_sw_l2_init_offl_wq(rvu, req->pcifunc, req->fw_up);
> + break;
> +
> + case SWDEV2AF_MSG_TYPE_REFRESH_FDB:
> + rc = rvu_sw_l2_fdb_list_entry_add(rvu, req->pcifunc, req->mac);
> + break;
> + }
> +
> + return rc;
> +}
[High]
Is there a reason this handler accepts SWDEV2AF messages from any pcifunc?
There is no is_cgx_config_permitted() style check that the sender is the
trusted switch-firmware PF.
The FW_STATUS path stores the caller-supplied req->pcifunc into
rvu->rswitch.pcifunc, and the REFRESH_FDB path forwards a caller-controlled
(pcifunc, mac) tuple to any PF via AF2PF_FDB_REFRESH — which ends up
invoking call_switchdev_notifiers(SWITCHDEV_FDB_ADD_TO_BRIDGE, ...) on the
target PF's netdev.
In an SR-IOV deployment, does this allow any VF to inject arbitrary
MAC->port entries into another tenant's bridge, or hijack the switch-firmware
identity by sending FW_STATUS(fw_up=true) with its own pcifunc?
[Medium]
The switch has no default: arm. SWDEV2AF_MSG_TYPE_REFRESH_FL is already
defined as BIT_ULL(2) but has no case here, so REFRESH_FL messages return
rc = 0 with no side effect and no diagnostic. Would returning
-EOPNOTSUPP / -EINVAL for unknown msg_type values make version-mismatch
issues visible?
[Low]
Also, since the SWDEV2AF_MSG_TYPE_* values are BIT_ULL(0), BIT_ULL(1),
BIT_ULL(2), the switch-equality only matches when exactly one bit is set.
Is msg_type intended to be a bitmask (in which case case labels won't
combine) or a plain enum (in which case using BIT_ULL() as the values is
misleading)?
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw_l2.c b/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw_l2.c
> index 5f805bfa81ed..bd0fe13b1fc9 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw_l2.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/switch/rvu_sw_l2.c
[ ... ]
> +/* Work queue for switchdev message handling. There is only
> + * on switch HW per SoC, so one instance of each type of
> + * workqueue is enough.
> + */
> +static struct rvu_sw_l2_work l2_offl_work;
> +static struct workqueue_struct *rvu_sw_l2_offl_wq;
[Low]
Nit: "There is only on switch HW" is missing the word "one".
[ ... ]
> +static int rvu_sw_l2_fdb_refresh(struct rvu *rvu, u16 pcifunc, u8 *mac)
> +{
> + struct af2pf_fdb_refresh_req *req;
> + int pf, vidx;
> +
> + pf = rvu_get_pf(rvu->pdev, pcifunc);
> +
> + mutex_lock(&rvu->mbox_lock);
> +
> + if (pf) {
> + req = otx2_mbox_alloc_msg_af2pf_fdb_refresh(rvu, pf);
> + if (!req) {
> + mutex_unlock(&rvu->mbox_lock);
> + return -ENOMEM;
> + }
> +
> + req->hdr.pcifunc = pcifunc;
> + ether_addr_copy(req->mac, mac);
> + req->pcifunc = pcifunc;
> +
> + otx2_mbox_wait_for_zero(&rvu->afpf_wq_info.mbox_up, pf);
> + otx2_mbox_msg_send_up(&rvu->afpf_wq_info.mbox_up, pf);
> + } else {
> + vidx = pcifunc - 1;
> +
> + req = (struct af2pf_fdb_refresh_req *)
> + otx2_mbox_alloc_msg_rsp(&rvu->afvf_wq_info.mbox_up, vidx,
> + sizeof(*req), sizeof(struct msg_rsp));
[Critical]
Where is the bounds check on pcifunc / vidx? pcifunc originates from
req->pcifunc of the peer-sent swdev2af_notify_req and reaches here verbatim
via rvu_sw_l2_fdb_list_entry_add() -> l2_entry->port_id.
In the else branch, when rvu_get_pf(rvu->pdev, pcifunc) == 0:
vidx = pcifunc - 1;
otx2_mbox_alloc_msg_rsp(&rvu->afvf_wq_info.mbox_up, vidx, ...);
If a peer sends REFRESH_FDB with pcifunc == 0, vidx becomes -1 and
otx2_mbox_alloc_msg_rsp() does:
struct otx2_mbox_dev *mdev = &mbox->dev[devid];
spin_lock(&mdev->mbox_lock);
which is an out-of-bounds access on mbox->dev[-1] followed by
otx2_mbox_wait_for_zero() / otx2_mbox_msg_send_up() dereferencing the
same address.
Similarly in the if branch, pf is used as devid for
rvu->afpf_wq_info.mbox_up without checking pf < afpf_wq_info.mbox_up.ndevs.
Would something like is_pf_func_valid() or an explicit ndevs range check
before dispatching the message be needed here?
[ ... ]
> +static bool fw_is_up;
> +
> +int rvu_sw_l2_init_offl_wq(struct rvu *rvu, u16 pcifunc, bool fw_up)
> +{
> + struct rvu_switch *rswitch;
> +
> + rswitch = &rvu->rswitch;
> +
> + if (fw_up && !fw_is_up) {
> + /* Switch HW sends an MBOX message to Host once it is ready */
> + rswitch->pcifunc = pcifunc;
> + fw_is_up = true;
> +
> + l2_offl_work.rvu = rvu;
> + INIT_WORK(&l2_offl_work.work, rvu_sw_l2_offl_rule_wq_handler);
> + rvu_sw_l2_offl_wq = alloc_workqueue("swdev_rvu_sw_l2_offl_wq", 0, 0);
> + if (!rvu_sw_l2_offl_wq) {
> + dev_err(rvu->dev, "L2 offl workqueue allocation failed\n");
> + return -ENOMEM;
> + }
> +
> + fdb_refresh_work.rvu = rvu;
> + INIT_WORK(&fdb_refresh_work.work, rvu_sw_l2_fdb_refresh_wq_handler);
> + fdb_refresh_wq = alloc_workqueue("swdev_fdb_refresg_wq", 0, 0);
> + if (!fdb_refresh_wq) {
> + dev_err(rvu->dev, "fdb refresh workqueue allocation failed\n");
> + return -ENOMEM;
> + }
> +
> + rswitch->flags |= RVU_SWITCH_FLAG_FW_READY;
> + return 0;
> + }
> +
> + rswitch->flags &= ~RVU_SWITCH_FLAG_FW_READY;
> + flush_work(&l2_offl_work.work);
> + rswitch->pcifunc = -1;
> + return 0;
> +}
[High]
Where does fw_is_up ever get reset back to false? It is never cleared —
not on fw_up=false, not on rvu_sw_l2_shutdown(), not on the -ENOMEM
paths above.
Consequences:
1. A firmware bounce (up -> down -> up) enters the second fw_up=true
message with fw_is_up already true. The guard fails, and the
else tail runs instead — actively clearing FW_READY,
flush_work()ing, and setting rswitch->pcifunc = -1.
2. Any fw_up=false event after init permanently disables offload,
since fw_up=true never re-enters the allocation branch.
3. rswitch->pcifunc is u16 and is assigned -1, which becomes 0xFFFF,
then flows back into rvu_get_pf() as a bogus PF index used as
an mbox devid.
Should fw_is_up be cleared on fw_up=false and on shutdown?
[High]
On the second alloc_workqueue failing while the first has succeeded,
this returns -ENOMEM without destroying rvu_sw_l2_offl_wq. Because
fw_is_up was set to true a few lines earlier, no subsequent FW_STATUS
message can retry the init. Additionally, rvu_sw_l2_shutdown() begins
with:
if (!fdb_refresh_wq)
return;
so if the driver ends up in this partial-init state, shutdown does
nothing and rvu_sw_l2_offl_wq (with a live worker kthread) leaks for
the module lifetime.
Could the error path destroy_workqueue(rvu_sw_l2_offl_wq), NULL the
pointer, and clear fw_is_up before returning?
[Medium]
The tail unconditionally runs:
flush_work(&l2_offl_work.work);
but INIT_WORK(&l2_offl_work.work, ...) only runs inside the if (fw_up &&
!fw_is_up) branch. If the very first swdev2af message received is
FW_STATUS(fw_up=false) — or any message that misses the init branch —
the work_struct is still BSS-zero-initialized when flush_work runs.
__flush_work() does WARN_ON(!work->func). Is that intended?
[Low]
Also, is the workqueue name "swdev_fdb_refresg_wq" a typo of "refresh"?
The name shows up in /proc/ and worker thread names.
[ ... ]
> +int rvu_sw_l2_fdb_list_entry_add(struct rvu *rvu, u16 pcifunc, u8 *mac)
> +{
> + struct l2_entry *l2_entry;
> +
> + l2_entry = kcalloc(1, sizeof(*l2_entry), GFP_KERNEL);
> + if (!l2_entry)
> + return -ENOMEM;
> +
> + l2_entry->port_id = pcifunc;
> + ether_addr_copy(l2_entry->mac, mac);
> +
> + mutex_lock(&fdb_refresh_list_lock);
> + list_add_tail(&l2_entry->list, &fdb_refresh_lh);
> + mutex_unlock(&fdb_refresh_list_lock);
> +
> + queue_work(fdb_refresh_wq, &fdb_refresh_work.work);
> + return 0;
> +}
[High]
This handler is reachable directly from rvu_mbox_handler_swdev2af_notify()
via SWDEV2AF_MSG_TYPE_REFRESH_FDB, and it does not check
RVU_SWITCH_FLAG_FW_READY or that fdb_refresh_wq is non-NULL (the sibling
rvu_mbox_handler_fdb_notify() below does check FW_READY).
If a peer sends REFRESH_FDB before any FW_STATUS(fw_up=true), fdb_refresh_wq
is still NULL and queue_work(NULL, ...) reaches __queue_work() which
dereferences wq->flags. Is the mbox ordering guaranteed by some external
mechanism, or should this path also gate on FW_READY / a non-NULL wq?
[Medium]
This function allocates and appends unconditionally on every incoming
REFRESH_FDB message, with no cap on list length or per-caller quota.
Combined with the identical pattern in rvu_mbox_handler_fdb_notify()
below, can a peer that sends messages faster than the workqueue drains
them drive unbounded kernel memory allocation until OOM? The workqueue
handler processes only 16 entries per invocation before re-queueing under
a mutex + mbox send + wait.
[ ... ]
> int rvu_mbox_handler_fdb_notify(struct rvu *rvu,
> struct fdb_notify_req *req,
> struct msg_rsp *rsp)
> {
> + struct l2_entry *l2_entry;
> +
> + if (!(rvu->rswitch.flags & RVU_SWITCH_FLAG_FW_READY))
> + return 0;
> +
> + l2_entry = kcalloc(1, sizeof(*l2_entry), GFP_KERNEL);
> + if (!l2_entry)
> + return -ENOMEM;
> +
> + l2_entry->port_id = rvu_sw_port_id(rvu, req->hdr.pcifunc);
> + ether_addr_copy(l2_entry->mac, req->mac);
> + l2_entry->flags = req->flags;
> +
> + mutex_lock(&l2_offl_list_lock);
> + list_add_tail(&l2_entry->list, &l2_offl_lh);
> + mutex_unlock(&l2_offl_list_lock);
> +
> + queue_work(rvu_sw_l2_offl_wq, &l2_offl_work.work);
> +
> return 0;
> }
[High]
Is the RVU_SWITCH_FLAG_FW_READY check safe against concurrent teardown?
The flag is read here, several statements happen (kcalloc, list mutex
acquire/release), and then queue_work() runs. Between the check and
queue_work(), another thread running rvu_sw_l2_init_offl_wq(fw_up=false)
or rvu_sw_shutdown() can clear the flag and destroy rvu_sw_l2_offl_wq,
leaving queue_work() to touch a freed workqueue.
An ad-hoc flag check without a covering lock (or RCU with
synchronize_rcu() before destroy_workqueue()) does not provide
synchronization. How is this expected to work?
> +void rvu_sw_l2_shutdown(void)
> +{
> + struct l2_entry *entry;
> + LIST_HEAD(tlist);
> +
> + if (!fdb_refresh_wq)
> + return;
> +
> + cancel_work_sync(&fdb_refresh_work.work);
> + destroy_workqueue(fdb_refresh_wq);
> +
> + mutex_lock(&fdb_refresh_list_lock);
> + while (1) {
> + entry = list_first_entry_or_null(&fdb_refresh_lh,
> + struct l2_entry, list);
> + if (!entry)
> + break;
> +
> + list_del_init(&entry->list);
> + kfree(entry);
> + }
> + mutex_unlock(&fdb_refresh_list_lock);
> +
> + cancel_work_sync(&l2_offl_work.work);
> + destroy_workqueue(rvu_sw_l2_offl_wq);
[High]
cancel_work_sync() only cancels currently-pending work; it does not stop
new invocations. Because RVU_SWITCH_FLAG_FW_READY is not cleared here,
rvu_mbox_handler_fdb_notify() can still pass its FW_READY check and call
queue_work(rvu_sw_l2_offl_wq, ...) after this destroy_workqueue(). Also,
rvu_sw_l2_fdb_list_entry_add() has no FW_READY guard at all.
Should the FW_READY flag be cleared (and the mbox handlers taught to see
that) before cancel_work_sync + destroy_workqueue, and should the two
global workqueue pointers be nulled at the end so the shutdown guard
"if (!fdb_refresh_wq) return;" actually distinguishes torn-down from
initialized?
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
> index b022f52c6845..4540ffacdddc 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
[ ... ]
> @@ -141,6 +142,22 @@ static int otx2vf_process_mbox_msg_up(struct otx2_nic *vf,
> err = otx2_mbox_up_handler_cgx_link_event(
> vf, (struct cgx_link_info_msg *)req, rsp);
> return err;
> +
> + case MBOX_MSG_AF2PF_FDB_REFRESH:
> + rsp = (struct msg_rsp *)otx2_mbox_alloc_msg(&vf->mbox.mbox_up, 0,
> + sizeof(struct msg_rsp));
> + if (!rsp)
> + return -ENOMEM;
> +
> + rsp->hdr.id = MBOX_MSG_AF2PF_FDB_REFRESH;
> + rsp->hdr.sig = OTX2_MBOX_RSP_SIG;
> + rsp->hdr.pcifunc = req->pcifunc;
> + rsp->hdr.rc = 0;
> + err = otx2_mbox_up_handler_af2pf_fdb_refresh(vf,
> + (struct af2pf_fdb_refresh_req *)req,
> + rsp);
> + return err;
> +
[High]
The AF-to-PF path is wired only in otx2_vf.c. Is the PF-side dispatcher
(otx2_process_mbox_msg_up() in nic/otx2_pf.c) updated in a companion patch?
Grepping this tree, otx2_pf.c's switch expands MBOX_UP_CGX_MESSAGES,
MBOX_UP_MCS_MESSAGES and MBOX_UP_REP_MESSAGES, but not
MBOX_UP_AF2PF_FDB_REFRESH_MESSAGES. When rvu_sw_l2_fdb_refresh() takes
its if (pf) branch and sends AF2PF_FDB_REFRESH over afpf_wq_info.mbox_up,
the receiving PF hits the default: arm and calls
otx2_reply_invalid_msg(), so the SWITCHDEV_FDB_ADD_TO_BRIDGE notifier is
never invoked on PF netdevs.
Since bridges are typically hosted on PF netdevs, this seems to defeat
the "invoke the switchdev notifier so the host bridge can learn the
updated FDB entry" goal from the commit message for the PF case.
[Medium]
Looking at struct af2pf_fdb_refresh_req, there is only pcifunc + mac[6]
— no flags — and otx2_mbox_up_handler_af2pf_fdb_refresh() hardcodes
SWITCHDEV_FDB_ADD_TO_BRIDGE. The downstream fdb_notify direction carries
FDB_ADD/FDB_DEL bits in req->flags, but the upstream refresh direction
can only add. If firmware ages an entry out, how does the host bridge
learn to delete it — is a delete-refresh planned, and if so should the
mailbox structure grow a flags field now to avoid an ABI change later?
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_fdb.c b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_fdb.c
> index 6842c8d91ffc..327a6efeb526 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_fdb.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_fdb.c
[ ... ]
> +static DEFINE_SPINLOCK(sw_fdb_llock);
> +static LIST_HEAD(sw_fdb_lh);
[ ... ]
> +static void sw_fdb_wq_handler(struct work_struct *work)
> +{
> + struct sw_fdb_list_entry *entry;
> + LIST_HEAD(tlist);
> +
> + spin_lock(&sw_fdb_llock);
> + list_splice_init(&sw_fdb_lh, &tlist);
> + spin_unlock(&sw_fdb_llock);
[ ... ]
> + spin_lock(&sw_fdb_llock);
> + if (!list_empty(&sw_fdb_lh))
> + queue_work(sw_fdb_wq, &sw_fdb_work);
> + spin_unlock(&sw_fdb_llock);
> +}
> +
> +int sw_fdb_add_to_list(struct net_device *dev, u8 *mac, bool add_fdb)
> +{
> + struct otx2_nic *pf = netdev_priv(dev);
> + struct sw_fdb_list_entry *entry;
> +
> + entry = kcalloc(1, sizeof(*entry), GFP_ATOMIC);
> + if (!entry)
> + return -ENOMEM;
> +
> + ether_addr_copy(entry->mac, mac);
> + entry->add_fdb = add_fdb;
> + entry->pf = pf;
> + dev_hold(dev);
> +
> + spin_lock(&sw_fdb_llock);
> + list_add_tail(&entry->list, &sw_fdb_lh);
> + queue_work(sw_fdb_wq, &sw_fdb_work);
> + spin_unlock(&sw_fdb_llock);
> +
> + return 0;
> +}
[High]
Can this deadlock? sw_fdb_add_to_list() is called from sw_nb_fdb_event()
via call_switchdev_notifiers(), and switchdev_notif_chain is an
ATOMIC_NOTIFIER_HEAD in net/switchdev/switchdev.c, so notifier callbacks
run in the caller's context — including softirq/BH context from bridge
learn paths like br_handle_frame_finish -> br_fdb_update ->
br_switchdev_fdb_notify -> call_switchdev_notifiers.
Meanwhile sw_fdb_wq_handler() runs in process context and takes
spin_lock(&sw_fdb_llock) without disabling BH.
process ctx (workqueue) softirq on same CPU
spin_lock(&sw_fdb_llock) (bridge learn arrives)
call_switchdev_notifiers()
sw_nb_fdb_event()
sw_fdb_add_to_list()
spin_lock(&sw_fdb_llock)
/* spins forever */
Should this be spin_lock_bh() / spin_unlock_bh() everywhere sw_fdb_llock
is taken?
[ ... ]
> int sw_fdb_init(void)
> {
> + INIT_WORK(&sw_fdb_work, sw_fdb_wq_handler);
> + sw_fdb_wq = alloc_workqueue("sw_fdb_wq", 0, 0);
> + if (!sw_fdb_wq)
> + return -ENOMEM;
> +
> return 0;
> }
[High]
sw_fdb_init() now returns -ENOMEM on alloc_workqueue failure, but
sw_nb_register() (drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb.c)
calls it as:
sw_fdb_init();
sw_fib_init();
sw_fl_init();
with the return value discarded. If sw_fdb_wq stays NULL, any subsequent
SWITCHDEV_FDB_ADD/DEL_TO_DEVICE event calls sw_fdb_add_to_list() ->
queue_work(NULL, ...), and later sw_fdb_deinit() calls
destroy_workqueue(NULL), each of which dereferences the NULL wq pointer.
Would it make sense to propagate the error out of sw_nb_register()?
[ ... ]
> +int otx2_mbox_up_handler_af2pf_fdb_refresh(struct otx2_nic *pf,
> + struct af2pf_fdb_refresh_req *req,
> + struct msg_rsp *rsp)
> +{
> + struct switchdev_notifier_fdb_info item = {0};
> +
> + item.addr = req->mac;
> + item.info.dev = pf->netdev;
> + call_switchdev_notifiers(SWITCHDEV_FDB_ADD_TO_BRIDGE,
> + item.info.dev, &item.info, NULL);
> +
> + return 0;
> }
> +#endif
> +EXPORT_SYMBOL(otx2_mbox_up_handler_af2pf_fdb_refresh);
[Medium]
otx2_mbox_up_handler_af2pf_fdb_refresh() is defined and exported from
sw_fdb.c, which the Makefile links into rvu_nicpf. otx2_vf.c (which
lives in rvu_nicvf) calls it, creating a hard module dependency: the VF
driver will fail to load without the PF driver present. Is that
intended? In VF-passthrough VMs, only rvu_nicvf is loaded.
Also, is EXPORT_SYMBOL (rather than EXPORT_SYMBOL_GPL) intentional for a
driver-internal helper?
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH v2 net-next 6/9] octeontx2-pf: register switch notifiers for eswitch offload
From: Paolo Abeni @ 2026-07-09 9:56 UTC (permalink / raw)
To: rkannoth
Cc: linux-kernel, netdev, andrew+netdev, davem, edumazet, kuba,
pabeni, sgoutham
In-Reply-To: <20260702045026.2914748-7-rkannoth@marvell.com>
From: AI Reviewer <ai@example.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
octeontx2-pf: register switch notifiers for eswitch offload
This change registers switchdev, FIB, netevent, inetaddr, inet6addr,
and netdevice notifiers when devlink switch mode is enabled, adds
sw_nb_v4.c/sw_nb_v6.c for family-specific handlers, and extends
sw_nb.c with device filtering for Cavium ports behind bridges and
VLANs.
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/rep.c b/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> index 257a2ae6a53e..e4c01ac87477 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> @@ -399,6 +400,9 @@ static void rvu_rep_get_stats64(struct net_device *dev,
>
> static int rvu_eswitch_config(struct otx2_nic *priv, u8 ena)
> {
> +#if IS_ENABLED(CONFIG_OCTEONTX_SWITCH)
> + struct net_device *netdev = priv->netdev;
> +#endif
> struct devlink_port_attrs attrs = {};
> struct esw_cfg_req *req;
>
> @@ -414,6 +418,11 @@ static int rvu_eswitch_config(struct otx2_nic *priv, u8 ena)
> memcpy(req->switch_id, attrs.switch_id.id, attrs.switch_id.id_len);
> otx2_sync_mbox_msg(&priv->mbox);
> mutex_unlock(&priv->mbox.lock);
> +
> +#if IS_ENABLED(CONFIG_OCTEONTX_SWITCH)
> + ena ? sw_nb_register(netdev) : sw_nb_unregister(netdev);
> +#endif
> +
> return 0;
> }
[Medium]
Is the return value from sw_nb_register() and sw_nb_unregister()
intentionally discarded here?
sw_nb_register() has six register_*_notifier() calls and a full
err1..err5 unwind path, but the ternary drops any error and the caller
returns 0. The mbox esw_cfg_req has already been committed to the AF
before this line runs, so on notifier registration failure the AF sees
switch mode enabled while the PF has no observation infrastructure,
and devlink still reports success.
[Medium]
The notifier_block instances (sw_nb_fdb, sw_nb_fib, sw_nb_netevent,
sw_nb_v4_inetaddr, sw_nb_v6_inetaddr, sw_nb_netdev) and sw_nb_pf_netdev
are file-scope singletons in sw_nb.c. When devlink switch mode is
enabled on a second octeontx2 PF (multi-port CN10K parts have a devlink
instance per PF), sw_nb_register() re-registers the same static
notifier_block pointers.
For notifier chains that keep a single linked list of nodes, adding the
same node twice either fails with -EEXIST (silently, per the previous
comment) or corrupts the chain. sw_nb_pf_netdev is also unconditionally
overwritten so any netdev_info/netdev_dbg(sw_nb_pf_netdev, ...) is
attributed to the wrong PF, and once the first PF's netdev is freed
the pointer becomes a use-after-free for any callback that still
dereferences it.
Should these notifier_blocks live in a per-PF structure instead?
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb.c b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb.c
> index 2d14a0590c5d..fb183265a9aa 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb.c
[ ... ]
> +bool sw_nb_is_valid_dev(struct net_device *netdev)
> +{
> + struct netdev_nested_priv priv;
> + struct net_device *br;
> + int cnt = 0;
> +
> + priv.flags = true;
> + priv.data = &cnt;
> +
> + if (netif_is_bridge_master(netdev) || is_vlan_dev(netdev)) {
> + /* sw_nb_is_valid_dev() is invoked in notifier callback context, so
> + * rtnl_lcok is already acquired
> + */
> + netdev_walk_all_lower_dev(netdev, sw_nb_check_slaves, &priv);
> + return priv.flags && !!*(int *)priv.data;
> + }
> +
> + if (netif_is_bridge_port(netdev)) {
> + br = netdev_master_upper_dev_get_rcu(netdev);
> + if (!br)
> + return false;
> +
> + netdev_walk_all_lower_dev(br, sw_nb_check_slaves, &priv);
> + return priv.flags && !!*(int *)priv.data;
> + }
> +
> + return sw_nb_is_cavium_dev(netdev);
> +}
[High]
Is the assumption in the comment ("rtnl_lcok is already acquired")
correct for all callers?
sw_nb_is_valid_dev() is invoked from:
sw_nb_net_event() - netevent chain, atomic_notifier
sw_nb_v4_fib_event() - FIB chain, atomic_notifier
sw_nb_v6_fib_event() - FIB chain, atomic_notifier
sw_nb_fdb_event() - switchdev, atomic
sw_nb_netdev_event() - netdevice chain, rtnl-held
register_netevent_notifier() and register_fib_notifier() install into
atomic notifier chains (see net/core/netevent.c and
net/core/fib_notifier.c), so those callbacks are not guaranteed to run
with rtnl held.
netdev_walk_all_lower_dev() (the non-RCU variant) walks
dev->adj_list.lower without rcu_dereference, and
netdev_master_upper_dev_get_rcu() dereferences adj_list.upper without
an outer rcu_read_lock. Concurrent adjacency changes under rtnl can
then race the walk. Also, note the typo "rtnl_lcok" in the comment.
[ ... ]
> +static int sw_nb_netdev_event(struct notifier_block *unused,
> + unsigned long event, void *ptr)
> +{
[ ... ]
> +}
> +
> +static struct notifier_block sw_nb_netdev = {
> + .notifier_call = sw_nb_netdev_event,
> +};
> +
> +int sw_nb_unregister(struct net_device *netdev)
> +{
[ ... ]
> +}
> +EXPORT_SYMBOL(sw_nb_unregister);
> +
> +int sw_nb_register(struct net_device *netdev)
> +{
[ ... ]
> +}
> +EXPORT_SYMBOL(sw_nb_register);
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb_v4.c b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb_v4.c
> new file mode 100644
> index 000000000000..947dafe586a0
> --- /dev/null
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb_v4.c
[ ... ]
> +int sw_nb_v4_inetaddr_event(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
> + struct in_ifaddr *ifa = (struct in_ifaddr *)ptr;
> + struct net_device *dev = ifa->ifa_dev->dev;
> + struct net_device *lower, *pf_dev;
> + struct netdev_hw_addr *dev_addr;
> + struct fib_entry *entry;
> + struct in_device *idev;
> + struct list_head *iter;
> + struct otx2_nic *pf;
> +
> + if (event != NETDEV_CHANGE &&
> + event != NETDEV_UP &&
> + event != NETDEV_DOWN) {
> + return NOTIFY_DONE;
> + }
> +
> + idev = __in_dev_get_rtnl(dev);
> + if (!idev || !idev->ifa_list)
> + return NOTIFY_DONE;
> +
> + entry = kcalloc(1, sizeof(*entry), GFP_ATOMIC);
> + entry->cmd = sw_nb_inetaddr_event_to_otx2_event(event, dev);
[High]
Can kcalloc() return NULL here? The very next line dereferences entry
without a NULL check. The same pattern is present in
sw_nb_net_v4_neigh_update(), sw_nb_v6_netdev_event(),
sw_nb_v6_inetaddr_event(), and sw_nb_net_v6_neigh_update(). Two other
handlers in this same patch (sw_nb_v4_netdev_event and
sw_nb_v4_fib_event) do have the NULL check.
These callbacks run from atomic notifier chains (netevent, inet6addr)
during frequent events such as ARP/ND updates and address changes,
which are triggerable by an unprivileged party sending traffic on the
interface.
[Medium]
For the bridge-master branch below:
> + pf_dev = dev;
> + if (netif_is_bridge_master(dev)) {
> + entry->bridge = 1;
> + netdev_for_each_lower_dev(dev, lower, iter) {
> + pf_dev = lower;
> + break;
> + }
> + } else if (is_vlan_dev(dev)) {
> + entry->vlan_valid = 1;
> + pf_dev = vlan_dev_real_dev(dev);
> + entry->vlan_tag = vlan_dev_vlan_id(dev);
> + }
> +
> + pf = netdev_priv(pf_dev);
> + entry->port_id = pf->pcifunc;
What happens when the bridge has zero slaves? pf_dev stays equal to
the bridge netdev itself, and netdev_priv(bridge) returns
struct net_bridge, not struct otx2_nic. Reading pcifunc off that
pointer produces an out-of-type read.
Unlike sw_nb_v4_fib_event() and sw_nb_net_v4_neigh_update(), this
handler is not gated by sw_nb_is_valid_dev(), so an empty bridge with
no ports still reaches this code.
[ ... ]
> +int sw_nb_v4_fib_event(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
[ ... ]
> + entries = kcalloc(fi->fib_nhs, sizeof(*entries), GFP_ATOMIC);
> + if (!entries)
> + return NOTIFY_DONE;
> +
> + haddr = kcalloc(fi->fib_nhs, sizeof(u32), GFP_ATOMIC);
> + if (!haddr) {
> + kfree(entries);
> + return NOTIFY_DONE;
> + }
[ ... ]
> + cnt = iter - entries;
> + if (!cnt)
> + return NOTIFY_DONE;
> +
> + netdev_dbg(pf_dev, "pf_dev is %s cnt=%d\n", pf_dev->name, cnt);
> + kfree(entries);
> +
> + if (!hcnt)
> + return NOTIFY_DONE;
> +
> + entries = kcalloc(hcnt, sizeof(*entries), GFP_ATOMIC);
> + if (!entries)
> + return NOTIFY_DONE;
[High]
Does this leak entries and haddr on the early-return paths?
The if (!cnt) return path returns before freeing entries or haddr.
The if (!hcnt) return path frees entries just above but then returns
without freeing haddr.
The second kcalloc failure path returns having freed only the first
entries allocation, leaving haddr allocated.
FIB events fire on every route change; a system where
sw_nb_is_valid_dev() rejects every nexthop will leak on every event.
This appears to be addressed in the follow-up commit
7f7c9aba2d92 ("octeontx2: offload host FIB updates to switch via AF
mailbox") which adds kfree() calls on each branch, but the leak is
present in this commit as-is.
[ ... ]
> +int sw_nb_net_v4_neigh_update(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
[ ... ]
> + entry = kcalloc(1, sizeof(*entry), GFP_ATOMIC);
> + entry->cmd = OTX2_NEIGH_UPDATE;
> + entry->dst = (__force u32)htonl(*(u32 *)n->primary_key);
> + entry->dst_len = n->tbl->key_len * 8;
> + entry->mac_valid = 1;
> + entry->nud_state = n->nud_state;
> + neigh_ha_snapshot(entry->mac, n, n->dev);
> + ether_addr_copy(entry->mac, n->ha);
[Medium]
Why does this call neigh_ha_snapshot() followed immediately by a
plain ether_addr_copy() on n->ha?
neigh_ha_snapshot() uses read_seqbegin/read_seqretry against
n->ha_lock precisely because n->ha updates from neigh_update() take
write_seqlock(&n->ha_lock) and are not atomic word-writes on all
architectures. The subsequent ether_addr_copy() is an unsynchronized
read of the same field and overwrites the snapshot with a possibly
torn value.
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb_v6.c b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb_v6.c
> new file mode 100644
> index 000000000000..cc908f565d24
> --- /dev/null
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/switch/sw_nb_v6.c
[ ... ]
> +int sw_nb_v6_netdev_event(struct notifier_block *unused,
> + unsigned long event, void *ptr)
> +{
[ ... ]
> + i6dev = __in6_dev_get(dev);
> +
> + rcu_read_lock();
> + ifp = list_first_entry_or_null(&i6dev->addr_list,
> + struct inet6_ifaddr, if_list);
> + if (!ifp) {
> + rcu_read_unlock();
> + return NOTIFY_DONE;
> + }
> +
> + if (ipv6_addr_type(&ifp->addr) & IPV6_ADDR_LINKLOCAL) {
> + rcu_read_unlock();
> + return NOTIFY_DONE;
> + }
> +
> + pf = netdev_priv(dev);
> +
> + entry = kcalloc(1, sizeof(*entry), GFP_KERNEL);
> + entry->cmd = sw_nb_inetaddr_event_to_otx2_event(event, dev);
[High]
Is GFP_KERNEL safe under rcu_read_lock()? GFP_KERNEL implies
__GFP_DIRECT_RECLAIM and may sleep, which is illegal inside an RCU
read-side critical section and will trip
"BUG: sleeping function called from invalid context" with
CONFIG_DEBUG_ATOMIC_SLEEP.
Would GFP_ATOMIC, or moving the allocation outside the
rcu_read_lock() region, be more appropriate?
[High]
Is there a missing NULL check on this kcalloc()? The next statement
dereferences entry unconditionally.
[ ... ]
> +int sw_nb_v6_inetaddr_event(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
[ ... ]
> + entry = kcalloc(1, sizeof(*entry), GFP_ATOMIC);
> + entry->cmd = sw_nb_inetaddr_event_to_otx2_event(event, dev);
[High]
Same pattern as noted above - can this dereference NULL when kcalloc()
fails under memory pressure?
> +int sw_nb_net_v6_neigh_update(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
[ ... ]
> + entry = kcalloc(1, sizeof(*entry), GFP_ATOMIC);
> + entry->cmd = OTX2_NEIGH_UPDATE;
[High]
Same as above - is a NULL check missing before dereferencing entry?
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH v2 net-next 5/9] octeontx2-af: PAN switch TL1 scheduling and NPC channel control
From: Paolo Abeni @ 2026-07-09 9:56 UTC (permalink / raw)
To: rkannoth
Cc: linux-kernel, netdev, andrew+netdev, davem, edumazet, kuba,
pabeni, sgoutham
In-Reply-To: <20260702045026.2914748-6-rkannoth@marvell.com>
From: AI Reviewer <ai@example.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
octeontx2-af: PAN switch TL1 scheduling and NPC channel control
This change adds NIX_TXSCH_ALLOC_FLAG_PAN so nix_txsch_alloc can request
more than one aggregate-level (TL1) index for PAN/switch mode, adds
TXSCHQ_FREE_PAN_TL1 to permit freeing those indices via
nix_txschq_free_one, and adds a set_chanmask field to
npc_install_flow_req. It also widens the NIX LF SQ bitmap allocation
and extends struct sg_list.
A few questions inline below.
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> index 7ac26629d9af..28ebf0a8d3d6 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> @@ -1157,6 +1157,8 @@ struct nix_txsch_alloc_req {
> /* Scheduler queue count request at each level */
> u16 schq_contig[NIX_TXSCH_LVL_CNT]; /* No of contiguous queues */
> u16 schq[NIX_TXSCH_LVL_CNT]; /* No of non-contiguous queues */
> +#define NIX_TXSCH_ALLOC_FLAG_PAN BIT_ULL(0)
> + u64 flags;
> };
>
> struct nix_txsch_alloc_rsp {
> @@ -1175,6 +1177,7 @@ struct nix_txsch_alloc_rsp {
> struct nix_txsch_free_req {
> struct mbox_msghdr hdr;
> #define TXSCHQ_FREE_ALL BIT_ULL(0)
> +#define TXSCHQ_FREE_PAN_TL1 BIT_ULL(1)
> u16 flags;
> /* Scheduler queue level to be freed */
> u16 schq_lvl;
[Critical]
Should these two new flags be gated on the requester being the AF via
is_pffunc_af(req->hdr.pcifunc)?
Before this change, the aggregate level (TL1) was intentionally opaque
to non-AF callers: nix_txschq_free_one() short-circuited with
if (lvl >= hw->cap.nix_tx_aggr_lvl) return 0; and nix_txsch_alloc()
forced a single TL queue tied to the caller's mapped Tx link.
With the new flags there does not appear to be any is_pffunc_af() check
on the alloc or free paths, so any PF/VF can pass NIX_TXSCH_ALLOC_FLAG_PAN
to bypass the Tx-link mapping and touch TL1 slots that belong to
other links or other pcifuncs, or pass TXSCHQ_FREE_PAN_TL1 to free a
TL1 index. The existing comment in nix_txschq_free() says:
/* Do not clear pcifunc in txsch->pfvf_map[schq] because VF might
* be using this TL1 queue
*/
so if a PF frees a TL1 that a VF is still transmitting through, after
rvu_free_rsrc(&txsch->schq, schq) and
pfvf_map[schq] = TXSCH_MAP(0, NIX_TXSCHQ_FREE) that hardware slot is
reallocatable while the VF still uses it. Is that intended?
> @@ -2105,6 +2108,7 @@ struct npc_install_flow_req {
> u8 hw_prio;
> u8 req_kw_type; /* Key type to be written */
> u8 alloc_entry; /* only for cn20k */
> + u8 set_chanmask;
[Medium]
The commit message says:
For NPC install flow, add set_chanmask so callers can keep a
non-default chan_mask when the requester is not the AF; without
it, chan_mask was always forced to 0xFFF for non-AF functions.
Where is set_chanmask consumed? A grep across the tree finds only this
struct field definition and no reader; the existing non-AF override in
rvu_npc_fs.c is unchanged:
if (!is_pffunc_af(req->hdr.pcifunc))
req->chan_mask = rvu_get_cpt_chan_mask(rvu);
Should the corresponding "keep chan_mask" hunk that consults
set_chanmask be part of this commit, or was it meant for a follow-up?
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> index d2734f6d6d92..35c91cdc50b9 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
[ ... ]
> @@ -2138,11 +2138,14 @@ static int nix_check_txschq_alloc_req(struct rvu *rvu, int lvl, u16 pcifunc,
> if (!req_schq)
> return 0;
>
> - link = nix_get_tx_link(rvu, pcifunc);
> + if (req->flags & NIX_TXSCH_ALLOC_FLAG_PAN)
> + link = hw->cgx_links + hw->lbk_links + 1;
> + else
> + link = nix_get_tx_link(rvu, pcifunc);
>
> /* For traffic aggregating scheduler level, one queue is enough */
> if (lvl >= hw->cap.nix_tx_aggr_lvl) {
> - if (req_schq != 1)
> + if (req_schq != 1 && !(req->flags & NIX_TXSCH_ALLOC_FLAG_PAN))
> return NIX_AF_ERR_TLX_ALLOC_FAIL;
> return 0;
> }
[Critical]
Can this branch let a caller-controlled count reach the alloc loop
without a MAX_TXSCHQ_PER_FUNC ceiling?
For non-PAN, aggregate-level requests were previously rejected unless
req_schq == 1. With NIX_TXSCH_ALLOC_FLAG_PAN, the check is skipped and
this function returns 0 before the general guard further down:
if (free_cnt < req_schq || req->schq[lvl] > MAX_TXSCHQ_PER_FUNC ||
req->schq_contig[lvl] > MAX_TXSCHQ_PER_FUNC)
return NIX_AF_ERR_TLX_ALLOC_FAIL;
rvu_mbox_handler_nix_txsch_alloc() later copies the request into rsp
and iterates:
for (idx = 0; idx < req->schq_contig[lvl]; idx++) {
schq = rsp->schq_contig_list[lvl][idx];
...
pfvf_map[schq] = TXSCH_MAP(pcifunc, 0);
nix_reset_tx_linkcfg(rvu, blkaddr, lvl, schq);
nix_reset_tx_shaping(rvu, blkaddr, nixlf, lvl, schq);
nix_reset_tx_schedule(rvu, blkaddr, lvl, schq);
}
rsp->schq_contig_list[lvl] and rsp->schq_list[lvl] are each
MAX_TXSCHQ_PER_FUNC entries. If req->schq_contig[lvl] exceeds
MAX_TXSCHQ_PER_FUNC, idx runs past the array, schq is read from
adjacent memory, and pfvf_map[schq] = TXSCH_MAP(pcifunc, 0) writes at
an attacker-controlled offset. Should the PAN branch keep the
MAX_TXSCHQ_PER_FUNC bound?
[High]
The PAN link index is computed as:
link = hw->cgx_links + hw->lbk_links + 1;
and the caller then does end = link + 1. For the TL1 level,
txsch->schq.max is programmed from NIX_AF_TL1_CONST in
nix_setup_txschq():
txsch->schq.max = cfg & 0xFFFF;
Is there anything guaranteeing that hw->cgx_links + hw->lbk_links + 2
is less than txsch->schq.max for TL1? The non-PAN path is bounded
because nix_get_tx_link() only returns values in the range
[0, cgx_links + lbk_links]. The new path adds one beyond that with no
ceiling check, so set_bit(schq, txsch->schq.bmap) below and later
NIX_AF_TL1X_CIR(schq) register writes could target undefined TL1
indices on silicon where TL1 count is small.
> @@ -2179,11 +2182,41 @@ static void nix_txsch_alloc(struct rvu *rvu, struct nix_txsch *txsch,
> struct rvu_hwinfo *hw = rvu->hw;
> u16 pcifunc = rsp->hdr.pcifunc;
> int idx, schq;
> + bool alloc;
>
> /* For traffic aggregating levels, queue alloc is based
> * on transmit link to which PF_FUNC is mapped to.
> */
> if (lvl >= hw->cap.nix_tx_aggr_lvl) {
> + if (start != end) {
> + idx = 0;
> + alloc = false;
> + for (schq = start; schq <= end; schq++, idx++) {
> + if (test_bit(schq, txsch->schq.bmap))
> + continue;
> +
> + set_bit(schq, txsch->schq.bmap);
> +
> + /* A single TL queue is allocated each time */
> + if (rsp->schq_contig[lvl]) {
> + alloc = true;
> + rsp->schq_contig_list[lvl][idx] = schq;
> + continue;
> + }
> +
> + if (rsp->schq[lvl]) {
> + alloc = true;
> + rsp->schq_list[lvl][idx] = schq;
> + continue;
> + }
> + }
> +
> + if (!alloc)
> + dev_err(rvu->dev,
> + "Could not allocate schq at lvl=%u start=%u end=%u\n",
> + lvl, start, end);
> + return;
> + }
[Critical]
A few concerns on this new block.
First, idx advances unconditionally in the for-clause even when the
continue-on-busy path is taken. Consider start=L, end=L+1 where slot
L is already set in the bitmap and req->schq_contig[lvl] == 1:
iter 1: schq=L busy -> continue (idx becomes 1)
iter 2: schq=L+1 free -> set_bit(L+1); rsp->schq_contig_list[lvl][1] = L+1
rsp->schq_contig_list[lvl][0] stays as 0. The caller then loops
idx = 0 .. req->schq_contig[lvl] - 1 in rvu_mbox_handler_nix_txsch_alloc()
and picks up schq = rsp->schq_contig_list[lvl][0] = 0. That triggers
pfvf_map[0] = TXSCH_MAP(pcifunc, 0) if NIX_TXSCHQ_CFG_DONE is not set,
and nix_reset_tx_linkcfg/shaping/schedule(rvu, blkaddr, lvl, 0),
disturbing TL1[0]. Can this hijack TL1[0] from its real owner?
Second, this loop calls set_bit(schq, txsch->schq.bmap) once for each
free slot in [start, end], but never decrements
rsp->schq_contig[lvl]/rsp->schq[lvl] or otherwise reports how many
were placed into the response list. With req->schq_contig[lvl] == 1
and both slots free, both bits get set, but only one is reported back.
The bit for the extra slot then has no matching entry in pfvf_map, and
nix_txschq_free_one() gates on:
if (TXSCH_MAP_FUNC(pfvf_map[schq]) != pcifunc) {
rc = NIX_AF_ERR_TLX_INVALID;
goto err;
}
so the client cannot free it. Is the resulting bitmap leak intended?
Third, on the "no slot free" path, alloc stays false, dev_err() is
logged, and the function returns without clearing rsp->schq_contig[lvl]
or rsp->schq[lvl]. The mbox handler returns success, so consumers see
non-zero counts and then read rsp->schq_contig_list[lvl][0] == 0
(zero-initialized rsp) as a valid schq. Should this path signal
failure back to the caller?
> + /* A single TL queue is allocated */
> if (rsp->schq_contig[lvl]) {
> rsp->schq_contig[lvl] = 1;
[ ... ]
> @@ -2300,11 +2333,14 @@ int rvu_mbox_handler_nix_txsch_alloc(struct rvu *rvu,
> rsp->schq[lvl] = req->schq[lvl];
> rsp->schq_contig[lvl] = req->schq_contig[lvl];
>
> - link = nix_get_tx_link(rvu, pcifunc);
> + if (req->flags & NIX_TXSCH_ALLOC_FLAG_PAN)
> + link = hw->cgx_links + hw->lbk_links + 1;
> + else
> + link = nix_get_tx_link(rvu, pcifunc);
>
> if (lvl >= hw->cap.nix_tx_aggr_lvl) {
> start = link;
> - end = link;
> + end = link + !!(req->flags & NIX_TXSCH_ALLOC_FLAG_PAN);
> } else if (hw->cap.nix_fixed_txschq_mapping) {
> nix_get_txschq_range(rvu, pcifunc, link, &start, &end);
> } else {
[High]
The commit message states:
Add TXSCHQ_FREE_PAN_TL1 so TL1 entries in that path can be freed
via nix_txsch_free where they were previously skipped.
Is nix_txsch_free() actually updated by this patch? Only
nix_txschq_free_one() gets a new gate. The bulk path nix_txschq_free()
still has if (lvl >= hw->cap.nix_tx_aggr_lvl) continue; in both its
scan and free loops, so aggregate-level bits set by the new
nix_txsch_alloc() block above are never cleared by that path, whether
it is reached via TXSCHQ_FREE_ALL or via nix_lf_teardown() on LF close
or FLR. Was a matching hunk in nix_txschq_free() intended?
> @@ -2669,7 +2705,9 @@ static int nix_txschq_free_one(struct rvu *rvu,
> schq = req->schq;
> txsch = &nix_hw->txsch[lvl];
>
> - if (lvl >= hw->cap.nix_tx_aggr_lvl || schq >= txsch->schq.max)
> + if ((lvl >= hw->cap.nix_tx_aggr_lvl &&
> + !(req->flags & TXSCHQ_FREE_PAN_TL1)) ||
> + schq >= txsch->schq.max)
> return 0;
>
> pfvf_map = txsch->pfvf_map;
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH v2 net-next 2/9] octeontx2-af: switch: Add switch dev to AF mboxes
From: Paolo Abeni @ 2026-07-09 9:56 UTC (permalink / raw)
To: rkannoth
Cc: linux-kernel, netdev, andrew+netdev, davem, edumazet, kuba,
pabeni, sgoutham
In-Reply-To: <20260702045026.2914748-3-rkannoth@marvell.com>
From: AI Reviewer <ai@example.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
Subject: octeontx2-af: switch: Add switch dev to AF mboxes
This patch introduces a set of new AF mailbox messages so that a
separate Marvell switch device (running its own Linux) can query
interface topology, mcam features, mcam multi-entry stats, and
multi-entry flow delete/free from the AF driver, and adds a stub
swdev2af_notify handler plus two up-direction message definitions.
A few questions on the diff below.
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> index 821895a7014e..43ff97441273 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> @@ -172,6 +172,10 @@ M(FL_NOTIFY, 0x012, fl_notify, \
> fl_notify_req, msg_rsp) \
> M(FL_GET_STATS, 0x013, fl_get_stats, \
> fl_get_stats_req, fl_get_stats_rsp) \
> +M(GET_IFACE_GET_INFO, 0x014, iface_get_info, msg_req, \
> + iface_get_info_rsp) \
[Low]
The generated enum symbol for this entry becomes
MBOX_MSG_GET_IFACE_GET_INFO, which has a duplicated GET_ prefix and
does not match the handler/struct naming (iface_get_info,
iface_get_info_rsp). Was IFACE_GET_INFO intended here? This enum name
becomes part of the mbox ID namespace once released.
> +M(SWDEV2AF_NOTIFY, 0x015, swdev2af_notify, \
> + swdev2af_notify_req, msg_rsp) \
> /* CGX mbox IDs (range 0x200 - 0x3FF) */ \
[ ... ]
> @@ -1587,6 +1607,30 @@ struct npc_mcam_alloc_entry_rsp {
> u16 entry_list[NPC_MAX_NONCONTIG_ENTRIES];
> };
>
> +struct npc_flow_del_n_free_req {
> + struct mbox_msghdr hdr;
> + u16 cnt;
> + u16 entry[256]; /* Entry index to be freed */
> +};
> +
> +struct npc_mcam_get_features_rsp {
> + struct mbox_msghdr hdr;
> + u64 rx_features;
> + u64 tx_features;
> +};
> +
> +struct npc_mcam_get_mul_stats_req {
> + struct mbox_msghdr hdr;
> + int cnt;
> + u16 entry[256]; /* mcam entry */
> +};
> +
> +struct npc_mcam_get_mul_stats_rsp {
> + struct mbox_msghdr hdr;
> + int cnt;
> + u64 stat[256]; /* counter stats */
> +};
[Low]
Is there a reason cnt is a signed int in both the request and the
response here, when npc_flow_del_n_free_req above uses u16 cnt?
The handler validates via:
if (!req->cnt || req->cnt > 256) {
which is a signed compare, so a negative req->cnt passes and rsp->cnt
is then assigned from req->cnt.
Any caller (current or future) that treats rsp->cnt as an unsigned
length before iterating rsp->stat[] would go out of bounds. Would a
u16 for both cnt fields be safer?
> +
> struct npc_mcam_free_entry_req {
> struct mbox_msghdr hdr;
> u16 entry; /* Entry index to be freed */
[ ... ]
> @@ -1909,6 +1953,83 @@ struct fl_get_stats_rsp {
> u64 pkts_diff;
> };
>
> +struct af2swdev_notify_req {
> + struct mbox_msghdr hdr;
> + u64 flags;
> + u32 port_id;
> + u32 switch_id;
> + union {
> + struct {
> + u8 mac[6];
> + };
> + struct {
> + u8 cnt;
> + struct fib_entry entry[16];
> + };
> +
> + struct {
> + unsigned long cookie;
> + u64 features;
> + struct fl_tuple tuple;
> + };
> + };
> +};
[Low]
Is unsigned long intentional in a mailbox payload? All other structs
in mbox.h use fixed-width types (u8/u16/u32/u64/__be16). The commit
message notes the switch runs on a separate Linux OS, so if the two
endpoints ever differ in bitness the on-wire size of cookie changes
silently. Same question applies to struct fl_info below. Would u64
be more appropriate?
> +
> +struct af2pf_fdb_refresh_req {
> + struct mbox_msghdr hdr;
> + u16 pcifunc;
> + u8 mac[6];
> +};
> +
> +struct iface_info {
> + u64 is_vf :1;
> + u64 is_sdp :1;
> + u16 pcifunc;
> + u16 rx_chan_base;
> + u16 tx_chan_base;
> + u16 sq_cnt;
> + u16 cq_cnt;
> + u16 rq_cnt;
> + u8 rx_chan_cnt;
> + u8 tx_chan_cnt;
> + u8 tx_link;
> + u8 nix;
> +};
[Low]
Was it intended to place is_vf and is_sdp as bitfields inside a u64
storage unit? That reserves 8 bytes per entry for two bits, and this
struct is embedded IFACE_MAX times in iface_get_info_rsp. Would
u8 is_vf:1; u8 is_sdp:1; work here?
> +
> +#define IFACE_MAX (256 + 32) /* 32 PFs + 256 Vs */
> +
> +struct iface_get_info_rsp {
> + struct mbox_msghdr hdr;
> + int cnt;
> + struct iface_info info[IFACE_MAX];
> +};
[Medium]
Is IFACE_MAX = 288 sufficient? rvu_hwinfo.total_vfs is a u16 populated
from a 12-bit HW register field, so total VFs can exceed 288 on some
SoCs. The handler silently truncates (goto done) and sets rsp->cnt to
the truncated count with no overflow indication, so the caller cannot
distinguish "all interfaces" from "truncated". This fixed cap also
becomes part of the effective wire layout once released.
> +
> +struct fl_info {
> + unsigned long cookie;
> + u16 mcam_idx[2];
> + u8 dis : 1;
> + u8 uni_di : 1;
> +};
> +
> +struct swdev2af_notify_req {
> + struct mbox_msghdr hdr;
> + u64 msg_type;
> +#define SWDEV2AF_MSG_TYPE_FW_STATUS BIT_ULL(0)
> +#define SWDEV2AF_MSG_TYPE_REFRESH_FDB BIT_ULL(1)
> +#define SWDEV2AF_MSG_TYPE_REFRESH_FL BIT_ULL(2)
> + u16 pcifunc;
> + union {
> + bool fw_up; // FW_STATUS message
> +
> + u8 mac[ETH_ALEN]; // fdb refresh message
> +
> + struct { // fl refresh message
> + int cnt;
> + struct fl_info fl[64];
> + };
> + };
> +};
[Low]
Is msg_type meant to be a bitmask or a selector? It is declared u64
with BIT_ULL(0..2) flags, which suggests multiple bits may be set
simultaneously, but the payload is an anonymous union whose members
are mutually exclusive. Would an enum-style tag value (0/1/2) match
the actual semantics better than BIT_ULL()? The choice is baked into
the mailbox ABI once merged.
> +
> struct flow_msg {
> unsigned char dmac[6];
> unsigned char smac[6];
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
> index ffba56ee8a60..6e10a58a0421 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
> @@ -1990,6 +1990,122 @@ int rvu_mbox_handler_msix_offset(struct rvu *rvu, struct msg_req *req,
> return 0;
> }
>
> +int rvu_mbox_handler_iface_get_info(struct rvu *rvu, struct msg_req *req,
> + struct iface_get_info_rsp *rsp)
> +{
> + struct iface_info *info;
> + struct rvu_pfvf *pfvf;
> + int pf, vf, numvfs;
> + u16 pcifunc;
> + int tot = 0;
> + u64 cfg;
> +
> + info = rsp->info;
> + for (pf = 0; pf < rvu->hw->total_pfs; pf++) {
[Medium]
Should this handler filter what it returns based on req->hdr.pcifunc?
As written, it walks every PF and every VF and returns pcifunc,
rx/tx channel base+count, SQ/CQ/RQ counts, tx_link, NIX block and
SDP flag for all of them, with no is_pffunc_af() gate and no
ownership/privilege check.
In multi-tenant SR-IOV where distinct PFs belong to distinct tenants,
a non-privileged PF sending mbox 0x014 would receive the full
resource layout of neighbouring PFs and VFs.
Was this intended to be an AF-only or privileged-caller-only message?
> + if (tot >= IFACE_MAX)
> + goto done;
> +
[ ... ]
> +done:
> + rsp->cnt = tot;
> +
> + return 0;
> +}
> +
[ ... ]
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> index c7bc0b3a29b9..d797cd05776e 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> @@ -3544,6 +3544,45 @@ int rvu_mbox_handler_npc_mcam_free_entry(struct rvu *rvu,
> return rc;
> }
>
> +int rvu_mbox_handler_npc_flow_del_n_free(struct rvu *rvu,
> + struct npc_flow_del_n_free_req *mreq,
> + struct msg_rsp *rsp)
> +{
> + struct npc_mcam_free_entry_req sreq = { 0 };
> + struct npc_delete_flow_req dreq = { 0 };
> + struct npc_delete_flow_rsp drsp = { 0 };
> + bool err = false;
> + int ret = 0, i;
> +
> + sreq.hdr.pcifunc = mreq->hdr.pcifunc;
> + dreq.hdr.pcifunc = mreq->hdr.pcifunc;
> +
> + if (!mreq->cnt || mreq->cnt > 256) {
> + dev_err(rvu->dev, "Invalid cnt=%d\n", mreq->cnt);
> + return -EINVAL;
> + }
> +
> + for (i = 0; i < mreq->cnt; i++) {
> + dreq.entry = mreq->entry[i];
> + ret = rvu_mbox_handler_npc_delete_flow(rvu, &dreq, &drsp);
> + if (ret) {
> + dev_err(rvu->dev, "delete flow error for i=%d entry=%d\n",
> + i, mreq->entry[i]);
> + err = true;
> + }
[Low]
Can this branch ever fire? Looking at rvu_mbox_handler_npc_delete_flow()
in rvu_npc_fs.c, it unconditionally ends with return 0 and logs
per-rule failures via dev_err inside its own loop, so ret here should
always be 0. Is the delete-flow error handling reachable, or is this
dead code that hides failures?
> +
> + sreq.entry = mreq->entry[i];
> + ret = rvu_mbox_handler_npc_mcam_free_entry(rvu, &sreq, rsp);
> + if (ret) {
> + dev_err(rvu->dev, "free entry error for i=%d entry=%d\n",
> + i, mreq->entry[i]);
> + err = true;
> + }
> + }
> +
> + return err ? -EINVAL : 0;
> +}
> +
[ ... ]
> @@ -4361,6 +4400,55 @@ int rvu_mbox_handler_npc_mcam_entry_stats(struct rvu *rvu,
> return 0;
> }
>
> +int rvu_mbox_handler_npc_mcam_mul_stats(struct rvu *rvu,
> + struct npc_mcam_get_mul_stats_req *req,
> + struct npc_mcam_get_mul_stats_rsp *rsp)
> +{
> + struct npc_mcam *mcam = &rvu->hw->mcam;
> + u16 pcifunc = req->hdr.pcifunc;
> + u16 index, cntr, entry;
> + int blkaddr;
> + u64 regval;
> + u32 bank;
> +
> + if (!req->cnt || req->cnt > 256) {
> + dev_err(rvu->dev, "%s invalid request cnt=%d\n",
> + __func__, req->cnt);
> + return -EINVAL;
> + }
> +
> + blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPC, 0);
> + if (blkaddr < 0)
> + return NPC_MCAM_INVALID_REQ;
> +
> + mutex_lock(&mcam->lock);
> +
> + for (int i = 0; i < req->cnt; i++) {
> + entry = npc_cn20k_vidx2idx(req->entry[i]);
> +
> + if (npc_mcam_verify_entry(mcam, pcifunc, entry)) {
> + dev_err(rvu->dev, "%s invalid mcam index=%d\n",
> + __func__, req->entry[i]);
> + return -EINVAL;
> + }
[High]
Does this early return leak mcam->lock? mutex_lock(&mcam->lock) was
taken just above the loop, and this path returns -EINVAL without
calling mutex_unlock().
Since req->entry[i] and req->hdr.pcifunc come from the mailbox, any
PF/VF can send a request containing an entry it does not own or one
that is out of range and permanently wedge mcam->lock. That mutex
serializes all NPC MCAM operations AF-wide, so once leaked, subsequent
allocate/free/read/enable/disable/stats operations from any PF or VF
would block forever.
Neighbouring handlers in this file (npc_mcam_entry_stats,
npc_mcam_free_entry, npc_mcam_read_entry, npc_mcam_dis_entry) unlock
before returning; would a "goto exit" style unlock path work here as
well?
[High]
Should this handler branch on is_cn20k(rvu->pdev) the way
rvu_mbox_handler_npc_mcam_entry_stats() does?
The single-entry stats handler has:
if (is_cn20k(rvu->pdev)) {
regval = rvu_read64(rvu, blkaddr,
NPC_AF_CN20K_MCAMEX_BANKX_STAT_EXT(index,
bank));
rsp->stat_ena = 1;
rsp->stat = regval;
...
return 0;
}
The new multi-entry handler skips this and always issues the legacy
STAT_ACT + MATCH_STATX sequence, which does not apply on CN20K
silicon. Would that produce garbage (or unmapped-address) reads on
CN20K?
> +
> + index = entry & (mcam->banksize - 1);
> + bank = npc_get_bank(mcam, entry);
> +
> + /* read MCAM entry STAT_ACT register */
> + regval = rvu_read64(rvu, blkaddr, NPC_AF_MCAMEX_BANKX_STAT_ACT(index, bank));
> + cntr = regval & 0x1FF;
> +
> + rsp->stat[i] = rvu_read64(rvu, blkaddr, NPC_AF_MATCH_STATX(cntr));
> + rsp->stat[i] &= BIT_ULL(48) - 1;
[Medium]
Should this loop also check rvu->hw->npc_stat_ena before deriving cntr
from STAT_ACT[8:0]?
The single-entry handler does:
if (!(regval & rvu->hw->npc_stat_ena)) {
rsp->stat_ena = 0;
mutex_unlock(&mcam->lock);
return 0;
}
For entries whose statistics counter is not enabled (a legal state),
this new code will take whatever the low 9 bits happen to be as a
counter index and return MATCH_STATX(random_index) to the caller as
that entry's stat value. Would checking npc_stat_ena per entry be
appropriate here too?
> + }
> +
> + rsp->cnt = req->cnt;
> +
> + mutex_unlock(&mcam->lock);
> + return 0;
> +}
> +
--
This is an AI-generated review.
^ permalink raw reply
* [PATCH rdma-next 13/13] RDMA/selftests: Add rxe_netns_names test
From: Jiri Pirko @ 2026-07-09 9:55 UTC (permalink / raw)
To: linux-rdma
Cc: cgroups, netdev, linux-s390, linux-kselftest, jgg, leon, parav,
mbloch, cmeiohas, roman.gushchin, bvanassche, zyjzyj2000, shuah,
tj, mkoutny, hannes, alibuda, dust.li, sidraya, wenjia
In-Reply-To: <20260709095532.855647-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@nvidia.com>
Add a kselftest script that exercises per-netns RDMA device naming
with RXE. Cover duplicate names across namespaces, move conflict
handling, move-with-rename, and same-namespace rename requests.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
tools/testing/selftests/rdma/Makefile | 3 +-
tools/testing/selftests/rdma/config | 2 +
.../testing/selftests/rdma/rxe_netns_names.sh | 282 ++++++++++++++++++
3 files changed, 286 insertions(+), 1 deletion(-)
create mode 100755 tools/testing/selftests/rdma/rxe_netns_names.sh
diff --git a/tools/testing/selftests/rdma/Makefile b/tools/testing/selftests/rdma/Makefile
index 07af7f15c1bf..a91c14c45006 100644
--- a/tools/testing/selftests/rdma/Makefile
+++ b/tools/testing/selftests/rdma/Makefile
@@ -3,6 +3,7 @@ TEST_PROGS := rxe_rping_between_netns.sh \
rxe_ipv6.sh \
rxe_socket_with_netns.sh \
rxe_test_NETDEV_UNREGISTER.sh \
- rxe_sent_rcvd_bytes.sh
+ rxe_sent_rcvd_bytes.sh \
+ rxe_netns_names.sh
include ../lib.mk
diff --git a/tools/testing/selftests/rdma/config b/tools/testing/selftests/rdma/config
index 4ffb814e253b..e1ff54ec0f57 100644
--- a/tools/testing/selftests/rdma/config
+++ b/tools/testing/selftests/rdma/config
@@ -1,3 +1,5 @@
CONFIG_TUN
CONFIG_VETH
+CONFIG_DUMMY
+CONFIG_NET_NS
CONFIG_RDMA_RXE
diff --git a/tools/testing/selftests/rdma/rxe_netns_names.sh b/tools/testing/selftests/rdma/rxe_netns_names.sh
new file mode 100755
index 000000000000..a7e57706fdff
--- /dev/null
+++ b/tools/testing/selftests/rdma/rxe_netns_names.sh
@@ -0,0 +1,282 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Exercise RDMA device name handling across network namespaces.
+
+source "$(dirname "$0")/../kselftest/ktap_helpers.sh"
+
+NAME_PREFIX="rxe_netns_names_$$"
+NETDEV_PREFIX="rxn$$"
+NS1="${NAME_PREFIX}ns1"
+NS2="${NAME_PREFIX}ns2"
+RXE_A="${NAME_PREFIX}rxe_a"
+RXE_B="${NAME_PREFIX}rxe_b"
+RXE_SAME="${NAME_PREFIX}rxe_same"
+RXE_NEW="${NAME_PREFIX}rxe_new"
+DUMMY_A="${NETDEV_PREFIX}a"
+DUMMY_B="${NETDEV_PREFIX}b"
+OLD_MODE=""
+MODE_CHANGED=0
+MODS=("dummy" "rdma_rxe")
+TEST_SAME_NAMES="same RDMA device name can exist in two net namespaces"
+TEST_MOVE_CONFLICT="move without rename fails on destination name conflict"
+TEST_MOVE_RENAME="move then rename succeeds"
+TEST_COMBINED_MOVE_RENAME="move with requested destination name succeeds"
+TEST_SAME_NETNS_DUP_RENAME="same-netns rename rejects duplicate name"
+TEST_TEARDOWN_RETURN="netns delete returns device to init_net and renames on conflict"
+
+ksft_skip()
+{
+ ktap_skip_all "$*"
+ exit "$KSFT_SKIP"
+}
+
+fail()
+{
+ ktap_exit_fail_msg "$*"
+}
+
+need_cmd()
+{
+ command -v "$1" >/dev/null 2>&1 || ksft_skip "missing command: $1"
+}
+
+rdma_ns()
+{
+ local ns=$1
+
+ shift
+ ip netns exec "$ns" rdma "$@"
+}
+
+rdma_dev_exists()
+{
+ local ns=$1
+ local dev=$2
+
+ if [ -n "$ns" ]; then
+ rdma_ns "$ns" dev show "$dev" >/dev/null 2>&1
+ else
+ rdma dev show "$dev" >/dev/null 2>&1
+ fi
+}
+
+add_dummy()
+{
+ local netdev=$1
+
+ ip link add "$netdev" type dummy || return 1
+ ip link set "$netdev" up || return 1
+}
+
+add_rxe()
+{
+ local dev=$1
+ local netdev=$2
+
+ rdma link add "$dev" type rxe netdev "$netdev"
+}
+
+rdma_dev_on_netdev()
+{
+ local netdev=$1
+
+ rdma link show 2>/dev/null | awk -v want="$netdev" '
+ {
+ for (i = 1; i < NF; i++)
+ if ($i == "netdev" && $(i + 1) == want) {
+ dev = $2
+ sub(/\/.*/, "", dev)
+ print dev
+ exit
+ }
+ }'
+}
+
+wait_rdma_dev_on_netdev()
+{
+ local netdev=$1
+ local dev
+ local i
+
+ for i in $(seq 1 50); do
+ dev=$(rdma_dev_on_netdev "$netdev")
+ if [ -n "$dev" ]; then
+ echo "$dev"
+ return 0
+ fi
+ sleep 0.1
+ done
+
+ return 1
+}
+
+setup_devs()
+{
+ cleanup_devs
+
+ add_dummy "$DUMMY_A" || return 1
+ add_dummy "$DUMMY_B" || return 1
+
+ add_rxe "$RXE_A" "$DUMMY_A" || return 1
+ add_rxe "$RXE_B" "$DUMMY_B" || return 1
+}
+
+cleanup_devs()
+{
+ ip link del "$DUMMY_A" 2>/dev/null
+ ip link del "$DUMMY_B" 2>/dev/null
+}
+
+setup()
+{
+ OLD_MODE=$(rdma system show 2>/dev/null |
+ sed -n 's/.*netns \([^ ]*\).*/\1/p')
+ [ -n "$OLD_MODE" ] || ksft_skip "failed to read RDMA netns mode"
+
+ rdma system set netns exclusive >/dev/null 2>&1 ||
+ ksft_skip "rdma netns exclusive mode is not supported"
+ MODE_CHANGED=1
+
+ ip netns add "$NS1" || return 1
+ ip netns add "$NS2" || return 1
+}
+
+cleanup()
+{
+ cleanup_devs
+
+ ip netns del "$NS1" 2>/dev/null
+ ip netns del "$NS2" 2>/dev/null
+
+ if [ "$MODE_CHANGED" -eq 1 ]; then
+ rdma system set netns "$OLD_MODE" 2>/dev/null
+ fi
+
+ for m in "${MODS[@]}"; do
+ modprobe -r "$m" 2>/dev/null
+ done
+}
+
+rdma_supports_combined_move_rename()
+{
+ rdma dev help 2>&1 | grep -Eq 'netns .*name|name .*netns'
+}
+
+[ "$(id -u)" -eq 0 ] || ksft_skip "must be run as root"
+need_cmd ip
+need_cmd rdma
+need_cmd modprobe
+
+trap cleanup EXIT
+
+for m in "${MODS[@]}"; do
+ modinfo "$m" >/dev/null 2>&1 || ksft_skip "module $m not found"
+ modprobe "$m" || fail "failed to load $m"
+done
+
+setup || fail "failed to create net namespaces"
+
+ktap_print_header
+ktap_set_plan 7
+
+if setup_devs &&
+ rdma dev set "$RXE_A" netns "$NS1" &&
+ rdma_ns "$NS1" dev set "$RXE_A" name "$RXE_SAME" &&
+ rdma dev set "$RXE_B" netns "$NS2" &&
+ rdma_ns "$NS2" dev set "$RXE_B" name "$RXE_SAME" &&
+ rdma_dev_exists "$NS1" "$RXE_SAME" &&
+ rdma_dev_exists "$NS2" "$RXE_SAME"; then
+ ktap_test_pass "$TEST_SAME_NAMES"
+else
+ ktap_test_fail "$TEST_SAME_NAMES"
+fi
+cleanup_devs
+
+if ! setup_devs ||
+ ! rdma dev set "$RXE_A" netns "$NS1" ||
+ ! rdma_ns "$NS1" dev set "$RXE_A" name "$RXE_SAME" ||
+ ! rdma dev set "$RXE_B" netns "$NS2" ||
+ ! rdma_ns "$NS2" dev set "$RXE_B" name "$RXE_SAME"; then
+ ktap_test_fail "$TEST_MOVE_CONFLICT"
+elif rdma_ns "$NS1" dev set "$RXE_SAME" netns "$NS2" >/dev/null 2>&1; then
+ ktap_test_fail "$TEST_MOVE_CONFLICT"
+elif rdma_dev_exists "$NS1" "$RXE_SAME" &&
+ rdma_dev_exists "$NS2" "$RXE_SAME"; then
+ ktap_test_pass "$TEST_MOVE_CONFLICT"
+else
+ ktap_test_fail "$TEST_MOVE_CONFLICT"
+fi
+cleanup_devs
+
+if ! setup_devs; then
+ ktap_test_fail "$TEST_MOVE_RENAME"
+elif rdma dev set "$RXE_A" netns "$NS2" &&
+ rdma_ns "$NS2" dev set "$RXE_A" name "$RXE_NEW"; then
+ if rdma_dev_exists "$NS2" "$RXE_NEW" &&
+ ! rdma_dev_exists "" "$RXE_A"; then
+ ktap_test_pass "$TEST_MOVE_RENAME"
+ else
+ ktap_test_fail "$TEST_MOVE_RENAME"
+ fi
+else
+ ktap_test_fail "$TEST_MOVE_RENAME"
+fi
+cleanup_devs
+
+if ! rdma_supports_combined_move_rename; then
+ ktap_test_skip "$TEST_COMBINED_MOVE_RENAME"
+elif ! setup_devs; then
+ ktap_test_fail "$TEST_COMBINED_MOVE_RENAME"
+elif rdma dev set "$RXE_A" netns "$NS2" name "$RXE_NEW"; then
+ if rdma_dev_exists "$NS2" "$RXE_NEW" &&
+ ! rdma_dev_exists "" "$RXE_A"; then
+ ktap_test_pass "$TEST_COMBINED_MOVE_RENAME"
+ else
+ ktap_test_fail "$TEST_COMBINED_MOVE_RENAME"
+ fi
+else
+ ktap_test_fail "$TEST_COMBINED_MOVE_RENAME"
+fi
+cleanup_devs
+
+if ! setup_devs; then
+ ktap_test_fail "$TEST_SAME_NETNS_DUP_RENAME"
+elif rdma dev set "$RXE_A" name "$RXE_SAME" &&
+ rdma dev set "$RXE_B" name "$RXE_NEW"; then
+ if rdma dev set "$RXE_A" name "$RXE_NEW" >/dev/null 2>&1; then
+ ktap_test_fail "$TEST_SAME_NETNS_DUP_RENAME"
+ elif rdma_dev_exists "" "$RXE_SAME" &&
+ rdma_dev_exists "" "$RXE_NEW"; then
+ ktap_test_pass "$TEST_SAME_NETNS_DUP_RENAME"
+ else
+ ktap_test_fail "$TEST_SAME_NETNS_DUP_RENAME"
+ fi
+else
+ ktap_test_fail "$TEST_SAME_NETNS_DUP_RENAME"
+fi
+cleanup_devs
+
+if ! setup_devs; then
+ ktap_test_fail "$TEST_TEARDOWN_RETURN"
+elif ! rdma dev set "$RXE_A" name "$RXE_SAME" ||
+ ! rdma dev set "$RXE_B" netns "$NS2" ||
+ ! rdma_ns "$NS2" dev set "$RXE_B" name "$RXE_SAME" ||
+ ! rdma_dev_exists "$NS2" "$RXE_SAME"; then
+ ktap_test_fail "$TEST_TEARDOWN_RETURN"
+else
+ ip netns del "$NS2"
+ returned=$(wait_rdma_dev_on_netdev "$DUMMY_B")
+ ktap_print_msg "device returned to init_net as '${returned:-<missing>}'"
+ if rdma_dev_exists "" "$RXE_SAME" &&
+ [ -n "$returned" ] &&
+ [ "$returned" != "$RXE_SAME" ] &&
+ [ "${returned#ibdev}" != "$returned" ]; then
+ ktap_test_pass "$TEST_TEARDOWN_RETURN"
+ else
+ ktap_test_fail "$TEST_TEARDOWN_RETURN"
+ fi
+fi
+cleanup_devs
+
+ktap_finished
--
2.54.0
^ permalink raw reply related
* [PATCH rdma-next 12/13] RDMA/rxe: Implement disassociate_ucontext callback
From: Jiri Pirko @ 2026-07-09 9:55 UTC (permalink / raw)
To: linux-rdma
Cc: cgroups, netdev, linux-s390, linux-kselftest, jgg, leon, parav,
mbloch, cmeiohas, roman.gushchin, bvanassche, zyjzyj2000, shuah,
tj, mkoutny, hannes, alibuda, dust.li, sidraya, wenjia
In-Reply-To: <20260709095532.855647-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@nvidia.com>
Implement an empty disassociate_ucontext() callback so the RDMA core
can move rxe devices between net namespaces. The core requires this
callback to reset user contexts without waiting for userspace.
rxe needs no teardown here: its user-mapped queues live in
reference-counted vmalloc memory (see rxe_mmap.c) that stays valid
while userspace holds the mappings.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
drivers/infiniband/sw/rxe/rxe_verbs.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index 1ec130fee8ea..6eb10d2f0653 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -240,6 +240,10 @@ static void rxe_dealloc_ucontext(struct ib_ucontext *ibuc)
rxe_err_uc(uc, "cleanup failed, err = %d\n", err);
}
+static void rxe_disassociate_ucontext(struct ib_ucontext *ibuc)
+{
+}
+
/* pd */
static int rxe_alloc_pd(struct ib_pd *ibpd, struct ib_udata *udata)
{
@@ -1478,6 +1482,7 @@ static const struct ib_device_ops rxe_dev_ops = {
.destroy_srq = rxe_destroy_srq,
.detach_mcast = rxe_detach_mcast,
.device_group = &rxe_attr_group,
+ .disassociate_ucontext = rxe_disassociate_ucontext,
.enable_driver = rxe_enable_driver,
.get_dma_mr = rxe_get_dma_mr,
.get_hw_stats = rxe_ib_get_hw_stats,
--
2.54.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox