diff mbox series

[net-next,02/12] net: dsa: add Renesas RZ/N1 switch tag driver

Message ID 20220414122250.158113-3-clement.leger@bootlin.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series add support for Renesas RZ/N1 ethernet subsystem devices | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 0 this patch: 6
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 0 this patch: 6
netdev/checkpatch warning WARNING: 'ment' may be misspelled - perhaps 'meant'? WARNING: Prefer "GPL" over "GPL v2" - see commit bf7fbeeae6db ("module: Cure the MODULE_LICENSE "GPL" vs. "GPL v2" bogosity") WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 82 exceeds 80 columns WARNING: please write a help paragraph that fully describes the config symbol
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Clément Léger April 14, 2022, 12:22 p.m. UTC
The switch that is present on the Renesas RZ/N1 SoC uses a specific
VLAN value followed by 6 bytes which contains forwarding configuration.

Signed-off-by: Clément Léger <clement.leger@bootlin.com>
---
 net/dsa/Kconfig          |   8 +++
 net/dsa/Makefile         |   1 +
 net/dsa/tag_rzn1_a5psw.c | 112 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 121 insertions(+)
 create mode 100644 net/dsa/tag_rzn1_a5psw.c

Comments

Vladimir Oltean April 14, 2022, 2:22 p.m. UTC | #1
On Thu, Apr 14, 2022 at 02:22:40PM +0200, Clément Léger wrote:
> The switch that is present on the Renesas RZ/N1 SoC uses a specific
> VLAN value followed by 6 bytes which contains forwarding configuration.
> 
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> ---
>  net/dsa/Kconfig          |   8 +++
>  net/dsa/Makefile         |   1 +
>  net/dsa/tag_rzn1_a5psw.c | 112 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 121 insertions(+)
>  create mode 100644 net/dsa/tag_rzn1_a5psw.c
> 
> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
> index 8cb87b5067ee..e5b17108fa22 100644
> --- a/net/dsa/Kconfig
> +++ b/net/dsa/Kconfig
> @@ -132,6 +132,13 @@ config NET_DSA_TAG_RTL8_4
>  	  Say Y or M if you want to enable support for tagging frames for Realtek
>  	  switches with 8 byte protocol 4 tags, such as the Realtek RTL8365MB-VC.
>  
> +config NET_DSA_TAG_RZN1_A5PSW
> +	tristate "Tag driver for Renesas RZ/N1 A5PSW switch"
> +	help
> +	  Say Y or M if you want to enable support for tagging frames for
> +	  Renesas RZ/N1 embedded switch that uses a 8 byte tag located after
> +	  destination MAC address.
> +
>  config NET_DSA_TAG_LAN9303
>  	tristate "Tag driver for SMSC/Microchip LAN9303 family of switches"
>  	help
> @@ -159,4 +166,5 @@ config NET_DSA_TAG_XRS700X
>  	  Say Y or M if you want to enable support for tagging frames for
>  	  Arrow SpeedChips XRS700x switches that use a single byte tag trailer.
>  
> +

Stray change.

>  endif
> diff --git a/net/dsa/Makefile b/net/dsa/Makefile
> index 9f75820e7c98..af28c24ead18 100644
> --- a/net/dsa/Makefile
> +++ b/net/dsa/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_NET_DSA_TAG_OCELOT_8021Q) += tag_ocelot_8021q.o
>  obj-$(CONFIG_NET_DSA_TAG_QCA) += tag_qca.o
>  obj-$(CONFIG_NET_DSA_TAG_RTL4_A) += tag_rtl4_a.o
>  obj-$(CONFIG_NET_DSA_TAG_RTL8_4) += tag_rtl8_4.o
> +obj-$(CONFIG_NET_DSA_TAG_RZN1_A5PSW) += tag_rzn1_a5psw.o
>  obj-$(CONFIG_NET_DSA_TAG_SJA1105) += tag_sja1105.o
>  obj-$(CONFIG_NET_DSA_TAG_TRAILER) += tag_trailer.o
>  obj-$(CONFIG_NET_DSA_TAG_XRS700X) += tag_xrs700x.o
> diff --git a/net/dsa/tag_rzn1_a5psw.c b/net/dsa/tag_rzn1_a5psw.c
> new file mode 100644
> index 000000000000..7818c1c0fca2
> --- /dev/null
> +++ b/net/dsa/tag_rzn1_a5psw.c
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2022 Schneider Electric
> + *
> + * Clément Léger <clement.leger@bootlin.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/etherdevice.h>
> +#include <net/dsa.h>
> +
> +#include "dsa_priv.h"
> +
> +/* To define the outgoing port and to discover the incoming port a TAG is
> + * inserted after Src MAC :
> + *
> + *       Dest MAC       Src MAC           TAG         Type
> + * ...| 1 2 3 4 5 6 | 1 2 3 4 5 6 | 1 2 3 4 5 6 7 8 | 1 2 |...
> + *                                |<--------------->|
> + *
> + * See struct a5psw_tag for layout
> + */
> +
> +#define A5PSW_TAG_VALUE			0xE001
> +#define A5PSW_TAG_LEN			8
> +#define A5PSW_CTRL_DATA_FORCE_FORWARD	BIT(0)
> +/* This is both used for xmit tag and rcv tagging */
> +#define A5PSW_CTRL_DATA_PORT		GENMASK(3, 0)
> +
> +struct a5psw_tag {

This needs to be packed.

> +	u16 ctrl_tag;
> +	u16 ctrl_data;

These need to be __be16.

> +	u32 ctrl_data2;

This needs to be __be32.

> +};
> +
> +static struct sk_buff *a5psw_tag_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct a5psw_tag *ptag, tag = {0};
> +	struct dsa_port *dp = dsa_slave_to_port(dev);

