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 933215A0272 for ; Wed, 7 Feb 2024 11:41:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1707302481; 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=+3t/B8m9XzC2NWAbGO9Cw1Ms4OZ7dqZM12CkV+Xi3Rg=; b=LqltnwDdbNvBLSVG+A8ZoGf4GpgFz3NA8mV62RQsXrmmV7Os+rYPAkZ7RXf0QIGKYvC9JV 2SiiXaUBzN7Wi96EKA86M/clLEyF3lagwR9E7sBECVplConX6OqGAEGNpwIdpQJDlNPbnr T8wsOGgr1aC2Bzof+8KAEDOe5Vo5sZc= Received: from mail-lf1-f71.google.com (mail-lf1-f71.google.com [209.85.167.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-542-wDYx3lnhOfOk5Wc0bNIsxw-1; Wed, 07 Feb 2024 05:41:20 -0500 X-MC-Unique: wDYx3lnhOfOk5Wc0bNIsxw-1 Received: by mail-lf1-f71.google.com with SMTP id 2adb3069b0e04-5115b1e3facso394783e87.3 for ; Wed, 07 Feb 2024 02:41:19 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707302477; x=1707907277; 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=+3t/B8m9XzC2NWAbGO9Cw1Ms4OZ7dqZM12CkV+Xi3Rg=; b=xEiDEsu5DCR8nHSY7UBX75Zsppye7i2Yj6DyoE8TqQUrB3009dh5CGLm6bRrDZmPqZ Qfri+6f6BCYsQNNCoNdix8BUcE+D0Z06HBNIo5lj9gTJv/fU0EE8RbIyRB1mtxiMKPuM Hza2iLO5ey1FFR6xRaeqXOyGnhMMCV8Yb8EfCXi3n5wSAap3j4202Xy4F4YDerzm5NQh hwlnLugfEDZPyltnXlvyDB+X4EshY6iIYMupmE7ISsOX74lf4steDU3mgjNeHV5FgM5E F6Z55V2G+73OQO7SHXxHMiCZerk0y48qoj+2+hvDvHD97cMNi8ZMW38Vd0YcWHCXH3C5 89UQ== X-Gm-Message-State: AOJu0Yw9qclEH/r+YDjzRg81xcgEt7i+V6QNIV/jt3o9aBZROnSmTrEW 9+tiJC+d3k9TZWZ8KQiRMZe+bFZ9ChrFCapBoqNYmkoVAstqQJCh3zWnBQCSp+/TX+u09QDkjXf +zMALhEhZBzIbqFDlKGqJYsE1kWq9AruloQumtN9fCMM7RxtZwx7JpH3BcQJx3BsKEfaVLlyiu8 gXHaz5maBM4xgQGsM2g6ylL1M+PWt7SfDlHlA= X-Received: by 2002:ac2:593a:0:b0:511:4edb:5acb with SMTP id v26-20020ac2593a000000b005114edb5acbmr3569040lfi.64.1707302477361; Wed, 07 Feb 2024 02:41:17 -0800 (PST) X-Google-Smtp-Source: AGHT+IELDI1ckh+9nzMKC3NZ23yz3ppPNoDlp4O6n1wwLH/Iiu0CNISBY3iQbDSK1iWtd0ySIY6lxw== X-Received: by 2002:ac2:593a:0:b0:511:4edb:5acb with SMTP id v26-20020ac2593a000000b005114edb5acbmr3569017lfi.64.1707302476773; Wed, 07 Feb 2024 02:41:16 -0800 (PST) Received: from maya.cloud.tilaa.com (maya.cloud.tilaa.com. [164.138.29.33]) by smtp.gmail.com with ESMTPSA id lc2-20020a170906f90200b00a37ad2c72dasm598084ejb.183.2024.02.07.02.41.15 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 07 Feb 2024 02:41:16 -0800 (PST) Date: Wed, 7 Feb 2024 11:40:40 +0100 From: Stefano Brivio To: Laurent Vivier Subject: Re: [PATCH 06/24] ip: move duplicate IPv4 checksum function to ip.h Message-ID: <20240207114040.78e7081f@elisabeth> In-Reply-To: <20240202141151.3762941-7-lvivier@redhat.com> References: <20240202141151.3762941-1-lvivier@redhat.com> <20240202141151.3762941-7-lvivier@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.1.1 (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: 5EUI7BMT4XFKPMZRRLLRLREIE2PH7POY X-Message-ID-Hash: 5EUI7BMT4XFKPMZRRLLRLREIE2PH7POY 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 Fri, 2 Feb 2024 15:11:33 +0100 Laurent Vivier wrote: > We can find the same function to compute the IPv4 header > checksum in tcp.c and udp.c > > Signed-off-by: Laurent Vivier > --- > ip.h | 14 ++++++++++++++ > tcp.c | 23 ++--------------------- > udp.c | 19 +------------------ > 3 files changed, 17 insertions(+), 39 deletions(-) > > diff --git a/ip.h b/ip.h > index b2e08bc049f3..ff7902c45a95 100644 > --- a/ip.h > +++ b/ip.h > @@ -9,6 +9,8 @@ > #include > #include > > +#include "checksum.h" > + > #define IN4_IS_ADDR_UNSPECIFIED(a) \ > ((a)->s_addr == htonl_constant(INADDR_ANY)) > #define IN4_IS_ADDR_BROADCAST(a) \ > @@ -83,4 +85,16 @@ struct ipv6_opt_hdr { > > char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *proto, > size_t *dlen); > +static inline uint16_t ipv4_hdr_checksum(struct iphdr *iph, int proto) A function comment would be nice. A couple of doubts: - why is this an inline in ip.h, instead of a function in checksum.c? That would be more natural, I think - this would be the first Layer-4 protocol number passed as int: we use uint8_t elsewhere. Now, socket(2) and similar all take an int, but using uint8_t internally keeps large arrays such as tap4_l4 a bit smaller. The only value defined in Linux UAPI exceeding eight bits is IPPROTO_MPTCP, 262, because that's never on the wire (the TCP protocol number is used instead). And we won't meet that either. In practice, it doesn't matter what we use here, but still uint8_t would be consistent. > +{ > + uint32_t sum = L2_BUF_IP4_PSUM(proto); > + > + sum += iph->tot_len; > + sum += (iph->saddr >> 16) & 0xffff; > + sum += iph->saddr & 0xffff; > + sum += (iph->daddr >> 16) & 0xffff; > + sum += iph->daddr & 0xffff; > + > + return ~csum_fold(sum); > +} > #endif /* IP_H */ > diff --git a/tcp.c b/tcp.c > index 4c9c5fb51c60..293ab12d8c21 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -934,23 +934,6 @@ static void tcp_sock_set_bufsize(const struct ctx *c, int s) > trace("TCP: failed to set SO_SNDBUF to %i", v); > } > > -/** > - * tcp_update_check_ip4() - Update IPv4 with variable parts from stored one > - * @buf: L2 packet buffer with final IPv4 header > - */ > -static void tcp_update_check_ip4(struct tcp4_l2_buf_t *buf) > -{ > - uint32_t sum = L2_BUF_IP4_PSUM(IPPROTO_TCP); > - > - sum += buf->iph.tot_len; > - sum += (buf->iph.saddr >> 16) & 0xffff; > - sum += buf->iph.saddr & 0xffff; > - sum += (buf->iph.daddr >> 16) & 0xffff; > - sum += buf->iph.daddr & 0xffff; > - > - buf->iph.check = (uint16_t)~csum_fold(sum); > -} > - > /** > * tcp_update_check_tcp4() - Update TCP checksum from stored one > * @buf: L2 packet buffer with final IPv4 header > @@ -1393,10 +1376,8 @@ do { \ > b->iph.saddr = a4->s_addr; > b->iph.daddr = c->ip4.addr_seen.s_addr; > > - if (check) > - b->iph.check = *check; > - else > - tcp_update_check_ip4(b); > + b->iph.check = check ? *check : > + ipv4_hdr_checksum(&b->iph, IPPROTO_TCP); > > SET_TCP_HEADER_COMMON_V4_V6(b, conn, seq); > > diff --git a/udp.c b/udp.c > index d514c864ab5b..6f867df81c05 100644 > --- a/udp.c > +++ b/udp.c > @@ -270,23 +270,6 @@ static void udp_invert_portmap(struct udp_port_fwd *fwd) > } > } > > -/** > - * udp_update_check4() - Update checksum with variable parts from stored one > - * @buf: L2 packet buffer with final IPv4 header > - */ > -static void udp_update_check4(struct udp4_l2_buf_t *buf) > -{ > - uint32_t sum = L2_BUF_IP4_PSUM(IPPROTO_UDP); > - > - sum += buf->iph.tot_len; > - sum += (buf->iph.saddr >> 16) & 0xffff; > - sum += buf->iph.saddr & 0xffff; > - sum += (buf->iph.daddr >> 16) & 0xffff; > - sum += buf->iph.daddr & 0xffff; > - > - buf->iph.check = (uint16_t)~csum_fold(sum); > -} > - > /** > * udp_update_l2_buf() - Update L2 buffers with Ethernet and IPv4 addresses > * @eth_d: Ethernet destination address, NULL if unchanged > @@ -614,7 +597,7 @@ static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport, > b->iph.saddr = b->s_in.sin_addr.s_addr; > } > > - udp_update_check4(b); > + b->iph.check = ipv4_hdr_checksum(&b->iph, IPPROTO_UDP); > b->uh.source = b->s_in.sin_port; > b->uh.dest = htons(dstport); > b->uh.len = htons(udp4_l2_mh_sock[n].msg_len + sizeof(b->uh)); -- Stefano