Enable set_field with Metadata register #16
Conversation
commandgjj
force-pushed
the
Enable_SetField_Metadata_Register
branch
from
75dcf64
to
1d72301
Compare
|
Maintainers, can you please help review this patch? Thanks! |
commandgjj
force-pushed
the
Enable_SetField_Metadata_Register
branch
from
1d72301
to
9951b72
Compare
@wenyingd Wenying, can you please help review this change? Thanks! |
commandgjj
force-pushed
the
Enable_SetField_Metadata_Register
branch
from
9951b72
to
cc4335d
Compare
commandgjj
force-pushed
the
Enable_SetField_Metadata_Register
branch
from
cc4335d
to
781cf31
Compare
ofctrl/fgraphFlow.go
Outdated
| @@ -843,12 +843,13 @@ func (self *Flow) installFlowActions(flowMod *openflow13.FlowMod, | |||
|
|
|||
| log.Debugf("flow install. Added setTunnelId Action: %+v", setTunnelAction) | |||
|
|
|||
| case "setMetadata": | |||
| // Set Metadata instruction | |||
| case ActTypeSetMetadata: | |||
There was a problem hiding this comment.
Actually, I am a bit confused. I know you want to add support for applyAction set metadata, so you add a new OFAction with it, but here you keep the original write_metadata instruction.
With your current code, if using SetMetadataAction.GetActionMessage, an ApplyAction message is generated, and can be add into ApplyActionInstruction, but if call Flow.installFlowActions it uses write_metadata instruction.
My point is, please keep the behavior compatible, maybe you should rename the original action type as write_meteadata, and add a new type set_metadata, and add call in both SetMetadataAction.GetActionMessage, and Flow.installFlowActions.
There was a problem hiding this comment.
Actually, I am a bit confused. I know you want to add support for applyAction set metadata, so you add a new OFAction with it, but here you keep the original write_metadata instruction.
With your current code, if using
SetMetadataAction.GetActionMessage, an ApplyAction message is generated, and can be add into ApplyActionInstruction, but if callFlow.installFlowActionsit uses write_metadata instruction.My point is, please keep the behavior compatible, maybe you should rename the original action type as write_meteadata, and add a new type set_metadata, and add call in both
SetMetadataAction.GetActionMessage, andFlow.installFlowActions.
OK. I know your point. I renamed the origianl action as "writeMetadata". Please help review it again, thanks @wenyingd .
commandgjj
force-pushed
the
Enable_SetField_Metadata_Register
branch
2 times, most recently
from
4371814
to
63c14da
Compare
WriteMetadata is write-action instruction which will be applied later than apply-action. In some case, we need use resubmit to do pipeline in the same table and set Metadata before resubmit at the same time. WriteMetadata cannot do the job. We need enable set_field for Metadata register. Actually, WriteMetadata do the same thing with set_field Metadata register in the OvS. From OpenFlow 1.5, Metadata register also can be used with set_field to set, not stricted only using WriteMetadata instruction. After this patch, ofnet can support WriteMetadata and set_field for Metadata meanwhile. Caller can use SetMetadata() or WriteMetadata() API to implement WriteMetadata action, and use SetMetadataAction() and ApplyAction() to implement set_field for Metadata. Signed-off-by: Jinjun Gao <[email protected]>
commandgjj
force-pushed
the
Enable_SetField_Metadata_Register
branch
from
63c14da
to
350e03f
Compare
Yes, we don't use FlowAction currently. |
|
@ashish-varma Would you give a review on this change? Will it introduce conflict if we switch to OF1.5? |
@ashish-varma Can you please take a look this change? Thanks! |
WriteMetadata is write-action instruction which will be applied later
than apply-action. In some case, we need use resubmit to do pipeline in
the same table and set Metadata before resubmit at the same time.
WriteMetadata cannot do the job. We need enable set_field for Metadata
register.
Actually, WriteMetadata do the same thing with set_field Metadata
register in the OvS. From OpenFlow 1.5, Metadata register also can be
used with set_field to set, not stricted only using WriteMetadata instruction.
Signed-off-by: Jinjun Gao [email protected]