> Subject: Re: [PATCH 2/2] dpaa2-eth: add XDP_REDIRECT support
> 
> Ioana Ciornei <[email protected]> writes:
> 
> > From: Ioana Radulescu <[email protected]>
> >
> > Implement support for the XDP_REDIRECT action.
> >
> > The redirected frame is transmitted and confirmed on the regular Tx/Tx
> > conf queues. Frame is marked with the "XDP" type in the software
> > annotation, since it requires special treatment.
> >
> > We don't have good hardware support for TX batching, so the
> > XDP_XMIT_FLUSH flag doesn't make a difference for now; ndo_xdp_xmit
> > performs the actual Tx operation on the spot.
> >
> > Signed-off-by: Ioana Ciornei <[email protected]>
> > Signed-off-by: Ioana Radulescu <[email protected]>
> > ---
> >  drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c   | 171
> +++++++++++++++++++--
> >  drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h   |  13 ++
> >  .../net/ethernet/freescale/dpaa2/dpaa2-ethtool.c   |   1 +
> >  3 files changed, 170 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > index 3acfd8c..fbd2f82 100644
> > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > @@ -296,6 +296,7 @@ static u32 run_xdp(struct dpaa2_eth_priv *priv,
> >     xdp.data_end = xdp.data + dpaa2_fd_get_len(fd);
> >     xdp.data_hard_start = xdp.data - XDP_PACKET_HEADROOM;
> >     xdp_set_data_meta_invalid(&xdp);
> > +   xdp.rxq = &ch->xdp_rxq;
> >
> >     xdp_act = bpf_prog_run_xdp(xdp_prog, &xdp);
> >
> > @@ -328,6 +329,17 @@ static u32 run_xdp(struct dpaa2_eth_priv *priv,
> >             xdp_release_buf(priv, ch, addr);
> >             ch->stats.xdp_drop++;
> >             break;
> > +   case XDP_REDIRECT:
> > +           dma_unmap_page(priv->net_dev->dev.parent, addr,
> > +                          DPAA2_ETH_RX_BUF_SIZE, DMA_BIDIRECTIONAL);
> > +           ch->buf_count--;
> > +           xdp.data_hard_start = vaddr;
> > +           err = xdp_do_redirect(priv->net_dev, &xdp, xdp_prog);
> > +           if (unlikely(err))
> > +                   ch->stats.xdp_drop++;
> > +           else
> > +                   ch->stats.xdp_redirect++;
> > +           break;
> >     }
> 
> You're missing a call to xdp_do_flush_map() somewhere; things are not going to
> work properly without it. From a quick perusal of dpaa2-eth.c I would guess it
> should be somewhere near the end of dpaa2_eth_poll()...
> 

Indeed we are missing the xdp_do_flush_map() call and did not catch this, 
unfortunately, because we are disregarding it on the XDP Tx path since our 
hardware does not have good batching support. The dpaa2_eth_poll() function is 
the place to add it.

We'll do so in the next iteration. Thanks for pointing this out.

Ioana C

Reply via email to