On Tue, Oct 30, 2018 at 10:02 AM Jason Wang <jasow...@redhat.com> wrote:
> > On 2018/10/29 下午7:01, Peter Maydell wrote: > > On 19 October 2018 at 04:22, Jason Wang <jasow...@redhat.com> wrote: > >> From: Zhang Chen <zhangc...@gmail.com> > >> > >> 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 > >