[RFC PATCH net] net: taprio: Validate offload support using NETIF_F_HW_TC in hw_features

[RFC PATCH net] net: taprio: Validate offload support using NETIF_F_HW_TC in hw_features

[Vineeth Karumanchi]

The current taprio offload validation relies solely on the presence of
.ndo_setup_tc, which is insufficient. Some IP versions of a driver expose
.ndo_setup_tc but lack actual hardware offload support for taprio.

To address this, add a check for NETIF_F_HW_TC in netdev->hw_features.
This ensures that taprio offload is only enabled on devices that
explicitly advertise hardware traffic control capabilities.

Note: Some drivers already set NETIF_F_HW_TC alongside .ndo_setup_tc.
Follow-up patches will be submitted to update remaining drivers if this
approach is accepted.

Signed-off-by: Vineeth Karumanchi <vineeth.karumanchi@amd.com>
---
 net/sched/sch_taprio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 2b14c81a87e5..a797995bdc8d 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -1506,7 +1506,7 @@ static int taprio_enable_offload(struct net_device *dev,
 	struct tc_taprio_caps caps;
 	int tc, err = 0;
 
-	if (!ops->ndo_setup_tc) {
+	if (!ops->ndo_setup_tc || !(dev->hw_features & NETIF_F_HW_TC)) {
 		NL_SET_ERR_MSG(extack,
 			       "Device does not support taprio offload");
 		return -EOPNOTSUPP;
-- 
2.34.1

Re: [RFC PATCH net] net: taprio: Validate offload support using NETIF_F_HW_TC in hw_features

[Eric Dumazet]

On Mon, Jul 28, 2025 at 11:10 PM Vineeth Karumanchi
<vineeth.karumanchi@amd.com> wrote:
>
> The current taprio offload validation relies solely on the presence of
> .ndo_setup_tc, which is insufficient. Some IP versions of a driver expose
> .ndo_setup_tc but lack actual hardware offload support for taprio.
>
> To address this, add a check for NETIF_F_HW_TC in netdev->hw_features.
> This ensures that taprio offload is only enabled on devices that
> explicitly advertise hardware traffic control capabilities.
>
> Note: Some drivers already set NETIF_F_HW_TC alongside .ndo_setup_tc.
> Follow-up patches will be submitted to update remaining drivers if this
> approach is accepted.

Hi Vineeth

Could you give more details ? "Some IP versions of a driver" and "Some
drivers" are rather vague.

Also what happens without your patch ? Freeze / crash, or nothing at all ?

>
> Signed-off-by: Vineeth Karumanchi <vineeth.karumanchi@amd.com>

Patches targeting net branch should include a Fixes: tag.

Thanks

> ---
>  net/sched/sch_taprio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> index 2b14c81a87e5..a797995bdc8d 100644
> --- a/net/sched/sch_taprio.c
> +++ b/net/sched/sch_taprio.c
> @@ -1506,7 +1506,7 @@ static int taprio_enable_offload(struct net_device *dev,
>         struct tc_taprio_caps caps;
>         int tc, err = 0;
>
> -       if (!ops->ndo_setup_tc) {
> +       if (!ops->ndo_setup_tc || !(dev->hw_features & NETIF_F_HW_TC)) {
>                 NL_SET_ERR_MSG(extack,
>                                "Device does not support taprio offload");
>                 return -EOPNOTSUPP;
> --
> 2.34.1
>

Re: [RFC PATCH net] net: taprio: Validate offload support using NETIF_F_HW_TC in hw_features

[Karumanchi, Vineeth]

Hi Eric,

On 7/29/2025 12:15 PM, Eric Dumazet wrote:
> On Mon, Jul 28, 2025 at 11:10 PM Vineeth Karumanchi
> <vineeth.karumanchi@amd.com> wrote:
>>
>> The current taprio offload validation relies solely on the presence of
>> .ndo_setup_tc, which is insufficient. Some IP versions of a driver expose
>> .ndo_setup_tc but lack actual hardware offload support for taprio.
>>
>> To address this, add a check for NETIF_F_HW_TC in netdev->hw_features.
>> This ensures that taprio offload is only enabled on devices that
>> explicitly advertise hardware traffic control capabilities.
>>
>> Note: Some drivers already set NETIF_F_HW_TC alongside .ndo_setup_tc.
>> Follow-up patches will be submitted to update remaining drivers if this
>> approach is accepted.
> 
> Hi Vineeth
> 
> Could you give more details ? "Some IP versions of a driver" and "Some
> drivers" are rather vague.

At present, I’m only familiar with the GEM IP, which supports TSN Qbv in 
its later versions. The GEM implementations found in Zynq and ZynqMP 
devices do not support TSN Qbv, whereas the updated versions integrated 
into Versal devices do offer TSN Qbv support.

> 
> Also what happens without your patch ? Freeze / crash, or nothing at all ?
> 

Crash!

root $# tc qdisc replace dev end0 parent root handle 100 taprio num_tc 2 
map 0 1 queues 1@0 1@1 base-time 500 sched-entry S 0x1 100000 
sched-entry S 0x2 50000 flags 2 cycle-time 250000

[   31.667952] Internal error: synchronous external abort: 
0000000096000210 [#1]  SMP
[   31.675529] Modules linked in:
[   31.678576] CPU: 0 UID: 0 PID: 660 Comm: tc Not tainted 
6.16.0-rc6-01628-g2933e636b919-dirty #14 NONE
[   31.687870] Hardware name: ZynqMP ZCU102 Rev1.0 (DT)
[   31.692819] pstate: 200000c5 (nzCv daIF -PAN -UAO -TCO -DIT -SSBS 
BTYPE=--)
[   31.699771] pc : hw_readl_native+0x8/0x10
[   31.703782] lr : macb_taprio_setup_replace+0x2b0/0x508
[   31.708912] sp : ffff80008223b4f0
[   31.712211] x29: ffff80008223b570 x28: 0000000000000040 x27: 
000000003b9aca00
[   31.719346] x26: 0000000000000000 x25: ffff00080b5b7048 x24: 
00000000000003e8
[   31.726473] x23: 0000000000000003 x22: ffff00080b5b49c0 x21: 
ffff000808727c80
[   31.733600] x20: ffff000800150208 x19: 0000000000000000 x18: 
020c49ba5e353f7d
[   31.740727] x17: 0000000000000002 x16: 00000000000249f0 x15: 
0044b82fa09b5a53
[   31.747854] x14: 00000000ee6b27ff x13: 000000003e7fe4a7 x12: 
ffff000808727c90
[   31.754981] x11: 0000000000000002 x10: 0000000000024be4 x9 : 
0000000000000000
[   31.762108] x8 : 0000000000000002 x7 : 00000000000061a8 x6 : 
000000000000186a
[   31.769235] x5 : 0000000000000000 x4 : 0000000000018894 x3 : 
0000000000000000
[   31.776362] x2 : ffff800080928d78 x1 : ffff80008190d880 x0 : 
ffff80008190d000
[   31.783490] Call trace:
[   31.785921]  hw_readl_native+0x8/0x10 (P)
[   31.789922]  macb_setup_tc+0x13c/0x190
[   31.793663]  taprio_change+0x768/0xb90
[   31.797405]  taprio_init+0x1d0/0x280
[   31.800973]  qdisc_create+0x114/0x40c
[   31.804627]  tc_modify_qdisc+0x4c8/0x804
[   31.808542]  rtnetlink_rcv_msg+0x284/0x374
[   31.812631]  netlink_rcv_skb+0x60/0x130
[   31.816459]  rtnetlink_rcv+0x18/0x24
[   31.820027]  netlink_unicast+0x1e8/0x304
[   31.823942]  netlink_sendmsg+0x168/0x3a4
[   31.827857]  ____sys_sendmsg+0x220/0x268
[   31.831772]  ___sys_sendmsg+0xb0/0x108
[   31.835514]  __sys_sendmsg+0x9c/0x100
[   31.839168]  __arm64_sys_sendmsg+0x24/0x30
[   31.843257]  invoke_syscall+0x48/0x10c
[   31.846999]  el0_svc_common.constprop.0+0xc0/0xe0
[   31.851695]  do_el0_svc+0x1c/0x28
[   31.855003]  el0_svc+0x34/0x104
[   31.858136]  el0t_64_sync_handler+0x10c/0x138
[   31.862485]  el0t_64_sync+0x198/0x19c
[   31.866144] Code: 88dffc00 88dffc00 f9400000 8b21c001 (b9400020)
[   31.872227] ---[ end trace 0000000000000000 ]---
[   31.876836] note: tc[660] exited with irqs disabled
Segmentation fault


>>
>> Signed-off-by: Vineeth Karumanchi <vineeth.karumanchi@amd.com>
> 
> Patches targeting net branch should include a Fixes: tag.
> 
> Thanks
> 
>> ---
>>   net/sched/sch_taprio.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
>> index 2b14c81a87e5..a797995bdc8d 100644
>> --- a/net/sched/sch_taprio.c
>> +++ b/net/sched/sch_taprio.c
>> @@ -1506,7 +1506,7 @@ static int taprio_enable_offload(struct net_device *dev,
>>          struct tc_taprio_caps caps;
>>          int tc, err = 0;
>>
>> -       if (!ops->ndo_setup_tc) {
>> +       if (!ops->ndo_setup_tc || !(dev->hw_features & NETIF_F_HW_TC)) {
>>                  NL_SET_ERR_MSG(extack,
>>                                 "Device does not support taprio offload");
>>                  return -EOPNOTSUPP;
>> --
>> 2.34.1
>>

-- 
🙏 vineeth

Re: [RFC PATCH net] net: taprio: Validate offload support using NETIF_F_HW_TC in hw_features

[Eric Dumazet]

On Tue, Jul 29, 2025 at 3:41 AM Karumanchi, Vineeth <vineeth@amd.com> wrote:
>
> Hi Eric,
>
> On 7/29/2025 12:15 PM, Eric Dumazet wrote:
> > On Mon, Jul 28, 2025 at 11:10 PM Vineeth Karumanchi
> > <vineeth.karumanchi@amd.com> wrote:
> >>
> >> The current taprio offload validation relies solely on the presence of
> >> .ndo_setup_tc, which is insufficient. Some IP versions of a driver expose
> >> .ndo_setup_tc but lack actual hardware offload support for taprio.
> >>
> >> To address this, add a check for NETIF_F_HW_TC in netdev->hw_features.
> >> This ensures that taprio offload is only enabled on devices that
> >> explicitly advertise hardware traffic control capabilities.
> >>
> >> Note: Some drivers already set NETIF_F_HW_TC alongside .ndo_setup_tc.
> >> Follow-up patches will be submitted to update remaining drivers if this
> >> approach is accepted.
> >
> > Hi Vineeth
> >
> > Could you give more details ? "Some IP versions of a driver" and "Some
> > drivers" are rather vague.
>
> At present, I’m only familiar with the GEM IP, which supports TSN Qbv in
> its later versions. The GEM implementations found in Zynq and ZynqMP
> devices do not support TSN Qbv, whereas the updated versions integrated
> into Versal devices do offer TSN Qbv support.

Is this an out-of-tree driver ? I do not find macb_taprio_setup_replace()

I think most drivers should return -EOPNOTSUPP in this case.

>
> >
> > Also what happens without your patch ? Freeze / crash, or nothing at all ?
> >
>
> Crash!
>
> root $# tc qdisc replace dev end0 parent root handle 100 taprio num_tc 2
> map 0 1 queues 1@0 1@1 base-time 500 sched-entry S 0x1 100000
> sched-entry S 0x2 50000 flags 2 cycle-time 250000
>
> [   31.667952] Internal error: synchronous external abort:
> 0000000096000210 [#1]  SMP
> [   31.675529] Modules linked in:
> [   31.678576] CPU: 0 UID: 0 PID: 660 Comm: tc Not tainted
> 6.16.0-rc6-01628-g2933e636b919-dirty #14 NONE
> [   31.687870] Hardware name: ZynqMP ZCU102 Rev1.0 (DT)
> [   31.692819] pstate: 200000c5 (nzCv daIF -PAN -UAO -TCO -DIT -SSBS
> BTYPE=--)
> [   31.699771] pc : hw_readl_native+0x8/0x10
> [   31.703782] lr : macb_taprio_setup_replace+0x2b0/0x508
> [   31.708912] sp : ffff80008223b4f0
> [   31.712211] x29: ffff80008223b570 x28: 0000000000000040 x27:
> 000000003b9aca00
> [   31.719346] x26: 0000000000000000 x25: ffff00080b5b7048 x24:
> 00000000000003e8
> [   31.726473] x23: 0000000000000003 x22: ffff00080b5b49c0 x21:
> ffff000808727c80
> [   31.733600] x20: ffff000800150208 x19: 0000000000000000 x18:
> 020c49ba5e353f7d
> [   31.740727] x17: 0000000000000002 x16: 00000000000249f0 x15:
> 0044b82fa09b5a53
> [   31.747854] x14: 00000000ee6b27ff x13: 000000003e7fe4a7 x12:
> ffff000808727c90
> [   31.754981] x11: 0000000000000002 x10: 0000000000024be4 x9 :
> 0000000000000000
> [   31.762108] x8 : 0000000000000002 x7 : 00000000000061a8 x6 :
> 000000000000186a
> [   31.769235] x5 : 0000000000000000 x4 : 0000000000018894 x3 :
> 0000000000000000
> [   31.776362] x2 : ffff800080928d78 x1 : ffff80008190d880 x0 :
> ffff80008190d000
> [   31.783490] Call trace:
> [   31.785921]  hw_readl_native+0x8/0x10 (P)
> [   31.789922]  macb_setup_tc+0x13c/0x190
> [   31.793663]  taprio_change+0x768/0xb90
> [   31.797405]  taprio_init+0x1d0/0x280
> [   31.800973]  qdisc_create+0x114/0x40c
> [   31.804627]  tc_modify_qdisc+0x4c8/0x804
> [   31.808542]  rtnetlink_rcv_msg+0x284/0x374
> [   31.812631]  netlink_rcv_skb+0x60/0x130
> [   31.816459]  rtnetlink_rcv+0x18/0x24
> [   31.820027]  netlink_unicast+0x1e8/0x304
> [   31.823942]  netlink_sendmsg+0x168/0x3a4
> [   31.827857]  ____sys_sendmsg+0x220/0x268
> [   31.831772]  ___sys_sendmsg+0xb0/0x108
> [   31.835514]  __sys_sendmsg+0x9c/0x100
> [   31.839168]  __arm64_sys_sendmsg+0x24/0x30
> [   31.843257]  invoke_syscall+0x48/0x10c
> [   31.846999]  el0_svc_common.constprop.0+0xc0/0xe0
> [   31.851695]  do_el0_svc+0x1c/0x28
> [   31.855003]  el0_svc+0x34/0x104
> [   31.858136]  el0t_64_sync_handler+0x10c/0x138
> [   31.862485]  el0t_64_sync+0x198/0x19c
> [   31.866144] Code: 88dffc00 88dffc00 f9400000 8b21c001 (b9400020)
> [   31.872227] ---[ end trace 0000000000000000 ]---
> [   31.876836] note: tc[660] exited with irqs disabled
> Segmentation fault
>
>
> >>
> >> Signed-off-by: Vineeth Karumanchi <vineeth.karumanchi@amd.com>
> >
> > Patches targeting net branch should include a Fixes: tag.
> >
> > Thanks
> >
> >> ---
> >>   net/sched/sch_taprio.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> >> index 2b14c81a87e5..a797995bdc8d 100644
> >> --- a/net/sched/sch_taprio.c
> >> +++ b/net/sched/sch_taprio.c
> >> @@ -1506,7 +1506,7 @@ static int taprio_enable_offload(struct net_device *dev,
> >>          struct tc_taprio_caps caps;
> >>          int tc, err = 0;
> >>
> >> -       if (!ops->ndo_setup_tc) {
> >> +       if (!ops->ndo_setup_tc || !(dev->hw_features & NETIF_F_HW_TC)) {
> >>                  NL_SET_ERR_MSG(extack,
> >>                                 "Device does not support taprio offload");
> >>                  return -EOPNOTSUPP;
> >> --
> >> 2.34.1
> >>
>
> --
> 🙏 vineeth
>

Re: [RFC PATCH net] net: taprio: Validate offload support using NETIF_F_HW_TC in hw_features

[Karumanchi, Vineeth]

Hi Eric,

On 7/29/2025 7:06 PM, Eric Dumazet wrote:
<...>
>>>> Note: Some drivers already set NETIF_F_HW_TC alongside .ndo_setup_tc.
>>>> Follow-up patches will be submitted to update remaining drivers if this
>>>> approach is accepted.
>>> Hi Vineeth
>>>
>>> Could you give more details ? "Some IP versions of a driver" and "Some
>>> drivers" are rather vague.
>> At present, I’m only familiar with the GEM IP, which supports TSN Qbv in
>> its later versions. The GEM implementations found in Zynq and ZynqMP
>> devices do not support TSN Qbv, whereas the updated versions integrated
>> into Versal devices do offer TSN Qbv support.
> Is this an out-of-tree driver ? I do not find macb_taprio_setup_replace()
> 
> I think most drivers should return -EOPNOTSUPP in this case.
> 


These are the patches of taprio implementation in macb.

https://lore.kernel.org/netdev/20250722154111.1871292-4-vineeth.karumanchi@amd.com/

https://lore.kernel.org/netdev/20250722154111.1871292-6-vineeth.karumanchi@amd.com/
Here’s a clearer and more polished version of your message:

  I initially considered adding the check in macb driver, but since it's 
a generic validation, I believe the ideal place for it would be within 
the TC framework.

But, I'm okay with adding the check in macb driver.

Thanks
-- 
🙏 vineeth