* [PATCH] net/vmnet: Pad short Ethernet frames
@ 2024-01-06 22:35 William Hooper
2024-01-07 22:23 ` Bin Meng
2024-01-08 15:36 ` Philippe Mathieu-Daudé
0 siblings, 2 replies; 11+ messages in thread
From: William Hooper @ 2024-01-06 22:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Bin Meng, William Hooper
At least on macOS 12.7.2, vmnet doesn't pad Ethernet frames, such as the
host's ARP replies, to the minimum size (60 bytes before the frame check
sequence) defined in IEEE Std 802.3-2022, so guests' Ethernet device
drivers may drop them with "frame too short" errors.
This patch calls eth_pad_short_frame() to add padding, as in net/tap.c
and net/slirp.c.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2058
Signed-off-by: William Hooper <wsh@wshooper.org>
---
net/vmnet-common.m | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/net/vmnet-common.m b/net/vmnet-common.m
index 2958283485..f8f7163226 100644
--- a/net/vmnet-common.m
+++ b/net/vmnet-common.m
@@ -18,6 +18,7 @@
#include "qemu/error-report.h"
#include "qapi/error.h"
#include "sysemu/runstate.h"
+#include "net/eth.h"
#include <vmnet/vmnet.h>
#include <dispatch/dispatch.h>
@@ -150,10 +151,23 @@ static int vmnet_read_packets(VmnetState *s)
*/
static void vmnet_write_packets_to_qemu(VmnetState *s)
{
+ uint8_t *pkt;
+ size_t pktsz;
+ uint8_t min_pkt[ETH_ZLEN];
+ size_t min_pktsz = sizeof(min_pkt);
+
while (s->packets_send_current_pos < s->packets_send_end_pos) {
- ssize_t size = qemu_send_packet_async(&s->nc,
- s->iov_buf[s->packets_send_current_pos].iov_base,
- s->packets_buf[s->packets_send_current_pos].vm_pkt_size,
+ pkt = s->iov_buf[s->packets_send_current_pos].iov_base;
+ pktsz = s->packets_buf[s->packets_send_current_pos].vm_pkt_size;
+
+ if (net_peer_needs_padding(&s->nc)) {
+ if (eth_pad_short_frame(min_pkt, &min_pktsz, pkt, pktsz)) {
+ pkt = min_pkt;
+ pktsz = min_pktsz;
+ }
+ }
+
+ ssize_t size = qemu_send_packet_async(&s->nc, pkt, pktsz,
vmnet_send_completed);
if (size == 0) {
--
2.37.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] net/vmnet: Pad short Ethernet frames
2024-01-06 22:35 [PATCH] net/vmnet: Pad short Ethernet frames William Hooper
@ 2024-01-07 22:23 ` Bin Meng
2024-01-08 15:36 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 11+ messages in thread
From: Bin Meng @ 2024-01-07 22:23 UTC (permalink / raw)
To: William Hooper; +Cc: qemu-devel, Bin Meng
On Sun, Jan 7, 2024 at 7:19 AM William Hooper <wsh@wshooper.org> wrote:
>
> At least on macOS 12.7.2, vmnet doesn't pad Ethernet frames, such as the
> host's ARP replies, to the minimum size (60 bytes before the frame check
> sequence) defined in IEEE Std 802.3-2022, so guests' Ethernet device
> drivers may drop them with "frame too short" errors.
>
> This patch calls eth_pad_short_frame() to add padding, as in net/tap.c
> and net/slirp.c.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2058
> Signed-off-by: William Hooper <wsh@wshooper.org>
> ---
> net/vmnet-common.m | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net/vmnet: Pad short Ethernet frames
2024-01-06 22:35 [PATCH] net/vmnet: Pad short Ethernet frames William Hooper
2024-01-07 22:23 ` Bin Meng
@ 2024-01-08 15:36 ` Philippe Mathieu-Daudé
2024-01-10 5:34 ` William Hooper
1 sibling, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-08 15:36 UTC (permalink / raw)
To: William Hooper, qemu-devel; +Cc: Bin Meng
Hi William,
On 6/1/24 23:35, William Hooper wrote:
> At least on macOS 12.7.2, vmnet doesn't pad Ethernet frames, such as the
> host's ARP replies, to the minimum size (60 bytes before the frame check
> sequence) defined in IEEE Std 802.3-2022, so guests' Ethernet device
> drivers may drop them with "frame too short" errors.
>
> This patch calls eth_pad_short_frame() to add padding, as in net/tap.c
> and net/slirp.c.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2058
> Signed-off-by: William Hooper <wsh@wshooper.org>
> ---
> net/vmnet-common.m | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/net/vmnet-common.m b/net/vmnet-common.m
> index 2958283485..f8f7163226 100644
> --- a/net/vmnet-common.m
> +++ b/net/vmnet-common.m
> @@ -18,6 +18,7 @@
> #include "qemu/error-report.h"
> #include "qapi/error.h"
> #include "sysemu/runstate.h"
> +#include "net/eth.h"
>
> #include <vmnet/vmnet.h>
> #include <dispatch/dispatch.h>
> @@ -150,10 +151,23 @@ static int vmnet_read_packets(VmnetState *s)
> */
> static void vmnet_write_packets_to_qemu(VmnetState *s)
> {
> + uint8_t *pkt;
> + size_t pktsz;
> + uint8_t min_pkt[ETH_ZLEN];
> + size_t min_pktsz = sizeof(min_pkt);
> +
> while (s->packets_send_current_pos < s->packets_send_end_pos) {
> - ssize_t size = qemu_send_packet_async(&s->nc,
> - s->iov_buf[s->packets_send_current_pos].iov_base,
> - s->packets_buf[s->packets_send_current_pos].vm_pkt_size,
> + pkt = s->iov_buf[s->packets_send_current_pos].iov_base;
> + pktsz = s->packets_buf[s->packets_send_current_pos].vm_pkt_size;
> +
> + if (net_peer_needs_padding(&s->nc)) {
Don't we want to initialize min_pktsz here ...
min_pktsz = sizeof(min_pkt);
> + if (eth_pad_short_frame(min_pkt, &min_pktsz, pkt, pktsz)) {
... because eth_pad_short_frame() update it?
> + pkt = min_pkt;
> + pktsz = min_pktsz;
> + }
> + }
> +
> + ssize_t size = qemu_send_packet_async(&s->nc, pkt, pktsz,
> vmnet_send_completed);
>
> if (size == 0) {
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net/vmnet: Pad short Ethernet frames
2024-01-08 15:36 ` Philippe Mathieu-Daudé
@ 2024-01-10 5:34 ` William Hooper
2024-08-18 6:33 ` [PATCH v2] " William Hooper
0 siblings, 1 reply; 11+ messages in thread
From: William Hooper @ 2024-01-10 5:34 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-devel, Bin Meng
On Mon, Jan 8, 2024 at 7:36 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> Don't we want to initialize min_pktsz here ...
>
> min_pktsz = sizeof(min_pkt);
>
> > + if (eth_pad_short_frame(min_pkt, &min_pktsz, pkt, pktsz)) {
>
> ... because eth_pad_short_frame() update it?
Thanks for the review.
The results would be the same, since eth_pad_short_frame() sets
min_pktsz, if at all, to ETH_ZLEN, the same value as the initializer.
I have no objection to re-initializing min_pktsz for each packet,
however, if only to reduce the risk of a bug being introduced if this
behavior of eth_pad_short_frame() were ever to be changed.
Would you like me to post a revised patch?
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] net/vmnet: Pad short Ethernet frames
2024-01-10 5:34 ` William Hooper
@ 2024-08-18 6:33 ` William Hooper
2024-10-20 3:16 ` Ping: " William Hooper
0 siblings, 1 reply; 11+ messages in thread
From: William Hooper @ 2024-08-18 6:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Bin Meng, Philippe Mathieu-Daudé
At least on macOS 12.7.2, vmnet doesn't pad Ethernet frames, such as the
host's ARP replies, to the minimum size (60 bytes before the frame check
sequence) defined in IEEE Std 802.3-2022, so guests' Ethernet device
drivers may drop them with "frame too short" errors.
This patch calls eth_pad_short_frame() to add padding, as in net/tap.c
and net/slirp.c. Thanks to Bin Meng and Philippe Mathieu-Daudé for
reviewing an earlier version.
Signed-off-by: William Hooper <wsh@wshooper.org>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2058
---
net/vmnet-common.m | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/net/vmnet-common.m b/net/vmnet-common.m
index 30c4e53c13..bce1cc590d 100644
--- a/net/vmnet-common.m
+++ b/net/vmnet-common.m
@@ -18,6 +18,7 @@
#include "qemu/error-report.h"
#include "qapi/error.h"
#include "sysemu/runstate.h"
+#include "net/eth.h"
#include <vmnet/vmnet.h>
#include <dispatch/dispatch.h>
@@ -147,10 +148,25 @@ static int vmnet_read_packets(VmnetState *s)
*/
static void vmnet_write_packets_to_qemu(VmnetState *s)
{
+ uint8_t *pkt;
+ size_t pktsz;
+ uint8_t min_pkt[ETH_ZLEN];
+ size_t min_pktsz;
+
while (s->packets_send_current_pos < s->packets_send_end_pos) {
- ssize_t size = qemu_send_packet_async(&s->nc,
- s->iov_buf[s->packets_send_current_pos].iov_base,
- s->packets_buf[s->packets_send_current_pos].vm_pkt_size,
+ pkt = s->iov_buf[s->packets_send_current_pos].iov_base;
+ pktsz = s->packets_buf[s->packets_send_current_pos].vm_pkt_size;
+
+ if (net_peer_needs_padding(&s->nc)) {
+ min_pktsz = sizeof(min_pkt);
+
+ if (eth_pad_short_frame(min_pkt, &min_pktsz, pkt, pktsz)) {
+ pkt = min_pkt;
+ pktsz = min_pktsz;
+ }
+ }
+
+ ssize_t size = qemu_send_packet_async(&s->nc, pkt, pktsz,
vmnet_send_completed);
if (size == 0) {
--
2.37.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Ping: [PATCH v2] net/vmnet: Pad short Ethernet frames
2024-08-18 6:33 ` [PATCH v2] " William Hooper
@ 2024-10-20 3:16 ` William Hooper
2024-10-30 12:49 ` Phil Dennis-Jordan
0 siblings, 1 reply; 11+ messages in thread
From: William Hooper @ 2024-10-20 3:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Bin Meng, Philippe Mathieu-Daudé
On Sat, Aug 17, 2024 at 11:33 PM William Hooper <wsh@wshooper.org> wrote:
> At least on macOS 12.7.2, vmnet doesn't pad Ethernet frames, such as the
> host's ARP replies, to the minimum size (60 bytes before the frame check
> sequence) defined in IEEE Std 802.3-2022, so guests' Ethernet device
> drivers may drop them with "frame too short" errors.
>
> This patch calls eth_pad_short_frame() to add padding, as in net/tap.c
> and net/slirp.c. Thanks to Bin Meng and Philippe Mathieu-Daudé for
> reviewing an earlier version.
>
> Signed-off-by: William Hooper <wsh@wshooper.org>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2058
> ---
> net/vmnet-common.m | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/net/vmnet-common.m b/net/vmnet-common.m
> index 30c4e53c13..bce1cc590d 100644
> --- a/net/vmnet-common.m
> +++ b/net/vmnet-common.m
> @@ -18,6 +18,7 @@
> #include "qemu/error-report.h"
> #include "qapi/error.h"
> #include "sysemu/runstate.h"
> +#include "net/eth.h"
>
> #include <vmnet/vmnet.h>
> #include <dispatch/dispatch.h>
> @@ -147,10 +148,25 @@ static int vmnet_read_packets(VmnetState *s)
> */
> static void vmnet_write_packets_to_qemu(VmnetState *s)
> {
> + uint8_t *pkt;
> + size_t pktsz;
> + uint8_t min_pkt[ETH_ZLEN];
> + size_t min_pktsz;
> +
> while (s->packets_send_current_pos < s->packets_send_end_pos) {
> - ssize_t size = qemu_send_packet_async(&s->nc,
> - s->iov_buf[s->packets_send_current_pos].iov_base,
> - s->packets_buf[s->packets_send_current_pos].vm_pkt_size,
> + pkt = s->iov_buf[s->packets_send_current_pos].iov_base;
> + pktsz = s->packets_buf[s->packets_send_current_pos].vm_pkt_size;
> +
> + if (net_peer_needs_padding(&s->nc)) {
> + min_pktsz = sizeof(min_pkt);
> +
> + if (eth_pad_short_frame(min_pkt, &min_pktsz, pkt, pktsz)) {
> + pkt = min_pkt;
> + pktsz = min_pktsz;
> + }
> + }
> +
> + ssize_t size = qemu_send_packet_async(&s->nc, pkt, pktsz,
> vmnet_send_completed);
>
> if (size == 0) {
> --
> 2.37.1
Ping?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Ping: [PATCH v2] net/vmnet: Pad short Ethernet frames
2024-10-20 3:16 ` Ping: " William Hooper
@ 2024-10-30 12:49 ` Phil Dennis-Jordan
2024-11-02 20:56 ` [PATCH v3] " William Hooper
0 siblings, 1 reply; 11+ messages in thread
From: Phil Dennis-Jordan @ 2024-10-30 12:49 UTC (permalink / raw)
To: William Hooper; +Cc: qemu-devel, Bin Meng, Philippe Mathieu-Daudé
On Sun, 20 Oct 2024 at 05:17, William Hooper <wsh@wshooper.org> wrote:
>
> On Sat, Aug 17, 2024 at 11:33 PM William Hooper <wsh@wshooper.org> wrote:
> > At least on macOS 12.7.2, vmnet doesn't pad Ethernet frames, such as the
> > host's ARP replies, to the minimum size (60 bytes before the frame check
> > sequence) defined in IEEE Std 802.3-2022, so guests' Ethernet device
> > drivers may drop them with "frame too short" errors.
> >
> > This patch calls eth_pad_short_frame() to add padding, as in net/tap.c
> > and net/slirp.c. Thanks to Bin Meng and Philippe Mathieu-Daudé for
> > reviewing an earlier version.
> >
> > Signed-off-by: William Hooper <wsh@wshooper.org>
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2058
> > ---
> > net/vmnet-common.m | 22 +++++++++++++++++++---
> > 1 file changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/vmnet-common.m b/net/vmnet-common.m
> > index 30c4e53c13..bce1cc590d 100644
> > --- a/net/vmnet-common.m
> > +++ b/net/vmnet-common.m
> > @@ -18,6 +18,7 @@
> > #include "qemu/error-report.h"
> > #include "qapi/error.h"
> > #include "sysemu/runstate.h"
> > +#include "net/eth.h"
> >
> > #include <vmnet/vmnet.h>
> > #include <dispatch/dispatch.h>
> > @@ -147,10 +148,25 @@ static int vmnet_read_packets(VmnetState *s)
> > */
> > static void vmnet_write_packets_to_qemu(VmnetState *s)
> > {
> > + uint8_t *pkt;
> > + size_t pktsz;
> > + uint8_t min_pkt[ETH_ZLEN];
> > + size_t min_pktsz;
> > +
> > while (s->packets_send_current_pos < s->packets_send_end_pos) {
> > - ssize_t size = qemu_send_packet_async(&s->nc,
> > - s->iov_buf[s->packets_send_current_pos].iov_base,
> > - s->packets_buf[s->packets_send_current_pos].vm_pkt_size,
> > + pkt = s->iov_buf[s->packets_send_current_pos].iov_base;
> > + pktsz = s->packets_buf[s->packets_send_current_pos].vm_pkt_size;
> > +
> > + if (net_peer_needs_padding(&s->nc)) {
> > + min_pktsz = sizeof(min_pkt);
> > +
> > + if (eth_pad_short_frame(min_pkt, &min_pktsz, pkt, pktsz)) {
> > + pkt = min_pkt;
> > + pktsz = min_pktsz;
> > + }
> > + }
> > +
> > + ssize_t size = qemu_send_packet_async(&s->nc, pkt, pktsz,
> > vmnet_send_completed);
Nit: Move the declaration of 'size' up to the function header with the
rest of the variables for consistency.
Apart from that:
Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
> > if (size == 0) {
> > --
> > 2.37.1
>
> Ping?
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3] net/vmnet: Pad short Ethernet frames
2024-10-30 12:49 ` Phil Dennis-Jordan
@ 2024-11-02 20:56 ` William Hooper
2024-11-17 2:08 ` Ping: " William Hooper
2024-12-30 21:05 ` Ping (2): " William Hooper
0 siblings, 2 replies; 11+ messages in thread
From: William Hooper @ 2024-11-02 20:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Bin Meng, Philippe Mathieu-Daudé, Phil Dennis-Jordan
At least on macOS 12.7.2, vmnet doesn't pad Ethernet frames, such as the
host's ARP replies, to the minimum size (60 bytes before the frame check
sequence) defined in IEEE Std 802.3-2022, so guests' Ethernet device
drivers may drop them with "frame too short" errors.
This patch calls eth_pad_short_frame() to add padding, as in net/tap.c
and net/slirp.c. Thanks to Bin Meng, Philippe Mathieu-Daudé, and Phil
Dennis-Jordan for reviewing earlier versions.
Signed-off-by: William Hooper <wsh@wshooper.org>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2058
Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
---
net/vmnet-common.m | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/net/vmnet-common.m b/net/vmnet-common.m
index 30c4e53c13..4b7e330c05 100644
--- a/net/vmnet-common.m
+++ b/net/vmnet-common.m
@@ -18,6 +18,7 @@
#include "qemu/error-report.h"
#include "qapi/error.h"
#include "sysemu/runstate.h"
+#include "net/eth.h"
#include <vmnet/vmnet.h>
#include <dispatch/dispatch.h>
@@ -147,10 +148,26 @@ static int vmnet_read_packets(VmnetState *s)
*/
static void vmnet_write_packets_to_qemu(VmnetState *s)
{
+ uint8_t *pkt;
+ size_t pktsz;
+ uint8_t min_pkt[ETH_ZLEN];
+ size_t min_pktsz;
+ ssize_t size;
+
while (s->packets_send_current_pos < s->packets_send_end_pos) {
- ssize_t size = qemu_send_packet_async(&s->nc,
- s->iov_buf[s->packets_send_current_pos].iov_base,
- s->packets_buf[s->packets_send_current_pos].vm_pkt_size,
+ pkt = s->iov_buf[s->packets_send_current_pos].iov_base;
+ pktsz = s->packets_buf[s->packets_send_current_pos].vm_pkt_size;
+
+ if (net_peer_needs_padding(&s->nc)) {
+ min_pktsz = sizeof(min_pkt);
+
+ if (eth_pad_short_frame(min_pkt, &min_pktsz, pkt, pktsz)) {
+ pkt = min_pkt;
+ pktsz = min_pktsz;
+ }
+ }
+
+ size = qemu_send_packet_async(&s->nc, pkt, pktsz,
vmnet_send_completed);
if (size == 0) {
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Ping: [PATCH v3] net/vmnet: Pad short Ethernet frames
2024-11-02 20:56 ` [PATCH v3] " William Hooper
@ 2024-11-17 2:08 ` William Hooper
2024-12-30 21:05 ` Ping (2): " William Hooper
1 sibling, 0 replies; 11+ messages in thread
From: William Hooper @ 2024-11-17 2:08 UTC (permalink / raw)
To: qemu-devel; +Cc: Bin Meng, Philippe Mathieu-Daudé, Phil Dennis-Jordan
On Sat, Nov 2, 2024 at 1:56 PM William Hooper <wsh@wshooper.org> wrote:
> At least on macOS 12.7.2, vmnet doesn't pad Ethernet frames, such as the
> host's ARP replies, to the minimum size (60 bytes before the frame check
> sequence) defined in IEEE Std 802.3-2022, so guests' Ethernet device
> drivers may drop them with "frame too short" errors.
>
> This patch calls eth_pad_short_frame() to add padding, as in net/tap.c
> and net/slirp.c. Thanks to Bin Meng, Philippe Mathieu-Daudé, and Phil
> Dennis-Jordan for reviewing earlier versions.
>
> Signed-off-by: William Hooper <wsh@wshooper.org>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2058
> Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
> ---
> net/vmnet-common.m | 23 ++++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/net/vmnet-common.m b/net/vmnet-common.m
> index 30c4e53c13..4b7e330c05 100644
> --- a/net/vmnet-common.m
> +++ b/net/vmnet-common.m
> @@ -18,6 +18,7 @@
> #include "qemu/error-report.h"
> #include "qapi/error.h"
> #include "sysemu/runstate.h"
> +#include "net/eth.h"
>
> #include <vmnet/vmnet.h>
> #include <dispatch/dispatch.h>
> @@ -147,10 +148,26 @@ static int vmnet_read_packets(VmnetState *s)
> */
> static void vmnet_write_packets_to_qemu(VmnetState *s)
> {
> + uint8_t *pkt;
> + size_t pktsz;
> + uint8_t min_pkt[ETH_ZLEN];
> + size_t min_pktsz;
> + ssize_t size;
> +
> while (s->packets_send_current_pos < s->packets_send_end_pos) {
> - ssize_t size = qemu_send_packet_async(&s->nc,
> - s->iov_buf[s->packets_send_current_pos].iov_base,
> - s->packets_buf[s->packets_send_current_pos].vm_pkt_size,
> + pkt = s->iov_buf[s->packets_send_current_pos].iov_base;
> + pktsz = s->packets_buf[s->packets_send_current_pos].vm_pkt_size;
> +
> + if (net_peer_needs_padding(&s->nc)) {
> + min_pktsz = sizeof(min_pkt);
> +
> + if (eth_pad_short_frame(min_pkt, &min_pktsz, pkt, pktsz)) {
> + pkt = min_pkt;
> + pktsz = min_pktsz;
> + }
> + }
> +
> + size = qemu_send_packet_async(&s->nc, pkt, pktsz,
> vmnet_send_completed);
>
> if (size == 0) {
Ping?
https://patchew.org/QEMU/20241102205653.30476-1-wsh@wshooper.org/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Ping (2): [PATCH v3] net/vmnet: Pad short Ethernet frames
2024-11-02 20:56 ` [PATCH v3] " William Hooper
2024-11-17 2:08 ` Ping: " William Hooper
@ 2024-12-30 21:05 ` William Hooper
2024-12-31 17:42 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 11+ messages in thread
From: William Hooper @ 2024-12-30 21:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Bin Meng, Philippe Mathieu-Daudé, Phil Dennis-Jordan
On Sat, Nov 2, 2024 at 1:56 PM William Hooper <wsh@wshooper.org> wrote:
> At least on macOS 12.7.2, vmnet doesn't pad Ethernet frames, such as the
> host's ARP replies, to the minimum size (60 bytes before the frame check
> sequence) defined in IEEE Std 802.3-2022, so guests' Ethernet device
> drivers may drop them with "frame too short" errors.
>
> This patch calls eth_pad_short_frame() to add padding, as in net/tap.c
> and net/slirp.c. Thanks to Bin Meng, Philippe Mathieu-Daudé, and Phil
> Dennis-Jordan for reviewing earlier versions.
>
> Signed-off-by: William Hooper <wsh@wshooper.org>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2058
> Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
> ---
> net/vmnet-common.m | 23 ++++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/net/vmnet-common.m b/net/vmnet-common.m
> index 30c4e53c13..4b7e330c05 100644
> --- a/net/vmnet-common.m
> +++ b/net/vmnet-common.m
> @@ -18,6 +18,7 @@
> #include "qemu/error-report.h"
> #include "qapi/error.h"
> #include "sysemu/runstate.h"
> +#include "net/eth.h"
>
> #include <vmnet/vmnet.h>
> #include <dispatch/dispatch.h>
> @@ -147,10 +148,26 @@ static int vmnet_read_packets(VmnetState *s)
> */
> static void vmnet_write_packets_to_qemu(VmnetState *s)
> {
> + uint8_t *pkt;
> + size_t pktsz;
> + uint8_t min_pkt[ETH_ZLEN];
> + size_t min_pktsz;
> + ssize_t size;
> +
> while (s->packets_send_current_pos < s->packets_send_end_pos) {
> - ssize_t size = qemu_send_packet_async(&s->nc,
> - s->iov_buf[s->packets_send_current_pos].iov_base,
> - s->packets_buf[s->packets_send_current_pos].vm_pkt_size,
> + pkt = s->iov_buf[s->packets_send_current_pos].iov_base;
> + pktsz = s->packets_buf[s->packets_send_current_pos].vm_pkt_size;
> +
> + if (net_peer_needs_padding(&s->nc)) {
> + min_pktsz = sizeof(min_pkt);
> +
> + if (eth_pad_short_frame(min_pkt, &min_pktsz, pkt, pktsz)) {
> + pkt = min_pkt;
> + pktsz = min_pktsz;
> + }
> + }
> +
> + size = qemu_send_packet_async(&s->nc, pkt, pktsz,
> vmnet_send_completed);
>
> if (size == 0) {
Ping?
https://patchew.org/QEMU/20241102205653.30476-1-wsh@wshooper.org/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Ping (2): [PATCH v3] net/vmnet: Pad short Ethernet frames
2024-12-30 21:05 ` Ping (2): " William Hooper
@ 2024-12-31 17:42 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-12-31 17:42 UTC (permalink / raw)
To: William Hooper, qemu-devel; +Cc: Bin Meng, Phil Dennis-Jordan, Jason Wang
Hi William,
On 30/12/24 22:05, William Hooper wrote:
> On Sat, Nov 2, 2024 at 1:56 PM William Hooper <wsh@wshooper.org> wrote:
>> At least on macOS 12.7.2, vmnet doesn't pad Ethernet frames, such as the
>> host's ARP replies, to the minimum size (60 bytes before the frame check
>> sequence) defined in IEEE Std 802.3-2022, so guests' Ethernet device
>> drivers may drop them with "frame too short" errors.
>>
>> This patch calls eth_pad_short_frame() to add padding, as in net/tap.c
>> and net/slirp.c. Thanks to Bin Meng, Philippe Mathieu-Daudé, and Phil
>> Dennis-Jordan for reviewing earlier versions.
>>
>> Signed-off-by: William Hooper <wsh@wshooper.org>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2058
>> Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
>> ---
>> net/vmnet-common.m | 23 ++++++++++++++++++++---
>> 1 file changed, 20 insertions(+), 3 deletions(-)
> Ping?
We missed this patch because:
1/ maintainers were not Cc'ed (Jason now added)
$ ./scripts/get_maintainer.pl -f net/vmnet-common.m
Jason Wang <jasowang@redhat.com> (maintainer:Network device ba...)
"Philippe Mathieu-Daudé" <philmd@linaro.org> (odd fixer:Darwin (macOS, iOS))
2/ it is buried as a reply on a previous patch, see [*]:
"Send each new revision as a new top-level thread, rather than burying
it in-reply-to an earlier revision, as many reviewers are not looking
inside deep threads for new patches."
Anyway, now queued!
Regards,
Phil.
[*]
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#when-resending-patches-add-a-version-tag
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-12-31 17:42 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-06 22:35 [PATCH] net/vmnet: Pad short Ethernet frames William Hooper
2024-01-07 22:23 ` Bin Meng
2024-01-08 15:36 ` Philippe Mathieu-Daudé
2024-01-10 5:34 ` William Hooper
2024-08-18 6:33 ` [PATCH v2] " William Hooper
2024-10-20 3:16 ` Ping: " William Hooper
2024-10-30 12:49 ` Phil Dennis-Jordan
2024-11-02 20:56 ` [PATCH v3] " William Hooper
2024-11-17 2:08 ` Ping: " William Hooper
2024-12-30 21:05 ` Ping (2): " William Hooper
2024-12-31 17:42 ` Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).