| Message ID | 1472497860-10628-1-git-send-email-pboca@cloudbasesolutions.com |
|---|---|
| State | Accepted |
| Delegated to: | Guru Shetty |
| Headers | show |
Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com> > -----Original Message----- > From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Paul Boca > Sent: Monday, August 29, 2016 10:11 PM > To: dev@openvswitch.org > Subject: [ovs-dev] [PATCH] datapath-windows: Fixed tcp flags byte order in > conntrack > > In user mode the flags are interpreted as little endian. > This fix makes the kernel mode compatible with user mode. > > Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> > --- > datapath-windows/ovsext/Conntrack-tcp.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/datapath-windows/ovsext/Conntrack-tcp.c b/datapath- > windows/ovsext/Conntrack-tcp.c > index 1820705..6adf490 100644 > --- a/datapath-windows/ovsext/Conntrack-tcp.c > +++ b/datapath-windows/ovsext/Conntrack-tcp.c > @@ -226,13 +226,13 @@ 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 tcp_flags = ntohs(tcp->flags); > uint16_t win = ntohs(tcp->window); > uint32_t ack, end, seq, orig_seq; > uint32_t p_len = OvsGetTcpPayloadLength(nbl); > int ackskew; > > - if (OvsCtInvalidTcpFlags(tcp->flags)) { > + if (OvsCtInvalidTcpFlags(tcp_flags)) { > return CT_UPDATE_INVALID; > } > > @@ -268,7 +268,7 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* > conn_, > if (src->state < CT_DPIF_TCPS_SYN_SENT) { > /* First packet from this end. Set its state */ > > - ack = ntohl(tcp->ack); > + ack = ntohl(tcp->ack_seq); > > end = seq + p_len; > if (tcp_flags & TCP_SYN) { > @@ -308,7 +308,7 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* > conn_, > } > > } else { > - ack = ntohl(tcp->ack); > + ack = ntohl(tcp->ack_seq); > end = seq + p_len; > if (tcp_flags & TCP_SYN) { > end++; > @@ -460,14 +460,16 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* > conn_, BOOLEAN OvsConntrackValidateTcpPacket(const TCPHdr *tcp) { > - if (tcp == NULL || OvsCtInvalidTcpFlags(tcp->flags)) { > + UINT16 tcp_flags = ntohs(tcp->flags); > + > + 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->flags & TCP_SYN) && (tcp->flags & TCP_ACK)) { > + if ((tcp_flags & TCP_SYN) && (tcp_flags & TCP_ACK)) { > return FALSE; > } > > -- > 2.7.2.windows.1 > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
Acked-by: Sairam Venugopal <vsairam@vmware.com> On 8/29/16, 12:11 PM, "Paul Boca" <pboca@cloudbasesolutions.com> wrote: >In user mode the flags are interpreted as little endian. >This fix makes the kernel mode compatible with user mode. > >Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> >--- > datapath-windows/ovsext/Conntrack-tcp.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > >diff --git a/datapath-windows/ovsext/Conntrack-tcp.c >b/datapath-windows/ovsext/Conntrack-tcp.c >index 1820705..6adf490 100644 >--- a/datapath-windows/ovsext/Conntrack-tcp.c >+++ b/datapath-windows/ovsext/Conntrack-tcp.c >@@ -226,13 +226,13 @@ 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 tcp_flags = ntohs(tcp->flags); > uint16_t win = ntohs(tcp->window); > uint32_t ack, end, seq, orig_seq; > uint32_t p_len = OvsGetTcpPayloadLength(nbl); > int ackskew; > >- if (OvsCtInvalidTcpFlags(tcp->flags)) { >+ if (OvsCtInvalidTcpFlags(tcp_flags)) { > return CT_UPDATE_INVALID; > } > >@@ -268,7 +268,7 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_, > if (src->state < CT_DPIF_TCPS_SYN_SENT) { > /* First packet from this end. Set its state */ > >- ack = ntohl(tcp->ack); >+ ack = ntohl(tcp->ack_seq); > > end = seq + p_len; > if (tcp_flags & TCP_SYN) { >@@ -308,7 +308,7 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_, > } > > } else { >- ack = ntohl(tcp->ack); >+ ack = ntohl(tcp->ack_seq); > end = seq + p_len; > if (tcp_flags & TCP_SYN) { > end++; >@@ -460,14 +460,16 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_, > BOOLEAN > OvsConntrackValidateTcpPacket(const TCPHdr *tcp) > { >- if (tcp == NULL || OvsCtInvalidTcpFlags(tcp->flags)) { >+ UINT16 tcp_flags = ntohs(tcp->flags); >+ >+ 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->flags & TCP_SYN) && (tcp->flags & TCP_ACK)) { >+ if ((tcp_flags & TCP_SYN) && (tcp_flags & TCP_ACK)) { > return FALSE; > } > >-- >2.7.2.windows.1 >_______________________________________________ >dev mailing list >dev@openvswitch.org >https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma >n_listinfo_dev&d=CwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Dc >ruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=-t_4jk9EGYkPrqqyTzqsf-MwYtIRao >UO_wtdXLKQQB8&s=ig_QjFu7-uTiDcXKO1ji6g5eqXfDDMCBsHpSncV6nr0&e=
On 29 August 2016 at 12:11, Paul Boca <pboca@cloudbasesolutions.com> wrote: > In user mode the flags are interpreted as little endian. > This fix makes the kernel mode compatible with user mode. > > Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> > Applied, thank you. > --- > datapath-windows/ovsext/Conntrack-tcp.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/datapath-windows/ovsext/Conntrack-tcp.c > b/datapath-windows/ovsext/Conntrack-tcp.c > index 1820705..6adf490 100644 > --- a/datapath-windows/ovsext/Conntrack-tcp.c > +++ b/datapath-windows/ovsext/Conntrack-tcp.c > @@ -226,13 +226,13 @@ 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 tcp_flags = ntohs(tcp->flags); > uint16_t win = ntohs(tcp->window); > uint32_t ack, end, seq, orig_seq; > uint32_t p_len = OvsGetTcpPayloadLength(nbl); > int ackskew; > > - if (OvsCtInvalidTcpFlags(tcp->flags)) { > + if (OvsCtInvalidTcpFlags(tcp_flags)) { > return CT_UPDATE_INVALID; > } > > @@ -268,7 +268,7 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_, > if (src->state < CT_DPIF_TCPS_SYN_SENT) { > /* First packet from this end. Set its state */ > > - ack = ntohl(tcp->ack); > + ack = ntohl(tcp->ack_seq); > > end = seq + p_len; > if (tcp_flags & TCP_SYN) { > @@ -308,7 +308,7 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_, > } > > } else { > - ack = ntohl(tcp->ack); > + ack = ntohl(tcp->ack_seq); > end = seq + p_len; > if (tcp_flags & TCP_SYN) { > end++; > @@ -460,14 +460,16 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_, > BOOLEAN > OvsConntrackValidateTcpPacket(const TCPHdr *tcp) > { > - if (tcp == NULL || OvsCtInvalidTcpFlags(tcp->flags)) { > + UINT16 tcp_flags = ntohs(tcp->flags); > + > + 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->flags & TCP_SYN) && (tcp->flags & TCP_ACK)) { > + if ((tcp_flags & TCP_SYN) && (tcp_flags & TCP_ACK)) { > return FALSE; > } > > -- > 2.7.2.windows.1 > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev >
On 8 September 2016 at 12:34, Guru Shetty <guru@ovn.org> wrote: > On 29 August 2016 at 12:11, Paul Boca <pboca@cloudbasesolutions.com> wrote: > >> In user mode the flags are interpreted as little endian. >> This fix makes the kernel mode compatible with user mode. >> >> Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> >> > Applied, thank you. Does this need to be applied to branch-2.6 as well?
> -----Original Message----- > From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Joe Stringer > Sent: Friday, September 9, 2016 12:37 AM > To: Guru Shetty <guru@ovn.org> > Cc: dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH] datapath-windows: Fixed tcp flags byte order > in conntrack > > On 8 September 2016 at 12:34, Guru Shetty <guru@ovn.org> wrote: > > On 29 August 2016 at 12:11, Paul Boca <pboca@cloudbasesolutions.com> > wrote: > > > >> In user mode the flags are interpreted as little endian. > >> This fix makes the kernel mode compatible with user mode. > >> > >> Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> > >> > > Applied, thank you. > > Does this need to be applied to branch-2.6 as well? [Alin Serdean] Yes, please. > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
On 9 September 2016 at 04:02, Alin Serdean <aserdean@cloudbasesolutions.com> wrote: > > > > -----Original Message----- > > From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Joe Stringer > > Sent: Friday, September 9, 2016 12:37 AM > > To: Guru Shetty <guru@ovn.org> > > Cc: dev@openvswitch.org > > Subject: Re: [ovs-dev] [PATCH] datapath-windows: Fixed tcp flags byte > order > > in conntrack > > > > On 8 September 2016 at 12:34, Guru Shetty <guru@ovn.org> wrote: > > > On 29 August 2016 at 12:11, Paul Boca <pboca@cloudbasesolutions.com> > > wrote: > > > > > >> In user mode the flags are interpreted as little endian. > > >> This fix makes the kernel mode compatible with user mode. > > >> > > >> Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> > > >> > > > Applied, thank you. > > > > Does this need to be applied to branch-2.6 as well? > [Alin Serdean] Yes, please. > Thanks, done. If there are any other bug fixes that you wanted to for 2.6 and does not exist, please do let me know. > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev >
diff --git a/datapath-windows/ovsext/Conntrack-tcp.c b/datapath-windows/ovsext/Conntrack-tcp.c index 1820705..6adf490 100644 --- a/datapath-windows/ovsext/Conntrack-tcp.c +++ b/datapath-windows/ovsext/Conntrack-tcp.c @@ -226,13 +226,13 @@ 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 tcp_flags = ntohs(tcp->flags); uint16_t win = ntohs(tcp->window); uint32_t ack, end, seq, orig_seq; uint32_t p_len = OvsGetTcpPayloadLength(nbl); int ackskew; - if (OvsCtInvalidTcpFlags(tcp->flags)) { + if (OvsCtInvalidTcpFlags(tcp_flags)) { return CT_UPDATE_INVALID; } @@ -268,7 +268,7 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_, if (src->state < CT_DPIF_TCPS_SYN_SENT) { /* First packet from this end. Set its state */ - ack = ntohl(tcp->ack); + ack = ntohl(tcp->ack_seq); end = seq + p_len; if (tcp_flags & TCP_SYN) { @@ -308,7 +308,7 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_, } } else { - ack = ntohl(tcp->ack); + ack = ntohl(tcp->ack_seq); end = seq + p_len; if (tcp_flags & TCP_SYN) { end++; @@ -460,14 +460,16 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_, BOOLEAN OvsConntrackValidateTcpPacket(const TCPHdr *tcp) { - if (tcp == NULL || OvsCtInvalidTcpFlags(tcp->flags)) { + UINT16 tcp_flags = ntohs(tcp->flags); + + 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->flags & TCP_SYN) && (tcp->flags & TCP_ACK)) { + if ((tcp_flags & TCP_SYN) && (tcp_flags & TCP_ACK)) { return FALSE; }
In user mode the flags are interpreted as little endian. This fix makes the kernel mode compatible with user mode. Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> --- datapath-windows/ovsext/Conntrack-tcp.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)