Re: [Qemu-devel] [PATCH v2 3/3] filter-rewriter: skip net_checksum_calculate() while offset = 0

[Hailiang Zhang <zhang.zhanghailiang@huawei.com>]

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
>