On Wed, 2020-10-07 at 16:31 +0200, Eric Dumazet wrote: > On Wed, Oct 7, 2020 at 4:09 PM Paolo Abeni <pab...@redhat.com> wrote: > > Hi, > > > > On Wed, 2020-10-07 at 01:42 -0700, Eric Dumazet wrote: > > > @@ -1232,9 +1233,10 @@ static rx_handler_result_t > > > macsec_handle_frame(struct sk_buff **pskb) > > > macsec_rxsc_put(rx_sc); > > > > > > skb_orphan(skb); > > > + len = skb->len; > > > ret = gro_cells_receive(&macsec->gro_cells, skb); > > > if (ret == NET_RX_SUCCESS) > > > - count_rx(dev, skb->len); > > > + count_rx(dev, len); > > > else > > > macsec->secy.netdev->stats.rx_dropped++; > > > > I'm sorry I'm low on coffee, but I can't see the race?!? here we are in > > a BH section, and the above code dereference the skb only if it's has > > been enqueued into the gro_cells napi. It could be dequeued/dropped > > only after we leave this section ?!? > > We should think of this as an alias for napi_gro_receive(), and not > make any assumptions. > Semantically the skb has been given to another layer. > netif_rx() can absolutely queue the skb to another cpu backlog (RPS, > RFS...), and the other cpu might have consumed the skb right away.
Ah! I completely missed that code path in gro_cells_receive()! Thank you for pointing that out! Acked-by: Paolo Abeni <pab...@redhat.com>
