From: Shan Wei <shanwei@cn•fujitsu.com>
To: "Michał Mirosław" <mirqus@gmail•com>
Cc: David Miller <davem@davemloft•net>,
netdev <netdev@vger•kernel.org>,
rusty@rustcorp•com.au, mst@redhat•com,
Eric Dumazet <eric.dumazet@gmail•com>,
mirq-linux@rere•qmqm.pl, bhutchings@solarflare•com,
dm@chelsio•com
Subject: Re: [PATCH 2/2 net-next] net: drivers: set TSO/UFO offload option explicitly
Date: Thu, 05 May 2011 16:51:59 +0800 [thread overview]
Message-ID: <4DC2652F.9080709@cn.fujitsu.com> (raw)
In-Reply-To: <BANLkTikckKWTBY0udurXi09UhdqOUExk6A@mail.gmail.com>
Michał Mirosław wrote, at 05/04/2011 08:36 PM:
> 2011/4/29 Shan Wei <shanwei@cn•fujitsu.com>:
>> The device drivers should not use NETIF_F_ALL_TSO mask to set hw_features(or features),
>> but have to explicitly set offload option. Because, This would make drivers automatically
>> clain to support any new TSO feature an the moment of NETIF_F_ALL_TSO is expanded.
>>
>> Some code style tuning. Just compile test.
>>
>> Signed-off-by: Shan Wei <shanwei@cn•fujitsu.com>
>> ---
>> drivers/net/loopback.c | 18 ++++++++----------
>> drivers/net/virtio_net.c | 9 ++++++---
>> 2 files changed, 14 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
>> index d70fb76..bfb6a4a 100644
>> --- a/drivers/net/loopback.c
>> +++ b/drivers/net/loopback.c
>> @@ -152,6 +152,9 @@ static const struct net_device_ops loopback_ops = {
>> .ndo_get_stats64 = loopback_get_stats64,
>> };
>>
>> +#define LOOPBACK_USER_FEATURES (NETIF_F_TSO | NETIF_F_TSO_ECN | \
>> + NETIF_F_TSO6 | NETIF_F_UFO)
>> +
>> /*
>> * The loopback device is special. There is only one instance
>> * per network namespace.
>> @@ -165,16 +168,11 @@ static void loopback_setup(struct net_device *dev)
>> dev->type = ARPHRD_LOOPBACK; /* 0x0001*/
>> dev->flags = IFF_LOOPBACK;
>> dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
>> - dev->hw_features = NETIF_F_ALL_TSO | NETIF_F_UFO;
>> - dev->features = NETIF_F_SG | NETIF_F_FRAGLIST
>> - | NETIF_F_ALL_TSO
>> - | NETIF_F_UFO
>> - | NETIF_F_NO_CSUM
>> - | NETIF_F_RXCSUM
>> - | NETIF_F_HIGHDMA
>> - | NETIF_F_LLTX
>> - | NETIF_F_NETNS_LOCAL
>> - | NETIF_F_VLAN_CHALLENGED;
>> + dev->hw_features = LOOPBACK_USER_FEATURES;
>> + dev->features = NETIF_F_SG | NETIF_F_FRAGLIST
>> + | LOOPBACK_USER_FEATURES | NETIF_F_NO_CSUM | NETIF_F_RXCSUM
>> + | NETIF_F_HIGHDMA | NETIF_F_LLTX
>> + | NETIF_F_NETNS_LOCAL | NETIF_F_VLAN_CHALLENGED;
>> dev->ethtool_ops = &loopback_ethtool_ops;
>> dev->header_ops = ð_header_ops;
>> dev->netdev_ops = &loopback_ops;
>
> You can add NETIF_F_HIGHDMA and NETIF_F_NO_CSUM to
> LOOPBACK_USER_FEATURES in one go. NETIF_F_RXCSUM could match
> NETIF_F_NO_CSUM state (this needs ndo_fix_features callback), but this
> won't have much real functional impact.
Do you mean that defining LOOPBACK_USER_FEATURES as:
#define LOOPBACK_USER_FEATURES (NETIF_F_TSO | NETIF_F_TSO_ECN | \
NETIF_F_TSO6 | NETIF_F_UFO | NETIF_F_HIGHDMA | NETIF_F_NO_CSUM)
Is it necessary to add NETIF_F_NO_CSUM to hw_features?
After that, user can change TX hw csum offload(NETIF_F_NO_CSUM is disabled). But, this is not surportted before.
--
Best Regards
-----
Shan Wei
next prev parent reply other threads:[~2011-05-05 8:55 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-29 5:34 [PATCH 2/2 net-next] net: drivers: set TSO/UFO offload option explicitly Shan Wei
2011-05-04 12:36 ` Michał Mirosław
2011-05-05 8:51 ` Shan Wei [this message]
2011-05-05 17:54 ` David Miller
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=4DC2652F.9080709@cn.fujitsu.com \
--to=shanwei@cn$(echo .)fujitsu.com \
--cc=bhutchings@solarflare$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=dm@chelsio$(echo .)com \
--cc=eric.dumazet@gmail$(echo .)com \
--cc=mirq-linux@rere$(echo .)qmqm.pl \
--cc=mirqus@gmail$(echo .)com \
--cc=mst@redhat$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=rusty@rustcorp$(echo .)com.au \
/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