All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jamal Hadi Salim <jhs@mojatatu.com>
To: Alexei Starovoitov <ast@plumgrid.com>,
	David Miller <davem@davemloft.net>
Cc: daniel@iogearbox.net, tgraf@suug.ch, jiri@resnulli.us,
	netdev@vger.kernel.org
Subject: Re: [PATCH v3 net-next 2/2] tc: add 'needs_l2' flag to ingress qdisc
Date: Thu, 09 Apr 2015 06:49:34 -0400	[thread overview]
Message-ID: <5526593E.4040608@mojatatu.com> (raw)
In-Reply-To: <5525EC69.1080606@plumgrid.com>

On 04/08/15 23:05, Alexei Starovoitov wrote:
[..]
> As I said I'd rather disable them attaching to ingress than force
> all program authors to always use network_offset.
> Offset 0 points to L2 was always fundamental assumption of BPF.

The issue is not really the ingress qdisc here - rather the ingress
qdisc tries to cope with _how things work_.
The challenge is *some netdevs* (not all) strip off the link
layer _before_ the ingres qdisc sees them. You have to recognize
those devices and only speacial case with them. Your assumptions of
blindly pushing/pulling will break  in some cases (take a look at
mirred).

Having said that:
why could you not use the AT bits to identify whether you are
being invoked at ingress in the ebpf cls/ac wrapper? IOW:
have bpf cls/act be responsible for that knowledge.
Your changes penalize everyone else because of this assumption
bpf makes. We have always tried to be sensitive to perfomance.
People like Eric D. complain about a single if statement in this
code path and you are adding a bunch more unconditionally.
If all i wanted to do was police or account (a good number of use
cases) suddenly this gets more costly.

cheers,
jamal

  parent reply	other threads:[~2015-04-09 10:49 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-08 23:26 [PATCH v3 net-next 1/2] net: introduce skb_postpush_rcsum() helper Alexei Starovoitov
2015-04-08 23:26 ` [PATCH v3 net-next 2/2] tc: add 'needs_l2' flag to ingress qdisc Alexei Starovoitov
2015-04-09  2:44   ` David Miller
2015-04-09  3:05     ` Alexei Starovoitov
2015-04-09  3:14       ` David Miller
2015-04-09  3:39         ` Alexei Starovoitov
2015-04-09  5:20           ` Alexei Starovoitov
2015-04-09  5:25             ` David Miller
2015-04-09 15:15             ` Daniel Borkmann
2015-04-09 15:36               ` Eric Dumazet
2015-04-09 17:20                 ` Alexei Starovoitov
2015-04-09 10:49       ` Jamal Hadi Salim [this message]
2015-04-09 11:02         ` Jamal Hadi Salim
2015-04-09 15:38         ` Daniel Borkmann
2015-04-10 11:49           ` Jamal Hadi Salim
2015-04-09 17:03         ` Alexei Starovoitov
2015-04-10 12:48           ` Jamal Hadi Salim
2015-04-10 22:35             ` Alexei Starovoitov
2015-04-13 14:28               ` Jamal Hadi Salim
2015-04-13 16:13                 ` Alexei Starovoitov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5526593E.4040608@mojatatu.com \
    --to=jhs@mojatatu.com \
    --cc=ast@plumgrid.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=jiri@resnulli.us \
    --cc=netdev@vger.kernel.org \
    --cc=tgraf@suug.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.