RE: [PATCH] net/filter: Enable the vnet_hdr_support by default

[Zhang, Chen]

> -----Original Message-----
> From: Jason Wang <jasowang@redhat.com>
> Sent: Friday, November 19, 2021 11:37 AM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: qemu-dev <qemu-devel@nongnu.org>; Markus Armbruster
> <armbru@redhat.com>; Li Zhijian <lizhijian@cn.fujitsu.com>
> Subject: Re: [PATCH] net/filter: Enable the vnet_hdr_support by default
> 
> On Fri, Nov 19, 2021 at 11:17 AM Jason Wang <jasowang@redhat.com>
> wrote:
> >
> >
> > 在 2021/11/10 上午10:39, Zhang Chen 写道:
> > > This patch make filters and colo-compare module support vnet_hdr by
> > > default. And also support -device non-virtio-net(like e1000.).
> > > But it can't avoid user manual configuration error between different
> > > filters when enable/disable virtio-net-pci.
> > >
> > > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > > ---
> >
> >
> > Applied.
> >
> > Thanks
> 
> Actually not, it breaks filter-mirror test:
> 
> 
> ERROR:../tests/qtest/test-filter-mirror.c:74:test_mirror: assertion failed
> (recv_buf == send_buf): ("" == "Hello! filter-mirror~") ERROR qtest-
> x86_64/test-filter-mirror - Bail out!
> ERROR:../tests/qtest/test-filter-mirror.c:74:test_mirror: assertion failed
> (recv_buf == send_buf): ("" == "Hello! filter-mirror~")
> ^Cmake[1]: *** [Makefile.mtest:784: run-test-96] Interrupt

Oh, This test used default way to send filter data without vnet_hdr_len.
But we changed default filter transfer protocol. I will fix it in another patch.

Thanks
Chen

> 
> Thanks
> 
> >
> >
> > >   net/colo-compare.c    | 2 +-
> > >   net/filter-mirror.c   | 4 ++--
> > >   net/filter-rewriter.c | 2 +-
> > >   qemu-options.hx       | 9 +++++----
> > >   4 files changed, 9 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/net/colo-compare.c b/net/colo-compare.c index
> > > b8876d7fd9..82d4d81710 100644
> > > --- a/net/colo-compare.c
> > > +++ b/net/colo-compare.c
> > > @@ -1397,7 +1397,7 @@ static void colo_compare_init(Object *obj)
> > >                           get_max_queue_size,
> > >                           set_max_queue_size, NULL, NULL);
> > >
> > > -    s->vnet_hdr = false;
> > > +    s->vnet_hdr = true;
> > >       object_property_add_bool(obj, "vnet_hdr_support",
> compare_get_vnet_hdr,
> > >                                compare_set_vnet_hdr);
> > >   }
> > > diff --git a/net/filter-mirror.c b/net/filter-mirror.c index
> > > f20240cc9f..adb0c6d89a 100644
> > > --- a/net/filter-mirror.c
> > > +++ b/net/filter-mirror.c
> > > @@ -406,14 +406,14 @@ static void filter_mirror_init(Object *obj)
> > >   {
> > >       MirrorState *s = FILTER_MIRROR(obj);
> > >
> > > -    s->vnet_hdr = false;
> > > +    s->vnet_hdr = true;
> > >   }
> > >
> > >   static void filter_redirector_init(Object *obj)
> > >   {
> > >       MirrorState *s = FILTER_REDIRECTOR(obj);
> > >
> > > -    s->vnet_hdr = false;
> > > +    s->vnet_hdr = true;
> > >   }
> > >
> > >   static void filter_mirror_fini(Object *obj) diff --git
> > > a/net/filter-rewriter.c b/net/filter-rewriter.c index
> > > bf05023dc3..5698cd39d1 100644
> > > --- a/net/filter-rewriter.c
> > > +++ b/net/filter-rewriter.c
> > > @@ -407,7 +407,7 @@ static void filter_rewriter_init(Object *obj)
> > >   {
> > >       RewriterState *s = FILTER_REWRITER(obj);
> > >
> > > -    s->vnet_hdr = false;
> > > +    s->vnet_hdr = true;
> > >       s->failover_mode = FAILOVER_MODE_OFF;
> > >   }
> > >
> > > diff --git a/qemu-options.hx b/qemu-options.hx index
> > > 7749f59300..c40e385ede 100644
> > > --- a/qemu-options.hx
> > > +++ b/qemu-options.hx
> > > @@ -4967,13 +4967,13 @@ SRST
> > >       ``-object filter-
> mirror,id=id,netdev=netdevid,outdev=chardevid,queue=all|rx|tx[,vnet_hdr
> _support][,position=head|tail|id=<id>][,insert=behind|before]``
> > >           filter-mirror on netdev netdevid,mirror net packet to
> > >           chardevchardevid, if it has the vnet\_hdr\_support flag,
> > > -        filter-mirror will mirror packet with vnet\_hdr\_len.
> > > +        filter-mirror will mirror packet with vnet\_hdr\_len(default: 
> > > on).
> > >
> > >       ``-object filter-
> redirector,id=id,netdev=netdevid,indev=chardevid,outdev=chardevid,queu
> e=all|rx|tx[,vnet_hdr_support][,position=head|tail|id=<id>][,insert=behind
> |before]``
> > >           filter-redirector on netdev netdevid,redirect filter's net
> > >           packet to chardev chardevid,and redirect indev's packet to
> > >           filter.if it has the vnet\_hdr\_support flag, filter-redirector
> > > -        will redirect packet with vnet\_hdr\_len. Create a
> > > +        will redirect packet with vnet\_hdr\_len(default: on).
> > > + Create a
> > >           filter-redirector we need to differ outdev id from indev id, id
> > >           can not be the same. we can just use indev or outdev, but at
> > >           least one of indev or outdev need to be specified.
> > > @@ -4983,7 +4983,8 @@ SRST
> > >           packet to secondary from primary to keep secondary tcp
> > >           connection,and rewrite tcp packet to primary from secondary make
> > >           tcp packet can be handled by client.if it has the
> > > -        vnet\_hdr\_support flag, we can parse packet with vnet header.
> > > +        vnet\_hdr\_support flag, we can parse packet with vnet
> > > +        header(default: on).
> > >
> > >           usage: colo secondary: -object
> > >           filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0
> > > -object @@ -5004,7 +5005,7 @@ SRST
> > >           checkpoint and send primary packet to out\_dev. In order to
> > >           improve efficiency, we need to put the task of comparison in
> > >           another iothread. If it has the vnet\_hdr\_support flag,
> > > -        colo compare will send/recv packet with vnet\_hdr\_len.
> > > +        colo compare will send/recv packet with vnet\_hdr\_len(default:
> on).
> > >           The compare\_timeout=@var{ms} determines the maximum time
> of the
> > >           colo-compare hold the packet. The expired\_scan\_cycle=@var{ms}
> > >           is to set the period of scanning expired primary node network
> packets.