netdev - Re: [PATCH net-next RESEND 2/2] FDDI: DEC FDDIcontroller 700 TURBOchannel card support

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:	Sun, 27 Apr 2014 19:13:46 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	macro@...ux-mips.org
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH net-next RESEND 2/2] FDDI: DEC FDDIcontroller 700
 TURBOchannel card support

From: "Maciej W. Rozycki" <macro@...ux-mips.org>
Date: Fri, 25 Apr 2014 10:35:15 +0100 (BST)

> +	/* Reset the board. */
> +	wmb();
> +	writew(FZA_RESET_INIT, &fp->regs->reset);
> +	iob();
> +	readw(&fp->regs->reset);	/* Read it back for a small delay. */
> +	iob();
> +	writew(FZA_RESET_CLR, &fp->regs->reset);

Using memory barriers around MMIO operations makes zero sense.

wmb() and friends order physical memory operations, and are only
to be used when the cpu must be synchronized with some other entity
looking at the same physical memory the cpu is, such as a device.

But that absolutely is not what is happening here, you're doing one
MMIO to non-physical memory after another.

> +{
> +	uint status, state;
> +
> +	unsigned long int flags;
> +	long int t;
> +

Use "unsigned int" and "long", and also please do not put empty lines
in the middle of the function variable declarations.

> +
> +	pr_info("%s: resetting the board...\n", fp->name);

Do not put more than one empty line between the local variables
and the first non-declaration statement of the function.

> +static struct fza_ring_cmd __iomem *fza_cmd_send(struct net_device *dev,
> +						 int command)

When a function declaration or definition or call spans multiple lines,
the second and subsequent lines should be indented to exactly the first
column after the openning parenthesis.

You must use the appropriate number of TAB and SPACE characters necessary
to do so.  Generally speaking, if you are only using TAB characters you
are indenting it incorrectly.

I'm sure there are lots of other coding style issues, please run
checkpatch on your changes unless you want to go back and forth
in review several more times as we find them too :-)

As per these SMT packets, don't even publish them to network taps.

We would have never seen them anyways, so keep them completely
internal to your driver.

That way you have no reason ti poke into internals like ptype_all
and dev_queue_xmit_nit().
--
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