diff mbox series

[net-next,08/13] net: dsa: hellcreek: deny tc-taprio changes to per-tc max SDU

Message ID 20220914153303.1792444-9-vladimir.oltean@nxp.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Add tc-taprio support for queueMaxSDU | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 5 this patch: 5
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang fail Errors and warnings before: 5 this patch: 5
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 5 this patch: 5
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 15 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Vladimir Oltean Sept. 14, 2022, 3:32 p.m. UTC
Since the driver does not act upon the max_sdu argument (which it
should, in full offload mode), deny any other values except the default
all-zeroes, which means that all traffic classes should use the same MTU
as the port itself.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/hirschmann/hellcreek.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Kurt Kanzenbach Sept. 14, 2022, 6:13 p.m. UTC | #1
On Wed Sep 14 2022, Vladimir Oltean wrote:
> Since the driver does not act upon the max_sdu argument (which it
> should, in full offload mode), deny any other values except the default
> all-zeroes, which means that all traffic classes should use the same MTU
> as the port itself.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/net/dsa/hirschmann/hellcreek.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
> index ea8bbfce0f1f..8ef7816627b6 100644
> --- a/drivers/net/dsa/hirschmann/hellcreek.c
> +++ b/drivers/net/dsa/hirschmann/hellcreek.c
> @@ -1814,10 +1814,15 @@ static int hellcreek_port_setup_tc(struct dsa_switch *ds, int port,
>  {
>  	struct tc_taprio_qopt_offload *taprio = type_data;
>  	struct hellcreek *hellcreek = ds->priv;
> +	int tc;
>  
>  	if (type != TC_SETUP_QDISC_TAPRIO)
>  		return -EOPNOTSUPP;
>  
> +	for (tc = 0; tc < TC_MAX_QUEUE; tc++)
> +		if (taprio->max_sdu[tc])
> +			return -EOPNOTSUPP;

I'd rather like to see that feature implemented :-). Something like below.

From 3d44683979bf50960125fa3005b1142af61525c7 Mon Sep 17 00:00:00 2001
From: Kurt Kanzenbach <kurt@linutronix.de>
Date: Wed, 14 Sep 2022 19:51:40 +0200
Subject: [PATCH] net: dsa: hellcreek: Offload per-tc max SDU from tc-taprio

Add support for configuring the max SDU per priority and per port. If not
specified, keep the default.

Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
---
 drivers/net/dsa/hirschmann/hellcreek.c | 61 +++++++++++++++++++++++---
 drivers/net/dsa/hirschmann/hellcreek.h |  7 +++
 2 files changed, 61 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 5ceee71d9a25..1b22710e1641 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -128,6 +128,16 @@ static void hellcreek_select_prio(struct hellcreek *hellcreek, int prio)
 	hellcreek_write(hellcreek, val, HR_PSEL);
 }
 
+static void hellcreek_select_port_prio(struct hellcreek *hellcreek, int port,
+				       int prio)
+{
+	u16 val = port << HR_PSEL_PTWSEL_SHIFT;
+
+	val |= prio << HR_PSEL_PRTCWSEL_SHIFT;
+
+	hellcreek_write(hellcreek, val, HR_PSEL);
+}
+
 static void hellcreek_select_counter(struct hellcreek *hellcreek, int counter)
 {
 	u16 val = counter << HR_CSEL_SHIFT;
@@ -1537,6 +1547,42 @@ hellcreek_port_prechangeupper(struct dsa_switch *ds, int port,
 	return ret;
 }
 
+static void hellcreek_setup_maxsdu(struct hellcreek *hellcreek, int port,
+				   const struct tc_taprio_qopt_offload *schedule)
+{
+	int tc;
+
+	for (tc = 0; tc < 8; ++tc) {
+		u16 val;
+
+		if (!schedule->max_sdu[tc])
+			continue;
+
+		hellcreek_select_port_prio(hellcreek, port, tc);
+
+		val = (schedule->max_sdu[tc] & HR_PTPRTCCFG_MAXSDU_MASK)
+			<< HR_PTPRTCCFG_MAXSDU_SHIFT;
+
+		hellcreek_write(hellcreek, val, HR_PTPRTCCFG);
+	}
+}
+
+static void hellcreek_reset_maxsdu(struct hellcreek *hellcreek, int port)
+{
+	int tc;
+
+	for (tc = 0; tc < 8; ++tc) {
+		u16 val;
+
+		hellcreek_select_port_prio(hellcreek, port, tc);
+
+		val = (HELLCREEK_DEFAULT_MAX_SDU & HR_PTPRTCCFG_MAXSDU_MASK)
+			<< HR_PTPRTCCFG_MAXSDU_SHIFT;
+
+		hellcreek_write(hellcreek, val, HR_PTPRTCCFG);
+	}
+}
+
 static void hellcreek_setup_gcl(struct hellcreek *hellcreek, int port,
 				const struct tc_taprio_qopt_offload *schedule)
 {
@@ -1720,7 +1766,10 @@ static int hellcreek_port_set_schedule(struct dsa_switch *ds, int port,
 	}
 	hellcreek_port->current_schedule = taprio_offload_get(taprio);
 
-	/* Then select port */
+	/* Configure max sdu */
+	hellcreek_setup_maxsdu(hellcreek, port, hellcreek_port->current_schedule);
+
+	/* Select tdg */
 	hellcreek_select_tgd(hellcreek, port);
 
 	/* Enable gating and keep defaults */
@@ -1772,7 +1821,10 @@ static int hellcreek_port_del_schedule(struct dsa_switch *ds, int port)
 		hellcreek_port->current_schedule = NULL;
 	}
 
-	/* Then select port */
+	/* Reset max sdu */
+	hellcreek_reset_maxsdu(hellcreek, port);
+
+	/* Select tgd */
 	hellcreek_select_tgd(hellcreek, port);
 
 	/* Disable gating and return to regular switching flow */
@@ -1814,15 +1866,10 @@ static int hellcreek_port_setup_tc(struct dsa_switch *ds, int port,
 {
 	struct tc_taprio_qopt_offload *taprio = type_data;
 	struct hellcreek *hellcreek = ds->priv;
-	int tc;
 
 	if (type != TC_SETUP_QDISC_TAPRIO)
 		return -EOPNOTSUPP;
 
-	for (tc = 0; tc < TC_MAX_QUEUE; tc++)
-		if (taprio->max_sdu[tc])
-			return -EOPNOTSUPP;
-
 	if (!hellcreek_validate_schedule(hellcreek, taprio))
 		return -EOPNOTSUPP;
 
diff --git a/drivers/net/dsa/hirschmann/hellcreek.h b/drivers/net/dsa/hirschmann/hellcreek.h
index c96b8c278904..66b989d946e4 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.h
+++ b/drivers/net/dsa/hirschmann/hellcreek.h
@@ -37,6 +37,7 @@
 #define HELLCREEK_VLAN_UNTAGGED_MEMBER	0x1
 #define HELLCREEK_VLAN_TAGGED_MEMBER	0x3
 #define HELLCREEK_NUM_EGRESS_QUEUES	8
+#define HELLCREEK_DEFAULT_MAX_SDU	1536
 
 /* Register definitions */
 #define HR_MODID_C			(0 * 2)
@@ -72,6 +73,12 @@
 #define HR_PRTCCFG_PCP_TC_MAP_SHIFT	0
 #define HR_PRTCCFG_PCP_TC_MAP_MASK	GENMASK(2, 0)
 
+#define HR_PTPRTCCFG			(0xa9 * 2)
+#define HR_PTPRTCCFG_SET_QTRACK		BIT(15)
+#define HR_PTPRTCCFG_REJECT		BIT(14)
+#define HR_PTPRTCCFG_MAXSDU_SHIFT	0
+#define HR_PTPRTCCFG_MAXSDU_MASK	GENMASK(10, 0)
+
 #define HR_CSEL				(0x8d * 2)
 #define HR_CSEL_SHIFT			0
 #define HR_CSEL_MASK			GENMASK(7, 0)
Vladimir Oltean Sept. 14, 2022, 6:40 p.m. UTC | #2
On Wed, Sep 14, 2022 at 08:13:53PM +0200, Kurt Kanzenbach wrote:
> I'd rather like to see that feature implemented :-). Something like below.
> 
> From 3d44683979bf50960125fa3005b1142af61525c7 Mon Sep 17 00:00:00 2001
> From: Kurt Kanzenbach <kurt@linutronix.de>
> Date: Wed, 14 Sep 2022 19:51:40 +0200
> Subject: [PATCH] net: dsa: hellcreek: Offload per-tc max SDU from tc-taprio
> 
> Add support for configuring the max SDU per priority and per port. If not
> specified, keep the default.
> 
> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
> ---

Nice :) Do you also want the iproute2 patch, so you can test it?

>  drivers/net/dsa/hirschmann/hellcreek.c | 61 +++++++++++++++++++++++---
>  drivers/net/dsa/hirschmann/hellcreek.h |  7 +++
>  2 files changed, 61 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
> index 5ceee71d9a25..1b22710e1641 100644
> --- a/drivers/net/dsa/hirschmann/hellcreek.c
> +++ b/drivers/net/dsa/hirschmann/hellcreek.c
> @@ -128,6 +128,16 @@ static void hellcreek_select_prio(struct hellcreek *hellcreek, int prio)
>  	hellcreek_write(hellcreek, val, HR_PSEL);
>  }
>  
> +static void hellcreek_select_port_prio(struct hellcreek *hellcreek, int port,
> +				       int prio)
> +{
> +	u16 val = port << HR_PSEL_PTWSEL_SHIFT;
> +
> +	val |= prio << HR_PSEL_PRTCWSEL_SHIFT;
> +
> +	hellcreek_write(hellcreek, val, HR_PSEL);
> +}
> +
>  static void hellcreek_select_counter(struct hellcreek *hellcreek, int counter)
>  {
>  	u16 val = counter << HR_CSEL_SHIFT;
> @@ -1537,6 +1547,42 @@ hellcreek_port_prechangeupper(struct dsa_switch *ds, int port,
>  	return ret;
>  }
>  
> +static void hellcreek_setup_maxsdu(struct hellcreek *hellcreek, int port,
> +				   const struct tc_taprio_qopt_offload *schedule)
> +{
> +	int tc;
> +
> +	for (tc = 0; tc < 8; ++tc) {
> +		u16 val;
> +
> +		if (!schedule->max_sdu[tc])
> +			continue;
> +
> +		hellcreek_select_port_prio(hellcreek, port, tc);
> +
> +		val = (schedule->max_sdu[tc] & HR_PTPRTCCFG_MAXSDU_MASK)
> +			<< HR_PTPRTCCFG_MAXSDU_SHIFT;
> +
> +		hellcreek_write(hellcreek, val, HR_PTPRTCCFG);

So the maxSDU hardware register tracks exactly the L2 payload size, like
the software variable does, or does it include the Ethernet header size
and/or FCS?

> +	}
> +}
Vladimir Oltean Sept. 14, 2022, 8:13 p.m. UTC | #3
On Wed, Sep 14, 2022 at 09:40:51PM +0300, Vladimir Oltean wrote:
> Nice :) Do you also want the iproute2 patch, so you can test it?

Need it or not, here it is:
https://patchwork.kernel.org/project/netdevbpf/patch/20220914200706.1961613-1-vladimir.oltean@nxp.com/
Kurt Kanzenbach Sept. 15, 2022, 6:15 a.m. UTC | #4
On Wed Sep 14 2022, Vladimir Oltean wrote:
> On Wed, Sep 14, 2022 at 08:13:53PM +0200, Kurt Kanzenbach wrote:
>> I'd rather like to see that feature implemented :-). Something like below.
>> 
>> From 3d44683979bf50960125fa3005b1142af61525c7 Mon Sep 17 00:00:00 2001
>> From: Kurt Kanzenbach <kurt@linutronix.de>
>> Date: Wed, 14 Sep 2022 19:51:40 +0200
>> Subject: [PATCH] net: dsa: hellcreek: Offload per-tc max SDU from tc-taprio
>> 
>> Add support for configuring the max SDU per priority and per port. If not
>> specified, keep the default.
>> 
>> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
>> ---
>
> Nice :) Do you also want the iproute2 patch, so you can test it?

Sure. I see you posted that patch already.

>
>>  drivers/net/dsa/hirschmann/hellcreek.c | 61 +++++++++++++++++++++++---
>>  drivers/net/dsa/hirschmann/hellcreek.h |  7 +++
>>  2 files changed, 61 insertions(+), 7 deletions(-)
>> 
>> diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
>> index 5ceee71d9a25..1b22710e1641 100644
>> --- a/drivers/net/dsa/hirschmann/hellcreek.c
>> +++ b/drivers/net/dsa/hirschmann/hellcreek.c
>> @@ -128,6 +128,16 @@ static void hellcreek_select_prio(struct hellcreek *hellcreek, int prio)
>>  	hellcreek_write(hellcreek, val, HR_PSEL);
>>  }
>>  
>> +static void hellcreek_select_port_prio(struct hellcreek *hellcreek, int port,
>> +				       int prio)
>> +{
>> +	u16 val = port << HR_PSEL_PTWSEL_SHIFT;
>> +
>> +	val |= prio << HR_PSEL_PRTCWSEL_SHIFT;
>> +
>> +	hellcreek_write(hellcreek, val, HR_PSEL);
>> +}
>> +
>>  static void hellcreek_select_counter(struct hellcreek *hellcreek, int counter)
>>  {
>>  	u16 val = counter << HR_CSEL_SHIFT;
>> @@ -1537,6 +1547,42 @@ hellcreek_port_prechangeupper(struct dsa_switch *ds, int port,
>>  	return ret;
>>  }
>>  
>> +static void hellcreek_setup_maxsdu(struct hellcreek *hellcreek, int port,
>> +				   const struct tc_taprio_qopt_offload *schedule)
>> +{
>> +	int tc;
>> +
>> +	for (tc = 0; tc < 8; ++tc) {
>> +		u16 val;
>> +
>> +		if (!schedule->max_sdu[tc])
>> +			continue;
>> +
>> +		hellcreek_select_port_prio(hellcreek, port, tc);
>> +
>> +		val = (schedule->max_sdu[tc] & HR_PTPRTCCFG_MAXSDU_MASK)
>> +			<< HR_PTPRTCCFG_MAXSDU_SHIFT;
>> +
>> +		hellcreek_write(hellcreek, val, HR_PTPRTCCFG);
>
> So the maxSDU hardware register tracks exactly the L2 payload size, like
> the software variable does, or does it include the Ethernet header size
> and/or FCS?

This is something I'm not sure about. I'll ask the HW engineer when he's
back from vacation.

Thanks,
Kurt
Vladimir Oltean Sept. 15, 2022, 11:59 a.m. UTC | #5
On Thu, Sep 15, 2022 at 08:15:54AM +0200, Kurt Kanzenbach wrote:
> > So the maxSDU hardware register tracks exactly the L2 payload size, like
> > the software variable does, or does it include the Ethernet header size
> > and/or FCS?
> 
> This is something I'm not sure about. I'll ask the HW engineer when he's
> back from vacation.

You can also probably figure this out by limiting the max-sdu to a value
like 200 and seeing what frame sizes pass through.
Kurt Kanzenbach Sept. 19, 2022, 1:36 p.m. UTC | #6
On Thu Sep 15 2022, Vladimir Oltean wrote:
> On Thu, Sep 15, 2022 at 08:15:54AM +0200, Kurt Kanzenbach wrote:
>> > So the maxSDU hardware register tracks exactly the L2 payload size, like
>> > the software variable does, or does it include the Ethernet header size
>> > and/or FCS?
>> 
>> This is something I'm not sure about. I'll ask the HW engineer when he's
>> back from vacation.
>
> You can also probably figure this out by limiting the max-sdu to a value
> like 200 and seeing what frame sizes pass through.

Oh, yes. I'll try that.

Thanks,
Kurt
Kurt Kanzenbach Sept. 21, 2022, 11:23 a.m. UTC | #7
On Thu Sep 15 2022, Vladimir Oltean wrote:
> On Thu, Sep 15, 2022 at 08:15:54AM +0200, Kurt Kanzenbach wrote:
>> > So the maxSDU hardware register tracks exactly the L2 payload size, like
>> > the software variable does, or does it include the Ethernet header size
>> > and/or FCS?
>> 
>> This is something I'm not sure about. I'll ask the HW engineer when he's
>> back from vacation.
>
> You can also probably figure this out by limiting the max-sdu to a value
> like 200 and seeing what frame sizes pass through.

So, configured to 128 and 132 bytes (including VLAN Ethernet header) is
the maximum frame size which passes through.

Thanks,
Kurt
Vladimir Oltean Sept. 21, 2022, 11:29 a.m. UTC | #8
On Wed, Sep 21, 2022 at 01:23:24PM +0200, Kurt Kanzenbach wrote:
> On Thu Sep 15 2022, Vladimir Oltean wrote:
> > On Thu, Sep 15, 2022 at 08:15:54AM +0200, Kurt Kanzenbach wrote:
> >> > So the maxSDU hardware register tracks exactly the L2 payload size, like
> >> > the software variable does, or does it include the Ethernet header size
> >> > and/or FCS?
> >> 
> >> This is something I'm not sure about. I'll ask the HW engineer when he's
> >> back from vacation.
> >
> > You can also probably figure this out by limiting the max-sdu to a value
> > like 200 and seeing what frame sizes pass through.
> 
> So, configured to 128 and 132 bytes (including VLAN Ethernet header) is
> the maximum frame size which passes through.

Frame size means MAC DA + MAC SA + VLAN header + Ethertype + L2 payload,
and without FCS, right?

Because max_sdu 128 only counts the L2 payload, so the maximum frame
size that passes should be 142 octets, or 146 octets with VLAN.

The same is true with the port MTU. When it's 1500, the maximum frame
size is 1514, or 1518 with VLAN.
Kurt Kanzenbach Sept. 21, 2022, 11:46 a.m. UTC | #9
On Wed Sep 21 2022, Vladimir Oltean wrote:
> On Wed, Sep 21, 2022 at 01:23:24PM +0200, Kurt Kanzenbach wrote:
>> On Thu Sep 15 2022, Vladimir Oltean wrote:
>> > On Thu, Sep 15, 2022 at 08:15:54AM +0200, Kurt Kanzenbach wrote:
>> >> > So the maxSDU hardware register tracks exactly the L2 payload size, like
>> >> > the software variable does, or does it include the Ethernet header size
>> >> > and/or FCS?
>> >> 
>> >> This is something I'm not sure about. I'll ask the HW engineer when he's
>> >> back from vacation.
>> >
>> > You can also probably figure this out by limiting the max-sdu to a value
>> > like 200 and seeing what frame sizes pass through.
>> 
>> So, configured to 128 and 132 bytes (including VLAN Ethernet header) is
>> the maximum frame size which passes through.
>
> Frame size means MAC DA + MAC SA + VLAN header + Ethertype + L2 payload,
> and without FCS, right?

Yes.

>
> Because max_sdu 128 only counts the L2 payload, so the maximum frame
> size that passes should be 142 octets, or 146 octets with VLAN.

Ok, i see. So, for 128 max-sdu we should end up with something like this
in the prio config register:

 schedule->max_sdu[tc] + VLAN_ETH_HLEN - 4

Thanks,
Kurt
Vladimir Oltean Sept. 21, 2022, 2:12 p.m. UTC | #10
On Wed, Sep 21, 2022 at 01:46:22PM +0200, Kurt Kanzenbach wrote:
> >> So, configured to 128 and 132 bytes (including VLAN Ethernet header) is
> >> the maximum frame size which passes through.
> >
> > Frame size means MAC DA + MAC SA + VLAN header + Ethertype + L2 payload,
> > and without FCS, right?
> 
> Yes.
> 
> >
> > Because max_sdu 128 only counts the L2 payload, so the maximum frame
> > size that passes should be 142 octets, or 146 octets with VLAN.
> 
> Ok, i see. So, for 128 max-sdu we should end up with something like this
> in the prio config register:
> 
>  schedule->max_sdu[tc] + VLAN_ETH_HLEN - 4

What does 4 represent? ETH_FCS_LEN?

So when schedule->max_sdu[tc] is 128, you write to HR_PTPRTCCFG_MAXSDU
the value of 128 + 18 - 4 = 142, and this will pass packets (VLAN-tagged
or not) with an skb->len (on xmit) <= 142, right?
Vladimir Oltean Sept. 21, 2022, 2:21 p.m. UTC | #11
On Wed, Sep 21, 2022 at 05:12:06PM +0300, Vladimir Oltean wrote:
> On Wed, Sep 21, 2022 at 01:46:22PM +0200, Kurt Kanzenbach wrote:
> > >> So, configured to 128 and 132 bytes (including VLAN Ethernet header) is
> > >> the maximum frame size which passes through.
> > >
> > > Frame size means MAC DA + MAC SA + VLAN header + Ethertype + L2 payload,
> > > and without FCS, right?
> > 
> > Yes.
> > 
> > >
> > > Because max_sdu 128 only counts the L2 payload, so the maximum frame
> > > size that passes should be 142 octets, or 146 octets with VLAN.
> > 
> > Ok, i see. So, for 128 max-sdu we should end up with something like this
> > in the prio config register:
> > 
> >  schedule->max_sdu[tc] + VLAN_ETH_HLEN - 4
> 
> What does 4 represent? ETH_FCS_LEN?
> 
> So when schedule->max_sdu[tc] is 128, you write to HR_PTPRTCCFG_MAXSDU
> the value of 128 + 18 - 4 = 142, and this will pass packets (VLAN-tagged
> or not) with an skb->len (on xmit) <= 142, right?

Mistake, I meant skb->len <= 146 (max_sdu[tc] + VLAN_ETH_HLEN). So the
hardware calculates the max frame len to include FCS, and skb->len is
without FCS, hence the 4 discrepancy.
Kurt Kanzenbach Sept. 22, 2022, 8:10 a.m. UTC | #12
On Wed Sep 21 2022, Vladimir Oltean wrote:
> On Wed, Sep 21, 2022 at 05:12:06PM +0300, Vladimir Oltean wrote:
>> On Wed, Sep 21, 2022 at 01:46:22PM +0200, Kurt Kanzenbach wrote:
>> > >> So, configured to 128 and 132 bytes (including VLAN Ethernet header) is
>> > >> the maximum frame size which passes through.
>> > >
>> > > Frame size means MAC DA + MAC SA + VLAN header + Ethertype + L2 payload,
>> > > and without FCS, right?
>> > 
>> > Yes.
>> > 
>> > >
>> > > Because max_sdu 128 only counts the L2 payload, so the maximum frame
>> > > size that passes should be 142 octets, or 146 octets with VLAN.
>> > 
>> > Ok, i see. So, for 128 max-sdu we should end up with something like this
>> > in the prio config register:
>> > 
>> >  schedule->max_sdu[tc] + VLAN_ETH_HLEN - 4
>> 
>> What does 4 represent? ETH_FCS_LEN?
>> 
>> So when schedule->max_sdu[tc] is 128, you write to HR_PTPRTCCFG_MAXSDU
>> the value of 128 + 18 - 4 = 142, and this will pass packets (VLAN-tagged
>> or not) with an skb->len (on xmit) <= 142, right?
>
> Mistake, I meant skb->len <= 146 (max_sdu[tc] + VLAN_ETH_HLEN). So the
> hardware calculates the max frame len to include FCS, and skb->len is
> without FCS, hence the 4 discrepancy.

Yes, seems like it.

In case you repost this series, can you replace your patch with the one
below?

From 6488e0f979f3ea8c6130beb1b3e661cdb7796916 Mon Sep 17 00:00:00 2001
From: Kurt Kanzenbach <kurt@linutronix.de>
Date: Wed, 14 Sep 2022 19:51:40 +0200
Subject: [PATCH] net: dsa: hellcreek: Offload per-tc max SDU from tc-taprio

Add support for configuring the max SDU per priority and per port. If not
specified, keep the default.

Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
---
 drivers/net/dsa/hirschmann/hellcreek.c | 64 +++++++++++++++++++++++---
 drivers/net/dsa/hirschmann/hellcreek.h |  7 +++
 2 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 5ceee71d9a25..c4b76b1e7a63 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -128,6 +128,16 @@ static void hellcreek_select_prio(struct hellcreek *hellcreek, int prio)
 	hellcreek_write(hellcreek, val, HR_PSEL);
 }
 
+static void hellcreek_select_port_prio(struct hellcreek *hellcreek, int port,
+				       int prio)
+{
+	u16 val = port << HR_PSEL_PTWSEL_SHIFT;
+
+	val |= prio << HR_PSEL_PRTCWSEL_SHIFT;
+
+	hellcreek_write(hellcreek, val, HR_PSEL);
+}
+
 static void hellcreek_select_counter(struct hellcreek *hellcreek, int counter)
 {
 	u16 val = counter << HR_CSEL_SHIFT;
@@ -1537,6 +1547,45 @@ hellcreek_port_prechangeupper(struct dsa_switch *ds, int port,
 	return ret;
 }
 
+static void hellcreek_setup_maxsdu(struct hellcreek *hellcreek, int port,
+				   const struct tc_taprio_qopt_offload *schedule)
+{
+	int tc;
+
+	for (tc = 0; tc < 8; ++tc) {
+		u32 max_sdu = schedule->max_sdu[tc] + VLAN_ETH_HLEN - ETH_FCS_LEN;
+		u16 val;
+
+		if (!schedule->max_sdu[tc])
+			continue;
+
+		dev_dbg(hellcreek->dev, "Configure max-sdu %u for tc %d on port %d\n",
+			max_sdu, tc, port);
+
+		hellcreek_select_port_prio(hellcreek, port, tc);
+
+		val = (max_sdu & HR_PTPRTCCFG_MAXSDU_MASK) << HR_PTPRTCCFG_MAXSDU_SHIFT;
+
+		hellcreek_write(hellcreek, val, HR_PTPRTCCFG);
+	}
+}
+
+static void hellcreek_reset_maxsdu(struct hellcreek *hellcreek, int port)
+{
+	int tc;
+
+	for (tc = 0; tc < 8; ++tc) {
+		u16 val;
+
+		hellcreek_select_port_prio(hellcreek, port, tc);
+
+		val = (HELLCREEK_DEFAULT_MAX_SDU & HR_PTPRTCCFG_MAXSDU_MASK)
+			<< HR_PTPRTCCFG_MAXSDU_SHIFT;
+
+		hellcreek_write(hellcreek, val, HR_PTPRTCCFG);
+	}
+}
+
 static void hellcreek_setup_gcl(struct hellcreek *hellcreek, int port,
 				const struct tc_taprio_qopt_offload *schedule)
 {
@@ -1720,7 +1769,10 @@ static int hellcreek_port_set_schedule(struct dsa_switch *ds, int port,
 	}
 	hellcreek_port->current_schedule = taprio_offload_get(taprio);
 
-	/* Then select port */
+	/* Configure max sdu */
+	hellcreek_setup_maxsdu(hellcreek, port, hellcreek_port->current_schedule);
+
+	/* Select tdg */
 	hellcreek_select_tgd(hellcreek, port);
 
 	/* Enable gating and keep defaults */
@@ -1772,7 +1824,10 @@ static int hellcreek_port_del_schedule(struct dsa_switch *ds, int port)
 		hellcreek_port->current_schedule = NULL;
 	}
 
-	/* Then select port */
+	/* Reset max sdu */
+	hellcreek_reset_maxsdu(hellcreek, port);
+
+	/* Select tgd */
 	hellcreek_select_tgd(hellcreek, port);
 
 	/* Disable gating and return to regular switching flow */
@@ -1814,15 +1869,10 @@ static int hellcreek_port_setup_tc(struct dsa_switch *ds, int port,
 {
 	struct tc_taprio_qopt_offload *taprio = type_data;
 	struct hellcreek *hellcreek = ds->priv;
-	int tc;
 
 	if (type != TC_SETUP_QDISC_TAPRIO)
 		return -EOPNOTSUPP;
 
-	for (tc = 0; tc < TC_MAX_QUEUE; tc++)
-		if (taprio->max_sdu[tc])
-			return -EOPNOTSUPP;
-
 	if (!hellcreek_validate_schedule(hellcreek, taprio))
 		return -EOPNOTSUPP;
 
diff --git a/drivers/net/dsa/hirschmann/hellcreek.h b/drivers/net/dsa/hirschmann/hellcreek.h
index c96b8c278904..66b989d946e4 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.h
+++ b/drivers/net/dsa/hirschmann/hellcreek.h
@@ -37,6 +37,7 @@
 #define HELLCREEK_VLAN_UNTAGGED_MEMBER	0x1
 #define HELLCREEK_VLAN_TAGGED_MEMBER	0x3
 #define HELLCREEK_NUM_EGRESS_QUEUES	8
+#define HELLCREEK_DEFAULT_MAX_SDU	1536
 
 /* Register definitions */
 #define HR_MODID_C			(0 * 2)
@@ -72,6 +73,12 @@
 #define HR_PRTCCFG_PCP_TC_MAP_SHIFT	0
 #define HR_PRTCCFG_PCP_TC_MAP_MASK	GENMASK(2, 0)
 
+#define HR_PTPRTCCFG			(0xa9 * 2)
+#define HR_PTPRTCCFG_SET_QTRACK		BIT(15)
+#define HR_PTPRTCCFG_REJECT		BIT(14)
+#define HR_PTPRTCCFG_MAXSDU_SHIFT	0
+#define HR_PTPRTCCFG_MAXSDU_MASK	GENMASK(10, 0)
+
 #define HR_CSEL				(0x8d * 2)
 #define HR_CSEL_SHIFT			0
 #define HR_CSEL_MASK			GENMASK(7, 0)
diff mbox series

Patch

diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index ea8bbfce0f1f..8ef7816627b6 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -1814,10 +1814,15 @@  static int hellcreek_port_setup_tc(struct dsa_switch *ds, int port,
 {
 	struct tc_taprio_qopt_offload *taprio = type_data;
 	struct hellcreek *hellcreek = ds->priv;
+	int tc;
 
 	if (type != TC_SETUP_QDISC_TAPRIO)
 		return -EOPNOTSUPP;
 
+	for (tc = 0; tc < TC_MAX_QUEUE; tc++)
+		if (taprio->max_sdu[tc])
+			return -EOPNOTSUPP;
+
 	if (!hellcreek_validate_schedule(hellcreek, taprio))
 		return -EOPNOTSUPP;