On Wed, Nov 13, 2024 at 9:08 AM Cong Wang <xiyou.wangcong@xxxxxxxxx> 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-
