Alin :) (not Sorin). Yes both ports are tunneling ports.
We cannot save the tunkey because it is different the two ports have different destination addresses. OvsOutputBeforeSetAction indeed reinitializes ovsFwdCtx->fwdDetail->SourcePortId and ovsFwdCtx->fwdDetail->SourceNicIndex (https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/Actions.c#L996), but NOT 'ovsFwdCtx->srcVportNo’ (it changes in https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/Actions.c#L978, and when reinitialize happens it is with the old value https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/Actions.c#L995 ). Alin. -----Mesaj original----- De la: Nithin Raju [mailto:nit...@vmware.com] Trimis: Friday, August 28, 2015 12:24 AM Către: Alin Serdean <aserd...@cloudbasesolutions.com> Cc: dev@openvswitch.org Subiect: Re: [ovs-dev] [PATCH v4] datapath-windows: Output a packet to two or more VXLAN ports hi Sorin, OK, if I understand you correctly, what you are saying is that, ‘output2' is also tunnel port, and when we output the packet to ‘output1’, all of the tunnel context is lost, and we’ll not be able to output the packet to ‘output2’. Is this right? If so, the fix you are putting in is not sufficient. We’ll have to save the ovsFwdCtx->tunKey, if it has been cleared while outputting the packet ‘output1’. Like I mentioned in the previous email, OvsOutputBeforeSetAction() reinitializes ovsFwdCtx for ‘newNbl', and you need not worry about 'ovsFwdCtx->srcVportNo’, ovsFwdCtx->fwdDetail->SourcePortId and ovsFwdCtx->fwdDetail->SourceNicIndex. thanks, -- Nithin > On Aug 27, 2015, at 2:05 PM, Alin Serdean <aserd...@cloudbasesolutions.com> > wrote: > > Hi Nithin, > > Regarding the question about the flows in the form: > strip_vlan,set_tunnel,output15,output16 the main user behind it is neutron. A > nice explanation why this is done can be found: > https://urldefense.proofpoint.com/v2/url?u=http-3A__assafmuller.com_2013_10_14_gre-2Dtunnels-2Din-2Dopenstack-2Dneutron_&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=1SXT6N05UcaKGQDzLb9HNbE_x7fwEuzdNaBPRR5z1Mg&s=HAbkxMu7OfTa2EsuSDDwdd9wlX0Lz3PIjZkU8mplicY&e= > (look at Table 21: cookie=0x0, duration=182103.777s, table=21, > n_packets=10235, n_bytes=3109310, idle_age=5, hard_age=65534, > priority=1,dl_vlan=1 actions=strip_vlan,set_tunnel:0x1388,output:3,output:2). > > > The problem is the following when trying to do an action of the form set, > output1, output2: > OvsOutputBeforeSetAction-> OvsOutputForwardingCtx -> OvsTunnelPortTx (here we > set srcVportNo, fwdDetail->SourcePortId, fwdDetail->SourceNicIndex) -> > OvsInitForwardingCtx with the former ovsFwdCtx->srcVportNo (indeed the saving > the fwdDetail will be redundant but will double check with a small test) this > is for output1. > The same calls will be done for output2 but in this case we lost the original > srcVportNo. > > I hope it makes sense. > > Alin. > > -----Mesaj original----- > De la: Nithin Raju [mailto:nit...@vmware.com] > Trimis: Thursday, August 27, 2015 10:44 PM > Către: Alin Serdean <aserd...@cloudbasesolutions.com> > Cc: dev@openvswitch.org > Subiect: Re: [ovs-dev] [PATCH v4] datapath-windows: Output a packet to > two or more VXLAN ports > > 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
