Re: [PATCH 3/6] [NET] dsa: add support for original DSA tagging format

Previous thread: [PATCH 1/6] phylib: add mdiobus_{read,write} by Lennert Buytenhek on Sunday, September 28, 2008 - 7:37 pm. (5 messages)

Next thread: [PATCH 2/6] [NET] Distributed Switch Architecture protocol support by Lennert Buytenhek on Sunday, September 28, 2008 - 7:38 pm. (1 message)
From: Lennert Buytenhek
Date: Sunday, September 28, 2008 - 7:38 pm

Most of the DSA switches currently in the field do not support the
Ethertype DSA tagging format that one of the previous patches added
support for, but only the original DSA tagging format.

The original DSA tagging format carries the same information as the
Ethertype DSA tagging format, but with the difference that it does not
have an ethertype field.  In other words, when receiving a packet that
is tagged with an original DSA tag, there is no way of telling in
eth_type_trans() that this packet is in fact a DSA-tagged packet.

This patch adds a hook into eth_type_trans() which is only compiled in
if support for a switch chip that doesn't support Ethertype DSA is
selected, and which checks whether there is a DSA switch driver
instance attached to this network device which uses the old tag format.
If so, it sets the protocol field to ETH_P_DSA without looking at the
packet, so that the packet ends up in the right place.

Signed-off-by: Lennert Buytenhek <buytenh@marvell.com>
Tested-by: Nicolas Pitre <nico@marvell.com>
---
 include/linux/if_ether.h  |    1 +
 include/linux/netdevice.h |   11 +++
 include/net/dsa.h         |    2 +
 net/dsa/Kconfig           |    4 +
 net/dsa/Makefile          |    1 +
 net/dsa/dsa.c             |   16 ++++
 net/dsa/dsa_priv.h        |    4 +
 net/dsa/mv88e6123_61_65.c |   17 +++--
 net/dsa/slave.c           |    5 +
 net/dsa/tag_dsa.c         |  194 +++++++++++++++++++++++++++++++++++++++++++++
 net/ethernet/eth.c        |   10 +++
 11 files changed, 258 insertions(+), 7 deletions(-)
 create mode 100644 net/dsa/tag_dsa.c

diff --git a/include/linux/if_ether.h b/include/linux/if_ether.h
index a85f5a2..864982b 100644
--- a/include/linux/if_ether.h
+++ b/include/linux/if_ether.h
@@ -100,6 +100,7 @@
 #define ETH_P_ECONET	0x0018		/* Acorn Econet			*/
 #define ETH_P_HDLC	0x0019		/* HDLC frames			*/
 #define ETH_P_ARCNET	0x001A		/* 1A for ArcNet :-)            */
+#define ETH_P_DSA	0x001B		/* Distributed Switch Arch.	*/
 
 /*
  *	This is an ...
From: Ben Hutchings
Date: Friday, October 3, 2008 - 10:25 am

[...]

Why should this go in eth_type_trans()?  Why don't you put the hook into
the specific network driver(s) that need it?

For that matter, why should dsa_ptr go in struct net_device and not in
the private state for the specific network drivers that need it?

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--

From: Lennert Buytenhek
Date: Friday, October 3, 2008 - 12:32 pm

DSA is just another protocol.  Putting hooks in specific network driver
to handle a certain protocol doesn't seem like the right thing to do to
me.
--

From: Ben Hutchings
Date: Saturday, October 4, 2008 - 4:22 am

If it's just another protocol then put dsa_ptr along with the other
protocol pointers (ip_ptr etc.).

For those drivers that work with the original DSA tagging format, it's
apparently not just another protocol.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
--

From: Lennert Buytenhek
Date: Sunday, October 5, 2008 - 4:56 pm

The original DSA tagging format can work with any NIC, as long as you
hook up that NIC directly to a DSA switch chip instead of to a PHY.  In
embedded designs you mostly see platform bus drivers like mv643xx_eth
and gianfar and such, but there's no reason you can't do it with any
other MAC that exposes a (R)(G)MII interface.

The only reason that eth_type_trans() needs a hack is that the original
DSA tagging format has information starting right after the ethernet
source address, so there's no way to tell that original DSA tagged
frames are in fact original DSA tagged frames.  (The way we tell is by
having the DSA switch driver decide -- it configures the directly
attached switch chip to always send DSA tagged packets to the cpu's
switch port, and then forces all packets received on the underlying
ethernet device to get ETH_P_DSA assigned to skb->protocol.)
--

From: Stephen Hemminger
Date: Friday, October 3, 2008 - 10:36 am

I would use:
extern bool dsa_uses_dsa_tags(const struct dsa_switch *dsa);

--

From: Lennert Buytenhek
Date: Friday, October 3, 2008 - 12:47 pm

OK.  Did the same thing for the trailer tag support.
--

Previous thread: [PATCH 1/6] phylib: add mdiobus_{read,write} by Lennert Buytenhek on Sunday, September 28, 2008 - 7:37 pm. (5 messages)

Next thread: [PATCH 2/6] [NET] Distributed Switch Architecture protocol support by Lennert Buytenhek on Sunday, September 28, 2008 - 7:38 pm. (1 message)