| Message ID | 20230418065327.72281-9-xuanzhuo@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | virtio_net: refactor xdp codes | expand |
| Context | Check | Description |
|---|---|---|
| netdev/series_format | success | Posting correctly formatted |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 18 this patch: 18 |
| netdev/cc_maintainers | success | CCed 13 of 13 maintainers |
| netdev/build_clang | success | Errors and warnings before: 18 this patch: 18 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/deprecated_api | success | None detected |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 18 this patch: 18 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 91 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Tue, Apr 18, 2023 at 2:53 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote: > > virtnet_build_xdp_buff_mrg() and virtnet_xdp_handler() auto > release xdp shinfo then the caller no need to careful the xdp shinfo. > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > --- > drivers/net/virtio_net.c | 29 +++++++++++++++++------------ > 1 file changed, 17 insertions(+), 12 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index e2eade87d2d4..266c1670beda 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -834,7 +834,7 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp, > xdpf = xdp_convert_buff_to_frame(xdp); > if (unlikely(!xdpf)) { > netdev_dbg(dev, "convert buff to frame failed for xdp\n"); > - return VIRTNET_XDP_RES_DROP; > + goto drop; > } > > err = virtnet_xdp_xmit(dev, 1, &xdpf, 0); > @@ -842,7 +842,7 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp, > xdp_return_frame_rx_napi(xdpf); > } else if (unlikely(err < 0)) { > trace_xdp_exception(dev, xdp_prog, act); > - return VIRTNET_XDP_RES_DROP; > + goto drop; > } > > *xdp_xmit |= VIRTIO_XDP_TX; > @@ -852,7 +852,7 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp, > stats->xdp_redirects++; > err = xdp_do_redirect(dev, xdp, xdp_prog); > if (err) > - return VIRTNET_XDP_RES_DROP; > + goto drop; > > *xdp_xmit |= VIRTIO_XDP_REDIR; > return VIRTNET_XDP_RES_CONSUMED; > @@ -864,8 +864,12 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp, > trace_xdp_exception(dev, xdp_prog, act); > fallthrough; > case XDP_DROP: > - return VIRTNET_XDP_RES_DROP; > + break; > } > + > +drop: > + put_xdp_frags(xdp); > + return VIRTNET_XDP_RES_DROP; > } Patch looks correct but we end up some inconsistency here. frags are automatically released. but the page still needs to be freed by the caller? Thanks > > static unsigned int virtnet_get_headroom(struct virtnet_info *vi) > @@ -1201,7 +1205,7 @@ static int virtnet_build_xdp_buff_mrg(struct net_device *dev, > dev->name, *num_buf, > virtio16_to_cpu(vi->vdev, hdr->num_buffers)); > dev->stats.rx_length_errors++; > - return -EINVAL; > + goto err; > } > > stats->bytes += len; > @@ -1220,7 +1224,7 @@ static int virtnet_build_xdp_buff_mrg(struct net_device *dev, > pr_debug("%s: rx error: len %u exceeds truesize %lu\n", > dev->name, len, (unsigned long)(truesize - room)); > dev->stats.rx_length_errors++; > - return -EINVAL; > + goto err; > } > > frag = &shinfo->frags[shinfo->nr_frags++]; > @@ -1235,6 +1239,10 @@ static int virtnet_build_xdp_buff_mrg(struct net_device *dev, > > *xdp_frags_truesize = xdp_frags_truesz; > return 0; > + > +err: > + put_xdp_frags(xdp); > + return -EINVAL; > } > > static void *mergeable_xdp_prepare(struct virtnet_info *vi, > @@ -1364,7 +1372,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); > > @@ -1372,7 +1380,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); > if (unlikely(!head_skb)) > - goto err_xdp_frags; > + goto err_xdp; > > rcu_read_unlock(); > return head_skb; > @@ -1382,11 +1390,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > goto xdp_xmit; > > case VIRTNET_XDP_RES_DROP: > - break; > + goto err_xdp; > } > -err_xdp_frags: > - put_xdp_frags(&xdp); > - goto err_xdp; > } > rcu_read_unlock(); > > -- > 2.32.0.3.g01195cf9f >
On Thu, 20 Apr 2023 13:59:30 +0800, Jason Wang <jasowang@redhat.com> wrote: > On Tue, Apr 18, 2023 at 2:53 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote: > > > > virtnet_build_xdp_buff_mrg() and virtnet_xdp_handler() auto > > release xdp shinfo then the caller no need to careful the xdp shinfo. > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > > --- > > drivers/net/virtio_net.c | 29 +++++++++++++++++------------ > > 1 file changed, 17 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > > index e2eade87d2d4..266c1670beda 100644 > > --- a/drivers/net/virtio_net.c > > +++ b/drivers/net/virtio_net.c > > @@ -834,7 +834,7 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp, > > xdpf = xdp_convert_buff_to_frame(xdp); > > if (unlikely(!xdpf)) { > > netdev_dbg(dev, "convert buff to frame failed for xdp\n"); > > - return VIRTNET_XDP_RES_DROP; > > + goto drop; > > } > > > > err = virtnet_xdp_xmit(dev, 1, &xdpf, 0); > > @@ -842,7 +842,7 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp, > > xdp_return_frame_rx_napi(xdpf); > > } else if (unlikely(err < 0)) { > > trace_xdp_exception(dev, xdp_prog, act); > > - return VIRTNET_XDP_RES_DROP; > > + goto drop; > > } > > > > *xdp_xmit |= VIRTIO_XDP_TX; > > @@ -852,7 +852,7 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp, > > stats->xdp_redirects++; > > err = xdp_do_redirect(dev, xdp, xdp_prog); > > if (err) > > - return VIRTNET_XDP_RES_DROP; > > + goto drop; > > > > *xdp_xmit |= VIRTIO_XDP_REDIR; > > return VIRTNET_XDP_RES_CONSUMED; > > @@ -864,8 +864,12 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp, > > trace_xdp_exception(dev, xdp_prog, act); > > fallthrough; > > case XDP_DROP: > > - return VIRTNET_XDP_RES_DROP; > > + break; > > } > > + > > +drop: > > + put_xdp_frags(xdp); > > + return VIRTNET_XDP_RES_DROP; > > } > > Patch looks correct but we end up some inconsistency here. > > frags are automatically released. > > but the page still needs to be freed by the caller? Yes. Do you want to auto free page? Thanks. > > Thanks > > > > > > static unsigned int virtnet_get_headroom(struct virtnet_info *vi) > > @@ -1201,7 +1205,7 @@ static int virtnet_build_xdp_buff_mrg(struct net_device *dev, > > dev->name, *num_buf, > > virtio16_to_cpu(vi->vdev, hdr->num_buffers)); > > dev->stats.rx_length_errors++; > > - return -EINVAL; > > + goto err; > > } > > > > stats->bytes += len; > > @@ -1220,7 +1224,7 @@ static int virtnet_build_xdp_buff_mrg(struct net_device *dev, > > pr_debug("%s: rx error: len %u exceeds truesize %lu\n", > > dev->name, len, (unsigned long)(truesize - room)); > > dev->stats.rx_length_errors++; > > - return -EINVAL; > > + goto err; > > } > > > > frag = &shinfo->frags[shinfo->nr_frags++]; > > @@ -1235,6 +1239,10 @@ static int virtnet_build_xdp_buff_mrg(struct net_device *dev, > > > > *xdp_frags_truesize = xdp_frags_truesz; > > return 0; > > + > > +err: > > + put_xdp_frags(xdp); > > + return -EINVAL; > > } > > > > static void *mergeable_xdp_prepare(struct virtnet_info *vi, > > @@ -1364,7 +1372,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); > > > > @@ -1372,7 +1380,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); > > if (unlikely(!head_skb)) > > - goto err_xdp_frags; > > + goto err_xdp; > > > > rcu_read_unlock(); > > return head_skb; > > @@ -1382,11 +1390,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > > goto xdp_xmit; > > > > case VIRTNET_XDP_RES_DROP: > > - break; > > + goto err_xdp; > > } > > -err_xdp_frags: > > - put_xdp_frags(&xdp); > > - goto err_xdp; > > } > > rcu_read_unlock(); > > > > -- > > 2.32.0.3.g01195cf9f > > >
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index e2eade87d2d4..266c1670beda 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -834,7 +834,7 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp, xdpf = xdp_convert_buff_to_frame(xdp); if (unlikely(!xdpf)) { netdev_dbg(dev, "convert buff to frame failed for xdp\n"); - return VIRTNET_XDP_RES_DROP; + goto drop; } err = virtnet_xdp_xmit(dev, 1, &xdpf, 0); @@ -842,7 +842,7 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp, xdp_return_frame_rx_napi(xdpf); } else if (unlikely(err < 0)) { trace_xdp_exception(dev, xdp_prog, act); - return VIRTNET_XDP_RES_DROP; + goto drop; } *xdp_xmit |= VIRTIO_XDP_TX; @@ -852,7 +852,7 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp, stats->xdp_redirects++; err = xdp_do_redirect(dev, xdp, xdp_prog); if (err) - return VIRTNET_XDP_RES_DROP; + goto drop; *xdp_xmit |= VIRTIO_XDP_REDIR; return VIRTNET_XDP_RES_CONSUMED; @@ -864,8 +864,12 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp, trace_xdp_exception(dev, xdp_prog, act); fallthrough; case XDP_DROP: - return VIRTNET_XDP_RES_DROP; + break; } + +drop: + put_xdp_frags(xdp); + return VIRTNET_XDP_RES_DROP; } static unsigned int virtnet_get_headroom(struct virtnet_info *vi) @@ -1201,7 +1205,7 @@ static int virtnet_build_xdp_buff_mrg(struct net_device *dev, dev->name, *num_buf, virtio16_to_cpu(vi->vdev, hdr->num_buffers)); dev->stats.rx_length_errors++; - return -EINVAL; + goto err; } stats->bytes += len; @@ -1220,7 +1224,7 @@ static int virtnet_build_xdp_buff_mrg(struct net_device *dev, pr_debug("%s: rx error: len %u exceeds truesize %lu\n", dev->name, len, (unsigned long)(truesize - room)); dev->stats.rx_length_errors++; - return -EINVAL; + goto err; } frag = &shinfo->frags[shinfo->nr_frags++]; @@ -1235,6 +1239,10 @@ static int virtnet_build_xdp_buff_mrg(struct net_device *dev, *xdp_frags_truesize = xdp_frags_truesz; return 0; + +err: + put_xdp_frags(xdp); + return -EINVAL; } static void *mergeable_xdp_prepare(struct virtnet_info *vi, @@ -1364,7 +1372,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); @@ -1372,7 +1380,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); if (unlikely(!head_skb)) - goto err_xdp_frags; + goto err_xdp; rcu_read_unlock(); return head_skb; @@ -1382,11 +1390,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, goto xdp_xmit; case VIRTNET_XDP_RES_DROP: - break; + goto err_xdp; } -err_xdp_frags: - put_xdp_frags(&xdp); - goto err_xdp; } rcu_read_unlock();
virtnet_build_xdp_buff_mrg() and virtnet_xdp_handler() auto release xdp shinfo then the caller no need to careful the xdp shinfo. Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> --- drivers/net/virtio_net.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-)