Please keep variable declarations sorted in decreasing order of line
length (applies throughout the patch series, I won't repeat this comment).

> +	u32 data2_val;
> +
> +	/* The Ethernet switch we are interfaced with needs packets to be at
> +	 * least 64 bytes (including FCS) otherwise they will be discarded when
> +	 * they enter the switch port logic. When tagging is enabled, we need
> +	 * to make sure that packets are at least 68 bytes (including FCS and
> +	 * tag).
> +	 */
> +	if (__skb_put_padto(skb, ETH_ZLEN + sizeof(tag), false))
> +		return NULL;
> +
> +	/* provide 'A5PSW_TAG_LEN' bytes additional space */
> +	skb_push(skb, A5PSW_TAG_LEN);
> +
> +	/* make room between MACs and Ether-Type to insert tag */
> +	dsa_alloc_etype_header(skb, A5PSW_TAG_LEN);
> +
> +	ptag = dsa_etype_header_pos_tx(skb);
> +
> +	data2_val = FIELD_PREP(A5PSW_CTRL_DATA_PORT, BIT(dp->index));
> +	tag.ctrl_tag = htons(A5PSW_TAG_VALUE);
> +	tag.ctrl_data = htons(A5PSW_CTRL_DATA_FORCE_FORWARD);
> +	tag.ctrl_data2 = htonl(data2_val);
> +
> +	memcpy(ptag, &tag, sizeof(struct a5psw_tag));

sizeof(tag), to be consistent with the other use of sizeof() above?
Although, hmm, I think you could get away with editing "ptag" in place.

> +
> +	return skb;
> +}
> +
> +static struct sk_buff *a5psw_tag_rcv(struct sk_buff *skb,
> +				     struct net_device *dev)
> +{
> +	struct a5psw_tag *tag;
> +	int port;
> +
> +	if (unlikely(!pskb_may_pull(skb, A5PSW_TAG_LEN))) {
> +		dev_warn_ratelimited(&dev->dev,
> +				     "Dropping packet, cannot pull\n");
> +		return NULL;
> +	}
> +
> +	tag = dsa_etype_header_pos_rx(skb);
> +
> +	if (tag->ctrl_tag != htons(A5PSW_TAG_VALUE)) {
> +		dev_warn_ratelimited(&dev->dev, "Dropping packet due to invalid TAG marker\n");
> +		return NULL;
> +	}
> +
> +	port = FIELD_GET(A5PSW_CTRL_DATA_PORT, ntohs(tag->ctrl_data));
> +
> +	skb->dev = dsa_master_find_slave(dev, 0, port);
> +	if (!skb->dev)
> +		return NULL;
> +
> +	skb_pull_rcsum(skb, A5PSW_TAG_LEN);
> +	dsa_strip_etype_header(skb, A5PSW_TAG_LEN);
> +
> +	dsa_default_offload_fwd_mark(skb);
> +
> +	return skb;
> +}
> +
> +static const struct dsa_device_ops a5psw_netdev_ops = {
> +	.name	= "a5psw",
> +	.proto	= DSA_TAG_PROTO_RZN1_A5PSW,
> +	.xmit	= a5psw_tag_xmit,
> +	.rcv	= a5psw_tag_rcv,
> +	.needed_headroom = A5PSW_TAG_LEN,
> +};
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_A5PSW);
> +module_dsa_tag_driver(a5psw_netdev_ops);
> -- 
> 2.34.1
>
Clément Léger April 14, 2022, 2:35 p.m. UTC | #2
Le Thu, 14 Apr 2022 17:22:42 +0300,
Vladimir Oltean <olteanv@gmail.com> a écrit :

> > +
> > +/* To define the outgoing port and to discover the incoming port a TAG is
> > + * inserted after Src MAC :
> > + *
> > + *       Dest MAC       Src MAC           TAG         Type
> > + * ...| 1 2 3 4 5 6 | 1 2 3 4 5 6 | 1 2 3 4 5 6 7 8 | 1 2 |...
> > + *                                |<--------------->|
> > + *
> > + * See struct a5psw_tag for layout
> > + */
> > +
> > +#define A5PSW_TAG_VALUE			0xE001
> > +#define A5PSW_TAG_LEN			8
> > +#define A5PSW_CTRL_DATA_FORCE_FORWARD	BIT(0)
> > +/* This is both used for xmit tag and rcv tagging */
> > +#define A5PSW_CTRL_DATA_PORT		GENMASK(3, 0)
> > +
> > +struct a5psw_tag {  
> 
> This needs to be packed.
> 
> > +	u16 ctrl_tag;
> > +	u16 ctrl_data;  
> 
> These need to be __be16.
> 
> > +	u32 ctrl_data2;  
> 
> This needs to be __be32.

Acked.

> 
> > +};
> > +
> > +static struct sk_buff *a5psw_tag_xmit(struct sk_buff *skb, struct net_device *dev)
> > +{
> > +	struct a5psw_tag *ptag, tag = {0};
> > +	struct dsa_port *dp = dsa_slave_to_port(dev);  
> 
> Please keep variable declarations sorted in decreasing order of line
> length (applies throughout the patch series, I won't repeat this comment).

Acked, both PCS and DSA driver are ok with that rule. Missed that one
though.
> 
> > +	u32 data2_val;
> > +
> > +	/* The Ethernet switch we are interfaced with needs packets to be at
> > +	 * least 64 bytes (including FCS) otherwise they will be discarded when
> > +	 * they enter the switch port logic. When tagging is enabled, we need
> > +	 * to make sure that packets are at least 68 bytes (including FCS and
> > +	 * tag).
> > +	 */
> > +	if (__skb_put_padto(skb, ETH_ZLEN + sizeof(tag), false))
> > +		return NULL;
> > +
> > +	/* provide 'A5PSW_TAG_LEN' bytes additional space */
> > +	skb_push(skb, A5PSW_TAG_LEN);
> > +
> > +	/* make room between MACs and Ether-Type to insert tag */
> > +	dsa_alloc_etype_header(skb, A5PSW_TAG_LEN);
> > +
> > +	ptag = dsa_etype_header_pos_tx(skb);
> > +
> > +	data2_val = FIELD_PREP(A5PSW_CTRL_DATA_PORT, BIT(dp->index));
> > +	tag.ctrl_tag = htons(A5PSW_TAG_VALUE);
> > +	tag.ctrl_data = htons(A5PSW_CTRL_DATA_FORCE_FORWARD);
> > +	tag.ctrl_data2 = htonl(data2_val);
> > +
> > +	memcpy(ptag, &tag, sizeof(struct a5psw_tag));  
> 
> sizeof(tag), to be consistent with the other use of sizeof() above?
> Although, hmm, I think you could get away with editing "ptag" in place.

I was not sure of the alignment guarantee I would have here. If the
VLAN header is guaranteed to be aligned on 2 bytes, then I guess it's
ok to do that in-place.
Vladimir Oltean April 14, 2022, 3:11 p.m. UTC | #3
On Thu, Apr 14, 2022 at 04:35:46PM +0200, Clément Léger wrote:
> > Please keep variable declarations sorted in decreasing order of line
> > length (applies throughout the patch series, I won't repeat this comment).
> 
> Acked, both PCS and DSA driver are ok with that rule. Missed that one
> though.

Are you sure? Because a5psw_port_stp_state_set() says otherwise.

> > sizeof(tag), to be consistent with the other use of sizeof() above?
> > Although, hmm, I think you could get away with editing "ptag" in place.
> 
> I was not sure of the alignment guarantee I would have here. If the
> VLAN header is guaranteed to be aligned on 2 bytes, then I guess it's
> ok to do that in-place.

If I look at Documentation/core-api/unaligned-memory-access.rst

| Alignment vs. Networking
| ========================
| 
| On architectures that require aligned loads, networking requires that the IP
| header is aligned on a four-byte boundary to optimise the IP stack. For
| regular ethernet hardware, the constant NET_IP_ALIGN is used. On most
| architectures this constant has the value 2 because the normal ethernet
| header is 14 bytes long, so in order to get proper alignment one needs to
| DMA to an address which can be expressed as 4*n + 2. One notable exception
| here is powerpc which defines NET_IP_ALIGN to 0 because DMA to unaligned
| addresses can be very expensive and dwarf the cost of unaligned loads.

Your struct a5psw_tag *ptag starts at 10 bytes (8 for tag, 2 for Ethertype)
before the IP header, so I'm thinking it is aligned at a 2 byte boundary
as well. A VLAN header between the DSA header and the IP header should
also not affect that alignment, since its length is 4 bytes.

If "ctrl_tag" is aligned at a 4 byte boundary - 10, it means "ctrl_data"
is aligned at a 4 byte boundary - 8, so also a 4 byte boundary.

But "ctrl_data2" is aligned at a 4 byte boundary + 2, so you might want
to break up the u32 access into 2 u16 accesses, just to be on the safe
side?
Clément Léger April 14, 2022, 4:18 p.m. UTC | #4
Le Thu, 14 Apr 2022 18:11:46 +0300,
Vladimir Oltean <olteanv@gmail.com> a écrit :

> On Thu, Apr 14, 2022 at 04:35:46PM +0200, Clément Léger wrote:
> > > Please keep variable declarations sorted in decreasing order of line
> > > length (applies throughout the patch series, I won't repeat this comment).  
> > 
> > Acked, both PCS and DSA driver are ok with that rule. Missed that one
> > though.  
> 
> Are you sure? Because a5psw_port_stp_state_set() says otherwise.

Weeeeell, ok let's say I missed these two. Would be useful to have such
checks in checkpatch.pl.

> 
> > > sizeof(tag), to be consistent with the other use of sizeof() above?
> > > Although, hmm, I think you could get away with editing "ptag" in place.  
> > 
> > I was not sure of the alignment guarantee I would have here. If the
> > VLAN header is guaranteed to be aligned on 2 bytes, then I guess it's
> > ok to do that in-place.  
> 
> If I look at Documentation/core-api/unaligned-memory-access.rst
> 
> | Alignment vs. Networking
> | ========================
> | 
> | On architectures that require aligned loads, networking requires that the IP
> | header is aligned on a four-byte boundary to optimise the IP stack. For
> | regular ethernet hardware, the constant NET_IP_ALIGN is used. On most
> | architectures this constant has the value 2 because the normal ethernet
> | header is 14 bytes long, so in order to get proper alignment one needs to
> | DMA to an address which can be expressed as 4*n + 2. One notable exception
> | here is powerpc which defines NET_IP_ALIGN to 0 because DMA to unaligned
> | addresses can be very expensive and dwarf the cost of unaligned loads.
> 
> Your struct a5psw_tag *ptag starts at 10 bytes (8 for tag, 2 for Ethertype)
> before the IP header, so I'm thinking it is aligned at a 2 byte boundary
> as well. A VLAN header between the DSA header and the IP header should
> also not affect that alignment, since its length is 4 bytes.
> 
> If "ctrl_tag" is aligned at a 4 byte boundary - 10, it means "ctrl_data"
> is aligned at a 4 byte boundary - 8, so also a 4 byte boundary.
> 
> But "ctrl_data2" is aligned at a 4 byte boundary + 2, so you might want
> to break up the u32 access into 2 u16 accesses, just to be on the safe
> side?

Thanks for finding these, looks like a good compromise, let's go that
way then.
Russell King (Oracle) April 14, 2022, 4:23 p.m. UTC | #5
On Thu, Apr 14, 2022 at 06:18:15PM +0200, Clément Léger wrote:
> Le Thu, 14 Apr 2022 18:11:46 +0300,
> Vladimir Oltean <olteanv@gmail.com> a écrit :
> 
> > On Thu, Apr 14, 2022 at 04:35:46PM +0200, Clément Léger wrote:
> > > > Please keep variable declarations sorted in decreasing order of line
> > > > length (applies throughout the patch series, I won't repeat this comment).  
> > > 
> > > Acked, both PCS and DSA driver are ok with that rule. Missed that one
> > > though.  
> > 
> > Are you sure? Because a5psw_port_stp_state_set() says otherwise.
> 
> Weeeeell, ok let's say I missed these two. Would be useful to have such
> checks in checkpatch.pl.

Note that it's a local networking coding-style issue, rather than being
kernel-wide.
Andrew Lunn April 14, 2022, 10:50 p.m. UTC | #6
On Thu, Apr 14, 2022 at 02:22:40PM +0200, Clément Léger wrote:
> The switch that is present on the Renesas RZ/N1 SoC uses a specific
> VLAN value followed by 6 bytes which contains forwarding configuration.
> 
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> ---
>  net/dsa/Kconfig          |   8 +++
>  net/dsa/Makefile         |   1 +
>  net/dsa/tag_rzn1_a5psw.c | 112 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 121 insertions(+)
>  create mode 100644 net/dsa/tag_rzn1_a5psw.c
> 
> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
> index 8cb87b5067ee..e5b17108fa22 100644
> --- a/net/dsa/Kconfig
> +++ b/net/dsa/Kconfig
> @@ -132,6 +132,13 @@ config NET_DSA_TAG_RTL8_4
>  	  Say Y or M if you want to enable support for tagging frames for Realtek
>  	  switches with 8 byte protocol 4 tags, such as the Realtek RTL8365MB-VC.
>  
> +config NET_DSA_TAG_RZN1_A5PSW
> +	tristate "Tag driver for Renesas RZ/N1 A5PSW switch"
> +	help
> +	  Say Y or M if you want to enable support for tagging frames for
> +	  Renesas RZ/N1 embedded switch that uses a 8 byte tag located after
> +	  destination MAC address.
> +
>  config NET_DSA_TAG_LAN9303
>  	tristate "Tag driver for SMSC/Microchip LAN9303 family of switches"
>  	help
> @@ -159,4 +166,5 @@ config NET_DSA_TAG_XRS700X
>  	  Say Y or M if you want to enable support for tagging frames for
>  	  Arrow SpeedChips XRS700x switches that use a single byte tag trailer.
>  
> +
>  endif
> diff --git a/net/dsa/Makefile b/net/dsa/Makefile
> index 9f75820e7c98..af28c24ead18 100644
> --- a/net/dsa/Makefile
> +++ b/net/dsa/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_NET_DSA_TAG_OCELOT_8021Q) += tag_ocelot_8021q.o
>  obj-$(CONFIG_NET_DSA_TAG_QCA) += tag_qca.o
>  obj-$(CONFIG_NET_DSA_TAG_RTL4_A) += tag_rtl4_a.o
>  obj-$(CONFIG_NET_DSA_TAG_RTL8_4) += tag_rtl8_4.o
> +obj-$(CONFIG_NET_DSA_TAG_RZN1_A5PSW) += tag_rzn1_a5psw.o
>  obj-$(CONFIG_NET_DSA_TAG_SJA1105) += tag_sja1105.o
>  obj-$(CONFIG_NET_DSA_TAG_TRAILER) += tag_trailer.o
>  obj-$(CONFIG_NET_DSA_TAG_XRS700X) += tag_xrs700x.o
> diff --git a/net/dsa/tag_rzn1_a5psw.c b/net/dsa/tag_rzn1_a5psw.c
> new file mode 100644
> index 000000000000..7818c1c0fca2
> --- /dev/null
> +++ b/net/dsa/tag_rzn1_a5psw.c
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2022 Schneider Electric
> + *
> + * Clément Léger <clement.leger@bootlin.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/etherdevice.h>
> +#include <net/dsa.h>
> +
> +#include "dsa_priv.h"
> +
> +/* To define the outgoing port and to discover the incoming port a TAG is
> + * inserted after Src MAC :
> + *
> + *       Dest MAC       Src MAC           TAG         Type
> + * ...| 1 2 3 4 5 6 | 1 2 3 4 5 6 | 1 2 3 4 5 6 7 8 | 1 2 |...
> + *                                |<--------------->|
> + *
> + * See struct a5psw_tag for layout
> + */
> +
> +#define A5PSW_TAG_VALUE			0xE001
> +#define A5PSW_TAG_LEN			8
> +#define A5PSW_CTRL_DATA_FORCE_FORWARD	BIT(0)
> +/* This is both used for xmit tag and rcv tagging */
> +#define A5PSW_CTRL_DATA_PORT		GENMASK(3, 0)
> +
> +struct a5psw_tag {
> +	u16 ctrl_tag;
> +	u16 ctrl_data;
> +	u32 ctrl_data2;
> +};
> +
> +static struct sk_buff *a5psw_tag_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct a5psw_tag *ptag, tag = {0};
> +	struct dsa_port *dp = dsa_slave_to_port(dev);
> +	u32 data2_val;

It might be worth adding a BUILD_BUG_ON(sizeof(tag) != A5PSW_TAG_LEN);

That does not cost anything at runtime, and avoids hard to find bugs
when the compiler does not do what you expect in terms of packing.

     Andrew
Clément Léger April 15, 2022, 7:23 a.m. UTC | #7
Le Thu, 14 Apr 2022 17:23:46 +0100,
"Russell King (Oracle)" <linux@armlinux.org.uk> a écrit :

> On Thu, Apr 14, 2022 at 06:18:15PM +0200, Clément Léger wrote:
> > Le Thu, 14 Apr 2022 18:11:46 +0300,
> > Vladimir Oltean <olteanv@gmail.com> a écrit :
> >   
> > > On Thu, Apr 14, 2022 at 04:35:46PM +0200, Clément Léger wrote:  
> > > > > Please keep variable declarations sorted in decreasing order of line
> > > > > length (applies throughout the patch series, I won't repeat this comment).    
> > > > 
> > > > Acked, both PCS and DSA driver are ok with that rule. Missed that one
> > > > though.    
> > > 
> > > Are you sure? Because a5psw_port_stp_state_set() says otherwise.  
> > 
> > Weeeeell, ok let's say I missed these two. Would be useful to have such
> > checks in checkpatch.pl.  
> 
> Note that it's a local networking coding-style issue, rather than being
> kernel-wide.
> 

Hi Russell, Yes I was aware of that but if I remember correctly, there
are some netowrking checks like multi line comments without an empty
first line in checkpatch. Anyway, I'll make sure to check that mroe
carefully next time.
diff mbox series

Patch

diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 8cb87b5067ee..e5b17108fa22 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -132,6 +132,13 @@  config NET_DSA_TAG_RTL8_4
 	  Say Y or M if you want to enable support for tagging frames for Realtek
 	  switches with 8 byte protocol 4 tags, such as the Realtek RTL8365MB-VC.
 
+config NET_DSA_TAG_RZN1_A5PSW
+	tristate "Tag driver for Renesas RZ/N1 A5PSW switch"
+	help
+	  Say Y or M if you want to enable support for tagging frames for
+	  Renesas RZ/N1 embedded switch that uses a 8 byte tag located after
+	  destination MAC address.
+
 config NET_DSA_TAG_LAN9303
 	tristate "Tag driver for SMSC/Microchip LAN9303 family of switches"
 	help
@@ -159,4 +166,5 @@  config NET_DSA_TAG_XRS700X
 	  Say Y or M if you want to enable support for tagging frames for
 	  Arrow SpeedChips XRS700x switches that use a single byte tag trailer.
 
+
 endif
diff --git a/net/dsa/Makefile b/net/dsa/Makefile
index 9f75820e7c98..af28c24ead18 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -17,6 +17,7 @@  obj-$(CONFIG_NET_DSA_TAG_OCELOT_8021Q) += tag_ocelot_8021q.o
 obj-$(CONFIG_NET_DSA_TAG_QCA) += tag_qca.o
 obj-$(CONFIG_NET_DSA_TAG_RTL4_A) += tag_rtl4_a.o
 obj-$(CONFIG_NET_DSA_TAG_RTL8_4) += tag_rtl8_4.o
+obj-$(CONFIG_NET_DSA_TAG_RZN1_A5PSW) += tag_rzn1_a5psw.o
 obj-$(CONFIG_NET_DSA_TAG_SJA1105) += tag_sja1105.o
 obj-$(CONFIG_NET_DSA_TAG_TRAILER) += tag_trailer.o
 obj-$(CONFIG_NET_DSA_TAG_XRS700X) += tag_xrs700x.o
diff --git a/net/dsa/tag_rzn1_a5psw.c b/net/dsa/tag_rzn1_a5psw.c
new file mode 100644
index 000000000000..7818c1c0fca2
--- /dev/null
+++ b/net/dsa/tag_rzn1_a5psw.c
@@ -0,0 +1,112 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 Schneider Electric
+ *
+ * Clément Léger <clement.leger@bootlin.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/etherdevice.h>
+#include <net/dsa.h>
+
+#include "dsa_priv.h"
+
+/* To define the outgoing port and to discover the incoming port a TAG is
+ * inserted after Src MAC :
+ *
+ *       Dest MAC       Src MAC           TAG         Type
+ * ...| 1 2 3 4 5 6 | 1 2 3 4 5 6 | 1 2 3 4 5 6 7 8 | 1 2 |...
+ *                                |<--------------->|
+ *
+ * See struct a5psw_tag for layout
+ */
+
+#define A5PSW_TAG_VALUE			0xE001
+#define A5PSW_TAG_LEN			8
+#define A5PSW_CTRL_DATA_FORCE_FORWARD	BIT(0)
+/* This is both used for xmit tag and rcv tagging */
+#define A5PSW_CTRL_DATA_PORT		GENMASK(3, 0)
+
+struct a5psw_tag {
+	u16 ctrl_tag;
+	u16 ctrl_data;
+	u32 ctrl_data2;
+};
+
+static struct sk_buff *a5psw_tag_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct a5psw_tag *ptag, tag = {0};
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	u32 data2_val;
+
+	/* The Ethernet switch we are interfaced with needs packets to be at
+	 * least 64 bytes (including FCS) otherwise they will be discarded when
+	 * they enter the switch port logic. When tagging is enabled, we need
+	 * to make sure that packets are at least 68 bytes (including FCS and
+	 * tag).
+	 */
+	if (__skb_put_padto(skb, ETH_ZLEN + sizeof(tag), false))
+		return NULL;
+
+	/* provide 'A5PSW_TAG_LEN' bytes additional space */
+	skb_push(skb, A5PSW_TAG_LEN);
+
+	/* make room between MACs and Ether-Type to insert tag */
+	dsa_alloc_etype_header(skb, A5PSW_TAG_LEN);
+
+	ptag = dsa_etype_header_pos_tx(skb);
+
+	data2_val = FIELD_PREP(A5PSW_CTRL_DATA_PORT, BIT(dp->index));
+	tag.ctrl_tag = htons(A5PSW_TAG_VALUE);
+	tag.ctrl_data = htons(A5PSW_CTRL_DATA_FORCE_FORWARD);
+	tag.ctrl_data2 = htonl(data2_val);
+
+	memcpy(ptag, &tag, sizeof(struct a5psw_tag));
+
+	return skb;
+}
+
+static struct sk_buff *a5psw_tag_rcv(struct sk_buff *skb,
+				     struct net_device *dev)
+{
+	struct a5psw_tag *tag;
+	int port;
+
+	if (unlikely(!pskb_may_pull(skb, A5PSW_TAG_LEN))) {
+		dev_warn_ratelimited(&dev->dev,
+				     "Dropping packet, cannot pull\n");
+		return NULL;
+	}
+
+	tag = dsa_etype_header_pos_rx(skb);
+
+	if (tag->ctrl_tag != htons(A5PSW_TAG_VALUE)) {
+		dev_warn_ratelimited(&dev->dev, "Dropping packet due to invalid TAG marker\n");
+		return NULL;
+	}
+
+	port = FIELD_GET(A5PSW_CTRL_DATA_PORT, ntohs(tag->ctrl_data));
+
+	skb->dev = dsa_master_find_slave(dev, 0, port);
+	if (!skb->dev)
+		return NULL;
+
+	skb_pull_rcsum(skb, A5PSW_TAG_LEN);
+	dsa_strip_etype_header(skb, A5PSW_TAG_LEN);
+
+	dsa_default_offload_fwd_mark(skb);
+
+	return skb;
+}
+
+static const struct dsa_device_ops a5psw_netdev_ops = {
+	.name	= "a5psw",
+	.proto	= DSA_TAG_PROTO_RZN1_A5PSW,
+	.xmit	= a5psw_tag_xmit,
+	.rcv	= a5psw_tag_rcv,
+	.needed_headroom = A5PSW_TAG_LEN,
+};
+
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_A5PSW);
+module_dsa_tag_driver(a5psw_netdev_ops);