Selector Packet Fanout - Core implementation#1329
Selector Packet Fanout - Core implementation#1329Hoooao wants to merge 4 commits intop4lang:mainfrom
Conversation
Hoooao
force-pushed
the
main
branch
2 times, most recently
from
040e724 to
9ee035b
Compare
Signed-off-by: Hoooao <hao021014@163.com>
Signed-off-by: Hoooao <hao021014@163.com>
|
@jonathan-dilorenzo FYI it would be good for someone in Google who wants this feature to learn enough about the code base to give it a good review. Unless you employ someone who knows this feature and how it works, or can contract with someone who does, any bugs discovered in it later are unlikely to be fixed by someone who is not interested in the feature. |
matthewtlam
left a comment
matthewtlam
left a comment
There was a problem hiding this comment.
Leaving behind first set of comments. Will add more later today
| ]) | ||
| ]) | ||
|
|
||
| # TODO: add this to cmake build as well |
There was a problem hiding this comment.
Do we still need this comment? I think you already added it to cmake build
| fanout_ctx_map.emplace(thread_id, FanoutCtx(buffer_push_fn)); | ||
| } | ||
|
|
||
| // TODO(Hao): deduplicate packets fanout, optional |
There was a problem hiding this comment.
Do we still need this comment?
| std::unordered_map<grp_hdl_t, std::vector<mbr_hdl_t>> groups; | ||
| }; | ||
|
|
||
| class FanoutPktMgr { |
There was a problem hiding this comment.
Let's add some documentation regarding the FanoutPktMgr. For example how it is used and use cases
| bm::Logger::get()->error("[{}] [cxt {}] " s, (pkt).get_unique_id(), \ | ||
| (pkt).get_context(), ##__VA_ARGS__) | ||
|
|
||
| #define BMLOG_WARN(...) bm::Logger::get()->warn(__VA_ARGS__) |
There was a problem hiding this comment.
Don't see this MACRO being used anywhere so we can remove? Correct me if I am incorrect
| packet->get_checksum_error() ? 1 : 0); | ||
| } | ||
| // Check if the packet has an optional continue node for pkt fanout | ||
| // TODO(Hao): update the doc/simple_switch.md |
There was a problem hiding this comment.
Do we still need this TODO?
|
@Hoooao What's the status of this PR? |
4 participants
This PR is derived from #1316 .
The testcases will be in another PR.
Main updates after #1316 :
--enable-pkt-fanouttoconfigure, which definesPKT_FANOUT_ON. Is not supported in cmake for now.selector_fanoutmode is used while the above option is not given.grp_selectorfor all specified selectors during switchstart_and_return_, less intrusive.ingress/egree_threadafter the original packet gets processed. Now the logic is colocated with packet replication, which is much cleaner.