From: Felix Fietkau <nbd@openwrt.org> To: Peter Oh <poh@codeaurora.org>, Peter Oh <poh@qca.qualcomm.com>, ath10k@lists.infradead.org Cc: linux-wireless@vger.kernel.org Subject: Re: [PATCH] ath10k: add modparam 'hw_csum' to make HW checksum configurable Date: Wed, 16 Dec 2015 21:53:55 +0100 [thread overview] Message-ID: <5671CF63.9040308@openwrt.org> (raw) In-Reply-To: <5671CDBD.5060006@codeaurora.org> 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@qca.qualcomm.com> >>>> 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. > 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? How is the user supposed to know when to change that module parameter? Trial and error? - Felix
WARNING: multiple messages have this Message-ID (diff)
From: Felix Fietkau <nbd@openwrt.org> To: Peter Oh <poh@codeaurora.org>, Peter Oh <poh@qca.qualcomm.com>, ath10k@lists.infradead.org Cc: linux-wireless@vger.kernel.org Subject: Re: [PATCH] ath10k: add modparam 'hw_csum' to make HW checksum configurable Date: Wed, 16 Dec 2015 21:53:55 +0100 [thread overview] Message-ID: <5671CF63.9040308@openwrt.org> (raw) In-Reply-To: <5671CDBD.5060006@codeaurora.org> 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@qca.qualcomm.com> >>>> 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. > 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? How is the user supposed to know when to change that module parameter? Trial and error? - Felix _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
next prev parent reply other threads:[~2015-12-16 20:54 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-12-16 18:20 [PATCH] ath10k: add modparam 'hw_csum' to make HW checksum configurable Peter Oh 2015-12-16 18:20 ` Peter Oh 2015-12-16 18:27 ` Felix Fietkau 2015-12-16 18:27 ` Felix Fietkau 2015-12-16 20:29 ` Peter Oh 2015-12-16 20:29 ` Peter Oh 2015-12-16 20:35 ` Felix Fietkau 2015-12-16 20:35 ` Felix Fietkau 2015-12-16 20:46 ` Peter Oh 2015-12-16 20:46 ` Peter Oh 2015-12-16 20:53 ` Felix Fietkau [this message] 2015-12-16 20:53 ` Felix Fietkau 2015-12-16 21:19 ` Peter Oh 2015-12-16 21:19 ` Peter Oh 2015-12-16 21:54 ` Felix Fietkau 2015-12-16 21:54 ` Felix Fietkau 2015-12-16 23:50 ` Peter Oh 2015-12-16 23:50 ` Peter Oh 2015-12-16 23:59 ` Felix Fietkau 2015-12-16 23:59 ` Felix Fietkau 2015-12-17 22:01 ` Peter Oh 2015-12-17 22:01 ` Peter Oh 2015-12-17 22:57 ` Felix Fietkau 2015-12-17 22:57 ` Felix Fietkau 2015-12-17 23:16 ` Peter Oh 2015-12-17 23:16 ` Peter Oh 2015-12-17 7:29 ` Michal Kazior 2015-12-17 7:29 ` Michal Kazior 2015-12-17 21:55 ` Peter Oh 2015-12-17 21:55 ` Peter Oh
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=5671CF63.9040308@openwrt.org \ --to=nbd@openwrt.org \ --cc=ath10k@lists.infradead.org \ --cc=linux-wireless@vger.kernel.org \ --cc=poh@codeaurora.org \ --cc=poh@qca.qualcomm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.