On Tue, Mar 24, 2026 at 09:20:56AM -0700, Erni Sri Satya Vennela wrote:
> On Mon, Mar 23, 2026 at 05:44:44PM -0700, Jakub Kicinski wrote:
> > On Thu, 19 Mar 2026 00:09:13 -0700 Erni Sri Satya Vennela wrote:
> > > Add debugfs entries to expose hardware configuration and diagnostic
> > > information that aids in debugging driver initialization and runtime
> > > operations without adding noise to dmesg.
> > >
> > > The debugfs directory creation and removal for each PCI device is
> > > integrated into mana_gd_setup() and mana_gd_cleanup_device()
> > > respectively, so that all callers (probe, remove, suspend, resume,
> > > shutdown) share a single code path.
> > >
> > > Device-level entries (under /sys/kernel/debug/mana/<slot>/):
> > > - num_msix_usable, max_num_queues: Max resources from hardware
> > > - gdma_protocol_ver, pf_cap_flags1: VF version negotiation results
> > > - num_vports, bm_hostmode: Device configuration
> > >
> > > Per-vPort entries (under /sys/kernel/debug/mana/<slot>/vportN/):
> > > - port_handle: Hardware vPort handle
> > > - max_sq, max_rq: Max queues from vPort config
> > > - indir_table_sz: Indirection table size
> > > - steer_rx, steer_rss, steer_update_tab, steer_cqe_coalescing:
> > > Last applied steering configuration parameters
> >
> > AI says:
> >
> > > @@ -1918,15 +1930,23 @@ static int mana_gd_setup(struct pci_dev *pdev)
> > > struct gdma_context *gc = pci_get_drvdata(pdev);
> > > int err;
> > >
> > > + if (gc->is_pf)
> > > + gc->mana_pci_debugfs = debugfs_create_dir("0",
> > > mana_debugfs_root);
> > > + else
> > > + gc->mana_pci_debugfs =
> > > debugfs_create_dir(pci_slot_name(pdev->slot),
> > > + mana_debugfs_root);
> >
> > If pdev->slot is NULL (which can happen for VFs in environments like generic
> > VFIO passthrough or nested KVM), will pci_slot_name(pdev->slot) cause a
> > NULL pointer dereference?
> >
> > Also, could this naming scheme cause name collisions? If multiple PFs are
> > present, they would all try to use "0". Similarly, VFs across different
> > PCI domains or buses might share the same physical slot identifier, leading
> > to -EEXIST errors. Would it be safer to use the unique PCI BDF via
> > pci_name(pdev) instead?
> Yes. that is a better way to handle it. I will use pci_name. We can
> remove if-else and use only one for both the cases.
> >
> > > @@ -3141,6 +3149,24 @@ static int mana_init_port(struct net_device *ndev)
> > > eth_hw_addr_set(ndev, apc->mac_addr);
> > > sprintf(vport, "vport%d", port_idx);
> > > apc->mana_port_debugfs = debugfs_create_dir(vport,
> > > gc->mana_pci_debugfs);
> > > +
> > > + debugfs_create_u64("port_handle", 0400, apc->mana_port_debugfs,
> > > + &apc->port_handle);
> >
> > When operations like changing the MTU or setting an XDP program trigger a
> > detach/attach cycle, mana_detach() invokes mana_cleanup_port_context(),
> > which recursively removes the apc->mana_port_debugfs directory.
> > During re-attachment, mana_attach() calls mana_init_port(), which
> > recreates the directory and the new files added in this patch. However, the
> > pre-existing current_speed file (created in mana_probe_port()) is not
> > recreated here.
> >
> > Does this cause the current_speed file to be permanently lost after a
> > detach/attach cycle? Should the creation of current_speed be moved to
> > mana_init_port() so it survives the cycle?
> Yes that is correct.
>
> Since these issues are pre-existing and not introduced from my patch.
> I'll plan to send them as different patch with fixes tag.
> > --
> > pw-bot: cr
Hi Jakub,
Just a quick follow‑up on this. Since these issues were pre‑existing and
not introduced by this patch, would you prefer that I send them as a
separate fix patch, or fold the fixes into the current patch?
Thanks,
Vennela
