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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 22 Dec 2022 07:58:02 -0800 From: William Tu <u9012063@...il.com> To: Alexander Duyck <alexander.duyck@...il.com> Cc: netdev@...r.kernel.org, tuc@...are.com, gyang@...are.com, doshir@...are.com Subject: Re: [PATCH v6] vmxnet3: Add XDP support. > > > > > + return nxmit; > > > > > +} > > > > > + > > > > > +static int > > > > > +__vmxnet3_run_xdp(struct vmxnet3_rx_queue *rq, void *data, int data_len, > > > > > + int headroom, int frame_sz, bool *need_xdp_flush, > > > > > + struct sk_buff *skb) > > > > > +{ > > > > > + struct xdp_frame *xdpf; > > > > > + void *buf_hard_start; > > > > > + struct xdp_buff xdp; > > > > > + struct page *page; > > > > > + void *orig_data; > > > > > + int err, delta; > > > > > + int delta_len; > > > > > + u32 act; > > > > > + > > > > > + buf_hard_start = data; > > > > > + xdp_init_buff(&xdp, frame_sz, &rq->xdp_rxq); > > > > > + xdp_prepare_buff(&xdp, buf_hard_start, headroom, data_len, true); > > > > > + orig_data = xdp.data; > > > > > + > > > > > + act = bpf_prog_run_xdp(rq->xdp_bpf_prog, &xdp); > > > > > + rq->stats.xdp_packets++; > > > > > + > > > > > + switch (act) { > > > > > + case XDP_DROP: > > > > > + rq->stats.xdp_drops++; > > > > > + break; > > > > > + case XDP_PASS: > > > > > + /* bpf prog might change len and data position. > > > > > + * dataring does not use skb so not support this. > > > > > + */ > > > > > + delta = xdp.data - orig_data; > > > > > + delta_len = (xdp.data_end - xdp.data) - data_len; > > > > > + if (skb) { > > > > > + skb_reserve(skb, delta); > > > > > + skb_put(skb, delta_len); > > > > > + } > > > > > + break; > > > > > + case XDP_TX: > > > > > + xdpf = xdp_convert_buff_to_frame(&xdp); > > > > > + if (!xdpf || > > > > > + vmxnet3_xdp_xmit_back(rq->adapter, xdpf)) { > > > > > + rq->stats.xdp_drops++; > > > > > + } else { > > > > > + rq->stats.xdp_tx++; > > > > > + } > > > > > + break; > > > > > + case XDP_ABORTED: > > > > > + trace_xdp_exception(rq->adapter->netdev, rq->xdp_bpf_prog, > > > > > + act); > > > > > + rq->stats.xdp_aborted++; > > > > > + break; > > > > > + case XDP_REDIRECT: > > > > > + page = alloc_page(GFP_ATOMIC); > > > > > + if (!page) { > > > > > + rq->stats.rx_buf_alloc_failure++; > > > > > + return XDP_DROP; > > > > > + } > > > > > > > > So I think I see the problem I had questions about here. If I am not > > > > mistaken you are copying the buffer to this page, and then copying this > > > > page to an skb right? I think you might be better off just writing off > > > > the Tx/Redirect pages and letting them go through their respective > > > > paths and just allocating new pages instead assuming these pages were > > > > consumed. Hi Alexander, I tried to implement your suggestion above: remove the alloc_page, let the original pages go through the redirect path (the xdp_do_redirect), and then allocate new buffer to refill rx ring but hit some memleak issue. The XDP_PASS/DROP/TX are good without leak, but not REDIRECT. I sent out v7, do you mind taking another look? Thanks! William > > > > > > I'm not sure I understand, can you elaborate? > > > > > > For XDP_TX, I'm doing 1 extra copy, copying to the newly allocated skb > > > in vmxnet3_xdp_xmit_back. > > > For XDP_REDIREC, I allocate a page and copy to the page and call > > > xdp_do_redirect, there is no copying to skb again. If I don't allocate a > > > new page, it always crashes at > > > [62020.425932] BUG: Bad page state in process cpumap/0/map:29 pfn:107548 > > > [62020.440905] kernel BUG at include/linux/mm.h:757! > > > VM_BUG_ON_PAGE(page_ref_count(page) == 0, page); > > > > What I was referring to was one copy here, and then another copy in > > your vmxnet3_xdp_xmit_frame() function with the page you allocated > > here possibly being leaked. > > > > Though based on the other trace you provided it would look like you > > are redirecting to something else as your code currently doesn't > > reference xdp_return_frame which is what I was referring to earlier in > > terms of missing the logic to free the frame in your transmit path. > yes, I tried a couple ways and now it's working. > basically I have to call get_page to increment the refcnt then call > xdp_return_frame. I will update performance number and send > next version. > > Thanks > William
Powered by blists - more mailing lists