Thread (10 messages) 10 messages, 4 authors, 2021-06-29

Re: [PATCH v2] net: sched: Add support for packet bursting.

From: Toke Høiland-Jørgensen <hidden>
Date: 2021-06-29 14:35:26

Niclas Hedam [off-list ref] writes:
Thanks for the valuable thoughts, Toke.

The patch started with me being tasked to try and mitigate timing
attacks caused by network latencies. I scouted over the current
network stack and didn't find anything that fully matched my use-case.
While I now understand that you can actually leverage the slots
functionality for this, I would still opt for a new interface and
implementation.
So what is the actual use case for this? If this is something you're
actually planning to deploy this in production, I'm not sure netem is
the best place for it...
I have not done any CPU benchmarks on the slots system, so I'm not
approaching this from the practical performance side per se.

Instead, I argue for seperation with reference to the Seperation of
Concern design principle. The slots functionality is not
built/designed to cater security guarantees, and my patch is not built
to cater duty cycles, etc.
Separation of concerns is all well and good, but you're still adding
this to an existing qdisc (and one mostly used for emulating networks,
at that), in a way that will silently disable most of the other
functionality. I.e., if the 'bursting' field is set, 'rate' and
'latency' will just silently stop working because you're skipping the
code path that uses those.

So you'll need to reject invalid combinations at configure time, which
AFAICT is any other feature combined with bursting. Also, please add an
unlikely() around the check for the bursting parameter to hint the
compiler that this is not the most commonly used feature.
If we opt to merge these two functionalities or discard mine, we have
to implement some guarantee that the slots functionality won't become
significantly slower or complex, which in my opinion is less
maintainable than two similar systems. Also, this patch is very
limited in lines of code, so maintaining it is pretty trivial.
Maintenance is not just about lines of code, it is also about
combination of features (e.g., dealing with things like "my rate limiter
stopped working after I turned on this 'security' feature"). At the very
least you'll need to clearly document interactions (and refuse invalid
combinations as mentioned above).

-Toke
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help