| Message ID | 20220830101237.22782-1-gal@nvidia.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: ieee802154: Fix compilation error when CONFIG_IEEE802154_NL802154_EXPERIMENTAL is disabled | expand |
On Tue, Aug 30, 2022 at 01:12:37PM +0300, Gal Pressman wrote: > When CONFIG_IEEE802154_NL802154_EXPERIMENTAL is disabled, > NL802154_CMD_DEL_SEC_LEVEL is undefined and results in a compilation > error: > net/ieee802154/nl802154.c:2503:19: error: 'NL802154_CMD_DEL_SEC_LEVEL' undeclared here (not in a function); did you mean 'NL802154_CMD_SET_CCA_ED_LEVEL'? > 2503 | .resv_start_op = NL802154_CMD_DEL_SEC_LEVEL + 1, > | ^~~~~~~~~~~~~~~~~~~~~~~~~~ > | NL802154_CMD_SET_CCA_ED_LEVEL > > Use __NL802154_CMD_AFTER_LAST instead of > 'NL802154_CMD_DEL_SEC_LEVEL + 1' to indicate the last command. > > Fixes: 9c5d03d36251 ("genetlink: start to validate reserved header bytes") > Reviewed-by: Leon Romanovsky <leonro@nvidia.com> > Signed-off-by: Gal Pressman <gal@nvidia.com> Tested-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
On Tue, 30 Aug 2022 13:12:37 +0300 Gal Pressman wrote: > When CONFIG_IEEE802154_NL802154_EXPERIMENTAL is disabled, > NL802154_CMD_DEL_SEC_LEVEL is undefined and results in a compilation > error: > net/ieee802154/nl802154.c:2503:19: error: 'NL802154_CMD_DEL_SEC_LEVEL' undeclared here (not in a function); did you mean 'NL802154_CMD_SET_CCA_ED_LEVEL'? > 2503 | .resv_start_op = NL802154_CMD_DEL_SEC_LEVEL + 1, > | ^~~~~~~~~~~~~~~~~~~~~~~~~~ > | NL802154_CMD_SET_CCA_ED_LEVEL > > Use __NL802154_CMD_AFTER_LAST instead of > 'NL802154_CMD_DEL_SEC_LEVEL + 1' to indicate the last command. > > Fixes: 9c5d03d36251 ("genetlink: start to validate reserved header bytes") > Reviewed-by: Leon Romanovsky <leonro@nvidia.com> > Signed-off-by: Gal Pressman <gal@nvidia.com> > --- > net/ieee802154/nl802154.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c > index 38c4f3cb010e..dbfd24c70bd0 100644 > --- a/net/ieee802154/nl802154.c > +++ b/net/ieee802154/nl802154.c > @@ -2500,7 +2500,7 @@ static struct genl_family nl802154_fam __ro_after_init = { > .module = THIS_MODULE, > .ops = nl802154_ops, > .n_ops = ARRAY_SIZE(nl802154_ops), > - .resv_start_op = NL802154_CMD_DEL_SEC_LEVEL + 1, > + .resv_start_op = __NL802154_CMD_AFTER_LAST, > .mcgrps = nl802154_mcgrps, > .n_mcgrps = ARRAY_SIZE(nl802154_mcgrps), > }; Thanks for the fix! I think we should switch to NL802154_CMD_SET_WPAN_PHY_NETNS + 1, tho. The point is to set the value to the cmd number after _currently_ last defined command. The meta-value like LAST will move next time someone adds a command, meaning the validation for new commands will never actually come.
On 31/08/2022 09:13, Jakub Kicinski wrote: > On Tue, 30 Aug 2022 13:12:37 +0300 Gal Pressman wrote: >> When CONFIG_IEEE802154_NL802154_EXPERIMENTAL is disabled, >> NL802154_CMD_DEL_SEC_LEVEL is undefined and results in a compilation >> error: >> net/ieee802154/nl802154.c:2503:19: error: 'NL802154_CMD_DEL_SEC_LEVEL' undeclared here (not in a function); did you mean 'NL802154_CMD_SET_CCA_ED_LEVEL'? >> 2503 | .resv_start_op = NL802154_CMD_DEL_SEC_LEVEL + 1, >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~ >> | NL802154_CMD_SET_CCA_ED_LEVEL >> >> Use __NL802154_CMD_AFTER_LAST instead of >> 'NL802154_CMD_DEL_SEC_LEVEL + 1' to indicate the last command. >> >> Fixes: 9c5d03d36251 ("genetlink: start to validate reserved header bytes") >> Reviewed-by: Leon Romanovsky <leonro@nvidia.com> >> Signed-off-by: Gal Pressman <gal@nvidia.com> >> --- >> net/ieee802154/nl802154.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c >> index 38c4f3cb010e..dbfd24c70bd0 100644 >> --- a/net/ieee802154/nl802154.c >> +++ b/net/ieee802154/nl802154.c >> @@ -2500,7 +2500,7 @@ static struct genl_family nl802154_fam __ro_after_init = { >> .module = THIS_MODULE, >> .ops = nl802154_ops, >> .n_ops = ARRAY_SIZE(nl802154_ops), >> - .resv_start_op = NL802154_CMD_DEL_SEC_LEVEL + 1, >> + .resv_start_op = __NL802154_CMD_AFTER_LAST, >> .mcgrps = nl802154_mcgrps, >> .n_mcgrps = ARRAY_SIZE(nl802154_mcgrps), >> }; > Thanks for the fix! I think we should switch to > NL802154_CMD_SET_WPAN_PHY_NETNS + 1, tho. > > The point is to set the value to the cmd number after _currently_ > last defined command. The meta-value like LAST will move next time > someone adds a command, meaning the validation for new commands will > never actually come. I see, missed that part. So, shouldn't it be: #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL .resv_start_op = NL802154_CMD_DEL_SEC_LEVEL + 1, #else .resv_start_op = NL802154_CMD_SET_WPAN_PHY_NETNS + 1, #endif ?
On Wed, Aug 31, 2022 at 09:20:39AM +0300, Gal Pressman wrote: > On 31/08/2022 09:13, Jakub Kicinski wrote: > > On Tue, 30 Aug 2022 13:12:37 +0300 Gal Pressman wrote: > >> When CONFIG_IEEE802154_NL802154_EXPERIMENTAL is disabled, > >> NL802154_CMD_DEL_SEC_LEVEL is undefined and results in a compilation > >> error: > >> net/ieee802154/nl802154.c:2503:19: error: 'NL802154_CMD_DEL_SEC_LEVEL' undeclared here (not in a function); did you mean 'NL802154_CMD_SET_CCA_ED_LEVEL'? > >> 2503 | .resv_start_op = NL802154_CMD_DEL_SEC_LEVEL + 1, > >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~ > >> | NL802154_CMD_SET_CCA_ED_LEVEL > >> > >> Use __NL802154_CMD_AFTER_LAST instead of > >> 'NL802154_CMD_DEL_SEC_LEVEL + 1' to indicate the last command. > >> > >> Fixes: 9c5d03d36251 ("genetlink: start to validate reserved header bytes") > >> Reviewed-by: Leon Romanovsky <leonro@nvidia.com> > >> Signed-off-by: Gal Pressman <gal@nvidia.com> > >> --- > >> net/ieee802154/nl802154.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c > >> index 38c4f3cb010e..dbfd24c70bd0 100644 > >> --- a/net/ieee802154/nl802154.c > >> +++ b/net/ieee802154/nl802154.c > >> @@ -2500,7 +2500,7 @@ static struct genl_family nl802154_fam __ro_after_init = { > >> .module = THIS_MODULE, > >> .ops = nl802154_ops, > >> .n_ops = ARRAY_SIZE(nl802154_ops), > >> - .resv_start_op = NL802154_CMD_DEL_SEC_LEVEL + 1, > >> + .resv_start_op = __NL802154_CMD_AFTER_LAST, > >> .mcgrps = nl802154_mcgrps, > >> .n_mcgrps = ARRAY_SIZE(nl802154_mcgrps), > >> }; > > Thanks for the fix! I think we should switch to > > NL802154_CMD_SET_WPAN_PHY_NETNS + 1, tho. > > > > The point is to set the value to the cmd number after _currently_ > > last defined command. The meta-value like LAST will move next time > > someone adds a command, meaning the validation for new commands will > > never actually come. > > I see, missed that part. > > So, shouldn't it be: > #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL > .resv_start_op = NL802154_CMD_DEL_SEC_LEVEL + 1, > #else > .resv_start_op = NL802154_CMD_SET_WPAN_PHY_NETNS + 1, > #endif +1, I wanted to propose the same snippet. Thanks > > ?
On Wed, 31 Aug 2022 09:20:39 +0300 Gal Pressman wrote: > So, shouldn't it be: > #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL > .resv_start_op = NL802154_CMD_DEL_SEC_LEVEL + 1, > #else > .resv_start_op = NL802154_CMD_SET_WPAN_PHY_NETNS + 1, > #endif Hm, let me add 802154 folks. Either we should treat the commands as reserved in terms of uAPI even if they get removed the IDs won't be reused, or they are for testing purposes only. In the former case we should just remove the #ifdef around the values in the enum, it just leads to #ifdef proliferation while having no functional impact. In the latter case we should start error checking from the last non-experimental command, as we don't care about breaking the experimental ones.
On Tue, 30 Aug 2022 23:31:24 -0700 Jakub Kicinski wrote: > Hm, let me add 802154 folks. > > Either we should treat the commands as reserved in terms of uAPI > even if they get removed the IDs won't be reused, or they are for > testing purposes only. > > In the former case we should just remove the #ifdef around the values > in the enum, it just leads to #ifdef proliferation while having no > functional impact. > > In the latter case we should start error checking from the last > non-experimental command, as we don't care about breaking the > experimental ones. I haven't gone thru all of my inbox yet, but I see no reply from Stefan or Alexander. My vote is to un-hide the EXPERIMENTAL commands.
Hi, On Wed, Aug 31, 2022 at 2:26 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Tue, 30 Aug 2022 23:31:24 -0700 Jakub Kicinski wrote: > > Hm, let me add 802154 folks. > > > > Either we should treat the commands as reserved in terms of uAPI > > even if they get removed the IDs won't be reused, or they are for > > testing purposes only. > > > > In the former case we should just remove the #ifdef around the values > > in the enum, it just leads to #ifdef proliferation while having no > > functional impact. > > > > In the latter case we should start error checking from the last > > non-experimental command, as we don't care about breaking the > > experimental ones. > > I haven't gone thru all of my inbox yet, but I see no reply from Stefan > or Alexander. My vote is to un-hide the EXPERIMENTAL commands. > fine for me if it's still in nl802154 and ends in error if somebody tries to use it and it's not compiled. But users should still consider it's not a stable API yet and we don't care about breaking it for now... - Alex
Hello Jakub. On 31.08.22 20:21, Jakub Kicinski wrote: > On Tue, 30 Aug 2022 23:31:24 -0700 Jakub Kicinski wrote: >> Hm, let me add 802154 folks. >> >> Either we should treat the commands as reserved in terms of uAPI >> even if they get removed the IDs won't be reused, or they are for >> testing purposes only. >> >> In the former case we should just remove the #ifdef around the values >> in the enum, it just leads to #ifdef proliferation while having no >> functional impact. >> >> In the latter case we should start error checking from the last >> non-experimental command, as we don't care about breaking the >> experimental ones. > > I haven't gone thru all of my inbox yet, but I see no reply from Stefan > or Alexander. My vote is to un-hide the EXPERIMENTAL commands. I was swamped today and I am only now finding time to go through mail. Given the problem these ifdef are raising I am ok with having these commands exposed without them. Our main reason for having this feature marked as experimental is that it does not have much exposure and we fear that some of it needs rewrites. If that really is going to happen we will simply treat the current commands as reserved/burned and come up with other ones if needed. While I hope this will not be needed it is a fair plan for mitigating this. regards Stefan Schmidt
On Wed, 31 Aug 2022 22:59:14 +0200 Stefan Schmidt wrote: > I was swamped today and I am only now finding time to go through mail. > > Given the problem these ifdef are raising I am ok with having these > commands exposed without them. > > Our main reason for having this feature marked as experimental is that > it does not have much exposure and we fear that some of it needs rewrites. > > If that really is going to happen we will simply treat the current > commands as reserved/burned and come up with other ones if needed. While > I hope this will not be needed it is a fair plan for mitigating this. Thanks for the replies. I keep going back and forth in my head on what's better - un-hiding or just using NL802154_CMD_SET_WPAN_PHY_NETNS + 1 as the start of validation, since it's okay to break experimental commands. Any preference?
Hello Jakub. On 31.08.22 23:09, Jakub Kicinski wrote: > On Wed, 31 Aug 2022 22:59:14 +0200 Stefan Schmidt wrote: >> I was swamped today and I am only now finding time to go through mail. >> >> Given the problem these ifdef are raising I am ok with having these >> commands exposed without them. >> >> Our main reason for having this feature marked as experimental is that >> it does not have much exposure and we fear that some of it needs rewrites. >> >> If that really is going to happen we will simply treat the current >> commands as reserved/burned and come up with other ones if needed. While >> I hope this will not be needed it is a fair plan for mitigating this. > > Thanks for the replies. I keep going back and forth in my head on > what's better - un-hiding or just using NL802154_CMD_SET_WPAN_PHY_NETNS + 1 > as the start of validation, since it's okay to break experimental commands. > > Any preference? We saw other problems with these being behind ifdefs from build and fuzzing bots. I say its time we un-hide and deal with them being formerly deprecated and replaced by something else if it really comes to changes for them (which we are not sure of) regards Stefan Schmidt
On Wed, Aug 31, 2022 at 02:09:47PM -0700, Jakub Kicinski wrote: > On Wed, 31 Aug 2022 22:59:14 +0200 Stefan Schmidt wrote: > > I was swamped today and I am only now finding time to go through mail. > > > > Given the problem these ifdef are raising I am ok with having these > > commands exposed without them. > > > > Our main reason for having this feature marked as experimental is that > > it does not have much exposure and we fear that some of it needs rewrites. > > > > If that really is going to happen we will simply treat the current > > commands as reserved/burned and come up with other ones if needed. While > > I hope this will not be needed it is a fair plan for mitigating this. > > Thanks for the replies. I keep going back and forth in my head on > what's better - un-hiding or just using NL802154_CMD_SET_WPAN_PHY_NETNS + 1 > as the start of validation, since it's okay to break experimental commands. > > Any preference? Jakub, There is no such thing like experimental UAPI. Once you put something in UAPI headers and/or allowed users to issue calls from userspace to kernel, they can use it. We don't control how users compile their kernels. So it is not break "experimental commands", but break commands that maybe shouldn't exist in first place. nl802154 code suffers from two basic mistakes: 1. User visible defines are not part of UAPI headers. For example, include/net/nl802154.h should be in include/uapi/net/.... 2. Used Kconfig option for pseudo-UAPI header. In this specific case, I checked that Fedora didn't enable this CONFIG_IEEE802154_NL802154_EXPERIMENTAL knob, but someone needs to check debian and other distros too. Most likely it is not used at all. https://src.fedoraproject.org/rpms/kernel/tree/rawhide Thanks
Hi, On Thu, Sep 1, 2022 at 2:38 AM Leon Romanovsky <leon@kernel.org> wrote: > > On Wed, Aug 31, 2022 at 02:09:47PM -0700, Jakub Kicinski wrote: > > On Wed, 31 Aug 2022 22:59:14 +0200 Stefan Schmidt wrote: > > > I was swamped today and I am only now finding time to go through mail. > > > > > > Given the problem these ifdef are raising I am ok with having these > > > commands exposed without them. > > > > > > Our main reason for having this feature marked as experimental is that > > > it does not have much exposure and we fear that some of it needs rewrites. > > > > > > If that really is going to happen we will simply treat the current > > > commands as reserved/burned and come up with other ones if needed. While > > > I hope this will not be needed it is a fair plan for mitigating this. > > > > Thanks for the replies. I keep going back and forth in my head on > > what's better - un-hiding or just using NL802154_CMD_SET_WPAN_PHY_NETNS + 1 > > as the start of validation, since it's okay to break experimental commands. > > > > Any preference? > > Jakub, > > There is no such thing like experimental UAPI. Once you put something > in UAPI headers and/or allowed users to issue calls from userspace > to kernel, they can use it. We don't control how users compile their > kernels. > > So it is not break "experimental commands", but break commands that > maybe shouldn't exist in first place. > > nl802154 code suffers from two basic mistakes: > 1. User visible defines are not part of UAPI headers. For example, > include/net/nl802154.h should be in include/uapi/net/.... yes, but no because then this will end in breaking UAPI because it will be exported to uapi headers if we change them? For now we say everybody needs to copy the header on their own into their user space application if they want to use the API which means it fits for the kernel that they copied from. > 2. Used Kconfig option for pseudo-UAPI header. > > In this specific case, I checked that Fedora didn't enable this > CONFIG_IEEE802154_NL802154_EXPERIMENTAL knob, but someone needs > to check debian and other distros too. > I would remove the CONFIG_IEEE802154_NL802154_EXPERIMENTAL config option but don't move the header to "include/uapi/..." which means that the whole nl802154 UAPI (and some others UAPIs) are still experimental because it's not part of UAPI "directory". btw: the whole subsystem is still experimental because f4671a90c418 ("net/ieee802154: remove depends on CONFIG_EXPERIMENTAL") was never acked by any maintainer... but indeed has other reasons why it got removed. - Alex
On Thu, Sep 01, 2022 at 08:50:16AM -0400, Alexander Aring wrote: > Hi, > > On Thu, Sep 1, 2022 at 2:38 AM Leon Romanovsky <leon@kernel.org> wrote: > > > > On Wed, Aug 31, 2022 at 02:09:47PM -0700, Jakub Kicinski wrote: > > > On Wed, 31 Aug 2022 22:59:14 +0200 Stefan Schmidt wrote: > > > > I was swamped today and I am only now finding time to go through mail. > > > > > > > > Given the problem these ifdef are raising I am ok with having these > > > > commands exposed without them. > > > > > > > > Our main reason for having this feature marked as experimental is that > > > > it does not have much exposure and we fear that some of it needs rewrites. > > > > > > > > If that really is going to happen we will simply treat the current > > > > commands as reserved/burned and come up with other ones if needed. While > > > > I hope this will not be needed it is a fair plan for mitigating this. > > > > > > Thanks for the replies. I keep going back and forth in my head on > > > what's better - un-hiding or just using NL802154_CMD_SET_WPAN_PHY_NETNS + 1 > > > as the start of validation, since it's okay to break experimental commands. > > > > > > Any preference? > > > > Jakub, > > > > There is no such thing like experimental UAPI. Once you put something > > in UAPI headers and/or allowed users to issue calls from userspace > > to kernel, they can use it. We don't control how users compile their > > kernels. > > > > So it is not break "experimental commands", but break commands that > > maybe shouldn't exist in first place. > > > > nl802154 code suffers from two basic mistakes: > > 1. User visible defines are not part of UAPI headers. For example, > > include/net/nl802154.h should be in include/uapi/net/.... > > yes, but no because then this will end in breaking UAPI because it > will be exported to uapi headers if we change them? > For now we say everybody needs to copy the header on their own into > their user space application if they want to use the API which means > it fits for the kernel that they copied from. It is not how UAPI works. Once you allowed me to send ID number XXX to the kernel without any header file. I can use it directly, so "hiding" files from users make their development harder, but not impossible. Basically, this is how vendoring and fuzzing works. > > > 2. Used Kconfig option for pseudo-UAPI header. > > > > In this specific case, I checked that Fedora didn't enable this > > CONFIG_IEEE802154_NL802154_EXPERIMENTAL knob, but someone needs > > to check debian and other distros too. > > > > I would remove the CONFIG_IEEE802154_NL802154_EXPERIMENTAL config > option but don't move the header to "include/uapi/..." which means > that the whole nl802154 UAPI (and some others UAPIs) are still > experimental because it's not part of UAPI "directory". > btw: the whole subsystem is still experimental because f4671a90c418 > ("net/ieee802154: remove depends on CONFIG_EXPERIMENTAL") was never > acked by any maintainer... but indeed has other reasons why it got > removed. I don't know anything about NL802154, just trying to explain that UAPI rules are both relevant to binary and compilation compatibility. In your case, concept of CONFIG_IEEE802154_NL802154_EXPERIMENTAL breaks binary compatibility. Thanks > > - Alex >
On Thu, 1 Sep 2022 09:38:35 +0300 Leon Romanovsky wrote: > There is no such thing like experimental UAPI. Once you put something > in UAPI headers and/or allowed users to issue calls from userspace > to kernel, they can use it. We don't control how users compile their > kernels. > > So it is not break "experimental commands", but break commands that > maybe shouldn't exist in first place. > > nl802154 code suffers from two basic mistakes: > 1. User visible defines are not part of UAPI headers. For example, > include/net/nl802154.h should be in include/uapi/net/.... > 2. Used Kconfig option for pseudo-UAPI header. > > In this specific case, I checked that Fedora didn't enable this > CONFIG_IEEE802154_NL802154_EXPERIMENTAL knob, but someone needs > to check debian and other distros too. > > Most likely it is not used at all. You're right, FWIW. I didn't want to get sidetracked into that before we fix the immediate build issue. It's not the only family playing uAPI games :(
Hi, On Thu, Sep 1, 2022 at 4:23 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Thu, 1 Sep 2022 09:38:35 +0300 Leon Romanovsky wrote: > > There is no such thing like experimental UAPI. Once you put something > > in UAPI headers and/or allowed users to issue calls from userspace > > to kernel, they can use it. We don't control how users compile their > > kernels. > > > > So it is not break "experimental commands", but break commands that > > maybe shouldn't exist in first place. > > > > nl802154 code suffers from two basic mistakes: > > 1. User visible defines are not part of UAPI headers. For example, > > include/net/nl802154.h should be in include/uapi/net/.... > > 2. Used Kconfig option for pseudo-UAPI header. > > > > In this specific case, I checked that Fedora didn't enable this > > CONFIG_IEEE802154_NL802154_EXPERIMENTAL knob, but someone needs > > to check debian and other distros too. > > > > Most likely it is not used at all. > > You're right, FWIW. I didn't want to get sidetracked into that before > we fix the immediate build issue. It's not the only family playing uAPI > games :( > I am not sure how to proceed here now, if removing the CONFIG_IEEE802154_NL802154_EXPERIMENTAL option is the way to go. Then do it? It was a mistake to introduce that whole thing and a probably better way is to change nl802154 is to mark it deprecated, after a while rename the enum value to some reserved value and remove the associated code. Then after some time it can be reused again? If this sounds like a better idea how to handle the use case we have here? I am sorry that this config currently causes such a big problem here. - Alex
On Thu, 1 Sep 2022 22:48:10 -0400 Alexander Aring wrote: > > You're right, FWIW. I didn't want to get sidetracked into that before > > we fix the immediate build issue. It's not the only family playing uAPI > > games :( > > I am not sure how to proceed here now, if removing the > CONFIG_IEEE802154_NL802154_EXPERIMENTAL option is the way to go. Then > do it? Right, I was kinda expecting v2 from Gal but the weekend may have started for him already, so let me send it myself.
On Thu, Sep 01, 2022 at 08:00:12PM -0700, Jakub Kicinski wrote: > On Thu, 1 Sep 2022 22:48:10 -0400 Alexander Aring wrote: > > > You're right, FWIW. I didn't want to get sidetracked into that before > > > we fix the immediate build issue. It's not the only family playing uAPI > > > games :( > > > > I am not sure how to proceed here now, if removing the > > CONFIG_IEEE802154_NL802154_EXPERIMENTAL option is the way to go. Then > > do it? > > Right, I was kinda expecting v2 from Gal but the weekend may have > started for him already, so let me send it myself. We didn't know how v2 should look like and waited for some sort of resolution. Thanks
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c index 38c4f3cb010e..dbfd24c70bd0 100644 --- a/net/ieee802154/nl802154.c +++ b/net/ieee802154/nl802154.c @@ -2500,7 +2500,7 @@ static struct genl_family nl802154_fam __ro_after_init = { .module = THIS_MODULE, .ops = nl802154_ops, .n_ops = ARRAY_SIZE(nl802154_ops), - .resv_start_op = NL802154_CMD_DEL_SEC_LEVEL + 1, + .resv_start_op = __NL802154_CMD_AFTER_LAST, .mcgrps = nl802154_mcgrps, .n_mcgrps = ARRAY_SIZE(nl802154_mcgrps), };
When CONFIG_IEEE802154_NL802154_EXPERIMENTAL is disabled, NL802154_CMD_DEL_SEC_LEVEL is undefined and results in a compilation error: net/ieee802154/nl802154.c:2503:19: error: 'NL802154_CMD_DEL_SEC_LEVEL' undeclared here (not in a function); did you mean 'NL802154_CMD_SET_CCA_ED_LEVEL'? 2503 | .resv_start_op = NL802154_CMD_DEL_SEC_LEVEL + 1, | ^~~~~~~~~~~~~~~~~~~~~~~~~~ | NL802154_CMD_SET_CCA_ED_LEVEL Use __NL802154_CMD_AFTER_LAST instead of 'NL802154_CMD_DEL_SEC_LEVEL + 1' to indicate the last command. Fixes: 9c5d03d36251 ("genetlink: start to validate reserved header bytes") Reviewed-by: Leon Romanovsky <leonro@nvidia.com> Signed-off-by: Gal Pressman <gal@nvidia.com> --- net/ieee802154/nl802154.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)