Re: [PATCH bpf-next 1/4] xdp: Support specifying expected existing program when attaching XDP

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 20 Mar 2020 22:55:43 +0100 Daniel Borkmann wrote:
> >> Another aspect that falls into this atomic replacement is also that the programs can
> >> actually be atomically replaced at runtime. Last time I looked, some drivers still do
> >> a down/up cycle on replacement and hence traffic would be interrupted. I would argue
> >> that such /atomic/ swap operation on bpf_link would cover a guarantee of not having to
> >> perform this as well (workaround today would be a simple tail call map as entry point).  
> > 
> > I don't think that's the case. Drivers generally have a fast path
> > for the active-active replace.
> > 
> > Up/Down is only done to remap DMA buffers and change RX buffer
> > allocation scheme. That's when program is installed or removed,
> > not replaced.  
> 
> I know; though it seems not all adhere to that scheme sadly. I don't have that HW so can
> only judge on the code, but one example that looked suspicious enough to me is qede_xdp().
> It calls qede_xdp_set(), which does a qede_reload() for /every/ prog update. The latter
> basically does ...
> 
>      if (edev->state == QEDE_STATE_OPEN) {
>          qede_unload(edev, QEDE_UNLOAD_NORMAL, true);
>          if (args)
>              args->func(edev, args);               <-- prog replace here
>          qede_load(edev, QEDE_LOAD_RELOAD, true);
>          [...]
>      }

Ack, one day maybe we can restructure things enough so that drivers
don't have to copy/paste this dance :(

> ... now that is one driver. I haven't checked all the others (aside from i40e/ixgbe/mlx4/
> mlx5/nfp), but in any case it's also fixable in the driver w/o the extra need for bpf_link.

Agreed



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux