b.a.t.m.a.n.lists.open-mesh.org archive mirror
 help / color / mirror / Atom feed
From: Matthias Schiffer <mschiffer@universe-factory.net>
To: Sven Eckelmann <sven@narfation.org>, b.a.t.m.a.n@lists.open-mesh.org
Cc: Maximilian Wilhelm <max@sdn.clinic>,
	Felix Kaechele <felix@kaechele.ca>, Mephisto <mephisto@mephis.to>
Subject: Re: [B.A.T.M.A.N.] [PATCH maint] batman-adv: fix packet checksum in receive path
Date: Mon, 22 Jan 2018 22:18:34 +0100	[thread overview]
Message-ID: <9411aa2d-e39f-718e-9c99-40be6a0143f8@universe-factory.net> (raw)
In-Reply-To: <2357913.Ue9Sgfg8BR@sven-edge>


[-- Attachment #1.1: Type: text/plain, Size: 2454 bytes --]

On 01/22/2018 09:52 PM, Sven Eckelmann wrote:
> On Montag, 22. Januar 2018 20:24:50 CET Matthias Schiffer wrote:
>> skb_postpull_rcsum() is necessary after eth_type_trans() to adjust the
>> skb checksum, otherwise log spam of the form "bat0: hw csum failure" will
>> result when packets with CHECKSUM_COMPLETE are received (at least in some
>> setups, e.g. when stacking batman-adv on top of VXLAN).
> 
> Would be nice to have a better explanation here.
> 
> The comment previously assumed that skb_pull_rcsum would be enough. But the 
> problem here is that the skb_pull_rcsum only pulls the batman-adv headers. The 
> actual pull of the ethernet header (with skb_pull_inline) happens inside 
> eth_type_trans. Or did I miss anything?

This is correct, eth_type_trans() contains a simple skb_pull(), so the csum
must be adjusted afterwards (grepping the kernel for eth_type_trans will
find a lot of this). I can send a v2 with a better commit message later.

> 
> [...]
>> I don't know what the exact circumstances are that trigger the log spam,
>> but it seems this was broken forever (I could also reproduce the issue with
>> our compat-14 legacy branch)... so please ask David to queue this up for
>> stable :)
> 
> Yes, this is broken since earliest commits. The most relevant commit in 
> batman-adv is:
> 
> Fixes: fe28a94c01e1 ("batman-adv: receive packets directly using skbs")
> 
> But I would propose to use following in the kernel tree:
> 
> Fixes: c6c8fea29769 ("net: Add batman-adv meshing protocol")
> 
> The 4.15 release will be soon(tm) and Simon is currently on vacation. So we 
> will most likely postpone the submission to David until Simon found a way out 
> of the snow and after 4.15 is released...
> 
> But it would be nice when some people could test the patch [1] (together with 
> vxlan?) on batman-adv or batman-adv-legacy. And please provide a 
> "Tested-by: Full Name <email@example.org>" [2] reply when it works.
> 
> Thanks,> 	Sven

I've tested this on Kernel 4.14.14 (everything working correctly now) and
4.4.110 (here, there are still checksum errors; it seems on older kernels,
the checksum handling in VXLAN is broken too? Still debugging this...)

Matthias



> 
> [1] https://patchwork.open-mesh.org/patch/17250/
> [2] https://www.kernel.org/doc/html/v4.12/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2018-01-22 21:18 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-22 19:24 [B.A.T.M.A.N.] [PATCH maint] batman-adv: fix packet checksum in receive path Matthias Schiffer
2018-01-22 20:52 ` Sven Eckelmann
2018-01-22 21:18   ` Matthias Schiffer [this message]
2018-01-23  9:12     ` Matthias Schiffer
2018-01-23 21:56       ` Maximilian Wilhelm
2018-01-23 14:25   ` Maximilian Wilhelm

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=9411aa2d-e39f-718e-9c99-40be6a0143f8@universe-factory.net \
    --to=mschiffer@universe-factory.net \
    --cc=b.a.t.m.a.n@lists.open-mesh.org \
    --cc=felix@kaechele.ca \
    --cc=max@sdn.clinic \
    --cc=mephisto@mephis.to \
    --cc=sven@narfation.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).