[Intel-wired-lan] [PATCH intel-net] i40e: fix broken XDP support
Jesper Dangaard Brouer
brouer at redhat.com
Thu Apr 29 09:10:56 UTC 2021
Hi Tony, (+ Kuba and DaveM),
What is the status on this patch[2] that fixes a crash[1] for i40e driver?
I'm getting offlist and internal IRC questions to why i40e doesn't
work, and I noticed that it seems this have not been applied.
I don't see it in net-next or net tree... would it make sense to route
this via DaveM, or does it depend on the other fixes for i40e.
[1] https://lore.kernel.org/netdev/20210422170508.22c58226@carbon/
[2] https://patchwork.kernel.org/project/netdevbpf/patch/20210426111401.28369-1-magnus.karlsson@gmail.com/
(top-post)
On Mon, 26 Apr 2021 13:14:01 +0200
Magnus Karlsson <magnus.karlsson at gmail.com> wrote:
> From: Magnus Karlsson <magnus.karlsson at intel.com>
>
> Commit 12738ac4754e ("i40e: Fix sparse errors in i40e_txrx.c") broke
> XDP support in the i40e driver. That commit was fixing a sparse error
> in the code by introducing a new variable xdp_res instead of
> overloading this into the skb pointer. The problem is that the code
> later uses the skb pointer in if statements and these where not
> extended to also test for the new xdp_res variable. Fix this by adding
> the correct tests for xdp_res in these places.
>
> The skb pointer was used to store the result of the XDP program by
> overloading the results in the errror pointer
> ERR_PTR(-result). Therefore, the allocation failure test that used to
> only test for !skb now need to be extended to also consider !xdp_res.
>
> i40e_cleanup_headers() had a check that based on the skb value being
> an error pointer, i.e. a result from the XDP program != XDP_PASS, and
> if so start to process a new packet immediately, instead of populating
> skb fields and sending the skb to the stack. This check is not needed
> anymore, since we have added an explicit test for xdp_res being set
> and if so just do continue to pick the next packet from the NIC.
>
> v1 -> v2:
>
> * Improved commit message.
>
> * Restored the xdp_res = 0 initialization to its original place
> outside the per-packet loop. The original reason to move it inside
> the loop was that it was only initialized inside the loop code if
> skb was not set. But as skb can only be non-null if we have packets
> consisting of multiple frames (skb is set for all frames except the
> last one in a packet) and when this is true XDP cannot be active, so
> this does not matter. xdp_res == 0 is the same as I40E_XDP_PASS
> which is the default action if XDP is not active and it is then true
> for every single packet in this case.
>
> Fixes: 12738ac4754e ("i40e: Fix sparse errors in i40e_txrx.c")
> Acked-by: Jesper Dangaard Brouer <brouer at redhat.com>
> Tested-by: Jesper Dangaard Brouer <brouer at redhat.com>
> Reported-by: Jesper Dangaard Brouer <brouer at redhat.com>
> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski at intel.com>
> Signed-off-by: Magnus Karlsson <magnus.karlsson at intel.com>
> ---
> drivers/net/ethernet/intel/i40e/i40e_txrx.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
More information about the Intel-wired-lan
mailing list