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] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 5 May 2022 15:48:06 +0200
From:   Clément Léger <clement.leger@...tlin.com>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Magnus Damm <magnus.damm@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Herve Codina <herve.codina@...tlin.com>,
        Miquèl Raynal <miquel.raynal@...tlin.com>,
        Milan Stevanovic <milan.stevanovic@...com>,
        Jimmy Lalande <jimmy.lalande@...com>,
        Pascal Eberhard <pascal.eberhard@...com>,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        linux-renesas-soc@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v3 02/12] net: dsa: add Renesas RZ/N1 switch
 tag driver

Le Wed, 4 May 2022 19:00:39 +0300,
Vladimir Oltean <olteanv@...il.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  
> 
> Maybe an ETH_P_DSA_A5PSW definition in include/uapi/linux/if_ether.h
> would be appropriate? I'm not sure.

That's a good question. Actually, this value is the default 
but is configurable in the hardware so I'm not sure this should be a
reserved value. Maybe it would make sense to add it anyway to have the
define shared between the switch driver and the tag driver.

> 
> > +#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 {
> > +	__be16 ctrl_tag;
> > +	__be16 ctrl_data;
> > +	__be16 ctrl_data2_hi;
> > +	__be16 ctrl_data2_lo;
> > +};
> > +
> > +static struct sk_buff *a5psw_tag_xmit(struct sk_buff *skb, struct net_device *dev)
> > +{
> > +	struct dsa_port *dp = dsa_slave_to_port(dev);
> > +	struct a5psw_tag *ptag;
> > +	u32 data2_val;
> > +
> > +	BUILD_BUG_ON(sizeof(*ptag) != A5PSW_TAG_LEN);
> > +
> > +	/* 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 70 bytes (including FCS and
> > +	 * tag).
> > +	 */
> > +	if (__skb_put_padto(skb, ETH_ZLEN + ETH_FCS_LEN + A5PSW_TAG_LEN, false))
> > +		return NULL;  
> 
> I'm confused by the inclusion of the FCS length in this calculation,
> since the FCS space isn't present in the skb buffer as far as I know?

I'm not sure either, the documentation is not really clear on what is
the requirement for the minimal size of a packet. This was the closest
thing I could find about that requirement:

"A frame has a valid length if it contains at least 64 octets and does
not exceed the programmed maximum length"

And the figure associated to the frame show that the frame length
includes the FCS which lead to a 64bytes frame.
> 
> "64 bytes including FCS" means "60 bytes excluding FCS".
> And ETH_ZLEN is 60...
> 
> And I'm also not sure how we got to the number 70? A5PSW_TAG_LEN is 8.
> If we add it to 60 we get 68. If we add it to 64 we get 72?
>

I'll check all these numbers and find the correct size that is expected
by the switch.

-- 
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com

Powered by blists - more mailing lists