VIRTIO_NET_HDR_F_RSC_INFO in virtio-net vs headers update

VIRTIO_NET_HDR_F_RSC_INFO in virtio-net vs headers update

[Cornelia Huck]

Hi,

I'm currently trying to prepare a linux-headers update to 5.7-rc3,
which adds the definition of VIRTIO_NET_HDR_F_RSC_INFO.

Unfortunately, this breaks the build of virtio-net, because now
virtio_net_rsc_ext_num_{packets,dupacks} are undefined (they are
guarded by existence of VIRTIO_NET_HDR_F_RSC_INFO).

What is the right way to fix this? Remove the constants that are now
provided by the header and keep the definitions of
virtio_net_rsc_ext_num_{packets,dupacks}?

[I'd like to queue a headers update as soon as possible, as the whole
s390 protected virt stuff depends on it...]

Re: VIRTIO_NET_HDR_F_RSC_INFO in virtio-net vs headers update

[Jason Wang]

[-- Attachment #1: Type: text/plain, Size: 916 bytes --]


On 2020/4/27 下午3:33, Cornelia Huck wrote:
> Hi,
>
> I'm currently trying to prepare a linux-headers update to 5.7-rc3,
> which adds the definition of VIRTIO_NET_HDR_F_RSC_INFO.
>
> Unfortunately, this breaks the build of virtio-net, because now
> virtio_net_rsc_ext_num_{packets,dupacks} are undefined (they are
> guarded by existence of VIRTIO_NET_HDR_F_RSC_INFO).
>
> What is the right way to fix this? Remove the constants that are now
> provided by the header and keep the definitions of
> virtio_net_rsc_ext_num_{packets,dupacks}?


We probably need to add a version of the above function when 
VIRTIO_NET_HDR_F_RSC_INFO is defined as attached.

But I fail to understand why we need a fallback when 
VIRTIO_NET_HDR_F_RSC_INFO is not defined.

Thanks


>
> [I'd like to queue a headers update as soon as possible, as the whole
> s390 protected virt stuff depends on it...]
>
>

[-- Attachment #2: 0001-.patch --]
[-- Type: text/x-patch, Size: 833 bytes --]

From 915144149cfef1c056ae3b96dfb714a2bd483759 Mon Sep 17 00:00:00 2001
From: Jason Wang <jasowang@redhat.com>
Date: Mon, 27 Apr 2020 16:38:51 +0800
Subject: [PATCH]

---
 hw/net/virtio-net.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index e5bf670c3f..dd671bcc0d 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -171,6 +171,20 @@ static inline __virtio16 *virtio_net_rsc_ext_num_dupacks(
     return &hdr->csum_offset;
 }
 
+#else
+
+static inline __virtio16 *virtio_net_rsc_ext_num_packets(
+    struct virtio_net_hdr *hdr)
+{
+    return &hdr->segments;
+}
+
+static inline __virtio16 *virtio_net_rsc_ext_num_dupacks(
+    struct virtio_net_hdr *hdr)
+{
+    return &hdr->dup_acks;
+}
+
 #endif
 
 static VirtIOFeature feature_sizes[] = {
-- 
2.20.1

Re: VIRTIO_NET_HDR_F_RSC_INFO in virtio-net vs headers update

[Cornelia Huck]

On Mon, 27 Apr 2020 16:41:30 +0800
Jason Wang <jasowang@redhat.com> wrote:

> 
> On 2020/4/27 下午3:33, Cornelia Huck wrote:
> > Hi,
> >
> > I'm currently trying to prepare a linux-headers update to 5.7-rc3,
> > which adds the definition of VIRTIO_NET_HDR_F_RSC_INFO.
> >
> > Unfortunately, this breaks the build of virtio-net, because now
> > virtio_net_rsc_ext_num_{packets,dupacks} are undefined (they are
> > guarded by existence of VIRTIO_NET_HDR_F_RSC_INFO).
> >
> > What is the right way to fix this? Remove the constants that are now
> > provided by the header and keep the definitions of
> > virtio_net_rsc_ext_num_{packets,dupacks}?
> 
> 
> We probably need to add a version of the above function when 
> VIRTIO_NET_HDR_F_RSC_INFO is defined as attached.
> 
> But I fail to understand why we need a fallback when 
> VIRTIO_NET_HDR_F_RSC_INFO is not defined.

Yes, the current code in virtio-net looks a bit odd, which is why I
asked.

I see two options:
- do the change you proposed, do the headers update, and then rip out
   the compat handling
- do the above in a single step

I'd prefer the second option.

> 
> Thanks
> 
> 
> >
> > [I'd like to queue a headers update as soon as possible, as the whole
> > s390 protected virt stuff depends on it...]
> >
> >

Re: VIRTIO_NET_HDR_F_RSC_INFO in virtio-net vs headers update

[Michael S. Tsirkin]

On Mon, Apr 27, 2020 at 11:10:29AM +0200, Cornelia Huck wrote:
> On Mon, 27 Apr 2020 16:41:30 +0800
> Jason Wang <jasowang@redhat.com> wrote:
> 
> > 
> > On 2020/4/27 下午3:33, Cornelia Huck wrote:
> > > Hi,
> > >
> > > I'm currently trying to prepare a linux-headers update to 5.7-rc3,
> > > which adds the definition of VIRTIO_NET_HDR_F_RSC_INFO.
> > >
> > > Unfortunately, this breaks the build of virtio-net, because now
> > > virtio_net_rsc_ext_num_{packets,dupacks} are undefined (they are
> > > guarded by existence of VIRTIO_NET_HDR_F_RSC_INFO).
> > >
> > > What is the right way to fix this? Remove the constants that are now
> > > provided by the header and keep the definitions of
> > > virtio_net_rsc_ext_num_{packets,dupacks}?
> > 
> > 
> > We probably need to add a version of the above function when 
> > VIRTIO_NET_HDR_F_RSC_INFO is defined as attached.
> > 
> > But I fail to understand why we need a fallback when 
> > VIRTIO_NET_HDR_F_RSC_INFO is not defined.
> 
> Yes, the current code in virtio-net looks a bit odd, which is why I
> asked.
> 
> I see two options:
> - do the change you proposed, do the headers update, and then rip out
>    the compat handling
> - do the above in a single step
> 
> I'd prefer the second option.

Slight preference for 1st one but both are ok.

> > 
> > Thanks
> > 
> > 
> > >
> > > [I'd like to queue a headers update as soon as possible, as the whole
> > > s390 protected virt stuff depends on it...]
> > >
> > >

Re: VIRTIO_NET_HDR_F_RSC_INFO in virtio-net vs headers update

[Yuri Benditovich]

[-- Attachment #1: Type: text/plain, Size: 1829 bytes --]

I'd suggest to do the single change in virtio-net.c with RSC definitions
from updated linux headers.
I can send respective patch if you want

On Mon, Apr 27, 2020 at 12:18 PM Michael S. Tsirkin <mst@redhat.com> wrote:

> On Mon, Apr 27, 2020 at 11:10:29AM +0200, Cornelia Huck wrote:
> > On Mon, 27 Apr 2020 16:41:30 +0800
> > Jason Wang <jasowang@redhat.com> wrote:
> >
> > >
> > > On 2020/4/27 下午3:33, Cornelia Huck wrote:
> > > > Hi,
> > > >
> > > > I'm currently trying to prepare a linux-headers update to 5.7-rc3,
> > > > which adds the definition of VIRTIO_NET_HDR_F_RSC_INFO.
> > > >
> > > > Unfortunately, this breaks the build of virtio-net, because now
> > > > virtio_net_rsc_ext_num_{packets,dupacks} are undefined (they are
> > > > guarded by existence of VIRTIO_NET_HDR_F_RSC_INFO).
> > > >
> > > > What is the right way to fix this? Remove the constants that are now
> > > > provided by the header and keep the definitions of
> > > > virtio_net_rsc_ext_num_{packets,dupacks}?
> > >
> > >
> > > We probably need to add a version of the above function when
> > > VIRTIO_NET_HDR_F_RSC_INFO is defined as attached.
> > >
> > > But I fail to understand why we need a fallback when
> > > VIRTIO_NET_HDR_F_RSC_INFO is not defined.
> >
> > Yes, the current code in virtio-net looks a bit odd, which is why I
> > asked.
> >
> > I see two options:
> > - do the change you proposed, do the headers update, and then rip out
> >    the compat handling
> > - do the above in a single step
> >
> > I'd prefer the second option.
>
> Slight preference for 1st one but both are ok.
>
> > >
> > > Thanks
> > >
> > >
> > > >
> > > > [I'd like to queue a headers update as soon as possible, as the whole
> > > > s390 protected virt stuff depends on it...]
> > > >
> > > >
>
>

[-- Attachment #2: Type: text/html, Size: 2568 bytes --]

Re: VIRTIO_NET_HDR_F_RSC_INFO in virtio-net vs headers update

[Cornelia Huck]

On Mon, 27 Apr 2020 12:52:26 +0300
Yuri Benditovich <yuri.benditovich@daynix.com> wrote:

> I'd suggest to do the single change in virtio-net.c with RSC definitions
> from updated linux headers.
> I can send respective patch if you want

Hm, I already sent something out before I saw your mail... can you
please look at that one?