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

[Jason Wang <jasowang@redhat.com>]

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 <zhangchen.fnst@cn.fujitsu.com>
>>>>> ---
>>>>>   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.

Thanks

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