netdev - Re: [PATCH v2 net-next 2/2] tc: make ingress and egress qdiscs consistent

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]

Message-ID: <20150408090520.GA2057@nanopsycho.orion>
Date:	Wed, 8 Apr 2015 11:05:20 +0200
From:	Jiri Pirko <jiri@...nulli.us>
To:	Daniel Borkmann <daniel@...earbox.net>
Cc:	Alexei Starovoitov <ast@...mgrid.com>,
	David Miller <davem@...emloft.net>, jhs@...atatu.com,
	netdev@...r.kernel.org, tgraf@...g.ch
Subject: Re: [PATCH v2 net-next 2/2] tc: make ingress and egress qdiscs
 consistent

Wed, Apr 08, 2015 at 10:36:08AM CEST, daniel@...earbox.net wrote:
>On 04/08/2015 06:48 AM, Alexei Starovoitov wrote:
>>On 4/7/15 8:22 PM, Alexei Starovoitov wrote:
>>>but it seems no one cares about using them with ingress, so I'll go back
>>>to cls_bpf specific skb_share_check and push.
>>
>>that didn't work either :(
>>we cannot replace skb via skb_share_check() inside cls/act. We cannot do
>>it inside ingress_enqueue() either. It can only be done at handle_ing()
>>level. And it's quite ugly to change the signatures of the whole
>>qdisc->enqueue() call chain just for cls_bpf. May be introducing
>>bpf-only ingress qdisc to decouple the logic is not such a bad idea?
>
>So it seems ingress qdisc is quite broken for various classifier
>and actions. :/ I wouldn't go that far to have a bpf-only ingress
>qdisc, but what about introducing l2/l3 ingress qdisc (or, name
>it "early ingress" and "ingress" qdisc), so at an early point in
>netif_receive_skb_internal(), we would have an l2_ingress hook,
>wrapped via static keys to have minimal impact if unused, and could
>do the push/pull similarly as in the PTP classifier w/o worry that
>it is referenced by other entities. There, we could at least still
>benefit from hw flow steering.

How about to just adjust ingress qdisc to do the right thing (of adjust
egress qdisc so they both behave the same). I don't like the idea of
having more ingres queue disk. Would be just confusing.

>
>The current ingress qdisc, we'd rename l3_ingress to make it clear
>what to expect (can also be aliased in iproute2). Maybe classifiers,
>actions could be flagged as l2/l3 capable and checked at config
>time where to apply, at least in the case of {cls,act}_bpf?
>
>The other thing I had in mind is that we could expose skb_iif to
>detect that we're actually coming from ingress qdisc from inside
>the ebpf prog, but that is very limited and you nevertheless miss
>out on l2 context.

As you said, this needs to be resolved for others as well.

>
>Thanks,
>Daniel
--
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