netdev - Re: [PATCH net-next] net-gre-gro: Fix a bug that breaks the forwarding path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 15 Jul 2014 21:23:07 +0300
From:	Or Gerlitz <or.gerlitz@...il.com>
To:	Jerry Chu <hkchu@...gle.com>
Cc:	Tom Herbert <therbert@...gle.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Or Gerlitz <ogerlitz@...lanox.com>,
	Wolfgang Walter <linux@...m.de>,
	David Miller <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] net-gre-gro: Fix a bug that breaks the
 forwarding path

On Tue, Jul 15, 2014 at 8:17 PM, Jerry Chu <hkchu@...gle.com> wrote:
> On Tue, Jul 15, 2014 at 7:06 AM, Or Gerlitz <or.gerlitz@...il.com> wrote:

>> what do you mean by "h/w support sLRO on tunneled pkts" here? as far
>> as I understand, the driver should set the bit when they know it's an
>> encapsulated  packet for which the HW offloaded checksum verification.

can you clarify the sLRO comment?

>> So my fishiness comment was referring to the fact that we see here
>> that the stack is setting this bit too sometimes.

> There are many other places in the "stack" that will set that bit if you
> grep it. I think many of them are there to make GSO work on tunneled pkts.
> The code here is no different.

I am not near the code now, but AFAIK, the "stack" sets it in the TX
path and the driver sets it in the RX path, any deviation you see
there for this convension except for the change introduced by this
patch.

> Davem wanted a more precise definition of skb->encapsulation and I thought
> some worthy discussion has happened as part of the thread of the
> original patch (even though it left with some more questions but
> that's often the case with complex kernel code, right?)


complex as it may be, if the design is valid (e.g doesn't contradict
itself, etc...) it should be subject to proper documentation and
implementation. So in that repspect, I read Dave's comment as saying
-- guys, this isn't the case with that skb encapsulation bit.

For a nearby example, see the documentation of the semantic of the
different checksum values (e.g none/unnecessary/complete/
partial) that the stack and drivers are setting, it wasn't very
clear/percise since maybe kernel 2.4 and only over one of the last
kernel cycles on Dec 2013 this patch made it clear "net: skbuff:
improve comment on checksumming"
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html