> On Wed, 16 Oct 2019 13:55:06 +0300
> Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote:
>
> > On Wed, Oct 16, 2019 at 12:09:00PM +0200, Lorenzo Bianconi wrote:
> > > > On Mon, 14 Oct 2019 12:49:55 +0200, Lorenzo Bianconi wrote:
> > > > > Implement XDP_TX verdict and ndo_xdp_xmit net_device_ops function
> > > > > pointer
> > > > >
> > > > > Signed-off-by: Lorenzo Bianconi <lore...@kernel.org>
> > > >
> > > > > @@ -1972,6 +1975,109 @@ int mvneta_rx_refill_queue(struct mvneta_port
> > > > > *pp, struct mvneta_rx_queue *rxq)
> > > > > return i;
> > > > > }
> > > > >
> > > > > +static int
> > > > > +mvneta_xdp_submit_frame(struct mvneta_port *pp, struct
> > > > > mvneta_tx_queue *txq,
> > > > > + struct xdp_frame *xdpf, bool dma_map)
> > > > > +{
> > > > > + struct mvneta_tx_desc *tx_desc;
> > > > > + struct mvneta_tx_buf *buf;
> > > > > + dma_addr_t dma_addr;
> > > > > +
> > > > > + if (txq->count >= txq->tx_stop_threshold)
> > > > > + return MVNETA_XDP_CONSUMED;
> > > > > +
> > > > > + tx_desc = mvneta_txq_next_desc_get(txq);
> > > > > +
> > > > > + buf = &txq->buf[txq->txq_put_index];
> > > > > + if (dma_map) {
> > > > > + /* ndo_xdp_xmit */
> > > > > + dma_addr = dma_map_single(pp->dev->dev.parent,
> > > > > xdpf->data,
> > > > > + xdpf->len, DMA_TO_DEVICE);
> > > > > + if (dma_mapping_error(pp->dev->dev.parent, dma_addr)) {
> > > > > + mvneta_txq_desc_put(txq);
> > > > > + return MVNETA_XDP_CONSUMED;
> > > > > + }
> > > > > + buf->type = MVNETA_TYPE_XDP_NDO;
> > > > > + } else {
> > > > > + struct page *page = virt_to_page(xdpf->data);
> > > > > +
> > > > > + dma_addr = page_pool_get_dma_addr(page) +
> > > > > + pp->rx_offset_correction + MVNETA_MH_SIZE;
> > > > > + dma_sync_single_for_device(pp->dev->dev.parent,
> > > > > dma_addr,
> > > > > + xdpf->len,
> > > > > DMA_BIDIRECTIONAL);
> > > >
> > > > This looks a little suspicious, XDP could have moved the start of frame
> > > > with adjust_head, right? You should also use xdpf->data to find where
> > > > the frame starts, no?
> > >
> > > uhm, right..we need to update the dma_addr doing something like:
> > >
> > > dma_addr = page_pool_get_dma_addr(page) + xdpf->data - xdpf;
> >
> > Can we do page_pool_get_dma_addr(page) + xdpf->headroom as well right?
>
> We actually have to do:
> page_pool_get_dma_addr(page) + xdpf->headroom + sizeof(struct xdp_frame)
>
> As part of part of headroom was reserved to xdpf. I've considered
> several times to change xdpf->headroom to include sizeof(struct xdp_frame)
> as use-cases (e.g. in veth and cpumap) ended needing the "full" headroom.correct, I will add it in v4. Thx. Regards, Lorenzo > > -- > Best regards, > Jesper Dangaard Brouer > MSc.CS, Principal Kernel Engineer at Red Hat > LinkedIn: http://www.linkedin.com/in/brouer
signature.asc
Description: PGP signature
