| Message ID | 20220218060959.6631-2-luizluca@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: realtek: add rtl8_4t tag | expand |
Luiz Angelo Daros de Luca <luizluca@gmail.com> writes: > The switch supports the same tag both before ethertype or before CRC. s/The switch supports/Realtek switches support/? I think you should update the documentation at the top of the file as well. > > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> > --- > include/net/dsa.h | 2 + > net/dsa/tag_rtl8_4.c | 127 +++++++++++++++++++++++++++++++++---------- > 2 files changed, 99 insertions(+), 30 deletions(-) > > diff --git a/include/net/dsa.h b/include/net/dsa.h > index fd1f62a6e0a8..b688ced04b0e 100644 > --- a/include/net/dsa.h > +++ b/include/net/dsa.h > @@ -52,6 +52,7 @@ struct phylink_link_state; > #define DSA_TAG_PROTO_BRCM_LEGACY_VALUE 22 > #define DSA_TAG_PROTO_SJA1110_VALUE 23 > #define DSA_TAG_PROTO_RTL8_4_VALUE 24 > +#define DSA_TAG_PROTO_RTL8_4T_VALUE 25 > > enum dsa_tag_protocol { > DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE, > @@ -79,6 +80,7 @@ enum dsa_tag_protocol { > DSA_TAG_PROTO_SEVILLE = DSA_TAG_PROTO_SEVILLE_VALUE, > DSA_TAG_PROTO_SJA1110 = DSA_TAG_PROTO_SJA1110_VALUE, > DSA_TAG_PROTO_RTL8_4 = DSA_TAG_PROTO_RTL8_4_VALUE, > + DSA_TAG_PROTO_RTL8_4T = DSA_TAG_PROTO_RTL8_4T_VALUE, > }; > > struct dsa_switch; > diff --git a/net/dsa/tag_rtl8_4.c b/net/dsa/tag_rtl8_4.c > index 02686ad4045d..d80357cb74b0 100644 > --- a/net/dsa/tag_rtl8_4.c > +++ b/net/dsa/tag_rtl8_4.c > @@ -84,87 +84,133 @@ > #define RTL8_4_TX GENMASK(3, 0) > #define RTL8_4_RX GENMASK(10, 0) > > -static struct sk_buff *rtl8_4_tag_xmit(struct sk_buff *skb, > - struct net_device *dev) > +static void rtl8_4_write_tag(struct sk_buff *skb, struct net_device *dev, > + void *tag) > { > struct dsa_port *dp = dsa_slave_to_port(dev); > - __be16 *tag; > - > - skb_push(skb, RTL8_4_TAG_LEN); > - > - dsa_alloc_etype_header(skb, RTL8_4_TAG_LEN); > - tag = dsa_etype_header_pos_tx(skb); > + __be16 tag16[RTL8_4_TAG_LEN / 2]; > > /* Set Realtek EtherType */ > - tag[0] = htons(ETH_P_REALTEK); > + tag16[0] = htons(ETH_P_REALTEK); > > /* Set Protocol; zero REASON */ > - tag[1] = htons(FIELD_PREP(RTL8_4_PROTOCOL, RTL8_4_PROTOCOL_RTL8365MB)); > + tag16[1] = htons(FIELD_PREP(RTL8_4_PROTOCOL, RTL8_4_PROTOCOL_RTL8365MB)); > > /* Zero FID_EN, FID, PRI_EN, PRI, KEEP; set LEARN_DIS */ > - tag[2] = htons(FIELD_PREP(RTL8_4_LEARN_DIS, 1)); > + tag16[2] = htons(FIELD_PREP(RTL8_4_LEARN_DIS, 1)); > > /* Zero ALLOW; set RX (CPU->switch) forwarding port mask */ > - tag[3] = htons(FIELD_PREP(RTL8_4_RX, BIT(dp->index))); > + tag16[3] = htons(FIELD_PREP(RTL8_4_RX, BIT(dp->index))); > + > + memcpy(tag, tag16, RTL8_4_TAG_LEN); Why not just cast tag to __be16 and avoid an extra memcpy for each xmit? > +} > + > +static struct sk_buff *rtl8_4_tag_xmit(struct sk_buff *skb, > + struct net_device *dev) > +{ > + skb_push(skb, RTL8_4_TAG_LEN); > + > + dsa_alloc_etype_header(skb, RTL8_4_TAG_LEN); > + > + rtl8_4_write_tag(skb, dev, dsa_etype_header_pos_tx(skb)); > > return skb; > } > > -static struct sk_buff *rtl8_4_tag_rcv(struct sk_buff *skb, > - struct net_device *dev) > +static struct sk_buff *rtl8_4t_tag_xmit(struct sk_buff *skb, > + struct net_device *dev) > +{ > + /* Calculate the checksum here if not done yet as trailing tags will > + * break either software and hardware based checksum > + */ > + if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb)) > + return NULL; > + > + rtl8_4_write_tag(skb, dev, skb_put(skb, RTL8_4_TAG_LEN)); > + > + return skb; > +} > + > +static int rtl8_4_read_tag(struct sk_buff *skb, struct net_device *dev, > + void *tag) > { > - __be16 *tag; > u16 etype; > u8 reason; > u8 proto; > u8 port; > + __be16 tag16[RTL8_4_TAG_LEN / 2]; nit: Reverse christmas-tree order? > > - if (unlikely(!pskb_may_pull(skb, RTL8_4_TAG_LEN))) > - return NULL; > - > - tag = dsa_etype_header_pos_rx(skb); > + memcpy(tag16, tag, RTL8_4_TAG_LEN); Likewise can you avoid this memcpy? > > /* Parse Realtek EtherType */ > - etype = ntohs(tag[0]); > + etype = ntohs(tag16[0]); > if (unlikely(etype != ETH_P_REALTEK)) { > dev_warn_ratelimited(&dev->dev, > "non-realtek ethertype 0x%04x\n", etype); > - return NULL; > + return -EPROTO; > } > > /* Parse Protocol */ > - proto = FIELD_GET(RTL8_4_PROTOCOL, ntohs(tag[1])); > + proto = FIELD_GET(RTL8_4_PROTOCOL, ntohs(tag16[1])); > if (unlikely(proto != RTL8_4_PROTOCOL_RTL8365MB)) { > dev_warn_ratelimited(&dev->dev, > "unknown realtek protocol 0x%02x\n", > proto); > - return NULL; > + return -EPROTO; > } > > /* Parse REASON */ > - reason = FIELD_GET(RTL8_4_REASON, ntohs(tag[1])); > + reason = FIELD_GET(RTL8_4_REASON, ntohs(tag16[1])); > > /* Parse TX (switch->CPU) */ > - port = FIELD_GET(RTL8_4_TX, ntohs(tag[3])); > + port = FIELD_GET(RTL8_4_TX, ntohs(tag16[3])); > skb->dev = dsa_master_find_slave(dev, 0, port); > if (!skb->dev) { > dev_warn_ratelimited(&dev->dev, > "could not find slave for port %d\n", > port); > - return NULL; > + return -ENOENT; > } > > + if (reason != RTL8_4_REASON_TRAP) > + dsa_default_offload_fwd_mark(skb); > + > + return 0; > +} > + > +static struct sk_buff *rtl8_4_tag_rcv(struct sk_buff *skb, > + struct net_device *dev) > +{ > + if (unlikely(!pskb_may_pull(skb, RTL8_4_TAG_LEN))) > + return NULL; > + > + if (unlikely(rtl8_4_read_tag(skb, dev, dsa_etype_header_pos_rx(skb)))) > + return NULL; > + > /* Remove tag and recalculate checksum */ > skb_pull_rcsum(skb, RTL8_4_TAG_LEN); > > dsa_strip_etype_header(skb, RTL8_4_TAG_LEN); > > - if (reason != RTL8_4_REASON_TRAP) > - dsa_default_offload_fwd_mark(skb); > + return skb; > +} > + > +static struct sk_buff *rtl8_4t_tag_rcv(struct sk_buff *skb, > + struct net_device *dev) > +{ I wonder if it's necessary to check pskb_may_pull() here too. > + if (skb_linearize(skb)) > + return NULL; > + > + if (unlikely(rtl8_4_read_tag(skb, dev, skb_tail_pointer(skb) - RTL8_4_TAG_LEN))) > + return NULL; > + > + if (pskb_trim_rcsum(skb, skb->len - RTL8_4_TAG_LEN)) > + return NULL; > > return skb; > } > > +/* Ethertype version */ > static const struct dsa_device_ops rtl8_4_netdev_ops = { > .name = "rtl8_4", > .proto = DSA_TAG_PROTO_RTL8_4, > @@ -172,7 +218,28 @@ static const struct dsa_device_ops rtl8_4_netdev_ops = { > .rcv = rtl8_4_tag_rcv, > .needed_headroom = RTL8_4_TAG_LEN, > }; > -module_dsa_tag_driver(rtl8_4_netdev_ops); > > -MODULE_LICENSE("GPL"); > +DSA_TAG_DRIVER(rtl8_4_netdev_ops); > + > MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL8_4); > + > +/* Tail version */ > +static const struct dsa_device_ops rtl8_4t_netdev_ops = { > + .name = "rtl8_4t", > + .proto = DSA_TAG_PROTO_RTL8_4T, > + .xmit = rtl8_4t_tag_xmit, > + .rcv = rtl8_4t_tag_rcv, > + .needed_tailroom = RTL8_4_TAG_LEN, > +}; > + > +DSA_TAG_DRIVER(rtl8_4t_netdev_ops); > + > +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL8_4L); > + > +static struct dsa_tag_driver *dsa_tag_drivers[] = { > + &DSA_TAG_DRIVER_NAME(rtl8_4_netdev_ops), > + &DSA_TAG_DRIVER_NAME(rtl8_4t_netdev_ops), > +}; > +module_dsa_tag_drivers(dsa_tag_drivers); > + > +MODULE_LICENSE("GPL");
Em sex., 18 de fev. de 2022 às 08:46, Alvin Šipraga <ALSI@bang-olufsen.dk> escreveu: > > Luiz Angelo Daros de Luca <luizluca@gmail.com> writes: > > > The switch supports the same tag both before ethertype or before CRC. > > s/The switch supports/Realtek switches support/? Thanks! > > I think you should update the documentation at the top of the file as > well. OK > > > > > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> > > --- > > include/net/dsa.h | 2 + > > net/dsa/tag_rtl8_4.c | 127 +++++++++++++++++++++++++++++++++---------- > > 2 files changed, 99 insertions(+), 30 deletions(-) > > > > diff --git a/include/net/dsa.h b/include/net/dsa.h > > index fd1f62a6e0a8..b688ced04b0e 100644 > > --- a/include/net/dsa.h > > +++ b/include/net/dsa.h > > @@ -52,6 +52,7 @@ struct phylink_link_state; > > #define DSA_TAG_PROTO_BRCM_LEGACY_VALUE 22 > > #define DSA_TAG_PROTO_SJA1110_VALUE 23 > > #define DSA_TAG_PROTO_RTL8_4_VALUE 24 > > +#define DSA_TAG_PROTO_RTL8_4T_VALUE 25 > > > > enum dsa_tag_protocol { > > DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE, > > @@ -79,6 +80,7 @@ enum dsa_tag_protocol { > > DSA_TAG_PROTO_SEVILLE = DSA_TAG_PROTO_SEVILLE_VALUE, > > DSA_TAG_PROTO_SJA1110 = DSA_TAG_PROTO_SJA1110_VALUE, > > DSA_TAG_PROTO_RTL8_4 = DSA_TAG_PROTO_RTL8_4_VALUE, > > + DSA_TAG_PROTO_RTL8_4T = DSA_TAG_PROTO_RTL8_4T_VALUE, > > }; > > > > struct dsa_switch; > > diff --git a/net/dsa/tag_rtl8_4.c b/net/dsa/tag_rtl8_4.c > > index 02686ad4045d..d80357cb74b0 100644 > > --- a/net/dsa/tag_rtl8_4.c > > +++ b/net/dsa/tag_rtl8_4.c > > @@ -84,87 +84,133 @@ > > #define RTL8_4_TX GENMASK(3, 0) > > #define RTL8_4_RX GENMASK(10, 0) > > > > -static struct sk_buff *rtl8_4_tag_xmit(struct sk_buff *skb, > > - struct net_device *dev) > > +static void rtl8_4_write_tag(struct sk_buff *skb, struct net_device *dev, > > + void *tag) > > { > > struct dsa_port *dp = dsa_slave_to_port(dev); > > - __be16 *tag; > > - > > - skb_push(skb, RTL8_4_TAG_LEN); > > - > > - dsa_alloc_etype_header(skb, RTL8_4_TAG_LEN); > > - tag = dsa_etype_header_pos_tx(skb); > > + __be16 tag16[RTL8_4_TAG_LEN / 2]; > > > > /* Set Realtek EtherType */ > > - tag[0] = htons(ETH_P_REALTEK); > > + tag16[0] = htons(ETH_P_REALTEK); > > > > /* Set Protocol; zero REASON */ > > - tag[1] = htons(FIELD_PREP(RTL8_4_PROTOCOL, RTL8_4_PROTOCOL_RTL8365MB)); > > + tag16[1] = htons(FIELD_PREP(RTL8_4_PROTOCOL, RTL8_4_PROTOCOL_RTL8365MB)); > > > > /* Zero FID_EN, FID, PRI_EN, PRI, KEEP; set LEARN_DIS */ > > - tag[2] = htons(FIELD_PREP(RTL8_4_LEARN_DIS, 1)); > > + tag16[2] = htons(FIELD_PREP(RTL8_4_LEARN_DIS, 1)); > > > > /* Zero ALLOW; set RX (CPU->switch) forwarding port mask */ > > - tag[3] = htons(FIELD_PREP(RTL8_4_RX, BIT(dp->index))); > > + tag16[3] = htons(FIELD_PREP(RTL8_4_RX, BIT(dp->index))); > > + > > + memcpy(tag, tag16, RTL8_4_TAG_LEN); > > Why not just cast tag to __be16 and avoid an extra memcpy for each xmit? The last version I sent, there was a valid concern about unaligned access. With ethertype tags, we know the exact position the tag will be. However, at the end of the packet, the two bytes might fall into different words depending on the payload. I did test different payloads without any issues but my big endian system might have helped. I checked the machine code and the compiler seems to be doing a good job here, converting the memcpy to a simple "register to memory" each byte at a time. > > > +} > > + > > +static struct sk_buff *rtl8_4_tag_xmit(struct sk_buff *skb, > > + struct net_device *dev) > > +{ > > + skb_push(skb, RTL8_4_TAG_LEN); > > + > > + dsa_alloc_etype_header(skb, RTL8_4_TAG_LEN); > > + > > + rtl8_4_write_tag(skb, dev, dsa_etype_header_pos_tx(skb)); > > > > return skb; > > } > > > > -static struct sk_buff *rtl8_4_tag_rcv(struct sk_buff *skb, > > - struct net_device *dev) > > +static struct sk_buff *rtl8_4t_tag_xmit(struct sk_buff *skb, > > + struct net_device *dev) > > +{ > > + /* Calculate the checksum here if not done yet as trailing tags will > > + * break either software and hardware based checksum > > + */ > > + if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb)) > > + return NULL; > > + > > + rtl8_4_write_tag(skb, dev, skb_put(skb, RTL8_4_TAG_LEN)); > > + > > + return skb; > > +} > > + > > +static int rtl8_4_read_tag(struct sk_buff *skb, struct net_device *dev, > > + void *tag) > > { > > - __be16 *tag; > > u16 etype; > > u8 reason; > > u8 proto; > > u8 port; > > + __be16 tag16[RTL8_4_TAG_LEN / 2]; > > nit: Reverse christmas-tree order? Sure! My bad. > > > > > - if (unlikely(!pskb_may_pull(skb, RTL8_4_TAG_LEN))) > > - return NULL; > > - > > - tag = dsa_etype_header_pos_rx(skb); > > + memcpy(tag16, tag, RTL8_4_TAG_LEN); > > Likewise can you avoid this memcpy? > > > > > /* Parse Realtek EtherType */ > > - etype = ntohs(tag[0]); > > + etype = ntohs(tag16[0]); > > if (unlikely(etype != ETH_P_REALTEK)) { > > dev_warn_ratelimited(&dev->dev, > > "non-realtek ethertype 0x%04x\n", etype); > > - return NULL; > > + return -EPROTO; > > } > > > > /* Parse Protocol */ > > - proto = FIELD_GET(RTL8_4_PROTOCOL, ntohs(tag[1])); > > + proto = FIELD_GET(RTL8_4_PROTOCOL, ntohs(tag16[1])); > > if (unlikely(proto != RTL8_4_PROTOCOL_RTL8365MB)) { > > dev_warn_ratelimited(&dev->dev, > > "unknown realtek protocol 0x%02x\n", > > proto); > > - return NULL; > > + return -EPROTO; > > } > > > > /* Parse REASON */ > > - reason = FIELD_GET(RTL8_4_REASON, ntohs(tag[1])); > > + reason = FIELD_GET(RTL8_4_REASON, ntohs(tag16[1])); > > > > /* Parse TX (switch->CPU) */ > > - port = FIELD_GET(RTL8_4_TX, ntohs(tag[3])); > > + port = FIELD_GET(RTL8_4_TX, ntohs(tag16[3])); > > skb->dev = dsa_master_find_slave(dev, 0, port); > > if (!skb->dev) { > > dev_warn_ratelimited(&dev->dev, > > "could not find slave for port %d\n", > > port); > > - return NULL; > > + return -ENOENT; > > } > > > > + if (reason != RTL8_4_REASON_TRAP) > > + dsa_default_offload_fwd_mark(skb); > > + > > + return 0; > > +} > > + > > +static struct sk_buff *rtl8_4_tag_rcv(struct sk_buff *skb, > > + struct net_device *dev) > > +{ > > + if (unlikely(!pskb_may_pull(skb, RTL8_4_TAG_LEN))) > > + return NULL; > > + > > + if (unlikely(rtl8_4_read_tag(skb, dev, dsa_etype_header_pos_rx(skb)))) > > + return NULL; > > + > > /* Remove tag and recalculate checksum */ > > skb_pull_rcsum(skb, RTL8_4_TAG_LEN); > > > > dsa_strip_etype_header(skb, RTL8_4_TAG_LEN); > > > > - if (reason != RTL8_4_REASON_TRAP) > > - dsa_default_offload_fwd_mark(skb); > > + return skb; > > +} > > + > > +static struct sk_buff *rtl8_4t_tag_rcv(struct sk_buff *skb, > > + struct net_device *dev) > > +{ > > I wonder if it's necessary to check pskb_may_pull() here too. I didn't add it because no trailing tag used it. I checked tag_hellcreek.c, tag_ksz.c, tag_xrs700x.c. tag_sja1105.c seems to use both head and tail space and it indeed use pskb_may_pull() but only related to the head space (SJA1110_HEADER_LEN). > > > + if (skb_linearize(skb)) > > + return NULL; > > + > > + if (unlikely(rtl8_4_read_tag(skb, dev, skb_tail_pointer(skb) - RTL8_4_TAG_LEN))) > > + return NULL; > > + > > + if (pskb_trim_rcsum(skb, skb->len - RTL8_4_TAG_LEN)) > > + return NULL; > > > > return skb; > > } > > > > +/* Ethertype version */ > > static const struct dsa_device_ops rtl8_4_netdev_ops = { > > .name = "rtl8_4", > > .proto = DSA_TAG_PROTO_RTL8_4, > > @@ -172,7 +218,28 @@ static const struct dsa_device_ops rtl8_4_netdev_ops = { > > .rcv = rtl8_4_tag_rcv, > > .needed_headroom = RTL8_4_TAG_LEN, > > }; > > -module_dsa_tag_driver(rtl8_4_netdev_ops); > > > > -MODULE_LICENSE("GPL"); > > +DSA_TAG_DRIVER(rtl8_4_netdev_ops); > > + > > MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL8_4); > > + > > +/* Tail version */ > > +static const struct dsa_device_ops rtl8_4t_netdev_ops = { > > + .name = "rtl8_4t", > > + .proto = DSA_TAG_PROTO_RTL8_4T, > > + .xmit = rtl8_4t_tag_xmit, > > + .rcv = rtl8_4t_tag_rcv, > > + .needed_tailroom = RTL8_4_TAG_LEN, > > +}; > > + > > +DSA_TAG_DRIVER(rtl8_4t_netdev_ops); > > + > > +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL8_4L); > > + > > +static struct dsa_tag_driver *dsa_tag_drivers[] = { > > + &DSA_TAG_DRIVER_NAME(rtl8_4_netdev_ops), > > + &DSA_TAG_DRIVER_NAME(rtl8_4t_netdev_ops), > > +}; > > +module_dsa_tag_drivers(dsa_tag_drivers); > > + > > +MODULE_LICENSE("GPL");
diff --git a/include/net/dsa.h b/include/net/dsa.h index fd1f62a6e0a8..b688ced04b0e 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -52,6 +52,7 @@ struct phylink_link_state; #define DSA_TAG_PROTO_BRCM_LEGACY_VALUE 22 #define DSA_TAG_PROTO_SJA1110_VALUE 23 #define DSA_TAG_PROTO_RTL8_4_VALUE 24 +#define DSA_TAG_PROTO_RTL8_4T_VALUE 25 enum dsa_tag_protocol { DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE, @@ -79,6 +80,7 @@ enum dsa_tag_protocol { DSA_TAG_PROTO_SEVILLE = DSA_TAG_PROTO_SEVILLE_VALUE, DSA_TAG_PROTO_SJA1110 = DSA_TAG_PROTO_SJA1110_VALUE, DSA_TAG_PROTO_RTL8_4 = DSA_TAG_PROTO_RTL8_4_VALUE, + DSA_TAG_PROTO_RTL8_4T = DSA_TAG_PROTO_RTL8_4T_VALUE, }; struct dsa_switch; diff --git a/net/dsa/tag_rtl8_4.c b/net/dsa/tag_rtl8_4.c index 02686ad4045d..d80357cb74b0 100644 --- a/net/dsa/tag_rtl8_4.c +++ b/net/dsa/tag_rtl8_4.c @@ -84,87 +84,133 @@ #define RTL8_4_TX GENMASK(3, 0) #define RTL8_4_RX GENMASK(10, 0) -static struct sk_buff *rtl8_4_tag_xmit(struct sk_buff *skb, - struct net_device *dev) +static void rtl8_4_write_tag(struct sk_buff *skb, struct net_device *dev, + void *tag) { struct dsa_port *dp = dsa_slave_to_port(dev); - __be16 *tag; - - skb_push(skb, RTL8_4_TAG_LEN); - - dsa_alloc_etype_header(skb, RTL8_4_TAG_LEN); - tag = dsa_etype_header_pos_tx(skb); + __be16 tag16[RTL8_4_TAG_LEN / 2]; /* Set Realtek EtherType */ - tag[0] = htons(ETH_P_REALTEK); + tag16[0] = htons(ETH_P_REALTEK); /* Set Protocol; zero REASON */ - tag[1] = htons(FIELD_PREP(RTL8_4_PROTOCOL, RTL8_4_PROTOCOL_RTL8365MB)); + tag16[1] = htons(FIELD_PREP(RTL8_4_PROTOCOL, RTL8_4_PROTOCOL_RTL8365MB)); /* Zero FID_EN, FID, PRI_EN, PRI, KEEP; set LEARN_DIS */ - tag[2] = htons(FIELD_PREP(RTL8_4_LEARN_DIS, 1)); + tag16[2] = htons(FIELD_PREP(RTL8_4_LEARN_DIS, 1)); /* Zero ALLOW; set RX (CPU->switch) forwarding port mask */ - tag[3] = htons(FIELD_PREP(RTL8_4_RX, BIT(dp->index))); + tag16[3] = htons(FIELD_PREP(RTL8_4_RX, BIT(dp->index))); + + memcpy(tag, tag16, RTL8_4_TAG_LEN); +} + +static struct sk_buff *rtl8_4_tag_xmit(struct sk_buff *skb, + struct net_device *dev) +{ + skb_push(skb, RTL8_4_TAG_LEN); + + dsa_alloc_etype_header(skb, RTL8_4_TAG_LEN); + + rtl8_4_write_tag(skb, dev, dsa_etype_header_pos_tx(skb)); return skb; } -static struct sk_buff *rtl8_4_tag_rcv(struct sk_buff *skb, - struct net_device *dev) +static struct sk_buff *rtl8_4t_tag_xmit(struct sk_buff *skb, + struct net_device *dev) +{ + /* Calculate the checksum here if not done yet as trailing tags will + * break either software and hardware based checksum + */ + if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb)) + return NULL; + + rtl8_4_write_tag(skb, dev, skb_put(skb, RTL8_4_TAG_LEN)); + + return skb; +} + +static int rtl8_4_read_tag(struct sk_buff *skb, struct net_device *dev, + void *tag) { - __be16 *tag; u16 etype; u8 reason; u8 proto; u8 port; + __be16 tag16[RTL8_4_TAG_LEN / 2]; - if (unlikely(!pskb_may_pull(skb, RTL8_4_TAG_LEN))) - return NULL; - - tag = dsa_etype_header_pos_rx(skb); + memcpy(tag16, tag, RTL8_4_TAG_LEN); /* Parse Realtek EtherType */ - etype = ntohs(tag[0]); + etype = ntohs(tag16[0]); if (unlikely(etype != ETH_P_REALTEK)) { dev_warn_ratelimited(&dev->dev, "non-realtek ethertype 0x%04x\n", etype); - return NULL; + return -EPROTO; } /* Parse Protocol */ - proto = FIELD_GET(RTL8_4_PROTOCOL, ntohs(tag[1])); + proto = FIELD_GET(RTL8_4_PROTOCOL, ntohs(tag16[1])); if (unlikely(proto != RTL8_4_PROTOCOL_RTL8365MB)) { dev_warn_ratelimited(&dev->dev, "unknown realtek protocol 0x%02x\n", proto); - return NULL; + return -EPROTO; } /* Parse REASON */ - reason = FIELD_GET(RTL8_4_REASON, ntohs(tag[1])); + reason = FIELD_GET(RTL8_4_REASON, ntohs(tag16[1])); /* Parse TX (switch->CPU) */ - port = FIELD_GET(RTL8_4_TX, ntohs(tag[3])); + port = FIELD_GET(RTL8_4_TX, ntohs(tag16[3])); skb->dev = dsa_master_find_slave(dev, 0, port); if (!skb->dev) { dev_warn_ratelimited(&dev->dev, "could not find slave for port %d\n", port); - return NULL; + return -ENOENT; } + if (reason != RTL8_4_REASON_TRAP) + dsa_default_offload_fwd_mark(skb); + + return 0; +} + +static struct sk_buff *rtl8_4_tag_rcv(struct sk_buff *skb, + struct net_device *dev) +{ + if (unlikely(!pskb_may_pull(skb, RTL8_4_TAG_LEN))) + return NULL; + + if (unlikely(rtl8_4_read_tag(skb, dev, dsa_etype_header_pos_rx(skb)))) + return NULL; + /* Remove tag and recalculate checksum */ skb_pull_rcsum(skb, RTL8_4_TAG_LEN); dsa_strip_etype_header(skb, RTL8_4_TAG_LEN); - if (reason != RTL8_4_REASON_TRAP) - dsa_default_offload_fwd_mark(skb); + return skb; +} + +static struct sk_buff *rtl8_4t_tag_rcv(struct sk_buff *skb, + struct net_device *dev) +{ + if (skb_linearize(skb)) + return NULL; + + if (unlikely(rtl8_4_read_tag(skb, dev, skb_tail_pointer(skb) - RTL8_4_TAG_LEN))) + return NULL; + + if (pskb_trim_rcsum(skb, skb->len - RTL8_4_TAG_LEN)) + return NULL; return skb; } +/* Ethertype version */ static const struct dsa_device_ops rtl8_4_netdev_ops = { .name = "rtl8_4", .proto = DSA_TAG_PROTO_RTL8_4, @@ -172,7 +218,28 @@ static const struct dsa_device_ops rtl8_4_netdev_ops = { .rcv = rtl8_4_tag_rcv, .needed_headroom = RTL8_4_TAG_LEN, }; -module_dsa_tag_driver(rtl8_4_netdev_ops); -MODULE_LICENSE("GPL"); +DSA_TAG_DRIVER(rtl8_4_netdev_ops); + MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL8_4); + +/* Tail version */ +static const struct dsa_device_ops rtl8_4t_netdev_ops = { + .name = "rtl8_4t", + .proto = DSA_TAG_PROTO_RTL8_4T, + .xmit = rtl8_4t_tag_xmit, + .rcv = rtl8_4t_tag_rcv, + .needed_tailroom = RTL8_4_TAG_LEN, +}; + +DSA_TAG_DRIVER(rtl8_4t_netdev_ops); + +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL8_4L); + +static struct dsa_tag_driver *dsa_tag_drivers[] = { + &DSA_TAG_DRIVER_NAME(rtl8_4_netdev_ops), + &DSA_TAG_DRIVER_NAME(rtl8_4t_netdev_ops), +}; +module_dsa_tag_drivers(dsa_tag_drivers); + +MODULE_LICENSE("GPL");
The switch supports the same tag both before ethertype or before CRC. Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> --- include/net/dsa.h | 2 + net/dsa/tag_rtl8_4.c | 127 +++++++++++++++++++++++++++++++++---------- 2 files changed, 99 insertions(+), 30 deletions(-)