This is achieved by propagating the psp_dev from the lower device
to the upper devices in the device stack via a netdevice notifier.
The lowest device owns the psp_dev pointer while the upper devices
just borrow the pointer. When the lower device is unlinked, the
borrowed pointer is cleared in the upper device.
Assumption being that psp_dev is set on the lowest device before
any upper devices stack are linked on that lowest device at a
later point of time.
In case of bond devices:
- If an upper device has multiple lower devices, the psp_dev is
not propogated as it would be ambiguous which slave will be used
for transmission.
- If a bond device transitions from having one slave device to
multiple slaves, any previously borrowed psp_dev pointer is
cleared to avoid ambiguity.
Signed-off-by: Kiran Kella <kiran.kella@broadcom.com>
Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>
---
net/psp/psp_main.c | 114 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 114 insertions(+)
diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
index a8534124f626..0bff50c9314d 100644
--- a/net/psp/psp_main.c
+++ b/net/psp/psp_main.c
@@ -313,10 +313,124 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
}
EXPORT_SYMBOL(psp_dev_rcv);
+/**
+ * psp_netdevice_event() - Handle netdevice events for PSP device propagation
+ * @nb: notifier block
+ * @event: netdevice event
+ * @ptr: netdevice notifier info
+ *
+ * Propagates psp_dev pointer from lower devices to upper devices when
+ * upper devices are created (e.g., VLAN subinterfaces).
+ */
+static int psp_netdevice_event(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct netdev_notifier_changeupper_info *info;
+ struct net_device *dev, *upper_dev;
+ struct psp_dev *psd;
+
+ if (event != NETDEV_CHANGEUPPER)
+ return NOTIFY_DONE;
+
+ info = ptr;
+ dev = netdev_notifier_info_to_dev(ptr);
+ upper_dev = info->upper_dev;
+
+ if (info->linking) {
+ bool has_multiple_lowers = false;
+ struct list_head *iter;
+
+ /* Lower device is being linked to an upper device.
+ * Propagate psp_dev from the immediate lower device to the
+ * upper device. The immediate lower device would have already
+ * got the psp_dev pointer set in a previous notification (or
+ * owns it if it's the lowest device),
+ * Upper devices just borrow the pointer.
+ *
+ * However, if the upper device has multiple lower devices,
+ * don't propagate psp_dev as it would be ambiguous
+ * which slave will be used for transmission (eg., bond device).
+ * If the upper device already has psp_dev set and is getting
+ * a second lower device, clear the psp_dev.
+ */
+
+ rcu_read_lock();
+ iter = &upper_dev->adj_list.lower;
+ /* Check if there's a second lower device */
+ if (netdev_next_lower_dev_rcu(upper_dev, &iter))
+ if (netdev_next_lower_dev_rcu(upper_dev, &iter))
+ has_multiple_lowers = true;
+
+ /* If upper device now has multiple lower devices, clear psp_dev
+ * if it was previously set (from when it had only one slave).
+ */
+ if (has_multiple_lowers) {
+ psd = rcu_dereference(upper_dev->psp_dev);
+ rcu_read_unlock();
+ if (psd)
+ rcu_assign_pointer(upper_dev->psp_dev, NULL);
+
+ return NOTIFY_DONE;
+ }
+
+ /* Get psp_dev from the immediate lower device */
+ psd = rcu_dereference(dev->psp_dev);
+ rcu_read_unlock();
+
+ /* Propagate psp_dev to upper device if found */
+ if (psd)
+ rcu_assign_pointer(upper_dev->psp_dev, psd);
+ } else {
+ struct net_device *remaining_lower = NULL;
+ bool has_single_lower = false;
+ struct list_head *iter;
+
+ /* Lower device is being unlinked from an upper device.
+ * After unlinking, check if the upper device now has exactly
+ * one lower device remaining. If so, propagate psp_dev from
+ * that remaining lower device to the upper device.
+ */
+
+ rcu_read_lock();
+ iter = &upper_dev->adj_list.lower;
+ remaining_lower = netdev_next_lower_dev_rcu(upper_dev, &iter);
+ /* Check if there's a second lower device */
+ if (remaining_lower)
+ if (!netdev_next_lower_dev_rcu(upper_dev, &iter))
+ has_single_lower = true;
+
+ if (has_single_lower) {
+ /* Upper device now has exactly one lower device.
+ * Propagate psp_dev from the remaining lower device.
+ */
+ psd = rcu_dereference(remaining_lower->psp_dev);
+ rcu_read_unlock();
+ if (psd)
+ rcu_assign_pointer(upper_dev->psp_dev, psd);
+ } else {
+ /* Upper device has zero or multiple lower devices.
+ * Clear psp_dev if it was previously set.
+ */
+ psd = rcu_dereference(upper_dev->psp_dev);
+ rcu_read_unlock();
+ if (psd)
+ rcu_assign_pointer(upper_dev->psp_dev, NULL);
+ }
+ }
+
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block psp_netdevice_notifier = {
+ .notifier_call = psp_netdevice_event,
+};
+
static int __init psp_init(void)
{
mutex_init(&psp_devs_lock);
+ register_netdevice_notifier(&psp_netdevice_notifier);
+
return genl_register_family(&psp_nl_family);
}
--
2.45.4
Haven't had the time to think about this TBH but FWIW:
This is an AI-generated review of your patch.
For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
psp: Support for transmit on logical device when the underlying transport
device supports PSP.
This patch propagates psp_dev pointers from lower devices to upper devices
(e.g., VLANs) via a netdevice notifier. Upper devices borrow the pointer
without taking a reference, and the borrowed pointer is cleared when the
link changes.
> diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
> index a8534124f626..0bff50c9314d 100644
> --- a/net/psp/psp_main.c
> +++ b/net/psp/psp_main.c
> @@ -313,10 +313,124 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
> }
> EXPORT_SYMBOL(psp_dev_rcv);
>
> +/**
> + * psp_netdevice_event() - Handle netdevice events for PSP device propagation
[ ... ]
> + * Propagates psp_dev pointer from lower devices to upper devices when
> + * upper devices are created (e.g., VLAN subinterfaces).
> + */
> +static int psp_netdevice_event(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
[ ... ]
> + /* Get psp_dev from the immediate lower device */
> + psd = rcu_dereference(dev->psp_dev);
> + rcu_read_unlock();
> +
> + /* Propagate psp_dev to upper device if found */
> + if (psd)
> + rcu_assign_pointer(upper_dev->psp_dev, psd);
The commit message states that borrowed pointers are cleared when the lower
device is "unlinked". However, what happens if psp_dev_unregister() is
called on the lower device without first unlinking the upper devices?
Looking at nsim_psp_rereg_write() in drivers/net/netdevsim/psp.c, it calls
nsim_psp_uninit()->psp_dev_unregister() without unregistering the netdevice.
If there are upper devices (e.g., VLANs) that have borrowed the psp_dev
pointer, would they be left with a dangling pointer after the RCU grace
period completes?
Similarly, nsim_destroy() calls nsim_psp_uninit() before calling
unregister_netdevice(), so the upper devices would still have borrowed
pointers when psp_dev_unregister() frees the psp_dev structure.
Could this lead to use-after-free if an upper device's psp_dev pointer is
accessed after the underlying psp_dev has been freed?
Would it make sense to either:
(a) Document that netdevice must be unregistered before psp_dev_unregister()
is called, or
(b) Walk the upper device stack in psp_dev_unregister() to clear borrowed
pointers, or
(c) Update existing callers (netdevsim) to ensure proper ordering?
> + } else {
[ ... ]
> + return NOTIFY_DONE;
> +}
--
pw-bot: cr
© 2016 - 2026 Red Hat, Inc.