| Message ID | 20240228085644.3618044-1-lukma@denx.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 51dd4ee0372228ffb0f7709fa7aa0678d4199d06 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] net: hsr: Use correct offset for HSR TLV values in supervisory HSR frames | expand |
Wed, Feb 28, 2024 at 09:56:44AM CET, lukma@denx.de wrote: >Current HSR implementation uses following supervisory frame (even for >HSRv1 the HSR tag is not is not present): > >00000000: 01 15 4e 00 01 2d XX YY ZZ 94 77 10 88 fb 00 01 >00000010: 7e 1c 17 06 XX YY ZZ 94 77 10 1e 06 XX YY ZZ 94 >00000020: 77 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >00000030: 00 00 00 00 00 00 00 00 00 00 00 00 > >The current code adds extra two bytes (i.e. sizeof(struct hsr_sup_tlv)) >when offset for skb_pull() is calculated. >This is wrong, as both 'struct hsrv1_ethhdr_sp' and 'hsrv0_ethhdr_sp' >already have 'struct hsr_sup_tag' defined in them, so there is no need >for adding extra two bytes. > >This code was working correctly as with no RedBox support, the check for >HSR_TLV_EOT (0x00) was off by two bytes, which were corresponding to >zeroed padded bytes for minimal packet size. > >Fixes: eafaa88b3eb7 ("net: hsr: Add support for redbox supervision frames") > And yet the extra empty line is still here :/ >Signed-off-by: Lukasz Majewski <lukma@denx.de> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
On Wed, 2024-02-28 at 13:31 +0100, Jiri Pirko wrote: > Wed, Feb 28, 2024 at 09:56:44AM CET, lukma@denx.de wrote: > > Current HSR implementation uses following supervisory frame (even for > > HSRv1 the HSR tag is not is not present): > > > > 00000000: 01 15 4e 00 01 2d XX YY ZZ 94 77 10 88 fb 00 01 > > 00000010: 7e 1c 17 06 XX YY ZZ 94 77 10 1e 06 XX YY ZZ 94 > > 00000020: 77 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > 00000030: 00 00 00 00 00 00 00 00 00 00 00 00 > > > > The current code adds extra two bytes (i.e. sizeof(struct hsr_sup_tlv)) > > when offset for skb_pull() is calculated. > > This is wrong, as both 'struct hsrv1_ethhdr_sp' and 'hsrv0_ethhdr_sp' > > already have 'struct hsr_sup_tag' defined in them, so there is no need > > for adding extra two bytes. > > > > This code was working correctly as with no RedBox support, the check for > > HSR_TLV_EOT (0x00) was off by two bytes, which were corresponding to > > zeroed padded bytes for minimal packet size. > > > > Fixes: eafaa88b3eb7 ("net: hsr: Add support for redbox supervision frames") > > > > And yet the extra empty line is still here :/ To avoid more back & forth on otherwise correct patch, I'll fix this while applying the patch. I hope it's clear to everybody this is an exception and not the new normal :) > > Signed-off-by: Lukasz Majewski <lukma@denx.de> Please have a better look to the Documentation before the next post, and use the checkpatch script before the submission. Thanks, Paolo
Hello: This patch was applied to netdev/net.git (main) by Paolo Abeni <pabeni@redhat.com>: On Wed, 28 Feb 2024 09:56:44 +0100 you wrote: > Current HSR implementation uses following supervisory frame (even for > HSRv1 the HSR tag is not is not present): > > 00000000: 01 15 4e 00 01 2d XX YY ZZ 94 77 10 88 fb 00 01 > 00000010: 7e 1c 17 06 XX YY ZZ 94 77 10 1e 06 XX YY ZZ 94 > 00000020: 77 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 00000030: 00 00 00 00 00 00 00 00 00 00 00 00 > > [...] Here is the summary with links: - [v2] net: hsr: Use correct offset for HSR TLV values in supervisory HSR frames https://git.kernel.org/netdev/net/c/51dd4ee03722 You are awesome, thank you!
diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c index 2afe28712a7a..5d68cb181695 100644 --- a/net/hsr/hsr_forward.c +++ b/net/hsr/hsr_forward.c @@ -83,7 +83,7 @@ static bool is_supervision_frame(struct hsr_priv *hsr, struct sk_buff *skb) return false; /* Get next tlv */ - total_length += sizeof(struct hsr_sup_tlv) + hsr_sup_tag->tlv.HSR_TLV_length; + total_length += hsr_sup_tag->tlv.HSR_TLV_length; if (!pskb_may_pull(skb, total_length)) return false; skb_pull(skb, total_length);
Current HSR implementation uses following supervisory frame (even for HSRv1 the HSR tag is not is not present): 00000000: 01 15 4e 00 01 2d XX YY ZZ 94 77 10 88 fb 00 01 00000010: 7e 1c 17 06 XX YY ZZ 94 77 10 1e 06 XX YY ZZ 94 00000020: 77 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00000030: 00 00 00 00 00 00 00 00 00 00 00 00 The current code adds extra two bytes (i.e. sizeof(struct hsr_sup_tlv)) when offset for skb_pull() is calculated. This is wrong, as both 'struct hsrv1_ethhdr_sp' and 'hsrv0_ethhdr_sp' already have 'struct hsr_sup_tag' defined in them, so there is no need for adding extra two bytes. This code was working correctly as with no RedBox support, the check for HSR_TLV_EOT (0x00) was off by two bytes, which were corresponding to zeroed padded bytes for minimal packet size. Fixes: eafaa88b3eb7 ("net: hsr: Add support for redbox supervision frames") Signed-off-by: Lukasz Majewski <lukma@denx.de> --- Changes for v2: - Update commit message to point to correct commit in "Fixes:" --- net/hsr/hsr_forward.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)