From: Andrew Lunn <andrew@lunn•ch>
To: "shenjian (K)" <shenjian15@huawei•com>
Cc: davem@davemloft•net, kuba@kernel•org, netdev@vger•kernel.org,
linuxarm@openeuler•org
Subject: Re: [RFC net-next] net: extend netdev features
Date: Tue, 13 Jul 2021 15:53:01 +0200 [thread overview]
Message-ID: <YO2avXo6XSBEzZb/@lunn.ch> (raw)
In-Reply-To: <2b6bc8a7-6fe3-b42e-070d-f9a81560ecda@huawei.com>
> Hi Andrew,
>
> Thanks for your advice.
>
> I have thought of using linkmode_ before (https://lists.openwall.net/netdev/
> 2021/06/19/11).
>
> In my humble opinion, there are too many logical operations in stack and
> ethernet drivers,
> I'm not sure changing them to the linkmode_set_ API is the best way. For
> example,
>
> below is codes in ethernet drivre:
> 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;
>
> When using linkmode_ API, I have two choices, one is to define several feature
> arrays or
> a whole features array including all the feature bits supported by the ethernet
> driver.
> const int hns3_nic_vlan_features_array[] = {
> NETIF_F_HW_VLAN_CTAG_FILTER,
> NETIF_F_HW_VLAN_CTAG_TX,
> NETIF_F_HW_VLAN_CTAG_RX,
> };
> const int hns3_nic_tso_features_array[] = {
> NETIF_F_GSO,
> NETIF_F_TSO,
> NETIF_F_TSO6,
> NETIF_F_GSO_GRE,
> ...
> };
Using arrays is the way to go. Hopefully Coccinelle can do most of the
work for you.
> linkmode_set_bit_array(hns3_nic_vlan_features_array, ARRAY_SIZE
> (hns3_nic_vlan_features_array), netedv->features);
I would probably add wrappers. So an API like
netdev_feature_set_array(ndev, int hns3_nic_tso_features_array),
ARRAY_SIZE(int hns3_nic_tso_features_array);
netdev_hw_feature_set_array(ndev, int hns3_nic_tso_features_array),
ARRAY_SIZE(int hns3_nic_vlan_features_array);
etc. This will help during the conversion. You can keep
netdev_features_t as a u64 while you convert to the mew API. Once the
conversion is done, you can then convert the implementation of the API
to a linux bitmap.
> When using netdev_feature_t *, then just need to add an arrary index.
>
> netdev->features[0] |= NETIF_F_HW_VLAN_CTAG_FILTER |
> NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX | xxxxxx
>
And you want to add
netdev->features[1] |= NETIF_F_NEW_FEATURE;
This is going to result in errors, where developers add
NETIF_F_NEW_FEATURE to feature[0], and the compiler will happily do
it, no warnings. Either you need the compiler to enforce the correct
assignment to the array members somehow, or you need a uniform API
which you cannot get wrong.
> By the way, could you introduce me some code re-writing tools ?
Coccinelle.
https://www.kernel.org/doc/html/latest/dev-tools/coccinelle.html
https://coccinelle.gitlabpages.inria.fr/website/
I've no idea if it can do this level of code re-write. You probably
want to post on the mailing list, describe what you want to do.
Andrew
next prev parent reply other threads:[~2021-07-13 13:53 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-10 9:40 [RFC net-next] net: extend netdev features Jian Shen
2021-07-10 15:11 ` Stephen Hemminger
2021-07-10 15:16 ` Andrew Lunn
2021-07-10 15:35 ` Dave Taht
2021-07-10 20:32 ` Stephen Hemminger
2021-07-12 2:43 ` shenjian (K)
2021-07-10 15:29 ` Andrew Lunn
[not found] ` <2b6bc8a7-6fe3-b42e-070d-f9a81560ecda@huawei.com>
2021-07-13 13:53 ` Andrew Lunn [this message]
2021-07-15 6:34 ` shenjian (K)
2021-07-10 19:05 ` Heiner Kallweit
2021-07-12 3:41 ` shenjian (K)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YO2avXo6XSBEzZb/@lunn.ch \
--to=andrew@lunn$(echo .)ch \
--cc=davem@davemloft$(echo .)net \
--cc=kuba@kernel$(echo .)org \
--cc=linuxarm@openeuler$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=shenjian15@huawei$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox