[PATCH] vmxnet3: add stub for encapsulation offload

Alexander Bulekov posted 1 patch 2 years, 9 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210806222300.417126-1-alxndr@bu.edu
Maintainers: Dmitry Fleytman <dmitry.fleytman@gmail.com>, Jason Wang <jasowang@redhat.com>
hw/net/vmxnet3.c | 5 +++++
hw/net/vmxnet3.h | 1 +
2 files changed, 6 insertions(+)
[PATCH] vmxnet3: add stub for encapsulation offload
Posted by Alexander Bulekov 2 years, 9 months ago
Encapsulation offload (offload mode 1) is a valid mode present in the
kernel that isn't implemented in QEMU, yet.

https://lore.kernel.org/lkml/20200528015426.8285-4-doshir@vmware.com/

Add a stub for this mode, to avoid the guest-triggerable assertion.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/517
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 hw/net/vmxnet3.c | 5 +++++
 hw/net/vmxnet3.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 41f796a247..62e11d0e3e 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -443,6 +443,11 @@ vmxnet3_setup_tx_offloads(VMXNET3State *s)
         net_tx_pkt_build_vheader(s->tx_pkt, false, false, 0);
         break;
 
+    case VMXNET3_OM_ENCAP:
+        VMW_PKPRN("Encapsulation offload requested, but not available\n");
+        return false;
+        break;
+
     case VMXNET3_OM_CSUM:
         net_tx_pkt_build_vheader(s->tx_pkt, false, true, 0);
         VMW_PKPRN("L4 CSO requested\n");
