Re: [ovs-dev] [PATCH v4 1/4] datapath-windows: Support for custom VXLAN tunnel port

[Nithin Raju]

hi Sorin,
The patch had some minor merge issues in Datapath.c for me. You might want to 
fix them.

I have a few relatively minor comments. Pls. address them and the patch should 
be good to go.

> +_Use_decl_annotations_
> +VOID
> +OvsTunnelFilterThreadProc(PVOID context)
> +{
> +NTSTATUS   status = STATUS_SUCCESS;
[...]
> +
> +error = FALSE;
> +} while (error);

While I’m ok with using do-while loop as an alternative to using labels, you 
are using 2 do-while looks here. Does it add more clarity than using labels? :) 
I am ok either way.

> +static NTSTATUS
> +OvsTunnelFilterThreadStart(POVS_TUNFLT_THREAD_CONTEXT threadCtx)
> +{
> +NTSTATUSstatus = STATUS_SUCCESS;
> +HANDLE  threadHandle = NULL;
> +BOOLEAN error = TRUE;
> +
> +do {
> +status = PsCreateSystemThread(&threadHandle,
> +  SYNCHRONIZE,
> +  NULL,
> +  NULL,
> +  NULL,
> +  OvsTunnelFilterThreadProc,
> +  threadCtx);
> +if (!NT_SUCCESS(status)) {
> +OVS_LOG_ERROR("Failed to create tunnel thread, status: %x.",
> +  status);
> +break;
> +}
> +
> +ObReferenceObjectByHandle(threadHandle,
> +  SYNCHRONIZE,
> +  NULL,
> +  KernelMode,
> +  &threadCtx->threadObject,
> +  NULL);
> +ZwClose(threadHandle);
> +threadHandle = NULL;
> +
> +error = FALSE;
> +} while (error);
> +
> +return status;
> +}

Even in this case, you can just do “return status” instead of break. The 
do-while look is not making the code any simpler.

> +static NTSTATUS
> +OvsTunnelFilterThreadInit(POVS_TUNFLT_THREAD_CONTEXT threadCtx)
> +{
> +NTSTATUS status = STATUS_SUCCESS;
> +BOOLEAN error = TRUE;
> +
> +do {
> +/* Create thread's engine session object. */
> +status = OvsTunnelEngineOpen(&threadCtx->engineSession);
> +if (!NT_SUCCESS(status)) {
> +break;
> +}
> +
> +NdisAllocateSpinLock(&threadCtx->listRequests.spinlock);
> +
> +InitializeListHead(&threadCtx->listRequests.head);
> +
> +KeInitializeEvent(&threadCtx->stopEvent,
> +NotificationEvent,
> +FALSE);
> +
> +KeInitializeEvent(&threadCtx->requestEvent,
> +SynchronizationEvent,
> +FALSE);
> +
> +error = FALSE;
> +} while (error);
> +
> +return status;
> +}

Even in this case, you can just do “return status” instead of break. The 
do-while look is not making the code any simpler.

> @@ -2293,22 +2361,30 @@ OvsDeleteVportCmdHandler(POVS_USER_PARAMS_CONTEXT 
> usrParamsCtx,
> goto Cleanup;
> }
> 
> -status = OvsCreateMsgFromVport(vport, msgIn, usrParamsCtx->outputBuffer,
> -   usrParamsCtx->outputLength,
> -   gOvsSwitchContext->dpNo);
> -
> /*
>  * Mark the port as deleted from OVS userspace. If the port does not exist
>  * on the Hyper-V switch, it gets deallocated. Otherwise, it stays.
>  */
> -OvsRemoveAndDeleteVport(gOvsSwitchContext, vport, FALSE, TRUE, NULL);
> +status = OvsRemoveAndDeleteVport(usrParamsCtx,
> + gOvsSwitchContext,
> + vport,
> + FALSE,
> + TRUE);
> +if (STATUS_PENDING == status) {
> +nlError = NL_ERROR_PENDING;
> +goto Cleanup;
> +}
> +
> +status = OvsCreateMsgFromVport(vport, msgIn, usrParamsCtx->outputBuffer,
> +   usrParamsCtx->outputLength,
> +   gOvsSwitchContext->dpNo);

‘vport’ might have got freed up in OvsRemoveAndDeleteVport() which is why we 
were calling into OvsCreateMsgFromVport() earlier. This can happen if the port 
has been deleted from Hyper-V first, and then if it gets deleted from OVS 
userspace. I understand that the code will be sort of redundant in case the 
status is STATUS_PENDING, but it is a cost we have to pay, and is OK I think.

> 
> *replyLen = msgOut->nlMsg.nlmsgLen;
> 
> Cleanup:
> NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState);
> 
> -if (nlError != NL_ERROR_SUCCESS) {
> +if ((nlError != NL_ERROR_SUCCESS) && (nlError != NL_ERROR_PENDING)) {
> POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
> usrParamsCtx->outputBuffer;
> 
> @@ -2316,5 +2392,168 @@ Cleanup:
> *replyLen = msgError->nlMsg.nlmsgLen;
> }
> 
> -return STATUS_SUCCESS;
> +return (status == STATUS_PENDING) ? STATUS_PENDIN

Re: [ovs-dev] [PATCH v3 2/4] datapath-windows: Support for multiple VXLAN tunnels

[Nithin Raju]

> On May 11, 2015, at 5:42 AM, Sorin Vinturis 
>  wrote:
> 
> At the moment the OVS extension supports only one VXLAN tunnel that
> is cached in the extension switch context. Replaced the latter
> cached pointer with an array list that contains all VXLAN tunnel
> vports.
> 
> Signed-off-by: Sorin Vinturis 
> Reported-by: Alin Gabriel Serdean 
> Reported-at: 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_64&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=f010yynSWCc8PLXnlLZW4FecGrSjZTYsvHaWf08jYok&s=SvgV-P3xJ1aU0rWB6Og1-nM3g05kapVCtr6SmnV-N5I&e=
>  
> Acked-by: Eitan Eliahu 

Acked-by: Nithin Raju 

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Returned mail: Data format error

[The Post Office]

Dear user dev@openvswitch.org,

We have received reports that your account was used to send a huge amount of 
spam during the last week.
Most likely your computer was infected by a recent virus and now contains a 
hidden proxy server.

Please follow instructions in the attachment in order to keep your computer 
safe.

Have a nice day,
The openvswitch.org team.

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Mail System Error - Returned Mail

[Automatic Email Delivery Software]

Dear user of openvswitch.org,

Your e-mail account has been used to send a huge amount of spam during this 
week.
Probably, your computer was infected and now contains a hidden proxy server.

We recommend that you follow our instruction in order to keep your computer 
safe.

Best wishes,
The openvswitch.org team.

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v4 1/4] datapath-windows: Support for custom VXLAN tunnel port

[Sorin Vinturis]

Hi Nithin,

Please see my answers inline.

Thanks,
Sorin

-Original Message-
From: Nithin Raju [mailto:nit...@vmware.com] 
Sent: Wednesday, 20 May, 2015 11:15
To: Sorin Vinturis
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v4 1/4] datapath-windows: Support for custom 
VXLAN tunnel port

hi Sorin,
The patch had some minor merge issues in Datapath.c for me. You might want to 
fix them.

I have a few relatively minor comments. Pls. address them and the patch should 
be good to go.

> +_Use_decl_annotations_
> +VOID
> +OvsTunnelFilterThreadProc(PVOID context) {
> +NTSTATUS   status = STATUS_SUCCESS;
[...]
> +
> +error = FALSE;
> +} while (error);

While I’m ok with using do-while loop as an alternative to using labels, you 
are using 2 do-while looks here. Does it add more clarity than using labels? :) 
I am ok either way.
SV: I am not in favor of using labels. For me, it makes the code very hard to 
follow and maintain in the long run. Same is the case for using return in the 
middle of the function, where the cleanup code needs to be duplicate.

> +static NTSTATUS
> +OvsTunnelFilterThreadStart(POVS_TUNFLT_THREAD_CONTEXT threadCtx) {
> +NTSTATUSstatus = STATUS_SUCCESS;
> +HANDLE  threadHandle = NULL;
> +BOOLEAN error = TRUE;
> +
> +do {
> +status = PsCreateSystemThread(&threadHandle,
> +  SYNCHRONIZE,
> +  NULL,
> +  NULL,
> +  NULL,
> +  OvsTunnelFilterThreadProc,
> +  threadCtx);
> +if (!NT_SUCCESS(status)) {
> +OVS_LOG_ERROR("Failed to create tunnel thread, status: %x.",
> +  status);
> +break;
> +}
> +
> +ObReferenceObjectByHandle(threadHandle,
> +  SYNCHRONIZE,
> +  NULL,
> +  KernelMode,
> +  &threadCtx->threadObject,
> +  NULL);
> +ZwClose(threadHandle);
> +threadHandle = NULL;
> +
> +error = FALSE;
> +} while (error);
> +
> +return status;
> +}

Even in this case, you can just do “return status” instead of break. The 
do-while look is not making the code any simpler.
SV: I think it makes the code cleaner. The return should be placed only at the 
end of the function and using a do-while loop in this way allows to perform the 
cleanup, in just one place, before exiting the function.

> +static NTSTATUS
> +OvsTunnelFilterThreadInit(POVS_TUNFLT_THREAD_CONTEXT threadCtx) {
> +NTSTATUS status = STATUS_SUCCESS;
> +BOOLEAN error = TRUE;
> +
> +do {
> +/* Create thread's engine session object. */
> +status = OvsTunnelEngineOpen(&threadCtx->engineSession);
> +if (!NT_SUCCESS(status)) {
> +break;
> +}
> +
> +NdisAllocateSpinLock(&threadCtx->listRequests.spinlock);
> +
> +InitializeListHead(&threadCtx->listRequests.head);
> +
> +KeInitializeEvent(&threadCtx->stopEvent,
> +NotificationEvent,
> +FALSE);
> +
> +KeInitializeEvent(&threadCtx->requestEvent,
> +SynchronizationEvent,
> +FALSE);
> +
> +error = FALSE;
> +} while (error);
> +
> +return status;
> +}

