From: Denis Kenzior <denkenz@gmail.com>
To: James Prestwood <prestwoj@gmail.com>, iwd@lists.linux.dev
Subject: Re: [PATCH 1/9] dpp: prep for moving AAD within dpp_append_wrapped_data
Date: Mon, 15 Apr 2024 14:00:58 -0500 [thread overview]
Message-ID: <e7b50887-b02a-4e5f-beb3-34e6b2a6a532@gmail.com> (raw)
In-Reply-To: <a8fddf49-da5d-4fc8-9084-34b2ed40cd2c@gmail.com>
Hi James,
On 4/15/24 09:02, James Prestwood wrote:
> Hi Denis,
>
> On 4/2/24 8:10 AM, Denis Kenzior wrote:
>> Hi James,
>>
>> On 3/27/24 10:19, James Prestwood wrote:
>>> The AAD pointers for DPP are specific to the frame type. This is
>>> currently sorted out by the caller within the respective frame
>>> building functions but its quite unreadable. There are some comments
>>> but lots of magic numbers. This should be moved within the
>>> dpp_append_wrapped_data utility but the first step is to make the
>>> frame buffer continuous. This will allow the entire frame to be
>>
>> continuous -> contiguous?
>>
>>> passed and dpp_append_wrapped_data can calculate the AAD offsets
>>> itself.
>>
>> I'm a bit confused as to why? Whether we have an iov or a contiguous buffer,
>> the end result is the same, no? If you want to omit parts of the header from
>> the wrapped data calculation, wouldn't you just break up the header into
>> multiple iovs instead?
>>
> I did this to prep for patch 2, which uses buffer offsets and the frame type to
> calculate the AAD data. Its true the AAD data is always broken up into header
> and payload but I didn't want to use iov's because we would then have to assume
> iov[0] is the header and iov[1] is the payload. A single buffer makes it a lot
TLDR: Using iovs this way was actually the intent.
Long time ago Andrew even had a set of patches that implemented / formalized
using iovs for the various layers some time ago as well. I don't remember the
exact details now, but the idea was to be able to pass iovs around and share
buffers without needing to copy data around. I think the implementation had
some problems, so it didn't go in.
> clearer: we can minimally parse the frame, find the type, and calculate AAD
> offsets. With iov's we would have to check iov[0] for the type, then use some
> offset in iov[1] for the second AAD chunk. This then requires the iov's be
> exactly what we expect, which isn't a great API.
Agreed, iovs are not ideal. Long term I would actually like us to implement
some sort of sk_buff equivalent in ell. It would abstract some of this and make
it easier to manage head rooms / tail rooms for the various transports we use.
>
> Yes, passing the entire frame also requires the buffer is exactly what we expect
> but documenting "the frame must start at the action byte" is a lot better than
> "iov[0] must be the header starting at the action byte" and "iov[1] must be the
> DPP attributes".
I'm not sure that having offset calculation is really any better, particularly
given that you have to make some rather invasive changes first.
Regards,
-Denis
prev parent reply other threads:[~2024-04-15 19:01 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-27 15:19 [PATCH 1/9] dpp: prep for moving AAD within dpp_append_wrapped_data James Prestwood
2024-03-27 15:19 ` [PATCH 2/9] dpp-util: move AAD logic within dpp_append_wrapped_attributes James Prestwood
2024-04-02 15:19 ` Denis Kenzior
2024-03-27 15:19 ` [PATCH 3/9] dpp-util: add dpp_append_point James Prestwood
2024-03-27 15:19 ` [PATCH 4/9] dpp: use dpp_append_point James Prestwood
2024-03-27 15:19 ` [PATCH 5/9] dpp-common: Skeleton for common DPP module James Prestwood
2024-03-27 15:19 ` [PATCH 6/9] dpp-common: add TX/RX handlers to dpp_sm James Prestwood
2024-04-02 15:27 ` Denis Kenzior
2024-04-15 14:05 ` James Prestwood
2024-03-27 15:19 ` [PATCH 7/9] build: add dpp-common.{c,h} James Prestwood
2024-03-27 15:19 ` [PATCH 8/9] dpp: remove most crypto/frame processing James Prestwood
2024-03-27 15:19 ` [PATCH 9/9] dpp: use common state machine James Prestwood
2024-04-02 15:10 ` [PATCH 1/9] dpp: prep for moving AAD within dpp_append_wrapped_data Denis Kenzior
2024-04-15 14:02 ` James Prestwood
2024-04-15 19:00 ` Denis Kenzior [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e7b50887-b02a-4e5f-beb3-34e6b2a6a532@gmail.com \
--to=denkenz@gmail.com \
--cc=iwd@lists.linux.dev \
--cc=prestwoj@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).