Date: Tue, 13 Jan 2015 13:22:09 -0500 From: John Baldwin <jhb@freebsd.org> To: Gleb Smirnoff <glebius@freebsd.org> Cc: svn-src-projects@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r277122 - projects/ifnet/sys/dev/msk Message-ID: <5330876.Sb1U9Iz8Cz@ralph.baldwin.cx> In-Reply-To: <201501130902.t0D927NE077024@svn.freebsd.org> References: <201501130902.t0D927NE077024@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tuesday, January 13, 2015 09:02:07 AM Gleb Smirnoff wrote: > Author: glebius > Date: Tue Jan 13 09:02:06 2015 > New Revision: 277122 > URL: https://svnweb.freebsd.org/changeset/base/277122 > > Log: > Welcome first real hardware NIC driver successfully converted to new > API, which hides struct ifnet from drivers entirely. > > This driver can be taken as an example, and this commit message will > probably be used to start a wiki page on conversion. > > List of changes required: > > o Remove at least if_var.h, bpf.h, if_arp.h, if_types.h, if_vlan_var.h > from includes list. > o Declare struct ifdriver in the beginning of a file. > o Convert from xxx_start(ifp) to xxx_transmit(ifp, m). > A simple conversion itself is quite straight: > * In ifdriver declaration define .ifdrv_maxqlen, take the value > from IFQ_SET_MAXLEN() macro. > * In ifdriver ifops declaration define if_transmit function. > * Rename xxx_start() to xxx_transmit() and change its prototype. > * The new named xxx_transmit() should: > - Try to if_snd_enqueue() the mbuf or return. > - Try to mtx_lock() the driver softc or return. Please no. This is a major source of pain with the current drivers that use buf_ring that is worked around in various (often broken) ways. The problem is that the other thread holding the lock might drop it right after your try lock fails and then the packet you just queued doesn't go out until you queue another packet. That's all fine and good if you are blasting packets out an interface. It is not fine and good if you have sparse traffic. That packet is now potentially delayed indefinitely. (In a real world scenario with a heartbeat protocol that sent out timing packets once a second or so to measure RTT between hosts this manifested as odd timings because we would see the timings jump by a second every so often when the packet was delayed until the next heartbeat). This kind of design decision is why I do not want to just blindly convert everything to suboptimal if_transmit routines. The current ifq based if_start model is better for 10/100 single-queue drivers than this. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5330876.Sb1U9Iz8Cz>