search for: err_xdp

...uff *head_skb; > + unsigned int frame_sz; > + struct xdp_buff xdp; > + void *data; > + u32 act; > + int err; > + > + data = mergeable_xdp_prepare(vi, rq, xdp_prog, ctx, &frame_sz, &num_buf, &page, > + offset, &len, hdr); > + if (!data) > + goto err_xdp; > + > + err = virtnet_build_xdp_buff_mrg(dev, vi, rq, &xdp, data, len, frame_sz, > + &num_buf, &xdp_frags_truesz, stats); > + if (unlikely(err)) > + goto err_xdp; > + > + act = virtnet_xdp_handler(xdp_prog, &xdp, dev, xdp_xmit, stats); > + > + switc...

Due to historical reasons, the implementation of XDP in virtio-net is relatively chaotic. For example, the processing of XDP actions has two copies of similar code. Such as page, xdp_page processing, etc. The purpose of this patch set is to refactor these code. Reduce the difficulty of subsequent maintenance. Subsequent developers will not introduce new bugs because of some complex logical

Due to historical reasons, the implementation of XDP in virtio-net is relatively chaotic. For example, the processing of XDP actions has two copies of similar code. Such as page, xdp_page processing, etc. The purpose of this patch set is to refactor these code. Reduce the difficulty of subsequent maintenance. Subsequent developers will not introduce new bugs because of some complex logical

Due to historical reasons, the implementation of XDP in virtio-net is relatively chaotic. For example, the processing of XDP actions has two copies of similar code. Such as page, xdp_page processing, etc. The purpose of this patch set is to refactor these code. Reduce the difficulty of subsequent maintenance. Subsequent developers will not introduce new bugs because of some complex logical

