Netdev List
 help / color / mirror / Atom feed
From: John Fastabend <john.fastabend@gmail.com>
To: "David Ahern" <dsahern@gmail.com>,
	"John Fastabend" <john.fastabend@gmail.com>,
	"Toke Høiland-Jørgensen" <toke@redhat.com>,
	"David Ahern" <dsahern@kernel.org>,
	netdev@vger.kernel.org
Cc: davem@davemloft.net, kuba@kernel.org,
	prashantbhole.linux@gmail.com, brouer@redhat.com,
	daniel@iogearbox.net, ast@kernel.org, kafai@fb.com,
	songliubraving@fb.com, yhs@fb.com, andriin@fb.com,
	David Ahern <dahern@digitalocean.com>
Subject: Re: [PATCH v5 bpf-next 00/11] net: Add support for XDP in egress path
Date: Mon, 18 May 2020 23:04:47 -0700	[thread overview]
Message-ID: <5ec376ff1680_2e852b10123785b4ae@john-XPS-13-9370.notmuch> (raw)
In-Reply-To: <cdd04862-dafd-080e-e90e-5161e568bac3@gmail.com>

David Ahern wrote:
> On 5/18/20 12:10 PM, John Fastabend wrote:
> >>
> >> host ingress to VM is one use case; VM to VM on the same host is another.
> > 
> > But host ingress to VM would still work with tail calls because the XDP
> > packet came from another XDP program. At least that is how I understand
> > it.
> > 
> > VM to VM case, again using tail calls on the sending VM ingress hook
> > would work also.
> 
> understood. I realize I can attach the program array all around, I just
> see that as complex control plane / performance hit depending on how the
> programs are wired up.
> 

Hard to argue with out a specific program. I think it could go either way.
I'll concede the control plane might be more complex but not so convinced
about performance. Either way having a program attached to the life cycle
of the VM seems like something that would be nice to have. In the tc skb
case if we attach to a qdisc it is removed automatically when the device
is removed. Having something similar for xdp is probably a good thing.

Worth following up in Daniel's thread. Another way to do that instead of
having the program associated with the ifindex is to have it associated
with the devmap entry. Basically when we add an entry in the devmap if
we had a program fd associated with it they could both be released when
the devmap entry is removed. This will happen automatically if the ifindex
is removed. But, rather than fragment threads too much I'll wait for
Daniel's reply.

> >>
> >> With respect to lifecycle management of the programs and the data,
> >> putting VM specific programs and maps on VM specific taps simplifies
> >> management. VM terminates, taps are deleted, programs and maps
> >> disappear. So no validator thread needed to handle stray data / programs
> >> from the inevitable cleanup problems when everything is lumped into 1
> >> program / map or even array of programs and maps.
> > 
> > OK. Also presumably you already have a hook into this event to insert
> > the tc filter programs so its probably a natural hook for mgmt.
> 
> For VMs there is no reason to have an skb at all, so no tc filter program.

+1 nice win for sure.

> 
> > 
> >>
> >> To me the distributed approach is the simplest and best. The program on
> >> the host nics can be stupid simple; no packet parsing beyond the
> >> ethernet header. It's job is just a traffic demuxer very much like a
> >> switch. All VM logic and data is local to the VM's interfaces.
> > 
> > IMO it seems more natural and efficient to use a tail call. But, I
> > can see how if the ingress program is a l2/l3 switch and the VM hook
> > is a l2/l3 filter it feels more like a switch+firewall layout we
> > would normally use on a "real" (v)switch. Also I think the above point
> > where cleanup is free because of the tap tear down is a win.
> 
> exactly. To the VM. the host is part of the network. The host should be
> passing the packets as fast and as simply as possible from ingress nic
> to vm. It can be done completely as xdp frames and doing so reduces the
> CPU cycles per packet in the host (yes, there are caveats to that
> statement).
> 
> VM to host nic, and VM to VM have their own challenges which need to be
> tackled next.
> 
> But the end goal is to have all VM traffic touched by the host as xdp
> frames and without creating a complex control plane. The distributed
> approach is much simpler and cleaner - and seems to follow what Cilium
> is doing to a degree, or that is my interpretation of

