| Message ID | 516CC0CB.9020202@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Apr 16, 2013 at 11:08:59AM +0800, Jason Wang wrote: > On 04/15/2013 04:28 PM, Aurelien Jarno wrote: > > On Mon, Apr 15, 2013 at 01:29:01PM +0800, Jason Wang wrote: > >> On 04/13/2013 09:17 PM, Aurelien Jarno wrote: > >>> On Fri, Jan 25, 2013 at 06:35:41PM +0800, Jason Wang wrote: > >>>> This patch implements both userspace and vhost support for multiple queue > >>>> virtio-net (VIRTIO_NET_F_MQ). This is done by introducing an array of > >>>> VirtIONetQueue to VirtIONet. > >>>> > >>>> Signed-off-by: Jason Wang <jasowang@redhat.com> > >>>> --- > >>>> hw/virtio-net.c | 317 +++++++++++++++++++++++++++++++++++++++++++------------ > >>>> hw/virtio-net.h | 28 +++++- > >>>> 2 files changed, 275 insertions(+), 70 deletions(-) > >>> This patch breaks virtio-net in Minix, even with multiqueue disable. I > >>> don't know virtio enough to know if it is a Minix or a QEMU problem. > >>> However I have been able to identify the part of the commit causing the > >>> failure: > >> Hi Aurelien: > >> > >> Thanks for the work. > >>>> diff --git a/hw/virtio-net.c b/hw/virtio-net.c > >>>> index ef522d5..cec91a7 100644 > >>>> --- a/hw/virtio-net.c > >>>> +++ b/hw/virtio-net.c > >>> ... > >>> > >>>> +static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl) > >>>> +{ > >>>> + VirtIODevice *vdev = &n->vdev; > >>>> + int i, max = multiqueue ? n->max_queues : 1; > >>>> + > >>>> + n->multiqueue = multiqueue; > >>>> + > >>>> + for (i = 2; i <= n->max_queues * 2 + 1; i++) { > >>>> + virtio_del_queue(vdev, i); > >>>> + } > >>>> + > >>> The for loop above is something which is new, even with multiqueue > >>> disabled. Even with max_queues=1 it calls virtio_del_queue with i = 2 > >>> and i = 3. Disabling this loop makes the code to work as before. > >> Looks like a bug here, need to change n->max_queues * 2 + 1 to > >> n->max_queues * 2. The reason we need to del queue 2 each time because > >> vq 2 has different meaning is multiqueue and single queue. In single > >> queue, vq 2 maybe ctrl vq, but in multiqueue mode it was rx1. > >> > >> Let's see whether this small change works. > > Unfortunately it doesn't fix the issue. I don't know a lot about virtio, > > but would it be possible to only delete the queue that have been > > enabled? > > The issue is vq 2 has different meanings in two modes, so it must be > deleted and reinitialized during feature negotiation. > > > >>> On the Minix side it triggers the following assertion: > >>> > >>> | virtio.c:370: assert "q->vaddr != NULL" failed, function "free_phys_queue" > >>> | virtio_net(73141): panic: assert failed > >>> > >>> This correspond to this function in lib/libvirtio/virtio.c: > >>> > >>> | static void > >>> | free_phys_queue(struct virtio_queue *q) > >>> | { > >>> | assert(q != NULL); > >>> | assert(q->vaddr != NULL); > >>> | > >>> | free_contig(q->vaddr, q->ring_size); > >>> | q->vaddr = NULL; > >>> | q->paddr = 0; > >>> | q->num = 0; > >>> | free_contig(q->data, sizeof(q->data[0])); > >>> | q->data = NULL; > >>> | } > >>> > >>> Do you have an idea if the problem is on the Minix side or on the QEMU > >>> side? > >>> > >> Haven't figured out the relationship between virtqueue dynamic del/add > >> and q->vaddr here. If the above changes does not work, I guess this > > Unfortunately I don't really know the minix code either. I happen to > > found the issue when testing other changes, and looked at the code to > > try to understand the problem. > > Me too. > >> problem happens only for ctrl vq (vq2)? And when does this happen? > > I guess so given removing the call to virtio_del_queue() for vq2 fixes > > or workarounds the issue. > > It will cause trouble for multiqueue guest who may think vq2 is rx queue. > > > >> Rebooting? > > It happens at the initial boot. > > Looks at the codes, looks like vq2 was initialized unconditionally at > start, how about the following patch? > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c > index 4bb49eb..4886aa0 100644 > --- a/hw/virtio-net.c > +++ b/hw/virtio-net.c > @@ -1300,7 +1300,6 @@ VirtIODevice *virtio_net_init(DeviceState *dev, > NICConf *conf, > virtio_net_handle_tx_bh); > n->vqs[0].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[0]); > } > - n->ctrl_vq = virtio_add_queue(&n->vdev, 64, virtio_net_handle_ctrl); > qemu_macaddr_default_if_unset(&conf->macaddr); > memcpy(&n->mac[0], &conf->macaddr, sizeof(n->mac)); > n->status = VIRTIO_NET_S_LINK_UP; > Unfortunately this patch doesn't work, I tried it with and without your previous suggested change (max_queues * 2 + 1) => max_queues * 2. Aurelien
On 04/17/2013 03:20 PM, Aurelien Jarno wrote: > On Tue, Apr 16, 2013 at 11:08:59AM +0800, Jason Wang wrote: >> On 04/15/2013 04:28 PM, Aurelien Jarno wrote: >>> On Mon, Apr 15, 2013 at 01:29:01PM +0800, Jason Wang wrote: >>>> On 04/13/2013 09:17 PM, Aurelien Jarno wrote: >>>>> On Fri, Jan 25, 2013 at 06:35:41PM +0800, Jason Wang wrote: >>>>>> This patch implements both userspace and vhost support for multiple queue >>>>>> virtio-net (VIRTIO_NET_F_MQ). This is done by introducing an array of >>>>>> VirtIONetQueue to VirtIONet. >>>>>> >>>>>> Signed-off-by: Jason Wang <jasowang@redhat.com> >>>>>> --- >>>>>> hw/virtio-net.c | 317 +++++++++++++++++++++++++++++++++++++++++++------------ >>>>>> hw/virtio-net.h | 28 +++++- >>>>>> 2 files changed, 275 insertions(+), 70 deletions(-) >>>>> This patch breaks virtio-net in Minix, even with multiqueue disable. I >>>>> don't know virtio enough to know if it is a Minix or a QEMU problem. >>>>> However I have been able to identify the part of the commit causing the >>>>> failure: >>>> Hi Aurelien: >>>> >>>> Thanks for the work. >>>>>> diff --git a/hw/virtio-net.c b/hw/virtio-net.c >>>>>> index ef522d5..cec91a7 100644 >>>>>> --- a/hw/virtio-net.c >>>>>> +++ b/hw/virtio-net.c >>>>> ... >>>>> >>>>>> +static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl) >>>>>> +{ >>>>>> + VirtIODevice *vdev = &n->vdev; >>>>>> + int i, max = multiqueue ? n->max_queues : 1; >>>>>> + >>>>>> + n->multiqueue = multiqueue; >>>>>> + >>>>>> + for (i = 2; i <= n->max_queues * 2 + 1; i++) { >>>>>> + virtio_del_queue(vdev, i); >>>>>> + } >>>>>> + >>>>> The for loop above is something which is new, even with multiqueue >>>>> disabled. Even with max_queues=1 it calls virtio_del_queue with i = 2 >>>>> and i = 3. Disabling this loop makes the code to work as before. >>>> Looks like a bug here, need to change n->max_queues * 2 + 1 to >>>> n->max_queues * 2. The reason we need to del queue 2 each time because >>>> vq 2 has different meaning is multiqueue and single queue. In single >>>> queue, vq 2 maybe ctrl vq, but in multiqueue mode it was rx1. >>>> >>>> Let's see whether this small change works. >>> Unfortunately it doesn't fix the issue. I don't know a lot about virtio, >>> but would it be possible to only delete the queue that have been >>> enabled? >> The issue is vq 2 has different meanings in two modes, so it must be >> deleted and reinitialized during feature negotiation. >>>>> On the Minix side it triggers the following assertion: >>>>> >>>>> | virtio.c:370: assert "q->vaddr != NULL" failed, function "free_phys_queue" >>>>> | virtio_net(73141): panic: assert failed >>>>> >>>>> This correspond to this function in lib/libvirtio/virtio.c: >>>>> >>>>> | static void >>>>> | free_phys_queue(struct virtio_queue *q) >>>>> | { >>>>> | assert(q != NULL); >>>>> | assert(q->vaddr != NULL); >>>>> | >>>>> | free_contig(q->vaddr, q->ring_size); >>>>> | q->vaddr = NULL; >>>>> | q->paddr = 0; >>>>> | q->num = 0; >>>>> | free_contig(q->data, sizeof(q->data[0])); >>>>> | q->data = NULL; >>>>> | } >>>>> >>>>> Do you have an idea if the problem is on the Minix side or on the QEMU >>>>> side? >>>>> >>>> Haven't figured out the relationship between virtqueue dynamic del/add >>>> and q->vaddr here. If the above changes does not work, I guess this >>> Unfortunately I don't really know the minix code either. I happen to >>> found the issue when testing other changes, and looked at the code to >>> try to understand the problem. >> Me too. >>>> problem happens only for ctrl vq (vq2)? And when does this happen? >>> I guess so given removing the call to virtio_del_queue() for vq2 fixes >>> or workarounds the issue. >> It will cause trouble for multiqueue guest who may think vq2 is rx queue. >>>> Rebooting? >>> It happens at the initial boot. >> Looks at the codes, looks like vq2 was initialized unconditionally at >> start, how about the following patch? >> >> diff --git a/hw/virtio-net.c b/hw/virtio-net.c >> index 4bb49eb..4886aa0 100644 >> --- a/hw/virtio-net.c >> +++ b/hw/virtio-net.c >> @@ -1300,7 +1300,6 @@ VirtIODevice *virtio_net_init(DeviceState *dev, >> NICConf *conf, >> virtio_net_handle_tx_bh); >> n->vqs[0].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[0]); >> } >> - n->ctrl_vq = virtio_add_queue(&n->vdev, 64, virtio_net_handle_ctrl); >> qemu_macaddr_default_if_unset(&conf->macaddr); >> memcpy(&n->mac[0], &conf->macaddr, sizeof(n->mac)); >> n->status = VIRTIO_NET_S_LINK_UP; >> > Unfortunately this patch doesn't work, I tried it with and without your > previous suggested change (max_queues * 2 + 1) => max_queues * 2. > > Aurelien > Thanks for the test, I will try to get a minix to see what happens and keep you updated.
On Wed, Apr 17, 2013 at 04:27:13PM +0800, Jason Wang wrote: > On 04/17/2013 03:20 PM, Aurelien Jarno wrote: > > On Tue, Apr 16, 2013 at 11:08:59AM +0800, Jason Wang wrote: > >> On 04/15/2013 04:28 PM, Aurelien Jarno wrote: > >>> On Mon, Apr 15, 2013 at 01:29:01PM +0800, Jason Wang wrote: > >>>> On 04/13/2013 09:17 PM, Aurelien Jarno wrote: > >>>>> On Fri, Jan 25, 2013 at 06:35:41PM +0800, Jason Wang wrote: > >>>>>> This patch implements both userspace and vhost support for multiple queue > >>>>>> virtio-net (VIRTIO_NET_F_MQ). This is done by introducing an array of > >>>>>> VirtIONetQueue to VirtIONet. > >>>>>> > >>>>>> Signed-off-by: Jason Wang <jasowang@redhat.com> > >>>>>> --- > >>>>>> hw/virtio-net.c | 317 +++++++++++++++++++++++++++++++++++++++++++------------ > >>>>>> hw/virtio-net.h | 28 +++++- > >>>>>> 2 files changed, 275 insertions(+), 70 deletions(-) > >>>>> This patch breaks virtio-net in Minix, even with multiqueue disable. I > >>>>> don't know virtio enough to know if it is a Minix or a QEMU problem. > >>>>> However I have been able to identify the part of the commit causing the > >>>>> failure: > >>>> Hi Aurelien: > >>>> > >>>> Thanks for the work. > >>>>>> diff --git a/hw/virtio-net.c b/hw/virtio-net.c > >>>>>> index ef522d5..cec91a7 100644 > >>>>>> --- a/hw/virtio-net.c > >>>>>> +++ b/hw/virtio-net.c > >>>>> ... > >>>>> > >>>>>> +static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl) > >>>>>> +{ > >>>>>> + VirtIODevice *vdev = &n->vdev; > >>>>>> + int i, max = multiqueue ? n->max_queues : 1; > >>>>>> + > >>>>>> + n->multiqueue = multiqueue; > >>>>>> + > >>>>>> + for (i = 2; i <= n->max_queues * 2 + 1; i++) { > >>>>>> + virtio_del_queue(vdev, i); > >>>>>> + } > >>>>>> + > >>>>> The for loop above is something which is new, even with multiqueue > >>>>> disabled. Even with max_queues=1 it calls virtio_del_queue with i = 2 > >>>>> and i = 3. Disabling this loop makes the code to work as before. > >>>> Looks like a bug here, need to change n->max_queues * 2 + 1 to > >>>> n->max_queues * 2. The reason we need to del queue 2 each time because > >>>> vq 2 has different meaning is multiqueue and single queue. In single > >>>> queue, vq 2 maybe ctrl vq, but in multiqueue mode it was rx1. > >>>> > >>>> Let's see whether this small change works. > >>> Unfortunately it doesn't fix the issue. I don't know a lot about virtio, > >>> but would it be possible to only delete the queue that have been > >>> enabled? > >> The issue is vq 2 has different meanings in two modes, so it must be > >> deleted and reinitialized during feature negotiation. > >>>>> On the Minix side it triggers the following assertion: > >>>>> > >>>>> | virtio.c:370: assert "q->vaddr != NULL" failed, function "free_phys_queue" > >>>>> | virtio_net(73141): panic: assert failed > >>>>> > >>>>> This correspond to this function in lib/libvirtio/virtio.c: > >>>>> > >>>>> | static void > >>>>> | free_phys_queue(struct virtio_queue *q) > >>>>> | { > >>>>> | assert(q != NULL); > >>>>> | assert(q->vaddr != NULL); > >>>>> | > >>>>> | free_contig(q->vaddr, q->ring_size); > >>>>> | q->vaddr = NULL; > >>>>> | q->paddr = 0; > >>>>> | q->num = 0; > >>>>> | free_contig(q->data, sizeof(q->data[0])); > >>>>> | q->data = NULL; > >>>>> | } > >>>>> > >>>>> Do you have an idea if the problem is on the Minix side or on the QEMU > >>>>> side? > >>>>> > >>>> Haven't figured out the relationship between virtqueue dynamic del/add > >>>> and q->vaddr here. If the above changes does not work, I guess this > >>> Unfortunately I don't really know the minix code either. I happen to > >>> found the issue when testing other changes, and looked at the code to > >>> try to understand the problem. > >> Me too. > >>>> problem happens only for ctrl vq (vq2)? And when does this happen? > >>> I guess so given removing the call to virtio_del_queue() for vq2 fixes > >>> or workarounds the issue. > >> It will cause trouble for multiqueue guest who may think vq2 is rx queue. > >>>> Rebooting? > >>> It happens at the initial boot. > >> Looks at the codes, looks like vq2 was initialized unconditionally at > >> start, how about the following patch? > >> > >> diff --git a/hw/virtio-net.c b/hw/virtio-net.c > >> index 4bb49eb..4886aa0 100644 > >> --- a/hw/virtio-net.c > >> +++ b/hw/virtio-net.c > >> @@ -1300,7 +1300,6 @@ VirtIODevice *virtio_net_init(DeviceState *dev, > >> NICConf *conf, > >> virtio_net_handle_tx_bh); > >> n->vqs[0].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[0]); > >> } > >> - n->ctrl_vq = virtio_add_queue(&n->vdev, 64, virtio_net_handle_ctrl); > >> qemu_macaddr_default_if_unset(&conf->macaddr); > >> memcpy(&n->mac[0], &conf->macaddr, sizeof(n->mac)); > >> n->status = VIRTIO_NET_S_LINK_UP; > >> > > Unfortunately this patch doesn't work, I tried it with and without your > > previous suggested change (max_queues * 2 + 1) => max_queues * 2. > > > > Aurelien > > > > Thanks for the test, I will try to get a minix to see what happens and > keep you updated. > Thanks! I have used minix 3.2.1 which include virtio drivers. See the following page, step 5 to know how to enable virtio: http://wiki.minix3.org/en/UsersGuide/RunningOnQemu
On 04/17/2013 04:42 PM, Aurelien Jarno wrote: > On Wed, Apr 17, 2013 at 04:27:13PM +0800, Jason Wang wrote: >> On 04/17/2013 03:20 PM, Aurelien Jarno wrote: >>> On Tue, Apr 16, 2013 at 11:08:59AM +0800, Jason Wang wrote: >>>> On 04/15/2013 04:28 PM, Aurelien Jarno wrote: >>>>> On Mon, Apr 15, 2013 at 01:29:01PM +0800, Jason Wang wrote: >>>>>> On 04/13/2013 09:17 PM, Aurelien Jarno wrote: >>>>>>> On Fri, Jan 25, 2013 at 06:35:41PM +0800, Jason Wang wrote: >>>>>>>> This patch implements both userspace and vhost support for multiple queue >>>>>>>> virtio-net (VIRTIO_NET_F_MQ). This is done by introducing an array of >>>>>>>> VirtIONetQueue to VirtIONet. >>>>>>>> >>>>>>>> Signed-off-by: Jason Wang <jasowang@redhat.com> >>>>>>>> --- >>>>>>>> hw/virtio-net.c | 317 +++++++++++++++++++++++++++++++++++++++++++------------ >>>>>>>> hw/virtio-net.h | 28 +++++- >>>>>>>> 2 files changed, 275 insertions(+), 70 deletions(-) >>>>>>> This patch breaks virtio-net in Minix, even with multiqueue disable. I >>>>>>> don't know virtio enough to know if it is a Minix or a QEMU problem. >>>>>>> However I have been able to identify the part of the commit causing the >>>>>>> failure: >>>>>> Hi Aurelien: >>>>>> >>>>>> Thanks for the work. >>>>>>>> diff --git a/hw/virtio-net.c b/hw/virtio-net.c >>>>>>>> index ef522d5..cec91a7 100644 >>>>>>>> --- a/hw/virtio-net.c >>>>>>>> +++ b/hw/virtio-net.c >>>>>>> ... >>>>>>> >>>>>>>> +static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl) >>>>>>>> +{ >>>>>>>> + VirtIODevice *vdev = &n->vdev; >>>>>>>> + int i, max = multiqueue ? n->max_queues : 1; >>>>>>>> + >>>>>>>> + n->multiqueue = multiqueue; >>>>>>>> + >>>>>>>> + for (i = 2; i <= n->max_queues * 2 + 1; i++) { >>>>>>>> + virtio_del_queue(vdev, i); >>>>>>>> + } >>>>>>>> + >>>>>>> The for loop above is something which is new, even with multiqueue >>>>>>> disabled. Even with max_queues=1 it calls virtio_del_queue with i = 2 >>>>>>> and i = 3. Disabling this loop makes the code to work as before. >>>>>> Looks like a bug here, need to change n->max_queues * 2 + 1 to >>>>>> n->max_queues * 2. The reason we need to del queue 2 each time because >>>>>> vq 2 has different meaning is multiqueue and single queue. In single >>>>>> queue, vq 2 maybe ctrl vq, but in multiqueue mode it was rx1. >>>>>> >>>>>> Let's see whether this small change works. >>>>> Unfortunately it doesn't fix the issue. I don't know a lot about virtio, >>>>> but would it be possible to only delete the queue that have been >>>>> enabled? >>>> The issue is vq 2 has different meanings in two modes, so it must be >>>> deleted and reinitialized during feature negotiation. >>>>>>> On the Minix side it triggers the following assertion: >>>>>>> >>>>>>> | virtio.c:370: assert "q->vaddr != NULL" failed, function "free_phys_queue" >>>>>>> | virtio_net(73141): panic: assert failed >>>>>>> >>>>>>> This correspond to this function in lib/libvirtio/virtio.c: >>>>>>> >>>>>>> | static void >>>>>>> | free_phys_queue(struct virtio_queue *q) >>>>>>> | { >>>>>>> | assert(q != NULL); >>>>>>> | assert(q->vaddr != NULL); >>>>>>> | >>>>>>> | free_contig(q->vaddr, q->ring_size); >>>>>>> | q->vaddr = NULL; >>>>>>> | q->paddr = 0; >>>>>>> | q->num = 0; >>>>>>> | free_contig(q->data, sizeof(q->data[0])); >>>>>>> | q->data = NULL; >>>>>>> | } >>>>>>> >>>>>>> Do you have an idea if the problem is on the Minix side or on the QEMU >>>>>>> side? >>>>>>> >>>>>> Haven't figured out the relationship between virtqueue dynamic del/add >>>>>> and q->vaddr here. If the above changes does not work, I guess this >>>>> Unfortunately I don't really know the minix code either. I happen to >>>>> found the issue when testing other changes, and looked at the code to >>>>> try to understand the problem. >>>> Me too. >>>>>> problem happens only for ctrl vq (vq2)? And when does this happen? >>>>> I guess so given removing the call to virtio_del_queue() for vq2 fixes >>>>> or workarounds the issue. >>>> It will cause trouble for multiqueue guest who may think vq2 is rx queue. >>>>>> Rebooting? >>>>> It happens at the initial boot. >>>> Looks at the codes, looks like vq2 was initialized unconditionally at >>>> start, how about the following patch? >>>> >>>> diff --git a/hw/virtio-net.c b/hw/virtio-net.c >>>> index 4bb49eb..4886aa0 100644 >>>> --- a/hw/virtio-net.c >>>> +++ b/hw/virtio-net.c >>>> @@ -1300,7 +1300,6 @@ VirtIODevice *virtio_net_init(DeviceState *dev, >>>> NICConf *conf, >>>> virtio_net_handle_tx_bh); >>>> n->vqs[0].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[0]); >>>> } >>>> - n->ctrl_vq = virtio_add_queue(&n->vdev, 64, virtio_net_handle_ctrl); >>>> qemu_macaddr_default_if_unset(&conf->macaddr); >>>> memcpy(&n->mac[0], &conf->macaddr, sizeof(n->mac)); >>>> n->status = VIRTIO_NET_S_LINK_UP; >>>> >>> Unfortunately this patch doesn't work, I tried it with and without your >>> previous suggested change (max_queues * 2 + 1) => max_queues * 2. >>> >>> Aurelien >>> >> Thanks for the test, I will try to get a minix to see what happens and >> keep you updated. >> > Thanks! I have used minix 3.2.1 which include virtio drivers. See the > following page, step 5 to know how to enable virtio: > > http://wiki.minix3.org/en/UsersGuide/RunningOnQemu > Not sure whether it's a bug of minix guest, since it always tries to identify the control vq even if it does not support it: From virtio_net_probe(): /* If the host supports the control queue, allocate it as well */ if (virtio_host_supports(net_dev, VIRTIO_NET_F_CTRL_VQ)) queues += 1; This violates the virtio spec: " If the VIRTIO_NET_F_CTRL_VQ feature bit is negotiated, identify the control virtqueue." The multiqueue patchset conditionally allocate control vq depends on whether guest support it instead of always allocating it. I will send a patch to keep this behaviour to preserve backward compatibility. Thanks
diff --git a/hw/virtio-net.c b/hw/virtio-net.c index 4bb49eb..4886aa0 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -1300,7 +1300,6 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, virtio_net_handle_tx_bh); n->vqs[0].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[0]); } - n->ctrl_vq = virtio_add_queue(&n->vdev, 64, virtio_net_handle_ctrl); qemu_macaddr_default_if_unset(&conf->macaddr); memcpy(&n->mac[0], &conf->macaddr, sizeof(n->mac));