| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87egpqtmjd.fsf@rustcorp.com.au>
Date: Mon, 16 Feb 2015 16:39:42 +1030
From: Rusty Russell <rusty@...tcorp.com.au>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: lkml <linux-kernel@...r.kernel.org>,
"netdev\@vger.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.
"Michael S. Tsirkin" <mst@...hat.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_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?
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" <mst@...hat.com>
Signed-off-by: Rusty Russell <rusty@...tcorp.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_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 */
-};
-
-/* 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_CSUM 1 /* 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_* */
+ __u8 flags;
+ /* See VIRTIO_NET_HDR_GSO_* */
+ __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 */
+};
#endif /* ...VIRTIO_NET_NO_LEGACY */
/*
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists