* Question about TPA/HDS feature of bnxt_en @ 2024-08-13 10:42 Taehee Yoo 2024-08-14 1:17 ` Jakub Kicinski 0 siblings, 1 reply; 7+ messages in thread From: Taehee Yoo @ 2024-08-13 10:42 UTC (permalink / raw) To: Michael Chan, David Wei, Somnath Kotur; +Cc: Mina Almasry, Netdev Hi, I'm currently testing the device memory TCP feature with the bnxt_en driver because Broadcom NICs support TPA/HDS, which is a mandatory feature for the devmem TCP. But it doesn't work for short-sized packets(under 300?) So, the devmem TCP stops or errors out if it receives non-header-splitted skb. I hope the bnxt_en driver or firmware has options that force TPA to work for short-sized packets. So, Can I get any condition information on TPA? Thanks a lot! Taehee Yoo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Question about TPA/HDS feature of bnxt_en 2024-08-13 10:42 Question about TPA/HDS feature of bnxt_en Taehee Yoo @ 2024-08-14 1:17 ` Jakub Kicinski 2024-08-14 2:08 ` Michael Chan 2024-08-14 7:27 ` Taehee Yoo 0 siblings, 2 replies; 7+ messages in thread From: Jakub Kicinski @ 2024-08-14 1:17 UTC (permalink / raw) To: Taehee Yoo; +Cc: Michael Chan, David Wei, Somnath Kotur, Mina Almasry, Netdev On Tue, 13 Aug 2024 19:42:51 +0900 Taehee Yoo wrote: > Hi, > I'm currently testing the device memory TCP feature with the bnxt_en > driver because Broadcom NICs support TPA/HDS, which is a mandatory > feature for the devmem TCP. > But it doesn't work for short-sized packets(under 300?) > So, the devmem TCP stops or errors out if it receives non-header-splitted skb. > > I hope the bnxt_en driver or firmware has options that force TPA to > work for short-sized packets. > So, Can I get any condition information on TPA? I don't have any non-public info but look around the driver for rx_copy_thresh, it seems to be sent to FW. I wonder if setting it to 1 or 0 would work. Especially this: static int bnxt_hwrm_vnic_set_hds(struct bnxt *bp, struct bnxt_vnic_info *vnic) ... req->hds_threshold = cpu_to_le16(bp->rx_copy_thresh); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Question about TPA/HDS feature of bnxt_en 2024-08-14 1:17 ` Jakub Kicinski @ 2024-08-14 2:08 ` Michael Chan 2024-08-14 7:51 ` Taehee Yoo 2024-08-14 7:27 ` Taehee Yoo 1 sibling, 1 reply; 7+ messages in thread From: Michael Chan @ 2024-08-14 2:08 UTC (permalink / raw) To: Jakub Kicinski; +Cc: Taehee Yoo, David Wei, Somnath Kotur, Mina Almasry, Netdev [-- Attachment #1: Type: text/plain, Size: 1132 bytes --] On Tue, Aug 13, 2024 at 6:17 PM Jakub Kicinski <kuba@kernel•org> wrote: > > On Tue, 13 Aug 2024 19:42:51 +0900 Taehee Yoo wrote: > > Hi, > > I'm currently testing the device memory TCP feature with the bnxt_en > > driver because Broadcom NICs support TPA/HDS, which is a mandatory > > feature for the devmem TCP. > > But it doesn't work for short-sized packets(under 300?) > > So, the devmem TCP stops or errors out if it receives non-header-splitted skb. > > > > I hope the bnxt_en driver or firmware has options that force TPA to > > work for short-sized packets. > > So, Can I get any condition information on TPA? > > I don't have any non-public info but look around the driver for > rx_copy_thresh, it seems to be sent to FW. > Yes, the rx_copy_thresh is also the HDS threshold. The default value is 256, meaning that packet sizes below 256 will not be split. So a 300-byte packet should be split. TPA is related but is separate. There is a min_agg_len that is currently set to 512 in bnxt_hwrm_vnic_set_tpa(). I think it should be fine to reduce this value for TPA to work on smaller packets. [-- Attachment #2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 4209 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Question about TPA/HDS feature of bnxt_en 2024-08-14 2:08 ` Michael Chan @ 2024-08-14 7:51 ` Taehee Yoo 2024-08-14 17:17 ` Michael Chan 0 siblings, 1 reply; 7+ messages in thread From: Taehee Yoo @ 2024-08-14 7:51 UTC (permalink / raw) To: Michael Chan Cc: Jakub Kicinski, David Wei, Somnath Kotur, Mina Almasry, Netdev On Wed, Aug 14, 2024 at 11:08 AM Michael Chan <michael.chan@broadcom•com> wrote: > Hi Michael, > On Tue, Aug 13, 2024 at 6:17 PM Jakub Kicinski <kuba@kernel•org> wrote: > > > > On Tue, 13 Aug 2024 19:42:51 +0900 Taehee Yoo wrote: > > > Hi, > > > I'm currently testing the device memory TCP feature with the bnxt_en > > > driver because Broadcom NICs support TPA/HDS, which is a mandatory > > > feature for the devmem TCP. > > > But it doesn't work for short-sized packets(under 300?) > > > So, the devmem TCP stops or errors out if it receives non-header-splitted skb. > > > > > > I hope the bnxt_en driver or firmware has options that force TPA to > > > work for short-sized packets. > > > So, Can I get any condition information on TPA? > > > > I don't have any non-public info but look around the driver for > > rx_copy_thresh, it seems to be sent to FW. > > > > Yes, the rx_copy_thresh is also the HDS threshold. The default value > is 256, meaning that packet sizes below 256 will not be split. So a > 300-byte packet should be split. > > TPA is related but is separate. There is a min_agg_len that is > currently set to 512 in bnxt_hwrm_vnic_set_tpa(). I think it should > be fine to reduce this value for TPA to work on smaller packets. Thank you so much for confirming the hds_threshold variable. I tested this variable, it worked as I expected. For testing, I kept the rx_copy_threshold and tpa settings unchanged, but modified the hds_threshold variable to 0. BTW, how about separating rx_copy_threshold into rx-copy-break and hds_threshold? If so, we can implement `ethtool --set-tunable eth0 rx-copybreak N` and `ethtool -G eth0 tcp-data-split on` Thank you so much again, Taehee Yoo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Question about TPA/HDS feature of bnxt_en 2024-08-14 7:51 ` Taehee Yoo @ 2024-08-14 17:17 ` Michael Chan 2024-08-15 9:06 ` Taehee Yoo 0 siblings, 1 reply; 7+ messages in thread From: Michael Chan @ 2024-08-14 17:17 UTC (permalink / raw) To: Taehee Yoo; +Cc: Jakub Kicinski, David Wei, Somnath Kotur, Mina Almasry, Netdev [-- Attachment #1: Type: text/plain, Size: 1180 bytes --] On Wed, Aug 14, 2024 at 12:51 AM Taehee Yoo <ap420073@gmail•com> wrote: > > On Wed, Aug 14, 2024 at 11:08 AM Michael Chan <michael.chan@broadcom•com> wrote: > > Yes, the rx_copy_thresh is also the HDS threshold. The default value > > is 256, meaning that packet sizes below 256 will not be split. So a > > 300-byte packet should be split. > > > > TPA is related but is separate. There is a min_agg_len that is > > currently set to 512 in bnxt_hwrm_vnic_set_tpa(). I think it should > > be fine to reduce this value for TPA to work on smaller packets. > > Thank you so much for confirming the hds_threshold variable. > I tested this variable, it worked as I expected. > For testing, I kept the rx_copy_threshold and tpa settings unchanged, > but modified the hds_threshold variable to 0. > > BTW, how about separating rx_copy_threshold into rx-copy-break > and hds_threshold? > If so, we can implement `ethtool --set-tunable eth0 rx-copybreak N` > and `ethtool -G eth0 tcp-data-split on` > Yes, we can do that. I have the rx-copybreak tunable implemented in the OOT driver already and can be sent upstream. tcp_data_split can also be added. Thanks. [-- Attachment #2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 4209 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Question about TPA/HDS feature of bnxt_en 2024-08-14 17:17 ` Michael Chan @ 2024-08-15 9:06 ` Taehee Yoo 0 siblings, 0 replies; 7+ messages in thread From: Taehee Yoo @ 2024-08-15 9:06 UTC (permalink / raw) To: Michael Chan Cc: Jakub Kicinski, David Wei, Somnath Kotur, Mina Almasry, Netdev On Thu, Aug 15, 2024 at 2:18 AM Michael Chan <michael.chan@broadcom•com> wrote: > Hi Michael > On Wed, Aug 14, 2024 at 12:51 AM Taehee Yoo <ap420073@gmail•com> wrote: > > > > On Wed, Aug 14, 2024 at 11:08 AM Michael Chan <michael.chan@broadcom•com> wrote: > > > Yes, the rx_copy_thresh is also the HDS threshold. The default value > > > is 256, meaning that packet sizes below 256 will not be split. So a > > > 300-byte packet should be split. > > > > > > TPA is related but is separate. There is a min_agg_len that is > > > currently set to 512 in bnxt_hwrm_vnic_set_tpa(). I think it should > > > be fine to reduce this value for TPA to work on smaller packets. > > > > Thank you so much for confirming the hds_threshold variable. > > I tested this variable, it worked as I expected. > > For testing, I kept the rx_copy_threshold and tpa settings unchanged, > > but modified the hds_threshold variable to 0. > > > > BTW, how about separating rx_copy_threshold into rx-copy-break > > and hds_threshold? > > If so, we can implement `ethtool --set-tunable eth0 rx-copybreak N` > > and `ethtool -G eth0 tcp-data-split on` > > > Yes, we can do that. I have the rx-copybreak tunable implemented in > the OOT driver already and can be sent upstream. tcp_data_split can > also be added. Thanks. Thanks a lot for sharing that! Taehee Yoo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Question about TPA/HDS feature of bnxt_en 2024-08-14 1:17 ` Jakub Kicinski 2024-08-14 2:08 ` Michael Chan @ 2024-08-14 7:27 ` Taehee Yoo 1 sibling, 0 replies; 7+ messages in thread From: Taehee Yoo @ 2024-08-14 7:27 UTC (permalink / raw) To: Jakub Kicinski Cc: Michael Chan, David Wei, Somnath Kotur, Mina Almasry, Netdev On Wed, Aug 14, 2024 at 10:17 AM Jakub Kicinski <kuba@kernel•org> wrote: > Hi Jakub, > On Tue, 13 Aug 2024 19:42:51 +0900 Taehee Yoo wrote: > > Hi, > > I'm currently testing the device memory TCP feature with the bnxt_en > > driver because Broadcom NICs support TPA/HDS, which is a mandatory > > feature for the devmem TCP. > > But it doesn't work for short-sized packets(under 300?) > > So, the devmem TCP stops or errors out if it receives non-header-splitted skb. > > > > I hope the bnxt_en driver or firmware has options that force TPA to > > work for short-sized packets. > > So, Can I get any condition information on TPA? > > I don't have any non-public info but look around the driver for > rx_copy_thresh, it seems to be sent to FW. I wonder if setting > it to 1 or 0 would work. Especially this: > > static int bnxt_hwrm_vnic_set_hds(struct bnxt *bp, struct bnxt_vnic_info *vnic) > ... > req->hds_threshold = cpu_to_le16(bp->rx_copy_thresh); Thank you so much for looking into this! As you said, I tested setting hds_threshold to 0, it works well. I think we can implement `ethtool -G eth0 tcp-data-split on` option with this. Thank you so much again, Taehee Yoo ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-08-15 9:07 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-13 10:42 Question about TPA/HDS feature of bnxt_en Taehee Yoo 2024-08-14 1:17 ` Jakub Kicinski 2024-08-14 2:08 ` Michael Chan 2024-08-14 7:51 ` Taehee Yoo 2024-08-14 17:17 ` Michael Chan 2024-08-15 9:06 ` Taehee Yoo 2024-08-14 7:27 ` Taehee Yoo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox