[PATCH net-next v4 0/2] net: hsr: strict supervision TLV validation

[PATCH net-next v4 0/2] net: hsr: strict supervision TLV validation

[luka.gejak]

From: Luka Gejak <luka.gejak@linux.dev>

This series improves the robustness of HSR supervision frame parsing. 
It enforces strict supervision frame TLV validation and improves Netlink
error reporting for invalid interlink attributes.

These were previously part of a 4-patch set. The first two patches 
(fixes) have been sent separately to the 'net' tree.

Changes in v4:
 - Split from a 4-patch series into 'net' and 'net-next' as requested.
 - Implemented a TLV walker in Patch 1 to correctly handle extension
   TLVs and avoid regressions on paged frames/non-linearized skbs.
 - Corrected pskb_may_pull() logic to include the TLV header size.

History of pre-separation series (v1-v3):
Changes in v3:
 - addressed Felix review feedback in the VLAN add unwind fix
 - removed the superfluous empty line

Changes in v2:
 - picked up Reviewed-by tags on patches 1, 3 and 4
 - changes in patch 2 per advice of Felix Maurer

Luka Gejak (2):
  net: hsr: require valid EOT supervision TLV
  net: hsr: reject unresolved interlink ifindex

 net/hsr/hsr_forward.c | 41 ++++++++++++++++++++++-------------------
 net/hsr/hsr_netlink.c |  7 ++++++-
 2 files changed, 28 insertions(+), 20 deletions(-)

-- 
2.53.0

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

[luka.gejak]

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>
---
 net/hsr/hsr_forward.c | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
index 0aca859c88cb..17b705235c4a 100644
--- a/net/hsr/hsr_forward.c
+++ b/net/hsr/hsr_forward.c
@@ -82,39 +82,42 @@ static bool is_supervision_frame(struct hsr_priv *hsr, struct sk_buff *skb)
 	    hsr_sup_tag->tlv.HSR_TLV_length != sizeof(struct hsr_sup_payload))
 		return false;
 
-	/* Get next tlv */
+	/* Advance past the first TLV payload to reach next TLV header */
 	total_length += hsr_sup_tag->tlv.HSR_TLV_length;
-	if (!pskb_may_pull(skb, total_length))
+	/* Linearize next TLV header before access */
+	if (!pskb_may_pull(skb, total_length + sizeof(struct hsr_sup_tlv)))
 		return false;
 	skb_pull(skb, total_length);
 	hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
 	skb_push(skb, total_length);
 
-	/* if this is a redbox supervision frame we need to verify
-	 * that more data is available
+	/* Walk through TLVs to find end-of-TLV marker, skipping any unknown
+	 * extension TLVs to maintain forward compatibility.
 	 */
-	if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
-		/* tlv length must be a length of a mac address */
-		if (hsr_sup_tlv->HSR_TLV_length != sizeof(struct hsr_sup_payload))
-			return false;
+	for (;;) {
+		if (hsr_sup_tlv->HSR_TLV_type == HSR_TLV_EOT &&
+		    hsr_sup_tlv->HSR_TLV_length == 0)
+			return true;
 
-		/* make sure another tlv follows */
-		total_length += sizeof(struct hsr_sup_tlv) + hsr_sup_tlv->HSR_TLV_length;
-		if (!pskb_may_pull(skb, total_length))
+		/* Validate known TLV types */
+		if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
+			if (hsr_sup_tlv->HSR_TLV_length !=
+			    sizeof(struct hsr_sup_payload))
+				return false;
+		}
+
+		/* Advance past current TLV: header + payload */
+		total_length += sizeof(struct hsr_sup_tlv) +
+				hsr_sup_tlv->HSR_TLV_length;
+		/* Linearize next TLV header before access */
+		if (!pskb_may_pull(skb,
+				   total_length + sizeof(struct hsr_sup_tlv)))
 			return false;
 
-		/* get next tlv */
 		skb_pull(skb, total_length);
 		hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
 		skb_push(skb, total_length);
 	}
