Re: [PATCH net-next 3/3] net: dropreason: reformat the comment fo skb drop reasons

From: Menglong Dong <menglong8.dong@gmail.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@redhat.com>,
	David Miller <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>,
	Neil Horman <nhorman@tuxdriver.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>, Martin Lau <kafai@fb.com>,
	Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Menglong Dong <imagedong@tencent.com>,
	David Ahern <dsahern@kernel.org>,
	Talal Ahmad <talalahmad@google.com>,
	Kees Cook <keescook@chromium.org>,
	LKML <linux-kernel@vger.kernel.org>,
	netdev <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH net-next 3/3] net: dropreason: reformat the comment fo skb drop reasons
Date: Sat, 28 May 2022 12:32:26 +0800	[thread overview]
Message-ID: <CADxym3ZM=eW3hNLsUuCYDfZe2F+=c_Q69YxVi9VQ_65GY2DbAQ@mail.gmail.com> (raw)
In-Reply-To: <20220527181915.6e776577@kernel.org>

On Sat, May 28, 2022 at 9:19 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 27 May 2022 15:15:22 +0800 menglong8.dong@gmail.com wrote:
> > From: Menglong Dong <imagedong@tencent.com>
> >
> > To make the code clear, reformat the comment in dropreason.h to k-doc
> > style.
> >
> > Now, the comment can pass the check of kernel-doc without warnning:
> >
> > $ ./scripts/kernel-doc -v -none include/linux/dropreason.h
> > include/linux/dropreason.h:7: info: Scanning doc for enum skb_drop_reason
> >
> > Signed-off-by: Menglong Dong <imagedong@tencent.com>
>
> I feel bad for suggesting this after you reformatted all the values
> but could we use inline notation here? With a huge enum like this
> there's a lot of scrolling between documentation and the value.
>
> /**
>  * enum skb_drop_reason - the reasons of skb drops
>  *
>  * The reason of skb drop, which is used in kfree_skb_reason().
>  * en...maybe they should be splited by group?
>  */
>  enum skb_drop_reason {
>         /**
>          * @SKB_NOT_DROPPED_YET: skb is not dropped yet (used for no-drop case)
>          */
>         SKB_NOT_DROPPED_YET = 0,
>         /** @SKB_DROP_REASON_NOT_SPECIFIED: drop reason is not specified */
>         SKB_DROP_REASON_NOT_SPECIFIED,
>         /** @SKB_DROP_REASON_NO_SOCKET: socket not found */
>         SKB_DROP_REASON_NO_SOCKET,
> ...

The inline comment is an option, which I thought of. But I found it
makes the code a little not tidy. Of course, inline comments are
easier to read. Anyway, I'll have a try and make them inline.

Thanks!
Menglong Dong