Race conditions when processing packet in the egress pipeline. about behavioral-model HOT 6 CLOSED

Interesting find.

I am not familiar enough with this code to be certain, but I would not be surprised if this set_source_info() call here: https://github.com/p4lang/behavioral-model/blob/main/src/bm_sim/actions.cpp#L371

would be best done once at P4 program load time, rather than once every time an action is invoked while processing a packet. That would be somewhat more efficient, as well as hopefully avoiding this data race.

from behavioral-model.

would be best done once at P4 program load time, rather than once every time an action is invoked while processing a packet. That would be somewhat more efficient, as well as hopefully avoiding this data race.

This was my first thought, but it is not an option here. There is a single ActionPrimitive object instantiated for each primitive action, and it is shared by all pipelines whenever the primitive is executed, no matter which action it is executed from.

The source info we store here is related to the primitive action "call", and so it depends on the location of the actual call site. On primitive can be used many times in a given program.

Fortunately, there is a simple fix here. The field needs to be switched to a static thread_local one. It is no different from how the pkt and phv fields are handled.

I can take care of it a bit later, unless someone wants to beat me to it.

from behavioral-model.

Sorry, I got a bit confused regarding which field you are referring to that needs to be switched to a static thread_local

from behavioral-model.

I included a code snippet in my post. It's the only field shown there that is not static thread_local.

from behavioral-model.

I tried to change the field to a static thread_local SourceInfo *call_source_info. I had issues with building and running my tests. Not entirely clear what's wrong.

error: undefined symbol: bm::ActionPrimitive_::call_source_info
>>> referenced by primitives.cpp
>>>               blaze-out/k8-fastbuild/bin/third_party/p4lang_behavioral_model/_objs/bm_sim/primitives.pic.o:(bm::core::assert_::operator()(bm::Data const&))
>>> referenced by primitives.cpp
>>>               blaze-out/k8-fastbuild/bin/third_party/p4lang_behavioral_model/_objs/bm_sim/primitives.pic.o:(bm::core::assume_::operator()(bm::Data const&))
>>> referenced by primitives.cpp
>>>               blaze-out/k8-fastbuild/bin/third_party/p4lang_behavioral_model/_objs/bm_sim/primitives.pic.o:(bm::core::log_msg::operator()(std::__u::basic_string<char, std::__u::char_traits<char>, std::__u::allocator<char>> const&, std::__u::vector<bm::Data, std::__u::allocator<bm::Data>>))
>>> referenced 2 more times
clang: error: linker command failed with exit code 1 (use -v to see invocation)

I see that there are references to call_source_info inside other files like primitives.cpp from a quick search, not particularly sure if these get affected or not.

from behavioral-model.

Did you define the static field in actions.cpp as well?

from behavioral-model.