-
-	/* end of tlvs must follow at the end */
-	if (hsr_sup_tlv->HSR_TLV_type == HSR_TLV_EOT &&
-	    hsr_sup_tlv->HSR_TLV_length != 0)
-		return false;
-
-	return true;
 }
 
 static bool is_proxy_supervision_frame(struct hsr_priv *hsr,
-- 
2.53.0

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

[Fernando Fernandez Mancera]

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.

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

[Luka Gejak]

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. 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

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

[Fernando Fernandez Mancera]

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
> 

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

[Luka Gejak]

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.
>
>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

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

[Fernando Fernandez Mancera]

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

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

[Luka Gejak]

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

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

[Luka Gejak]

On Wed Apr 1, 2026 at 11:52 AM CEST, Fernando Fernandez Mancera 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,
One more question, should I include Assisted-by tag in v5 if AI was not 
used for writing code but only for formating and translation of the 
emails to English as I previously mentioned.

Best regards,
Luka Gejak

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

[Fernando Fernandez Mancera]

On 4/1/26 7:05 PM, Luka Gejak wrote:
> On Wed Apr 1, 2026 at 11:52 AM CEST, Fernando Fernandez Mancera 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,
> One more question, should I include Assisted-by tag in v5 if AI was not
> used for writing code but only for formating and translation of the
> emails to English as I previously mentioned.
> 

I think yes, you should. I also think that AI was actually used for the 
generated code and also for spotting the valid and invalid issues. If 
you don't have a real environment for HSR, why would you look into it?

Sorry, it is not my intention to be harsh but there are some things that 
don't add up for me. Maybe I am being too careful here and you just 
coincidentally found these problems.

> Best regards,
> Luka Gejak
> 

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

[Luka Gejak]

On April 2, 2026 1:30:57 AM GMT+02:00, Fernando Fernandez Mancera <fmancera@suse.de> wrote:
>On 4/1/26 7:05 PM, Luka Gejak wrote:
>> On Wed Apr 1, 2026 at 11:52 AM CEST, Fernando Fernandez Mancera 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,
>> One more question, should I include Assisted-by tag in v5 if AI was not
>> used for writing code but only for formating and translation of the
>> emails to English as I previously mentioned.
>> 
>
>I think yes, you should. I also think that AI was actually used for the generated code and also for spotting the valid and invalid issues. If you don't have a real environment for HSR, why would you look into it?
>
>Sorry, it is not my intention to be harsh but there are some things that don't add up for me. Maybe I am being too careful here and you just coincidentally found these problems.
>
>> Best regards,
>> Luka Gejak
>> 
>

Hi Fernando,
I understand the concern. To clarify, I am a student, and I discovered
these issues while reading the HSR source to learn about network 
redundancy. Since I lack industrial hardware, I verified the logic 
using network namespaces and veth pairs on my Arch laptop. The code 
and logic are entirely my. As previously mentioned, I only use AI for 
English translation and formatting to ensure my communication is 
clear. As requested, I will wait for Felix's review before proceeding 
with any changes for v5.
Best regards,
Luka Gejak

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

[Fernando Fernandez Mancera]

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>
> ---
>   net/hsr/hsr_forward.c | 41 ++++++++++++++++++++++-------------------
>   1 file changed, 22 insertions(+), 19 deletions(-)
> 
> diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
> index 0aca859c88cb..17b705235c4a 100644
> --- a/net/hsr/hsr_forward.c
> +++ b/net/hsr/hsr_forward.c
> @@ -82,39 +82,42 @@ static bool is_supervision_frame(struct hsr_priv *hsr, struct sk_buff *skb)
>   	    hsr_sup_tag->tlv.HSR_TLV_length != sizeof(struct hsr_sup_payload))
>   		return false;
>   
> -	/* Get next tlv */
> +	/* Advance past the first TLV payload to reach next TLV header */
>   	total_length += hsr_sup_tag->tlv.HSR_TLV_length;
> -	if (!pskb_may_pull(skb, total_length))
> +	/* Linearize next TLV header before access */
> +	if (!pskb_may_pull(skb, total_length + sizeof(struct hsr_sup_tlv)))
>   		return false;
>   	skb_pull(skb, total_length);
>   	hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
>   	skb_push(skb, total_length);
>   
> -	/* if this is a redbox supervision frame we need to verify
> -	 * that more data is available
> +	/* Walk through TLVs to find end-of-TLV marker, skipping any unknown
> +	 * extension TLVs to maintain forward compatibility.
>   	 */
> -	if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
> -		/* tlv length must be a length of a mac address */
> -		if (hsr_sup_tlv->HSR_TLV_length != sizeof(struct hsr_sup_payload))
> -			return false;
> +	for (;;) {
> +		if (hsr_sup_tlv->HSR_TLV_type == HSR_TLV_EOT &&
> +		    hsr_sup_tlv->HSR_TLV_length == 0)
> +			return true;
>   

I do not follow this approach, why a loop? From IEC 62439-3, I do not 
understand that supervision frames could have multiple 
PRP_TLV_REDBOX_MAC TLVs. The current code handles the TLVs correctly.

Which makes me wonder, how are you testing this? Do you have some 
hardware with HSR/PRP support that is sending these frames? If so, which 
one? Are you testing this using a HSR/PRP environment with purely Linux 
devices?

Thanks,
Fernando.

> -		/* make sure another tlv follows */
> -		total_length += sizeof(struct hsr_sup_tlv) + hsr_sup_tlv->HSR_TLV_length;
> -		if (!pskb_may_pull(skb, total_length))
> +		/* Validate known TLV types */
> +		if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
> +			if (hsr_sup_tlv->HSR_TLV_length !=
> +			    sizeof(struct hsr_sup_payload))
> +				return false;
> +		}
> +
> +		/* Advance past current TLV: header + payload */
> +		total_length += sizeof(struct hsr_sup_tlv) +
> +				hsr_sup_tlv->HSR_TLV_length;
> +		/* Linearize next TLV header before access */
> +		if (!pskb_may_pull(skb,
> +				   total_length + sizeof(struct hsr_sup_tlv)))
>   			return false;
>   
> -		/* get next tlv */
>   		skb_pull(skb, total_length);
>   		hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
>   		skb_push(skb, total_length);
>   	}

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

