History

net: dsa: qca8k: fix wrong length value for mgmt eth packet

The assumption that Documentation was right about how this value work was wrong. It was discovered that the length value of the mgmt header is in step of word size. As an example to process 4 byte of data the correct length to set is 2. To process 8 byte 4, 12 byte 6, 16 byte 8... Odd values will always return the next size on the ack packet. (length of 3 (6 byte) will always return 8 bytes of data) This means that a value of 15 (0xf) actually means reading/writing 32 bytes of data instead of 16 bytes. This behaviour is totally absent and not documented in the switch Documentation. In fact from Documentation the max value that mgmt eth can process is 16 byte of data while in reality it can process 32 bytes at once. To handle this we always round up the length after deviding it for word size. We check if the result is odd and we round another time to align to what the switch will provide in the ack packet. The workaround for the length limit of 15 is still needed as the length reg max value is 0xf(15)

Christian Marangi committed 3 years ago

Commit 9807ae61 parent d0395351 file · +35 −10Browse files

Unified · Split

drivers/net/dsa/qca/qca8k-8xxx.c+35−10
@@ -146,7 +146,16 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb) · 145 unmodified lines
146146
147147 command = get_unaligned_le32(&mgmt_ethhdr->command);
148148 cmd = FIELD_GET(QCA_HDR_MGMT_CMD, command);
149+
149150 len = FIELD_GET(QCA_HDR_MGMT_LENGTH, command);
151+ /* Special case for len of 15 as this is the max value for len and needs to
152+ * be increased before converting it from word to dword.
153+ */
154+ if (len == 15)
155+ len++;
156+
157+ /* We can ignore odd value, we always round up them in the alloc function. */
158+ len *= sizeof(u16);
150159
151160 /* Make sure the seq match the requested packet */
152161 if (get_unaligned_le32(&mgmt_ethhdr->seq) == mgmt_eth_data->seq)
@@ -193,17 +202,33 @@ static struct sk_buff *qca8k_alloc_mdio_header(enum mdio_cmd cmd, u32 reg, u32 * · 40 unmodified lines
193202 if (!skb)
194203 return NULL;
195204
196− /* Max value for len reg is 15 (0xf) but the switch actually return 16 byte
197− * Actually for some reason the steps are:
198− * 0: nothing
199− * 1-4: first 4 byte
200− * 5-6: first 12 byte
201− * 7-15: all 16 byte
205+ /* Hdr mgmt length value is in step of word size.
206+ * As an example to process 4 byte of data the correct length to set is 2.
207+ * To process 8 byte 4, 12 byte 6, 16 byte 8...
208+ *
209+ * Odd values will always return the next size on the ack packet.
210+ * (length of 3 (6 byte) will always return 8 bytes of data)
211+ *
212+ * This means that a value of 15 (0xf) actually means reading/writing 32 bytes
213+ * of data.
214+ *
215+ * To correctly calculate the length we devide the requested len by word and
216+ * round up.
217+ * On the ack function we can skip the odd check as we already handle the
218+ * case here.
219+ */
220+ real_len = DIV_ROUND_UP(len, sizeof(u16));
221+
222+ /* We check if the result len is odd and we round up another time to
223+ * the next size. (length of 3 will be increased to 4 as switch will always
224+ * return 8 bytes)
202225 */
203− if (len == 16)
204− real_len = 15;
205− else
206− real_len = len;
226+ if (real_len % sizeof(u16) != 0)
227+ real_len++;
228+
229+ /* Max reg value is 0xf(15) but switch will always return the next size (32 byte) */
230+ if (real_len == 16)
231+ real_len--;
207232
208233 skb_reset_mac_header(skb);
209234 skb_set_network_header(skb, skb->len);