hi Alin, I am trying to reason with what this patch is trying to accomplish. I might be missing something here, but here’s what I’m thinking:
OvsOutputBeforeSetAction() is typically called for an action such as: outport1, set_tunnel(), outport2 Here ‘outport2’ is a tunnel port, and output1 is a non-tunnel port. So, when we parse the action ‘outport2’ by making modifications to the packet, we need to make sure that we output to packet without modifications to outport1. Hence we call into OvsOutputBeforeSetAction() as the name indicates. OvsOutputBeforeSetAction() on its part makes a copy of the packet, sends out the original, and updates ‘ovsFwdCtx’ to point to the new (copied) packet. So, everything you are trying to do - saving srcVportNo, SourcePortId and SourceNicIndex is already taken care of. If not, then OvsPartialCopyNBL() has a bug that it should be fixed there. I had reviewed the v1 of the patch where you had made changes in OvsOutputForwardingCtx() and had given out comments. But, looking at the caller of OvsOutputForwardingCtx(), I don’t think any changes are necessary. Can you pls. explain more? Like I said, I may be missing something. Also, when will you end up with a flow action such as: pop_vlan(), set_tunnel(context), 15, 16, 17? If 15, 16 and 17 are indeed different VXLAN ports, we should be setting up the tunnel context for each port before outputting to it. So your action would look more like: pop_vlan(), set_tunnel(context-15), 15, set_tunnel(context-16), 16, set_tunnel(context-17), 17 Can you pls. post the vsctl show, dpctl show, dpctl dump-flows output. Maybe I’ll understand more. thanks, -- Nithin > On Jul 15, 2015, at 7:45 PM, Alin Serdean <aserd...@cloudbasesolutions.com> > wrote: > > If we have a flow rule in the following form: > actions=strip_vlan,set_tunnel:0x3e9,15,16,17 (Where port 15, 16 and 17 are > VXLAN tunnels with different tunnelling information) > > A packet which will hit that specific flow will only be sent out encapsulated > with the first tunnelling information. > > This patch saves the initial packet vport, source port and NIC index for > further use of the currently implemented pipeline and ignores the latter if it > is the last tunnelling port. > > Signed-off-by: Alin Gabriel Serdean <aserd...@cloudbasesolutions.com> > --- > This patch should also be applied on 2.4 > v4 Relax condition when saving ports > v3 Fix formatting > v2 Address comments > --- > datapath-windows/ovsext/Actions.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/datapath-windows/ovsext/Actions.c > b/datapath-windows/ovsext/Actions.c > index c8de7c5..c824e71 100644 > --- a/datapath-windows/ovsext/Actions.c > +++ b/datapath-windows/ovsext/Actions.c > @@ -975,6 +975,9 @@ OvsOutputBeforeSetAction(OvsForwardingContext *ovsFwdCtx) > ovsFwdCtx->tunnelTxNic != NULL || ovsFwdCtx->tunnelRxNic != NULL); > > /* Send the original packet out */ > + UINT32 tempVportNo = ovsFwdCtx->srcVportNo; > + UINT32 tempSourcePortId = ovsFwdCtx->fwdDetail->SourcePortId; > + UINT32 tempNicIndex = ovsFwdCtx->fwdDetail->SourceNicIndex; > status = OvsOutputForwardingCtx(ovsFwdCtx); > ASSERT(ovsFwdCtx->curNbl == NULL); > ASSERT(ovsFwdCtx->destPortsSizeOut == 0); > @@ -996,6 +999,9 @@ OvsOutputBeforeSetAction(OvsForwardingContext *ovsFwdCtx) > > NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(newNbl), > ovsFwdCtx->completionList, > &ovsFwdCtx->layers, FALSE); > + ovsFwdCtx->srcVportNo = tempVportNo; > + ovsFwdCtx->fwdDetail->SourcePortId = tempSourcePortId; > + ovsFwdCtx->fwdDetail->SourceNicIndex = tempNicIndex; > } _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