[Luka Gejak]

On Wed Apr 1, 2026 at 4:47 PM CEST, Fernando Fernandez Mancera 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>
>> ---
>>   net/hsr/hsr_forward.c | 41 ++++++++++++++++++++++-------------------
>>   1 file changed, 22 insertions(+), 19 deletions(-)
>> 
>> diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
>> index 0aca859c88cb..17b705235c4a 100644
>> --- a/net/hsr/hsr_forward.c
>> +++ b/net/hsr/hsr_forward.c
>> @@ -82,39 +82,42 @@ static bool is_supervision_frame(struct hsr_priv *hsr, struct sk_buff *skb)
>>   	    hsr_sup_tag->tlv.HSR_TLV_length != sizeof(struct hsr_sup_payload))
>>   		return false;
>>   
>> -	/* Get next tlv */
>> +	/* Advance past the first TLV payload to reach next TLV header */
>>   	total_length += hsr_sup_tag->tlv.HSR_TLV_length;
>> -	if (!pskb_may_pull(skb, total_length))
>> +	/* Linearize next TLV header before access */
>> +	if (!pskb_may_pull(skb, total_length + sizeof(struct hsr_sup_tlv)))
>>   		return false;
>>   	skb_pull(skb, total_length);
>>   	hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
>>   	skb_push(skb, total_length);
>>   
>> -	/* if this is a redbox supervision frame we need to verify
>> -	 * that more data is available
>> +	/* Walk through TLVs to find end-of-TLV marker, skipping any unknown
>> +	 * extension TLVs to maintain forward compatibility.
>>   	 */
>> -	if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
>> -		/* tlv length must be a length of a mac address */
>> -		if (hsr_sup_tlv->HSR_TLV_length != sizeof(struct hsr_sup_payload))
>> -			return false;
>> +	for (;;) {
>> +		if (hsr_sup_tlv->HSR_TLV_type == HSR_TLV_EOT &&
>> +		    hsr_sup_tlv->HSR_TLV_length == 0)
>> +			return true;
>>   
>
> I do not follow this approach, why a loop? From IEC 62439-3, I do not 
> understand that supervision frames could have multiple 
> PRP_TLV_REDBOX_MAC TLVs. The current code handles the TLVs correctly.
>
> Which makes me wonder, how are you testing this? Do you have some 
> hardware with HSR/PRP support that is sending these frames? If so, which 
> one? Are you testing this using a HSR/PRP environment with purely Linux 
> devices?
>
> Thanks,
> Fernando.
>
>> -		/* make sure another tlv follows */
>> -		total_length += sizeof(struct hsr_sup_tlv) + hsr_sup_tlv->HSR_TLV_length;
>> -		if (!pskb_may_pull(skb, total_length))
>> +		/* Validate known TLV types */
>> +		if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
>> +			if (hsr_sup_tlv->HSR_TLV_length !=
>> +			    sizeof(struct hsr_sup_payload))
>> +				return false;
>> +		}
>> +
>> +		/* Advance past current TLV: header + payload */
>> +		total_length += sizeof(struct hsr_sup_tlv) +
>> +				hsr_sup_tlv->HSR_TLV_length;
>> +		/* Linearize next TLV header before access */
>> +		if (!pskb_may_pull(skb,
>> +				   total_length + sizeof(struct hsr_sup_tlv)))
>>   			return false;
>>   
>> -		/* get next tlv */
>>   		skb_pull(skb, total_length);
>>   		hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
>>   		skb_push(skb, total_length);
>>   	}

