On 12/16/2015 01:54 PM, Felix Fietkau wrote:
On 2015-12-16 22:19, Peter Oh wrote:
On 12/16/2015 12:53 PM, Felix Fietkau wrote:
On 2015-12-16 21:46, Peter Oh wrote:
On 12/16/2015 12:35 PM, Felix Fietkau wrote:
On 2015-12-16 21:29, Peter Oh wrote:
On 12/16/2015 10:27 AM, Felix Fietkau wrote:
On 2015-12-16 19:20, Peter Oh wrote:
Some hardwares such as QCA988X and QCA99X0 doesn't have
capability of checksum offload when frame formats are not
suitable for it such as Mesh frame.
Hence add a module parameter, hw_csum, to make checksum offload
configurable during module registration time.
Signed-off-by: Peter Oh <poh@xxxxxxxxxxxxxxxx>
How about instead of inventing yet another crappy module parameter, you
call skb_checksum_help() in the driver in cases where the hardware is
unable to offload the checksum calculation.
That way the user has to worry about less driver specific hackery ;)
That will be good option for hardware not supporting HW checksum, but I
mind that using the function will add more workload per every packet on
critical data path when HW supports checksum resulting in throughput down.
I didn't mean calling it for every single frame in the data path.
What I'm suggesting is calling it selectively only for mesh frames, or
any other frames that the hardware cannot offload, and leaving the rest
for the hardware to process.
There should be no performance difference between disabling checksum
offload and calling skb_checksum_help from the driver.
To call it selectively for Mesh frame or interface, we need to add it on
mac80211 layer such as ieee80211_build_hdr() since driver layer does not
care the interface type in data path.
No need to change mac80211 - it only touches the headers, and
skb_checksum_help does not care about that. The skb has enough
information for it to find the right range to calculate the checksum and
the place to store it.
If mentioned to use the function to mesh frame only without touching
mac80211, then how do you suggest it to apply it only to mesh frame
without interfere other data frames?
Can you share your example?
It's trivial - in ath10k_tx you do this:
if (vif->type == NL80211_IFTYPE_MESH_POINT &&
skb->ip_summed == CHECKSUM_PARTIAL)
skb_checksum_help(skb);
Thank you Felix for the quick response.
I agree on your user experience opinion,
but what do you think when ath10k has a new chip supporting HW checksum
for Mesh?
In that case it will also introduce throughput degrade to HW that
supports HW checksum for Mesh.
This doesn't make any sense to me. Are you saying that there's no way
for the driver to detect the cases where the hardware cannot do checksum
offloading?
I'm saying the case that HW supports checksum except for specific frame
such as Mesh and to make driver support both case dynamically at code
level, it requires extra codes which need to check if the frame is Mesh
or not. Since this approach requires extra workload especially in data
path, it will degrade driver's performance.
The check is cheap enough that it will not have any visible impact. And
the improved user experience is certainly worth it ;)
- Felix
Thanks,
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
