| Message ID | 1615525383-59071-6-git-send-email-lei.rao@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixed some bugs and optimized some codes for COLO. | expand |
On 3/12/21 1:02 PM, leirao wrote: > From: "Rao, Lei" <lei.rao@intel.com> > > if we put the data copy outside the packet_new(), then for the > filter-rewrite module, there will be one less memory copy in the > processing of each network packet. > > Signed-off-by: Lei Rao <lei.rao@intel.com> > --- > net/colo-compare.c | 7 +++++-- > net/colo.c | 4 ++-- > net/colo.h | 2 +- > net/filter-rewriter.c | 1 - > 4 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c > index 9e18baa..8bdf5a8 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -247,14 +247,17 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con) > ConnectionKey key; > Packet *pkt = NULL; > Connection *conn; > + char *data = NULL; > int ret; > > if (mode == PRIMARY_IN) { > - pkt = packet_new(s->pri_rs.buf, > + data = g_memdup(s->pri_rs.buf, s->pri_rs.packet_len); > + pkt = packet_new(data, > s->pri_rs.packet_len, > s->pri_rs.vnet_hdr_len); > } else { > - pkt = packet_new(s->sec_rs.buf, > + data = g_memdup(s->sec_rs.buf, s->sec_rs.packet_len); > + pkt = packet_new(data, > s->sec_rs.packet_len, > s->sec_rs.vnet_hdr_len); > } > diff --git a/net/colo.c b/net/colo.c > index ef00609..08fb37e 100644 > --- a/net/colo.c > +++ b/net/colo.c > @@ -155,11 +155,11 @@ void connection_destroy(void *opaque) > g_slice_free(Connection, conn); > } > > -Packet *packet_new(const void *data, int size, int vnet_hdr_len) > +Packet *packet_new(void *data, int size, int vnet_hdr_len) > { > Packet *pkt = g_slice_new(Packet); > > - pkt->data = g_memdup(data, size); > + pkt->data = data; if so, should packet_destroy() free() data which may be not alloc by itself Thanks Zhijian
How about redefine a function named packet_new_nocopy? In comments, we can tell the caller don't release the buffer and the packet_destroy will release it. Thanks, Lei. -----Original Message----- From: lizhijian@fujitsu.com <lizhijian@fujitsu.com> Sent: Friday, March 12, 2021 2:53 PM To: Rao, Lei <lei.rao@intel.com>; Zhang, Chen <chen.zhang@intel.com>; jasowang@redhat.com; quintela@redhat.com; dgilbert@redhat.com; pbonzini@redhat.com; lukasstraub2@web.de Cc: qemu-devel@nongnu.org Subject: Re: [PATCH v2 05/10] Optimize the function of packet_new On 3/12/21 1:02 PM, leirao wrote: > From: "Rao, Lei" <lei.rao@intel.com> > > if we put the data copy outside the packet_new(), then for the > filter-rewrite module, there will be one less memory copy in the > processing of each network packet. > > Signed-off-by: Lei Rao <lei.rao@intel.com> > --- > net/colo-compare.c | 7 +++++-- > net/colo.c | 4 ++-- > net/colo.h | 2 +- > net/filter-rewriter.c | 1 - > 4 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c index > 9e18baa..8bdf5a8 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -247,14 +247,17 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con) > ConnectionKey key; > Packet *pkt = NULL; > Connection *conn; > + char *data = NULL; > int ret; > > if (mode == PRIMARY_IN) { > - pkt = packet_new(s->pri_rs.buf, > + data = g_memdup(s->pri_rs.buf, s->pri_rs.packet_len); > + pkt = packet_new(data, > s->pri_rs.packet_len, > s->pri_rs.vnet_hdr_len); > } else { > - pkt = packet_new(s->sec_rs.buf, > + data = g_memdup(s->sec_rs.buf, s->sec_rs.packet_len); > + pkt = packet_new(data, > s->sec_rs.packet_len, > s->sec_rs.vnet_hdr_len); > } > diff --git a/net/colo.c b/net/colo.c > index ef00609..08fb37e 100644 > --- a/net/colo.c > +++ b/net/colo.c > @@ -155,11 +155,11 @@ void connection_destroy(void *opaque) > g_slice_free(Connection, conn); > } > > -Packet *packet_new(const void *data, int size, int vnet_hdr_len) > +Packet *packet_new(void *data, int size, int vnet_hdr_len) > { > Packet *pkt = g_slice_new(Packet); > > - pkt->data = g_memdup(data, size); > + pkt->data = data; if so, should packet_destroy() free() data which may be not alloc by itself Thanks Zhijian
> + offset = colo_bitmap_find_dirty(ram_state, block, offset, > + &num); IIUC, this return value would pass to the next round as start index, so you should skip the already checked one. Thanks On 3/12/21 5:56 PM, Rao, Lei wrote: > How about redefine a function named packet_new_nocopy? > In comments, we can tell the caller don't release the buffer and the packet_destroy will release it. > > Thanks, > Lei. > -----Original Message----- > From:lizhijian@fujitsu.com <lizhijian@fujitsu.com> > Sent: Friday, March 12, 2021 2:53 PM > To: Rao, Lei<lei.rao@intel.com>; Zhang, Chen<chen.zhang@intel.com>;jasowang@redhat.com;quintela@redhat.com;dgilbert@redhat.com;pbonzini@redhat.com;lukasstraub2@web.de > Cc:qemu-devel@nongnu.org > Subject: Re: [PATCH v2 05/10] Optimize the function of packet_new > >
Oh, I understand what you mean, and will change it in V3. Thanks, Lei. -----Original Message----- From: lizhijian@fujitsu.com <lizhijian@fujitsu.com> Sent: Friday, March 12, 2021 6:23 PM To: Rao, Lei <lei.rao@intel.com>; Zhang, Chen <chen.zhang@intel.com>; jasowang@redhat.com; quintela@redhat.com; dgilbert@redhat.com; pbonzini@redhat.com; lukasstraub2@web.de Cc: qemu-devel@nongnu.org Subject: Re: [PATCH v2 05/10] Optimize the function of packet_new > + offset = colo_bitmap_find_dirty(ram_state, block, offset, > + &num); IIUC, this return value would pass to the next round as start index, so you should skip the already checked one. Thanks On 3/12/21 5:56 PM, Rao, Lei wrote: > How about redefine a function named packet_new_nocopy? > In comments, we can tell the caller don't release the buffer and the packet_destroy will release it. > > Thanks, > Lei. > -----Original Message----- > From:lizhijian@fujitsu.com <lizhijian@fujitsu.com> > Sent: Friday, March 12, 2021 2:53 PM > To: Rao, Lei<lei.rao@intel.com>; Zhang, > Chen<chen.zhang@intel.com>;jasowang@redhat.com;quintela@redhat.com;dgi > lbert@redhat.com;pbonzini@redhat.com;lukasstraub2@web.de > Cc:qemu-devel@nongnu.org > Subject: Re: [PATCH v2 05/10] Optimize the function of packet_new > >
diff --git a/net/colo-compare.c b/net/colo-compare.c index 9e18baa..8bdf5a8 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -247,14 +247,17 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con) ConnectionKey key; Packet *pkt = NULL; Connection *conn; + char *data = NULL; int ret; if (mode == PRIMARY_IN) { - pkt = packet_new(s->pri_rs.buf, + data = g_memdup(s->pri_rs.buf, s->pri_rs.packet_len); + pkt = packet_new(data, s->pri_rs.packet_len, s->pri_rs.vnet_hdr_len); } else { - pkt = packet_new(s->sec_rs.buf, + data = g_memdup(s->sec_rs.buf, s->sec_rs.packet_len); + pkt = packet_new(data, s->sec_rs.packet_len, s->sec_rs.vnet_hdr_len); } diff --git a/net/colo.c b/net/colo.c index ef00609..08fb37e 100644 --- a/net/colo.c +++ b/net/colo.c @@ -155,11 +155,11 @@ void connection_destroy(void *opaque) g_slice_free(Connection, conn); } -Packet *packet_new(const void *data, int size, int vnet_hdr_len) +Packet *packet_new(void *data, int size, int vnet_hdr_len) { Packet *pkt = g_slice_new(Packet); - pkt->data = g_memdup(data, size); + pkt->data = data; pkt->size = size; pkt->creation_ms = qemu_clock_get_ms(QEMU_CLOCK_HOST); pkt->vnet_hdr_len = vnet_hdr_len; diff --git a/net/colo.h b/net/colo.h index 573ab91..bd2d719 100644 --- a/net/colo.h +++ b/net/colo.h @@ -100,7 +100,7 @@ Connection *connection_get(GHashTable *connection_track_table, bool connection_has_tracked(GHashTable *connection_track_table, ConnectionKey *key); void connection_hashtable_reset(GHashTable *connection_track_table); -Packet *packet_new(const void *data, int size, int vnet_hdr_len); +Packet *packet_new(void *data, int size, int vnet_hdr_len); void packet_destroy(void *opaque, void *user_data); void packet_destroy_partial(void *opaque, void *user_data); diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c index 10fe393..599f0c3 100644 --- a/net/filter-rewriter.c +++ b/net/filter-rewriter.c @@ -271,7 +271,6 @@ static ssize_t colo_rewriter_receive_iov(NetFilterState *nf, } pkt = packet_new(buf, size, vnet_hdr_len); - g_free(buf); /* * if we get tcp packet