On 21 June 2017 at 10:08, Sairam Venugopal <[email protected]> wrote:
> OvsCtAddEntry returns TRUE or FALSE depending on whether > OvsNatTranslateCtEntry was successful or not. In the case of an > unsuccesful NAT translation, this will fail to insert the newly created > entry to the Conntrack Table. This entry needs to be freed and the states > should be accordingly in the flowKey instead of returning out. > > Consolidated the parentEntry lookup and assignment portion across > different protocols and some minor refactoring to make the code more > readable. > > Tests Done: Enabled driver verifier and tested the following: > - TCP & ICMP traffic through Conntrack Module. > - Flushed Conntrack Entries while traffic was flowing. > - Uninstalled and re-installed the driver when traffic was in progress. > > Signed-off-by: Sairam Venugopal <[email protected]> > Applied, thanks! > --- > datapath-windows/ovsext/Conntrack.c | 123 +++++++++++++++++------------- > ------ > 1 file changed, 57 insertions(+), 66 deletions(-) > > diff --git a/datapath-windows/ovsext/Conntrack.c > b/datapath-windows/ovsext/Conntrack.c > index 68ed395..b010484 100644 > --- a/datapath-windows/ovsext/Conntrack.c > +++ b/datapath-windows/ovsext/Conntrack.c > @@ -214,87 +214,78 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx, > BOOLEAN *entryCreated) > { > POVS_CT_ENTRY entry = NULL; > - *entryCreated = FALSE; > UINT32 state = 0; > + POVS_CT_ENTRY parentEntry; > PNET_BUFFER_LIST curNbl = fwdCtx->curNbl; > - switch (ipProto) > - { > - case IPPROTO_TCP: > - { > - TCPHdr tcpStorage; > - const TCPHdr *tcp; > - tcp = OvsGetTcp(curNbl, l4Offset, &tcpStorage); > - if (!OvsConntrackValidateTcpPacket(tcp)) { > - goto invalid; > - } > - > - state |= OVS_CS_F_NEW; > - POVS_CT_ENTRY parentEntry; > - parentEntry = OvsCtRelatedLookup(ctx->key, currentTime); > - if (parentEntry != NULL) { > - state |= OVS_CS_F_RELATED; > - } > > - if (commit) { > - entry = OvsConntrackCreateTcpEntry(tcp, curNbl, > currentTime); > - if (!entry) { > - return NULL; > - } > + *entryCreated = FALSE; > + state |= OVS_CS_F_NEW; > > - /* Set parent entry for related FTP connections */ > - entry->parent = parentEntry; > + parentEntry = OvsCtRelatedLookup(ctx->key, currentTime); > + if (parentEntry != NULL) { > + state |= OVS_CS_F_RELATED; > + } > > - *entryCreated = TRUE; > - } > + switch (ipProto) { > + case IPPROTO_TCP: > + { > + TCPHdr tcpStorage; > + const TCPHdr *tcp; > + tcp = OvsGetTcp(curNbl, l4Offset, &tcpStorage); > + if (!OvsConntrackValidateTcpPacket(tcp)) { > + state = OVS_CS_F_INVALID; > break; > } > - case IPPROTO_ICMP: > - { > - ICMPHdr storage; > - const ICMPHdr *icmp; > - icmp = OvsGetIcmp(curNbl, l4Offset, &storage); > - if (!OvsConntrackValidateIcmpPacket(icmp)) { > - goto invalid; > - } > > - state |= OVS_CS_F_NEW; > - if (commit) { > - entry = OvsConntrackCreateIcmpEntry(currentTime); > - if (entry) { > - /* XXX Add support for ICMP-Related */ > - entry->parent = NULL; > - } > - *entryCreated = TRUE; > - } > + if (commit) { > + entry = OvsConntrackCreateTcpEntry(tcp, curNbl, currentTime); > + } > + break; > + } > + case IPPROTO_ICMP: > + { > + ICMPHdr storage; > + const ICMPHdr *icmp; > + icmp = OvsGetIcmp(curNbl, l4Offset, &storage); > + if (!OvsConntrackValidateIcmpPacket(icmp)) { > + state = OVS_CS_F_INVALID; > break; > } > - case IPPROTO_UDP: > - { > - state |= OVS_CS_F_NEW; > - if (commit) { > - entry = OvsConntrackCreateOtherEntry(currentTime); > - if (entry) { > - /* Default UDP related to NULL until TFTP is > supported */ > - entry->parent = NULL; > - } > + > + if (commit) { > + entry = OvsConntrackCreateIcmpEntry(currentTime); > + } > + break; > + } > + case IPPROTO_UDP: > + { > + if (commit) { > + entry = OvsConntrackCreateOtherEntry(currentTime); > + } > + break; > + } > + default: > + state = OVS_CS_F_INVALID; > + break; > + } > + > + if (state != OVS_CS_F_INVALID && commit) { > + if (entry) { > + entry->parent = parentEntry; > + if (OvsCtAddEntry(entry, ctx, natInfo, currentTime)) { > *entryCreated = TRUE; > + } else { > + /* Unable to add entry to the list */ > + OvsFreeMemoryWithTag(entry, OVS_CT_POOL_TAG); > + state = OVS_CS_F_INVALID; > + entry = NULL; > } > - break; > + } else { > + /* OvsAllocateMemoryWithTag returned NULL; treat as invalid */ > + state = OVS_CS_F_INVALID; > } > - default: > - goto invalid; > } > > - if (commit && !entry) { > - return NULL; > - } > - if (entry && !OvsCtAddEntry(entry, ctx, natInfo, currentTime)) { > - return NULL; > - } > - OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL); > - return entry; > -invalid: > - state |= OVS_CS_F_INVALID; > OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL); > return entry; > } > -- > 2.9.0.windows.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
