In virtio-net, we have not yet supported multi-buffer XDP packet in zerocopy mode when there is a binding XDP program. However, in that case, when receiving multi-buffer XDP packet, we skip the XDP program and return XDP_PASS. As a result, the packet is passed to normal network stack which is an incorrect behavior. This commit instead returns XDP_DROP in that case.
Fixes: 99c861b44eb1 ("virtio_net: xsk: rx: support recv merge mode") Cc: stable@vger.kernel.org Signed-off-by: Bui Quang Minh minhquangbui99@gmail.com --- drivers/net/virtio_net.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index e53ba600605a..4c35324d6e5b 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1309,9 +1309,14 @@ static struct sk_buff *virtnet_receive_xsk_merge(struct net_device *dev, struct ret = XDP_PASS; rcu_read_lock(); prog = rcu_dereference(rq->xdp_prog); - /* TODO: support multi buffer. */ - if (prog && num_buf == 1) - ret = virtnet_xdp_handler(prog, xdp, dev, xdp_xmit, stats); + if (prog) { + /* TODO: support multi buffer. */ + if (num_buf == 1) + ret = virtnet_xdp_handler(prog, xdp, dev, xdp_xmit, + stats); + else + ret = XDP_DROP; + } rcu_read_unlock();
switch (ret) {
On Tue, Jun 3, 2025 at 11:07 PM Bui Quang Minh minhquangbui99@gmail.com wrote:
In virtio-net, we have not yet supported multi-buffer XDP packet in zerocopy mode when there is a binding XDP program. However, in that case, when receiving multi-buffer XDP packet, we skip the XDP program and return XDP_PASS. As a result, the packet is passed to normal network stack which is an incorrect behavior. This commit instead returns XDP_DROP in that case.
Fixes: 99c861b44eb1 ("virtio_net: xsk: rx: support recv merge mode") Cc: stable@vger.kernel.org Signed-off-by: Bui Quang Minh minhquangbui99@gmail.com
drivers/net/virtio_net.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index e53ba600605a..4c35324d6e5b 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1309,9 +1309,14 @@ static struct sk_buff *virtnet_receive_xsk_merge(struct net_device *dev, struct ret = XDP_PASS;
It would be simpler to just assign XDP_DROP here?
Or if you wish to stick to the way, we can simply remove this assignment.
rcu_read_lock(); prog = rcu_dereference(rq->xdp_prog);
/* TODO: support multi buffer. */if (prog && num_buf == 1)ret = virtnet_xdp_handler(prog, xdp, dev, xdp_xmit, stats);
if (prog) {/* TODO: support multi buffer. */if (num_buf == 1)ret = virtnet_xdp_handler(prog, xdp, dev, xdp_xmit,stats);elseret = XDP_DROP;} rcu_read_unlock(); switch (ret) {-- 2.43.0
Thanks
On 6/4/25 07:37, Jason Wang wrote:
On Tue, Jun 3, 2025 at 11:07 PM Bui Quang Minh minhquangbui99@gmail.com wrote:
In virtio-net, we have not yet supported multi-buffer XDP packet in zerocopy mode when there is a binding XDP program. However, in that case, when receiving multi-buffer XDP packet, we skip the XDP program and return XDP_PASS. As a result, the packet is passed to normal network stack which is an incorrect behavior. This commit instead returns XDP_DROP in that case.
Fixes: 99c861b44eb1 ("virtio_net: xsk: rx: support recv merge mode") Cc: stable@vger.kernel.org Signed-off-by: Bui Quang Minh minhquangbui99@gmail.com
drivers/net/virtio_net.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index e53ba600605a..4c35324d6e5b 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1309,9 +1309,14 @@ static struct sk_buff *virtnet_receive_xsk_merge(struct net_device *dev, struct ret = XDP_PASS;
It would be simpler to just assign XDP_DROP here?
Or if you wish to stick to the way, we can simply remove this assignment.
This XDP_PASS is returned for the case when there is no XDP program binding (!prog).
rcu_read_lock(); prog = rcu_dereference(rq->xdp_prog);
/* TODO: support multi buffer. */if (prog && num_buf == 1)ret = virtnet_xdp_handler(prog, xdp, dev, xdp_xmit, stats);
if (prog) {/* TODO: support multi buffer. */if (num_buf == 1)ret = virtnet_xdp_handler(prog, xdp, dev, xdp_xmit,stats);elseret = XDP_DROP;} rcu_read_unlock(); switch (ret) {-- 2.43.0
Thanks
Thanks, Quang Minh.
On Wed, Jun 4, 2025 at 10:17 PM Bui Quang Minh minhquangbui99@gmail.com wrote:
On 6/4/25 07:37, Jason Wang wrote:
On Tue, Jun 3, 2025 at 11:07 PM Bui Quang Minh minhquangbui99@gmail.com wrote:
In virtio-net, we have not yet supported multi-buffer XDP packet in zerocopy mode when there is a binding XDP program. However, in that case, when receiving multi-buffer XDP packet, we skip the XDP program and return XDP_PASS. As a result, the packet is passed to normal network stack which is an incorrect behavior. This commit instead returns XDP_DROP in that case.
Fixes: 99c861b44eb1 ("virtio_net: xsk: rx: support recv merge mode") Cc: stable@vger.kernel.org Signed-off-by: Bui Quang Minh minhquangbui99@gmail.com
drivers/net/virtio_net.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index e53ba600605a..4c35324d6e5b 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1309,9 +1309,14 @@ static struct sk_buff *virtnet_receive_xsk_merge(struct net_device *dev, struct ret = XDP_PASS;
It would be simpler to just assign XDP_DROP here?
Or if you wish to stick to the way, we can simply remove this assignment.
This XDP_PASS is returned for the case when there is no XDP program binding (!prog).
You're right.
Acked-by: Jason Wang jasowang@redhat.com
Thanks
rcu_read_lock(); prog = rcu_dereference(rq->xdp_prog);
/* TODO: support multi buffer. */if (prog && num_buf == 1)ret = virtnet_xdp_handler(prog, xdp, dev, xdp_xmit, stats);
if (prog) {/* TODO: support multi buffer. */if (num_buf == 1)ret = virtnet_xdp_handler(prog, xdp, dev, xdp_xmit,stats);elseret = XDP_DROP;} rcu_read_unlock(); switch (ret) {-- 2.43.0
Thanks
Thanks, Quang Minh.
On Tue, Jun 3, 2025 at 8:09 AM Bui Quang Minh minhquangbui99@gmail.com wrote:
In virtio-net, we have not yet supported multi-buffer XDP packet in zerocopy mode when there is a binding XDP program. However, in that case, when receiving multi-buffer XDP packet, we skip the XDP program and return XDP_PASS. As a result, the packet is passed to normal network stack which is an incorrect behavior. This commit instead returns XDP_DROP in that case.
Does it make more sense to return XDP_ABORTED? This seems like an unexpected exception case to me, but I'm not familiar enough with virtio-net's multibuffer support.
Fixes: 99c861b44eb1 ("virtio_net: xsk: rx: support recv merge mode") Cc: stable@vger.kernel.org Signed-off-by: Bui Quang Minh minhquangbui99@gmail.com
drivers/net/virtio_net.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index e53ba600605a..4c35324d6e5b 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1309,9 +1309,14 @@ static struct sk_buff *virtnet_receive_xsk_merge(struct net_device *dev, struct ret = XDP_PASS; rcu_read_lock(); prog = rcu_dereference(rq->xdp_prog);
- /* TODO: support multi buffer. */
- if (prog && num_buf == 1)
- ret = virtnet_xdp_handler(prog, xdp, dev, xdp_xmit, stats);
- if (prog) {
- /* TODO: support multi buffer. */
- if (num_buf == 1)
- ret = virtnet_xdp_handler(prog, xdp, dev, xdp_xmit,
- stats);
- else
- ret = XDP_DROP;
- }
rcu_read_unlock();
switch (ret) {
2.43.0
On 6/4/25 23:55, Zvi Effron wrote:
On Tue, Jun 3, 2025 at 8:09 AM Bui Quang Minh minhquangbui99@gmail.com wrote:
In virtio-net, we have not yet supported multi-buffer XDP packet in zerocopy mode when there is a binding XDP program. However, in that case, when receiving multi-buffer XDP packet, we skip the XDP program and return XDP_PASS. As a result, the packet is passed to normal network stack which is an incorrect behavior. This commit instead returns XDP_DROP in that case.
Does it make more sense to return XDP_ABORTED? This seems like an unexpected exception case to me, but I'm not familiar enough with virtio-net's multibuffer support.
The following code after this treats XDP_DROP and XDP_ABORTED in the same way. I don't have strong opinion between these 2 values here. We may add a call to trace_xdp_exception in case we want XDP_ABORTED here.
Thanks, Quang Minh.
Fixes: 99c861b44eb1 ("virtio_net: xsk: rx: support recv merge mode") Cc: stable@vger.kernel.org Signed-off-by: Bui Quang Minh minhquangbui99@gmail.com
drivers/net/virtio_net.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index e53ba600605a..4c35324d6e5b 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1309,9 +1309,14 @@ static struct sk_buff *virtnet_receive_xsk_merge(struct net_device *dev, struct ret = XDP_PASS; rcu_read_lock(); prog = rcu_dereference(rq->xdp_prog);
- /* TODO: support multi buffer. */
- if (prog && num_buf == 1)
- ret = virtnet_xdp_handler(prog, xdp, dev, xdp_xmit, stats);
- if (prog) {
- /* TODO: support multi buffer. */
- if (num_buf == 1)
- ret = virtnet_xdp_handler(prog, xdp, dev, xdp_xmit,
- stats);
- else
- ret = XDP_DROP;
- }
rcu_read_unlock();
switch (ret) {
2.43.0
On 6/3/25 5:06 PM, Bui Quang Minh wrote:
In virtio-net, we have not yet supported multi-buffer XDP packet in zerocopy mode when there is a binding XDP program. However, in that case, when receiving multi-buffer XDP packet, we skip the XDP program and return XDP_PASS. As a result, the packet is passed to normal network stack which is an incorrect behavior.
Why? AFAICS the multi-buffer mode depends on features negotiation, which is not controlled by the VM user.
Let's suppose the user wants to attach an XDP program to do some per packet stats accounting. That suddenly would cause drop packets depending on conditions not controlled by the (guest) user. It looks wrong to me.
XDP_ABORTED looks like a better choice.
/P
On 6/5/25 18:03, Paolo Abeni wrote:
On 6/3/25 5:06 PM, Bui Quang Minh wrote:
In virtio-net, we have not yet supported multi-buffer XDP packet in zerocopy mode when there is a binding XDP program. However, in that case, when receiving multi-buffer XDP packet, we skip the XDP program and return XDP_PASS. As a result, the packet is passed to normal network stack which is an incorrect behavior.
Why? AFAICS the multi-buffer mode depends on features negotiation, which is not controlled by the VM user.
Let's suppose the user wants to attach an XDP program to do some per packet stats accounting. That suddenly would cause drop packets depending on conditions not controlled by the (guest) user. It looks wrong to me.
But currently, if a multi-buffer packet arrives, it will not go through XDP program so it doesn't increase the stats but still goes to network stack. So I think it's not a correct behavior.
XDP_ABORTED looks like a better choice.
/P
Thanks, Quang Minh.
On Thu, 5 Jun 2025 21:33:26 +0700 Bui Quang Minh wrote:
On 6/5/25 18:03, Paolo Abeni wrote:
On 6/3/25 5:06 PM, Bui Quang Minh wrote:
In virtio-net, we have not yet supported multi-buffer XDP packet in zerocopy mode when there is a binding XDP program. However, in that case, when receiving multi-buffer XDP packet, we skip the XDP program and return XDP_PASS. As a result, the packet is passed to normal network stack which is an incorrect behavior.
Why? AFAICS the multi-buffer mode depends on features negotiation, which is not controlled by the VM user.
Let's suppose the user wants to attach an XDP program to do some per packet stats accounting. That suddenly would cause drop packets depending on conditions not controlled by the (guest) user. It looks wrong to me.
But currently, if a multi-buffer packet arrives, it will not go through XDP program so it doesn't increase the stats but still goes to network stack. So I think it's not a correct behavior.
Sounds fair, but at a glance the normal XDP path seems to be trying to linearize the frame. Can we not try to flatten the frame here? If it's simply to long for the chunk size that's a frame length error, right?
linux-stable-mirror@lists.linaro.org
-
Bui Quang Minh -
Jakub Kicinski -
Jason Wang -
Paolo Abeni -
Zvi Effron