netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
To: netdev@vger.kernel.org, Horatiu Vultur <horatiu.vultur@microchip.com>
Cc: linux-kernel@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Subject: [PATCH net 1/2] net: mrp: fix definitions of MRP test packets
Date: Wed, 23 Dec 2020 15:45:32 +0100	[thread overview]
Message-ID: <20201223144533.4145-2-rasmus.villemoes@prevas.dk> (raw)
In-Reply-To: <20201223144533.4145-1-rasmus.villemoes@prevas.dk>

Wireshark says that the MRP test packets cannot be decoded - and the
reason for that is that there's a two-byte hole filled with garbage
between the "transitions" and "timestamp" members.

So Wireshark decodes the two garbage bytes and the top two bytes of
the timestamp written by the kernel as the timestamp value (which thus
fluctuates wildly), and interprets the lower two bytes of the
timestamp as a new (type, length) pair, which is of course broken.

While my copy of the MRP standard is still under way [*], I cannot
imagine the standard specifying a two-byte hole here, and whoever
wrote the Wireshark decoding code seems to agree with that.

The struct definitions live under include/uapi/, but they are not
really part of any kernel<->userspace API/ABI, so fixing the
definitions by adding the packed attribute should not cause any
compatibility issues.

The remaining on-the-wire packet formats likely also don't contain
holes, but pahole and manual inspection says the current definitions
suffice. So adding the packed attribute to those is not strictly
needed, but might be done for good measure.

[*] I will never understand how something hidden behind a +1000$
paywall can be called a standard.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 include/uapi/linux/mrp_bridge.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/mrp_bridge.h b/include/uapi/linux/mrp_bridge.h
index 6aeb13ef0b1e..d1d0cf65916d 100644
--- a/include/uapi/linux/mrp_bridge.h
+++ b/include/uapi/linux/mrp_bridge.h
@@ -96,7 +96,7 @@ struct br_mrp_ring_test_hdr {
 	__be16 state;
 	__be16 transitions;
 	__be32 timestamp;
-};
+} __attribute__((__packed__));
 
 struct br_mrp_ring_topo_hdr {
 	__be16 prio;
@@ -141,7 +141,7 @@ struct br_mrp_in_test_hdr {
 	__be16 state;
 	__be16 transitions;
 	__be32 timestamp;
-};
+} __attribute__((__packed__));
 
 struct br_mrp_in_topo_hdr {
 	__u8 sa[ETH_ALEN];
-- 
2.23.0


  reply	other threads:[~2020-12-23 14:47 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-23 14:45 [PATCH net 0/2] MRP without hardware offload? Rasmus Villemoes
2020-12-23 14:45 ` Rasmus Villemoes [this message]
2020-12-23 17:59   ` [PATCH net 1/2] net: mrp: fix definitions of MRP test packets Horatiu Vultur
2020-12-23 18:41     ` Andrew Lunn
2020-12-23 19:22       ` Horatiu Vultur
2020-12-28 22:24   ` Jakub Kicinski
2021-01-03 13:29     ` Horatiu Vultur
2021-01-19 15:42     ` Rasmus Villemoes
2021-01-19 17:45       ` Jakub Kicinski
2020-12-23 14:45 ` [PATCH net 2/2] net: switchdev: don't set port_obj_info->handled true when -EOPNOTSUPP Rasmus Villemoes
2020-12-28 22:26   ` Jakub Kicinski
2020-12-23 18:14 ` [PATCH net 0/2] MRP without hardware offload? Horatiu Vultur

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201223144533.4145-2-rasmus.villemoes@prevas.dk \
    --to=rasmus.villemoes@prevas.dk \
    --cc=davem@davemloft.net \
    --cc=horatiu.vultur@microchip.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).