Message-ID: <20181203014258.GJ30556@wantstofly.org>
Date:   Mon, 3 Dec 2018 03:42:58 +0200
From:   Lennert Buytenhek <buytenh@...tstofly.org>
To:     Pavel Machek <pavel@....cz>
Cc:     Andrew Lunn <andrew@...n.ch>, netdev@...r.kernel.org,
        f.fainelli@...il.com
Subject: Re: DSA support for Marvell 88e6065 switch

On Thu, Nov 22, 2018 at 09:27:24PM +0100, Pavel Machek wrote:

> Hi!

Hello!


> > > > > If I wanted it to work, what do I need to do? AFAICT phy autoprobing
> > > > > should just attach it as soon as it is compiled in?
> > > > 
> > > > Nope. It is a switch, not a PHY. Switches are never auto-probed
> > > > because they are not guaranteed to have ID registers.
> > > > 
> > > > You need to use the legacy device tree binding. Look in
> > > > Documentation/devicetree/bindings/net/dsa/dsa.txt, section Deprecated
> > > > Binding. You can get more examples if you checkout old kernels. Or
> > > > kirkwood-rd88f6281.dtsi, the dsa { } node which is disabled.
> > > 
> > > Thanks; I ported code from mv88e66xx in the meantime, and switch
> > > appears to be detected.
> > > 
> > > But I'm running into problems with tagging code, and I guess I'd like
> > > some help understanding.
> > > 
> > > tag_trailer: allocates new skb, then copies data around.
> > > 
> > > tag_qca: does dev->stats.tx_packets++, and reuses existing skb.
> > > 
> > > tag_brcm: reuses existing skb.
> 
> Any idea why tag trailer allocates new skb,

I wrote this code over 10 years ago, so I don't remember all that
well, but I think that it is because you have to do manual checksumming
of the packet, as there's no way to pass down the stack that you don't
want to checksum all the way down to the end of the data area (and you
don't want the tag to be included in the checksum), and so you want to
do that before you add the trailer tag, and you'll probably have to
reallocate the data area to be able to add the tag, and you probably
won't get an exclusive skb here anyway, so you might as well allocate
a new one.


> and what is going on with dev->stats.tx_packets++?

trailer_xmit would be the hard_start_xmit function for the virtual
(slave) network interface, so this would be the right thing to do?


> > > Is qca wrong in adjusting the statistics? Why does trailer allocate
> > > new skb?
> > > 
> > > 6065 seems to use 2-byte header between "SFD" and "Destination
> > > address" in the ethernet frame. That's ... strange place to put
> > > header, as addresses are now shifted. I need to put ethernet in
> > > promisc mode (by running tcpdump) to get data moving.. and can not
> > > figure out what to do in tag_...
> > 
> > Does this switch chip not also support trailer mode?
> > 
> > There's basically four tagging modes for Marvell switch chips: header
> > mode (the one you described), trailer mode (tag_trailer.c), DSA and
> > ethertype DSA.  The switch chips I worked on that didn't support
> > (ethertype) DSA tagging did support both header and trailer modes,
> > and I chose to run them in trailer mode for the reasons you describe
> > above, but if your chip doesn't support trailer mode, then yes,
> > you'll have to add support for header mode and put the underlying
> > interface into promiscuous mode and such.
> 
> It seems that 6060 supports both header (probably, parts of docs are
> redacted) and trailer mode... but I'm working with 6065. That does not
> support trailer mode... or at least word "trailer" does not appear
> anywhere in the documentation.
> 
> What chip were you working with? I may want to take a look on their
> wording.

I think I added trailer mode just for the 6060, since it doesn't (IIRC)
support (ethertype) DSA tagging.


> 6065 indeed has some kind of "egress tagging mode" (with four
> options), but I have trouble understanding what it really does.

What are the options?

Powered by blists - more mailing lists