Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.

2015-02-15 Thread Rusty Russell
"Michael S. Tsirkin"  writes:
>>  struct virtio_net_hdr hdr;
>>  __virtio16 num_buffers; /* Number of merged rx buffers */
>>  };
>> +#else /* ... VIRTIO_NET_NO_LEGACY */
>> +/*
>> + * This header comes first in the scatter-gather list.  If you don't
>> + * specify GSO or CSUM features, you can simply ignore the header.
>> + *
>> + * This is bitwise-equivalent to the legacy struct virtio_net_hdr_mrg_rxbuf.
>
> I would add "but with all fields squashed into the main structure".

Yep.

>> + */
>> +struct virtio_net_hdr_v1 {
>> +#define VIRTIO_NET_HDR_F_NEEDS_CSUM 1   /* Use csum_start, csum_offset 
>> */
>> +#define VIRTIO_NET_HDR_F_DATA_VALID 2   /* Csum is valid */
>> +__u8 flags;
>> +#define VIRTIO_NET_HDR_GSO_NONE 0   /* Not a GSO frame */
>> +#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_ECN  0x80/* TCP has ECN set */
>> +__u8 gso_type;
>> +__virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
>> +__virtio16 gso_size;/* Bytes to append to hdr_len per frame */
>> +__virtio16 csum_start;  /* Position to start checksumming from */
>> +__virtio16 csum_offset; /* Offset after that to place checksum */
>> +__virtio16 num_buffers; /* Number of merged rx buffers */
>> +};
>> +#endif /* ...VIRTIO_NET_NO_LEGACY */
>
> I note that this way host code can't be structured like this:
>
>
>   struct virtio_net_hdr_v1 modern;
>   /* handle modern guests */
>   
>
> #ifndef VIRTIO_NET_NO_LEGACY
>   struct virtio_net_hdr_mrg_rxbuf mrg;
>   /* handle legacy guests */
>   
> #endif
>
>
> Define virtio_net_hdr_v1 unconditionally?

Thanks, that's a good idea!  Here's the incremental:

virtio_net: unconditionally define struct virtio_net_hdr_v1.

This was introduced in commit ed9ecb0415b97b5f9f91f146e1977bb372c74c6d,
but only defined if !VIRTIO_NET_NO_LEGACY.  We should always define
it: easier for users to have conditional legacy code.

Suggested-by: "Michael S. Tsirkin" 
Signed-off-by: Rusty Russell 

diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 4a9b58113d6e..7bbee79ca293 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -74,39 +74,12 @@ struct virtio_net_config {
__u16 max_virtqueue_pairs;
 } __attribute__((packed));
 
-#ifndef VIRTIO_NET_NO_LEGACY
-/* This header comes first in the scatter-gather list.
- * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
- * be the first element of the scatter-gather list.  If you don't
- * specify GSO or CSUM features, you can simply ignore the header. */
-struct virtio_net_hdr {
-#define VIRTIO_NET_HDR_F_NEEDS_CSUM1   // Use csum_start, csum_offset
-#define VIRTIO_NET_HDR_F_DATA_VALID2   // Csum is valid
-   __u8 flags;
-#define VIRTIO_NET_HDR_GSO_NONE0   // Not a GSO frame
-#define VIRTIO_NET_HDR_GSO_TCPV4   1   // GSO frame, IPv4 TCP (TSO)
-#define VIRTIO_NET_HDR_GSO_UDP 3   // GSO frame, IPv4 UDP (UFO)
-#define VIRTIO_NET_HDR_GSO_TCPV6   4   // GSO frame, IPv6 TCP
-#define VIRTIO_NET_HDR_GSO_ECN 0x80// TCP has ECN set
-   __u8 gso_type;
-   __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
-   __virtio16 gso_size;/* Bytes to append to hdr_len per frame 
*/
-   __virtio16 csum_start;  /* Position to start checksumming from */
-   __virtio16 csum_offset; /* Offset after that to place checksum */
-};
-
-/* This is the version of the header to use when the MRG_RXBUF
- * feature has been negotiated. */
-struct virtio_net_hdr_mrg_rxbuf {
-   struct virtio_net_hdr hdr;
-   __virtio16 num_buffers; /* Number of merged rx buffers */
-};
-#else /* ... VIRTIO_NET_NO_LEGACY */
 /*
  * This header comes first in the scatter-gather list.  If you don't
  * specify GSO or CSUM features, you can simply ignore the header.
  *
- * This is bitwise-equivalent to the legacy struct virtio_net_hdr_mrg_rxbuf.
+ * This is bitwise-equivalent to the legacy struct virtio_net_hdr_mrg_rxbuf,
+ * only flattened.
  */
 struct virtio_net_hdr_v1 {
 #define VIRTIO_NET_HDR_F_NEEDS_CSUM1   /* Use csum_start, csum_offset 
*/
@@ -124,6 +97,29 @@ struct virtio_net_hdr_v1 {
__virtio16 csum_offset; /* Offset after that to place checksum */
__virtio16 num_buffers; /* Number of merged rx buffers */
 };
+
+#ifndef VIRTIO_NET_NO_LEGACY
+/* This header comes first in the scatter-gather list.
+ * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
+ * be the first element of the scatter-gather list.  If you don't
+ * specify GSO or CSUM features, you can simply ignore the header. */
+struct virtio_net_hdr {
+   /* See 

Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.

2015-02-15 Thread Michael S. Tsirkin
>   struct virtio_net_hdr hdr;
>   __virtio16 num_buffers; /* Number of merged rx buffers */
>  };
> +#else /* ... VIRTIO_NET_NO_LEGACY */
> +/*
> + * This header comes first in the scatter-gather list.  If you don't
> + * specify GSO or CSUM features, you can simply ignore the header.
> + *
> + * This is bitwise-equivalent to the legacy struct virtio_net_hdr_mrg_rxbuf.

I would add "but with all fields squashed into the main structure".

> + */
> +struct virtio_net_hdr_v1 {
> +#define VIRTIO_NET_HDR_F_NEEDS_CSUM  1   /* Use csum_start, csum_offset 
> */
> +#define VIRTIO_NET_HDR_F_DATA_VALID  2   /* Csum is valid */
> + __u8 flags;
> +#define VIRTIO_NET_HDR_GSO_NONE  0   /* Not a GSO frame */
> +#define VIRTIO_NET_HDR_GSO_TCPV4 1   /* GSO frame, IPv4 TCP (TSO) */
> +#define VIRTIO_NET_HDR_GSO_UDP   3   /* GSO frame, IPv4 UDP 
> (UFO) */
> +#define VIRTIO_NET_HDR_GSO_TCPV6 4   /* GSO frame, IPv6 TCP */
> +#define VIRTIO_NET_HDR_GSO_ECN   0x80/* TCP has ECN set */
> + __u8 gso_type;
> + __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
> + __virtio16 gso_size;/* Bytes to append to hdr_len per frame */
> + __virtio16 csum_start;  /* Position to start checksumming from */
> + __virtio16 csum_offset; /* Offset after that to place checksum */
> + __virtio16 num_buffers; /* Number of merged rx buffers */
> +};
> +#endif /* ...VIRTIO_NET_NO_LEGACY */

I note that this way host code can't be structured like this:


struct virtio_net_hdr_v1 modern;
/* handle modern guests */


#ifndef VIRTIO_NET_NO_LEGACY
struct virtio_net_hdr_mrg_rxbuf mrg;
/* handle legacy guests */

#endif


Define virtio_net_hdr_v1 unconditionally?










>  /*
>   * Control virtqueue data structures
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.

2015-02-15 Thread Rusty Russell
"Michael S. Tsirkin"  writes:
> On Sun, Feb 08, 2015 at 11:59:08AM +0100, Michael S. Tsirkin wrote:
>> On Fri, Feb 06, 2015 at 03:36:54PM +1030, Rusty Russell wrote:
>> > In particular, the virtio header always has the u16 num_buffers field.
>> > We define a new 'struct virtio_net_modern_hdr' for this (rather than
>> > simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
>> > if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.
>> 
>> This kind of masks the fact that it's the same as
>> virtio_net_hdr_mrg_rxbuf. So it's forcing people to duplicate
>> code for transitional devices.
>> 
>> How about
>> struct virtio_net_modern_hdr {
>>  struct virtio_net_hdr_mrg_rxbuf hdr;
>> }
>> 
>> 
>> This will also make it look nicer when we start
>> adding stuff in the header, the main header
>> is separated in a struct by its own, so it's
>> easy to apply operations such as sizeof.
>
> Ping.
> Would you like a patch on top that does this?

Hmm, I thought I replied...

Indeed, I did.  I disagreed, and simply renamed struct
virtio_net_modern_hdr to virtio_net_hdr_v1.

Cheers,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.

2015-02-15 Thread Rusty Russell
Michael S. Tsirkin m...@redhat.com writes:
 On Sun, Feb 08, 2015 at 11:59:08AM +0100, Michael S. Tsirkin wrote:
 On Fri, Feb 06, 2015 at 03:36:54PM +1030, Rusty Russell wrote:
  In particular, the virtio header always has the u16 num_buffers field.
  We define a new 'struct virtio_net_modern_hdr' for this (rather than
  simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
  if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.
 
 This kind of masks the fact that it's the same as
 virtio_net_hdr_mrg_rxbuf. So it's forcing people to duplicate
 code for transitional devices.
 
 How about
 struct virtio_net_modern_hdr {
  struct virtio_net_hdr_mrg_rxbuf hdr;
 }
 
 
 This will also make it look nicer when we start
 adding stuff in the header, the main header
 is separated in a struct by its own, so it's
 easy to apply operations such as sizeof.

 Ping.
 Would you like a patch on top that does this?

Hmm, I thought I replied...

Indeed, I did.  I disagreed, and simply renamed struct
virtio_net_modern_hdr to virtio_net_hdr_v1.

Cheers,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.

2015-02-15 Thread Michael S. Tsirkin
   struct virtio_net_hdr hdr;
   __virtio16 num_buffers; /* Number of merged rx buffers */
  };
 +#else /* ... VIRTIO_NET_NO_LEGACY */
 +/*
 + * This header comes first in the scatter-gather list.  If you don't
 + * specify GSO or CSUM features, you can simply ignore the header.
 + *
 + * This is bitwise-equivalent to the legacy struct virtio_net_hdr_mrg_rxbuf.

I would add but with all fields squashed into the main structure.

 + */
 +struct virtio_net_hdr_v1 {
 +#define VIRTIO_NET_HDR_F_NEEDS_CSUM  1   /* Use csum_start, csum_offset 
 */
 +#define VIRTIO_NET_HDR_F_DATA_VALID  2   /* Csum is valid */
 + __u8 flags;
 +#define VIRTIO_NET_HDR_GSO_NONE  0   /* Not a GSO frame */
 +#define VIRTIO_NET_HDR_GSO_TCPV4 1   /* GSO frame, IPv4 TCP (TSO) */
 +#define VIRTIO_NET_HDR_GSO_UDP   3   /* GSO frame, IPv4 UDP 
 (UFO) */
 +#define VIRTIO_NET_HDR_GSO_TCPV6 4   /* GSO frame, IPv6 TCP */
 +#define VIRTIO_NET_HDR_GSO_ECN   0x80/* TCP has ECN set */
 + __u8 gso_type;
 + __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
 + __virtio16 gso_size;/* Bytes to append to hdr_len per frame */
 + __virtio16 csum_start;  /* Position to start checksumming from */
 + __virtio16 csum_offset; /* Offset after that to place checksum */
 + __virtio16 num_buffers; /* Number of merged rx buffers */
 +};
 +#endif /* ...VIRTIO_NET_NO_LEGACY */

