• Jakub Kicinski's avatar
    nfp: bpf: don't stop offload if replace failed · 68d676a0
    Jakub Kicinski authored
    Stopping offload completely if replace of program failed dates
    back to days of transparent offload.  Back then we wanted to
    silently fall back to the in-driver processing.  Today we mark
    programs for offload when they are loaded into the kernel, so
    the transparent offload is no longer a reality.
    
    Flags check in the driver will only allow replace of a driver
    program with another driver program or an offload program with
    another offload program.
    
    When driver program is replaced stopping offload is a no-op,
    because driver program isn't offloaded.  When replacing
    offloaded program if the offload fails the entire operation
    will fail all the way back to user space and we should continue
    using the old program.  IOW when replacing a driver program
    stopping offload is unnecessary and when replacing offloaded
    program - it's a bug, old program should continue to run.
    
    In practice this bug would mean that if offload operation was to
    fail (either due to FW communication error, kernel OOM or new
    program being offloaded but for a different netdev) driver
    would continue reporting that previous XDP program is offloaded
    but in fact no program will be loaded in hardware.  The failure
    is fairly unlikely (found by inspection, when working on the code)
    but it's unpleasant.
    
    Backport note: even though the bug was introduced in commit
    cafa92ac ("nfp: bpf: add support for XDP_FLAGS_HW_MODE"),
    this fix depends on commit 441a3303 ("net: xdp: don't allow
    device-bound programs in driver mode"), so this fix is sufficient
    only in v4.15 or newer.  Kernels v4.13.x and v4.14.x do need to
    stop offload if it was transparent/opportunistic, i.e. if
    XDP_FLAGS_HW_MODE was not set on running program.
    
    Fixes: cafa92ac ("nfp: bpf: add support for XDP_FLAGS_HW_MODE")
    Signed-off-by: default avatarJakub Kicinski <jakub.kicinski@netronome.com>
    Reviewed-by: default avatarQuentin Monnet <quentin.monnet@netronome.com>
    Acked-by: default avatarSong Liu <songliubraving@fb.com>
    Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
    68d676a0
main.c 12.1 KB