netdev - Re: [net-next 5/9] ice: Add support for AF

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 30 Oct 2019 23:04:06 +0100
From:   Maciej Fijalkowski <maciejromanfijalkowski@...il.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     Jeff Kirsher <jeffrey.t.kirsher@...el.com>, davem@...emloft.net,
        Krzysztof Kazimierczak <krzysztof.kazimierczak@...el.com>,
        netdev@...r.kernel.org, nhorman@...hat.com, sassmann@...hat.com,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
        Tony Nguyen <anthony.l.nguyen@...el.com>,
        Andrew Bowers <andrewx.bowers@...el.com>
Subject: Re: [net-next 5/9] ice: Add support for AF_XDP

On Wed, 30 Oct 2019 11:50:09 -0700
Jakub Kicinski <jakub.kicinski@...ronome.com> wrote:

> On Tue, 29 Oct 2019 20:29:06 -0700, Jeff Kirsher wrote:
> > +/**
> > + * ice_run_xdp_zc - Executes an XDP program in zero-copy path
> > + * @rx_ring: Rx ring
> > + * @xdp: xdp_buff used as input to the XDP program
> > + *
> > + * Returns any of ICE_XDP_{PASS, CONSUMED, TX, REDIR}
> > + */
> > +static int
> > +ice_run_xdp_zc(struct ice_ring *rx_ring, struct xdp_buff *xdp)
> > +{
> > +	int err, result = ICE_XDP_PASS;
> > +	struct bpf_prog *xdp_prog;
> > +	struct ice_ring *xdp_ring;
> > +	u32 act;
> > +
> > +	rcu_read_lock();
> > +	xdp_prog = READ_ONCE(rx_ring->xdp_prog);
> > +	if (!xdp_prog) {
> > +		rcu_read_unlock();
> > +		return ICE_XDP_PASS;
> > +	}
> > +
> > +	act = bpf_prog_run_xdp(xdp_prog, xdp);
> > +	xdp->handle += xdp->data - xdp->data_hard_start;
> > +	switch (act) {
> > +	case XDP_PASS:
> > +		break;
> > +	case XDP_TX:
> > +		xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->q_index];
> > +		result = ice_xmit_xdp_buff(xdp, xdp_ring);
> > +		break;  
> 
> From the quick look at the code it wasn't clear to me how you deal with
> XDP_TX on ZC frames. Could you describe the flow of such frames a
> little bit?

Sure, here we go.
The ice_xmit_xdp_buff() is calling a convert_to_xdp_frame(xdp) which in that
case falls to the xdp_convert_zc_to_xdp_frame(xdp), because xdp->rxq->mem.type
is set to MEM_TYPE_ZERO_COPY. 

Now we are in the xdp_convert_zc_to_xdp_frame(xdp), where we allocate the
extra page dedicated for struct xdp_frame, copy into this page the data that are
within the current struct xdp_buff and everything is finished with a
xdp_return_buff(xdp) call. Again, the mem->type is the MEM_TYPE_ZERO_COPY so
the callback for freeing ZC frames is invoked, via:

xa->zc_alloc->free(xa->zc_alloc, handle);

And this guy was set during the Rx context initialization:

ring->zca.free = ice_zca_free;
err = xdp_rxq_info_reg_mem_model(&ring->xdp_rxq,
				 MEM_TYPE_ZERO_COPY,
				 &ring->zca);

Finally, the ice_zca_free will put the ZC frame back to HW Rx ring. At this
point, the ZC frame was recycled and the content from that frame sit in the
standalone page, represented by struct xdp_frame, so we are free to proceed
with transmission and that extra page will be freed during the cleanup of Tx
irq, after it got transmitted successfully.

I might over-complicate this description with too much code examples, so let me
know if that makes sense to you.

Maciej

> 
> > +	case XDP_REDIRECT:
> > +		err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog);
> > +		result = !err ? ICE_XDP_REDIR : ICE_XDP_CONSUMED;
> > +		break;
> > +	default:
> > +		bpf_warn_invalid_xdp_action(act);
> > +		/* fallthrough -- not supported action */
> > +	case XDP_ABORTED:
> > +		trace_xdp_exception(rx_ring->netdev, xdp_prog, act);
> > +		/* fallthrough -- handle aborts by dropping frame */
> > +	case XDP_DROP:
> > +		result = ICE_XDP_CONSUMED;
> > +		break;
> > +	}
> > +
> > +	rcu_read_unlock();
> > +	return result;
> > +}  
>