On 12/12/18(Wed) 12:03, David Gwynne wrote:
> with the previous if_ethersubr.c diff, this allows etherip(4) to output
> directly to the network stack.
What do you mean with "directly"?
To my understanding ip{,6}_etherip_output() call ip{,6}_send() to enqueue
packets.
> direct output relies on the interface using priq, since hfsc uses the
> ifq machinery to work. priq implies you dont want to delay packets, so
> it lets etherip push the packet straight through.
So "direct output" means skip the queue(s)?
So are we heading towards a design where ifq_enqueue(9) is going to be
bypassed for pseudo-drivers unless people use HFSC? If that's the case
I'm afraid that code path will rot.
> i dont think the ifq stuff is slow, but it does allow the stack to
> concurrently send packets with etherip without having to create an ifq
> per cpu. it does rely on per cpu counters, but theyre relatively small.
I don't understand how it allows the stack to concurrently send packets.
All packets seems to be serialized in a single task when ip{,6}_send()
is called.
> the other advantage is it stops ether_output recursing in the etherip
> path. this helps profiling, but that is a fairly niche benefit so maybe
> just focus on the concurrency.
I don't understand. How is ether_output() recursing?
> ok?
I'm a bit sad about where this is going. Because the design you adopted
will forces us to modify all pseudo-drivers. Apart the fact that you're
now breaking etherip(4) into bridge(4) /!\, I believe we can do better.
We had this design:
STACK DRIVER
ether_output() -> if_enqueue() -> ifq_enqueue() + ifq_start() | drv_start()
`. |
`--> bridge_output() |
The problem that you want to solve, if I'm not mistaken, is to remove
the useless stack/driver separation for pseudo-drivers. In other words
stop queueing & delaying packets. This only makes sense for "physical"
nics where mbufs are put into DMA buckets, right?
In that case I'd suggest modifying if_enqueue() or ifq_enqueue() to treat
IFXF_CLONED devices as special if they are not using HSFC.
In other words I suggest you only need to modify what happens *after*
if_enqueue(), the driver part or actual drv_start(). Why about having
a generic:
if (ifp->if_enqueue != NULL && (ifq_is_priq(&ifp->if_snd)) {
counters_pkt(ifp->if_counters, ifc_opackets, ifc_obytes,
m->m_pkthdr.len);
ifp->if_enqueue(ifp, m);
return (0);
}
I'm not sure if we can do better than introducing a new per-ifp method.
This is what you called etherip_send() in your diff. But I believe the
vlan(4) example is more interesting because if calls if_enqueue(9) directly!
So I disagree with the changes requiring a custom `if_output' and with the
bypassing of if_enqueue(). That said I could be completely mistaken :)
