On Tue, Dec 12, 2023 at 05:32:33PM -0800, Alexei Starovoitov wrote: > On Tue, Dec 12, 2023 at 3:36 PM David Vernet <void@xxxxxxxxxxxxx> wrote: > > > > > It only supports atomic_add and no other atomics. > > > > Ahh, I misunderstood when I discussed with Kuba. I guess they supported > > only atomic_add because packets can be delivered out of order. > > Not sure why it has anything to do with packets. My understanding is that the ordering of packets is an impedance with the host's ordering model. If you offload a BPF program from the host which expects to see packets in order, and then issues some atomics which process the packets in order, it won't work on the device because the packets are delivered out of order. Kuba (cc'd) can give more details if he wants, but it doesn't really matter. The salient point is that the chip could have done all of the BPF atomic instructions and it wouldn't have been much more work to implement them. > > So fair > > enough on that point, but I still stand by the claim though that if you > > need one type of atomic, it's reasonable to infer that you may need all > > of them. I would be curious to hear how much work it would have been to > > add support for the others. If there was an atomic conformance group, > > maybe they would have. > > The netronome wasn't trying to offload this or that insn to be > in compliance. Together, netronome and bpf folks decided to focus > on a set of real XDP applications and try to offload as much as practical. > At that time there were -mcpu=v1 and v2 insn sets only and offloading > wasn't really working well. alu32 in llvm, verifier and nfp was added > to make offload practical. Eventually it became -mcpu=v3. > So compliance with any future group (basic, atomic, etc) in ISA cannot > be evaluated in isolation, because nfp is not compliant with -mcpu=v4 > and not compliant with -mcpu=v1, > but works well with -mcpu=v3 while v3 is an extension of v1 and v2. > Which is nonsensical from standard compliance pov. > netronome offload is a success because it demonstrated > how real production XDP applications can run in a NIC at speeds > that traditional CPUs cannot dream of. > It's a success despite the complexity and ugliness of BPF ISA. > It's working because practical applications compiled with -mcpu=v3 produce > "compliant enough" bpf code. Something I want to make sure is clearly spelled out: are you of the opinion that a program written for offload to a Netronome device cannot and should not ever be able to run on any other NIC with BPF offload? > > Well, maybe not for Netronome, or maybe not even for any vendor (though > > we have no way of knowing that yet), but what about for other contexts > > like Windows / Linux cross-platform compat? > > bpf on windows started similar to netronome. The goal was to > demonstrate real cilium progs running on windows. And it was done. > Since windows is a software there was no need to add or remove anything > from ISA, but due to licensing the prevail verifier had to be used which > doesn't support a whole bunch of things. > This software deficiencies of non-linux verifier shouldn't be > dictating grouping of the insns in the standard. > > If linux can do it, windows should be able to do it just as well. > So I see no problem saying that bpf on windows will be non-compliant > until they support all of -mcpu=v4 insns. It's a software project > with a deterministic timeline. > > The standard should focus on compatibility between > HW-ish offloads where no amount of software can add support for > all of -mcpu=v4. I don't agree that there's no value in standardizing for the sake of software as well, but yes it's different than what we're trying to accomplish for hardware, and I agree that hardware is the main customer here. Even if you assume that we should completely ignore software and focus on hardware compatibility though, that seems to be orthogonal to what you're proposing here. What compatibility are we guaranteeing if there's no compliance? > And here I believe compliance with "basic" is not practical. > When nvme HW architects will get to implement "basic" ISA they might > realize that it has too much. > Producing "conformance groups" without HW folks thinking through the > implementation is not going to be a success. > I worry that it will have the opposite effect. > We'll have a standard with basic, atomic, etc. > Then folks will deliver this standard on the desk of HW architects. > They will give it a try and will reject the idea of implementing BPF in HW, > because not implementing "basic" would mean that this vendor > is not in compliance which means no business. I don't know enough about how compliance informs the cost-calculus and decision making of HW vendors to really make an intelligent point here, but I have to imagine that there's an equally plausible scenario where a vendor will look at the non-legacy instructions and think, "There's no possible way we could support all of these instructions", and make the same decision? Why else would they be asking for a standard if not to have some guidelines of what to implement? > Hence the standard shouldn't overfocus on compliance and groups. > Just legacy and the rest will do for nvme. > legacy means "don't bother looking at those". > the rest means "pls implement these insns because they are useful, > their semantics and encoding is standardized, > but pick what makes sense for your use case and your HW". How do we know the semantics of the instructions won't be prohibitively expensive or impractical for certain vendors? What value do we get out of dictating semantics in the standard if we're not expecting any of these programs to be cross-compatible anyways? > And to make such HW offload a success we'd need to work together. > compiler, kernel, run-time, hw folks. Which kernel? Which compiler? If we all need to be in the room every time a decision is made by any vendor, then what value is the standard even providing? > "Here is a standard. Go implement it" won't work. What is the point of a standard if not to say, "Here's what you should go implement"?
Attachment:
signature.asc
Description: PGP signature
