netdev - Re: [PATCHv3 net-next 1/3] sunvnet: upgrade to VIO protocol version 1.6

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:	Sat, 13 Sep 2014 16:18:42 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	david.stevens@...cle.com
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCHv3 net-next 1/3] sunvnet: upgrade to VIO protocol
 version 1.6

From: David L Stevens <david.stevens@...cle.com>
Date: Sat, 13 Sep 2014 12:01:06 -0400

> @@ -368,6 +381,8 @@ struct vio_driver_state {
>  	char			*name;
>  
>  	struct vio_driver_ops	*ops;
> +
> +	u64			rmtu;	/* remote MTU */
>  };
>  
>  #define viodbg(TYPE, f, a...) \

Is this really applicable to devices other than sunvnet?  Just keep
this attribute in the sunvnet driver "struct vnet_port" private state.

> +	/* v1.6 and higher, ACK with desired, supported mode, or NACK */
> +	if (vio->ver.major <= 1 && vio->ver.minor >= 6) {
 ...
> +	if (vio->ver.major <= 1 && vio->ver.minor < 2)
 ...
> +	/* for version < 1.2, VIO_DRING_MODE = 0x3 and no bitmask */
> +	if ((vio->ver.major <= 1 &&  vio->ver.minor < 2) &&
...
> +	if (vio->ver.major == 1 && vio->ver.minor < 3) {
	...
> +	} else if (vio->ver.major == 1 && vio->ver.minor == 3) {
	...
> +	} else {
	...
> +	}
 ...
> +	/* for version >= 1.6, ACK packet mode we support */
> +	if (vio->ver.major <= 1 && vio->ver.minor >= 6) {

These version tests give me a headache, and some of them accept
impossible things like major version zero.  If we only have major
version 1 and later in our version lists, testing things like "major
<= 1" makes no sense at all.

That last test quoted above definitely looks wrong, it should
be testing "major >= 1" if anything if it is trying to test
"version >= 1.6"

Sadly, I expect that we'll have more and more of these version tests
over time.  So why not make some generic helpers in the VIO or LDC
layer?

static inline bool vio_version_before(struct vio_driver_state *vio,
				      u16 major, u16 minor)
{
	u32 have = (u32)vio->major << 16 | vio->minor;
	u32 want = (u32)major << 16 | minor;

	return have < want;
}
static inline bool vio_version_after_eq(struct vio_driver_state *vio,
					u16 major, u16 minor)
{
	u32 have = (u32)vio->major << 16 | vio->minor;
	u32 want = (u32)major << 16 | minor;

	return have >= want;
}

Something like that.
--
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