[ovs-dev] [PATCH v3 3/4] netdev-offload: Add xdp flow api provider

Toshiaki Makita toshiaki.makita1 at gmail.com
Wed Jul 22 08:20:20 UTC 2020


On 2020/07/22 0:38, Aaron Conole wrote:
> William Tu <u9012063 at gmail.com> writes:
> 
>> On Tue, Jun 30, 2020 at 12:11 AM Toshiaki Makita
>> <toshiaki.makita1 at gmail.com> wrote:
>>>
>>> On 2020/06/30 1:17, 0-day Robot wrote:
>>>> Bleep bloop.  Greetings Toshiaki Makita, I am a robot and I have tried out your patch.
>>>> Thanks for your contribution.
>>>>
>>>> I encountered some error that I wasn't expecting.  See the details below.
>>>>
>>>>
>>>> checkpatch:
>>>> WARNING: Comment with 'xxx' marker
>>>> #252 FILE: lib/netdev-afxdp.c:329:
>>>>           /* XXX: close output_map_fd somewhere? */
>>>>
>>>> ERROR: Improper whitespace around control block
>>>> #734 FILE: lib/netdev-offload-xdp.c:258:
>>>>       FLOWMAP_FOR_EACH_INDEX(idx, mf->map) {
>>>
>>> Adding a whitespace like
>>>
>>>        FLOWMAP_FOR_EACH_INDEX (idx, mf->map) {
>>>
>>> fixes the error, but as far as I can see, all existing usage of this macro
>>> does not have this kind of whitespace.
>>>
>>> Which is correct, need whitespace or not?
> 
> Need whitespace.  The usage of FLOWMAP_FOR_EACH throughout the codebase
> predates automated checking.  You'll note that when someone contributes
> new versions (ex: tests/oss-fuzz/miniflow_target.c) they have the
> whitespace.
> 
> Prior to a checkpatch utility, it was more difficult to catch instances
> of style violations, and some snuck through.  Even with checkpatch, some
> make it through (though hopefully fewer).
> 
>> I think we need whitespace here.
> 
> Yes.

William, Aaron,

Thank you for the confirmation.
Will fix them in the next version.

Toshiaki Makita


More information about the dev mailing list