netdev - Re: [PATCH net-next v2] virtio-net: ethtool configurable LRO

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: <20181220.191839.1463129129152804862.davem@davemloft.net>
Date:   Thu, 20 Dec 2018 19:18:39 -0800 (PST)
From:   David Miller <davem@...emloft.net>
To:     mst@...hat.com
Cc:     willemdebruijn.kernel@...il.com, netdev@...r.kernel.org,
        jasowang@...hat.com, willemb@...gle.com
Subject: Re: [PATCH net-next v2] virtio-net: ethtool configurable LRO

From: "Michael S. Tsirkin" <mst@...hat.com>
Date: Thu, 20 Dec 2018 17:34:23 -0500

> On Thu, Dec 20, 2018 at 05:14:54PM -0500, Willem de Bruijn wrote:
>> From: Willem de Bruijn <willemb@...gle.com>
>> 
>> Virtio-net devices negotiate LRO support with the host.
>> Display the initially negotiated state with ethtool -k.
>> 
>> Also allow configuring it with ethtool -K, reusing the existing
>> virtnet_set_guest_offloads helper that configures LRO for XDP.
>> This is conditional on VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.
>> 
>> Virtio-net negotiates TSO4 and TSO6 separately, but ethtool does not
>> distinguish between the two. Display LRO as on only if any offload
>> is active.
>> 
>> RTNL is held while calling virtnet_set_features, same as on the path
>> from virtnet_xdp_set.
>> 
>> Changes v1 -> v2
>>   - allow ethtool config (-K) only if VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
>>   - show LRO as enabled if any LRO variant is enabled
>>   - do not allow configuration while XDP is active
>>   - differentiate current features from the capable set, to restore
>>     on XDP down only those features that were active on XDP up
>>   - move test out of VIRTIO_NET_F_CSUM/TSO branch, which is tx only
> 
> This part shouldn't be in the commit log, right? Should be after "---".

As Willem stated, I really like to see the changelog in the commit message
proper.  It is just my taste, because I have on countless times relied on
that information when reading over old changes.

And I know it helps other people too.

>> Signed-off-by: Willem de Bruijn <willemb@...gle.com>
> 
> Acked-by: Michael S. Tsirkin <mst@...hat.com>

Applied, thanks for reviewing.