netdev - Re: [PATCH v3 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <5526B0E0.7060000@plumgrid.com>
Date:	Thu, 09 Apr 2015 10:03:28 -0700
From:	Alexei Starovoitov <ast@...mgrid.com>
To:	Jamal Hadi Salim <jhs@...atatu.com>,
	David Miller <davem@...emloft.net>
CC:	daniel@...earbox.net, tgraf@...g.ch, jiri@...nulli.us,
	netdev@...r.kernel.org
Subject: Re: [PATCH v3 net-next 2/2] tc: add 'needs_l2' flag to ingress qdisc

On 4/9/15 3:49 AM, Jamal Hadi Salim wrote:
> those devices and only speacial case with them. Your assumptions of
> blindly pushing/pulling will break  in some cases (take a look at
> mirred).

you underestimated how much time I've spent studying mirred ;)
In particular my v2 patch fixes two bugs in it:
- it was forwarding packets with L2 present to tunnel devices if used
   with egress qdisc
- it wasn't updating skb->csum with ingress qdisc

In v3 since 'needs_l2' is an optional flag, I didn't touch mirred
and left its bugs for the future patches.

> Your changes penalize everyone else because of this assumption
> bpf makes. We have always tried to be sensitive to perfomance.

hmm. penalize everyone? it's an optional flag.
If ingress_l2 is used with cls_bpf plus some other classifier,
sure, the other classifier is paying potential cost skb_share_check
if taps are active. The chances of having more than a couple classifiers
on a single device are very slim, since they're called sequentially
and killing performance regardless.
In most of the use cases I'm after, cls_bpf will be the only classifier
attached. In some cases there may be few others, but cls_bpf will be
the first and the heaviest one. So it makes sense to optimize for it.

> code path and you are adding a bunch more unconditionally.

not true at all. Looks like you misread the v3 patch completely.
It's an optional flag.

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