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 4BE8E85215D for ; Fri, 28 May 2021 17:33:14 +0200 (CEST) Authentication-Results: mail.toke.dk; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=T0RiavZ+ DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1622215991; 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=aG7aKcU8mSqQi/NUnDmaA2VHTBJVy10N7rbmevbo+iU=; b=T0RiavZ+5ObUmqWLE2oZbAkkKIm2mlxQsoZ7FV23x/KJpiBDSdIGIJqmL5uI0Z5wIXpZJR QHBxXsVsHazqhcpW4beEccx/71dhEKhbZzuL9RxQqnOJA0GU7HMLD3iLdCKe0kQZBoZUwl TMulNwzlOQtVMUXn59yt8abzYt6pdF0= Received: from mail-ed1-f72.google.com (mail-ed1-f72.google.com [209.85.208.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-76-R7diUvIPO5GFrzweDqmduw-1; Fri, 28 May 2021 11:33:10 -0400 X-MC-Unique: R7diUvIPO5GFrzweDqmduw-1 Received: by mail-ed1-f72.google.com with SMTP id s10-20020aa7d78a0000b029038fa8d5e1e1so2302859edq.10 for ; Fri, 28 May 2021 08:33:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version:content-transfer-encoding; bh=wf8GYvzF/y2zAgAsqAPYzyetAWdd3I1acwWIFartyWg=; b=bskRYj+eHhKHuzCuH2Z1xmLwgPwv/mP5vE4DB93wIDehKadSa9j8mto4MfNJJCDchT Jm+turS8MeeztCE4Xrjf5mW/Vic+SUuFR4BZBxOUfVSQy+mzfa3jyf8oN/VsVWmi+AEb wM1DFbrMbe4AQLv7dDTDJHe3wS94bCJ+Yuno9v2muA2eej+Rb02NZo7SOgG5sDAcsJRF a4yR637WZ8WALsGXxIqAijkmlwc8VbgcP+berK12YwkIt35PAg1bVHsv4mDcpc+8w5ux p5e6Mcriv/H4OoTo9B7388AVXu2uSEPblw0KV+/B1sVTofJooSD7XhJ877XRQZwGTnod iH+A== X-Gm-Message-State: AOAM530cywckbo03VkTg7RyITieFREYzS4bzznQEMWL3MuXicij9KLM1 5WkYV2qBK+jyfwp7dRMUWlAgPD+GsWq9/h4cjNWt+LXI1LXMemF587leF9prXyt5jeQq6EvtF+w AHbpAks+XvODlPfSV5wd9 X-Received: by 2002:aa7:c0c4:: with SMTP id j4mr10434626edp.168.1622215988524; Fri, 28 May 2021 08:33:08 -0700 (PDT) X-Google-Smtp-Source: ABdhPJy4dQjvpDrn5nR3EMb+ZD1h4G6MnSzksoEjOi5G9oMD3STu/Sp8ix73ofJUcllxd6lbCi3JEw== X-Received: by 2002:aa7:c0c4:: with SMTP id j4mr10434567edp.168.1622215988036; Fri, 28 May 2021 08:33:08 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id p11sm2854758edt.22.2021.05.28.08.33.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 28 May 2021 08:33:07 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 5BDFB180720; Fri, 28 May 2021 17:33:06 +0200 (CEST) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= To: John Fastabend , Andrii Nakryiko Subject: Re: XDP-hints: Howto support multiple BTF types per packet basis? In-Reply-To: <60b0ffb63a21a_1cf82089e@john-XPS-13-9370.notmuch> References: <20210526125848.1c7adbb0@carbon> <60aeb01ebcd10_fe49208b8@john-XPS-13-9370.notmuch> <60aeeb5252147_19a622085a@john-XPS-13-9370.notmuch> <60b08442b18d5_1cf8208a0@john-XPS-13-9370.notmuch> <87fsy7gqv7.fsf@toke.dk> <60b0ffb63a21a_1cf82089e@john-XPS-13-9370.notmuch> X-Clacks-Overhead: GNU Terry Pratchett Date: Fri, 28 May 2021 17:33:06 +0200 Message-ID: <87o8cug9fx.fsf@toke.dk> MIME-Version: 1.0 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=toke@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Message-ID-Hash: 5EBSFQU2NGEBMDNSV7TWJNEYWU3TKPJ4 X-Message-ID-Hash: 5EBSFQU2NGEBMDNSV7TWJNEYWU3TKPJ4 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: Jesper Dangaard Brouer , BPF-dev-list , Alexander Lobakin , "Karlsson, Magnus" , Magnus Karlsson , David Ahern , =?utf-8?B?QmrDtnJuIFTDtnBlbA==?= , Saeed Mahameed , "kurt@linutronix.de" , "Raczynski, Piotr" , "Zhang, Jessica" , "Maloor, Kishen" , "Gomes, Vinicius" , "Brandeburg, Jesse" , "Swiatkowski, Michal" , "Plantykow, Marta A" , "Desouza, Ederson" , "Song, Yoong Siang" , "Czapnik, Lukasz" , "Joseph, Jithu" , William Tu , Ong Boon Leong , xdp-hints@xdp-project.net X-Mailman-Version: 3.3.4 Precedence: list List-Id: XDP hardware hints design discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: John Fastabend writes: > Toke H=C3=B8iland-J=C3=B8rgensen wrote: >> John Fastabend writes: >>=20 >> >> > > union and independent set of BTFs are two different things, I'll = let >> >> > > you guys figure out which one you need, but I replied how it coul= d >> >> > > look like in CO-RE world >> >> > >> >> > I think a union is sufficient and more aligned with how the >> >> > hardware would actually work. >> >>=20 >> >> Sure. And I think those are two orthogonal concerns. You can start >> >> with a single struct mynic_metadata with union inside it, and later >> >> add the ability to swap mynic_metadata with another >> >> mynic_metadata___v2 that will have a similar union but with a >> >> different layout. >> > >> > Right and then you just have normal upgrade/downgrade problems with >> > any struct. >> > >> > Seems like a workable path to me. But, need to circle back to the >> > what we want to do with it part that Jesper replied to. >>=20 >> So while this seems to be a viable path for getting libbpf to do all the >> relocations (and thanks for hashing that out, I did not have a good grip >> of the details), doing it all in userspace means that there is no way >> for the XDP program to react to changes once it has been loaded. So this >> leaves us with a selection of non-very-attractive options, IMO. I.e., >> we would have to: > > I don't really understand what this means 'having XDP program to > react to changes once it has been loaded.' What would a program look > like thats dynamic? You can always version your metadata and > write programs like this, > > if (meta->version =3D=3D VERSION1) {do_foo} > else {do_bar} > > And then have a headeer, > > struct meta { > int version; > union ... // union of versions > } > > I fail to see how a program could 'react' dynamically. An agent could > load new programs dynamically into tail call maps of fentry with > the need handlers, which would work as well and avoid unions. By "react" I meant "not break", as in the program should still be able to parse the metadata even if config changes. See below: >>=20 >> - have to block any modifications to the hardware config that would >> change the metadata format; this will probably result in irate users > > I'll need a concrete example if I swap out my parser block, I should > also swap out my BPF for my shiny new protocol. I don't see how a > user might write programs for things they've not configured hardware > for yet. Leaving aside knobs like VLAN on/off, VXLAN on/off, and > such which brings the next point. > >>=20 >> - require XDP programs to deal with all possible metadata permutations >> supported by that driver (by exporting them all via a BTF union or >> similar); this means a potential for combinatorial explosion of config >> options and as NICs become programmable themselves I'm not even sure >> if it's possible for the driver to know ahead of time > > I don't see the problem sorry. For current things that exist I can't > think up too many fields vlan, timestamp, checksum(?), pkt_type, > hash maybe. Even with five fields (assuming they can be individually toggled), that's 32 different metadata formats. Add two more and we're at 128. That's what I meant with "combinatorial explosion" (although I suppose it's only exponential, not combinatorial if we fix the order of the fields). I suppose it may be that you're right and that in practice the number of fields is small enough that it's manageable, but right off the bat it seems like a pretty limiting design to me. > For programmable pipelines (P4) then I don't see a problem with > reloading your program or swapping out a program. I don't see the > value of adding a new protocol for example dynamically. Surely the > hardware is going to get hit with a big reset anyways. Hmm, okay, I do buy that completely reprogramming the NIC is probably not something that is done as dynamically as toggling existing feature bits, so maybe that is not such a huge concern... >> - throw up our hands and just let the user deal with it (i.e., to >> nothing and so require XDP programs to be reloaded if the NIC config >> changes); this is not very friendly and is likely to lead to subtle >> bugs if an XDP program parses the metadata assuming it is in a >> different format than it is > > I'm not opposed to user error causing logic bugs. If I give > users power to reprogram their NICs they should be capabable > of managing a few BPF programs. And if not then its a space > where a distro/vendor should help them with tooling. > >>=20 >> Given that hardware config changes are not just done by ethtool, but >> also by things like running `tcpdump -j`, I really think we have to >> assume that they can be quite dynamic; which IMO means we have to solve >> this as part of the initial design. And I have a hard time seeing how >> this is possible without involving the kernel somehow. > > I guess here your talking about building an skb? Wouldn't it > use whatever logic it uses today to include the timestamp. > This is a bit of an aside from metadata in the BPF program. Building skbs is a separate concern, yeah, but that was not actually what I meant here. Say I install an XDP program that reads metadata like (after CO-RE rewriting): struct meta { u32 rxhash; u8 vlan; }; and that is merrily running and doing its thing, but then someone runs `tcpdump -j`, causing the NIC to turn on hardware timestamping, thus changing the effective metadata layout to: struct meta { u32 rxhash; u32 timestamp; u8 vlan; }; suddenly my XDP program will be reading garbage without knowing it, even though it's not interested in the timestamp at all. >> Unless I'm missing something? WDYT? > > Distilling above down. I think we disagree on how useful > dynamic programs are because of two reasons. First I don't > see a large list of common attributes that would make the > union approach as painful as you fear. See above; but I wouldn't actually mind being proven wrong here, I'm just worried that we end up setting something in stone ABI-wise so we can't change it later should there end up being a need for it. > And two, I believe users who are touching core hardware firmware need > to also be smart enough (or have smart tools) to swap out their BPF > programs in the correct order so as to not create subtle races. I > didn't do it here but if we agree walking through that program swap > flow with firmware update would be useful. Sure, I do think this would be useful; I only have a very fuzzy idea how this is likely to work. But I think we may also differ in the assumption of who controls the XDP programs: I very much view it as in scope for a system to be able to run different XDP programs from different applications without any other point of coordination than what the kernel and libbpf/libxdp APIs offer. So if application A needs to reprogram the hardware, how does application B's XDP program get re-loaded so it can get its CO-RE relocations re-applied with the new BTF format? -Toke