LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Dominique Martinet <asmadeus@codewreck.org>
To: David Miller <davem@davemloft.net>
Cc: doronrk@fb.com, tom@quantonium.net, davejwatson@fb.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] kcm: remove any offset before parsing messages
Date: Fri, 15 Feb 2019 02:00:29 +0100	[thread overview]
Message-ID: <20190215010029.GA10899@nautica> (raw)
In-Reply-To: <20181031025657.GA17861@nautica>

Dominique Martinet wrote on Wed, Oct 31, 2018:
> Anyway, that probably explains I have no problem with bigger VM
> (uselessly more memory available) or without KASAN (I guess there's
> overhead?), but I'm sending at most 300k of data and the VM has a 1.5GB
> of ram, so if there's an allocation failure there I think there's a
> problem ! . . .
> 
> So, well, I'm not sure on the way forward. Adding a bpf helper and
> document that kcm users should mind the offset?

bump on this - I had mostly forgotten about it but the nfs-ganesha
community that could make use of KCM reminded me of my patch that's
waiting for this.

Summary for people coming back after four months:
 - kcm is great, but the bpf function that's supposed to be called for
each packet does not automatically adjust the offset so that it can
assume the skb starts with the packet it needs to look at

 - there is some workaround code that is far from obvious and
undocumented, see the original thread[1]:
[1] http://lkml.kernel.org/r/20180822183852.jnwlxnz54gbbf6po@davejwatson-mba.dhcp.thefacebook.com

 - my patch here tried to automatically pull the corresponding packet to
the front, but apparently with KASAN can trigger out of memory
behaviours on "small" VMs, so even if it doesn't seem to impact
performance much without KASAN I don't think it's really ok to
potentially hang the connection due to oom under severe conditions.


The best alternative I see is adding a proper helper to get
"kcm_rx_msg(skb)->offset" from bpf and document it so users aren't as
lost as I have been; I'm not quite sure how/where to add such a helper
though as I've barely looked at the bpf code until now, but should we go
for that?


(it really feels wrong to me that some random person who just started by
trying to use kcm has to put this much effort to keep the ball rolling,
if nobody cares about kcm I'm also open to have it removed completely
despite the obvious performance gain I benchmarked for ganesha[2] ;
barely maintained feature is worse than no feature)

[2] https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/421314

Thanks,
-- 
Dominique

  reply	other threads:[~2019-02-15  1:00 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-11  9:21 Dominique Martinet
2018-09-12  5:36 ` Dominique Martinet
2018-09-18  1:45   ` David Miller
2018-09-18  1:57     ` Dominique Martinet
2018-09-18  2:40       ` David Miller
2018-09-18  2:45         ` Dominique Martinet
2018-09-18  2:51           ` David Miller
2018-09-18  2:58             ` Dominique Martinet
2018-10-31  2:56       ` Dominique Martinet
2019-02-15  1:00         ` Dominique Martinet [this message]
2019-02-15  1:20           ` Tom Herbert
2019-02-15  1:57             ` Dominique Martinet
2019-02-15  2:48               ` Tom Herbert
2019-02-15  3:31                 ` Dominique Martinet
2019-02-15  4:01                   ` Tom Herbert
2019-02-15  4:52                     ` Dominique Martinet
2019-02-20  4:11                       ` Dominique Martinet
2019-02-20 16:18                         ` Tom Herbert
2019-02-21  8:22                           ` Dominique Martinet
2019-02-22 19:24                             ` Tom Herbert
2019-02-22 20:27                               ` Dominique Martinet
2019-02-22 21:01                                 ` Tom Herbert

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=20190215010029.GA10899@nautica \
    --to=asmadeus@codewreck.org \
    --cc=davejwatson@fb.com \
    --cc=davem@davemloft.net \
    --cc=doronrk@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=tom@quantonium.net \
    --subject='Re: [PATCH v2] kcm: remove any offset before parsing messages' \
    /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

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).