netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 1/5] dsa: add support for Microchip KSZ tail tagging
@ 2017-05-05 23:17 Woojung.Huh
  2017-05-05 23:34 ` Florian Fainelli
  2017-05-06  3:38 ` Andrew Lunn
  0 siblings, 2 replies; 4+ messages in thread
From: Woojung.Huh @ 2017-05-05 23:17 UTC (permalink / raw)
  To: andrew, f.fainelli, vivien.didelot; +Cc: netdev, davem, UNGLinuxDriver

From: Woojung Huh <Woojung.Huh@microchip.com>

Adding support for the Microchip KSZ switch family tail tagging.

Signed-off-by: Woojung Huh <Woojung.Huh@microchip.com>
---
 include/net/dsa.h  |  1 +
 net/dsa/Kconfig    |  3 ++
 net/dsa/Makefile   |  1 +
 net/dsa/dsa.c      |  3 ++
 net/dsa/dsa_priv.h |  3 ++
 net/dsa/tag_ksz.c  | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 109 insertions(+)
 create mode 100644 net/dsa/tag_ksz.c

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 8e24677..c92204a 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -34,6 +34,7 @@ enum dsa_tag_protocol {
 	DSA_TAG_PROTO_QCA,
 	DSA_TAG_PROTO_MTK,
 	DSA_TAG_PROTO_LAN9303,
+	DSA_TAG_PROTO_KSZ,
 	DSA_TAG_LAST,		/* MUST BE LAST */
 };
 
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 81a0868..ce31428 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -37,4 +37,7 @@ config NET_DSA_TAG_MTK
 config NET_DSA_TAG_LAN9303
 	bool
 
+config NET_DSA_TAG_KSZ
+	bool
+
 endif
diff --git a/net/dsa/Makefile b/net/dsa/Makefile
index 0b747d7..8becb26 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -10,3 +10,4 @@ dsa_core-$(CONFIG_NET_DSA_TAG_TRAILER) += tag_trailer.o
 dsa_core-$(CONFIG_NET_DSA_TAG_QCA) += tag_qca.o
 dsa_core-$(CONFIG_NET_DSA_TAG_MTK) += tag_mtk.o
 dsa_core-$(CONFIG_NET_DSA_TAG_LAN9303) += tag_lan9303.o
+dsa_core-$(CONFIG_NET_DSA_TAG_KSZ) += tag_ksz.o
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 26130ae..6340323 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -61,6 +61,9 @@ const struct dsa_device_ops *dsa_device_ops[DSA_TAG_LAST] = {
 #ifdef CONFIG_NET_DSA_TAG_LAN9303
 	[DSA_TAG_PROTO_LAN9303] = &lan9303_netdev_ops,
 #endif
+#ifdef CONFIG_NET_DSA_TAG_KSZ
+	[DSA_TAG_PROTO_KSZ] = &ksz_netdev_ops,
+#endif
 	[DSA_TAG_PROTO_NONE] = &none_ops,
 };
 
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index f4a88e4..70183ac 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -96,4 +96,7 @@ extern const struct dsa_device_ops mtk_netdev_ops;
 /* tag_lan9303.c */
 extern const struct dsa_device_ops lan9303_netdev_ops;
 
