forked from kytos/of_core
-
Notifications
You must be signed in to change notification settings - Fork 5
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
Action Set Field class should also consider the OXM_FIELD to decide the subclass #139
Labels
future_release
Planned for the next release
Comments
Workaround we created:
|
I believe that we should include this solution into of_core directly (to avoid someone else having to depending on our Napp or ended up overriding our subclasses when using together) |
Usage examples after deploying the solution above (https://github.com/hackinsdn/containment):
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hi,
Currently, the ActionSetField class is only considering the action type on the deserialization process, as defined here:
of_core/v0x04/flow.py
Line 189 in 3545b1c
If one were to implement a new ActionSetField (for instance, ActionSetFieldIPv4Dst), we would have to also consider the OXM_FIELD to take the decision of the proper subclass to deserialize the OF Action data.
Use case: in the HackInSDN project we are defining new Action Set Fields to implement Custom Containment actions to mitigate cyberattacks. Some actions we are adding include: ActionSetFieldIPv4Dst, ActionSetFieldIPv6Dst, ActionSetFieldTCPDst, etc.
On our implementation we ended up overriding the OFActionSetField as showed below:
Then after adding a flow with set_ipv4_dst and also set_vlan, we got the following error:
This error happened because our class ended up being responsible for deserializing all OFActionSetField packets.
It seems that using a ActionSetField Factory to proper identify the subclass responsible for each OXM fields would be proper solution.
cc @talitarp since she is also working on this
The text was updated successfully, but these errors were encountered: