* 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 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
* 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
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