netdev - Re: [PATCH net-next 3/4] igb: enable internal PPS for the i210.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 20 Nov 2014 10:18:04 +0100
From:	Richard Cochran <richardcochran@...il.com>
To:	"Keller, Jacob E" <jacob.e.keller@...el.com>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"Allan, Bruce W" <bruce.w.allan@...el.com>,
	"Ronciak, John" <john.ronciak@...el.com>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"Vick, Matthew" <matthew.vick@...el.com>
Subject: Re: [PATCH net-next 3/4] igb: enable internal PPS for the i210.

On Wed, Nov 19, 2014 at 09:06:19PM +0000, Keller, Jacob E wrote:
> On Wed, 2014-11-19 at 21:26 +0100, Richard Cochran wrote:
> > On Wed, Nov 19, 2014 at 07:32:33PM +0000, Keller, Jacob E wrote:
> > > Good catch :)

I have not been able to reproduce the crash, and so the cause is not
what I thought it was. Maybe it was my patch that preserved the
enabled interrupts in igb_ptp_reset(). I didn't notice that the driver
frees and reallocates the ptp_clock. I would never do that, myself.

> I think you need something here, but it should be clearing that register
> after a MAC reset, so it needs to be re-initialized. I'm not sure if
> that reset path was used in the same place in the past.

Okay, lets figure this out. Why is there a PTP reset function at all?
I don't know, lets see who calls it...

  Finding functions calling: igb_ptp_reset
  ----------------------------------------
  *** drivers/net/ethernet/intel/igb/igb_main.c:
  igb_reset[2033]                igb_ptp_reset(adapter);

Easy enough to understand. But who is calling igb_reset?

  Finding functions calling: igb_reset
  ------------------------------------
  *** drivers/net/ethernet/intel/igb/igb_ethtool.c:
  igb_set_settings[345]          igb_reset(adapter);
  igb_set_pauseparam[409]        igb_reset(adapter);
  igb_diag_test[2016]            igb_reset(adapter);
  igb_set_eee[2729]              igb_reset(adapter);
  *** drivers/net/ethernet/intel/igb/igb_main.c:
  igb_down[1814]                 igb_reset(adapter);
  igb_set_features[2069]         igb_reset(adapter);
  igb_probe[2526]                igb_reset(adapter);
  __igb_open[3110]               igb_reset(adapter);
  igb_watchdog_task[4231]        igb_reset(adapter);
  igb_change_mtu[5189]           igb_reset(adapter);
  igb_resume[7460]               igb_reset(adapter);
  igb_sriov_reinit[7545]         igb_reset(adapter);
  igb_io_slot_reset[7678]        igb_reset(adapter);

Wow, that is quite much. So, whenever any random parameter is changed,
we reset the PTP clock. Great.

Really, wouldn't better to reset the clock functions only when
absolutely necessary?

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html