netdev - Re: [PATCH v4 net-next 2/2] tc: add 'needs

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] [day] [month] [year] [list]

Message-Id: <20150414.140506.401099074422805944.davem@davemloft.net>
Date:	Tue, 14 Apr 2015 14:05:06 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	ast@...mgrid.com
Cc:	cwang@...pensource.com, daniel@...earbox.net, edumazet@...gle.com,
	tgraf@...g.ch, jiri@...nulli.us, jhs@...atatu.com,
	netdev@...r.kernel.org
Subject: Re: [PATCH v4 net-next 2/2] tc: add 'needs_l2' flag to ingress
 qdisc

From: Alexei Starovoitov <ast@...mgrid.com>
Date: Mon, 13 Apr 2015 17:57:21 -0700

> On 4/13/15 3:44 PM, Cong Wang wrote:
>>
>> My suggestion is fairly simple: move skb_pull() out of
>> eth_type_trans()
>> and move it to netif_receive_skb(), it apparently needs much more work
>> but it worth the effort. This would eliminate the skb_push() in mirred
>> and in ifb too, dev_forward_skb() would benefit too.
> 
> wow. That would be massive. We're talking months worth of work here.

I think it's worth the effort.

Alexei I cannot apply your needs_l2 patch, it's fundamentally broken
because it assumes the whole chain of classifiers and actions starting
from a given qdisc root want the skb->data in one place or another.
It can be mixed, and quite easily so, and for that scenerio it's
broken.

You've been told multiple ways to keep the existing stuff working
yet give cls_bpf what it wants.  I'll add another one so you'll have
three options:

1) Firstly, to propagate the packet offset into the code generated
   for the BPF classifier when being attached to the ingress qdisc.
   Not only is this doable, you'll know you aren't being shared with
   other kinds of qdiscs so it'll just work.

2) Adjust where skb_pull() happens, Cong's idea.

3) Do the SKB share_check/pull/push in cls_bpf.  No impact on anyone
   else in the qdisc paths, mixed configurations handled in a guaranteed
   way.

I know none of these fit into your unreasonable idealistic model of
how you want all of this to work, but surprise this is the real world
and you'll need to compromise some of your absolute requirements in
order to meet your goals of proper functionality.

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