netdev - RE: [PATCH 2/2] dpaa2-eth: add XDP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 1 Mar 2019 16:42:21 +0000
From:   Ioana Ciornei <ioana.ciornei@....com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>
CC:     Ioana Ciocoi Radulescu <ruxandra.radulescu@....com>,
        "brouer@...hat.com" <brouer@...hat.com>,
        "ilias.apalodimas@...aro.org" <ilias.apalodimas@...aro.org>
Subject: RE: [PATCH 2/2] dpaa2-eth: add XDP_REDIRECT support

> Subject: Re: [PATCH 2/2] dpaa2-eth: add XDP_REDIRECT support
> 
> Ioana Ciornei <ioana.ciornei@....com> writes:
> 
> > From: Ioana Radulescu <ruxandra.radulescu@....com>
> >
> > 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 <ioana.ciornei@....com>
> > Signed-off-by: Ioana Radulescu <ruxandra.radulescu@....com>
> > ---
> >  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