I note that this way host code can't be structured like this:


struct virtio_net_hdr_v1 modern;
/* handle modern guests */


#ifndef VIRTIO_NET_NO_LEGACY
struct virtio_net_hdr_mrg_rxbuf mrg;
/* handle legacy guests */

#endif


Define virtio_net_hdr_v1 unconditionally?










  /*
   * Control virtqueue data structures
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.

2015-02-15 Thread Rusty Russell
Michael S. Tsirkin m...@redhat.com writes:
  struct virtio_net_hdr hdr;
  __virtio16 num_buffers; /* Number of merged rx buffers */
  };
 +#else /* ... VIRTIO_NET_NO_LEGACY */
 +/*
 + * This header comes first in the scatter-gather list.  If you don't
 + * specify GSO or CSUM features, you can simply ignore the header.
 + *
 + * This is bitwise-equivalent to the legacy struct virtio_net_hdr_mrg_rxbuf.

 I would add but with all fields squashed into the main structure.

Yep.

 + */
 +struct virtio_net_hdr_v1 {
 +#define VIRTIO_NET_HDR_F_NEEDS_CSUM 1   /* Use csum_start, csum_offset 
 */
 +#define VIRTIO_NET_HDR_F_DATA_VALID 2   /* Csum is valid */
 +__u8 flags;
 +#define VIRTIO_NET_HDR_GSO_NONE 0   /* Not a GSO frame */
 +#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_ECN  0x80/* TCP has ECN set */
 +__u8 gso_type;
 +__virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
 +__virtio16 gso_size;/* Bytes to append to hdr_len per frame */
 +__virtio16 csum_start;  /* Position to start checksumming from */
 +__virtio16 csum_offset; /* Offset after that to place checksum */
 +__virtio16 num_buffers; /* Number of merged rx buffers */
 +};
 +#endif /* ...VIRTIO_NET_NO_LEGACY */

 I note that this way host code can't be structured like this:


   struct virtio_net_hdr_v1 modern;
   /* handle modern guests */
   

 #ifndef VIRTIO_NET_NO_LEGACY
   struct virtio_net_hdr_mrg_rxbuf mrg;
   /* handle legacy guests */
   
 #endif


 Define virtio_net_hdr_v1 unconditionally?

Thanks, that's a good idea!  Here's the incremental:

virtio_net: unconditionally define struct virtio_net_hdr_v1.

This was introduced in commit ed9ecb0415b97b5f9f91f146e1977bb372c74c6d,
but only defined if !VIRTIO_NET_NO_LEGACY.  We should always define
it: easier for users to have conditional legacy code.

Suggested-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Rusty Russell ru...@rustcorp.com.au

diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 4a9b58113d6e..7bbee79ca293 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -74,39 +74,12 @@ struct virtio_net_config {
__u16 max_virtqueue_pairs;
 } __attribute__((packed));
 
