On Wed, May 06, 2020 at 04:19:40PM +0800, Jason Wang wrote:

On 2020/5/6 下午3:53, Michael S. Tsirkin wrote:

quoted

On Wed, May 06, 2020 at 02:16:32PM +0800, Jason Wang wrote:

quoted

We tried to reserve space for vnet header before
xdp.data_hard_start. But this is useless since the packet could be
modified by XDP which may invalidate the information stored in the
header and there's no way for XDP to know the existence of the vnet
header currently.

What do you mean? Doesn't XDP_PASS use the header in the buffer?

We don't, see 436c9453a1ac0 ("virtio-net: keep vnet header zeroed after
processing XDP")

If there's other place, it should be a bug.

quoted

quoted

So let's just not reserve space for vnet header in this case.

In any case, we can find out XDP does head adjustments
if we need to.

But XDP program can modify the packets without adjusting headers.

Thanks

Then what's the problem?

quoted

quoted

Cc: Jesper Dangaard Brouer <redacted>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
  drivers/net/virtio_net.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 11f722460513..98dd75b665a5 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c

@@ -684,8 +684,8 @@ static struct sk_buff *receive_small(struct net_device *dev,
  			page = xdp_page;
  		}
-		xdp.data_hard_start = buf + VIRTNET_RX_PAD + vi->hdr_len;
-		xdp.data = xdp.data_hard_start + xdp_headroom;
+		xdp.data_hard_start = buf + VIRTNET_RX_PAD;
+		xdp.data = xdp.data_hard_start + xdp_headroom + vi->hdr_len;
  		xdp.data_end = xdp.data + len;
  		xdp.data_meta = xdp.data;
  		xdp.rxq = &rq->xdp_rxq;

@@ -845,7 +845,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
  		 * the descriptor on if we get an XDP_TX return code.
  		 */
  		data = page_address(xdp_page) + offset;
-		xdp.data_hard_start = data - VIRTIO_XDP_HEADROOM + vi->hdr_len;
+		xdp.data_hard_start = data - VIRTIO_XDP_HEADROOM;
  		xdp.data = data + vi->hdr_len;
  		xdp.data_end = xdp.data + (len - vi->hdr_len);
  		xdp.data_meta = xdp.data;

-- 
2.20.1