diff --git a/hw/net/vmxnet3.h b/hw/net/vmxnet3.h
index 5b3b76ba7a..36a17b82aa 100644
--- a/hw/net/vmxnet3.h
+++ b/hw/net/vmxnet3.h
@@ -273,6 +273,7 @@ struct Vmxnet3_TxDesc {
 
 /* TxDesc.OM values */
 #define VMXNET3_OM_NONE        0
+#define VMXNET3_OM_ENCAP       1
 #define VMXNET3_OM_CSUM        2
 #define VMXNET3_OM_TSO        3
 
-- 
2.30.2


Re: [PATCH] vmxnet3: add stub for encapsulation offload
Posted by Thomas Huth 1 year, 9 months ago
On 07/08/2021 00.23, Alexander Bulekov wrote:
> Encapsulation offload (offload mode 1) is a valid mode present in the
> kernel that isn't implemented in QEMU, yet.
> 
> https://lore.kernel.org/lkml/20200528015426.8285-4-doshir@vmware.com/
> 
> Add a stub for this mode, to avoid the guest-triggerable assertion.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/517
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>   hw/net/vmxnet3.c | 5 +++++
>   hw/net/vmxnet3.h | 1 +
>   2 files changed, 6 insertions(+)
> 
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 41f796a247..62e11d0e3e 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -443,6 +443,11 @@ vmxnet3_setup_tx_offloads(VMXNET3State *s)
>           net_tx_pkt_build_vheader(s->tx_pkt, false, false, 0);
>           break;
>   
> +    case VMXNET3_OM_ENCAP:
> +        VMW_PKPRN("Encapsulation offload requested, but not available\n");
> +        return false;
> +        break;

I think I'd also remove the break, anyway:

Reviewed-by: Thomas Huth <thuth@redhat.com>

Jason, could you maybe also pick this up for QEMU 7.1, so we can close issue 
517 ?

>       case VMXNET3_OM_CSUM:
>           net_tx_pkt_build_vheader(s->tx_pkt, false, true, 0);
>           VMW_PKPRN("L4 CSO requested\n");
> diff --git a/hw/net/vmxnet3.h b/hw/net/vmxnet3.h
> index 5b3b76ba7a..36a17b82aa 100644
> --- a/hw/net/vmxnet3.h
> +++ b/hw/net/vmxnet3.h
> @@ -273,6 +273,7 @@ struct Vmxnet3_TxDesc {
>   
>   /* TxDesc.OM values */
>   #define VMXNET3_OM_NONE        0
> +#define VMXNET3_OM_ENCAP       1
>   #define VMXNET3_OM_CSUM        2
>   #define VMXNET3_OM_TSO        3
>
Re: [PATCH] vmxnet3: add stub for encapsulation offload
Posted by Philippe Mathieu-Daudé 2 years, 9 months ago
On 8/7/21 12:23 AM, Alexander Bulekov wrote:
> Encapsulation offload (offload mode 1) is a valid mode present in the
> kernel that isn't implemented in QEMU, yet.
> 
> https://lore.kernel.org/lkml/20200528015426.8285-4-doshir@vmware.com/
> 
> Add a stub for this mode, to avoid the guest-triggerable assertion.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/517
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  hw/net/vmxnet3.c | 5 +++++
>  hw/net/vmxnet3.h | 1 +
>  2 files changed, 6 insertions(+)
> 
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 41f796a247..62e11d0e3e 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -443,6 +443,11 @@ vmxnet3_setup_tx_offloads(VMXNET3State *s)
>          net_tx_pkt_build_vheader(s->tx_pkt, false, false, 0);
>          break;
>  
> +    case VMXNET3_OM_ENCAP:
> +        VMW_PKPRN("Encapsulation offload requested, but not available\n");
> +        return false;
> +        break;

No need to break if you returned (unreachable).

> +
>      case VMXNET3_OM_CSUM:
>          net_tx_pkt_build_vheader(s->tx_pkt, false, true, 0);
>          VMW_PKPRN("L4 CSO requested\n");
> diff --git a/hw/net/vmxnet3.h b/hw/net/vmxnet3.h
> index 5b3b76ba7a..36a17b82aa 100644
> --- a/hw/net/vmxnet3.h
> +++ b/hw/net/vmxnet3.h
> @@ -273,6 +273,7 @@ struct Vmxnet3_TxDesc {
>  
>  /* TxDesc.OM values */
>  #define VMXNET3_OM_NONE        0
> +#define VMXNET3_OM_ENCAP       1
>  #define VMXNET3_OM_CSUM        2
>  #define VMXNET3_OM_TSO        3
>  
> 


Re: [PATCH] vmxnet3: add stub for encapsulation offload
Posted by Alexander Bulekov 2 years, 9 months ago
On 210807 1019, Philippe Mathieu-Daudé wrote:
> On 8/7/21 12:23 AM, Alexander Bulekov wrote:
> > Encapsulation offload (offload mode 1) is a valid mode present in the
> > kernel that isn't implemented in QEMU, yet.
> > 
> > https://lore.kernel.org/lkml/20200528015426.8285-4-doshir@vmware.com/
> > 
> > Add a stub for this mode, to avoid the guest-triggerable assertion.
> > 
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/517
> > Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> > ---
> >  hw/net/vmxnet3.c | 5 +++++
> >  hw/net/vmxnet3.h | 1 +
> >  2 files changed, 6 insertions(+)
> > 
> > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> > index 41f796a247..62e11d0e3e 100644
> > --- a/hw/net/vmxnet3.c
> > +++ b/hw/net/vmxnet3.c
> > @@ -443,6 +443,11 @@ vmxnet3_setup_tx_offloads(VMXNET3State *s)
> >          net_tx_pkt_build_vheader(s->tx_pkt, false, false, 0);
> >          break;
> >  
> > +    case VMXNET3_OM_ENCAP:
> > +        VMW_PKPRN("Encapsulation offload requested, but not available\n");
> > +        return false;
> > +        break;
> 
> No need to break if you returned (unreachable).

I included it in case the feature ends up being added, so there is one
less thing to worry about, but I can remove it. There are a couple of
places in QEMU where this occurs, so I wasn't sure about the best
practice.

-Alex

> 
> > +
> >      case VMXNET3_OM_CSUM:
> >          net_tx_pkt_build_vheader(s->tx_pkt, false, true, 0);
> >          VMW_PKPRN("L4 CSO requested\n");
> > diff --git a/hw/net/vmxnet3.h b/hw/net/vmxnet3.h
> > index 5b3b76ba7a..36a17b82aa 100644
> > --- a/hw/net/vmxnet3.h
> > +++ b/hw/net/vmxnet3.h
> > @@ -273,6 +273,7 @@ struct Vmxnet3_TxDesc {
> >  
> >  /* TxDesc.OM values */
> >  #define VMXNET3_OM_NONE        0
> > +#define VMXNET3_OM_ENCAP       1
> >  #define VMXNET3_OM_CSUM        2
> >  #define VMXNET3_OM_TSO        3
> >  
> > 
> 

Re: [PATCH] vmxnet3: add stub for encapsulation offload
Posted by Philippe Mathieu-Daudé 2 years, 9 months ago
On 8/7/21 10:25 PM, Alexander Bulekov wrote:
> On 210807 1019, Philippe Mathieu-Daudé wrote:
>> On 8/7/21 12:23 AM, Alexander Bulekov wrote:
>>> Encapsulation offload (offload mode 1) is a valid mode present in the
>>> kernel that isn't implemented in QEMU, yet.
>>>
>>> https://lore.kernel.org/lkml/20200528015426.8285-4-doshir@vmware.com/
>>>
>>> Add a stub for this mode, to avoid the guest-triggerable assertion.
>>>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/517
>>> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
>>> ---
>>>  hw/net/vmxnet3.c | 5 +++++
>>>  hw/net/vmxnet3.h | 1 +
>>>  2 files changed, 6 insertions(+)
>>>
>>> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
>>> index 41f796a247..62e11d0e3e 100644
>>> --- a/hw/net/vmxnet3.c
>>> +++ b/hw/net/vmxnet3.c
>>> @@ -443,6 +443,11 @@ vmxnet3_setup_tx_offloads(VMXNET3State *s)
>>>          net_tx_pkt_build_vheader(s->tx_pkt, false, false, 0);
>>>          break;
>>>  
>>> +    case VMXNET3_OM_ENCAP:
>>> +        VMW_PKPRN("Encapsulation offload requested, but not available\n");
>>> +        return false;
>>> +        break;
>>
>> No need to break if you returned (unreachable).
> 
> I included it in case the feature ends up being added, so there is one
> less thing to worry about, but I can remove it. There are a couple of
> places in QEMU where this occurs, so I wasn't sure about the best
> practice.

I don't know if there is an official "best practice". My style /
recommendation is to try to use single exit point when possible
so I can easily add trace events.