Re: [PATCH net-next 0/2] Improvements to the DSA deferred xmit

From: Vladimir Oltean
Date: Thu Jan 02 2020 - 17:48:17 EST

Hi David,

On Thu, 2 Jan 2020 at 23:49, David Miller <davem@xxxxxxxxxxxxx> wrote:
> Two comments about this patch series, I think it needs more work:

Thanks for looking at this series.

> 1) This adds the thread and the xmit queue but not code that actually
> uses it. You really have to provide the support code in the driver
> at the same time you add the new facitlity so we can actually see
> how it'll be used.

There is no API change here. There was, and still is, a single caller
of dsa_defer_xmit in the kernel, from net/dsa/tag_sja1105.c:

if (unlikely(sja1105_is_link_local(skb)))
return dsa_defer_xmit(skb, netdev);

The whole difference is that what used to be a schedule_work() in that
function is now a kthread_queue_work() call.

> 2) Patch #1 talks about a tradeoff. Replacing the CB initialization of
> the field skb_get(). But this skb_get() is an atomic operation and
> thus much more expensive for users of the deferred xmit scheme.

Ok, I'll admit I hadn't considered the exact penalty introduced by
skb_get, but I think it is a matter of proportions.
Worst case I expect no more than 64 packets per second to be
transmitted using the deferred xmit mechanism: there are 4 switch
ports, PTP runs with a sync interval of 1/8 seconds, and the STP hello
timer is 2 seconds. So, not a lot of traffic.
On the other hand, clearing the deferred_xmit bool from the skb->cb is
something that everybody else (including this driver for "normal"
traffic) needs to do at line rate, just for the above 64 packets per
second (in the worst case) to be possible.

> Thanks.