+/* tag_ksz.c */
+extern const struct dsa_device_ops ksz_netdev_ops;
+
 #endif
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
new file mode 100644
index 0000000..270bfb9
--- /dev/null
+++ b/net/dsa/tag_ksz.c
@@ -0,0 +1,98 @@
+/*
+ * net/dsa/tag_ksz.c - Microchip KSZ Switch tag format handling
+ * Copyright (c) 2017 Microchip Technology
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/etherdevice.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <net/dsa.h>
+#include "dsa_priv.h"
+
+/* For Ingress (Host -> KSZ), 2 bytes are added before FCS.
+ * ---------------------------------------------------------------------------
+ * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes)
+ * ---------------------------------------------------------------------------
+ * tag0 : Prioritization (not used now)
+ * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x10=port5)
+ *
+ * For Egress (KSZ -> Host), 1 byte is added before FCS.
+ * ---------------------------------------------------------------------------
+ * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
+ * ---------------------------------------------------------------------------
+ * tag0 : zero-based value represents port
+ *	  (eg, 0x00=port1, 0x02=port3, 0x06=port7)
+ */
+
+static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct sk_buff *nskb;
+	int padlen;
+	u8 *tag;
+
+	padlen = 0;
+	if (skb->len < 60)
+		padlen = 60 - skb->len;
+
+	nskb = alloc_skb(NET_IP_ALIGN + skb->len + padlen + 2, GFP_ATOMIC);
+	if (!nskb) {
+		kfree_skb(skb);
+		return NULL;
+	}
+	skb_reserve(nskb, NET_IP_ALIGN);
+
+	skb_reset_mac_header(nskb);
+	skb_set_network_header(nskb, skb_network_header(skb) - skb->head);
+	skb_set_transport_header(nskb, skb_transport_header(skb) - skb->head);
+	skb_copy_and_csum_dev(skb, skb_put(nskb, skb->len));
+	kfree_skb(skb);
+
+	if (padlen) {
+		u8 *pad = skb_put(nskb, padlen);
+
+		memset(pad, 0, padlen);
+	}
+
+	tag = skb_put(nskb, 2);
+	tag[0] = 0;
+	tag[1] = 1 << p->dp->index; /* destnation port */
+
+	return nskb;
+}
+
+struct sk_buff *ksz_rcv(struct sk_buff *skb, struct net_device *dev,
+			struct packet_type *pt, struct net_device *orig_dev)
+{
+	struct dsa_switch_tree *dst = dev->dsa_ptr;
+	struct dsa_switch *ds;
+	u8 *tag;
+	int source_port;
+
+	ds = dst->cpu_switch;
+
+	if (skb_linearize(skb))
+		return NULL;
+
+	tag = skb_tail_pointer(skb) - 1;
+
+	source_port = tag[0] & 7;
+	if (source_port >= ds->num_ports || !ds->ports[source_port].netdev)
+		return NULL;
+
+	pskb_trim_rcsum(skb, skb->len - 1);
+
+	skb->dev = ds->ports[source_port].netdev;
+
+	return skb;
+}
+
+const struct dsa_device_ops ksz_netdev_ops = {
+	.xmit	= ksz_xmit,
+	.rcv	= ksz_rcv,
+};
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next 1/5] dsa: add support for Microchip KSZ tail tagging
  2017-05-05 23:17 [PATCH net-next 1/5] dsa: add support for Microchip KSZ tail tagging Woojung.Huh
