Re: [PATCH net-next v8 0/4] send credit update during setting SO_RCVLOWAT

[Arseniy Krasnov <avkrasnov@salutedevices.com>]

On 13.12.2023 12:41, Stefano Garzarella wrote:
> On Wed, Dec 13, 2023 at 12:08:27PM +0300, Arseniy Krasnov wrote:
>>
>>
>> On 13.12.2023 11:43, Stefano Garzarella wrote:
>>> On Tue, Dec 12, 2023 at 08:43:07PM +0300, Arseniy Krasnov wrote:
>>>>
>>>>
>>>> On 12.12.2023 19:12, Michael S. Tsirkin wrote:
>>>>> On Tue, Dec 12, 2023 at 06:59:03PM +0300, Arseniy Krasnov wrote:
>>>>>>
>>>>>>
>>>>>> On 12.12.2023 18:54, Michael S. Tsirkin wrote:
>>>>>>> On Tue, Dec 12, 2023 at 12:16:54AM +0300, Arseniy Krasnov wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>>                                DESCRIPTION
>>>>>>>>
>>>>>>>> This patchset fixes old problem with hungup of both rx/tx sides and adds
>>>>>>>> test for it. This happens due to non-default SO_RCVLOWAT value and
>>>>>>>> deferred credit update in virtio/vsock. Link to previous old patchset:
>>>>>>>> https://lore.kernel.org/netdev/39b2e9fd-601b-189d-39a9-914e5574524c@sberdevices.ru/
>>>>>>>
>>>>>>>
>>>>>>> Patchset:
>>>>>>>
>>>>>>> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>>>>>>
>>>>>> Thanks!
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> But I worry whether we actually need 3/8 in net not in net-next.
>>>>>>
>>>>>> Because of "Fixes" tag ? I think this problem is not critical and reproducible
>>>>>> only in special cases, but i'm not familiar with netdev process so good, so I don't
>>>>>> have strong opinion. I guess @Stefano knows better.
>>>>>>
>>>>>> Thanks, Arseniy
>>>>>
>>>>> Fixes means "if you have that other commit then you need this commit
>>>>> too". I think as a minimum you need to rearrange patches to make the
>>>>> fix go in first. We don't want a regression followed by a fix.
>>>>
>>>> I see, ok, @Stefano WDYT? I think rearrange doesn't break anything, because this
>>>> patch fixes problem that is not related with the new patches from this patchset.
>>>
>>> I agree, patch 3 is for sure net material (I'm fine with both rearrangement or send it separately), but IMHO also patch 2 could be.
>>> I think with the same fixes tag, since before commit b89d882dc9fc ("vsock/virtio: reduce credit update messages") we sent a credit update
>>> for every bytes we read, so we should not have this problem, right?
>>
>> Agree for 2, so I think I can rearrange: two fixes go first, then current 0001, and then tests. And send it as V9 for 'net' only ?
> 
> Maybe you can add this to patch 1 if we want it on net:
> 
> Fixes: e38f22c860ed ("vsock: SO_RCVLOWAT transport set callback")
> 
> Then I think that patch should go before patch 2, so we don't need to
> touch that code multiple times.
> 
> so, IMHO the order should be the actual order or 3 - 1 - 2 - 4.
> 
> Another option is to send just 2 & 3 to net, and the rest (1 & 4) to net-next. IMHO should be fine to send the entire series to net with the fixes tag also in patch 1.

Ok, agree that it is good to send whole patchset to net without splitting it.

> 
> Net maintainers and Michael might have a different advice.

Ok

> 
> Thanks,
> Stefano
>