Hi Fernando,

You are right that IEC 62439-3 does not specify multiple 
PRP_TLV_REDBOX_MAC TLVs. My intention with the loop was not to handle 
multiple RedBox MACs, but rather to make the parser robust against 
unknown TLV types. If a future revision of the standard or a vendor 
extension introduces a new TLV, the loop allows the kernel to safely 
skip over unrecognized TLVs by reading their length, ensuring it can 
still validate the HSR_TLV_EOT marker at the end.

However, if the preference for the HSR subsystem is strict adherence to 
only currently defined TLVs over forward compatibility, I completely 
understand.

Furthermore, I am testing this using a purely Linux environment by 
using a virtual HSR environment on Arch Linux. I set up two network 
namespaces connected via veth pairs and instantiated HSR interfaces.

The nodes successfully synchronized and maintained the connection. I 
confirmed this by observing the expected duplicate packets (DUP!) 
during ping tests between namespaces and by verifying that supervision 
frames were correctly parsed, allowing the nodes to populate their 
remote node tables.

Let me know if you'd prefer I drop the loop for v5.
Best regards,
Luka Gejak

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

[Fernando Fernandez Mancera]

On 4/1/26 6:59 PM, Luka Gejak wrote:
> On Wed Apr 1, 2026 at 4:47 PM CEST, Fernando Fernandez Mancera 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>
>>> ---
>>>    net/hsr/hsr_forward.c | 41 ++++++++++++++++++++++-------------------
>>>    1 file changed, 22 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
>>> index 0aca859c88cb..17b705235c4a 100644
>>> --- a/net/hsr/hsr_forward.c
>>> +++ b/net/hsr/hsr_forward.c
>>> @@ -82,39 +82,42 @@ static bool is_supervision_frame(struct hsr_priv *hsr, struct sk_buff *skb)
>>>    	    hsr_sup_tag->tlv.HSR_TLV_length != sizeof(struct hsr_sup_payload))
>>>    		return false;
>>>    
>>> -	/* Get next tlv */
>>> +	/* Advance past the first TLV payload to reach next TLV header */
>>>    	total_length += hsr_sup_tag->tlv.HSR_TLV_length;
>>> -	if (!pskb_may_pull(skb, total_length))
>>> +	/* Linearize next TLV header before access */
>>> +	if (!pskb_may_pull(skb, total_length + sizeof(struct hsr_sup_tlv)))
>>>    		return false;
>>>    	skb_pull(skb, total_length);
>>>    	hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
>>>    	skb_push(skb, total_length);
>>>    
>>> -	/* if this is a redbox supervision frame we need to verify
>>> -	 * that more data is available
>>> +	/* Walk through TLVs to find end-of-TLV marker, skipping any unknown
>>> +	 * extension TLVs to maintain forward compatibility.
>>>    	 */
>>> -	if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
>>> -		/* tlv length must be a length of a mac address */
>>> -		if (hsr_sup_tlv->HSR_TLV_length != sizeof(struct hsr_sup_payload))
>>> -			return false;
>>> +	for (;;) {
>>> +		if (hsr_sup_tlv->HSR_TLV_type == HSR_TLV_EOT &&
>>> +		    hsr_sup_tlv->HSR_TLV_length == 0)
>>> +			return true;
>>>    
>>
>> I do not follow this approach, why a loop? From IEC 62439-3, I do not
>> understand that supervision frames could have multiple
>> PRP_TLV_REDBOX_MAC TLVs. The current code handles the TLVs correctly.
>>
>> Which makes me wonder, how are you testing this? Do you have some
>> hardware with HSR/PRP support that is sending these frames? If so, which
>> one? Are you testing this using a HSR/PRP environment with purely Linux
>> devices?
>>
>> Thanks,
>> Fernando.
>>
>>> -		/* make sure another tlv follows */
>>> -		total_length += sizeof(struct hsr_sup_tlv) + hsr_sup_tlv->HSR_TLV_length;
>>> -		if (!pskb_may_pull(skb, total_length))
>>> +		/* Validate known TLV types */
>>> +		if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
>>> +			if (hsr_sup_tlv->HSR_TLV_length !=
>>> +			    sizeof(struct hsr_sup_payload))
>>> +				return false;
>>> +		}
>>> +
>>> +		/* Advance past current TLV: header + payload */
>>> +		total_length += sizeof(struct hsr_sup_tlv) +
>>> +				hsr_sup_tlv->HSR_TLV_length;
>>> +		/* Linearize next TLV header before access */
>>> +		if (!pskb_may_pull(skb,
>>> +				   total_length + sizeof(struct hsr_sup_tlv)))
>>>    			return false;
>>>    
>>> -		/* get next tlv */
>>>    		skb_pull(skb, total_length);
>>>    		hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
>>>    		skb_push(skb, total_length);
>>>    	}
> 
> Hi Fernando,
> 
> You are right that IEC 62439-3 does not specify multiple
> PRP_TLV_REDBOX_MAC TLVs. My intention with the loop was not to handle
> multiple RedBox MACs, but rather to make the parser robust against
> unknown TLV types. If a future revision of the standard or a vendor
> extension introduces a new TLV, the loop allows the kernel to safely
> skip over unrecognized TLVs by reading their length, ensuring it can
> still validate the HSR_TLV_EOT marker at the end.
>

