From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=dDMsCHqH; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id B828E5A026F for ; Tue, 11 Mar 2025 23:45:14 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1741733113; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=PWO9tEdKcxSgH/RicEHpp+FG4KCMFGM5Q3CkDd77Fow=; b=dDMsCHqHUp+xocREi2UR+ygTdMuCjcFtG/Ujn3y/gdiBffPueWaUxZZv45VaSUTxNrPuqO s4xKRJmkNT2ceQstWbsn5RdYeN2NVa8OZEfhcUV2BFhD3AyKcETFU1weq/5kk+WFn+fvOG H7PmXWjUqeggRBrNn4jIG5EejqbE948= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-345-p-FQsyylMG6PA5rm0M0Fcw-1; Tue, 11 Mar 2025 18:45:12 -0400 X-MC-Unique: p-FQsyylMG6PA5rm0M0Fcw-1 X-Mimecast-MFC-AGG-ID: p-FQsyylMG6PA5rm0M0Fcw_1741733111 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-3913aea90b4so1621953f8f.2 for ; Tue, 11 Mar 2025 15:45:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741733111; x=1742337911; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=PWO9tEdKcxSgH/RicEHpp+FG4KCMFGM5Q3CkDd77Fow=; b=ucv4LGtWlWBCMKfn4cnl5vN5ZsxOp74iGRU1UY2KaMFaUVyZd39jqzhFFOKftWIGMV yFetmA6utiEh4vb2fYP29XJA7hfxSF93+stP2kmJj1RifJ2Nn6CiGnnlbdTwTgOxE7UU tqZ1JXGfrB5ukOSBiNSpqVoX/zP/dKYKixnnPd52vB3bX3NiqP0MhRGebLn+deXU+vW9 8aVrhHpGOAbhgaUFpmoryFpN1v5kju4JpnWZsU1cPhPB3xqMS//EO1Bj74kl73Wc4bAL Kcez0JzatSnzGXfJi3pDfx+84XkRdZ/FRpVzOLM161CwPTFsnLhjq1LJYWt9fjxNeUlA nnGw== X-Gm-Message-State: AOJu0YzWxmVHSHRkRAEmh1akzE5Cyv6oZC6Bn96K/lGnQlTeFfZKb768 CSYaSHBiTQeYTrEw6IigiTB/2OfA08xhKGEuVj5JoengbB+jX2HQ4Fbp3ewFljjhaVIw8TRWz/u I/wnBN0FOWxhmz9AnXgjdm1H2+2ILsXgzMqafS5Dqao4E8hDMwaG24vjxoA== X-Gm-Gg: ASbGncu/LehXd73IatnU76p16tEjyzYHjtiEQxwDdufsXONghAsVE6LWhX7rXNZQVD9 ykY7AUtnJ8qAYFC91l5d3jxj+LrE2zxB/nTEs9YKlDis9Cny2OOELPUDFpQZ7Ezrd6KQ/3c1E+w 5kIAV0cxAhqinlRGTQlxE6L4Xdq9HoTv4S7em7b/96uPtlaN3tlSAww/yz1frSlRbFPs1UOLSJj /vt6XkyKTSRzYQ2NCgYS0m6xt8lxOll3PEQWfiCVAuzW6HIEncjOphaQI1QBWlmpjon25QHEiEO kSyJ2/1g4CHd1Tse0eJ1gWlJOa0= X-Received: by 2002:a05:6000:1a87:b0:38d:dffc:c133 with SMTP id ffacd0b85a97d-39132dc56aamr15365014f8f.44.1741733111273; Tue, 11 Mar 2025 15:45:11 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEtF4pf51rcwQoekdD5bshCaRyW5gDR0ggsNTcQk3pFZtgmILTI/P3W3vMjfEL7A7Jdq4LMoQ== X-Received: by 2002:a05:6000:1a87:b0:38d:dffc:c133 with SMTP id ffacd0b85a97d-39132dc56aamr15365004f8f.44.1741733110914; Tue, 11 Mar 2025 15:45:10 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-3912c0e2eecsm19128290f8f.79.2025.03.11.15.45.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 11 Mar 2025 15:45:10 -0700 (PDT) Date: Tue, 11 Mar 2025 23:45:09 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 6/9] tap: Use explicit defines for maximum length of L2 frame Message-ID: <20250311234509.00badc22@elisabeth> In-Reply-To: <20250311060318.1502861-7-david@gibson.dropbear.id.au> References: <20250311060318.1502861-1-david@gibson.dropbear.id.au> <20250311060318.1502861-7-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: jj0XUqEYQQoOeJNv1HuMR5uiISKcZ_KteU8ZjdRXpZc_1741733111 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: 6JMQCGPQ7KCQ6ZO76QPYI3OQNE5EHP25 X-Message-ID-Hash: 6JMQCGPQ7KCQ6ZO76QPYI3OQNE5EHP25 X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, David Gibson X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Nits only (I can fix it all up on merge): On Tue, 11 Mar 2025 17:03:15 +1100 David Gibson wrote: > Currently in tap.c we (mostly) use ETH_MAX_MTU as the maximum length of > an L2 frame. This define comes from the kernel, but it's badly named and > used confusingly. > > First, it doesn't really have anything to do with Ethernet, which has no > structural limit on frame lengths. It comes more from either a) IP which > imposes a 64k datagram limit or b) from internal buffers used in various > places in the kernel (and in passt). > > Worse, MTU generally means the maximum size of the IP (L3) datagram which > may be transferred, _not_ counting the L2 headers. In the kernel > ETH_MAX_MTU is sometimes used that way, but sometimes seems to be used as > a maximum frame length, _including_ L2 headers. In tap.c we're mostly > using it in the second way. > > Finally, each of our tap backends could have different limits on the frame > size imposed by the mechanisms they're using. > > Start clearing up this confusion by replacing it in tap.c with new > L2_MAX_LEN_* defines which specifically refer to the maximum L2 frame > length for each backend. > > Signed-off-by: David Gibson > --- > tap.c | 18 ++++++++++++++---- > tap.h | 25 +++++++++++++++++++++++++ > 2 files changed, 39 insertions(+), 4 deletions(-) > > diff --git a/tap.c b/tap.c > index fb306e75..4840dcfa 100644 > --- a/tap.c > +++ b/tap.c > @@ -62,6 +62,15 @@ > #include "vhost_user.h" > #include "vu_common.h" > > +/* Maximum allowed frame lengths (including L2 header) */ > + > +static_assert(L2_MAX_LEN_PASTA <= PACKET_MAX_LEN, > + "packet pool can't store maximum size pasta frame"); > +static_assert(L2_MAX_LEN_PASST <= PACKET_MAX_LEN, > + "packet pool can't store maximum size qemu socket frame"); > +static_assert(L2_MAX_LEN_VU <= PACKET_MAX_LEN, > + "packet pool can't store maximum size vhost-user frame"); > + > /* IPv4 (plus ARP) and IPv6 message batches from tap/guest to IP handlers */ > static PACKET_POOL_NOINIT(pool_tap4, TAP_MSGS, pkt_buf); > static PACKET_POOL_NOINIT(pool_tap6, TAP_MSGS, pkt_buf); > @@ -1097,7 +1106,8 @@ static void tap_passt_input(struct ctx *c, const struct timespec *now) > while (n >= (ssize_t)sizeof(uint32_t)) { > uint32_t l2len = ntohl_unaligned(p); > > - if (l2len < sizeof(struct ethhdr) || l2len > ETH_MAX_MTU) { > + if (l2len < sizeof(struct ethhdr) || > + l2len > L2_MAX_LEN_PASST) { No need to wrap this. > err("Bad frame size from guest, resetting connection"); > tap_sock_reset(c); > return; > @@ -1151,8 +1161,8 @@ static void tap_pasta_input(struct ctx *c, const struct timespec *now) > > tap_flush_pools(); > > - for (n = 0; n <= (ssize_t)(sizeof(pkt_buf) - ETH_MAX_MTU); n += len) { > - len = read(c->fd_tap, pkt_buf + n, ETH_MAX_MTU); > + for (n = 0; n <= (ssize_t)(sizeof(pkt_buf) - L2_MAX_LEN_PASTA); n += len) { n += len should go on its own line now. > + len = read(c->fd_tap, pkt_buf + n, L2_MAX_LEN_PASTA); > > if (len == 0) { > die("EOF on tap device, exiting"); > @@ -1170,7 +1180,7 @@ static void tap_pasta_input(struct ctx *c, const struct timespec *now) > > /* Ignore frames of bad length */ > if (len < (ssize_t)sizeof(struct ethhdr) || > - len > (ssize_t)ETH_MAX_MTU) > + len > (ssize_t)L2_MAX_LEN_PASTA) > continue; > > tap_add_packet(c, len, pkt_buf + n); > diff --git a/tap.h b/tap.h > index a2c3b87d..140e3305 100644 > --- a/tap.h > +++ b/tap.h > @@ -6,6 +6,31 @@ > #ifndef TAP_H > #define TAP_H > > +/** L2_MAX_LEN_PASTA - Maximum frame length for pasta mode (with L2 header) > + * > + * The kernel tuntap device imposes a maximum frame size of 65535 including > + * 'hard_header_len' (14 bytes for L2 Ethernet in the case of "tap" mode). Extra whitespaces in indentation. > + */ > +#define L2_MAX_LEN_PASTA USHRT_MAX > + > +/** L2_MAX_LEN_PASST - Maximum frame length for passt mode (with L2 header) > + * > + * The only structural limit the Qemu socket protocol imposes on frames is QEMU > + * (2^32-1) bytes, but that would be ludicrously long in practice. For now, > + * limit it somewhat arbitrarily to 65535 bytes. FIXME: Work out an appropriate > + * limit with more precision. > + */ > +#define L2_MAX_LEN_PASST USHRT_MAX > + > +/** L2_MAX_LEN_VU - Maximum frame length for vhost-user mode (with L2 header) > + * > + * VU allows multiple buffers per frame, each of which can be quite large, so vhost-user > + * the inherent frame size limit is rather large. Much larger than is actually > + * useful for IP. For now limit arbitrarily to 65535 bytes. FIXME: Work out an > + * appropriate limit with more precision. > + */ > +#define L2_MAX_LEN_VU USHRT_MAX > + > struct udphdr; > > /** -- Stefano