Re: [PATCH v9 16/20] virtio-net: Do not write hashes to peer buffer

[Akihiko Odaki <akihiko.odaki@daynix.com>]

On 2024/04/08 17:06, Yuri Benditovich wrote:
> On Mon, Apr 8, 2024 at 10:57 AM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>>
>> On 2024/04/08 16:54, Yuri Benditovich wrote:
>>> On Mon, Apr 8, 2024 at 10:42 AM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>>>>
>>>> On 2024/04/08 16:40, Yuri Benditovich wrote:
>>>>> On Mon, Apr 8, 2024 at 4:30 AM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>>>>>>
>>>>>> On 2024/04/08 7:09, Yuri Benditovich wrote:
>>>>>>> On Wed, Apr 3, 2024 at 2:12 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>>>>>>>>
>>>>>>>> The peer buffer is qualified with const and not meant to be modified.
>>>>>>>
>>>>>>> IMHO, this buffer is not so 'const' (although the prototype states so),
>>>>>>> it is allocated in net.c
>>>>>>> btw, another procedure in this file also modifies the buffer
>>>>>>> (work_around_broken_dhclient)
>>>>>>
>>>>>> Right but it has a FIXME comment.
>>>>>>
>>>>>>>
>>>>>>>> It also prevents enabling VIRTIO_NET_F_HASH_REPORT for peers without
>>>>>>>> virtio-net header support.
>>>>>>>
>>>>>>> Does it mean _this commit_ prevents enabling VIRTIO_NET_F_HASH_REPORT
>>>>>>> for peers without
>>>>>>> virtio-net header support? Where?
>>>>>>
>>>>>> No, but I meant that this patch fixes such a problem.
>>>>>
>>>>> No, it does not. Such a problem does not exist in the master, the
>>>>> hash_report feature
>>>>> is silently dropped in such case:
>>>>> https://github.com/qemu/qemu/blob/master/hw/net/virtio-net.c#L816
>>>>
>>>> Well, silently dropping VIRTIO_NET_F_HASH_REPORT is not different from
>>>> preventing enabling VIRTIO_NET_F_HASH_REPORT, is it?
>>>>
>>> But how is your patch involved in it? Should this line be removed from
>>> the commit message?
>>
>> In the master, VIRTIO_NET_F_HASH_REPORT is silently dropped, but this
>> patch will change to work without dropping it, which is worth to mention.
> After applying this series of patches the VIRTIO_NET_F_HASH_REPORT is
> dropped _the same way_ as in the master

You are right. I forgot that I dropped patch "virtio-net: Do not clear 
VIRTIO_NET_F_HASH_REPORT" with v7. I'll drop the line in the next 
version accordingly. Thanks for pointing out that.

Regards,
Akihiko Odaki