On Mon, Sep 12, 2016 at 9:25 PM, Tom Herbert <[email protected]> wrote: > On Mon, Sep 12, 2016 at 8:00 PM, Alexander Duyck > <[email protected]> wrote: >> On Mon, Sep 12, 2016 at 3:13 PM, John Fastabend >> <[email protected]> wrote: >>> The BQL API does not reference the sk_buff nor does the driver need to >>> reference the sk_buff to calculate the length of a transmitted frame. >>> This patch removes an sk_buff reference from the xmit irq path and >>> also allows packets sent from XDP to use BQL. >>> >>> Signed-off-by: John Fastabend <[email protected]> >>> --- >>> drivers/net/ethernet/intel/e1000/e1000_main.c | 7 ++----- >>> 1 file changed, 2 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c >>> b/drivers/net/ethernet/intel/e1000/e1000_main.c >>> index f42129d..62a7f8d 100644 >>> --- a/drivers/net/ethernet/intel/e1000/e1000_main.c >>> +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c >>> @@ -3882,11 +3882,8 @@ static bool e1000_clean_tx_irq(struct e1000_adapter >>> *adapter, >>> if (cleaned) { >>> total_tx_packets += buffer_info->segs; >>> total_tx_bytes += buffer_info->bytecount; >>> - if (buffer_info->skb) { >>> - bytes_compl += >>> buffer_info->skb->len; >>> - pkts_compl++; >>> - } >>> - >>> + bytes_compl += buffer_info->length; >>> + pkts_compl++; >>> } >>> e1000_unmap_and_free_tx_resource(adapter, >>> buffer_info); >>> tx_desc->upper.data = 0; >> >> Actually it might be worth looking into why we have two different >> stats for tracking bytecount and segs. From what I can tell the >> pkts_compl value is never actually used. The function doesn't even >> use the value so it is just wasted cycles. And as far as the bytes go > > Transmit flow steering which I posted and is pending on some testing > uses the pkt count BQL to track inflight packets. > >> the accounting would be more accurate if you were to use bytecount >> instead of buffer_info->skb->len. You would just need to update the >> xmit function to use that on the other side so that they match.
Okay, that makes sense. But as I was saying we might be better off using the segs and bytecount values instead of the skb->len in the xmit and cleanup paths to get more accurate accounting for the total bytes/packets coming and going from the interface. That way we can avoid any significant change in behavior between TSO and GSO. - Alex
