lkml.org 
[lkml]   [2023]   [Jun]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH net-next v6 3/6] net: bcmasp: Add support for ASP2.0 Ethernet controller
    From


    On 6/2/2023 11:58 PM, Jakub Kicinski wrote:
    > On Thu, 1 Jun 2023 15:12:28 -0700 Justin Chen wrote:
    >> + /* general stats */
    >> + STAT_NETDEV(rx_packets),
    >> + STAT_NETDEV(tx_packets),
    >> + STAT_NETDEV(rx_bytes),
    >> + STAT_NETDEV(tx_bytes),
    >> + STAT_NETDEV(rx_errors),
    >> + STAT_NETDEV(tx_errors),
    >> + STAT_NETDEV(rx_dropped),
    >> + STAT_NETDEV(tx_dropped),
    >> + STAT_NETDEV(multicast),
    >
    > please don't report standard interface stats in ethtool -S
    >
    >> + /* UniMAC RSV counters */
    >> + STAT_BCMASP_MIB_RX("rx_64_octets", mib.rx.pkt_cnt.cnt_64),
    >> + STAT_BCMASP_MIB_RX("rx_65_127_oct", mib.rx.pkt_cnt.cnt_127),
    >> + STAT_BCMASP_MIB_RX("rx_128_255_oct", mib.rx.pkt_cnt.cnt_255),
    >> + STAT_BCMASP_MIB_RX("rx_256_511_oct", mib.rx.pkt_cnt.cnt_511),
    >> + STAT_BCMASP_MIB_RX("rx_512_1023_oct", mib.rx.pkt_cnt.cnt_1023),
    >> + STAT_BCMASP_MIB_RX("rx_1024_1518_oct", mib.rx.pkt_cnt.cnt_1518),
    >> + STAT_BCMASP_MIB_RX("rx_vlan_1519_1522_oct", mib.rx.pkt_cnt.cnt_mgv),
    >> + STAT_BCMASP_MIB_RX("rx_1522_2047_oct", mib.rx.pkt_cnt.cnt_2047),
    >> + STAT_BCMASP_MIB_RX("rx_2048_4095_oct", mib.rx.pkt_cnt.cnt_4095),
    >> + STAT_BCMASP_MIB_RX("rx_4096_9216_oct", mib.rx.pkt_cnt.cnt_9216),
    >
    > these should also be removed, and you should implement @get_rmon_stats.
    >
    >> + STAT_BCMASP_MIB_RX("rx_pkts", mib.rx.pkt),
    >> + STAT_BCMASP_MIB_RX("rx_bytes", mib.rx.bytes),
    >> + STAT_BCMASP_MIB_RX("rx_multicast", mib.rx.mca),
    >> + STAT_BCMASP_MIB_RX("rx_broadcast", mib.rx.bca),
    >> + STAT_BCMASP_MIB_RX("rx_fcs", mib.rx.fcs),
    >
    > there's a FCS error statistic in the standard stats, no need to
    > duplicate
    >
    >> + STAT_BCMASP_MIB_RX("rx_control", mib.rx.cf),
    >> + STAT_BCMASP_MIB_RX("rx_pause", mib.rx.pf),
    >
    > @get_pause_stats
    >
    >> + STAT_BCMASP_MIB_RX("rx_unknown", mib.rx.uo),
    >> + STAT_BCMASP_MIB_RX("rx_align", mib.rx.aln),
    >> + STAT_BCMASP_MIB_RX("rx_outrange", mib.rx.flr),
    >> + STAT_BCMASP_MIB_RX("rx_code", mib.rx.cde),
    >> + STAT_BCMASP_MIB_RX("rx_carrier", mib.rx.fcr),
    >> + STAT_BCMASP_MIB_RX("rx_oversize", mib.rx.ovr),
    >> + STAT_BCMASP_MIB_RX("rx_jabber", mib.rx.jbr),
    >
    > these look like candidates from standard stats, too.
    > Please read thru:
    >
    > https://docs.kernel.org/next/networking/statistics.html
    >
    >> + STAT_BCMASP_MIB_RX("rx_mtu_err", mib.rx.mtue),
    >> + STAT_BCMASP_MIB_RX("rx_good_pkts", mib.rx.pok),
    >> + STAT_BCMASP_MIB_RX("rx_unicast", mib.rx.uc),
    >> + STAT_BCMASP_MIB_RX("rx_ppp", mib.rx.ppp),
    >> + STAT_BCMASP_MIB_RX("rx_crc", mib.rx.rcrc),
    >
    > hm, what's the difference between rx_crc and rx_fcs ?

    Since you are going to respin to address Jakub's feedback, we should
    also consider using the shared unimac.h header file. We could even make
    a library out of it for standardized statistics once this driver gets
    accepted.
    --
    Florian
    [unhandled content-type:application/pkcs7-signature]
    \
     
     \ /
      Last update: 2023-06-06 00:04    [W:2.815 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site