Re: [PATCH net-next v4 1/2] net: hsr: require valid EOT supervision TLV

[Luka Gejak <luka.gejak@linux.dev>]

On April 1, 2026 3:44:16 PM GMT+02:00, Fernando Fernandez Mancera <fmancera@suse.de> wrote:
>On 4/1/26 3:31 PM, Luka Gejak wrote:
>> On April 1, 2026 2:05:49 PM GMT+02:00, Fernando Fernandez Mancera <fmancera@suse.de> wrote:
>>> On 4/1/26 1:06 PM, Luka Gejak wrote:
>>>> On April 1, 2026 11:52:02 AM GMT+02:00, Fernando Fernandez Mancera <fmancera@suse.de> wrote:
>>>>> On 4/1/26 11:23 AM, luka.gejak@linux.dev wrote:
>>>>>> From: Luka Gejak <luka.gejak@linux.dev>
>>>>>> 
>>>>>> Supervision frames are only valid if terminated with a zero-length EOT
>>>>>> TLV. The current check fails to reject non-EOT entries as the terminal
>>>>>> TLV, potentially allowing malformed supervision traffic.
>>>>>> 
>>>>>> Fix this by strictly requiring the terminal TLV to be HSR_TLV_EOT
>>>>>> with a length of zero.
>>>>>> 
>>>>>> Reviewed-by: Felix Maurer <fmaurer@redhat.com>
>>>>>> Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
>>>>>> ---
>>>>> 
>>>>> Hi,
>>>>> 
>>>>> This has not been reviewed by Felix. Felix provided his Reviewed-by tag for the v1 which was completely different than this.
>>>>> 
>>>>> Revisions of this patch:
>>>>> 
>>>>> v3: https://lore.kernel.org/all/20260329112313.17164-4-luka.gejak@linux.dev/
>>>>> 
>>>>> v2: https://lore.kernel.org/all/20260326154715.38405-4-luka.gejak@linux.dev/
>>>>> 
>>>>> v1: https://lore.kernel.org/all/20260324143503.187642-4-luka.gejak@linux.dev/
>>>>> 
>>>>> Are these contributions LLM/AI generated? I believe so based on the email history.
>>>>> 
>>>>> AI generated review on rtl8723bs: https://lore.kernel.org/all/B2394A3C-25FD-4CEA-8557-3E68F1F60357@linux.dev/
>>>>> 
>>>>> Another AI generated review on rtl8723bs: https://lore.kernel.org/all/3831D599-655E-40B2-9E5D-9DF956013088@linux.dev/
>>>>> 
>>>>> Likely an AI generated review on a 1 year old HSR patch: https://lore.kernel.org/all/DHFG26KI6L23.1YCOVQ5SSYMO5@linux.dev/
>>>>> 
>>>>> If these are indeed, AI generated contributions or reviews they should be disclosed beforehand. Also there is the Assisted-by: tag. Also note that developer must take full responsibility for the contribution which means understanding it completely.
>>>>> 
>>>>> https://docs.kernel.org/process/coding-assistants.html#signed-off-by-and-developer-certificate-of-origin
>>>>> 
>>>>> Thanks,
>>>>> Fernando.
>>>> 
>>>> Hi Fernando,
>>>> About the Reviewed-by tag, that was my mistake. I forgot to remove it
>>>> when rebasing. And about AI. I’ve been using it to help format my
>>>> emails and translate them into English since it isn't my native
>>>> language. However, the technical logic and the code itself are my own
>>>> work, written without AI.
>>> 
>>> So I don't think this change in code is related to rebasing at all. The code in this function is identical when comparing net and net-next tree so why a rebase would cause such change in the code? Am I missing something here?
>>> 
>>> In addition to that, the new logic does not make any sense. What motivated the diff from v3 to v4? All that information is missing.
>>> 
>>> Thanks,
>>> Fernando.
>>> 
>>> Should I send v5, and if so, what should I
>>>> do besides stripping the Reviewed-by tag? I've read the documentation
>>>> on coding assistants you linked and will make sure to follow it on the
>>>> next revision.
>>>> Best regards,
>>>> Luka Gejak
>>>> 
>>> 
>> 
>> Hi Fernando,
>> These 2 patches were in the same patch series as 2 other patches.
>> Then, when rebasing to edit 3rd patch in original patch series(3/4), I
>> forgot to remove the Reviewed-by tag(I ran git commit --amend
>> --no-edit). Then I separated 1st 2 and 2nd 2 patches into separate
>> series (one for net and other for net-next) because I was instructed
>> to do so by Jakub Kicinski. His email:
>>> I think that patches 1 and 2 need to go to net with a Fixes tag.
>>> They look like run of the mill bug fixes. 3 and 4 are logical
>>> fixes and change behavior so net-next makes sense.
>>> 
>
>This is not addressing my concern. I am not complaining that it went into a separate series. That makes sense. The v3 version of the patch "net: hsr: require valid EOT supervision TLV" is now something completely different but the commit message is the same.
>
>v3: https://lore.kernel.org/all/20260329112313.17164-4-luka.gejak@linux.dev/
>
>I could guess the reason is that in order to make sure the last TLV is a EOT TLV with 0 length but all that explanation is missing on the commit message.
>
>>> FWIW AI has something to say about patch 3, I did not investigate:
>>> https://sashiko.dev/#/patchset/20260329112313.17164-2-luka.gejak@linux.dev
>>> -- 
>>> pw-bot: cr
>

Hi Fernando,
I apologize for the confusion. Now I get what you were talking about.
The commit message in v4 didn't reflect the logic change from v3. I 
will update commit message accordingly. The motivation for the "TLV 
walker" refactor in v4 was to fix two issues in v3: linearization/
paged frames and forward compatibility.
Best regards,
Luka Gejak