Corinna Vinschen <vinsc...@redhat.com> writes: > On Apr 24 17:06, Vinicius Costa Gomes wrote: >> Andrew Lunn <and...@lunn.ch> writes: >> >> > On Tue, Apr 23, 2024 at 12:24:54PM +0200, Corinna Vinschen wrote: >> >> During successful probe, igc logs this: >> >> >> >> [ 5.133667] igc 0000:01:00.0 (unnamed net_device) (uninitialized): PHC >> >> added >> >> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >> >> The reason is that igc_ptp_init() is called very early, even before >> >> register_netdev() has been called. So the netdev_info() call works >> >> on a partially uninitialized netdev. >> >> >> >> Fix this by calling igc_ptp_init() after register_netdev(), right >> >> after the media autosense check, just as in igb. Add a comment, >> >> just as in igb. >> > >> > The network stack can start sending and receiving packet before >> > register_netdev() returns. This is typical of NFS root for example. Is >> > there anything in igc_ptp_init() which could cause such packet >> > transfers to explode? >> > >> >> There might be a very narrow window (probably impossible?), what I can >> see is: >> >> 1. the netdevice is exposed to userspace; >> 2. userspace does the SIOCSHWTSTAMP ioctl() to enable TX timestamps; >> 3. userspace sends a packet that is going to be timestamped; >> >> if this happens before igc_ptp_init() is called, adapter->ptp_tx_lock is >> going to be uninitialized, and (3) is going to crash. > > The same would then be possible on igb as well, wouldn't it? >
Given how many years igb is being used, perhaps "possible" is too strong :-) On igb what exists is slightly different, as there's no ptp_tx_lock there, the "problem" there is trying to enqueue a job on a workqueue that is going to be uninitialized, during this time window. And to be sure, I am still uncertain that this is possible. > >> If there's anything that makes this impossible/extremely unlikely, the >> patch looks good: >> >> Acked-by: Vinicius Costa Gomes <vinicius.go...@intel.com> >> >> >> Cheers, >> -- >> Vinicius > > > Corinna > Cheers, -- Vinicius
