[lkml]   [2002]   [Mar]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [patch] ns83820 0.17 (Re: Broadcom 5700/5701 Gigabit Ethernet Adapters)
    Benjamin LaHaise wrote:

    >>3) Seeing "volatile" in your code. Cruft? volatile's meaning change in
    >>recent gcc versions implies to me that your code may need some addition
    >>rmb/wmb calls perhaps, which are getting hidden via the driver's
    >>dependency on a compiler-version-specific implementation of "volatile."
    >Paranoia during writing. I'll reaudit. That said, volatile behaviour
    >is not compiler version specific.
    gcc 3.1 volatile behavior changes, so, yes, it is...

    >>4) Do you really mean to allocate memory for "REAL_RX_BUF_SIZE + 16"?
    >> Why not plain old REAL_RX_BUF_SIZE?
    >The +16 is for alignment (just like the comment says). The hardware
    >requires that rx buffers be 64 bit aligned.
    Cool... just checking. Both RX_BUF_SIZE and REAL_RX_BUF_SIZE are defined as
    foo + magic_number

    so I wasn't sure if the alignment space was -already- accounted for, in
    the definition of RX_BUF_SIZE, thus making the addition op next to
    allocations of REAL_RX_BUF_SIZE superfluous. But, I stand corrected,

    >5) Random question, do you call netif_carrier_{on,off,ok} for link
    >> status manipulation? (if not, you should...)
    >Ah, api updates. Added to the todo.
    More than just api updates... You have a bunch of hack-y logic for when
    the link goes down and up, messing around with netif_stop_queue and
    netif_wake_queue. That stuff will be simplified or simply go away. The
    basic idea is, if netif_carrier_ok(dev) is not true, then the net stack
    will not be sending you any packets. So those extra
    netif_{stop,wake}_queue calls are superflouous.

    We're also about to start sending link up/down messages async-ly via
    netlink, so that's even more added value as well.

    >>6) skb_mangle_for_davem is pretty gross... curious: what sort of NIC
    >>alignment restrictions are there on rx and tx buffers (not descriptors)?
    >> None? 32-bit? Ignore CPU alignment for a moment here...
    >tx descriptors have no alignment restriction, rx descriptors must be
    >64 bit aligned. Someone chose not to include the transistors for a
    >barrel shifter in the rx engine.

    Sigh :)

    >>7) What are the criteria for netif_wake_queue? If you are waking when
    >>the TX is still "mostly full" you probably generate excessive wakeups...
    >Hrm? Currently it will do a wakeup when at least one packet (8 sg
    >descriptors) can be sent. Given that the tx done code is only called
    >when a tx desc (every 1/4 or so of the tx queue) or txidle interrupt
    >occurs, it shouldn't be that often.

    Cool. As FYI (_not_ advice on your driver), here's the logic I was
    referring to:

    if (free slots < MAX_SKB_FRAGS)
    queue packet
    if (free slots < MAX_SKB_FRAGS)

    if (some tx interrupt)
    complete as many TX's as possible
    if (netif_queue_stopped && (free slots > (TX_RING_SIZE / 4)))

    But as long as your TX interrupts are well mitigated (and it sounds like
    they are), you can get by with your current scheme just fine.


    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2005-03-22 13:25    [W:0.028 / U:1.948 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site