Re: [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support.

[Zhang Chen]

On 05/05/2017 05:25 PM, Jason Wang wrote:
> On 2017年05月05日 16:44, Zhang Chen wrote:
> > On 05/03/2017 06:19 PM, Jason Wang wrote:
> > > On 2017年05月03日 11:18, Zhang Chen wrote:
> > > > On 05/02/2017 12:47 PM, Jason Wang wrote:
> > > > > On 2017年04月28日 17:47, Zhang Chen wrote:
> > > > > > In this patch, we change the send packet format from
> > > > > > struct {int size; const uint8_t buf[];} to {int size; int 
> > > > > > vnet_hdr_len; const uint8_t buf[];}.
> > > > > > make other module(like colo-compare) know how to parse net packet 
> > > > > > correctly.
> > > > > >
> > > > > > Signed-off-by: Zhang Chen <address@hidden>
> > > > > > ---
> > > > > >   net/filter-mirror.c | 28 +++++++++++++++++++++++-----
> > > > > >   1 file changed, 23 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> > > > > > index 72fa7c2..bb9ecf3 100644
> > > > > > --- a/net/filter-mirror.c
> > > > > > +++ b/net/filter-mirror.c
> > > > > > @@ -43,12 +43,14 @@ typedef struct MirrorState {
> > > > > >       SocketReadState rs;
> > > > > >   } MirrorState;
> > > > > >   -static int filter_mirror_send(CharBackend *chr_out,
> > > > > > +static int filter_mirror_send(MirrorState *s,
> > > > > >                                 const struct iovec *iov,
> > > > > >                                 int iovcnt)
> > > > > >   {
> > > > > > +    NetFilterState *nf = NETFILTER(s);
> > > > > >       int ret = 0;
> > > > > >       ssize_t size = 0;
> > > > > > +    ssize_t vnet_hdr_len;
> > > > > >       uint32_t len = 0;
> > > > > >       char *buf;
> > > > > >   @@ -58,14 +60,30 @@ static int filter_mirror_send(CharBackend 
> > > > > > *chr_out,
> > > > > >       }
> > > > > >         len = htonl(size);
> > > > > > -    ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)&len, 
> > > > > > sizeof(len));
> > > > > > +    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, 
> > > > > > sizeof(len));
> > > > > > +    if (ret != sizeof(len)) {
> > > > > > +        goto err;
> > > > > > +    }
> > > > > > +
> > > > > > +    /*
> > > > > > +     * We send vnet header len make other module(like colo-compare)
> > > > > > +     * know how to parse net packet correctly.
> > > > > > +     */
> > > > > > +    if (qemu_get_using_vnet_hdr(nf->netdev)) {
> > > > > > +        vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev);
> > > > > > +    } else {
> > > > > > +        vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev->peer);
> > > > > > +    }
> > > > >
> > > > > Any reason to query peer here?
> > > >
> > > > That's depend on using NetClientState, If we using nf->netdev that 
> > > > need to query,
> > > > Otherwise we query nf->netdev->peer, then we can get the real 
> > > > vnet_hdr_len in my test.
> > > >
> > > > Thanks
> > > > Zhang Chen 
> > >
> > > Confused, I think nf->netdev won't be a nic?
> >
> > I don't know whether I fully understand.
> > I think it's depend on the sender, we must query sender to get real 
> > vnet_hdr_len ,
> > like that in filter.c:
> >
> >         if (sender == nf->netdev) {
> >             /* This packet is sent by netdev itself */
> >             direction = NET_FILTER_DIRECTION_TX;
> >         } else {
> >             direction = NET_FILTER_DIRECTION_RX;
> >         }
> >
> > Thanks
> > Zhang Chen
>
> The problem is nf->netdev->peer should be a nic. But we don't care 
> about its vnet_hdr_len. Take virtio-net as an example, we only care 
> about host_hdr_len, since guest will strip the part that host does not 
> care.

Yes, you are right. In anytime, nf->netdev and nf->netdev->peer both 
have a vnet_hdr_len,
Here we just find out which is we need. When filter set RX or TX that 
the real vnet_hdr_len are different.
We need query different netdev to get in my test.

Thanks
Zhang Chen

> Thanks
>
> > > Thanks
> > >
> > >
> > > .
>
>
>
> .

--
Thanks
Zhang Chen