On Wed, 26 Jun 2019 11:20:45 -0400 Willem de Bruijn <willemdebruijn.kernel@xxxxxxxxx> wrote: > On Wed, Jun 26, 2019 at 11:01 AM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote: > > Jesper Dangaard Brouer <brouer@xxxxxxxxxx> writes: > > > On Wed, 26 Jun 2019 13:52:16 +0200 > > > Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote: > > > > > >> Jesper Dangaard Brouer <brouer@xxxxxxxxxx> writes: > > >> [...] > > > > > > You touch upon some interesting complications already: > > > > > > 1. It is valuable for XDP bpf_prog to know "full" length? > > > (if so, then we need to extend xdp ctx with info) > > > > Valuable, quite likely. A hard requirement, probably not (for all use > > cases). > > Agreed. > > One common validation use would be to drop any packets whose header > length disagrees with the actual packet length. That is a good point. Added a section "XDP access to full packet length?" to capture this: - https://github.com/xdp-project/xdp-project/commit/da5b84264b85b0d - https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org#xdp-access-to-full-packet-length > > > But if we need to know the full length, when the first-buffer is > > > processed. Then realize that this affect the drivers RX-loop, because > > > then we need to "collect" all the buffers before we can know the > > > length (although some HW provide this in first descriptor). > > > > > > We likely have to change drivers RX-loop anyhow, as XDP_TX and > > > XDP_REDIRECT will also need to "collect" all buffers before the packet > > > can be forwarded. (Although this could potentially happen later in > > > driver loop when it meet/find the End-Of-Packet descriptor bit). > > Yes, this might be quite a bit of refactoring of device driver code. > > Should we move forward with some initial constraints, e.g., no > XDP_REDIRECT, no "full" length and no bpf_xdp_adjust_tail? I generally like this... If not adding "full" length. Maybe we could add an indication to XDP-developer, that his is a multi-buffer/multi-segment packet, such that header length validation code against packet length (data_end-data) is not possible. This is user visible, so we would have to keep it forever... I'm leaning towards adding "full" length from beginning. > That already allows many useful programs. > > As long as we don't arrive at a design that cannot be extended with > those features later. That is the important part... > > > 2. Can we even allow helper bpf_xdp_adjust_tail() ? [...] > > > > > Perhaps it is better to let bpf_xdp_adjust_tail() fail runtime? > > > > If we do disallow it, I think I'd lean towards failing the call at > > runtime... > > Disagree. I'd rather have a program fail at load if it depends on > multi-frag support while the (driver) implementation does not yet > support it. I usually agree that we should fail the program, early at load time. For XDP we are unfortunately missing some knobs to do this, see[1]. Specifically for bpf_xdp_adjust_tail(), it might be better to fail runtime. Because, the driver might have enabled TSO for TCP packets, while your XDP use-case is for adjusting UDP-packets (and do XDP level replies), which will never see multi-buffer packets. If we fail use of bpf_xdp_adjust_tail(), then you would have to disable TSO to allow loading your XDP-prog, hurting the other TSO-TCP use-case. [1] http://vger.kernel.org/netconf2019_files/xdp-feature-detection.pdf -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
|