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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+h21horyGwqBTyBSVDRSSOSAPr_3i1dvz40=qKQMD_Nddtk3Q@mail.gmail.com>
Date:   Fri, 3 Jan 2020 00:47:57 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     David Miller <davem@...emloft.net>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Andrew Lunn <andrew@...n.ch>, netdev <netdev@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 0/2] Improvements to the DSA deferred xmit

Hi David,

On Thu, 2 Jan 2020 at 23:49, David Miller <davem@...emloft.net> 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.

Regards,
-Vladimir