Will address this in v2. Thanks, Sairam
On 7/22/16, 6:20 PM, "Alin Serdean" <aserd...@cloudbasesolutions.com> wrote: >Just one comment inlined. > > > >> -----Mesaj original----- > >> De la: dev [mailto:dev-boun...@openvswitch.org] În numele Sairam > >> Venugopal > >> Trimis: Thursday, July 14, 2016 2:39 AM > >> Către: dev@openvswitch.org > >> Subiect: [ovs-dev] [PATCH 8/9] datapath-windows: Update > >> OvsReadEventCmdHandler in Datapath.c to support different events > >> > >> OvsReadEventCmdHandler must now reflect the right event being read. If > >> the event is a Conntrack related event, then convert the entry to >>netlink > >> format and send it to userspace. If it's Vport event, retain the >>existing > >> workflow. > >> > >> Signed-off-by: Sairam Venugopal <vsai...@vmware.com> > >> --- > >> datapath-windows/ovsext/Datapath.c | 59 > >> +++++++++++++++++++++++++++++--------- > >> 1 file changed, 45 insertions(+), 14 deletions(-) > >> > >> diff --git a/datapath-windows/ovsext/Datapath.c b/datapath- > >> windows/ovsext/Datapath.c > >> index a5a0b35..fff788a 100644 > >> --- a/datapath-windows/ovsext/Datapath.c > >> +++ b/datapath-windows/ovsext/Datapath.c > >> @@ -1674,7 +1674,6 @@ > >> OvsReadEventCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, > >> #endif > >> NL_BUFFER nlBuf; > >> NTSTATUS status; > >> - OVS_VPORT_EVENT_ENTRY eventEntry; > >> > >> ASSERT(usrParamsCtx->devOp == OVS_READ_DEV_OP); > >> > >> @@ -1687,21 +1686,53 @@ > >> OvsReadEventCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, > >> /* Output buffer has been validated while validating read dev op. >>*/ > >> ASSERT(msgOut != NULL && usrParamsCtx->outputLength >= sizeof > >> *msgOut); > >> > >> - NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, usrParamsCtx- > >> >outputLength); > >> + if (instance->protocol == NETLINK_NETFILTER) { > >> + if (!instance->mcastMask) { > >> + status = STATUS_SUCCESS; > >> + *replyLen = 0; > >> + goto cleanup; > >> + } > >> > >> - /* remove an event entry from the event queue */ > >> - status = OvsRemoveVportEventEntry(usrParamsCtx->ovsInstance, > >> - &eventEntry); > >> - if (status != STATUS_SUCCESS) { > >> - /* If there were not elements, read should return no data. */ > >> - status = STATUS_SUCCESS; > >> - *replyLen = 0; > >> - goto cleanup; > >> - } > >> + OVS_CT_EVENT_ENTRY ctEventEntry; > >> + status = OvsRemoveCtEventEntry(usrParamsCtx->ovsInstance, > >> + &ctEventEntry); > >> > >> - status = OvsPortFillInfo(usrParamsCtx, &eventEntry, &nlBuf); > >> - if (status == NDIS_STATUS_SUCCESS) { > >> - *replyLen = NlBufSize(&nlBuf); > >> + if (status != STATUS_SUCCESS) { > >> + /* If there were not elements, read should return no data. >>*/ > >> + status = STATUS_SUCCESS; > >> + *replyLen = 0; > >> + goto cleanup; > >> + } > >> + > >> + status = OvsCreateNlMsgFromCtEntry(&ctEventEntry.entry, > >> + usrParamsCtx->outputBuffer, > >> + usrParamsCtx->outputLength, > >> + ctEventEntry.type, > >> + 0, > >[Alin Gabriel Serdean: ] Why hard zero for the sequence instead of using >the input > >> + >>usrParamsCtx->ovsInstance->pid, > >> + NFNETLINK_V0, > >> + 0); > >[Alin Gabriel Serdean: ] Again why hard use of constants for the reply >message? > >> + if (status == NDIS_STATUS_SUCCESS) { > >> + *replyLen = msgOut->nlMsg.nlmsgLen; > >> + } > >> + } else if (instance->protocol == NETLINK_GENERIC) { > >> + NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, > >> + usrParamsCtx->outputLength); > >> + > >> + OVS_VPORT_EVENT_ENTRY eventEntry; > >> + /* remove vport event entry from the vport event queue */ > >> + status = OvsRemoveVportEventEntry(usrParamsCtx->ovsInstance, > >> &eventEntry); > >> + if (status != STATUS_SUCCESS) { > >> + /* If there were not elements, read should return no data. >>*/ > >> + status = STATUS_SUCCESS; > >> + *replyLen = 0; > >> + goto cleanup; > >> + } > >> + > >> + status = OvsPortFillInfo(usrParamsCtx, &eventEntry, &nlBuf); > >> + if (status == NDIS_STATUS_SUCCESS) { > >> + *replyLen = NlBufSize(&nlBuf); > >> + } > >> + } else { > >> + status = STATUS_INVALID_PARAMETER; > >> } > >> > >> cleanup: > >> -- > >> 2.9.0.windows.1 > >> > >> _______________________________________________ > >> dev mailing list > >> dev@openvswitch.org > >> >>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailm >>an_listinfo_dev&d=CwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r= >>Dcruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=FkOeG1F2duk7N91KHAhL4SgIRq8 >>qCjWSob7BcaMNStc&s=04CxzZ4GI0h9zcAgSPwEL4_3Fk9hidKntaAu_eiZ2E0&e= > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev