Re: status of bpf_mtap()/bpf_mtap_softint() ?

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

On Tue, May 23, 2017 at 7:33 PM, Manuel Bouyer <bouyer%antioche.eu.org@localhost> wrote:
> On Tue, May 23, 2017 at 11:51:39AM +0900, Ryota Ozaki wrote:
>> On Tue, May 23, 2017 at 12:41 AM, Manuel Bouyer <bouyer%antioche.eu.org@localhost> wrote:
>> > Hello
>> > looking more closely at bpf for CAN, there is something I don't understand.
>> > It looks like bpf_mtap() can't be called from interrupt context any more,
>> > as shown by the KASSERT() in bpf_deliver().
>> > bpf_mtap_softint() was was introduced for this.
>>
>> bpf_mtap needs different treatments for Rx and Tx.
>>
>> For Rx, network device drivers should (1) implement the Rx interrupt handler
>> as sofint or (2) use if_percpuq facility to ensure the interface input
>> routine (traditionally it's ifp->if_input) and bpf_mtap (and upper layers)
>> run in softint. Most drivers need to just use if_percpuq_enqueue instead of
>> ifp->if_input.
>>
>> bpf_mtap_softint was introduced for bpf_mtap that were difficult to run
>> it in softint (e.g., to do so, fundamental changes are needed). Basically
>> we shouldn't use bpf_mtap_softint and should adopt either of the above two
>> approaches.
>>
>> For Tx, bpf_mtap is called in XXX_start that is normally executed in
>> a normal packet transmission path (LWP or softint). However, some drivers
>> try to call XXX_start in the Tx interrupt handler. To avoid such calls,
>> the deferred if_start mechanism was introduced. What we need is just to
>> replace XXX_start called in an interrupt handler with
>> if_schedule_deffered_start.
>
> thanks.
> For CAN this is quite a bit of overhead. CAN controllers usually don't do
> DMA and can handle only one packet at a time. This means one extra softint
> per received packet, and eventually one extra softint per transmitted packet.
>
> bpf_mtap_softint() is probably cheaper because it will schedule an
> extra softint only if there is a bpf listener.
> Can we remove the KASSERT(cpu_intr_p()) in bpf_mtap_softint() ?
> I can't see why it couldn't be called from thread context. Using
> bpf_mtap() or bpf_mtap_softint() depending on context can lead to
> out of order packet delivery at the bpf level.

bpf_mtap_softint intends to be used at a place that unavoidably
runs in hardware interrupt context but needs to do bpf_mtap.
It shouldn't be used at other places.

And of course bpf_mtap and bpf_mtap_softint shouldn't be mixed
on one direction.

  ozaki-r