-
Notifications
You must be signed in to change notification settings - Fork 152
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
issue: 2247939 Add option VMA_VMAD_ENABLED #907
base: master
Are you sure you want to change the base?
Conversation
Test PASSed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please consider checking for g_p_agent == NULL
instead of safe_mce_sys().vmad_enabled
.
It looks as more universal way that independent from approach if VMA has special env variable or not.
In this case it is enough to handle enable/disable just in one place in main().
Probably additional check is redundant for ring_tap because warning should be appeared once on initialization when g_p_agent is initialized on not (see possible warning at output_fatal()
)
This option allows to disable vmad functionality in VMA. Running vmad process can lead to performance degradation in case of multiple TCP socket state changes. Signed-off-by: Dmytro Podgornyi <dmytrop@mellanox.com>
Test PASSed. |
@@ -371,6 +371,9 @@ int ring_tap::prepare_flow_message(vma_msg_flow& data, msg_flow_t flow_action, | |||
{ | |||
int rc = 0; | |||
|
|||
if (unlikely(g_p_agent == NULL)) | |||
return 0; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is needless and incorrect because it conflicts with
/* create egress rule (redirect traffic from tap device to physical interface) */
rc = prepare_flow_message(data, VMA_MSG_FLOW_ADD);
if (rc != 0) {
ring_logwarn("Add TC rule failed with error=%d", rc);
}
ring_tap() does not work correctly w/o vma daemon.
So preferable way is to report error once ring_tap() is called. It can be before call or inside constructor.
@@ -400,6 +403,9 @@ int ring_tap::prepare_flow_message(vma_msg_flow& data, msg_flow_t flow_action) | |||
{ | |||
int rc = 0; | |||
|
|||
if (unlikely(g_p_agent == NULL)) | |||
return 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the same as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see my comments
Can one of the admins verify this patch? |
This option allows to disable vmad functionality in VMA. Running vmad process
can lead to performance degradation in case of multiple TCP socket state
changes.