On Mon, 3 Jun 2019 21:20:30 +0000
Saeed Mahameed [off-list ref] wrote:

On Mon, 2019-06-03 at 11:04 +0200, Björn Töpel wrote:

quoted

On Sat, 1 Jun 2019 at 21:42, Jakub Kicinski
[off-list ref] wrote:  

quoted

On Fri, 31 May 2019 19:18:17 +0000, Saeed Mahameed wrote:  

quoted

On Fri, 2019-05-31 at 11:42 +0200, Björn Töpel wrote:  

quoted

From: Björn Töpel <redacted>

All XDP capable drivers need to implement the
XDP_QUERY_PROG{,_HW} command of ndo_bpf. The query code is
fairly generic. This commit refactors the query code up from
the drivers to the netdev level.

The struct net_device has gained two new members: xdp_prog_hw
and xdp_flags. The former is the offloaded XDP program, if
any, and the latter tracks the flags that the supplied when
attaching the XDP  program. The flags only apply to SKB_MODE
or DRV_MODE, not HW_MODE.

The xdp_prog member, previously only used for SKB_MODE, is
shared with DRV_MODE. This is OK, due to the fact that
SKB_MODE and DRV_MODE are mutually exclusive. To
differentiate between the two modes, a new internal flag is
introduced as well.  

Just thinking out loud, why can't we allow any combination of
HW/DRV/SKB modes? they are totally different attach points in a
totally different checkpoints in a frame life cycle.  

The DRV_MODE and SKB_MODE is tricky to run concurrently. The XDP-redirect
scheme (designed by John) use a BPF helper (bpf_xdp_redirect_map) that
set global per-CPU state (this_cpu_ptr(&bpf_redirect_info)).

The tricky part (which I warned about, and we already have some fixes
for) is that the XDP/BPF-prog can call bpf_redirect_map, which update
the per-CPU state, but it can still choose not to return XDP_REDIRECT,
which then miss an invocation of xdp_do_redirect().  
 Later, your SKB_MODE XDP/BPF-prog can return XDP_REDIRECT without
calling the helper, and then use/reach to the per-CPU info set by the
DRV_MODE program, which is NOT something I want us to "support".

quoted

quoted

FWIW see Message-ID: [ref]

I've always seen the SKB-mode as something that will eventually be
removed.

I don't think so, we are too deep into SKB-mode.

I wish we could remove it.  After we got native XDP support in veth,
then its original purpose is gone, which were making it easier for
developers to get something working on their laptop, without having to
deploy it to the server with physical hardware all the time.

quoted

Clickable link:
 https://lore.kernel.org/netdev/20190201080236.446d84d4@redhat.com/ (local)

So we are all hanging on Jesper's refactoring ideas that are not
getting any priority for now :).

Well, that is not true.  This patchset _is_ the refactor idea that
Bjørn is taking over and working on.  Specifically [2] from above link.

[2] https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp_per_rxq01.org#refactor-idea-xdpbpf_prog-into-netdev_rx_queuenet_device
 

quoted

quoted

quoted

Down the road i think we will utilize this fact and start
introducing SKB helpers for SKB mode and driver helpers for DRV
mode..  

Any reason why we would want the extra complexity?  There is
cls_bpf if someone wants skb features after all..  

Donno, SKB mode is earlier in the stack maybe .. 

From a BPF perspective you cannot introduce SKB helpers for SKB mode.
An XDP-prog have bpf prog type XDP (BPF_PROG_TYPE_XDP), and the program
itself is identical regardless of attaching for DRV_MODE or SKB_MODE.
You cannot detect this at attach time, due to tail-calls (which have
painted us into a corner).

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer