-
Notifications
You must be signed in to change notification settings - Fork 20
ON-13283: allow application to register callback for NIC reset #72
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
base: master
Are you sure you want to change the base?
Conversation
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.
Implementing it this way round seems dangerous. If we were to ship this feature as-is then customers might start writing code to reset zf stacks in the callback, which would then prevent us in the future from doing the stack reset internally
@@ -256,6 +263,9 @@ zf_reactor_process_event(struct zf_stack* st, int nic, ef_vi* vi, ef_event* ev) | |||
case EF_EVENT_TYPE_RX_REF_DISCARD: | |||
zf_reactor_handle_rx_ref_discard(st, nic, vi, ev); | |||
break; | |||
case EF_EVENT_TYPE_RESET: | |||
zf_reactor_handle_reset_event(st); |
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.
This is be called in an exciting context, i.e. with the stack mid-poll. If that's the way you want to do it then you'll have to write some really big disclaimers in the documentation about the things you are and are not permitted to do from within the callback.
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 documentation could state that the user is not permitted to perform any actions on the stack, which I think would be recommended anyway.
Setting a flag within TCPDirect that triggers the callback after processing the list of events would be a change that could be implemented but I don't think it would really add that much safety to the stack.
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.
We already have a way of reporting events to be handled outside the context of a zf call using the zf_muxer. Have you looked at implementing this with a new event type that can be registered in the wait set and notified? Probably needs a bit of consideration to allow waiting on a stack rather than a socket, but the interface would be safer.
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.
I took this route due to a previous implementation that added this functionality. I could look at using the muxer instead for this, I was keen on being able to report the failure quickly and without a separate call and hence went with the reactor_perform route.
Add ability for an application to register a callback if the NIC used by the stack is reset. User is able to specify an argument to be passed to it. This allows the application to take action in that scenario.
Updated
efsink
to use the callback and show the interface name being passed through to the function.