rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Andrew Lunn <andrew@lunn.ch>
Cc: FUJITA Tomonori <fujita.tomonori@gmail.com>,
	netdev@vger.kernel.org, rust-for-linux@vger.kernel.org,
	aliceryhl@google.com, miguel.ojeda.sandonis@gmail.com
Subject: Re: [PATCH 0/5] Rust abstractions for network device drivers
Date: Thu, 15 Jun 2023 19:02:52 -0700	[thread overview]
Message-ID: <20230615190252.4e010230@kernel.org> (raw)
In-Reply-To: <8e9e2908-c0da-49ec-86ef-b20fb3bd71c3@lunn.ch>

On Thu, 15 Jun 2023 14:51:10 +0200 Andrew Lunn wrote:
> On Wed, Jun 14, 2023 at 11:01:28PM -0700, Jakub Kicinski wrote:
> > On Tue, 13 Jun 2023 13:53:21 +0900 FUJITA Tomonori wrote:  
> > > This patchset adds minimum Rust abstractions for network device
> > > drivers and an example of a Rust network device driver, a simpler
> > > version of drivers/net/dummy.c.
> > > 
> > > The dummy network device driver doesn't attach any bus such as PCI so
> > > the dependency is minimum. Hopefully, it would make reviewing easier.
> > > 
> > > Thanks a lot for reviewing on RFC patchset at rust-for-linux ml.
> > > Hopefully, I've addressed all the issues.  
> > 
> > First things first, what are the current expectations for subsystems
> > accepting rust code?
> > 
> > I was hoping someone from the Rust side is going to review this.
> > We try to review stuff within 48h at netdev, and there's no review :S  
> 
> As pointed out elsewhere, i've looked the code over. I cannot say
> anything about the rust, but i don't see anything too obviously wrong
> with the way it use a few netdev API calls.

The skb freeing looks shady from functional perspective.
I'm guessing some form of destructor frees the skb automatically
in xmit handler(?), but (a) no reason support, (b) kfree_skb_reason()
is most certainly not safe to call on all xmit paths...

> > My immediate instinct is that I'd rather not merge toy implementations
> > unless someone within the netdev community can vouch for the code.  
> 
> It is definitely toy. But you have to start somewhere.
> 
> What might be useful is an idea of the roadmap. How does this go from
> toy to something useful?
> 
> I see two different threads of work.
> 
> One is getting enough in place to find where rust is lacking. netdev
> uses a lot of inline C functions, which don't seem to map too well to
> Rust. And rust does not seem to have the concept of per CPU memory,
> needed for statistics. So it would be good to be able to have some
> code which can be profiled to see how bad this actually is, and then
> provide a test bed for finding solutions for these problems.
> 
> The second is just wrapping more API calls to allow more capable
> drivers to be written. Implementing the loopback driver seems like the
> next obvious step. Then maybe a big jump to virtio_net?

I'd have thought that a simple SPI driver or some such would be 
a better match.

> Like dummy and loopback, it is something which everybody can have, no
> physical hardware needed. It also has a lot of netdev features, NAPI,
> RSS, statistics, BPF & XDP. So it is a good playground, both for
> features and to profiling and performance optimisation.

IMO we have too much useless playground stuff in C already, don't get me
started. Let's get a real device prove that this thing can actually work
:( We can waste years implementing imaginary device drivers that nobody
ends up using.

  reply	other threads:[~2023-06-16  2:03 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-13  4:53 [PATCH 0/5] Rust abstractions for network device drivers FUJITA Tomonori
2023-06-13  4:53 ` [PATCH 1/5] rust: core " FUJITA Tomonori
2023-06-15 13:01   ` Benno Lossin
2023-06-21 13:13     ` FUJITA Tomonori
2023-06-25  9:52       ` Benno Lossin
2023-06-25 14:27         ` FUJITA Tomonori
2023-06-25 17:06           ` Benno Lossin
2023-06-21 22:44   ` Boqun Feng
2023-06-22  0:19     ` FUJITA Tomonori
2023-06-13  4:53 ` [PATCH 2/5] rust: add support for ethernet operations FUJITA Tomonori
2023-06-13  7:19   ` Ariel Miculas
2023-06-15 13:03   ` Benno Lossin
2023-06-15 13:44     ` Andrew Lunn
2023-06-13  4:53 ` [PATCH 3/5] rust: add support for get_stats64 in struct net_device_ops FUJITA Tomonori
2023-06-13  4:53 ` [PATCH 4/5] rust: add methods for configure net_device FUJITA Tomonori
2023-06-15 13:06   ` Benno Lossin
2023-06-13  4:53 ` [PATCH 5/5] samples: rust: add dummy network driver FUJITA Tomonori
2023-06-15 13:08   ` Benno Lossin
2023-06-22  0:23     ` FUJITA Tomonori
2023-06-15  6:01 ` [PATCH 0/5] Rust abstractions for network device drivers Jakub Kicinski
2023-06-15  8:58   ` Miguel Ojeda
2023-06-16  2:19     ` Jakub Kicinski
2023-06-16 12:18       ` FUJITA Tomonori
2023-06-16 13:23         ` Miguel Ojeda
2023-06-16 13:41           ` FUJITA Tomonori
2023-06-16 18:26           ` Jakub Kicinski
2023-06-16 20:05             ` Miguel Ojeda
2023-06-16 13:04       ` Andrew Lunn
2023-06-16 18:31         ` Jakub Kicinski
2023-06-16 13:18       ` Miguel Ojeda
2023-06-15 12:51   ` Andrew Lunn
2023-06-16  2:02     ` Jakub Kicinski [this message]
2023-06-16  3:47       ` Richard Cochran
2023-06-16 17:59         ` Andrew Lunn
2023-06-16 13:02       ` FUJITA Tomonori
2023-06-16 13:14         ` Andrew Lunn
2023-06-16 13:48           ` Miguel Ojeda
2023-06-16 14:43             ` Andrew Lunn
2023-06-16 16:01               ` Miguel Ojeda
2023-06-19 11:27               ` Emilio Cobos Álvarez
2023-06-20 18:09                 ` Miguel Ojeda
2023-06-20 19:12                   ` Andreas Hindborg (Samsung)
2023-06-21 12:30             ` Andreas Hindborg (Samsung)
2023-06-16 18:40         ` Jakub Kicinski
2023-06-16 19:00           ` Alice Ryhl
2023-06-16 19:10             ` Jakub Kicinski
2023-06-16 19:23               ` Alice Ryhl
2023-06-16 20:04                 ` Andrew Lunn
2023-06-17 10:08                   ` Alice Ryhl
2023-06-17 10:15                     ` Greg KH
2023-06-19  8:50                     ` FUJITA Tomonori
2023-06-19  9:46                       ` Greg KH
2023-06-19 11:05                         ` FUJITA Tomonori
2023-06-19 11:14                           ` Greg KH
2023-06-19 13:20                           ` Andrew Lunn
2023-06-20 11:16                             ` David Laight
2023-06-20 15:47                     ` Jakub Kicinski
2023-06-20 16:56                       ` Alice Ryhl
2023-06-20 17:44                         ` Miguel Ojeda
2023-06-20 17:55                           ` Miguel Ojeda
2023-06-16 12:28   ` Alice Ryhl
2023-06-16 13:20     ` Andrew Lunn
2023-06-16 13:24       ` Alice Ryhl

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=20230615190252.4e010230@kernel.org \
    --to=kuba@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=andrew@lunn.ch \
    --cc=fujita.tomonori@gmail.com \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=rust-for-linux@vger.kernel.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).