Hi Nithin, " devOp should be set to OVS_READ_EVENT_DEV_OP or OVS_READ_PACKET_DEV_OP" Do you see any purpose to set the devOp for Event Read and Packet Read to a value other than other than OVS_READ_DEV_OP ? Thanks, Eitan
-----Original Message----- From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Eitan Eliahu Sent: Wednesday, October 15, 2014 3:13 PM To: Nithin Raju Cc: <dev@openvswitch.org> Subject: Re: [ovs-dev] [PATCH] datapath-windows: packet miss read NL command Thanks Nithin. I am able to complete OvsReadDpIoctl now as my previous patch was applied. -----Original Message----- From: Nithin Raju Sent: Wednesday, October 15, 2014 3:08 PM To: Eitan Eliahu Cc: <dev@openvswitch.org> Subject: Re: [ovs-dev] [PATCH] datapath-windows: packet miss read NL command hi Eitan, I think there are additional changes needed in OvsReadDpIoctl() for checksum calculation. It might be easier to explain it in a patch: diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c index e27bd76..9d15ef9 100644 --- a/datapath-windows/ovsext/User.c +++ b/datapath-windows/ovsext/User.c @@ -223,13 +223,12 @@ OvsReadDpIoctl(PFILE_OBJECT fileObject, if ((elem->hdrInfo.tcpCsumNeeded || elem->hdrInfo.udpCsumNeeded) && len == elem->packet.totalLen) { UINT16 sum, *ptr; - UINT16 size = (UINT16)(elem->packet.userDataLen + - elem->hdrInfo.l4Offset + - (UINT16)sizeof (OVS_PACKET_INFO)); - RtlCopyMemory(outputBuffer, &elem->packet, size); - ASSERT(len - size >= elem->hdrInfo.l4PayLoad); + UINT16 size = (UINT16)(elem->packet.payload - elem->packet.data + + elem->hdrInfo.l4Offset); + RtlCopyMemory(outputBuffer, &elem->packet.data, size); + ASSERT(len - size >= elem->hdrInfo.l4PayLoad); sum = CopyAndCalculateChecksum((UINT8 *)outputBuffer + size, - (UINT8 *)&elem->packet + size, + (UINT8 *)&elem->packet.data + + size, elem->hdrInfo.l4PayLoad, 0); ptr =(UINT16 *)((UINT8 *)outputBuffer + size + (elem->hdrInfo.tcpCsumNeeded ? I had a few other minor comments. On Oct 14, 2014, at 9:55 PM, Eitan Eliahu <elia...@vmware.com> wrote: > The change include the Packet Read handler. > The current implementation reads once packet at a time. This should be > updated once user mode code is in place. Also, the packet offset > should be updated to reflect the new NL format. > > signed-off-by: Eitan Eliahu <elia...@vmware.com> > --- > datapath-windows/include/OvsDpInterfaceExt.h | 3 +- > datapath-windows/ovsext/Datapath.c | 53 +++++++++++++++++++++++++++- > 2 files changed, 54 insertions(+), 2 deletions(-) > > diff --git a/datapath-windows/include/OvsDpInterfaceExt.h > b/datapath-windows/include/OvsDpInterfaceExt.h > index e9faecc..d357a16 100644 > --- a/datapath-windows/include/OvsDpInterfaceExt.h > +++ b/datapath-windows/include/OvsDpInterfaceExt.h > @@ -80,7 +80,8 @@ enum ovs_win_control_cmd { > OVS_CTRL_CMD_MC_SUBSCRIBE_REQ, > > /* This command is logically belong to the Vport family */ > - OVS_CTRL_CMD_EVENT_NOTIFY > + OVS_CTRL_CMD_EVENT_NOTIFY, > + OVS_CTRL_CMD_READ_NOTIFY > }; > > /* NL Attributes for joining/unjoining an MC group */ diff --git > a/datapath-windows/ovsext/Datapath.c > b/datapath-windows/ovsext/Datapath.c > index 6c78ab8..06b3e59 100644 > --- a/datapath-windows/ovsext/Datapath.c > +++ b/datapath-windows/ovsext/Datapath.c > @@ -91,6 +91,7 @@ static NetlinkCmdHandler OvsGetPidCmdHandler, > OvsPendEventCmdHandler, > OvsSubscribeEventCmdHandler, > OvsReadEventCmdHandler, > + OvsReadPacketCmdHandler, > OvsNewDpCmdHandler, > OvsGetDpCmdHandler, > OvsSetDpCmdHandler, @@ -135,6 +136,11 @@ > NETLINK_CMD nlControlFamilyCmdOps[] = { > .handler = OvsReadEventCmdHandler, > .supportedDevOp = OVS_READ_EVENT_DEV_OP, > .validateDpIndex = FALSE, > + }, > + { .cmd = OVS_CTRL_CMD_READ_NOTIFY, > + .handler = OvsReadPacketCmdHandler, > + .supportedDevOp = OVS_READ_EVENT_DEV_OP, .supportedDevOp should be OVS_READ_PACKET_DEV_OP. > + .validateDpIndex = FALSE, > } > }; > > @@ -690,6 +696,7 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject, > break; > > case OVS_IOCTL_READ_EVENT: > + case OVS_IOCTL_READ_PACKET: > /* This IOCTL is used to read events */ > if (outputBufferLen != 0) { > status = MapIrpOutputBuffer(irp, outputBufferLen, @@ > -708,7 +715,9 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject, > ovsMsg = &ovsMsgReadOp; > ovsMsg->nlMsg.nlmsgType = OVS_WIN_NL_CTRL_FAMILY_ID; > /* An "artificial" command so we can use NL family function table*/ > - ovsMsg->genlMsg.cmd = OVS_CTRL_CMD_EVENT_NOTIFY; > + ovsMsg->genlMsg.cmd = (code == OVS_IOCTL_READ_EVENT) ? > + OVS_CTRL_CMD_EVENT_NOTIFY : > + OVS_CTRL_CMD_READ_NOTIFY; > devOp = OVS_READ_DEV_OP; devOp should be set to OVS_READ_EVENT_DEV_OP or OVS_READ_PACKET_DEV_OP. > break; > > @@ -2152,4 +2161,46 @@ cleanup: > OvsReleaseCtrlLock(); > return status; > } > + > +/* > + * > +--------------------------------------------------------------------- > +----- > + * Handler for reading missed pacckets from the driver event queue. > +This > + * handler is executed when user modes issues a socket receive on a > +socket > + * > +--------------------------------------------------------------------- > +----- > + */ > +static NTSTATUS > +OvsReadPacketCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, > + UINT32 *replyLen) { #ifdef DBG > + POVS_MESSAGE msgOut = (POVS_MESSAGE)usrParamsCtx->outputBuffer; > +#endif > + POVS_OPEN_INSTANCE instance = > + (POVS_OPEN_INSTANCE)usrParamsCtx->ovsInstance; > + NTSTATUS status; > + > + ASSERT(usrParamsCtx->devOp == OVS_READ_DEV_OP); > + > + /* Should never read events with a dump socket */ > + ASSERT(instance->dumpState.ovsMsg == NULL); > + > + /* Must have an packet queue */ > + ASSERT(instance->packetQueue != NULL); > + > + /* Output buffer has been validated while validating read dev op. */ > + ASSERT(msgOut != NULL && usrParamsCtx->outputLength >= sizeof > + *msgOut); > + > + OvsAcquireCtrlLock(); > + if (!gOvsSwitchContext) { > + status = STATUS_SUCCESS; > + OvsReleaseCtrlLock(); > + return status; > + } > + OvsReleaseCtrlLock(); > + > + /* Read a packet from the instance queue */ > + status = OvsReadDpIoctl(instance->fileObject, usrParamsCtx->outputBuffer, > + usrParamsCtx->outputLength, replyLen); > + return status; > +} > #endif /* OVS_USE_NL_INTERFACE */ thanks, -- Nithin _______________________________________________ dev mailing list dev@openvswitch.org https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=yTvML8OxA42Jb6ViHe7fUXbvPVOYDPVq87w43doxtlY%3D%0A&m=L4z99ImIcqRDFzG%2FbzFOcbISCDh8w5%2B9yig%2Fj4LV1%2F4%3D%0A&s=fbd0ef5a230bebeb3ffeaa1ca060d59bf1ca1e1eb28e05c91531a8acfa5b05fe _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
