Message ID | 169767396671.6692.9945461089943525792.stgit@anambiarhost.jf.intel.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) | expand |
Hi Amritha, kernel test robot noticed the following build warnings: [auto build test WARNING on net-next/main] url: https://github.com/intel-lab-lkp/linux/commits/Amritha-Nambiar/netdev-genl-spec-Extend-netdev-netlink-spec-in-YAML-for-queue/20231019-082941 base: net-next/main patch link: https://lore.kernel.org/r/169767396671.6692.9945461089943525792.stgit%40anambiarhost.jf.intel.com patch subject: [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue reproduce: (https://download.01.org/0day-ci/archive/20231019/202310190900.9Dzgkbev-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202310190900.9Dzgkbev-lkp@intel.com/ # many are suggestions rather than must-fix WARNING:SPACING: space prohibited between function name and open parenthesis '(' #547: FILE: tools/net/ynl/generated/netdev-user.h:181: + struct netdev_queue_get_rsp obj __attribute__ ((aligned (8)));
On 10/18/2023 7:12 PM, kernel test robot wrote: > Hi Amritha, > > kernel test robot noticed the following build warnings: > > [auto build test WARNING on net-next/main] > > url: https://github.com/intel-lab-lkp/linux/commits/Amritha-Nambiar/netdev-genl-spec-Extend-netdev-netlink-spec-in-YAML-for-queue/20231019-082941 > base: net-next/main > patch link: https://lore.kernel.org/r/169767396671.6692.9945461089943525792.stgit%40anambiarhost.jf.intel.com > patch subject: [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue > reproduce: (https://download.01.org/0day-ci/archive/20231019/202310190900.9Dzgkbev-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202310190900.9Dzgkbev-lkp@intel.com/ > > # many are suggestions rather than must-fix > > WARNING:SPACING: space prohibited between function name and open parenthesis '(' > #547: FILE: tools/net/ynl/generated/netdev-user.h:181: > + struct netdev_queue_get_rsp obj __attribute__ ((aligned (8))); > Looks like the series is in "Changes Requested" state following these warnings. Is there any recommendation for warnings on generated code? I see similar issues on existing code in the generated file.
On Fri, 20 Oct 2023 13:26:38 -0700 Nambiar, Amritha wrote: > > WARNING:SPACING: space prohibited between function name and open parenthesis '(' > > #547: FILE: tools/net/ynl/generated/netdev-user.h:181: > > + struct netdev_queue_get_rsp obj __attribute__ ((aligned (8))); > > Looks like the series is in "Changes Requested" state following these > warnings. Is there any recommendation for warnings on generated code? I > see similar issues on existing code in the generated file. Yes, ignore this one. I'll post a change to the codegen. The warning on patch 3 is legit, right? kernel test bot folks, please be careful with the checkpatch warnings. Some of them are bogus. TBH I'm not sure how much value running checkpatch in the bot adds. It's really trivial to run for the maintainers when applying patches.
On 10/20/2023 3:05 PM, Jakub Kicinski wrote: > On Fri, 20 Oct 2023 13:26:38 -0700 Nambiar, Amritha wrote: >>> WARNING:SPACING: space prohibited between function name and open parenthesis '(' >>> #547: FILE: tools/net/ynl/generated/netdev-user.h:181: >>> + struct netdev_queue_get_rsp obj __attribute__ ((aligned (8))); >> >> Looks like the series is in "Changes Requested" state following these >> warnings. Is there any recommendation for warnings on generated code? I >> see similar issues on existing code in the generated file. > > Yes, ignore this one. I'll post a change to the codegen. > The warning on patch 3 is legit, right? > Thanks for the fix to codegen. Yes, the warning on patch 3 is legit. I'll fix it up in the next version together with addressing other review feedback on this v5 series. > kernel test bot folks, please be careful with the checkpatch warnings. > Some of them are bogus. TBH I'm not sure how much value running > checkpatch in the bot adds. It's really trivial to run for the > maintainers when applying patches. >
On Fri, Oct 20, 2023 at 03:05:57PM -0700, Jakub Kicinski wrote: > On Fri, 20 Oct 2023 13:26:38 -0700 Nambiar, Amritha wrote: > > > WARNING:SPACING: space prohibited between function name and open parenthesis '(' > > > #547: FILE: tools/net/ynl/generated/netdev-user.h:181: > > > + struct netdev_queue_get_rsp obj __attribute__ ((aligned (8))); > > > > Looks like the series is in "Changes Requested" state following these > > warnings. Is there any recommendation for warnings on generated code? I > > see similar issues on existing code in the generated file. > > Yes, ignore this one. I'll post a change to the codegen. > The warning on patch 3 is legit, right? > > kernel test bot folks, please be careful with the checkpatch warnings. Thanks Jakub, we will be very careful and continuously monitor the reports and feedbacks to filter unnecessary warnings/errors from checkpatch. > Some of them are bogus. TBH I'm not sure how much value running > checkpatch in the bot adds. It's really trivial to run for the It is found there're quite some checkpatch related fix commits on mainline. Thus the bot wants to extend the coverage and do shift left testing on developer repos and mailing list patches. But as you mentioned above, we will take furture care to the output of checkpatch to be conservative for the reporting. > maintainers when applying patches. >
On Sat, 21 Oct 2023 09:53:03 +0800 Philip Li wrote: > > Some of them are bogus. TBH I'm not sure how much value running > > checkpatch in the bot adds. It's really trivial to run for the > > It is found there're quite some checkpatch related fix commits on > mainline. Those changes are mostly for old code, aren't they? It'd be useful to do some analysis of how long ago the mis-formatted code has been introduced. Because if new code doesn't get fixes there's no point testing new patches.. > Thus the bot wants to extend the coverage and do shift > left testing on developer repos and mailing list patches. I understand and appreciate the effort. I think that false positive has about a 100x the negative effect of a true positive. If more than 1% of checkpatch warnings are ignored, we should *not* report them to the list. Currently in networking we fully trust the build bot and as soon as a patch set gets a reply from you it gets auto-dropped from our review queue. It'd be quite bad if we have to double check the reports. Speaking of false positive rate - we disabled some checks in our own use of checkpatch: https://github.com/kuba-moo/nipa/blob/master/tests/patch/checkpatch/checkpatch.sh#L6-L12 and we still get about 26% false positive rate! (Count by looking at checks that failed and were ignored, because patch was merged anyway). A lot of those may be line length related (we still prefer 80 char limit) but even without that - checkpatch false positives a lot. And the maintainer is not very receptive to improvements for false positives: https://lore.kernel.org/all/20231013172739.1113964-1-kuba@kernel.org/ > But as you mentioned above, we will take furture care to the output > of checkpatch to be conservative for the reporting. FWIW the most issues that "get through" in networking are issues in documentation (warnings for make htmldocs) :(
On 10/23/2023 7:52 AM, Jakub Kicinski wrote: > On Sat, 21 Oct 2023 09:53:03 +0800 Philip Li wrote: >>> Some of them are bogus. TBH I'm not sure how much value running >>> checkpatch in the bot adds. It's really trivial to run for the >> >> It is found there're quite some checkpatch related fix commits on >> mainline. > > Those changes are mostly for old code, aren't they? > It'd be useful to do some analysis of how long ago the mis-formatted > code has been introduced. Because if new code doesn't get fixes > there's no point testing new patches.. > >> Thus the bot wants to extend the coverage and do shift >> left testing on developer repos and mailing list patches. > > I understand and appreciate the effort. > > I think that false positive has about a 100x the negative effect of a > true positive. If more than 1% of checkpatch warnings are ignored, we > should *not* report them to the list. Currently in networking we fully > trust the build bot and as soon as a patch set gets a reply from you it > gets auto-dropped from our review queue. Hi Jakub, Just checking if this series is dropped from the review queue because of the build bot warnings... should I submit a v6 with the single line fix for the warning (legit) on patch-3, or should I wait for more feedback (if there is a chance this v5 series would still be reviewed) and address them all together submitting v6. > It'd be quite bad if we have to double check the reports. > > Speaking of false positive rate - we disabled some checks in our own > use of checkpatch: > https://github.com/kuba-moo/nipa/blob/master/tests/patch/checkpatch/checkpatch.sh#L6-L12 > and we still get about 26% false positive rate! (Count by looking at > checks that failed and were ignored, because patch was merged anyway). > A lot of those may be line length related (we still prefer 80 char > limit) but even without that - checkpatch false positives a lot. > > And the maintainer is not very receptive to improvements for false > positives: > https://lore.kernel.org/all/20231013172739.1113964-1-kuba@kernel.org/ > >> But as you mentioned above, we will take furture care to the output >> of checkpatch to be conservative for the reporting. > > FWIW the most issues that "get through" in networking are issues > in documentation (warnings for make htmldocs) :(
On Mon, 23 Oct 2023 12:09:33 -0700 Nambiar, Amritha wrote: > Just checking if this series is dropped from the review queue because of > the build bot warnings... > should I submit a v6 with the single line fix for the warning (legit) on > patch-3, or > should I wait for more feedback (if there is a chance this v5 series > would still be reviewed) and address them all together submitting v6. Repost please, I almost never review stuff that doesn't build cleanly. I'll have to re-review before applying, anyway.
On Mon, Oct 23, 2023 at 07:52:21AM -0700, Jakub Kicinski wrote: > On Sat, 21 Oct 2023 09:53:03 +0800 Philip Li wrote: > > > Some of them are bogus. TBH I'm not sure how much value running > > > checkpatch in the bot adds. It's really trivial to run for the > > > > It is found there're quite some checkpatch related fix commits on > > mainline. > > Those changes are mostly for old code, aren't they? Got it, we don't have the statistics data for this yet. I think this is helpful for us to understand the status in depth. We will think of this to gather some data. > It'd be useful to do some analysis of how long ago the mis-formatted > code has been introduced. Because if new code doesn't get fixes > there's no point testing new patches.. > > > Thus the bot wants to extend the coverage and do shift > > left testing on developer repos and mailing list patches. > > I understand and appreciate the effort. > > I think that false positive has about a 100x the negative effect of a > true positive. If more than 1% of checkpatch warnings are ignored, we > should *not* report them to the list. Currently in networking we fully > trust the build bot and as soon as a patch set gets a reply from you it > gets auto-dropped from our review queue. Thanks for the trust. Sorry I didn't notice the false checkpatch report leads to trouble. From below info, may i understand networking already runs own checkpatch? Also consider the checkpatch reports from bot still contains quite some false ones, probably we can pause the checkpatch reporting for network side if it doesn't add much value and causes trouble? > It'd be quite bad if we have to double check the reports. Got it, We are aware of keeping the reports in high quality to make it fully useful. Usually for static analysis tool like sparse/smatch/cocci, we maintain both high confidence pattern and low confidence one, which is continously improved per developer feedback and our own analysis. For low confidence one, it need manual check to be sent out. Since we fully enable checkpatch not long ago, it causes various negative reports. Sorry about this. > > Speaking of false positive rate - we disabled some checks in our own > use of checkpatch: > https://github.com/kuba-moo/nipa/blob/master/tests/patch/checkpatch/checkpatch.sh#L6-L12 > and we still get about 26% false positive rate! (Count by looking at > checks that failed and were ignored, because patch was merged anyway). > A lot of those may be line length related (we still prefer 80 char > limit) but even without that - checkpatch false positives a lot. Thanks for the great info, this is very helpful to us to learn from this. We will adopt some practices here and continue improving the reports. > > And the maintainer is not very receptive to improvements for false > positives: > https://lore.kernel.org/all/20231013172739.1113964-1-kuba@kernel.org/ I see. We got this pattern as well, what we do now is to maintain the pattern internally to avoid unnecessary reports (some are extracted below). I'm looking for publishing these patterns later, which may get more inputs to filter out unnecessary reports. == part of low confidence patterns of checkpatch in bot == __func__ should be used instead of gcc specific __FUNCTION__ line over 80 characters LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged Missing commit description - Add an appropriate one please write a help paragraph that fully describes the config symbol Possible repeated word: 'Google' Possible unwrapped commit description \(prefer a maximum 75 chars per line\) > > > But as you mentioned above, we will take furture care to the output > > of checkpatch to be conservative for the reporting. > > FWIW the most issues that "get through" in networking are issues > in documentation (warnings for make htmldocs) :( Do you suggest that warnings for make htmldocs or kernel-doc warning when building with W=1 can be ignored and no need to send them to networking side? >
On Tue, 24 Oct 2023 09:02:46 +0800 Philip Li wrote: > > I understand and appreciate the effort. > > > > I think that false positive has about a 100x the negative effect of a > > true positive. If more than 1% of checkpatch warnings are ignored, we > > should *not* report them to the list. Currently in networking we fully > > trust the build bot and as soon as a patch set gets a reply from you it > > gets auto-dropped from our review queue. > > Thanks for the trust. Sorry I didn't notice the false checkpatch report leads > to trouble. From below info, may i understand networking already runs own > checkpatch? Also consider the checkpatch reports from bot still contains quite > some false ones, probably we can pause the checkpatch reporting for network > side if it doesn't add much value and causes trouble? Yes, correct, we already run checkpatch --strict on all patches. If you have the ability to selectively disable checkpatch for net/ and drivers/net, and/or patches which CC netdev@vger, that'd be great! FWIW we have a simple dashboard reporting which checks in our own local build fail the most: https://netdev.bots.linux.dev/checks.html Not sure if it's of any interest to you, but that's where I got the false positive rate I mentioned previously. > > And the maintainer is not very receptive to improvements for false > > positives: > > https://lore.kernel.org/all/20231013172739.1113964-1-kuba@kernel.org/ > > I see. We got this pattern as well, what we do now is to maintain the pattern > internally to avoid unnecessary reports (some are extracted below). I'm looking > for publishing these patterns later, which may get more inputs to filter out > unnecessary reports. > > == part of low confidence patterns of checkpatch in bot == Interesting! > __func__ should be used instead of gcc specific __FUNCTION__ This one I don't see failing often. > line over 80 characters This one happens a lot, yes. > LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged This is very rare upstream. > Missing commit description - Add an appropriate one Should be rare upstream.. > please write a help paragraph that fully describes the config symbol This check I think is semi-broken in checkpatch. Sometimes it just doesn't recognize the help even if symbol has it. So yes, we see if false-positive as well. > Possible repeated word: 'Google' Yes! :) > Possible unwrapped commit description \(prefer a maximum 75 chars per line\) This one indeed has a lot of false positives. It should check if *majority* of the commit message lines (excluding tags) are too long, not any single line. Because one line can be a crash dump or a commit reference, and be longer for legit reasons.. Every now and then I feel like we should fork checkpatch or start a new tool which would report only high-confidence problems. > > > But as you mentioned above, we will take furture care to the output > > > of checkpatch to be conservative for the reporting. > > > > FWIW the most issues that "get through" in networking are issues > > in documentation (warnings for make htmldocs) :( > > Do you suggest that warnings for make htmldocs or kernel-doc warning when building > with W=1 can be ignored and no need to send them to networking side? No, no, the opposite! Documentation is one part we currently don't test, even tho we should. Do you run make htmldocs as part of kernel build bot? As you allude to - W=1 checks kdoc already, and scripts/kernel-doc can be used to validate headers even more easily. But to validate the ReST files under Documentation/ one has to actually run make htmldocs (or perhaps some other docs target), not just a normal build.
On Mon, Oct 23, 2023 at 06:44:11PM -0700, Jakub Kicinski wrote: > On Tue, 24 Oct 2023 09:02:46 +0800 Philip Li wrote: > > > I understand and appreciate the effort. > > > > > > I think that false positive has about a 100x the negative effect of a > > > true positive. If more than 1% of checkpatch warnings are ignored, we > > > should *not* report them to the list. Currently in networking we fully > > > trust the build bot and as soon as a patch set gets a reply from you it > > > gets auto-dropped from our review queue. > > > > Thanks for the trust. Sorry I didn't notice the false checkpatch report leads > > to trouble. From below info, may i understand networking already runs own > > checkpatch? Also consider the checkpatch reports from bot still contains quite > > some false ones, probably we can pause the checkpatch reporting for network > > side if it doesn't add much value and causes trouble? > > Yes, correct, we already run checkpatch --strict on all patches. > > If you have the ability to selectively disable checkpatch for net/ and > drivers/net, and/or patches which CC netdev@vger, that'd be great! Got it, thanks for the detail info, we will pause the reports for these places. Before that, i will also pause the overall checkpatch check until we have resolved this for networking side. > > > FWIW we have a simple dashboard reporting which checks in our own > local build fail the most: https://netdev.bots.linux.dev/checks.html > Not sure if it's of any interest to you, but that's where I got the > false positive rate I mentioned previously. This is very advanced and clear! Thanks for sharing, let me dig into it to learn from the dashboard. > > > > And the maintainer is not very receptive to improvements for false > > > positives: > > > https://lore.kernel.org/all/20231013172739.1113964-1-kuba@kernel.org/ > > > > I see. We got this pattern as well, what we do now is to maintain the pattern > > internally to avoid unnecessary reports (some are extracted below). I'm looking > > for publishing these patterns later, which may get more inputs to filter out > > unnecessary reports. > > > > == part of low confidence patterns of checkpatch in bot == > > Interesting! > > > __func__ should be used instead of gcc specific __FUNCTION__ > > This one I don't see failing often. > > > line over 80 characters > > This one happens a lot, yes. > > > LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged > > This is very rare upstream. > > > Missing commit description - Add an appropriate one > > Should be rare upstream.. > > > please write a help paragraph that fully describes the config symbol > > This check I think is semi-broken in checkpatch. > Sometimes it just doesn't recognize the help even if symbol has it. > So yes, we see if false-positive as well. > > > Possible repeated word: 'Google' > > Yes! :) > > > Possible unwrapped commit description \(prefer a maximum 75 chars per line\) > > This one indeed has a lot of false positives. It should check if > *majority* of the commit message lines (excluding tags) are too long, > not any single line. Because one line can be a crash dump or a commit > reference, and be longer for legit reasons.. Thanks for all above comments/analysis/experience, which brings a lot insights. > > Every now and then I feel like we should fork checkpatch or start a new > tool which would report only high-confidence problems. :) > > > > > But as you mentioned above, we will take furture care to the output > > > > of checkpatch to be conservative for the reporting. > > > > > > FWIW the most issues that "get through" in networking are issues > > > in documentation (warnings for make htmldocs) :( > > > > Do you suggest that warnings for make htmldocs or kernel-doc warning when building > > with W=1 can be ignored and no need to send them to networking side? > > No, no, the opposite! Documentation is one part we currently don't test, > even tho we should. > > Do you run make htmldocs as part of kernel build bot? As you allude to - yes, the bot runs make htmldocs check as part of various checks such as includecheck, dtcheck, etc. We will continue doing this for networking side. > W=1 checks kdoc already, and scripts/kernel-doc can be used to validate > headers even more easily. But to validate the ReST files under > Documentation/ one has to actually run make htmldocs (or perhaps some > other docs target), not just a normal build. >
diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml index 14511b13f305..56d45995edfa 100644 --- a/Documentation/netlink/specs/netdev.yaml +++ b/Documentation/netlink/specs/netdev.yaml @@ -55,6 +55,10 @@ definitions: name: hash doc: Device is capable of exposing receive packet hash via bpf_xdp_metadata_rx_hash(). + - + name: queue-type + type: enum + entries: [ rx, tx ] attribute-sets: - @@ -87,6 +91,29 @@ attribute-sets: type: u64 enum: xdp-rx-metadata + - + name: queue + attributes: + - + name: queue-id + doc: queue index + type: u32 + - + name: ifindex + doc: netdev ifindex + type: u32 + checks: + min: 1 + - + name: queue-type + doc: queue type as rx, tx + type: u32 + enum: queue-type + - + name: napi-id + doc: napi id + type: u32 + operations: list: - @@ -120,6 +147,27 @@ operations: doc: Notification about device configuration being changed. notify: dev-get mcgrp: mgmt + - + name: queue-get + doc: queue information + attribute-set: queue + do: + request: + attributes: + - ifindex + - queue-type + - queue-id + reply: &queue-get-op + attributes: + - queue-id + - queue-type + - napi-id + - ifindex + dump: + request: + attributes: + - ifindex + reply: *queue-get-op mcast-groups: list: diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h index 2943a151d4f1..a3997efb6786 100644 --- a/include/uapi/linux/netdev.h +++ b/include/uapi/linux/netdev.h @@ -53,6 +53,11 @@ enum netdev_xdp_rx_metadata { NETDEV_XDP_RX_METADATA_MASK = 3, }; +enum netdev_queue_type { + NETDEV_QUEUE_TYPE_RX, + NETDEV_QUEUE_TYPE_TX, +}; + enum { NETDEV_A_DEV_IFINDEX = 1, NETDEV_A_DEV_PAD, @@ -64,11 +69,22 @@ enum { NETDEV_A_DEV_MAX = (__NETDEV_A_DEV_MAX - 1) }; +enum { + NETDEV_A_QUEUE_QUEUE_ID = 1, + NETDEV_A_QUEUE_IFINDEX, + NETDEV_A_QUEUE_QUEUE_TYPE, + NETDEV_A_QUEUE_NAPI_ID, + + __NETDEV_A_QUEUE_MAX, + NETDEV_A_QUEUE_MAX = (__NETDEV_A_QUEUE_MAX - 1) +}; + enum { NETDEV_CMD_DEV_GET = 1, NETDEV_CMD_DEV_ADD_NTF, NETDEV_CMD_DEV_DEL_NTF, NETDEV_CMD_DEV_CHANGE_NTF, + NETDEV_CMD_QUEUE_GET, __NETDEV_CMD_MAX, NETDEV_CMD_MAX = (__NETDEV_CMD_MAX - 1) diff --git a/net/core/netdev-genl-gen.c b/net/core/netdev-genl-gen.c index ea9231378aa6..85d556a051db 100644 --- a/net/core/netdev-genl-gen.c +++ b/net/core/netdev-genl-gen.c @@ -15,6 +15,18 @@ static const struct nla_policy netdev_dev_get_nl_policy[NETDEV_A_DEV_IFINDEX + 1 [NETDEV_A_DEV_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1), }; +/* NETDEV_CMD_QUEUE_GET - do */ +static const struct nla_policy netdev_queue_get_do_nl_policy[NETDEV_A_QUEUE_QUEUE_TYPE + 1] = { + [NETDEV_A_QUEUE_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1), + [NETDEV_A_QUEUE_QUEUE_TYPE] = NLA_POLICY_MAX(NLA_U32, 1), + [NETDEV_A_QUEUE_QUEUE_ID] = { .type = NLA_U32, }, +}; + +/* NETDEV_CMD_QUEUE_GET - dump */ +static const struct nla_policy netdev_queue_get_dump_nl_policy[NETDEV_A_QUEUE_IFINDEX + 1] = { + [NETDEV_A_QUEUE_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1), +}; + /* Ops table for netdev */ static const struct genl_split_ops netdev_nl_ops[] = { { @@ -29,6 +41,20 @@ static const struct genl_split_ops netdev_nl_ops[] = { .dumpit = netdev_nl_dev_get_dumpit, .flags = GENL_CMD_CAP_DUMP, }, + { + .cmd = NETDEV_CMD_QUEUE_GET, + .doit = netdev_nl_queue_get_doit, + .policy = netdev_queue_get_do_nl_policy, + .maxattr = NETDEV_A_QUEUE_QUEUE_TYPE, + .flags = GENL_CMD_CAP_DO, + }, + { + .cmd = NETDEV_CMD_QUEUE_GET, + .dumpit = netdev_nl_queue_get_dumpit, + .policy = netdev_queue_get_dump_nl_policy, + .maxattr = NETDEV_A_QUEUE_IFINDEX, + .flags = GENL_CMD_CAP_DUMP, + }, }; static const struct genl_multicast_group netdev_nl_mcgrps[] = { diff --git a/net/core/netdev-genl-gen.h b/net/core/netdev-genl-gen.h index 7b370c073e7d..263c94f77bad 100644 --- a/net/core/netdev-genl-gen.h +++ b/net/core/netdev-genl-gen.h @@ -13,6 +13,9 @@ int netdev_nl_dev_get_doit(struct sk_buff *skb, struct genl_info *info); int netdev_nl_dev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb); +int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info); +int netdev_nl_queue_get_dumpit(struct sk_buff *skb, + struct netlink_callback *cb); enum { NETDEV_NLGRP_MGMT, diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c index fe61f85bcf33..336c608e6a6b 100644 --- a/net/core/netdev-genl.c +++ b/net/core/netdev-genl.c @@ -129,6 +129,16 @@ int netdev_nl_dev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb) return skb->len; } +int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info) +{ + return -EOPNOTSUPP; +} + +int netdev_nl_queue_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb) +{ + return -EOPNOTSUPP; +} + static int netdev_genl_netdevice_event(struct notifier_block *nb, unsigned long event, void *ptr) { diff --git a/tools/include/uapi/linux/netdev.h b/tools/include/uapi/linux/netdev.h index 2943a151d4f1..a3997efb6786 100644 --- a/tools/include/uapi/linux/netdev.h +++ b/tools/include/uapi/linux/netdev.h @@ -53,6 +53,11 @@ enum netdev_xdp_rx_metadata { NETDEV_XDP_RX_METADATA_MASK = 3, }; +enum netdev_queue_type { + NETDEV_QUEUE_TYPE_RX, + NETDEV_QUEUE_TYPE_TX, +}; + enum { NETDEV_A_DEV_IFINDEX = 1, NETDEV_A_DEV_PAD, @@ -64,11 +69,22 @@ enum { NETDEV_A_DEV_MAX = (__NETDEV_A_DEV_MAX - 1) }; +enum { + NETDEV_A_QUEUE_QUEUE_ID = 1, + NETDEV_A_QUEUE_IFINDEX, + NETDEV_A_QUEUE_QUEUE_TYPE, + NETDEV_A_QUEUE_NAPI_ID, + + __NETDEV_A_QUEUE_MAX, + NETDEV_A_QUEUE_MAX = (__NETDEV_A_QUEUE_MAX - 1) +}; + enum { NETDEV_CMD_DEV_GET = 1, NETDEV_CMD_DEV_ADD_NTF, NETDEV_CMD_DEV_DEL_NTF, NETDEV_CMD_DEV_CHANGE_NTF, + NETDEV_CMD_QUEUE_GET, __NETDEV_CMD_MAX, NETDEV_CMD_MAX = (__NETDEV_CMD_MAX - 1) diff --git a/tools/net/ynl/generated/netdev-user.c b/tools/net/ynl/generated/netdev-user.c index b5ffe8cd1144..9d6f96d80ba1 100644 --- a/tools/net/ynl/generated/netdev-user.c +++ b/tools/net/ynl/generated/netdev-user.c @@ -18,6 +18,7 @@ static const char * const netdev_op_strmap[] = { [NETDEV_CMD_DEV_ADD_NTF] = "dev-add-ntf", [NETDEV_CMD_DEV_DEL_NTF] = "dev-del-ntf", [NETDEV_CMD_DEV_CHANGE_NTF] = "dev-change-ntf", + [NETDEV_CMD_QUEUE_GET] = "queue-get", }; const char *netdev_op_str(int op) @@ -58,6 +59,18 @@ const char *netdev_xdp_rx_metadata_str(enum netdev_xdp_rx_metadata value) return netdev_xdp_rx_metadata_strmap[value]; } +static const char * const netdev_queue_type_strmap[] = { + [0] = "rx", + [1] = "tx", +}; + +const char *netdev_queue_type_str(enum netdev_queue_type value) +{ + if (value < 0 || value >= (int)MNL_ARRAY_SIZE(netdev_queue_type_strmap)) + return NULL; + return netdev_queue_type_strmap[value]; +} + /* Policies */ struct ynl_policy_attr netdev_dev_policy[NETDEV_A_DEV_MAX + 1] = { [NETDEV_A_DEV_IFINDEX] = { .name = "ifindex", .type = YNL_PT_U32, }, @@ -72,6 +85,18 @@ struct ynl_policy_nest netdev_dev_nest = { .table = netdev_dev_policy, }; +struct ynl_policy_attr netdev_queue_policy[NETDEV_A_QUEUE_MAX + 1] = { + [NETDEV_A_QUEUE_QUEUE_ID] = { .name = "queue-id", .type = YNL_PT_U32, }, + [NETDEV_A_QUEUE_IFINDEX] = { .name = "ifindex", .type = YNL_PT_U32, }, + [NETDEV_A_QUEUE_QUEUE_TYPE] = { .name = "queue-type", .type = YNL_PT_U32, }, + [NETDEV_A_QUEUE_NAPI_ID] = { .name = "napi-id", .type = YNL_PT_U32, }, +}; + +struct ynl_policy_nest netdev_queue_nest = { + .max_attr = NETDEV_A_QUEUE_MAX, + .table = netdev_queue_policy, +}; + /* Common nested types */ /* ============== NETDEV_CMD_DEV_GET ============== */ /* NETDEV_CMD_DEV_GET - do */ @@ -197,6 +222,134 @@ void netdev_dev_get_ntf_free(struct netdev_dev_get_ntf *rsp) free(rsp); } +/* ============== NETDEV_CMD_QUEUE_GET ============== */ +/* NETDEV_CMD_QUEUE_GET - do */ +void netdev_queue_get_req_free(struct netdev_queue_get_req *req) +{ + free(req); +} + +void netdev_queue_get_rsp_free(struct netdev_queue_get_rsp *rsp) +{ + free(rsp); +} + +int netdev_queue_get_rsp_parse(const struct nlmsghdr *nlh, void *data) +{ + struct ynl_parse_arg *yarg = data; + struct netdev_queue_get_rsp *dst; + const struct nlattr *attr; + + dst = yarg->data; + + mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) { + unsigned int type = mnl_attr_get_type(attr); + + if (type == NETDEV_A_QUEUE_QUEUE_ID) { + if (ynl_attr_validate(yarg, attr)) + return MNL_CB_ERROR; + dst->_present.queue_id = 1; + dst->queue_id = mnl_attr_get_u32(attr); + } else if (type == NETDEV_A_QUEUE_QUEUE_TYPE) { + if (ynl_attr_validate(yarg, attr)) + return MNL_CB_ERROR; + dst->_present.queue_type = 1; + dst->queue_type = mnl_attr_get_u32(attr); + } else if (type == NETDEV_A_QUEUE_NAPI_ID) { + if (ynl_attr_validate(yarg, attr)) + return MNL_CB_ERROR; + dst->_present.napi_id = 1; + dst->napi_id = mnl_attr_get_u32(attr); + } else if (type == NETDEV_A_QUEUE_IFINDEX) { + if (ynl_attr_validate(yarg, attr)) + return MNL_CB_ERROR; + dst->_present.ifindex = 1; + dst->ifindex = mnl_attr_get_u32(attr); + } + } + + return MNL_CB_OK; +} + +struct netdev_queue_get_rsp * +netdev_queue_get(struct ynl_sock *ys, struct netdev_queue_get_req *req) +{ + struct ynl_req_state yrs = { .yarg = { .ys = ys, }, }; + struct netdev_queue_get_rsp *rsp; + struct nlmsghdr *nlh; + int err; + + nlh = ynl_gemsg_start_req(ys, ys->family_id, NETDEV_CMD_QUEUE_GET, 1); + ys->req_policy = &netdev_queue_nest; + yrs.yarg.rsp_policy = &netdev_queue_nest; + + if (req->_present.ifindex) + mnl_attr_put_u32(nlh, NETDEV_A_QUEUE_IFINDEX, req->ifindex); + if (req->_present.queue_type) + mnl_attr_put_u32(nlh, NETDEV_A_QUEUE_QUEUE_TYPE, req->queue_type); + if (req->_present.queue_id) + mnl_attr_put_u32(nlh, NETDEV_A_QUEUE_QUEUE_ID, req->queue_id); + + rsp = calloc(1, sizeof(*rsp)); + yrs.yarg.data = rsp; + yrs.cb = netdev_queue_get_rsp_parse; + yrs.rsp_cmd = NETDEV_CMD_QUEUE_GET; + + err = ynl_exec(ys, nlh, &yrs); + if (err < 0) + goto err_free; + + return rsp; + +err_free: + netdev_queue_get_rsp_free(rsp); + return NULL; +} + +/* NETDEV_CMD_QUEUE_GET - dump */ +void netdev_queue_get_list_free(struct netdev_queue_get_list *rsp) +{ + struct netdev_queue_get_list *next = rsp; + + while ((void *)next != YNL_LIST_END) { + rsp = next; + next = rsp->next; + + free(rsp); + } +} + +struct netdev_queue_get_list * +netdev_queue_get_dump(struct ynl_sock *ys, + struct netdev_queue_get_req_dump *req) +{ + struct ynl_dump_state yds = {}; + struct nlmsghdr *nlh; + int err; + + yds.ys = ys; + yds.alloc_sz = sizeof(struct netdev_queue_get_list); + yds.cb = netdev_queue_get_rsp_parse; + yds.rsp_cmd = NETDEV_CMD_QUEUE_GET; + yds.rsp_policy = &netdev_queue_nest; + + nlh = ynl_gemsg_start_dump(ys, ys->family_id, NETDEV_CMD_QUEUE_GET, 1); + ys->req_policy = &netdev_queue_nest; + + if (req->_present.ifindex) + mnl_attr_put_u32(nlh, NETDEV_A_QUEUE_IFINDEX, req->ifindex); + + err = ynl_exec_dump(ys, nlh, &yds); + if (err < 0) + goto free_list; + + return yds.first; + +free_list: + netdev_queue_get_list_free(yds.first); + return NULL; +} + static const struct ynl_ntf_info netdev_ntf_info[] = { [NETDEV_CMD_DEV_ADD_NTF] = { .alloc_sz = sizeof(struct netdev_dev_get_ntf), diff --git a/tools/net/ynl/generated/netdev-user.h b/tools/net/ynl/generated/netdev-user.h index b4351ff34595..bb01368a0595 100644 --- a/tools/net/ynl/generated/netdev-user.h +++ b/tools/net/ynl/generated/netdev-user.h @@ -19,6 +19,7 @@ extern const struct ynl_family ynl_netdev_family; const char *netdev_op_str(int op); const char *netdev_xdp_act_str(enum netdev_xdp_act value); const char *netdev_xdp_rx_metadata_str(enum netdev_xdp_rx_metadata value); +const char *netdev_queue_type_str(enum netdev_queue_type value); /* Common nested types */ /* ============== NETDEV_CMD_DEV_GET ============== */ @@ -87,4 +88,103 @@ struct netdev_dev_get_ntf { void netdev_dev_get_ntf_free(struct netdev_dev_get_ntf *rsp); +/* ============== NETDEV_CMD_QUEUE_GET ============== */ +/* NETDEV_CMD_QUEUE_GET - do */ +struct netdev_queue_get_req { + struct { + __u32 ifindex:1; + __u32 queue_type:1; + __u32 queue_id:1; + } _present; + + __u32 ifindex; + enum netdev_queue_type queue_type; + __u32 queue_id; +}; + +static inline struct netdev_queue_get_req *netdev_queue_get_req_alloc(void) +{ + return calloc(1, sizeof(struct netdev_queue_get_req)); +} +void netdev_queue_get_req_free(struct netdev_queue_get_req *req); + +static inline void +netdev_queue_get_req_set_ifindex(struct netdev_queue_get_req *req, + __u32 ifindex) +{ + req->_present.ifindex = 1; + req->ifindex = ifindex; +} +static inline void +netdev_queue_get_req_set_queue_type(struct netdev_queue_get_req *req, + enum netdev_queue_type queue_type) +{ + req->_present.queue_type = 1; + req->queue_type = queue_type; +} +static inline void +netdev_queue_get_req_set_queue_id(struct netdev_queue_get_req *req, + __u32 queue_id) +{ + req->_present.queue_id = 1; + req->queue_id = queue_id; +} + +struct netdev_queue_get_rsp { + struct { + __u32 queue_id:1; + __u32 queue_type:1; + __u32 napi_id:1; + __u32 ifindex:1; + } _present; + + __u32 queue_id; + enum netdev_queue_type queue_type; + __u32 napi_id; + __u32 ifindex; +}; + +void netdev_queue_get_rsp_free(struct netdev_queue_get_rsp *rsp); + +/* + * queue information + */ +struct netdev_queue_get_rsp * +netdev_queue_get(struct ynl_sock *ys, struct netdev_queue_get_req *req); + +/* NETDEV_CMD_QUEUE_GET - dump */ +struct netdev_queue_get_req_dump { + struct { + __u32 ifindex:1; + } _present; + + __u32 ifindex; +}; + +static inline struct netdev_queue_get_req_dump * +netdev_queue_get_req_dump_alloc(void) +{ + return calloc(1, sizeof(struct netdev_queue_get_req_dump)); +} +void netdev_queue_get_req_dump_free(struct netdev_queue_get_req_dump *req); + +static inline void +netdev_queue_get_req_dump_set_ifindex(struct netdev_queue_get_req_dump *req, + __u32 ifindex) +{ + req->_present.ifindex = 1; + req->ifindex = ifindex; +} + +struct netdev_queue_get_list { + struct netdev_queue_get_list *next; + struct netdev_queue_get_rsp obj __attribute__ ((aligned (8))); +}; + +void netdev_queue_get_list_free(struct netdev_queue_get_list *rsp); + +struct netdev_queue_get_list * +netdev_queue_get_dump(struct ynl_sock *ys, + struct netdev_queue_get_req_dump *req); + #endif /* _LINUX_NETDEV_GEN_H */