From: Jacob Keller <jacob.e.keller@intel.com>
To: Jakub Kicinski <kuba@kernel.org>,
Johannes Berg <johannes@sipsolutions.net>
Cc: <davem@davemloft.net>, <netdev@vger.kernel.org>,
<edumazet@google.com>, <pabeni@redhat.com>, <jiri@resnulli.us>,
<razor@blackwall.org>, <nicolas.dichtel@6wind.com>,
<gnault@redhat.com>, <fw@strlen.de>
Subject: Re: [PATCH net-next 01/13] genetlink: refactor the cmd <> policy mapping dump
Date: Wed, 19 Oct 2022 14:20:00 -0700 [thread overview]
Message-ID: <d29c7980-7a7e-3a75-3e54-ce081ffdb8cd@intel.com> (raw)
In-Reply-To: <20221019085911.78c61724@kernel.org>
On 10/19/2022 8:59 AM, Jakub Kicinski wrote:
> On Wed, 19 Oct 2022 09:50:16 +0200 Johannes Berg wrote:
>>> + if (ctrl_dumppolicy_put_op(skb, cb, &op))
>>> + return skb->len;
>>> +
>>> + ctx->opidx = genl_get_cmd_cnt(ctx->rt);
>>
>> This (now without a comment that you removed rather than changed), still
>> strikes me as odd.
>>
>> I guess if we add a comment /* don't enter the loop below */ that'd be
>> nicer, but I feel maybe putting the loop into the else instead would be
>> nicer?
>
> The comment got eaten in rebases, it comes back in patch 10 apparently..
> I'll put it back in v2.
Thanks, I had the same confusion when reviewing.
-Jake
next prev parent reply other threads:[~2022-10-19 21:20 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-18 23:07 [PATCH net-next 00/13] genetlink: support per op type policies Jakub Kicinski
2022-10-18 23:07 ` [PATCH net-next 01/13] genetlink: refactor the cmd <> policy mapping dump Jakub Kicinski
2022-10-19 7:50 ` Johannes Berg
2022-10-19 15:59 ` Jakub Kicinski
2022-10-19 21:20 ` Jacob Keller [this message]
2022-10-18 23:07 ` [PATCH net-next 02/13] genetlink: move the private fields in struct genl_family Jakub Kicinski
2022-10-19 7:51 ` Johannes Berg
2022-10-19 21:21 ` Jacob Keller
2022-10-18 23:07 ` [PATCH net-next 03/13] genetlink: introduce split op representation Jakub Kicinski
2022-10-19 7:59 ` Johannes Berg
2022-10-19 19:14 ` Jakub Kicinski
2022-10-19 19:36 ` Johannes Berg
2022-10-19 19:50 ` Jakub Kicinski
2022-10-19 21:28 ` Jacob Keller
2022-10-18 23:07 ` [PATCH net-next 04/13] genetlink: load policy based on validation flags Jakub Kicinski
2022-10-19 8:01 ` Johannes Berg
2022-10-19 19:20 ` Jakub Kicinski
2022-10-19 19:33 ` Johannes Berg
2022-10-19 19:49 ` Jakub Kicinski
2022-10-18 23:07 ` [PATCH net-next 05/13] genetlink: check for callback type at op load time Jakub Kicinski
2022-10-19 21:33 ` Jacob Keller
2022-10-19 21:45 ` Jakub Kicinski
2022-10-18 23:07 ` [PATCH net-next 06/13] genetlink: add policies for both doit and dumpit in ctrl_dumppolicy_start() Jakub Kicinski
2022-10-19 8:08 ` Johannes Berg
2022-10-19 19:22 ` Jakub Kicinski
2022-10-18 23:07 ` [PATCH net-next 07/13] genetlink: support split policies in ctrl_dumppolicy_put_op() Jakub Kicinski
2022-10-19 21:38 ` Jacob Keller
2022-10-19 21:46 ` Jakub Kicinski
2022-10-18 23:07 ` [PATCH net-next 08/13] genetlink: inline genl_get_cmd() Jakub Kicinski
2022-10-19 21:46 ` Jacob Keller
2022-10-18 23:07 ` [PATCH net-next 09/13] genetlink: add iterator for walking family ops Jakub Kicinski
2022-10-19 21:49 ` Jacob Keller
2022-10-18 23:07 ` [PATCH net-next 10/13] genetlink: use iterator in the op to policy map dumping Jakub Kicinski
2022-10-19 21:53 ` Jacob Keller
2022-10-18 23:07 ` [PATCH net-next 11/13] genetlink: inline old iteration helpers Jakub Kicinski
2022-10-19 22:15 ` Jacob Keller
2022-10-18 23:07 ` [PATCH net-next 12/13] genetlink: allow families to use split ops directly Jakub Kicinski
2022-10-19 8:15 ` Johannes Berg
2022-10-19 19:25 ` Jakub Kicinski
2022-10-19 19:37 ` Johannes Berg
2022-10-19 19:57 ` Jakub Kicinski
2022-10-20 7:32 ` Johannes Berg
2022-10-20 18:09 ` Jakub Kicinski
2022-10-21 11:02 ` Johannes Berg
2022-10-21 15:01 ` Jakub Kicinski
2022-10-18 23:07 ` [PATCH net-next 13/13] genetlink: convert control family to split ops Jakub Kicinski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d29c7980-7a7e-3a75-3e54-ce081ffdb8cd@intel.com \
--to=jacob.e.keller@intel.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=fw@strlen.de \
--cc=gnault@redhat.com \
--cc=jiri@resnulli.us \
--cc=johannes@sipsolutions.net \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nicolas.dichtel@6wind.com \
--cc=pabeni@redhat.com \
--cc=razor@blackwall.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).