netdev - Re: [PATCH net-next v2 1/1] net: fec: Add VLAN receive HW 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]

Message-Id: <20130701.170929.2145379004044730481.davem@davemloft.net>
Date:	Mon, 01 Jul 2013 17:09:29 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	jim_baxter@...tor.com
Cc:	Frank.Li@...escale.com, B38611@...escale.com,
	fabio.estevam@...escale.com, l.stach@...gutronix.de,
	shawn.guo@...aro.org, netdev@...r.kernel.org,
	bhutchings@...arflare.com
Subject: Re: [PATCH net-next v2 1/1] net: fec: Add VLAN receive HW support.

From: Jim Baxter <jim_baxter@...tor.com>
Date: Fri, 28 Jun 2013 15:08:23 +0100

> @@ -803,6 +807,9 @@ fec_enet_rx(struct net_device *ndev, int budget)
>  	ushort	pkt_len;
>  	__u8 *data;
>  	int	pkt_received = 0;
> +	struct	bufdesc_ex *ebdp = NULL;
> +	bool	vlan_packet_rcvd = false;
> +	u16	vlan_tag;
>  
>  #ifdef CONFIG_M532x
>  	flush_cache_all();
> @@ -866,6 +873,24 @@ fec_enet_rx(struct net_device *ndev, int budget)
>  		if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
>  			swap_buffer(data, pkt_len);
>  
> +		/* Extract the enhanced buffer descriptor */
> +		ebdp = NULL;
> +		if (fep->bufdesc_ex)
> +			ebdp = (struct bufdesc_ex *)bdp;

I would use a union here, so you'd have something like:

	union {
		struct bufdesc		*bdp;
		struct bufdesc_ex	*bdp_ex;
	} *p;

Alternatively, you can always use "struct bufdesc_ex *p", along with
the boolean saying if the extended descriptors are in use.

> +		if ((ndev->features & NETIF_F_HW_VLAN_CTAG_RX) &&
> +				ebdp && (ebdp->cbd_esc & BD_ENET_RX_VLAN)) {

This is not indented properly.   The "ebp" on that second line must line
up exactly with the first column after the openning parenthesis on the
first line.

I see what you're trying to do, purely using TAB characters to indent
that second line.  But you must take the care and time to use the
appropriate number of TAB and space characters to place things at the
proper column.

> +			skb_copy_to_linear_data_offset(skb, (2 * ETH_ALEN),
> +					data + payload_offset,
> +					pkt_len - 4 - (2 * ETH_ALEN));

Again, line up the first non-space character on the second and third
lines of this function call so that they line up to the first column
after the openning parenthesis of the first line.

> +				__vlan_hwaccel_put_tag(skb,
> +						htons(ETH_P_8021Q), vlan_tag);

Likewise.
--
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