| Message ID | 20160713233838.47648-9-vsairam@vmware.com |
|---|---|
| State | Superseded |
| Delegated to: | Guru Shetty |
| Headers | show |
Besides some alignment issues, looks god. Acked-by: Paul Boca <pboca@cloudbasesolutions.com> > -----Original Message----- > From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Sairam > Venugopal > Sent: Thursday, July 14, 2016 2:39 AM > To: dev@openvswitch.org > Subject: [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 <vsairam@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); [Paul Boca] This exceeds the 80 columns limit > > - 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, > + usrParamsCtx->ovsInstance->pid, > + NFNETLINK_V0, > + 0); > + if (status == NDIS_STATUS_SUCCESS) { > + *replyLen = msgOut->nlMsg.nlmsgLen; > + } > + } else if (instance->protocol == NETLINK_GENERIC) { > + NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, usrParamsCtx- > >outputLength); [Paul Boca] Same > + > + OVS_VPORT_EVENT_ENTRY eventEntry; > + /* remove vport event entry from the vport event queue */ > + status = OvsRemoveVportEventEntry(usrParamsCtx->ovsInstance, > &eventEntry); [Paul Boca] This exceeds the 80 columns limit also > + 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 > http://openvswitch.org/mailman/listinfo/dev
Just one comment inlined. > -----Mesaj original----- > De la: dev [mailto:dev-bounces@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 <vsairam@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 > http://openvswitch.org/mailman/listinfo/dev
Will address this in v2. Thanks, Sairam On 7/22/16, 6:20 PM, "Alin Serdean" <aserdean@cloudbasesolutions.com> wrote: >Just one comment inlined. > > > >> -----Mesaj original----- > >> De la: dev [mailto:dev-bounces@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 <vsairam@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= >
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, + usrParamsCtx->ovsInstance->pid, + NFNETLINK_V0, + 0); + 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:
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 <vsairam@vmware.com> --- datapath-windows/ovsext/Datapath.c | 59 +++++++++++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 14 deletions(-)