Currently our handling of short network packets (i.e. those which are smaller than the minimum ethernet frame size of 60 bytes) is a little confused. The intention is that we allow network device models to transmit short packets, but we protect them from having to deal with receiving short packets unless they explicitly opt into it. This means we don't have to have a lot of code in each network device to try to handle the short packet size -- historically that code was untested, and often a source of "carefully crafted packet can cause an out-of-bounds memory access" bugs.
Our implementation of the padding is currently done when each network backend feeds a packet into the system via one of the qemu_send_packet family of functions. This mostly works, except that we missed some of the more obscure backends (e.g. the socket backend), and it means code duplication in each backend. We'd like to switch to making qemu_send_packet do the padding itself. To do that, we first need to make sure that all the NetClientState implementations which do not want padding set the do_not_pad flag. We do this by setting the flag for NetClientState instances created by qemu_new_net_client() and qemu_new_net_control_client(). Emulated network devices use qemu_new_nic(), which does not set the flag. Note that this also causes the net/hub client to request no padding. This is the right behaviour: we want the hub to accept short packets and forward them on to all destinations. Each destination then gets to choose whether it sees a padded packet or the short one. The hub.c code is length-agnostic and doesn't look inside the packets it is passing on, so it has no issues with short packets. Cc: [email protected] Signed-off-by: Peter Maydell <[email protected]> --- include/net/net.h | 3 ++- net/net.c | 10 ++++++---- net/tap.c | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/include/net/net.h b/include/net/net.h index fdf6a2ddd44..3c5b99c806e 100644 --- a/include/net/net.h +++ b/include/net/net.h @@ -169,7 +169,8 @@ void qemu_net_client_setup(NetClientState *nc, const char *model, const char *name, NetClientDestructor *destructor, - bool is_datapath); + bool is_datapath, + bool do_not_pad); NetClientState *qemu_new_net_client(NetClientInfo *info, NetClientState *peer, const char *model, diff --git a/net/net.c b/net/net.c index d7fa637ce51..1042a9d3a8e 100644 --- a/net/net.c +++ b/net/net.c @@ -267,7 +267,8 @@ void qemu_net_client_setup(NetClientState *nc, const char *model, const char *name, NetClientDestructor *destructor, - bool is_datapath) + bool is_datapath, + bool do_not_pad) { nc->info = info; nc->model = g_strdup(model); @@ -287,6 +288,7 @@ void qemu_net_client_setup(NetClientState *nc, nc->incoming_queue = qemu_new_net_queue(qemu_deliver_packet_iov, nc); nc->destructor = destructor; nc->is_datapath = is_datapath; + nc->do_not_pad = do_not_pad; QTAILQ_INIT(&nc->filters); } @@ -301,7 +303,7 @@ NetClientState *qemu_new_net_client(NetClientInfo *info, nc = g_malloc0(info->size); qemu_net_client_setup(nc, info, peer, model, name, - qemu_net_client_destructor, true); + qemu_net_client_destructor, true, true); return nc; } @@ -317,7 +319,7 @@ NetClientState *qemu_new_net_control_client(NetClientInfo *info, nc = g_malloc0(info->size); qemu_net_client_setup(nc, info, peer, model, name, - qemu_net_client_destructor, false); + qemu_net_client_destructor, false, true); return nc; } @@ -344,7 +346,7 @@ NICState *qemu_new_nic(NetClientInfo *info, for (i = 0; i < queues; i++) { qemu_net_client_setup(&nic->ncs[i], info, peers[i], model, name, - NULL, true); + NULL, true, false); nic->ncs[i].queue_index = i; } diff --git a/net/tap.c b/net/tap.c index d71e3dd1175..ada69a21021 100644 --- a/net/tap.c +++ b/net/tap.c @@ -620,7 +620,7 @@ static TAPState *new_tap(NetClientState *peer, TAPState *s = TAP_NETDEV(object_new(TYPE_TAP_NETDEV)); qemu_net_client_setup(&s->nc, &net_tap_info, peer, model, name, - tap_net_client_destructor, true); + tap_net_client_destructor, true, true); s->queue_index = queue_index; -- 2.43.0
