| Message ID | 20220908173534.1324239-1-bjorn@mork.no |
|---|---|
| State | Accepted |
| Delegated to: | Sander Vanheule |
| Headers | show |
| Series | realtek: fix RTL838x receive tag decoding | expand |
Hi Bjørn, On Thu, 2022-09-08 at 19:35 +0200, Bjørn Mork wrote: > Commit dc9cc0d3e2a1 ("realtek: add QoS and rate control") replaced a > 16 bit reserved field in the RTL83xx packet header with the initial > cpu_tag word, shifting the real cpu_tag fields by one. Adjusting for > this new shift was partially forgotten in the new RX tag decoders. > > This caused the switch to block IGMP, effectively blocking IPv4 > multicast. > > The bug was partially fixed by commit 9d847244d9fd ("realtek: fix > RTL839X receive tag decoding") > > Fix on RTL838x too, including correct NIC_RX_REASON_SPECIAL_TRAP value. > > Based-on-fix-by: Jan Hoffmann <jan@3e8.eu> checkpatch.pl complains about this tag: WARNING: Non-standard signature: Based-on-fix-by: Mind if I change it to Suggested-by? > Fixes: dc9cc0d3e2a1 ("realtek: add QoS and rate control") > Signed-off-by: Bjørn Mork <bjorn@mork.no> Thanks for the patch! Best, Sander > --- > .../files-5.10/drivers/net/ethernet/rtl838x_eth.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/target/linux/realtek/files- > 5.10/drivers/net/ethernet/rtl838x_eth.c b/target/linux/realtek/files- > 5.10/drivers/net/ethernet/rtl838x_eth.c > index 0eee06d803ad..d9ade6552698 100644 > --- a/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.c > +++ b/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.c > @@ -58,6 +58,7 @@ struct p_hdr { > uint16_t size; /* buffer size */ > uint16_t offset; > uint16_t len; /* pkt len */ > + /* cpu_tag[0] is a reserved uint16_t on RTL83xx */ > uint16_t cpu_tag[10]; > } __packed __aligned(1); > > @@ -262,13 +263,14 @@ struct dsa_tag { > > bool rtl838x_decode_tag(struct p_hdr *h, struct dsa_tag *t) > { > - t->reason = h->cpu_tag[3] & 0xf; > - t->queue = (h->cpu_tag[0] & 0xe0) >> 5; > + /* cpu_tag[0] is reserved. Fields are off-by-one */ > + t->reason = h->cpu_tag[4] & 0xf; > + t->queue = (h->cpu_tag[1] & 0xe0) >> 5; > t->port = h->cpu_tag[1] & 0x1f; > t->crc_error = t->reason == 13; > > pr_debug("Reason: %d\n", t->reason); > - if (t->reason != 4) // NIC_RX_REASON_SPECIAL_TRAP > + if (t->reason != 6) // NIC_RX_REASON_SPECIAL_TRAP > t->l2_offloaded = 1; > else > t->l2_offloaded = 0; > @@ -278,6 +280,7 @@ bool rtl838x_decode_tag(struct p_hdr *h, struct dsa_tag > *t) > > bool rtl839x_decode_tag(struct p_hdr *h, struct dsa_tag *t) > { > + /* cpu_tag[0] is reserved. Fields are off-by-one */ > t->reason = h->cpu_tag[5] & 0x1f; > t->queue = (h->cpu_tag[3] & 0xe000) >> 13; > t->port = h->cpu_tag[1] & 0x3f;
Sander Vanheule <sander@svanheule.net> writes: >> Based-on-fix-by: Jan Hoffmann <jan@3e8.eu> > > checkpatch.pl complains about this tag: > WARNING: Non-standard signature: Based-on-fix-by: Yes, I noticed and didn't care ;-) > Mind if I change it to Suggested-by? No problem. Bjørn
On Thu, 2022-09-08 at 22:23 +0200, Bjørn Mork wrote: > Sander Vanheule <sander@svanheule.net> writes: > > > > Based-on-fix-by: Jan Hoffmann <jan@3e8.eu> > > > > checkpatch.pl complains about this tag: > > WARNING: Non-standard signature: Based-on-fix-by: > > Yes, I noticed and didn't care ;-) Just as I thought ;) > > > Mind if I change it to Suggested-by? > > No problem. OK, then I'm happy an I'll merge this patch. Thanks! Best, Sander
On Thu, 2022-09-08 at 19:35 +0200, Bjørn Mork wrote: > Commit dc9cc0d3e2a1 ("realtek: add QoS and rate control") replaced a > 16 bit reserved field in the RTL83xx packet header with the initial > cpu_tag word, shifting the real cpu_tag fields by one. Adjusting for > this new shift was partially forgotten in the new RX tag decoders. > > This caused the switch to block IGMP, effectively blocking IPv4 > multicast. > > The bug was partially fixed by commit 9d847244d9fd ("realtek: fix > RTL839X receive tag decoding") > > Fix on RTL838x too, including correct NIC_RX_REASON_SPECIAL_TRAP value. > > Based-on-fix-by: Jan Hoffmann <jan@3e8.eu> > Fixes: dc9cc0d3e2a1 ("realtek: add QoS and rate control") > Signed-off-by: Bjørn Mork <bjorn@mork.no> I've cherry picked this patch for 22.03 too, so stable releases will also benefit from this fix. Best, Sander
diff --git a/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.c b/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.c index 0eee06d803ad..d9ade6552698 100644 --- a/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.c +++ b/target/linux/realtek/files-5.10/drivers/net/ethernet/rtl838x_eth.c @@ -58,6 +58,7 @@ struct p_hdr { uint16_t size; /* buffer size */ uint16_t offset; uint16_t len; /* pkt len */ + /* cpu_tag[0] is a reserved uint16_t on RTL83xx */ uint16_t cpu_tag[10]; } __packed __aligned(1); @@ -262,13 +263,14 @@ struct dsa_tag { bool rtl838x_decode_tag(struct p_hdr *h, struct dsa_tag *t) { - t->reason = h->cpu_tag[3] & 0xf; - t->queue = (h->cpu_tag[0] & 0xe0) >> 5; + /* cpu_tag[0] is reserved. Fields are off-by-one */ + t->reason = h->cpu_tag[4] & 0xf; + t->queue = (h->cpu_tag[1] & 0xe0) >> 5; t->port = h->cpu_tag[1] & 0x1f; t->crc_error = t->reason == 13; pr_debug("Reason: %d\n", t->reason); - if (t->reason != 4) // NIC_RX_REASON_SPECIAL_TRAP + if (t->reason != 6) // NIC_RX_REASON_SPECIAL_TRAP t->l2_offloaded = 1; else t->l2_offloaded = 0; @@ -278,6 +280,7 @@ bool rtl838x_decode_tag(struct p_hdr *h, struct dsa_tag *t) bool rtl839x_decode_tag(struct p_hdr *h, struct dsa_tag *t) { + /* cpu_tag[0] is reserved. Fields are off-by-one */ t->reason = h->cpu_tag[5] & 0x1f; t->queue = (h->cpu_tag[3] & 0xe000) >> 13; t->port = h->cpu_tag[1] & 0x3f;
Commit dc9cc0d3e2a1 ("realtek: add QoS and rate control") replaced a 16 bit reserved field in the RTL83xx packet header with the initial cpu_tag word, shifting the real cpu_tag fields by one. Adjusting for this new shift was partially forgotten in the new RX tag decoders. This caused the switch to block IGMP, effectively blocking IPv4 multicast. The bug was partially fixed by commit 9d847244d9fd ("realtek: fix RTL839X receive tag decoding") Fix on RTL838x too, including correct NIC_RX_REASON_SPECIAL_TRAP value. Based-on-fix-by: Jan Hoffmann <jan@3e8.eu> Fixes: dc9cc0d3e2a1 ("realtek: add QoS and rate control") Signed-off-by: Bjørn Mork <bjorn@mork.no> --- .../files-5.10/drivers/net/ethernet/rtl838x_eth.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)