headers update and virtio-net fixup

[PATCH 3/3] virtio-net: remove VIRTIO_NET_HDR_F_RSC_INFO compat handling

Posted by Cornelia Huck 5 years, 6 months ago

VIRTIO_NET_HDR_F_RSC_INFO is available in the headers now.

Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/net/virtio-net.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index e85d902588b3..7449570c7123 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -77,14 +77,6 @@
    tso/gso/gro 'off'. */
 #define VIRTIO_NET_RSC_DEFAULT_INTERVAL 300000
 
-/* temporary until standard header include it */
-#if !defined(VIRTIO_NET_HDR_F_RSC_INFO)
-
-#define VIRTIO_NET_HDR_F_RSC_INFO  4 /* rsc_ext data in csum_ fields */
-#define VIRTIO_NET_F_RSC_EXT       61
-
-#endif
-
 static inline __virtio16 *virtio_net_rsc_ext_num_packets(
     struct virtio_net_hdr *hdr)
 {
-- 
2.21.1

Re: [PATCH 3/3] virtio-net: remove VIRTIO_NET_HDR_F_RSC_INFO compat handling

Posted by Michael S. Tsirkin 5 years, 6 months ago

On Mon, Apr 27, 2020 at 12:24:15PM +0200, Cornelia Huck wrote:
> VIRTIO_NET_HDR_F_RSC_INFO is available in the headers now.
> 
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/net/virtio-net.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index e85d902588b3..7449570c7123 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -77,14 +77,6 @@
>     tso/gso/gro 'off'. */
>  #define VIRTIO_NET_RSC_DEFAULT_INTERVAL 300000
>  
> -/* temporary until standard header include it */
> -#if !defined(VIRTIO_NET_HDR_F_RSC_INFO)
> -
> -#define VIRTIO_NET_HDR_F_RSC_INFO  4 /* rsc_ext data in csum_ fields */
> -#define VIRTIO_NET_F_RSC_EXT       61
> -
> -#endif
> -
>  static inline __virtio16 *virtio_net_rsc_ext_num_packets(
>      struct virtio_net_hdr *hdr)
>  {
> -- 
> 2.21.1

Re: [PATCH 3/3] virtio-net: remove VIRTIO_NET_HDR_F_RSC_INFO compat handling

Posted by Jason Wang 5 years, 6 months ago

On 2020/4/27 下午6:24, Cornelia Huck wrote:
> VIRTIO_NET_HDR_F_RSC_INFO is available in the headers now.
>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>   hw/net/virtio-net.c | 8 --------
>   1 file changed, 8 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index e85d902588b3..7449570c7123 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -77,14 +77,6 @@
>      tso/gso/gro 'off'. */
>   #define VIRTIO_NET_RSC_DEFAULT_INTERVAL 300000
>   
> -/* temporary until standard header include it */
> -#if !defined(VIRTIO_NET_HDR_F_RSC_INFO)
> -
> -#define VIRTIO_NET_HDR_F_RSC_INFO  4 /* rsc_ext data in csum_ fields */
> -#define VIRTIO_NET_F_RSC_EXT       61
> -
> -#endif
> -
>   static inline __virtio16 *virtio_net_rsc_ext_num_packets(
>       struct virtio_net_hdr *hdr)
>   {


I think we should not keep the those tricky num_packets/dup_acks.

Thanks

Re: [PATCH 3/3] virtio-net: remove VIRTIO_NET_HDR_F_RSC_INFO compat handling

Posted by Cornelia Huck 5 years, 6 months ago

On Tue, 28 Apr 2020 16:19:15 +0800
Jason Wang <jasowang@redhat.com> wrote:

> On 2020/4/27 下午6:24, Cornelia Huck wrote:
> > VIRTIO_NET_HDR_F_RSC_INFO is available in the headers now.
> >
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> >   hw/net/virtio-net.c | 8 --------
> >   1 file changed, 8 deletions(-)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index e85d902588b3..7449570c7123 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -77,14 +77,6 @@
> >      tso/gso/gro 'off'. */
> >   #define VIRTIO_NET_RSC_DEFAULT_INTERVAL 300000
> >   
> > -/* temporary until standard header include it */
> > -#if !defined(VIRTIO_NET_HDR_F_RSC_INFO)
> > -
> > -#define VIRTIO_NET_HDR_F_RSC_INFO  4 /* rsc_ext data in csum_ fields */
> > -#define VIRTIO_NET_F_RSC_EXT       61
> > -
> > -#endif
> > -
> >   static inline __virtio16 *virtio_net_rsc_ext_num_packets(
> >       struct virtio_net_hdr *hdr)
> >   {  
> 
> 
> I think we should not keep the those tricky num_packets/dup_acks.

No real opinion here, patch 3 is only a cleanup.

The important one is patch 1, because without it I cannot do a headers
update.

Re: [PATCH 3/3] virtio-net: remove VIRTIO_NET_HDR_F_RSC_INFO compat handling

Posted by Jason Wang 5 years, 6 months ago

On 2020/4/28 下午4:34, Cornelia Huck wrote:
> On Tue, 28 Apr 2020 16:19:15 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
>> On 2020/4/27 下午6:24, Cornelia Huck wrote:
>>> VIRTIO_NET_HDR_F_RSC_INFO is available in the headers now.
>>>
>>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>>> ---
>>>    hw/net/virtio-net.c | 8 --------
>>>    1 file changed, 8 deletions(-)
>>>
>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>>> index e85d902588b3..7449570c7123 100644
>>> --- a/hw/net/virtio-net.c
>>> +++ b/hw/net/virtio-net.c
>>> @@ -77,14 +77,6 @@
>>>       tso/gso/gro 'off'. */
>>>    #define VIRTIO_NET_RSC_DEFAULT_INTERVAL 300000
>>>    
>>> -/* temporary until standard header include it */
>>> -#if !defined(VIRTIO_NET_HDR_F_RSC_INFO)
>>> -
>>> -#define VIRTIO_NET_HDR_F_RSC_INFO  4 /* rsc_ext data in csum_ fields */
>>> -#define VIRTIO_NET_F_RSC_EXT       61
>>> -
>>> -#endif
>>> -
>>>    static inline __virtio16 *virtio_net_rsc_ext_num_packets(
>>>        struct virtio_net_hdr *hdr)
>>>    {
>>
>> I think we should not keep the those tricky num_packets/dup_acks.
> No real opinion here, patch 3 is only a cleanup.
>
> The important one is patch 1, because without it I cannot do a headers
> update.


Yes, at least we should dereference segments/dup_acks instead of 
csum_start/csum_offsets since the header has been synced.

Thanks


>

Re: [PATCH 3/3] virtio-net: remove VIRTIO_NET_HDR_F_RSC_INFO compat handling

Posted by Cornelia Huck 5 years, 6 months ago

On Tue, 28 Apr 2020 16:58:44 +0800
Jason Wang <jasowang@redhat.com> wrote:

> On 2020/4/28 下午4:34, Cornelia Huck wrote:
> > On Tue, 28 Apr 2020 16:19:15 +0800
> > Jason Wang <jasowang@redhat.com> wrote:
> >  
> >> On 2020/4/27 下午6:24, Cornelia Huck wrote:  
> >>> VIRTIO_NET_HDR_F_RSC_INFO is available in the headers now.
> >>>
> >>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> >>> ---
> >>>    hw/net/virtio-net.c | 8 --------
> >>>    1 file changed, 8 deletions(-)
> >>>
> >>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >>> index e85d902588b3..7449570c7123 100644
> >>> --- a/hw/net/virtio-net.c
> >>> +++ b/hw/net/virtio-net.c
> >>> @@ -77,14 +77,6 @@
> >>>       tso/gso/gro 'off'. */
> >>>    #define VIRTIO_NET_RSC_DEFAULT_INTERVAL 300000
> >>>    
> >>> -/* temporary until standard header include it */
> >>> -#if !defined(VIRTIO_NET_HDR_F_RSC_INFO)
> >>> -
> >>> -#define VIRTIO_NET_HDR_F_RSC_INFO  4 /* rsc_ext data in csum_ fields */
> >>> -#define VIRTIO_NET_F_RSC_EXT       61
> >>> -
> >>> -#endif
> >>> -
> >>>    static inline __virtio16 *virtio_net_rsc_ext_num_packets(
> >>>        struct virtio_net_hdr *hdr)
> >>>    {  
> >>
> >> I think we should not keep the those tricky num_packets/dup_acks.  
> > No real opinion here, patch 3 is only a cleanup.
> >
> > The important one is patch 1, because without it I cannot do a headers
> > update.  
> 
> 
> Yes, at least we should dereference segments/dup_acks instead of 
> csum_start/csum_offsets since the header has been synced.

So what about:

- I merge patch 1 and the header sync now (because I have a bunch of
  patches that depend on it...)
- We change virtio-net to handle that properly on top (probably best
  done by someone familiar with the code base ;)

Re: [PATCH 3/3] virtio-net: remove VIRTIO_NET_HDR_F_RSC_INFO compat handling

Posted by Yuri Benditovich 5 years, 6 months ago

On Tue, Apr 28, 2020 at 12:18 PM Cornelia Huck <cohuck@redhat.com> wrote:

> On Tue, 28 Apr 2020 16:58:44 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
> > On 2020/4/28 下午4:34, Cornelia Huck wrote:
> > > On Tue, 28 Apr 2020 16:19:15 +0800
> > > Jason Wang <jasowang@redhat.com> wrote:
> > >
> > >> On 2020/4/27 下午6:24, Cornelia Huck wrote:
> > >>> VIRTIO_NET_HDR_F_RSC_INFO is available in the headers now.
> > >>>
> > >>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > >>> ---
> > >>>    hw/net/virtio-net.c | 8 --------
> > >>>    1 file changed, 8 deletions(-)
> > >>>
> > >>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > >>> index e85d902588b3..7449570c7123 100644
> > >>> --- a/hw/net/virtio-net.c
> > >>> +++ b/hw/net/virtio-net.c
> > >>> @@ -77,14 +77,6 @@
> > >>>       tso/gso/gro 'off'. */
> > >>>    #define VIRTIO_NET_RSC_DEFAULT_INTERVAL 300000
> > >>>
> > >>> -/* temporary until standard header include it */
> > >>> -#if !defined(VIRTIO_NET_HDR_F_RSC_INFO)
> > >>> -
> > >>> -#define VIRTIO_NET_HDR_F_RSC_INFO  4 /* rsc_ext data in csum_
> fields */
> > >>> -#define VIRTIO_NET_F_RSC_EXT       61
> > >>> -
> > >>> -#endif
> > >>> -
> > >>>    static inline __virtio16 *virtio_net_rsc_ext_num_packets(
> > >>>        struct virtio_net_hdr *hdr)
> > >>>    {
> > >>
> > >> I think we should not keep the those tricky num_packets/dup_acks.
> > > No real opinion here, patch 3 is only a cleanup.
> > >
> > > The important one is patch 1, because without it I cannot do a headers
> > > update.
> >
> >
> > Yes, at least we should dereference segments/dup_acks instead of
> > csum_start/csum_offsets since the header has been synced.
>
> So what about:
>
> - I merge patch 1 and the header sync now (because I have a bunch of
>   patches that depend on it...)
> - We change virtio-net to handle that properly on top (probably best
>   done by someone familiar with the code base ;)
>
>
Jason,
This series just solves the conflict caused by the update of Linux headers.
After this series is applied I can submit further patch to use actual RSC
definitions from linux headers.

Thanks,
Yuri

Re: [PATCH 3/3] virtio-net: remove VIRTIO_NET_HDR_F_RSC_INFO compat handling

Posted by Jason Wang 5 years, 6 months ago

On 2020/4/28 下午6:55, Yuri Benditovich wrote:
>
> On Tue, Apr 28, 2020 at 12:18 PM Cornelia Huck <cohuck@redhat.com 
> <mailto:cohuck@redhat.com>> wrote:
>
>     On Tue, 28 Apr 2020 16:58:44 +0800
>     Jason Wang <jasowang@redhat.com <mailto:jasowang@redhat.com>> wrote:
>
>     > On 2020/4/28 下午4:34, Cornelia Huck wrote:
>     > > On Tue, 28 Apr 2020 16:19:15 +0800
>     > > Jason Wang <jasowang@redhat.com <mailto:jasowang@redhat.com>>
>     wrote:
>     > >
>     > >> On 2020/4/27 下午6:24, Cornelia Huck wrote:
>     > >>> VIRTIO_NET_HDR_F_RSC_INFO is available in the headers now.
>     > >>>
>     > >>> Signed-off-by: Cornelia Huck <cohuck@redhat.com
>     <mailto:cohuck@redhat.com>>
>     > >>> ---
>     > >>>    hw/net/virtio-net.c | 8 --------
>     > >>>    1 file changed, 8 deletions(-)
>     > >>>
>     > >>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>     > >>> index e85d902588b3..7449570c7123 100644
>     > >>> --- a/hw/net/virtio-net.c
>     > >>> +++ b/hw/net/virtio-net.c
>     > >>> @@ -77,14 +77,6 @@
>     > >>>       tso/gso/gro 'off'. */
>     > >>>    #define VIRTIO_NET_RSC_DEFAULT_INTERVAL 300000
>     > >>>
>     > >>> -/* temporary until standard header include it */
>     > >>> -#if !defined(VIRTIO_NET_HDR_F_RSC_INFO)
>     > >>> -
>     > >>> -#define VIRTIO_NET_HDR_F_RSC_INFO  4 /* rsc_ext data in
>     csum_ fields */
>     > >>> -#define VIRTIO_NET_F_RSC_EXT       61
>     > >>> -
>     > >>> -#endif
>     > >>> -
>     > >>>    static inline __virtio16 *virtio_net_rsc_ext_num_packets(
>     > >>>        struct virtio_net_hdr *hdr)
>     > >>>    {
>     > >>
>     > >> I think we should not keep the those tricky
>     num_packets/dup_acks.
>     > > No real opinion here, patch 3 is only a cleanup.
>     > >
>     > > The important one is patch 1, because without it I cannot do a
>     headers
>     > > update.
>     >
>     >
>     > Yes, at least we should dereference segments/dup_acks instead of
>     > csum_start/csum_offsets since the header has been synced.
>
>     So what about:
>
>     - I merge patch 1 and the header sync now (because I have a bunch of
>       patches that depend on it...)
>     - We change virtio-net to handle that properly on top (probably best
>       done by someone familiar with the code base ;)
>
>
> Jason,
> This series just solves the conflict caused by the update of Linux 
> headers.
> After this series is applied I can submit further patch to use actual 
> RSC definitions from linux headers.
>
> Thanks,
> Yuri


Yes, please.

Thanks

Re: [PATCH 3/3] virtio-net: remove VIRTIO_NET_HDR_F_RSC_INFO compat handling

Posted by Jason Wang 5 years, 6 months ago

On 2020/4/28 下午5:18, Cornelia Huck wrote:
> On Tue, 28 Apr 2020 16:58:44 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
>> On 2020/4/28 下午4:34, Cornelia Huck wrote:
>>> On Tue, 28 Apr 2020 16:19:15 +0800
>>> Jason Wang <jasowang@redhat.com> wrote:
>>>   
>>>> On 2020/4/27 下午6:24, Cornelia Huck wrote:
>>>>> VIRTIO_NET_HDR_F_RSC_INFO is available in the headers now.
>>>>>
>>>>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>>>>> ---
>>>>>     hw/net/virtio-net.c | 8 --------
>>>>>     1 file changed, 8 deletions(-)
>>>>>
>>>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>>>>> index e85d902588b3..7449570c7123 100644
>>>>> --- a/hw/net/virtio-net.c
>>>>> +++ b/hw/net/virtio-net.c
>>>>> @@ -77,14 +77,6 @@
>>>>>        tso/gso/gro 'off'. */
>>>>>     #define VIRTIO_NET_RSC_DEFAULT_INTERVAL 300000
>>>>>     
>>>>> -/* temporary until standard header include it */
>>>>> -#if !defined(VIRTIO_NET_HDR_F_RSC_INFO)
>>>>> -
>>>>> -#define VIRTIO_NET_HDR_F_RSC_INFO  4 /* rsc_ext data in csum_ fields */
>>>>> -#define VIRTIO_NET_F_RSC_EXT       61
>>>>> -
>>>>> -#endif
>>>>> -
>>>>>     static inline __virtio16 *virtio_net_rsc_ext_num_packets(
>>>>>         struct virtio_net_hdr *hdr)
>>>>>     {
>>>> I think we should not keep the those tricky num_packets/dup_acks.
>>> No real opinion here, patch 3 is only a cleanup.
>>>
>>> The important one is patch 1, because without it I cannot do a headers
>>> update.
>>
>> Yes, at least we should dereference segments/dup_acks instead of
>> csum_start/csum_offsets since the header has been synced.
> So what about:
>
> - I merge patch 1 and the header sync now (because I have a bunch of
>    patches that depend on it...)
> - We change virtio-net to handle that properly on top (probably best
>    done by someone familiar with the code base ;)


That's fine.

Thanks