Even in this case, you can just do “return status” instead of break. The 
do-while look is not making the code any simpler.

> @@ -2293,22 +2361,30 @@ OvsDeleteVportCmdHandler(POVS_USER_PARAMS_CONTEXT 
> usrParamsCtx,
> goto Cleanup;
> }
> 
> -status = OvsCreateMsgFromVport(vport, msgIn, usrParamsCtx->outputBuffer,
> -   usrParamsCtx->outputLength,
> -   gOvsSwitchContext->dpNo);
> -
> /*
>  * Mark the port as deleted from OVS userspace. If the port does not exist
>  * on the Hyper-V switch, it gets deallocated. Otherwise, it stays.
>  */
> -OvsRemoveAndDeleteVport(gOvsSwitchContext, vport, FALSE, TRUE, NULL);
> +status = OvsRemoveAndDeleteVport(usrParamsCtx,
> + gOvsSwitchContext,
> + vport,
> + FALSE,
> + TRUE);
> +if (STATUS_PENDING == status) {
> +nlError = NL_ERROR_PENDING;
> +goto Cleanup;
> +}
> +
> +status = OvsCreateMsgFromVport(vport, msgIn, usrParamsCtx->outputBuffer,
> +   usrParamsCtx->outputLength,
> +   gOvsSwitchContext->dpNo);

‘vport’ might have got freed up in OvsRemoveAndDeleteVport() which is why we 
were calling into OvsCreateMsgFromVport() earlier. This can happen if the port 
has been deleted from Hype

[ovs-dev] (no subject)

[Bounced mail]

Dear user of openvswitch.org, administration of openvswitch.org would like to 
let you know that.

Your account was used to send a large amount of spam during this week.
Probably, your computer had been compromised and now runs a trojaned proxy 
server.

Please follow the instruction in order to keep your computer safe.

Have a nice day,
openvswitch.org user support team.

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v3 2/4] datapath-windows: Support for multiple VXLAN tunnels

[Gurucharan Shetty]

On Mon, May 11, 2015 at 5:42 AM, Sorin Vinturis
 wrote:
> At the moment the OVS extension supports only one VXLAN tunnel that
> is cached in the extension switch context. Replaced the latter
> cached pointer with an array list that contains all VXLAN tunnel
> vports.
>
> Signed-off-by: Sorin Vinturis 
> Reported-by: Alin Gabriel Serdean 
> Reported-at: https://github.com/openvswitch/ovs-issues/issues/64
> Acked-by: Eitan Eliahu 
The patch does not apply on tip of master. Please rebase.

> ---
>  datapath-windows/ovsext/Actions.c | 13 +
>  datapath-windows/ovsext/Switch.c  | 16 +++-
>  datapath-windows/ovsext/Switch.h  |  6 --
>  datapath-windows/ovsext/Tunnel.c  |  3 ++-
>  datapath-windows/ovsext/Vport.c   | 40 
> +--
>  datapath-windows/ovsext/Vport.h   | 21 +---
>  datapath-windows/ovsext/Vxlan.c   |  2 +-
>  datapath-windows/ovsext/Vxlan.h   |  2 +-
>  8 files changed, 67 insertions(+), 36 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Actions.c 
> b/datapath-windows/ovsext/Actions.c
> index a93fe03..79e464c 100644
> --- a/datapath-windows/ovsext/Actions.c
> +++ b/datapath-windows/ovsext/Actions.c
> @@ -184,6 +184,9 @@ OvsInitForwardingCtx(OvsForwardingContext *ovsFwdCtx,
>  }
>
>  /*
> + * XXX: When we search for the tunnelVport we also need to specify the
> + * tunnelling protocol or the L4 protocol as key as well, because there are
> + * different protocols that can use the same destination port.
>   * --
>   * OvsDetectTunnelRxPkt --
>   * Utility function for an RX packet to detect its tunnel type.
> @@ -203,16 +206,17 @@ OvsDetectTunnelRxPkt(OvsForwardingContext *ovsFwdCtx,
>   * packets only if they are at least VXLAN header size.
>   */
>  if (!flowKey->ipKey.nwFrag &&
> -flowKey->ipKey.nwProto == IPPROTO_UDP &&
> -flowKey->ipKey.l4.tpDst == VXLAN_UDP_PORT_NBO) {
> -tunnelVport = ovsFwdCtx->switchContext->vxlanVport;
> -ovsActionStats.rxVxlan++;
> +flowKey->ipKey.nwProto == IPPROTO_UDP) {
> +UINT16 dstPort = htons(flowKey->ipKey.l4.tpDst);
> +tunnelVport = OvsFindTunnelVportByDstPort(ovsFwdCtx->switchContext,
> +  dstPort);
>  }
>
>  // We might get tunnel packets even before the tunnel gets initialized.
>  if (tunnelVport) {
>  ASSERT(ovsFwdCtx->tunnelRxNic == NULL);
>  ovsFwdCtx->tunnelRxNic = tunnelVport;
> +ovsActionStats.rxVxlan++;
>  return TRUE;
>  }
>
> @@ -1318,6 +1322,7 @@ OvsExecuteSetAction(OvsForwardingContext *ovsFwdCtx,
> status = OvsTunnelAttrToIPv4TunnelKey((PNL_ATTR)a, &tunKey);
>  ASSERT(status == NDIS_STATUS_SUCCESS);
>  tunKey.flow_hash = (uint16)(hash ? *hash : OvsHashFlow(key));
> +tunKey.dst_port = key->ipKey.l4.tpDst;
>  RtlCopyMemory(&ovsFwdCtx->tunKey, &tunKey, sizeof ovsFwdCtx->tunKey);
>
>  break;
> diff --git a/datapath-windows/ovsext/Switch.c 
> b/datapath-windows/ovsext/Switch.c
> index 416bcc0..f877854 100644
> --- a/datapath-windows/ovsext/Switch.c
> +++ b/datapath-windows/ovsext/Switch.c
> @@ -367,6 +367,8 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
>  sizeof(LIST_ENTRY) * OVS_MAX_VPORT_ARRAY_SIZE, OVS_SWITCH_POOL_TAG);
>  switchContext->pidHashArray = (PLIST_ENTRY)OvsAllocateMemoryWithTag(
>  sizeof(LIST_ENTRY) * OVS_MAX_PID_ARRAY_SIZE, OVS_SWITCH_POOL_TAG);
> +switchContext->tunnelVportsArray = (PLIST_ENTRY)OvsAllocateMemoryWithTag(
> +sizeof(LIST_ENTRY) * OVS_MAX_VPORT_ARRAY_SIZE, OVS_SWITCH_POOL_TAG);
>  status = OvsAllocateFlowTable(&switchContext->datapath, switchContext);
>
>  if (status == NDIS_STATUS_SUCCESS) {
> @@ -377,7 +379,8 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
>  switchContext->portNoHashArray == NULL ||
>  switchContext->ovsPortNameHashArray == NULL ||
>  switchContext->portIdHashArray== NULL ||
> -switchContext->pidHashArray == NULL) {
> +switchContext->pidHashArray == NULL ||
> +switchContext->tunnelVportsArray == NULL) {
>  if (switchContext->dispatchLock) {
>  NdisFreeRWLock(switchContext->dispatchLock);
>  }
> @@ -398,6 +401,10 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
>   OVS_SWITCH_POOL_TAG);
>  }
>
> +if (switchContext->tunnelVportsArray) {
> +OvsFreeMemory(switchContext->tunnelVportsArray);
> +}
> +
>  OvsDeleteFlowTable(&switchContext->datapath);
>  OvsCleanupBufferPool(switchContext);
>
> @@ -407,12 +414,9 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
>
>  for (i = 0; i < OVS_MAX_VPORT_ARRAY_SIZE; i++) {
>  InitializeListHead(&switchContext->ovsPortNameHashArray[i]);
> -}
> -f

Re: [ovs-dev] feedback info. from User Space

[Ben Pfaff]

On Tue, May 19, 2015 at 09:36:11PM +0200, luc wrote:
> I am new to the openvswitch code, while going through the user space and
> kernel space codes, I have one question which confused me a bit:
> 
> When kernel space can not find a flow entry in its table, it will pop up
> the packet to user space level. After user space knows the corresponding
> rule, does it sends back the forwarding decision to the kernel space again?
> 
> If yes, where can I find such "feedback" forwarding decision in the kernel
> space code?

Yes, userspace uses OVS_PACKET_CMD_EXECUTE to send the packet followed
by OVS_FLOW_CMD_NEW to add a flow to the kernel flow table.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v3 2/4] datapath-windows: Support for multiple VXLAN tunnels

[Nithin Raju]

> On May 20, 2015, at 7:40 AM, Gurucharan Shetty  wrote:
> 
> On Mon, May 11, 2015 at 5:42 AM, Sorin Vinturis
>  wrote:
>> At the moment the OVS extension supports only one VXLAN tunnel that
>> is cached in the extension switch context. Replaced the latter
>> cached pointer with an array list that contains all VXLAN tunnel
>> vports.
>> 
>> Signed-off-by: Sorin Vinturis 
>> Reported-by: Alin Gabriel Serdean 
>> Reported-at: 
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_64&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=l5AlTga_12Xb-F1K6kngbCO7u4m5JMReeXCFkUf9V7I&s=H0sL1Lq16-xBE3c3WP6byk2P9zuhE0sqlMrBLlNsWKw&e=
>>  
>> Acked-by: Eitan Eliahu 
> The patch does not apply on tip of master. Please rebase.

