RE: [PATCH v3 1/2] igb: RX descriptors handling cleanup

[Tomasz Dzieciol/VIM Integration (NC) /SRPOL/Engineer/Samsung Electronics]

I will remove checking RXCSUM.IPPCSE flag then.

-----Original Message-----
From: Akihiko Odaki <akihiko.odaki@daynix.com> 
Sent: środa, 3 maja 2023 17:11
To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>; Tomasz Dzieciol/VIM 
Integration (NC) /SRPOL/Engineer/Samsung Electronics 
<t.dzieciol@partner.samsung.com>; qemu-devel@nongnu.org
Cc: jasowang@redhat.com; k.kwiecien@samsung.com; m.sochacki@samsung.com
Subject: Re: [PATCH v3 1/2] igb: RX descriptors handling cleanup

On 2023/05/03 16:46, Sriram Yagnaraman wrote:
> 
> 
>> -----Original Message-----
>> From: Tomasz Dzieciol/VIM Integration (NC) /SRPOL/Engineer/Samsung 
>> Electronics <t.dzieciol@partner.samsung.com>
>> Sent: Tuesday, 2 May 2023 16:01
>> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>; qemu- 
>> devel@nongnu.org; akihiko.odaki@daynix.com
>> Cc: jasowang@redhat.com; k.kwiecien@samsung.com; 
>> m.sochacki@samsung.com
>> Subject: RE: [PATCH v3 1/2] igb: RX descriptors handling cleanup
>>
>> Not Linux/DPDK/FreeBSD for IGB.
>>
>> Change here adds additional condition (RXCSUM.IPPCSE set) to enable 
>> putting IP ID into descriptor, besides clearing RXCSUM.PCSD (required 
>> according to Intel 82576 datasheet) that was not present in the e1000e code.
>>
> 
> Yes, we can't even use ethtool to set this field.
> My suggestion is to not add/maintain code that we cannot test. I leave it up 
> to Akhikho to decide if we really need to implement IPPCSE.
> The default value of RXCSUM.IPPCSE is unset, so we could as well ignore this 
> field until there is a user who sets this.

In general I won't reject a patch to implement a feature not used by a known 
guest, but I don't recommend that. It just doesn't make sense to spend time to 
write code that can turn out so buggy that it is unusable in practice, which is 
often the case with untested code.