Hi Sai,
Thanks for incorporating the comments so far. It looks good but we need to
treat corner cases like no valid resource allocations and NULL checks for
parameter.
A few small nits inlined.
Thanks,
Alin.
<-----------cut-------->
> ctx->key.src.port = flowKey->ipKey.l4.tpSrc;
> ctx->key.dst.port = flowKey->ipKey.l4.tpDst;
> + if (flowKey->ipKey.nwProto == IPPROTO_ICMP) {
> + ICMPHdr icmpStorage;
> + const ICMPHdr *icmp;
> + icmp = OvsGetIcmp(curNbl, l4Offset, &icmpStorage);
> + ASSERT(icmp);
> + ctx->key.src.port = ctx->key.dst.port =
> + icmp->fields.echo.id;
> +
> + /* Related bit is set when ICMP has an error */
> + /* XXX parse out the appropriate src and dst from inner pkt */
> + switch (icmp->type) {
> + case ICMP4_DEST_UNREACH:
> + case ICMP4_TIME_EXCEEDED:
> + case ICMP4_PARAM_PROB:
> + case ICMP4_SOURCE_QUENCH:
> + case ICMP4_REDIRECT: {
> + ctx->related = TRUE;
> + }
[Alin Gabriel Serdean: ] break;
> + default:
> + ctx->related = FALSE;
[Alin Gabriel Serdean: ] break; Without breaks ctx->related will always be
false;
> + }
> + }
<-----------cut-------->
> //Delete and update the Conntrack
> OvsCtEntryDelete(ctx->entry);
> ctx->entry = NULL;
> - entry = OvsCtEntryCreate(tcp, curNbl, ctx, key,
> - commit, currentTime);
> + entry = OvsCtEntryCreate(curNbl, key->ipKey.nwProto, l4Offset,
> + ctx, key, commit, currentTime);
[Alin Gabriel Serdean: ] entry can be null, if for example the tcp packet was
invalid. We need to treat that case because the following code will run after.
/* Copy mark and label from entry into flowKey. If actions specify
different mark and label, update the flowKey. */
OvsCtUpdateFlowKey(key, state, zone, entry->mark, &entry->labels);
> break;
> }
> }
> @@ -401,15 +493,12 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
> NDIS_STATUS status = NDIS_STATUS_SUCCESS;
> POVS_CT_ENTRY entry = NULL;
> OvsConntrackKeyLookupCtx ctx = { 0 };
> - TCPHdr tcpStorage;
> - UINT64 currentTime;
> LOCK_STATE_EX lockState;
> - const TCPHdr *tcp;
> - tcp = OvsGetTcp(curNbl, layers->l4Offset, &tcpStorage);
> + UINT64 currentTime;
> NdisGetCurrentSystemTime((LARGE_INTEGER *) ¤tTime);
>
> /* Retrieve the Conntrack Key related fields from packet */
> - OvsCtSetupLookupCtx(key, zone, &ctx);
> + OvsCtSetupLookupCtx(key, zone, &ctx, curNbl, layers->l4Offset);
>
> NdisAcquireRWLockWrite(ovsConntrackLockObj, &lockState, 0);
>
> @@ -418,11 +507,12 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
>
> if (!entry) {
> /* If no matching entry was found, create one and add New state */
> - entry = OvsCtEntryCreate(tcp, curNbl, &ctx,
> + entry = OvsCtEntryCreate(curNbl, key->ipKey.nwProto,
> + layers->l4Offset, &ctx,
> key, commit, currentTime);
> } else {
> /* Process the entry and update CT flags */
> - entry = OvsProcessConntrackEntry(curNbl, tcp, &ctx, key,
> + entry = OvsProcessConntrackEntry(curNbl, layers->l4Offset,
> + &ctx, key,
> zone, commit, currentTime);
> }
>
> diff --git a/datapath-windows/ovsext/Conntrack.h b/datapath-
> windows/ovsext/Conntrack.h
> index a754544..883ac57 100644
> --- a/datapath-windows/ovsext/Conntrack.h
> +++ b/datapath-windows/ovsext/Conntrack.h
> @@ -80,8 +80,11 @@ typedef struct OvsConntrackKeyLookupCtx {
>
> #define CT_HASH_TABLE_SIZE ((UINT32)1 << 10) #define
> CT_HASH_TABLE_MASK (CT_HASH_TABLE_SIZE - 1)
> -#define CT_ENTRY_TIMEOUT (2 * 600000000) // 2m
> -#define CT_CLEANUP_INTERVAL (2 * 600000000) // 2m
> +#define CT_INTERVAL_SEC 10000000LL //1s
> +#define CT_ENTRY_TIMEOUT (2 * 60 * CT_INTERVAL_SEC) // 2m
> +#define CT_CLEANUP_INTERVAL (2 * 60 * CT_INTERVAL_SEC) // 2m
> +
> +
> /* Given POINTER, the address of the given MEMBER in a STRUCT object,
> returns
> the STRUCT object. */
> #define CONTAINER_OF(POINTER, STRUCT, MEMBER) \
> @@ -99,9 +102,13 @@ BOOLEAN OvsConntrackValidateTcpPacket(const
> TCPHdr *tcp); OVS_CT_ENTRY * OvsConntrackCreateTcpEntry(const TCPHdr
> *tcp,
> PNET_BUFFER_LIST nbl,
> UINT64 now);
> +OVS_CT_ENTRY * OvsConntrackCreateOtherEntry(UINT64 now);
> enum CT_UPDATE_RES OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY*
> conn_,
> const TCPHdr *tcp,
> PNET_BUFFER_LIST nbl,
> BOOLEAN reply,
> UINT64 now); -#endif /*
> __OVS_CONNTRACK_H_ */
> \ No newline at end of file
> +enum ct_update_res OvsConntrackUpdateOtherEntry(OVS_CT_ENTRY
> *conn_,
> + BOOLEAN reply,
> + UINT64 now); #endif /*
> +__OVS_CONNTRACK_H_ */
> diff --git a/datapath-windows/ovsext/ovsext.vcxproj b/datapath-
> windows/ovsext/ovsext.vcxproj
> index 0356ddf..0ad4c58 100644
> --- a/datapath-windows/ovsext/ovsext.vcxproj
> +++ b/datapath-windows/ovsext/ovsext.vcxproj
> @@ -176,6 +176,7 @@
> <ItemGroup>
> <ClCompile Include="Actions.c" />
> <ClCompile Include="BufferMgmt.c" />
> + <ClCompile Include="Conntrack-other.c" />
> <ClCompile Include="Conntrack-tcp.c" />
> <ClCompile Include="Conntrack.c" />
> <ClCompile Include="Debug.c" />
> --
> 2.5.0.windows.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
