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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20081005235658.GA30332@xi.wantstofly.org>
Date:	Mon, 6 Oct 2008 01:56:58 +0200
From:	Lennert Buytenhek <buytenh@...tstofly.org>
To:	Ben Hutchings <bhutchings@...arflare.com>
Cc:	netdev@...r.kernel.org, Byron Bradley <byron.bbradley@...il.com>,
	Jesper Dangaard Brouer <jdb@...x.dk>,
	Tim Ellis <tim.ellis@....com>,
	Andy Fleming <afleming@...escale.com>,
	Imre Kaloz <kaloz@...nwrt.org>, Nicolas Pitre <nico@....org>,
	Dirk Teurlings <dirk@...xia.nl>,
	Peter van Valderen <p.v.valderen@...il.com>
Subject: Re: [PATCH 3/6] [NET] dsa: add support for original DSA tagging format

On Sat, Oct 04, 2008 at 12:22:37PM +0100, Ben Hutchings wrote:

> > > > 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.
> > > [...]
> > > 
> > > 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?
> > 
> > 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.
> 
> If it's just another protocol then put dsa_ptr along with the other
> protocol pointers (ip_ptr etc.).

OK, I'll do that.


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

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.)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html