[ovs-dev] [PATCH] datapath-windows: Cleanup conntrack-tcp.c
Sairam Venugopal
vsairam at vmware.com
Sat Jun 25 01:14:34 UTC 2016
Update the code to use tcp->flags. This keeps the kernel conntrack-tcp.c file in sync with userspace version.
This patch also addresses an warning - 'Comparison of a boolean expression with an integer other than 0 or 1' - (tcp_flags & (TCP_ACK|TCP_RST)) == (TCP_ACK|TCP_RST))
Signed-off-by: Sairam Venugopal <vsairam at vmware.com>
---
datapath-windows/ovsext/Conntrack-tcp.c | 79 +++++++++++++++------------------
datapath-windows/ovsext/NetProto.h | 25 ++++++-----
2 files changed, 51 insertions(+), 53 deletions(-)
diff --git a/datapath-windows/ovsext/Conntrack-tcp.c b/datapath-windows/ovsext/Conntrack-tcp.c
index a0ee791..1820705 100644
--- a/datapath-windows/ovsext/Conntrack-tcp.c
+++ b/datapath-windows/ovsext/Conntrack-tcp.c
@@ -128,27 +128,22 @@ enum ct_dpif_tcp_state {
/* pf does this in in pf_normalize_tcp(), and it is called only if scrub
* is enabled. We're not scrubbing, but this check seems reasonable. */
static __inline BOOLEAN
-OvsConntrackValidateTcpFlags(const TCPHdr *tcp)
+OvsCtInvalidTcpFlags(uint16_t flags)
{
- if (tcp->syn) {
- if (tcp->rst) {
- return TRUE;
- }
- if (tcp->fin) {
- /* Here pf removes the fin flag. We simply mark the packet as
- * invalid */
+ if (flags & TCP_SYN) {
+ if (flags & TCP_RST || flags & TCP_FIN) {
return TRUE;
}
} else {
/* Illegal packet */
- if (!(tcp->ack || tcp->rst)) {
+ if (!(flags & (TCP_ACK|TCP_RST))) {
return TRUE;
}
}
- if (!(tcp->ack)) {
+ if (!(flags & TCP_ACK)) {
/* These flags are only valid if ACK is set */
- if ((tcp->fin) || (tcp->psh) || (tcp->urg)) {
+ if ((flags & TCP_FIN) || (flags & TCP_PSH) || (flags & TCP_URG)) {
return TRUE;
}
}
@@ -165,10 +160,9 @@ OvsTcpGetWscale(const TCPHdr *tcp)
uint8_t optlen;
while (len >= 3) {
- if (*opt == TCPOPT_EOL) {
- break;
- }
switch (*opt) {
+ case TCPOPT_EOL:
+ return wscale;
case TCPOPT_NOP:
opt++;
len--;
@@ -232,31 +226,33 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_,
/* The peer that should receive 'pkt' */
struct tcp_peer *dst = &conn->peer[reply ? 0 : 1];
uint8_t sws = 0, dws = 0;
+ UINT16 tcp_flags = tcp->flags;
uint16_t win = ntohs(tcp->window);
uint32_t ack, end, seq, orig_seq;
uint32_t p_len = OvsGetTcpPayloadLength(nbl);
int ackskew;
- if (OvsConntrackValidateTcpFlags(tcp)) {
+ if (OvsCtInvalidTcpFlags(tcp->flags)) {
return CT_UPDATE_INVALID;
}
- if ((tcp->syn) && dst->state >= CT_DPIF_TCPS_FIN_WAIT_2 &&
- src->state >= CT_DPIF_TCPS_FIN_WAIT_2) {
+ if (((tcp_flags & (TCP_SYN|TCP_ACK)) == TCP_SYN)
+ && dst->state >= CT_DPIF_TCPS_FIN_WAIT_2
+ && src->state >= CT_DPIF_TCPS_FIN_WAIT_2) {
src->state = dst->state = CT_DPIF_TCPS_CLOSED;
return CT_UPDATE_NEW;
}
if (src->wscale & CT_WSCALE_FLAG
&& dst->wscale & CT_WSCALE_FLAG
- && !(tcp->syn)) {
+ && !(tcp_flags & TCP_SYN)) {
sws = src->wscale & CT_WSCALE_MASK;
dws = dst->wscale & CT_WSCALE_MASK;
} else if (src->wscale & CT_WSCALE_UNKNOWN
&& dst->wscale & CT_WSCALE_UNKNOWN
- && !(tcp->syn)) {
+ && !(tcp_flags & TCP_SYN)) {
sws = TCP_MAX_WSCALE;
dws = TCP_MAX_WSCALE;
@@ -275,7 +271,7 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_,
ack = ntohl(tcp->ack);
end = seq + p_len;
- if (tcp->syn) {
+ if (tcp_flags & TCP_SYN) {
end++;
if (dst->wscale & CT_WSCALE_FLAG) {
src->wscale = OvsTcpGetWscale(tcp);
@@ -286,14 +282,13 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_,
dws = dst->wscale & CT_WSCALE_MASK;
} else {
/* fixup other window */
- dst->max_win <<= dst->wscale &
- CT_WSCALE_MASK;
+ dst->max_win <<= dst->wscale & CT_WSCALE_MASK;
/* in case of a retrans SYN|ACK */
dst->wscale = 0;
}
}
}
- if (tcp->fin) {
+ if (tcp_flags & TCP_FIN) {
end++;
}
@@ -305,8 +300,7 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_,
* after establishment)
*/
if (src->seqhi == 1 ||
- SEQ_GEQ(end + MAX(1, dst->max_win << dws),
- src->seqhi)) {
+ SEQ_GEQ(end + MAX(1, dst->max_win << dws), src->seqhi)) {
src->seqhi = end + MAX(1, dst->max_win << dws);
}
if (win > src->max_win) {
@@ -316,19 +310,19 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_,
} else {
ack = ntohl(tcp->ack);
end = seq + p_len;
- if (tcp->syn) {
+ if (tcp_flags & TCP_SYN) {
end++;
}
- if (tcp->fin) {
+ if (tcp_flags & TCP_FIN) {
end++;
}
}
- if ((tcp->ack) == 0) {
+ if ((tcp_flags & TCP_ACK) == 0) {
/* Let it pass through the ack skew check */
ack = dst->seqlo;
} else if ((ack == 0
- && (tcp->ack && tcp->rst) == (TCP_ACK|TCP_RST))
+ && (tcp_flags & (TCP_ACK|TCP_RST)) == (TCP_ACK|TCP_RST))
/* broken tcp stacks do not set ack */) {
/* Many stacks (ours included) will set the ACK number in an
* FIN|ACK if the SYN times out -- no sequence to ACK. */
@@ -351,7 +345,7 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_,
/* Acking not more than one reassembled fragment backwards */
&& (ackskew <= (MAXACKWINDOW << sws))
/* Acking not more than one window forward */
- && ((tcp->rst) == 0 || orig_seq == src->seqlo
+ && ((tcp_flags & TCP_RST) == 0 || orig_seq == src->seqlo
|| (orig_seq == src->seqlo + 1)
|| (orig_seq + 1 == src->seqlo))) {
/* Require an exact/+1 sequence match on resets when possible */
@@ -370,20 +364,20 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_,
}
/* update states */
- if (tcp->syn && src->state < CT_DPIF_TCPS_SYN_SENT) {
+ if (tcp_flags & TCP_SYN && src->state < CT_DPIF_TCPS_SYN_SENT) {
src->state = CT_DPIF_TCPS_SYN_SENT;
}
- if (tcp->fin && src->state < CT_DPIF_TCPS_CLOSING) {
+ if (tcp_flags & TCP_FIN && src->state < CT_DPIF_TCPS_CLOSING) {
src->state = CT_DPIF_TCPS_CLOSING;
}
- if (tcp->ack) {
+ if (tcp_flags & TCP_ACK) {
if (dst->state == CT_DPIF_TCPS_SYN_SENT) {
dst->state = CT_DPIF_TCPS_ESTABLISHED;
} else if (dst->state == CT_DPIF_TCPS_CLOSING) {
dst->state = CT_DPIF_TCPS_FIN_WAIT_2;
}
}
- if (tcp->rst) {
+ if (tcp_flags & TCP_RST) {
src->state = dst->state = CT_DPIF_TCPS_TIME_WAIT;
}
@@ -449,11 +443,11 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_,
* SYN and not an already established connection.
*/
- if (tcp->fin && src->state < CT_DPIF_TCPS_CLOSING) {
+ if (tcp_flags & TCP_FIN && src->state < CT_DPIF_TCPS_CLOSING) {
src->state = CT_DPIF_TCPS_CLOSING;
}
- if (tcp->rst) {
+ if (tcp_flags & TCP_RST) {
src->state = dst->state = CT_DPIF_TCPS_TIME_WAIT;
}
} else {
@@ -466,14 +460,14 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_,
BOOLEAN
OvsConntrackValidateTcpPacket(const TCPHdr *tcp)
{
- if (tcp == NULL || OvsConntrackValidateTcpFlags(tcp)) {
+ if (tcp == NULL || OvsCtInvalidTcpFlags(tcp->flags)) {
return FALSE;
}
/* A syn+ack is not allowed to create a connection. We want to allow
* totally new connections (syn) or already established, not partially
* open (syn+ack). */
- if ((tcp->syn) && (tcp->ack)) {
+ if ((tcp->flags & TCP_SYN) && (tcp->flags & TCP_ACK)) {
return FALSE;
}
@@ -485,7 +479,7 @@ OvsConntrackCreateTcpEntry(const TCPHdr *tcp,
PNET_BUFFER_LIST nbl,
UINT64 now)
{
- struct conn_tcp* newconn = NULL;
+ struct conn_tcp* newconn;
struct tcp_peer *src, *dst;
newconn = OvsAllocateMemoryWithTag(sizeof(struct conn_tcp),
@@ -501,7 +495,7 @@ OvsConntrackCreateTcpEntry(const TCPHdr *tcp,
src->seqlo = ntohl(tcp->seq);
src->seqhi = src->seqlo + OvsGetTcpPayloadLength(nbl) + 1;
- if (tcp->syn) {
+ if (tcp->flags & TCP_SYN) {
src->seqhi++;
src->wscale = OvsTcpGetWscale(tcp);
} else {
@@ -512,10 +506,9 @@ OvsConntrackCreateTcpEntry(const TCPHdr *tcp,
if (src->wscale & CT_WSCALE_MASK) {
/* Remove scale factor from initial window */
uint8_t sws = src->wscale & CT_WSCALE_MASK;
- src->max_win = DIV_ROUND_UP((uint32_t) src->max_win,
- 1 << sws);
+ src->max_win = DIV_ROUND_UP((uint32_t) src->max_win, 1 << sws);
}
- if (tcp->fin) {
+ if (tcp->flags & TCP_FIN) {
src->seqhi++;
}
dst->seqhi = 1;
diff --git a/datapath-windows/ovsext/NetProto.h b/datapath-windows/ovsext/NetProto.h
index 6cf6d8e..92d6611 100644
--- a/datapath-windows/ovsext/NetProto.h
+++ b/datapath-windows/ovsext/NetProto.h
@@ -339,16 +339,21 @@ typedef struct TCPHdr {
UINT16 dest;
UINT32 seq;
UINT32 ack_seq;
- UINT16 res1:4,
- doff:4,
- fin:1,
- syn:1,
- rst:1,
- psh:1,
- ack:1,
- urg:1,
- ece:1,
- cwr:1;
+ union {
+ struct {
+ UINT16 res1:4,
+ doff:4,
+ fin:1,
+ syn:1,
+ rst:1,
+ psh:1,
+ ack:1,
+ urg:1,
+ ece:1,
+ cwr:1;
+ };
+ UINT16 flags;
+ };
UINT16 window;
UINT16 check;
UINT16 urg_ptr;
--
2.5.0.windows.1
More information about the dev
mailing list