netdev - Re: [RFCv4 PATCH net-next] net: extend netdev_features

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <86fa46f8-2a20-8912-7ec2-19257d6598db@huawei.com>
Date:   Wed, 10 Nov 2021 09:17:12 +0800
From:   "shenjian (K)" <shenjian15@...wei.com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     <davem@...emloft.net>, <kuba@...nel.org>, <ecree.xilinx@...il.com>,
        <hkallweit1@...il.com>, <alexandr.lobakin@...el.com>,
        <saeed@...nel.org>, <netdev@...r.kernel.org>,
        <linuxarm@...neuler.org>
Subject: Re: [RFCv4 PATCH net-next] net: extend netdev_features_t



在 2021/11/10 6:32, Andrew Lunn 写道:
>> -	if ((netdev->features & NETIF_F_HW_TC) > (features & NETIF_F_HW_TC) &&
>> +	if ((netdev_active_features_test_bit(netdev, NETIF_F_HW_TC_BIT) >
>> +	    netdev_features_test_bit(NETIF_F_NTUPLE_BIT, features)) &&
> Using > is interesting.
will use

if (netdev_active_features_test_bit(netdev, NETIF_F_HW_TC_BIT) &&
     !netdev_features_test_bit(netdev, NETIF_F_HW_TC_BIT))

instead.

> But where did NETIF_F_NTUPLE_BIT come from?
Thanks for catching this!


>
>> -	netdev->features |= NETIF_F_HW_VLAN_CTAG_FILTER |
>> -		NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX |
>> -		NETIF_F_RXCSUM | NETIF_F_SG | NETIF_F_GSO |
>> -		NETIF_F_GRO | NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_GSO_GRE |
>> -		NETIF_F_GSO_GRE_CSUM | NETIF_F_GSO_UDP_TUNNEL |
>> -		NETIF_F_SCTP_CRC | NETIF_F_FRAGLIST;
>> +	netdev_features_zero(&features);
>> +	netdev_features_set_array(hns3_default_features_array,
>> +				  ARRAY_SIZE(hns3_default_features_array),
>> +				  &features);
> The original code is netdev->features |= so it is appending these
> bits. Yet the first thing the new code does is zero features?
>
>        Andrew
> .
The features is a local variable, the change for netdev->active_features 
is later, by calling

netdev_active_features_direct_or(netdev, features);


By the way, have you reveiwed the rest of patch ? Is there anything else need to rework ?

I wonder whether to go ahead.

Thanks!

     Jian