-#ifndef VIRTIO_NET_NO_LEGACY
-/* This header comes first in the scatter-gather list.
- * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
- * be the first element of the scatter-gather list.  If you don't
- * specify GSO or CSUM features, you can simply ignore the header. */
-struct virtio_net_hdr {
-#define VIRTIO_NET_HDR_F_NEEDS_CSUM1   // Use csum_start, csum_offset
-#define VIRTIO_NET_HDR_F_DATA_VALID2   // Csum is valid
-   __u8 flags;
-#define VIRTIO_NET_HDR_GSO_NONE0   // Not a GSO frame
-#define VIRTIO_NET_HDR_GSO_TCPV4   1   // GSO frame, IPv4 TCP (TSO)
-#define VIRTIO_NET_HDR_GSO_UDP 3   // GSO frame, IPv4 UDP (UFO)
-#define VIRTIO_NET_HDR_GSO_TCPV6   4   // GSO frame, IPv6 TCP
-#define VIRTIO_NET_HDR_GSO_ECN 0x80// TCP has ECN set
-   __u8 gso_type;
-   __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
-   __virtio16 gso_size;/* Bytes to append to hdr_len per frame 
*/
-   __virtio16 csum_start;  /* Position to start checksumming from */
-   __virtio16 csum_offset; /* Offset after that to place checksum */
-};
-
-/* This is the version of the header to use when the MRG_RXBUF
- * feature has been negotiated. */
-struct virtio_net_hdr_mrg_rxbuf {
-   struct virtio_net_hdr hdr;
-   __virtio16 num_buffers; /* Number of merged rx buffers */
-};
-#else /* ... VIRTIO_NET_NO_LEGACY */
 /*
  * This header comes first in the scatter-gather list.  If you don't
  * specify GSO or CSUM features, you can simply ignore the header.
  *
- * This is bitwise-equivalent to the legacy struct virtio_net_hdr_mrg_rxbuf.
+ * This is bitwise-equivalent to the legacy struct virtio_net_hdr_mrg_rxbuf,
+ * only flattened.
  */
 struct virtio_net_hdr_v1 {
 #define VIRTIO_NET_HDR_F_NEEDS_CSUM1   /* Use csum_start, csum_offset 
*/
@@ -124,6 +97,29 @@ struct virtio_net_hdr_v1 {
__virtio16 csum_offset; /* Offset after that to place checksum */
__virtio16 num_buffers; /* Number of merged rx buffers */
 };
+
+#ifndef VIRTIO_NET_NO_LEGACY
+/* This header comes first in the scatter-gather list.
+ * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
+ * be the first element of the scatter-gather list.  If you don't
+ * specify GSO or CSUM features, you can simply ignore the header. */
+struct virtio_net_hdr {
+   /* See VIRTIO_NET_HDR_F_* */
+   

Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.

2015-02-14 Thread Michael S. Tsirkin
On Sun, Feb 08, 2015 at 11:59:08AM +0100, Michael S. Tsirkin wrote:
> On Fri, Feb 06, 2015 at 03:36:54PM +1030, Rusty Russell wrote:
> > In particular, the virtio header always has the u16 num_buffers field.
> > We define a new 'struct virtio_net_modern_hdr' for this (rather than
> > simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
> > if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.
> > 
> > Signed-off-by: Rusty Russell 
> > ---
> >  include/uapi/linux/virtio_net.h | 30 --
> >  1 file changed, 28 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/uapi/linux/virtio_net.h 
> > b/include/uapi/linux/virtio_net.h
> > index b5f1677b291c..32754f3000e8 100644
> > --- a/include/uapi/linux/virtio_net.h
> > +++ b/include/uapi/linux/virtio_net.h
> > @@ -35,7 +35,6 @@
> >  #define VIRTIO_NET_F_CSUM  0   /* Host handles pkts w/ partial csum */
> >  #define VIRTIO_NET_F_GUEST_CSUM1   /* Guest handles pkts w/ 
> > partial csum */
> >  #define VIRTIO_NET_F_MAC   5   /* Host has given MAC address. */
> > -#define VIRTIO_NET_F_GSO   6   /* Host handles pkts w/ any GSO type */
> >  #define VIRTIO_NET_F_GUEST_TSO47   /* Guest can handle TSOv4 in. */
> >  #define VIRTIO_NET_F_GUEST_TSO68   /* Guest can handle TSOv6 in. */
> >  #define VIRTIO_NET_F_GUEST_ECN 9   /* Guest can handle TSO[6] w/ 
> > ECN in. */
> > @@ -56,6 +55,10 @@
> >  * Steering */
> >  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23  /* Set MAC address */
> >  
> > +#ifndef VIRTIO_NET_NO_LEGACY
> > +#define VIRTIO_NET_F_GSO   6   /* Host handles pkts w/ any GSO type */
> > +#endif /* VIRTIO_NET_NO_LEGACY */
> > +
> >  #define VIRTIO_NET_S_LINK_UP   1   /* Link is up */
> >  #define VIRTIO_NET_S_ANNOUNCE  2   /* Announcement is needed */
> >  
> > @@ -71,8 +74,9 @@ struct virtio_net_config {
> > __u16 max_virtqueue_pairs;
> >  } __attribute__((packed));
> >  
> > +#ifndef VIRTIO_NET_NO_LEGACY
> >  /* This header comes first in the scatter-gather list.
> > - * If VIRTIO_F_ANY_LAYOUT is not negotiated, it must
> > + * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
> >   * be the first element of the scatter-gather list.  If you don't
> >   * specify GSO or CSUM features, you can simply ignore the header. */
> >  struct virtio_net_hdr {
> > @@ -97,6 +101,28 @@ struct virtio_net_hdr_mrg_rxbuf {
> > struct virtio_net_hdr hdr;
> > __virtio16 num_buffers; /* Number of merged rx buffers */
> >  };
> > +#else /* ... VIRTIO_NET_NO_LEGACY */
> > +/*
> > + * This header comes first in the scatter-gather list.  If you don't
> > + * specify GSO or CSUM features, you can simply ignore the header.
> > + */
> > +struct virtio_net_modern_hdr {
> > +#define VIRTIO_NET_HDR_F_NEEDS_CSUM1   /* Use csum_start, 
> > csum_offset */
> > +#define VIRTIO_NET_HDR_F_DATA_VALID2   /* Csum is valid */
> > +   __u8 flags;
> > +#define VIRTIO_NET_HDR_GSO_NONE0   /* Not a GSO frame */
> > +#define VIRTIO_NET_HDR_GSO_TCPV4   1   /* GSO frame, IPv4 TCP (TSO) */
> > +#define VIRTIO_NET_HDR_GSO_UDP 3   /* GSO frame, IPv4 UDP 
> > (UFO) */
> > +#define VIRTIO_NET_HDR_GSO_TCPV6   4   /* GSO frame, IPv6 TCP */
> > +#define VIRTIO_NET_HDR_GSO_ECN 0x80/* TCP has ECN set */
> > +   __u8 gso_type;
> > +   __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
> > +   __virtio16 gso_size;/* Bytes to append to hdr_len per frame */
> > +   __virtio16 csum_start;  /* Position to start checksumming from */
> > +   __virtio16 csum_offset; /* Offset after that to place checksum */
> > +   __virtio16 num_buffers; /* Number of merged rx buffers */
> > +};
> > +#endif /* ...VIRTIO_NET_NO_LEGACY */
> 
> This kind of masks the fact that it's the same as
> virtio_net_hdr_mrg_rxbuf. So it's forcing people to duplicate
> code for transitional devices.
> 
> How about
> struct virtio_net_modern_hdr {
>   struct virtio_net_hdr_mrg_rxbuf hdr;
> }
> 
> 
> This will also make it look nicer when we start
> adding stuff in the header, the main header
> is separated in a struct by its own, so it's
> easy to apply operations such as sizeof.

Ping.
Would you like a patch on top that does this?


> 
> >  /*
> >   * Control virtqueue data structures
> > -- 
> > 2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.

2015-02-14 Thread Michael S. Tsirkin
On Sun, Feb 08, 2015 at 11:59:08AM +0100, Michael S. Tsirkin wrote:
 On Fri, Feb 06, 2015 at 03:36:54PM +1030, Rusty Russell wrote:
  In particular, the virtio header always has the u16 num_buffers field.
  We define a new 'struct virtio_net_modern_hdr' for this (rather than
  simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
  if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.
  
  Signed-off-by: Rusty Russell ru...@rustcorp.com.au
  ---
   include/uapi/linux/virtio_net.h | 30 --
   1 file changed, 28 insertions(+), 2 deletions(-)
  
  diff --git a/include/uapi/linux/virtio_net.h 
  b/include/uapi/linux/virtio_net.h
  index b5f1677b291c..32754f3000e8 100644
  --- a/include/uapi/linux/virtio_net.h
  +++ b/include/uapi/linux/virtio_net.h
  @@ -35,7 +35,6 @@
   #define VIRTIO_NET_F_CSUM  0   /* Host handles pkts w/ partial csum */
   #define VIRTIO_NET_F_GUEST_CSUM1   /* Guest handles pkts w/ 
  partial csum */
   #define VIRTIO_NET_F_MAC   5   /* Host has given MAC address. */
  -#define VIRTIO_NET_F_GSO   6   /* Host handles pkts w/ any GSO type */
   #define VIRTIO_NET_F_GUEST_TSO47   /* Guest can handle TSOv4 in. */
   #define VIRTIO_NET_F_GUEST_TSO68   /* Guest can handle TSOv6 in. */
   #define VIRTIO_NET_F_GUEST_ECN 9   /* Guest can handle TSO[6] w/ 
  ECN in. */
  @@ -56,6 +55,10 @@
   * Steering */
   #define VIRTIO_NET_F_CTRL_MAC_ADDR 23  /* Set MAC address */
   
  +#ifndef VIRTIO_NET_NO_LEGACY
  +#define VIRTIO_NET_F_GSO   6   /* Host handles pkts w/ any GSO type */
  +#endif /* VIRTIO_NET_NO_LEGACY */
  +
   #define VIRTIO_NET_S_LINK_UP   1   /* Link is up */
   #define VIRTIO_NET_S_ANNOUNCE  2   /* Announcement is needed */
   
  @@ -71,8 +74,9 @@ struct virtio_net_config {
  __u16 max_virtqueue_pairs;
   } __attribute__((packed));
   
  +#ifndef VIRTIO_NET_NO_LEGACY
   /* This header comes first in the scatter-gather list.
  - * If VIRTIO_F_ANY_LAYOUT is not negotiated, it must
  + * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
* be the first element of the scatter-gather list.  If you don't
* specify GSO or CSUM features, you can simply ignore the header. */
   struct virtio_net_hdr {
  @@ -97,6 +101,28 @@ struct virtio_net_hdr_mrg_rxbuf {
  struct virtio_net_hdr hdr;
  __virtio16 num_buffers; /* Number of merged rx buffers */
   };
  +#else /* ... VIRTIO_NET_NO_LEGACY */
  +/*
  + * This header comes first in the scatter-gather list.  If you don't
  + * specify GSO or CSUM features, you can simply ignore the header.
  + */
  +struct virtio_net_modern_hdr {
  +#define VIRTIO_NET_HDR_F_NEEDS_CSUM1   /* Use csum_start, 
  csum_offset */
  +#define VIRTIO_NET_HDR_F_DATA_VALID2   /* Csum is valid */
  +   __u8 flags;
  +#define VIRTIO_NET_HDR_GSO_NONE0   /* Not a GSO frame */
  +#define VIRTIO_NET_HDR_GSO_TCPV4   1   /* GSO frame, IPv4 TCP (TSO) */
  +#define VIRTIO_NET_HDR_GSO_UDP 3   /* GSO frame, IPv4 UDP 
  (UFO) */
  +#define VIRTIO_NET_HDR_GSO_TCPV6   4   /* GSO frame, IPv6 TCP */
  +#define VIRTIO_NET_HDR_GSO_ECN 0x80/* TCP has ECN set */
  +   __u8 gso_type;
  +   __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
  +   __virtio16 gso_size;/* Bytes to append to hdr_len per frame */
  +   __virtio16 csum_start;  /* Position to start checksumming from */
  +   __virtio16 csum_offset; /* Offset after that to place checksum */
  +   __virtio16 num_buffers; /* Number of merged rx buffers */
  +};
  +#endif /* ...VIRTIO_NET_NO_LEGACY */
 
 This kind of masks the fact that it's the same as
 virtio_net_hdr_mrg_rxbuf. So it's forcing people to duplicate
 code for transitional devices.
 
 How about
 struct virtio_net_modern_hdr {
   struct virtio_net_hdr_mrg_rxbuf hdr;
 }
 
 
 This will also make it look nicer when we start
 adding stuff in the header, the main header
 is separated in a struct by its own, so it's
 easy to apply operations such as sizeof.

Ping.
Would you like a patch on top that does this?


 
   /*
* Control virtqueue data structures
  -- 
  2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.

2015-02-10 Thread Rusty Russell
Rusty Russell  writes:
> "Michael S. Tsirkin"  writes:
>> On Fri, Feb 06, 2015 at 03:36:54PM +1030, Rusty Russell wrote:
>>> In particular, the virtio header always has the u16 num_buffers field.
>>> We define a new 'struct virtio_net_modern_hdr' for this (rather than
>>> simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
>>> if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.
>>> 
>>> Signed-off-by: Rusty Russell 
>>> ---
>>>  include/uapi/linux/virtio_net.h | 30 --
>>>  1 file changed, 28 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/include/uapi/linux/virtio_net.h 
>>> b/include/uapi/linux/virtio_net.h
>>> index b5f1677b291c..32754f3000e8 100644
>>> --- a/include/uapi/linux/virtio_net.h
>>> +++ b/include/uapi/linux/virtio_net.h
>>> @@ -35,7 +35,6 @@
>>>  #define VIRTIO_NET_F_CSUM  0   /* Host handles pkts w/ partial csum */
>>>  #define VIRTIO_NET_F_GUEST_CSUM1   /* Guest handles pkts w/ 
>>> partial csum */
>>>  #define VIRTIO_NET_F_MAC   5   /* Host has given MAC address. */
>>> -#define VIRTIO_NET_F_GSO   6   /* Host handles pkts w/ any GSO type */
>>>  #define VIRTIO_NET_F_GUEST_TSO47   /* Guest can handle TSOv4 in. */
>>>  #define VIRTIO_NET_F_GUEST_TSO68   /* Guest can handle TSOv6 in. */
>>>  #define VIRTIO_NET_F_GUEST_ECN 9   /* Guest can handle TSO[6] w/ 
>>> ECN in. */
>>> @@ -56,6 +55,10 @@
>>>  * Steering */
>>>  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23  /* Set MAC address */
>>>  
>>> +#ifndef VIRTIO_NET_NO_LEGACY
>>> +#define VIRTIO_NET_F_GSO   6   /* Host handles pkts w/ any GSO type */
>>> +#endif /* VIRTIO_NET_NO_LEGACY */
>>> +
>>>  #define VIRTIO_NET_S_LINK_UP   1   /* Link is up */
>>>  #define VIRTIO_NET_S_ANNOUNCE  2   /* Announcement is needed */
>>>  
>>> @@ -71,8 +74,9 @@ struct virtio_net_config {
>>> __u16 max_virtqueue_pairs;
>>>  } __attribute__((packed));
>>>  
>>> +#ifndef VIRTIO_NET_NO_LEGACY
>>>  /* This header comes first in the scatter-gather list.
>>> - * If VIRTIO_F_ANY_LAYOUT is not negotiated, it must
>>> + * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
>>>   * be the first element of the scatter-gather list.  If you don't
>>>   * specify GSO or CSUM features, you can simply ignore the header. */
>>>  struct virtio_net_hdr {
>>> @@ -97,6 +101,28 @@ struct virtio_net_hdr_mrg_rxbuf {
>>> struct virtio_net_hdr hdr;
>>> __virtio16 num_buffers; /* Number of merged rx buffers */
>>>  };
>>> +#else /* ... VIRTIO_NET_NO_LEGACY */
>>> +/*
>>> + * This header comes first in the scatter-gather list.  If you don't
>>> + * specify GSO or CSUM features, you can simply ignore the header.
>>> + */
>>> +struct virtio_net_modern_hdr {
>>> +#define VIRTIO_NET_HDR_F_NEEDS_CSUM1   /* Use csum_start, 
>>> csum_offset */
>>> +#define VIRTIO_NET_HDR_F_DATA_VALID2   /* Csum is valid */
>>> +   __u8 flags;
>>> +#define VIRTIO_NET_HDR_GSO_NONE0   /* Not a GSO frame */
>>> +#define VIRTIO_NET_HDR_GSO_TCPV4   1   /* GSO frame, IPv4 TCP (TSO) */
>>> +#define VIRTIO_NET_HDR_GSO_UDP 3   /* GSO frame, IPv4 UDP 
>>> (UFO) */
>>> +#define VIRTIO_NET_HDR_GSO_TCPV6   4   /* GSO frame, IPv6 TCP */
>>> +#define VIRTIO_NET_HDR_GSO_ECN 0x80/* TCP has ECN set */
>>> +   __u8 gso_type;
>>> +   __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
>>> +   __virtio16 gso_size;/* Bytes to append to hdr_len per frame */
>>> +   __virtio16 csum_start;  /* Position to start checksumming from */
>>> +   __virtio16 csum_offset; /* Offset after that to place checksum */
>>> +   __virtio16 num_buffers; /* Number of merged rx buffers */
>>> +};
>>> +#endif /* ...VIRTIO_NET_NO_LEGACY */
>>
>> This kind of masks the fact that it's the same as
>> virtio_net_hdr_mrg_rxbuf. So it's forcing people to duplicate
>> code for transitional devices.
>
> It's a hard problem to solve; someone suffers.  My preference is to make
> non-legacy implementations as "clean" as possible, even if transitional
> devices suffer (since their suffering should be shorter-lived).
>
> Transitional devices can't define VIRTIO_NET_NO_LEGACY, so they'll
> have to use virtio_net_hdr_mrg_rxbuf anyway.

Here's what I ended up with.  In effect, it's just the name change
and a comment that it's the same as the legacy virtio_net_hdr_mrg_rxbuf:


Subject: virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY 
defined.

In particular, the virtio header always has the u16 num_buffers field.
We define a new 'struct virtio_net_hdr_v1' for this (rather than
simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.

Transitional devices (which can't define VIRTIO_NET_NO_LEGACY) will
have to keep using struct virtio_net_hdr_mrg_rxbuf, which has the same
byte layout as struct 

Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.

2015-02-10 Thread Rusty Russell
Rusty Russell ru...@rustcorp.com.au writes:
 Michael S. Tsirkin m...@redhat.com writes:
 On Fri, Feb 06, 2015 at 03:36:54PM +1030, Rusty Russell wrote:
 In particular, the virtio header always has the u16 num_buffers field.
 We define a new 'struct virtio_net_modern_hdr' for this (rather than
 simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
 if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.
 
 Signed-off-by: Rusty Russell ru...@rustcorp.com.au
 ---
  include/uapi/linux/virtio_net.h | 30 --
  1 file changed, 28 insertions(+), 2 deletions(-)
 
 diff --git a/include/uapi/linux/virtio_net.h 
 b/include/uapi/linux/virtio_net.h
 index b5f1677b291c..32754f3000e8 100644
 --- a/include/uapi/linux/virtio_net.h
 +++ b/include/uapi/linux/virtio_net.h
 @@ -35,7 +35,6 @@
  #define VIRTIO_NET_F_CSUM  0   /* Host handles pkts w/ partial csum */
  #define VIRTIO_NET_F_GUEST_CSUM1   /* Guest handles pkts w/ 
 partial csum */
  #define VIRTIO_NET_F_MAC   5   /* Host has given MAC address. */
 -#define VIRTIO_NET_F_GSO   6   /* Host handles pkts w/ any GSO type */
  #define VIRTIO_NET_F_GUEST_TSO47   /* Guest can handle TSOv4 in. */
  #define VIRTIO_NET_F_GUEST_TSO68   /* Guest can handle TSOv6 in. */
  #define VIRTIO_NET_F_GUEST_ECN 9   /* Guest can handle TSO[6] w/ 
 ECN in. */
 @@ -56,6 +55,10 @@
  * Steering */
  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23  /* Set MAC address */
  
 +#ifndef VIRTIO_NET_NO_LEGACY
 +#define VIRTIO_NET_F_GSO   6   /* Host handles pkts w/ any GSO type */
 +#endif /* VIRTIO_NET_NO_LEGACY */
 +
  #define VIRTIO_NET_S_LINK_UP   1   /* Link is up */
  #define VIRTIO_NET_S_ANNOUNCE  2   /* Announcement is needed */
  
 @@ -71,8 +74,9 @@ struct virtio_net_config {
 __u16 max_virtqueue_pairs;
  } __attribute__((packed));
  
 +#ifndef VIRTIO_NET_NO_LEGACY
  /* This header comes first in the scatter-gather list.
 - * If VIRTIO_F_ANY_LAYOUT is not negotiated, it must
 + * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
   * be the first element of the scatter-gather list.  If you don't
   * specify GSO or CSUM features, you can simply ignore the header. */
  struct virtio_net_hdr {
 @@ -97,6 +101,28 @@ struct virtio_net_hdr_mrg_rxbuf {
 struct virtio_net_hdr hdr;
 __virtio16 num_buffers; /* Number of merged rx buffers */
  };
 +#else /* ... VIRTIO_NET_NO_LEGACY */
 +/*
 + * This header comes first in the scatter-gather list.  If you don't
 + * specify GSO or CSUM features, you can simply ignore the header.
 + */
 +struct virtio_net_modern_hdr {
 +#define VIRTIO_NET_HDR_F_NEEDS_CSUM1   /* Use csum_start, 
 csum_offset */
 +#define VIRTIO_NET_HDR_F_DATA_VALID2   /* Csum is valid */
 +   __u8 flags;
 +#define VIRTIO_NET_HDR_GSO_NONE0   /* Not a GSO frame */
 +#define VIRTIO_NET_HDR_GSO_TCPV4   1   /* GSO frame, IPv4 TCP (TSO) */
 +#define VIRTIO_NET_HDR_GSO_UDP 3   /* GSO frame, IPv4 UDP 
 (UFO) */
 +#define VIRTIO_NET_HDR_GSO_TCPV6   4   /* GSO frame, IPv6 TCP */
 +#define VIRTIO_NET_HDR_GSO_ECN 0x80/* TCP has ECN set */
 +   __u8 gso_type;
 +   __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
 +   __virtio16 gso_size;/* Bytes to append to hdr_len per frame */
 +   __virtio16 csum_start;  /* Position to start checksumming from */
 +   __virtio16 csum_offset; /* Offset after that to place checksum */
 +   __virtio16 num_buffers; /* Number of merged rx buffers */
 +};
 +#endif /* ...VIRTIO_NET_NO_LEGACY */

 This kind of masks the fact that it's the same as
 virtio_net_hdr_mrg_rxbuf. So it's forcing people to duplicate
 code for transitional devices.

 It's a hard problem to solve; someone suffers.  My preference is to make
 non-legacy implementations as clean as possible, even if transitional
 devices suffer (since their suffering should be shorter-lived).

 Transitional devices can't define VIRTIO_NET_NO_LEGACY, so they'll
 have to use virtio_net_hdr_mrg_rxbuf anyway.

Here's what I ended up with.  In effect, it's just the name change
and a comment that it's the same as the legacy virtio_net_hdr_mrg_rxbuf:


Subject: virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY 
defined.

In particular, the virtio header always has the u16 num_buffers field.
We define a new 'struct virtio_net_hdr_v1' for this (rather than
simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.

Transitional devices (which can't define VIRTIO_NET_NO_LEGACY) will
have to keep using struct virtio_net_hdr_mrg_rxbuf, which has the same
byte layout as struct virtio_net_hdr_v1.

Signed-off-by: Rusty Russell ru...@rustcorp.com.au

diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index b5f1677b291c..4a9b58113d6e 100644

Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.

2015-02-08 Thread Rusty Russell
"Michael S. Tsirkin"  writes:
> On Fri, Feb 06, 2015 at 03:36:54PM +1030, Rusty Russell wrote:
>> In particular, the virtio header always has the u16 num_buffers field.
>> We define a new 'struct virtio_net_modern_hdr' for this (rather than
>> simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
>> if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.
>> 
>> Signed-off-by: Rusty Russell 
>> ---
>>  include/uapi/linux/virtio_net.h | 30 --
>>  1 file changed, 28 insertions(+), 2 deletions(-)
>> 
>> diff --git a/include/uapi/linux/virtio_net.h 
>> b/include/uapi/linux/virtio_net.h
>> index b5f1677b291c..32754f3000e8 100644
>> --- a/include/uapi/linux/virtio_net.h
>> +++ b/include/uapi/linux/virtio_net.h
>> @@ -35,7 +35,6 @@
>>  #define VIRTIO_NET_F_CSUM   0   /* Host handles pkts w/ partial csum */
>>  #define VIRTIO_NET_F_GUEST_CSUM 1   /* Guest handles pkts w/ 
>> partial csum */
>>  #define VIRTIO_NET_F_MAC5   /* Host has given MAC address. */
>> -#define VIRTIO_NET_F_GSO6   /* Host handles pkts w/ any GSO type */
>>  #define VIRTIO_NET_F_GUEST_TSO4 7   /* Guest can handle TSOv4 in. */
>>  #define VIRTIO_NET_F_GUEST_TSO6 8   /* Guest can handle TSOv6 in. */
>>  #define VIRTIO_NET_F_GUEST_ECN  9   /* Guest can handle TSO[6] w/ 
>> ECN in. */
>> @@ -56,6 +55,10 @@
>>   * Steering */
>>  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23   /* Set MAC address */
>>  
>> +#ifndef VIRTIO_NET_NO_LEGACY
>> +#define VIRTIO_NET_F_GSO6   /* Host handles pkts w/ any GSO type */
>> +#endif /* VIRTIO_NET_NO_LEGACY */
>> +
>>  #define VIRTIO_NET_S_LINK_UP1   /* Link is up */
>>  #define VIRTIO_NET_S_ANNOUNCE   2   /* Announcement is needed */
>>  
>> @@ -71,8 +74,9 @@ struct virtio_net_config {
>>  __u16 max_virtqueue_pairs;
>>  } __attribute__((packed));
>>  
>> +#ifndef VIRTIO_NET_NO_LEGACY
>>  /* This header comes first in the scatter-gather list.
>> - * If VIRTIO_F_ANY_LAYOUT is not negotiated, it must
>> + * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
>>   * be the first element of the scatter-gather list.  If you don't
>>   * specify GSO or CSUM features, you can simply ignore the header. */
>>  struct virtio_net_hdr {
>> @@ -97,6 +101,28 @@ struct virtio_net_hdr_mrg_rxbuf {
>>  struct virtio_net_hdr hdr;
>>  __virtio16 num_buffers; /* Number of merged rx buffers */
>>  };
>> +#else /* ... VIRTIO_NET_NO_LEGACY */
>> +/*
>> + * This header comes first in the scatter-gather list.  If you don't
>> + * specify GSO or CSUM features, you can simply ignore the header.
>> + */
>> +struct virtio_net_modern_hdr {
>> +#define VIRTIO_NET_HDR_F_NEEDS_CSUM 1   /* Use csum_start, csum_offset 
>> */
>> +#define VIRTIO_NET_HDR_F_DATA_VALID 2   /* Csum is valid */
>> +__u8 flags;
>> +#define VIRTIO_NET_HDR_GSO_NONE 0   /* Not a GSO frame */
>> +#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_ECN  0x80/* TCP has ECN set */
>> +__u8 gso_type;
>> +__virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
>> +__virtio16 gso_size;/* Bytes to append to hdr_len per frame */
>> +__virtio16 csum_start;  /* Position to start checksumming from */
>> +__virtio16 csum_offset; /* Offset after that to place checksum */
>> +__virtio16 num_buffers; /* Number of merged rx buffers */
>> +};
>> +#endif /* ...VIRTIO_NET_NO_LEGACY */
>
> This kind of masks the fact that it's the same as
> virtio_net_hdr_mrg_rxbuf. So it's forcing people to duplicate
> code for transitional devices.

It's a hard problem to solve; someone suffers.  My preference is to make
non-legacy implementations as "clean" as possible, even if transitional
devices suffer (since their suffering should be shorter-lived).

Transitional devices can't define VIRTIO_NET_NO_LEGACY, so they'll
have to use virtio_net_hdr_mrg_rxbuf anyway.

> How about
> struct virtio_net_modern_hdr {
>   struct virtio_net_hdr_mrg_rxbuf hdr;
> }

But modern code now has refer to a field like this:

hdr.hdr.hdr.flags

> This will also make it look nicer when we start
> adding stuff in the header, the main header
> is separated in a struct by its own, so it's
> easy to apply operations such as sizeof.

net is the only header we've extended, and I'm not aware of any plans to
extend again.  If we did, we could wrap the header to make it a bit more
awkward to use, but easier for next time, but that won't help for the
time *after* that.

We might rename struct virtio_net_modern_hdr to struct virtio_net_hdr_v1
though (which makes future naming easier).

Cheers,
Rusty.
--
To unsubscribe from this list: send the 

Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.

2015-02-08 Thread Michael S. Tsirkin
On Fri, Feb 06, 2015 at 03:36:54PM +1030, Rusty Russell wrote:
> In particular, the virtio header always has the u16 num_buffers field.
> We define a new 'struct virtio_net_modern_hdr' for this (rather than
> simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
> if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.
> 
> Signed-off-by: Rusty Russell 
> ---
>  include/uapi/linux/virtio_net.h | 30 --
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index b5f1677b291c..32754f3000e8 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -35,7 +35,6 @@
>  #define VIRTIO_NET_F_CSUM0   /* Host handles pkts w/ partial csum */
>  #define VIRTIO_NET_F_GUEST_CSUM  1   /* Guest handles pkts w/ 
> partial csum */
>  #define VIRTIO_NET_F_MAC 5   /* Host has given MAC address. */
> -#define VIRTIO_NET_F_GSO 6   /* Host handles pkts w/ any GSO type */
>  #define VIRTIO_NET_F_GUEST_TSO4  7   /* Guest can handle TSOv4 in. */
>  #define VIRTIO_NET_F_GUEST_TSO6  8   /* Guest can handle TSOv6 in. */
>  #define VIRTIO_NET_F_GUEST_ECN   9   /* Guest can handle TSO[6] w/ 
> ECN in. */
> @@ -56,6 +55,10 @@
>* Steering */
>  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */
>  
> +#ifndef VIRTIO_NET_NO_LEGACY
> +#define VIRTIO_NET_F_GSO 6   /* Host handles pkts w/ any GSO type */
> +#endif /* VIRTIO_NET_NO_LEGACY */
> +
>  #define VIRTIO_NET_S_LINK_UP 1   /* Link is up */
>  #define VIRTIO_NET_S_ANNOUNCE2   /* Announcement is needed */
>  
> @@ -71,8 +74,9 @@ struct virtio_net_config {
>   __u16 max_virtqueue_pairs;
>  } __attribute__((packed));
>  
> +#ifndef VIRTIO_NET_NO_LEGACY
>  /* This header comes first in the scatter-gather list.
> - * If VIRTIO_F_ANY_LAYOUT is not negotiated, it must
> + * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
>   * be the first element of the scatter-gather list.  If you don't
>   * specify GSO or CSUM features, you can simply ignore the header. */
>  struct virtio_net_hdr {
> @@ -97,6 +101,28 @@ struct virtio_net_hdr_mrg_rxbuf {
>   struct virtio_net_hdr hdr;
>   __virtio16 num_buffers; /* Number of merged rx buffers */
>  };
> +#else /* ... VIRTIO_NET_NO_LEGACY */
> +/*
> + * This header comes first in the scatter-gather list.  If you don't
> + * specify GSO or CSUM features, you can simply ignore the header.
> + */
> +struct virtio_net_modern_hdr {
> +#define VIRTIO_NET_HDR_F_NEEDS_CSUM  1   /* Use csum_start, csum_offset 
> */
> +#define VIRTIO_NET_HDR_F_DATA_VALID  2   /* Csum is valid */
> + __u8 flags;
> +#define VIRTIO_NET_HDR_GSO_NONE  0   /* Not a GSO frame */
> +#define VIRTIO_NET_HDR_GSO_TCPV4 1   /* GSO frame, IPv4 TCP (TSO) */
> +#define VIRTIO_NET_HDR_GSO_UDP   3   /* GSO frame, IPv4 UDP 
> (UFO) */
> +#define VIRTIO_NET_HDR_GSO_TCPV6 4   /* GSO frame, IPv6 TCP */
> +#define VIRTIO_NET_HDR_GSO_ECN   0x80/* TCP has ECN set */
> + __u8 gso_type;
> + __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
> + __virtio16 gso_size;/* Bytes to append to hdr_len per frame */
> + __virtio16 csum_start;  /* Position to start checksumming from */
> + __virtio16 csum_offset; /* Offset after that to place checksum */
> + __virtio16 num_buffers; /* Number of merged rx buffers */
> +};
> +#endif /* ...VIRTIO_NET_NO_LEGACY */

This kind of masks the fact that it's the same as
virtio_net_hdr_mrg_rxbuf. So it's forcing people to duplicate
code for transitional devices.

How about
struct virtio_net_modern_hdr {
struct virtio_net_hdr_mrg_rxbuf hdr;
}


This will also make it look nicer when we start
adding stuff in the header, the main header
is separated in a struct by its own, so it's
easy to apply operations such as sizeof.


>  /*
>   * Control virtqueue data structures
> -- 
> 2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.

2015-02-08 Thread Michael S. Tsirkin
On Fri, Feb 06, 2015 at 03:36:54PM +1030, Rusty Russell wrote:
 In particular, the virtio header always has the u16 num_buffers field.
 We define a new 'struct virtio_net_modern_hdr' for this (rather than
 simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
 if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.
 
 Signed-off-by: Rusty Russell ru...@rustcorp.com.au
 ---
  include/uapi/linux/virtio_net.h | 30 --
  1 file changed, 28 insertions(+), 2 deletions(-)
 
 diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
 index b5f1677b291c..32754f3000e8 100644
 --- a/include/uapi/linux/virtio_net.h
 +++ b/include/uapi/linux/virtio_net.h
 @@ -35,7 +35,6 @@
  #define VIRTIO_NET_F_CSUM0   /* Host handles pkts w/ partial csum */
  #define VIRTIO_NET_F_GUEST_CSUM  1   /* Guest handles pkts w/ 
 partial csum */
  #define VIRTIO_NET_F_MAC 5   /* Host has given MAC address. */
 -#define VIRTIO_NET_F_GSO 6   /* Host handles pkts w/ any GSO type */
  #define VIRTIO_NET_F_GUEST_TSO4  7   /* Guest can handle TSOv4 in. */
  #define VIRTIO_NET_F_GUEST_TSO6  8   /* Guest can handle TSOv6 in. */
  #define VIRTIO_NET_F_GUEST_ECN   9   /* Guest can handle TSO[6] w/ 
 ECN in. */
 @@ -56,6 +55,10 @@
* Steering */
  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */
  
 +#ifndef VIRTIO_NET_NO_LEGACY
 +#define VIRTIO_NET_F_GSO 6   /* Host handles pkts w/ any GSO type */
 +#endif /* VIRTIO_NET_NO_LEGACY */
 +
  #define VIRTIO_NET_S_LINK_UP 1   /* Link is up */
  #define VIRTIO_NET_S_ANNOUNCE2   /* Announcement is needed */
  
 @@ -71,8 +74,9 @@ struct virtio_net_config {
   __u16 max_virtqueue_pairs;
  } __attribute__((packed));
  
 +#ifndef VIRTIO_NET_NO_LEGACY
  /* This header comes first in the scatter-gather list.
 - * If VIRTIO_F_ANY_LAYOUT is not negotiated, it must
 + * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
   * be the first element of the scatter-gather list.  If you don't
   * specify GSO or CSUM features, you can simply ignore the header. */
  struct virtio_net_hdr {
 @@ -97,6 +101,28 @@ struct virtio_net_hdr_mrg_rxbuf {
   struct virtio_net_hdr hdr;
   __virtio16 num_buffers; /* Number of merged rx buffers */
  };
 +#else /* ... VIRTIO_NET_NO_LEGACY */
 +/*
 + * This header comes first in the scatter-gather list.  If you don't
 + * specify GSO or CSUM features, you can simply ignore the header.
 + */
 +struct virtio_net_modern_hdr {
 +#define VIRTIO_NET_HDR_F_NEEDS_CSUM  1   /* Use csum_start, csum_offset 
 */
 +#define VIRTIO_NET_HDR_F_DATA_VALID  2   /* Csum is valid */
 + __u8 flags;
 +#define VIRTIO_NET_HDR_GSO_NONE  0   /* Not a GSO frame */
 +#define VIRTIO_NET_HDR_GSO_TCPV4 1   /* GSO frame, IPv4 TCP (TSO) */
 +#define VIRTIO_NET_HDR_GSO_UDP   3   /* GSO frame, IPv4 UDP 
 (UFO) */
 +#define VIRTIO_NET_HDR_GSO_TCPV6 4   /* GSO frame, IPv6 TCP */
 +#define VIRTIO_NET_HDR_GSO_ECN   0x80/* TCP has ECN set */
 + __u8 gso_type;
 + __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
 + __virtio16 gso_size;/* Bytes to append to hdr_len per frame */
 + __virtio16 csum_start;  /* Position to start checksumming from */
 + __virtio16 csum_offset; /* Offset after that to place checksum */
 + __virtio16 num_buffers; /* Number of merged rx buffers */
 +};
 +#endif /* ...VIRTIO_NET_NO_LEGACY */

This kind of masks the fact that it's the same as
virtio_net_hdr_mrg_rxbuf. So it's forcing people to duplicate
code for transitional devices.

How about
struct virtio_net_modern_hdr {
struct virtio_net_hdr_mrg_rxbuf hdr;
}


This will also make it look nicer when we start
adding stuff in the header, the main header
is separated in a struct by its own, so it's
easy to apply operations such as sizeof.


  /*
   * Control virtqueue data structures
 -- 
 2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.

2015-02-08 Thread Rusty Russell
Michael S. Tsirkin m...@redhat.com writes:
 On Fri, Feb 06, 2015 at 03:36:54PM +1030, Rusty Russell wrote:
 In particular, the virtio header always has the u16 num_buffers field.
 We define a new 'struct virtio_net_modern_hdr' for this (rather than
 simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
 if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.
 
 Signed-off-by: Rusty Russell ru...@rustcorp.com.au
 ---
  include/uapi/linux/virtio_net.h | 30 --
  1 file changed, 28 insertions(+), 2 deletions(-)
 
 diff --git a/include/uapi/linux/virtio_net.h 
 b/include/uapi/linux/virtio_net.h
 index b5f1677b291c..32754f3000e8 100644
 --- a/include/uapi/linux/virtio_net.h
 +++ b/include/uapi/linux/virtio_net.h
 @@ -35,7 +35,6 @@
  #define VIRTIO_NET_F_CSUM   0   /* Host handles pkts w/ partial csum */
  #define VIRTIO_NET_F_GUEST_CSUM 1   /* Guest handles pkts w/ 
 partial csum */
  #define VIRTIO_NET_F_MAC5   /* Host has given MAC address. */
 -#define VIRTIO_NET_F_GSO6   /* Host handles pkts w/ any GSO type */
  #define VIRTIO_NET_F_GUEST_TSO4 7   /* Guest can handle TSOv4 in. */
  #define VIRTIO_NET_F_GUEST_TSO6 8   /* Guest can handle TSOv6 in. */
  #define VIRTIO_NET_F_GUEST_ECN  9   /* Guest can handle TSO[6] w/ 
 ECN in. */
 @@ -56,6 +55,10 @@
   * Steering */
  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23   /* Set MAC address */
  
 +#ifndef VIRTIO_NET_NO_LEGACY
 +#define VIRTIO_NET_F_GSO6   /* Host handles pkts w/ any GSO type */
 +#endif /* VIRTIO_NET_NO_LEGACY */
 +
  #define VIRTIO_NET_S_LINK_UP1   /* Link is up */
  #define VIRTIO_NET_S_ANNOUNCE   2   /* Announcement is needed */
  
 @@ -71,8 +74,9 @@ struct virtio_net_config {
  __u16 max_virtqueue_pairs;
  } __attribute__((packed));
  
 +#ifndef VIRTIO_NET_NO_LEGACY
  /* This header comes first in the scatter-gather list.
 - * If VIRTIO_F_ANY_LAYOUT is not negotiated, it must
 + * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
   * be the first element of the scatter-gather list.  If you don't
   * specify GSO or CSUM features, you can simply ignore the header. */
  struct virtio_net_hdr {
 @@ -97,6 +101,28 @@ struct virtio_net_hdr_mrg_rxbuf {
  struct virtio_net_hdr hdr;
  __virtio16 num_buffers; /* Number of merged rx buffers */
  };
 +#else /* ... VIRTIO_NET_NO_LEGACY */
 +/*
 + * This header comes first in the scatter-gather list.  If you don't
 + * specify GSO or CSUM features, you can simply ignore the header.
 + */
 +struct virtio_net_modern_hdr {
 +#define VIRTIO_NET_HDR_F_NEEDS_CSUM 1   /* Use csum_start, csum_offset 
 */
 +#define VIRTIO_NET_HDR_F_DATA_VALID 2   /* Csum is valid */
 +__u8 flags;
 +#define VIRTIO_NET_HDR_GSO_NONE 0   /* Not a GSO frame */
 +#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_ECN  0x80/* TCP has ECN set */
 +__u8 gso_type;
 +__virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
 +__virtio16 gso_size;/* Bytes to append to hdr_len per frame */
 +__virtio16 csum_start;  /* Position to start checksumming from */
 +__virtio16 csum_offset; /* Offset after that to place checksum */
 +__virtio16 num_buffers; /* Number of merged rx buffers */
 +};
 +#endif /* ...VIRTIO_NET_NO_LEGACY */

 This kind of masks the fact that it's the same as
 virtio_net_hdr_mrg_rxbuf. So it's forcing people to duplicate
 code for transitional devices.

It's a hard problem to solve; someone suffers.  My preference is to make
non-legacy implementations as clean as possible, even if transitional
devices suffer (since their suffering should be shorter-lived).

Transitional devices can't define VIRTIO_NET_NO_LEGACY, so they'll
have to use virtio_net_hdr_mrg_rxbuf anyway.

 How about
 struct virtio_net_modern_hdr {
   struct virtio_net_hdr_mrg_rxbuf hdr;
 }

But modern code now has refer to a field like this:

hdr.hdr.hdr.flags

 This will also make it look nicer when we start
 adding stuff in the header, the main header
 is separated in a struct by its own, so it's
 easy to apply operations such as sizeof.

net is the only header we've extended, and I'm not aware of any plans to
extend again.  If we did, we could wrap the header to make it a bit more
awkward to use, but easier for next time, but that won't help for the
time *after* that.

We might rename struct virtio_net_modern_hdr to struct virtio_net_hdr_v1
though (which makes future naming easier).

Cheers,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

[PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.

2015-02-05 Thread Rusty Russell
In particular, the virtio header always has the u16 num_buffers field.
We define a new 'struct virtio_net_modern_hdr' for this (rather than
simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.

Signed-off-by: Rusty Russell 
---
 include/uapi/linux/virtio_net.h | 30 --
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index b5f1677b291c..32754f3000e8 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -35,7 +35,6 @@
 #define VIRTIO_NET_F_CSUM  0   /* Host handles pkts w/ partial csum */
 #define VIRTIO_NET_F_GUEST_CSUM1   /* Guest handles pkts w/ 
partial csum */
 #define VIRTIO_NET_F_MAC   5   /* Host has given MAC address. */
-#define VIRTIO_NET_F_GSO   6   /* Host handles pkts w/ any GSO type */
 #define VIRTIO_NET_F_GUEST_TSO47   /* Guest can handle TSOv4 in. */
 #define VIRTIO_NET_F_GUEST_TSO68   /* Guest can handle TSOv6 in. */
 #define VIRTIO_NET_F_GUEST_ECN 9   /* Guest can handle TSO[6] w/ ECN in. */
@@ -56,6 +55,10 @@
 * Steering */
 #define VIRTIO_NET_F_CTRL_MAC_ADDR 23  /* Set MAC address */
 
+#ifndef VIRTIO_NET_NO_LEGACY
+#define VIRTIO_NET_F_GSO   6   /* Host handles pkts w/ any GSO type */
+#endif /* VIRTIO_NET_NO_LEGACY */
+
 #define VIRTIO_NET_S_LINK_UP   1   /* Link is up */
 #define VIRTIO_NET_S_ANNOUNCE  2   /* Announcement is needed */
 
@@ -71,8 +74,9 @@ struct virtio_net_config {
__u16 max_virtqueue_pairs;
 } __attribute__((packed));
 
+#ifndef VIRTIO_NET_NO_LEGACY
 /* This header comes first in the scatter-gather list.
- * If VIRTIO_F_ANY_LAYOUT is not negotiated, it must
+ * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
  * be the first element of the scatter-gather list.  If you don't
  * specify GSO or CSUM features, you can simply ignore the header. */
 struct virtio_net_hdr {
@@ -97,6 +101,28 @@ struct virtio_net_hdr_mrg_rxbuf {
struct virtio_net_hdr hdr;
__virtio16 num_buffers; /* Number of merged rx buffers */
 };
+#else /* ... VIRTIO_NET_NO_LEGACY */
+/*
+ * This header comes first in the scatter-gather list.  If you don't
+ * specify GSO or CSUM features, you can simply ignore the header.
+ */
+struct virtio_net_modern_hdr {
+#define VIRTIO_NET_HDR_F_NEEDS_CSUM1   /* Use csum_start, csum_offset 
*/
+#define VIRTIO_NET_HDR_F_DATA_VALID2   /* Csum is valid */
+   __u8 flags;
+#define VIRTIO_NET_HDR_GSO_NONE0   /* Not a GSO frame */
+#define VIRTIO_NET_HDR_GSO_TCPV4   1   /* GSO frame, IPv4 TCP (TSO) */
+#define VIRTIO_NET_HDR_GSO_UDP 3   /* GSO frame, IPv4 UDP (UFO) */
+#define VIRTIO_NET_HDR_GSO_TCPV6   4   /* GSO frame, IPv6 TCP */
+#define VIRTIO_NET_HDR_GSO_ECN 0x80/* TCP has ECN set */
+   __u8 gso_type;
+   __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
+   __virtio16 gso_size;/* Bytes to append to hdr_len per frame */
+   __virtio16 csum_start;  /* Position to start checksumming from */
+   __virtio16 csum_offset; /* Offset after that to place checksum */
+   __virtio16 num_buffers; /* Number of merged rx buffers */
+};
+#endif /* ...VIRTIO_NET_NO_LEGACY */
 
 /*
  * Control virtqueue data structures
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.

2015-02-05 Thread Rusty Russell
In particular, the virtio header always has the u16 num_buffers field.
We define a new 'struct virtio_net_modern_hdr' for this (rather than
simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.

Signed-off-by: Rusty Russell ru...@rustcorp.com.au
---
 include/uapi/linux/virtio_net.h | 30 --
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index b5f1677b291c..32754f3000e8 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -35,7 +35,6 @@
 #define VIRTIO_NET_F_CSUM  0   /* Host handles pkts w/ partial csum */
 #define VIRTIO_NET_F_GUEST_CSUM1   /* Guest handles pkts w/ 
partial csum */
 #define VIRTIO_NET_F_MAC   5   /* Host has given MAC address. */
-#define VIRTIO_NET_F_GSO   6   /* Host handles pkts w/ any GSO type */
 #define VIRTIO_NET_F_GUEST_TSO47   /* Guest can handle TSOv4 in. */
 #define VIRTIO_NET_F_GUEST_TSO68   /* Guest can handle TSOv6 in. */
 #define VIRTIO_NET_F_GUEST_ECN 9   /* Guest can handle TSO[6] w/ ECN in. */
@@ -56,6 +55,10 @@
 * Steering */
 #define VIRTIO_NET_F_CTRL_MAC_ADDR 23  /* Set MAC address */
 
+#ifndef VIRTIO_NET_NO_LEGACY
+#define VIRTIO_NET_F_GSO   6   /* Host handles pkts w/ any GSO type */
+#endif /* VIRTIO_NET_NO_LEGACY */
+
 #define VIRTIO_NET_S_LINK_UP   1   /* Link is up */
 #define VIRTIO_NET_S_ANNOUNCE  2   /* Announcement is needed */
 
@@ -71,8 +74,9 @@ struct virtio_net_config {
__u16 max_virtqueue_pairs;
 } __attribute__((packed));
 
+#ifndef VIRTIO_NET_NO_LEGACY
 /* This header comes first in the scatter-gather list.
- * If VIRTIO_F_ANY_LAYOUT is not negotiated, it must
+ * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
  * be the first element of the scatter-gather list.  If you don't
  * specify GSO or CSUM features, you can simply ignore the header. */
 struct virtio_net_hdr {
@@ -97,6 +101,28 @@ struct virtio_net_hdr_mrg_rxbuf {
struct virtio_net_hdr hdr;
__virtio16 num_buffers; /* Number of merged rx buffers */
 };
+#else /* ... VIRTIO_NET_NO_LEGACY */
+/*
+ * This header comes first in the scatter-gather list.  If you don't
+ * specify GSO or CSUM features, you can simply ignore the header.
+ */
+struct virtio_net_modern_hdr {
+#define VIRTIO_NET_HDR_F_NEEDS_CSUM1   /* Use csum_start, csum_offset 
*/
+#define VIRTIO_NET_HDR_F_DATA_VALID2   /* Csum is valid */
+   __u8 flags;
+#define VIRTIO_NET_HDR_GSO_NONE0   /* Not a GSO frame */
+#define VIRTIO_NET_HDR_GSO_TCPV4   1   /* GSO frame, IPv4 TCP (TSO) */
+#define VIRTIO_NET_HDR_GSO_UDP 3   /* GSO frame, IPv4 UDP (UFO) */
+#define VIRTIO_NET_HDR_GSO_TCPV6   4   /* GSO frame, IPv6 TCP */
+#define VIRTIO_NET_HDR_GSO_ECN 0x80/* TCP has ECN set */
+   __u8 gso_type;
+   __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
+   __virtio16 gso_size;/* Bytes to append to hdr_len per frame */
+   __virtio16 csum_start;  /* Position to start checksumming from */
+   __virtio16 csum_offset; /* Offset after that to place checksum */
+   __virtio16 num_buffers; /* Number of merged rx buffers */
+};
+#endif /* ...VIRTIO_NET_NO_LEGACY */
 
 /*
  * Control virtqueue data structures
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/