virtio-net: Add additional MACs via a filter table
PATCH 5/5 virtio-net: Add additional MACs via a filter table
---
qemu/hw/virtio-net.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++- qemu/hw/virtio-net.h | 4 +++ 2 files changed, 77 insertions(+), 1 deletions(-)
qemu/hw/virtio-net.c +73 -1
@@ -21,7 +21,7 @@#define TAP_VNET_HDR -#define VIRTIO_VM_VERSION 3 +#define VIRTIO_VM_VERSION 4 typedef struct VirtIONet {
@@ -36,6 +36,11 @@ typedef struct VirtIONetint mergeable_rx_bufs;int promisc;int allmulti; +struct { +int entries; +int in_use; +uint8_t *macs; + } mac_table; } VirtIONet;/* TODO
@@ -69,6 +74,17 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config) } } +static void virtio_net_reset (VirtIODevice *vdev) +{ +VirtIONet *n =to_virtio_net (vdev); + +/* Allow the MAC filter table to be re-allocated after a device reset */ + n->mac_table.in_use =0 ; + n->mac_table.entries =0 ; +qemu_free (n->mac_table.macs); + n->mac_table.macs =NULL ; +} +static uint32_t virtio_net_get_features (VirtIODevice *vdev) {uint32_t features = (1 <<VIRTIO_NET_F_MAC );
@@ -157,10 +173,46 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) n->allmulti = *on; else *status =VIRTIO_NET_ERR ; + + }else if (ctrl->class ==VIRTIO_NET_CTRL_MAC_TABLE ) { +if (ctrl->cmd ==VIRTIO_NET_CTRL_MAC_TABLE_ALLOC ) { +uint32_t *entries; + +if (n->mac_table.entries || elem.out_num !=2 || + elem.out_sg[1 ].iov_len !=sizeof (*entries)) { + *status =VIRTIO_NET_ERR ; + goto reply; + } + + entries = (void *)elem.out_sg[1 ].iov_base; + n->mac_table.macs =qemu_mallocz (*entries *6 ); +if (!n->mac_table.macs) { + *status =VIRTIO_NET_ERR ; + goto reply; + } + + n->mac_table.entries = *entries; + *status =VIRTIO_NET_OK ; + }else if (ctrl->cmd ==VIRTIO_NET_CTRL_MAC_TABLE_SET ) { +if (!n->mac_table.entries || (elem.out_num ==2 && + (elem.out_sg[1 ].iov_len /6 ) > n->mac_table.entries)) { + *status =VIRTIO_NET_ERR ; + goto reply; + } + +if (elem.out_num ==2 ) { +memcpy (n->mac_table.macs, elem.out_sg[1 ].iov_base, + elem.out_sg[1 ].iov_len); + n->mac_table.in_use = elem.out_sg[1 ].iov_len /6 ; + }else { + n->mac_table.in_use =0 ; + } + } }else { *status =VIRTIO_NET_ERR ; } +reply:virtqueue_push (vq, &elem,sizeof (*status));virtio_notify (vdev, vq); }
@@ -276,6 +328,7 @@ static int receive_header(VirtIONet *n, struct iovec *iov, int iovcnt,static int receive_filter (VirtIONet *n,const uint8_t *buf,int size) {static uint8_t bcast[] = {0xff ,0xff ,0xff ,0xff ,0xff ,0xff }; +int i;if (n->promisc)return 1 ;
@@ -289,6 +342,11 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)if (!memcmp (buf, n->mac,6 ))return 1 ; +for (i =0 ; i < n->mac_table.in_use; i++) { +if (!memcmp (buf, &n->mac_table.macs[i *6 ],6 )) +return 1 ; + } +return 0 ; }
@@ -458,6 +516,10 @@ static void virtio_net_save(QEMUFile *f, void *opaque)#endif qemu_put_be32 (f, n->promisc);qemu_put_be32 (f, n->allmulti); +qemu_put_be32 (f, n->mac_table.entries); +qemu_put_be32 (f, n->mac_table.in_use); +if (n->mac_table.entries) +qemu_put_buffer (f, n->mac_table.macs, n->mac_table.entries *6 ); }static int virtio_net_load (QEMUFile *f,void *opaque,int version_id)
@@ -485,6 +547,15 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id) n->promisc =1 ; } +if (version_id >=4 ) { + n->mac_table.entries =qemu_get_be32 (f); + n->mac_table.in_use =qemu_get_be32 (f); +if (n->mac_table.entries) { + n->mac_table.macs =qemu_mallocz (n->mac_table.entries *6 ); +qemu_get_buffer (f, n->mac_table.macs, n->mac_table.entries *6 ); + } + } +if (n->tx_timer_active) {qemu_mod_timer (n->tx_timer,qemu_get_clock (vm_clock) +TX_TIMER_INTERVAL );
@@ -509,6 +580,7 @@ PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn) n->vdev.set_config = virtio_net_set_config; n->vdev.get_features = virtio_net_get_features; n->vdev.set_features = virtio_net_set_features; + n->vdev.reset = virtio_net_reset; n->rx_vq =virtio_add_queue (&n->vdev,256 , virtio_net_handle_rx); n->tx_vq =virtio_add_queue (&n->vdev,256 , virtio_net_handle_tx); n->ctrl_vq =virtio_add_queue (&n->vdev,16 , virtio_net_handle_ctrl);
qemu/hw/virtio-net.h +4 -0
@@ -84,4 +84,8 @@ PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn);#define VIRTIO_NET_CTRL_RX_MODE_PROMISC 0#define VIRTIO_NET_CTRL_RX_MODE_ALLMULTI 1 +#define VIRTIO_NET_CTRL_MAC_TABLE 1 +#define VIRTIO_NET_CTRL_MAC_TABLE_ALLOC 0 +#define VIRTIO_NET_CTRL_MAC_TABLE_SET 1 +#endif
Hi Alex,
Whole series looks good to me, my suggestions are mostly trivial.
Quoted 12 lines · Alex Williamson
Signed-off-by Alex Williamson <[email protected]>---
qemu/hw/virtio-net.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++- qemu/hw/virtio-net.h | 4 +++ 2 files changed, 77 insertions(+), 1 deletions(-)qemu/hw/virtio-net.c +0
-0
qemu/hw/virtio-net.c line 24
Quoted 5 lines
qemu/hw/virtio-net.c +1
-1@@ -21,7 +21,7 @@#define TAP_VNET_HDR -#define VIRTIO_VM_VERSION 3 +#define VIRTIO_VM_VERSION 4
We could probably do with an ETH_ALEN macro at this stage. There's a lot of '6's around the place :-)
...
qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 177
Quoted 6 lines
qemu/hw/virtio-net.c +2
-0@@ -157,10 +173,46 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) n->allmulti = *on; else *status =VIRTIO_NET_ERR ; + + }else if (ctrl->class ==VIRTIO_NET_CTRL_MAC_TABLE ) {
Hmm, it'd be nice to factor each of the commands out in their own function.
qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 187
Quoted 10 lines
qemu/hw/virtio-net.c +10
-0+if (ctrl->cmd ==VIRTIO_NET_CTRL_MAC_TABLE_ALLOC ) { +uint32_t *entries; + +if (n->mac_table.entries || elem.out_num !=2 || + elem.out_sg[1 ].iov_len !=sizeof (*entries)) { + *status =VIRTIO_NET_ERR ; + goto reply; + } + + entries = (void *)elem.out_sg[1 ].iov_base;
No need for the cast.
qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 188
Quoted 1 line
qemu/hw/virtio-net.c +1
-0+ n->mac_table.macs =qemu_mallocz (*entries *6 );
We should put a limit on the size of the table.
qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 197
Quoted 9 lines
qemu/hw/virtio-net.c +9
-0+if (!n->mac_table.macs) { + *status =VIRTIO_NET_ERR ; + goto reply; + } + + n->mac_table.entries = *entries; + *status =VIRTIO_NET_OK ; + }else if (ctrl->cmd ==VIRTIO_NET_CTRL_MAC_TABLE_SET ) { + if (!n->mac_table.entries || (elem.out_num == 2 &&
Think I'd just check that out_num is 1 or 2 here ...
then e.g.
n_entries = 0;
if (elem.out_num == 2)
n_entries = elem.out_sg[1].iov_len / 6;
if (n_entries > n->mac_table.entries) {
*status = VIRTIO_NET_HDR
...
n->mac_table.in_use = 0;
if (n_entries) {
...qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 210
Quoted 13 lines
qemu/hw/virtio-net.c +13
-0+ (elem.out_sg[1].iov_len / 6) > n->mac_table.entries)) { + *status =VIRTIO_NET_ERR ; + goto reply; + } + +if (elem.out_num ==2 ) { +memcpy (n->mac_table.macs, elem.out_sg[1 ].iov_base, + elem.out_sg[1 ].iov_len); + n->mac_table.in_use = elem.out_sg[1 ].iov_len /6 ; + }else { + n->mac_table.in_use =0 ; + } + }
...
Cheers,
Mark.
Quoted 5 lines · Mark McLoughlin
qemu/hw/virtio-net.c line 24
Quoted 2 lines
qemu/hw/virtio-net.c +1
-1-#define VIRTIO_VM_VERSION 3 +#define VIRTIO_VM_VERSION 4We could probably do with an
ETH_ALENmacro at this stage. There's a lot of '6's around the place :-)
Yep, that'd be easier to search for too. I'll at least make a local define for that.
Quoted 10 lines
...
qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 177
Quoted 6 lines
qemu/hw/virtio-net.c +2
-0@@ -157,10 +173,46 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) n->allmulti = *on; else *status =VIRTIO_NET_ERR ; + + }else if (ctrl->class ==VIRTIO_NET_CTRL_MAC_TABLE ) {Hmm, it'd be nice to factor each of the commands out in their own function.
It's setup to do that, but each case is still pretty short and simple, so I hadn't made the cut yet.
Quoted 13 lines
qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 187
Quoted 10 lines
qemu/hw/virtio-net.c +10
-0+if (ctrl->cmd ==VIRTIO_NET_CTRL_MAC_TABLE_ALLOC ) { +uint32_t *entries; + +if (n->mac_table.entries || elem.out_num !=2 || + elem.out_sg[1 ].iov_len !=sizeof (*entries)) { + *status =VIRTIO_NET_ERR ; + goto reply; + } + + entries = (void *)elem.out_sg[1 ].iov_base;No need for the cast.
Quoted 3 lines
qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 188
Quoted 1 line
qemu/hw/virtio-net.c +1
-0+ n->mac_table.macs =qemu_mallocz (*entries *6 );We should put a limit on the size of the table.
Agree, I put one on the guest driver and thought I had one here, but must have missed it. Need to protect from malicious guests.
Quoted 11 lines
qemu/hw/virtio-net.c static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) · line 197
Quoted 9 lines
qemu/hw/virtio-net.c +9
-0+if (!n->mac_table.macs) { + *status =VIRTIO_NET_ERR ; + goto reply; + } + + n->mac_table.entries = *entries; + *status =VIRTIO_NET_OK ; + }else if (ctrl->cmd ==VIRTIO_NET_CTRL_MAC_TABLE_SET ) { + if (!n->mac_table.entries || (elem.out_num == 2 &&Think I'd just check that
out_numis 1 or 2 here ...
Good idea, I'll play with this and see what comes out. Thanks for the comments.
Alex
Quoted 28 lines
then e.g.
n_entries = 0; if (elem.out_num == 2) n_entries = elem.out_sg[1].iov_len / 6; if (n_entries > n->mac_table.entries) { *status = VIRTIO_NET_HDR ... n->mac_table.in_use = 0; if (n_entries) { ...Quoted 13 lines
qemu/hw/virtio-net.c +13
-0+ (elem.out_sg[1].iov_len / 6) > n->mac_table.entries)) { + *status =VIRTIO_NET_ERR ; + goto reply; + } + +if (elem.out_num ==2 ) { +memcpy (n->mac_table.macs, elem.out_sg[1 ].iov_base, + elem.out_sg[1 ].iov_len); + n->mac_table.in_use = elem.out_sg[1 ].iov_len /6 ; + }else { + n->mac_table.in_use =0 ; + } + }...
Signature · 1 line
Alex Williamson HP Open Source & Linux Org.