From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id A8ADD5A0274 for ; Thu, 29 Feb 2024 17:30:39 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1709224238; 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=cNzF/pKzRwwfsi0Svc9vA5LxSW0xA5z7N/FUhIxZs4s=; b=cyWxHHGR8Agt3H6yC5XGj3v338Rty7KSyk5rowVdjnJJ2OXVitA/zkYbZmIM+3+Qi/ndlR hBP1k7/8WgadGhEVuvLnYzV0LXADcftKMfNNY/3Skd7whkhlCFWb2ZHTfZNkojHu9W2Atj xD6+qwaYksR38D3sOZK3sG3Cyp2zz/Q= Received: from mail-ej1-f69.google.com (mail-ej1-f69.google.com [209.85.218.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-372-aQ2GLRdhPdOxbD2LkqYqew-1; Thu, 29 Feb 2024 11:30:37 -0500 X-MC-Unique: aQ2GLRdhPdOxbD2LkqYqew-1 Received: by mail-ej1-f69.google.com with SMTP id a640c23a62f3a-a3f84b660b8so86822766b.0 for ; Thu, 29 Feb 2024 08:30:37 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709224235; x=1709829035; 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=cNzF/pKzRwwfsi0Svc9vA5LxSW0xA5z7N/FUhIxZs4s=; b=iQoPuQIOK5RQLv23ZFc1k+lGRyKpcSw26/nc/bnCkESzLAsrteF4MDoedlT8GPiQsW jdSep3qsQjmF0/am2+7WpEp18lAk8HwCZFtOW2fpE4Ksdmp8JK2m57cj3i33tx1xLUYe 6e1RxVntHugL+bg/BRAxx+z2T1ZnE/aYzz/qR+p+LHrR2XKICuMU0qoK6Q7Z7ZwhDckG JauqrQYm3IgCPAF9TyU6sbmbK40k24Yp3/AVJuGpBPLOm8U4b4xxynLLDoYLWEtYBKNO yXQGLguqPBRTYKfvXfyh+yLGjp3ynpvSLR5azFfFjdmrSu6xgkkF4dWMjBirHnTkDbj8 Jufw== X-Gm-Message-State: AOJu0YzLqp/MvLgFc6JaF4ZO9qb7o5lkj+88MkvJXoIz0p4WgZyVazWV JtQPBI2qVo+Qds6qPS+E806s17NL4d/5VD50YDbyvY6LF0+sXMvLMsYPVaaT35BURp95npzJcn1 El5YCD0XQBewgK3gffUOoaCVQ2Vc50A7yA3FoDWCpTeaso/2hJk6PEwxoMjNWJBnIX3DETEEX0H eWzPPEMaxQ8psI/kWRR25gDKq3iJn2NgXY7ZA= X-Received: by 2002:a17:906:81d5:b0:a44:48c5:83de with SMTP id e21-20020a17090681d500b00a4448c583demr1366150ejx.20.1709224235057; Thu, 29 Feb 2024 08:30:35 -0800 (PST) X-Google-Smtp-Source: AGHT+IE/01RvCPWo1GhWhMbnhlzQqzqFhaIs5g5nECWgNT6z1V5+Cuw6aek81NCXar4r+YtqIT6v4w== X-Received: by 2002:a17:906:81d5:b0:a44:48c5:83de with SMTP id e21-20020a17090681d500b00a4448c583demr1366133ejx.20.1709224234644; Thu, 29 Feb 2024 08:30:34 -0800 (PST) Received: from maya.cloud.tilaa.com (maya.cloud.tilaa.com. [164.138.29.33]) by smtp.gmail.com with ESMTPSA id gb18-20020a170907961200b00a3efa4e033asm830359ejc.151.2024.02.29.08.30.33 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 29 Feb 2024 08:30:34 -0800 (PST) Date: Thu, 29 Feb 2024 17:29:59 +0100 From: Stefano Brivio To: Laurent Vivier Subject: Re: [PATCH v3 9/9] tcp: Introduce ipv4_fill_headers()/ipv6_fill_headers() Message-ID: <20240229172959.797f4ca0@elisabeth> In-Reply-To: <20240217150725.661467-10-lvivier@redhat.com> References: <20240217150725.661467-1-lvivier@redhat.com> <20240217150725.661467-10-lvivier@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.36; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: JW6N3GND5HZJJZ4KIHH6ZIKLLO6QSSRE X-Message-ID-Hash: JW6N3GND5HZJJZ4KIHH6ZIKLLO6QSSRE 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 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: On Sat, 17 Feb 2024 16:07:25 +0100 Laurent Vivier wrote: > Replace the macro SET_TCP_HEADER_COMMON_V4_V6() by a new function > tcp_fill_header(). > > Move IPv4 and IPv6 code from tcp_l2_buf_fill_headers() to > tcp_fill_ipv4_header() and tcp_fill_ipv6_header() > > Signed-off-by: Laurent Vivier > --- > > Notes: > v3: > - add to sub-series part 1 > > v2: > - extract header filling functions from > "tcp: extract buffer management from tcp_send_flag()" > - rename them tcp_fill_flag_header()/tcp_fill_ipv4_header(), > tcp_fill_ipv6_header() > - use upside-down Christmas tree arguments order > - replace (void *) by (struct tcphdr *) > > tcp.c | 154 +++++++++++++++++++++++++++++++++++++++------------------- > 1 file changed, 104 insertions(+), 50 deletions(-) > > diff --git a/tcp.c b/tcp.c > index aa03c20712f6..bc57a4f6e611 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -1324,6 +1324,108 @@ void tcp_defer_handler(struct ctx *c) > tcp_l2_data_buf_flush(c); > } > > +/** > + * tcp_fill_header() - Fill the TCP header fields for a given TCP segment. > + * > + * @th: Pointer to the TCP header structure > + * @conn: Pointer to the TCP connection structure > + * @seq: Sequence number > + */ > +static void tcp_fill_header(struct tcphdr *th, > + const struct tcp_tap_conn *conn, uint32_t seq) > +{ > + th->source = htons(conn->fport); > + th->dest = htons(conn->eport); > + th->seq = htonl(seq); > + th->ack_seq = htonl(conn->seq_ack_to_tap); > + if (conn->events & ESTABLISHED) { > + th->window = htons(conn->wnd_to_tap); > + } else { > + unsigned wnd = conn->wnd_to_tap << conn->ws_to_tap; > + > + th->window = htons(MIN(wnd, USHRT_MAX)); > + } > +} > + > +/** > + * tcp_fill_ipv4_header() - Fill 802.3, IPv4, TCP headers in pre-cooked buffers > + * @c: Execution context > + * @conn: Connection pointer > + * @iph: Pointer to IPv4 header, immediately followed by a TCP header > + * @plen: Payload length (including TCP header options) > + * @check: Checksum, if already known > + * @seq: Sequence number for this segment > + * > + * Return: IP frame length including L2 headers, host order > + */ > +static size_t tcp_fill_ipv4_header(const struct ctx *c, > + const struct tcp_tap_conn *conn, > + struct iphdr *iph, size_t plen, > + const uint16_t *check, uint32_t seq) > +{ > + size_t ip_len = plen + sizeof(struct iphdr) + sizeof(struct tcphdr); > + const struct in_addr *a4 = inany_v4(&conn->faddr); > + struct tcphdr *th = (struct tcphdr *)(iph + 1); > + > + iph->tot_len = htons(ip_len); > + iph->saddr = a4->s_addr; The reasoning behind the fact that a4 isn't NULL here is relatively simple to follow: you already check for inany_v4(&conn->faddr) in the caller, if it evaluates to true, you call this. Still, it's a bit too convoluted for Coverity's taste. Could you perhaps add an ASSERT(a4) before this block to make it obvious? It's a bit annoying that we extract the address twice, but I don't see a much better alternative compared to what you did. -- Stefano