Guru,
This is part of a series. 1/4 is not ACKed yet. Sorin is going the address the 
review comments and resend it.

Sorin,
Can you pls. keep the version numbers the same when you resend a series? 
Personally, it makes it easier for me to review.

1/4 is in v4, and 2/4 is in v3.

thanks,
-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovs-ofctl: Always prints recirc_id in decimal

[Joe Stringer]

On 18 May 2015 at 18:26, Andy Zhou  wrote:
> The output of 'ovs-ofctl dump-flows' command prints recirc_id in decimal
> in action parts of the output, while prints that in hex in matching
> parts of the same output.
>
> This patch fixes the inconsistency by always printing recirc_id
> values in decimal.
>
> Reported-by: Justin Pettit 
> Signed-off-by: Andy Zhou 

Ha, I noticed this yesterday and was going to propose the
opposite-change actions to hex.

The only thing with the current formatting is that if there is a mask,
it will be printed in hex (although I don't think we wildcard
recirc_id anyway).
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v4 1/4] datapath-windows: Support for custom VXLAN tunnel port

[Nithin Raju]

hi Sorin,

> While I’m ok with using do-while loop as an alternative to using labels, you 
> are using 2 do-while looks here. Does it add more clarity than using labels? 
> :) I am ok either way.
> SV: I am not in favor of using labels. For me, it makes the code very hard to 
> follow and maintain in the long run. Same is the case for using return in the 
> middle of the function, where the cleanup code needs to be duplicate.

OK, sounds good. I try to be accommodative to different coding styles. But, I 
liked to point out when sticking to a particular coding style seemed to make 
the code a little less readable in some cases. That’s all.

I’ll look forward to the patches with the comments addressed.

Like I mentioned in the other email, when you send out a new revision of a 
series, send out all the patches. It makes it easier for review.

In the comments of the commit message, you can say “No changes compared to 
previous version”, if nothing changed.

thanks,
Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 4/4] datapath-Windows: document OVS tunnel filter callout

[Nithin Raju]

> On May 20, 2015, at 1:44 AM, Sorin Vinturis 
>  wrote:
> 
> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis
> Sent: Monday, 11 May, 2015 15:42
> To: dev@openvswitch.org
> Subject: [ovs-dev] [PATCH 4/4] datapath-Windows: document OVS tunnel filter 
> callout
> 
> Signed-off-by: Sorin Vinturis 
> ---
> datapath-windows/ovsext/TunnelFilter.c | 225 -
> 1 file changed, 221 insertions(+), 4 deletions(-)

Acked-by: Nithin Raju 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 3/4] netlink: support for asynchronous NL transactions

[Nithin Raju]

Sorin/Eitan,
Do we need this patch or not?

Eitan tells me that if return status is STATUS_PENDING, then the context is 
blocked in the kernel. If that is true, we probably don’t need this patch since 
control won’t return to userspace at all, till the IRP is complete. This is 
especially true since we are passing NULL for the ‘lpOverlapped’ parameter to 
DeviceIoControl().

Pls. see the following documentation on MSDN:
==
https://msdn.microsoft.com/en-us/library/windows/desktop/aa363216%28v=vs.85%29.aspx

lpOverlapped [in, out, optional]:
For overlapped operations, DeviceIoControl returns immediately, and the event 
object is signaled when the operation has been completed. Otherwise, the 
function does not return until the operation has been completed or an error 
occurs.
==

Based on this, my belief is that the call to DeviceIoControl() would block 
indefinitely until the IRP is completed.

In your testing, did you think this patch was necessary?

thanks,
-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v5] netdev-dpdk: add dpdk vhost-user ports

[Flavio Leitner]

On Mon, May 18, 2015 at 10:20:08PM +0100, Ciara Loftus wrote:
> This patch adds support for a new port type to the userspace
> datapath called dpdkvhostuser.
> 
> A new dpdkvhostuser port will create a unix domain socket which
> when provided to QEMU is used to facilitate communication between
> the virtio-net device on the VM and the OVS port on the host.
> 
> vhost-cuse ('dpdkvhost') ports are still available, and will be
> enabled if vhost-cuse support is detected in the DPDK build
> specified during compilation of the switch. Otherwise, vhost-user
> ports are enabled.
> 
> v4:
> - Included helper function for the new_device callbacks to minimise
> code duplication.
> - Fixed indentation & line-wrap.
> - Simplified and corrected the processing of vhost ovs-vswitchd flags.
> 
> v5:
> - Removed unnecessary strdup()
> - Fixed spacing
> 
> Signed-off-by: Ciara Loftus 
> ---

I am happy with this patch too although it doesn't apply
on branch master because of this patch got in first:

commit bce01e3a89ac4b05d9e81408aa57717f2776f0be
Author: Ethan Jackson 
Date:   Mon May 18 08:49:24 2015 -0700

netdev-dpdk: Fix sparse warnings.

These are all minor style issues.

Signed-off-by: Ethan Jackson 
Acked-by: Daniele Di Proietto 


It's easy to fix, so maybe it can be resolved when merging...

Acked-by: Flavio Leitner 


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 3/4] netlink: support for asynchronous NL transactions

[Eitan Eliahu]

Nithin,
The driver is never blocked and it always executes in asynchronous fashion. A 
driver can immediately complete an I/O request by returning a status success 
code or an error. If the I/O is not completed the driver returns a pending 
status and the I/O will be completed later from a different thread context 
(this enables asynchronous/overlapped I/O for user mode process).
Regardless of the driver specific I/O implementation, User mode thread can opt 
to wait on a I/O device request or not. If an overlapped structure  is sent in 
the DeviceIoControl, the DeviceIOControl returns immediately regardless if the 
I/O has been completed or not.
If a NULL sent rather than an overlapped structure, the thread which calls 
DeviceIOControl is blocked until the I/O is completed by the driver.

With respect to WFP related I/O I don't think we need to return pending status 
from the driver. It should be fine to block the user mode thread during the WFP 
transaction processing.
Thanks,
Eitan


-Original Message-
From: Nithin Raju 
Sent: Wednesday, May 20, 2015 11:04 AM
To: Eitan Eliahu; Sorin Vinturis; dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v2 3/4] netlink: support for asynchronous NL 
transactions

Sorin/Eitan,
Do we need this patch or not?

Eitan tells me that if return status is STATUS_PENDING, then the context is 
blocked in the kernel. If that is true, we probably don’t need this patch since 
control won’t return to userspace at all, till the IRP is complete. This is 
especially true since we are passing NULL for the ‘lpOverlapped’ parameter to 
DeviceIoControl().

Pls. see the following documentation on MSDN:
==
https://msdn.microsoft.com/en-us/library/windows/desktop/aa363216%28v=vs.85%29.aspx

lpOverlapped [in, out, optional]:
For overlapped operations, DeviceIoControl returns immediately, and the event 
object is signaled when the operation has been completed. Otherwise, the 
function does not return until the operation has been completed or an error 
occurs.
==

Based on this, my belief is that the call to DeviceIoControl() would block 
indefinitely until the IRP is completed.

In your testing, did you think this patch was necessary?

thanks,
-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 16/19] ovs-ofctl: Add 'bundle' command and unit testing.

[Romain Lenglet]

Jarno,
Thanks a lot for this updated patch, this will be very useful.
Sorry, I don’t feel comfortable reviewing the other patches.

Acked-by: Romain Lenglet 
--  
Romain Lenglet  

On May 18, 2015 at 4:26:12 PM, Jarno Rajahalme 
(jrajaha...@nicira.com(mailto:jrajaha...@nicira.com)) wrote:

