<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">
<br class="">
<div><br class="">
<blockquote type="cite" class="">
<div class="">On May 29, 2019, at 12:14 PM, Jakub Kicinski <<a href="mailto:jakub.kicinski@netronome.com" class="">jakub.kicinski@netronome.com</a>> wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="">On Wed, 29 May 2019 17:06:49 +0000, Patel, Vedang wrote:<br class="">
<blockquote type="cite" class="">
<blockquote type="cite" class="">On May 28, 2019, at 3:45 PM, Jakub Kicinski <<a href="mailto:jakub.kicinski@netronome.com" class="">jakub.kicinski@netronome.com</a>> wrote:<br class="">
On Tue, 28 May 2019 10:46:44 -0700, Vedang Patel wrote: <br class="">
<blockquote type="cite" class="">From: Vinicius Costa Gomes <<a href="mailto:vinicius.gomes@intel.com" class="">vinicius.gomes@intel.com</a>><br class="">
<br class="">
This adds the UAPI and the core bits necessary for userspace to<br class="">
request hardware offloading to be enabled.<br class="">
<br class="">
The future commits will enable hybrid or full offloading for taprio. This<br class="">
commit sets up the infrastructure to enable it via the netlink interface.<br class="">
<br class="">
Signed-off-by: Vinicius Costa Gomes <<a href="mailto:vinicius.gomes@intel.com" class="">vinicius.gomes@intel.com</a>><br class="">
Signed-off-by: Vedang Patel <<a href="mailto:vedang.patel@intel.com" class="">vedang.patel@intel.com</a>> <br class="">
</blockquote>
<br class="">
Other qdiscs offload by default, this offload-level selection here is a<br class="">
little bit inconsistent with that :(<br class="">
<br class="">
</blockquote>
There are 2 different offload modes introduced in this patch.<br class="">
<br class="">
1. Txtime offload mode: This mode depends on skip_sock_check flag being set in the etf qdisc. Also, it requires some manual configuration which might be specific to the network adapter card. For example, for the i210 card, the user will have to route all the
traffic to the highest priority queue and install etf qdisc with offload enabled on that queue. So, I don’t think this mode should be enabled by default.<br class="">
</blockquote>
<br class="">
Excellent, it looks like there will be driver patches necessary for<br class="">
this offload to function, also it seems your offload enable function<br class="">
still contains this after the series:<br class="">
<br class="">
<span class="Apple-tab-span" style="white-space:pre"></span>/* FIXME: enable offloading */<br class="">
<br class="">
Please only post offload infrastructure when fully fleshed out and with<br class="">
driver patches making use of it.<br class="">
<br class="">
</div>
</div>
</blockquote>
The above comment refers to the full offload mode described below. It is not needed for txtime offload mode. Txtime offload mode is essentially setting the transmit time for each packet depending on what interval it is going to be transmitted instead of relying
on the hrtimers to open gates and send packets. More details about why exactly this is done is mentioned in patch #5[1] of this series.</div>
<div><br class="">
</div>
<div>What we can do is just add the txtime offload related flag and add the full offload flag whenever the driver bits are ready. Does that address your concern?</div>
<div><br class="">
</div>
<div>Thanks,</div>
<div>Vedang</div>
<div><br class="">
</div>
<div>[1] - <a href="https://lore.kernel.org/netdev/1559065608-27888-1-git-send-email-vedang.patel@intel.com/T/#mcbcd70a02eb3aea0622bf2a41e492398353b6923" class="">https://lore.kernel.org/netdev/1559065608-27888-1-git-send-email-vedang.patel@intel.com/T/#mcbcd70a02eb3aea0622bf2a41e492398353b6923</a><br class="">
<blockquote type="cite" class="">
<div class="">
<div class="">
<blockquote type="cite" class="">2. Full offload mode: This mode is currently not supported by any network driver. The support for this will be coming soon. But, we can enable this mode by default.
<br class="">
<br class="">
Also, from what Vinicius tells me, offload modes for cbs, etf and mqprio are also disabled by default. So, it will make more sense to keep it disabled to be consistent with other qdiscs similar to this one.<br class="">
</blockquote>
</div>
</div>
</blockquote>
</div>
<br class="">
</body>
</html>