[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL V2 01/26] filter-rewriter: Add TCP state machine
From: |
Zhang Chen |
Subject: |
Re: [Qemu-devel] [PULL V2 01/26] filter-rewriter: Add TCP state machine and fix memory leak in connection_track_table |
Date: |
Tue, 30 Oct 2018 10:29:08 +0800 |
On Tue, Oct 30, 2018 at 10:02 AM Jason Wang <address@hidden> wrote:
>
> On 2018/10/29 下午7:01, Peter Maydell wrote:
> > On 19 October 2018 at 04:22, Jason Wang <address@hidden> wrote:
> >> From: Zhang Chen <address@hidden>
> >>
> >> We add almost full TCP state machine in filter-rewriter, except
> >> TCPS_LISTEN and some simplify in VM active close FIN states.
> >> The reason for this simplify job is because guest kernel will track
> >> the TCP status and wait 2MSL time too, if client resend the FIN packet,
> >> guest will resend the last ACK, so we needn't wait 2MSL time in
> filter-rewriter.
> >>
> >> After a net connection is closed, we didn't clear its related resources
> >> in connection_track_table, which will lead to memory leak.
> >>
> >> Let's track the state of net connection, if it is closed, its related
> >> resources will be cleared up.
> > Hi. Coverity (CID 1396477) points out that here:
> >
> >> + /*
> >> + * Active close step 2.
> >> + */
> >> + if (conn->tcp_state == TCPS_FIN_WAIT_1) {
> >> + conn->tcp_state = TCPS_TIME_WAIT;
> > ...this assignment to conn->tcp_state has no effect, because...
> >
> >> + /*
> >> + * For simplify implementation, we needn't wait 2MSL time
> >> + * in filter rewriter. Because guest kernel will track the
> >> + * TCP status and wait 2MSL time, if client resend the FIN
> >> + * packet, guest will apply the last ACK too.
> >> + */
> >> + conn->tcp_state = TCPS_CLOSED;
> > ...we immediately overwrite it with a different value.
> >
> >> + g_hash_table_remove(rf->connection_track_table, key);
> >> + }
> >> }
> > What was the intention of the code here?
> >
> > thanks
> > -- PMM
>
>
> Looks not.
>
> Chen, can you please send a patch to fix this?
>
>
Sure, I will send a patch for this issue.
Thanks
Zhang Chen
> Thanks
>
>
- [Qemu-devel] [PULL V2 00/26] Net patches, Jason Wang, 2018/10/18
- [Qemu-devel] [PULL V2 02/26] colo-compare: implement the process of checkpoint, Jason Wang, 2018/10/18
- [Qemu-devel] [PULL V2 04/26] COLO: integrate colo compare with colo frame, Jason Wang, 2018/10/18
- [Qemu-devel] [PULL V2 03/26] colo-compare: use notifier to notify packets comparing result, Jason Wang, 2018/10/18
- [Qemu-devel] [PULL V2 05/26] COLO: Add block replication into colo process, Jason Wang, 2018/10/18
- [Qemu-devel] [PULL V2 07/26] COLO: Load dirty pages into SVM's RAM cache firstly, Jason Wang, 2018/10/18
- [Qemu-devel] [PULL V2 06/26] COLO: Remove colo_state migration struct, Jason Wang, 2018/10/18
- [Qemu-devel] [PULL V2 08/26] ram/COLO: Record the dirty pages that SVM received, Jason Wang, 2018/10/18
- [Qemu-devel] [PULL V2 09/26] COLO: Flush memory data from ram cache, Jason Wang, 2018/10/18
- [Qemu-devel] [PULL V2 10/26] qmp event: Add COLO_EXIT event to notify users while exited COLO, Jason Wang, 2018/10/18
- [Qemu-devel] [PULL V2 11/26] qapi/migration.json: Rename COLO unknown mode to none mode., Jason Wang, 2018/10/18