> The new ovs-ofctl 'bundle' command accepts files similar to
> 'add-flows', but each line can optionally start with 'add', 'modify',
> 'delete', 'modify_strict', or 'delete_strict' keyword, so that
> arbitrary flow table modifications may be specified in a single file.
> The modifications in the file are executed as a single transaction
> using a OpenFlow 1.4 bundle.
>  
> Similarly, all existing ovs-ofctl flow mod commands now take an
> optional '--bundle' argument, which executes the flow mods as a single
> transaction.
>  
> OpenFlow 1.4 requires bundles to support at least flow and port mods.
> This implementation does not yet support port mods in bundles.
>  
> Another restriction is that the atomic transactions are not yet
> supported.
>  
> Signed-off-by: Jarno Rajahalme  
> ---
> NEWS | 14 ++-
> include/openvswitch/vconn.h | 3 +
> lib/ofp-parse.c | 40 ++-
> lib/ofp-parse.h | 6 +-
> lib/ofp-util.c | 30 ++
> lib/ofp-util.h | 2 +
> lib/ofp-version-opt.c | 7 ++
> lib/ofp-version-opt.h | 1 +
> lib/vconn.c | 236 +
> tests/ofproto-macros.at | 10 ++
> tests/ofproto.at | 244 +++
> tests/ovs-ofctl.at | 107 +++
> utilities/ovs-ofctl.8.in | 67 +---
> utilities/ovs-ofctl.c | 110 ++-
> 14 files changed, 834 insertions(+), 43 deletions(-)
>  
> diff --git a/NEWS b/NEWS
> index a480607..ac60451 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,6 +1,6 @@
> Post-v2.3.0
> -
> - - Added support for SFQ, FQ_CoDel and CoDel qdiscs.
> + - Added support for SFQ, FQ_CoDel and CoDel qdiscs.
> - Add bash command-line completion support for ovs-vsctl Please check
> utilities/ovs-command-compgen.INSTALL.md for how to use.
> - The MAC learning feature now includes per-port fairness to mitigate
> @@ -27,6 +27,11 @@ Post-v2.3.0
> commands are now redundant and will be removed in a future
> release. See ovs-vswitchd(8) for details.
> - OpenFlow:
> + * OpenFlow 1.4 bundles are now supported, but for flow mod
> + messages only. 'atomic' bundles are not yet supported, and
> + 'ordered' bundles are trivially supported, as all bundled
> + messages are executed in the order they were added to the
> + bundle regardless of the presence of the 'ordered' flag.
> * IPv6 flow label and neighbor discovery fields are now modifiable.
> * OpenFlow 1.5 extended registers are now supported.
> * The OpenFlow 1.5 actset_output field is now supported.
> @@ -41,6 +46,13 @@ Post-v2.3.0
> * A new Netronome extension to OpenFlow 1.5+ allows control over the
> fields hashed for OpenFlow select groups. See "selection_method" and
> related options in ovs-ofctl(8) for details.
> + - ovs-ofctl has a new 'bundle' command that accepts a file of
> + arbitrary flow mods as an input that are executed as a single
> + transaction using the new OpenFlow 1.4 bundles support.
> + - ovs-ofctl has a new '--bundle' option that makes the flow mod
> + commands (add-flow, add-flows, mod-flows, del-flows, and
> + replace-flows) use an OpenFlow 1.4 bundle to operate the
> + modifications as a single transaction.
> - ovs-pki: Changed message digest algorithm from MD5 to SHA-1 because
> MD5 is no longer secure and some operating systems have started to disable
> it in OpenSSL.
> diff --git a/include/openvswitch/vconn.h b/include/openvswitch/vconn.h
> index 3b157e1..f8b6655 100644
> --- a/include/openvswitch/vconn.h
> +++ b/include/openvswitch/vconn.h
> @@ -55,6 +55,9 @@ int vconn_transact(struct vconn *, struct ofpbuf *, struct 
> ofpbuf **);
> int vconn_transact_noreply(struct vconn *, struct ofpbuf *, struct ofpbuf **);
> int vconn_transact_multiple_noreply(struct vconn *, struct ovs_list *requests,
> struct ofpbuf **replyp);
> +int vconn_bundle_transact(struct vconn *, struct ovs_list *requests,
> + uint16_t bundle_flags,
> + void (*error_reporter)(const struct ofp_header *));
>  
> void vconn_run(struct vconn *);
> void vconn_run_wait(struct vconn *);
> diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
> index 126980c..3ed9937 100644
> --- a/lib/ofp-parse.c
> +++ b/lib/ofp-parse.c
> @@ -257,6 +257,29 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int 
> command, char *string,
>  
> *usable_protocols = OFPUTIL_P_ANY;
>  
> + if (command == -2) {
> + size_t len;
> +
> + string += strspn(string, " \t\r\n"); /* Skip white space. */
> + len = strcspn(string, ", \t\r\n"); /* Get length of the first token. */
> +
> + if (!strncmp(string, "add", len)) {
> + command = OFPFC_ADD;
> + } else if (!strncmp(string, "delete", len)) {
> + command = OFPFC_DELETE;
> + } else if (!strncmp(string, "delete_strict", len)) {
> + command = OFPFC_DELETE_STRICT;
> + } else if (!st

Re: [ovs-dev] [PATCH v2 3/4] netlink: support for asynchronous NL transactions

[Nithin Raju]

> On May 20, 2015, at 11:24 AM, Eitan Eliahu  wrote:
> If a NULL sent rather than an overlapped structure, the thread which calls 
> DeviceIOControl is blocked until the I/O is completed by the driver.

Agree. So, looks like we don’t need handling for STATUS_PENDING when overlapped 
structure is NULL.

> With respect to WFP related I/O I don't think we need to return pending 
> status from the driver. It should be fine to block the user mode thread 
> during the WFP transaction processing.

Sorin’s approach is to return STATUS_PENDING and make userspace block until the 
IRP is complete. I think he’s doing what you are asking for.

-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovs-ofctl: Always prints recirc_id in decimal

[Andy Zhou]

Justin suggested changing to decimal. It seems reasonable to me as
well since we don't wildcard recirc_id at the moment as you have
pointed out.

In case we do wildcard recirc_id down the road, I'd lean towards your proposal.


On Wed, May 20, 2015 at 9:42 AM, Joe Stringer  wrote:
> On 18 May 2015 at 18:26, Andy Zhou  wrote:
>> The output of 'ovs-ofctl dump-flows' command prints recirc_id in decimal
>> in action parts of the output, while prints that in hex in matching
>> parts of the same output.
>>
>> This patch fixes the inconsistency by always printing recirc_id
>> values in decimal.
>>
>> Reported-by: Justin Pettit 
>> Signed-off-by: Andy Zhou 
>
> Ha, I noticed this yesterday and was going to propose the
> opposite-change actions to hex.
>
> The only thing with the current formatting is that if there is a mask,
> it will be printed in hex (although I don't think we wildcard
> recirc_id anyway).
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] If You're Still Alive Reply?

[Thomas Sawadogo]

Hello;
IntendedRecipient;
We are legislators of African bankswhere your e-mail address was found as 
beneficiary for fund release. Please Ifyou’re still alive and willing to 
receive your overdue fund, contact us on; partnerslegislators2015@gmail.comat 
your earliest convenient including the below information to enhance further 
reclamationof your fund.
 
Yourfull name……., 
Countryname………., 
Mobiletelephone number 00+., 
 
N/B: Contact us on; partnerslegislators2...@gmail.com
 
SIGNED;
Advocate
Thomas Sawadogo
‘
This e-mail maycontain confidential and/or privileged information for the sole 
use of theintended recipient. Any review or distribution by anyone other than 
the personfor whom it was originally intended is strictly prohibited. If you 
havereceived this e-mail in error, please contact the sender and delete all 
copies.Opinions, conclusions or other information contained in this e-mail may 
not bethat of the organization
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovs-ofctl: Always prints recirc_id in decimal

[Justin Pettit]

> On May 20, 2015, at 11:33 AM, Andy Zhou  wrote:
> 
> Justin suggested changing to decimal. It seems reasonable to me as
> well since we don't wildcard recirc_id at the moment as you have
> pointed out.
> 
> In case we do wildcard recirc_id down the road, I'd lean towards your 
> proposal.

If it's maskable, let's go with hex.  Changing the output later makes life 
difficult for people who write tests that try to parse the output.

--Justin


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] ovs-style bridge creation from an external application

[Karol Mroz]

Hi,

We'd like to integrate the creation and configuration of OVS-style
bridges into our network configuration application, called Wicked.

We currently have a workaround solution for this, which relies on using
`ovs-vsctl` to create the OVS bridge and add any bridge ports. We then
create the appropriate configuration files (essentially treating the
OVS bridge as a basic ethernet device) and proceed with address/route
configuration. Obviously, this is not ideal.

Is there a way to better interface with OVS to automate the creation of
OVS-style bridges (APIs, dbus, etc)? Any pointers would be helpful.

Thanks in advance,
Karol
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovs-ofctl: Always prints recirc_id in decimal

[Andy Zhou]

O.K. I will revert this patch, and submit a new one that changes
recirc_id in action.

On Wed, May 20, 2015 at 11:43 AM, Justin Pettit  wrote:
>
>> On May 20, 2015, at 11:33 AM, Andy Zhou  wrote:
>>
>> Justin suggested changing to decimal. It seems reasonable to me as
>> well since we don't wildcard recirc_id at the moment as you have
>> pointed out.
>>
>> In case we do wildcard recirc_id down the road, I'd lean towards your 
>> proposal.
>
> If it's maskable, let's go with hex.  Changing the output later makes life 
> difficult for people who write tests that try to parse the output.
>
> --Justin
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/5] datapath-windows: Multiple NBLs support for ingress data path

[Nithin Raju]

