| Message ID | 1487735198-127300-4-git-send-email-zhang.zhanghailiang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/22/2017 11:46 AM, zhanghailiang wrote: > While the offset of packets's sequence for primary side and > secondary side is zero, it is unnecessary to call net_checksum_calculate() > to recalculate the checksume value of packets. > > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > --- > net/filter-rewriter.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c > index 7e7ec35..c9a6d43 100644 > --- a/net/filter-rewriter.c > +++ b/net/filter-rewriter.c > @@ -93,10 +93,12 @@ static int handle_primary_tcp_pkt(RewriterState *rf, > conn->offset -= (ntohl(tcp_pkt->th_ack) - 1); > conn->syn_flag = 0; > } > - /* handle packets to the secondary from the primary */ > - tcp_pkt->th_ack = htonl(ntohl(tcp_pkt->th_ack) + conn->offset); > + if (conn->offset) { This is wrong, conn->offset maybe is a negative value(like -1000), So you can change here to "if (conn->offset == 0) {" > + /* handle packets to the secondary from the primary */ > + tcp_pkt->th_ack = htonl(ntohl(tcp_pkt->th_ack) + conn->offset); > > - net_checksum_calculate((uint8_t *)pkt->data, pkt->size); > + net_checksum_calculate((uint8_t *)pkt->data, pkt->size); > + } > /* > * Case 1: > * The *server* side of this connect is VM, *client* tries to close > @@ -112,7 +114,6 @@ static int handle_primary_tcp_pkt(RewriterState *rf, > */ > if ((conn->tcp_state == TCPS_LAST_ACK) && > (ntohl(tcp_pkt->th_ack) == (conn->fin_ack_seq + 1))) { > - fprintf(stderr, "Remove conn " Here need fix. > g_hash_table_remove(rf->connection_track_table, key); > } > } > @@ -159,10 +160,13 @@ static int handle_secondary_tcp_pkt(RewriterState *rf, > } > > if ((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_ACK) { > - /* handle packets to the primary from the secondary*/ > - tcp_pkt->th_seq = htonl(ntohl(tcp_pkt->th_seq) - conn->offset); > + /* Only need to adjust seq while offset is Non-zero */ > + if (conn->offset) { Refer to the above comments. Thanks Zhang Chen > + /* handle packets to the primary from the secondary*/ > + tcp_pkt->th_seq = htonl(ntohl(tcp_pkt->th_seq) - conn->offset); > > - net_checksum_calculate((uint8_t *)pkt->data, pkt->size); > + net_checksum_calculate((uint8_t *)pkt->data, pkt->size); > + } > /* > * Case 2: > * The *server* side of this connect is VM, *server* tries to close
On 02/24/2017 04:08 PM, Zhang Chen wrote: > > > On 02/22/2017 11:46 AM, zhanghailiang wrote: >> While the offset of packets's sequence for primary side and >> secondary side is zero, it is unnecessary to call >> net_checksum_calculate() >> to recalculate the checksume value of packets. >> >> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >> --- >> net/filter-rewriter.c | 18 +++++++++++------- >> 1 file changed, 11 insertions(+), 7 deletions(-) >> >> diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c >> index 7e7ec35..c9a6d43 100644 >> --- a/net/filter-rewriter.c >> +++ b/net/filter-rewriter.c >> @@ -93,10 +93,12 @@ static int handle_primary_tcp_pkt(RewriterState *rf, >> conn->offset -= (ntohl(tcp_pkt->th_ack) - 1); >> conn->syn_flag = 0; >> } >> - /* handle packets to the secondary from the primary */ >> - tcp_pkt->th_ack = htonl(ntohl(tcp_pkt->th_ack) + conn->offset); >> + if (conn->offset) { > > This is wrong, conn->offset maybe is a negative value(like -1000), > So you can change here to "if (conn->offset == 0) {" s/conn->offset == 0 / conn->offset != 0 > > >> + /* handle packets to the secondary from the primary */ >> + tcp_pkt->th_ack = htonl(ntohl(tcp_pkt->th_ack) + >> conn->offset); >> - net_checksum_calculate((uint8_t *)pkt->data, pkt->size); >> + net_checksum_calculate((uint8_t *)pkt->data, pkt->size); >> + } >> /* >> * Case 1: >> * The *server* side of this connect is VM, *client* tries >> to close >> @@ -112,7 +114,6 @@ static int handle_primary_tcp_pkt(RewriterState *rf, >> */ >> if ((conn->tcp_state == TCPS_LAST_ACK) && >> (ntohl(tcp_pkt->th_ack) == (conn->fin_ack_seq + 1))) { >> - fprintf(stderr, "Remove conn " > > Here need fix. > >> g_hash_table_remove(rf->connection_track_table, key); >> } >> } >> @@ -159,10 +160,13 @@ static int >> handle_secondary_tcp_pkt(RewriterState *rf, >> } >> if ((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_ACK) { >> - /* handle packets to the primary from the secondary*/ >> - tcp_pkt->th_seq = htonl(ntohl(tcp_pkt->th_seq) - conn->offset); >> + /* Only need to adjust seq while offset is Non-zero */ >> + if (conn->offset) { > > Refer to the above comments. > > Thanks > Zhang Chen > >> + /* handle packets to the primary from the secondary*/ >> + tcp_pkt->th_seq = htonl(ntohl(tcp_pkt->th_seq) - >> conn->offset); >> - net_checksum_calculate((uint8_t *)pkt->data, pkt->size); >> + net_checksum_calculate((uint8_t *)pkt->data, pkt->size); >> + } >> /* >> * Case 2: >> * The *server* side of this connect is VM, *server* tries >> to close >
On 2017/2/24 16:08, Zhang Chen wrote: > > > On 02/22/2017 11:46 AM, zhanghailiang wrote: >> While the offset of packets's sequence for primary side and >> secondary side is zero, it is unnecessary to call net_checksum_calculate() >> to recalculate the checksume value of packets. >> >> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >> --- >> net/filter-rewriter.c | 18 +++++++++++------- >> 1 file changed, 11 insertions(+), 7 deletions(-) >> >> diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c >> index 7e7ec35..c9a6d43 100644 >> --- a/net/filter-rewriter.c >> +++ b/net/filter-rewriter.c >> @@ -93,10 +93,12 @@ static int handle_primary_tcp_pkt(RewriterState *rf, >> conn->offset -= (ntohl(tcp_pkt->th_ack) - 1); >> conn->syn_flag = 0; >> } >> - /* handle packets to the secondary from the primary */ >> - tcp_pkt->th_ack = htonl(ntohl(tcp_pkt->th_ack) + conn->offset); >> + if (conn->offset) { > > This is wrong, conn->offset maybe is a negative value(like -1000), > So you can change here to "if (conn->offset == 0) {" > Er, if it is a negative value, it can still go into this if (conn->offset) branch, and we need to adjust the checksum value in this case. > >> + /* handle packets to the secondary from the primary */ >> + tcp_pkt->th_ack = htonl(ntohl(tcp_pkt->th_ack) + conn->offset); >> >> - net_checksum_calculate((uint8_t *)pkt->data, pkt->size); >> + net_checksum_calculate((uint8_t *)pkt->data, pkt->size); >> + } >> /* >> * Case 1: >> * The *server* side of this connect is VM, *client* tries to close >> @@ -112,7 +114,6 @@ static int handle_primary_tcp_pkt(RewriterState *rf, >> */ >> if ((conn->tcp_state == TCPS_LAST_ACK) && >> (ntohl(tcp_pkt->th_ack) == (conn->fin_ack_seq + 1))) { >> - fprintf(stderr, "Remove conn " > > Here need fix. > OK. >> g_hash_table_remove(rf->connection_track_table, key); >> } >> } >> @@ -159,10 +160,13 @@ static int handle_secondary_tcp_pkt(RewriterState *rf, >> } >> >> if ((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_ACK) { >> - /* handle packets to the primary from the secondary*/ >> - tcp_pkt->th_seq = htonl(ntohl(tcp_pkt->th_seq) - conn->offset); >> + /* Only need to adjust seq while offset is Non-zero */ >> + if (conn->offset) { > > Refer to the above comments. > > Thanks > Zhang Chen > >> + /* handle packets to the primary from the secondary*/ >> + tcp_pkt->th_seq = htonl(ntohl(tcp_pkt->th_seq) - conn->offset); >> >> - net_checksum_calculate((uint8_t *)pkt->data, pkt->size); >> + net_checksum_calculate((uint8_t *)pkt->data, pkt->size); >> + } >> /* >> * Case 2: >> * The *server* side of this connect is VM, *server* tries to close >
On 02/27/2017 09:36 AM, Hailiang Zhang wrote: > On 2017/2/24 16:08, Zhang Chen wrote: >> >> >> On 02/22/2017 11:46 AM, zhanghailiang wrote: >>> While the offset of packets's sequence for primary side and >>> secondary side is zero, it is unnecessary to call >>> net_checksum_calculate() >>> to recalculate the checksume value of packets. >>> >>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >>> --- >>> net/filter-rewriter.c | 18 +++++++++++------- >>> 1 file changed, 11 insertions(+), 7 deletions(-) >>> >>> diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c >>> index 7e7ec35..c9a6d43 100644 >>> --- a/net/filter-rewriter.c >>> +++ b/net/filter-rewriter.c >>> @@ -93,10 +93,12 @@ static int handle_primary_tcp_pkt(RewriterState >>> *rf, >>> conn->offset -= (ntohl(tcp_pkt->th_ack) - 1); >>> conn->syn_flag = 0; >>> } >>> - /* handle packets to the secondary from the primary */ >>> - tcp_pkt->th_ack = htonl(ntohl(tcp_pkt->th_ack) + >>> conn->offset); >>> + if (conn->offset) { >> >> This is wrong, conn->offset maybe is a negative value(like -1000), >> So you can change here to "if (conn->offset == 0) {" >> > > Er, if it is a negative value, it can still go into this if > (conn->offset) > branch, and we need to adjust the checksum value in this case. > Sorry, I misunderstand it, ignore. Thanks Zhang Chen >> >>> + /* handle packets to the secondary from the primary */ >>> + tcp_pkt->th_ack = htonl(ntohl(tcp_pkt->th_ack) + >>> conn->offset); >>> >>> - net_checksum_calculate((uint8_t *)pkt->data, pkt->size); >>> + net_checksum_calculate((uint8_t *)pkt->data, pkt->size); >>> + } >>> /* >>> * Case 1: >>> * The *server* side of this connect is VM, *client* >>> tries to close >>> @@ -112,7 +114,6 @@ static int handle_primary_tcp_pkt(RewriterState >>> *rf, >>> */ >>> if ((conn->tcp_state == TCPS_LAST_ACK) && >>> (ntohl(tcp_pkt->th_ack) == (conn->fin_ack_seq + 1))) { >>> - fprintf(stderr, "Remove conn " >> >> Here need fix. >> > > OK. > >>> g_hash_table_remove(rf->connection_track_table, key); >>> } >>> } >>> @@ -159,10 +160,13 @@ static int >>> handle_secondary_tcp_pkt(RewriterState *rf, >>> } >>> >>> if ((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_ACK) { >>> - /* handle packets to the primary from the secondary*/ >>> - tcp_pkt->th_seq = htonl(ntohl(tcp_pkt->th_seq) - >>> conn->offset); >>> + /* Only need to adjust seq while offset is Non-zero */ >>> + if (conn->offset) { >> >> Refer to the above comments. >> >> Thanks >> Zhang Chen >> >>> + /* handle packets to the primary from the secondary*/ >>> + tcp_pkt->th_seq = htonl(ntohl(tcp_pkt->th_seq) - >>> conn->offset); >>> >>> - net_checksum_calculate((uint8_t *)pkt->data, pkt->size); >>> + net_checksum_calculate((uint8_t *)pkt->data, pkt->size); >>> + } >>> /* >>> * Case 2: >>> * The *server* side of this connect is VM, *server* >>> tries to close >> > > > > . >
diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c index 7e7ec35..c9a6d43 100644 --- a/net/filter-rewriter.c +++ b/net/filter-rewriter.c @@ -93,10 +93,12 @@ static int handle_primary_tcp_pkt(RewriterState *rf, conn->offset -= (ntohl(tcp_pkt->th_ack) - 1); conn->syn_flag = 0; } - /* handle packets to the secondary from the primary */ - tcp_pkt->th_ack = htonl(ntohl(tcp_pkt->th_ack) + conn->offset); + if (conn->offset) { + /* handle packets to the secondary from the primary */ + tcp_pkt->th_ack = htonl(ntohl(tcp_pkt->th_ack) + conn->offset); - net_checksum_calculate((uint8_t *)pkt->data, pkt->size); + net_checksum_calculate((uint8_t *)pkt->data, pkt->size); + } /* * Case 1: * The *server* side of this connect is VM, *client* tries to close @@ -112,7 +114,6 @@ static int handle_primary_tcp_pkt(RewriterState *rf, */ if ((conn->tcp_state == TCPS_LAST_ACK) && (ntohl(tcp_pkt->th_ack) == (conn->fin_ack_seq + 1))) { - fprintf(stderr, "Remove conn " g_hash_table_remove(rf->connection_track_table, key); } } @@ -159,10 +160,13 @@ static int handle_secondary_tcp_pkt(RewriterState *rf, } if ((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_ACK) { - /* handle packets to the primary from the secondary*/ - tcp_pkt->th_seq = htonl(ntohl(tcp_pkt->th_seq) - conn->offset); + /* Only need to adjust seq while offset is Non-zero */ + if (conn->offset) { + /* handle packets to the primary from the secondary*/ + tcp_pkt->th_seq = htonl(ntohl(tcp_pkt->th_seq) - conn->offset); - net_checksum_calculate((uint8_t *)pkt->data, pkt->size); + net_checksum_calculate((uint8_t *)pkt->data, pkt->size); + } /* * Case 2: * The *server* side of this connect is VM, *server* tries to close
While the offset of packets's sequence for primary side and secondary side is zero, it is unnecessary to call net_checksum_calculate() to recalculate the checksume value of packets. Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> --- net/filter-rewriter.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)