I am unsure if we actual need another file but that maybe a personal preference.
Other comments inlined.
Alin.
> +
> +static const long long other_timeouts[] = {
> + [OTHERS_FIRST] = 60 * 10000000,
> + [OTHERS_MULTIPLE] = 60 * 10000000,
> + [OTHERS_BIDIR] = 30 * 10000000,
> +};
[Alin Gabriel Serdean: ] Maybe define 10000000 somewhere and add a comment on
how many ms/s they will be.
> +
> +static __inline struct conn_other*
> +OvsCastConntrackEntryToOtherEntry(OVS_CT_ENTRY *conn) {
> + return CONTAINER_OF(conn, struct conn_other, up); }
[Alin Gabriel Serdean: ] Add an assert conn
> +
> +static __inline VOID
> +OvsConntrackUpdateExpiration(struct conn_other *conn, long long now) {
> + conn->up.expiration = now + other_timeouts[conn->state]; }
[Alin Gabriel Serdean: ] Add an assert to conn
> +
> +enum ct_update_res
> +OvsConntrackUpdateOtherEntry(OVS_CT_ENTRY *conn_,
> + BOOLEAN reply,
> + UINT64 now) {
> + struct conn_other *conn = OvsCastConntrackEntryToOtherEntry(conn_);
[Alin Gabriel Serdean: ] Add an assert to conn_
> +
> +OVS_CT_ENTRY *
> +OvsConntrackCreateOtherEntry(UINT64 now) {
> + struct conn_other *conn;
> + conn = OvsAllocateMemoryWithTag(sizeof(struct conn_other),
> + OVS_CT_POOL_TAG);
> + /* XXX Handle memory allocation error*/
[Alin Gabriel Serdean: ] Please handle the memory allocation or at least add an
assert to it
> + conn->up = (OVS_CT_ENTRY) {0};
> + conn->state = OTHERS_FIRST;
> + OvsConntrackUpdateExpiration(conn, now);
> + return &conn->up;
> +}
> \ No newline at end of file
[Alin Gabriel Serdean: ] Nit no new line
> diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-
> windows/ovsext/Conntrack.c
> index 544fd51..e193e9f 100644
> --- a/datapath-windows/ovsext/Conntrack.c
> +++ b/datapath-windows/ovsext/Conntrack.c
> @@ -146,9 +146,20 @@ OvsCtUpdateFlowKey(struct OvsFlowKey *key,
> }
> }
>
> +static __inline VOID
> +OvsCtAddEntry(POVS_CT_ENTRY entry, OvsConntrackKeyLookupCtx *ctx) {
> + NdisMoveMemory(&entry->key, &ctx->key, sizeof (OVS_CT_KEY));
> + NdisMoveMemory(&entry->rev_key, &ctx->key, sizeof (OVS_CT_KEY));
[Alin Gabriel Serdean: ] Maybe use RtlCopyMemory
> + OvsCtKeyReverse(&entry->rev_key);
> + InsertHeadList(&ovsConntrackTable[ctx->hash &
> CT_HASH_TABLE_MASK],
> + &entry->link);
> +}
> +
> static __inline POVS_CT_ENTRY
> -OvsCtEntryCreate(const TCPHdr *tcp,
> - PNET_BUFFER_LIST curNbl,
> +OvsCtEntryCreate(PNET_BUFFER_LIST curNbl,
> + UINT8 ipProto,
> + UINT32 l4Offset,
> OvsConntrackKeyLookupCtx *ctx,
> OvsFlowKey *key,
> BOOLEAN commit,
> @@ -156,26 +167,71 @@ OvsCtEntryCreate(const TCPHdr *tcp, {
> POVS_CT_ENTRY entry = NULL;
> UINT32 state = 0;
> - if (!OvsConntrackValidateTcpPacket(tcp)) {
> - state |= OVS_CS_F_INVALID;
> - OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL);
> - return entry;
> - }
> + 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;
> + if (commit) {
> + entry = OvsConntrackCreateTcpEntry(tcp, curNbl, currentTime);
> + OvsCtAddEntry(entry, ctx);
> + }
> +
> + OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL);
> + return entry;
> + }
> + case IPPROTO_ICMP:
> + case IPPROTO_UDP:
> + state |= OVS_CS_F_NEW;
> + if (commit) {
> + entry = OvsConntrackCreateOtherEntry(currentTime);
> + OvsCtAddEntry(entry, ctx);
> + }
>
> - state |= OVS_CS_F_NEW;
> - if (commit) {
> - entry = OvsConntrackCreateTcpEntry(tcp, curNbl, currentTime);
> - NdisMoveMemory(&entry->key, &ctx->key, sizeof (OVS_CT_KEY));
> - NdisMoveMemory(&entry->rev_key, &ctx->key, sizeof (OVS_CT_KEY));
> - OvsCtKeyReverse(&entry->rev_key);
> - InsertHeadList(&ovsConntrackTable[ctx->hash &
> CT_HASH_TABLE_MASK],
> - &entry->link);
> + OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL);
> + return entry;
> + default:
> + goto invalid;
> }
>
> +invalid:
> + state |= OVS_CS_F_INVALID;
> OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL);
> return entry;
> }
>
> +static enum CT_UPDATE_RES
> +OvsCtUpdateEntry(OVS_CT_ENTRY* entry,
> + PNET_BUFFER_LIST nbl,
> + UINT8 ipProto,
> + UINT32 l4Offset,
> + BOOLEAN reply,
> + UINT64 now)
> +{
> + switch (ipProto)
> + {
> + case IPPROTO_TCP:
> + {
> + TCPHdr tcpStorage;
> + const TCPHdr *tcp;
> + tcp = OvsGetTcp(nbl, l4Offset, &tcpStorage);
[Alin Gabriel Serdean: ] add in if (tcp) in the case it may be null
> + return OvsConntrackUpdateTcpEntry(entry, tcp, nbl, reply, now);
> + }
> + case IPPROTO_ICMP:
> + case IPPROTO_UDP:
> + return OvsConntrackUpdateOtherEntry(entry, reply, now);
> + default:
> + return CT_UPDATE_INVALID;
> + }
> +}
> +
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
