netdev - Re: [PATCH net-next v2 00/12] Begin upstreaming Homa transport protocol

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGXJAmyGTwjFo6fGoROY=hQFXbR5RdpmpkEc9Zm6DOoD2nbwNA@mail.gmail.com>
Date: Thu, 14 Nov 2024 08:59:40 -0800
From: John Ousterhout <ouster@...stanford.edu>
To: Cong Wang <xiyou.wangcong@...il.com>
Cc: netdev@...r.kernel.org, linux-api@...r.kernel.org
Subject: Re: [PATCH net-next v2 00/12] Begin upstreaming Homa transport protocol

On Wed, Nov 13, 2024 at 9:08 AM Cong Wang <xiyou.wangcong@...il.com> wrote:
>
> On Mon, Nov 11, 2024 at 03:39:53PM -0800, John Ousterhout wrote:
> >  MAINTAINERS               |    7 +
> >  include/uapi/linux/homa.h |  165 ++++++
> >  net/Kconfig               |    1 +
> >  net/Makefile              |    1 +
> >  net/homa/Kconfig          |   19 +
> >  net/homa/Makefile         |   14 +
> >  net/homa/homa_impl.h      |  767 ++++++++++++++++++++++++++
> >  net/homa/homa_incoming.c  | 1076 +++++++++++++++++++++++++++++++++++++
> >  net/homa/homa_outgoing.c  |  854 +++++++++++++++++++++++++++++
> >  net/homa/homa_peer.c      |  319 +++++++++++
> >  net/homa/homa_peer.h      |  234 ++++++++
> >  net/homa/homa_plumbing.c  |  965 +++++++++++++++++++++++++++++++++
> >  net/homa/homa_pool.c      |  420 +++++++++++++++
> >  net/homa/homa_pool.h      |  152 ++++++
> >  net/homa/homa_rpc.c       |  488 +++++++++++++++++
> >  net/homa/homa_rpc.h       |  446 +++++++++++++++
> >  net/homa/homa_sock.c      |  380 +++++++++++++
> >  net/homa/homa_sock.h      |  426 +++++++++++++++
> >  net/homa/homa_stub.h      |   80 +++
> >  net/homa/homa_timer.c     |  156 ++++++
> >  net/homa/homa_utils.c     |  150 ++++++
> >  net/homa/homa_wire.h      |  378 +++++++++++++
>
> Hi John,
>
> Thanks for your efforts to push them upstream!
>
> Just some very high-level comments:
>
> 1. Please run scripts/checkpatch.pl to make sure the coding style is
> aligned with upstream, since I noticed there are still some C++ style
> comments in your patchset.

I have been running checkpatch.pl, but it didn't complain about C++
style comments. Those comments really shouldn't be there, though: they
are for pieces of code I've temporarily commented out, and those
chunks shouldn't be in the upstream version of Homa. I'll fix things
so they don't appear in the future.

> 2. Please consider adding socket diagnostics, see net/ipv4/inet_diag.c.

I wasn't familiar with them before your email; I'll take a look.

> 3. Please consider adding test cases, you can pretty much follow
> tools/testing/vsock/vsock_test.c.

Homa has extensive unit tests in the GitHub repo, but I thought I'd
wait to try upstreaming them until all of Homa has been upstreamed
(they are based on a modified version of kselftest.h, so they may be a
bit controversial). One way or another, though, I'm committed to
having good unit tests for Homa.

Thank you for your feedback.

-John-