[ovs-dev] [PATCH v4] datapath-windows: Output a packet to two or more VXLAN ports

Nithin Raju nithin at vmware.com
Thu Aug 27 21:24:27 UTC 2015


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 <aserdean at 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:nithin at vmware.com] 
> Trimis: Thursday, August 27, 2015 10:44 PM
> Către: Alin Serdean <aserdean at cloudbasesolutions.com>
> Cc: dev at 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 <aserdean at 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 <aserdean at 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;
>>    }
> 
> 



More information about the dev mailing list