+1 agree everything as xdp pkt is a great goal.

> 
> "By attaching to the TC ingress hook of the host side of this veth pair
> Cilium can monitor and enforce policy on all traffic exiting a
> container. By attaching a BPF program to the veth pair associated with
> each container and routing all network traffic to the host side virtual
> devices with another BPF program attached to the tc ingress hook as well
> Cilium can monitor and enforce policy on all traffic entering or exiting
> the node."
> 
> https://docs.cilium.io/en/v1.7/architecture/

In many configurations there are no egress hooks though because policy (the
firewall piece) is implemented as part of the ingress hook. Because the
ingress TC hook "knows" where it will redirect a packet it can also run
the policy logic for that pod/VM/etc.

  reply	other threads:[~2020-05-19  6:04 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-13  1:45 [PATCH v5 bpf-next 00/11] net: Add support for XDP in egress path David Ahern
2020-05-13  1:45 ` [PATCH v5 bpf-next 01/11] net: Refactor convert_to_xdp_frame David Ahern
2020-05-13  1:45 ` [PATCH v5 bpf-next 02/11] net: uapi for XDP programs in the egress path David Ahern
2020-05-13  1:45 ` [PATCH v5 bpf-next 03/11] net: Add XDP setup and query commands for Tx programs David Ahern
2020-05-13  1:46 ` [PATCH v5 bpf-next 04/11] net: Add BPF_XDP_EGRESS as a bpf_attach_type David Ahern
2020-05-13  1:46 ` [PATCH v5 bpf-next 05/11] xdp: Add xdp_txq_info to xdp_buff David Ahern
2020-05-13  1:46 ` [PATCH v5 bpf-next 06/11] net: set XDP egress program on netdevice David Ahern
2020-05-13  1:46 ` [PATCH v5 bpf-next 07/11] net: Support xdp in the Tx path for xdp_frames David Ahern
2020-05-13  1:46 ` [PATCH v5 bpf-next 08/11] libbpf: Add egress XDP support David Ahern
2020-05-13  1:46 ` [PATCH v5 bpf-next 09/11] bpftool: Add support for XDP egress David Ahern
2020-05-13  1:46 ` [PATCH v5 bpf-next 10/11] selftest: Add xdp_egress attach tests David Ahern
2020-05-13  1:46 ` [PATCH v5 bpf-next 11/11] samples/bpf: add XDP egress support to xdp1 David Ahern
2020-05-13 10:43 ` [PATCH v5 bpf-next 00/11] net: Add support for XDP in egress path Toke Høiland-Jørgensen
2020-05-13 19:37   ` David Ahern
2020-05-15 22:54     ` John Fastabend
2020-05-15 23:15       ` David Ahern
2020-05-18 18:10         ` John Fastabend
2020-05-18 23:52           ` David Ahern
2020-05-19  6:04             ` John Fastabend [this message]
2020-05-18  3:40       ` David Ahern
2020-05-18  9:08     ` Toke Høiland-Jørgensen
2020-05-18 14:44       ` David Ahern
2020-05-18 18:00         ` Toke Høiland-Jørgensen
2020-05-18 21:06           ` Daniel Borkmann
2020-05-19  0:02             ` David Ahern
2020-05-19 13:31               ` Daniel Borkmann
2020-05-19 14:21                 ` Jesper Dangaard Brouer
2020-05-19 16:58                   ` Lorenzo Bianconi
2020-05-19 14:52                 ` Toke Høiland-Jørgensen
2020-05-19 16:37                 ` David Ahern
2020-05-18 23:37           ` David Ahern
2020-05-18 21:23         ` Daniel Borkmann

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=5ec376ff1680_2e852b10123785b4ae@john-XPS-13-9370.notmuch \
    --to=john.fastabend@gmail.com \
    --cc=andriin@fb.com \
    --cc=ast@kernel.org \
    --cc=brouer@redhat.com \
    --cc=dahern@digitalocean.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=dsahern@kernel.org \
    --cc=kafai@fb.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=prashantbhole.linux@gmail.com \
    --cc=songliubraving@fb.com \
    --cc=toke@redhat.com \
    --cc=yhs@fb.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox