Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why bmi_port.c is a C code? #1249

Open
at-nojavan opened this issue May 18, 2024 · 2 comments
Open

Why bmi_port.c is a C code? #1249

at-nojavan opened this issue May 18, 2024 · 2 comments

Comments

@at-nojavan
Copy link

Hello everyone!

I'm trying to make a backward-compatible edit to the BMv2 simple switch.
Simple switch passes a packet handler from the place that static void packet_handler(int port_num, const char *buffer, int len, void *cookie) function is defined here, all the way to the place it's used in the bmi_port.c.

I see that in the dev_mgr_bmi.cpp here, a transform is performed on the handler from std::function to C style function pointer, because the next code in the chain is a C code (bmi_port.c).
I am trying to pass a std::function that has capture and this transform is causing some issues for me.
Now I have a question. Why bmi_port.c is a C code and not C++. What is the restriction that binds us to use C code? Is it possible to change this to C++?

@antoninbas
Copy link
Member

This code is written in C for legacy reasons. It was "copied over" years ago from the previous version of the behavioral model: https://github.com/p4lang/p4factory/tree/master/modules/BMI. There was no strong reason to rewrite this code at the time (although I think it may have been marginally improved over time), as it was working and this is a part of the code that is both essential and hard to test in an automatic way.

That being said, I think rewriting this code in C++ and taking a more modern approach (I would say the select-based approach is probably a bit obsolete now) is a great idea. While I will not work on this, I would be happy to review such a change.
I would say that if anyone wants to work on this, please do not start any implementation work until first presenting your proposed approach here and getting some sort of consensus. I would be fine with something that is based on Boost Asio.

Before approving such a change, and because we don't have automated tests for the I/O code, I would expect the following:

@fruffy
Copy link
Contributor

fruffy commented May 21, 2024

a successful test run of the p4c PTF test suite using the modified bmv2 + veths for packet I/O. Assuming there is such a thing... cc @fruffy

It depends... we do not have many static PTF tests available in the suite, but P4Testgen generates PTF tests for all the BMv2 programs with each CI run. That can be used for testing at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants