| Message ID | 4D5D30CA020000780003263A@vpn.id2.novell.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
From: "Jan Beulich" <JBeulich@novell.com> Date: Thu, 17 Feb 2011 13:29:30 +0000 > The CHELSIO_T{3,4}_DEPENDS options are really awkward, and can be > easily dropped if the reverse dependencies of SCSI_CXGB{3,4}_ISCSI on > the former get converted to normal (forward) ones referring to > CHELSIO_T{3,4}. > > Signed-off-by: Jan Beulich <jbeulich@novell.com> I think the goal of these strange rules is not to be complicated on purpose, but rather to cause the iSCSI drivers to appear without the user having to know that he needs to enable the networking driver in order for that to happen. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> On 22.02.11 at 19:14, David Miller <davem@davemloft.net> wrote: > From: "Jan Beulich" <JBeulich@novell.com> > Date: Thu, 17 Feb 2011 13:29:30 +0000 > >> The CHELSIO_T{3,4}_DEPENDS options are really awkward, and can be >> easily dropped if the reverse dependencies of SCSI_CXGB{3,4}_ISCSI on >> the former get converted to normal (forward) ones referring to >> CHELSIO_T{3,4}. >> >> Signed-off-by: Jan Beulich <jbeulich@novell.com> > > I think the goal of these strange rules is not to be complicated > on purpose, but rather to cause the iSCSI drivers to appear without > the user having to know that he needs to enable the networking > driver in order for that to happen. While I realize that this might have been the reason, it's completely contrary to how everyone else writes dependencies, and hence I think these should be removed. Jan -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: "Jan Beulich" <JBeulich@novell.com> Date: Wed, 23 Feb 2011 09:46:10 +0000 >>>> On 22.02.11 at 19:14, David Miller <davem@davemloft.net> wrote: >> From: "Jan Beulich" <JBeulich@novell.com> >> Date: Thu, 17 Feb 2011 13:29:30 +0000 >> >>> The CHELSIO_T{3,4}_DEPENDS options are really awkward, and can be >>> easily dropped if the reverse dependencies of SCSI_CXGB{3,4}_ISCSI on >>> the former get converted to normal (forward) ones referring to >>> CHELSIO_T{3,4}. >>> >>> Signed-off-by: Jan Beulich <jbeulich@novell.com> >> >> I think the goal of these strange rules is not to be complicated >> on purpose, but rather to cause the iSCSI drivers to appear without >> the user having to know that he needs to enable the networking >> driver in order for that to happen. > > While I realize that this might have been the reason, it's completely > contrary to how everyone else writes dependencies, and hence I > think these should be removed. If you knew you were changing the behavior of the config option in this way, you sure didn't think it was worth mentioning in your commit message. I definitely would never expect to have to enable a scsi option to get some network driver visible to enable in the config, and therefore I could see the opposite being insanely frustrating too. You can't ignore these issues and just say "that's not the normal way so I'm going to change it anyways." -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> On 23.02.11 at 21:27, David Miller <davem@davemloft.net> wrote: > From: "Jan Beulich" <JBeulich@novell.com> > Date: Wed, 23 Feb 2011 09:46:10 +0000 > >>>>> On 22.02.11 at 19:14, David Miller <davem@davemloft.net> wrote: >>> From: "Jan Beulich" <JBeulich@novell.com> >>> Date: Thu, 17 Feb 2011 13:29:30 +0000 >>> >>>> The CHELSIO_T{3,4}_DEPENDS options are really awkward, and can be >>>> easily dropped if the reverse dependencies of SCSI_CXGB{3,4}_ISCSI on >>>> the former get converted to normal (forward) ones referring to >>>> CHELSIO_T{3,4}. >>>> >>>> Signed-off-by: Jan Beulich <jbeulich@novell.com> >>> >>> I think the goal of these strange rules is not to be complicated >>> on purpose, but rather to cause the iSCSI drivers to appear without >>> the user having to know that he needs to enable the networking >>> driver in order for that to happen. >> >> While I realize that this might have been the reason, it's completely >> contrary to how everyone else writes dependencies, and hence I >> think these should be removed. > > If you knew you were changing the behavior of the config option in > this way, you sure didn't think it was worth mentioning in your commit > message. I stated in the comment what I think this is - awkward. > I definitely would never expect to have to enable a scsi option to get > some network driver visible to enable in the config, and therefore I > could see the opposite being insanely frustrating too. The resulting dependency seems quite logical to me: Some higher level networking functionality (iSCSI) depends on some lower level networking functionality (an actual driver). > You can't ignore these issues and just say "that's not the normal way > so I'm going to change it anyways." Admittedly I considered only my personal perspective. Now, to get the whole discussion productive again - where do we go from here? I don't think these drivers are so special that they really need to behave backwards to how (almost?) everything else is done... If changing it the way I did in the first try isn't deemed acceptable, would it be at least acceptable to remove those helper options (or, not as welcome from my perspective not the least because of the odd dependency on INET instead of NET, fold them into a single more generic one that others could also benefit from)? As to that INET vs NET dependency - is it possible that the network drivers really just need NET, but the iSCSI ones need INET? In which case the only common dependency would be PCI - certainly not worth a custom helper option. Jan -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jan Beulich wrote: > As to that INET vs NET dependency - is it possible that the > network drivers really just need NET, but the iSCSI ones need > INET? In which case the only common dependency would be > PCI - certainly not worth a custom helper option. I see about a dozen network drivers that depend on INET. These may be the result of cut&paste from other drivers' Kconfig entries rather than actual dependencies. Also some of these drivers select or selected in the past INET_LRO and that may have something to do with their INET dependency, not sure. Reading the commit message that introduced CHELSIO_T3_DEPENDS, it talks of hidden dependencies that select does not see. I am not sure which exactly but since it's been a few years since that commit I'll try to see what the situation is today without the *_DEPENDS symbols and let you know. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- 2.6.38-rc5/drivers/net/Kconfig +++ 2.6.38-rc5-kconfig-chelsio/drivers/net/Kconfig @@ -2594,14 +2594,9 @@ config CHELSIO_T1_1G Enables support for Chelsio's gigabit Ethernet PCI cards. If you are using only 10G cards say 'N' here. -config CHELSIO_T3_DEPENDS - tristate - depends on PCI && INET - default y - config CHELSIO_T3 tristate "Chelsio Communications T3 10Gb Ethernet support" - depends on CHELSIO_T3_DEPENDS + depends on PCI && INET select FW_LOADER select MDIO help @@ -2619,14 +2614,9 @@ config CHELSIO_T3 To compile this driver as a module, choose M here: the module will be called cxgb3. -config CHELSIO_T4_DEPENDS - tristate - depends on PCI && INET - default y - config CHELSIO_T4 tristate "Chelsio Communications T4 Ethernet support" - depends on CHELSIO_T4_DEPENDS + depends on PCI && INET select FW_LOADER select MDIO help --- 2.6.38-rc5/drivers/scsi/cxgbi/cxgb3i/Kconfig +++ 2.6.38-rc5-kconfig-chelsio/drivers/scsi/cxgbi/cxgb3i/Kconfig @@ -1,7 +1,6 @@ config SCSI_CXGB3_ISCSI tristate "Chelsio T3 iSCSI support" - depends on CHELSIO_T3_DEPENDS - select CHELSIO_T3 + depends on CHELSIO_T3 select SCSI_ISCSI_ATTRS ---help--- This driver supports iSCSI offload for the Chelsio T3 devices. --- 2.6.38-rc5/drivers/scsi/cxgbi/cxgb4i/Kconfig +++ 2.6.38-rc5-kconfig-chelsio/drivers/scsi/cxgbi/cxgb4i/Kconfig @@ -1,7 +1,6 @@ config SCSI_CXGB4_ISCSI tristate "Chelsio T4 iSCSI support" - depends on CHELSIO_T4_DEPENDS - select CHELSIO_T4 + depends on CHELSIO_T4 select SCSI_ISCSI_ATTRS ---help--- This driver supports iSCSI offload for the Chelsio T4 devices.
The CHELSIO_T{3,4}_DEPENDS options are really awkward, and can be easily dropped if the reverse dependencies of SCSI_CXGB{3,4}_ISCSI on the former get converted to normal (forward) ones referring to CHELSIO_T{3,4}. Signed-off-by: Jan Beulich <jbeulich@novell.com> --- drivers/net/Kconfig | 14 ++------------ drivers/scsi/cxgbi/cxgb3i/Kconfig | 3 +-- drivers/scsi/cxgbi/cxgb4i/Kconfig | 3 +-- 3 files changed, 4 insertions(+), 16 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html