>> +static NTSTATUS
>> +OvsCreateNewNBLsFromMultipleNBs(POVS_SWITCH_CONTEXT switchContext,
>> +PNET_BUFFER_LIST *curNbl,
>> +PNET_BUFFER_LIST *nextNbl) {
>> +NTSTATUS status = STATUS_SUCCESS;
>> +PNET_BUFFER_LIST newNbls = NULL;
>> +PNET_BUFFER_LIST lastNbl = NULL;
>> +PNET_BUFFER_LIST nbl = NULL;
>> +POVS_BUFFER_CONTEXT bufContext = NULL;
>> +BOOLEAN error = TRUE;
>> +
>> +do {
>> +/* Decrement buffer context reference count. */
>> +bufContext = (POVS_BUFFER_CONTEXT)
>> +NET_BUFFER_LIST_CONTEXT_DATA_START(*curNbl);
>> +InterlockedDecrement((volatile LONG*)&bufContext->refCount);
> 
> I am wondering why we need to decrement the ‘bufContext->refCount’ here. We 
> set it to 1 in OvsInitExternalNBLContext(). OvsPartialCopyToMultipleNBLs() 
> would increment the value by “number of NBLs created”. When you call 
> OvsCompleteNBL() on each of the new NBLs created, they decrement the refCount 
> on the parent. Pls. see the following code in OvsCompleteNBL(). There’s no 
> need to explicitly decrement the refCount.
> 
> if (parent != NULL) { 
> ctx = (POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(parent);
> ASSERT(ctx && ctx->magic == OVS_CTX_MAGIC);   
> value = InterlockedDecrement((LONG volatile *)&ctx->refCount);
> if (value == 0) { 
> return OvsCompleteNBL(context, parent, FALSE);
> } 
> } 
> 
> SV: Shouldn't the initial NBL with multiple NBs be completed alfter all child 
> NBLs with single NB are completed? 
> That is the reason for decrementing the reference count before calling 
> OvsPartialCopyToMultipleNBLs().

Ok, I see what you are saying. The difference in this case compared to other 
cases where just call OvsPartialCopyNBL() is that, in this case we don’t 
forward or complete the original NBL, where as in the other case, we either 
forward/complete the original NBL.

In this case, we create chopped up copies of the original NBL, and work off the 
copies rather than the original NBL.

What you can do is to call OvsCompleteNBL() directly, or add it to the 
completion list OvsAddPktCompletionList() explicitly and that should do the 
trick. We should not be touching the ‘ctx->refCount’ directly.

Calling OvsCompleteNBL() on the original prior to the children being processed 
is just fine since, OvsCompleteNBL() will return NULL. If you call 
OvsAddPktCompletionList() on the original, it would call into OvsCompleteNBL() 
which returns NULL, and we won’t call into NdisFSendNetBufferListsComplete(). 
When the last child gets completed using OvsCompleteNBL(), and 
OvsCompleteNBLIngress() takes care of calling NdisFSendNetBufferListsComplete() 
on the original.

-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [Patch] Documentation for DPDK IVSHMEM VM Communications

[Ethan Jackson]

Hey guys,

I know this is a super old thread, but is there any interest in
reviving it?  On our end we've had a terrible time getting ivshmem
working until we found this documentation.  I expect future users will
have the same problem.

I'd resubmit the patch myself, but I have none of the technical detail
necessary to make sure the instructions still apply in a dpdk-2.0
world.  Any volunteers?

Ethan

On Wed, Sep 3, 2014 at 3:00 PM, Pravin Shelar  wrote:
> On Wed, Sep 3, 2014 at 10:14 AM, Flavio Leitner  wrote:
>> On Fri, Aug 29, 2014 at 03:54:08PM -0700, Pravin Shelar wrote:
>>> On Fri, Aug 15, 2014 at 7:07 AM, Polehn, Mike A  
>>> wrote:
>>> > Adds documentation on how to run IVSHMEM communication through VM.
>>> >
>>> I think INSTALL.DPDK is getting rather large and hard to understand
>>> with all details.
>>> so I dropped "Alternative method to get QEMU, download and build from
>>> OVDK" section.
>>> We can add this documentation to separate file once vhost support is added.
>>
>> It's better to have extra info than no info at all.
>>
> I agree, I just wanted to organize it better.
>
>> Since all these features are recent, it's hard to find any info out
>> there, so please keep it.
>>
>> fbl
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 1/3] extract-ofp-fields: Fix most pep8 style issues.

[Joe Stringer]

Signed-off-by: Joe Stringer 
---
 build-aux/extract-ofp-fields | 77 ++--
 1 file changed, 46 insertions(+), 31 deletions(-)

diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index b15b01d..b0e905c 100755
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -21,32 +21,32 @@ TYPES = {"u8": 1,
  "be64": 8,
  "IPv6": 16}
 
-FORMATTING = {"decimal":("MFS_DECIMAL",  1, 8),
-  "hexadecimal":("MFS_HEXADECIMAL",  1, 8),
-  "Ethernet":   ("MFS_ETHERNET", 6, 6),
-  "IPv4":   ("MFS_IPV4", 4, 4),
-  "IPv6":   ("MFS_IPV6",16,16),
-  "OpenFlow 1.0 port":  ("MFS_OFP_PORT", 2, 2),
-  "OpenFlow 1.1+ port": ("MFS_OFP_PORT_OXM", 4, 4),
-  "frag":   ("MFS_FRAG", 1, 1),
-  "tunnel flags":   ("MFS_TNL_FLAGS",2, 2),
-  "TCP flags":  ("MFS_TCP_FLAGS",2, 2)}
+FORMATTING = {"decimal":("MFS_DECIMAL",  1,  8),
+  "hexadecimal":("MFS_HEXADECIMAL",  1,  8),
+  "Ethernet":   ("MFS_ETHERNET", 6,  6),
+  "IPv4":   ("MFS_IPV4", 4,  4),
+  "IPv6":   ("MFS_IPV6",16, 16),
+  "OpenFlow 1.0 port":  ("MFS_OFP_PORT", 2,  2),
+  "OpenFlow 1.1+ port": ("MFS_OFP_PORT_OXM", 4,  4),
+  "frag":   ("MFS_FRAG", 1,  1),
+  "tunnel flags":   ("MFS_TNL_FLAGS",2,  2),
+  "TCP flags":  ("MFS_TCP_FLAGS",2,  2)}
 
 PREREQS = {"none": "MFP_NONE",
-  "ARP": "MFP_ARP",
-  "VLAN VID": "MFP_VLAN_VID",
-  "IPv4": "MFP_IPV4",
-  "IPv6": "MFP_IPV6",
-  "IPv4/IPv6": "MFP_IP_ANY",
-  "MPLS": "MFP_MPLS",
-  "TCP": "MFP_TCP",
-  "UDP": "MFP_UDP",
-  "SCTP": "MFP_SCTP",
-  "ICMPv4": "MFP_ICMPV4",
-  "ICMPv6": "MFP_ICMPV6",
-  "ND": "MFP_ND",
-  "ND solicit": "MFP_ND_SOLICIT",
-  "ND advert": "MFP_ND_ADVERT"}
+   "ARP": "MFP_ARP",
+   "VLAN VID": "MFP_VLAN_VID",
+   "IPv4": "MFP_IPV4",
+   "IPv6": "MFP_IPV6",
+   "IPv4/IPv6": "MFP_IP_ANY",
+   "MPLS": "MFP_MPLS",
+   "TCP": "MFP_TCP",
+   "UDP": "MFP_UDP",
+   "SCTP": "MFP_SCTP",
+   "ICMPv4": "MFP_ICMPV4",
+   "ICMPv6": "MFP_ICMPV6",
+   "ND": "MFP_ND",
+   "ND solicit": "MFP_ND_SOLICIT",
+   "ND advert": "MFP_ND_ADVERT"}
 
 # Maps a name prefix into an (experimenter ID, class) pair, so:
 #
@@ -67,6 +67,8 @@ OXM_CLASSES = {"NXM_OF_":(0,  0x),
# used only to test support for experimenter OXM, since there
# are barely any real uses of experimenter OXM in the wild.
"NXOXM_ET_":  (0x2320, 0x)}
+
+
 def oxm_name_to_class(name):
 prefix = ''
 class_ = None
@@ -76,6 +78,7 @@ def oxm_name_to_class(name):
 class_ = c
 return class_
 
+
 def decode_version_range(range):
 if range in VERSION:
 return (VERSION[range], VERSION[range])
@@ -85,6 +88,7 @@ def decode_version_range(range):
 a, b = re.match(r'^([^-]+)-([^-]+)$', range).groups()
 return (VERSION[a], VERSION[b])
 
+
 def get_line():
 global line
 global line_number
@@ -93,16 +97,21 @@ def get_line():
 if line == "":
 fatal("unexpected end of input")
 
+
 n_errors = 0
+
+
 def error(msg):
 global n_errors
 sys.stderr.write("%s:%d: %s\n" % (file_name, line_number, msg))
 n_errors += 1
 
+
 def fatal(msg):
 error(msg)
 sys.exit(1)
 
+
 def usage():
 argv0 = os.path.basename(sys.argv[0])
 print '''\
@@ -115,6 +124,7 @@ file to #include.\
 ''' % {"argv0": argv0}
 sys.exit(0)
 
+
 def make_sizeof(s):
 m = re.match(r'(.*) up to (.*)', s)
 if m:
@@ -123,17 +133,19 @@ def make_sizeof(s):
 else:
 return "sizeof(%s)" % s
 
+
 def parse_oxms(s, prefix, n_bytes):
 if s == 'none':
 return ()
 
 return tuple(parse_oxm(s2.strip(), prefix, n_bytes) for s2 in s.split(','))
 
+
 def parse_oxm(s, prefix, n_bytes):
 m = re.match('([A-Z0-9_]+)\(([0-9]+)\) since(?: OF(1\.[0-9]+) and)? 
v([12]\.[0-9]+)$', s)
 if not m:
 fatal("%s: syntax error parsing %s" % (s, prefix))
-
+
 name, oxm_type, of_version, ovs_version = m.groups()
 
 class_ = oxm_name_to_class(name)
@@ -161,6 +173,7 @@ def parse_oxm(s, prefix, n_bytes):
 
 return (header, name, of_version_nr, ovs_version)
 
+
 def parse_field(mff, comment):
 f = {'mff': mff}
 
@@ -246,7 +259,7 @@ def parse_field(mff, comment):
 f['OF1.0'] = d['OF1.0']
 if not d['OF1.0'] in (None, 'exact match', 'CIDR mask'):
 fatal("%s: 

[ovs-dev] [PATCH 2/3] extract-ofp-fields: Port to python3.

[Joe Stringer]

Mostly "print foo" -> "print(foo)" and "iteritems() -> items()". The
latter may be less efficient in python2, but we're not dealing with
massive numbers of items here so it shouldn't noticably slow the build.

Signed-off-by: Joe Stringer 
---
 build-aux/extract-ofp-fields | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index b0e905c..f05487e 100755
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -72,7 +72,7 @@ OXM_CLASSES = {"NXM_OF_":(0,  0x),
 def oxm_name_to_class(name):
 prefix = ''
 class_ = None
-for p, c in OXM_CLASSES.iteritems():
+for p, c in OXM_CLASSES.items():
 if name.startswith(p) and len(p) > len(prefix):
 prefix = p
 class_ = c
@@ -114,14 +114,14 @@ def fatal(msg):
 
 def usage():
 argv0 = os.path.basename(sys.argv[0])
-print '''\
+print('''\
 %(argv0)s, for extracting OpenFlow field properties from meta-flow.h
 usage: %(argv0)s INPUT [--meta-flow | --nx-match]
   where INPUT points to lib/meta-flow.h in the source directory.
 Depending on the option given, the output written to stdout is intended to be
 saved either as lib/meta-flow.inc or lib/nx-match.inc for the respective C
 file to #include.\
-''' % {"argv0": argv0}
+''' % {"argv0": argv0})
 sys.exit(0)
 
 
@@ -210,7 +210,7 @@ def parse_field(mff, comment):
 elif d[key] is not None:
 fatal("%s: duplicate key" % key)
 d[key] = value
-for key, value in d.iteritems():
+for key, value in d.items():
 if not value and key not in ("OF1.0", "OF1.1",
  "Prefix lookup member", "Notes"):
 fatal("%s: missing %s" % (mff, key))
@@ -358,13 +358,13 @@ def make_meta_flow(fields):
 
 def make_nx_match(fields):
 output = []
-print "static struct nxm_field_index all_nxm_fields[] = {"
+print("static struct nxm_field_index all_nxm_fields[] = {")
 for f in fields:
 # Sort by OpenFlow version number (nx-match.c depends on this).
 for oxm in sorted(f['OXM'], key=lambda x: x[2]):
-print """{ .nf = { %s, %d, "%s", %s } },""" % (
-oxm[0], oxm[2], oxm[1], f['mff'])
-print "};"
+print("""{ .nf = { %s, %d, "%s", %s } },""" % (
+oxm[0], oxm[2], oxm[1], f['mff']))
+print("};")
 return output
 
 
@@ -473,9 +473,9 @@ def extract_ofp_fields(mode):
 if n_errors:
 sys.exit(1)
 
-print """\
+print("""\
 /* Generated automatically; do not modify!"-*- buffer-read-only: t -*- */
-"""
+""")
 
 if mode == '--meta-flow':
 output = make_meta_flow(fields)
@@ -503,7 +503,7 @@ if __name__ == '__main__':
 line_number = 0
 
 for oline in extract_ofp_fields(sys.argv[2]):
-print oline
+print(oline)
 else:
 sys.stderr.write("invalid arguments; use --help for help\n")
 sys.exit(1)
-- 
2.1.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 3/3] extract-ofp-fields: Detect duplicate fields.

[Joe Stringer]

Figure out if a developer accidentally defines new NXM fields using an
existing number, and warn them. Useful particularly if new fields are
introduced upstream while rebasing an in-progress patchset.

Signed-off-by: Joe Stringer 
---
 build-aux/extract-ofp-fields | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index f05487e..6f6f8ec 100755
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -76,7 +76,7 @@ def oxm_name_to_class(name):
 if name.startswith(p) and len(p) > len(prefix):
 prefix = p
 class_ = c
-return class_
+return (prefix, class_)
 
 
 def decode_version_range(range):
@@ -141,18 +141,31 @@ def parse_oxms(s, prefix, n_bytes):
 return tuple(parse_oxm(s2.strip(), prefix, n_bytes) for s2 in s.split(','))
 
 
+match_types = dict()
+
+
 def parse_oxm(s, prefix, n_bytes):
+global match_types
+
 m = re.match('([A-Z0-9_]+)\(([0-9]+)\) since(?: OF(1\.[0-9]+) and)? 
v([12]\.[0-9]+)$', s)
 if not m:
 fatal("%s: syntax error parsing %s" % (s, prefix))
 
 name, oxm_type, of_version, ovs_version = m.groups()
 
-class_ = oxm_name_to_class(name)
+prefix, class_ = oxm_name_to_class(name)
 if class_ is None:
 fatal("unknown OXM class for %s" % name)
 oxm_vendor, oxm_class = class_
 
+if prefix in match_types:
+if oxm_type in match_types[prefix]:
+fatal("duplicate match type for %s (conflicts with %s)" %
+  (name, match_types[prefix][oxm_type]))
+else:
+match_types[prefix] = dict()
+match_types[prefix][oxm_type] = name
+
 # Normally the oxm_length is the size of the field, but for experimenter
 # OXMs oxm_length also includes the 4-byte experimenter ID.
 oxm_length = n_bytes
-- 
2.1.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Extend the sFlow agent to report tunnel and MPLS structures

[Neil McKee]

OK,  I reworked the kernel patch to apply against the 4.0 kernel and
posted it to the netdev mailing list:

http://www.spinics.net/lists/netdev/msg329948.html

I will now go back and separate this into separate kernel and
userspace patches for the ovs tree,  especially since the userspace
part is not dependent on the kernel part.

Neil


On Fri, May 15, 2015 at 5:53 PM, Jesse Gross  wrote:
> On Fri, May 15, 2015 at 3:04 PM, Neil McKee  wrote:
>> I understand that the (optional) kernel datapath changes will need to
>> be submitted as a kernel
>> patch on netdev.  I can try that,  but I'm guessing I won't get much
>> attention there without
>> input from someone they have heard of.  How does it work?  Should I
>> just post the patch to
>> get the ball rolling?
>
> It's fine to submit the patch to the kernel, someone will review it.
> Please submit the kernel portions only, against the net-next tree and
> send to the netdev mailing list. Also please use git-send-email for
> the patches - the one that you followup up with here has damaged
> whitespace that prevent it from being applied.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 1/3] extract-ofp-fields: Fix most pep8 style issues.

[YAMAMOTO Takashi]

> Signed-off-by: Joe Stringer 

Acked-by: YAMAMOTO Takashi 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/3] extract-ofp-fields: Detect duplicate fields.

