netdev - Re: [PATCH next 3/3] net: Use l3_dev instead of skb->dev for L3 processing

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:	Thu, 3 Mar 2016 17:42:44 -0800
From:	Mahesh Bandewar <maheshb@...gle.com>
To:	Cong Wang <xiyou.wangcong@...il.com>
Cc:	Mahesh Bandewar <mahesh@...dewar.net>,
	David Miller <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	netdev <netdev@...r.kernel.org>, Tim Hockin <thockin@...gle.com>,
	Alex Pollitt <alex.pollitt@...aswitch.com>,
	Matthew Dupre <matthew.dupre@...aswitch.com>
Subject: Re: [PATCH next 3/3] net: Use l3_dev instead of skb->dev for L3 processing

>>> As you mentioned logically we should be able to pass the skb in master's
>>> ns
>>> until L3 processing is completed. This patch series attempts to do that by
>>> disassociating this logic from skb->dev and adding it to l3_dev. This
>>> should
>>> include not just IPT but all that is done in L3 phase (IPT, routing etc.)
>>> Also since dev->l3_dev is same as dev, this should not break any existing
>>> logic.
>>>
>> Well, looking at the code I realized that I missed few places (especially
>> routing
>> logic) which continues using skb->dev in ingress path and should be
>> corrected to
>> use l3_dev. I'll correct those places and send the next version.
>
>
> Look, even you yourself are missing something here. ;) This is exactly why
> I suggest to consider another approach. Please don't introduce any code
> that is hard to debug even for yourself. The struct net pointer is passed
> around in kernel network subsystem almost everywhere, it is not easy to make
> it bug-free by just switching skb->dev.
>
I disagree. Conceptually this is very easy to understand as we are taking L3
processing off of skb->dev and loading it onto dev->l3_dev. So
everything that is
associated with l3_dev is for L3. This should neither make debugging harder
nor add complicated code.

>
>>> That's the generic implementation as far as the stack is concerned and
>>> IPvlan
>>> uses it to make the IPT hooks symmetric.
>>>
>>> Another IPT hook may be good enough (however I haven't
>>> given much thought to it) for IPvlan, but this generic approach will be
>>> for
>>> whole of L3. Also currently this I have implemented for the ingress path
>>> but that does not mean the same cannot be extended for the egress path
>>> (in fact I'm thinking about that)
>>
>
> This is logically correct and easier to understand or debug, since IPT hook
> is very common in network subsystem even ingress qdisc uses it.