@ 2017-05-05 23:34 ` Florian Fainelli
  2017-05-06  0:35   ` Woojung.Huh
  2017-05-06  3:38 ` Andrew Lunn
  1 sibling, 1 reply; 4+ messages in thread
From: Florian Fainelli @ 2017-05-05 23:34 UTC (permalink / raw)
  To: Woojung.Huh, andrew, vivien.didelot; +Cc: netdev, davem, UNGLinuxDriver

On 05/05/2017 04:17 PM, Woojung.Huh@microchip.com wrote:
> From: Woojung Huh <Woojung.Huh@microchip.com>
> 
> Adding support for the Microchip KSZ switch family tail tagging.
> 
> Signed-off-by: Woojung Huh <Woojung.Huh@microchip.com>
> ---
>  include/net/dsa.h  |  1 +
>  net/dsa/Kconfig    |  3 ++
>  net/dsa/Makefile   |  1 +
>  net/dsa/dsa.c      |  3 ++
>  net/dsa/dsa_priv.h |  3 ++
>  net/dsa/tag_ksz.c  | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 109 insertions(+)
>  create mode 100644 net/dsa/tag_ksz.c
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 8e24677..c92204a 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -34,6 +34,7 @@ enum dsa_tag_protocol {
>  	DSA_TAG_PROTO_QCA,
>  	DSA_TAG_PROTO_MTK,
>  	DSA_TAG_PROTO_LAN9303,
> +	DSA_TAG_PROTO_KSZ,
>  	DSA_TAG_LAST,		/* MUST BE LAST */
>  };
>  
> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
> index 81a0868..ce31428 100644
> --- a/net/dsa/Kconfig
> +++ b/net/dsa/Kconfig
> @@ -37,4 +37,7 @@ config NET_DSA_TAG_MTK
>  config NET_DSA_TAG_LAN9303
>  	bool
>  
> +config NET_DSA_TAG_KSZ
> +	bool
> +
>  endif
> diff --git a/net/dsa/Makefile b/net/dsa/Makefile
> index 0b747d7..8becb26 100644
> --- a/net/dsa/Makefile
> +++ b/net/dsa/Makefile
> @@ -10,3 +10,4 @@ dsa_core-$(CONFIG_NET_DSA_TAG_TRAILER) += tag_trailer.o
>  dsa_core-$(CONFIG_NET_DSA_TAG_QCA) += tag_qca.o
>  dsa_core-$(CONFIG_NET_DSA_TAG_MTK) += tag_mtk.o
>  dsa_core-$(CONFIG_NET_DSA_TAG_LAN9303) += tag_lan9303.o
> +dsa_core-$(CONFIG_NET_DSA_TAG_KSZ) += tag_ksz.o
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 26130ae..6340323 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -61,6 +61,9 @@ const struct dsa_device_ops *dsa_device_ops[DSA_TAG_LAST] = {
>  #ifdef CONFIG_NET_DSA_TAG_LAN9303
>  	[DSA_TAG_PROTO_LAN9303] = &lan9303_netdev_ops,
>  #endif
> +#ifdef CONFIG_NET_DSA_TAG_KSZ
> +	[DSA_TAG_PROTO_KSZ] = &ksz_netdev_ops,
> +#endif
>  	[DSA_TAG_PROTO_NONE] = &none_ops,
>  };
>  
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index f4a88e4..70183ac 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -96,4 +96,7 @@ extern const struct dsa_device_ops mtk_netdev_ops;
>  /* tag_lan9303.c */
>  extern const struct dsa_device_ops lan9303_netdev_ops;
>  
> +/* tag_ksz.c */
> +extern const struct dsa_device_ops ksz_netdev_ops;
> +
>  #endif
> diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
> new file mode 100644
> index 0000000..270bfb9
> --- /dev/null
> +++ b/net/dsa/tag_ksz.c
> @@ -0,0 +1,98 @@
> +/*
> + * net/dsa/tag_ksz.c - Microchip KSZ Switch tag format handling
> + * Copyright (c) 2017 Microchip Technology
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/etherdevice.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +#include <net/dsa.h>
> +#include "dsa_priv.h"
> +
> +/* For Ingress (Host -> KSZ), 2 bytes are added before FCS.
> + * ---------------------------------------------------------------------------
> + * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes)
> + * ---------------------------------------------------------------------------
> + * tag0 : Prioritization (not used now)
> + * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x10=port5)
> + *
> + * For Egress (KSZ -> Host), 1 byte is added before FCS.
> + * ---------------------------------------------------------------------------
> + * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
> + * ---------------------------------------------------------------------------
> + * tag0 : zero-based value represents port
> + *	  (eg, 0x00=port1, 0x02=port3, 0x06=port7)
> + */
> +
> +static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct dsa_slave_priv *p = netdev_priv(dev);
> +	struct sk_buff *nskb;
> +	int padlen;
> +	u8 *tag;
> +
> +	padlen = 0;
> +	if (skb->len < 60)
> +		padlen = 60 - skb->len;

Can you use ETH_ZLEN instead of 60 such that it is clear this is padding
to a minimum packet size (minus FCS)?

> +
> +	nskb = alloc_skb(NET_IP_ALIGN + skb->len + padlen + 2, GFP_ATOMIC);

Can you also define the "2" at the beginning of the file as being e.g:
TAG_KSZ_LEN?

> +	if (!nskb) {
> +		kfree_skb(skb);
> +		return NULL;
> +	}
> +	skb_reserve(nskb, NET_IP_ALIGN);
> +
> +	skb_reset_mac_header(nskb);
> +	skb_set_network_header(nskb, skb_network_header(skb) - skb->head);
> +	skb_set_transport_header(nskb, skb_transport_header(skb) - skb->head);
> +	skb_copy_and_csum_dev(skb, skb_put(nskb, skb->len));
> +	kfree_skb(skb);
> +
> +	if (padlen) {
> +		u8 *pad = skb_put(nskb, padlen);
> +
> +		memset(pad, 0, padlen);
> +	}
> +
> +	tag = skb_put(nskb, 2);
> +	tag[0] = 0;
> +	tag[1] = 1 << p->dp->index; /* destnation port */
> +
> +	return nskb;
> +}
> +
> +struct sk_buff *ksz_rcv(struct sk_buff *skb, struct net_device *dev,
> +			struct packet_type *pt, struct net_device *orig_dev)
> +{
> +	struct dsa_switch_tree *dst = dev->dsa_ptr;
> +	struct dsa_switch *ds;
> +	u8 *tag;
> +	int source_port;
> +
> +	ds = dst->cpu_switch;
> +
> +	if (skb_linearize(skb))
> +		return NULL;

Is that really necessary?

> +
> +	tag = skb_tail_pointer(skb) - 1;
> +
> +	source_port = tag[0] & 7;
> +	if (source_port >= ds->num_ports || !ds->ports[source_port].netdev)
> +		return NULL;
> +
> +	pskb_trim_rcsum(skb, skb->len - 1);

Humm, so we are still keeping tag[1] at the end of the frame?

> +
> +	skb->dev = ds->ports[source_port].netdev;
> +
> +	return skb;
> +}
> +
> +const struct dsa_device_ops ksz_netdev_ops = {
> +	.xmit	= ksz_xmit,
> +	.rcv	= ksz_rcv,
> +};
> 


-- 
Florian

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH net-next 1/5] dsa: add support for Microchip KSZ tail tagging
  2017-05-05 23:34 ` Florian Fainelli
