linux-kernel - Re: [PATCH v1 RFC 7/7] Modify tag_ksz.c so that tail tag code can be used by other KSZ switch drivers

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]

Message-ID: <20171011204528.GA31288@amd>
Date:   Wed, 11 Oct 2017 22:45:28 +0200
From:   Pavel Machek <pavel@....cz>
To:     Tristram.Ha@...rochip.com
Cc:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Ruediger Schmitt <ruediger.schmitt@...lips.com>,
        muvarov@...il.com, nathan.leigh.conrad@...il.com,
        vivien.didelot@...oirfairelinux.com, UNGLinuxDriver@...rochip.com,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 RFC 7/7] Modify tag_ksz.c so that tail tag code can be
 used by other KSZ switch drivers

Hi!

> +#define	KSZ_INGRESS_TAG_LEN		1

This define is now (or should be) unused, so you can delete it, no?

> _#define	KSZ_EGRESS_TAG_LEN	1

And I'd delete this define, too. Having constant for something that's
variable is quite confusing :-).

Plus you are really doing too much inside single patch.

> + * For Egress (KSZ9477 -> 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)
> + */
> +
> +#define KSZ9477_INGRESS_TAG_LEN		2
> +#define KSZ9477_PTP_TAG_LEN		4
> +#define KSZ9477_PTP_TAG_INDICATION	0x80
> +
> +#define KSZ9477_TAIL_TAG_OVERRIDE	BIT(9)
> +#define KSZ9477_TAIL_TAG_LOOKUP		BIT(10)
> +
> +static int ksz9477_get_tag(u8 *tag, int *port)
> +{
> +	int len = KSZ_EGRESS_TAG_LEN;
> +
> +	/* Extra 4-bytes PTP timestamp */
> +	if (tag[0] & KSZ9477_PTP_TAG_INDICATION)
> +		len += KSZ9477_PTP_TAG_LEN;
> +	*port = tag[0] & 7;
> +	return len;
> +}
> +
> +static void ksz9477_set_tag(void *ptr, u8 *addr, int p)
> +{
> +	u16 *tag = (u16 *)ptr;
> +
> +	*tag = 1 << p;
> +	if (!memcmp(addr, special_mult_addr, ETH_ALEN))
> +		*tag |= KSZ9477_TAIL_TAG_OVERRIDE;
> +	*tag = cpu_to_be16(*tag);
> +}

These are new features that were not there before, right?
								Pavel
								
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)