linux-kernel - Re: [PATCH net-next 5/6] net: macb: Implement TAPRIO TC offload command interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <7b4d7b89-a69f-453a-bf9c-d0ccdd12d76e@tuxon.dev>
Date: Thu, 31 Jul 2025 12:07:28 +0300
From: Claudiu Beznea <claudiu.beznea@...on.dev>
To: "Karumanchi, Vineeth" <vineeth@....com>, vineeth.karumanchi@....com,
 nicolas.ferre@...rochip.com, andrew+netdev@...n.ch, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com
Cc: git@....com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 5/6] net: macb: Implement TAPRIO TC offload
 command interface



On 29.07.2025 12:38, Karumanchi, Vineeth wrote:
> Hi Claudiu,
> 
> 
> On 7/26/2025 5:59 PM, claudiu beznea (tuxon) wrote:
> <...>
>>> +    int err = 0;
>>> +
>>> +    switch (taprio->cmd) {
>>> +    case TAPRIO_CMD_REPLACE:
>>> +        err = macb_taprio_setup_replace(ndev, taprio);
>>> +        break;
>>> +    case TAPRIO_CMD_DESTROY:
>>> +        macb_taprio_destroy(ndev);
>>
>> macb_taprio_setup_replace() along with macb_taprio_destroy() touch HW
>> registers. Could macb_setup_taprio() be called when the interface is
>> runtime suspended?
>>
>>
> 
> Nice catch!
> 
> I will leverage pm_runtime_suspended(&pdev->dev) check before configuring.
> 
>>> +        break;
>>> +    default:
>>> +        err = -EOPNOTSUPP;
>>> +    }
>>> +
>>> +    return err;
>>> +}
>>> +
>>> +static int macb_setup_tc(struct net_device *dev, enum tc_setup_type
>>> type, void *type_data)
>>> +{
>>> +    if (!dev || !type_data)
>>> +        return -EINVAL;
>>> +
>>> +    switch (type) {
>>> +    case TC_SETUP_QDISC_TAPRIO:
>>> +        return macb_setup_taprio(dev, type_data);
>>
>> Same here.
>>
>>> +    default:
>>> +        return -EOPNOTSUPP;
>>> +    }
>>> +}
>>> +
>>>   static const struct net_device_ops macb_netdev_ops = {
>>>       .ndo_open        = macb_open,
>>>       .ndo_stop        = macb_close,
>>> @@ -4284,6 +4316,7 @@ static const struct net_device_ops macb_netdev_ops
>>> = {
>>>       .ndo_features_check    = macb_features_check,
>>>       .ndo_hwtstamp_set    = macb_hwtstamp_set,
>>>       .ndo_hwtstamp_get    = macb_hwtstamp_get,
>>> +    .ndo_setup_tc        = macb_setup_tc,
>>
>> This patch (or parts of it) should be merged with the previous ones.
>> Otherwise you introduce patches with code that is unused.
>>
> 
> Clubbing all comments on patch organization:
> I see that patch series gets merged into 2 set only.
> 
> 1/6 + 2/6 + 3/6 + 4/6 + 5/6 ==> 1/2
> 6/6 ==> 2/2

That should be good.

Thank you,
Claudiu

> 
> Please let me know your thoughts or suggestions.
> 
> 
> Thanks