Hi Sam, Looks good. Some comments: Shouldn't we check the return code from NlMsgPutTailU32() ? Do we check the size of the out message before we populate it with the port NL attributes? Thanks, Eitan
-----Original Message----- From: Samuel Ghinet [mailto:sghi...@cloudbasesolutions.com] Sent: Wednesday, September 17, 2014 6:09 AM To: dev@openvswitch.org Cc: Nithin Raju; Eitan Eliahu; Saurabh Shah; Ankur Sharma; Alin Serdean Subject: [PATCH v2] datapath-windows: Netlink command: vport dump Functionality for vport dump. Later, when we will add more netlink dump commands, some common code will need to be split to functions. Notes: a) the current implementation of vport assumes the datapath feature "multiple upcall pids" is not used. A single upcall pid is used instead. c) the vxlan destination udp port is currently a constant. When it will become configurable, the vport options netlink attribute will become relevant. Signed-off-by: Samuel Ghinet <sghi...@cloudbasesolutions.com> --- datapath-windows/ovsext/Datapath.c | 222 ++++++++++++++++++++++++++++++++++++- datapath-windows/ovsext/Vport.h | 11 +- 2 files changed, 227 insertions(+), 6 deletions(-) diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c index d90ae23..5e9aadf 100644 --- a/datapath-windows/ovsext/Datapath.c +++ b/datapath-windows/ovsext/Datapath.c @@ -101,6 +101,9 @@ static NTSTATUS OvsGetPidCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, static NTSTATUS OvsGetDpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, UINT32 *replyLen); +static NTSTATUS OvsGetVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, + UINT32 *replyLen); + /* * The various netlink families, along with the supported commands. Most of * these families and commands are part of the openvswitch specification for a @@ -156,14 +159,20 @@ NETLINK_FAMILY nlPacketFamilyOps = { }; /* Netlink vport family. */ +NETLINK_CMD nlVportFamilyCmdOps[] = { + { OVS_VPORT_CMD_GET, OvsGetVportCmdHandler, + OVS_WRITE_DEV_OP | OVS_READ_DEV_OP, FALSE } }; + +/* Netlink vport family. */ /* XXX: Add commands here. */ NETLINK_FAMILY nlVportFamilyOps = { .name = OVS_VPORT_FAMILY, .id = OVS_WIN_NL_VPORT_FAMILY_ID, .version = OVS_VPORT_VERSION, .maxAttr = OVS_VPORT_ATTR_MAX, - .cmds = NULL, /* XXX: placeholder. */ - .opsCount = 0 + .cmds = nlVportFamilyCmdOps, + .opsCount = ARRAY_SIZE(nlVportFamilyCmdOps) }; /* Netlink flow family. */ @@ -668,10 +677,11 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject, break; case OVS_WIN_NL_PACKET_FAMILY_ID: case OVS_WIN_NL_FLOW_FAMILY_ID: - case OVS_WIN_NL_VPORT_FAMILY_ID: status = STATUS_NOT_IMPLEMENTED; goto done; - + case OVS_WIN_NL_VPORT_FAMILY_ID: + nlFamilyOps = &nlVportFamilyOps; + break; default: status = STATUS_INVALID_PARAMETER; goto done; @@ -966,6 +976,210 @@ OvsSetupDumpStart(POVS_USER_PARAMS_CONTEXT usrParamsCtx) } /* +* XXX: When "dump done" and "error" netlink types will be implemented, +we will +* need a few more BuildMsgOut functions, which would ultimately call a +generic +* BuildMsgOutFromMsgIn. +*/ + +static VOID +BuildMsgOutFromMsgIn(POVS_MESSAGE msgIn, POVS_MESSAGE msgOut, UINT16 +flags) { + msgOut->nlMsg.nlmsgType = msgIn->nlMsg.nlmsgType; + msgOut->nlMsg.nlmsgFlags = flags; + msgOut->nlMsg.nlmsgSeq = msgIn->nlMsg.nlmsgSeq; + msgOut->nlMsg.nlmsgPid = msgIn->nlMsg.nlmsgPid; + msgOut->nlMsg.nlmsgLen = sizeof *msgOut; + + msgOut->genlMsg.cmd = msgIn->genlMsg.cmd; + msgOut->genlMsg.version = nlDatapathFamilyOps.version; + msgOut->genlMsg.reserved = 0; +} + +static NTSTATUS +OvsCreateMsgFromVport(POVS_VPORT_ENTRY vport, + POVS_MESSAGE msgOut, + UINT32 maxPayloadSize) { + NL_BUFFER nlBuffer; + OVS_VPORT_FULL_STATS vportStats; + + NlBufInit(&nlBuffer, (PCHAR)msgOut + sizeof (*msgOut), + maxPayloadSize); + + NlMsgPutTailU32(&nlBuffer, OVS_VPORT_ATTR_PORT_NO, vport->portNo); + NlMsgPutTailU32(&nlBuffer, OVS_VPORT_ATTR_TYPE, vport->ovsType); + + NlMsgPutTailString(&nlBuffer, OVS_VPORT_ATTR_NAME, vport->ovsName); + + /* + * XXX: when we implement OVS_DP_ATTR_USER_FEATURES in datapath, + * we'll need to check the OVS_DP_F_VPORT_PIDS flag: if it is set, + * it means we have an array of pids, instead of a single pid. + * ATM we assume we have one pid only. + */ + + NlMsgPutTailU32(&nlBuffer, OVS_VPORT_ATTR_UPCALL_PID, + vport->upcallPid); + + /*stats*/ + vportStats.rxPackets = vport->stats.rxPackets; + vportStats.rxBytes = vport->stats.rxBytes; + vportStats.txPackets = vport->stats.txPackets; + vportStats.txBytes = vport->stats.txBytes; + vportStats.rxErrors = vport->errStats.rxErrors; + vportStats.txErrors = vport->errStats.txErrors; + vportStats.rxDropped = vport->errStats.rxDropped; + vportStats.txDropped = vport->errStats.txDropped; + + NlMsgPutTailUnspec(&nlBuffer, OVS_VPORT_ATTR_STATS, (PCHAR)&vportStats, + sizeof(OVS_VPORT_FULL_STATS)); + + /* + * XXX: when vxlan udp dest port becomes configurable, we will also need + * to add vport options + */ + + msgOut->nlMsg.nlmsgLen = NlBufSize(&nlBuffer) + + sizeof(OVS_MESSAGE); + + return STATUS_SUCCESS; +} + +static NTSTATUS +OvsGetVportDumpNext(POVS_USER_PARAMS_CONTEXT usrParamsCtx, + UINT32 *replyLen) +{ + POVS_MESSAGE msgIn; + POVS_MESSAGE msgOut = (POVS_MESSAGE)usrParamsCtx->outputBuffer; + POVS_OPEN_INSTANCE instance = + (POVS_OPEN_INSTANCE)usrParamsCtx->ovsInstance; + LOCK_STATE_EX lockState; + UINT32 i = 0; + + /* + * XXX: this function shares some code with other dump command(s). + * In the future, we will need to refactor the dump functions + */ + + ASSERT(usrParamsCtx->devOp == OVS_READ_DEV_OP); + + if (instance->dumpState.ovsMsg == NULL) { + ASSERT(FALSE); + return STATUS_INVALID_DEVICE_STATE; + } + + /* Output buffer has been validated while validating read dev op. */ + ASSERT(msgOut != NULL && usrParamsCtx->outputLength >= sizeof + *msgOut); + + msgIn = instance->dumpState.ovsMsg; + BuildMsgOutFromMsgIn(msgIn, msgOut, NLM_F_MULTI); + + OvsAcquireCtrlLock(); + if (!gOvsSwitchContext) { + /* Treat this as a dump done. */ + OvsReleaseCtrlLock(); + *replyLen = 0; + FreeUserDumpState(instance); + return STATUS_SUCCESS; + } + + msgOut->ovsHdr.dp_ifindex = gOvsSwitchContext->dpNo; + + /* + * XXX: when we implement OVS_DP_ATTR_USER_FEATURES in datapath, + * we'll need to check the OVS_DP_F_VPORT_PIDS flag: if it is set, + * it means we have an array of pids, instead of a single pid. + * ATM we assume we have one pid only. + */ + + NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState, + 0); + + if (gOvsSwitchContext->numVports > 0) { + /* inBucket: the bucket, used for lookup */ + UINT32 inBucket = instance->dumpState.index[0]; + /* inIndex: index within the given bucket, used for lookup */ + UINT32 inIndex = instance->dumpState.index[1]; + /* the bucket to be used for the next dump operation */ + UINT32 outBucket = 0; + /* the index within the outBucket to be used for the next dump */ + UINT32 outIndex = 0; + UINT32 maxSize = usrParamsCtx->outputLength; + + for (i = inBucket; i < OVS_MAX_VPORT_ARRAY_SIZE; i++) { + PLIST_ENTRY head, link; + head = &(gOvsSwitchContext->portHashArray[i]); + POVS_VPORT_ENTRY vport = NULL; + + outIndex = 0; + LIST_FORALL(head, link) { + + /* + * if one or more dumps were previously done on this same bucket, + * inIndex will be > 0, so we'll need to reply with the + * inIndex + 1 vport from the bucket. + */ + if (outIndex >= inIndex) { + vport = CONTAINING_RECORD(link, OVS_VPORT_ENTRY, portLink); + OvsCreateMsgFromVport(vport, msgOut, maxSize); + ++outIndex; + break; + } + + ++outIndex; + } + + if (vport) { + break; + } + + /* + * if no vport was found above, check the next bucket, beginning + * with the first (i.e. index 0) elem from within that bucket + */ + inIndex = 0; + } + + outBucket = i; + + /* XXX: what about NLMSG_DONE (as msg type)? */ + instance->dumpState.index[0] = outBucket; + instance->dumpState.index[1] = outIndex; + } + + NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState); + + OvsReleaseCtrlLock(); + + *replyLen = msgOut->nlMsg.nlmsgLen; + + /* if i is OVS_MAX_VPORT_ARRAY_SIZE => no vport was found => dump done*/ + if (i == OVS_MAX_VPORT_ARRAY_SIZE) { + *replyLen = 0; + /* Free up the dump state, since there's no more data to continue. */ + FreeUserDumpState(instance); + } + + return STATUS_SUCCESS; +} + +/* +* +----------------------------------------------------------------------- +--- +* Handler for the get vport command. The function handles the initial +call to +* setup the dump state, as well as subsequent calls to continue dumping data. +* +----------------------------------------------------------------------- +--- +*/ +static NTSTATUS +OvsGetVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, + UINT32 *replyLen) { + if (usrParamsCtx->devOp == OVS_WRITE_DEV_OP) { + *replyLen = 0; + OvsSetupDumpStart(usrParamsCtx); + } else { + return OvsGetVportDumpNext(usrParamsCtx, replyLen); + } + + return STATUS_SUCCESS; +} + +/* * -------------------------------------------------------------------------- * Utility function to map the output buffer in an IRP. The buffer is assumed * to have been passed down using METHOD_OUT_DIRECT (Direct I/O). diff --git a/datapath-windows/ovsext/Vport.h b/datapath-windows/ovsext/Vport.h index 80bdea8..0305d35 100644 --- a/datapath-windows/ovsext/Vport.h +++ b/datapath-windows/ovsext/Vport.h @@ -39,10 +39,10 @@ typedef enum { } OVS_VPORT_STATE; typedef struct _OVS_VPORT_STATS { - UINT64 rxBytes; UINT64 rxPackets; - UINT64 txBytes; UINT64 txPackets; + UINT64 rxBytes; + UINT64 txBytes; } OVS_VPORT_STATS; typedef struct _OVS_VPORT_ERR_STATS { @@ -51,6 +51,12 @@ typedef struct _OVS_VPORT_ERR_STATS { UINT64 rxDropped; UINT64 txDropped; } OVS_VPORT_ERR_STATS; + +/* used for vport netlink commands. */ +typedef struct _OVS_VPORT_FULL_STATS { + OVS_VPORT_STATS; + OVS_VPORT_ERR_STATS; +}OVS_VPORT_FULL_STATS; /* * Each internal, external adapter or vritual adapter has * one vport entry. In addition, we have one vport for each @@ -86,6 +92,7 @@ typedef struct _OVS_VPORT_ENTRY { NDIS_SWITCH_NIC_NAME nicName; NDIS_VM_NAME vmName; GUID netCfgInstanceId; + UINT32 upcallPid; /* netlink upcall port id */ } OVS_VPORT_ENTRY, *POVS_VPORT_ENTRY; struct _OVS_SWITCH_CONTEXT; -- 1.8.3.msysgit.0 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
