| Message ID | 1526891706-18516-5-git-send-email-jasowang@redhat.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | Fix several issues of virtio-net mergeable XDP | expand |
On Mon, May 21, 2018 at 04:35:06PM +0800, Jason Wang wrote: > We need to drop refcnt to xdp_page if we see a gso packet. Otherwise > it will be leaked. Fixing this by moving the check of gso packet above > the linearizing logic. > > Cc: John Fastabend <john.fastabend@gmail.com> > Fixes: 72979a6c3590 ("virtio_net: xdp, add slowpath case for non contiguous buffers") > Signed-off-by: Jason Wang <jasowang@redhat.com> typo in subject > --- > drivers/net/virtio_net.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 165a922..f8db809 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -707,6 +707,14 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > void *data; > u32 act; > > + /* Transient failure which in theory could occur if > + * in-flight packets from before XDP was enabled reach > + * the receive path after XDP is loaded. In practice I > + * was not able to create this condition. BTW we should probably drop the last sentence. It says in theory, should be enough. > + */ > + if (unlikely(hdr->hdr.gso_type)) > + goto err_xdp; > + > /* This happens when rx buffer size is underestimated > * or headroom is not enough because of the buffer > * was refilled before XDP is set. This should only > @@ -728,14 +736,6 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > xdp_page = page; > } > > - /* Transient failure which in theory could occur if > - * in-flight packets from before XDP was enabled reach > - * the receive path after XDP is loaded. In practice I > - * was not able to create this condition. > - */ > - if (unlikely(hdr->hdr.gso_type)) > - goto err_xdp; > - > /* Allow consuming headroom but reserve enough space to push > * the descriptor on if we get an XDP_TX return code. > */ > -- > 2.7.4
On 2018年05月21日 23:01, Michael S. Tsirkin wrote: > On Mon, May 21, 2018 at 04:35:06PM +0800, Jason Wang wrote: >> We need to drop refcnt to xdp_page if we see a gso packet. Otherwise >> it will be leaked. Fixing this by moving the check of gso packet above >> the linearizing logic. >> >> Cc: John Fastabend <john.fastabend@gmail.com> >> Fixes: 72979a6c3590 ("virtio_net: xdp, add slowpath case for non contiguous buffers") >> Signed-off-by: Jason Wang <jasowang@redhat.com> > typo in subject Let me fix it in V2. >> --- >> drivers/net/virtio_net.c | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c >> index 165a922..f8db809 100644 >> --- a/drivers/net/virtio_net.c >> +++ b/drivers/net/virtio_net.c >> @@ -707,6 +707,14 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, >> void *data; >> u32 act; >> >> + /* Transient failure which in theory could occur if >> + * in-flight packets from before XDP was enabled reach >> + * the receive path after XDP is loaded. In practice I >> + * was not able to create this condition. > BTW we should probably drop the last sentence. It says in theory, should be enough. Ok. Thanks >> + */ >> + if (unlikely(hdr->hdr.gso_type)) >> + goto err_xdp; >> + >> /* This happens when rx buffer size is underestimated >> * or headroom is not enough because of the buffer >> * was refilled before XDP is set. This should only >> @@ -728,14 +736,6 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, >> xdp_page = page; >> } >> >> - /* Transient failure which in theory could occur if >> - * in-flight packets from before XDP was enabled reach >> - * the receive path after XDP is loaded. In practice I >> - * was not able to create this condition. >> - */ >> - if (unlikely(hdr->hdr.gso_type)) >> - goto err_xdp; >> - >> /* Allow consuming headroom but reserve enough space to push >> * the descriptor on if we get an XDP_TX return code. >> */ >> -- >> 2.7.4
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 165a922..f8db809 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -707,6 +707,14 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, void *data; u32 act; + /* Transient failure which in theory could occur if + * in-flight packets from before XDP was enabled reach + * the receive path after XDP is loaded. In practice I + * was not able to create this condition. + */ + if (unlikely(hdr->hdr.gso_type)) + goto err_xdp; + /* This happens when rx buffer size is underestimated * or headroom is not enough because of the buffer * was refilled before XDP is set. This should only @@ -728,14 +736,6 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, xdp_page = page; } - /* Transient failure which in theory could occur if - * in-flight packets from before XDP was enabled reach - * the receive path after XDP is loaded. In practice I - * was not able to create this condition. - */ - if (unlikely(hdr->hdr.gso_type)) - goto err_xdp; - /* Allow consuming headroom but reserve enough space to push * the descriptor on if we get an XDP_TX return code. */
We need to drop refcnt to xdp_page if we see a gso packet. Otherwise it will be leaked. Fixing this by moving the check of gso packet above the linearizing logic. Cc: John Fastabend <john.fastabend@gmail.com> Fixes: 72979a6c3590 ("virtio_net: xdp, add slowpath case for non contiguous buffers") Signed-off-by: Jason Wang <jasowang@redhat.com> --- drivers/net/virtio_net.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)