AFAIU, the TLVs must be in the right order. I don't know, it doesn't 
sound very convincing to me that we are anticipating to new TLVs. 
HSR/PRP isn't a very active protocol and it has few users in Kernel 
probably compare to other protocols because it is used in a very 
specific industry domain.

If a new revision of the protocol specs is released we can always update 
our implementation.

Anyway, since Felix reviewed the initial patch let's wait for his review.

> However, if the preference for the HSR subsystem is strict adherence to
> only currently defined TLVs over forward compatibility, I completely
> understand.
> 
> Furthermore, I am testing this using a purely Linux environment by
> using a virtual HSR environment on Arch Linux. I set up two network
> namespaces connected via veth pairs and instantiated HSR interfaces.
> 
> The nodes successfully synchronized and maintained the connection. I
> confirmed this by observing the expected duplicate packets (DUP!)
> during ping tests between namespaces and by verifying that supervision
> frames were correctly parsed, allowing the nodes to populate their
> remote node tables.
> 
> Let me know if you'd prefer I drop the loop for v5.
> Best regards,
> Luka Gejak
> 

[PATCH net-next v4 2/2] net: hsr: reject unresolved interlink ifindex

[luka.gejak]

From: Luka Gejak <luka.gejak@linux.dev>

In hsr_newlink(), a provided but invalid IFLA_HSR_INTERLINK attribute
was silently ignored if __dev_get_by_index() returned NULL. This leads
to incorrect RedBox topology creation without notifying the user.

Fix this by returning -EINVAL and an extack message when the
interlink attribute is present but cannot be resolved.

Reviewed-by: Felix Maurer <fmaurer@redhat.com>
Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
---
 net/hsr/hsr_netlink.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/hsr/hsr_netlink.c b/net/hsr/hsr_netlink.c
index db0b0af7a692..f0ca23da3ab9 100644
--- a/net/hsr/hsr_netlink.c
+++ b/net/hsr/hsr_netlink.c
@@ -76,9 +76,14 @@ static int hsr_newlink(struct net_device *dev,
 		return -EINVAL;
 	}
 
-	if (data[IFLA_HSR_INTERLINK])
+	if (data[IFLA_HSR_INTERLINK]) {
 		interlink = __dev_get_by_index(link_net,
 					       nla_get_u32(data[IFLA_HSR_INTERLINK]));
+		if (!interlink) {
+			NL_SET_ERR_MSG_MOD(extack, "Interlink does not exist");
+			return -EINVAL;
+		}
+	}
 
 	if (interlink && interlink == link[0]) {
 		NL_SET_ERR_MSG_MOD(extack, "Interlink and Slave1 are the same");
-- 
2.53.0