* Re: [PATCH net-next v2 1/6] net: hold instance lock around NETDEV_DOWN/GOING_DOWN
From: Kuniyuki Iwashima @ 2026-07-14 11:36 UTC (permalink / raw)
To: sdf.kernel; +Cc: davem, edumazet, kuba, netdev, pabeni
In-Reply-To: <20260702224150.3730033-2-sdf@fomichev.me>
From: Stanislav Fomichev <sdf.kernel@gmail.com>
Date: Thu, 2 Jul 2026 15:41:45 -0700
> Mirror what call_netdevice_register_net_notifiers does but for the
> teardown. Cover only DOWN and GOING_DOWN. UNREGISTER is still unlocked
> because of the SW devices using dev_xxx methods.
>
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> ---
> net/core/dev.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 4b3d5cfdf6e0..9d49493f4fb5 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1912,9 +1912,11 @@ static void call_netdevice_unregister_notifiers(struct notifier_block *nb,
> struct net_device *dev)
> {
> if (dev->flags & IFF_UP) {
> + netdev_lock_ops(dev);
> call_netdevice_notifier(nb, NETDEV_GOING_DOWN,
> dev);
> call_netdevice_notifier(nb, NETDEV_DOWN, dev);
FYI, this is the report I mentioned yesterday.
Most likely false-positive because open/close, which takes RTNL,
are never called under epoll lock.
https://lore.kernel.org/netdev/6a55fc56.c90005c7.37d349.001c.GAE@google.com/
^ permalink raw reply
* [PATCHv3 net-next 2/9] net: usb: centralize usbnet_cdc_zte_rx_fixup in usbnet
From: Oliver Neukum @ 2026-07-14 11:44 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, manuelebner, netdev,
linux-kernel
Cc: Oliver Neukum
In-Reply-To: <20260714114429.1073434-1-oneukum@suse.com>
This helper is used by multiple drivers using usbnet.
It is better to be provided by usbnet than one of them.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
v2:
- spelling issues
- issue with lost synchronization with SPLIT packets
drivers/net/usb/cdc_ether.c | 19 -------------------
drivers/net/usb/usbnet.c | 19 +++++++++++++++++++
2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index b4df32e18461..e688fb99c61d 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -404,25 +404,6 @@ static int usbnet_cdc_zte_bind(struct usbnet *dev, struct usb_interface *intf)
return status;
}
-/* Make sure packets have correct destination MAC address
- *
- * A firmware bug observed on some devices (ZTE MF823/831/910) is that the
- * device sends packets with a static, bogus, random MAC address (event if
- * device MAC address has been updated). Always set MAC address to that of the
- * device.
- */
-int usbnet_cdc_zte_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
-{
- if (skb->len < ETH_HLEN || !(skb->data[0] & 0x02))
- return 1;
-
- skb_reset_mac_header(skb);
- ether_addr_copy(eth_hdr(skb)->h_dest, dev->net->dev_addr);
-
- return 1;
-}
-EXPORT_SYMBOL_GPL(usbnet_cdc_zte_rx_fixup);
-
/* Ensure correct link state
*
* Some devices (ZTE MF823/831/910) export two carrier on notifications when
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 21c55d70f27c..14e9f1b1e0a2 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -2350,6 +2350,25 @@ void usbnet_cdc_status(struct usbnet *dev, struct urb *urb)
}
}
EXPORT_SYMBOL_GPL(usbnet_cdc_status);
+
+/* Make sure packets have correct destination MAC address
+ *
+ * A firmware bug observed on some devices (ZTE MF823/831/910) is that the
+ * device sends packets with a static, bogus, random MAC address (event if
+ * device MAC address has been updated). Always set MAC address to that of the
+ * device.
+ */
+int usbnet_cdc_zte_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
+{
+ if (skb->len < ETH_HLEN || !(skb->data[0] & 0x02))
+ return 1;
+
+ skb_reset_mac_header(skb);
+ ether_addr_copy(eth_hdr(skb)->h_dest, dev->net->dev_addr);
+
+ return 1;
+}
+EXPORT_SYMBOL_GPL(usbnet_cdc_zte_rx_fixup);
/*-------------------------------------------------------------------------*/
static int __init usbnet_init(void)
--
2.54.0
^ permalink raw reply related
* [PATCHv3 net-next 1/9] net: usb: move updating filter and status from cdc drivers to usbnet
From: Oliver Neukum @ 2026-07-14 11:44 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, manuelebner, netdev,
linux-kernel
Cc: Oliver Neukum
These helpers are used by multiple drivers and do not depend
on the rest. For example rndis_host will also need cdc_ether.
Leaving them in a cdc driver means that additional drivers are loaded
just as a library, not to support hardware by themselves.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
v2:
- update change log entry
drivers/net/usb/cdc_ether.c | 75 -----------------------------------
drivers/net/usb/usbnet.c | 79 +++++++++++++++++++++++++++++++++++++
2 files changed, 79 insertions(+), 75 deletions(-)
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index a0a5740590b9..b4df32e18461 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -63,35 +63,6 @@ static const u8 mbm_guid[16] = {
0xa6, 0x07, 0xc0, 0xff, 0xcb, 0x7e, 0x39, 0x2a,
};
-void usbnet_cdc_update_filter(struct usbnet *dev)
-{
- struct net_device *net = dev->net;
-
- u16 cdc_filter = USB_CDC_PACKET_TYPE_DIRECTED
- | USB_CDC_PACKET_TYPE_BROADCAST;
-
- /* filtering on the device is an optional feature and not worth
- * the hassle so we just roughly care about snooping and if any
- * multicast is requested, we take every multicast
- */
- if (net->flags & IFF_PROMISC)
- cdc_filter |= USB_CDC_PACKET_TYPE_PROMISCUOUS;
- if (!netdev_mc_empty(net) || (net->flags & IFF_ALLMULTI))
- cdc_filter |= USB_CDC_PACKET_TYPE_ALL_MULTICAST;
-
- usb_control_msg(dev->udev,
- usb_sndctrlpipe(dev->udev, 0),
- USB_CDC_SET_ETHERNET_PACKET_FILTER,
- USB_TYPE_CLASS | USB_RECIP_INTERFACE,
- cdc_filter,
- dev->intf->cur_altsetting->desc.bInterfaceNumber,
- NULL,
- 0,
- USB_CTRL_SET_TIMEOUT
- );
-}
-EXPORT_SYMBOL_GPL(usbnet_cdc_update_filter);
-
/* We need to override usbnet_*_link_ksettings in bind() */
static const struct ethtool_ops cdc_ether_ethtool_ops = {
.get_link = usbnet_get_link,
@@ -400,52 +371,6 @@ EXPORT_SYMBOL_GPL(usbnet_cdc_unbind);
* (by Brad Hards) talked with, with more functionality.
*/
-static void speed_change(struct usbnet *dev, __le32 *speeds)
-{
- dev->tx_speed = __le32_to_cpu(speeds[0]);
- dev->rx_speed = __le32_to_cpu(speeds[1]);
-}
-
-void usbnet_cdc_status(struct usbnet *dev, struct urb *urb)
-{
- struct usb_cdc_notification *event;
-
- if (urb->actual_length < sizeof(*event))
- return;
-
- /* SPEED_CHANGE can get split into two 8-byte packets */
- if (test_and_clear_bit(EVENT_STS_SPLIT, &dev->flags)) {
- speed_change(dev, (__le32 *) urb->transfer_buffer);
- return;
- }
-
- event = urb->transfer_buffer;
- switch (event->bNotificationType) {
- case USB_CDC_NOTIFY_NETWORK_CONNECTION:
- netif_dbg(dev, timer, dev->net, "CDC: carrier %s\n",
- event->wValue ? "on" : "off");
- if (netif_carrier_ok(dev->net) != !!event->wValue)
- usbnet_link_change(dev, !!event->wValue, 0);
- break;
- case USB_CDC_NOTIFY_SPEED_CHANGE: /* tx/rx rates */
- netif_dbg(dev, timer, dev->net, "CDC: speed change (len %d)\n",
- urb->actual_length);
- if (urb->actual_length != (sizeof(*event) + 8))
- set_bit(EVENT_STS_SPLIT, &dev->flags);
- else
- speed_change(dev, (__le32 *) &event[1]);
- break;
- /* USB_CDC_NOTIFY_RESPONSE_AVAILABLE can happen too (e.g. RNDIS),
- * but there are no standard formats for the response data.
- */
- default:
- netdev_err(dev->net, "CDC: unexpected notification %02x!\n",
- event->bNotificationType);
- break;
- }
-}
-EXPORT_SYMBOL_GPL(usbnet_cdc_status);
-
int usbnet_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
{
int status;
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 25518635b7b7..21c55d70f27c 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -22,6 +22,7 @@
#include <linux/init.h>
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
+#include <linux/usb/cdc.h>
#include <linux/ctype.h>
#include <linux/ethtool.h>
#include <linux/workqueue.h>
@@ -2271,6 +2272,84 @@ int usbnet_write_cmd_async(struct usbnet *dev, u8 cmd, u8 reqtype,
}
EXPORT_SYMBOL_GPL(usbnet_write_cmd_async);
+
+void usbnet_cdc_update_filter(struct usbnet *dev)
+{
+ struct net_device *net = dev->net;
+
+ u16 cdc_filter = USB_CDC_PACKET_TYPE_DIRECTED
+ | USB_CDC_PACKET_TYPE_BROADCAST;
+
+ /* filtering on the device is an optional feature and not worth
+ * the hassle so we just roughly care about snooping and if any
+ * multicast is requested, we take every multicast
+ */
+ if (net->flags & IFF_PROMISC)
+ cdc_filter |= USB_CDC_PACKET_TYPE_PROMISCUOUS;
+ if (!netdev_mc_empty(net) || (net->flags & IFF_ALLMULTI))
+ cdc_filter |= USB_CDC_PACKET_TYPE_ALL_MULTICAST;
+
+ usb_control_msg(dev->udev,
+ usb_sndctrlpipe(dev->udev, 0),
+ USB_CDC_SET_ETHERNET_PACKET_FILTER,
+ USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+ cdc_filter,
+ dev->intf->cur_altsetting->desc.bInterfaceNumber,
+ NULL,
+ 0,
+ USB_CTRL_SET_TIMEOUT
+ );
+}
+EXPORT_SYMBOL_GPL(usbnet_cdc_update_filter);
+
+static void speed_change(struct usbnet *dev, __le32 *speeds)
+{
+ dev->tx_speed = __le32_to_cpu(speeds[0]);
+ dev->rx_speed = __le32_to_cpu(speeds[1]);
+}
+
+void usbnet_cdc_status(struct usbnet *dev, struct urb *urb)
+{
+ struct usb_cdc_notification *event;
+
+ if (urb->actual_length < sizeof(*event)) {
+ /* we cannot assume the device is in sync with us*/
+ clear_bit(EVENT_STS_SPLIT, &dev->flags);
+ return;
+ }
+
+ /* SPEED_CHANGE can get split into two 8-byte packets */
+ if (test_and_clear_bit(EVENT_STS_SPLIT, &dev->flags)) {
+ speed_change(dev, (__le32 *)urb->transfer_buffer);
+ return;
+ }
+
+ event = urb->transfer_buffer;
+ switch (event->bNotificationType) {
+ case USB_CDC_NOTIFY_NETWORK_CONNECTION:
+ netif_dbg(dev, timer, dev->net, "CDC: carrier %s\n",
+ event->wValue ? "on" : "off");
+ if (netif_carrier_ok(dev->net) != !!event->wValue)
+ usbnet_link_change(dev, !!event->wValue, 0);
+ break;
+ case USB_CDC_NOTIFY_SPEED_CHANGE: /* tx/rx rates */
+ netif_dbg(dev, timer, dev->net, "CDC: speed change (len %d)\n",
+ urb->actual_length);
+ if (urb->actual_length != (sizeof(*event) + 8))
+ set_bit(EVENT_STS_SPLIT, &dev->flags);
+ else
+ speed_change(dev, (__le32 *)&event[1]);
+ break;
+ /* USB_CDC_NOTIFY_RESPONSE_AVAILABLE can happen too (e.g. RNDIS),
+ * but there are no standard formats for the response data.
+ */
+ default:
+ netdev_err(dev->net, "CDC: unexpected notification %02x!\n",
+ event->bNotificationType);
+ break;
+ }
+}
+EXPORT_SYMBOL_GPL(usbnet_cdc_status);
/*-------------------------------------------------------------------------*/
static int __init usbnet_init(void)
--
2.54.0
^ permalink raw reply related
* [PATCHv3 net-next 4/9] net: usb: use cdc_state in struct usbnet
From: Oliver Neukum @ 2026-07-14 11:44 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, manuelebner, netdev,
linux-kernel
Cc: Oliver Neukum
In-Reply-To: <20260714114429.1073434-1-oneukum@suse.com>
Remove private copies as now a central state can be used.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
drivers/net/usb/cdc_ether.c | 9 +++------
drivers/net/usb/rndis_host.c | 6 +++---
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index e688fb99c61d..76ad4ffa950a 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -85,7 +85,7 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
u8 *buf = intf->cur_altsetting->extra;
int len = intf->cur_altsetting->extralen;
struct usb_interface_descriptor *d;
- struct cdc_state *info = (void *) &dev->data;
+ struct cdc_state *info = &dev->cdc;
int status = -ENODEV;
int rndis;
bool android_rndis_quirk = false;
@@ -336,7 +336,7 @@ EXPORT_SYMBOL_GPL(usbnet_ether_cdc_bind);
void usbnet_cdc_unbind(struct usbnet *dev, struct usb_interface *intf)
{
- struct cdc_state *info = (void *) &dev->data;
+ struct cdc_state *info = &dev->cdc;
struct usb_driver *driver = driver_of(intf);
/* combined interface - nothing to do */
@@ -374,10 +374,7 @@ EXPORT_SYMBOL_GPL(usbnet_cdc_unbind);
int usbnet_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
{
int status;
- struct cdc_state *info = (void *) &dev->data;
-
- BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data)
- < sizeof(struct cdc_state)));
+ struct cdc_state *info = &dev->cdc;
status = usbnet_ether_cdc_bind(dev, intf);
if (status < 0)
diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
index 5e39d05a2d7b..d539654687c3 100644
--- a/drivers/net/usb/rndis_host.c
+++ b/drivers/net/usb/rndis_host.c
@@ -57,7 +57,7 @@ EXPORT_SYMBOL_GPL(rndis_status);
static void rndis_msg_indicate(struct usbnet *dev, struct rndis_indicate *msg,
int buflen)
{
- struct cdc_state *info = (void *)&dev->data;
+ struct cdc_state *info = &dev->cdc;
struct device *udev = &info->control->dev;
if (dev->driver_info->indication) {
@@ -90,7 +90,7 @@ static void rndis_msg_indicate(struct usbnet *dev, struct rndis_indicate *msg,
*/
int rndis_command(struct usbnet *dev, struct rndis_msg_hdr *buf, int buflen)
{
- struct cdc_state *info = (void *) &dev->data;
+ struct cdc_state *info = &dev->cdc;
struct usb_cdc_notification notification;
int master_ifnum;
int retval;
@@ -290,7 +290,7 @@ generic_rndis_bind(struct usbnet *dev, struct usb_interface *intf, int flags)
{
int retval;
struct net_device *net = dev->net;
- struct cdc_state *info = (void *) &dev->data;
+ struct cdc_state *info = &dev->cdc;
union {
void *buf;
struct rndis_msg_hdr *header;
--
2.54.0
^ permalink raw reply related
* [PATCHv3 net-next 3/9] net: usb: usbnet: add cdc_state to struct usbnet
From: Oliver Neukum @ 2026-07-14 11:44 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, manuelebner, netdev,
linux-kernel
Cc: Oliver Neukum
In-Reply-To: <20260714114429.1073434-1-oneukum@suse.com>
This allows centralisation of code using cdc_state in usbnet, reducing
code duplication. No functional change intended.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
include/linux/usb/usbnet.h | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index bbf799ccf3b3..79f48eb388ee 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -16,6 +16,14 @@
#include <linux/usb.h>
#include <linux/spinlock.h>
+struct cdc_state {
+ struct usb_cdc_header_desc *header;
+ struct usb_cdc_union_desc *u;
+ struct usb_cdc_ether_desc *ether;
+ struct usb_interface *control;
+ struct usb_interface *data;
+};
+
/* interface from usbnet core to each USB networking link we handle */
struct usbnet {
/* housekeeping */
@@ -41,6 +49,7 @@ struct usbnet {
/* protocol/interface state */
struct net_device *net;
int msg_enable;
+ struct cdc_state cdc; /* too common to leave out*/
unsigned long data[5];
u32 xid;
u32 hard_mtu; /* count any extra framing */
@@ -211,13 +220,6 @@ extern int usbnet_write_cmd_async(struct usbnet *dev, u8 cmd, u8 reqtype,
* (notably, using multiple interfaces according to the CDC
* union descriptor) get some helper code.
*/
-struct cdc_state {
- struct usb_cdc_header_desc *header;
- struct usb_cdc_union_desc *u;
- struct usb_cdc_ether_desc *ether;
- struct usb_interface *control;
- struct usb_interface *data;
-};
extern void usbnet_cdc_update_filter(struct usbnet *dev);
extern int usbnet_generic_cdc_bind(struct usbnet *, struct usb_interface *);
--
2.54.0
^ permalink raw reply related
* [PATCHv3 net-next 8/9] net: usb: usbnet: remove outdated sanity check
From: Oliver Neukum @ 2026-07-14 11:44 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, manuelebner, netdev,
linux-kernel
Cc: Oliver Neukum
In-Reply-To: <20260714114429.1073434-1-oneukum@suse.com>
The cdc state is now included as a proper member in the descriptor.
There is no point in checking whether it fits into the scratchpad
area. Just remove the check.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
drivers/net/usb/usbnet.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index ce932c81382e..0347f6887222 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -2457,9 +2457,6 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
struct usb_driver *driver = driver_of(intf);
struct usb_cdc_parsed_header header;
- if (sizeof(dev->data) < sizeof(*info))
- return -EDOM;
-
/* expect strict spec conformance for the descriptors, but
* cope with firmware which stores them in the wrong place
*/
--
2.54.0
^ permalink raw reply related
* [PATCHv3 net-next 5/9] net: usb: int51x1: drop dependency on cdc_ether
From: Oliver Neukum @ 2026-07-14 11:44 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, manuelebner, netdev,
linux-kernel
Cc: Oliver Neukum
In-Reply-To: <20260714114429.1073434-1-oneukum@suse.com>
The driver depended on cdc_ether only for usbnet_cdc_update_filter().
This has been shifted to usbnet. Drop the dependency.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
v2:
- added as missed opportunity
drivers/net/usb/Kconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index 52a5c0922c79..da0f6a138f4f 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -563,7 +563,6 @@ config USB_HSO
config USB_NET_INT51X1
tristate "Intellon PLC based usb adapter"
depends on USB_USBNET
- select USB_NET_CDCETHER
help
Choose this option if you're using a 14Mb USB-based PLC
(Powerline Communications) solution with an Intellon
--
2.54.0
^ permalink raw reply related
* [PATCHv3 net-next 9/9] net: usb: usbnet: corrections in the comments
From: Oliver Neukum @ 2026-07-14 11:44 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, manuelebner, netdev,
linux-kernel
Cc: Oliver Neukum
In-Reply-To: <20260714114429.1073434-1-oneukum@suse.com>
Correct grammar in comments.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
v3:
- added to fix concerns about language in comments
drivers/net/usb/usbnet.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 0347f6887222..76bbbfb7a24e 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -2351,12 +2351,12 @@ void usbnet_cdc_status(struct usbnet *dev, struct urb *urb)
}
EXPORT_SYMBOL_GPL(usbnet_cdc_status);
-/* Make sure packets have correct destination MAC address
+/* Make sure packets have the correct destination MAC address
*
* A firmware bug observed on some devices (ZTE MF823/831/910) is that the
- * device sends packets with a static, bogus, random MAC address (event if
- * device MAC address has been updated). Always set MAC address to that of the
- * device.
+ * device sends packets with a static, bogus, random MAC address (event if the
+ * device MAC address has been updated). Always set the MAC address to that
+ * of the device.
*/
int usbnet_cdc_zte_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
{
--
2.54.0
^ permalink raw reply related
* [PATCHv3 net-next 6/9] net: usb: move exported symbols from cdc_ether to usbnet
From: Oliver Neukum @ 2026-07-14 11:44 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, manuelebner, netdev,
linux-kernel
Cc: Oliver Neukum
In-Reply-To: <20260714114429.1073434-1-oneukum@suse.com>
Move the rest of the symbols to usbnet, so that the cdc_ether
driver does not need to be loaded as a library.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
drivers/net/usb/cdc_ether.c | 372 ------------------------------------
drivers/net/usb/usbnet.c | 356 ++++++++++++++++++++++++++++++++++
include/linux/usb/usbnet.h | 17 ++
3 files changed, 373 insertions(+), 372 deletions(-)
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 76ad4ffa950a..6bf6538f3468 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -19,378 +19,6 @@
#include <linux/usb/usbnet.h>
-#if IS_ENABLED(CONFIG_USB_NET_RNDIS_HOST)
-
-static int is_rndis(struct usb_interface_descriptor *desc)
-{
- return (desc->bInterfaceClass == USB_CLASS_COMM &&
- desc->bInterfaceSubClass == 2 &&
- desc->bInterfaceProtocol == 0xff);
-}
-
-static int is_activesync(struct usb_interface_descriptor *desc)
-{
- return (desc->bInterfaceClass == USB_CLASS_MISC &&
- desc->bInterfaceSubClass == 1 &&
- desc->bInterfaceProtocol == 1);
-}
-
-static int is_wireless_rndis(struct usb_interface_descriptor *desc)
-{
- return (desc->bInterfaceClass == USB_CLASS_WIRELESS_CONTROLLER &&
- desc->bInterfaceSubClass == 1 &&
- desc->bInterfaceProtocol == 3);
-}
-
-static int is_novatel_rndis(struct usb_interface_descriptor *desc)
-{
- return (desc->bInterfaceClass == USB_CLASS_MISC &&
- desc->bInterfaceSubClass == 4 &&
- desc->bInterfaceProtocol == 1);
-}
-
-#else
-
-#define is_rndis(desc) 0
-#define is_activesync(desc) 0
-#define is_wireless_rndis(desc) 0
-#define is_novatel_rndis(desc) 0
-
-#endif
-
-static const u8 mbm_guid[16] = {
- 0xa3, 0x17, 0xa8, 0x8b, 0x04, 0x5e, 0x4f, 0x01,
- 0xa6, 0x07, 0xc0, 0xff, 0xcb, 0x7e, 0x39, 0x2a,
-};
-
-/* We need to override usbnet_*_link_ksettings in bind() */
-static const struct ethtool_ops cdc_ether_ethtool_ops = {
- .get_link = usbnet_get_link,
- .nway_reset = usbnet_nway_reset,
- .get_drvinfo = usbnet_get_drvinfo,
- .get_msglevel = usbnet_get_msglevel,
- .set_msglevel = usbnet_set_msglevel,
- .get_ts_info = ethtool_op_get_ts_info,
- .get_link_ksettings = usbnet_get_link_ksettings_internal,
- .set_link_ksettings = NULL,
-};
-
-/* probes control interface, claims data interface, collects the bulk
- * endpoints, activates data interface (if needed), maybe sets MTU.
- * all pure cdc, except for certain firmware workarounds, and knowing
- * that rndis uses one different rule.
- */
-int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
-{
- u8 *buf = intf->cur_altsetting->extra;
- int len = intf->cur_altsetting->extralen;
- struct usb_interface_descriptor *d;
- struct cdc_state *info = &dev->cdc;
- int status = -ENODEV;
- int rndis;
- bool android_rndis_quirk = false;
- struct usb_driver *driver = driver_of(intf);
- struct usb_cdc_parsed_header header;
-
- if (sizeof(dev->data) < sizeof(*info))
- return -EDOM;
-
- /* expect strict spec conformance for the descriptors, but
- * cope with firmware which stores them in the wrong place
- */
- if (len == 0 && dev->udev->actconfig->extralen) {
- /* Motorola SB4100 (and others: Brad Hards says it's
- * from a Broadcom design) put CDC descriptors here
- */
- buf = dev->udev->actconfig->extra;
- len = dev->udev->actconfig->extralen;
- dev_dbg(&intf->dev, "CDC descriptors on config\n");
- }
-
- /* Maybe CDC descriptors are after the endpoint? This bug has
- * been seen on some 2Wire Inc RNDIS-ish products.
- */
- if (len == 0) {
- struct usb_host_endpoint *hep;
-
- hep = intf->cur_altsetting->endpoint;
- if (hep) {
- buf = hep->extra;
- len = hep->extralen;
- }
- if (len)
- dev_dbg(&intf->dev,
- "CDC descriptors on endpoint\n");
- }
-
- /* this assumes that if there's a non-RNDIS vendor variant
- * of cdc-acm, it'll fail RNDIS requests cleanly.
- */
- rndis = (is_rndis(&intf->cur_altsetting->desc) ||
- is_activesync(&intf->cur_altsetting->desc) ||
- is_wireless_rndis(&intf->cur_altsetting->desc) ||
- is_novatel_rndis(&intf->cur_altsetting->desc));
-
- memset(info, 0, sizeof(*info));
- info->control = intf;
-
- cdc_parse_cdc_header(&header, intf, buf, len);
-
- info->u = header.usb_cdc_union_desc;
- info->header = header.usb_cdc_header_desc;
- info->ether = header.usb_cdc_ether_desc;
- if (!info->u) {
- if (rndis) {
- goto skip;
- } else {
- /* in that case a quirk is mandatory */
- dev_err(&dev->udev->dev, "No union descriptors\n");
- goto bad_desc;
- }
- }
- /* we need a master/control interface (what we're
- * probed with) and a slave/data interface; union
- * descriptors sort this all out.
- */
- info->control = usb_ifnum_to_if(dev->udev, info->u->bMasterInterface0);
- info->data = usb_ifnum_to_if(dev->udev, info->u->bSlaveInterface0);
- if (!info->control || !info->data) {
- dev_dbg(&intf->dev,
- "master #%u/%p slave #%u/%p\n",
- info->u->bMasterInterface0,
- info->control,
- info->u->bSlaveInterface0,
- info->data);
- /* fall back to hard-wiring for RNDIS */
- if (rndis) {
- android_rndis_quirk = true;
- goto skip;
- }
- dev_err(&intf->dev, "bad CDC descriptors\n");
- goto bad_desc;
- }
- if (info->control != intf) {
- /* Ambit USB Cable Modem (and maybe others)
- * interchanges master and slave interface.
- */
- if (info->data == intf) {
- info->data = info->control;
- info->control = intf;
- } else {
- dev_err(&intf->dev, "bogus CDC Union\n");
- goto bad_desc;
- }
- }
-
- /* some devices merge these - skip class check */
- if (info->control == info->data)
- goto skip;
-
- /* a data interface altsetting does the real i/o */
- d = &info->data->cur_altsetting->desc;
- if (d->bInterfaceClass != USB_CLASS_CDC_DATA) {
- dev_err(&intf->dev, "slave class %u\n", d->bInterfaceClass);
- goto bad_desc;
- }
-skip:
- /* Communication class functions with bmCapabilities are not
- * RNDIS. But some Wireless class RNDIS functions use
- * bmCapabilities for their own purpose. The failsafe is
- * therefore applied only to Communication class RNDIS
- * functions. The rndis test is redundant, but a cheap
- * optimization.
- */
- if (rndis && is_rndis(&intf->cur_altsetting->desc) &&
- header.usb_cdc_acm_descriptor &&
- header.usb_cdc_acm_descriptor->bmCapabilities) {
- dev_err(&intf->dev,
- "ACM capabilities %02x, not really RNDIS?\n",
- header.usb_cdc_acm_descriptor->bmCapabilities);
- goto bad_desc;
- }
-
- if (header.usb_cdc_ether_desc && info->ether->wMaxSegmentSize) {
- dev->hard_mtu = le16_to_cpu(info->ether->wMaxSegmentSize);
- /* because of Zaurus, we may be ignoring the host
- * side link address we were given.
- */
- }
-
- if (header.usb_cdc_mdlm_desc &&
- memcmp(header.usb_cdc_mdlm_desc->bGUID, mbm_guid, 16)) {
- dev_err(&intf->dev, "GUID doesn't match\n");
- goto bad_desc;
- }
-
- if (header.usb_cdc_mdlm_detail_desc &&
- header.usb_cdc_mdlm_detail_desc->bLength <
- (sizeof(struct usb_cdc_mdlm_detail_desc) + 1)) {
- dev_err(&intf->dev, "Descriptor too short\n");
- goto bad_desc;
- }
-
-
-
- /* Microsoft ActiveSync based and some regular RNDIS devices lack the
- * CDC descriptors, so we'll hard-wire the interfaces and not check
- * for descriptors.
- *
- * Some Android RNDIS devices have a CDC Union descriptor pointing
- * to non-existing interfaces. Ignore that and attempt the same
- * hard-wired 0 and 1 interfaces.
- */
- if (rndis && (!info->u || android_rndis_quirk)) {
- info->control = usb_ifnum_to_if(dev->udev, 0);
- info->data = usb_ifnum_to_if(dev->udev, 1);
- if (!info->control || !info->data || info->control != intf) {
- dev_err(&intf->dev,
- "rndis: master #0/%p slave #1/%p\n",
- info->control,
- info->data);
- goto bad_desc;
- }
-
- } else if (!info->header || (!rndis && !info->ether)) {
- dev_err(&intf->dev, "missing cdc %s%s%sdescriptor\n",
- info->header ? "" : "header ",
- info->u ? "" : "union ",
- info->ether ? "" : "ether ");
- goto bad_desc;
- }
-
- /* claim data interface and set it up ... with side effects.
- * network traffic can't flow until an altsetting is enabled.
- */
- if (info->data != info->control) {
- status = usb_driver_claim_interface(driver, info->data, dev);
- if (status < 0) {
- dev_err(&intf->dev, "Second interface unclaimable\n");
- goto bad_desc;
- }
- }
- status = usbnet_get_endpoints(dev, info->data);
- if (status < 0) {
- dev_dbg(&intf->dev, "Mandatory endpoints missing\n");
- goto bail_out_and_release;
- }
-
- /* status endpoint: optional for CDC Ethernet, not RNDIS (or ACM) */
- if (info->data != info->control)
- dev->status = NULL;
- if (info->control->cur_altsetting->desc.bNumEndpoints == 1) {
- struct usb_endpoint_descriptor *desc;
-
- dev->status = &info->control->cur_altsetting->endpoint[0];
- desc = &dev->status->desc;
- if (!usb_endpoint_is_int_in(desc) ||
- (le16_to_cpu(desc->wMaxPacketSize)
- < sizeof(struct usb_cdc_notification)) ||
- !desc->bInterval) {
- dev_dbg(&intf->dev, "bad notification endpoint\n");
- dev->status = NULL;
- }
- }
- if (rndis && !dev->status) {
- dev_err(&intf->dev, "missing RNDIS status endpoint\n");
- status = -ENODEV;
- goto bail_out_and_release;
- }
-
- /* override ethtool_ops */
- dev->net->ethtool_ops = &cdc_ether_ethtool_ops;
-
- return 0;
-
-bail_out_and_release:
- usb_set_intfdata(info->data, NULL);
- if (info->data != info->control)
- usb_driver_release_interface(driver, info->data);
-bad_desc:
- return status;
-}
-EXPORT_SYMBOL_GPL(usbnet_generic_cdc_bind);
-
-
-/* like usbnet_generic_cdc_bind() but handles filter initialization
- * correctly
- */
-int usbnet_ether_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
-{
- int rv;
-
- rv = usbnet_generic_cdc_bind(dev, intf);
- if (rv < 0)
- goto bail_out;
-
- /* Some devices don't initialise properly. In particular
- * the packet filter is not reset. There are devices that
- * don't do reset all the way. So the packet filter should
- * be set to a sane initial value.
- */
- usbnet_cdc_update_filter(dev);
-
-bail_out:
- return rv;
-}
-EXPORT_SYMBOL_GPL(usbnet_ether_cdc_bind);
-
-void usbnet_cdc_unbind(struct usbnet *dev, struct usb_interface *intf)
-{
- struct cdc_state *info = &dev->cdc;
- struct usb_driver *driver = driver_of(intf);
-
- /* combined interface - nothing to do */
- if (info->data == info->control)
- return;
-
- /* disconnect master --> disconnect slave */
- if (intf == info->control && info->data) {
- /* ensure immediate exit from usbnet_disconnect */
- usb_set_intfdata(info->data, NULL);
- usb_driver_release_interface(driver, info->data);
- info->data = NULL;
- }
-
- /* and vice versa (just in case) */
- else if (intf == info->data && info->control) {
- /* ensure immediate exit from usbnet_disconnect */
- usb_set_intfdata(info->control, NULL);
- usb_driver_release_interface(driver, info->control);
- info->control = NULL;
- }
-}
-EXPORT_SYMBOL_GPL(usbnet_cdc_unbind);
-
-/* Communications Device Class, Ethernet Control model
- *
- * Takes two interfaces. The DATA interface is inactive till an altsetting
- * is selected. Configuration data includes class descriptors. There's
- * an optional status endpoint on the control interface.
- *
- * This should interop with whatever the 2.4 "CDCEther.c" driver
- * (by Brad Hards) talked with, with more functionality.
- */
-
-int usbnet_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
-{
- int status;
- struct cdc_state *info = &dev->cdc;
-
- status = usbnet_ether_cdc_bind(dev, intf);
- if (status < 0)
- return status;
-
- status = usbnet_get_ethernet_addr(dev, info->ether->iMACAddress);
- if (status < 0) {
- usb_set_intfdata(info->data, NULL);
- usb_driver_release_interface(driver_of(intf), info->data);
- return status;
- }
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(usbnet_cdc_bind);
-
static int usbnet_cdc_zte_bind(struct usbnet *dev, struct usb_interface *intf)
{
int status = usbnet_cdc_bind(dev, intf);
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 14e9f1b1e0a2..ce932c81382e 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -2369,6 +2369,362 @@ int usbnet_cdc_zte_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
return 1;
}
EXPORT_SYMBOL_GPL(usbnet_cdc_zte_rx_fixup);
+
+
+#if IS_ENABLED(CONFIG_USB_NET_RNDIS_HOST)
+
+static int is_rndis(struct usb_interface_descriptor *desc)
+{
+ return (desc->bInterfaceClass == USB_CLASS_COMM &&
+ desc->bInterfaceSubClass == 2 &&
+ desc->bInterfaceProtocol == 0xff);
+}
+
+static int is_activesync(struct usb_interface_descriptor *desc)
+{
+ return (desc->bInterfaceClass == USB_CLASS_MISC &&
+ desc->bInterfaceSubClass == 1 &&
+ desc->bInterfaceProtocol == 1);
+}
+
+static int is_wireless_rndis(struct usb_interface_descriptor *desc)
+{
+ return (desc->bInterfaceClass == USB_CLASS_WIRELESS_CONTROLLER &&
+ desc->bInterfaceSubClass == 1 &&
+ desc->bInterfaceProtocol == 3);
+}
+
+static int is_novatel_rndis(struct usb_interface_descriptor *desc)
+{
+ return (desc->bInterfaceClass == USB_CLASS_MISC &&
+ desc->bInterfaceSubClass == 4 &&
+ desc->bInterfaceProtocol == 1);
+}
+
+#else
+
+#define is_rndis(desc) 0
+#define is_activesync(desc) 0
+#define is_wireless_rndis(desc) 0
+#define is_novatel_rndis(desc) 0
+
+#endif
+
+/* Communications Device Class, Ethernet Control model
+ *
+ * Takes two interfaces. The DATA interface is inactive till an altsetting
+ * is selected. Configuration data includes class descriptors. There's
+ * an optional status endpoint on the control interface.
+ *
+ * This should interop with whatever the 2.4 "CDCEther.c" driver
+ * (by Brad Hards) talked with, with more functionality.
+ */
+
+int usbnet_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
+{
+ int status;
+ struct cdc_state *info = &dev->cdc;
+
+ status = usbnet_ether_cdc_bind(dev, intf);
+ if (status < 0)
+ return status;
+
+ status = usbnet_get_ethernet_addr(dev, info->ether->iMACAddress);
+ if (status < 0) {
+ usb_set_intfdata(info->data, NULL);
+ usb_driver_release_interface(driver_of(intf), info->data);
+ return status;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(usbnet_cdc_bind);
+
+/* probes control interface, claims data interface, collects the bulk
+ * endpoints, activates data interface (if needed), maybe sets MTU.
+ * all pure cdc, except for certain firmware workarounds, and knowing
+ * that rndis uses one different rule.
+ */
+int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
+{
+ u8 *buf = intf->cur_altsetting->extra;
+ int len = intf->cur_altsetting->extralen;
+ struct usb_interface_descriptor *d;
+ struct cdc_state *info = &dev->cdc;
+ int status = -ENODEV;
+ int rndis;
+ bool android_rndis_quirk = false;
+ struct usb_driver *driver = driver_of(intf);
+ struct usb_cdc_parsed_header header;
+
+ if (sizeof(dev->data) < sizeof(*info))
+ return -EDOM;
+
+ /* expect strict spec conformance for the descriptors, but
+ * cope with firmware which stores them in the wrong place
+ */
+ if (len == 0 && dev->udev->actconfig->extralen) {
+ /* Motorola SB4100 (and others: Brad Hards says it's
+ * from a Broadcom design) put CDC descriptors here
+ */
+ buf = dev->udev->actconfig->extra;
+ len = dev->udev->actconfig->extralen;
+ dev_dbg(&intf->dev, "CDC descriptors on config\n");
+ }
+
+ /* Maybe CDC descriptors are after the endpoint? This bug has
+ * been seen on some 2Wire Inc RNDIS-ish products.
+ */
+ if (len == 0) {
+ struct usb_host_endpoint *hep;
+
+ hep = intf->cur_altsetting->endpoint;
+ if (hep) {
+ buf = hep->extra;
+ len = hep->extralen;
+ }
+ if (len)
+ dev_dbg(&intf->dev,
+ "CDC descriptors on endpoint\n");
+ }
+
+ /* this assumes that if there's a non-RNDIS vendor variant
+ * of cdc-acm, it'll fail RNDIS requests cleanly.
+ */
+ rndis = (is_rndis(&intf->cur_altsetting->desc) ||
+ is_activesync(&intf->cur_altsetting->desc) ||
+ is_wireless_rndis(&intf->cur_altsetting->desc) ||
+ is_novatel_rndis(&intf->cur_altsetting->desc));
+
+ memset(info, 0, sizeof(*info));
+ info->control = intf;
+
+ cdc_parse_cdc_header(&header, intf, buf, len);
+
+ info->u = header.usb_cdc_union_desc;
+ info->header = header.usb_cdc_header_desc;
+ info->ether = header.usb_cdc_ether_desc;
+ if (!info->u) {
+ if (rndis) {
+ goto skip;
+ } else {
+ /* in that case a quirk is mandatory */
+ dev_err(&dev->udev->dev, "No union descriptors\n");
+ goto bad_desc;
+ }
+ }
+ /* we need a master/control interface (what we're
+ * probed with) and a slave/data interface; union
+ * descriptors sort this all out.
+ */
+ info->control = usb_ifnum_to_if(dev->udev, info->u->bMasterInterface0);
+ info->data = usb_ifnum_to_if(dev->udev, info->u->bSlaveInterface0);
+ if (!info->control || !info->data) {
+ dev_dbg(&intf->dev,
+ "master #%u/%p slave #%u/%p\n",
+ info->u->bMasterInterface0,
+ info->control,
+ info->u->bSlaveInterface0,
+ info->data);
+ /* fall back to hard-wiring for RNDIS */
+ if (rndis) {
+ android_rndis_quirk = true;
+ goto skip;
+ }
+ dev_err(&intf->dev, "bad CDC descriptors\n");
+ goto bad_desc;
+ }
+ if (info->control != intf) {
+ /* Ambit USB Cable Modem (and maybe others)
+ * interchanges master and slave interface.
+ */
+ if (info->data == intf) {
+ info->data = info->control;
+ info->control = intf;
+ } else {
+ dev_err(&intf->dev, "bogus CDC Union\n");
+ goto bad_desc;
+ }
+ }
+
+ /* some devices merge these - skip class check */
+ if (info->control == info->data)
+ goto skip;
+
+ /* a data interface altsetting does the real i/o */
+ d = &info->data->cur_altsetting->desc;
+ if (d->bInterfaceClass != USB_CLASS_CDC_DATA) {
+ dev_err(&intf->dev, "slave class %u\n", d->bInterfaceClass);
+ goto bad_desc;
+ }
+skip:
+ /* Communication class functions with bmCapabilities are not
+ * RNDIS. But some Wireless class RNDIS functions use
+ * bmCapabilities for their own purpose. The failsafe is
+ * therefore applied only to Communication class RNDIS
+ * functions. The rndis test is redundant, but a cheap
+ * optimization.
+ */
+ if (rndis && is_rndis(&intf->cur_altsetting->desc) &&
+ header.usb_cdc_acm_descriptor &&
+ header.usb_cdc_acm_descriptor->bmCapabilities) {
+ dev_err(&intf->dev,
+ "ACM capabilities %02x, not really RNDIS?\n",
+ header.usb_cdc_acm_descriptor->bmCapabilities);
+ goto bad_desc;
+ }
+
+ if (header.usb_cdc_ether_desc && info->ether->wMaxSegmentSize) {
+ dev->hard_mtu = le16_to_cpu(info->ether->wMaxSegmentSize);
+ /* because of Zaurus, we may be ignoring the host
+ * side link address we were given.
+ */
+ }
+
+ if (header.usb_cdc_mdlm_desc &&
+ memcmp(header.usb_cdc_mdlm_desc->bGUID, mbm_guid, 16)) {
+ dev_err(&intf->dev, "GUID doesn't match\n");
+ goto bad_desc;
+ }
+
+ if (header.usb_cdc_mdlm_detail_desc &&
+ header.usb_cdc_mdlm_detail_desc->bLength <
+ (sizeof(struct usb_cdc_mdlm_detail_desc) + 1)) {
+ dev_err(&intf->dev, "Descriptor too short\n");
+ goto bad_desc;
+ }
+
+
+
+ /* Microsoft ActiveSync based and some regular RNDIS devices lack the
+ * CDC descriptors, so we'll hard-wire the interfaces and not check
+ * for descriptors.
+ *
+ * Some Android RNDIS devices have a CDC Union descriptor pointing
+ * to non-existing interfaces. Ignore that and attempt the same
+ * hard-wired 0 and 1 interfaces.
+ */
+ if (rndis && (!info->u || android_rndis_quirk)) {
+ info->control = usb_ifnum_to_if(dev->udev, 0);
+ info->data = usb_ifnum_to_if(dev->udev, 1);
+ if (!info->control || !info->data || info->control != intf) {
+ dev_err(&intf->dev,
+ "rndis: master #0/%p slave #1/%p\n",
+ info->control,
+ info->data);
+ goto bad_desc;
+ }
+
+ } else if (!info->header || (!rndis && !info->ether)) {
+ dev_err(&intf->dev, "missing cdc %s%s%sdescriptor\n",
+ info->header ? "" : "header ",
+ info->u ? "" : "union ",
+ info->ether ? "" : "ether ");
+ goto bad_desc;
+ }
+
+ /* claim data interface and set it up ... with side effects.
+ * network traffic can't flow until an altsetting is enabled.
+ */
+ if (info->data != info->control) {
+ status = usb_driver_claim_interface(driver, info->data, dev);
+ if (status < 0) {
+ dev_err(&intf->dev, "Second interface unclaimable\n");
+ goto bad_desc;
+ }
+ }
+ status = usbnet_get_endpoints(dev, info->data);
+ if (status < 0) {
+ dev_dbg(&intf->dev, "Mandatory endpoints missing\n");
+ goto bail_out_and_release;
+ }
+
+ /* status endpoint: optional for CDC Ethernet, not RNDIS (or ACM) */
+ if (info->data != info->control)
+ dev->status = NULL;
+ if (info->control->cur_altsetting->desc.bNumEndpoints == 1) {
+ struct usb_endpoint_descriptor *desc;
+
+ dev->status = &info->control->cur_altsetting->endpoint[0];
+ desc = &dev->status->desc;
+ if (!usb_endpoint_is_int_in(desc) ||
+ (le16_to_cpu(desc->wMaxPacketSize)
+ < sizeof(struct usb_cdc_notification)) ||
+ !desc->bInterval) {
+ dev_dbg(&intf->dev, "bad notification endpoint\n");
+ dev->status = NULL;
+ }
+ }
+ if (rndis && !dev->status) {
+ dev_err(&intf->dev, "missing RNDIS status endpoint\n");
+ status = -ENODEV;
+ goto bail_out_and_release;
+ }
+
+ /* override ethtool_ops */
+ dev->net->ethtool_ops = &cdc_ether_ethtool_ops;
+
+ return 0;
+
+bail_out_and_release:
+ usb_set_intfdata(info->data, NULL);
+ if (info->data != info->control)
+ usb_driver_release_interface(driver, info->data);
+bad_desc:
+ return status;
+}
+EXPORT_SYMBOL_GPL(usbnet_generic_cdc_bind);
+
+/* like usbnet_generic_cdc_bind() but handles filter initialization
+ * correctly
+ */
+int usbnet_ether_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
+{
+ int rv;
+
+ rv = usbnet_generic_cdc_bind(dev, intf);
+ if (rv < 0)
+ goto bail_out;
+
+ /* Some devices don't initialise properly. In particular
+ * the packet filter is not reset. There are devices that
+ * don't do reset all the way. So the packet filter should
+ * be set to a sane initial value.
+ */
+ usbnet_cdc_update_filter(dev);
+
+bail_out:
+ return rv;
+}
+EXPORT_SYMBOL_GPL(usbnet_ether_cdc_bind);
+
+void usbnet_cdc_unbind(struct usbnet *dev, struct usb_interface *intf)
+{
+ struct cdc_state *info = &dev->cdc;
+ struct usb_driver *driver = driver_of(intf);
+
+ /* combined interface - nothing to do */
+ if (info->data == info->control)
+ return;
+
+ /* disconnect master --> disconnect slave */
+ if (intf == info->control && info->data) {
+ /* ensure immediate exit from usbnet_disconnect */
+ usb_set_intfdata(info->data, NULL);
+ usb_driver_release_interface(driver, info->data);
+ info->data = NULL;
+ }
+
+ /* and vice versa (just in case) */
+ else if (intf == info->data && info->control) {
+ /* ensure immediate exit from usbnet_disconnect */
+ usb_set_intfdata(info->control, NULL);
+ usb_driver_release_interface(driver, info->control);
+ info->control = NULL;
+ }
+}
+EXPORT_SYMBOL_GPL(usbnet_cdc_unbind);
+
/*-------------------------------------------------------------------------*/
static int __init usbnet_init(void)
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 79f48eb388ee..b3a77078fb10 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -16,6 +16,11 @@
#include <linux/usb.h>
#include <linux/spinlock.h>
+static const u8 mbm_guid[16] = {
+ 0xa3, 0x17, 0xa8, 0x8b, 0x04, 0x5e, 0x4f, 0x01,
+ 0xa6, 0x07, 0xc0, 0xff, 0xcb, 0x7e, 0x39, 0x2a,
+};
+
struct cdc_state {
struct usb_cdc_header_desc *header;
struct usb_cdc_union_desc *u;
@@ -304,4 +309,16 @@ extern void usbnet_status_stop(struct usbnet *dev);
extern void usbnet_update_max_qlen(struct usbnet *dev);
+/* We need to override usbnet_*_link_ksettings in bind() */
+static const struct ethtool_ops cdc_ether_ethtool_ops = {
+ .get_link = usbnet_get_link,
+ .nway_reset = usbnet_nway_reset,
+ .get_drvinfo = usbnet_get_drvinfo,
+ .get_msglevel = usbnet_get_msglevel,
+ .set_msglevel = usbnet_set_msglevel,
+ .get_ts_info = ethtool_op_get_ts_info,
+ .get_link_ksettings = usbnet_get_link_ksettings_internal,
+ .set_link_ksettings = NULL,
+};
+
#endif /* __LINUX_USB_USBNET_H */
--
2.54.0
^ permalink raw reply related
* [PATCHv3 net-next 7/9] net: usb: remove dependencies on cdc_ether
From: Oliver Neukum @ 2026-07-14 11:44 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, manuelebner, netdev,
linux-kernel
Cc: Oliver Neukum
In-Reply-To: <20260714114429.1073434-1-oneukum@suse.com>
Now that the driver no longer exports symbols to act
as a library for other drivers other drivers don't
depend on it. Remove the dependencies.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
v3:
- removed unneeded parantheses
drivers/net/usb/Kconfig | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index da0f6a138f4f..732e0d5f331d 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -258,7 +258,6 @@ config USB_NET_CDC_EEM
config USB_NET_CDC_NCM
tristate "CDC NCM support"
depends on USB_USBNET
- select USB_NET_CDCETHER
default y
help
This driver provides support for CDC NCM (Network Control Model
@@ -399,7 +398,6 @@ config USB_NET_MCS7830
config USB_NET_RNDIS_HOST
tristate "Host for RNDIS and ActiveSync devices"
depends on USB_USBNET
- select USB_NET_CDCETHER
help
This option enables hosting "Remote NDIS" USB networking links,
as encouraged by Microsoft (instead of CDC Ethernet!) for use in
@@ -494,7 +492,6 @@ config USB_KC2190
config USB_NET_ZAURUS
tristate "Sharp Zaurus (stock ROMs) and compatible"
depends on USB_USBNET
- select USB_NET_CDCETHER
select CRC32
default y
help
@@ -597,7 +594,7 @@ config USB_SIERRA_NET
config USB_VL600
tristate "LG VL600 modem dongle"
- depends on USB_NET_CDCETHER && TTY
+ depends on TTY
select USB_ACM
help
Select this if you want to use an LG Electronics 4G/LTE usb modem
@@ -634,7 +631,7 @@ config USB_NET_AQC111
config USB_RTL8153_ECM
tristate
- depends on USB_NET_CDCETHER && (USB_RTL8152 || USB_RTL8152=n)
+ depends on USB_RTL8152 || USB_RTL8152=n
default y
help
This option supports ECM mode for RTL8153 ethernet adapter, when
--
2.54.0
^ permalink raw reply related
* [PATCH net v2] ila: reload IPv6 header after pskb_may_pull in checksum adjust
From: Michael Bommarito @ 2026-07-14 11:49 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Antoine Tenart, Simon Horman, Tom Herbert, netdev, linux-kernel,
stable
ila_csum_adjust_transport() caches ip6h = ipv6_hdr(skb) before calling
pskb_may_pull(). On a non-linear skb whose transport header sits in a page
fragment, pskb_may_pull() can call __pskb_pull_tail() / pskb_expand_head()
and free the old skb head, leaving ip6h dangling; the following
get_csum_diff(ip6h, p) then reads freed memory. ila_update_ipv6_locator()
uses ip6h (and the iaddr derived from it) again after the csum-adjust
call and additionally writes the new locator through that pointer.
Impact: a remote IPv6 packet routed through a configured ILA
csum-adjust-transport route or receive-side mapping triggers a
slab-use-after-free in ila_update_ipv6_locator() (KASAN). The route or
mapping requires CAP_NET_ADMIN to configure, but trigger packets are
unauthenticated once it exists.
Reload ip6h after each pskb_may_pull() in ila_csum_adjust_transport()
before the csum-diff read. In ila_update_ipv6_locator() only the
ILA_CSUM_ADJUST_TRANSPORT case pulls the skb, so reload ip6h and iaddr in
that case alone before the destination-address write; the neutral-map
modes never pull and keep their cached pointers.
Fixes: 33f11d16142b ("ila: Create net/ipv6/ila directory")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
v2: In ila_update_ipv6_locator() reload ip6h/iaddr only in the
ILA_CSUM_ADJUST_TRANSPORT case instead of unconditionally, per
Antoine Tenart's review; the neutral-map modes never pull the skb,
so their cached pointers remain valid.
v1: https://lore.kernel.org/netdev/20260711150648.2915106-1-michael.bommarito@gmail.com/
Evidence: a KUnit case on UML+KASAN drives ila_update_ipv6_locator()
with a non-linear skb whose transport header sits in a fragment, so the
pskb_may_pull() in ila_csum_adjust_transport() reallocates the head.
Stock: BUG: KASAN: slab-use-after-free in ila_update_ipv6_locator, Read of
size 4 (the stale ip6h/iaddr). Patched: both the valid-linear control and
the fragmented case pass, KASAN-clean. Built clean, no new warnings.
net/ipv6/ila/ila_common.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/net/ipv6/ila/ila_common.c b/net/ipv6/ila/ila_common.c
index e71571455c8a0..b78179bfc4c72 100644
--- a/net/ipv6/ila/ila_common.c
+++ b/net/ipv6/ila/ila_common.c
@@ -85,6 +85,7 @@ static void ila_csum_adjust_transport(struct sk_buff *skb,
struct tcphdr *th = (struct tcphdr *)
(skb_network_header(skb) + nhoff);
+ ip6h = ipv6_hdr(skb);
diff = get_csum_diff(ip6h, p);
inet_proto_csum_replace_by_diff(&th->check, skb,
diff, true, true);
@@ -96,6 +97,7 @@ static void ila_csum_adjust_transport(struct sk_buff *skb,
(skb_network_header(skb) + nhoff);
if (uh->check || skb->ip_summed == CHECKSUM_PARTIAL) {
+ ip6h = ipv6_hdr(skb);
diff = get_csum_diff(ip6h, p);
inet_proto_csum_replace_by_diff(&uh->check, skb,
diff, true, true);
@@ -110,6 +112,7 @@ static void ila_csum_adjust_transport(struct sk_buff *skb,
struct icmp6hdr *ih = (struct icmp6hdr *)
(skb_network_header(skb) + nhoff);
+ ip6h = ipv6_hdr(skb);
diff = get_csum_diff(ip6h, p);
inet_proto_csum_replace_by_diff(&ih->icmp6_cksum, skb,
diff, true, true);
@@ -127,6 +130,15 @@ void ila_update_ipv6_locator(struct sk_buff *skb, struct ila_params *p,
switch (p->csum_mode) {
case ILA_CSUM_ADJUST_TRANSPORT:
ila_csum_adjust_transport(skb, p);
+ /*
+ * ila_csum_adjust_transport() calls pskb_may_pull(), which can
+ * reallocate the skb head and leave ip6h (and the iaddr derived
+ * from it) dangling; reload both before the write below. The
+ * other csum modes do not pull, so their cached pointers stay
+ * valid.
+ */
+ ip6h = ipv6_hdr(skb);
+ iaddr = ila_a2i(&ip6h->daddr);
break;
case ILA_CSUM_NEUTRAL_MAP:
if (sir2ila) {
--
2.53.0
^ permalink raw reply related
* Re: [PATCH net v2] tipc: clear sock->sk on the failed-insert path in tipc_sk_create()
From: Breno Leitao @ 2026-07-14 11:55 UTC (permalink / raw)
To: Daehyeon Ko
Cc: netdev, Jon Maloy, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Tung Quang Nguyen, tipc-discussion,
linux-kernel, stable
In-Reply-To: <20260713082342.3803379-1-4ncienth@gmail.com>
On Mon, Jul 13, 2026 at 05:23:42PM +0900, Daehyeon Ko wrote:
> Clear sock->sk on the failed-insert path so the existing tipc_release()
> NULL check fires and the use-after-free is avoided.
The fix itself looks right: clearing sock->sk on the failed-insert path
is what __sock_create() expects from pf->create() on failure, and it
mirrors the same dangling-sk fix done for AF_SMC in commit d293958a8595
("net/smc: do not leave a dangling sk pointer in __smc_create()").
Reviewed-by: Breno Leitao <leitao@debian.org>
> Fixes: 07f6c4bc048a ("tipc: convert tipc reference table to use generic rhashtable")
Is 07f6c4bc048a the commit that actually introduced this? Or the
sk_free() that got added by commit 00aff3590fc0a ("net: tipc: fix
possible refcount leak in tipc_sk_create()") ?
^ permalink raw reply
* Re: [PATCH net-next V6 4/4] devlink: Apply eswitch mode boot defaults
From: Jiri Pirko @ 2026-07-14 12:02 UTC (permalink / raw)
To: Mark Bloch
Cc: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Andrew Lunn,
Jonathan Corbet, Shuah Khan, netdev, linux-rdma, linux-doc
In-Reply-To: <20260714061731.531849-5-mbloch@nvidia.com>
Tue, Jul 14, 2026 at 08:17:30AM +0200, mbloch@nvidia.com wrote:
>Apply parsed devlink_eswitch_mode= defaults after devlink registration
>and after successful reload.
>
>Mark the default mode as pending when a devlink instance is allocated.
>Before devl_unlock() releases the instance lock, apply a pending default
>when the instance is registered.
>
>Clear the pending state before calling into the driver so the boot
>default remains a one-shot operation even if the mode change fails.
>
>For successful reloads that performed DRIVER_REINIT, devlink_reload()
>already holds the devlink instance lock and the driver has completed
>reload_up(). Clear the pending state and apply the default directly from
>the reload path.
>
>Treat an explicit user eswitch mode request as consuming the pending
>default mode.
>
Looks fine to me now. One nit below:
[..]
>+void devlink_default_esw_mode_instance_init(struct devlink *devlink)
Forgotten "instance" here. With this fixed, feel free to add my
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
[..]
^ permalink raw reply
* Re: [ovs-dev] [PATCH net-next v2] selftests/net/openvswitch: add SCTP flow key test
From: Ilya Maximets @ 2026-07-14 12:17 UTC (permalink / raw)
To: 侯敏熙
Cc: i.maximets, dev, linux-kselftest, netdev, linux-kernel, edumazet,
horms, kuba, pabeni, shuah, davem
In-Reply-To: <CAJ0BgHc_3fRLFpaUkb2P2jY4avtB3XJQmk0jE-4UC=1abtKLaQ@mail.gmail.com>
On 7/14/26 2:01 PM, 侯敏熙 wrote:
> Hi Ilya,
>
> Thanks for pointing this out. I looked into this and found that
> nc (netcat-openbsd) on Debian/Ubuntu does not have SCTP support at
> all. The upstream OpenBSD nc only handles TCP, UDP, and
> Unix-domain sockets, and the Debian patches don't add SCTP either.
AFAICT, 'nc' in netdev CI is the nmap version of netcat, it's not
the openbsd one. Does nmap version support sctp? Google seems to
think so at least. The test should be skipped though if sctp is
not supported.
Note: sashiko had some comments on the daemon pid handling in v2.
Best regards, Ilya Maximets.
>
> I see two cleaner alternatives that already have precedent in the
> kernel selftests:
>
> 1. socat: net/netfilter/nft_queue.sh already uses
> "socat -u SCTP-LISTEN:PORT STDOUT" for SCTP data-plane testing.
> Many drivers/net/ selftests already gate on require_cmd("socat").
>
> 2. A small C helper: similar to sctp_hello.c used by sctp_vrf.sh
> and conntrack_sctp_collision.sh. The shell test would call
> "./sctp_test server/client IP PORT" instead of ncat.
>
> Which approach would you prefer for v3?
>
> Best regards,
> Minxi
>
> 侯敏熙 <houminxi@gmail.com> 于2026年7月13日周一 11:02写道：
>
>> Ah so many thanks for your suggestions, Please forgive me,
>> I've always used ncat to write test cases.
>>
>> I will re-spin this patch.
>>
>> Ilya Maximets <i.maximets@ovn.org> 于2026年7月13日周一 10:39写道：
>>
>>> On 7/7/26 5:47 AM, Minxi Hou wrote:
>>>> Register OVS_KEY_ATTR_SCTP in the flow key parser so that sctp()
>>>> can be used in flow specifications. The ovs_key_sctp class already
>>>> exists (with src/dst fields matching the TCP/UDP siblings) but was
>>>> not wired into the parser, so the token was silently dropped and the
>>>> kernel rejected the flow.
>>>>
>>>> Add test_sctp_connect_v4 exercising the SCTP flow key with
>>>> port-specific matching: sctp(dst=4443) for client-to-server and
>>>> sctp(src=4443) for server-to-client.
>>>>
>>>> Wait for ncat readiness with ovs_wait instead of a fixed sleep so
>>>> the test does not race against ncat startup. Use grep -c on the
>>>> listening message to distinguish between successive ncat instances
>>>> that share the same stderr log. Kill the previous server before
>>>> respawning to avoid EADDRINUSE on the SCTP port.
>>>>
>>>> Signed-off-by: Minxi Hou <houminxi@gmail.com>
>>>> ---
>>>> v1 -> v2: replace sleep with ovs_wait on ncat listening output,
>>>> kill previous ncat server before respawning to avoid
>>>> port conflict (Aaron review feedback)
>>>>
>>>> .../selftests/net/openvswitch/openvswitch.sh | 102 ++++++++++++++++++
>>>> .../selftests/net/openvswitch/ovs-dpctl.py | 5 +
>>>> 2 files changed, 107 insertions(+)
>>>>
>>>> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh
>>> b/tools/testing/selftests/net/openvswitch/openvswitch.sh
>>>> index 2954245129a2..9c364eeb2ec2 100755
>>>> --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
>>>> +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
>>>> @@ -32,6 +32,7 @@ tests="
>>>> dec_ttl ttl: dec_ttl decrements
>>> IP TTL
>>>> flow_set flow-set: Flow modify
>>>> action_set set: SET action rewrites
>>> fields
>>>> + sctp_connect_v4 sctp: SCTP flow key
>>> matching
>>>> psample psample: Sampling packets
>>> with psample"
>>>>
>>>> info() {
>>>> @@ -443,6 +444,107 @@ test_action_set() {
>>>> return 0
>>>> }
>>>>
>>>> +# sctp_connect_v4 test
>>>> +# - sctp(dst=4443) matches client-to-server INIT
>>>> +# - sctp(src=4443) matches server-to-client INIT-ACK
>>>> +# - remove flows and verify connection fails, reinstall and recover
>>>> +test_sctp_connect_v4() {
>>>> + local t="test_sctp_connect_v4"
>>>> +
>>>> + which ncat >/dev/null 2>&1 || return $ksft_skip
>>>
>>> Not a full review, but can we avoid ncat? We already use nc in other
>>> tests, using different implementations of the same thing in different
>>> tests doesn't sound like a good idea. On mnay systems nc and ncat
>>> will be different implementations with different options and behavior.
>>>
>>> Best regards, Ilya Maximets.
>>>
>>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
^ permalink raw reply
* [PATCH net] ipv4: require matching source address for route hint reuse
From: Yizhou Zhao @ 2026-07-14 12:26 UTC (permalink / raw)
To: netdev
Cc: Yizhou Zhao, David Ahern, Ido Schimmel, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
linux-kernel, Yuxiang Yang, Ao Wang, Xuewei Feng, Qi Li, Ke Xu,
stable
IPv4 list receive can reuse a route from the previous skb in the same
receive batch. The current eligibility check only compares the destination
address and TOS before calling ip_route_use_hint().
For forwarded routes, ip_route_use_hint() skips fib_validate_source()
unless the hinted route is local. This means a packet with a different
source address can reuse a forwarding dst created for an earlier packet
and avoid source validation such as strict rp_filter.
In a KASAN QEMU router with strict rp_filter on the ingress device, a
bad-only burst was dropped entirely, however, a paired valid/bad burst
with the same destination/TOS made all of the bad packets pass rp_filter.
Require the source address to match before reusing the hint. Packets from
the same source/destination/TOS still take the fast path; packets whose
source changes go through the normal route lookup and source validation
path.
Fixes: 02b24941619f ("ipv4: use dst hint for ipv4 list receive")
Cc: stable@vger.kernel.org
Reported-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
Reported-by: Yuxiang Yang <yangyx22@mails.tsinghua.edu.cn>
Reported-by: Ao Wang <wangao@seu.edu.cn>
Reported-by: Xuewei Feng <fengxw06@126.com>
Reported-by: Qi Li <qli01@tsinghua.edu.cn>
Reported-by: Ke Xu <xuke@tsinghua.edu.cn>
Assisted-by: Claude-Code:GLM-5.2-special
Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
---
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 9860178752b8..970a2c11ec2a 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -316,6 +316,7 @@ static bool ip_can_use_hint(const struct sk_buff *skb, const struct iphdr *iph,
const struct sk_buff *hint)
{
return hint && !skb_dst(skb) && ip_hdr(hint)->daddr == iph->daddr &&
+ ip_hdr(hint)->saddr == iph->saddr &&
ip_hdr(hint)->tos == iph->tos;
}
--
2.47.3
^ permalink raw reply related
* Re: [PATCH net-next v3 05/15] ibmveth: Refactor RX interrupt control for MQ RX queues
From: Simon Horman @ 2026-07-14 12:43 UTC (permalink / raw)
To: Mingming Cao
Cc: netdev, bjking1, haren, ricklind, kuba, edumazet, pabeni,
linuxppc-dev, maddy, mpe, Dave Marquardt
In-Reply-To: <20260706193603.8039-6-mmc@linux.ibm.com>
On Mon, Jul 06, 2026 at 12:35:53PM -0700, Mingming Cao wrote:
> Queue 0 and subordinate RX queues use different interrupt control
> interfaces in PHYP:
>
> - queue 0: h_vio_signal() after h_register_logical_lan()
> - queue N: H_VIOCTL against the queue handle/hwirq mapping
>
> The current code is single-queue oriented and cannot safely scale to
> multiple RX queues in poll completion and open/close IRQ setup.
>
> Introduce queue-indexed interrupt helpers:
>
> ibmveth_enable_irq(adapter, queue_index)
> ibmveth_disable_irq(adapter, queue_index)
> ibmveth_setup_rx_interrupts()
> ibmveth_cleanup_rx_interrupts()
>
> These helpers centralize queue0-vs-subordinate dispatch and make IRQ
> lifecycle symmetric across open/close and future resize paths.
>
> request_irq() is wired with &adapter->napi[i] as dev_id per queue, so
> interrupt ownership follows the NAPI instance that services that RX
> queue.
>
> Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
> Reviewed-by: Dave Marquardt <davemarq@linux.ibm.com>
Although it is added later in this patch set, I believe linux/irqdomain.h
needs to be included in this patch so that irq_dispose_mapping is defined.
Also, I think it would be best to add these helpers in the same patch(es)
that they are first used. As this will avoid temporal compiler warnings
about declared but otherwise unused functions. Similarly for patch 13/15.
Overall, please make sure that when each patch of the series is applied
no new warnings or errors are introduced for allmodconfig W=1 builds.
FWIIW, I exercised this using gcc 16.1 from
https://www.kernel.org/pub/tools/crosstool/
...
--
pw-bot: changes-requested
^ permalink raw reply
* Re: [PATCH net-next v3 07/15] ibmveth: Add RX queue register/deregister helpers for MQ
From: Simon Horman @ 2026-07-14 12:43 UTC (permalink / raw)
To: mmc
Cc: Simon Horman, netdev, bjking1, haren, ricklind, kuba, edumazet,
pabeni, linuxppc-dev, maddy, mpe, davemarq
In-Reply-To: <20260706193603.8039-8-mmc@linux.ibm.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
ibmveth: Add RX queue register/deregister helpers for MQ
This patch introduces new helpers to handle RX queue registration and
deregistration for MQ support. It replaces the single adapter-level
register/free paradigm with per-queue register calls to the hypervisor,
fetching queue handles and IRQs. These new paths are only fully exercised
when multi-queue mode is enabled.
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
> index 63b0184c622a4..7fc11a4e1f61d 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
[ ... ]
> @@ -401,6 +401,28 @@ ibmveth_dispose_subordinate_irq_mappings(struct ibmveth_adapter *adapter)
[ ... ]
> +static int __maybe_unused
> +ibmveth_setup_rx_interrupts(struct ibmveth_adapter *adapter)
> +{
> + struct net_device *netdev = adapter->netdev;
> + int i, rc;
> +
> + for (i = 0; i < adapter->num_rx_queues; i++) {
> + if (!adapter->queue_irq[i]) {
> + netdev_err(netdev, "queue %d has invalid IRQ (0)\n", i);
> + rc = -EINVAL;
> + goto err_free_irqs;
> + }
> +
> + rc = request_irq(adapter->queue_irq[i], ibmveth_interrupt,
> + 0, netdev->name, &adapter->napi[i]);
> + if (rc) {
> + netdev_err(netdev,
> + "request_irq() failed for irq 0x%x queue %d: %d\n",
> + adapter->queue_irq[i], i, rc);
> + goto err_free_irqs;
> + }
> + }
> +
> + for (i = 0; i < adapter->num_rx_queues; i++)
> + napi_enable(&adapter->napi[i]);
[Severity: Medium]
Could this sequence race with packet reception during interface bring-up?
By calling request_irq() before napi_enable(), the hardware interrupt is
unmasked while the NAPI state is still NAPI_STATE_DISABLE. If an interrupt
fires in this window, could ibmveth_interrupt() fail to schedule the NAPI
poll, potentially leading to dropped events or an interrupt storm?
[ ... ]
> @@ -1136,6 +1176,293 @@ static void __maybe_unused ibmveth_free_rx_qstats(struct ibmveth_adapter *adapte
[ ... ]
> +static int
> +ibmveth_register_logical_lan_queue(struct ibmveth_adapter *adapter,
> + union ibmveth_buf_desc rxq_desc,
> + int queue_index)
> +{
[ ... ]
> + if (lpar_rc == H_FUNCTION) {
> + if (adapter->multi_queue) {
> + netdev_info(adapter->netdev,
> + "Multi queue mode not supported by firmware, falling back to single queue\n");
> + adapter->multi_queue = 0;
> + } else {
> + netdev_err(adapter->netdev,
> + "Unexpected H_FUNCTION for queue %d registration (MQ mode already disabled)\n",
> + queue_index);
> + }
> + return lpar_rc;
[Severity: Medium]
Does returning lpar_rc here cause a regression for the single queue fallback?
Since this error propagates up to ibmveth_register_rx_queues(), it seems it
will unconditionally jump to err_unregister and fail the interface open
rather than allowing it to continue with a single queue.
> + }
> +
> + if (try_again) {
> + try_again = 0;
> + goto retry;
[Severity: Low]
Does this retry loop provide any benefit? Unlike the primary queue which
calls h_free_logical_lan() before retrying to clear stale state, this
jumps back to retry the exact same hypercall with the exact same arguments,
which seems like it will immediately fail again.
> + }
[ ... ]
> +static int
> +ibmveth_register_rx_queues(struct ibmveth_adapter *adapter, u64 mac_address)
> +{
[ ... ]
> + for (i = 1; i < adapter->num_rx_queues; i++) {
> + rc = ibmveth_register_single_rx_queue(adapter, i, mac_address);
> + if (rc) {
> + if (!adapter->queue_handle[i] || !adapter->queue_irq[i]) {
> + netdev_err(netdev,
> + "Invalid hypervisor return for queue %d: handle=0x%llx irq=%u\n",
> + i, adapter->queue_handle[i],
> + adapter->queue_irq[i]);
> + }
[Severity: Low]
Is this validation check placed correctly? Because it sits inside the
if (rc) error block, it will log "Invalid hypervisor return" whenever
registration fails normally (e.g., due to resource constraints). It appears
the hypervisor is not expected to populate the handle and irq on failure.
> + goto err_unregister;
> + }
> + }
^ permalink raw reply
* Re: [PATCH net-next v3 08/15] ibmveth: Refactor open/close into MQ-ready resource pipeline
From: Simon Horman @ 2026-07-14 12:47 UTC (permalink / raw)
To: Mingming Cao
Cc: netdev, bjking1, haren, ricklind, kuba, edumazet, pabeni,
linuxppc-dev, maddy, mpe, Dave Marquardt
In-Reply-To: <20260706193603.8039-9-mmc@linux.ibm.com>
On Mon, Jul 06, 2026 at 12:35:56PM -0700, Mingming Cao wrote:
...
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
...
> /**
> * ibmveth_register_logical_lan_queue - Register subordinate queue with hypervisor
> * @adapter: ibmveth adapter structure
> @@ -1466,208 +1479,108 @@ ibmveth_register_rx_queues(struct ibmveth_adapter *adapter, u64 mac_address)
> static int ibmveth_open(struct net_device *netdev)
> {
> struct ibmveth_adapter *adapter = netdev_priv(netdev);
> - u64 mac_address;
> + u64 mac_address = ether_addr_to_u64(netdev->dev_addr);
> int rxq_entries = 1;
> - unsigned long lpar_rc;
> int rc;
> - union ibmveth_buf_desc rxq_desc;
> int i;
> - struct device *dev;
>
> netdev_dbg(netdev, "open starting\n");
>
> - napi_enable(&adapter->napi[0]);
> -
> - for(i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++)
> + for (i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++)
> rxq_entries += adapter->rx_buff_pool[0][i].size;
>
> - rc = -ENOMEM;
> - adapter->buffer_list_addr[0] = (void *)get_zeroed_page(GFP_KERNEL);
> - if (!adapter->buffer_list_addr[0]) {
> - netdev_err(netdev, "unable to allocate list pages\n");
> + rc = ibmveth_alloc_rx_qstats(adapter);
> + if (rc)
> goto out;
> - }
>
> - adapter->filter_list_addr = (void*) get_zeroed_page(GFP_KERNEL);
> - if (!adapter->filter_list_addr) {
> - netdev_err(netdev, "unable to allocate filter pages\n");
> - goto out_free_buffer_list;
> - }
> -
> - dev = &adapter->vdev->dev;
> + rc = ibmveth_alloc_filter_list(adapter);
> + if (rc)
> + goto out_free_rx_qstats;
>
> - adapter->rx_queue[0].queue_len = sizeof(struct ibmveth_rx_q_entry) *
> - rxq_entries;
> - adapter->rx_queue[0].queue_addr =
> - dma_alloc_coherent(dev, adapter->rx_queue[0].queue_len,
> - &adapter->rx_queue[0].queue_dma, GFP_KERNEL);
> - if (!adapter->rx_queue[0].queue_addr)
> + rc = ibmveth_alloc_rx_queues(adapter, rxq_entries);
> + if (rc)
> goto out_free_filter_list;
>
> - adapter->buffer_list_dma[0] = dma_map_single(dev,
> - adapter->buffer_list_addr[0],
> - 4096, DMA_BIDIRECTIONAL);
> - if (dma_mapping_error(dev, adapter->buffer_list_dma[0])) {
> - netdev_err(netdev, "unable to map buffer list pages\n");
> + rc = ibmveth_alloc_buffer_pools(adapter);
> + if (rc)
> goto out_free_queue_mem;
> - }
>
> - adapter->filter_list_dma = dma_map_single(dev,
> - adapter->filter_list_addr, 4096, DMA_BIDIRECTIONAL);
> - if (dma_mapping_error(dev, adapter->filter_list_dma)) {
> - netdev_err(netdev, "unable to map filter list pages\n");
> - goto out_unmap_buffer_list;
> - }
> + rc = ibmveth_register_rx_queues(adapter, mac_address);
> + if (rc)
> + goto out_free_buffer_pools;
>
> - for (i = 0; i < netdev->real_num_tx_queues; i++) {
> - if (ibmveth_allocate_tx_ltb(adapter, i))
> - goto out_free_tx_ltb;
> + rc = netif_set_real_num_rx_queues(netdev, adapter->num_rx_queues);
> + if (rc) {
> + netdev_err(netdev, "failed to set number of rx queues\n");
> + goto out_unregister_queues;
> }
>
> - adapter->rx_queue[0].index = 0;
> - adapter->rx_queue[0].num_slots = rxq_entries;
> - adapter->rx_queue[0].toggle = 1;
> -
> - mac_address = ether_addr_to_u64(netdev->dev_addr);
> -
> - rxq_desc.fields.flags_len = IBMVETH_BUF_VALID |
> - adapter->rx_queue[0].queue_len;
> - rxq_desc.fields.address = adapter->rx_queue[0].queue_dma;
> -
> - netdev_dbg(netdev, "buffer list @ 0x%p\n", adapter->buffer_list_addr[0]);
> - netdev_dbg(netdev, "filter list @ 0x%p\n", adapter->filter_list_addr);
> - netdev_dbg(netdev, "receive q @ 0x%p\n", adapter->rx_queue[0].queue_addr);
> -
> - h_vio_signal(adapter->vdev->unit_address, VIO_IRQ_DISABLE);
> -
> - lpar_rc = ibmveth_register_logical_lan(adapter, rxq_desc, mac_address);
> -
> - if (lpar_rc != H_SUCCESS) {
> - netdev_err(netdev, "h_register_logical_lan failed with %ld\n",
> - lpar_rc);
> - netdev_err(netdev, "buffer TCE:0x%llx filter TCE:0x%llx rxq "
> - "desc:0x%llx MAC:0x%llx\n",
> - adapter->buffer_list_dma[0],
> - adapter->filter_list_dma,
> - rxq_desc.desc,
> - mac_address);
> - rc = -ENONET;
> - goto out_unmap_filter_list;
> - }
> + rc = ibmveth_setup_rx_interrupts(adapter);
> + if (rc)
> + goto out_unregister_queues;
>
> - for (i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++) {
> - if (!adapter->rx_buff_pool[0][i].active)
> - continue;
> - if (ibmveth_alloc_buffer_pool(&adapter->rx_buff_pool[0][i])) {
> - netdev_err(netdev, "unable to alloc pool\n");
> - adapter->rx_buff_pool[0][i].active = 0;
> - rc = -ENOMEM;
> - goto out_free_buffer_pools;
> + if (adapter->num_rx_queues > 1) {
> + for (i = 0; i < adapter->num_rx_queues; i++) {
> + netdev_dbg(netdev, "initial replenish cycle for queue %d\n", i);
> + ibmveth_replenish_task(adapter, i);
ibmveth_replenish_task() only has one parameter
until a later patch in this series.
> }
> + } else {
> + netdev_dbg(netdev, "initial replenish cycle\n");
> + ibmveth_interrupt(adapter->queue_irq[0], &adapter->napi[0]);
> }
>
> - netdev_dbg(netdev, "registering irq 0x%x\n", netdev->irq);
> - rc = request_irq(netdev->irq, ibmveth_interrupt, 0, netdev->name,
> - netdev);
> - if (rc != 0) {
> - netdev_err(netdev, "unable to request irq 0x%x, rc %d\n",
> - netdev->irq, rc);
> - do {
> - lpar_rc = h_free_logical_lan(adapter->vdev->unit_address);
> - } while (H_IS_LONG_BUSY(lpar_rc) || (lpar_rc == H_BUSY));
> -
> - goto out_free_buffer_pools;
> - }
> -
> - rc = -ENOMEM;
> -
> - netdev_dbg(netdev, "initial replenish cycle\n");
> - ibmveth_interrupt(netdev->irq, netdev);
> + rc = ibmveth_alloc_tx_resources(adapter);
> + if (rc)
> + goto out_cleanup_rx_interrupts;
>
> netif_tx_start_all_queues(netdev);
>
> netdev_dbg(netdev, "open complete\n");
> -
> return 0;
>
> +out_cleanup_rx_interrupts:
> + ibmveth_cleanup_rx_interrupts(adapter);
> +out_free_tx_resources:
> + ibmveth_free_tx_resources(adapter);
The out_free_tx_resources label is unused until a later patch of this
series, so it should be added in that patch rather than this one.
And it's not clear to me that ibmveth_free_tx_resources() should
be called when jumping to out_cleanup_rx_interrupts as
in that case ibmveth_alloc_tx_resources() hasn't run successfully.
The AI-generated review on sashiko.dev also highlights the error handling
here:
"Are the unwind labels ordered incorrectly here?
"If ibmveth_setup_rx_interrupts() fails, it jumps to
out_unregister_queues, which is placed after out_free_buffer_pools. Does
this mean we skip freeing the buffer pools and leak memory?
"Also, if ibmveth_alloc_tx_resources() fails, it internally frees
partially allocated LTBs. It then jumps to out_cleanup_rx_interrupts and
falls through to out_free_tx_resources. Because ibmveth_free_tx_ltb()
calls dma_unmap_single() unconditionally without checking or zeroing
tx_ltb_dma, will this cause a double free and an invalid DMA unmap?
"Finally, in the fall-through path from out_cleanup_rx_interrupts,
out_free_buffer_pools is executed before out_unregister_queues (which
calls ibmveth_free_all_queues() to unregister the logical LAN). Does this
free and unmap the RX buffers while the hypervisor's logical LAN is still
active, potentially allowing the hypervisor to DMA incoming packets into
freed memory?
> out_free_buffer_pools:
> - while (--i >= 0) {
> - if (adapter->rx_buff_pool[0][i].active)
> - ibmveth_free_buffer_pool(adapter,
> - &adapter->rx_buff_pool[0][i]);
> - }
> -out_unmap_filter_list:
> - dma_unmap_single(dev, adapter->filter_list_dma, 4096,
> - DMA_BIDIRECTIONAL);
> -
> -out_free_tx_ltb:
> - while (--i >= 0) {
> - ibmveth_free_tx_ltb(adapter, i);
> - }
> -
> -out_unmap_buffer_list:
> - dma_unmap_single(dev, adapter->buffer_list_dma[0], 4096,
> - DMA_BIDIRECTIONAL);
> + ibmveth_free_buffer_pools(adapter);
> +out_unregister_queues:
> + ibmveth_dispose_subordinate_irq_mappings(adapter);
> + ibmveth_free_all_queues(adapter);
> out_free_queue_mem:
> - dma_free_coherent(dev, adapter->rx_queue[0].queue_len,
> - adapter->rx_queue[0].queue_addr,
> - adapter->rx_queue[0].queue_dma);
> + ibmveth_cleanup_rx_resources(adapter);
> out_free_filter_list:
> - free_page((unsigned long)adapter->filter_list_addr);
> -out_free_buffer_list:
> - free_page((unsigned long)adapter->buffer_list_addr[0]);
> + ibmveth_free_filter_list(adapter);
> +out_free_rx_qstats:
> + ibmveth_free_rx_qstats(adapter);
> out:
> - napi_disable(&adapter->napi[0]);
> return rc;
> }
...
^ permalink raw reply
* [PATCH RFC net-next] net: phy: sfp: drop 1000Base-T support for FCLF8521P2BTL
From: Michael Walle @ 2026-07-14 12:49 UTC (permalink / raw)
To: Russell King, Andrew Lunn, Heiner Kallweit, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Michael Walle
The FCLF8521P2BTL is marketed as a drop in replacement for fiber
modules using 1000Base-X autoneg towards the host as default. See the
referenced application note, esp. question #11. Drop the 1000baseT
capability, so 1000Base-X will eventually be used.
This is esp. important if the TX_DISABLE pin is not connected on a
board. Usually, pin is used as a reset line to the PHY on the copper
SFP. If a bootloader expects the default mode and doesn't do any
reconfiguration of the SFP module, a link might not be established.
Link: https://www.coherent.com/resources/application-note/networking/1000base-t-sfp-faq-an-2036.pdf
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
I'm not sure, this is the correct place for the fix, nor if it goes in
the right direction. There is a comment in
/*
* Clause 22 copper SFP modules normally operate in Cisco SGMII mode with
* negotiation enabled, but some may be in 1000base-X - which is for the
* PHY driver to determine.
*/
I haven't found any traces of that though. So any pointers, what flags
have to be modified is appreciated (phydev->supported?
phydev->possible_interfaces?).
I also compared the EEPROMs of both a FCLF-8520-3 and a FCLF-8521-3 (I
don't have a FCLF8520P2BTL) and they are the same except for the P/N and
the serial number. So they both advertise the same capabilites. Although
I'd suspect that the power-on strapping is different and could be read
by the PHY driver.
---
drivers/net/phy/sfp.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index b9d84612ac6f..387753cb5c62 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -478,6 +478,16 @@ static void sfp_quirk_2500basex(const struct sfp_eeprom_id *id,
__set_bit(PHY_INTERFACE_MODE_2500BASEX, caps->interfaces);
}
+static void sfp_quirk_no_sgmii(const struct sfp_eeprom_id *id,
+ struct sfp_module_caps *caps)
+{
+ linkmode_clear_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
+ caps->link_modes);
+ linkmode_clear_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+ caps->link_modes);
+ __clear_bit(PHY_INTERFACE_MODE_SGMII, caps->interfaces);
+}
+
static void sfp_quirk_disable_autoneg(const struct sfp_eeprom_id *id,
struct sfp_module_caps *caps)
{
@@ -539,6 +549,10 @@ static const struct sfp_quirk sfp_quirks[] = {
// PHY.
SFP_QUIRK_F("FS", "SFP-10G-T", sfp_fixup_fs_10gt),
+ // These are sold as 1000base-X compatible. Thus, make sure we don't
+ // use SGMII.
+ SFP_QUIRK_S("FINISAR CORP.", "FCLF8521P2BTL", sfp_quirk_no_sgmii),
+
// Fiberstore SFP-2.5G-T and SFP-10GM-T uses Rollball protocol to talk
// to the PHY and needs 4 sec wait before probing the PHY.
SFP_QUIRK_F("FS", "SFP-2.5G-T", sfp_fixup_rollball_wait4s),
--
2.47.3
^ permalink raw reply related
* Re: [PATCH net-next v3 09/15] ibmveth: Add queue-aware RX buffer submit helper for MQ
From: Simon Horman @ 2026-07-14 12:50 UTC (permalink / raw)
To: mmc
Cc: Simon Horman, netdev, bjking1, haren, ricklind, kuba, edumazet,
pabeni, linuxppc-dev, maddy, mpe, davemarq
In-Reply-To: <20260706193603.8039-10-mmc@linux.ibm.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
ibmveth: Add queue-aware RX buffer submit helper for MQ
This patch updates the ibmveth driver's buffer replenishment path to
be queue-aware for multi-queue receive support. It adds a helper to select
the correct hypervisor call and handles fallback to single-buffer mode if
batch operations are unsupported.
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
> index fa2d4777ffc7b..b3b3886c3eedb 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
> @@ -597,11 +597,73 @@ static inline void ibmveth_flush_buffer(void *addr, unsigned long length)
[ ... ]
> +static long ibmveth_add_logical_lan_buffers(struct ibmveth_adapter *adapter,
> + union ibmveth_buf_desc *descs,
> + int filled,
> + unsigned long buff_size,
> + int queue_index)
> +{
> + struct vio_dev *vdev = adapter->vdev;
> + unsigned long rc;
> +
> + if (adapter->multi_queue) {
> + unsigned long buffersznum = (buff_size << 32) | filled;
> + unsigned long ioba[IBMVETH_MAX_RX_PER_HCALL / 2] = {0};
> + int i;
> +
> + /* Pack descriptor addresses into ioba pairs.
> + * Each ioba holds two 32-bit addresses packed into 64 bits:
> + * - Even descriptors (0,2,4...) go in high 32 bits
> + * - Odd descriptors (1,3,5...) go in low 32 bits
> + */
> + for (i = 0; i < filled && i < IBMVETH_MAX_RX_PER_HCALL; i++) {
> + int pair_idx = i / 2; /* Which pair: 0-5 */
> + int is_high = (i % 2 == 0); /* High or low 32 bits */
> +
> + if (is_high)
> + ioba[pair_idx] = (unsigned long)descs[i].fields.address << 32;
> + else
> + ioba[pair_idx] |= descs[i].fields.address;
> + }
> +
> + rc = h_add_logical_lan_buffers_queue(vdev->unit_address,
> + adapter->queue_handle[queue_index],
> + buffersznum,
> + ioba[0], ioba[1], ioba[2],
> + ioba[3], ioba[4], ioba[5]);
> + adapter->hcall_stats.add_bufs_queue++;
[Severity: Medium]
Will this counter increment lose updates when multiple NAPI poll routines run
concurrently on different CPUs?
Since multi-queue enables concurrent execution, it seems global statistic
counters like hcall_stats.add_bufs_queue and replenish_add_buff_success might
need atomic or per-CPU operations to avoid read-modify-write data races.
> + } else if (filled == 1) {
> + rc = h_add_logical_lan_buffer(vdev->unit_address,
> + descs[0].desc);
> + adapter->hcall_stats.add_buf++;
> + } else {
> + rc = h_add_logical_lan_buffers(vdev->unit_address,
> + descs[0].desc, descs[1].desc,
> + descs[2].desc, descs[3].desc,
> + descs[4].desc, descs[5].desc,
> + descs[6].desc, descs[7].desc);
> + adapter->hcall_stats.add_bufs++;
> + }
> +
> + return rc;
> +}
[ ... ]
> @@ -745,24 +798,19 @@ static void ibmveth_replenish_buffer_pool(struct ibmveth_adapter *adapter,
> }
> adapter->replenish_add_buff_failure += filled;
>
> - /*
> - * If multi rx buffers hcall is no longer supported by FW
> - * e.g. in the case of Live Partition Migration
> - */
> - if (batch > 1 && lpar_rc == H_FUNCTION) {
> - /*
[ ... ]
> + if (lpar_rc == H_FUNCTION) {
> + if (adapter->multi_queue) {
> + netdev_err(adapter->netdev,
> + "Unexpected H_FUNCTION from multi-queue buffer add (queue=%d, batch=%d)\n",
> + queue_index, batch);
> + break;
> + } else if (batch > 1) {
> + netdev_warn(adapter->netdev,
> + "H_FUNCTION from legacy batch buffer add (batch=%d), falling back to single buffer mode\n",
> + batch);
> + adapter->rx_buffers_per_hcall = 1;
> + continue;
[Severity: High]
Does this continue statement cause an infinite loop?
Looking at ibmveth_replenish_buffer_pool(), the local batch variable is set
before the while loop:
batch = adapter->rx_buffers_per_hcall;
while (remaining > 0) {
...
If the fallback updates adapter->rx_buffers_per_hcall to 1 and issues a continue,
the loop will restart but the local batch variable will retain its original
value.
Would this lead to the loop repeatedly attempting the unsupported batch size
and failing with H_FUNCTION forever?
> + }
> }
> break;
> }
> @@ -784,18 +832,24 @@ static void ibmveth_update_rx_no_buffer(struct ibmveth_adapter *adapter)
> }
>
> /* replenish routine */
> -static void ibmveth_replenish_task(struct ibmveth_adapter *adapter)
> +static void ibmveth_replenish_task(struct ibmveth_adapter *adapter,
> + int queue_index)
> {
> int i;
>
> + if (queue_index >= adapter->num_rx_queues)
> + return;
> +
> adapter->replenish_task_cycles++;
[Severity: Medium]
Similar to the hcall_stats counters above, could this increment suffer from data
races when multiple queues are processed concurrently across different CPUs?
^ permalink raw reply
* Re: [PATCH net-next V6 3/4] devlink: Parse eswitch mode boot defaults
From: Jiri Pirko @ 2026-07-14 12:58 UTC (permalink / raw)
To: Mark Bloch
Cc: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Andrew Lunn,
Jonathan Corbet, Shuah Khan, netdev, linux-rdma, linux-doc
In-Reply-To: <20260714061731.531849-4-mbloch@nvidia.com>
Tue, Jul 14, 2026 at 08:17:29AM +0200, mbloch@nvidia.com wrote:
>Add devlink_eswitch_mode= kernel command line parsing for a default
>eswitch mode.
>
>The supported syntax selects either all devlink handles or one explicit
>comma-separated handle list:
>
> devlink_eswitch_mode=*=<mode>
>
> devlink_eswitch_mode=<handle>[,<handle>...]=<mode>
>
>where <mode> is one of legacy, switchdev or switchdev_inactive. All
>selected handles receive the same mode. Assigning different modes to
>different handle lists in the same parameter value is not supported.
>
>Store the parsed selector and mode in devlink core so the default can be
>applied by a downstream patch.
>
>Document the devlink_eswitch_mode= syntax and duplicate handle handling.
>
>Signed-off-by: Mark Bloch <mbloch@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
^ permalink raw reply
* Re: [PATCH net-next v3 10/15] ibmveth: Enable multi-queue RX receive path
From: Simon Horman @ 2026-07-14 12:55 UTC (permalink / raw)
To: mmc
Cc: Simon Horman, netdev, bjking1, haren, ricklind, kuba, edumazet,
pabeni, linuxppc-dev, maddy, mpe, davemarq
In-Reply-To: <20260706193603.8039-11-mmc@linux.ibm.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
ibmveth: Enable multi-queue RX receive path
This is the first patch that sets multi_queue from H_ILLAN_ATTRIBUTES
and switches registration, buffer posting, and receive to the MQ
hcall path. It also raises num_rx_queues and enables per-queue NAPI.
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
> index b3b3886c3eedb..863e5c68b42c7 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
[ ... ]
> @@ -826,15 +841,23 @@ static void ibmveth_replenish_buffer_pool(struct ibmveth_adapter *adapter,
> */
> static void ibmveth_update_rx_no_buffer(struct ibmveth_adapter *adapter)
> {
> - __be64 *p = adapter->buffer_list_addr[0] + 4096 - 8;
> + int i;
> +
> + for (i = 0; i < adapter->num_rx_queues; i++) {
> + __be64 *p = adapter->buffer_list_addr[i] + 4096 - 8;
> + u64 drops = be64_to_cpup(p);
>
> - adapter->rx_no_buffer = be64_to_cpup(p);
> + if (i == 0)
> + adapter->rx_no_buffer = drops;
> + }
> }
[Severity: Medium]
Will this logic silently discard the hardware drop counts for all queues
other than queue 0?
Since ethtool -S reports the global adapter->rx_no_buffer statistic, shouldn't
this loop accumulate the drops across all queues instead of only recording
them when evaluating the first queue?
[ ... ]
> @@ -842,6 +865,8 @@ static void ibmveth_replenish_task(struct ibmveth_adapter *adapter,
> int queue_index)
> {
> + struct ibmveth_rx_q *rxq = &adapter->rx_queue[queue_index];
> + unsigned long flags;
> int i;
>
> if (queue_index >= adapter->num_rx_queues)
> @@ -842,6 +865,8 @@ static void ibmveth_replenish_task(struct ibmveth_adapter *adapter,
>
> adapter->replenish_task_cycles++;
>
> + spin_lock_irqsave(&rxq->replenish_lock, flags);
> +
[Severity: Medium]
With multi-queue RX enabled, can multiple NAPI threads execute this task
concurrently for different queues?
If so, does this non-atomic increment of the global replenish_task_cycles
stat risk data corruption, torn reads on 32-bit platforms, or lost updates?
Similarly, in ibmveth_replenish_buffer_pool():
drivers/net/ethernet/ibm/ibmveth.c:ibmveth_replenish_buffer_pool() {
...
buffers_added += filled;
adapter->replenish_add_buff_success += filled;
remaining -= filled;
...
}
Even though the caller acquires the per-queue rxq->replenish_lock,
replenish_add_buff_success is a global adapter statistic. Does this update
also race with other CPUs holding the locks for their respective queues?
[ ... ]
> @@ -2268,34 +2299,40 @@ static void ibmveth_rx_csum_helper(struct sk_buff *skb,
>
> static int ibmveth_poll(struct napi_struct *napi, int budget)
> {
[ ... ]
> } else {
> struct sk_buff *skb, *new_skb;
> - int length = ibmveth_rxq_frame_length(adapter);
> - int offset = ibmveth_rxq_frame_offset(adapter);
> - int csum_good = ibmveth_rxq_csum_good(adapter);
> - int lrg_pkt = ibmveth_rxq_large_packet(adapter);
> + int length = ibmveth_rxq_frame_length(adapter, queue_index);
> + int offset = ibmveth_rxq_frame_offset(adapter, queue_index);
> + int csum_good = ibmveth_rxq_csum_good(adapter, queue_index);
> + int lrg_pkt = ibmveth_rxq_large_packet(adapter, queue_index);
[Severity: High]
This isn't a bug introduced by this patch, but does this code validate the
length and offset provided by the hardware descriptor before modifying the
SKB bounds?
Later in ibmveth_poll(), these values are used directly:
drivers/net/ethernet/ibm/ibmveth.c:ibmveth_poll() {
...
skb_reserve(skb, offset);
skb_put(skb, length);
...
}
If the hypervisor provides an untrusted or artificially large length, could
the call to skb_put() trigger an skb_over_panic() and crash the guest
kernel?
[ ... ]
Also, I noticed a potential memory leak in the error unwinding path of
ibmveth_open():
drivers/net/ethernet/ibm/ibmveth.c:ibmveth_open() {
...
if (rc) {
netdev_err(netdev, "failed to set number of rx queues\n");
goto out_unregister_queues;
}
...
out_free_buffer_pools:
ibmveth_free_buffer_pools(adapter);
out_unregister_queues:
ibmveth_dispose_subordinate_irq_mappings(adapter);
ibmveth_free_all_queues(adapter);
}
[Severity: Medium]
This is a pre-existing issue, but if netif_set_real_num_rx_queues() fails,
the code jumps to the out_unregister_queues label.
Does this bypass out_free_buffer_pools, leaving the RX buffer pools unfreed
and causing a memory leak when cleaning up after a failure?
^ permalink raw reply
* Re: [PATCH net-next V6 2/4] devlink: Factor out eswitch mode setting
From: Jiri Pirko @ 2026-07-14 12:58 UTC (permalink / raw)
To: Mark Bloch
Cc: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Andrew Lunn,
Jonathan Corbet, Shuah Khan, netdev, linux-rdma, linux-doc
In-Reply-To: <20260714061731.531849-3-mbloch@nvidia.com>
Tue, Jul 14, 2026 at 08:17:28AM +0200, mbloch@nvidia.com wrote:
>Move the common eswitch mode set checks into a small helper and use it
>from the netlink eswitch set command. This makes the same validation
>available to the devlink core path that applies eswitch mode defaults.
>
>Signed-off-by: Mark Bloch <mbloch@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
^ permalink raw reply
* Re: [PATCH net-next v3 11/15] ibmveth: Add per-queue RX and TX statistics collection and reporting
From: Simon Horman @ 2026-07-14 12:59 UTC (permalink / raw)
To: mmc
Cc: Simon Horman, netdev, bjking1, haren, ricklind, kuba, edumazet,
pabeni, linuxppc-dev, maddy, mpe, davemarq
In-Reply-To: <20260706193603.8039-12-mmc@linux.ibm.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
ibmveth: Add per-queue RX and TX statistics collection and reporting
This commit introduces per-queue statistics tracking for both receive and
transmit paths in the ibmveth driver. The counters are aggregated for legacy
interfaces and exposed individually via ethtool stringsets.
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
> index 863e5c68b42c7..4e3f49b6346fd 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
[ ... ]
> @@ -1655,6 +1655,10 @@ static int ibmveth_open(struct net_device *netdev)
> if (rc)
> goto out_cleanup_rx_interrupts;
>
> + rc = ibmveth_alloc_tx_qstats(adapter);
> + if (rc)
> + goto out_free_tx_resources;
> +
> netif_tx_start_all_queues(netdev);
>
> netdev_dbg(netdev, "open complete\n");
[ ... ]
> out_cleanup_rx_interrupts:
> ibmveth_cleanup_rx_interrupts(adapter);
> out_free_tx_resources:
> ibmveth_free_tx_resources(adapter);
[Severity: High]
Does the goto out_free_tx_resources on ibmveth_alloc_tx_qstats() failure
bypass the cleanup for RX interrupts?
Since out_free_tx_resources is physically below out_cleanup_rx_interrupts in
ibmveth_open(), failing here leaves the IRQs requested and NAPI enabled. If
an interrupt subsequently fires, could the handler access the freed RX queue
memory and cause a kernel panic?
[ ... ]
> @@ -1699,6 +1699,7 @@ static int ibmveth_close(struct net_device *netdev)
> }
> }
>
> + ibmveth_free_tx_qstats(adapter);
> ibmveth_free_tx_resources(adapter);
> ibmveth_cleanup_rx_interrupts(adapter);
> ibmveth_update_rx_no_buffer(adapter);
[ ... ]
> ibmveth_free_rx_qstats(adapter);
[Severity: High]
If ibmveth_close() is called to bring down the interface, does this
unconditionally free adapter->tx_qstats and adapter->rx_qstats without a
grace period?
Since ibmveth_get_stats64() can be called concurrently under RCU, and
ibmveth_start_xmit() can run concurrently if the datapath is not completely
synchronized, could this lead to a use-after-free or NULL pointer dereference
when these functions access the freed arrays?
[ ... ]
> @@ -1967,22 +1967,107 @@ static int ibmveth_set_features(struct net_device *dev,
[ ... ]
> +static void ibmveth_aggregate_rx_qstats(struct ibmveth_adapter *adapter)
> +{
> + u64 total_invalid = 0;
> + u64 total_large = 0;
> + int i;
> +
> + if (!adapter->rx_qstats)
> + return;
> +
> + for (i = 0; i < adapter->num_rx_queues; i++) {
> + total_invalid += adapter->rx_qstats[i].invalid_buffers;
> + total_large += adapter->rx_qstats[i].large_packets;
> + }
> +
> + adapter->rx_invalid_buffer = total_invalid;
> + adapter->rx_large_packets = total_large;
> +}
[Severity: High]
Does overwriting adapter->rx_invalid_buffer and other persistent global
counters with the dynamically allocated adapter->rx_qstats values cause a
permanent loss of interface statistics?
When the interface is brought down and back up, ibmveth_close() frees the
queue stats arrays and ibmveth_open() reallocates them as zeroed. Will this
cause all historical statistics to be wiped out on every interface reset,
breaking standard persistence behavior?
[ ... ]
> +static void ibmveth_get_strings(struct net_device *dev, u32 stringset, u8 *data)
> +{
[ ... ]
> + for (i = 0; i < adapter->num_rx_queues; i++) {
> + ethtool_sprintf(&p, "rx%d_packets", i);
> + ethtool_sprintf(&p, "rx%d_bytes", i);
[Severity: Low]
Are these private ethtool stringsets duplicating standard kernel statistics?
According to the subsystem guidelines, per-queue packet and byte statistics
that have a standard UAPI representation (like struct netdev_queue_stats_rx
exported via netlink) should not be duplicated as private ethtool strings.
> diff --git a/drivers/net/ethernet/ibm/ibmveth.h b/drivers/net/ethernet/ibm/ibmveth.h
> index f7b20fd01acb5..390c660af979e 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.h
> +++ b/drivers/net/ethernet/ibm/ibmveth.h
> @@ -316,9 +316,21 @@ struct ibmveth_rx_queue_stats {
> u64 no_buffer_drops;
> };
>
> +struct ibmveth_tx_queue_stats {
> + u64 packets;
> + u64 bytes;
> + u64 large_packets;
> + u64 dropped_packets;
> + u64 send_failures;
> + u64 checksum_offload;
> +};
[Severity: Medium]
Will this structure layout prevent cache line false sharing?
The commit message mentions tracking transmit counters per TX queue to avoid
cache line contention, but struct ibmveth_tx_queue_stats appears to be
exactly 48 bytes and lacks cacheline alignment annotations like
____cacheline_aligned. When allocated as a contiguous array, will multiple
CPUs processing different queues end up writing to the same hardware
cacheline?
^ permalink raw reply
* [PATCH net] dpll: use pin owner's dpll ref for pin-level attribute reporting
From: Ivan Vecera @ 2026-07-14 12:59 UTC (permalink / raw)
To: netdev
Cc: Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Aleksandr Loktionov, Grzegorz Nitka, Jakub Kicinski, open list
Commit c191b319f208 ("dpll: allow registering FW-identified pin with a
different DPLL") relaxed dpll_pin_register() to let fwnode-identified pins
register with DPLLs from a different driver. This allows, for example, the
ICE driver to register a zl3073x-created pin with its TXC DPLL using
ice_dpll_txclk_ops, which lack frequency_get and phase_adjust_get
callbacks.
After such cross-driver registration, the pin's dpll_refs xarray contains
refs from both drivers. dpll_cmd_pin_get_one() calls
dpll_xa_ref_dpll_first() which returns the ref with the lowest DPLL id.
When the foreign DPLL (e.g. ICE TXC) has a lower id than the owner DPLL
(e.g. zl3073x), the foreign ops are used for reporting. Since those ops
lack callbacks like frequency_get, pin-level attributes are silently
omitted from the netlink response.
For example, a zl3073x output pin that should report frequency and
phase-adjust shows neither:
Before:
# dpll pin show id 45
pin id 45:
module-name: zl3073x
clock-id: 3427468959636104019
board-label: 156M25_NAC0_CLKREF_SYNC
package-label: OUT3
type: synce-eth-port
capabilities: 0x0
phase-adjust-min: -2147483648
phase-adjust-max: 2147483647
phase-adjust-gran: 800
parent-device:
...
After:
# dpll pin show id 19
pin id 19:
module-name: zl3073x
clock-id: 15964355450360090479
board-label: 156M25_NAC0_CLKREF_SYNC
package-label: OUT3
type: synce-eth-port
frequency: 156250000 Hz
frequency-supported:
156250000 Hz
capabilities: 0x0
phase-adjust-min: -2147483648
phase-adjust-max: 2147483647
phase-adjust-gran: 800
phase-adjust: 0
parent-device:
...
Fix this by:
1. Adding dpll_pin_own_dpll_ref_first() helper that returns the first ref
whose DPLL matches the pin's (module, clock_id) tuple — i.e. the DPLL
from the driver that created the pin and has the complete set of ops.
Return NULL if no owner ref is found.
2. Using dpll_pin_own_dpll_ref_first() in dpll_cmd_pin_get_one() with a
fallback to dpll_xa_ref_dpll_first() for pin-on-pin child pins whose
dpll_refs all point to a different driver's DPLLs.
3. Using dpll_pin_own_dpll_ref_first() in SET operations
(dpll_pin_freq_set, dpll_pin_esync_set, dpll_pin_ref_sync_state_set,
dpll_pin_phase_adj_set) returning -ENODEV if no owner ref exists.
Replacing the validation loops that rejected the entire operation when
any ref's ops lacked the required callback — instead validate only the
owner refs so that foreign DPLLs with incomplete ops no longer block
SET operations.
4. Guarding all SET and rollback xa_for_each loops against NULL set
callbacks so that foreign refs without the operation are safely skipped
instead of causing a NULL pointer dereference.
Fixes: c191b319f208 ("dpll: allow registering FW-identified pin with a different DPLL")
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
drivers/dpll/dpll_core.c | 27 +++++++++++++++++
drivers/dpll/dpll_core.h | 1 +
drivers/dpll/dpll_netlink.c | 60 ++++++++++++++++++++++++++++++-------
3 files changed, 78 insertions(+), 10 deletions(-)
diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index bb1e8650c9d59..245f625f4b3b4 100644
--- a/drivers/dpll/dpll_core.c
+++ b/drivers/dpll/dpll_core.c
@@ -1146,6 +1146,33 @@ void *dpll_pin_on_pin_priv(struct dpll_pin *parent,
return reg->priv;
}
+/**
+ * dpll_pin_own_dpll_ref_first - find the first owner dpll ref of a pin
+ * @pin: pointer to a dpll pin
+ *
+ * Search pin's dpll_refs for a ref whose dpll matches the pin's
+ * (module, clock_id) tuple, i.e. the dpll registered by the driver
+ * that created the pin. This ensures pin-level attributes are
+ * reported and modified using the owner's ops even when the pin is
+ * also registered with dplls from other drivers.
+ *
+ * Return: pointer to the owner's dpll_pin_ref, or NULL if no
+ * owner ref is found.
+ */
+struct dpll_pin_ref *dpll_pin_own_dpll_ref_first(struct dpll_pin *pin)
+{
+ struct dpll_pin_ref *ref;
+ unsigned long i;
+
+ xa_for_each(&pin->dpll_refs, i, ref) {
+ if (ref->dpll->module == pin->module &&
+ ref->dpll->clock_id == pin->clock_id)
+ return ref;
+ }
+
+ return NULL;
+}
+
const struct dpll_pin_ops *dpll_pin_ops(struct dpll_pin_ref *ref)
{
struct dpll_pin_registration *reg;
diff --git a/drivers/dpll/dpll_core.h b/drivers/dpll/dpll_core.h
index e245771134317..da8a369556ed1 100644
--- a/drivers/dpll/dpll_core.h
+++ b/drivers/dpll/dpll_core.h
@@ -93,6 +93,7 @@ void *dpll_pin_on_pin_priv(struct dpll_pin *parent, struct dpll_pin *pin);
const struct dpll_device_ops *dpll_device_ops(struct dpll_device *dpll);
struct dpll_device *dpll_device_get_by_id(int id);
+struct dpll_pin_ref *dpll_pin_own_dpll_ref_first(struct dpll_pin *pin);
const struct dpll_pin_ops *dpll_pin_ops(struct dpll_pin_ref *ref);
struct dpll_pin_ref *dpll_xa_ref_dpll_first(struct xarray *xa_refs);
extern struct xarray dpll_device_xa;
diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
index bf729cde796a7..b3f0cb7d3d349 100644
--- a/drivers/dpll/dpll_netlink.c
+++ b/drivers/dpll/dpll_netlink.c
@@ -696,7 +696,9 @@ dpll_cmd_pin_get_one(struct sk_buff *msg, struct dpll_pin *pin,
struct dpll_pin_ref *ref;
int ret;
- ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
+ ref = dpll_pin_own_dpll_ref_first(pin);
+ if (!ref)
+ ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
ASSERT_NOT_NULL(ref);
ret = dpll_msg_add_pin_handle(msg, pin);
@@ -1087,12 +1089,19 @@ dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
xa_for_each(&pin->dpll_refs, i, ref) {
ops = dpll_pin_ops(ref);
- if (!ops->frequency_set || !ops->frequency_get) {
- NL_SET_ERR_MSG(extack, "frequency set not supported by the device");
+ if ((!ops->frequency_set || !ops->frequency_get) &&
+ ref->dpll->module == pin->module &&
+ ref->dpll->clock_id == pin->clock_id) {
+ NL_SET_ERR_MSG(extack,
+ "frequency set not supported by the device");
return -EOPNOTSUPP;
}
}
- ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
+ ref = dpll_pin_own_dpll_ref_first(pin);
+ if (!ref) {
+ NL_SET_ERR_MSG(extack, "pin owner dpll not found");
+ return -ENODEV;
+ }
ops = dpll_pin_ops(ref);
dpll = ref->dpll;
ret = ops->frequency_get(pin, dpll_pin_on_dpll_priv(dpll, pin), dpll,
@@ -1106,6 +1115,8 @@ dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
xa_for_each(&pin->dpll_refs, i, ref) {
ops = dpll_pin_ops(ref);
+ if (!ops->frequency_set)
+ continue;
dpll = ref->dpll;
ret = ops->frequency_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
dpll, dpll_priv(dpll), freq, extack);
@@ -1125,6 +1136,8 @@ dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
if (ref == failed)
break;
ops = dpll_pin_ops(ref);
+ if (!ops->frequency_set)
+ continue;
dpll = ref->dpll;
if (ops->frequency_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
dpll, dpll_priv(dpll), old_freq, extack))
@@ -1148,13 +1161,19 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
xa_for_each(&pin->dpll_refs, i, ref) {
ops = dpll_pin_ops(ref);
- if (!ops->esync_set || !ops->esync_get) {
+ if ((!ops->esync_set || !ops->esync_get) &&
+ ref->dpll->module == pin->module &&
+ ref->dpll->clock_id == pin->clock_id) {
NL_SET_ERR_MSG(extack,
"embedded sync feature is not supported by this device");
return -EOPNOTSUPP;
}
}
- ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
+ ref = dpll_pin_own_dpll_ref_first(pin);
+ if (!ref) {
+ NL_SET_ERR_MSG(extack, "pin owner dpll not found");
+ return -ENODEV;
+ }
ops = dpll_pin_ops(ref);
dpll = ref->dpll;
ret = ops->esync_get(pin, dpll_pin_on_dpll_priv(dpll, pin), dpll,
@@ -1178,6 +1197,8 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
void *pin_dpll_priv;
ops = dpll_pin_ops(ref);
+ if (!ops->esync_set)
+ continue;
dpll = ref->dpll;
pin_dpll_priv = dpll_pin_on_dpll_priv(dpll, pin);
ret = ops->esync_set(pin, pin_dpll_priv, dpll, dpll_priv(dpll),
@@ -1201,6 +1222,8 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
if (ref == failed)
break;
ops = dpll_pin_ops(ref);
+ if (!ops->esync_set)
+ continue;
dpll = ref->dpll;
pin_dpll_priv = dpll_pin_on_dpll_priv(dpll, pin);
if (ops->esync_set(pin, pin_dpll_priv, dpll, dpll_priv(dpll),
@@ -1235,8 +1258,11 @@ dpll_pin_ref_sync_state_set(struct dpll_pin *pin,
NL_SET_ERR_MSG(extack, "reference sync pin not available");
return -EINVAL;
}
- ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
- ASSERT_NOT_NULL(ref);
+ ref = dpll_pin_own_dpll_ref_first(pin);
+ if (!ref) {
+ NL_SET_ERR_MSG(extack, "pin owner dpll not found");
+ return -ENODEV;
+ }
ops = dpll_pin_ops(ref);
if (!ops->ref_sync_set || !ops->ref_sync_get) {
NL_SET_ERR_MSG(extack, "reference sync not supported by this pin");
@@ -1255,6 +1281,8 @@ dpll_pin_ref_sync_state_set(struct dpll_pin *pin,
return 0;
xa_for_each(&pin->dpll_refs, i, ref) {
ops = dpll_pin_ops(ref);
+ if (!ops->ref_sync_set)
+ continue;
dpll = ref->dpll;
ret = ops->ref_sync_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
ref_sync_pin,
@@ -1277,6 +1305,8 @@ dpll_pin_ref_sync_state_set(struct dpll_pin *pin,
if (ref == failed)
break;
ops = dpll_pin_ops(ref);
+ if (!ops->ref_sync_set)
+ continue;
dpll = ref->dpll;
if (ops->ref_sync_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
ref_sync_pin,
@@ -1468,12 +1498,18 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
xa_for_each(&pin->dpll_refs, i, ref) {
ops = dpll_pin_ops(ref);
- if (!ops->phase_adjust_set || !ops->phase_adjust_get) {
+ if ((!ops->phase_adjust_set || !ops->phase_adjust_get) &&
+ ref->dpll->module == pin->module &&
+ ref->dpll->clock_id == pin->clock_id) {
NL_SET_ERR_MSG(extack, "phase adjust not supported");
return -EOPNOTSUPP;
}
}
- ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
+ ref = dpll_pin_own_dpll_ref_first(pin);
+ if (!ref) {
+ NL_SET_ERR_MSG(extack, "pin owner dpll not found");
+ return -ENODEV;
+ }
ops = dpll_pin_ops(ref);
dpll = ref->dpll;
ret = ops->phase_adjust_get(pin, dpll_pin_on_dpll_priv(dpll, pin),
@@ -1488,6 +1524,8 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
xa_for_each(&pin->dpll_refs, i, ref) {
ops = dpll_pin_ops(ref);
+ if (!ops->phase_adjust_set)
+ continue;
dpll = ref->dpll;
ret = ops->phase_adjust_set(pin,
dpll_pin_on_dpll_priv(dpll, pin),
@@ -1510,6 +1548,8 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
if (ref == failed)
break;
ops = dpll_pin_ops(ref);
+ if (!ops->phase_adjust_set)
+ continue;
dpll = ref->dpll;
if (ops->phase_adjust_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
dpll, dpll_priv(dpll), old_phase_adj,
--
2.53.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;
as well as URLs for NNTP newsgroup(s).