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 mail.toke.dk (Postfix) with ESMTPS id 5E10C9CD2AE for ; Thu, 8 Dec 2022 23:39:20 +0100 (CET) Authentication-Results: mail.toke.dk; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=IDYd/Z+M DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1670539159; 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: in-reply-to:in-reply-to:references:references; bh=aOy7fSBce5nodIDIVe0jCC3B4r5Wf3j3XgDgyttb5Q8=; b=IDYd/Z+MeDtkBBrLZ+lApyUe4vqYp9qDSgvAscG3ko1v6GtTLSfqLR2KiPRRzRzNQ2bzBU gfYwOZUYdHDzSMZXfhZRYHNPTQppmKOXdtxEQ1oMueQVGohd58eUy5hVv/JrbQ7j5HO6V2 QKmuuBFhDnhMgnUbk/YJZECm+dxUdVo= Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-592-zN_fybdsOAaxdfHIL8S0yw-1; Thu, 08 Dec 2022 17:39:18 -0500 X-MC-Unique: zN_fybdsOAaxdfHIL8S0yw-1 Received: by mail-ed1-f70.google.com with SMTP id m18-20020a056402511200b0046db14dc1c9so310607edd.10 for ; Thu, 08 Dec 2022 14:39:18 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=aOy7fSBce5nodIDIVe0jCC3B4r5Wf3j3XgDgyttb5Q8=; b=Vbw9g/ZkUROryBqvgjPNnphzP+5q+XC1YIO3yYXHCRfR5QG0YHPOEafj1+W6UoBEXm dzPazJxtkNeK1taY8zPSbP/t3QL6aifAOLvWQ3qfPI0aqk0jwB7oB7GkB2X3PKZs06d9 M2o6VYGlTRu83JCNadvR7WSqqZW8zK2eYNr127edQn+PmL7caQaGbCRKbTRLRa8PA+ph /MOyvxGm19Lnl9fsH7pljuGWdb/aVHx3k3ryKnTjLNfCX1RIpg+itCVDgOsa4nvNs09C M6H71Kvyn9k6MrY5qGsfsR+L6IGDOqjfR9Rfi9i4WNEoy87O0Ccm5w23Oe43Zw234Bp8 BT3Q== X-Gm-Message-State: ANoB5pnb+UlrayJ+sS7ALpJLlaGwOOr28+wxP5jHhQJpor7N2fvaX41q yi+xmFdNW6Mkr5oHLiTdipqrofvh1rOanr2zZRlFh2dn+EDFgnIlIDXMHh1cFkRoPcuZGtPcelC MSRNVIGqkZpQGZNfx7rLb X-Received: by 2002:a17:906:a0cc:b0:7ad:b791:6e37 with SMTP id bh12-20020a170906a0cc00b007adb7916e37mr4194434ejb.35.1670539157360; Thu, 08 Dec 2022 14:39:17 -0800 (PST) X-Google-Smtp-Source: AA0mqf6yFBSOtrmQ80Neu9IqpOzleiVZhN0g3CAz+oJheiDaOro3KDYQaLxqHHfr8ChP9Mm4xX+aqA== X-Received: by 2002:a17:906:a0cc:b0:7ad:b791:6e37 with SMTP id bh12-20020a170906a0cc00b007adb7916e37mr4194398ejb.35.1670539156998; Thu, 08 Dec 2022 14:39:16 -0800 (PST) Received: from alrua-x1.borgediget.toke.dk ([45.145.92.2]) by smtp.gmail.com with ESMTPSA id ga18-20020a170906b85200b00781be3e7badsm10178470ejb.53.2022.12.08.14.39.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Dec 2022 14:39:16 -0800 (PST) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 1836E82E9A4; Thu, 8 Dec 2022 23:39:15 +0100 (CET) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= To: Stanislav Fomichev , bpf@vger.kernel.org In-Reply-To: <20221206024554.3826186-4-sdf@google.com> References: <20221206024554.3826186-1-sdf@google.com> <20221206024554.3826186-4-sdf@google.com> X-Clacks-Overhead: GNU Terry Pratchett Date: Thu, 08 Dec 2022 23:39:15 +0100 Message-ID: <878rjhldv0.fsf@toke.dk> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain Message-ID-Hash: RYJXMMKO5XOEWRU62TLFE3C44XJV45RG X-Message-ID-Hash: RYJXMMKO5XOEWRU62TLFE3C44XJV45RG X-MailFrom: toke@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: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, martin.lau@linux.dev, song@kernel.org, yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com, haoluo@google.com, jolsa@kernel.org, David Ahern , Jakub Kicinski , Willem de Bruijn , Jesper Dangaard Brouer , Anatoly Burakov , Alexander Lobakin , Magnus Karlsson , Maryam Tahhan , xdp-hints@xdp-project.net, netdev@vger.kernel.org X-Mailman-Version: 3.3.7 Precedence: list Subject: [xdp-hints] Re: [PATCH bpf-next v3 03/12] bpf: XDP metadata RX kfuncs List-Id: XDP hardware hints design discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Stanislav Fomichev writes: > There is an ndo handler per kfunc, the verifier replaces a call to the > generic kfunc with a call to the per-device one. > > For XDP, we define a new kfunc set (xdp_metadata_kfunc_ids) which > implements all possible metatada kfuncs. Not all devices have to > implement them. If kfunc is not supported by the target device, > the default implementation is called instead. So one unfortunate consequence of this "fallback to the default implementation" is that it's really easy to get a step wrong and end up with something that doesn't work. Specifically, if you load an XDP program that calls the metadata kfuncs, but don't set the ifindex and flag on load, the kfunc resolution will work just fine, but you'll end up calling the default kfunc implementations (and get no data). I ran into this multiple times just now when playing around with it and implementing the freplace support. So I really think it would be a better user experience if we completely block (with a nice error message!) the calling of the metadata kfuncs if the program is not device-bound... Another UX thing I ran into is that libbpf will bail out if it can't find the kfunc in the kernel vmlinux, even if the code calling the function is behind an always-false if statement (which would be eliminated as dead code from the verifier). This makes it a bit hard to conditionally use them. Should libbpf just allow the load without performing the relocation (and let the verifier worry about it), or should we have a bpf_core_kfunc_exists() macro to use for checking? Maybe both? > Upon loading, if BPF_F_XDP_HAS_METADATA is passed via prog_flags, > we treat prog_index as target device for kfunc resolution. [...] > - if (!bpf_prog_map_compatible(map, prog)) { > - bpf_prog_put(prog); > - return ERR_PTR(-EINVAL); > - } > + /* When tail-calling from a non-dev-bound program to a dev-bound one, > + * XDP metadata helpers should be disabled. Until it's implemented, > + * prohibit adding dev-bound programs to tail-call maps. > + */ > + if (bpf_prog_is_dev_bound(prog->aux)) > + goto err; > + > + if (!bpf_prog_map_compatible(map, prog)) > + goto err; I think it's better to move the new check into bpf_prog_map_compatible() itself; that way it'll cover cpumaps and devmaps as well :) -Toke