From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yw1-f177.google.com (mail-yw1-f177.google.com [209.85.128.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B958A230264 for ; Tue, 12 May 2026 00:51:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.177 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778547094; cv=none; b=GP0UQwF0v2e23LDXnTkOJFEXO5IlxLzgKq/csz4UW2r1qXQFfbjf3o69URJWhZ06XbpJMLdUvU9CxIaPrR89RgRQA+0T71YjguKJy5G3wIQFVlRN/xGp/M4ZqQtKHnsFbfQbjknXd1zyv4Vs1WuZg1PEQJGZH8jTfF7H5lv1lGs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778547094; c=relaxed/simple; bh=f0RRwhVwZZY0XRS6uTtiUkwdoDW0fO8pPZBXqz2/eOM=; h=Date:From:To:Cc:Message-ID:In-Reply-To:References:Subject: Mime-Version:Content-Type; b=IzVsKtKwp2ka/+YK9Q776DWdzoQjAiVk2aBcd6pblqnshK9Hd7omHKfT8CBkXg4wVaBCsQH3D6M0pxe8gdgSD5UB2hb4rfUCjVkM1MhSKKFFqjjN03I2ysi+lhHRSAHPODs+XqCx2JIuUkNdbWoI8LZCVg+d4Pyh8mIAwcFDRnk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=g1xZLS8x; arc=none smtp.client-ip=209.85.128.177 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="g1xZLS8x" Received: by mail-yw1-f177.google.com with SMTP id 00721157ae682-7bf0b47d2f1so46090187b3.3 for ; Mon, 11 May 2026 17:51:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1778547092; x=1779151892; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:subject:references :in-reply-to:message-id:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=zziufg3DXdLJHPbUcdcMN7kXgJx9C/jwfLS4a4Ly0sw=; b=g1xZLS8xJdRtCEV0JR9MGKUMi2yqWthQR20n9gVvDyEulEgEt1dS1AMc0UYZMD64jP uJgh7TgsuWeZMNxi33r47uUBrvZ8xf+8Z0m9NnbWknAYx8FSXGjJ9Rg5ZGaUkZQD7epz qRGnKmn7ovyhz80pusP9Cwr2PAuTXBmkCFxPgIlqHvHX4XsXswI/7gkQEQwO3CSrazYj SOfqD9iJ6Uu3UHBnOXiofnf5c6zpKlfq/HCN8U/M9FPUC0+pY2hKv9iUnVs7RD1nX3n0 WUUbqDfXq/UrR7fuVgJ8nQCXxsM5XHnoOzbqqRzV8+AkDD69ufzjdolElU2+rAu2ySAf yCRg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1778547092; x=1779151892; h=content-transfer-encoding:mime-version:subject:references :in-reply-to:message-id:cc:to:from:date:x-gm-gg:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=zziufg3DXdLJHPbUcdcMN7kXgJx9C/jwfLS4a4Ly0sw=; b=mvoAA8gupm+EOgDFn1dZH8aFzBObVLO4yldLkJRLQg4X9liP4IRUqGVvlilSYo83Yg KL9RAOg0sTVj/htUq1cixeGHmcBZKzp42oKMIswFVS1r/F25Qfj5ezlLR061lfW3tA3A aMEruOfUz1WdgCj2gXpuwwLTAzYsJiNOkucAH8WNBALnDp6xRzVjVE0h2ORxwXaESxij tm0BXArEjNRdlEjFfpW7XYgC+Rg5wVFFIzLljnZYb/PADr433RnzQrp6gicLpNe+ny8N FCX84hMR0s1aJzqetiKyhUdfrjGiBD15LyS13v/36xOa2NOkvhXaDj5BAct5396Xct7A M1dQ== X-Forwarded-Encrypted: i=1; AFNElJ/KcdCd7UBz/Xrfx/E2zRmny+UlbAMPbLPKHywzf9gwmMWRrJroQzGQ1nSSXIueaFyIbm/SR1z9WbKENDg=@vger.kernel.org X-Gm-Message-State: AOJu0YzFIWB9hZAyZ1PEMi1NHPwh4+a/9OpXwYic5BBcfCiMJhsXIHCI YpNivXj2JcT5opZU48gPd2if/7fuaUa3NvPdbBhTbKTgRYh8ozBuq/eq X-Gm-Gg: Acq92OG3sCPtS4Z8CNlfj5uwowjcyC+jaFtsKSNENvQldk4DOl1Sjjv6Rg02RG3nDYr OoQohZi85cQhI3I2oxgU68y43yVpLW/2amCw3j0Sge0C7tWXXJ7OqW+G5JMj4o8uSR2J6nzjudp oyWSZlD7nUfQadI/2VR/oQMxZpcA2stSRoeZEzjKNH8z1wkzjKc4gNhPWSRU/hSwYmYyXgLR8ue 59yGmSibltXTbt+TcLPzfwsi1845H0yAzHqipz0ZuLXTXaGIiqfSgVHjBd4gWEZoT8Ng8VO5XPv KIiVT8OKgmlmBb3/XnAugkbzF6wQ9Y5KCLEjaxhKVoldZeLpVLNNUqirhViN/Jw5G8oKLn4Wmea 0d97eBYc1N/z/HnXZ/TP4Njaw+AZOJ6Yi2YWspUti/cP0DUSG0mnhvShN+6yJbKC/+CGZixvjax 3pX9FZL+R3hOQ7pE9HO6GwK+uHoSPdRmmQXF4PBzP9H1NwsAkIT+oAJZEQklxaU3BEW2+TDi4i8 mjc X-Received: by 2002:a05:690c:e3ce:b0:7bd:9f34:5998 with SMTP id 00721157ae682-7bfb9aa172emr168105257b3.34.1778547091734; Mon, 11 May 2026 17:51:31 -0700 (PDT) Received: from gmail.com (172.235.85.34.bc.googleusercontent.com. [34.85.235.172]) by smtp.gmail.com with ESMTPSA id 00721157ae682-7bd6683836asm160318087b3.25.2026.05.11.17.51.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 11 May 2026 17:51:31 -0700 (PDT) Date: Mon, 11 May 2026 20:51:30 -0400 From: Willem de Bruijn To: Daniel Zahka , Willem de Bruijn , Jakub Kicinski , Andrew Lunn , "David S. Miller" , Eric Dumazet , Paolo Abeni Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org Message-ID: In-Reply-To: References: <20260508-nsim-psp-crypto-v1-0-4b50ed09b794@gmail.com> <20260508-nsim-psp-crypto-v1-3-4b50ed09b794@gmail.com> Subject: Re: [PATCH net-next 3/6] netdevsim: psp: move rx processing into nsim_poll() Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Daniel Zahka wrote: > > On 5/11/26 4:03 PM, Willem de Bruijn wrote: > > Daniel Zahka wrote: > >> nsim_do_psp() does PSP decap and skb extension creation in the tx > >> path. This has the slightly undesirable property of not allowing the > >> psp rx code to run on PSP packets cooked up in userspace and > >> transmitted on a packet socket from the peer dev (e.g. packetdrill). > > Whether this happens in the nsim_start_xmit tx side handler directly > > or is deferred to nsim_napi_rx is irrelevant, isn't it? > > > You're right. The way netdevsim works, it is entirely immaterial. I'll > correct the erroneous commit message, but I still think having the decap > code in the napi_poll side makes a little bit more logical sense here. Agreed > > >> +/* Returns true if skb was consumed, false otherwise. */ > >> +bool nsim_psp_handle_rx(struct netdevsim *ns, struct sk_buff *skb) > >> +{ > >> + struct psp_dev *psd; > >> + struct psphdr *psph; > >> + struct udphdr *uh; > >> + int payload_len; > >> + u32 versions; > >> + int psp_off; > >> + bool is_udp; > >> + int l3_hlen; > >> + u8 version; > >> + u32 psd_id; > >> + int err; > >> + > >> + if (skb->protocol == htons(ETH_P_IP)) { > >> + struct iphdr *iph; > >> + > >> + if (!pskb_may_pull(skb, sizeof(struct iphdr))) > >> + return false; > >> + > >> + iph = (struct iphdr *)skb->data; > >> + if (iph->ihl < 5) > >> + return false; > >> + > >> + is_udp = iph->protocol == IPPROTO_UDP; > >> + l3_hlen = iph->ihl * 4; > >> + } else if (skb->protocol == htons(ETH_P_IPV6)) { > >> + struct ipv6hdr *ip6h; > >> + > >> + if (!pskb_may_pull(skb, sizeof(struct ipv6hdr))) > >> + return false; > >> + ip6h = (struct ipv6hdr *)skb->data; > >> + is_udp = ip6h->nexthdr == IPPROTO_UDP; > >> + l3_hlen = sizeof(struct ipv6hdr); > >> + } else { > >> + return false; > >> + } > >> + > >> + if (!is_udp) > >> + return false; > >> + > >> + if (!pskb_may_pull(skb, l3_hlen + sizeof(struct udphdr) + PSP_HDR_SIZE)) > >> + return false; > >> + > >> + uh = (struct udphdr *)(skb->data + l3_hlen); > >> + if (uh->dest != htons(PSP_DEFAULT_UDP_PORT)) > >> + return false; > >> + > >> + psph = (struct psphdr *)(uh + 1); > >> + version = FIELD_GET(PSPHDR_VERFL_VERSION, psph->verfl); > > This seems to reimplement a lot of psp_dev_rcv. Is that needed? > > > > Is it a hint that this psp driver API needs some work? > > > It could be. I'd have to split the parsing from the decap logic in > psp_dev_rcv(). I just wonder if another user of the two separate halves > other than netdevsim will come along. Fair. Why does this driver need it. Only for that version check? If so, can that move after the generic decap. > > >> + rcu_read_lock(); > >> + psd = rcu_dereference(ns->psp.dev); > >> + if (psd) { > >> + versions = READ_ONCE(psd->config.versions); > >> + psd_id = psd->id; > >> + } > >> + rcu_read_unlock(); > >> + > >> + if (!psd || !(versions & (1 << version))) { > >> + skb->ip_summed = CHECKSUM_NONE; > >> + return false; > >> + } > >> + > >> + psp_off = l3_hlen + sizeof(struct udphdr); > >> + payload_len = skb->len - psp_off - PSP_HDR_SIZE - PSP_TRL_SIZE; > >> + if (payload_len < 0) > >> + goto drop; > >> + > >> + skb_push(skb, ETH_HLEN); > >> + skb->mac_len = ETH_HLEN; > >> + err = psp_dev_rcv(skb, psd_id, 0, false); > >> + if (err) > >> + goto drop; > >> + > >> + skb_reset_mac_header(skb); > >> + skb_pull(skb, ETH_HLEN); > > Similarly this is a bit of a hack, pushing and pulling a fake Ethernet > > offset. > > > > And is this skb_reset_mac_header needed? > > > skb_reset_mac_header() is needed because psp_dev_rcv() shifts the l2 and l3 headers forward, and the mac header has already been set. psp_dev_rcv() expects the mac header to be there. I hear what you're saying though. The driver api could probably handle these for us, but I also didn't want to overfit to netdevsim without another user. I'll explore some options before I post this again. Ack, thanks.