From dc090de854b9d7fdbc6f4df70bd7fc1b43eeccf8 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Thu, 21 Jan 2021 21:40:36 +0100 Subject: net: mrp: fix definitions of MRP test packets 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. Even though this makes the timestamp field in the struct unaligned, it actually makes it end up on a 32 bit boundary in the frame as mandated by the standard, since it is preceded by a two byte TLV header. 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. Signed-off-by: Rasmus Villemoes Reviewed-by: Horatiu Vultur Signed-off-by: Jakub Kicinski --- include/uapi/linux/mrp_bridge.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'include/uapi') diff --git a/include/uapi/linux/mrp_bridge.h b/include/uapi/linux/mrp_bridge.h index 9744773de5ff..5ce771327e32 100644 --- a/include/uapi/linux/mrp_bridge.h +++ b/include/uapi/linux/mrp_bridge.h @@ -97,7 +97,7 @@ struct br_mrp_ring_test_hdr { __be16 state; __be16 transitions; __be32 timestamp; -}; +} __attribute__((__packed__)); struct br_mrp_ring_topo_hdr { __be16 prio; @@ -142,7 +142,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]; -- cgit v1.2.3 From 6781939054a1a161e06e7a7955a4846be770a711 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Thu, 21 Jan 2021 21:40:37 +0100 Subject: net: mrp: move struct definitions out of uapi None of these are actually used in the kernel/userspace interface - there's a userspace component of implementing MRP, and userspace will need to construct certain frames to put on the wire, but there's no reason the kernel should provide the relevant definitions in a UAPI header. In fact, some of those definitions were broken until previous commit, so only keep the few that are actually referenced in the kernel code, and move them to the br_private_mrp.h header. Signed-off-by: Rasmus Villemoes Signed-off-by: Jakub Kicinski --- include/uapi/linux/mrp_bridge.h | 86 ----------------------------------------- 1 file changed, 86 deletions(-) (limited to 'include/uapi') diff --git a/include/uapi/linux/mrp_bridge.h b/include/uapi/linux/mrp_bridge.h index 5ce771327e32..bd4424de56ff 100644 --- a/include/uapi/linux/mrp_bridge.h +++ b/include/uapi/linux/mrp_bridge.h @@ -71,90 +71,4 @@ enum br_mrp_sub_tlv_header_type { BR_MRP_SUB_TLV_HEADER_TEST_AUTO_MGR = 0x3, }; -struct br_mrp_tlv_hdr { - __u8 type; - __u8 length; -}; - -struct br_mrp_sub_tlv_hdr { - __u8 type; - __u8 length; -}; - -struct br_mrp_end_hdr { - struct br_mrp_tlv_hdr hdr; -}; - -struct br_mrp_common_hdr { - __be16 seq_id; - __u8 domain[MRP_DOMAIN_UUID_LENGTH]; -}; - -struct br_mrp_ring_test_hdr { - __be16 prio; - __u8 sa[ETH_ALEN]; - __be16 port_role; - __be16 state; - __be16 transitions; - __be32 timestamp; -} __attribute__((__packed__)); - -struct br_mrp_ring_topo_hdr { - __be16 prio; - __u8 sa[ETH_ALEN]; - __be16 interval; -}; - -struct br_mrp_ring_link_hdr { - __u8 sa[ETH_ALEN]; - __be16 port_role; - __be16 interval; - __be16 blocked; -}; - -struct br_mrp_sub_opt_hdr { - __u8 type; - __u8 manufacture_data[MRP_MANUFACTURE_DATA_LENGTH]; -}; - -struct br_mrp_test_mgr_nack_hdr { - __be16 prio; - __u8 sa[ETH_ALEN]; - __be16 other_prio; - __u8 other_sa[ETH_ALEN]; -}; - -struct br_mrp_test_prop_hdr { - __be16 prio; - __u8 sa[ETH_ALEN]; - __be16 other_prio; - __u8 other_sa[ETH_ALEN]; -}; - -struct br_mrp_oui_hdr { - __u8 oui[MRP_OUI_LENGTH]; -}; - -struct br_mrp_in_test_hdr { - __be16 id; - __u8 sa[ETH_ALEN]; - __be16 port_role; - __be16 state; - __be16 transitions; - __be32 timestamp; -} __attribute__((__packed__)); - -struct br_mrp_in_topo_hdr { - __u8 sa[ETH_ALEN]; - __be16 id; - __be16 interval; -}; - -struct br_mrp_in_link_hdr { - __u8 sa[ETH_ALEN]; - __be16 port_role; - __be16 id; - __be16 interval; -}; - #endif -- cgit v1.2.3 From 07d46d93c9acdfe0614071d73c415dd5f745cc6e Mon Sep 17 00:00:00 2001 From: Justin Iurman Date: Thu, 21 Jan 2021 23:00:44 +0100 Subject: uapi: fix big endian definition of ipv6_rpl_sr_hdr Following RFC 6554 [1], the current order of fields is wrong for big endian definition. Indeed, here is how the header looks like: +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | Next Header | Hdr Ext Len | Routing Type | Segments Left | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | CmprI | CmprE | Pad | Reserved | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ This patch reorders fields so that big endian definition is now correct. [1] https://tools.ietf.org/html/rfc6554#section-3 Fixes: cfa933d938d8 ("include: uapi: linux: add rpl sr header definition") Signed-off-by: Justin Iurman Signed-off-by: Jakub Kicinski --- include/uapi/linux/rpl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'include/uapi') diff --git a/include/uapi/linux/rpl.h b/include/uapi/linux/rpl.h index 1dccb55cf8c6..708adddf9f13 100644 --- a/include/uapi/linux/rpl.h +++ b/include/uapi/linux/rpl.h @@ -28,10 +28,10 @@ struct ipv6_rpl_sr_hdr { pad:4, reserved1:16; #elif defined(__BIG_ENDIAN_BITFIELD) - __u32 reserved:20, + __u32 cmpri:4, + cmpre:4, pad:4, - cmpri:4, - cmpre:4; + reserved:20; #else #error "Please fix " #endif -- cgit v1.2.3