On Fri, Sep 9, 2016 at 5:01 PM, John Fastabend <john.fastab...@gmail.com> wrote: > On 16-09-09 04:44 PM, Tom Herbert wrote: >> On Fri, Sep 9, 2016 at 2:29 PM, John Fastabend <john.fastab...@gmail.com> >> wrote: >>> e1000 supports a single TX queue so it is being shared with the stack >>> when XDP runs XDP_TX action. This requires taking the xmit lock to >>> ensure we don't corrupt the tx ring. To avoid taking and dropping the >>> lock per packet this patch adds a bundling implementation to submit >>> a bundle of packets to the xmit routine. >>> >>> I tested this patch running e1000 in a VM using KVM over a tap >>> device using pktgen to generate traffic along with 'ping -f -l 100'. >>> >> Hi John, >> >> How does this interact with BQL on e1000? >> >> Tom >> > > Let me check if I have the API correct. When we enqueue a packet to > be sent we must issue a netdev_sent_queue() call and then on actual > transmission issue a netdev_completed_queue(). > > The patch attached here missed a few things though. > > But it looks like I just need to call netdev_sent_queue() from the > e1000_xmit_raw_frame() routine and then let the tx completion logic > kick in which will call netdev_completed_queue() correctly. > > I'll need to add a check for the queue state as well. So if I do these > three things, > > check __QUEUE_STATE_XOFF before sending > netdev_sent_queue() -> on XDP_TX > netdev_completed_queue() > > It should work agree? Now should we do this even when XDP owns the > queue? Or is this purely an issue with sharing the queue between > XDP and stack. > But what is the action for XDP_TX if the queue is stopped? There is no qdisc to back pressure in the XDP path. Would we just start dropping packets then?
Tom > .John >