Skip to content

Post init promisc config#30

Merged
kivkiv12345 merged 2 commits intomasterfrom
post_init_promisc_config
Aug 13, 2025
Merged

Post init promisc config#30
kivkiv12345 merged 2 commits intomasterfrom
post_init_promisc_config

Conversation

@kivkiv12345
Copy link

@kivkiv12345 kivkiv12345 commented Aug 11, 2025

Added csp_zmqhub_<add/remove>_filters() to change promisc settings after init.

Tested with Valgrind without warnings or errors. I didn't add a mutex, as it didn't appear to be necessary.

This is currently used by the following PyCSH commit: spaceinventor/PyCSH@0e398e4#diff-a1457fd5997277b0ab808d1e18fdd8b8d874b88ead8ebd1b33ee4e6e4ff8fdb9
Which is implemented in pycsh_core here: spaceinventor/pycsh_core@c07939a#diff-b8f67f3336f931c16d8f7da067ee582289b8a735040cb5a4fbc2735deaf72e96R228

I got tired of having to close CSH because someone added promisc to my interfaces :)

drv->iface.nexthop = csp_zmqhub_tx;

drv->iface.addr = addr;
drv->iface.netmask = netmask;
Copy link

@jeanbaptistelab jeanbaptistelab Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That netmask assignment is new, isn't it ? Actually, so is the addr assignment.
I don't think it's an issue, I just noticed this wasn't there before. I've looked at the other csp_zmqhub_init_* functions and they do set the addr but not the netmask.
Whether it's something we should streamline or not, I don't know...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For completeness I'll answer here as well.
Previously we made this assignment in CSH after the csp_zmqhub_init_filter2 call, which seems pretty dumb since every possible caller will have to make the same assignment (from arguments which are given to csp_zmqhub_init_filter2 no less).
For this PR to work, I need the fields to be assigned before calling the promisc functions.
So it was a good excuse to improve some wonky call semantics.

@kivkiv12345 kivkiv12345 merged commit a8e72cd into master Aug 13, 2025
68 of 102 checks passed
@kivkiv12345 kivkiv12345 deleted the post_init_promisc_config branch August 13, 2025 12:36
@kivkiv12345
Copy link
Author

Actually @jeanbaptistelab, I know this PR has already been merged, but I've been wondering if it would make sense to make a more generic way to call the promisc/unpromisc functions for CAN as well (once they are implemented).
For now it will be up to the user to determine the type of the interface (likely by checking which .nexthop is used), which is not very nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants