Re: [PATCH net-next RFC 7/7] bnxt_en: Add bnxt_en initial port params table and register it
Thu, Dec 06, 2018 at 11:31:26AM CET, vasundhara-v.vo...@broadcom.com wrote: >On Thu, Dec 6, 2018 at 2:41 PM Jiri Pirko wrote: >> >> Thu, Dec 06, 2018 at 09:57:05AM CET, michael.c...@broadcom.com wrote: >> >On Wed, Dec 5, 2018 at 11:11 PM Jakub Kicinski >> > wrote: >> >> >> >> On Wed, 5 Dec 2018 22:41:43 -0800, Michael Chan wrote: >> >> > >> >> > It will be in the BIOS only for a LOM, I think. For a NIC, it should >> >> > be in the NIC's NVRAM. >> >> >> >> This is all vague. Could you please clearly state the use case. >> >> >> >Well, the WoL setting's use case should be quite simple, right? If >> >the card's NVRAM WoL setting is ON, when you plug the card in a slot >> >that has Vaux power, it will assert PME# when a magic packet is >> >received. Again, the WoL setting in this context is similar to other >> >power up settings such as PCIe Gen2 or Gen3. >> > >> >Let's say the power up setting is ON and it boots up to Linux for the >> >first time after receiving a magic packet. The Linux user can then >> >run ethtool -s to set the driver's non persistent WoL setting. It can >> >be the same as the NVRAM's power up setting, or different. Ethtool >> >may support additional WoL packet types that the power up setting does >> >> Wouldn't it make sense to also support multiple wol packet types in >> devlink param, not just true/false? Your device may not support that but >> others may. So enum instead of bool. >There is no type enum in devlink types as of now. Instead it can be >defined as u8 and >a enum structure can be defined for wol types. Yes. >> >> >> >not support. Let's say the Linux user sets the ethtool WoL setting to >> >OFF and shuts down the system. That card now will not wake up the >> >system. But if there is a power failure and power comes back on >> >later, the card will lose the ethtool setting and go back to the power >> >up WoL setting, which is ON in this example.
Re: [PATCH net-next RFC 7/7] bnxt_en: Add bnxt_en initial port params table and register it
On Thu, Dec 6, 2018 at 2:37 AM Jakub Kicinski wrote: > > On Thu, 6 Dec 2018 00:57:05 -0800, Michael Chan wrote: > > On Wed, Dec 5, 2018 at 11:11 PM Jakub Kicinski wrote: > > > > > > On Wed, 5 Dec 2018 22:41:43 -0800, Michael Chan wrote: > > > > > > > > It will be in the BIOS only for a LOM, I think. For a NIC, it should > > > > be in the NIC's NVRAM. > > > > > > This is all vague. Could you please clearly state the use case. > > > > > Well, the WoL setting's use case should be quite simple, right? If > > the card's NVRAM WoL setting is ON, when you plug the card in a slot > > that has Vaux power, it will assert PME# when a magic packet is > > received. Again, the WoL setting in this context is similar to other > > power up settings such as PCIe Gen2 or Gen3. > > If there was some configuration of PME# involved, maybe, but > basic networking configuration has its APIs already. > > > Let's say the power up setting is ON and it boots up to Linux for the > > first time after receiving a magic packet. The Linux user can then > > run ethtool -s to set the driver's non persistent WoL setting. It can > > be the same as the NVRAM's power up setting, or different. Ethtool > > may support additional WoL packet types that the power up setting does > > not support. Let's say the Linux user sets the ethtool WoL setting to > > OFF and shuts down the system. That card now will not wake up the > > system. But if there is a power failure and power comes back on > > later, the card will lose the ethtool setting and go back to the power > > up WoL setting, which is ON in this example. > > So in your example there is a machine with a 25/40/100G NIC that > doesn't have any remote BMC control, and connected to a L2 network > where a magic packet can be received. > > In my experience machines are either low end/embedded and they just > boot on power on fully (to Linux), or they are proper machines which > support IPMI etc. > > If you could illuminate the use case some more I'd really appreciate > that. In your hypothetical scenario you still have to get the link > up, so if we apply this patch a logical extension would be to add all > ethtool link settings as devlink parameters as well. Florian recently > added an option to wake based on a packet that matched an n-tuple > filter. If your use case is legit, doing the same thing with n-tuple > filters instead of Magic Packets is very much legit, too. So we will > poke n-tuple filters via devlink params? We only store a magic packet WoL bit in the NVRAM for basic power up WoL setting. I doubt that people will store the entire n-tuple WoL pattern in NVRAM for basic power up WoL. The whole idea is to have a basic method to wake up the machine after power up with Vaux. If the cable is connected, the NIC will autoneg to some lower speed that Vaux can support. I think we've been supporting this since the tg3 days.
Re: [PATCH net-next RFC 7/7] bnxt_en: Add bnxt_en initial port params table and register it
On Thu, 6 Dec 2018 00:57:05 -0800, Michael Chan wrote: > On Wed, Dec 5, 2018 at 11:11 PM Jakub Kicinski wrote: > > > > On Wed, 5 Dec 2018 22:41:43 -0800, Michael Chan wrote: > > > > > > It will be in the BIOS only for a LOM, I think. For a NIC, it should > > > be in the NIC's NVRAM. > > > > This is all vague. Could you please clearly state the use case. > > > Well, the WoL setting's use case should be quite simple, right? If > the card's NVRAM WoL setting is ON, when you plug the card in a slot > that has Vaux power, it will assert PME# when a magic packet is > received. Again, the WoL setting in this context is similar to other > power up settings such as PCIe Gen2 or Gen3. If there was some configuration of PME# involved, maybe, but basic networking configuration has its APIs already. > Let's say the power up setting is ON and it boots up to Linux for the > first time after receiving a magic packet. The Linux user can then > run ethtool -s to set the driver's non persistent WoL setting. It can > be the same as the NVRAM's power up setting, or different. Ethtool > may support additional WoL packet types that the power up setting does > not support. Let's say the Linux user sets the ethtool WoL setting to > OFF and shuts down the system. That card now will not wake up the > system. But if there is a power failure and power comes back on > later, the card will lose the ethtool setting and go back to the power > up WoL setting, which is ON in this example. So in your example there is a machine with a 25/40/100G NIC that doesn't have any remote BMC control, and connected to a L2 network where a magic packet can be received. In my experience machines are either low end/embedded and they just boot on power on fully (to Linux), or they are proper machines which support IPMI etc. If you could illuminate the use case some more I'd really appreciate that. In your hypothetical scenario you still have to get the link up, so if we apply this patch a logical extension would be to add all ethtool link settings as devlink parameters as well. Florian recently added an option to wake based on a packet that matched an n-tuple filter. If your use case is legit, doing the same thing with n-tuple filters instead of Magic Packets is very much legit, too. So we will poke n-tuple filters via devlink params?
Re: [PATCH net-next RFC 7/7] bnxt_en: Add bnxt_en initial port params table and register it
On Thu, Dec 6, 2018 at 2:41 PM Jiri Pirko wrote: > > Thu, Dec 06, 2018 at 09:57:05AM CET, michael.c...@broadcom.com wrote: > >On Wed, Dec 5, 2018 at 11:11 PM Jakub Kicinski > > wrote: > >> > >> On Wed, 5 Dec 2018 22:41:43 -0800, Michael Chan wrote: > >> > > >> > It will be in the BIOS only for a LOM, I think. For a NIC, it should > >> > be in the NIC's NVRAM. > >> > >> This is all vague. Could you please clearly state the use case. > >> > >Well, the WoL setting's use case should be quite simple, right? If > >the card's NVRAM WoL setting is ON, when you plug the card in a slot > >that has Vaux power, it will assert PME# when a magic packet is > >received. Again, the WoL setting in this context is similar to other > >power up settings such as PCIe Gen2 or Gen3. > > > >Let's say the power up setting is ON and it boots up to Linux for the > >first time after receiving a magic packet. The Linux user can then > >run ethtool -s to set the driver's non persistent WoL setting. It can > >be the same as the NVRAM's power up setting, or different. Ethtool > >may support additional WoL packet types that the power up setting does > > Wouldn't it make sense to also support multiple wol packet types in > devlink param, not just true/false? Your device may not support that but > others may. So enum instead of bool. There is no type enum in devlink types as of now. Instead it can be defined as u8 and a enum structure can be defined for wol types. > > > >not support. Let's say the Linux user sets the ethtool WoL setting to > >OFF and shuts down the system. That card now will not wake up the > >system. But if there is a power failure and power comes back on > >later, the card will lose the ethtool setting and go back to the power > >up WoL setting, which is ON in this example.
Re: [PATCH net-next RFC 7/7] bnxt_en: Add bnxt_en initial port params table and register it
Thu, Dec 06, 2018 at 09:57:05AM CET, michael.c...@broadcom.com wrote: >On Wed, Dec 5, 2018 at 11:11 PM Jakub Kicinski > wrote: >> >> On Wed, 5 Dec 2018 22:41:43 -0800, Michael Chan wrote: >> > >> > It will be in the BIOS only for a LOM, I think. For a NIC, it should >> > be in the NIC's NVRAM. >> >> This is all vague. Could you please clearly state the use case. >> >Well, the WoL setting's use case should be quite simple, right? If >the card's NVRAM WoL setting is ON, when you plug the card in a slot >that has Vaux power, it will assert PME# when a magic packet is >received. Again, the WoL setting in this context is similar to other >power up settings such as PCIe Gen2 or Gen3. > >Let's say the power up setting is ON and it boots up to Linux for the >first time after receiving a magic packet. The Linux user can then >run ethtool -s to set the driver's non persistent WoL setting. It can >be the same as the NVRAM's power up setting, or different. Ethtool >may support additional WoL packet types that the power up setting does Wouldn't it make sense to also support multiple wol packet types in devlink param, not just true/false? Your device may not support that but others may. So enum instead of bool. >not support. Let's say the Linux user sets the ethtool WoL setting to >OFF and shuts down the system. That card now will not wake up the >system. But if there is a power failure and power comes back on >later, the card will lose the ethtool setting and go back to the power >up WoL setting, which is ON in this example.
Re: [PATCH net-next RFC 7/7] bnxt_en: Add bnxt_en initial port params table and register it
On Wed, Dec 5, 2018 at 11:11 PM Jakub Kicinski wrote: > > On Wed, 5 Dec 2018 22:41:43 -0800, Michael Chan wrote: > > > > It will be in the BIOS only for a LOM, I think. For a NIC, it should > > be in the NIC's NVRAM. > > This is all vague. Could you please clearly state the use case. > Well, the WoL setting's use case should be quite simple, right? If the card's NVRAM WoL setting is ON, when you plug the card in a slot that has Vaux power, it will assert PME# when a magic packet is received. Again, the WoL setting in this context is similar to other power up settings such as PCIe Gen2 or Gen3. Let's say the power up setting is ON and it boots up to Linux for the first time after receiving a magic packet. The Linux user can then run ethtool -s to set the driver's non persistent WoL setting. It can be the same as the NVRAM's power up setting, or different. Ethtool may support additional WoL packet types that the power up setting does not support. Let's say the Linux user sets the ethtool WoL setting to OFF and shuts down the system. That card now will not wake up the system. But if there is a power failure and power comes back on later, the card will lose the ethtool setting and go back to the power up WoL setting, which is ON in this example.
Re: [PATCH net-next RFC 7/7] bnxt_en: Add bnxt_en initial port params table and register it
On Wed, 5 Dec 2018 22:41:43 -0800, Michael Chan wrote: > > > We do have a parameter in NVRAM that controls default WoL. I think > > > this is to expose that parameter so it can be set one way or the > > > other. There are scenarios where Linux has not booted yet (and so > > > there is no opportunity to run ethtool -s or any daemons yet) and this > > > parameter will control whether the machine will wake up or not. > > > > Isn't that set in BIOS/setup? The config before any OS boots? Because > > the BMC or whatnot has to actually configure the board to power > > appropriate things up. Please clarify. > > It will be in the BIOS only for a LOM, I think. For a NIC, it should > be in the NIC's NVRAM. This is all vague. Could you please clearly state the use case. > > And *if* it is proven this config is more than just setting the default > > IMHO the setting belongs in the ethtool API. We can't just add devlink > > params for all existing config APIs just because it has persistence. > > I'm not sure I understand your point. I believe the NIC firmware will > set up the NIC's WoL setting right after power up based on this NVRAM > parameter. Similar to how the firmware will setup PCIe Gen2 or Gen3 > right after power up, for example. We have no PCIe config interface therefore the crutch of devlink params was allowed there. We *do* have an existing interface to configure WoL. > So why would this belong to ethtool? I understand the confusion that > ethtool -s has a similar WoL setting. But again, that's different. Perhaps you're looking at this from firmware perspective? FW NVM knob == devlink param? > This one is the power up setting that impacts whether a magic packet > can or cannot wake up the system right after power up (before booting > up to Linux or other OS).
Re: [PATCH net-next RFC 7/7] bnxt_en: Add bnxt_en initial port params table and register it
On Wed, Dec 5, 2018 at 10:00 PM Jakub Kicinski wrote: > > On Wed, 5 Dec 2018 17:18:52 -0800, Michael Chan wrote: > > On Wed, Dec 5, 2018 at 4:42 PM Jakub Kicinski wrote: > > > On Wed, 5 Dec 2018 16:01:08 -0800, Michael Chan wrote: > > > > On Wed, Dec 5, 2018 at 3:33 PM Jakub Kicinski wrote: > > > > > On Wed, 5 Dec 2018 11:27:00 +0530, Vasundhara Volam wrote: > > > > > > Register devlink_port with devlink and create initial port params > > > > > > table for bnxt_en. The table consists of a generic parameter: > > > > > > > > > > > > wake-on-lan: Enables Wake on Lan for this port when magic packet > > > > > > is received with this port's MAC address using ACPI pattern. > > > > > > If enabled, the controller asserts a wake pin upon reception of > > > > > > WoL packet. ACPI (Advanced Configuration and Power Interface) is > > > > > > an industry specification for the efficient handling of power > > > > > > consumption in desktop and mobile computers. > > > > > > > > > > > > Cc: Michael Chan > > > > > > Signed-off-by: Vasundhara Volam > > > > > > > > > > Why do we need a WoL as a devlink parameter (rather than ethtool -s)? > > > > > > > > I believe ethtool -s for WoL is a non-persistent setting, meaning that > > > > if you power cycle the system, the WoL setting will go back to > > > > default. > > > > > > > > devlink on the other hand is a permanent setting. ethtool should > > > > initially report the default WoL setting and it can then be changed > > > > (in a non permanent way) using ethtool -s. > > > > > > All network configuration settings in Linux are non-persistent AFAIK. > > > That's why network configuration daemons exist: > > > > > > https://wiki.debian.org/WakeOnLan > > > > > > Perhaps the objective to move more of the network configuration into the > > > firmware? That'd be a bleak scenario, so probably not.. > > > > > > My understanding was the persistent devlink settings are for things > > > which have to be set at device init time. Like say PCI endpoint > > > configuration. FW loading configuration. > > > > > > Besides, the parameter you add is just true/false, when ethtool has > > > multiple options. > > > > > > It feels to me like we moved from ioctls to Netlink, and now even > > > before ethtool was converted to Netlink we may move to unstructured > > > strings. That's not a step forward, if you ask me. > > > > We do have a parameter in NVRAM that controls default WoL. I think > > this is to expose that parameter so it can be set one way or the > > other. There are scenarios where Linux has not booted yet (and so > > there is no opportunity to run ethtool -s or any daemons yet) and this > > parameter will control whether the machine will wake up or not. > > Isn't that set in BIOS/setup? The config before any OS boots? Because > the BMC or whatnot has to actually configure the board to power > appropriate things up. Please clarify. It will be in the BIOS only for a LOM, I think. For a NIC, it should be in the NIC's NVRAM. > > And *if* it is proven this config is more than just setting the default > IMHO the setting belongs in the ethtool API. We can't just add devlink > params for all existing config APIs just because it has persistence. I'm not sure I understand your point. I believe the NIC firmware will set up the NIC's WoL setting right after power up based on this NVRAM parameter. Similar to how the firmware will setup PCIe Gen2 or Gen3 right after power up, for example. So why would this belong to ethtool? I understand the confusion that ethtool -s has a similar WoL setting. But again, that's different. This one is the power up setting that impacts whether a magic packet can or cannot wake up the system right after power up (before booting up to Linux or other OS).
Re: [PATCH net-next RFC 7/7] bnxt_en: Add bnxt_en initial port params table and register it
On Wed, 5 Dec 2018 17:18:52 -0800, Michael Chan wrote: > On Wed, Dec 5, 2018 at 4:42 PM Jakub Kicinski wrote: > > On Wed, 5 Dec 2018 16:01:08 -0800, Michael Chan wrote: > > > On Wed, Dec 5, 2018 at 3:33 PM Jakub Kicinski wrote: > > > > On Wed, 5 Dec 2018 11:27:00 +0530, Vasundhara Volam wrote: > > > > > Register devlink_port with devlink and create initial port params > > > > > table for bnxt_en. The table consists of a generic parameter: > > > > > > > > > > wake-on-lan: Enables Wake on Lan for this port when magic packet > > > > > is received with this port's MAC address using ACPI pattern. > > > > > If enabled, the controller asserts a wake pin upon reception of > > > > > WoL packet. ACPI (Advanced Configuration and Power Interface) is > > > > > an industry specification for the efficient handling of power > > > > > consumption in desktop and mobile computers. > > > > > > > > > > Cc: Michael Chan > > > > > Signed-off-by: Vasundhara Volam > > > > > > > > Why do we need a WoL as a devlink parameter (rather than ethtool -s)? > > > > > > I believe ethtool -s for WoL is a non-persistent setting, meaning that > > > if you power cycle the system, the WoL setting will go back to > > > default. > > > > > > devlink on the other hand is a permanent setting. ethtool should > > > initially report the default WoL setting and it can then be changed > > > (in a non permanent way) using ethtool -s. > > > > All network configuration settings in Linux are non-persistent AFAIK. > > That's why network configuration daemons exist: > > > > https://wiki.debian.org/WakeOnLan > > > > Perhaps the objective to move more of the network configuration into the > > firmware? That'd be a bleak scenario, so probably not.. > > > > My understanding was the persistent devlink settings are for things > > which have to be set at device init time. Like say PCI endpoint > > configuration. FW loading configuration. > > > > Besides, the parameter you add is just true/false, when ethtool has > > multiple options. > > > > It feels to me like we moved from ioctls to Netlink, and now even > > before ethtool was converted to Netlink we may move to unstructured > > strings. That's not a step forward, if you ask me. > > We do have a parameter in NVRAM that controls default WoL. I think > this is to expose that parameter so it can be set one way or the > other. There are scenarios where Linux has not booted yet (and so > there is no opportunity to run ethtool -s or any daemons yet) and this > parameter will control whether the machine will wake up or not. Isn't that set in BIOS/setup? The config before any OS boots? Because the BMC or whatnot has to actually configure the board to power appropriate things up. Please clarify. And *if* it is proven this config is more than just setting the default IMHO the setting belongs in the ethtool API. We can't just add devlink params for all existing config APIs just because it has persistence.
Re: [PATCH net-next RFC 7/7] bnxt_en: Add bnxt_en initial port params table and register it
On Wed, Dec 5, 2018 at 4:42 PM Jakub Kicinski wrote: > > On Wed, 5 Dec 2018 16:01:08 -0800, Michael Chan wrote: > > On Wed, Dec 5, 2018 at 3:33 PM Jakub Kicinski > > wrote: > > > > > > On Wed, 5 Dec 2018 11:27:00 +0530, Vasundhara Volam wrote: > > > > Register devlink_port with devlink and create initial port params > > > > table for bnxt_en. The table consists of a generic parameter: > > > > > > > > wake-on-lan: Enables Wake on Lan for this port when magic packet > > > > is received with this port's MAC address using ACPI pattern. > > > > If enabled, the controller asserts a wake pin upon reception of > > > > WoL packet. ACPI (Advanced Configuration and Power Interface) is > > > > an industry specification for the efficient handling of power > > > > consumption in desktop and mobile computers. > > > > > > > > Cc: Michael Chan > > > > Signed-off-by: Vasundhara Volam > > > > > > Why do we need a WoL as a devlink parameter (rather than ethtool -s)? > > > > I believe ethtool -s for WoL is a non-persistent setting, meaning that > > if you power cycle the system, the WoL setting will go back to > > default. > > > > devlink on the other hand is a permanent setting. ethtool should > > initially report the default WoL setting and it can then be changed > > (in a non permanent way) using ethtool -s. > > All network configuration settings in Linux are non-persistent AFAIK. > That's why network configuration daemons exist: > > https://wiki.debian.org/WakeOnLan > > Perhaps the objective to move more of the network configuration into the > firmware? That'd be a bleak scenario, so probably not.. > > My understanding was the persistent devlink settings are for things > which have to be set at device init time. Like say PCI endpoint > configuration. FW loading configuration. > > Besides, the parameter you add is just true/false, when ethtool has > multiple options. > > It feels to me like we moved from ioctls to Netlink, and now even > before ethtool was converted to Netlink we may move to unstructured > strings. That's not a step forward, if you ask me. We do have a parameter in NVRAM that controls default WoL. I think this is to expose that parameter so it can be set one way or the other. There are scenarios where Linux has not booted yet (and so there is no opportunity to run ethtool -s or any daemons yet) and this parameter will control whether the machine will wake up or not.
Re: [PATCH net-next RFC 7/7] bnxt_en: Add bnxt_en initial port params table and register it
On Wed, 5 Dec 2018 16:01:08 -0800, Michael Chan wrote: > On Wed, Dec 5, 2018 at 3:33 PM Jakub Kicinski > wrote: > > > > On Wed, 5 Dec 2018 11:27:00 +0530, Vasundhara Volam wrote: > > > Register devlink_port with devlink and create initial port params > > > table for bnxt_en. The table consists of a generic parameter: > > > > > > wake-on-lan: Enables Wake on Lan for this port when magic packet > > > is received with this port's MAC address using ACPI pattern. > > > If enabled, the controller asserts a wake pin upon reception of > > > WoL packet. ACPI (Advanced Configuration and Power Interface) is > > > an industry specification for the efficient handling of power > > > consumption in desktop and mobile computers. > > > > > > Cc: Michael Chan > > > Signed-off-by: Vasundhara Volam > > > > Why do we need a WoL as a devlink parameter (rather than ethtool -s)? > > I believe ethtool -s for WoL is a non-persistent setting, meaning that > if you power cycle the system, the WoL setting will go back to > default. > > devlink on the other hand is a permanent setting. ethtool should > initially report the default WoL setting and it can then be changed > (in a non permanent way) using ethtool -s. All network configuration settings in Linux are non-persistent AFAIK. That's why network configuration daemons exist: https://wiki.debian.org/WakeOnLan Perhaps the objective to move more of the network configuration into the firmware? That'd be a bleak scenario, so probably not.. My understanding was the persistent devlink settings are for things which have to be set at device init time. Like say PCI endpoint configuration. FW loading configuration. Besides, the parameter you add is just true/false, when ethtool has multiple options. It feels to me like we moved from ioctls to Netlink, and now even before ethtool was converted to Netlink we may move to unstructured strings. That's not a step forward, if you ask me.
Re: [PATCH net-next RFC 7/7] bnxt_en: Add bnxt_en initial port params table and register it
On Wed, Dec 5, 2018 at 3:33 PM Jakub Kicinski wrote: > > On Wed, 5 Dec 2018 11:27:00 +0530, Vasundhara Volam wrote: > > Register devlink_port with devlink and create initial port params > > table for bnxt_en. The table consists of a generic parameter: > > > > wake-on-lan: Enables Wake on Lan for this port when magic packet > > is received with this port's MAC address using ACPI pattern. > > If enabled, the controller asserts a wake pin upon reception of > > WoL packet. ACPI (Advanced Configuration and Power Interface) is > > an industry specification for the efficient handling of power > > consumption in desktop and mobile computers. > > > > Cc: Michael Chan > > Signed-off-by: Vasundhara Volam > > Why do we need a WoL as a devlink parameter (rather than ethtool -s)? I believe ethtool -s for WoL is a non-persistent setting, meaning that if you power cycle the system, the WoL setting will go back to default. devlink on the other hand is a permanent setting. ethtool should initially report the default WoL setting and it can then be changed (in a non permanent way) using ethtool -s.
Re: [PATCH net-next RFC 7/7] bnxt_en: Add bnxt_en initial port params table and register it
On Wed, 5 Dec 2018 11:27:00 +0530, Vasundhara Volam wrote: > Register devlink_port with devlink and create initial port params > table for bnxt_en. The table consists of a generic parameter: > > wake-on-lan: Enables Wake on Lan for this port when magic packet > is received with this port's MAC address using ACPI pattern. > If enabled, the controller asserts a wake pin upon reception of > WoL packet. ACPI (Advanced Configuration and Power Interface) is > an industry specification for the efficient handling of power > consumption in desktop and mobile computers. > > Cc: Michael Chan > Signed-off-by: Vasundhara Volam Why do we need a WoL as a devlink parameter (rather than ethtool -s)?
Re: [PATCH net-next RFC 7/7] bnxt_en: Add bnxt_en initial port params table and register it
Wed, Dec 05, 2018 at 06:57:00AM CET, vasundhara-v.vo...@broadcom.com wrote:
>Register devlink_port with devlink and create initial port params
>table for bnxt_en. The table consists of a generic parameter:
>
>wake-on-lan: Enables Wake on Lan for this port when magic packet
>is received with this port's MAC address using ACPI pattern.
>If enabled, the controller asserts a wake pin upon reception of
>WoL packet. ACPI (Advanced Configuration and Power Interface) is
>an industry specification for the efficient handling of power
>consumption in desktop and mobile computers.
>
>Cc: Michael Chan
>Signed-off-by: Vasundhara Volam
>---
> drivers/net/ethernet/broadcom/bnxt/bnxt.h | 1 +
> drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 35 +++
> drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 1 +
> 3 files changed, 37 insertions(+)
>
>diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
>b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
>index 9e99d4a..0ba62b3 100644
>--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
>+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
>@@ -1585,6 +1585,7 @@ struct bnxt {
>
> /* devlink interface and vf-rep structs */
> struct devlink *dl;
>+ struct devlink_port dl_port;
> enum devlink_eswitch_mode eswitch_mode;
> struct bnxt_vf_rep **vf_reps; /* array of vf-rep ptrs */
> u16 *cfa_code_map; /* cfa_code -> vf_idx map */
>diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>index 140dbd6..a2930d5 100644
>--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>@@ -26,6 +26,10 @@ enum bnxt_dl_param_id {
> BNXT_DEVLINK_PARAM_ID_GRE_VER_CHECK,
> };
>
>+enum bnxt_dl_port_param_id {
>+ BNXT_DEVLINK_PORT_PARAM_ID_BASE = DEVLINK_PORT_PARAM_GENERIC_ID_MAX,
>+};
>+
> static const struct bnxt_dl_nvm_param nvm_params[] = {
> {DEVLINK_PARAM_GENERIC_ID_ENABLE_SRIOV, NVM_OFF_ENABLE_SRIOV,
>BNXT_NVM_SHARED_CFG, 1},
>@@ -37,6 +41,8 @@ enum bnxt_dl_param_id {
>NVM_OFF_MSIX_VEC_PER_PF_MIN, BNXT_NVM_SHARED_CFG, 7},
> {BNXT_DEVLINK_PARAM_ID_GRE_VER_CHECK, NVM_OFF_DIS_GRE_VER_CHECK,
>BNXT_NVM_SHARED_CFG, 1},
>+
>+ {DEVLINK_PORT_PARAM_GENERIC_ID_WOL, NVM_OFF_WOL, BNXT_NVM_PORT_CFG, 1},
> };
>
> static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
>@@ -188,6 +194,12 @@ static int bnxt_dl_msix_validate(struct devlink *dl, u32
>id,
>NULL),
> };
>
>+static const struct devlink_param bnxt_dl_port_params[] = {
>+ DEVLINK_PORT_PARAM_GENERIC(WOL, BIT(DEVLINK_PARAM_CMODE_PERMANENT),
>+ bnxt_dl_nvm_param_get, bnxt_dl_nvm_param_set,
>+ NULL),
>+};
>+
> int bnxt_dl_register(struct bnxt *bp)
> {
> struct devlink *dl;
>@@ -225,8 +237,28 @@ int bnxt_dl_register(struct bnxt *bp)
> goto err_dl_unreg;
> }
>
>+ rc = devlink_port_register(dl, >dl_port, bp->pf.port_id);
>+ if (rc) {
>+ netdev_warn(bp->dev, "devlink_port_register failed. rc=%d",
>+ rc);
>+ goto err_dl_param_unreg;
>+ }
>+ bp->dl_port.type = DEVLINK_PORT_TYPE_ETH;
Please use devlink_port_type_set()
>+
>+ rc = devlink_port_params_register(>dl_port, bnxt_dl_port_params,
>+ARRAY_SIZE(bnxt_dl_port_params));
>+ if (rc) {
>+ netdev_warn(bp->dev, "devlink_port_params_register failed.
>rc=%d",
>+ rc);
>+ goto err_dl_port_unreg;
>+ }
> return 0;
>
>+err_dl_port_unreg:
>+ devlink_port_unregister(>dl_port);
>+err_dl_param_unreg:
>+ devlink_params_unregister(dl, bnxt_dl_params,
>+ARRAY_SIZE(bnxt_dl_params));
> err_dl_unreg:
> devlink_unregister(dl);
> err_dl_free:
>@@ -242,6 +274,9 @@ void bnxt_dl_unregister(struct bnxt *bp)
> if (!dl)
> return;
>
>+ devlink_port_params_unregister(>dl_port, bnxt_dl_port_params,
>+ ARRAY_SIZE(bnxt_dl_port_params));
>+ devlink_port_unregister(>dl_port);
> devlink_params_unregister(dl, bnxt_dl_params,
> ARRAY_SIZE(bnxt_dl_params));
> devlink_unregister(dl);
>diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
>b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
>index 5b6b2c7..da065ca 100644
>--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
>+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
>@@ -35,6 +35,7 @@ static inline void bnxt_link_bp_to_dl(struct bnxt *bp,
>struct devlink *dl)
>
> #define NVM_OFF_MSIX_VEC_PER_PF_MAX 108
> #define NVM_OFF_MSIX_VEC_PER_PF_MIN 114
>+#define NVM_OFF_WOL 152
> #define NVM_OFF_IGNORE_ARI164
>
[PATCH net-next RFC 7/7] bnxt_en: Add bnxt_en initial port params table and register it
Register devlink_port with devlink and create initial port params
table for bnxt_en. The table consists of a generic parameter:
wake-on-lan: Enables Wake on Lan for this port when magic packet
is received with this port's MAC address using ACPI pattern.
If enabled, the controller asserts a wake pin upon reception of
WoL packet. ACPI (Advanced Configuration and Power Interface) is
an industry specification for the efficient handling of power
consumption in desktop and mobile computers.
Cc: Michael Chan
Signed-off-by: Vasundhara Volam
---
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 1 +
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 35 +++
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 1 +
3 files changed, 37 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 9e99d4a..0ba62b3 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1585,6 +1585,7 @@ struct bnxt {
/* devlink interface and vf-rep structs */
struct devlink *dl;
+ struct devlink_port dl_port;
enum devlink_eswitch_mode eswitch_mode;
struct bnxt_vf_rep **vf_reps; /* array of vf-rep ptrs */
u16 *cfa_code_map; /* cfa_code -> vf_idx map */
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 140dbd6..a2930d5 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -26,6 +26,10 @@ enum bnxt_dl_param_id {
BNXT_DEVLINK_PARAM_ID_GRE_VER_CHECK,
};
+enum bnxt_dl_port_param_id {
+ BNXT_DEVLINK_PORT_PARAM_ID_BASE = DEVLINK_PORT_PARAM_GENERIC_ID_MAX,
+};
+
static const struct bnxt_dl_nvm_param nvm_params[] = {
{DEVLINK_PARAM_GENERIC_ID_ENABLE_SRIOV, NVM_OFF_ENABLE_SRIOV,
BNXT_NVM_SHARED_CFG, 1},
@@ -37,6 +41,8 @@ enum bnxt_dl_param_id {
NVM_OFF_MSIX_VEC_PER_PF_MIN, BNXT_NVM_SHARED_CFG, 7},
{BNXT_DEVLINK_PARAM_ID_GRE_VER_CHECK, NVM_OFF_DIS_GRE_VER_CHECK,
BNXT_NVM_SHARED_CFG, 1},
+
+ {DEVLINK_PORT_PARAM_GENERIC_ID_WOL, NVM_OFF_WOL, BNXT_NVM_PORT_CFG, 1},
};
static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
@@ -188,6 +194,12 @@ static int bnxt_dl_msix_validate(struct devlink *dl, u32
id,
NULL),
};
+static const struct devlink_param bnxt_dl_port_params[] = {
+ DEVLINK_PORT_PARAM_GENERIC(WOL, BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+ bnxt_dl_nvm_param_get, bnxt_dl_nvm_param_set,
+ NULL),
+};
+
int bnxt_dl_register(struct bnxt *bp)
{
struct devlink *dl;
@@ -225,8 +237,28 @@ int bnxt_dl_register(struct bnxt *bp)
goto err_dl_unreg;
}
+ rc = devlink_port_register(dl, >dl_port, bp->pf.port_id);
+ if (rc) {
+ netdev_warn(bp->dev, "devlink_port_register failed. rc=%d",
+ rc);
+ goto err_dl_param_unreg;
+ }
+ bp->dl_port.type = DEVLINK_PORT_TYPE_ETH;
+
+ rc = devlink_port_params_register(>dl_port, bnxt_dl_port_params,
+ ARRAY_SIZE(bnxt_dl_port_params));
+ if (rc) {
+ netdev_warn(bp->dev, "devlink_port_params_register failed.
rc=%d",
+ rc);
+ goto err_dl_port_unreg;
+ }
return 0;
+err_dl_port_unreg:
+ devlink_port_unregister(>dl_port);
+err_dl_param_unreg:
+ devlink_params_unregister(dl, bnxt_dl_params,
+ ARRAY_SIZE(bnxt_dl_params));
err_dl_unreg:
devlink_unregister(dl);
err_dl_free:
@@ -242,6 +274,9 @@ void bnxt_dl_unregister(struct bnxt *bp)
if (!dl)
return;
+ devlink_port_params_unregister(>dl_port, bnxt_dl_port_params,
+ ARRAY_SIZE(bnxt_dl_port_params));
+ devlink_port_unregister(>dl_port);
devlink_params_unregister(dl, bnxt_dl_params,
ARRAY_SIZE(bnxt_dl_params));
devlink_unregister(dl);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
index 5b6b2c7..da065ca 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
@@ -35,6 +35,7 @@ static inline void bnxt_link_bp_to_dl(struct bnxt *bp, struct
devlink *dl)
#define NVM_OFF_MSIX_VEC_PER_PF_MAX108
#define NVM_OFF_MSIX_VEC_PER_PF_MIN114
+#define NVM_OFF_WOL152
#define NVM_OFF_IGNORE_ARI 164
#define NVM_OFF_DIS_GRE_VER_CHECK 171
#define NVM_OFF_ENABLE_SRIOV 401
--
1.8.3.1
