Hi Marc, On Tue. 9 Mar 2021 at 21:57, Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote: > > On 09.03.2021 21:45:58, Vincent MAILHOL wrote: > > > On 3/8/21 5:34 PM, Vincent Mailhol wrote: > > > > 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 <arunachalam.santhanam@xxxxxxxxxxxx> > > > > Signed-off-by: Arunachalam Santhanam <arunachalam.santhanam@xxxxxxxxxxxx> > > > > Signed-off-by: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx> > > > > > > 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. > > > 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 <therbert@xxxxxxxxxx> > - Eric Dumazet <eric.dumazet@xxxxxxxxx> > 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