@ 2017-05-06  0:35   ` Woojung.Huh
  0 siblings, 0 replies; 4+ messages in thread
From: Woojung.Huh @ 2017-05-06  0:35 UTC (permalink / raw)
  To: f.fainelli, andrew, vivien.didelot; +Cc: netdev, davem, UNGLinuxDriver

> > +	padlen = 0;
> > +	if (skb->len < 60)
> > +		padlen = 60 - skb->len;
> 
> Can you use ETH_ZLEN instead of 60 such that it is clear this is padding
> to a minimum packet size (minus FCS)?
> 
> > +
> > +	nskb = alloc_skb(NET_IP_ALIGN + skb->len + padlen + 2,
> GFP_ATOMIC);
> 
> Can you also define the "2" at the beginning of the file as being e.g:
> TAG_KSZ_LEN?
Got it. Will update.

> > +	ds = dst->cpu_switch;
> > +
> > +	if (skb_linearize(skb))
> > +		return NULL;
> 
> Is that really necessary?
I don't think so. Will check it.

> > +
> > +	tag = skb_tail_pointer(skb) - 1;
> > +
> > +	source_port = tag[0] & 7;
> > +	if (source_port >= ds->num_ports || !ds-
> >ports[source_port].netdev)
> > +		return NULL;
> > +
> > +	pskb_trim_rcsum(skb, skb->len - 1);
> 
> Humm, so we are still keeping tag[1] at the end of the frame?
It tags 2 bytes for ingress (ksz_xmit) and 1 byte for egress (ksz_rcv).
Put comment at the begin of net/dsa/tag_ksz.c.

Thanks.
Woojung

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next 1/5] dsa: add support for Microchip KSZ tail tagging
  2017-05-05 23:17 [PATCH net-next 1/5] dsa: add support for Microchip KSZ tail tagging Woojung.Huh
  2017-05-05 23:34 ` Florian Fainelli
@ 2017-05-06  3:38 ` Andrew Lunn
  1 sibling, 0 replies; 4+ messages in thread
From: Andrew Lunn @ 2017-05-06  3:38 UTC (permalink / raw)
  To: Woojung.Huh; +Cc: f.fainelli, vivien.didelot, netdev, davem, UNGLinuxDriver

> +static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct dsa_slave_priv *p = netdev_priv(dev);
> +	struct sk_buff *nskb;
> +	int padlen;
> +	u8 *tag;
> +
> +	padlen = 0;
> +	if (skb->len < 60)
> +		padlen = 60 - skb->len;
> +
> +	nskb = alloc_skb(NET_IP_ALIGN + skb->len + padlen + 2, GFP_ATOMIC);
> +	if (!nskb) {
> +		kfree_skb(skb);
> +		return NULL;
> +	}
> +	skb_reserve(nskb, NET_IP_ALIGN);
> +
> +	skb_reset_mac_header(nskb);
> +	skb_set_network_header(nskb, skb_network_header(skb) - skb->head);
> +	skb_set_transport_header(nskb, skb_transport_header(skb) - skb->head);
> +	skb_copy_and_csum_dev(skb, skb_put(nskb, skb->len));
> +	kfree_skb(skb);

Hi Woojung

Is there really no way to add data to the end of an existing skb?
skb_put(), once you have checked there is space? Only do the copy if
there is no space.

	   Andrew

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-05-06  3:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-05 23:17 [PATCH net-next 1/5] dsa: add support for Microchip KSZ tail tagging Woojung.Huh
2017-05-05 23:34 ` Florian Fainelli
2017-05-06  0:35   ` Woojung.Huh
2017-05-06  3:38 ` Andrew Lunn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).