Re: [PATCH 0/3] ixgbe: request_firmware for configuration parameters
On Fri, Aug 16, 2013 at 05:18:12PM -0700, Ali Ayoub wrote: > On 8/16/2013 3:39 PM, Greg KH wrote: > > On Fri, Aug 16, 2013 at 03:14:31PM -0700, Ali Ayoub wrote: > >> On 1/11/2013 11:41 AM, Greg KH wrote: > > > > Seriously? Restarting a thread from over 6 months ago? > > Yes, it's an old thread, but still relevant for current code. Which specific code are you referring to? > > Why? > > Because currently there is no good alternative for module params for > device drivers that need to have low level configs in init time. I disagree, see below for why module params are broken for this. > >> Other device drivers of other vendors (not only netdevs) need such a > >> mechanism as well, > > > > Specifics please. > > > >> I think it's a general requirement for many drivers that normally need > >> low level configurations for device initialization in the very first > >> stage of the driver load. > > > > I do not, but feel free to prove me otherwise. > > A driver that claims the PCI device needs some configuration in init > time such as: amount of resources to be allocated in the cache, > interrupt mode, maximum allowed resources to be created for a specific > type, number of event queues, etc. See for example: > /drivers/net/ethernet/mellanox/mlx4/main.c > > This driver doesn't necessary register a logical device, it may claim > the PCI device only (e.g. for an HCA), and provide an infrastructure for > upper layer protocols to register a logical device (nic, hba, etc.) on > top of it. And how do you handle hundreds of the same card in the same machine? You can't do that with module paramaters at all, it doesn't scale, or make any sense. What about hotplug of new devices after the module is loaded? That will not work at all for module parameters either. That is why we created sysfs, and configfs. > The type of configuration needed varies between vendors, and they are > normally passed in HW initialization stage. So even if we have a tool to > configure the device (such as ethtool for netdevs) it wouldn't be a > replacement for the module param, because some systems requires > non-default (init) configurations, and if we load the driver with the > default, and then use a user-space tool to "adjust" the configuration > we'll have a "glitch" where the driver was loaded with unsuitable > configs for the system. No, just remain in a "unusable" state until you are initialized from userspace. No "glitch" at all. > >>> All of the above issues you seem to have with sysfs and configfs can be > >>> resolved with userspace code, and having your driver not do anything to > >>> the hardware until it is told to by userspace. > >> > >> To tell the driver not to do anything until it's configured by a > >> userspace code will require a module param for non-default-configs > >> (which brings us back to the original argument of avoiding module params). > > > > No, never use a module paramater for configuring a driver or a device. > > That's not what they are there for. > > > >> By having userspace code to feed configfs/sysfs nodes, and making it > >> available in initrd; we will end up having similar mechanism to > >> request_firmware(). > > > > Close, but not the same. That's why we created configfs, please use it. > > configfs requires from the driver to provide the hooks before the HW is > initialized, while module params allow passing information to the driver > in init time before any driver hooks are ready. And module params don't work at all for the common usage of hotplug devices, and multiple numbers of devices that show up _after_ the module is loaded. > The proposal of changing the driver not to configure the HW until it's > triggered by userspace service through configfs, means that we need: > > a. To have driver config to toggle between the mode where the driver > "waits" for configfs, and the "auto" mode where the driver loads with > the default-configs (when using mod params for example, the driver > simply loads with the defaults when there is modprobe config files). > > b. To have the userspace mechanism to feed the configs nodes, to store > the configs in a file to keep them persistent between reboots, and make > these userspace services available in initrd. Does this really need to be in the initrd? That's only needed for your boot device, and if you need it there, great, put it there, that's what dracut is there for, it's a full Linux system in the initrd. configfs, for complex initializations like you are proposing, works, that is why it was created, and what it is used for. Yes, it's "rough" in places, and a bit complex. Patches to fix that are always gladly appreciated. > I don't see how this is better than module params, or > request_firmware_config(). request_firmware is for just that, firmware, not configuration values, sorry. Use configfs for configuration, that's what it is there for. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kerne
Re: [PATCH 0/3] ixgbe: request_firmware for configuration parameters
On Fri, Aug 16, 2013 at 03:14:31PM -0700, Ali Ayoub wrote: > On 1/11/2013 11:41 AM, Greg KH wrote: Seriously? Restarting a thread from over 6 months ago? Why? > > On Fri, Jan 11, 2013 at 11:30:54AM -0800, Shannon Nelson wrote: > >> On Fri, Jan 11, 2013 at 10:25 AM, Greg KH > >> wrote: > >>> On Thu, Jan 10, 2013 at 06:02:20PM -0800, Shannon Nelson wrote: > Most networking dials and knobs can be set using ethtool, ifconfig, ip > link > commands, or sysfs entries, all of which can be driven by startup scripts > and other configuration tools. However, they all depend on having a > netdev > already set up, and we have some low-level device functionality that > needs > to be sorted out before we start setting up MSI-x and memory allocations. > >> > >>> Ick, please don't abuse request_firmware() for this type of thing. > >> > >> Yeah, it seemed ugly to me at first as well, but it grew on me as I > >> realized that it does solve a problem in a rather elegant way. While > >> working this up I discussed this with Mr. Woodhouse thinking that as a > >> firmware tree maintainer he'd have a similar reaction, but he actually > >> wasn't opposed to it (David, please speak up if I'm misrepresenting > >> your comments). > > > > David maintains the external firmware tree repo, not the in-kernel > > firmware core code (which I used to maintain.) > > > >>> What's wrong with configfs? It sounds like it will fit your need, and > >>> that is what is created for. > >> > >> configfs has similar problems as sysfs - the driver needs to create > >> the hooks before it has all the info it might need for some hooks, > >> there is no persistence across reboots, and I don't think it will help > >> for initrd images. Additionally, there would need to be some userland > >> mechanism to notice that the hooks were there and to feed it the > >> startup info. Using a file in the firmware path gives us persistence > >> and a way for the driver to get info before having to set up > >> filesystem hooks. It also gives us a way to get special config info > >> into the boot image. And the whole mechanism already exists, > >> including UDEV hooks that can do more fancy stuff if needed. > > > > Yes, but you are now starting to use "configuration files" for kernel > > drivers, which we have resisted for 20+ years for a variety of good > > reasons. You can't just ignore all of the arguments to not do this all > > of a sudden because you feel your driver is somehow "special" here. > > Other device drivers of other vendors (not only netdevs) need such a > mechanism as well, Specifics please. > I think it's a general requirement for many drivers that normally need > low level configurations for device initialization in the very first > stage of the driver load. I do not, but feel free to prove me otherwise. > > All of the above issues you seem to have with sysfs and configfs can be > > resolved with userspace code, and having your driver not do anything to > > the hardware until it is told to by userspace. > > To tell the driver not to do anything until it's configured by a > userspace code will require a module param for non-default-configs > (which brings us back to the original argument of avoiding module params). No, never use a module paramater for configuring a driver or a device. That's not what they are there for. > By having userspace code to feed configfs/sysfs nodes, and making it > available in initrd; we will end up having similar mechanism to > request_firmware(). Close, but not the same. That's why we created configfs, please use it. > I think this kind of "low level init configuration" can be seen as a > firmware configuration, we can put some limitation on fetching the > config file, or propose a new function such as request_firmware_config() > that uses the same uevent hooks, and leverages the available userspace > tools that already supported in initrd and meant to serve the same > purpose - of feeding the driver the suitable firmware and configuration > to get started. Nope, I do not, please don't abuse the interface like this, it's not going to be allowed at this point in time. Wait, what "point in time", this was a 6 month old conversation... greg "what month is this?" k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] ixgbe: request_firmware for configuration parameters
On 8/16/2013 3:39 PM, Greg KH wrote: > On Fri, Aug 16, 2013 at 03:14:31PM -0700, Ali Ayoub wrote: >> On 1/11/2013 11:41 AM, Greg KH wrote: > > Seriously? Restarting a thread from over 6 months ago? Yes, it's an old thread, but still relevant for current code. > Why? Because currently there is no good alternative for module params for device drivers that need to have low level configs in init time. I found Shannon proposal in this thread the closest to what I am looking for to configure the device driver and yet not to use module params. Thus, pinged this thread. >> Other device drivers of other vendors (not only netdevs) need such a >> mechanism as well, > > Specifics please. > >> I think it's a general requirement for many drivers that normally need >> low level configurations for device initialization in the very first >> stage of the driver load. > > I do not, but feel free to prove me otherwise. A driver that claims the PCI device needs some configuration in init time such as: amount of resources to be allocated in the cache, interrupt mode, maximum allowed resources to be created for a specific type, number of event queues, etc. See for example: /drivers/net/ethernet/mellanox/mlx4/main.c This driver doesn't necessary register a logical device, it may claim the PCI device only (e.g. for an HCA), and provide an infrastructure for upper layer protocols to register a logical device (nic, hba, etc.) on top of it. The type of configuration needed varies between vendors, and they are normally passed in HW initialization stage. So even if we have a tool to configure the device (such as ethtool for netdevs) it wouldn't be a replacement for the module param, because some systems requires non-default (init) configurations, and if we load the driver with the default, and then use a user-space tool to "adjust" the configuration we'll have a "glitch" where the driver was loaded with unsuitable configs for the system. >>> All of the above issues you seem to have with sysfs and configfs can be >>> resolved with userspace code, and having your driver not do anything to >>> the hardware until it is told to by userspace. >> >> To tell the driver not to do anything until it's configured by a >> userspace code will require a module param for non-default-configs >> (which brings us back to the original argument of avoiding module params). > > No, never use a module paramater for configuring a driver or a device. > That's not what they are there for. > >> By having userspace code to feed configfs/sysfs nodes, and making it >> available in initrd; we will end up having similar mechanism to >> request_firmware(). > > Close, but not the same. That's why we created configfs, please use it. configfs requires from the driver to provide the hooks before the HW is initialized, while module params allow passing information to the driver in init time before any driver hooks are ready. The proposal of changing the driver not to configure the HW until it's triggered by userspace service through configfs, means that we need: a. To have driver config to toggle between the mode where the driver "waits" for configfs, and the "auto" mode where the driver loads with the default-configs (when using mod params for example, the driver simply loads with the defaults when there is modprobe config files). b. To have the userspace mechanism to feed the configs nodes, to store the configs in a file to keep them persistent between reboots, and make these userspace services available in initrd. I don't see how this is better than module params, or request_firmware_config(). -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] ixgbe: request_firmware for configuration parameters
On 1/11/2013 11:41 AM, Greg KH wrote: > On Fri, Jan 11, 2013 at 11:30:54AM -0800, Shannon Nelson wrote: >> On Fri, Jan 11, 2013 at 10:25 AM, Greg KH wrote: >>> On Thu, Jan 10, 2013 at 06:02:20PM -0800, Shannon Nelson wrote: Most networking dials and knobs can be set using ethtool, ifconfig, ip link commands, or sysfs entries, all of which can be driven by startup scripts and other configuration tools. However, they all depend on having a netdev already set up, and we have some low-level device functionality that needs to be sorted out before we start setting up MSI-x and memory allocations. >> >>> Ick, please don't abuse request_firmware() for this type of thing. >> >> Yeah, it seemed ugly to me at first as well, but it grew on me as I >> realized that it does solve a problem in a rather elegant way. While >> working this up I discussed this with Mr. Woodhouse thinking that as a >> firmware tree maintainer he'd have a similar reaction, but he actually >> wasn't opposed to it (David, please speak up if I'm misrepresenting >> your comments). > > David maintains the external firmware tree repo, not the in-kernel > firmware core code (which I used to maintain.) > >>> What's wrong with configfs? It sounds like it will fit your need, and >>> that is what is created for. >> >> configfs has similar problems as sysfs - the driver needs to create >> the hooks before it has all the info it might need for some hooks, >> there is no persistence across reboots, and I don't think it will help >> for initrd images. Additionally, there would need to be some userland >> mechanism to notice that the hooks were there and to feed it the >> startup info. Using a file in the firmware path gives us persistence >> and a way for the driver to get info before having to set up >> filesystem hooks. It also gives us a way to get special config info >> into the boot image. And the whole mechanism already exists, >> including UDEV hooks that can do more fancy stuff if needed. > > Yes, but you are now starting to use "configuration files" for kernel > drivers, which we have resisted for 20+ years for a variety of good > reasons. You can't just ignore all of the arguments to not do this all > of a sudden because you feel your driver is somehow "special" here. Other device drivers of other vendors (not only netdevs) need such a mechanism as well, I think it's a general requirement for many drivers that normally need low level configurations for device initialization in the very first stage of the driver load. > All of the above issues you seem to have with sysfs and configfs can be > resolved with userspace code, and having your driver not do anything to > the hardware until it is told to by userspace. To tell the driver not to do anything until it's configured by a userspace code will require a module param for non-default-configs (which brings us back to the original argument of avoiding module params). By having userspace code to feed configfs/sysfs nodes, and making it available in initrd; we will end up having similar mechanism to request_firmware(). I think this kind of "low level init configuration" can be seen as a firmware configuration, we can put some limitation on fetching the config file, or propose a new function such as request_firmware_config() that uses the same uevent hooks, and leverages the available userspace tools that already supported in initrd and meant to serve the same purpose - of feeding the driver the suitable firmware and configuration to get started. Ali; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] ixgbe: request_firmware for configuration parameters
On Fri, Jan 11, 2013 at 11:30:54AM -0800, Shannon Nelson wrote: > On Fri, Jan 11, 2013 at 10:25 AM, Greg KH wrote: > > On Thu, Jan 10, 2013 at 06:02:20PM -0800, Shannon Nelson wrote: > >> Most networking dials and knobs can be set using ethtool, ifconfig, ip link > >> commands, or sysfs entries, all of which can be driven by startup scripts > >> and other configuration tools. However, they all depend on having a netdev > >> already set up, and we have some low-level device functionality that needs > >> to be sorted out before we start setting up MSI-x and memory allocations. > > > Ick, please don't abuse request_firmware() for this type of thing. > > Yeah, it seemed ugly to me at first as well, but it grew on me as I > realized that it does solve a problem in a rather elegant way. While > working this up I discussed this with Mr. Woodhouse thinking that as a > firmware tree maintainer he'd have a similar reaction, but he actually > wasn't opposed to it (David, please speak up if I'm misrepresenting > your comments). David maintains the external firmware tree repo, not the in-kernel firmware core code (which I used to maintain.) > > What's wrong with configfs? It sounds like it will fit your need, and > > that is what is created for. > > configfs has similar problems as sysfs - the driver needs to create > the hooks before it has all the info it might need for some hooks, > there is no persistence across reboots, and I don't think it will help > for initrd images. Additionally, there would need to be some userland > mechanism to notice that the hooks were there and to feed it the > startup info. Using a file in the firmware path gives us persistence > and a way for the driver to get info before having to set up > filesystem hooks. It also gives us a way to get special config info > into the boot image. And the whole mechanism already exists, > including UDEV hooks that can do more fancy stuff if needed. Yes, but you are now starting to use "configuration files" for kernel drivers, which we have resisted for 20+ years for a variety of good reasons. You can't just ignore all of the arguments to not do this all of a sudden because you feel your driver is somehow "special" here. All of the above issues you seem to have with sysfs and configfs can be resolved with userspace code, and having your driver not do anything to the hardware until it is told to by userspace. The boot image problem is harder, but I would argue that your driver better fall-back to some "known good" configuration for that type of instance, as you will need to do that anyway if your "firmware" file isn't present in the first place. So please try configfs again, the idea of loading configuration files from the filesystem, no matter what the mechanism, into your driver, isn't ok to do, sorry. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] ixgbe: request_firmware for configuration parameters
On Fri, Jan 11, 2013 at 10:25 AM, Greg KH wrote: > On Thu, Jan 10, 2013 at 06:02:20PM -0800, Shannon Nelson wrote: >> Most networking dials and knobs can be set using ethtool, ifconfig, ip link >> commands, or sysfs entries, all of which can be driven by startup scripts >> and other configuration tools. However, they all depend on having a netdev >> already set up, and we have some low-level device functionality that needs >> to be sorted out before we start setting up MSI-x and memory allocations. > Ick, please don't abuse request_firmware() for this type of thing. Yeah, it seemed ugly to me at first as well, but it grew on me as I realized that it does solve a problem in a rather elegant way. While working this up I discussed this with Mr. Woodhouse thinking that as a firmware tree maintainer he'd have a similar reaction, but he actually wasn't opposed to it (David, please speak up if I'm misrepresenting your comments). > What's wrong with configfs? It sounds like it will fit your need, and > that is what is created for. configfs has similar problems as sysfs - the driver needs to create the hooks before it has all the info it might need for some hooks, there is no persistence across reboots, and I don't think it will help for initrd images. Additionally, there would need to be some userland mechanism to notice that the hooks were there and to feed it the startup info. Using a file in the firmware path gives us persistence and a way for the driver to get info before having to set up filesystem hooks. It also gives us a way to get special config info into the boot image. And the whole mechanism already exists, including UDEV hooks that can do more fancy stuff if needed. sln -- == Mr. Shannon Nelson Parents can't afford to be squeamish. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] ixgbe: request_firmware for configuration parameters
On Thu, Jan 10, 2013 at 06:02:20PM -0800, Shannon Nelson wrote: > Most networking dials and knobs can be set using ethtool, ifconfig, ip link > commands, or sysfs entries, all of which can be driven by startup scripts > and other configuration tools. However, they all depend on having a netdev > already set up, and we have some low-level device functionality that needs > to be sorted out before we start setting up MSI-x and memory allocations. > > In order to do early device configuration, most kernel drivers use module > parameters whose settings can be persisted in modprobe.d config files. > However, these can be clumsy to use and manage, difficult to specify port > specific values in a multiport device, and are actively discouraged in > some circles. > > In this patchset, the driver uses the existing request_firmware() and > match_token() facilities to grab an ASCII config file from userspace to > find special startup-time configuration information that needs persistence > across reboots. The configuration strings are formed similar to the > mount options that get passed from /etc/fstab into filesystem modules. Ick, please don't abuse request_firmware() for this type of thing. What's wrong with configfs? It sounds like it will fit your need, and that is what is created for. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] ixgbe: request_firmware for configuration parameters
On Thu, Jan 10, 2013 at 6:02 PM, Shannon Nelson wrote: [...] > > In these RFC patches for ixgbe, > Yeah, these should have the "RFC" in the Subject line. Sorry about that. sln -- == Mr. Shannon Nelson Parents can't afford to be squeamish. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3] ixgbe: request_firmware for configuration parameters
Most networking dials and knobs can be set using ethtool, ifconfig, ip link commands, or sysfs entries, all of which can be driven by startup scripts and other configuration tools. However, they all depend on having a netdev already set up, and we have some low-level device functionality that needs to be sorted out before we start setting up MSI-x and memory allocations. In order to do early device configuration, most kernel drivers use module parameters whose settings can be persisted in modprobe.d config files. However, these can be clumsy to use and manage, difficult to specify port specific values in a multiport device, and are actively discouraged in some circles. In this patchset, the driver uses the existing request_firmware() and match_token() facilities to grab an ASCII config file from userspace to find special startup-time configuration information that needs persistence across reboots. The configuration strings are formed similar to the mount options that get passed from /etc/fstab into filesystem modules. We can assume that the driver and device will come up with sane defaults that will make the part reasonably useful - we're not solving something for basic usability. This capability is for folks that need special configurations for their virtualization server or cloud configuration or whatever network server where they've taken the time to tune things more specifically to their application. We are using existing facilities so that we don't have to build any userland utilities, and a config file format that is both humanly and potentially mechanically editable. After doing some digging around, it seems that the combination of ASCII config files stored where request_firmware() can find them, plus the kind of option configurations used in fstab for mount commands can solve the need. Using the lib/parser.c tools seems obvious - we don't want to add any other parser code to the kernel, and heaven forbid someone tries to extend any new parser into an XML solution. Using the request_firmware() framework also allows the configuration file to get included when building an initrd image. In these RFC patches for ixgbe, the configuration data would be found in the file /lib/firmware/ixgbe.conf, and might look something like this: # # example ixgbe driver configuration # # this is a generic setting for the driver ixgbe allow_unsupported_sfp,debug=0xf,AtrSampleRate=0 # msix restricted on one port for testing purposes 00:1b:21:12:4e:60 nomsix Signed-off-by: Shannon Nelson Cc: Jeff Kirsher Cc: David Woodhouse --- Shannon Nelson (3): ixgbe: add interrupt control parameters ixgbe: add additional parameter options ixgbe: replace module options with configuration through request_firmware drivers/net/ethernet/intel/ixgbe/ixgbe.h |4 drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c |6 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 305 +++-- 3 files changed, 289 insertions(+), 26 deletions(-) -- == Mr. Shannon Nelson LAN Access Division, Intel Corp. shannon.nel...@intel.comI don't speak for Intel (503) 712-7659Parents can't afford to be squeamish -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
