netdev - Re: Subject: [RFC][PATCH 07/11] broadcom: trivial sparse annotations

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] [day] [month] [year] [list]
Message-ID: <069dbe6b-e83d-d838-f5a1-926b23a360cd@gmail.com>
Date:   Sun, 7 Jan 2018 17:31:55 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Al Viro <viro@....linux.org.uk>, netdev@...r.kernel.org,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: Subject: [RFC][PATCH 07/11] broadcom: trivial sparse annotations



On 01/05/2018 11:32 AM, Al Viro wrote:
> 
> Signed-off-by: Al Viro <viro@...iv.linux.org.uk>
> ---
>  drivers/net/ethernet/broadcom/bcmsysport.c     | 2 +-
>  drivers/net/ethernet/broadcom/bgmac.h          | 6 +++---
>  drivers/net/ethernet/broadcom/bnx2.c           | 6 +++---
>  drivers/net/ethernet/broadcom/cnic_if.h        | 8 ++++----
>  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 4 ++--
>  drivers/net/ethernet/broadcom/tg3.c            | 2 +-
>  6 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
> index f15a8fc6dfc9..c2969b260aed 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
> @@ -1156,7 +1156,7 @@ static struct sk_buff *bcm_sysport_insert_tsb(struct sk_buff *skb,
>  	memset(tsb, 0, sizeof(*tsb));
>  
>  	if (skb->ip_summed == CHECKSUM_PARTIAL) {
> -		ip_ver = htons(skb->protocol);
> +		ip_ver = ntohs(skb->protocol);

Al can you make sure you CC drivers maintainers for these changes? You
change bcmsysport.c and bcmgenet.c but you leave tg3.c with the same
type of construct in tg3_start_xmit() any reason for that? I am also
curious about this conversion considering the following numbers:

grep 'skb->protocol == htons' {net/*,drivers/net/*} | wc -l
151
git grep 'skb->protocol == ntohs' {net/*,drivers/net/*} | wc -l
0

>  		switch (ip_ver) {
>  		case ETH_P_IP:
>  			ip_proto = ip_hdr(skb)->protocol;
> diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
> index 4040d846da8e..40d02fec2747 100644
> --- a/drivers/net/ethernet/broadcom/bgmac.h
> +++ b/drivers/net/ethernet/broadcom/bgmac.h
> @@ -479,9 +479,9 @@ struct bgmac_rx_header {
>  struct bgmac {
>  	union {
>  		struct {
> -			void *base;
> -			void *idm_base;
> -			void *nicpm_base;
> +			void __iomem *base;
> +			void __iomem *idm_base;
> +			void __iomem *nicpm_base;
>  		} plat;

This part of the patch is correct.

>  		struct {
>  			struct bcma_device *core;
> diff --git a/drivers/net/ethernet/broadcom/bnx2.c b/drivers/net/ethernet/broadcom/bnx2.c
> index 7919f6112ecf..154866e8517a 100644
> --- a/drivers/net/ethernet/broadcom/bnx2.c
> +++ b/drivers/net/ethernet/broadcom/bnx2.c
> @@ -8330,9 +8330,9 @@ bnx2_init_board(struct pci_dev *pdev, struct net_device *dev)
>  		if (j < 32)
>  			bp->fw_version[j++] = ' ';
>  		for (i = 0; i < 3 && j < 28; i++) {
> -			reg = bnx2_reg_rd_ind(bp, addr + i * 4);
> -			reg = be32_to_cpu(reg);
> -			memcpy(&bp->fw_version[j], &reg, 4);
> +			__be32 v;
> +			v = cpu_to_be32(bnx2_reg_rd_ind(bp, addr + i * 4));
> +			memcpy(&bp->fw_version[j], &v, 4);
>  			j += 4;
>  		}
>  	}
> diff --git a/drivers/net/ethernet/broadcom/cnic_if.h b/drivers/net/ethernet/broadcom/cnic_if.h
> index 789e5c7e9311..8cfef07a8e3d 100644
> --- a/drivers/net/ethernet/broadcom/cnic_if.h
> +++ b/drivers/net/ethernet/broadcom/cnic_if.h
> @@ -260,10 +260,10 @@ struct cnic_sockaddr {
>  struct cnic_sock {
>  	struct cnic_dev *dev;
>  	void	*context;
> -	u32	src_ip[4];
> -	u32	dst_ip[4];
> -	u16	src_port;
> -	u16	dst_port;
> +	__be32	src_ip[4];
> +	__be32	dst_ip[4];
> +	__be16	src_port;
> +	__be16	dst_port;
>  	u16	vlan_id;
>  	unsigned char old_ha[ETH_ALEN];
>  	unsigned char ha[ETH_ALEN];
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 24b4f4ceceef..77154f1479a9 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -1321,7 +1321,7 @@ static struct sk_buff *bcmgenet_free_tx_cb(struct device *dev,
>  		dma_unmap_addr_set(cb, dma_addr, 0);
>  	}
>  
> -	return 0;
> +	return NULL;

And this part as well is correct.

>  }
>  
>  /* Simple helper to free a receive control block's resources */
> @@ -1480,7 +1480,7 @@ static struct sk_buff *bcmgenet_put_tx_csum(struct net_device *dev,
>  	status = (struct status_64 *)skb->data;
>  
>  	if (skb->ip_summed  == CHECKSUM_PARTIAL) {
> -		ip_ver = htons(skb->protocol);
> +		ip_ver = ntohs(skb->protocol);
>  		switch (ip_ver) {
>  		case ETH_P_IP:
>  			ip_proto = ip_hdr(skb)->protocol;
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index a77ee2f8fb8d..2bd77d9990f2 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -3744,7 +3744,7 @@ static int tg3_load_firmware_cpu(struct tg3 *tp, u32 cpu_base,
>  	}
>  
>  	do {
> -		u32 *fw_data = (u32 *)(fw_hdr + 1);
> +		__be32 *fw_data = (__be32 *)(fw_hdr + 1);
>  		for (i = 0; i < tg3_fw_data_len(tp, fw_hdr); i++)
>  			write_op(tp, cpu_scratch_base +
>  				     (be32_to_cpu(fw_hdr->base_addr) & 0xffff) +
> 

-- 
Florian