RE: [PATCH v4 net-next 09/15] net: enetc: add MAC address filtering support for VFs of ENETC v4

quoted

+/* message format of class_id 0x20 for hash MAC filter.
+ * cmd_id 0x3: set MAC hash table
+ */
+struct enetc_msg_mac_hash_filter {
+	struct enetc_msg_header hdr;
+	/* bit 0 ~ 5: ENETC_MSG_MAC_HASH_SIZE
+	 * bit 6~7: ENETC_MSG_MAC_TYPE
+	 */
+	u8 sz_type;
+	u8 resv[3];
+	u32 hash_tbl[];
+};

[Severity: Low]

Should hash_tbl[] carry an explicit endianness annotation?

This struct describes a wire format: the message lives in a DMA buffer that
the mailbox hardware copies from the VSI buffer into the PSI buffer, and the
header in this same file already declares the byte order of its multi-byte
field:

	struct enetc_msg_header {
		__be16 crc16;
		...
	};

hash_tbl[] is the only other multi-byte payload field and it is plain u32,
so both endpoints are implicitly required to have the same byte order. The
producer fills it in host order and enetc_msg_set_vf_mac_hash_filter()
recombines it in host order:

		uc_hash = (u64)msg->hash_tbl[1] << 32 | msg->hash_tbl[0];

If a big-endian VSI kernel ever talks to a little-endian PSI (or a
non-Linux PSI implementing the documented protocol), would the hash buckets
end up byte-swapped, so the SI filters on the wrong buckets while both
sides report success? The CRC16 is computed byte-wise over the buffer, so
it would not catch that.

Would declaring the array as __le32 and converting with cpu_to_le32() /
le32_to_cpu() on both ends be preferable now, while the format is being
introduced?

This combination does not exist in practice for ENETC. Both PF(host) and VF(VM)
use the same endianness, so no need an explicit endianness annotation.

quoted

+
+/* message format of class_id 0x20 for MAC promiscuous mode.
+ * cmd_id 0x5: set MAC promiscuous mode
+ */
+struct enetc_msg_mac_promisc_mode {
+	struct enetc_msg_header hdr;
+	/* bit 0: ENETC_MSG_MAC_FLUSH_MACS
+	 * bit 1: ENETC_MSG_MAC_PROMISC_MODE
+	 * bit 6~7: ENETC_MSG_MAC_TYPE
+	 */
+	u8 config;
+	u8 resv[15];
+};
+
 /* The generic message format applies to the following messages:
  * Get IP revision message, class_id 0xf0.
  * cmd_id 1: get IP minor revision

quoted

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_msg.c

b/drivers/net/ethernet/freescale/enetc/enetc_msg.c

quoted

index 08a9ffdd3eb0..22f5485e968a 100644

--- a/drivers/net/ethernet/freescale/enetc/enetc_msg.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_msg.c

[ ... ]

quoted

@@ -116,6 +242,10 @@ static u16 enetc_msg_handle_mac_filter(struct

enetc_pf *pf, int vf_id,

quoted

 	switch (msg_hdr->cmd_id) {
 	case ENETC_MSG_SET_PRIMARY_MAC:
 		return enetc_msg_set_vf_primary_mac_addr(pf, vf_id, vf_msg);
+	case ENETC_MSG_SET_MAC_HASH_TABLE:
+		return enetc_msg_set_vf_mac_hash_filter(pf, vf_id, vf_msg);
+	case ENETC_MSG_SET_MAC_PROMISC_MODE:
+		return enetc_msg_set_vf_mac_promisc_mode(pf, vf_id, vf_msg);
 	default:
 		return ENETC_PF_MSG_NOTSUPP;
 	}

[Severity: Low]

At this commit, is there anything in the tree that sends
ENETC_MSG_SET_MAC_HASH_TABLE or
ENETC_MSG_SET_MAC_PROMISC_MODE?

Both new handlers here, and the UCF_DENY mapping added to
enetc_msg_vsi_send() below, only become reachable with the later patch in
the series ("net: enetc: implement ndo_set_rx_mode_async for ENETC v4 VF"),
which adds enetc_vf_set_mac_hash_filter() and enetc_vf_set_mac_promisc()
and wires .ndo_set_rx_mode_async = enetc_vf_set_rx_mode into
enetc4_ndev_ops. So this is only a patch-ordering observation, the receiver
landing one patch before the sender, not a defect at the end of the series.

This is not an issue, no need fix.