...*mergeable_xdp_prepare(struct virtnet_info *vi, @@ -1361,7 +1369,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, err = virtnet_build_xdp_buff_mrg(dev, vi, rq, &xdp, data, len, frame_sz, &num_buf, &xdp_frags_truesz, stats); if (unlikely(err)) - goto err_xdp_frags; + goto err_xdp; act = virtnet_xdp_handler(xdp_prog, &xdp, dev, xdp_xmit, stats); @@ -1369,7 +1377,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, case VIRTNET_XDP_RES_PASS: head_skb = build_skb_from_xdp_buff(dev, vi, &xdp, xdp_frags_truesz);...

...ct virtnet_info *vi, > @@ -1361,7 +1369,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > err = virtnet_build_xdp_buff_mrg(dev, vi, rq, &xdp, data, len, frame_sz, > &num_buf, &xdp_frags_truesz, stats); > if (unlikely(err)) > - goto err_xdp_frags; > + goto err_xdp; > > act = virtnet_xdp_handler(xdp_prog, &xdp, dev, xdp_xmit, stats); > > @@ -1369,7 +1377,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > case VIRTNET_XDP_RES_PASS: > head_skb = build_skb_from_xdp_buff(de...

Hi: Please review the patches that tries to fix sevreal issues of virtio-net mergeable XDP. Thanks Jason Wang (4): virtio-net: correctly redirect linearized packet virtio-net: correctly transmit XDP buff after linearizing virtio-net: reset num_buf to 1 after linearizing packet virito-net: fix leaking page for gso packet during mergeable XDP drivers/net/virtio_net.c | 21

...og_run_xdp(xdp_prog, &xdp); - if (act != XDP_PASS) - ewma_pkt_len_add(&rq->mrg_avg_pkt_len, len); - switch (act) { case XDP_PASS: /* recalculate offset to account for any header @@ -770,6 +774,19 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, goto err_xdp; rcu_read_unlock(); goto xdp_xmit; + case XDP_REDIRECT: + err = xdp_do_redirect(dev, &xdp, xdp_prog); + if (err) { + trace_xdp_exception(vi->dev, xdp_prog, act); + if (unlikely(xdp_page != page)) + put_page(xdp_page); + goto err_xdp; + } + *xdp_xmit = true; +...

...og_run_xdp(xdp_prog, &xdp); - if (act != XDP_PASS) - ewma_pkt_len_add(&rq->mrg_avg_pkt_len, len); - switch (act) { case XDP_PASS: /* recalculate offset to account for any header @@ -770,6 +774,19 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, goto err_xdp; rcu_read_unlock(); goto xdp_xmit; + case XDP_REDIRECT: + err = xdp_do_redirect(dev, &xdp, xdp_prog); + if (err) { + trace_xdp_exception(vi->dev, xdp_prog, act); + if (unlikely(xdp_page != page)) + put_page(xdp_page); + goto err_xdp; + } + *xdp_xmit = true; +...

...xdp.rxq = &tfile->xdp_rxq; > - orig_data = xdp.data; > - act = bpf_prog_run_xdp(xdp_prog, &xdp); > - > - switch (act) { > - case XDP_REDIRECT: > - err = xdp_do_redirect(tun->dev, &xdp, xdp_prog); > - xdp_do_flush_map(); > - if (err) > - goto err_xdp; > - goto out; > - case XDP_TX: > - if (tun_xdp_tx(tun->dev, &xdp) < 0) > - goto err_xdp; > - goto out; > - case XDP_PASS: > - delta = orig_data - xdp.data; > - len = xdp.data_end - xdp.data; > - break; > - default: > - bpf_warn_invalid...

...xdp.rxq = &tfile->xdp_rxq; > - orig_data = xdp.data; > - act = bpf_prog_run_xdp(xdp_prog, &xdp); > - > - switch (act) { > - case XDP_REDIRECT: > - err = xdp_do_redirect(tun->dev, &xdp, xdp_prog); > - xdp_do_flush_map(); > - if (err) > - goto err_xdp; > - goto out; > - case XDP_TX: > - if (tun_xdp_tx(tun->dev, &xdp) < 0) > - goto err_xdp; > - goto out; > - case XDP_PASS: > - delta = orig_data - xdp.data; > - len = xdp.data_end - xdp.data; > - break; > - default: > - bpf_warn_invalid...

...xdp.data_meta; > break; > - case XDP_TX: > - stats->xdp_tx++; > - xdpf = xdp_convert_buff_to_frame(&xdp); > - if (unlikely(!xdpf)) > - goto err_xdp; > - err = virtnet_xdp_xmit(dev, 1, &xdpf, 0); > - if (unlikely(!err)) { > - xdp_return_frame_rx_napi(xdpf); > - } else if (unlikely(err < 0)) { > - tr...

...t pointer to be passed to > the function. So, I guess you mean this: switch (act) { case XDP_PASS: /* handle pass */ return skb; case XDP_TX: *xdp_xmit |= VIRTIO_XDP_TX; goto xmit; case XDP_REDIRECT: *xdp_xmit |= VIRTIO_XDP_REDIR; goto xmit; case XDP_DROP: default: goto err_xdp; } I have to say there is no problem from the perspective of code implementation. But if the a new ACTION liking XDP_TX,XDP_REDIRECT is added in the future, then we must modify all the callers. This is the benefit of using CUNSUMED. I think it is a good advantage to put xdp_xmit in virtnet_xdp_...

...e(alloc_frag->page); > - alloc_frag->offset += buflen; > err = xdp_do_redirect(tun->dev, &xdp, xdp_prog); > xdp_do_flush_map(); > if (err) > - goto err_redirect; > - rcu_read_unlock(); > - local_bh_enable(); > - return NULL; > + goto err_xdp; > + goto out; > case XDP_TX: > - get_page(alloc_frag->page); > - alloc_frag->offset += buflen; > if (tun_xdp_tx(tun->dev, &xdp) < 0) > - goto err_redirect; > - rcu_read_unlock(); > - local_bh_enable(); > - return NULL; > + goto...

...e(alloc_frag->page); > - alloc_frag->offset += buflen; > err = xdp_do_redirect(tun->dev, &xdp, xdp_prog); > xdp_do_flush_map(); > if (err) > - goto err_redirect; > - rcu_read_unlock(); > - local_bh_enable(); > - return NULL; > + goto err_xdp; > + goto out; > case XDP_TX: > - get_page(alloc_frag->page); > - alloc_frag->offset += buflen; > if (tun_xdp_tx(tun->dev, &xdp) < 0) > - goto err_redirect; > - rcu_read_unlock(); > - local_bh_enable(); > - return NULL; > + goto...

...+ unsigned int xdp_frags_truesz = 0; + struct sk_buff *head_skb; + unsigned int frame_sz; + struct xdp_buff xdp; + void *data; + u32 act; + int err; + + data = mergeable_xdp_prepare(vi, rq, xdp_prog, ctx, &frame_sz, &num_buf, &page, + offset, &len, hdr); + if (!data) + goto err_xdp; + + err = virtnet_build_xdp_buff_mrg(dev, vi, rq, &xdp, data, len, frame_sz, + &num_buf, &xdp_frags_truesz, stats); + if (unlikely(err)) + goto err_xdp; + + act = virtnet_xdp_handler(xdp_prog, &xdp, dev, xdp_xmit, stats); + + switch (act) { + case VIRTNET_XDP_RES_PASS: + hea...

...to release. This is complicated for the processing of action, and be careful. In the entire process, we have such principles: * If xdp_page is used (PASS, TX, Redirect), then we release the old page. * If it is a drop case, we will release two. The old page obtained from buf is release inside err_xdp, and xdp_page needs be relased by us. But in fact, when we allocate a new page, we can release the old page immediately. Then just one is using, we just need to release the new page for drop case. On the drop path, err_xdp will release the variable "page", so we only need to let "pa...

...case XDP_TX: > *xdp_xmit |= VIRTIO_XDP_TX; > goto xmit; > > case XDP_REDIRECT: > *xdp_xmit |= VIRTIO_XDP_REDIR; > goto xmit; > > case XDP_DROP: > default: > goto err_xdp; > } > > I have to say there is no problem from the perspective of code implementation. Note that this is the current logic where it is determined in receive_small() and receive_mergeable(). > > But if the a new ACTION liking XDP_TX,XDP_REDIRECT is added in the future, then...

...p_xmit_one(vi, sq, xdpf); -} - static int virtnet_xdp_xmit(struct net_device *dev, int n, struct xdp_frame **frames, u32 flags) { @@ -707,10 +687,8 @@ static struct sk_buff *receive_small(struct net_device *dev, xdpf = convert_to_xdp_frame(&xdp); if (unlikely(!xdpf)) goto err_xdp; - stats->tx.xdp_tx++; - err = __virtnet_xdp_tx_xmit(vi, xdpf); - if (unlikely(err)) { - stats->tx.xdp_tx_drops++; + err = virtnet_xdp_xmit(dev, 1, &xdpf, 0); + if (unlikely(err < 0)) { trace_xdp_exception(vi->dev, xdp_prog, act); goto err_xdp; } @@ -879,1...

...p_xmit_one(vi, sq, xdpf); -} - static int virtnet_xdp_xmit(struct net_device *dev, int n, struct xdp_frame **frames, u32 flags) { @@ -707,10 +687,8 @@ static struct sk_buff *receive_small(struct net_device *dev, xdpf = convert_to_xdp_frame(&xdp); if (unlikely(!xdpf)) goto err_xdp; - stats->tx.xdp_tx++; - err = __virtnet_xdp_tx_xmit(vi, xdpf); - if (unlikely(err)) { - stats->tx.xdp_tx_drops++; + err = virtnet_xdp_xmit(dev, 1, &xdpf, 0); + if (unlikely(err < 0)) { trace_xdp_exception(vi->dev, xdp_prog, act); goto err_xdp; } @@ -879,1...