Re: [RFC PATCH 0/4] eBPF RSS through QMP support.
Oh yeah, I'll fix that. Thank you! On Wed, Mar 29, 2023 at 2:52 PM Xuan Zhuo wrote: > > Is this a patch-set of QEMU? If yes, why are the email lists all kernel mail > list without QEMU mail list? > > Thanks. > > On Wed, 29 Mar 2023 13:45:41 +0300, Andrew Melnychenko > wrote: > > This series of patches provides the ability to retrieve eBPF program > > through qmp, so management application may load bpf blob with proper > > capabilities. > > Now, virtio-net devices can accept eBPF programs and maps through properties > > as external file descriptors. Access to the eBPF map is direct through > > mmap() > > call, so it should not require additional capabilities to bpf* calls. > > eBPF file descriptors can be passed to QEMU from parent process or by unix > > socket with sendfd() qmp command. > > > > Overall, the basic scenario of using the helper looks like this: > > * Libvirt checks for ebpf_fds property. > > * Libvirt requests eBPF blob through QMP. > > * Libvirt loads blob for virtio-net. > > * Libvirt launches the QEMU with eBPF fds passed. > > > > Andrew Melnychenko (4): > > ebpf: Added eBPF initialization by fds and map update. > > virtio-net: Added property to load eBPF RSS with fds. > > ebpf: Added declaration/initialization routines. > > qmp: Added new command to retrieve eBPF blob. > > > > ebpf/ebpf.c| 48 + > > ebpf/ebpf.h| 25 +++ > > ebpf/ebpf_rss-stub.c | 6 ++ > > ebpf/ebpf_rss.c| 124 +++-- > > ebpf/ebpf_rss.h| 10 +++ > > ebpf/meson.build | 1 + > > hw/net/virtio-net.c| 77 ++-- > > include/hw/virtio/virtio-net.h | 1 + > > monitor/qmp-cmds.c | 17 + > > qapi/misc.json | 25 +++ > > 10 files changed, 307 insertions(+), 27 deletions(-) > > create mode 100644 ebpf/ebpf.c > > create mode 100644 ebpf/ebpf.h > > > > -- > > 2.39.1 > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v4 0/6] TUN/VirtioNet USO features support.
Sorry, got issues with the internet during sending it. Now, all should be done. On Fri, Dec 2, 2022 at 12:33 AM Michael S. Tsirkin wrote: > > On Thu, Dec 01, 2022 at 11:56:38PM +0200, Andrew Melnychenko wrote: > > Added new offloads for TUN devices TUN_F_USO4 and TUN_F_USO6. > > Technically they enable NETIF_F_GSO_UDP_L4 > > (and only if USO4 & USO6 are set simultaneously). > > It allows the transmission of large UDP packets. > > > > UDP Segmentation Offload (USO/GSO_UDP_L4) - ability to split UDP packets > > into several segments. It's similar to UFO, except it doesn't use IP > > fragmentation. The drivers may push big packets and the NIC will split > > them(or assemble them in case of receive), but in the case of VirtioNet > > we just pass big UDP to the host. So we are freeing the driver from doing > > the unnecessary job of splitting. The same thing for several guests > > on one host, we can pass big packets between guests. > > > > Different features USO4 and USO6 are required for qemu where Windows > > guests can enable disable USO receives for IPv4 and IPv6 separately. > > On the other side, Linux can't really differentiate USO4 and USO6, for now. > > For now, to enable USO for TUN it requires enabling USO4 and USO6 together. > > In the future, there would be a mechanism to control UDP_L4 GSO separately. > > > > New types for virtio-net already in virtio-net specification: > > https://github.com/oasis-tcs/virtio-spec/issues/120 > > > > Test it WIP Qemu https://github.com/daynix/qemu/tree/USOv3 > > > > Andrew (5): > > uapi/linux/if_tun.h: Added new offload types for USO4/6. > > driver/net/tun: Added features for USO. > > uapi/linux/virtio_net.h: Added USO types. > > linux/virtio_net.h: Support USO offload in vnet header. > > drivers/net/virtio_net.c: Added USO support. > > > > Andrew Melnychenko (1): > > udp: allow header check for dodgy GSO_UDP_L4 packets. > > I don't see patches except 0 on list. > > > drivers/net/tap.c | 10 -- > > drivers/net/tun.c | 8 +++- > > drivers/net/virtio_net.c| 24 +--- > > include/linux/virtio_net.h | 9 + > > include/uapi/linux/if_tun.h | 2 ++ > > include/uapi/linux/virtio_net.h | 5 + > > net/ipv4/udp_offload.c | 3 ++- > > net/ipv6/udp_offload.c | 3 ++- > > 8 files changed, 56 insertions(+), 8 deletions(-) > > > > -- > > 2.38.1 > > > > ___ > > Virtualization mailing list > > Virtualization@lists.linux-foundation.org > > https://lists.linuxfoundation.org/mailman/listinfo/virtualization > > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 1/6] udp: allow header check for dodgy GSO_UDP_L4 packets.
Hi all, On Thu, Sep 8, 2022 at 3:40 AM David Ahern wrote: > > On 9/7/22 6:50 AM, Andrew Melnychenko wrote: > > diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c > > index 6d1a4bec2614..8e002419b4d5 100644 > > --- a/net/ipv4/udp_offload.c > > +++ b/net/ipv4/udp_offload.c > > @@ -387,7 +387,7 @@ static struct sk_buff *udp4_ufo_fragment(struct sk_buff > > *skb, > > if (!pskb_may_pull(skb, sizeof(struct udphdr))) > > goto out; > > > > - if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4) > > + if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4 && !skb_gso_ok(skb, > > features | NETIF_F_GSO_ROBUST)) > > that line needs to be wrapped. Ok, I'll wrap it. > > > return __udp_gso_segment(skb, features, false); > > > > mss = skb_shinfo(skb)->gso_size; > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 0/6] TUN/VirtioNet USO features support.
Hi all, On Thu, Sep 8, 2022 at 3:44 AM David Ahern wrote: > > On 9/7/22 6:50 AM, Andrew Melnychenko wrote: > > Added new offloads for TUN devices TUN_F_USO4 and TUN_F_USO6. > > Technically they enable NETIF_F_GSO_UDP_L4 > > (and only if USO4 & USO6 are set simultaneously). > > It allows the transmission of large UDP packets. > > Please spell out USO at least once in the cover letter to make sure the > acronym is clear. USO - UDP Segmentation Offload. Ability to split UDP packets into several segments. Allows sending/receiving big UDP packets. At some point, it looks like UFO(fragmentation), but each segment contains a UDP header. > > > > > Different features USO4 and USO6 are required for qemu where Windows guests > > can > > enable disable USO receives for IPv4 and IPv6 separately. > > On the other side, Linux can't really differentiate USO4 and USO6, for now. > > Why is that and what is needed for Linux to differentiate between the 2 > protocols? Well, this feature requires for Windows VM guest interaction. Windows may have a separate configuration for USO4/USO6. Currently, we support Windows guests with enabled USO4 and USO6 simultaneously. To implement this on Linux host, will require at least one additional netdev feature and changes in Linux network stack. Discussion about that will be in the future after some research. > > > For now, to enable USO for TUN it requires enabling USO4 and USO6 together. > > In the future, there would be a mechanism to control UDP_L4 GSO separately. > > > > New types for virtio-net already in virtio-net specification: > > https://github.com/oasis-tcs/virtio-spec/issues/120 > > > > Test it WIP Qemu https://github.com/daynix/qemu/tree/USOv3 > > > > Andrew (5): > > uapi/linux/if_tun.h: Added new offload types for USO4/6. > > driver/net/tun: Added features for USO. > > uapi/linux/virtio_net.h: Added USO types. > > linux/virtio_net.h: Support USO offload in vnet header. > > drivers/net/virtio_net.c: Added USO support. > > > > Andrew Melnychenko (1): > > udp: allow header check for dodgy GSO_UDP_L4 packets. > > > > drivers/net/tap.c | 10 -- > > drivers/net/tun.c | 8 +++- > > drivers/net/virtio_net.c| 19 +++ > > include/linux/virtio_net.h | 9 + > > include/uapi/linux/if_tun.h | 2 ++ > > include/uapi/linux/virtio_net.h | 5 + > > net/ipv4/udp_offload.c | 2 +- > > 7 files changed, 47 insertions(+), 8 deletions(-) > > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio_net: fix endian-ness for RSS
Reviewed-by: Andrew Melnychenko and...@daynix.com
On Fri, Aug 12, 2022 at 1:30 PM wrote:
>
> Hello:
>
> This patch was applied to netdev/net.git (master)
> by David S. Miller :
>
> On Thu, 11 Aug 2022 08:51:58 -0400 you wrote:
> > Using native endian-ness for device supplied fields is wrong
> > on BE platforms. Sparse warns about this.
> >
> > Fixes: 91f41f01d219 ("drivers/net/virtio_net: Added RSS hash report.")
> > Cc: "Andrew Melnychenko"
> > Signed-off-by: Michael S. Tsirkin
> >
> > [...]
>
> Here is the summary with links:
> - virtio_net: fix endian-ness for RSS
> https://git.kernel.org/netdev/net/c/95bb633048fa
>
> You are awesome, thank you!
> --
> Deet-doot-dot, I am a bot.
> https://korg.docs.kernel.org/patchwork/pwbot.html
>
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v2 0/5] TUN/VirtioNet USO features support.
Hi, Jason Apparently, your patch should work. For now, I have an issue where segmentation between two guests on one host still occurs. Neither previous "hack" nor your patch helps. Now I'm looking what the issue may be. If you have some suggestions on where may I look, it would be helpful, thanks! On Thu, May 26, 2022 at 3:18 PM Andrew Melnichenko wrote: > > I'll check it, thank you! > > On Thu, May 26, 2022 at 9:56 AM Jason Wang wrote: > > > > On Tue, May 24, 2022 at 7:07 PM Andrew Melnichenko > > wrote: > > > > > > Hi all, > > > > > > The issue is that host segments packets between guests on the same host. > > > Tests show that it happens because SKB_GSO_DODGY skb offload in > > > virtio_net_hdr_from_skb(). > > > To do segmentation you need to remove SKB_GSO_DODGY or add SKB_GSO_PARTIAL > > > The solution with DODGY/PARTIAL offload looks like a dirty hack, so > > > for now, I've lived it as it is for further investigation. > > > > Ok, I managed to find the previous discussion. It looks to me the > > reason is that __udp_gso_segment will segment dodgy packets > > unconditionally. > > > > I wonder if the attached patch works? (compile test only). > > > > Thanks > > > > > > > > > > > On Tue, May 17, 2022 at 9:32 AM Jason Wang wrote: > > > > > > > > On Thu, May 12, 2022 at 7:33 PM Andrew Melnychenko > > > > wrote: > > > > > > > > > > Added new offloads for TUN devices TUN_F_USO4 and TUN_F_USO6. > > > > > Technically they enable NETIF_F_GSO_UDP_L4 > > > > > (and only if USO4 & USO6 are set simultaneously). > > > > > It allows to transmission of large UDP packets. > > > > > > > > > > Different features USO4 and USO6 are required for qemu where Windows > > > > > guests can > > > > > enable disable USO receives for IPv4 and IPv6 separately. > > > > > On the other side, Linux can't really differentiate USO4 and USO6, > > > > > for now. > > > > > For now, to enable USO for TUN it requires enabling USO4 and USO6 > > > > > together. > > > > > In the future, there would be a mechanism to control UDP_L4 GSO > > > > > separately. > > > > > > > > > > Test it WIP Qemu https://github.com/daynix/qemu/tree/Dev_USOv2 > > > > > > > > > > New types for VirtioNet already on mailing: > > > > > https://lists.oasis-open.org/archives/virtio-comment/202110/msg00010.html > > > > > > > > > > Also, there is a known issue with transmitting packages between two > > > > > guests. > > > > > > > > Could you explain this more? It looks like a bug. (Or any pointer to > > > > the discussion) > > > > > > > > Thanks > > > > > > > > > Without hacks with skb's GSO - packages are still segmented on the > > > > > host's postrouting. > > > > > > > > > > Andrew (5): > > > > > uapi/linux/if_tun.h: Added new offload types for USO4/6. > > > > > driver/net/tun: Added features for USO. > > > > > uapi/linux/virtio_net.h: Added USO types. > > > > > linux/virtio_net.h: Support USO offload in vnet header. > > > > > drivers/net/virtio_net.c: Added USO support. > > > > > > > > > > drivers/net/tap.c | 10 -- > > > > > drivers/net/tun.c | 8 +++- > > > > > drivers/net/virtio_net.c| 19 +++ > > > > > include/linux/virtio_net.h | 9 + > > > > > include/uapi/linux/if_tun.h | 2 ++ > > > > > include/uapi/linux/virtio_net.h | 4 > > > > > 6 files changed, 45 insertions(+), 7 deletions(-) > > > > > > > > > > -- > > > > > 2.35.1 > > > > > > > > > > > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v2 0/5] TUN/VirtioNet USO features support.
I'll check it, thank you! On Thu, May 26, 2022 at 9:56 AM Jason Wang wrote: > > On Tue, May 24, 2022 at 7:07 PM Andrew Melnichenko wrote: > > > > Hi all, > > > > The issue is that host segments packets between guests on the same host. > > Tests show that it happens because SKB_GSO_DODGY skb offload in > > virtio_net_hdr_from_skb(). > > To do segmentation you need to remove SKB_GSO_DODGY or add SKB_GSO_PARTIAL > > The solution with DODGY/PARTIAL offload looks like a dirty hack, so > > for now, I've lived it as it is for further investigation. > > Ok, I managed to find the previous discussion. It looks to me the > reason is that __udp_gso_segment will segment dodgy packets > unconditionally. > > I wonder if the attached patch works? (compile test only). > > Thanks > > > > > > > On Tue, May 17, 2022 at 9:32 AM Jason Wang wrote: > > > > > > On Thu, May 12, 2022 at 7:33 PM Andrew Melnychenko > > > wrote: > > > > > > > > Added new offloads for TUN devices TUN_F_USO4 and TUN_F_USO6. > > > > Technically they enable NETIF_F_GSO_UDP_L4 > > > > (and only if USO4 & USO6 are set simultaneously). > > > > It allows to transmission of large UDP packets. > > > > > > > > Different features USO4 and USO6 are required for qemu where Windows > > > > guests can > > > > enable disable USO receives for IPv4 and IPv6 separately. > > > > On the other side, Linux can't really differentiate USO4 and USO6, for > > > > now. > > > > For now, to enable USO for TUN it requires enabling USO4 and USO6 > > > > together. > > > > In the future, there would be a mechanism to control UDP_L4 GSO > > > > separately. > > > > > > > > Test it WIP Qemu https://github.com/daynix/qemu/tree/Dev_USOv2 > > > > > > > > New types for VirtioNet already on mailing: > > > > https://lists.oasis-open.org/archives/virtio-comment/202110/msg00010.html > > > > > > > > Also, there is a known issue with transmitting packages between two > > > > guests. > > > > > > Could you explain this more? It looks like a bug. (Or any pointer to > > > the discussion) > > > > > > Thanks > > > > > > > Without hacks with skb's GSO - packages are still segmented on the > > > > host's postrouting. > > > > > > > > Andrew (5): > > > > uapi/linux/if_tun.h: Added new offload types for USO4/6. > > > > driver/net/tun: Added features for USO. > > > > uapi/linux/virtio_net.h: Added USO types. > > > > linux/virtio_net.h: Support USO offload in vnet header. > > > > drivers/net/virtio_net.c: Added USO support. > > > > > > > > drivers/net/tap.c | 10 -- > > > > drivers/net/tun.c | 8 +++- > > > > drivers/net/virtio_net.c| 19 +++ > > > > include/linux/virtio_net.h | 9 + > > > > include/uapi/linux/if_tun.h | 2 ++ > > > > include/uapi/linux/virtio_net.h | 4 > > > > 6 files changed, 45 insertions(+), 7 deletions(-) > > > > > > > > -- > > > > 2.35.1 > > > > > > > > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v2 0/5] TUN/VirtioNet USO features support.
Hi all, The issue is that host segments packets between guests on the same host. Tests show that it happens because SKB_GSO_DODGY skb offload in virtio_net_hdr_from_skb(). To do segmentation you need to remove SKB_GSO_DODGY or add SKB_GSO_PARTIAL The solution with DODGY/PARTIAL offload looks like a dirty hack, so for now, I've lived it as it is for further investigation. On Tue, May 17, 2022 at 9:32 AM Jason Wang wrote: > > On Thu, May 12, 2022 at 7:33 PM Andrew Melnychenko wrote: > > > > Added new offloads for TUN devices TUN_F_USO4 and TUN_F_USO6. > > Technically they enable NETIF_F_GSO_UDP_L4 > > (and only if USO4 & USO6 are set simultaneously). > > It allows to transmission of large UDP packets. > > > > Different features USO4 and USO6 are required for qemu where Windows guests > > can > > enable disable USO receives for IPv4 and IPv6 separately. > > On the other side, Linux can't really differentiate USO4 and USO6, for now. > > For now, to enable USO for TUN it requires enabling USO4 and USO6 together. > > In the future, there would be a mechanism to control UDP_L4 GSO separately. > > > > Test it WIP Qemu https://github.com/daynix/qemu/tree/Dev_USOv2 > > > > New types for VirtioNet already on mailing: > > https://lists.oasis-open.org/archives/virtio-comment/202110/msg00010.html > > > > Also, there is a known issue with transmitting packages between two guests. > > Could you explain this more? It looks like a bug. (Or any pointer to > the discussion) > > Thanks > > > Without hacks with skb's GSO - packages are still segmented on the host's > > postrouting. > > > > Andrew (5): > > uapi/linux/if_tun.h: Added new offload types for USO4/6. > > driver/net/tun: Added features for USO. > > uapi/linux/virtio_net.h: Added USO types. > > linux/virtio_net.h: Support USO offload in vnet header. > > drivers/net/virtio_net.c: Added USO support. > > > > drivers/net/tap.c | 10 -- > > drivers/net/tun.c | 8 +++- > > drivers/net/virtio_net.c| 19 +++ > > include/linux/virtio_net.h | 9 + > > include/uapi/linux/if_tun.h | 2 ++ > > include/uapi/linux/virtio_net.h | 4 > > 6 files changed, 45 insertions(+), 7 deletions(-) > > > > -- > > 2.35.1 > > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v4 3/4] drivers/net/virtio_net: Added RSS hash report.
Hi all,
Yes, I'll prepare a new commit later.
On Fri, Mar 4, 2022 at 10:08 AM Michael S. Tsirkin wrote:
>
> On Wed, Feb 23, 2022 at 03:15:28AM +0800, kernel test robot wrote:
> > Hi Andrew,
> >
> > Thank you for the patch! Perhaps something to improve:
> >
> > [auto build test WARNING on mst-vhost/linux-next]
> > [also build test WARNING on net/master horms-ipvs/master net-next/master
> > linus/master v5.17-rc5 next-20220217]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch]
>
>
> Andrew,
> do you plan to fix this?
>
> > url:
> > https://github.com/0day-ci/linux/commits/Andrew-Melnychenko/RSS-support-for-VirtioNet/20220222-200334
> > base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git
> > linux-next
> > config: i386-randconfig-s002-20220221
> > (https://download.01.org/0day-ci/archive/20220223/202202230342.hpye6dha-...@intel.com/config)
> > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> > reproduce:
> > # apt-get install sparse
> > # sparse version: v0.6.4-dirty
> > #
> > https://github.com/0day-ci/linux/commit/4fda71c17afd24d8afb675baa0bb14dbbc6cd23c
> > git remote add linux-review https://github.com/0day-ci/linux
> > git fetch --no-tags linux-review
> > Andrew-Melnychenko/RSS-support-for-VirtioNet/20220222-200334
> > git checkout 4fda71c17afd24d8afb675baa0bb14dbbc6cd23c
> > # save the config file to linux build tree
> > mkdir build_dir
> > make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
> > O=build_dir ARCH=i386 SHELL=/bin/bash
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot
> >
> >
> > sparse warnings: (new ones prefixed by >>)
> >drivers/net/virtio_net.c:1160:25: sparse: sparse: restricted __le16
> > degrades to integer
> >drivers/net/virtio_net.c:1160:25: sparse: sparse: restricted __le16
> > degrades to integer
> >drivers/net/virtio_net.c:1160:25: sparse: sparse: restricted __le16
> > degrades to integer
> >drivers/net/virtio_net.c:1160:25: sparse: sparse: restricted __le16
> > degrades to integer
> >drivers/net/virtio_net.c:1160:25: sparse: sparse: restricted __le16
> > degrades to integer
> >drivers/net/virtio_net.c:1160:25: sparse: sparse: restricted __le16
> > degrades to integer
> >drivers/net/virtio_net.c:1160:25: sparse: sparse: restricted __le16
> > degrades to integer
> >drivers/net/virtio_net.c:1160:25: sparse: sparse: restricted __le16
> > degrades to integer
> >drivers/net/virtio_net.c:1160:25: sparse: sparse: restricted __le16
> > degrades to integer
> > >> drivers/net/virtio_net.c:1178:35: sparse: sparse: incorrect type in
> > >> argument 2 (different base types) @@ expected unsigned int
> > >> [usertype] hash @@ got restricted __le32 const [usertype] hash_value
> > >> @@
> >drivers/net/virtio_net.c:1178:35: sparse: expected unsigned int
> > [usertype] hash
> >drivers/net/virtio_net.c:1178:35: sparse: got restricted __le32
> > const [usertype] hash_value
> >
> > vim +1178 drivers/net/virtio_net.c
> >
> > 1151
> > 1152static void virtio_skb_set_hash(const struct
> > virtio_net_hdr_v1_hash *hdr_hash,
> > 1153struct sk_buff *skb)
> > 1154{
> > 1155enum pkt_hash_types rss_hash_type;
> > 1156
> > 1157if (!hdr_hash || !skb)
> > 1158return;
> > 1159
> > 1160switch (hdr_hash->hash_report) {
> > 1161case VIRTIO_NET_HASH_REPORT_TCPv4:
> > 1162case VIRTIO_NET_HASH_REPORT_UDPv4:
> > 1163case VIRTIO_NET_HASH_REPORT_TCPv6:
> > 1164case VIRTIO_NET_HASH_REPORT_UDPv6:
> > 1165case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
> > 1166case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
> > 1167rss_hash_type = PKT_HASH_TYPE_L4;
> > 1168break;
> > 1169case VIRTIO_NET_HASH_REPORT_IPv4:
> > 1170case VIRTIO_NET_HASH_REPORT_IPv6:
> > 1171case VIRTIO_NET_HASH_REPORT_IPv6_EX:
> > 1172rss_hash_type = PKT_HASH_TYPE_L3;
> > 1173break;
> > 1174case VIRTIO_NET_HASH_REPORT_NONE:
> > 1175default:
> > 1176rss_hash_type = PKT_HASH_TYPE_NONE;
> > 1177}
> > > 1178skb_set_hash(skb, hdr_hash->hash_value,
> > > rss_hash_type);
> > 1179}
> > 1180
> >
> > ---
> > 0-DAY CI Kernel Test Service, Intel Corporation
> > https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
>
___
Virtualization mailing list
Re: [RFC PATCH 1/5] uapi/linux/if_tun.h: Added new ioctl for tun/tap.
Hi all,
On Wed, Feb 9, 2022 at 6:26 AM Jason Wang wrote:
>
>
> 在 2022/1/25 下午4:46, Andrew Melnychenko 写道:
> > Added TUNGETSUPPORTEDOFFLOADS that should allow
> > to get bits of supported offloads.
>
>
> So we don't use dedicated ioctls in the past, instead, we just probing
> by checking the return value of TUNSETOFFLOADS.
>
> E.g qemu has the following codes:
>
> int tap_probe_has_ufo(int fd)
> {
> unsigned offload;
>
> offload = TUN_F_CSUM | TUN_F_UFO;
>
> if (ioctl(fd, TUNSETOFFLOAD, offload) < 0)
> return 0;
>
> return 1;
> }
>
> Any reason we can't keep using that?
>
> Thanks
>
Well, even in this example. To check the ufo feature, we trying to set it.
What if we don't need to "enable" UFO and/or do not change its state?
I think it's a good idea to have the ability to get supported offloads
without changing device behavior.
>
> > Added 2 additional offlloads for USO(IPv4 & IPv6).
> > Separate offloads are required for Windows VM guests,
> > g.e. Windows may set USO rx only for IPv4.
> >
> > Signed-off-by: Andrew Melnychenko
> > ---
> > include/uapi/linux/if_tun.h | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
> > index 454ae31b93c7..07680fae6e18 100644
> > --- a/include/uapi/linux/if_tun.h
> > +++ b/include/uapi/linux/if_tun.h
> > @@ -61,6 +61,7 @@
> > #define TUNSETFILTEREBPF _IOR('T', 225, int)
> > #define TUNSETCARRIER _IOW('T', 226, int)
> > #define TUNGETDEVNETNS _IO('T', 227)
> > +#define TUNGETSUPPORTEDOFFLOADS _IOR('T', 228, unsigned int)
> >
> > /* TUNSETIFF ifr flags */
> > #define IFF_TUN 0x0001
> > @@ -88,6 +89,8 @@
> > #define TUN_F_TSO6 0x04/* I can handle TSO for IPv6 packets */
> > #define TUN_F_TSO_ECN 0x08/* I can handle TSO with ECN bits. */
> > #define TUN_F_UFO 0x10/* I can handle UFO packets */
> > +#define TUN_F_USO4 0x20/* I can handle USO for IPv4 packets */
> > +#define TUN_F_USO6 0x40/* I can handle USO for IPv6 packets */
> >
> > /* Protocol info prepended to the packets (when IFF_NO_PI is not set) */
> > #define TUN_PKT_STRIP 0x0001
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 2/5] driver/net/tun: Added features for USO.
On Wed, Feb 9, 2022 at 6:39 AM Jason Wang wrote:
>
>
> 在 2022/1/25 下午4:46, Andrew Melnychenko 写道:
> > Added support for USO4 and USO6, also added code for new ioctl
> > TUNGETSUPPORTEDOFFLOADS.
> > For now, to "enable" USO, it's required to set both USO4 and USO6
> > simultaneously.
> > USO enables NETIF_F_GSO_UDP_L4.
> >
> > Signed-off-by: Andrew Melnychenko
> > ---
> > drivers/net/tap.c | 18 --
> > drivers/net/tun.c | 15 ++-
> > 2 files changed, 30 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> > index 8e3a28ba6b28..82d742ba78b1 100644
> > --- a/drivers/net/tap.c
> > +++ b/drivers/net/tap.c
> > @@ -940,6 +940,10 @@ static int set_offload(struct tap_queue *q, unsigned
> > long arg)
> > if (arg & TUN_F_TSO6)
> > feature_mask |= NETIF_F_TSO6;
> > }
> > +
> > + /* TODO: for now USO4 and USO6 should work simultaneously */
> > + if (arg & (TUN_F_USO4 | TUN_F_USO6) == (TUN_F_USO4 |
> > TUN_F_USO6))
> > + features |= NETIF_F_GSO_UDP_L4;
>
>
> If kernel doesn't want to split the GSO_UDP features, I wonder how much
> value to keep separated features for TUN and virtio.
>
> Thanks
>
It's important for Windows guests that may request USO receive only
for IPv4 or IPv6.
Or there is possible to implement one feature and change its
"meanings" when "split" happens.
I think it's a good idea to implement an interface for iUSO4/USO6 and
do it right away.
>
> > }
> >
> > /* tun/tap driver inverts the usage for TSO offloads, where
> > @@ -950,7 +954,8 @@ static int set_offload(struct tap_queue *q, unsigned
> > long arg)
> >* When user space turns off TSO, we turn off GSO/LRO so that
> >* user-space will not receive TSO frames.
> >*/
> > - if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6))
> > + if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6) ||
> > + feature_mask & (TUN_F_USO4 | TUN_F_USO6) == (TUN_F_USO4 |
> > TUN_F_USO6))
> > features |= RX_OFFLOADS;
> > else
> > features &= ~RX_OFFLOADS;
> > @@ -979,6 +984,7 @@ static long tap_ioctl(struct file *file, unsigned int
> > cmd,
> > unsigned short u;
> > int __user *sp = argp;
> > struct sockaddr sa;
> > + unsigned int supported_offloads;
> > int s;
> > int ret;
> >
> > @@ -1074,7 +1080,8 @@ static long tap_ioctl(struct file *file, unsigned int
> > cmd,
> > case TUNSETOFFLOAD:
> > /* let the user check for future flags */
> > if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
> > - TUN_F_TSO_ECN | TUN_F_UFO))
> > + TUN_F_TSO_ECN | TUN_F_UFO |
> > + TUN_F_USO4 | TUN_F_USO6))
> > return -EINVAL;
> >
> > rtnl_lock();
> > @@ -1082,6 +1089,13 @@ static long tap_ioctl(struct file *file, unsigned
> > int cmd,
> > rtnl_unlock();
> > return ret;
> >
> > + case TUNGETSUPPORTEDOFFLOADS:
> > + supported_offloads = TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
> > + TUN_F_TSO_ECN | TUN_F_UFO |
> > TUN_F_USO4 | TUN_F_USO6;
> > + if (copy_to_user(, _offloads,
> > sizeof(supported_offloads)))
> > + return -EFAULT;
> > + return 0;
> > +
> > case SIOCGIFHWADDR:
> > rtnl_lock();
> > tap = tap_get_tap_dev(q);
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index fed85447701a..4f2105d1e6f1 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -185,7 +185,7 @@ struct tun_struct {
> > struct net_device *dev;
> > netdev_features_t set_features;
> > #define TUN_USER_FEATURES (NETIF_F_HW_CSUM|NETIF_F_TSO_ECN|NETIF_F_TSO| \
> > - NETIF_F_TSO6)
> > + NETIF_F_TSO6 | NETIF_F_GSO_UDP_L4)
> >
> > int align;
> > int vnet_hdr_sz;
> > @@ -2821,6 +2821,12 @@ static int set_offload(struct tun_struct *tun,
> > unsigned long arg)
> > }
> >
> > arg &= ~TUN_F_UFO;
> > +
> > + /* TODO: for now USO4 and USO6 should work simultaneously */
> > + if (arg & TUN_F_USO4 && arg & TUN_F_USO6) {
> > + features |= NETIF_F_GSO_UDP_L4;
> > + arg &= ~(TUN_F_USO4 | TUN_F_USO6);
> > + }
> > }
> >
> > /* This gives the user a way to test for new features in future by
> > @@ -2991,6 +2997,7 @@ static long __tun_chr_ioctl(struct file *file,
> > unsigned int cmd,
> > int sndbuf;
> > int vnet_hdr_sz;
> > int le;
> > + unsigned int supported_offloads;
> > int ret;
> > bool do_notify = false;
> >
> > @@ -3154,6 +3161,12 @@ static long
Re: [RFC PATCH 3/5] uapi/linux/virtio_net.h: Added USO types.
Hi all,
On Wed, Feb 9, 2022 at 6:41 AM Jason Wang wrote:
>
>
> 在 2022/1/25 下午4:47, Andrew Melnychenko 写道:
> > Added new GSO type for USO: VIRTIO_NET_HDR_GSO_UDP_L4.
> > Feature VIRTIO_NET_F_HOST_USO allows to enable NETIF_F_GSO_UDP_L4.
> > Separated VIRTIO_NET_F_GUEST_USO4 & VIRTIO_NET_F_GUEST_USO6 features
> > required for Windows guests.
> >
> > Signed-off-by: Andrew Melnychenko
> > ---
> > include/uapi/linux/virtio_net.h | 4
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/include/uapi/linux/virtio_net.h
> > b/include/uapi/linux/virtio_net.h
> > index 3f55a4215f11..620addc5767b 100644
> > --- a/include/uapi/linux/virtio_net.h
> > +++ b/include/uapi/linux/virtio_net.h
> > @@ -56,6 +56,9 @@
> > #define VIRTIO_NET_F_MQ 22 /* Device supports Receive Flow
> >* Steering */
> > #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
> > +#define VIRTIO_NET_F_GUEST_USO4 54 /* Guest can handle USOv4 in.
> > */
> > +#define VIRTIO_NET_F_GUEST_USO6 55 /* Guest can handle USOv6 in.
> > */
> > +#define VIRTIO_NET_F_HOST_USO56 /* Host can handle USO in. */
>
>
> I think it's better to be consistent here. Either we split in both guest
> and host or not.
>
> Thanks
>
The main reason that receives USO packets depends on the kernel, where
transmitting the feature that VirtIO implements.
Windows systems have the option to manipulate receive offload. That's
why there are two GUEST_USO features.
For HOST_USO - technically there is no point in "split" it, and there
is should not be any difference between IPv4/IPv6.
Technically, we either support transmitting big UDP packets or not.
>
> >
> > #define VIRTIO_NET_F_HASH_REPORT 57/* Supports hash report */
> > #define VIRTIO_NET_F_RSS 60/* Supports RSS RX steering */
> > @@ -130,6 +133,7 @@ struct virtio_net_hdr_v1 {
> > #define VIRTIO_NET_HDR_GSO_TCPV41 /* GSO frame, IPv4 TCP (TSO)
> > */
> > #define VIRTIO_NET_HDR_GSO_UDP 3 /* GSO frame, IPv4
> > UDP (UFO) */
> > #define VIRTIO_NET_HDR_GSO_TCPV64 /* GSO frame, IPv6 TCP */
> > +#define VIRTIO_NET_HDR_GSO_UDP_L45 /* GSO frame, IPv4 & IPv6 UDP
> > (USO) */
> > #define VIRTIO_NET_HDR_GSO_ECN 0x80/* TCP has ECN set */
> > __u8 gso_type;
> > __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 0/5] TUN/VirtioNet USO features support.
Hi all, On Wed, Feb 9, 2022 at 7:41 AM Jason Wang wrote: > > > 在 2022/2/8 下午9:09, Andrew Melnichenko 写道: > > Hi people, > > Can you please review this series? > > > Are there any performance number to demonstrate the difference? > > Thanks > Yeah, I've used udpgso_bench from Linux to test. Here are some numbers: Sending packets with size 1 Without USO: ``` $ ./udpgso_bench_tx -4 -D 192.168.15.1 -s 1 -S 1000 random: crng init done random: 7 urandom warning(s) missed due to ratelimiting udp tx: 36 MB/s 3863 calls/s 3863 msg/s udp tx: 32 MB/s 3360 calls/s 3360 msg/s udp tx: 31 MB/s 3340 calls/s 3340 msg/s udp tx: 31 MB/s 3353 calls/s 3353 msg/s udp tx: 32 MB/s 3359 calls/s 3359 msg/s udp tx: 32 MB/s 3370 calls/s 3370 msg/s ``` With USO: ``` $ ./udpgso_bench_tx -4 -D 192.168.15.1 -s 1 -S 1000 random: crng init done random: 7 urandom warning(s) missed due to ratelimiting udp tx:120 MB/s12596 calls/s 12596 msg/s udp tx:122 MB/s12885 calls/s 12885 msg/s udp tx:120 MB/s12667 calls/s 12667 msg/s udp tx:123 MB/s12969 calls/s 12969 msg/s udp tx:116 MB/s12232 calls/s 12232 msg/s udp tx:108 MB/s11389 calls/s 11389 msg/s ``` > > > > > On Wed, Jan 26, 2022 at 10:32 AM Yuri Benditovich > > wrote: > >> On Wed, Jan 26, 2022 at 9:54 AM Xuan Zhuo > >> wrote: > >>> On Tue, 25 Jan 2022 10:46:57 +0200, Andrew Melnychenko > >>> wrote: > >>>> Added new offloads for TUN devices TUN_F_USO4 and TUN_F_USO6. > >>>> Technically they enable NETIF_F_GSO_UDP_L4 > >>>> (and only if USO4 & USO6 are set simultaneously). > >>>> It allows to transmission of large UDP packets. > >>>> > >>>> Different features USO4 and USO6 are required for qemu where Windows > >>>> guests can > >>>> enable disable USO receives for IPv4 and IPv6 separately. > >>>> On the other side, Linux can't really differentiate USO4 and USO6, for > >>>> now. > >>>> For now, to enable USO for TUN it requires enabling USO4 and USO6 > >>>> together. > >>>> In the future, there would be a mechanism to control UDP_L4 GSO > >>>> separately. > >>>> > >>>> Test it WIP Qemu https://github.com/daynix/qemu/tree/Dev_USOv2 > >>>> > >>>> New types for VirtioNet already on mailing: > >>>> https://lists.oasis-open.org/archives/virtio-comment/202110/msg00010.html > >>> Seems like this hasn't been upvoted yet. > >>> > >>> https://github.com/oasis-tcs/virtio-spec#use-of-github-issues > >> Yes, correct. This is a reason why this series of patches is RFC. > >> > >>> Thanks. > >>> > >>>> Also, there is a known issue with transmitting packages between two > >>>> guests. > >>>> Without hacks with skb's GSO - packages are still segmented on the > >>>> host's postrouting. > >>>> > >>>> Andrew Melnychenko (5): > >>>>uapi/linux/if_tun.h: Added new ioctl for tun/tap. > >>>>driver/net/tun: Added features for USO. > >>>>uapi/linux/virtio_net.h: Added USO types. > >>>>linux/virtio_net.h: Added Support for GSO_UDP_L4 offload. > >>>>drivers/net/virtio_net.c: Added USO support. > >>>> > >>>> drivers/net/tap.c | 18 -- > >>>> drivers/net/tun.c | 15 ++- > >>>> drivers/net/virtio_net.c| 22 ++ > >>>> include/linux/virtio_net.h | 11 +++ > >>>> include/uapi/linux/if_tun.h | 3 +++ > >>>> include/uapi/linux/virtio_net.h | 4 > >>>> 6 files changed, 66 insertions(+), 7 deletions(-) > >>>> > >>>> -- > >>>> 2.34.1 > >>>> > >>>> ___ > >>>> Virtualization mailing list > >>>> Virtualization@lists.linux-foundation.org > >>>> https://lists.linuxfoundation.org/mailman/listinfo/virtualization > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 2/4] drivers/net/virtio_net: Added basic RSS support.
Hi all,
On Mon, Feb 14, 2022 at 12:09 AM Willem de Bruijn
wrote:
>
> > > > @@ -3113,13 +3270,14 @@ static int virtnet_probe(struct virtio_device
> > > > *vdev)
> > > > u16 max_queue_pairs;
> > > > int mtu;
> > > >
> > > > - /* Find if host supports multiqueue virtio_net device */
> > > > - err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
> > > > - struct virtio_net_config,
> > > > - max_virtqueue_pairs,
> > > > _queue_pairs);
> > > > + /* Find if host supports multiqueue/rss virtio_net device */
> > > > + max_queue_pairs = 1;
> > > > + if (virtio_has_feature(vdev, VIRTIO_NET_F_MQ) ||
> > > > virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
> > > > + max_queue_pairs =
> > > > +virtio_cread16(vdev, offsetof(struct
> > > > virtio_net_config, max_virtqueue_pairs));
> > >
> > > Instead of testing either feature and treating them as somewhat equal,
> > > shouldn't RSS be dependent on MQ?
> >
> > No, RSS is dependent on CTRL_VQ. Technically RSS and MQ are similar
> > features.
>
> RSS depends on having multiple queues.
>
> What would enabling VIRTIO_NET_F_RSS without VIRTIO_NET_F_MQ do?
RSS would work.
According to virtio spec article 5.1.6.5.5:
> A device MAY support one of these features or both. The driver MAY negotiate
> any set of these features
> that the device supports.
Also, in 5.1.3.1:
> VIRTIO_NET_F_RSS Requires VIRTIO_NET_F_CTRL_VQ.
>
> > >
> > > >
> > > > /* We need at least 2 queue's */
> > > > - if (err || max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
> > > > + if (max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
> > > > max_queue_pairs > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX ||
> > > > !virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
> > > > max_queue_pairs = 1;
> > > > @@ -3207,6 +3365,23 @@ static int virtnet_probe(struct virtio_device
> > > > *vdev)
> > > > if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
> > > > vi->mergeable_rx_bufs = true;
> > > >
> > > > + if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> > > > + vi->has_rss = true;
> > > > + vi->rss_indir_table_size =
> > > > + virtio_cread16(vdev, offsetof(struct
> > > > virtio_net_config,
> > > > + rss_max_indirection_table_length));
> > > > + vi->rss_key_size =
> > > > + virtio_cread8(vdev, offsetof(struct
> > > > virtio_net_config, rss_max_key_size));
> > > > +
> > > > + vi->rss_hash_types_supported =
> > > > + virtio_cread32(vdev, offsetof(struct
> > > > virtio_net_config, supported_hash_types));
> > > > + vi->rss_hash_types_supported &=
> > > > + ~(VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
> > > > + VIRTIO_NET_RSS_HASH_TYPE_TCP_EX |
> > > > + VIRTIO_NET_RSS_HASH_TYPE_UDP_EX);
> > > > +
> > > > + dev->hw_features |= NETIF_F_RXHASH;
> > >
> > > Only make the feature visible when the hash is actually reported in
> > > the skb, patch 3.
> >
> > VirtioNET has two features: RSS(steering only) and hash(hash report in
> > vnet header)
> > Both features may be enabled/disabled separately:
> > 1. rss on and hash off - packets steered to the corresponding vqs
> > 2. rss off and hash on - packets steered by tap(like mq) but headers
> > have properly calculated hash.
> > 3. rss on and hash on - packets steered to corresponding vqs and hash
> > is present in the header.
> >
> > RXHASH feature allows the user to enable/disable the rss/hash(any
> > combination).
>
> I find that confusing, but.. I see that there is prior art where some
> drivers enable/disable entire RSS load balancing based on this flag.
> So ok.
>
> > I think it's a good idea to leave RXHASH in patch 2/4 to give the user
> > ability to manipulate the rss only feature.
> > But, if you think that it requires to move it to the 3/4, I'll do it.
> >
> > >
> > > Also, clearly separate the feature patches (2) rss, (3) rxhash, (4)
> > > rxhash config.
> >
> > Currently:
> > Patch 2/4 - adds VirtioNet rss feature.
> > Patch 3/4 - adds VirtioNet hash report feature.
> > Patch 4/4 - adds the ability to manipulate supported hash types.
> >
> > Can you provide more detailed suggestions on how to move hunks?
>
> I gave one in the follow-on patch, to which you responded. That's probably it.
I'll add zero size table check and move hunk for padded header length
from 3/4 to 1/4.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 4/4] drivers/net/virtio_net: Added RSS hash report control.
Hi all,
On Tue, Feb 8, 2022 at 10:59 PM Willem de Bruijn
wrote:
>
> On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko wrote:
> >
> > Now it's possible to control supported hashflows.
> > Added hashflow set/get callbacks.
> > Also, disabling RXH_IP_SRC/DST for TCP would disable then for UDP.
>
> I don't follow this comment. Can you elaborate?
I'll rephrase it in next version of patches.
The idea is that VirtioNet RSS doesn't distinguish IP hashes between
TCP and UDP.
For TCP and UDP it's possible to set IP+PORT hashes.
But disabling IP hashes will disable them for TCP and UDP simultaneously.
It's possible to set IP+PORT for TCP and IP for everything else(UDP, ICMP etc.)
>
> > TCP and UDP supports only:
> > ethtool -U eth0 rx-flow-hash tcp4 sd
> > RXH_IP_SRC + RXH_IP_DST
> > ethtool -U eth0 rx-flow-hash tcp4 sdfn
> > RXH_IP_SRC + RXH_IP_DST + RXH_L4_B_0_1 + RXH_L4_B_2_3
> >
> > Signed-off-by: Andrew Melnychenko
> > ---
> > drivers/net/virtio_net.c | 141 ++-
> > 1 file changed, 140 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 543da2fbdd2d..88759d5e693c 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -231,6 +231,7 @@ struct virtnet_info {
> > u8 rss_key_size;
> > u16 rss_indir_table_size;
> > u32 rss_hash_types_supported;
> > + u32 rss_hash_types_saved;
>
> hash_types_active?
I think "hash_types_saved" is more suitable for the current field.
Idea is that the user may disable RSS/HASH and we need to save
what hash type configurations previously were enabled.
So, we can restore it when the user will enable RSS/HASH back.
>
> > +static bool virtnet_set_hashflow(struct virtnet_info *vi, struct
> > ethtool_rxnfc *info)
> > +{
> > + u32 new_hashtypes = vi->rss_hash_types_saved;
> > + bool is_disable = info->data & RXH_DISCARD;
> > + bool is_l4 = info->data == (RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1
> > | RXH_L4_B_2_3);
> > +
> > + /* supports only 'sd', 'sdfn' and 'r' */
> > + if (!((info->data == (RXH_IP_SRC | RXH_IP_DST)) | is_l4 |
> > is_disable))
>
> maybe add an is_l3
There used to be "is_l3", but that variable was used only in that
condition statement.
So I've decided to inplace it.
>
> > + return false;
> > +
> > + switch (info->flow_type) {
> > + case TCP_V4_FLOW:
> > + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv4 |
> > VIRTIO_NET_RSS_HASH_TYPE_TCPv4);
> > + if (!is_disable)
> > + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4
> > + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_TCPv4 :
> > 0);
> > + break;
> > + case UDP_V4_FLOW:
> > + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv4 |
> > VIRTIO_NET_RSS_HASH_TYPE_UDPv4);
> > + if (!is_disable)
> > + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4
> > + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_UDPv4 :
> > 0);
> > + break;
> > + case IPV4_FLOW:
> > + new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> > + if (!is_disable)
> > + new_hashtypes = VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> > + break;
> > + case TCP_V6_FLOW:
> > + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv6 |
> > VIRTIO_NET_RSS_HASH_TYPE_TCPv6);
> > + if (!is_disable)
> > + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6
> > + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_TCPv6 :
> > 0);
> > + break;
> > + case UDP_V6_FLOW:
> > + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv6 |
> > VIRTIO_NET_RSS_HASH_TYPE_UDPv6);
> > + if (!is_disable)
> > + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6
> > + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_UDPv6 :
> > 0);
> > + break;
> > + case IPV6_FLOW:
> > + new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> > + if (!is_disable)
> > + new_hashtypes = VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> > + break;
> > + default:
> > + /* unsupported flow */
> > + return false;
> > + }
> > +
> > + /* if unsupported hashtype was set */
> > + if (new_hashtypes != (new_hashtypes & vi->rss_hash_types_supported))
> > + return false;
> > +
> > + if (new_hashtypes != vi->rss_hash_types_saved) {
> > + vi->rss_hash_types_saved = new_hashtypes;
>
> should only be updated if the commit function returned success?
Not really, we already made all checks against "supported" hash types.
Also, the commit function may not be called if RSS is disabled by the user.
Re: [PATCH v3 3/4] drivers/net/virtio_net: Added RSS hash report.
Hi all,
On Tue, Feb 8, 2022 at 10:55 PM Willem de Bruijn
wrote:
>
> On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko wrote:
> >
> > Added features for RSS hash report.
> > If hash is provided - it sets to skb.
> > Added checks if rss and/or hash are enabled together.
> >
> > Signed-off-by: Andrew Melnychenko
> > ---
> > drivers/net/virtio_net.c | 51 ++--
> > 1 file changed, 44 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 495aed524e33..543da2fbdd2d 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -227,6 +227,7 @@ struct virtnet_info {
> >
> > /* Host supports rss and/or hash report */
> > bool has_rss;
> > + bool has_rss_hash_report;
> > u8 rss_key_size;
> > u16 rss_indir_table_size;
> > u32 rss_hash_types_supported;
> > @@ -421,7 +422,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info
> > *vi,
> >
> > hdr_len = vi->hdr_len;
> > if (vi->mergeable_rx_bufs)
> > - hdr_padded_len = sizeof(*hdr);
> > + hdr_padded_len = hdr_len;
>
> Belongs in patch 1?
Yeah, I'll move it.
>
> > else
> > hdr_padded_len = sizeof(struct padded_vnet_hdr);
> >
> > @@ -1156,6 +1157,8 @@ static void receive_buf(struct virtnet_info *vi,
> > struct receive_queue *rq,
> > struct net_device *dev = vi->dev;
> > struct sk_buff *skb;
> > struct virtio_net_hdr_mrg_rxbuf *hdr;
> > + struct virtio_net_hdr_v1_hash *hdr_hash;
> > + enum pkt_hash_types rss_hash_type;
> >
> > if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
> > pr_debug("%s: short packet %i\n", dev->name, len);
> > @@ -1182,6 +1185,29 @@ static void receive_buf(struct virtnet_info *vi,
> > struct receive_queue *rq,
> > return;
> >
> > hdr = skb_vnet_hdr(skb);
> > + if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report) {
>
> Can the first be true if the second is not?
Yes, RSS may be enabled, but the hash report feature is disabled.
For now, it's possible to enable/disable VirtioNet RSS by manipulating RXHASH.
>
> > + hdr_hash = (struct virtio_net_hdr_v1_hash *)(hdr);
> > +
> > + switch (hdr_hash->hash_report) {
> > + case VIRTIO_NET_HASH_REPORT_TCPv4:
> > + case VIRTIO_NET_HASH_REPORT_UDPv4:
> > + case VIRTIO_NET_HASH_REPORT_TCPv6:
> > + case VIRTIO_NET_HASH_REPORT_UDPv6:
> > + case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
> > + case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
> > + rss_hash_type = PKT_HASH_TYPE_L4;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_IPv4:
> > + case VIRTIO_NET_HASH_REPORT_IPv6:
> > + case VIRTIO_NET_HASH_REPORT_IPv6_EX:
> > + rss_hash_type = PKT_HASH_TYPE_L3;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_NONE:
> > + default:
> > + rss_hash_type = PKT_HASH_TYPE_NONE;
> > + }
> > + skb_set_hash(skb, hdr_hash->hash_value, rss_hash_type);
> > + }
>
> so many lines, perhaps deserves a helper function
Ok, I'll create the helper.
>
> >
> > if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID)
> > skb->ip_summed = CHECKSUM_UNNECESSARY;
> > @@ -2232,7 +2258,8 @@ static bool virtnet_commit_rss_command(struct
> > virtnet_info *vi)
> > sg_set_buf([3], vi->ctrl->rss.key, sg_buf_size);
> >
> > if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> > - VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
> > + vi->has_rss ?
> > VIRTIO_NET_CTRL_MQ_RSS_CONFIG
> > + : VIRTIO_NET_CTRL_MQ_HASH_CONFIG, sgs)) {
> > dev_warn(>dev, "VIRTIONET issue with committing RSS
> > sgs\n");
> > return false;
> > }
> > @@ -3230,6 +3257,8 @@ static bool virtnet_validate_features(struct
> > virtio_device *vdev)
> > VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
> > "VIRTIO_NET_F_CTRL_VQ") ||
> > VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS,
> > +"VIRTIO_NET_F_CTRL_VQ") ||
> > +VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
> > "VIRTIO_NET_F_CTRL_VQ"))) {
> > return false;
> > }
> > @@ -3365,8 +3394,13 @@ static int virtnet_probe(struct virtio_device *vdev)
> > if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
> > vi->mergeable_rx_bufs = true;
> >
> > - if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> > + if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
> > +
Re: [PATCH v3 2/4] drivers/net/virtio_net: Added basic RSS support.
Hi all,
On Tue, Feb 8, 2022 at 10:37 PM Willem de Bruijn
wrote:
>
> On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko wrote:
> >
> > Added features for RSS.
> > Added initialization, RXHASH feature and ethtool ops.
> > By default RSS/RXHASH is disabled.
> > Virtio RSS "IPv6 extensions" hashes disabled.
> > Added ethtools ops to set key and indirection table.
> >
> > Signed-off-by: Andrew Melnychenko
> > ---
> > drivers/net/virtio_net.c | 191 +--
> > 1 file changed, 185 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 1404e683a2fd..495aed524e33 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -169,6 +169,24 @@ struct receive_queue {
> > struct xdp_rxq_info xdp_rxq;
> > };
> >
> > +/* This structure can contain rss message with maximum settings for
> > indirection table and keysize
> > + * Note, that default structure that describes RSS configuration
> > virtio_net_rss_config
> > + * contains same info but can't handle table values.
> > + * In any case, structure would be passed to virtio hw through sg_buf
> > split by parts
> > + * because table sizes may be differ according to the device configuration.
> > + */
> > +#define VIRTIO_NET_RSS_MAX_KEY_SIZE 40
>
> Future proof, may want to support larger sizes.
>
> netdevice.h defines NETDEV_RSS_KEY_LEN at 52.
>
> tools/testing/selftests/net/toeplitz.c supports up to 60
According to virtio specification, the length of the key is
40bytes(and an indirection table is 128 entries max).
So for now, we support a maximum of the spec regardless of what the
kernel is capable of.
>
> > +#define VIRTIO_NET_RSS_MAX_TABLE_LEN128
> > +struct virtio_net_ctrl_rss {
> > + u32 hash_types;
>
> conversely, u32 is a bit extreme?
No, the structure virtio_net_ctrl_rss is specified by the specification.
>
> > + u16 indirection_table_mask;
> > + u16 unclassified_queue;
> > + u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> > + u16 max_tx_vq;
> > + u8 hash_key_length;
> > + u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> > +};
> > +
> > /* Control VQ buffers: protected by the rtnl lock */
> > struct control_buf {
> > struct virtio_net_ctrl_hdr hdr;
> > @@ -178,6 +196,7 @@ struct control_buf {
> > u8 allmulti;
> > __virtio16 vid;
> > __virtio64 offloads;
> > + struct virtio_net_ctrl_rss rss;
> > };
> >
> > struct virtnet_info {
> > @@ -206,6 +225,12 @@ struct virtnet_info {
> > /* Host will merge rx buffers for big packets (shake it! shake it!)
> > */
> > bool mergeable_rx_bufs;
> >
> > + /* Host supports rss and/or hash report */
> > + bool has_rss;
> > + u8 rss_key_size;
> > + u16 rss_indir_table_size;
> > + u32 rss_hash_types_supported;
> > +
> > /* Has control virtqueue */
> > bool has_cvq;
> >
> > @@ -2184,6 +2209,56 @@ static void virtnet_get_ringparam(struct net_device
> > *dev,
> > ring->tx_pending = ring->tx_max_pending;
> > }
> >
> > +static bool virtnet_commit_rss_command(struct virtnet_info *vi)
> > +{
> > + struct net_device *dev = vi->dev;
> > + struct scatterlist sgs[4];
> > + unsigned int sg_buf_size;
> > +
> > + /* prepare sgs */
> > + sg_init_table(sgs, 4);
> > +
> > + sg_buf_size = offsetof(struct virtio_net_ctrl_rss,
> > indirection_table);
> > + sg_set_buf([0], >ctrl->rss, sg_buf_size);
> > +
> > + sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
> > + sg_set_buf([1], vi->ctrl->rss.indirection_table, sg_buf_size);
> > +
> > + sg_buf_size = offsetof(struct virtio_net_ctrl_rss, key)
> > + - offsetof(struct virtio_net_ctrl_rss, max_tx_vq);
> > + sg_set_buf([2], >ctrl->rss.max_tx_vq, sg_buf_size);
> > +
> > + sg_buf_size = vi->rss_key_size;
> > + sg_set_buf([3], vi->ctrl->rss.key, sg_buf_size);
> > +
> > + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> > + VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
> > + dev_warn(>dev, "VIRTIONET issue with committing RSS
> > sgs\n");
> > + return false;
> > + }
> > + return true;
> > +}
> > +
> > +static void virtnet_init_default_rss(struct virtnet_info *vi)
> > +{
> > + u32 indir_val = 0;
> > + int i = 0;
> > +
> > + vi->ctrl->rss.hash_types = vi->rss_hash_types_supported;
> > + vi->ctrl->rss.indirection_table_mask = vi->rss_indir_table_size - 1;
>
> Is table size always a power of two?
Yes, it should be.
>
> > + vi->ctrl->rss.unclassified_queue = 0;
> > +
> > + for (; i < vi->rss_indir_table_size; ++i) {
> > + indir_val = ethtool_rxfh_indir_default(i,
> > vi->curr_queue_pairs);
> > + vi->ctrl->rss.indirection_table[i] = indir_val;
> > + }
> > +
> > +
Re: [RFC PATCH 0/5] TUN/VirtioNet USO features support.
Hi people, Can you please review this series? On Wed, Jan 26, 2022 at 10:32 AM Yuri Benditovich wrote: > > On Wed, Jan 26, 2022 at 9:54 AM Xuan Zhuo wrote: > > > > On Tue, 25 Jan 2022 10:46:57 +0200, Andrew Melnychenko > > wrote: > > > Added new offloads for TUN devices TUN_F_USO4 and TUN_F_USO6. > > > Technically they enable NETIF_F_GSO_UDP_L4 > > > (and only if USO4 & USO6 are set simultaneously). > > > It allows to transmission of large UDP packets. > > > > > > Different features USO4 and USO6 are required for qemu where Windows > > > guests can > > > enable disable USO receives for IPv4 and IPv6 separately. > > > On the other side, Linux can't really differentiate USO4 and USO6, for > > > now. > > > For now, to enable USO for TUN it requires enabling USO4 and USO6 > > > together. > > > In the future, there would be a mechanism to control UDP_L4 GSO > > > separately. > > > > > > Test it WIP Qemu https://github.com/daynix/qemu/tree/Dev_USOv2 > > > > > > New types for VirtioNet already on mailing: > > > https://lists.oasis-open.org/archives/virtio-comment/202110/msg00010.html > > > > Seems like this hasn't been upvoted yet. > > > > https://github.com/oasis-tcs/virtio-spec#use-of-github-issues > > Yes, correct. This is a reason why this series of patches is RFC. > > > > > Thanks. > > > > > > > > Also, there is a known issue with transmitting packages between two > > > guests. > > > Without hacks with skb's GSO - packages are still segmented on the host's > > > postrouting. > > > > > > Andrew Melnychenko (5): > > > uapi/linux/if_tun.h: Added new ioctl for tun/tap. > > > driver/net/tun: Added features for USO. > > > uapi/linux/virtio_net.h: Added USO types. > > > linux/virtio_net.h: Added Support for GSO_UDP_L4 offload. > > > drivers/net/virtio_net.c: Added USO support. > > > > > > drivers/net/tap.c | 18 -- > > > drivers/net/tun.c | 15 ++- > > > drivers/net/virtio_net.c| 22 ++ > > > include/linux/virtio_net.h | 11 +++ > > > include/uapi/linux/if_tun.h | 3 +++ > > > include/uapi/linux/virtio_net.h | 4 > > > 6 files changed, 66 insertions(+), 7 deletions(-) > > > > > > -- > > > 2.34.1 > > > > > > ___ > > > Virtualization mailing list > > > Virtualization@lists.linux-foundation.org > > > https://lists.linuxfoundation.org/mailman/listinfo/virtualization ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] drivers/net/virtio_net: Added RSS hash report control.
Hi all,
> e.g RXH_VLAN with port hash?
> Any way to merge the two switch? The code is hard to be reviewed anyhow.
I'll refactor virtnet_set_hashflow.
> I think it's better to use VIRTIO_NET_HASH_REPORT_NONE here.
Yes, I'll fix that.
On Tue, Jan 11, 2022 at 6:33 AM Jason Wang wrote:
>
>
> 在 2022/1/10 上午5:06, Andrew Melnychenko 写道:
> > Now it's possible to control supported hashflows.
> > Also added hashflow set/get callbacks.
> > Also, disabling RXH_IP_SRC/DST for TCP would disable then for UDP.
> > TCP and UDP supports only:
> > ethtool -U eth0 rx-flow-hash tcp4 sd
> > RXH_IP_SRC + RXH_IP_DST
> > ethtool -U eth0 rx-flow-hash tcp4 sdfn
> > RXH_IP_SRC + RXH_IP_DST + RXH_L4_B_0_1 + RXH_L4_B_2_3
> >
> > Signed-off-by: Andrew Melnychenko
> > ---
> > drivers/net/virtio_net.c | 159 +++
> > 1 file changed, 159 insertions(+)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 6e7461b01f87..1b8dd384483c 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -235,6 +235,7 @@ struct virtnet_info {
> > u8 rss_key_size;
> > u16 rss_indir_table_size;
> > u32 rss_hash_types_supported;
> > + u32 rss_hash_types_saved;
> >
> > /* Has control virtqueue */
> > bool has_cvq;
> > @@ -2275,6 +2276,7 @@ static void virtnet_init_default_rss(struct
> > virtnet_info *vi)
> > int i = 0;
> >
> > vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
> > + vi->rss_hash_types_saved = vi->rss_hash_types_supported;
> > vi->ctrl->rss.table_info.indirection_table_mask =
> > vi->rss_indir_table_size - 1;
> > vi->ctrl->rss.table_info.unclassified_queue = 0;
> >
> > @@ -2289,6 +2291,131 @@ static void virtnet_init_default_rss(struct
> > virtnet_info *vi)
> > netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
> > }
> >
> > +static void virtnet_get_hashflow(const struct virtnet_info *vi, struct
> > ethtool_rxnfc *info)
> > +{
> > + info->data = 0;
> > + switch (info->flow_type) {
> > + case TCP_V4_FLOW:
> > + if (vi->rss_hash_types_saved &
> > VIRTIO_NET_RSS_HASH_TYPE_TCPv4) {
> > + info->data = RXH_IP_SRC | RXH_IP_DST |
> > + RXH_L4_B_0_1 | RXH_L4_B_2_3;
> > + } else if (vi->rss_hash_types_saved &
> > VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
> > + info->data = RXH_IP_SRC | RXH_IP_DST;
> > + }
> > + break;
> > + case TCP_V6_FLOW:
> > + if (vi->rss_hash_types_saved &
> > VIRTIO_NET_RSS_HASH_TYPE_TCPv6) {
> > + info->data = RXH_IP_SRC | RXH_IP_DST |
> > + RXH_L4_B_0_1 | RXH_L4_B_2_3;
> > + } else if (vi->rss_hash_types_saved &
> > VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
> > + info->data = RXH_IP_SRC | RXH_IP_DST;
> > + }
> > + break;
> > + case UDP_V4_FLOW:
> > + if (vi->rss_hash_types_saved &
> > VIRTIO_NET_RSS_HASH_TYPE_UDPv4) {
> > + info->data = RXH_IP_SRC | RXH_IP_DST |
> > + RXH_L4_B_0_1 | RXH_L4_B_2_3;
> > + } else if (vi->rss_hash_types_saved &
> > VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
> > + info->data = RXH_IP_SRC | RXH_IP_DST;
> > + }
> > + break;
> > + case UDP_V6_FLOW:
> > + if (vi->rss_hash_types_saved &
> > VIRTIO_NET_RSS_HASH_TYPE_UDPv6) {
> > + info->data = RXH_IP_SRC | RXH_IP_DST |
> > + RXH_L4_B_0_1 | RXH_L4_B_2_3;
> > + } else if (vi->rss_hash_types_saved &
> > VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
> > + info->data = RXH_IP_SRC | RXH_IP_DST;
> > + }
> > + break;
> > + case IPV4_FLOW:
> > + if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
> > + info->data = RXH_IP_SRC | RXH_IP_DST;
> > +
> > + break;
> > + case IPV6_FLOW:
> > + if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
> > + info->data = RXH_IP_SRC | RXH_IP_DST;
> > +
> > + break;
> > + default:
> > + info->data = 0;
> > + break;
> > + }
> > +}
> > +
> > +static bool virtnet_set_hashflow(struct virtnet_info *vi, struct
> > ethtool_rxnfc *info)
> > +{
> > + u64 is_iphash = info->data & (RXH_IP_SRC | RXH_IP_DST);
> > + u64 is_porthash = info->data & (RXH_L4_B_0_1 | RXH_L4_B_2_3);
> > + u32 new_hashtypes = vi->rss_hash_types_saved;
> > +
> > + if ((is_iphash && (is_iphash != (RXH_IP_SRC | RXH_IP_DST))) ||
> > + (is_porthash && (is_porthash != (RXH_L4_B_0_1 | RXH_L4_B_2_3 {
> > + return false;
> > + }
> > +
> > + if (!is_iphash && is_porthash)
Re: [PATCH 3/4] drivers/net/virtio_net: Added RSS hash report.
Hi all.
> I think we should make RSS depend on CTRL_VQ.
> Need to depend on CTRL_VQ here.
I'll fix that.
> Any reason to initialize RSS feature here not the init_default_rss()?
init_default_rss() initializes virtio_net_ctrl_rss structure only, I
think it's a good idea not to mix field initializations.
On Tue, Jan 11, 2022 at 6:06 AM Jason Wang wrote:
>
>
> 在 2022/1/10 上午5:06, Andrew Melnychenko 写道:
> > Added features for RSS hash report.
> > If hash is provided - it sets to skb.
> > Added checks if rss and/or hash are enabled together.
> >
> > Signed-off-by: Andrew Melnychenko
> > ---
> > drivers/net/virtio_net.c | 56 ++--
> > 1 file changed, 48 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 21794731fc75..6e7461b01f87 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -231,6 +231,7 @@ struct virtnet_info {
> >
> > /* Host supports rss and/or hash report */
> > bool has_rss;
> > + bool has_rss_hash_report;
> > u8 rss_key_size;
> > u16 rss_indir_table_size;
> > u32 rss_hash_types_supported;
> > @@ -424,7 +425,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info
> > *vi,
> > hdr_p = p;
> >
> > hdr_len = vi->hdr_len;
> > - if (vi->mergeable_rx_bufs)
> > + if (vi->has_rss_hash_report)
> > + hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> > + else if (vi->mergeable_rx_bufs)
> > hdr_padded_len = sizeof(*hdr);
> > else
> > hdr_padded_len = sizeof(struct padded_vnet_hdr);
> > @@ -1160,6 +1163,8 @@ static void receive_buf(struct virtnet_info *vi,
> > struct receive_queue *rq,
> > struct net_device *dev = vi->dev;
> > struct sk_buff *skb;
> > struct virtio_net_hdr_mrg_rxbuf *hdr;
> > + struct virtio_net_hdr_v1_hash *hdr_hash;
> > + enum pkt_hash_types rss_hash_type;
> >
> > if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
> > pr_debug("%s: short packet %i\n", dev->name, len);
> > @@ -1186,6 +1191,29 @@ static void receive_buf(struct virtnet_info *vi,
> > struct receive_queue *rq,
> > return;
> >
> > hdr = skb_vnet_hdr(skb);
> > + if (dev->features & NETIF_F_RXHASH) {
> > + hdr_hash = (struct virtio_net_hdr_v1_hash *)(hdr);
> > +
> > + switch (hdr_hash->hash_report) {
> > + case VIRTIO_NET_HASH_REPORT_TCPv4:
> > + case VIRTIO_NET_HASH_REPORT_UDPv4:
> > + case VIRTIO_NET_HASH_REPORT_TCPv6:
> > + case VIRTIO_NET_HASH_REPORT_UDPv6:
> > + case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
> > + case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
> > + rss_hash_type = PKT_HASH_TYPE_L4;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_IPv4:
> > + case VIRTIO_NET_HASH_REPORT_IPv6:
> > + case VIRTIO_NET_HASH_REPORT_IPv6_EX:
> > + rss_hash_type = PKT_HASH_TYPE_L3;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_NONE:
> > + default:
> > + rss_hash_type = PKT_HASH_TYPE_NONE;
> > + }
> > + skb_set_hash(skb, hdr_hash->hash_value, rss_hash_type);
> > + }
> >
> > if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID)
> > skb->ip_summed = CHECKSUM_UNNECESSARY;
> > @@ -2233,7 +2261,8 @@ static bool virtnet_commit_rss_command(struct
> > virtnet_info *vi)
> > sg_set_buf([3], vi->ctrl->rss.key, sg_buf_size);
> >
> > if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> > - VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
> > + vi->has_rss ? VIRTIO_NET_CTRL_MQ_RSS_CONFIG
> > + : VIRTIO_NET_CTRL_MQ_HASH_CONFIG, sgs)) {
> > dev_warn(>dev, "VIRTIONET issue with committing RSS
> > sgs\n");
> > return false;
> > }
> > @@ -3220,7 +3249,9 @@ static bool virtnet_validate_features(struct
> > virtio_device *vdev)
> >VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
> >VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
> >"VIRTIO_NET_F_CTRL_VQ") ||
> > - VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS"))) {
> > + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS") ||
>
>
> I think we should make RSS depend on CTRL_VQ.
>
>
> > + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
> > + "VIRTIO_NET_F_HASH_REPORT"))) {
>
>
> Need to depend on CTRL_VQ here.
>
>
> > return false;
> > }
> >
> > @@ -3355,6 +3386,12 @@ static int virtnet_probe(struct virtio_device *vdev)
> > if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
> > vi->mergeable_rx_bufs = true;
> >
> >
Re: [PATCH 2/4] drivers/net/virtio_net: Added basic RSS support.
Hi all
> Is this correct if both mergeable_rx_bufs and hash_report are set?
Yes, there is a similar code in qemu.
> Can we simply do virtio_cread_feature(vdev, VIRTIO_NET_F_MQ |
> VIRTIO_NET_F_RSS, ...) ?
No, VIRTIO_NET_F_* is bit offset - so in the end "1 <<
(VIRTIO_NET_F_MQ | VIRTIO_NET_F_RSS)" is not valid.
> Is rtnl_lock() really needed here consider we haven't even register netdev?
I'll remove rtnl lock.
> Generally best to avoid __packed.
I'll refactor the structure.
On Tue, Jan 11, 2022 at 2:00 PM Michael S. Tsirkin wrote:
>
> On Sun, Jan 09, 2022 at 11:06:57PM +0200, Andrew Melnychenko wrote:
> > Added features for RSS.
> > Added initialization, RXHASH feature and ethtool ops.
> > By default RSS/RXHASH is disabled.
> > Virtio RSS "IPv6 extensions" hashes disabled.
> > Added ethtools ops to set key and indirection table.
> >
> > Signed-off-by: Andrew Melnychenko
> > ---
> > drivers/net/virtio_net.c | 194 +--
> > 1 file changed, 184 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 66439ca488f4..21794731fc75 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -169,6 +169,28 @@ struct receive_queue {
> > struct xdp_rxq_info xdp_rxq;
> > };
> >
> > +/* This structure can contain rss message with maximum settings for
> > indirection table and keysize
> > + * Note, that default structure that describes RSS configuration
> > virtio_net_rss_config
> > + * contains same info but can't handle table values.
> > + * In any case, structure would be passed to virtio hw through sg_buf
> > split by parts
> > + * because table sizes may be differ according to the device configuration.
> > + */
> > +#define VIRTIO_NET_RSS_MAX_KEY_SIZE 40
> > +#define VIRTIO_NET_RSS_MAX_TABLE_LEN128
> > +struct virtio_net_ctrl_rss {
> > + struct {
> > + __le32 hash_types;
> > + __le16 indirection_table_mask;
> > + __le16 unclassified_queue;
> > + } __packed table_info;
> > + u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> > + struct {
> > + u16 max_tx_vq; /* queues */
> > + u8 hash_key_length;
> > + } __packed key_info;
> > + u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> > +};
> > +
>
> Generally best to avoid __packed.
> I think it's not a bad idea to just follow the spec when
> you lay out the structures. Makes it easier to follow
> that it matches. Spec has just a single struct:
>
> struct virtio_net_rss_config {
> le32 hash_types;
> le16 indirection_table_mask;
> le16 unclassified_queue;
> le16 indirection_table[indirection_table_length];
> le16 max_tx_vq;
> u8 hash_key_length;
> u8 hash_key_data[hash_key_length];
> };
>
> and with this layout you don't need __packed.
>
>
>
> > /* Control VQ buffers: protected by the rtnl lock */
> > struct control_buf {
> > struct virtio_net_ctrl_hdr hdr;
> > @@ -178,6 +200,7 @@ struct control_buf {
> > u8 allmulti;
> > __virtio16 vid;
> > __virtio64 offloads;
> > + struct virtio_net_ctrl_rss rss;
> > };
> >
> > struct virtnet_info {
> > @@ -206,6 +229,12 @@ struct virtnet_info {
> > /* Host will merge rx buffers for big packets (shake it! shake it!) */
> > bool mergeable_rx_bufs;
> >
> > + /* Host supports rss and/or hash report */
> > + bool has_rss;
> > + u8 rss_key_size;
> > + u16 rss_indir_table_size;
> > + u32 rss_hash_types_supported;
> > +
> > /* Has control virtqueue */
> > bool has_cvq;
> >
> > @@ -395,9 +424,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info
> > *vi,
> > hdr_p = p;
> >
> > hdr_len = vi->hdr_len;
> > - if (vi->has_rss_hash_report)
> > - hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> > - else if (vi->mergeable_rx_bufs)
> > + if (vi->mergeable_rx_bufs)
> > hdr_padded_len = sizeof(*hdr);
> > else
> > hdr_padded_len = sizeof(struct padded_vnet_hdr);
> > @@ -2184,6 +2211,55 @@ static void virtnet_get_ringparam(struct net_device
> > *dev,
> > ring->tx_pending = ring->tx_max_pending;
> > }
> >
> > +static bool virtnet_commit_rss_command(struct virtnet_info *vi)
> > +{
> > + struct net_device *dev = vi->dev;
> > + struct scatterlist sgs[4];
> > + unsigned int sg_buf_size;
> > +
> > + /* prepare sgs */
> > + sg_init_table(sgs, 4);
> > +
> > + sg_buf_size = sizeof(vi->ctrl->rss.table_info);
> > + sg_set_buf([0], >ctrl->rss.table_info, sg_buf_size);
> > +
> > + sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
> > + sg_set_buf([1], vi->ctrl->rss.indirection_table, sg_buf_size);
> > +
> > + sg_buf_size = sizeof(vi->ctrl->rss.key_info);
> > + sg_set_buf([2], >ctrl->rss.key_info, sg_buf_size);
> > +
> > + sg_buf_size = vi->rss_key_size;
> > + sg_set_buf([3], vi->ctrl->rss.key,
Re: [RFC PATCH 3/4] drivers/net/virtio_net: Added basic RSS support.
On Sun, Oct 31, 2021 at 5:33 PM Willem de Bruijn
wrote:
>
> On Sun, Oct 31, 2021 at 1:00 AM Andrew Melnychenko wrote:
> >
> > Added features for RSS and RSS hash report.
> > Added initialization, RXHASH feature and ethtool ops.
> > By default RSS/RXHASH is disabled.
> > Added ethtools ops to set key and indirection table.
> >
> > Signed-off-by: Andrew Melnychenko
> > ---
> > drivers/net/virtio_net.c | 232 +--
> > 1 file changed, 223 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index abca2e93355d..cff7340f40bb 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -167,6 +167,28 @@ struct receive_queue {
> > struct xdp_rxq_info xdp_rxq;
> > };
> >
> > +/* This structure can contain rss message with maximum settings for
> > indirection table and keysize
> > + * Note, that default structure that describes RSS configuration
> > virtio_net_rss_config
> > + * contains same info but can't handle table values.
> > + * In any case, structure would be passed to virtio hw through sg_buf
> > split by parts
> > + * because table sizes may be differ according to the device configuration.
> > + */
> > +#define VIRTIO_NET_RSS_MAX_KEY_SIZE 40
>
> Unless there is a technical reason, this probably should be no shorter
> than TOEPLITZ_KEY_LEN
Well yeah, technically if the device requests for shorter key, we
still may provide it.
I think we may check and 'disable' RSS if some configurations are inadequate.
>
> > +#define VIRTIO_NET_RSS_MAX_TABLE_LEN128
> > +struct virtio_net_ctrl_rss {
> > + struct {
> > + __le32 hash_types;
> > + __le16 indirection_table_mask;
> > + __le16 unclassified_queue;
>
> Is this explicit variable needed?
Yes, it's a part of the message to be sent to the device.
>
> > + } __packed table_info;
> > + u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> > + struct {
> > + u16 max_tx_vq; /* queues */
> > + u8 hash_key_length;
> > + } __packed key_info;
> > + u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> > +};
> > +
> > /* Control VQ buffers: protected by the rtnl lock */
> > struct control_buf {
> > struct virtio_net_ctrl_hdr hdr;
> > @@ -176,6 +198,7 @@ struct control_buf {
> > u8 allmulti;
> > __virtio16 vid;
> > __virtio64 offloads;
> > + struct virtio_net_ctrl_rss rss;
> > };
> >
> > struct virtnet_info {
> > @@ -204,6 +227,12 @@ struct virtnet_info {
> > /* Host will merge rx buffers for big packets (shake it! shake it!)
> > */
> > bool mergeable_rx_bufs;
> >
> > + /* Host supports rss and/or hash report */
> > + bool has_rss;
>
> Superfluous, can be derived form non-zero rss_key_size?
I think that the explicit 'has_rss' field is better. "non-zero
rss_key_size" should work, I'll change RSS derivation in the next
patches.
>
> > + bool has_rss_hash_report;
> > + u8 rss_key_size;
> > + u16 rss_indir_table_size;
> > +
> > /* Has control virtqueue */
> > bool has_cvq;
> >
> > @@ -1119,6 +1148,8 @@ static void receive_buf(struct virtnet_info *vi,
> > struct receive_queue *rq,
> > struct net_device *dev = vi->dev;
> > struct sk_buff *skb;
> > struct virtio_net_hdr_mrg_rxbuf *hdr;
> > + struct virtio_net_hdr_v1_hash *hdr_hash;
> > + enum pkt_hash_types rss_hash_type;
> >
> > if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
> > pr_debug("%s: short packet %i\n", dev->name, len);
> > @@ -1145,6 +1176,29 @@ static void receive_buf(struct virtnet_info *vi,
> > struct receive_queue *rq,
> > return;
> >
> > hdr = skb_vnet_hdr(skb);
> > + if (vi->has_rss_hash_report && (dev->features & NETIF_F_RXHASH)) {
>
> Only the second test is needed? It should be impossible to configure
> the feature unless the device advertises has_rss_hash_report
Well, you can have RSS without a hash population. So, need to check,
is the device supports hash population and rxhash is enabled(g.e.
through ethtool).
>
> > + hdr_hash = (struct virtio_net_hdr_v1_hash *)(hdr);
> > +
> > + switch (hdr_hash->hash_report) {
> > + case VIRTIO_NET_HASH_REPORT_TCPv4:
> > + case VIRTIO_NET_HASH_REPORT_UDPv4:
> > + case VIRTIO_NET_HASH_REPORT_TCPv6:
> > + case VIRTIO_NET_HASH_REPORT_UDPv6:
> > + case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
> > + case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
> > + rss_hash_type = PKT_HASH_TYPE_L4;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_IPv4:
> > + case VIRTIO_NET_HASH_REPORT_IPv6:
> > + case VIRTIO_NET_HASH_REPORT_IPv6_EX:
> > + rss_hash_type = PKT_HASH_TYPE_L3;
> >
Re: [RFC PATCH 1/4] drivers/net/virtio_net: Fixed vheader to use v1.
On Mon, Nov 1, 2021 at 10:40 AM Michael S. Tsirkin wrote:
>
> On Sun, Oct 31, 2021 at 06:59:56AM +0200, Andrew Melnychenko wrote:
> > The header v1 provides additional info about RSS.
> > Added changes to computing proper header length.
> > In the next patches, the header may contain RSS hash info
> > for the hash population.
> >
> > Signed-off-by: Andrew Melnychenko
> > ---
> > drivers/net/virtio_net.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 4ad25a8b0870..b72b21ac8ebd 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -240,13 +240,13 @@ struct virtnet_info {
> > };
> >
> > struct padded_vnet_hdr {
> > - struct virtio_net_hdr_mrg_rxbuf hdr;
> > + struct virtio_net_hdr_v1_hash hdr;
> > /*
> >* hdr is in a separate sg buffer, and data sg buffer shares same page
> >* with this header sg. This padding makes next sg 16 byte aligned
> >* after the header.
> >*/
> > - char padding[4];
> > + char padding[12];
> > };
> >
> > static bool is_xdp_frame(void *ptr)
>
>
> This is not helpful as a separate patch, just reserving extra space.
> better squash with the patches making use of the change.
Ok.
>
> > @@ -1636,7 +1636,7 @@ static int xmit_skb(struct send_queue *sq, struct
> > sk_buff *skb)
> > const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
> > struct virtnet_info *vi = sq->vq->vdev->priv;
> > int num_sg;
> > - unsigned hdr_len = vi->hdr_len;
> > + unsigned int hdr_len = vi->hdr_len;
> > bool can_push;
>
>
> if we want this, pls make it a separate patch.
Ok. I've added that change after checkpatch warnings. Technically,
checkpatch should not fail on the patch without that line.
>
>
> >
> > pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
> > --
> > 2.33.1
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 2/4] drivers/net/virtio_net: Changed mergeable buffer length calculation.
On Mon, Nov 1, 2021 at 10:44 AM Michael S. Tsirkin wrote:
>
> On Sun, Oct 31, 2021 at 06:59:57AM +0200, Andrew Melnychenko wrote:
> > Now minimal virtual header length is may include the entire v1 header
> > if the hash report were populated.
> >
> > Signed-off-by: Andrew Melnychenko
>
> subject isn't really descriptive. changed it how?
>
> And I couldn't really decypher what this log entry means either.
>
I'll change it in the next patch.
So, I've tried to ensure that the v1 header with the hash report will
be available if required in new patches.
> > ---
> > drivers/net/virtio_net.c | 8 +---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index b72b21ac8ebd..abca2e93355d 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -393,7 +393,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info
> > *vi,
> > hdr_p = p;
> >
> > hdr_len = vi->hdr_len;
> > - if (vi->mergeable_rx_bufs)
> > + if (vi->has_rss_hash_report)
> > + hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> > + else if (vi->mergeable_rx_bufs)
> > hdr_padded_len = sizeof(*hdr);
> > else
> > hdr_padded_len = sizeof(struct padded_vnet_hdr);
> > @@ -1252,7 +1254,7 @@ static unsigned int get_mergeable_buf_len(struct
> > receive_queue *rq,
> > struct ewma_pkt_len *avg_pkt_len,
> > unsigned int room)
> > {
> > - const size_t hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> > + const size_t hdr_len = ((struct virtnet_info
> > *)(rq->vq->vdev->priv))->hdr_len;
> > unsigned int len;
> >
> > if (room)
>
> Is this pointer chasing the best we can do?
I'll change that.
>
> > @@ -2817,7 +2819,7 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
> > */
> > static unsigned int mergeable_min_buf_len(struct virtnet_info *vi, struct
> > virtqueue *vq)
> > {
> > - const unsigned int hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> > + const unsigned int hdr_len = vi->hdr_len;
> > unsigned int rq_size = virtqueue_get_vring_size(vq);
> > unsigned int packet_len = vi->big_packets ? IP_MAX_MTU :
> > vi->dev->max_mtu;
> > unsigned int buf_len = hdr_len + ETH_HLEN + VLAN_HLEN + packet_len;
> > --
> > 2.33.1
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 0/3] drivers/net/virtio_net: Added RSS support.
Hi guys, Can you please review possible virtio-net driver RSS support? Do you have any comments or proposals? On Wed, Aug 18, 2021 at 8:55 PM Andrew Melnychenko wrote: > This series of RFC patches for comments and additional proposals. > > Virtio-net supports "hardware" RSS with toeplitz key. > Also, it allows receiving calculated hash in vheader > that may be used with RPS. > Added ethtools callbacks to manipulate RSS. > > Technically hash calculation may be set only for > SRC+DST and SRC+DST+PORTSRC+PORTDST hashflows. > The completely disabling hash calculation for TCP or UDP > would disable hash calculation for IP. > > RSS/RXHASH is disabled by default. > > Andrew Melnychenko (3): > drivers/net/virtio_net: Fixed vheader to use v1. > drivers/net/virtio_net: Added basic RSS support. > drivers/net/virtio_net: Added RSS hash report. > > drivers/net/virtio_net.c | 402 +-- > 1 file changed, 385 insertions(+), 17 deletions(-) > > -- > 2.31.1 > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] Fix: buffer overflow during hvc_alloc().
> > Description of problem: > Guest get 'Call Trace' when loading module "virtio_console" and unloading > it frequently > > > Version-Release number of selected component (if applicable): > Guest > kernel-4.18.0-167.el8.x86_64 > seabios-bin-1.11.1-4.module+el8.1.0+4066+0f1aadab.noarch > # modinfo virtio_console >filename: /lib/modules/4.18.0- >167.el8.x86_64/kernel/drivers/char/virtio_console.ko.xz >license:GPL >description:Virtio console driver >rhelversion:8.2 >srcversion: 55224090DD07750FAD75C9C >alias: virtio:d0003v* >depends: >intree: Y >name: virtio_console >vermagic: 4.18.0-167.el8.x86_64 SMP mod_unload modversions > Host: > qemu-kvm-4.2.0-2.scrmod+el8.2.0+5159+d8aa4d83.x86_64 > kernel-4.18.0-165.el8.x86_64 > seabios-bin-1.12.0-5.scrmod+el8.2.0+5159+d8aa4d83.noarch > > > > How reproducible: 100% > > > Steps to Reproduce: > > 1. boot guest with command [1] > 2. load and unload virtio_console inside guest with loop.sh ># cat loop.sh > while [ 1 ] > do > modprobe virtio_console > lsmod | grep virt > modprobe -r virtio_console > lsmod | grep virt > done > > > > Actual results: > Guest reboot and can get vmcore-dmesg.txt file > > > Expected results: > Guest works well without error > > > Additional info: > The whole log will attach to the attachments. > > Call Trace: > [ 22.974500] fuse: init (API version 7.31) > [ 81.498208] [ cut here ] > [ 81.499263] pvqspinlock: lock 0x92080020 has corrupted value > 0xc0774ca0! > [ 81.501000] WARNING: CPU: 0 PID: 785 at > kernel/locking/qspinlock_paravirt.h:500 > __pv_queued_spin_unlock_slowpath+0xc0/0xd0 > [ 81.503173] Modules linked in: virtio_console fuse xt_CHECKSUM > ipt_MASQUERADE xt_conntrack ipt_REJECT nft_counter nf_nat_tftp nft_objref > nf_conntrack_tftp tun bridge stp llc nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 > nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct > nf_tables_set nft_chain_nat_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 > nf_nat_ipv6 nft_chain_route_ipv6 nft_chain_nat_ipv4 nf_conntrack_ipv4 > nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack nft_chain_route_ipv4 > ip6_tables nft_compat ip_set nf_tables nfnetlink sunrpc bochs_drm > drm_vram_helper ttm drm_kms_helper syscopyarea sysfillrect sysimgblt > fb_sys_fops drm i2c_piix4 pcspkr crct10dif_pclmul crc32_pclmul joydev > ghash_clmulni_intel ip_tables xfs libcrc32c sd_mod sg ata_generic ata_piix > virtio_net libata crc32c_intel net_failover failover serio_raw virtio_scsi > dm_mirror dm_region_hash dm_log dm_mod [last unloaded: virtio_console] > [ 81.517019] CPU: 0 PID: 785 Comm: kworker/0:2 Kdump: loaded Not tainted > 4.18.0-167.el8.x86_64 #1 > [ 81.518639] Hardware name: Red Hat KVM, BIOS > 1.12.0-5.scrmod+el8.2.0+5159+d8aa4d83 04/01/2014 > [ 81.520205] Workqueue: events control_work_handler [virtio_console] > [ 81.521354] RIP: 0010:__pv_queued_spin_unlock_slowpath+0xc0/0xd0 > [ 81.522450] Code: 07 00 48 63 7a 10 e8 bf 64 f5 ff 66 90 c3 8b 05 e6 cf > d6 01 85 c0 74 01 c3 8b 17 48 89 fe 48 c7 c7 38 4b 29 91 e8 3a 6c fa ff > <0f> 0b c3 0f 0b 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44 00 00 48 > [ 81.525830] RSP: 0018:b51a01ffbd70 EFLAGS: 00010282 > [ 81.526798] RAX: RBX: 0010 RCX: > > [ 81.528110] RDX: 9e66f1826480 RSI: 9e66f1816a08 RDI: > 9e66f1816a08 > [ 81.529437] RBP: 9153ff10 R08: 026c R09: > 0053 > [ 81.530732] R10: R11: b51a01ffbc18 R12: > 9e66cd682200 > [ 81.532133] R13: 9153ff10 R14: 9e6685569500 R15: > 9e66cd682000 > [ 81.533442] FS: () GS:9e66f180() > knlGS: > [ 81.534914] CS: 0010 DS: ES: CR0: 80050033 > [ 81.535971] CR2: 5624c55b14d0 CR3: 0003a023c000 CR4: > 003406f0 > [ 81.537283] Call Trace: > [ 81.537763] > __raw_callee_save___pv_queued_spin_unlock_slowpath+0x11/0x20 > [ 81.539011] .slowpath+0x9/0xe > [ 81.539585] hvc_alloc+0x25e/0x300 > [ 81.540237] init_port_console+0x28/0x100 [virtio_console] > [ 81.541251] handle_control_message.constprop.27+0x1c4/0x310 > [virtio_console] > [ 81.542546] control_work_handler+0x70/0x10c [virtio_console] > [ 81.543601] process_one_work+0x1a7/0x3b0 > [ 81.544356] worker_thread+0x30/0x390 > [ 81.545025] ? create_worker+0x1a0/0x1a0 > [ 81.545749] kthread+0x112/0x130 > [ 81.546358] ? kthread_flush_work_fn+0x10/0x10 > [ 81.547183] ret_from_fork+0x22/0x40 > [ 81.547842] ---[ end trace aa97649bd16c8655 ]--- > [ 83.546539] general protection fault: [#1] SMP NOPTI > [ 83.547422] CPU: 5 PID: 3225 Comm: modprobe Kdump: loaded Tainted: G > W- - - 4.18.0-167.el8.x86_64 #1 > [ 83.549191]
