[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 2/2] colo: compare the packet based on the tc
From: |
Zhang Chen |
Subject: |
Re: [Qemu-devel] [PATCH v4 2/2] colo: compare the packet based on the tcp sequence number |
Date: |
Mon, 14 Jan 2019 18:39:55 +0800 |
On Mon, Jan 14, 2019, 6:34 PM Thomas Huth <address@hidden wrote:
> On 2017-12-25 03:54, Mao Zhongyi wrote:
> > Packet size some time different or when network is busy.
> > Based on same payload size, but TCP protocol can not
> > guarantee send the same one packet in the same way,
> [...]
> > Signed-off-by: Mao Zhongyi <address@hidden>
> > Signed-off-by: Li Zhijian <address@hidden>
> > Signed-off-by: Zhang Chen <address@hidden>
> > Reviewed-by: Zhang Chen <address@hidden>
> > Tested-by: Zhang Chen <address@hidden>
> > ---
> > net/colo-compare.c | 343
> +++++++++++++++++++++++++++++++++++------------------
> > net/colo.c | 9 ++
> > net/colo.h | 15 +++
> > net/trace-events | 2 +-
> > 4 files changed, 250 insertions(+), 119 deletions(-)
> >
> > diff --git a/net/colo-compare.c b/net/colo-compare.c
> > index f39ca02..8622b0b 100644
> > --- a/net/colo-compare.c
> > +++ b/net/colo-compare.c
> [...]
> > @@ -214,104 +254,175 @@ static int colo_compare_packet_payload(Packet
> *ppkt,
> > }
> >
> > /*
> > - * Called from the compare thread on the primary
> > - * for compare tcp packet
> > - * compare_tcp copied from Dr. David Alan Gilbert's branch
> > - */
> > -static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
> > + * return true means that the payload is consist and
> > + * need to make the next comparison, false means do
> > + * the checkpoint
> > +*/
> > +static bool colo_mark_tcp_pkt(Packet *ppkt, Packet *spkt,
> > + int8_t *mark, uint32_t max_ack)
> > {
> > - struct tcphdr *ptcp, *stcp;
> > - int res;
> > + *mark = 0;
> > +
> > + if (ppkt->tcp_seq == spkt->tcp_seq && ppkt->seq_end ==
> spkt->seq_end) {
> > + if (colo_compare_packet_payload(ppkt, spkt,
> > + ppkt->header_size,
> spkt->header_size,
> > + ppkt->payload_size)) {
> > + *mark = COLO_COMPARE_FREE_SECONDARY |
> COLO_COMPARE_FREE_PRIMARY;
> > + return true;
> > + }
> > + }
> > + if (ppkt->tcp_seq == spkt->tcp_seq && ppkt->seq_end ==
> spkt->seq_end) {
> > + if (colo_compare_packet_payload(ppkt, spkt,
> > + ppkt->header_size,
> spkt->header_size,
> > + ppkt->payload_size)) {
> > + *mark = COLO_COMPARE_FREE_SECONDARY |
> COLO_COMPARE_FREE_PRIMARY;
> > + return true;
> > + }
> > + }
>
> Hi,
>
> seems like this patch introduced some duplicated code, see this bug
> ticket here:
>
> https://bugs.launchpad.net/qemu/+bug/1811499
>
> Is the second if-statement here on purpose? If yes, maybe you could add
> a comment here? If no, could you please send a patch to remove it?
>
I think it is a rebase issue, I will send a patch to remove it.
Thanks
Zhang Chen
> Thanks,
> Thomas
>