netdev - Re: [PATCH iproute2-next v2] ss: Shorter display format for TLS zerocopy sendfile

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]

Date:   Tue, 7 Jun 2022 13:35:19 +0300
From:   Maxim Mikityanskiy <maximmi@...dia.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     dsahern@...il.com, netdev@...r.kernel.org,
        stephen@...workplumber.org, tariqt@...dia.com
Subject: Re: [PATCH iproute2-next v2] ss: Shorter display format for TLS
 zerocopy sendfile

On 2022-06-06 20:59, Jakub Kicinski wrote:
> On Mon, 6 Jun 2022 14:29:02 +0300 Maxim Mikityanskiy wrote:
>>> The difference is that the person writing the code (who will interact
>>> with kernel defines) is likely to have a deeper understanding of the
>>> technology and have read the doc. My concern is that an ss user will
>>> have much more superficial understanding of the internals so we need
>>> to be more careful to present the information in the most meaningful
>>> way.
>>>
>>> E.g. see the patch for changing dev->operstate to UP from UNKNOWN
>>> because users are "confused". If you just call the thing "zc is enabled"
>>> I'm afraid users will start reporting that the "go fast mode" is not
>>> engaged as a bug, without appreciation for the possible side effects.
>>
>> That makes some sense to me. What about calling the ss flag
>> "zc_sendfile_ro" or "zc_ro_sendfile"? It will still be clear it's
>> zerocopy, but with some nuance.
> 
> That'd be an acceptable compromise. Hopefully sufficiently forewarned
> users will mentally remove the zc_ part and still have a meaningful
> amount of info about what the flag does.
> 
> Any reason why we wouldn't reuse the same knob for zc sendmsg()? If we
> plan to reuse it we can s/sendfile/send/ to shorten the name, perhaps.

We can even make it as short as zc_ro_tx in that case.

Regarding sendmsg, I can't anticipate what knob will be used. There is 
MSG_ZEROCOPY which is also a candidate.

Note that the constant in the header file has "SENDFILE" in its name, so 
if you want to reuse it for the future sendmsg zerocopy, we should think 
about renaming it in advance, before anyone starts using it. 
Alternatively, an alias for this constant can be added in the future.

>>> Dunno if it's useful but FWIW I pushed my WIP branch out:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/kuba/linux.git/commit/?h=tls-wip&id=d923f1049a1ae1c2bdc1d8f0081fd9f3a35d4155
>>> https://git.kernel.org/pub/scm/linux/kernel/git/kuba/linux.git/commit/?h=tls-wip&id=b814ee782eef62d6e2602ab3ba7b31ca03cfe44c
>>
>> I took a glance, and I agree zerocopy isn't the best name for your
>> feature. If I wanted to indicate it saves one copy, I would call it
>> "direct decrypt". "Expect no pad" also works from the point of view of
>> declaring limitations.
>>
>> Another topic to consider is whether TLS 1.3 should be part of the name,
>> and should "TlsDecryptRetry" be more specific (if a future feature also
>> retries decryption as a fallback, do we want to count these retries in
>> the same counter or in a new counter?)
> 
> I wanted to avoid the versions because TLS 1.4 may need the same
> optimization.
> 
> You have a point about the more specific counter, let me add a counter
> for NoPad being violated (tail == 0) as well as the overall "decryption
> happened twice" counter.