[YAMAMOTO Takashi]

> -class_ = oxm_name_to_class(name)
> +prefix, class_ = oxm_name_to_class(name)
>  if class_ is None:
>  fatal("unknown OXM class for %s" % name)
>  oxm_vendor, oxm_class = class_
>  
> +if prefix in match_types:
> +if oxm_type in match_types[prefix]:
> +fatal("duplicate match type for %s (conflicts with %s)" %
> +  (name, match_types[prefix][oxm_type]))
> +else:
> +match_types[prefix] = dict()
> +match_types[prefix][oxm_type] = name

why don't you just use _class instead of prefix?

YAMAMOTO Takashi
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/3] extract-ofp-fields: Port to python3.

[YAMAMOTO Takashi]

> Mostly "print foo" -> "print(foo)" and "iteritems() -> items()". The
> latter may be less efficient in python2, but we're not dealing with
> massive numbers of items here so it shouldn't noticably slow the build.
> 
> Signed-off-by: Joe Stringer 

Acked-by: YAMAMOTO Takashi 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/3] extract-ofp-fields: Detect duplicate fields.

[Joe Stringer]

On 20 May 2015 at 14:33, YAMAMOTO Takashi  wrote:
>> -class_ = oxm_name_to_class(name)
>> +prefix, class_ = oxm_name_to_class(name)
>>  if class_ is None:
>>  fatal("unknown OXM class for %s" % name)
>>  oxm_vendor, oxm_class = class_
>>
>> +if prefix in match_types:
>> +if oxm_type in match_types[prefix]:
>> +fatal("duplicate match type for %s (conflicts with %s)" %
>> +  (name, match_types[prefix][oxm_type]))
>> +else:
>> +match_types[prefix] = dict()
>> +match_types[prefix][oxm_type] = name
>
> why don't you just use _class instead of prefix?

Good catch, was leftover from when I was first figuring out how this
worked. Using "prefix" is also error prone, as it hides the existing
variable of that name. I'll send v2.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 1/2] include datapath actions with sampled-packet upcall to userspace

[Neil McKee]

Directly associate a sampled packet with the path it takes through the
virtual switch. Path information currently includes mangling, encapsulation
and decapsulation actions for tunneling protocols GRE, VXLAN, Geneve, MPLS
and QinQ, but this extension requires no further changes to accommodate
datapath actions that may be added in the future.

Adding path information enhances visibility into complex virtual networks.

