netdev - Re: [PATCH] [v9] net: emac: emac gigabit ethernet controller driver

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: <57C1FB42.104@codeaurora.org>
Date:   Sat, 27 Aug 2016 15:42:42 -0500
From:   Timur Tabi <timur@...eaurora.org>
To:     Rami Rosen <roszenrami@...il.com>
Cc:     Netdev <netdev@...r.kernel.org>, devicetree@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, sdharia@...eaurora.org,
        shankerd@...eaurora.org, vikrams@...eaurora.org,
        cov@...eaurora.org, gavidov@...eaurora.org, robh+dt@...nel.org,
        andrew@...n.ch, bjorn.andersson@...aro.org, mlangsdo@...hat.com,
        jcm@...hat.com, agross@...eaurora.org,
        David Miller <davem@...emloft.net>,
        Florian Fainelli <f.fainelli@...il.com>, LinoSanfilippo@....de
Subject: Re: [PATCH] [v9] net: emac: emac gigabit ethernet controller driver

Rami Rosen wrote:
> Hi,
>
> Shouldn't it be obj-$(CONFIG_QCOM_EMAC) instead obj-y?

Yes, I noticed this soon after I posted this patch.  I will fix it in v10.

> The return type of the method emac_tx_tpd_create()  should be void, as
> it always returns true.

Will fix, thanks.

> Seems that there are several unused members in the emac_stats struct:
>
>> +struct emac_stats {
> ...
> ...
> Both rx_bcast_byte_cnt and rx_mcast_byte_cnt are not used anywhere/
>> +       u64 rx_bcast_byte_cnt;  /* broadcast packets byte count (without FCS) */
>> +       u64 rx_mcast_byte_cnt;  /* multicast packets byte count (without FCS) */
> ...
> rx_err_addr is not used
>> +       u64 rx_err_addr;        /* packets dropped due to address filtering */

I'll go through the structure and remove the unused fields.

>
> Typo: it is jabbers and not jubbers; greppping for "jubber" under
> drivers/net/ethernet gives
> 0 results, whereas:
> /net-next/drivers/net/ethernet$ grep -R jabber | wc -l
> gives 241 results.
>
>> +       u64 rx_jubbers;         /* jubbers */

Thanks.  I'll fix that in v10.

>
>
> ...
> ..
> tx_rd_eop and tx_len_err are not used
>> +       u64 tx_rd_eop;          /* count of reads beyond EOP */
>> +       u64 tx_len_err;         /* packets with length mismatch */
>
> Both tx_bcast_byte and tx_mcast_byte are not used.
>> +       u64 tx_bcast_byte;      /* broadcast packets byte count (without FCS) */
>> +       u64 tx_mcast_byte;      /* multicast packets byte count (without FCS) */
>
> Thanks for your good work, it seems that the driver is almost done!

I'm grateful for the opportunity to improve this driver over the crappy 
one that's been bouncing around internally for years, and for all the 
people who helped me along the way.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.