| Message ID | 20220514150656.122108-3-maxime.chevallier@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | net: ipqess: introduce Qualcomm IPQESS driver | expand |
Hi Maxime, On 5/14/2022 8:06 AM, Maxime Chevallier wrote: > This tagging protocol is designed for the situation where the link > between the MAC and the Switch is designed such that the Destination > Port, which is usually embedded in some part of the Ethernet Header, is > sent out-of-band, and isn't present at all in the Ethernet frame. > > This can happen when the MAC and Switch are tightly integrated on an > SoC, as is the case with the Qualcomm IPQ4019 for example, where the DSA > tag is inserted directly into the DMA descriptors. In that case, > the MAC driver is responsible for sending the tag to the switch using > the out-of-band medium. To do so, the MAC driver needs to have the > information of the destination port for that skb. > > This out-of-band tagging protocol is using the very beggining of the skb > headroom to store the tag. The drawback of this approch is that the > headroom isn't initialized upon allocating it, therefore we have a > chance that the garbage data that lies there at allocation time actually > ressembles a valid oob tag. This is only problematic if we are > sending/receiving traffic on the master port, which isn't a valid DSA > use-case from the beggining. When dealing from traffic to/from a slave > port, then the oob tag will be initialized properly by the tagger or the > mac driver through the use of the dsa_oob_tag_push() call. What I like about your approach is that you have aligned the way an out of band switch tag is communicated to the networking stack the same way that an "in-band" switch tag would be communicated. I think this is a good way forward to provide the out of band tag and I don't think it creates a performance problem because the Ethernet frame is hot in the cache (dma_unmap_single()) and we already have an "expensive" read of the DMA descriptor in coherent memory anyway. You could possibly optimize the data flow a bit to limit the amount of sk_buff data movement by asking your Ethernet controller to DMA into the data buffer N bytes into the beginning of the data buffer. That way, if you have reserved say, 2 bytes at the front data buffer you can deposit the QCA tag there and you do not need to push, process the tag, then pop it, just process and pop. Consider using the 2byte stuffing that the Ethernet controller might be adding to the beginning of the Ethernet frame to align the IP header on a 4-byte boundary to provide the tag in there? If we want to have a generic out of band tagger like you propose, it seems to me that we will need to invent a synthetic DSA tagging format which is the largest common denominator of the out of band tags that we want to support. We could imagine being more compact in the representation for instance by using an u8 for storing a bitmask of ports (works for both RX and TX then) and another u8 for various packet forwarding reasons. Then we would request the various Ethernet MAC drivers to marshall their proprietary tag into the DSA synthetic one on receive, and unmarshall it on transmit. Another approach IMHO which maybe helps the maintainability of the code moving forward as well as ensuring that all Ethernet switch tagging code lives in one place, is to teach each tagger driver how to optimize their data paths to minimize the amount of data movements and checksum re-calculations, this is what I had in mind a few years ago: https://lore.kernel.org/lkml/1438322920.20182.144.camel@edumazet-glaptop2.roam.corp.google.com/T/ This might scale a little less well, and maybe this makes too many assumptions as to where and how the checksums are calculated on the packet contents, but at least, you don't have logic processing the same type of switch tag scattered between the Ethernet MAC drivers (beyond copying/pushing) and DSA switch taggers. I would like to hear other's opinion on this.
On Sat, May 14, 2022 at 05:06:53PM +0200, Maxime Chevallier wrote: > This tagging protocol is designed for the situation where the link > between the MAC and the Switch is designed such that the Destination > Port, which is usually embedded in some part of the Ethernet Header, is > sent out-of-band, and isn't present at all in the Ethernet frame. > > This can happen when the MAC and Switch are tightly integrated on an > SoC, as is the case with the Qualcomm IPQ4019 for example, where the DSA > tag is inserted directly into the DMA descriptors. In that case, > the MAC driver is responsible for sending the tag to the switch using > the out-of-band medium. To do so, the MAC driver needs to have the > information of the destination port for that skb. > > This out-of-band tagging protocol is using the very beggining of the skb > headroom to store the tag. The drawback of this approch is that the > headroom isn't initialized upon allocating it, therefore we have a > chance that the garbage data that lies there at allocation time actually > ressembles a valid oob tag. This is only problematic if we are > sending/receiving traffic on the master port, which isn't a valid DSA > use-case from the beggining. When dealing from traffic to/from a slave > port, then the oob tag will be initialized properly by the tagger or the > mac driver through the use of the dsa_oob_tag_push() call. > > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> > --- Why put the DSA pseudo-header at skb->head rather than push it using skb_push()? I thought you were going to check for the presence of a DSA header using something like skb->mac_len == ETH_HLEN + tag len, but right now it sounds like treating garbage in the headroom as a valid DSA tag is indeed a potential problem. If you can't sort that out using information from the header offsets alone, maybe an skb extension is required?
On Sat, 14 May 2022 17:06:53 +0200 Maxime Chevallier wrote: > This tagging protocol is designed for the situation where the link > between the MAC and the Switch is designed such that the Destination > Port, which is usually embedded in some part of the Ethernet Header, is > sent out-of-band, and isn't present at all in the Ethernet frame. > > This can happen when the MAC and Switch are tightly integrated on an > SoC, as is the case with the Qualcomm IPQ4019 for example, where the DSA > tag is inserted directly into the DMA descriptors. In that case, > the MAC driver is responsible for sending the tag to the switch using > the out-of-band medium. To do so, the MAC driver needs to have the > information of the destination port for that skb. > > This out-of-band tagging protocol is using the very beggining of the skb > headroom to store the tag. The drawback of this approch is that the > headroom isn't initialized upon allocating it, therefore we have a > chance that the garbage data that lies there at allocation time actually > ressembles a valid oob tag. This is only problematic if we are > sending/receiving traffic on the master port, which isn't a valid DSA > use-case from the beggining. When dealing from traffic to/from a slave > port, then the oob tag will be initialized properly by the tagger or the > mac driver through the use of the dsa_oob_tag_push() call. > > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> This must had been asked on v1 but there's no trace of it in the current submission afaict... If the tag is passed in the descriptor how is this not a pure switchdev driver? The explanation must be preserved somehow.
Hello Jakub, On Mon, 16 May 2022 12:20:48 -0700 Jakub Kicinski <kuba@kernel.org> wrote: > On Sat, 14 May 2022 17:06:53 +0200 Maxime Chevallier wrote: > > This tagging protocol is designed for the situation where the link > > between the MAC and the Switch is designed such that the Destination > > Port, which is usually embedded in some part of the Ethernet > > Header, is sent out-of-band, and isn't present at all in the > > Ethernet frame. > > > > This can happen when the MAC and Switch are tightly integrated on an > > SoC, as is the case with the Qualcomm IPQ4019 for example, where > > the DSA tag is inserted directly into the DMA descriptors. In that > > case, the MAC driver is responsible for sending the tag to the > > switch using the out-of-band medium. To do so, the MAC driver needs > > to have the information of the destination port for that skb. > > > > This out-of-band tagging protocol is using the very beggining of > > the skb headroom to store the tag. The drawback of this approch is > > that the headroom isn't initialized upon allocating it, therefore > > we have a chance that the garbage data that lies there at > > allocation time actually ressembles a valid oob tag. This is only > > problematic if we are sending/receiving traffic on the master port, > > which isn't a valid DSA use-case from the beggining. When dealing > > from traffic to/from a slave port, then the oob tag will be > > initialized properly by the tagger or the mac driver through the > > use of the dsa_oob_tag_push() call. > > > > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> > > This must had been asked on v1 but there's no trace of it in the > current submission afaict... No you're correct, this wasn't explained. > If the tag is passed in the descriptor how is this not a pure > switchdev driver? The explanation must be preserved somehow. The main reason is that although the MAC and switch are rightly coupled on that platform, the switch is actually a QC8K that can live on it's own, as an external switch. Here, it's just a slightly modified version of this IP. The same goes for the MAC IP, but so far we don't support any other platform that have the MAC as a standalone controller. As far as we can tell, platforms that have this MAC also include a QCA8K, but the datasheet also mentions other modes (like outputing RGMII). Is this valid to have it as a standalone ethernet driver in that situation ? Thanks, Maxime
Hi Vlad, On Sat, 14 May 2022 22:40:03 +0000 Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > On Sat, May 14, 2022 at 05:06:53PM +0200, Maxime Chevallier wrote: > > This tagging protocol is designed for the situation where the link > > between the MAC and the Switch is designed such that the Destination > > Port, which is usually embedded in some part of the Ethernet > > Header, is sent out-of-band, and isn't present at all in the > > Ethernet frame. > > > > This can happen when the MAC and Switch are tightly integrated on an > > SoC, as is the case with the Qualcomm IPQ4019 for example, where > > the DSA tag is inserted directly into the DMA descriptors. In that > > case, the MAC driver is responsible for sending the tag to the > > switch using the out-of-band medium. To do so, the MAC driver needs > > to have the information of the destination port for that skb. > > > > This out-of-band tagging protocol is using the very beggining of > > the skb headroom to store the tag. The drawback of this approch is > > that the headroom isn't initialized upon allocating it, therefore > > we have a chance that the garbage data that lies there at > > allocation time actually ressembles a valid oob tag. This is only > > problematic if we are sending/receiving traffic on the master port, > > which isn't a valid DSA use-case from the beggining. When dealing > > from traffic to/from a slave port, then the oob tag will be > > initialized properly by the tagger or the mac driver through the > > use of the dsa_oob_tag_push() call. > > > > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> > > --- > > Why put the DSA pseudo-header at skb->head rather than push it using > skb_push()? I thought you were going to check for the presence of a > DSA header using something like skb->mac_len == ETH_HLEN + tag len, > but right now it sounds like treating garbage in the headroom as a > valid DSA tag is indeed a potential problem. If you can't sort that > out using information from the header offsets alone, maybe an skb > extension is required? Indeed, I thought of that, the main reason is that pushing/poping in itself is not enough, you also have to move the whole mac_header to leave room for the tag, and then re-set it in it's original location. There's nothing wrong with this, but it looked a bit cumbersome just to insert a dummy tag that gets removed rightaway. Does that make sense ? But yes I would really like to get a way to know wether the tag is there or not, I'll dig a bit more to see if I can find a way to get this info from the various skb offsets in a reliable way. Thanks, Maxime
Hi Florian, On Sat, 14 May 2022 09:33:44 -0700 Florian Fainelli <f.fainelli@gmail.com> wrote: > Hi Maxime, > > On 5/14/2022 8:06 AM, Maxime Chevallier wrote: > > This tagging protocol is designed for the situation where the link > > between the MAC and the Switch is designed such that the Destination > > Port, which is usually embedded in some part of the Ethernet > > Header, is sent out-of-band, and isn't present at all in the > > Ethernet frame. > > > > This can happen when the MAC and Switch are tightly integrated on an > > SoC, as is the case with the Qualcomm IPQ4019 for example, where > > the DSA tag is inserted directly into the DMA descriptors. In that > > case, the MAC driver is responsible for sending the tag to the > > switch using the out-of-band medium. To do so, the MAC driver needs > > to have the information of the destination port for that skb. > > > > This out-of-band tagging protocol is using the very beggining of > > the skb headroom to store the tag. The drawback of this approch is > > that the headroom isn't initialized upon allocating it, therefore > > we have a chance that the garbage data that lies there at > > allocation time actually ressembles a valid oob tag. This is only > > problematic if we are sending/receiving traffic on the master port, > > which isn't a valid DSA use-case from the beggining. When dealing > > from traffic to/from a slave port, then the oob tag will be > > initialized properly by the tagger or the mac driver through the > > use of the dsa_oob_tag_push() call. > > What I like about your approach is that you have aligned the way an > out of band switch tag is communicated to the networking stack the > same way that an "in-band" switch tag would be communicated. I think > this is a good way forward to provide the out of band tag and I don't > think it creates a performance problem because the Ethernet frame is > hot in the cache (dma_unmap_single()) and we already have an > "expensive" read of the DMA descriptor in coherent memory anyway. > > You could possibly optimize the data flow a bit to limit the amount > of sk_buff data movement by asking your Ethernet controller to DMA > into the data buffer N bytes into the beginning of the data buffer. > That way, if you have reserved say, 2 bytes at the front data buffer > you can deposit the QCA tag there and you do not need to push, > process the tag, then pop it, just process and pop. Consider using > the 2byte stuffing that the Ethernet controller might be adding to > the beginning of the Ethernet frame to align the IP header on a > 4-byte boundary to provide the tag in there? > > If we want to have a generic out of band tagger like you propose, it > seems to me that we will need to invent a synthetic DSA tagging > format which is the largest common denominator of the out of band > tags that we want to support. We could imagine being more compact in > the representation for instance by using an u8 for storing a bitmask > of ports (works for both RX and TX then) and another u8 for various > packet forwarding reasons. Thanks, that was my initial idea indeed. Having a generic tagger that can be re-used would be great IMO. I'll modify the format as you propose, and also give a try to you approach of DMA'ing 2 bytes forward so that the tag location is already allocated, that's a nice idea. > Then we would request the various Ethernet MAC drivers to marshall > their proprietary tag into the DSA synthetic one on receive, and > unmarshall it on transmit. > > Another approach IMHO which maybe helps the maintainability of the > code moving forward as well as ensuring that all Ethernet switch > tagging code lives in one place, is to teach each tagger driver how > to optimize their data paths to minimize the amount of data movements > and checksum re-calculations, this is what I had in mind a few years > ago: > > https://lore.kernel.org/lkml/1438322920.20182.144.camel@edumazet-glaptop2.roam.corp.google.com/T/ > > This might scale a little less well, and maybe this makes too many > assumptions as to where and how the checksums are calculated on the > packet contents, but at least, you don't have logic processing the > same type of switch tag scattered between the Ethernet MAC drivers > (beyond copying/pushing) and DSA switch taggers. That would definitely fit well with this tagger, I didn't know about that series ! Thanks for the review, Maxime > I would like to hear other's opinion on this.
On Tue, 17 May 2022 08:53:55 +0200 Maxime Chevallier wrote: > > This must had been asked on v1 but there's no trace of it in the > > current submission afaict... > > No you're correct, this wasn't explained. > > > If the tag is passed in the descriptor how is this not a pure > > switchdev driver? The explanation must be preserved somehow. > > The main reason is that although the MAC and switch are rightly coupled > on that platform, the switch is actually a QC8K that can live on it's > own, as an external switch. Here, it's just a slightly modified version > of this IP. > > The same goes for the MAC IP, but so far we don't support any other > platform that have the MAC as a standalone controller. As far as we can > tell, platforms that have this MAC also include a QCA8K, but the > datasheet also mentions other modes (like outputing RGMII). Got it, thanks! Please weave this justification more explicitly into the cover letter. > Is this valid to have it as a standalone ethernet driver in that > situation ? Quite possibly.. I won't pretend I've looked at the code, I defer to the reviewers :)
On Tue, May 17, 2022 at 09:01:56AM +0200, Maxime Chevallier wrote: > Hi Vlad, > > On Sat, 14 May 2022 22:40:03 +0000 > Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > > > On Sat, May 14, 2022 at 05:06:53PM +0200, Maxime Chevallier wrote: > > > This tagging protocol is designed for the situation where the link > > > between the MAC and the Switch is designed such that the Destination > > > Port, which is usually embedded in some part of the Ethernet > > > Header, is sent out-of-band, and isn't present at all in the > > > Ethernet frame. > > > > > > This can happen when the MAC and Switch are tightly integrated on an > > > SoC, as is the case with the Qualcomm IPQ4019 for example, where > > > the DSA tag is inserted directly into the DMA descriptors. In that > > > case, the MAC driver is responsible for sending the tag to the > > > switch using the out-of-band medium. To do so, the MAC driver needs > > > to have the information of the destination port for that skb. > > > > > > This out-of-band tagging protocol is using the very beggining of > > > the skb headroom to store the tag. The drawback of this approch is > > > that the headroom isn't initialized upon allocating it, therefore > > > we have a chance that the garbage data that lies there at > > > allocation time actually ressembles a valid oob tag. This is only > > > problematic if we are sending/receiving traffic on the master port, > > > which isn't a valid DSA use-case from the beggining. When dealing > > > from traffic to/from a slave port, then the oob tag will be > > > initialized properly by the tagger or the mac driver through the > > > use of the dsa_oob_tag_push() call. > > > > > > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> > > > --- > > > > Why put the DSA pseudo-header at skb->head rather than push it using > > skb_push()? I thought you were going to check for the presence of a > > DSA header using something like skb->mac_len == ETH_HLEN + tag len, > > but right now it sounds like treating garbage in the headroom as a > > valid DSA tag is indeed a potential problem. If you can't sort that > > out using information from the header offsets alone, maybe an skb > > extension is required? > > Indeed, I thought of that, the main reason is that pushing/poping in > itself is not enough, you also have to move the whole mac_header to > leave room for the tag, and then re-set it in it's original location. > There's nothing wrong with this, but it looked a bit cumbersome just to > insert a dummy tag that gets removed rightaway. Does that make sense ? You're thinking about inserting a header before the EtherType. But what has been said was to _prepend_ a header, i.e. put it before the Ethernet MAC DA. That way you don't need to move the Ethernet header. But anyway, too much talk for mostly nothing, see below. > But yes I would really like to get a way to know wether the tag is > there or not, I'll dig a bit more to see if I can find a way to get > this info from the various skb offsets in a reliable way. Without an skb extension, this seems like an impossible task to me (which should also answer Florian's request for feedback on the proposal to share skb->cb with GRO, the qdisc, and whomever else there might be in the path between the DSA master and the switch).
On 5/19/2022 7:52 AM, Vladimir Oltean wrote: > On Tue, May 17, 2022 at 09:01:56AM +0200, Maxime Chevallier wrote: >> Hi Vlad, >> >> On Sat, 14 May 2022 22:40:03 +0000 >> Vladimir Oltean <vladimir.oltean@nxp.com> wrote: >> >>> On Sat, May 14, 2022 at 05:06:53PM +0200, Maxime Chevallier wrote: >>>> This tagging protocol is designed for the situation where the link >>>> between the MAC and the Switch is designed such that the Destination >>>> Port, which is usually embedded in some part of the Ethernet >>>> Header, is sent out-of-band, and isn't present at all in the >>>> Ethernet frame. >>>> >>>> This can happen when the MAC and Switch are tightly integrated on an >>>> SoC, as is the case with the Qualcomm IPQ4019 for example, where >>>> the DSA tag is inserted directly into the DMA descriptors. In that >>>> case, the MAC driver is responsible for sending the tag to the >>>> switch using the out-of-band medium. To do so, the MAC driver needs >>>> to have the information of the destination port for that skb. >>>> >>>> This out-of-band tagging protocol is using the very beggining of >>>> the skb headroom to store the tag. The drawback of this approch is >>>> that the headroom isn't initialized upon allocating it, therefore >>>> we have a chance that the garbage data that lies there at >>>> allocation time actually ressembles a valid oob tag. This is only >>>> problematic if we are sending/receiving traffic on the master port, >>>> which isn't a valid DSA use-case from the beggining. When dealing >>>> from traffic to/from a slave port, then the oob tag will be >>>> initialized properly by the tagger or the mac driver through the >>>> use of the dsa_oob_tag_push() call. >>>> >>>> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> >>>> --- >>> >>> Why put the DSA pseudo-header at skb->head rather than push it using >>> skb_push()? I thought you were going to check for the presence of a >>> DSA header using something like skb->mac_len == ETH_HLEN + tag len, >>> but right now it sounds like treating garbage in the headroom as a >>> valid DSA tag is indeed a potential problem. If you can't sort that >>> out using information from the header offsets alone, maybe an skb >>> extension is required? >> >> Indeed, I thought of that, the main reason is that pushing/poping in >> itself is not enough, you also have to move the whole mac_header to >> leave room for the tag, and then re-set it in it's original location. >> There's nothing wrong with this, but it looked a bit cumbersome just to >> insert a dummy tag that gets removed rightaway. Does that make sense ? > > You're thinking about inserting a header before the EtherType. But what > has been said was to _prepend_ a header, i.e. put it before the Ethernet > MAC DA. That way you don't need to move the Ethernet header. > > But anyway, too much talk for mostly nothing, see below. > >> But yes I would really like to get a way to know wether the tag is >> there or not, I'll dig a bit more to see if I can find a way to get >> this info from the various skb offsets in a reliable way. > > Without an skb extension, this seems like an impossible task to me > (which should also answer Florian's request for feedback on the proposal > to share skb->cb with GRO, the qdisc, and whomever else there might be > in the path between the DSA master and the switch). Sorry I should have been clearer, the patch series that I pointed Maxime at earlier: https://lore.kernel.org/lkml/1438322920.20182.144.camel@edumazet-glaptop2.roam.corp.google.com/T/ was initially accepted only to be reverted later on because on 64-bit host, there was not enough room in skb->cb[] to insert 4 bytes, so it got reverted. So yes, I think we need to allocate a custom SKB extension if we want to convey the tag, unless we somehow manage to put it in the linear portion of the SKB to avoid using any control buffer or extension.
On Thu, May 19, 2022 at 10:11:13AM -0700, Florian Fainelli wrote: > unless we somehow manage to put it in the linear portion of > the SKB to avoid using any control buffer or extension. But how? Essentially the DSA master has to look at a packet and determine whether it came from DSA based on something which non-DSA code could not have done. In fact, I'm looking at the calls to skb_reset_mac_{header,len} from net/core/skbuff.c, specifically at VLAN and MPLS, and I believe (but haven't tested) that pushing such headers would also alter skb->mac_len to some value != ETH_HLEN. So simply having the DSA master check whether DSA was there by checking whether skb->mac_len is ETH_HLEN + DSA tag len could easily confuse DSA with some other protocol of same header size.
diff --git a/include/linux/dsa/oob.h b/include/linux/dsa/oob.h new file mode 100644 index 000000000000..dbb4a6fb1ce4 --- /dev/null +++ b/include/linux/dsa/oob.h @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0-only + * Copyright (C) 2022 Maxime Chevallier <maxime.chevallier@bootlin.com> + */ + +#ifndef _NET_DSA_OOB_H +#define _NET_DSA_OOB_H + +#include <linux/skbuff.h> + +struct dsa_oob_tag_info { + u16 proto; + u16 dp; +}; + +int dsa_oob_tag_push(struct sk_buff *skb, struct dsa_oob_tag_info *ti); +int dsa_oob_tag_pop(struct sk_buff *skb, struct dsa_oob_tag_info *ti); +#endif diff --git a/include/net/dsa.h b/include/net/dsa.h index 14e10cda7267..9951df858912 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -53,6 +53,7 @@ struct phylink_link_state; #define DSA_TAG_PROTO_SJA1110_VALUE 23 #define DSA_TAG_PROTO_RTL8_4_VALUE 24 #define DSA_TAG_PROTO_RTL8_4T_VALUE 25 +#define DSA_TAG_PROTO_OOB_VALUE 26 enum dsa_tag_protocol { DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE, @@ -81,6 +82,7 @@ enum dsa_tag_protocol { 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, + DSA_TAG_PROTO_OOB = DSA_TAG_PROTO_OOB_VALUE, }; struct dsa_switch; diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig index 8cb87b5067ee..b7aa4d8552b2 100644 --- a/net/dsa/Kconfig +++ b/net/dsa/Kconfig @@ -57,6 +57,13 @@ config NET_DSA_TAG_HELLCREEK Say Y or M if you want to enable support for tagging frames for the Hirschmann Hellcreek TSN switches. +config NET_DSA_TAG_OOB + tristate "Tag driver for Out-of-band tagging drivers" + help + Say Y or M if you want to enable support for tagging out-of-band. In + that case, the MAC driver becomes responsible for sending the tag to + the switch, outside the inband data. + config NET_DSA_TAG_GSWIP tristate "Tag driver for Lantiq / Intel GSWIP switches" help diff --git a/net/dsa/Makefile b/net/dsa/Makefile index 9f75820e7c98..b156e20f9c0a 100644 --- a/net/dsa/Makefile +++ b/net/dsa/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_NET_DSA_TAG_BRCM_COMMON) += tag_brcm.o obj-$(CONFIG_NET_DSA_TAG_DSA_COMMON) += tag_dsa.o obj-$(CONFIG_NET_DSA_TAG_GSWIP) += tag_gswip.o obj-$(CONFIG_NET_DSA_TAG_HELLCREEK) += tag_hellcreek.o +obj-$(CONFIG_NET_DSA_TAG_OOB) += tag_oob.o obj-$(CONFIG_NET_DSA_TAG_KSZ) += tag_ksz.o obj-$(CONFIG_NET_DSA_TAG_LAN9303) += tag_lan9303.o obj-$(CONFIG_NET_DSA_TAG_MTK) += tag_mtk.o diff --git a/net/dsa/tag_oob.c b/net/dsa/tag_oob.c new file mode 100644 index 000000000000..45ee3df5a7f9 --- /dev/null +++ b/net/dsa/tag_oob.c @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: GPL-2.0-only + +/* Copyright (c) 2022, Maxime Chevallier <maxime.chevallier@bootlin.com> */ + +#include <linux/bitfield.h> +#include <linux/dsa/oob.h> + +#include "dsa_priv.h" + +#define DSA_OOB_TAG_LEN 4 + +int dsa_oob_tag_push(struct sk_buff *skb, struct dsa_oob_tag_info *ti) +{ + struct dsa_oob_tag_info *tag_info; + + tag_info = (struct dsa_oob_tag_info *)skb->head; + + tag_info->proto = ti->proto; + tag_info->dp = ti->dp; + + return 0; +} +EXPORT_SYMBOL(dsa_oob_tag_push); + +int dsa_oob_tag_pop(struct sk_buff *skb, struct dsa_oob_tag_info *ti) +{ + struct dsa_oob_tag_info *tag_info; + + tag_info = (struct dsa_oob_tag_info *)skb->head; + + if (tag_info->proto != DSA_TAG_PROTO_OOB) + return -EINVAL; + + ti->proto = tag_info->proto; + ti->dp = tag_info->dp; + + return 0; +} +EXPORT_SYMBOL(dsa_oob_tag_pop); + +static struct sk_buff *oob_tag_xmit(struct sk_buff *skb, + struct net_device *dev) +{ + struct dsa_port *dp = dsa_slave_to_port(dev); + struct dsa_oob_tag_info tag_info; + + tag_info.dp = dp->index; + tag_info.proto = DSA_TAG_PROTO_OOB; + + if (dsa_oob_tag_push(skb, &tag_info)) + return NULL; + + return skb; +} + +static struct sk_buff *oob_tag_rcv(struct sk_buff *skb, + struct net_device *dev) +{ + struct dsa_oob_tag_info tag_info; + + if (dsa_oob_tag_pop(skb, &tag_info)) + return NULL; + + skb->dev = dsa_master_find_slave(dev, 0, tag_info.dp); + if (!skb->dev) + return NULL; + + return skb; +} + +const struct dsa_device_ops oob_tag_dsa_ops = { + .name = "oob", + .proto = DSA_TAG_PROTO_OOB, + .xmit = oob_tag_xmit, + .rcv = oob_tag_rcv, + .needed_headroom = DSA_OOB_TAG_LEN, +}; + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("DSA tag driver for out-of-band tagging"); +MODULE_AUTHOR("Maxime Chevallier <maxime.chevallier@bootlin.com>"); +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_OOB); + +module_dsa_tag_driver(oob_tag_dsa_ops);
This tagging protocol is designed for the situation where the link between the MAC and the Switch is designed such that the Destination Port, which is usually embedded in some part of the Ethernet Header, is sent out-of-band, and isn't present at all in the Ethernet frame. This can happen when the MAC and Switch are tightly integrated on an SoC, as is the case with the Qualcomm IPQ4019 for example, where the DSA tag is inserted directly into the DMA descriptors. In that case, the MAC driver is responsible for sending the tag to the switch using the out-of-band medium. To do so, the MAC driver needs to have the information of the destination port for that skb. This out-of-band tagging protocol is using the very beggining of the skb headroom to store the tag. The drawback of this approch is that the headroom isn't initialized upon allocating it, therefore we have a chance that the garbage data that lies there at allocation time actually ressembles a valid oob tag. This is only problematic if we are sending/receiving traffic on the master port, which isn't a valid DSA use-case from the beggining. When dealing from traffic to/from a slave port, then the oob tag will be initialized properly by the tagger or the mac driver through the use of the dsa_oob_tag_push() call. Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- V1->V2: - Reworked the tagging method, putting the tag at skb->head instead of putting it into skb->shinfo, as per Andrew, Florian and Vlad's reviews include/linux/dsa/oob.h | 17 +++++++++ include/net/dsa.h | 2 + net/dsa/Kconfig | 7 ++++ net/dsa/Makefile | 1 + net/dsa/tag_oob.c | 84 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 111 insertions(+) create mode 100644 include/linux/dsa/oob.h create mode 100644 net/dsa/tag_oob.c