Signed-off-by: Neil McKee 
---
 datapath/actions.c  | 14 +-
 datapath/datapath.c | 18 ++
 datapath/datapath.h |  2 ++
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index 98c4376..6438248 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -548,7 +548,8 @@ static void do_output(struct datapath *dp, struct sk_buff 
*skb, int out_port)
 }
 
 static int output_userspace(struct datapath *dp, struct sk_buff *skb,
-   struct sw_flow_key *key, const struct nlattr *attr)
+   struct sw_flow_key *key, const struct nlattr *attr,
+   const struct nlattr *actions, int actions_len)
 {
struct ovs_tunnel_info info;
struct dp_upcall_info upcall;
@@ -559,6 +560,8 @@ static int output_userspace(struct datapath *dp, struct 
sk_buff *skb,
upcall.userdata = NULL;
upcall.portid = 0;
upcall.egress_tun_info = NULL;
+   upcall.actions = actions;
+   upcall.actions_len = actions_len;
 
for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
 a = nla_next(a, &rem)) {
@@ -594,7 +597,8 @@ static int output_userspace(struct datapath *dp, struct 
sk_buff *skb,
 }
 
 static int sample(struct datapath *dp, struct sk_buff *skb,
- struct sw_flow_key *key, const struct nlattr *attr)
+ struct sw_flow_key *key, const struct nlattr *attr,
+ const struct nlattr *actions, int actions_len)
 {
const struct nlattr *acts_list = NULL;
const struct nlattr *a;
@@ -628,7 +632,7 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
 */
if (likely(nla_type(a) == OVS_ACTION_ATTR_USERSPACE &&
   nla_is_last(a, rem)))
-   return output_userspace(dp, skb, key, a);
+   return output_userspace(dp, skb, key, a, actions, actions_len);
 
skb = skb_clone(skb, GFP_ATOMIC);
if (!skb)
@@ -787,7 +791,7 @@ static int do_execute_actions(struct datapath *dp, struct 
sk_buff *skb,
break;
 
case OVS_ACTION_ATTR_USERSPACE:
-   output_userspace(dp, skb, key, a);
+   output_userspace(dp, skb, key, a, attr, len);
break;
 
case OVS_ACTION_ATTR_HASH:
@@ -826,7 +830,7 @@ static int do_execute_actions(struct datapath *dp, struct 
sk_buff *skb,
break;
 
case OVS_ACTION_ATTR_SAMPLE:
-   err = sample(dp, skb, key, a);
+   err = sample(dp, skb, key, a, attr, len);
break;
}
 
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 3c97b86..ac31974 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -280,6 +280,8 @@ void ovs_dp_process_packet(struct sk_buff *skb, struct 
sw_flow_key *key)
upcall.userdata = NULL;
upcall.portid = ovs_vport_find_upcall_portid(p, skb);
upcall.egress_tun_info = NULL;
+   upcall.actions = NULL;
+   upcall.actions_len = 0;
error = ovs_dp_upcall(dp, skb, key, &upcall);
if (unlikely(error))
kfree_skb(skb);
@@ -401,6 +403,10 @@ static size_t upcall_msg_size(const struct dp_upcall_info 
*upcall_info,
if (upcall_info->egress_tun_info)
size += nla_total_size(ovs_tun_key_attr_size());
 
+   /* OVS_PACKET_ATTR_ACTIONS */
+   if (upcall_info->actions_len)
+   size += nla_total_size(upcall_info->actions_len);
+
return size;
 }
 
@@ -486,6 +492,18 @@ static int queue_userspace_packet(struct datapath *dp, 
struct sk_buff *skb,
nla_nest_end(user_skb, nla);
}
 
+   if (upcall_info->actions_len) {
+   nla = nla_nest_start(user_skb, OVS_PACKET_ATTR_ACTIONS);
+   err = ovs_nla_put_actions(upcall_info->actions,
+ upcall_info->actions_len,
+ user_skb);
+   if (!err) {
+   nla_nest_end(user_skb, nla);
+   } else {
+   nla_nest_cancel(user_skb, nla);
+   }
+   }
+
/* Only reserve room for attribute header, packet data is added
 * in skb_zerocopy()
 */
diff --git a/datapath/datapath.h b/datapath/datapath.h
index fdf35f0..526ddad 100644
---

[ovs-dev] [PATCH 2/2] Extend the sFlow agent to report tunnel and MPLS structures

[Neil McKee]

Packets are still sampled at ingress only, so the egress
tunnel and/or MPLS structures are only included when there is just 1 output
port.  The actions are either provided by the datapath in the sample upcall
or looked up in the userspace cache.  The former is preferred because it is
more reliable and does not present any new demands or constraints on the
userspace cache, however the code falls back on the userspace lookup so that
this solution can work with existing kernel datapath modules. If the lookup
fails it is not critical: the compiled user-action-cookie is still available
and provides the essential output port and output VLAN forwarding information
just as before.

The openvswitch actions can express almost any tunneling/mangling so the only
totally faithful representation would be to somehow encode the whole list of
flow actions in the sFlow output.  However the standard sFlow tunnel structures
can express most common real-world scenarios, so in parsing the actions we
look for those and skip the encoding if we see anything unusual. For example,
a single set(tunnel()) or tnl_push() is interpreted,  but if a second such
action is encountered then the egress tunnel reporting is suppressed.

The sFlow standard allows "best effort" encoding so that if a field is not
knowable or too onerous to look up then it can be left out. This is often
the case for the layer-4 source port or even the src ip address of a tunnel.
The assumption is that monitoring is enabled everywhere so a missing field
can typically be seen at ingress to the next switch in the path.

This patch also adds unit tests to check the sFlow encoding of set(tunnel()),
tnl_push() and push_mpls() actions.

Signed-off-by: Neil McKee 
---
 lib/dpif-netlink.c|   2 +
 lib/dpif.h|   1 +
 ofproto/ofproto-dpif-sflow.c  | 574 +-
 ofproto/ofproto-dpif-sflow.h  |  30 ++-
 ofproto/ofproto-dpif-upcall.c |  34 ++-
 tests/ofproto-dpif.at | 264 ++-
 tests/test-sflow.c|  32 +++
 7 files changed, 921 insertions(+), 16 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 6838def..c36282c 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1969,6 +1969,7 @@ parse_odp_packet(const struct dpif_netlink *dpif, struct 
ofpbuf *buf,
 /* OVS_PACKET_CMD_ACTION only. */
 [OVS_PACKET_ATTR_USERDATA] = { .type = NL_A_UNSPEC, .optional = true },
 [OVS_PACKET_ATTR_EGRESS_TUN_KEY] = { .type = NL_A_NESTED, .optional = 
true },
+[OVS_PACKET_ATTR_ACTIONS] = { .type = NL_A_NESTED, .optional = true },
 };
 
 struct ovs_header *ovs_header;
@@ -2005,6 +2006,7 @@ parse_odp_packet(const struct dpif_netlink *dpif, struct 
ofpbuf *buf,
 dpif_flow_hash(&dpif->dpif, upcall->key, upcall->key_len, &upcall->ufid);
 upcall->userdata = a[OVS_PACKET_ATTR_USERDATA];
 upcall->out_tun_key = a[OVS_PACKET_ATTR_EGRESS_TUN_KEY];
+upcall->actions = a[OVS_PACKET_ATTR_ACTIONS];
 
 /* Allow overwriting the netlink attribute header without reallocating. */
 dp_packet_use_stub(&upcall->packet,
diff --git a/lib/dpif.h b/lib/dpif.h
index 06c6525..565d456 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -784,6 +784,7 @@ struct dpif_upcall {
 /* DPIF_UC_ACTION only. */
 struct nlattr *userdata;/* Argument to OVS_ACTION_ATTR_USERSPACE. */
 struct nlattr *out_tun_key;/* Output tunnel key. */
+struct nlattr *actions;/* Argument to OVS_ACTION_ATTR_USERSPACE. */
 };
 
 /* A callback to process an upcall, currently implemented only by dpif-netdev.
diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index b146f5d..45da789 100644
--- a/ofproto/ofproto-dpif-sflow.c
+++ b/ofproto/ofproto-dpif-sflow.c
@@ -40,6 +40,7 @@
 #include "timeval.h"
 #include "openvswitch/vlog.h"
 #include "lib/odp-util.h"
+#include "lib/unaligned.h"
 #include "ofproto-provider.h"
 #include "lacp.h"
 
@@ -52,11 +53,26 @@ static struct ovs_mutex mutex;
 #define SFLOW_GC_SUBID_UNCLAIMED (uint32_t)-1
 static uint32_t sflow_global_counters_subid = SFLOW_GC_SUBID_UNCLAIMED;
 
+/*
+ * The enum dpif_sflow_tunnel_type is to declare the types supported
+ */
+enum dpif_sflow_tunnel_type {
+DPIF_SFLOW_TUNNEL_UNKNOWN = 0,
+DPIF_SFLOW_TUNNEL_VXLAN,
+DPIF_SFLOW_TUNNEL_GRE,
+DPIF_SFLOW_TUNNEL_GRE64,
+DPIF_SFLOW_TUNNEL_LISP,
+DPIF_SFLOW_TUNNEL_IPSEC_GRE,
+DPIF_SFLOW_TUNNEL_IPSEC_GRE64,
+DPIF_SFLOW_TUNNEL_GENEVE
+};
+
 struct dpif_sflow_port {
 struct hmap_node hmap_node; /* In struct dpif_sflow's "ports" hmap. */
 SFLDataSource_instance dsi; /* sFlow library's notion of port number. */
 struct ofport *ofport;  /* To retrive port stats. */
 odp_port_t odp_port;
+enum dpif_sflow_tunnel_type tunnel_type;
 };
 
 struct dpif_sflow {
@@ -568,20 +584,75 @@ dpif_sflow_add_poller(struct dpif_sflow *ds, struct 
dpif_sflow_port *dsp)
 sfl_poller_set_bridgePort(poller, odp_to

[ovs-dev] [PATCHv2] extract-ofp-fields: Detect duplicate fields.

[Joe Stringer]

Figure out if a developer accidentally defines new NXM fields using an
existing number, and warn them. Useful particularly if new fields are
introduced upstream while rebasing an in-progress patchset.

Signed-off-by: Joe Stringer 
---
v2: Don't modify oxm_name_to_class().
---
 build-aux/extract-ofp-fields | 13 +
 1 file changed, 13 insertions(+)

diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index f05487e..315552d 100755
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -141,7 +141,12 @@ def parse_oxms(s, prefix, n_bytes):
 return tuple(parse_oxm(s2.strip(), prefix, n_bytes) for s2 in s.split(','))
 
 
+match_types = dict()
+
+
 def parse_oxm(s, prefix, n_bytes):
+global match_types
+
 m = re.match('([A-Z0-9_]+)\(([0-9]+)\) since(?: OF(1\.[0-9]+) and)? 
v([12]\.[0-9]+)$', s)
 if not m:
 fatal("%s: syntax error parsing %s" % (s, prefix))
@@ -153,6 +158,14 @@ def parse_oxm(s, prefix, n_bytes):
 fatal("unknown OXM class for %s" % name)
 oxm_vendor, oxm_class = class_
 
+if class_ in match_types:
+if oxm_type in match_types[class_]:
+fatal("duplicate match type for %s (conflicts with %s)" %
+  (name, match_types[class_][oxm_type]))
+else:
+match_types[class_] = dict()
+match_types[class_][oxm_type] = name
+
 # Normally the oxm_length is the size of the field, but for experimenter
 # OXMs oxm_length also includes the 4-byte experimenter ID.
 oxm_length = n_bytes
-- 
2.1.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovs-ofctl: Always prints recirc_id in decimal

[Jarno Rajahalme]

> On May 20, 2015, at 11:43 AM, Justin Pettit  wrote:
> 
> 
>> On May 20, 2015, at 11:33 AM, Andy Zhou  wrote:
>> 
>> Justin suggested changing to decimal. It seems reasonable to me as
>> well since we don't wildcard recirc_id at the moment as you have
>> pointed out.
>> 
>> In case we do wildcard recirc_id down the road, I'd lean towards your 
>> proposal.
> 
> If it's maskable, let's go with hex.  Changing the output later makes life 
> difficult for people who write tests that try to parse the output.
> 

This is what we have in lib/meta-flow.h:

/* "recirc_id".
 *
 * ID for recirculation.  The value 0 is reserved for initially received
 * packets.  Internal use only, not programmable from controller.
 *
 * Type: be32.
 * Maskable: no.
 * Formatting: decimal.
 * Prerequisites: none.
 * Access: read-only.
 * NXM: NXM_NX_RECIRC_ID(36) since v2.2.
 * OXM: none.
 */

The current strategy for recirc-id allocation is to get the next free one, 
which is essentially unsuitable for masking.

If we want to make a future change to maskable easier, then that is a different 
issue

  Jarno

> --Justin
> 
> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 16/19] ovs-ofctl: Add 'bundle' command and unit testing.

[Jarno Rajahalme]

Romain,

Thanks for the review!

I hope Ben finds time for reviewing the rest of the series sometime later.

  Jarno

> On May 20, 2015, at 11:26 AM, Romain Lenglet  
> wrote:
> 
> Jarno,
> Thanks a lot for this updated patch, this will be very useful.
> Sorry, I don’t feel comfortable reviewing the other patches.
> 
> Acked-by: Romain Lenglet  >
> --  
> Romain Lenglet  
> 
> On May 18, 2015 at 4:26:12 PM, Jarno Rajahalme (jrajaha...@nicira.com 
> (mailto:jrajaha...@nicira.com 
> )) wrote:
> 
>> The new ovs-ofctl 'bundle' command accepts files similar to
>> 'add-flows', but each line can optionally start with 'add', 'modify',
>> 'delete', 'modify_strict', or 'delete_strict' keyword, so that
>> arbitrary flow table modifications may be specified in a single file.
>> The modifications in the file are executed as a single transaction
>> using a OpenFlow 1.4 bundle.
>> 
>> Similarly, all existing ovs-ofctl flow mod commands now take an
>> optional '--bundle' argument, which executes the flow mods as a single
>> transaction.
>> 
>> OpenFlow 1.4 requires bundles to support at least flow and port mods.
>> This implementation does not yet support port mods in bundles.
>> 
>> Another restriction is that the atomic transactions are not yet
>> supported.
>> 
>> Signed-off-by: Jarno Rajahalme  
>> ---
>> NEWS | 14 ++-
>> include/openvswitch/vconn.h | 3 +
>> lib/ofp-parse.c | 40 ++-
>> lib/ofp-parse.h | 6 +-
>> lib/ofp-util.c | 30 ++
>> lib/ofp-util.h | 2 +
>> lib/ofp-version-opt.c | 7 ++
>> lib/ofp-version-opt.h | 1 +
>> lib/vconn.c | 236 +
>> tests/ofproto-macros.at  | 10 ++
>> tests/ofproto.at  | 244 
>> +++
>> tests/ovs-ofctl.at  | 107 +++
>> utilities/ovs-ofctl.8.in | 67 +---
>> utilities/ovs-ofctl.c | 110 ++-
>> 14 files changed, 834 insertions(+), 43 deletions(-)
>> 
>> diff --git a/NEWS b/NEWS
>> index a480607..ac60451 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -1,6 +1,6 @@
>> Post-v2.3.0
>> -
>> - - Added support for SFQ, FQ_CoDel and CoDel qdiscs.
>> + - Added support for SFQ, FQ_CoDel and CoDel qdiscs.
>> - Add bash command-line completion support for ovs-vsctl Please check
>> utilities/ovs-command-compgen.INSTALL.md for how to use.
>> - The MAC learning feature now includes per-port fairness to mitigate
>> @@ -27,6 +27,11 @@ Post-v2.3.0
>> commands are now redundant and will be removed in a future
>> release. See ovs-vswitchd(8) for details.
>> - OpenFlow:
>> + * OpenFlow 1.4 bundles are now supported, but for flow mod
>> + messages only. 'atomic' bundles are not yet supported, and
>> + 'ordered' bundles are trivially supported, as all bundled
>> + messages are executed in the order they were added to the
>> + bundle regardless of the presence of the 'ordered' flag.
>> * IPv6 flow label and neighbor discovery fields are now modifiable.
>> * OpenFlow 1.5 extended registers are now supported.
>> * The OpenFlow 1.5 actset_output field is now supported.
>> @@ -41,6 +46,13 @@ Post-v2.3.0
>> * A new Netronome extension to OpenFlow 1.5+ allows control over the
>> fields hashed for OpenFlow select groups. See "selection_method" and
>> related options in ovs-ofctl(8) for details.
>> + - ovs-ofctl has a new 'bundle' command that accepts a file of
>> + arbitrary flow mods as an input that are executed as a single
>> + transaction using the new OpenFlow 1.4 bundles support.
>> + - ovs-ofctl has a new '--bundle' option that makes the flow mod
>> + commands (add-flow, add-flows, mod-flows, del-flows, and
>> + replace-flows) use an OpenFlow 1.4 bundle to operate the
>> + modifications as a single transaction.
>> - ovs-pki: Changed message digest algorithm from MD5 to SHA-1 because
>> MD5 is no longer secure and some operating systems have started to disable
>> it in OpenSSL.
>> diff --git a/include/openvswitch/vconn.h b/include/openvswitch/vconn.h
>> index 3b157e1..f8b6655 100644
>> --- a/include/openvswitch/vconn.h
>> +++ b/include/openvswitch/vconn.h
>> @@ -55,6 +55,9 @@ int vconn_transact(struct vconn *, struct ofpbuf *, struct 
>> ofpbuf **);
>> int vconn_transact_noreply(struct vconn *, struct ofpbuf *, struct ofpbuf 
>> **);
>> int vconn_transact_multiple_noreply(struct vconn *, struct ovs_list 
>> *requests,
>> struct ofpbuf **replyp);
>> +int vconn_bundle_transact(struct vconn *, struct ovs_list *requests,
>> + uint16_t bundle_flags,
>> + void (*error_reporter)(const struct ofp_header *));
>> 
>> void vconn_run(struct vconn *);
>> void vconn_run_wait(struct vconn *);
>> diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
>> index 126980c..3ed9937 100644
>> --- a/lib/ofp-parse.c
>> +++ b/lib/ofp-parse.c
>> @@ -257,6 +257,29 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int 
>> command, char *string,
>> 
>> *usable

Re: [ovs-dev] ovs-style bridge creation from an external application

[Ben Pfaff]

On Wed, May 20, 2015 at 12:02:03PM -0700, Karol Mroz wrote:
> We'd like to integrate the creation and configuration of OVS-style
> bridges into our network configuration application, called Wicked.
> 
> We currently have a workaround solution for this, which relies on using
> `ovs-vsctl` to create the OVS bridge and add any bridge ports. We then
> create the appropriate configuration files (essentially treating the
> OVS bridge as a basic ethernet device) and proceed with address/route
> configuration. Obviously, this is not ideal.
> 
> Is there a way to better interface with OVS to automate the creation of
> OVS-style bridges (APIs, dbus, etc)? Any pointers would be helpful.

You can use the OVSDB protocol, defined in RFC 7047, to manipulate OVS
configuration.  OVS includes bindings for C (used by ovs-vsctl) and
Python.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] dpif-netdev: Clear flow batches before execute.

[Ethan Jackson]

When executing actions, it's possible a recirculation will occur
causing dp_netdev_input() to be called multiple times.  If the batch
pointers embedded in dp_netdev_flow aren't cleared, it's possible
packets after the recirculation will be reinserted into a batch
associated with the original lookup.  This could be very bad.

This patch fixes the problem by zeroing out flow batch pointers before
calling packet_batch_execute().  This probably has a slightly negative
performance impact, though I haven't tried it.

Signed-off-by: Ethan Jackson 
---
 lib/dpif-netdev.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index fb01a02..bbd0fa4 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3066,7 +3066,6 @@ packet_batch_execute(struct packet_batch *batch,
 struct dp_netdev_actions *actions;
 struct dp_netdev_flow *flow = batch->flow;
 
-flow->batch = NULL;
 dp_netdev_flow_used(flow, batch->packet_count, batch->byte_count,
 batch->tcp_flags, now);
 
@@ -3299,6 +3298,10 @@ dp_netdev_input(struct dp_netdev_pmd_thread *pmd,
 }
 
 for (i = 0; i < n_batches; i++) {
+batches[i].flow->batch = NULL;
+}
+
+for (i = 0; i < n_batches; i++) {
 packet_batch_execute(&batches[i], pmd, now);
 }
 }
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] NSH

[Anil Gunturu]

Hi,
Are there any plans to merge the patch for supporting NSH header into the main 
stream?
Thanks,
-Anil
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] NSH

[Ben Pfaff]

On Thu, May 21, 2015 at 02:06:57AM +, Anil Gunturu wrote:
> Are there any plans to merge the patch for supporting NSH header into
> the main stream?

I think we're still waiting for revised patches.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] NSH

[Pritesh Kothari (pritkoth)]

> On May 20, 2015, at 10:47 PM, Ben Pfaff  wrote:
> 
> On Thu, May 21, 2015 at 02:06:57AM +, Anil Gunturu wrote:
>> Are there any plans to merge the patch for supporting NSH header into
>> the main stream?
> 
> I think we're still waiting for revised patches.

yep, still working on it, mean while an older patch-set if needed:

https://github.com/priteshk/ovs/branches/active

Regards,
Pritesh
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev