Hi Marc,

On Tue. 9 Mar 2021 at 21:57, Marc Kleine-Budde [off-list ref] wrote:

On 09.03.2021 21:45:58, Vincent MAILHOL wrote:

quoted

quoted

On 3/8/21 5:34 PM, Vincent Mailhol wrote:

quoted

This driver supports the ES581.4, ES582.1 and ES584.1 interfaces from
ETAS GmbH (https://www.etas.com/en/products/es58x.php).

Co-developed-by: Arunachalam Santhanam <redacted>
Signed-off-by: Arunachalam Santhanam <redacted>
Signed-off-by: Vincent Mailhol <redacted>

I'm not sure if you're supposed to change dql.min_limit from the driver.

One thing for sure, I am the only one to do it.

The reason to do so is because benchmarks show me that values
below this threshold are not good for this device (and I try to
be very permissive on the values).

USB introduces a lot of latency and the small PDU of CAN does not
help. The BQL is here to remediate, however, the algorithms can
take time to adjust, especially if there are small bursts.
Modifying the dql.min_limit was the only solution I found to make
sure that packets can be sent in bulk even during small burst
events.

The BQL was not designed for USB nor was it designed for CAN
which probably explains why I am the first one to ever have
thought of using dql.min_limit like this.

We can try to sneak it into the kernel or ask on the net-dev list for a
proper interface[1] for setting sensible defaults to the min_limit.

quoted

Using dql.min_limit is a hack and I pledge guilty for it. However,
because this hack brings performance improvement, I would like to keep
it if you do not mind.

Your explanation is very good and clear, what about sending new mail
with this problem description to the netdev list? A proper solution
might be something like dql_set_min_limit() with a static inline no-op
of BQL is disabled.

Looking at 114cf5802165 ("bql: Byte queue limits"), you want to include:
- Tom Herbert [off-list ref]
- Eric Dumazet [off-list ref]

Sounds good to me. I will prepare a patch to explain the issue
and try to introduce the dql_set_min_limit() function.

Meanwhile, I would be thankful if you could continue the review :)


Yours sincerely,
Vincent