netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: John Ousterhout <ouster@cs.stanford.edu>, netdev@vger.kernel.org
Subject: Re: Upstream Homa?
Date: Fri, 11 Nov 2022 00:23:56 +0100	[thread overview]
Message-ID: <Y22IDLhefwvjRnGX@lunn.ch> (raw)
In-Reply-To: <20221110132540.44c9463c@hermes.local>

On Thu, Nov 10, 2022 at 01:25:40PM -0800, Stephen Hemminger wrote:
> On Thu, 10 Nov 2022 11:42:35 -0800
> John Ousterhout <ouster@cs.stanford.edu> wrote:
> 
> > Several people at the netdev conference asked me if I was working to
> > upstream the Homa transport protocol into the kernel. I have assumed
> > that this is premature, given that there is not yet significant usage of
> > Homa, but they encouraged me to start a discussion about upstreaming
> > with the netdev community.
> > 
> > So, I'm sending this message to ask for advice about (a) what state
> > Homa needs to reach before it would be appropriate to upstream it,
> > and, (b) if/when that time is reached, what is the right way to go about it.
> > Homa currently has about 13K lines of code, which I assume is far too
> > large for a single patch set; at the same time, it's hard to envision a
> > manageable first patch set with enough functionality to be useful by itself.
> > 
> > -John-

Hi John

> The usual upstream problem areas are:
>  - coding style

You can get a good feeling about what sort of coding style review
comments you will get by running ./scripts/checkpatch.pl over your
files. You don't need to be completely checkpatch clean, it does get
things wrong sometimes.

Adding to Stephens list.

- You have reinvented something which the kernel already has. You need
  to throw away your version and use the kernel version.

- You have used deprecated things, like /proc, ioctls rather than
  netlink.

- 32 bit kernel problems. Since this is about data center, your code
  might make assumptions about running on a 64 bit machine. Statistics
  tend to be done wrong, unless you are using the correct kernel
  helpers to deal with 64 bit counters on 32 bit machines.

Andrew

  reply	other threads:[~2022-11-10 23:24 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-10 19:42 Upstream Homa? John Ousterhout
2022-11-10 21:25 ` Stephen Hemminger
2022-11-10 23:23   ` Andrew Lunn [this message]
     [not found]     ` <CAGXJAmw=NY17=6TnDh0oV9WTmNkQCe9Q9F3Z=uGjG9x5NKn7TQ@mail.gmail.com>
2022-11-11 19:10       ` Stephen Hemminger
2022-11-11 19:25       ` Andrew Lunn
2022-11-12  7:53         ` Jiri Pirko
2022-11-13  6:25           ` John Ousterhout
2022-11-13 17:10             ` Andrew Lunn
2022-11-13 20:10               ` John Ousterhout
2022-11-13 20:37                 ` Andrew Lunn
2022-11-14  5:37                   ` John Ousterhout
2022-11-13  6:09         ` John Ousterhout
2022-11-13  8:24           ` Jiri Pirko
2022-11-13 18:53             ` Andrew Lunn
2022-12-04 18:17 ` Jamal Hadi Salim

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=Y22IDLhefwvjRnGX@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=netdev@vger.kernel.org \
    --cc=ouster@cs.stanford.edu \
    --cc=stephen@networkplumber.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).