From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f176.google.com (mail-pf1-f176.google.com [209.85.210.176]) (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 A55D03932D4 for ; Tue, 23 Jun 2026 18:29:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.176 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782239343; cv=none; b=H4W7nQrvvjd0lR2bRLa1tswZD7noGKLCGuOQDTe4VZKcJVY4JW1J9BGz2enz1GScuT/r+7xarw+udI6gjhwlLaOshA07nP34Tt5dHDftXg8snBFfJGDcE3PsdmdUJTvppINUl0vZOtc+mE6yv7taLOUrYUs9uwWKHvtntg4T16I= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782239343; c=relaxed/simple; bh=ur9YcBIx9OOlzB4KIAFGKXMYB4qRT61i90PE7nu7grM=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=hAQ8Up0BmXJX+hSJ+YH2yaXHhrFmasZhTHjo+U+Ha6S6v5Yq9AWRiZlQ3E5jElu7pvL+a/PMXmfrDnANfDe3ZrhuOA6FENKVpAzTKtCgY57PJzPx8JeR1zbh5WGtUGDH4Be3jivPUw0PdYYDPW7XD+kYjYUBPS0gEm4is/jYvEA= 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=dVAUnxd2; arc=none smtp.client-ip=209.85.210.176 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="dVAUnxd2" Received: by mail-pf1-f176.google.com with SMTP id d2e1a72fcca58-84592b55832so158091b3a.3 for ; Tue, 23 Jun 2026 11:29:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1782239342; x=1782844142; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=bTYF1+0lyIvhIpVE2MUXYwIwgLw0923N5fDH7s7CEkc=; b=dVAUnxd21rzpHnZY+B0acolaFB2lQ/rSbryQr5/laYEv5DoVJ30e40G/FimJRVPOfD V8bZ7HqM+qBS8h3ZDJTjYyFrxicvx4wfPFfSQ+dM55uMiJFeOpq5NyxldSmVYrah1laK I2vedj2wEvkv0Q7OqL/IaeA3gdEtBYEGexVyK/bqEsj/iDpqQ7gm8B674Rplj2il39gj FsdWZkUbLUAmHeIb0QLRQH8l/WT5G1JWkwcM1O0fxxLCMqvEBLQ3D4gFBrjToLmYKCVQ fYDlATYC8MDtpuYtpjLh9V5t1ei3n2+mrkQExzSHzSQFW9CYRFKf9NWh5HAj80RY/OZw ZNCg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782239342; x=1782844142; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=bTYF1+0lyIvhIpVE2MUXYwIwgLw0923N5fDH7s7CEkc=; b=mccJCnwWrni062BBOYxcvcrbAU/5Wj2EW6Rr6dyt0JQvyOkp/lwerHui0vTx4tjJFZ ArmcW5HN3i2Xpn3sOfeYxloyLGNQjYkRB9MRlaiR3odULGgIedQWMxY828ygVSnWYlfx el92t4Bpb94JFxx40P/LRA+DcAri1EjU3mpg6PJiqiwCKIRUEaKZvzyn5p2OzP5zblIw afGCe953EmEGA79AQLfX65VtG1oc04IvX389nufZEi3ry9ZtZcjphbh99Q45OeXU2fOH l2eSDXIZRsyf4IQSlMvmlykFU0A4gH0uoSYEeXR6YeIC8kl1Z7zvTuchPmd8q7xCbtHc 7icA== X-Forwarded-Encrypted: i=1; AFNElJ/E2xbrL51PWHjD7eLcV14cNivizQ1YpF/5w0uRp9R/AxnoCwLbr3KHcoa4lR3yM3Pkdy1NHVWdPRHX2U4=@vger.kernel.org X-Gm-Message-State: AOJu0YwwNR8grJb4fg3plyIQYRHj7y6KMISUITk46YQeqtt7NZ8Uk2Bj vgZkup4MBraTwFoHwztHvh6TcMk8/hUaBOiDNOcx1ir6wWfKBLoULogM X-Gm-Gg: AfdE7clt0YHDgrs0CfQ0kqLg9T2J5PmP2ovkjq5yRSpcGky8p0+6QqIrQPMKVat1D9w ox9jM2Lfu82DGOTELWIDY1Ef9dP5ASPioyVySYeVG5iekfosYt+loY04SvxHFUW24/G7KUP3ehN 0ajP5ZuPOnvBWE5sSgHA/Rrj9TAN1lzBU7dCs46c47Yp9zOOS2s8h1d38w2EKN2wjcE+IJmCYXy GWEpMu08GO2zqmFHlzD1gfhnlxmh0XfOEVUPtPfs0/iPyji4CFWJ0VLAGPzUaHofDR80cZ4T3rn 9LBWuq5hAIWr2N0UR88+ZB4khdcl3QJCP1oaFVu0YBXtHLmdaX7Z8PGO5kRTlhSwwQ3Ig75G8sd z0uth+3UbfjLdllFRULE9VRtUjaxzarsVuV4utD7xE5/c8XkFHma5LtvL+zYQ9OqXlEJiKVVD4R uEfhWen++jaKXh7kf+lzUf1qMvNZJfk81mlpCMzn5UFuEJ0TZ5ZkqYGqZN6b7i X-Received: by 2002:a05:6a00:6c90:b0:845:3033:6cb7 with SMTP id d2e1a72fcca58-8459524d4d5mr5244859b3a.9.1782239341727; Tue, 23 Jun 2026 11:29:01 -0700 (PDT) Received: from r912.tailbb6e1e.ts.net ([182.70.116.80]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-84564d6c5dbsm10835213b3a.12.2026.06.23.11.28.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Jun 2026 11:29:01 -0700 (PDT) From: Avinash Duduskar To: =?UTF-8?q?Toke=20H=C3=B8iland-J=C3=B8rgensen?= , ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org Cc: eddyz87@gmail.com, memxor@gmail.com, martin.lau@linux.dev, song@kernel.org, yonghong.song@linux.dev, jolsa@kernel.org, emil@etsalapatis.com, john.fastabend@gmail.com, sdf@fomichev.me, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, horms@kernel.org, shuah@kernel.org, hawk@kernel.org, yatsenko@meta.com, leon.hwang@linux.dev, kpsingh@kernel.org, a.s.protopopov@gmail.com, ameryhung@gmail.com, rongtao@cestc.cn, eyal.birger@gmail.com, bpf@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, dsahern@kernel.org Subject: Re: [PATCH bpf-next v4 1/3] bpf: Add BPF_FIB_LOOKUP_VLAN flag to bpf_fib_lookup() helper Date: Tue, 23 Jun 2026 23:58:49 +0530 Message-ID: <20260623182849.2623521-1-avinash.duduskar@gmail.com> X-Mailer: git-send-email 2.54.0 In-Reply-To: <877bnpeaeq.fsf@toke.dk> References: <20260623025147.1001664-1-avinash.duduskar@gmail.com> <20260623025147.1001664-2-avinash.duduskar@gmail.com> <877bnpeaeq.fsf@toke.dk> 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: 8bit Toke Høiland-Jørgensen writes: > I think it's better to just move the assignment of params->ifindex > entirely into bpf_fib_set_fwd_params(), instead of this restore dance. > That way this can be simplified to: > > err = bpf_fib_set_fwd_params(dev, params, flags, mtu); > if (!err && fwd_dev) > *fwd_dev = dev; > return err; The caller-side restore is ungainly, agreed, but the assignment can't move all the way into the helper. The early params->ifindex = dev->ifindex sits above the neighbour lookup on purpose: that is d1c362e1dd68a ("bpf: Always return target ifindex in bpf_fib_lookup"), which took it out of bpf_fib_set_fwd_params() and put it there so a program still gets the target ifindex on the BPF_FIB_LKUP_RET_NO_NEIGH path and can bpf_redirect_neigh() on it. bpf_fib_set_fwd_params() is called only at the set_fwd_params label, below the NO_NEIGH return (and below the IPv6 NO_SRC_ADDR return), so an assignment living in the helper never runs on those paths and params->ifindex falls back to the input. That would change the reported ifindex for plain bpf_fib_lookup() callers hitting NO_NEIGH, not only the VLAN ones. I can still get the caller down to your form by keeping the early write and moving just the VLAN_FAILURE rewind into the helper, with one extra parameter, the input ifindex saved before the egress write: err = bpf_fib_set_fwd_params(dev, params, flags, mtu, in_ifindex); if (!err && fwd_dev) *fwd_dev = dev; return err; and the helper owning the rewind in the unreducible branch: } else { params->ifindex = in_ifindex; return BPF_FIB_LKUP_RET_VLAN_FAILURE; } So the restore leaves the caller; the early egress write stays because NO_NEIGH and NO_SRC_ADDR depend on it. 3/3 adds a NO_NEIGH arm that pins the egress ifindex (input != egress): with the assignment moved into the helper, that case reports the input ifindex instead, while the return code stays NO_NEIGH, only the ifindex flips. It passes with the early write kept. > If you move the ifdef into the if statement, the if statement can have > an else-branch that assigns params->ifindex, so you don't need the > restore dance (see below). Same constraint: an else-branch inside bpf_fib_set_fwd_params() only runs when the helper runs, which is never on the NO_NEIGH/NO_SRC_ADDR returns, so it cannot be the sole writer of the egress ifindex. Does the in_ifindex version look right to you? The alternative is to route the error returns through the label so the assignment can live fully in the helper; threading the return codes back through it works, but it is its own kind of dance and reads worse to me. Thanks, Avinash