netdev - Re: [net-next 10/13] igb: Tidy up wrapping for CONFIG_IGB

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]

Message-ID: <1345827088.2694.11.camel@bwh-desktop.uk.solarflarecom.com>
Date:	Fri, 24 Aug 2012 17:51:28 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Richard Cochran <richardcochran@...il.com>
CC:	"Vick, Matthew" <matthew.vick@...el.com>,
	"Keller, Jacob E" <jacob.e.keller@...el.com>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"gospo@...hat.com" <gospo@...hat.com>,
	"sassmann@...hat.com" <sassmann@...hat.com>
Subject: Re: [net-next 10/13] igb: Tidy up wrapping for CONFIG_IGB_PTP.

On Fri, 2012-08-24 at 12:10 +0200, Richard Cochran wrote:
> On Thu, Aug 23, 2012 at 06:40:25PM +0000, Vick, Matthew wrote:
> > 
> > I tend to agree with Jake here--I like having the information. I'm fine removing them, but I'd like to do it for all CONFIG_IGB_PTP wrapping if we're going to do it. What do you think, Richard?
> 
> Come to think of it, I never liked the CONFIG_IGB_PTP very much in the
> first place. These were added after the fact by Jeff Kirsher. He had
> said off list that there was some issue with CONFIG_PTP_1588_CLOCK and
> igb as a module, or something like that. At that time I said, just go
> ahead and fix it up.

If there is some feature of a driver that depends on an optional
modularisable subsystem, and you want the driver to be built without
that feature when the subsystem is missing, then the feature has to be
disabled when the driver is built-in and the subsystem is a module.  So
in general you need:

config DRIVER_FEATURE
	bool "This is a really neat feature for the driver"
	depends on DRIVER && SUBSYSTEM && !(DRIVER=y && SUBYSTEM=m)

> I think it would be better if the "time stamp all Rx packets" of the
> 82580 were always available, and that the PHC feature always be
> compiled when CONFIG_PTP_1588_CLOCK is selected.
> 
> Maybe you could ask Jeff what the issue was, and then see if there is
> a way to remove CONFIG_IGB_PTP altogether.

Even if they are to be enabled automatically, CONFIG_IGB_PTP and similar
config symbols are important as shorthand.  So I think what you want is:

config IGB_PTP
	def_bool y
	depends on IGB && PTP_1588_CLOCK && !(IGB=y && PTP_1588_CLOCK=m)

(But currently it's actually *selecting* PTP_1588_CLOCK.  We should
probably have drivers consistently select or depend on it, not a
mixture.)

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
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