Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 22 Aug 2018 11:38:52 -0700
From:   Dave Watson <davejwatson@fb.com>
To:     Dominique Martinet <asmadeus@codewreck.org>
CC:     Doron Roberts-Kedes <doronrk@fb.com>,
        Tom Herbert <tom@quantonium.net>,
        "David S. Miller" <davem@davemloft.net>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] strparser: remove any offset before parsing messages
Message-ID: <20180822183852.jnwlxnz54gbbf6po@davejwatson-mba.dhcp.thefacebook.com>
References: <1533854411-28184-1-git-send-email-asmadeus@codewreck.org>
 <1534855906-22870-1-git-send-email-asmadeus@codewreck.org>
 <20180821145321.GA44710@doronrk-mbp>
 <20180821193655.GA15354@nautica>
 <20180821211504.GA76892@doronrk-mbp.dhcp.thefacebook.com>
 <20180821225113.GA6515@nautica>
 <20180821233549.GA96607@doronrk-mbp.dhcp.thefacebook.com>
 <20180822004647.GA10656@nautica>
 <20180822023308.GA5970@doronrk-mbp.dhcp.thefacebook.com>
 <20180822054707.GA13455@nautica>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20180822054707.GA13455@nautica>
User-Agent: NeoMutt/20180716
Received-SPF: None (protection.outlook.com: fb.com does not designate
 permitted sender hosts)
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Aug 2018 18:38:56.0524 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 55003cb6-c59c-4347-de54-08d6085e8481
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR15MB1123
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-08-22_10:,,
 signatures=0
X-Proofpoint-Spam-Reason: safe
X-FB-Internal: Safe
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 08/22/18 07:47 AM, Dominique Martinet wrote:
> > > As I wrote above, I think it should not be possible, so we're not
> > > even talking about a small percentage here.
> > > The reason I didn't use skb_pull (the head-only variant) is that I'd
> > > rather have the overhead than a BUG() if I'm wrong on this...
> > 
> > A printk in that section when (orig_offset + eaten > skb_headlen(head)) 
> > confirms that this case is not uncommon or impossible. Would have to do
> > more work to see how many hundreds of times per second, but it is not a
> > philosophical concern.
> 
> Hmm, right, it does happen if I force two bigger packets in a single
> write() on my reproducer; I guess my workload didn't exhibit that
> behaviour with a 9p client...
> 
> I've tried measuring that overhead as well by writing a more complex bpf
> program that would fetch the offset in the skb but for some reason I'm
> reading a 0 offset when it's not zero... well, not like there's much
> choice for this at this point anyway; I don't think we'll do this
> without pull, I'll put that on background.

For what it is worth we checked the offset in bpf, something
along the lines of
	  
struct kcm_rx_msg {   int full_len;  int offset;};
static inline struct kcm_rx_msg *kcm_rx_msg(struct __sk_buff *skb)
      { return (struct kcm_rx_msg *)skb->cb;}

int decode_framing(struct __sk_buff *skb)
{ return load_word(skb, kcm_rx_msg(skb)->offset);}

Although it did puzzle me for a while figuring that out when I ran in
to it.