transform/rpc: prevent cross shard memory writes

[rockwotj]

transform/rpc: prevent cross shard memory writes

Currently there is code in the transform/rpc layer that does something
along the following:

iobuf somedata;
co_await seastar::submit_to(shard_id, [&somedata] {
        somedata = fetch_some_data();
});

This executes a function on another CPU and writes memory to a stack
location on the requesting CPU. It's not clear if this is safe to do.

Instead change to use standard seastar mechanisms to pass memory between
cores:

iobuf somedata = co_await seastar::submit_to(shard_id, [] {
        return fetch_some_data();
});

The actual pieces here are more complicated than the above example, but
this is what motivates the change itself.

Backports Required

• none - not a bug fix
• none - this is a backport
• none - issue does not exist in previous branches
• none - papercut/not impactful enough to backport
• v23.3.x
• v23.2.x

Release Notes

• none

[vbotbuildovich]

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/47574#018ec3f3-cf45-4b9e-a3ec-97cb4251fc6e

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/47574#018ec3fc-412d-4eed-9dc1-9e30f043df3d

[rockwotj]

CI Failures: gtest_raft_rpunit

[dotnwat]

Thanks a lot for expanding the description.

Code looks fine, but a few things to consider:

Both of the cases you cited look safe to me. The one where a stack variable is written from another core should be fine, at least in so far as the co_await establishes a clear happens-after barrier between the write and subsequent reads. i don't know if you'd need any kind of additional memory synchronization, but i'm pretty sure there are other places in the tree where we do this.

The other aspect to consider here is memory ownership. In the second case where you build the iobuf on one core, and return it to a different core all of the memory of the iobuf is going to be owned by the core where the iobuf was built. this isn't so much a problem for small things, but it can be problematic for large data as you might have a "hot" core building heavy weight iobufs and other cores keeping that data alive, so the memory utilization becomes asymetric. this is the case for fetch/produce so we usually copy the data to where its going to / from.

if you go with the current version, then you can also avoid the CAS loop in the memory subsystem in seastar associated with cross-core freeing of memory by using foreign pointer.

so it's probably reasonable to switch to the second form as it's just easier to think about / cleaner than passing the stack variable reference, but the memory ownership / x-core freeing are still valid questions.

[rockwotj]

The other aspect to consider here is memory ownership. In the second case where you build the iobuf on one core, and return it to a different core all of the memory of the iobuf is going to be owned by the core where the iobuf was built. this isn't so much a problem for small things, but it can be problematic for large data as you might have a "hot" core building heavy weight iobufs and other cores keeping that data alive, so the memory utilization becomes asymetric. this is the case for fetch/produce so we usually copy the data to where its going to / from.

Ack - the iobuf here is for Wasm binaries, and they are only fetched once. So a low frequency operation of rather large binaries. I will send out another PR to use a foreign pointer, good callout.

[vbotbuildovich]

/backport v23.3.x

[vbotbuildovich]

Failed to create a backport PR to v23.3.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-17702-v23.3.x-18 remotes/upstream/v23.3.x
git cherry-pick -x aac5e671f8daa36f887ca130f4c76489992120d1

Workflow run logs.